Merge pull request #1229 from OneSignal/fix/new_session_triggering_from_2nd_init

Fix new session triggering from 2nd init

Author: jkasten2

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

@@ -692,7 +692,6 @@ synchronized private static void init(Context context) {
          if (notificationOpenedHandler != null)
             fireCallbackForOpenedNotifications();
          logger.debug("OneSignal SDK initialization already completed.");
-         doSessionInit();
          return;
       }
 
@@ -824,21 +823,19 @@ private static void handleAmazonPurchase() {
    // If we don't have a OneSignal player_id yet make the call to create it regardless of focus
    private static void doSessionInit() {
       // Check session time to determine whether to start a new session or not
-      if (isPastOnSessionTime()) {
-         if (inForeground) {
-            logger.debug("Starting new session");
-
-            OneSignalStateSynchronizer.setNewSession();
-            outcomeEventsController.cleanOutcomes();
-            sessionManager.restartSessionIfNeeded(getAppEntryState());
-            getInAppMessageController().resetSessionLaunchTime();
-            setLastSessionTime(time.getCurrentTimeMillis());
-         }
-      } else {
-         logger.debug("Continue on same session");
-
+      if (shouldStartNewSession()) {
+         logger.debug("Starting new session with appEntryState: " + getAppEntryState());
+
+         OneSignalStateSynchronizer.setNewSession();
+         outcomeEventsController.cleanOutcomes();
+         sessionManager.restartSessionIfNeeded(getAppEntryState());
+         getInAppMessageController().resetSessionLaunchTime();
+         setLastSessionTime(time.getCurrentTimeMillis());
+      } else if (isInForeground()) {
+         logger.debug("Continue on same session with appEntryState: " + getAppEntryState());
          sessionManager.attemptSessionUpgrade(getAppEntryState());
       }
+
       getInAppMessageController().initWithCachedInAppMessages();
 
       // We still want register the user to OneSignal if the SDK was initialized
@@ -2914,6 +2911,16 @@ static boolean isAppActive() {
       return initDone && isInForeground();
    }
 
+   private static boolean shouldStartNewSession() {
+      if (!isInForeground())
+         return false;
+
+      if (!isPastOnSessionTime())
+         return false;
+
+      return true;
+   }
+
    private static boolean isPastOnSessionTime() {
       long currentTimeMillis = OneSignal.getTime().getCurrentTimeMillis();
       long lastSessionTime = getLastSessionTime();

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

@@ -28,6 +28,8 @@
 public class OneSignalPackagePrivateHelper {
    public static final String IN_APP_MESSAGES_JSON_KEY = com.onesignal.OSInAppMessageController.IN_APP_MESSAGES_JSON_KEY;
 
+   public static final long MIN_ON_SESSION_TIME_MILLIS = com.onesignal.OneSignal.MIN_ON_SESSION_TIME_MILLIS;
+
    private static abstract class RunnableArg<T> {
       abstract void run(T object) throws Exception;
    }

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

@@ -2,6 +2,7 @@
 
 import androidx.test.core.app.ApplicationProvider;
 
+import com.onesignal.MockOSTimeImpl;
 import com.onesignal.OneSignal;
 import com.onesignal.ShadowCustomTabsClient;
 import com.onesignal.ShadowCustomTabsSession;
@@ -21,7 +22,10 @@
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowLog;
 
+import static com.onesignal.OneSignalPackagePrivateHelper.MIN_ON_SESSION_TIME_MILLIS;
+import static com.onesignal.OneSignalPackagePrivateHelper.OneSignal_setTime;
 import static com.onesignal.ShadowOneSignalRestClient.setRemoteParamsGetHtmlResponse;
+import static com.test.onesignal.RestClientAsserts.assertNumberOfOnSessions;
 import static com.test.onesignal.TestHelpers.threadAndTaskWait;
 
 @Config(
@@ -39,6 +43,7 @@
 @RunWith(RobolectricTestRunner.class)
 public class OneSignalInitializationIntegrationTestsRunner {
     private ActivityController<BlankActivity> blankActivityController;
+    private MockOSTimeImpl time;
 
     @BeforeClass // Runs only once, before any tests
     public static void setUpClass() throws Exception {
@@ -52,12 +57,23 @@ public void beforeEachTest() throws Exception {
         TestHelpers.beforeTestInitAndCleanup();
         setRemoteParamsGetHtmlResponse();
         blankActivityController = Robolectric.buildActivity(BlankActivity.class).create();
+
+        time = new MockOSTimeImpl();
+        OneSignal_setTime(time);
     }
 
+    private static final long MIN_ON_SESSION_TIME_SEC = MIN_ON_SESSION_TIME_MILLIS / 1_000L;
     private static final String APP_ID = "11111111-2222-3333-4444-55555555555";
     private static void helper_OneSignal_initWithAppContext() {
         OneSignal.initWithContext(ApplicationProvider.getApplicationContext());
     }
+    private void helper_OneSignal_initWithActivity() {
+        OneSignal.initWithContext(blankActivityController.get());
+    }
+
+    private void helper_advanceSystemTimeToNextOnSession() {
+        time.advanceSystemAndElapsedTimeBy(MIN_ON_SESSION_TIME_SEC + 1);
+    }
 
     @Test
     public void setRequiresUserPrivacyConsent_withTrue_CalledFirst_DoesNOTCreatePlayer() throws Exception {
@@ -82,4 +98,27 @@ public void setRequiresUserPrivacyConsent_withFalseAndRemoteTrue_DoesNOTCreatePl
         RestClientAsserts.assertRemoteParamsWasTheOnlyNetworkCall();
     }
 
+    /**
+     * on_session calls should only be made if the user left the app for MIN_ON_SESSION_TIME_SEC or longer
+     * This test ensures that we meet the out of focus requirement, at lest through initWithContext
+     *   being called a 2nd time code path.
+     */
+    @Test
+    public void initWithContext_calledA2ndTimeAfter30OrMoreSeconds_doesNotStartNewSession() throws Exception {
+        // 1. Basic OneSignal init with Activity
+        OneSignal.setAppId(APP_ID);
+        helper_OneSignal_initWithActivity();
+        threadAndTaskWait();
+
+        // 2. Keep the app in focus for 30+ seconds, which is the time required to
+        helper_advanceSystemTimeToNextOnSession();
+
+        // 3. Developer or OneSignal internally calls OneSignal.initWithContext
+        helper_OneSignal_initWithActivity();
+        threadAndTaskWait();
+
+        // 4. Ensure we do NOT make an /players/{player_id}/on_session network call.
+        assertNumberOfOnSessions(0);
+    }
+
 }

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

@@ -113,6 +113,18 @@ static void assertOnSessionAtIndex(int index) {
       assertOnSessionUrl(request.url);
    }
 
+   static void assertNumberOfOnSessions(int number) {
+      int successfulAsserts = 0;
+      for (Request request : ShadowOneSignalRestClient.requests) {
+         try {
+            assertOnSessionUrl(request.url);
+            successfulAsserts++;
+         } catch (AssertionError ignored) { }
+      }
+
+      assertEquals(number, successfulAsserts);
+   }
+
    static void assertOnFocusAtIndex(int index, int focusTimeSec) throws JSONException {
       assertOnFocusAtIndex(index, new JSONObject().put("active_time", focusTimeSec));
    }
@@ -256,6 +268,8 @@ static void assertOnFocusUrlWithPlayerId(@NonNull String url, @NonNull String id
    static void assertOnSessionUrl(String url) {
       String[] parts = url.split("/");
       assertEquals("players", parts[0]);
+      if (parts.length == 1)
+         fail("Not an on_session as player_id and on_session is missing from the URL");
       assertIsUUID(parts[1]);
       assertEquals("on_session", parts[2]);
    }