fix rare app not opening from notification bug

In rare cases when tapping on a notification the app would not open,
as well as other issues like the click not being counted either.

The issue was due to a race condition between
NotificationListener.start and processing the click event. To fix
and to avoid this mistake in the future moved all the code from
NotificationListener into NotificationLifecycleService. There
wasn't a clear set of responsibility difference between these
to classes so this is also a win to remove complexity.

Was able to reproduce the issue 1/5 times on an Android 14
(no extension level) emulator. After testing 20 times I am
no longer able to reproduce the issue now.

Author: jkasten2

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/NotificationsModule.kt

@@ -39,7 +39,6 @@ import com.onesignal.notifications.internal.lifecycle.impl.NotificationLifecycle
 import com.onesignal.notifications.internal.limiting.INotificationLimitManager
 import com.onesignal.notifications.internal.limiting.impl.NotificationLimitManager
 import com.onesignal.notifications.internal.listeners.DeviceRegistrationListener
-import com.onesignal.notifications.internal.listeners.NotificationListener
 import com.onesignal.notifications.internal.open.INotificationOpenedProcessor
 import com.onesignal.notifications.internal.open.INotificationOpenedProcessorHMS
 import com.onesignal.notifications.internal.open.impl.NotificationOpenedProcessor
@@ -94,6 +93,7 @@ internal class NotificationsModule : IModule {
 
         builder.register<NotificationLifecycleService>()
             .provides<INotificationLifecycleService>()
+            .provides<INotificationActivityOpener>()
 
         builder.register {
             if (FirebaseAnalyticsTracker.canTrack()) {
@@ -141,10 +141,8 @@ internal class NotificationsModule : IModule {
 
         // Startable services
         builder.register<DeviceRegistrationListener>().provides<IStartableService>()
-        builder.register<NotificationListener>().provides<IStartableService>()
 
         builder.register<NotificationsManager>()
             .provides<INotificationsManager>()
-            .provides<INotificationActivityOpener>()
     }
 }

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/NotificationsManager.kt

@@ -10,7 +10,6 @@ import com.onesignal.notifications.INotificationClickListener
 import com.onesignal.notifications.INotificationLifecycleListener
 import com.onesignal.notifications.INotificationsManager
 import com.onesignal.notifications.IPermissionObserver
-import com.onesignal.notifications.internal.common.GenerateNotificationOpenIntentFromPushPayload
 import com.onesignal.notifications.internal.common.NotificationHelper
 import com.onesignal.notifications.internal.data.INotificationRepository
 import com.onesignal.notifications.internal.lifecycle.INotificationLifecycleService
@@ -21,7 +20,6 @@ import com.onesignal.notifications.internal.summary.INotificationSummaryManager
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import org.json.JSONArray
-import org.json.JSONException
 
 interface INotificationActivityOpener {
     suspend fun openDestinationActivity(
@@ -42,7 +40,6 @@ internal class NotificationsManager(
     private val _notificationDataController: INotificationRepository,
     private val _summaryManager: INotificationSummaryManager,
 ) : INotificationsManager,
-    INotificationActivityOpener,
     INotificationPermissionChangedHandler,
     IApplicationLifecycleHandler {
     override var permission: Boolean = NotificationHelper.areNotificationsEnabled(_applicationService.appContext)
@@ -159,26 +156,4 @@ internal class NotificationsManager(
         Logging.debug("NotificationsManager.removeClickListener(listener: $listener)")
         _notificationLifecycleService.removeExternalClickListener(listener)
     }
-
-    override suspend fun openDestinationActivity(
-        activity: Activity,
-        pushPayloads: JSONArray,
-    ) {
-        try {
-            // Always use the top most notification if user tapped on the summary notification
-            val firstPayloadItem = pushPayloads.getJSONObject(0)
-            // val isHandled = _notificationLifecycleService.canOpenNotification(activity, firstPayloadItem)
-            val intentGenerator = GenerateNotificationOpenIntentFromPushPayload.create(activity, firstPayloadItem)
-
-            val intent = intentGenerator.getIntentVisible()
-            if (intent != null) {
-                Logging.info("SDK running startActivity with Intent: $intent")
-                activity.startActivity(intent)
-            } else {
-                Logging.info("SDK not showing an Activity automatically due to it's settings.")
-            }
-        } catch (e: JSONException) {
-            e.printStackTrace()
-        }
-    }
 }

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/lifecycle/INotificationLifecycleEventHandler.kt

@@ -18,7 +18,6 @@ import org.json.JSONObject
  * --------
  * [INotificationLifecycleCallback]: Can be used by other modules to use push
  * notifications to drive their own processing, and prevent it from being received and/or opened by the user.
- * [INotificationLifecycleEventHandler]: Used to hook into the received/opened events of a notification.
  *
  * External
  * --------
@@ -32,17 +31,11 @@ import org.json.JSONObject
  *  3. [INotificationServiceExtension.onNotificationReceived] To pre-process the notification, notification may be removed/changed. (Specified in AndroidManifest.xml).
  *  4. [INotificationLifecycleListener.onWillDisplay] To pre-process the notification while app in foreground, notification may be removed/changed.
  *  5. Process/Display the notification
- *  6. [INotificationLifecycleEventHandler.onNotificationReceived] To indicate the notification has been received and processed.
- *  7. User "opens" or "dismisses" the notification
- *  8. [INotificationLifecycleCallback.canOpenNotification] To determine if the notification can be opened by the notification module, or should be ignored.
- *  9. [INotificationLifecycleEventHandler.onNotificationOpened] To indicate the notification has been opened.
- * 10. [INotificationClickListener.onClick] To indicate the notification has been opened.
+ *  6. User "opens" or "dismisses" the notification
+ *  7. [INotificationLifecycleCallback.canOpenNotification] To determine if the notification can be opened by the notification module, or should be ignored.
+ *  8. [INotificationClickListener.onClick] To indicate the notification has been opened.
  */
 interface INotificationLifecycleService {
-    fun addInternalNotificationLifecycleEventHandler(handler: INotificationLifecycleEventHandler)
-
-    fun removeInternalNotificationLifecycleEventHandler(handler: INotificationLifecycleEventHandler)
-
     fun setInternalNotificationLifecycleCallback(callback: INotificationLifecycleCallback?)
 
     fun addExternalForegroundLifecycleListener(listener: INotificationLifecycleListener)

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/lifecycle/INotificationLifecycleService.kt

@@ -3,45 +3,58 @@ package com.onesignal.notifications.internal.lifecycle.impl
 import android.app.Activity
 import android.content.Context
 import com.onesignal.common.AndroidUtils
+import com.onesignal.common.JSONUtils
 import com.onesignal.common.events.CallbackProducer
 import com.onesignal.common.events.EventProducer
+import com.onesignal.common.exceptions.BackendException
+import com.onesignal.core.internal.application.AppEntryAction
 import com.onesignal.core.internal.application.IApplicationService
+import com.onesignal.core.internal.config.ConfigModelStore
+import com.onesignal.core.internal.device.IDeviceService
 import com.onesignal.core.internal.time.ITime
 import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.notifications.INotificationClickListener
 import com.onesignal.notifications.INotificationLifecycleListener
 import com.onesignal.notifications.INotificationReceivedEvent
 import com.onesignal.notifications.INotificationServiceExtension
 import com.onesignal.notifications.INotificationWillDisplayEvent
+import com.onesignal.notifications.internal.INotificationActivityOpener
+import com.onesignal.notifications.internal.analytics.IAnalyticsTracker
+import com.onesignal.notifications.internal.backend.INotificationBackendService
+import com.onesignal.notifications.internal.common.GenerateNotificationOpenIntentFromPushPayload
 import com.onesignal.notifications.internal.common.NotificationConstants
+import com.onesignal.notifications.internal.common.NotificationFormatHelper
 import com.onesignal.notifications.internal.common.NotificationGenerationJob
 import com.onesignal.notifications.internal.common.NotificationHelper
+import com.onesignal.notifications.internal.common.OSNotificationOpenAppSettings
 import com.onesignal.notifications.internal.lifecycle.INotificationLifecycleCallback
-import com.onesignal.notifications.internal.lifecycle.INotificationLifecycleEventHandler
 import com.onesignal.notifications.internal.lifecycle.INotificationLifecycleService
+import com.onesignal.notifications.internal.receivereceipt.IReceiveReceiptWorkManager
+import com.onesignal.session.internal.influence.IInfluenceManager
+import com.onesignal.user.internal.subscriptions.ISubscriptionManager
 import org.json.JSONArray
+import org.json.JSONException
 import org.json.JSONObject
 
 internal class NotificationLifecycleService(
     applicationService: IApplicationService,
     private val _time: ITime,
-) : INotificationLifecycleService {
-    private val intLifecycleHandler = EventProducer<INotificationLifecycleEventHandler>()
+    private val _applicationService: IApplicationService,
+    private val _configModelStore: ConfigModelStore,
+    private val _influenceManager: IInfluenceManager,
+    private val _subscriptionManager: ISubscriptionManager,
+    private val _deviceService: IDeviceService,
+    private val _backend: INotificationBackendService,
+    private val _receiveReceiptWorkManager: IReceiveReceiptWorkManager,
+    private val _analyticsTracker: IAnalyticsTracker,
+) : INotificationLifecycleService,
+    INotificationActivityOpener {
     private val intLifecycleCallback = CallbackProducer<INotificationLifecycleCallback>()
     private val extRemoteReceivedCallback = CallbackProducer<INotificationServiceExtension>()
     private val extWillShowInForegroundCallback = EventProducer<INotificationLifecycleListener>()
     private val extOpenedCallback = EventProducer<INotificationClickListener>()
     private val unprocessedOpenedNotifs: ArrayDeque<JSONArray> = ArrayDeque()
-
-    override fun addInternalNotificationLifecycleEventHandler(handler: INotificationLifecycleEventHandler) =
-        intLifecycleHandler.subscribe(
-            handler,
-        )
-
-    override fun removeInternalNotificationLifecycleEventHandler(handler: INotificationLifecycleEventHandler) =
-        intLifecycleHandler.unsubscribe(
-            handler,
-        )
+    private val postedOpenedNotifIds = mutableSetOf<String>()
 
     override fun setInternalNotificationLifecycleCallback(callback: INotificationLifecycleCallback?) = intLifecycleCallback.set(callback)
 
@@ -75,12 +88,28 @@ internal class NotificationLifecycleService(
 
     override suspend fun canReceiveNotification(jsonPayload: JSONObject): Boolean {
         var canReceive = true
+        // TODO: This can have late binding issues too
         intLifecycleCallback.suspendingFire { canReceive = it.canReceiveNotification(jsonPayload) }
         return canReceive
     }
 
     override suspend fun notificationReceived(notificationJob: NotificationGenerationJob) {
-        intLifecycleHandler.suspendingFire { it.onNotificationReceived(notificationJob) }
+        _receiveReceiptWorkManager.enqueueReceiveReceipt(notificationJob.apiNotificationId)
+
+        _influenceManager.onNotificationReceived(notificationJob.apiNotificationId)
+
+        try {
+            val jsonObject = JSONObject(notificationJob.jsonPayload.toString())
+            jsonObject.put(NotificationConstants.BUNDLE_KEY_ANDROID_NOTIFICATION_ID, notificationJob.androidId)
+            val openResult = NotificationHelper.generateNotificationOpenedResult(JSONUtils.wrapInJsonArray(jsonObject), _time)
+
+            _analyticsTracker.trackReceivedEvent(
+                openResult.notification.notificationId!!,
+                NotificationHelper.getCampaignNameFromNotification(openResult.notification),
+            )
+        } catch (e: JSONException) {
+            e.printStackTrace()
+        }
     }
 
     override suspend fun canOpenNotification(
@@ -96,7 +125,50 @@ internal class NotificationLifecycleService(
         activity: Activity,
         data: JSONArray,
     ) {
-        intLifecycleHandler.suspendingFire { it.onNotificationOpened(activity, data) }
+        val config = _configModelStore.model
+        val appId: String = config.appId ?: ""
+        val subscriptionId: String = _subscriptionManager.subscriptions.push.id
+        val deviceType = _deviceService.deviceType
+
+        for (i in 0 until data.length()) {
+            val notificationId = NotificationFormatHelper.getOSNotificationIdFromJson(data[i] as JSONObject?) ?: continue
+
+            if (postedOpenedNotifIds.contains(notificationId)) {
+                continue
+            }
+
+            postedOpenedNotifIds.add(notificationId)
+
+            try {
+                _backend.updateNotificationAsOpened(
+                    appId,
+                    notificationId,
+                    subscriptionId,
+                    deviceType,
+                )
+            } catch (ex: BackendException) {
+                Logging.error("Notification opened confirmation failed with statusCode: ${ex.statusCode} response: ${ex.response}")
+            }
+        }
+
+        val openResult = NotificationHelper.generateNotificationOpenedResult(data, _time)
+        _analyticsTracker.trackOpenedEvent(
+            openResult.notification.notificationId!!,
+            NotificationHelper.getCampaignNameFromNotification(openResult.notification),
+        )
+
+        // Handle the first element in the data array as the latest notification
+        val latestNotificationId = getLatestNotificationId(data)
+
+        if (shouldInitDirectSessionFromNotificationOpen(activity)) {
+            // We want to set the app entry state to NOTIFICATION_CLICK when coming from background
+            _applicationService.entryState = AppEntryAction.NOTIFICATION_CLICK
+            if (latestNotificationId != null) {
+                _influenceManager.onDirectInfluenceFromNotification(latestNotificationId)
+            }
+        }
+
+        openDestinationActivity(activity, data)
 
         // queue up the opened notification in case the handler hasn't been set yet. Once set,
         // we will immediately fire the handler.
@@ -161,4 +233,43 @@ internal class NotificationLifecycleService(
             e.printStackTrace()
         }
     }
+
+    private fun shouldInitDirectSessionFromNotificationOpen(context: Activity): Boolean {
+        if (_applicationService.isInForeground) {
+            return false
+        }
+
+        try {
+            return OSNotificationOpenAppSettings.getShouldOpenActivity(context)
+        } catch (e: JSONException) {
+            e.printStackTrace()
+        }
+        return true
+    }
+
+    private fun getLatestNotificationId(data: JSONArray): String? {
+        val latestNotification = if (data.length() > 0) data[0] as JSONObject else null
+        return NotificationFormatHelper.getOSNotificationIdFromJson(latestNotification)
+    }
+
+    override suspend fun openDestinationActivity(
+        activity: Activity,
+        pushPayloads: JSONArray,
+    ) {
+        try {
+            // Always use the top most notification if user tapped on the summary notification
+            val firstPayloadItem = pushPayloads.getJSONObject(0)
+            val intentGenerator = GenerateNotificationOpenIntentFromPushPayload.create(activity, firstPayloadItem)
+
+            val intent = intentGenerator.getIntentVisible()
+            if (intent != null) {
+                Logging.info("SDK running startActivity with Intent: $intent")
+                activity.startActivity(intent)
+            } else {
+                Logging.info("SDK not showing an Activity automatically due to it's settings.")
+            }
+        } catch (e: JSONException) {
+            e.printStackTrace()
+        }
+    }
 }