Skip to content

Commit

Permalink
Use DummyCriteo if not initialized properly (#89)
Browse files Browse the repository at this point in the history
* Use DummyCriteo if not initialized properly

Problem:
When an error happens during the initialization path, Criteo instance
will be `null` and any future call to Criteo.getInstance() will throw a
RuntimeException.

Solution:
DummyCriteo is assigned to Criteo singleton object, when any runtime
exception is thrown during initialization. While the integration will
not work, it will prevent the host application from crashing. Logging
will allow us to investigate and correct the root cause with the
publisher.

* Guard against null application in CriteoInterstitial

Problem:
An `application` object is required for the SDK to operate properly.
However, nothing prevents CriteoInterstitialActivity from being open even if
`application` is null.

Solution:
CriteoInterstitial APIs must check wehther `application` is null or
return immediately. This is a workaround to prevent apps from crashing
in production but actions need to be taken in the future to:
1. Understand how `application` can be null for publishers.
2. Check whether we can reduce the scope of `application` within the SDk
and use an Activity context when appropriate (in this case, the Activity
context that is passed to CriteoInterstitial could be used).
3. Understand the implications of making the application object
nullable.
  • Loading branch information
Amokrane authored Sep 9, 2020
1 parent 3818b77 commit d3f90a1
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 9 deletions.
5 changes: 2 additions & 3 deletions publisher-sdk/src/main/java/com/criteo/publisher/Criteo.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,8 @@ private static Criteo init(
} else {
criteo = new DummyCriteo();
}
} catch (IllegalArgumentException iae) {
throw iae;
} catch (Throwable tr) {
} catch(Throwable tr) {
criteo = new DummyCriteo();
Log.e(TAG, "Internal error initializing Criteo instance.", tr);
throw new CriteoInitException("Internal error initializing Criteo instance.", tr);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ final class CriteoInternal extends Criteo {
userPrivacyUtil.storeUsPrivacyOptout(usPrivacyOptout);
}

// this nulll check ensures that instantiating Criteo object with null mopub consent value,
// this null check ensures that instantiating Criteo object with null mopub consent value,
// doesn't erase the previously stored consent value
if (mopubConsent != null) {
userPrivacyUtil.storeMopubConsent(mopubConsent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,23 @@ public CriteoInterstitial(
}

public void setCriteoInterstitialAdListener(
@Nullable CriteoInterstitialAdListener criteoInterstitialAdListener) {
@Nullable CriteoInterstitialAdListener criteoInterstitialAdListener
) {
this.criteoInterstitialAdListener = criteoInterstitialAdListener;
}

public void setCriteoInterstitialAdDisplayListener(
@Nullable CriteoInterstitialAdDisplayListener criteoInterstitialAdDisplayListener) {
@Nullable CriteoInterstitialAdDisplayListener criteoInterstitialAdDisplayListener
) {
this.criteoInterstitialAdDisplayListener = criteoInterstitialAdDisplayListener;
}

public void loadAd() {
if (!DependencyProvider.getInstance().isApplicationSet()) {
Log.w(TAG, "Calling CriteoInterstitial#loadAd with a null application");
return;
}

try {
doLoadAd();
} catch (Throwable tr) {
Expand All @@ -102,6 +109,11 @@ private void doLoadAd() {
}

public void loadAd(@Nullable BidToken bidToken) {
if (!DependencyProvider.getInstance().isApplicationSet()) {
Log.w(TAG, "Calling CriteoInterstitial#loadAd(bidToken) with a null application");
return;
}

try {
doLoadAd(bidToken);
} catch (Throwable tr) {
Expand All @@ -111,6 +123,11 @@ public void loadAd(@Nullable BidToken bidToken) {

@Keep
public void loadAdWithDisplayData(@NonNull String displayData) {
if (!DependencyProvider.getInstance().isApplicationSet()) {
Log.w(TAG, "Calling CriteoInterstitial#loadAdWithDisplayData with a null application");
return;
}

getOrCreateController().notifyFor(CriteoListenerCode.VALID);
getOrCreateController().fetchCreativeAsync(displayData);
}
Expand All @@ -133,6 +150,11 @@ public boolean isAdLoaded() {
}

public void show() {
if (!DependencyProvider.getInstance().isApplicationSet()) {
Log.w(TAG, "Calling CriteoInterstitial#show with a null application");
return;
}

try {
doShow();
} catch (Throwable tr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class CriteoNotInitializedException extends IllegalStateException {

public CriteoNotInitializedException(String message) {
super(message + "\n"
+ "Did you initialize the Criteo SDK ?\n"
+ "Did you properly initialize the Criteo SDK ?\n"
+ "Please follow this step: https://publisherdocs.criteotilt.com/app/android/standalone/#sdk-initialization\n");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ public void setCriteoPublisherId(@NonNull String criteoPublisherId) {
this.criteoPublisherId = criteoPublisherId;
checkCriteoPublisherIdIsSet();
}

boolean isApplicationSet() {
try {
DependencyProvider.getInstance().checkApplicationIsSet();
} catch (Exception e) {
return false;
}
return true;
}

private void checkApplicationIsSet() {
if (application == null) {
Expand Down Expand Up @@ -285,7 +294,8 @@ public DeviceInfo provideDeviceInfo() {
public DeviceInfo create() {
return new DeviceInfo(
provideContext(),
provideRunOnUiThreadExecutor());
provideRunOnUiThreadExecutor()
);
}
});
}
Expand All @@ -298,7 +308,8 @@ public AdUnitMapper provideAdUnitMapper() {
public AdUnitMapper create() {
return new AdUnitMapper(
DependencyProvider.this.provideAndroidUtil(),
DependencyProvider.this.provideDeviceUtil());
DependencyProvider.this.provideDeviceUtil()
);
}
});
}
Expand Down Expand Up @@ -771,6 +782,7 @@ public LoggerFactory create() {
}

public interface Factory<T> {

@NonNull
T create();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;

import android.app.Application;
import android.content.Context;
import com.criteo.publisher.integration.Integration;
import com.criteo.publisher.integration.IntegrationRegistry;
Expand Down Expand Up @@ -161,6 +162,61 @@ public void displayAd_GivenController_DelegateToIt() throws Exception {
verify(controller).fetchCreativeAsync("fake_display_data");
}

@Test
public void loadAd_GivenNullApplication_ReturnImmediately() throws Exception {
CriteoInterstitialEventController controller = givenMockedController();
Application application = givenNullApplication();

interstitial.loadAd();

verifyNoMoreInteractions(controller);
DependencyProvider.getInstance().setApplication(application);
}

@Test
public void loadAdWithBidToken_GivenNullApplication_ReturnImmediately() throws Exception {
CriteoInterstitialEventController controller = givenMockedController();
Application application = givenNullApplication();
BidToken token = new BidToken(UUID.randomUUID(), adUnit);

interstitial.loadAd(token);

verifyNoMoreInteractions(controller);
DependencyProvider.getInstance().setApplication(application);
}

@Test
public void loadAdWithDisplayData_GivenNullApplication_ReturnImmediately() throws Exception {
CriteoInterstitialEventController controller = givenMockedController();
Application application = givenNullApplication();

interstitial.loadAdWithDisplayData("");

verifyNoMoreInteractions(controller);
DependencyProvider.getInstance().setApplication(application);
}


@Test
public void show_GivenNullApplication_ReturnImmediately() throws Exception {
CriteoInterstitialEventController controller = givenMockedController();
Application application = givenNullApplication();

interstitial.show();

verifyNoMoreInteractions(controller);
DependencyProvider.getInstance().setApplication(application);
}

private Application givenNullApplication() {
Application application = DependencyProvider.getInstance().provideApplication();
try {
DependencyProvider.getInstance().setApplication(null);
} catch (Exception e) {
}
return application;
}

private CriteoInterstitialEventController givenMockedController() {
CriteoInterstitialEventController controller = mock(CriteoInterstitialEventController.class);
doReturn(controller).when(interstitial).getOrCreateController();
Expand Down
35 changes: 35 additions & 0 deletions publisher-sdk/src/test/java/com/criteo/publisher/CriteoTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright 2020 Criteo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.criteo.publisher;

import static junit.framework.TestCase.assertEquals;

import org.junit.Test;

public class CriteoTest {

@Test
public void criteoInit_GivenNullApplication_DoesNotCrash() {
try {
new Criteo.Builder(null, "").init();
} catch (CriteoInitException e) {
}

assertEquals(DummyCriteo.class, Criteo.getInstance().getClass());
}

}

0 comments on commit d3f90a1

Please sign in to comment.