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/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationReceivedEvent.kt index f415bbf7aa..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 @@ -45,6 +45,19 @@ interface INotificationReceivedEvent { */ val notification: IDisplayableMutableNotification + /** + * 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. + * + * Skip one-time work like analytics. Still call `notification.setExtender(...)` so a rebuilt + * notification keeps your customizations. + * 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 + /** * 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/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/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/common/NotificationRestoreReason.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt new file mode 100644 index 0000000000..3c9fa583bd --- /dev/null +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationRestoreReason.kt @@ -0,0 +1,13 @@ +package com.onesignal.notifications.internal.common + +/** Why a notification is being shown again. */ +internal enum class NotificationRestoreReason { + /** Android cleared the shade, such as after a reboot, app update, or force-stop. */ + SHADE_RESTORE, + + /** + * 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 f89ce42902..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 @@ -107,6 +107,15 @@ internal interface INotificationRepository { */ suspend fun markAsDismissed(androidId: Int): Boolean + /** + * Mark as dismissed without cancelling the notification from the shade. + * + * @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/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..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 @@ -111,6 +111,11 @@ 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 { + Logging.verbose("Restoring notification $notificationId on channel $it") + notifBuilder?.setChannelId(it) + } _notificationDisplayBuilder.removeNotifyOptions(notifBuilder) } @@ -200,7 +205,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) } } 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 5cdd5ef6f7..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 @@ -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,10 +68,12 @@ 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) + val notificationReceivedEvent = NotificationReceivedEvent(context, notification, isRestoring) withTimeout(EXTERNAL_CALLBACKS_TIMEOUT) { launchOnIO { _lifecycleService.externalRemoteNotificationReceived(notificationReceivedEvent) @@ -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 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. */ 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,20 @@ 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 - markNotificationAsDismissed(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) { + // Restore can include notifications still on screen. Don't cancel them. + NotificationRestoreReason.SHADE_RESTORE -> + _dataController.markAsDismissedWithoutCancel(notificationJob.androidId) + + // Still in the shade. The user did not dismiss it. + 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 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..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 @@ -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,25 @@ 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() + 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 } + } + + // 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 { + 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/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/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 + } +}) 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..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 @@ -10,10 +10,12 @@ 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 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 +60,8 @@ private class Mocks { run { 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(), @@ -75,6 +79,8 @@ private class Mocks { mockNotificationRepository } + val notificationSummaryManager = mockk(relaxed = true) + val notificationGenerationProcessor = run { val mock = spyk( NotificationGenerationProcessor( @@ -82,7 +88,7 @@ private class Mocks { notificationDisplayer, MockHelper.configModelStore(), notificationRepository, - mockk(), + notificationSummaryManager, notificationLifecycleService, MockHelper.time(1111), ), recordPrivateCalls = true @@ -135,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) { @@ -163,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) { @@ -180,6 +186,26 @@ 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, 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 + // 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") { // Given val mocks = Mocks() @@ -189,13 +215,103 @@ 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 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 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. + 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.SHADE_RESTORE, 1111) + + // Then + coVerify(exactly = 0) { + mocks.notificationDisplayer.displayNotification(any()) + } + // Without this the notification comes back on every later restore. + coVerify(exactly = 1) { + mocks.notificationRepository.markAsDismissedWithoutCancel(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 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 + // 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()) + } } test("processNotificationData should display notification when external callback takes longer than 30 seconds") { @@ -207,7 +323,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) { @@ -234,7 +350,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 @@ -252,7 +368,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 +401,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) } } @@ -301,7 +417,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) } } @@ -326,7 +442,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) { @@ -348,7 +464,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/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 + } +}) 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(), ) } }