From c6812cb6ea6e632f2d81ccfa8b6cb0ebadb31e85 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 09:54:10 -0700 Subject: [PATCH 01/11] fix: [SDK-5011] dismiss restored notifications suppressed by preventDefault markNotificationAsDismissed returned early whenever isNotificationToDisplay was false, which is always the case on the suppress path, so a restored notification the app rejected was never marked dismissed and came back on every later restore. Co-authored-by: Cursor --- .../impl/NotificationGenerationProcessor.kt | 7 +++- .../NotificationGenerationProcessorTests.kt | 35 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt index 5cdd5ef6f7..bcef2c2f74 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt @@ -187,7 +187,7 @@ internal class NotificationGenerationProcessor( if (isRestoring) { // If we are not displaying a restored notification make sure we mark it as dismissed // This will prevent it from being restored again - markNotificationAsDismissed(notificationJob) + dismissNotification(notificationJob) } else { // indicate the notification job did not display. We process it as "opened" to prevent // a duplicate from coming in and us having to process it again. @@ -296,10 +296,15 @@ internal class NotificationGenerationProcessor( } private suspend fun markNotificationAsDismissed(notifiJob: NotificationGenerationJob) { + // Nothing was posted, so there is nothing to dismiss. if (!notifiJob.isNotificationToDisplay) { return } + dismissNotification(notifiJob) + } + + private suspend fun dismissNotification(notifiJob: NotificationGenerationJob) { Logging.debug("Marking restored or disabled notifications as dismissed: $notifiJob") val didDismiss = _dataController.markAsDismissed(notifiJob.androidId) diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt index b992a70565..665648b13a 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt @@ -14,6 +14,7 @@ import com.onesignal.notifications.internal.data.INotificationRepository import com.onesignal.notifications.internal.display.INotificationDisplayer import com.onesignal.notifications.internal.generation.impl.NotificationGenerationProcessor import com.onesignal.notifications.internal.lifecycle.INotificationLifecycleService +import com.onesignal.notifications.internal.summary.INotificationSummaryManager import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.coEvery @@ -58,6 +59,7 @@ private class Mocks { run { val mockNotificationRepository = mockk() coEvery { mockNotificationRepository.doesNotificationExist(any()) } returns false + coEvery { mockNotificationRepository.markAsDismissed(any()) } returns true coEvery { mockNotificationRepository.createNotification( any(), @@ -75,6 +77,8 @@ private class Mocks { mockNotificationRepository } + val notificationSummaryManager = mockk(relaxed = true) + val notificationGenerationProcessor = run { val mock = spyk( NotificationGenerationProcessor( @@ -82,7 +86,7 @@ private class Mocks { notificationDisplayer, MockHelper.configModelStore(), notificationRepository, - mockk(), + notificationSummaryManager, notificationLifecycleService, MockHelper.time(1111), ), recordPrivateCalls = true @@ -196,6 +200,35 @@ class NotificationGenerationProcessorTests : FunSpec({ coVerify(exactly = 1) { mocks.notificationLifecycleService.notificationReceived(any()) } + // Nothing was posted, so it is saved as opened rather than dismissed. + coVerify(exactly = 0) { + mocks.notificationRepository.markAsDismissed(any()) + } + } + + test("processNotificationData should mark a restored notification dismissed when the received event prevents display") { + // Given + val mocks = Mocks() + // The suite default of 10ms can expire before Dispatchers.IO runs the callback. + every { mocks.notificationGenerationProcessor getProperty "EXTERNAL_CALLBACKS_TIMEOUT" } answers { 1_000L } + coEvery { mocks.notificationLifecycleService.externalRemoteNotificationReceived(any()) } answers { + firstArg().preventDefault(true) + } + + // When + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, true, 1111) + + // Then + coVerify(exactly = 0) { + mocks.notificationDisplayer.displayNotification(any()) + } + // Without this the notification comes back on every later restore. + coVerify(exactly = 1) { + mocks.notificationRepository.markAsDismissed(1) + } + coVerify(exactly = 1) { + mocks.notificationSummaryManager.updatePossibleDependentSummaryOnDismiss(1) + } } test("processNotificationData should display notification when external callback takes longer than 30 seconds") { From 0a374247d02c66bcf6272be4c18b34c3cd399fdf Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 09:54:24 -0700 Subject: [PATCH 02/11] feat: [SDK-5011] expose restoring on INotificationReceivedEvent Apps had no way to tell a restored notification apart from a new one inside their notification service extension, so work meant to run once ran again on every restore. v3 exposed this flag and it was dropped in the v4 refactor. Co-authored-by: Cursor --- .../INotificationReceivedEvent.kt | 10 ++++++++++ .../internal/NotificationReceivedEvent.kt | 1 + .../impl/NotificationGenerationProcessor.kt | 2 +- .../NotificationGenerationProcessorTests.kt | 18 ++++++++++++++++++ 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt index f415bbf7aa..6b1d9b9993 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt @@ -45,6 +45,16 @@ interface INotificationReceivedEvent { */ val notification: IDisplayableMutableNotification + /** + * Whether OneSignal is showing this notification again because Android cleared it from the + * notification shade, such as after a reboot or an app update. Your app already received + * this notification once before. + * + * Use it to skip work that should only run the first time, or call [preventDefault] to stop + * the notification from showing again. + */ + val restoring: Boolean + /** * Call this to prevent OneSignal from displaying the notification automatically. The notification * can still be manually displayed using `notification.display()`. diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/NotificationReceivedEvent.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/NotificationReceivedEvent.kt index 9bf6cbf709..810b97b097 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/NotificationReceivedEvent.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/NotificationReceivedEvent.kt @@ -7,6 +7,7 @@ import com.onesignal.notifications.INotificationReceivedEvent internal class NotificationReceivedEvent( override val context: Context, override val notification: Notification, + override val restoring: Boolean, ) : INotificationReceivedEvent { var isPreventDefault: Boolean = false var discard: Boolean = false diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt index bcef2c2f74..f2e041f067 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt @@ -69,7 +69,7 @@ internal class NotificationGenerationProcessor( Logging.info("Fire remoteNotificationReceived") try { - val notificationReceivedEvent = NotificationReceivedEvent(context, notification) + val notificationReceivedEvent = NotificationReceivedEvent(context, notification, isRestoring) withTimeout(EXTERNAL_CALLBACKS_TIMEOUT) { launchOnIO { _lifecycleService.externalRemoteNotificationReceived(notificationReceivedEvent) diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt index 665648b13a..77056049df 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt @@ -184,6 +184,24 @@ class NotificationGenerationProcessorTests : FunSpec({ } } + test("processNotificationData should tell the received event whether it is restoring") { + // Given + val mocks = Mocks() + val restoringFlags = mutableListOf() + coEvery { mocks.notificationDisplayer.displayNotification(any()) } returns true + coEvery { mocks.notificationLifecycleService.externalRemoteNotificationReceived(any()) } answers { + restoringFlags.add(firstArg().restoring) + } + coEvery { mocks.notificationLifecycleService.externalNotificationWillShowInForeground(any()) } just runs + + // When + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 2, mocks.notificationPayload, true, 1111) + + // Then + restoringFlags shouldBe listOf(false, true) + } + test("processNotificationData should not display notification when external callback indicates not to") { // Given val mocks = Mocks() From fb7dd4ba946f1a7813e6f059832a20103400cb7d Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 09:54:37 -0700 Subject: [PATCH 03/11] fix: [SDK-5011] keep restored notifications on the low-importance channel An extender calling setChannelId moved a restored notification off the silent Restored channel, and since the channel governs alerting on Android O+, removeNotifyOptions could not stop it from showing as a heads-up banner. Co-authored-by: Cursor --- .../impl/NotificationDisplayBuilder.kt | 4 + .../display/impl/NotificationDisplayer.kt | 2 + .../display/NotificationDisplayerTests.kt | 135 ++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/display/NotificationDisplayerTests.kt diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayBuilder.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayBuilder.kt index f72f4e98f3..2a3513436d 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayBuilder.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayBuilder.kt @@ -83,6 +83,7 @@ internal class NotificationDisplayBuilder( val fcmJson: JSONObject = notificationJob.jsonPayload!! val oneSignalNotificationBuilder = OneSignalNotificationBuilder() val channelId = _notificationChannelManager.createNotificationChannel(notificationJob) + oneSignalNotificationBuilder.channelId = channelId val notificationBuilder = NotificationCompat.Builder(currentContext, channelId) val message = fcmJson.optString("alert", null) notificationBuilder @@ -461,5 +462,8 @@ internal class NotificationDisplayBuilder( internal class OneSignalNotificationBuilder { var compatBuilder: NotificationCompat.Builder? = null var hasLargeIcon = false + + // The channel OneSignal picked, before any extender could change it. + var channelId: String? = null } } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt index fe24d9e765..1cb3a46a53 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt @@ -111,6 +111,8 @@ internal class NotificationDisplayer( // Keeps notification from playing sound + vibrating again if (notificationJob.isRestoring) { + // An extender may have changed the channel, and the channel controls alerting on O+. + oneSignalNotificationBuilder.channelId?.let { notifBuilder?.setChannelId(it) } _notificationDisplayBuilder.removeNotifyOptions(notifBuilder) } diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/display/NotificationDisplayerTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/display/NotificationDisplayerTests.kt new file mode 100644 index 0000000000..c967fa79b3 --- /dev/null +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/display/NotificationDisplayerTests.kt @@ -0,0 +1,135 @@ +package com.onesignal.notifications.internal.display + +import android.app.NotificationChannel +import android.app.NotificationManager +import android.content.Context +import android.net.Uri +import androidx.core.app.NotificationCompat +import androidx.test.core.app.ApplicationProvider +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.mocks.AndroidMockHelper +import com.onesignal.mocks.MockHelper +import com.onesignal.notifications.internal.channels.impl.NotificationChannelManager +import com.onesignal.notifications.internal.common.NotificationGenerationJob +import com.onesignal.notifications.internal.display.impl.NotificationDisplayBuilder +import com.onesignal.notifications.internal.display.impl.NotificationDisplayer +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.mockk.every +import io.mockk.mockk +import io.mockk.spyk +import org.json.JSONObject +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +private const val RESTORE_CHANNEL_ID = "restored_OS_notifications" +private const val APP_CHANNEL_ID = "app_high_importance" + +@Config( + packageName = "com.onesignal.example", + sdk = [26], +) +@RobolectricTest +class NotificationDisplayerTests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + fun notificationManager(): NotificationManager = + ApplicationProvider.getApplicationContext() + .getSystemService(NotificationManager::class.java) + + fun createAppChannel() { + notificationManager().createNotificationChannel( + NotificationChannel(APP_CHANNEL_ID, "App", NotificationManager.IMPORTANCE_HIGH), + ) + } + + suspend fun display( + isRestoring: Boolean, + extender: NotificationCompat.Extender?, + ): NotificationGenerationJob { + val applicationService = AndroidMockHelper.applicationService() + val payload = + JSONObject() + .put("alert", "test message") + .put("title", "test title") + .put("custom", JSONObject().put("i", "UUID1")) + + val job = NotificationGenerationJob(payload, MockHelper.time(1111)) + job.isRestoring = isRestoring + job.notification.setExtender(extender) + + val displayer = + spyk( + NotificationDisplayer( + applicationService, + mockk(relaxed = true), + mockk(relaxed = true), + NotificationDisplayBuilder( + applicationService, + NotificationChannelManager(applicationService, MockHelper.languageContext()), + ), + ), + ) + // displayNotification refuses to post from the main thread, which is where Robolectric runs. + every { displayer.isRunningOnMainThreadCheck } returns Unit + + displayer.displayNotification(job) + return job + } + + fun postedChannelId(): String? = shadowOf(notificationManager()).allNotifications.last().channelId + + test("restored notification is posted to the silent restore channel") { + // When + display(isRestoring = true, extender = null) + + // Then + postedChannelId() shouldBe RESTORE_CHANNEL_ID + } + + test("restored notification stays on the restore channel when an extender picks another channel") { + // Given + createAppChannel() + + // When + val job = + display(isRestoring = true) { + it.setChannelId(APP_CHANNEL_ID).setContentTitle("CUSTOM TITLE") + } + + // Then + // An app channel here would show every restored notification as a heads-up banner. + postedChannelId() shouldBe RESTORE_CHANNEL_ID + // Proves the extender actually ran, so the check above is not passing for free. + job.overriddenTitleFromExtender shouldBe "CUSTOM TITLE" + } + + test("notification that is not being restored keeps the channel its extender picked") { + // Given + createAppChannel() + + // When + display(isRestoring = false) { it.setChannelId(APP_CHANNEL_ID) } + + // Then + postedChannelId() shouldBe APP_CHANNEL_ID + } + + test("sound an extender sets is recorded separately from the payload sound") { + // Given + val extenderSound = Uri.parse("content://media/internal/audio/media/7") + + // When + val job = display(isRestoring = false) { it.setSound(extenderSound) } + + // Then + // SummaryNotificationDisplayer compares these two on API 21-23, so they must stay separate. + job.overriddenSound shouldBe extenderSound + job.orgSound shouldNotBe extenderSound + } +}) From ab4235d27ae64c7dcf40869599c338377b84ff60 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 09:54:45 -0700 Subject: [PATCH 04/11] refactor: [SDK-5011] drop unused orgFlags and simplify the sound workaround orgFlags was written after every extender ran and never read. Its sibling orgSound is still needed by the pre-Android 7.0 grouped-sound workaround, whose minSdk 21 version guard and stale comment are cleaned up here instead. Co-authored-by: Cursor --- .../internal/common/NotificationGenerationJob.kt | 4 ++-- .../internal/display/impl/NotificationDisplayer.kt | 1 - .../display/impl/SummaryNotificationDisplayer.kt | 11 ++++------- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt index 71f3ccc1d2..ac4a4d98ef 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt @@ -27,7 +27,8 @@ class NotificationGenerationJob( var overriddenTitleFromExtender: CharSequence? = null var overriddenSound: Uri? = null var overriddenFlags: Int? = null - var orgFlags: Int? = null + + // The payload's sound, saved before the extender runs so we can tell if the extender changed it. var orgSound: Uri? = null constructor(jsonPayload: JSONObject, time: ITime) : this( @@ -73,7 +74,6 @@ class NotificationGenerationJob( ", overriddenTitleFromExtender=" + overriddenTitleFromExtender + ", overriddenSound=" + overriddenSound + ", overriddenFlags=" + overriddenFlags + - ", orgFlags=" + orgFlags + ", orgSound=" + orgSound + ", notification=" + notification + '}' diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt index 1cb3a46a53..07197da7f2 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt @@ -202,7 +202,6 @@ internal class NotificationDisplayer( NotificationCompat.Builder::class.java.getDeclaredField("mNotification") mNotificationField.isAccessible = true var mNotification = mNotificationField[notificationBuilder] as Notification - notificationJob.orgFlags = mNotification.flags notificationJob.orgSound = mNotification.sound notificationBuilder!!.extend(notificationJob.notification!!.notificationExtender!!) mNotification = mNotificationField[notificationBuilder] as Notification diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/SummaryNotificationDisplayer.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/SummaryNotificationDisplayer.kt index 807844e94d..05284cb47b 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/SummaryNotificationDisplayer.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/SummaryNotificationDisplayer.kt @@ -66,15 +66,12 @@ internal class SummaryNotificationDisplayer( notificationJob: NotificationGenerationJob, notifBuilder: NotificationCompat.Builder?, ): Notification { - // Includes Android 4.3 through 6.0.1. Android 7.1 handles this correctly without this. - // Android 4.2 and older just post the summary only. + // Needed on Android 5.0 through 6.0.1. Android 7.0 handles this correctly without this. val singleNotifWorkArounds = - Build.VERSION.SDK_INT > Build.VERSION_CODES.JELLY_BEAN_MR1 && Build.VERSION.SDK_INT < Build.VERSION_CODES.N && !notificationJob.isRestoring + Build.VERSION.SDK_INT < Build.VERSION_CODES.N && !notificationJob.isRestoring if (singleNotifWorkArounds) { - if ((notificationJob.overriddenSound != null) && - !notificationJob.overriddenSound!! - .equals(notificationJob.orgSound) - ) { + val overriddenSound = notificationJob.overriddenSound + if (overriddenSound != null && overriddenSound != notificationJob.orgSound) { notifBuilder!!.setSound(null) } } From 2fbca1d398dbb238776dc7b7055c971bf4f20808 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 11:43:01 -0700 Subject: [PATCH 05/11] fix: [SDK-5011] only dismiss suppressed notifications on a real shade restore isRestoring meant two different things. NotificationSummaryManager.restoreSummary re-enters generation with isRestoring = true when a group drops to one member, which happens on an ordinary swipe or open. Dismissing on the suppress path therefore took down the surviving sibling the user never touched, for any app whose extension calls preventDefault(true) on a received event. Thread a NotificationRestoreReason through enqueue so the two are distinguishable. Only SHADE_RESTORE records the dismissal; GROUP_REGROUP keeps its previous behavior of leaving the notification and its summary in place. Record that dismissal without cancelling the shade. The restore pass includes notifications that are still showing on API 21 and 22, and whenever getActiveNotifications swallows a throwable and reports an empty shade. Skip the dependent summary rebuild as well. The restore pass already walks every outstanding notification, so rebuilding from here enqueues generation a second time for a sibling that is about to be restored anyway. Co-authored-by: Cursor --- .../detekt/detekt-baseline-notifications.xml | 12 +-- .../impl/NotificationBundleProcessor.kt | 6 +- .../common/NotificationRestoreReason.kt | 22 ++++++ .../internal/data/INotificationRepository.kt | 13 ++++ .../data/impl/NotificationRepository.kt | 27 +++++-- .../display/impl/NotificationDisplayer.kt | 5 +- .../INotificationGenerationProcessor.kt | 3 +- .../INotificationGenerationWorkManager.kt | 3 +- .../impl/NotificationGenerationProcessor.kt | 48 +++++++----- .../impl/NotificationGenerationWorkManager.kt | 27 +++++-- .../INotificationRestoreProcessor.kt | 2 + .../impl/NotificationRestoreProcessor.kt | 10 ++- .../impl/NotificationSummaryManager.kt | 6 +- .../NotificationGenerationProcessorTests.kt | 76 ++++++++++++++----- .../NotificationSummaryManagerTests.kt | 7 +- 15 files changed, 200 insertions(+), 67 deletions(-) create mode 100644 OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt diff --git a/OneSignalSDK/detekt/detekt-baseline-notifications.xml b/OneSignalSDK/detekt/detekt-baseline-notifications.xml index b40dadcd41..38b63c798d 100644 --- a/OneSignalSDK/detekt/detekt-baseline-notifications.xml +++ b/OneSignalSDK/detekt/detekt-baseline-notifications.xml @@ -4,7 +4,7 @@ ComplexCondition:SummaryNotificationDisplayer.kt$SummaryNotificationDisplayer$updateSummary && summaryList.size > 1 || !updateSummary && summaryList.size > 0 ComplexMethod:NotificationChannelManager.kt$NotificationChannelManager$@RequiresApi(api = Build.VERSION_CODES.O) @Throws( JSONException::class, ) private fun createChannel( context: Context, notificationManager: NotificationManager, payload: JSONObject, ): String - ComplexMethod:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$override suspend fun processNotificationData( context: Context, androidNotificationId: Int, jsonPayload: JSONObject, isRestoring: Boolean, timestamp: Long, ) + ComplexMethod:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$override suspend fun processNotificationData( context: Context, androidNotificationId: Int, jsonPayload: JSONObject, restoreReason: NotificationRestoreReason?, timestamp: Long, ) ComplexMethod:SummaryNotificationDisplayer.kt$SummaryNotificationDisplayer$override suspend fun createSummaryNotification( notificationJob: NotificationGenerationJob, notifBuilder: NotificationDisplayBuilder.OneSignalNotificationBuilder?, groupAlertBehavior: Int, ) ConstructorParameterNaming:BadgeCountUpdater.kt$BadgeCountUpdater$private val _applicationService: IApplicationService ConstructorParameterNaming:BadgeCountUpdater.kt$BadgeCountUpdater$private val _databaseProvider: IDatabaseProvider @@ -124,7 +124,7 @@ LongMethod:NotificationDisplayBuilder.kt$NotificationDisplayBuilder$override fun getBaseOneSignalNotificationBuilder(notificationJob: NotificationGenerationJob): OneSignalNotificationBuilder LongMethod:NotificationDisplayer.kt$NotificationDisplayer$@Throws(Throwable::class) private fun addBackgroundImage( fcmJson: JSONObject, notifBuilder: NotificationCompat.Builder?, ) LongMethod:NotificationDisplayer.kt$NotificationDisplayer$private suspend fun showNotification(notificationJob: NotificationGenerationJob): Boolean - LongMethod:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$override suspend fun processNotificationData( context: Context, androidNotificationId: Int, jsonPayload: JSONObject, isRestoring: Boolean, timestamp: Long, ) + LongMethod:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$override suspend fun processNotificationData( context: Context, androidNotificationId: Int, jsonPayload: JSONObject, restoreReason: NotificationRestoreReason?, timestamp: Long, ) LongMethod:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$private suspend fun saveNotification( notificationJob: NotificationGenerationJob, opened: Boolean, ) LongMethod:NotificationLifecycleService.kt$NotificationLifecycleService$override suspend fun notificationOpened( activity: Activity, data: JSONArray, ) LongMethod:NotificationRepository.kt$NotificationRepository$override suspend fun createNotification( id: String, groupId: String?, collapseKey: String?, shouldDismissIdenticals: Boolean, isOpened: Boolean, androidId: Int, title: String?, body: String?, expireTime: Long, jsonPayload: String, ) @@ -134,7 +134,7 @@ LongMethod:NotificationsModule.kt$NotificationsModule$override fun register(builder: ServiceBuilder) LongMethod:SummaryNotificationDisplayer.kt$SummaryNotificationDisplayer$@RequiresApi(api = Build.VERSION_CODES.M) override suspend fun createGrouplessSummaryNotification( notificationJob: NotificationGenerationJob, intentGenerator: IntentGeneratorForAttachingToNotifications, grouplessNotifCount: Int, groupAlertBehavior: Int, ) LongMethod:SummaryNotificationDisplayer.kt$SummaryNotificationDisplayer$override suspend fun createSummaryNotification( notificationJob: NotificationGenerationJob, notifBuilder: NotificationDisplayBuilder.OneSignalNotificationBuilder?, groupAlertBehavior: Int, ) - LongParameterList:INotificationGenerationWorkManager.kt$INotificationGenerationWorkManager$( context: Context, osNotificationId: String, androidNotificationId: Int, jsonPayload: JSONObject?, timestamp: Long, isRestoring: Boolean, isHighPriority: Boolean, ) + LongParameterList:INotificationGenerationWorkManager.kt$INotificationGenerationWorkManager$( context: Context, osNotificationId: String, androidNotificationId: Int, jsonPayload: JSONObject?, timestamp: Long, restoreReason: NotificationRestoreReason?, isHighPriority: Boolean, ) LongParameterList:INotificationRepository.kt$INotificationRepository$( id: String, groupId: String?, collapseKey: String?, shouldDismissIdenticals: Boolean, isOpened: Boolean, androidId: Int, title: String?, body: String?, expireTime: Long, jsonPayload: String, ) LongParameterList:NotificationLifecycleService.kt$NotificationLifecycleService$( private val _applicationService: IApplicationService, private val _time: ITime, 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, ) LoopWithTooManyJumpStatements:NotificationLifecycleService.kt$NotificationLifecycleService$for (i in 0 until data.length()) { val notificationId = NotificationFormatHelper.getOSNotificationIdFromJson(data[i] as JSONObject?) ?: continue if (postedOpenedNotifIds.contains(notificationId)) { continue } postedOpenedNotifIds.add(notificationId) suspendifyWithErrorHandling( useIO = true, // or false for CPU operations block = { confirmNotificationOpened(appId, notificationId, subscriptionId, deviceType) }, onError = { ex -> if (ex is BackendException) { Logging.info("Notification opened confirmation failed with statusCode: ${ex.statusCode} response: ${ex.response}") } else { Logging.info("Unexpected error in notification opened confirmation", ex) } }, ) } @@ -203,10 +203,10 @@ ReturnCount:NotificationDisplayer.kt$NotificationDisplayer$private fun getBitmapFromAssetsOrResourceName(bitmapStr: String): Bitmap? ReturnCount:NotificationDisplayer.kt$NotificationDisplayer$private fun getResourceIcon(iconName: String?): Int ReturnCount:NotificationFormatHelper.kt$NotificationFormatHelper$private fun getOSNotificationIdFromBundle(bundle: Bundle?): String? - ReturnCount:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$override suspend fun processNotificationData( context: Context, androidNotificationId: Int, jsonPayload: JSONObject, isRestoring: Boolean, timestamp: Long, ) + ReturnCount:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$override suspend fun processNotificationData( context: Context, androidNotificationId: Int, jsonPayload: JSONObject, restoreReason: NotificationRestoreReason?, timestamp: Long, ) ReturnCount:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$private fun shouldFireForegroundHandlers(notificationJob: NotificationGenerationJob): Boolean - ReturnCount:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$private suspend fun processHandlerResponse( notificationJob: NotificationGenerationJob, wantsToDisplay: Boolean, isRestoring: Boolean, ): Boolean? - ReturnCount:NotificationGenerationWorkManager.kt$NotificationGenerationWorkManager$override fun beginEnqueueingWork( context: Context, osNotificationId: String, androidNotificationId: Int, jsonPayload: JSONObject?, timestamp: Long, isRestoring: Boolean, isHighPriority: Boolean, ): Boolean + ReturnCount:NotificationGenerationProcessor.kt$NotificationGenerationProcessor$private suspend fun processHandlerResponse( notificationJob: NotificationGenerationJob, wantsToDisplay: Boolean, restoreReason: NotificationRestoreReason?, ): Boolean? + ReturnCount:NotificationGenerationWorkManager.kt$NotificationGenerationWorkManager$override fun beginEnqueueingWork( context: Context, osNotificationId: String, androidNotificationId: Int, jsonPayload: JSONObject?, timestamp: Long, restoreReason: NotificationRestoreReason?, isHighPriority: Boolean, ): Boolean ReturnCount:NotificationGenerationWorkManager.kt$NotificationGenerationWorkManager.NotificationGenerationWorker$override suspend fun doWork(): Result ReturnCount:NotificationHelper.kt$NotificationHelper$fun areNotificationsEnabled( context: Context, channelId: String? = null, ): Boolean ReturnCount:NotificationHelper.kt$NotificationHelper$fun getCampaignNameFromNotification(notification: INotification): String diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/bundle/impl/NotificationBundleProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/bundle/impl/NotificationBundleProcessor.kt index 138f2bc0c7..3eee0e61c7 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/bundle/impl/NotificationBundleProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/bundle/impl/NotificationBundleProcessor.kt @@ -8,6 +8,7 @@ import com.onesignal.notifications.internal.bundle.INotificationBundleProcessor import com.onesignal.notifications.internal.common.NotificationConstants import com.onesignal.notifications.internal.common.NotificationFormatHelper import com.onesignal.notifications.internal.common.NotificationPriorityMapper +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.generation.INotificationGenerationWorkManager import org.json.JSONArray import org.json.JSONException @@ -84,7 +85,8 @@ internal class NotificationBundleProcessor( val jsonPayload = JSONUtils.bundleAsJSONObject(bundle) val timestamp = _time.currentTimeMillis / 1000L - val isRestoring = bundle.getBoolean("is_restoring", false) + val restoreReason = + if (bundle.getBoolean("is_restoring", false)) NotificationRestoreReason.SHADE_RESTORE else null val isHighPriority = NotificationPriorityMapper.isHighPriority(bundle.getString("pri", "0").toInt()) val osNotificationId = NotificationFormatHelper.getOSNotificationIdFromJson(jsonPayload) @@ -103,7 +105,7 @@ internal class NotificationBundleProcessor( androidNotificationId, jsonPayload, timestamp, - isRestoring, + restoreReason, isHighPriority, ) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt new file mode 100644 index 0000000000..d121abc47d --- /dev/null +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt @@ -0,0 +1,22 @@ +package com.onesignal.notifications.internal.common + +/** + * Why OneSignal is sending a notification through the generation pipeline again, having already + * done so once when the notification first arrived. + */ +internal enum class NotificationRestoreReason { + /** + * Android dropped the notification from the shade and OneSignal is showing it again. Happens + * after a reboot, an app update, or a cold start following a force stop. + */ + SHADE_RESTORE, + + /** + * A group dropped to one remaining member, so that member is rebuilt to render on its own + * instead of inside a summary. The notification is still in the shade. + * + * See NotificationSummaryManager. The generation pipeline is what applies the app's extender, + * so the rebuilt notification has to go back through it to keep the app's customizations. + */ + GROUP_REGROUP, +} diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt index f89ce42902..a21ed0aeda 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt @@ -107,6 +107,19 @@ internal interface INotificationRepository { */ suspend fun markAsDismissed(androidId: Int): Boolean + /** + * Mark as dismissed the notification with the Android ID provided, leaving whatever is in the + * notification shade for that ID in place. + * + * Use this when the notification should stop being restored but may still be on screen, such as + * when a restore is suppressed on a device where the SDK cannot tell what is currently showing. + * + * @param androidId The notification's Android ID + * + * @return true if a notification was marked as dismissed, false otherwise. + */ + suspend fun markAsDismissedWithoutCancel(androidId: Int): Boolean + suspend fun markAsDismissedForGroup(group: String) suspend fun markAsDismissedForOutstanding() diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/impl/NotificationRepository.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/impl/NotificationRepository.kt index 056540cac9..7b1dc8c01a 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/impl/NotificationRepository.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/impl/NotificationRepository.kt @@ -121,19 +121,27 @@ internal class NotificationRepository( } } - override suspend fun markAsDismissed(androidId: Int): Boolean { + override suspend fun markAsDismissed(androidId: Int): Boolean = markAsDismissed(androidId, cancelFromShade = true) + + override suspend fun markAsDismissedWithoutCancel(androidId: Int): Boolean = markAsDismissed(androidId, cancelFromShade = false) + + private suspend fun markAsDismissed( + androidId: Int, + cancelFromShade: Boolean, + ): Boolean { var didDismiss: Boolean = false withContext(Dispatchers.IO) { - didDismiss = internalMarkAsDismissed(androidId) + didDismiss = internalMarkAsDismissed(androidId, cancelFromShade) } return didDismiss } - private fun internalMarkAsDismissed(androidId: Int): Boolean { - val appContext = _applicationService.appContext - + private fun internalMarkAsDismissed( + androidId: Int, + cancelFromShade: Boolean, + ): Boolean { val whereStr: String = OneSignalDbContract.NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID.toString() + " = " + androidId + " AND " + OneSignalDbContract.NotificationTable.COLUMN_NAME_OPENED + " = 0 AND " + @@ -146,8 +154,11 @@ internal class NotificationRepository( _badgeCountUpdater.update() - val notificationManager: NotificationManager = NotificationHelper.getNotificationManager(appContext) - notificationManager.cancel(androidId) + if (cancelFromShade) { + val notificationManager: NotificationManager = + NotificationHelper.getNotificationManager(_applicationService.appContext) + notificationManager.cancel(androidId) + } return didDismiss } @@ -424,7 +435,7 @@ internal class NotificationRepository( while (it.moveToNext()) { val existingId = it.getInt(OneSignalDbContract.NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID) - internalMarkAsDismissed(existingId) + internalMarkAsDismissed(existingId, cancelFromShade = true) if (--notificationsToClear <= 0) break } } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt index 07197da7f2..3331b5c8e2 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/display/impl/NotificationDisplayer.kt @@ -112,7 +112,10 @@ internal class NotificationDisplayer( // Keeps notification from playing sound + vibrating again if (notificationJob.isRestoring) { // An extender may have changed the channel, and the channel controls alerting on O+. - oneSignalNotificationBuilder.channelId?.let { notifBuilder?.setChannelId(it) } + oneSignalNotificationBuilder.channelId?.let { + Logging.verbose("Restoring notification $notificationId on channel $it") + notifBuilder?.setChannelId(it) + } _notificationDisplayBuilder.removeNotifyOptions(notifBuilder) } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationProcessor.kt index da04ed8e05..c3d57e0e68 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationProcessor.kt @@ -1,6 +1,7 @@ package com.onesignal.notifications.internal.generation import android.content.Context +import com.onesignal.notifications.internal.common.NotificationRestoreReason import org.json.JSONObject internal interface INotificationGenerationProcessor { @@ -8,7 +9,7 @@ internal interface INotificationGenerationProcessor { context: Context, androidNotificationId: Int, jsonPayload: JSONObject, - isRestoring: Boolean, + restoreReason: NotificationRestoreReason?, timestamp: Long, ) } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationWorkManager.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationWorkManager.kt index 011fd149ec..6ab99df2e9 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationWorkManager.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/INotificationGenerationWorkManager.kt @@ -1,6 +1,7 @@ package com.onesignal.notifications.internal.generation import android.content.Context +import com.onesignal.notifications.internal.common.NotificationRestoreReason import org.json.JSONObject internal interface INotificationGenerationWorkManager { @@ -10,7 +11,7 @@ internal interface INotificationGenerationWorkManager { androidNotificationId: Int, jsonPayload: JSONObject?, timestamp: Long, - isRestoring: Boolean, + restoreReason: NotificationRestoreReason?, isHighPriority: Boolean, ): Boolean } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt index f2e041f067..0a79264884 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt @@ -13,6 +13,7 @@ import com.onesignal.notifications.internal.NotificationReceivedEvent import com.onesignal.notifications.internal.NotificationWillDisplayEvent import com.onesignal.notifications.internal.common.NotificationConstants import com.onesignal.notifications.internal.common.NotificationGenerationJob +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.data.INotificationRepository import com.onesignal.notifications.internal.display.INotificationDisplayer import com.onesignal.notifications.internal.generation.INotificationGenerationProcessor @@ -44,7 +45,7 @@ internal class NotificationGenerationProcessor( context: Context, androidNotificationId: Int, jsonPayload: JSONObject, - isRestoring: Boolean, + restoreReason: NotificationRestoreReason?, timestamp: Long, ) { if (!_lifecycleService.canReceiveNotification(jsonPayload)) { @@ -52,6 +53,7 @@ internal class NotificationGenerationProcessor( return } + val isRestoring = restoreReason != null val notification = Notification(null, jsonPayload, androidNotificationId, _time) // When restoring it will always be seen as a duplicate, because we are restoring... @@ -66,7 +68,9 @@ internal class NotificationGenerationProcessor( var didDisplay = false var wantsToDisplay = true - Logging.info("Fire remoteNotificationReceived") + Logging.info( + "Fire remoteNotificationReceived with androidNotificationId: $androidNotificationId and restoreReason: $restoreReason", + ) try { val notificationReceivedEvent = NotificationReceivedEvent(context, notification, isRestoring) @@ -92,7 +96,7 @@ internal class NotificationGenerationProcessor( } var shouldDisplay = - processHandlerResponse(notificationJob, wantsToDisplay, isRestoring) + processHandlerResponse(notificationJob, wantsToDisplay, restoreReason) ?: return if (shouldDisplay) { @@ -127,7 +131,7 @@ internal class NotificationGenerationProcessor( ) } - shouldDisplay = processHandlerResponse(notificationJob, wantsToDisplay, isRestoring) + shouldDisplay = processHandlerResponse(notificationJob, wantsToDisplay, restoreReason) ?: return } @@ -155,14 +159,14 @@ internal class NotificationGenerationProcessor( * * @param notificationJob The notification job covering the context the handler was called under. * @param wantsToDisplay Whether the SDK (and callback) wants to display the notification. - * @param isRestoring Whether this notification is being processed because of a restore. + * @param restoreReason Why the notification is going through generation again, or null if it just arrived. * * @return true if the job should continue display, false if the job should continue but not display, null if processing should stop. */ private suspend fun processHandlerResponse( notificationJob: NotificationGenerationJob, wantsToDisplay: Boolean, - isRestoring: Boolean, + restoreReason: NotificationRestoreReason?, ): Boolean? { if (wantsToDisplay) { val canDisplay = AndroidUtils.isStringNotEmpty(notificationJob.notification.body) @@ -184,15 +188,24 @@ internal class NotificationGenerationProcessor( // Processing should stop, save the notification as processed to prevent possible duplicate // calls from canonical ids. - if (isRestoring) { - // If we are not displaying a restored notification make sure we mark it as dismissed - // This will prevent it from being restored again - dismissNotification(notificationJob) - } else { - // indicate the notification job did not display. We process it as "opened" to prevent - // a duplicate from coming in and us having to process it again. - notificationJob.isNotificationToDisplay = false - postProcessNotification(notificationJob, true, false) + when (restoreReason) { + // Nothing is in the shade to take down, so record the dismissal without cancelling. + // The restore pass still includes notifications that are showing on API 21 and 22, and + // whenever getActiveNotifications fails, so cancelling here can take down a + // notification the user can see. + NotificationRestoreReason.SHADE_RESTORE -> + _dataController.markAsDismissedWithoutCancel(notificationJob.androidId) + + // The notification is still in the shade and the user never dismissed it. Leave it and + // its summary alone. + NotificationRestoreReason.GROUP_REGROUP -> Unit + + null -> { + // indicate the notification job did not display. We process it as "opened" to prevent + // a duplicate from coming in and us having to process it again. + notificationJob.isNotificationToDisplay = false + postProcessNotification(notificationJob, true, false) + } } return null @@ -296,15 +309,10 @@ internal class NotificationGenerationProcessor( } private suspend fun markNotificationAsDismissed(notifiJob: NotificationGenerationJob) { - // Nothing was posted, so there is nothing to dismiss. if (!notifiJob.isNotificationToDisplay) { return } - dismissNotification(notifiJob) - } - - private suspend fun dismissNotification(notifiJob: NotificationGenerationJob) { Logging.debug("Marking restored or disabled notifications as dismissed: $notifiJob") val didDismiss = _dataController.markAsDismissed(notifiJob.androidId) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt index 0dc570df7e..6f9dfe926d 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt @@ -10,6 +10,7 @@ import com.onesignal.OneSignal import com.onesignal.common.AndroidUtils import com.onesignal.debug.internal.logging.Logging import com.onesignal.notifications.internal.common.NotificationFormatHelper +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.common.OSWorkManagerHelper import com.onesignal.notifications.internal.generation.INotificationGenerationProcessor import com.onesignal.notifications.internal.generation.INotificationGenerationWorkManager @@ -24,7 +25,7 @@ internal class NotificationGenerationWorkManager : INotificationGenerationWorkMa androidNotificationId: Int, jsonPayload: JSONObject?, timestamp: Long, - isRestoring: Boolean, + restoreReason: NotificationRestoreReason?, isHighPriority: Boolean, ): Boolean { val id: String? = NotificationFormatHelper.getOSNotificationIdFromJson(jsonPayload) @@ -46,7 +47,7 @@ internal class NotificationGenerationWorkManager : INotificationGenerationWorkMa .putInt(ANDROID_NOTIF_ID_WORKER_DATA_PARAM, androidNotificationId) .putString(JSON_PAYLOAD_WORKER_DATA_PARAM, jsonPayload.toString()) .putLong(TIMESTAMP_WORKER_DATA_PARAM, timestamp) - .putBoolean(IS_RESTORING_WORKER_DATA_PARAM, isRestoring) + .putString(RESTORE_REASON_WORKER_DATA_PARAM, restoreReason?.name) .build() val workRequest = OneTimeWorkRequest.Builder(NotificationGenerationWorker::class.java) @@ -82,13 +83,11 @@ internal class NotificationGenerationWorkManager : INotificationGenerationWorkMa TIMESTAMP_WORKER_DATA_PARAM, System.currentTimeMillis() / 1000L, ) - val isRestoring = inputData.getBoolean(IS_RESTORING_WORKER_DATA_PARAM, false) - notificationProcessor.processNotificationData( applicationContext, androidNotificationId, jsonPayload, - isRestoring, + readRestoreReason(inputData), timestamp, ) Result.success() @@ -106,10 +105,26 @@ internal class NotificationGenerationWorkManager : INotificationGenerationWorkMa private const val ANDROID_NOTIF_ID_WORKER_DATA_PARAM = "android_notif_id" private const val JSON_PAYLOAD_WORKER_DATA_PARAM = "json_payload" private const val TIMESTAMP_WORKER_DATA_PARAM = "timestamp" - private const val IS_RESTORING_WORKER_DATA_PARAM = "is_restoring" + private const val RESTORE_REASON_WORKER_DATA_PARAM = "restore_reason" + private const val LEGACY_IS_RESTORING_WORKER_DATA_PARAM = "is_restoring" private val notificationIds = ConcurrentHashMap() + private fun readRestoreReason(inputData: Data): NotificationRestoreReason? { + val name = inputData.getString(RESTORE_REASON_WORKER_DATA_PARAM) + if (name != null) { + return NotificationRestoreReason.values().firstOrNull { it.name == name } + } + + // Work enqueued before this key existed only carries the boolean, and the only restore + // it could describe is a shade restore. + return if (inputData.getBoolean(LEGACY_IS_RESTORING_WORKER_DATA_PARAM, false)) { + NotificationRestoreReason.SHADE_RESTORE + } else { + null + } + } + fun addNotificationIdProcessed(osNotificationId: String): Boolean { // Duplicate control // Keep in memory on going processed notifications, to avoid fast duplicates that already finished work process but are not completed yet diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/INotificationRestoreProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/INotificationRestoreProcessor.kt index 73bf7afdf8..bda6bf2e25 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/INotificationRestoreProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/INotificationRestoreProcessor.kt @@ -1,5 +1,6 @@ package com.onesignal.notifications.internal.restoration +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.data.INotificationRepository internal interface INotificationRestoreProcessor { @@ -7,6 +8,7 @@ internal interface INotificationRestoreProcessor { suspend fun processNotification( notification: INotificationRepository.NotificationData, + reason: NotificationRestoreReason, delay: Int = 0, ) } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/impl/NotificationRestoreProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/impl/NotificationRestoreProcessor.kt index bc2ba38c7d..5de66b9503 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/impl/NotificationRestoreProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/restoration/impl/NotificationRestoreProcessor.kt @@ -5,6 +5,7 @@ import com.onesignal.core.internal.application.IApplicationService import com.onesignal.debug.internal.logging.Logging import com.onesignal.notifications.internal.badges.IBadgeCountUpdater import com.onesignal.notifications.internal.common.NotificationHelper +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.data.INotificationRepository import com.onesignal.notifications.internal.generation.INotificationGenerationWorkManager import com.onesignal.notifications.internal.restoration.INotificationRestoreProcessor @@ -25,7 +26,11 @@ internal class NotificationRestoreProcessor( var outstandingNotifications = _dataController.listNotificationsForOutstanding(excludeAndroidIds) for (notification in outstandingNotifications) { - processNotification(notification, DELAY_BETWEEN_NOTIFICATION_RESTORES_MS) + processNotification( + notification, + NotificationRestoreReason.SHADE_RESTORE, + DELAY_BETWEEN_NOTIFICATION_RESTORES_MS, + ) } _badgeCountUpdater.update() @@ -36,6 +41,7 @@ internal class NotificationRestoreProcessor( override suspend fun processNotification( notification: INotificationRepository.NotificationData, + reason: NotificationRestoreReason, delay: Int, ) { _workManager.beginEnqueueingWork( @@ -44,7 +50,7 @@ internal class NotificationRestoreProcessor( notification.androidId, JSONObject(notification.fullData), notification.createdAt, - true, + reason, false, ) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/summary/impl/NotificationSummaryManager.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/summary/impl/NotificationSummaryManager.kt index f5acb7d0d0..c13bfbcbf4 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/summary/impl/NotificationSummaryManager.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/summary/impl/NotificationSummaryManager.kt @@ -5,6 +5,7 @@ import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.time.ITime import com.onesignal.notifications.internal.common.NotificationGenerationJob import com.onesignal.notifications.internal.common.NotificationHelper +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.data.INotificationRepository import com.onesignal.notifications.internal.display.ISummaryNotificationDisplayer import com.onesignal.notifications.internal.restoration.INotificationRestoreProcessor @@ -87,7 +88,10 @@ internal class NotificationSummaryManager( private suspend fun restoreSummary(group: String) { val notifications = _dataController.listNotificationsForGroup(group) for (notification in notifications) - _notificationRestoreProcessor.processNotification(notification) + _notificationRestoreProcessor.processNotification( + notification, + NotificationRestoreReason.GROUP_REGROUP, + ) } /** diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt index 77056049df..c1b6d8dc11 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt @@ -10,6 +10,7 @@ import com.onesignal.mocks.IOMockHelper import com.onesignal.mocks.MockHelper import com.onesignal.notifications.INotificationReceivedEvent import com.onesignal.notifications.INotificationWillDisplayEvent +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.data.INotificationRepository import com.onesignal.notifications.internal.display.INotificationDisplayer import com.onesignal.notifications.internal.generation.impl.NotificationGenerationProcessor @@ -60,6 +61,7 @@ private class Mocks { val mockNotificationRepository = mockk() coEvery { mockNotificationRepository.doesNotificationExist(any()) } returns false coEvery { mockNotificationRepository.markAsDismissed(any()) } returns true + coEvery { mockNotificationRepository.markAsDismissedWithoutCancel(any()) } returns true coEvery { mockNotificationRepository.createNotification( any(), @@ -139,7 +141,7 @@ class NotificationGenerationProcessorTests : FunSpec({ coEvery { mocks.notificationLifecycleService.externalNotificationWillShowInForeground(any()) } just runs // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, null, 1111) // Then coVerify(exactly = 1) { @@ -167,7 +169,7 @@ class NotificationGenerationProcessorTests : FunSpec({ coEvery { mocks.notificationLifecycleService.externalNotificationWillShowInForeground(any()) } just runs // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, true, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, NotificationRestoreReason.SHADE_RESTORE, 1111) // Then coVerify(exactly = 1) { @@ -195,11 +197,13 @@ class NotificationGenerationProcessorTests : FunSpec({ coEvery { mocks.notificationLifecycleService.externalNotificationWillShowInForeground(any()) } just runs // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 2, mocks.notificationPayload, true, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, null, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 2, mocks.notificationPayload, NotificationRestoreReason.SHADE_RESTORE, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 3, mocks.notificationPayload, NotificationRestoreReason.GROUP_REGROUP, 1111) // Then - restoringFlags shouldBe listOf(false, true) + // Both reasons mean the app has seen this notification before, which is what the flag says. + restoringFlags shouldBe listOf(false, true, true) } test("processNotificationData should not display notification when external callback indicates not to") { @@ -211,7 +215,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, null, 1111) // Then // notificationReceived should be called @@ -224,7 +228,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } } - test("processNotificationData should mark a restored notification dismissed when the received event prevents display") { + test("processNotificationData should mark a shade restore dismissed without clearing the shade when the received event prevents display") { // Given val mocks = Mocks() // The suite default of 10ms can expire before Dispatchers.IO runs the callback. @@ -234,7 +238,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, true, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, NotificationRestoreReason.SHADE_RESTORE, 1111) // Then coVerify(exactly = 0) { @@ -242,10 +246,46 @@ class NotificationGenerationProcessorTests : FunSpec({ } // Without this the notification comes back on every later restore. coVerify(exactly = 1) { - mocks.notificationRepository.markAsDismissed(1) + mocks.notificationRepository.markAsDismissedWithoutCancel(1) } - coVerify(exactly = 1) { - mocks.notificationSummaryManager.updatePossibleDependentSummaryOnDismiss(1) + // markAsDismissed cancels the shade. The restore pass includes notifications that are still + // showing on API 21 and 22, and whenever getActiveNotifications fails. + coVerify(exactly = 0) { + mocks.notificationRepository.markAsDismissed(any()) + } + // The restore pass walks every outstanding notification already. Rebuilding the summary from + // here enqueues generation for a sibling that is about to be restored anyway. + coVerify(exactly = 0) { + mocks.notificationSummaryManager.updatePossibleDependentSummaryOnDismiss(any()) + } + } + + test("processNotificationData should leave a regrouped notification alone when the received event prevents display") { + // A group dropping to one member sends that member back through generation. It is still in + // the shade and the user never dismissed it, so suppressing the rebuild must not take it + // down or dismiss the record. + // Given + val mocks = Mocks() + every { mocks.notificationGenerationProcessor getProperty "EXTERNAL_CALLBACKS_TIMEOUT" } answers { 1_000L } + coEvery { mocks.notificationLifecycleService.externalRemoteNotificationReceived(any()) } answers { + firstArg().preventDefault(true) + } + + // When + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, NotificationRestoreReason.GROUP_REGROUP, 1111) + + // Then + coVerify(exactly = 0) { + mocks.notificationDisplayer.displayNotification(any()) + } + coVerify(exactly = 0) { + mocks.notificationRepository.markAsDismissed(any()) + } + coVerify(exactly = 0) { + mocks.notificationRepository.markAsDismissedWithoutCancel(any()) + } + coVerify(exactly = 0) { + mocks.notificationSummaryManager.updatePossibleDependentSummaryOnDismiss(any()) } } @@ -258,7 +298,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, true, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, NotificationRestoreReason.SHADE_RESTORE, 1111) // Then coVerify(exactly = 1) { @@ -285,7 +325,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, null, 1111) // Then // notificationReceived should be called @@ -303,7 +343,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, true, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, NotificationRestoreReason.SHADE_RESTORE, 1111) // Then coVerify(exactly = 1) { @@ -336,7 +376,7 @@ class NotificationGenerationProcessorTests : FunSpec({ // If discard is set to false this should timeout waiting for display() withTimeout(1_000) { - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, null, 1111) } } @@ -352,7 +392,7 @@ class NotificationGenerationProcessorTests : FunSpec({ // If discard is set to false this should timeout waiting for display() withTimeout(1_000) { - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, null, 1111) } } @@ -377,7 +417,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, false, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, null, 1111) // Then coVerify(exactly = 0) { @@ -399,7 +439,7 @@ class NotificationGenerationProcessorTests : FunSpec({ } // When - mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, true, 1111) + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, NotificationRestoreReason.SHADE_RESTORE, 1111) // Then coVerify(exactly = 0) { diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/summary/NotificationSummaryManagerTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/summary/NotificationSummaryManagerTests.kt index 75bf2aa479..990a8e4f31 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/summary/NotificationSummaryManagerTests.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/summary/NotificationSummaryManagerTests.kt @@ -5,6 +5,7 @@ import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.mocks.AndroidMockHelper import com.onesignal.mocks.MockHelper +import com.onesignal.notifications.internal.common.NotificationRestoreReason import com.onesignal.notifications.internal.data.INotificationRepository import com.onesignal.notifications.internal.display.ISummaryNotificationDisplayer import com.onesignal.notifications.internal.restoration.INotificationRestoreProcessor @@ -132,7 +133,7 @@ class NotificationSummaryManagerTests : FunSpec({ coEvery { mockNotificationRepository.getAndroidIdForGroup("groupId", true) } returns 99 val mockSummaryNotificationDisplayer = mockk() val mockNotificationRestoreProcessor = mockk() - coEvery { mockNotificationRestoreProcessor.processNotification(any()) } just runs + coEvery { mockNotificationRestoreProcessor.processNotification(any(), any(), any()) } just runs val notificationSummaryManager = NotificationSummaryManager( @@ -160,6 +161,10 @@ class NotificationSummaryManagerTests : FunSpec({ it.title shouldBe "title2" it.message shouldBe "message2" }, + // Not a shade restore. The notification is still showing, so suppressing this + // rebuild must not dismiss or cancel it. + NotificationRestoreReason.GROUP_REGROUP, + any(), ) } } From b7a87996da2fa6cbfe8fa2e89c456d578cb4845e Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 11:43:10 -0700 Subject: [PATCH 06/11] docs: [SDK-5011] describe both restore triggers on the restoring flag The previous wording said restoring only meant Android cleared the shade, which is not what the SDK reports. It is also true when a group collapses to a single notification that has to be rebuilt. Point restore suppression at preventDefault(true). The no-argument overload parks on the display waiter for the full 30 second timeout, which is the wrong tool for dropping a restore and would hold a worker per notification. Warn that setExtender still has to run when restoring is true, since the rebuilt notification is what the app's customizations get applied to. Give the property a default so implementations outside this SDK keep compiling. Co-authored-by: Cursor --- .../INotificationReceivedEvent.kt | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt index 6b1d9b9993..478680b5d6 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt @@ -46,14 +46,23 @@ interface INotificationReceivedEvent { val notification: IDisplayableMutableNotification /** - * Whether OneSignal is showing this notification again because Android cleared it from the - * notification shade, such as after a reboot or an app update. Your app already received - * this notification once before. + * Whether your app already received this notification and OneSignal is handing it back. The + * usual cause is Android clearing the notification shade, such as after a reboot or an app + * update. It also happens when a group of notifications drops to a single one, which OneSignal + * rebuilds so it no longer renders inside a summary. * - * Use it to skip work that should only run the first time, or call [preventDefault] to stop - * the notification from showing again. + * Use it to skip work that should only happen once, such as counting the notification in your + * analytics. Keep calling `notification.setExtender(...)` even when this is true, otherwise a + * rebuilt notification loses your customizations. + * + * To drop the notification instead of showing it again, call `preventDefault(true)`. That marks + * it dismissed so OneSignal stops restoring it. The no-argument [preventDefault] is for the + * asynchronous `notification.display()` flow and waits up to 30 seconds before giving up. + * + * Defaulted rather than abstract so that existing implementations outside this SDK still compile. */ val restoring: Boolean + get() = false /** * Call this to prevent OneSignal from displaying the notification automatically. The notification From dd95cb8efbc69ed291ea302aba6adabb751df0e2 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 11:44:14 -0700 Subject: [PATCH 07/11] test: [SDK-5011] cover the no-argument preventDefault on a shade restore An app that calls preventDefault() and never calls display() reaches the same suppress path, just after the callback timeout rather than immediately. Pin that it dismisses the record without cancelling the shade. Co-authored-by: Cursor --- .../NotificationGenerationProcessorTests.kt | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt index c1b6d8dc11..bb8ac57aff 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationProcessorTests.kt @@ -260,6 +260,31 @@ class NotificationGenerationProcessorTests : FunSpec({ } } + test("processNotificationData should mark a shade restore dismissed when the received event never calls display") { + // The no-argument preventDefault parks on the display waiter, so this only settles once the + // callback timeout fires. The outcome has to match preventDefault(true). + // Given + val mocks = Mocks() + every { mocks.notificationGenerationProcessor getProperty "EXTERNAL_CALLBACKS_TIMEOUT" } answers { 200L } + coEvery { mocks.notificationLifecycleService.externalRemoteNotificationReceived(any()) } answers { + firstArg().preventDefault() + } + + // When + mocks.notificationGenerationProcessor.processNotificationData(mocks.context, 1, mocks.notificationPayload, NotificationRestoreReason.SHADE_RESTORE, 1111) + + // Then + coVerify(exactly = 0) { + mocks.notificationDisplayer.displayNotification(any()) + } + coVerify(exactly = 1) { + mocks.notificationRepository.markAsDismissedWithoutCancel(1) + } + coVerify(exactly = 0) { + mocks.notificationRepository.markAsDismissed(any()) + } + } + test("processNotificationData should leave a regrouped notification alone when the received event prevents display") { // A group dropping to one member sends that member back through generation. It is still in // the shade and the user never dismissed it, so suppressing the rebuild must not take it From 4673cff62115e2410848e54532655a1f57684d20 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 12:08:38 -0700 Subject: [PATCH 08/11] docs: [SDK-5011] trim restore-reason comments Drop the source-compat note and the API-level walkthroughs. Keep when restoring is true, which preventDefault to call, and why regroup must not cancel the shade. Co-authored-by: Cursor --- .../INotificationReceivedEvent.kt | 19 ++++++------------- .../common/NotificationRestoreReason.kt | 17 ++++------------- .../internal/data/INotificationRepository.kt | 6 +----- .../impl/NotificationGenerationProcessor.kt | 10 +++------- .../impl/NotificationGenerationWorkManager.kt | 3 +-- 5 files changed, 15 insertions(+), 40 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt index 478680b5d6..1634b8102e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt @@ -46,20 +46,13 @@ interface INotificationReceivedEvent { val notification: IDisplayableMutableNotification /** - * Whether your app already received this notification and OneSignal is handing it back. The - * usual cause is Android clearing the notification shade, such as after a reboot or an app - * update. It also happens when a group of notifications drops to a single one, which OneSignal - * rebuilds so it no longer renders inside a summary. + * True when OneSignal is showing this notification again after your app already received it. + * Happens after a reboot or app update, and when a group collapses to one notification. * - * Use it to skip work that should only happen once, such as counting the notification in your - * analytics. Keep calling `notification.setExtender(...)` even when this is true, otherwise a - * rebuilt notification loses your customizations. - * - * To drop the notification instead of showing it again, call `preventDefault(true)`. That marks - * it dismissed so OneSignal stops restoring it. The no-argument [preventDefault] is for the - * asynchronous `notification.display()` flow and waits up to 30 seconds before giving up. - * - * Defaulted rather than abstract so that existing implementations outside this SDK still compile. + * Skip one-time work like analytics. Still call `notification.setExtender(...)` so a rebuilt + * notification keeps your customizations. + * Call `preventDefault(true)` to drop it. The no-argument [preventDefault] waits up to 30 + * seconds for `notification.display()`. */ val restoring: Boolean get() = false diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt index d121abc47d..3c9fa583bd 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt @@ -1,22 +1,13 @@ package com.onesignal.notifications.internal.common -/** - * Why OneSignal is sending a notification through the generation pipeline again, having already - * done so once when the notification first arrived. - */ +/** Why a notification is being shown again. */ internal enum class NotificationRestoreReason { - /** - * Android dropped the notification from the shade and OneSignal is showing it again. Happens - * after a reboot, an app update, or a cold start following a force stop. - */ + /** Android cleared the shade, such as after a reboot, app update, or force-stop. */ SHADE_RESTORE, /** - * A group dropped to one remaining member, so that member is rebuilt to render on its own - * instead of inside a summary. The notification is still in the shade. - * - * See NotificationSummaryManager. The generation pipeline is what applies the app's extender, - * so the rebuilt notification has to go back through it to keep the app's customizations. + * A group dropped to one member. Shown as a standalone notification, still in the shade, + * so the app's extender still applies. */ GROUP_REGROUP, } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt index a21ed0aeda..1866948118 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/data/INotificationRepository.kt @@ -108,11 +108,7 @@ internal interface INotificationRepository { suspend fun markAsDismissed(androidId: Int): Boolean /** - * Mark as dismissed the notification with the Android ID provided, leaving whatever is in the - * notification shade for that ID in place. - * - * Use this when the notification should stop being restored but may still be on screen, such as - * when a restore is suppressed on a device where the SDK cannot tell what is currently showing. + * Mark as dismissed without cancelling the notification from the shade. * * @param androidId The notification's Android ID * diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt index 0a79264884..95052eae52 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt @@ -159,7 +159,7 @@ internal class NotificationGenerationProcessor( * * @param notificationJob The notification job covering the context the handler was called under. * @param wantsToDisplay Whether the SDK (and callback) wants to display the notification. - * @param restoreReason Why the notification is going through generation again, or null if it just arrived. + * @param restoreReason Why the notification is being shown again, or null if it just arrived. * * @return true if the job should continue display, false if the job should continue but not display, null if processing should stop. */ @@ -189,15 +189,11 @@ internal class NotificationGenerationProcessor( // Processing should stop, save the notification as processed to prevent possible duplicate // calls from canonical ids. when (restoreReason) { - // Nothing is in the shade to take down, so record the dismissal without cancelling. - // The restore pass still includes notifications that are showing on API 21 and 22, and - // whenever getActiveNotifications fails, so cancelling here can take down a - // notification the user can see. + // Restore can include notifications still on screen. Don't cancel them. NotificationRestoreReason.SHADE_RESTORE -> _dataController.markAsDismissedWithoutCancel(notificationJob.androidId) - // The notification is still in the shade and the user never dismissed it. Leave it and - // its summary alone. + // Still in the shade. The user did not dismiss it. NotificationRestoreReason.GROUP_REGROUP -> Unit null -> { diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt index 6f9dfe926d..eed05fe3f1 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt @@ -116,8 +116,7 @@ internal class NotificationGenerationWorkManager : INotificationGenerationWorkMa return NotificationRestoreReason.values().firstOrNull { it.name == name } } - // Work enqueued before this key existed only carries the boolean, and the only restore - // it could describe is a shade restore. + // Older work only has the boolean, which always meant shade restore. return if (inputData.getBoolean(LEGACY_IS_RESTORING_WORKER_DATA_PARAM, false)) { NotificationRestoreReason.SHADE_RESTORE } else { From 905c1af39c08887e8a064e4b59518085084ff640 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 13:44:02 -0700 Subject: [PATCH 09/11] docs: [SDK-5011] preventDefault(true) only drops a shade restore A group-collapse rebuild is left in place. Saying "drop it" for every restoring event would surprise an app that still sees the leftover child and summary. Co-authored-by: Cursor --- .../onesignal/notifications/INotificationReceivedEvent.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt index 1634b8102e..f4f7ea47cb 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt @@ -51,8 +51,9 @@ interface INotificationReceivedEvent { * * Skip one-time work like analytics. Still call `notification.setExtender(...)` so a rebuilt * notification keeps your customizations. - * Call `preventDefault(true)` to drop it. The no-argument [preventDefault] waits up to 30 - * seconds for `notification.display()`. + * Call `preventDefault(true)` to stop a shade restore from coming back. A group-collapse + * rebuild is left in place. The no-argument [preventDefault] waits up to 30 seconds for + * `notification.display()`. */ val restoring: Boolean get() = false From 11e6240e679f0ecd38bf2e4c8853adcefc285cce Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 13:44:02 -0700 Subject: [PATCH 10/11] test: [SDK-5011] prove skip-cancel and restore-reason round-trip Processor tests mock the repository, so they never showed that markAsDismissedWithoutCancel leaves the shade. readRestoreReason was untested for the enum, the legacy boolean, and an unknown name. Co-authored-by: Cursor --- .../impl/NotificationGenerationWorkManager.kt | 2 +- .../data/NotificationRepositoryTests.kt | 53 +++++++++++++++++++ .../NotificationGenerationWorkManagerTests.kt | 51 ++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/data/NotificationRepositoryTests.kt create mode 100644 OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationWorkManagerTests.kt diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt index eed05fe3f1..55c19dc5a6 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationWorkManager.kt @@ -110,7 +110,7 @@ internal class NotificationGenerationWorkManager : INotificationGenerationWorkMa private val notificationIds = ConcurrentHashMap() - private fun readRestoreReason(inputData: Data): NotificationRestoreReason? { + internal fun readRestoreReason(inputData: Data): NotificationRestoreReason? { val name = inputData.getString(RESTORE_REASON_WORKER_DATA_PARAM) if (name != null) { return NotificationRestoreReason.values().firstOrNull { it.name == name } diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/data/NotificationRepositoryTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/data/NotificationRepositoryTests.kt new file mode 100644 index 0000000000..c198085cf1 --- /dev/null +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/data/NotificationRepositoryTests.kt @@ -0,0 +1,53 @@ +package com.onesignal.notifications.internal.data + +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.mocks.AndroidMockHelper +import com.onesignal.mocks.DatabaseMockHelper +import com.onesignal.mocks.MockHelper +import com.onesignal.notifications.internal.badges.IBadgeCountUpdater +import com.onesignal.notifications.internal.data.impl.NotificationRepository +import com.onesignal.notifications.shadows.ShadowRoboNotificationManager +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.mockk +import org.robolectric.annotation.Config + +@Config( + packageName = "com.onesignal.example", + shadows = [ShadowRoboNotificationManager::class], + sdk = [26], +) +@RobolectricTest +class NotificationRepositoryTests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + ShadowRoboNotificationManager.reset() + } + + fun repository(): NotificationRepository { + val database = DatabaseMockHelper.databaseProvider("notification") + every { database.second.update(any(), any(), any(), any()) } returns 1 + return NotificationRepository( + AndroidMockHelper.applicationService(), + mockk(relaxed = true), + database.first, + MockHelper.time(1111), + mockk(relaxed = true), + ) + } + + test("markAsDismissedWithoutCancel should not cancel the notification from the shade") { + repository().markAsDismissedWithoutCancel(7) + + ShadowRoboNotificationManager.cancelledNotifications shouldBe emptyList() + } + + test("markAsDismissed should cancel the notification from the shade") { + repository().markAsDismissed(7) + + ShadowRoboNotificationManager.cancelledNotifications shouldBe listOf(7) + } +}) diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationWorkManagerTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationWorkManagerTests.kt new file mode 100644 index 0000000000..cb69677e5e --- /dev/null +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/generation/NotificationGenerationWorkManagerTests.kt @@ -0,0 +1,51 @@ +package com.onesignal.notifications.internal.generation + +import androidx.work.Data +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.notifications.internal.common.NotificationRestoreReason +import com.onesignal.notifications.internal.generation.impl.NotificationGenerationWorkManager +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe + +class NotificationGenerationWorkManagerTests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + test("readRestoreReason returns the enum written by enqueue") { + val data = + Data.Builder() + .putString("restore_reason", NotificationRestoreReason.GROUP_REGROUP.name) + .build() + + NotificationGenerationWorkManager.readRestoreReason(data) shouldBe NotificationRestoreReason.GROUP_REGROUP + } + + test("readRestoreReason treats legacy is_restoring true as a shade restore") { + val data = + Data.Builder() + .putBoolean("is_restoring", true) + .build() + + NotificationGenerationWorkManager.readRestoreReason(data) shouldBe NotificationRestoreReason.SHADE_RESTORE + } + + test("readRestoreReason treats legacy is_restoring false as a new notification") { + val data = + Data.Builder() + .putBoolean("is_restoring", false) + .build() + + NotificationGenerationWorkManager.readRestoreReason(data) shouldBe null + } + + test("readRestoreReason ignores an unknown enum name") { + val data = + Data.Builder() + .putString("restore_reason", "NOT_A_REASON") + .build() + + NotificationGenerationWorkManager.readRestoreReason(data) shouldBe null + } +}) From 80589531c2b9f225067d99e8cf47f3c768997b5b Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 21 Aug 2026 14:50:23 -0700 Subject: [PATCH 11/11] chore: re-run CI so Skip Coverage Check is visible