Merge pull request #1539 from OneSignal/fix/outcomeEventsController_init_npe

Fix NPE for `outcomeEventsController` during initialization

Author: nan-li

OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java

@@ -391,7 +391,8 @@ static Activity getCurrentActivity() {
    private static String smsId = null;
    private static int subscribableStatus = Integer.MAX_VALUE;
 
-   private static LanguageContext languageContext = null;
+   // changed from private to package-private for unit test access
+   static LanguageContext languageContext = null;
 
    static OSRemoteNotificationReceivedHandler remoteNotificationReceivedHandler;
    static OSNotificationWillShowInForegroundHandler notificationWillShowInForegroundHandler;
@@ -463,6 +464,22 @@ static OSSharedPreferences getSharedPreferences() {
    @Nullable private static OSOutcomeEventsController outcomeEventsController;
    @Nullable private static OSOutcomeEventsFactory outcomeEventsFactory;
    @Nullable private static OSNotificationDataController notificationDataController;
+   private static final Object outcomeEventsControllerSyncLock = new Object() {};
+
+   static OSOutcomeEventsController getOutcomeEventsController() {
+      if (outcomeEventsController == null) {
+         synchronized(outcomeEventsControllerSyncLock) {
+            if (outcomeEventsController == null) {
+               if (outcomeEventsFactory == null) {
+                  OneSignalDbHelper dbHelper = getDBHelperInstance();
+                  outcomeEventsFactory = new OSOutcomeEventsFactory(logger, apiClient, dbHelper, preferences);
+               }
+               outcomeEventsController = new OSOutcomeEventsController(sessionManager, outcomeEventsFactory);
+            }
+         }
+      }
+      return outcomeEventsController;
+   }
 
    @SuppressWarnings("WeakerAccess")
    public static String sdkType = "native";
@@ -855,7 +872,7 @@ synchronized private static void init(Context context) {
       initDone = true;
       OneSignal.Log(LOG_LEVEL.VERBOSE, "OneSignal SDK initialization done.");
 
-      outcomeEventsController.sendSavedOutcomes();
+      getOutcomeEventsController().sendSavedOutcomes();
 
       // Clean up any pending tasks that were queued up before initialization
       taskRemoteController.startPendingTasks();
@@ -892,8 +909,7 @@ private static void setupContextListeners(boolean wasAppContextNull) {
             outcomeEventsFactory = new OSOutcomeEventsFactory(logger, apiClient, dbHelper, preferences);
 
          sessionManager.initSessionFromCache();
-         outcomeEventsController = new OSOutcomeEventsController(sessionManager, outcomeEventsFactory);
-         outcomeEventsController.cleanCachedUniqueOutcomes();
+         getOutcomeEventsController().cleanCachedUniqueOutcomes();
       }
    }
 
@@ -971,7 +987,7 @@ private static void doSessionInit() {
          logger.debug("Starting new session with appEntryState: " + getAppEntryState());
 
          OneSignalStateSynchronizer.setNewSession();
-         outcomeEventsController.cleanOutcomes();
+         getOutcomeEventsController().cleanOutcomes();
          sessionManager.restartSessionIfNeeded(getAppEntryState());
          getInAppMessageController().resetSessionLaunchTime();
          setLastSessionTime(time.getCurrentTimeMillis());

OneSignalSDK/unittest/src/test/java/com/onesignal/ShadowOneSignalWithMockSetupContextListeners.java

@@ -0,0 +1,27 @@
+package com.onesignal;
+
+import com.onesignal.language.LanguageContext;
+
+import org.robolectric.annotation.Implementation;
+import org.robolectric.annotation.Implements;
+
+/**
+ * This shadow is added for the test initWithContext_setupContextListenersNotCompleted_doesNotProduceNPE
+ * Changes the behavior of one method without affecting other unit tests using ShadowOneSignal
+ */
+@Implements(OneSignal.class)
+public class ShadowOneSignalWithMockSetupContextListeners {
+
+   /**
+    * Simulates setupContextListeners() in initWithContext() not completing.
+    * However, languageContext initialization is needed for later, so that is the only code kept
+    */
+   @Implementation
+   public static void setupContextListeners(boolean wasAppContextNull) {
+
+      // Do work here that should only happen once or at the start of a new lifecycle
+      if (wasAppContextNull) {
+         OneSignal.languageContext = new LanguageContext(OneSignal.getSharedPreferences());
+      }
+   }
+}

OneSignalSDK/unittest/src/test/java/com/test/onesignal/OneSignalInitializationIntegrationTestsRunner.java

@@ -7,6 +7,7 @@
 import com.onesignal.ShadowCustomTabsClient;
 import com.onesignal.ShadowCustomTabsSession;
 import com.onesignal.ShadowOSUtils;
+import com.onesignal.ShadowOneSignalWithMockSetupContextListeners;
 import com.onesignal.ShadowOneSignalRestClient;
 import com.onesignal.ShadowPushRegistratorFCM;
 import com.onesignal.StaticResetHelper;
@@ -88,6 +89,19 @@ public void setRequiresUserPrivacyConsent_withTrue_CalledFirst_DoesNOTCreatePlay
         RestClientAsserts.assertRemoteParamsWasTheOnlyNetworkCall();
     }
 
+    // This test reproduces https://github.com/OneSignal/OneSignal-Android-SDK/issues/1514
+    @Test
+    @Config(shadows = { ShadowOneSignalWithMockSetupContextListeners.class })
+    public void initWithContext_setupContextListenersNotCompleted_doesNotProduceNPE() throws Exception {
+        OneSignal.setAppId(APP_ID);
+
+        // call initWithContext() but don't complete setupContextListeners() via the Shadow class
+        // this prevents the initialization of outcomeEventsController in setupContextListeners()
+        helper_OneSignal_initWithAppContext();
+        threadAndTaskWait();
+        // we implicitly test that no exception is thrown
+    }
+
     @Test
     public void setRequiresUserPrivacyConsent_withFalseAndRemoteTrue_DoesNOTCreatePlayer() throws Exception {
         ShadowOneSignalRestClient.setRemoteParamsRequirePrivacyConsent(true);