Merge pull request #2304 from OneSignal/screen-frozon-click-numerous-notifications

Fix: NotificationOpenedActivity "freeze" when a large amount of notification is clicked

Author: jinliu9508

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

@@ -7,6 +7,7 @@ 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.common.threading.OSPrimaryCoroutineScope
 import com.onesignal.core.internal.application.AppEntryAction
 import com.onesignal.core.internal.application.IApplicationService
 import com.onesignal.core.internal.config.ConfigModelStore
@@ -138,15 +139,17 @@ internal class NotificationLifecycleService(
 
             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}")
+            OSPrimaryCoroutineScope.execute {
+                try {
+                    _backend.updateNotificationAsOpened(
+                        appId,
+                        notificationId,
+                        subscriptionId,
+                        deviceType,
+                    )
+                } catch (ex: BackendException) {
+                    Logging.error("Notification opened confirmation failed with statusCode: ${ex.statusCode} response: ${ex.response}")
+                }
             }
         }
 

OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/lifecycle/NotificationLifecycleServiceTests.kt

@@ -23,9 +23,67 @@ import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.spyk
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.withTimeout
 import org.json.JSONArray
 import org.json.JSONObject
 import org.robolectric.Robolectric
+import org.robolectric.android.controller.ActivityController
+
+private class Mocks {
+    val context = ApplicationProvider.getApplicationContext<Context>()
+    val applicationService =
+        run {
+            val applicationService = ApplicationService()
+            applicationService.start(context)
+            applicationService
+        }
+
+    val mockSubscriptionManager: ISubscriptionManager =
+        run {
+            val mockSubManager = mockk<ISubscriptionManager>()
+            every { mockSubManager.subscriptions.push } returns
+                mockk<IPushSubscription>().apply { every { id } returns "UUID1" }
+            mockSubManager
+        }
+
+    val notificationLifecycleService =
+        spyk(
+            NotificationLifecycleService(
+                applicationService,
+                MockHelper.time(0),
+                MockHelper.configModelStore(),
+                mockk<IInfluenceManager>().apply {
+                    every { onDirectInfluenceFromNotification(any()) } returns Unit
+                },
+                mockSubscriptionManager,
+                mockk<IDeviceService>().apply {
+                    every { deviceType } returns IDeviceService.DeviceType.Android
+                },
+                mockk<INotificationBackendService>().apply {
+                    coEvery { updateNotificationAsOpened(any(), any(), any(), any()) } coAnswers {
+                        // assume every updateNotificationAsOpened call takes 5 ms
+                        delay(5)
+                        Unit
+                    }
+                },
+                mockk<IReceiveReceiptWorkManager>(),
+                mockk<IAnalyticsTracker>().apply {
+                    every { trackOpenedEvent(any(), any()) } returns Unit
+                },
+            ),
+        )
+
+    val activity: Activity =
+        run {
+            val activityController: ActivityController<Activity>
+            Robolectric.buildActivity(Activity::class.java).use { controller ->
+                controller.setup() // Moves Activity to RESUMED state
+                activityController = controller
+            }
+            activityController.get()
+        }
+}
 
 @RobolectricTest
 class NotificationLifecycleServiceTests : FunSpec({
@@ -36,41 +94,9 @@ class NotificationLifecycleServiceTests : FunSpec({
 
     test("Fires openDestinationActivity") {
         // Given
-        val context = ApplicationProvider.getApplicationContext<Context>()
-        val applicationService = ApplicationService()
-        applicationService.start(context)
-
-        val mockSubscriptionManager = mockk<ISubscriptionManager>()
-        every { mockSubscriptionManager.subscriptions.push } returns
-            mockk<IPushSubscription>().apply { every { id } returns "UUID1" }
-
-        val notificationLifecycleService =
-            spyk(
-                NotificationLifecycleService(
-                    applicationService,
-                    MockHelper.time(0),
-                    MockHelper.configModelStore(),
-                    mockk<IInfluenceManager>().apply {
-                        every { onDirectInfluenceFromNotification(any()) } returns Unit
-                    },
-                    mockSubscriptionManager,
-                    mockk<IDeviceService>().apply {
-                        every { deviceType } returns IDeviceService.DeviceType.Android
-                    },
-                    mockk<INotificationBackendService>().apply {
-                        coEvery { updateNotificationAsOpened(any(), any(), any(), any()) } returns Unit
-                    },
-                    mockk<IReceiveReceiptWorkManager>(),
-                    mockk<IAnalyticsTracker>().apply {
-                        every { trackOpenedEvent(any(), any()) } returns Unit
-                    },
-                ),
-            )
-        val activity: Activity
-        Robolectric.buildActivity(Activity::class.java).use { controller ->
-            controller.setup() // Moves Activity to RESUMED state
-            activity = controller.get()
-        }
+        val mocks = Mocks()
+        val notificationLifecycleService = mocks.notificationLifecycleService
+        val activity = mocks.activity
 
         // When
         val payload =
@@ -94,4 +120,41 @@ class NotificationLifecycleServiceTests : FunSpec({
             )
         }
     }
+
+    test("ensure notificationOpened makes backend updates in a background process") {
+        // Given
+        val mocks = Mocks()
+        val notificationLifecycleService = mocks.notificationLifecycleService
+        val activity = mocks.activity
+
+        // When
+        val payload = JSONArray()
+        for (i in 1..1000) {
+            // adding 1000 different notifications
+            payload.put(
+                JSONObject()
+                    .put("alert", "test message")
+                    .put(
+                        "custom",
+                        JSONObject()
+                            .put("i", "UUID$i"),
+                    ),
+            )
+        }
+
+        withTimeout(500) {
+            // 1000 notifications should be handled within a small amount of time
+            notificationLifecycleService.notificationOpened(activity, payload)
+        }
+
+        // Then
+        coVerify(exactly = 1) {
+            // ensure openDestinationActivity is called within the timeout, prove that the increasing
+            // number of notifications clicked does not delay the main thread proportionally
+            notificationLifecycleService.openDestinationActivity(
+                withArg { Any() },
+                withArg { Any() },
+            )
+        }
+    }
 })