Skip to content

Commit 3a6e913

Browse files
authored
Merge pull request #2304 from OneSignal/screen-frozon-click-numerous-notifications
Fix: NotificationOpenedActivity "freeze" when a large amount of notification is clicked
2 parents 77d2769 + 817a9cf commit 3a6e913

File tree

2 files changed

+110
-44
lines changed

2 files changed

+110
-44
lines changed

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.onesignal.common.JSONUtils
77
import com.onesignal.common.events.CallbackProducer
88
import com.onesignal.common.events.EventProducer
99
import com.onesignal.common.exceptions.BackendException
10+
import com.onesignal.common.threading.OSPrimaryCoroutineScope
1011
import com.onesignal.core.internal.application.AppEntryAction
1112
import com.onesignal.core.internal.application.IApplicationService
1213
import com.onesignal.core.internal.config.ConfigModelStore
@@ -138,15 +139,17 @@ internal class NotificationLifecycleService(
138139

139140
postedOpenedNotifIds.add(notificationId)
140141

141-
try {
142-
_backend.updateNotificationAsOpened(
143-
appId,
144-
notificationId,
145-
subscriptionId,
146-
deviceType,
147-
)
148-
} catch (ex: BackendException) {
149-
Logging.error("Notification opened confirmation failed with statusCode: ${ex.statusCode} response: ${ex.response}")
142+
OSPrimaryCoroutineScope.execute {
143+
try {
144+
_backend.updateNotificationAsOpened(
145+
appId,
146+
notificationId,
147+
subscriptionId,
148+
deviceType,
149+
)
150+
} catch (ex: BackendException) {
151+
Logging.error("Notification opened confirmation failed with statusCode: ${ex.statusCode} response: ${ex.response}")
152+
}
150153
}
151154
}
152155

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

Lines changed: 98 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,67 @@ import io.mockk.coVerify
2323
import io.mockk.every
2424
import io.mockk.mockk
2525
import io.mockk.spyk
26+
import kotlinx.coroutines.delay
27+
import kotlinx.coroutines.withTimeout
2628
import org.json.JSONArray
2729
import org.json.JSONObject
2830
import org.robolectric.Robolectric
31+
import org.robolectric.android.controller.ActivityController
32+
33+
private class Mocks {
34+
val context = ApplicationProvider.getApplicationContext<Context>()
35+
val applicationService =
36+
run {
37+
val applicationService = ApplicationService()
38+
applicationService.start(context)
39+
applicationService
40+
}
41+
42+
val mockSubscriptionManager: ISubscriptionManager =
43+
run {
44+
val mockSubManager = mockk<ISubscriptionManager>()
45+
every { mockSubManager.subscriptions.push } returns
46+
mockk<IPushSubscription>().apply { every { id } returns "UUID1" }
47+
mockSubManager
48+
}
49+
50+
val notificationLifecycleService =
51+
spyk(
52+
NotificationLifecycleService(
53+
applicationService,
54+
MockHelper.time(0),
55+
MockHelper.configModelStore(),
56+
mockk<IInfluenceManager>().apply {
57+
every { onDirectInfluenceFromNotification(any()) } returns Unit
58+
},
59+
mockSubscriptionManager,
60+
mockk<IDeviceService>().apply {
61+
every { deviceType } returns IDeviceService.DeviceType.Android
62+
},
63+
mockk<INotificationBackendService>().apply {
64+
coEvery { updateNotificationAsOpened(any(), any(), any(), any()) } coAnswers {
65+
// assume every updateNotificationAsOpened call takes 5 ms
66+
delay(5)
67+
Unit
68+
}
69+
},
70+
mockk<IReceiveReceiptWorkManager>(),
71+
mockk<IAnalyticsTracker>().apply {
72+
every { trackOpenedEvent(any(), any()) } returns Unit
73+
},
74+
),
75+
)
76+
77+
val activity: Activity =
78+
run {
79+
val activityController: ActivityController<Activity>
80+
Robolectric.buildActivity(Activity::class.java).use { controller ->
81+
controller.setup() // Moves Activity to RESUMED state
82+
activityController = controller
83+
}
84+
activityController.get()
85+
}
86+
}
2987

3088
@RobolectricTest
3189
class NotificationLifecycleServiceTests : FunSpec({
@@ -36,41 +94,9 @@ class NotificationLifecycleServiceTests : FunSpec({
3694

3795
test("Fires openDestinationActivity") {
3896
// Given
39-
val context = ApplicationProvider.getApplicationContext<Context>()
40-
val applicationService = ApplicationService()
41-
applicationService.start(context)
42-
43-
val mockSubscriptionManager = mockk<ISubscriptionManager>()
44-
every { mockSubscriptionManager.subscriptions.push } returns
45-
mockk<IPushSubscription>().apply { every { id } returns "UUID1" }
46-
47-
val notificationLifecycleService =
48-
spyk(
49-
NotificationLifecycleService(
50-
applicationService,
51-
MockHelper.time(0),
52-
MockHelper.configModelStore(),
53-
mockk<IInfluenceManager>().apply {
54-
every { onDirectInfluenceFromNotification(any()) } returns Unit
55-
},
56-
mockSubscriptionManager,
57-
mockk<IDeviceService>().apply {
58-
every { deviceType } returns IDeviceService.DeviceType.Android
59-
},
60-
mockk<INotificationBackendService>().apply {
61-
coEvery { updateNotificationAsOpened(any(), any(), any(), any()) } returns Unit
62-
},
63-
mockk<IReceiveReceiptWorkManager>(),
64-
mockk<IAnalyticsTracker>().apply {
65-
every { trackOpenedEvent(any(), any()) } returns Unit
66-
},
67-
),
68-
)
69-
val activity: Activity
70-
Robolectric.buildActivity(Activity::class.java).use { controller ->
71-
controller.setup() // Moves Activity to RESUMED state
72-
activity = controller.get()
73-
}
97+
val mocks = Mocks()
98+
val notificationLifecycleService = mocks.notificationLifecycleService
99+
val activity = mocks.activity
74100

75101
// When
76102
val payload =
@@ -94,4 +120,41 @@ class NotificationLifecycleServiceTests : FunSpec({
94120
)
95121
}
96122
}
123+
124+
test("ensure notificationOpened makes backend updates in a background process") {
125+
// Given
126+
val mocks = Mocks()
127+
val notificationLifecycleService = mocks.notificationLifecycleService
128+
val activity = mocks.activity
129+
130+
// When
131+
val payload = JSONArray()
132+
for (i in 1..1000) {
133+
// adding 1000 different notifications
134+
payload.put(
135+
JSONObject()
136+
.put("alert", "test message")
137+
.put(
138+
"custom",
139+
JSONObject()
140+
.put("i", "UUID$i"),
141+
),
142+
)
143+
}
144+
145+
withTimeout(500) {
146+
// 1000 notifications should be handled within a small amount of time
147+
notificationLifecycleService.notificationOpened(activity, payload)
148+
}
149+
150+
// Then
151+
coVerify(exactly = 1) {
152+
// ensure openDestinationActivity is called within the timeout, prove that the increasing
153+
// number of notifications clicked does not delay the main thread proportionally
154+
notificationLifecycleService.openDestinationActivity(
155+
withArg { Any() },
156+
withArg { Any() },
157+
)
158+
}
159+
}
97160
})

0 commit comments

Comments
 (0)