Merge pull request #2113 from OneSignal/fix/cold_start_new_session

[Feat] Application service focus update, cold start creates new session and drives user refresh

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/application/IApplicationLifecycleHandler.kt

@@ -10,8 +10,13 @@ import android.app.Application.ActivityLifecycleCallbacks
 interface IApplicationLifecycleHandler {
     /**
      * Called when the application is brought into the foreground.
+     * This callback can be fired immediately on subscribing to the IApplicationService (when the
+     * IApplicationService itself is started too late to capture the application's early lifecycle events),
+     * or through natural application lifecycle callbacks.
+     *
+     * @param firedOnSubscribe Method is fired from subscribing or from application lifecycle callbacks
      */
-    fun onFocus()
+    fun onFocus(firedOnSubscribe: Boolean)
 
     /**
      * Called when the application has been brought out of the foreground, to the background.
@@ -24,7 +29,7 @@ interface IApplicationLifecycleHandler {
  * can use this if they only want to override a subset of the callbacks that make up this interface.
  */
 open class ApplicationLifecycleHandlerBase : IApplicationLifecycleHandler {
-    override fun onFocus() {}
+    override fun onFocus(firedOnSubscribe: Boolean) {}
 
     override fun onUnfocused() {}
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/application/impl/ApplicationService.kt

@@ -67,6 +67,9 @@ class ApplicationService() : IApplicationService, ActivityLifecycleCallbacks, On
     private var activityReferences = 0
     private var isActivityChangingConfigurations = false
 
+    private val wasInBackground: Boolean
+        get() = !isInForeground || nextResumeIsFirstActivity
+
     /**
      * Call to "start" this service, expected to be called during initialization of the SDK.
      *
@@ -117,6 +120,11 @@ class ApplicationService() : IApplicationService, ActivityLifecycleCallbacks, On
 
     override fun addApplicationLifecycleHandler(handler: IApplicationLifecycleHandler) {
         applicationLifecycleNotifier.subscribe(handler)
+        if (current != null) {
+            // When a listener subscribes, fire its callback
+            // The listener is too late to receive the earlier onFocus call
+            handler.onFocus(true)
+        }
     }
 
     override fun removeApplicationLifecycleHandler(handler: IApplicationLifecycleHandler) {
@@ -150,7 +158,7 @@ class ApplicationService() : IApplicationService, ActivityLifecycleCallbacks, On
 
         current = activity
 
-        if ((!isInForeground || nextResumeIsFirstActivity) && !isActivityChangingConfigurations) {
+        if (wasInBackground && !isActivityChangingConfigurations) {
             activityReferences = 1
             handleFocus()
         } else {
@@ -170,7 +178,7 @@ class ApplicationService() : IApplicationService, ActivityLifecycleCallbacks, On
             current = activity
         }
 
-        if ((!isInForeground || nextResumeIsFirstActivity) && !isActivityChangingConfigurations) {
+        if (wasInBackground && !isActivityChangingConfigurations) {
             activityReferences = 1
             handleFocus()
         }
@@ -373,7 +381,7 @@ class ApplicationService() : IApplicationService, ActivityLifecycleCallbacks, On
     }
 
     private fun handleFocus() {
-        if (!isInForeground || nextResumeIsFirstActivity) {
+        if (wasInBackground) {
             Logging.debug(
                 "ApplicationService.handleFocus: application is now in focus, nextResumeIsFirstActivity=$nextResumeIsFirstActivity",
             )
@@ -384,7 +392,7 @@ class ApplicationService() : IApplicationService, ActivityLifecycleCallbacks, On
                 entryState = AppEntryAction.APP_OPEN
             }
 
-            applicationLifecycleNotifier.fire { it.onFocus() }
+            applicationLifecycleNotifier.fire { it.onFocus(false) }
         } else {
             Logging.debug("ApplicationService.handleFocus: application never lost focus")
         }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/background/impl/BackgroundManager.kt

@@ -79,7 +79,7 @@ internal class BackgroundManager(
         _applicationService.addApplicationLifecycleHandler(this)
     }
 
-    override fun onFocus() {
+    override fun onFocus(firedOnSubscribe: Boolean) {
         cancelSyncTask()
     }
 

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/purchases/impl/TrackAmazonPurchase.kt

@@ -115,7 +115,7 @@ internal class TrackAmazonPurchase(
         e.printStackTrace()
     }
 
-    override fun onFocus() { }
+    override fun onFocus(firedOnSubscribe: Boolean) { }
 
     override fun onUnfocused() {
         if (!canTrack) return

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/purchases/impl/TrackGooglePurchase.kt

@@ -98,7 +98,7 @@ internal class TrackGooglePurchase(
         trackIAP()
     }
 
-    override fun onFocus() {
+    override fun onFocus(firedOnSubscribe: Boolean) {
         trackIAP()
     }
 

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/SessionModel.kt

@@ -19,13 +19,14 @@ class SessionModel : Model() {
      * Whether the session is valid.
      */
     var isValid: Boolean
-        get() = getBooleanProperty(::isValid.name) { true }
+        get() = getBooleanProperty(::isValid.name) { false }
         set(value) {
             setBooleanProperty(::isValid.name, value)
         }
 
     /**
      * When this session started, in Unix time milliseconds.
+     * This is used by In-App Message triggers, and not used in detecting session time.
      */
     var startTime: Long
         get() = getLongProperty(::startTime.name) { System.currentTimeMillis() }
@@ -37,7 +38,7 @@ class SessionModel : Model() {
      * When this app was last focused, in Unix time milliseconds.
      */
     var focusTime: Long
-        get() = getLongProperty(::focusTime.name) { 0 }
+        get() = getLongProperty(::focusTime.name) { System.currentTimeMillis() }
         set(value) {
             setLongProperty(::focusTime.name, value)
         }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt

@@ -4,6 +4,7 @@ import com.onesignal.common.threading.suspendifyOnThread
 import com.onesignal.core.internal.config.ConfigModelStore
 import com.onesignal.core.internal.operations.IOperationRepo
 import com.onesignal.core.internal.startup.IStartableService
+import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.session.internal.outcomes.IOutcomeEventsController
 import com.onesignal.session.internal.session.ISessionLifecycleHandler
 import com.onesignal.session.internal.session.ISessionService
@@ -47,6 +48,12 @@ internal class SessionListener(
 
     override fun onSessionEnded(duration: Long) {
         val durationInSeconds = duration / 1000
+
+        // Time is erroneous if below 1 second or over a day
+        if (durationInSeconds < 1L || durationInSeconds > SECONDS_IN_A_DAY) {
+            Logging.error("SessionListener.onSessionEnded sending duration of $durationInSeconds seconds")
+        }
+
         _operationRepo.enqueue(
             TrackSessionEndOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId, durationInSeconds),
         )
@@ -55,4 +62,8 @@ internal class SessionListener(
             _outcomeEventsController.sendSessionEndOutcomeEvent(durationInSeconds)
         }
     }
+
+    companion object {
+        const val SECONDS_IN_A_DAY = 86_400L
+    }
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionService.kt

@@ -45,35 +45,42 @@ internal class SessionService(
     private val sessionLifeCycleNotifier: EventProducer<ISessionLifecycleHandler> = EventProducer()
     private var session: SessionModel? = null
     private var config: ConfigModel? = null
+    private var shouldFireOnSubscribe = false
 
     override fun start() {
         session = _sessionModelStore.model
         config = _configModelStore.model
+        // Reset the session validity property to drive a new session
+        session!!.isValid = false
         _applicationService.addApplicationLifecycleHandler(this)
     }
 
     override suspend fun backgroundRun() {
-        Logging.log(LogLevel.DEBUG, "SessionService.backgroundRun()")
-
-        if (!session!!.isValid) {
-            return
-        }
-
+        val activeDuration = session!!.activeDuration
         // end the session
-        Logging.debug("SessionService: Session ended. activeDuration: ${session!!.activeDuration}")
+        Logging.debug("SessionService.backgroundRun: Session ended. activeDuration: $activeDuration")
         session!!.isValid = false
-        sessionLifeCycleNotifier.fire { it.onSessionEnded(session!!.activeDuration) }
+        sessionLifeCycleNotifier.fire { it.onSessionEnded(activeDuration) }
+        session!!.activeDuration = 0L
     }
 
-    override fun onFocus() {
-        Logging.log(LogLevel.DEBUG, "SessionService.onFocus()")
-
+    /**
+     * NOTE: When `firedOnSubscribe = true`
+     *
+     * Typically, the app foregrounding will trigger this callback via the IApplicationService.
+     * However, it is possible for OneSignal to initialize too late to capture the Android lifecycle callbacks.
+     * In this case, the app is already foregrounded, so this method is fired immediately on subscribing
+     * to the IApplicationService. Listeners of this service will not subscribe in time to capture
+     * the `onSessionStarted()` callback here, so fire it when they themselves subscribe.
+     */
+    override fun onFocus(firedOnSubscribe: Boolean) {
+        Logging.log(LogLevel.DEBUG, "SessionService.onFocus() - fired from start: $firedOnSubscribe")
         if (!session!!.isValid) {
             // As the old session was made inactive, we need to create a new session
+            shouldFireOnSubscribe = firedOnSubscribe
             session!!.sessionId = UUID.randomUUID().toString()
             session!!.startTime = _time.currentTimeMillis
             session!!.focusTime = session!!.startTime
-            session!!.activeDuration = 0L
             session!!.isValid = true
 
             Logging.debug("SessionService: New session started at ${session!!.startTime}")
@@ -87,14 +94,17 @@ internal class SessionService(
     }
 
     override fun onUnfocused() {
-        Logging.log(LogLevel.DEBUG, "SessionService.onUnfocused()")
-
         // capture the amount of time the app was focused
         val dt = _time.currentTimeMillis - session!!.focusTime
         session!!.activeDuration += dt
+        Logging.log(LogLevel.DEBUG, "SessionService.onUnfocused adding time $dt for total: ${session!!.activeDuration}")
     }
 
-    override fun subscribe(handler: ISessionLifecycleHandler) = sessionLifeCycleNotifier.subscribe(handler)
+    override fun subscribe(handler: ISessionLifecycleHandler) {
+        sessionLifeCycleNotifier.subscribe(handler)
+        // If a handler subscribes too late to capture the initial onSessionStarted.
+        if (shouldFireOnSubscribe) handler.onSessionStarted()
+    }
 
     override fun unsubscribe(handler: ISessionLifecycleHandler) = sessionLifeCycleNotifier.unsubscribe(handler)
 

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/service/UserRefreshService.kt

@@ -1,28 +1,32 @@
 package com.onesignal.user.internal.service
 
 import com.onesignal.common.IDManager
-import com.onesignal.core.internal.application.IApplicationLifecycleHandler
 import com.onesignal.core.internal.application.IApplicationService
 import com.onesignal.core.internal.config.ConfigModelStore
 import com.onesignal.core.internal.operations.IOperationRepo
 import com.onesignal.core.internal.startup.IStartableService
+import com.onesignal.session.internal.session.ISessionLifecycleHandler
+import com.onesignal.session.internal.session.ISessionService
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.operations.RefreshUserOperation
 
-// Ensure cache for the user is refreshed once per cold start when app
+// Ensure user is refreshed only when app
 // is in the foreground. This saves resources as there are a number of
 // events (such as push received or non-OneSignal events) that start
 // the app in the background but will never read/write any user
 // properties.
 class UserRefreshService(
     private val _applicationService: IApplicationService,
+    private val _sessionService: ISessionService,
     private val _operationRepo: IOperationRepo,
     private val _configModelStore: ConfigModelStore,
     private val _identityModelStore: IdentityModelStore,
 ) : IStartableService,
-    IApplicationLifecycleHandler {
+    ISessionLifecycleHandler {
     private fun refreshUser() {
-        if (IDManager.isLocalId(_identityModelStore.model.onesignalId)) return
+        if (IDManager.isLocalId(_identityModelStore.model.onesignalId) || !_applicationService.isInForeground) {
+            return
+        }
 
         _operationRepo.enqueue(
             RefreshUserOperation(
@@ -32,21 +36,11 @@ class UserRefreshService(
         )
     }
 
-    override fun start() {
-        if (_applicationService.isInForeground) {
-            refreshUser()
-        } else {
-            _applicationService.addApplicationLifecycleHandler(this)
-        }
-    }
+    override fun start() = _sessionService.subscribe(this)
 
-    private var onFocusCalled: Boolean = false
+    override fun onSessionStarted() = refreshUser()
 
-    override fun onFocus() {
-        if (onFocusCalled) return
-        onFocusCalled = true
-        refreshUser()
-    }
+    override fun onSessionActive() { }
 
-    override fun onUnfocused() { }
+    override fun onSessionEnded(duration: Long) { }
 }

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/ApplicationServiceTests.kt

@@ -175,7 +175,27 @@ class ApplicationServiceTests : FunSpec({
         // Then
         currentActivity shouldBe activity2
         verify(exactly = 1) { mockApplicationLifecycleHandler.onUnfocused() }
-        verify(exactly = 1) { mockApplicationLifecycleHandler.onFocus() }
+        verify(exactly = 1) { mockApplicationLifecycleHandler.onFocus(false) }
+    }
+
+    test("focus will occur on subscribe when activity is already started") {
+        // Given
+        val activity: Activity
+
+        Robolectric.buildActivity(Activity::class.java).use { controller ->
+            controller.setup() // Moves Activity to RESUMED state
+            activity = controller.get()
+        }
+
+        val applicationService = ApplicationService()
+        val mockApplicationLifecycleHandler = spyk<IApplicationLifecycleHandler>()
+
+        // When
+        applicationService.start(activity)
+        applicationService.addApplicationLifecycleHandler(mockApplicationLifecycleHandler)
+
+        // Then
+        verify(exactly = 1) { mockApplicationLifecycleHandler.onFocus(true) }
     }
 
     test("wait until system condition returns false when there is no activity") {