diff --git a/OneSignalSDK/detekt/detekt-baseline-notifications.xml b/OneSignalSDK/detekt/detekt-baseline-notifications.xml index 7cf8ee4ff4..b7eee741d0 100644 --- a/OneSignalSDK/detekt/detekt-baseline-notifications.xml +++ b/OneSignalSDK/detekt/detekt-baseline-notifications.xml @@ -45,6 +45,7 @@ ConstructorParameterNaming:NotificationLifecycleService.kt$NotificationLifecycleService$private val _backend: INotificationBackendService ConstructorParameterNaming:NotificationLifecycleService.kt$NotificationLifecycleService$private val _configModelStore: ConfigModelStore ConstructorParameterNaming:NotificationLifecycleService.kt$NotificationLifecycleService$private val _deviceService: IDeviceService + ConstructorParameterNaming:NotificationLifecycleService.kt$NotificationLifecycleService$private val _identityModelStore: IdentityModelStore ConstructorParameterNaming:NotificationLifecycleService.kt$NotificationLifecycleService$private val _influenceManager: IInfluenceManager ConstructorParameterNaming:NotificationLifecycleService.kt$NotificationLifecycleService$private val _receiveReceiptWorkManager: IReceiveReceiptWorkManager ConstructorParameterNaming:NotificationLifecycleService.kt$NotificationLifecycleService$private val _subscriptionManager: ISubscriptionManager @@ -99,6 +100,7 @@ ConstructorParameterNaming:ReceiveReceiptProcessor.kt$ReceiveReceiptProcessor$private val _deviceService: IDeviceService ConstructorParameterNaming:ReceiveReceiptWorkManager.kt$ReceiveReceiptWorkManager$private val _applicationService: IApplicationService ConstructorParameterNaming:ReceiveReceiptWorkManager.kt$ReceiveReceiptWorkManager$private val _configModelStore: ConfigModelStore + ConstructorParameterNaming:ReceiveReceiptWorkManager.kt$ReceiveReceiptWorkManager$private val _identityModelStore: IdentityModelStore ConstructorParameterNaming:ReceiveReceiptWorkManager.kt$ReceiveReceiptWorkManager$private val _subscriptionManager: ISubscriptionManager ConstructorParameterNaming:SummaryNotificationDisplayer.kt$SummaryNotificationDisplayer$private val _applicationService: IApplicationService ConstructorParameterNaming:SummaryNotificationDisplayer.kt$SummaryNotificationDisplayer$private val _dataController: INotificationRepository @@ -136,8 +138,8 @@ 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, 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) } }, ) } + 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 _identityModelStore: IdentityModelStore, 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, onesignalId) }, 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) } }, ) } MagicNumber:FirebaseAnalyticsTracker.kt$FirebaseAnalyticsTracker$1000 MagicNumber:FirebaseAnalyticsTracker.kt$FirebaseAnalyticsTracker$30 MagicNumber:FirebaseAnalyticsTracker.kt$FirebaseAnalyticsTracker$60 diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/identity/IdentityModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/identity/IdentityModelStore.kt index 6e9d535611..aa2f79a7b1 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/identity/IdentityModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/identity/IdentityModelStore.kt @@ -1,5 +1,6 @@ package com.onesignal.user.internal.identity +import com.onesignal.common.IDManager import com.onesignal.common.modeling.SimpleModelStore import com.onesignal.common.modeling.SingletonModelStore import com.onesignal.core.internal.preferences.IPreferencesService @@ -16,3 +17,13 @@ open class IdentityModelStore(prefs: IPreferencesService) : SingletonModelStore< * Used to determine if a user is already initialized or needs to be created. */ fun IdentityModelStore.hasOneSignalId(): Boolean = model.hasProperty(IdentityConstants.ONESIGNAL_ID) + +/** + * The OneSignal ID the backend assigned to the current user, or null while the user only + * has a local placeholder ID (before the create user response) or no ID at all. + */ +fun IdentityModelStore.backendOneSignalIdOrNull(): String? { + if (!hasOneSignalId()) return null + val onesignalId = model.onesignalId + return if (onesignalId.isEmpty() || IDManager.isLocalId(onesignalId)) null else onesignalId +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/identity/IdentityModelStoreTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/identity/IdentityModelStoreTests.kt new file mode 100644 index 0000000000..e98375b4a1 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/identity/IdentityModelStoreTests.kt @@ -0,0 +1,47 @@ +package com.onesignal.user.internal.identity + +import com.onesignal.common.IDManager +import com.onesignal.mocks.MockHelper +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldBeNull +import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.mockk + +class IdentityModelStoreTests : FunSpec({ + test("backendOneSignalIdOrNull returns the id once the backend has assigned one") { + // Given + val store = MockHelper.identityModelStore { it.onesignalId = "onesignalId" } + + // When / Then + store.backendOneSignalIdOrNull() shouldBe "onesignalId" + } + + test("backendOneSignalIdOrNull is null while the user only has a local placeholder id") { + // Given: first launch, login() and logout() all assign a local id that the create user + // response later replaces. Requests that name the user by onesignal_id must not send it, + // since the server would look up a user it has never seen. + val store = MockHelper.identityModelStore { it.onesignalId = IDManager.createLocalId() } + + // When / Then + store.backendOneSignalIdOrNull().shouldBeNull() + } + + test("backendOneSignalIdOrNull is null when the id is empty") { + // Given + val store = MockHelper.identityModelStore { it.onesignalId = "" } + + // When / Then + store.backendOneSignalIdOrNull().shouldBeNull() + } + + test("backendOneSignalIdOrNull is null when the identity model has no id at all") { + // Given: the singleton store hands out an empty model before the first user is created, + // and reading onesignalId on it throws, so the helper has to check for the property first. + val store = mockk(relaxed = true) + every { store.model } returns IdentityModel() + + // When / Then + store.backendOneSignalIdOrNull().shouldBeNull() + } +}) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/INotificationBackendService.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/INotificationBackendService.kt index 6ed0d936e2..d54ffbb020 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/INotificationBackendService.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/INotificationBackendService.kt @@ -16,12 +16,16 @@ internal interface INotificationBackendService { * @param notificationId The ID of the notification within the [appId] that has been received. * @param subscriptionId The specific subscription within the [appId] the notification has been received for. * @param deviceType The type of device the notification was received at. + * @param onesignalId The backend-assigned OneSignal ID of the user the subscription belonged to + * when the notification was received, or null when the SDK did not have one. Omitted from the + * request when null. */ suspend fun updateNotificationAsReceived( appId: String, notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType, + onesignalId: String?, ) /** @@ -33,11 +37,14 @@ internal interface INotificationBackendService { * @param notificationId The ID of the notification within the [appId] that has been received. * @param subscriptionId The specific subscription within the [appId] the notification has been received for. * @param deviceType The type of device the notification was received at. + * @param onesignalId The backend-assigned OneSignal ID of the user the subscription belongs to, + * or null when the SDK does not have one. Omitted from the request when null. */ suspend fun updateNotificationAsOpened( appId: String, notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType, + onesignalId: String?, ) } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/impl/NotificationBackendService.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/impl/NotificationBackendService.kt index 6ac5165a1c..76c3fed21f 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/impl/NotificationBackendService.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/backend/impl/NotificationBackendService.kt @@ -14,6 +14,7 @@ internal class NotificationBackendService( notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType, + onesignalId: String?, ) { val jsonBody: JSONObject = JSONObject() @@ -21,6 +22,10 @@ internal class NotificationBackendService( .put("player_id", subscriptionId) .put("device_type", deviceType.value) + if (onesignalId != null) { + jsonBody.put("onesignal_id", onesignalId) + } + var response = _httpClient.put("notifications/$notificationId/report_received", jsonBody) if (!response.isSuccess) { @@ -33,12 +38,16 @@ internal class NotificationBackendService( notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType, + onesignalId: String?, ) { val jsonBody = JSONObject() jsonBody.put("app_id", appId) jsonBody.put("player_id", subscriptionId) jsonBody.put("opened", true) jsonBody.put("device_type", deviceType.value) + if (onesignalId != null) { + jsonBody.put("onesignal_id", onesignalId) + } var response = _httpClient.put("notifications/$notificationId", jsonBody) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/lifecycle/impl/NotificationLifecycleService.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/lifecycle/impl/NotificationLifecycleService.kt index 2e0b8aab5e..c977102034 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/lifecycle/impl/NotificationLifecycleService.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/lifecycle/impl/NotificationLifecycleService.kt @@ -35,6 +35,8 @@ import com.onesignal.notifications.internal.lifecycle.INotificationLifecycleCall import com.onesignal.notifications.internal.lifecycle.INotificationLifecycleService import com.onesignal.notifications.internal.receivereceipt.IReceiveReceiptWorkManager import com.onesignal.session.internal.influence.IInfluenceManager +import com.onesignal.user.internal.identity.IdentityModelStore +import com.onesignal.user.internal.identity.backendOneSignalIdOrNull import com.onesignal.user.internal.subscriptions.ISubscriptionManager import kotlinx.coroutines.delay import org.json.JSONArray @@ -47,6 +49,7 @@ internal class NotificationLifecycleService( private val _configModelStore: ConfigModelStore, private val _influenceManager: IInfluenceManager, private val _subscriptionManager: ISubscriptionManager, + private val _identityModelStore: IdentityModelStore, private val _deviceService: IDeviceService, private val _backend: INotificationBackendService, private val _receiveReceiptWorkManager: IReceiveReceiptWorkManager, @@ -135,6 +138,7 @@ internal class NotificationLifecycleService( val config = _configModelStore.model val appId: String = config.appId ?: "" val subscriptionId: String = _subscriptionManager.subscriptions.push.id + val onesignalId = _identityModelStore.backendOneSignalIdOrNull() val deviceType = _deviceService.deviceType for (i in 0 until data.length()) { @@ -150,7 +154,7 @@ internal class NotificationLifecycleService( useIO = true, // or false for CPU operations block = { - confirmNotificationOpened(appId, notificationId, subscriptionId, deviceType) + confirmNotificationOpened(appId, notificationId, subscriptionId, deviceType, onesignalId) }, onError = { ex -> if (ex is BackendException) { @@ -204,10 +208,11 @@ internal class NotificationLifecycleService( notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType, + onesignalId: String?, ) { for (attempt in 1..NetworkUtils.maxNetworkRequestAttemptCount) { try { - _backend.updateNotificationAsOpened(appId, notificationId, subscriptionId, deviceType) + _backend.updateNotificationAsOpened(appId, notificationId, subscriptionId, deviceType, onesignalId) return } catch (ex: BackendException) { val responseType = NetworkUtils.getResponseStatusType(ex.statusCode) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/IReceiveReceiptProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/IReceiveReceiptProcessor.kt index 77cb6cec38..5c111cfd58 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/IReceiveReceiptProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/IReceiveReceiptProcessor.kt @@ -12,10 +12,13 @@ internal interface IReceiveReceiptProcessor { * @param appId The id of the application the notification was received under. * @param subscriptionId The id of the subscription the notification was received under. * @param notificationId The id of the notification that has been received. + * @param onesignalId The backend-assigned OneSignal id of the user the subscription belonged to + * when the notification was received, or null when the SDK did not have one. */ suspend fun sendReceiveReceipt( appId: String, subscriptionId: String, notificationId: String, + onesignalId: String?, ) } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptProcessor.kt index e15937d1d3..1c77b4c492 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptProcessor.kt @@ -14,11 +14,12 @@ internal class ReceiveReceiptProcessor( appId: String, subscriptionId: String, notificationId: String, + onesignalId: String?, ) { val deviceType = _deviceService.deviceType try { - _backend.updateNotificationAsReceived(appId, notificationId, subscriptionId, deviceType) + _backend.updateNotificationAsReceived(appId, notificationId, subscriptionId, deviceType, onesignalId) } catch (ex: BackendException) { Logging.info("Receive receipt failed with statusCode: ${ex.statusCode} response: ${ex.response}") } diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptWorkManager.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptWorkManager.kt index a888205f7f..e25fc5dfd2 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptWorkManager.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/receivereceipt/impl/ReceiveReceiptWorkManager.kt @@ -16,6 +16,8 @@ import com.onesignal.debug.internal.logging.Logging import com.onesignal.notifications.internal.common.OSWorkManagerHelper import com.onesignal.notifications.internal.receivereceipt.IReceiveReceiptProcessor import com.onesignal.notifications.internal.receivereceipt.IReceiveReceiptWorkManager +import com.onesignal.user.internal.identity.IdentityModelStore +import com.onesignal.user.internal.identity.backendOneSignalIdOrNull import com.onesignal.user.internal.subscriptions.ISubscriptionManager import java.util.concurrent.TimeUnit @@ -23,6 +25,7 @@ internal class ReceiveReceiptWorkManager( private val _applicationService: IApplicationService, private val _configModelStore: ConfigModelStore, private val _subscriptionManager: ISubscriptionManager, + private val _identityModelStore: IdentityModelStore, ) : IReceiveReceiptWorkManager { private val minDelay = 0 private val maxDelay = 25 @@ -35,6 +38,10 @@ internal class ReceiveReceiptWorkManager( val appId: String = _configModelStore.model.appId val subscriptionId = _subscriptionManager.subscriptions.push.id + // Captured now, with the subscription, so the receipt names the user who held the + // subscription when the notification arrived rather than whoever is logged in when the + // delayed worker runs. + val onesignalId = _identityModelStore.backendOneSignalIdOrNull() if (subscriptionId.isEmpty() || appId.isEmpty()) { Logging.debug("ReceiveReceiptWorkManager: No push subscription or appId!") @@ -46,6 +53,7 @@ internal class ReceiveReceiptWorkManager( .putString(OS_NOTIFICATION_ID, notificationId) .putString(OS_APP_ID, appId) .putString(OS_SUBSCRIPTION_ID, subscriptionId) + .putString(OS_ONESIGNAL_ID, onesignalId) .build() val constraints = buildConstraints() val workRequest = @@ -81,9 +89,10 @@ internal class ReceiveReceiptWorkManager( val notificationId = inputData.getString(OS_NOTIFICATION_ID) ?: return Result.failure() val appId = inputData.getString(OS_APP_ID) ?: return Result.failure() val subscriptionId = inputData.getString(OS_SUBSCRIPTION_ID) ?: return Result.failure() + val onesignalId = inputData.getString(OS_ONESIGNAL_ID) val receiveReceiptProcessor = OneSignal.getService() - receiveReceiptProcessor.sendReceiveReceipt(appId, subscriptionId, notificationId) + receiveReceiptProcessor.sendReceiveReceipt(appId, subscriptionId, notificationId, onesignalId) return Result.success() } } @@ -92,5 +101,6 @@ internal class ReceiveReceiptWorkManager( private const val OS_NOTIFICATION_ID = "os_notification_id" private const val OS_APP_ID = "os_app_id" private const val OS_SUBSCRIPTION_ID = "os_subscription_id" + private const val OS_ONESIGNAL_ID = "os_onesignal_id" } } diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/backend/NotificationBackendServiceTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/backend/NotificationBackendServiceTests.kt index e1c7cba1ce..1528c54307 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/backend/NotificationBackendServiceTests.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/backend/NotificationBackendServiceTests.kt @@ -32,6 +32,7 @@ class NotificationBackendServiceTests : FunSpec({ "notificationId", "subscriptionId", IDeviceService.DeviceType.Android, + "onesignalId", ) // Then @@ -42,6 +43,37 @@ class NotificationBackendServiceTests : FunSpec({ it.getString("app_id") shouldBe "appId" it.getString("player_id") shouldBe "subscriptionId" it.getInt("device_type") shouldBe IDeviceService.DeviceType.Android.value + it.getString("onesignal_id") shouldBe "onesignalId" + }, + ) + } + } + + test("updateNotificationAsReceived omits onesignal_id when the SDK has no backend id") { + // Given: the receipt was enqueued while the user still had a local placeholder id. The + // server looks the user up by this field, so a placeholder must never be sent, and the + // agreed contract is to leave the key out entirely rather than send null. + val spyHttpClient = mockk() + coEvery { spyHttpClient.put(any(), any()) } returns HttpResponse(202, null) + + val notificationBackendService = NotificationBackendService(spyHttpClient) + + // When + notificationBackendService.updateNotificationAsReceived( + "appId", + "notificationId", + "subscriptionId", + IDeviceService.DeviceType.Android, + null, + ) + + // Then + coVerify { + spyHttpClient.put( + "notifications/notificationId/report_received", + withArg { + it.getString("player_id") shouldBe "subscriptionId" + it.has("onesignal_id") shouldBe false }, ) } @@ -62,6 +94,7 @@ class NotificationBackendServiceTests : FunSpec({ "notificationId", "subscriptionId", IDeviceService.DeviceType.Android, + null, ) } @@ -82,6 +115,7 @@ class NotificationBackendServiceTests : FunSpec({ "notificationId", "subscriptionId", IDeviceService.DeviceType.Android, + "onesignalId", ) // Then @@ -93,6 +127,38 @@ class NotificationBackendServiceTests : FunSpec({ it.getString("player_id") shouldBe "subscriptionId" it.getBoolean("opened") shouldBe true it.getInt("device_type") shouldBe IDeviceService.DeviceType.Android.value + it.getString("onesignal_id") shouldBe "onesignalId" + }, + ) + } + } + + test("updateNotificationAsOpened omits onesignal_id when the SDK has no backend id") { + // Given: the tap happened while the user still had a local placeholder id. The server + // looks the user up by this field, so a placeholder must never be sent, and the agreed + // contract is to leave the key out entirely rather than send null. + val spyHttpClient = mockk() + coEvery { spyHttpClient.put(any(), any()) } returns HttpResponse(202, null) + + val notificationBackendService = NotificationBackendService(spyHttpClient) + + // When + notificationBackendService.updateNotificationAsOpened( + "appId", + "notificationId", + "subscriptionId", + IDeviceService.DeviceType.Android, + null, + ) + + // Then + coVerify { + spyHttpClient.put( + "notifications/notificationId", + withArg { + it.getString("player_id") shouldBe "subscriptionId" + it.getBoolean("opened") shouldBe true + it.has("onesignal_id") shouldBe false }, ) } @@ -113,6 +179,7 @@ class NotificationBackendServiceTests : FunSpec({ "notificationId", "subscriptionId", IDeviceService.DeviceType.Android, + null, ) } diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/lifecycle/NotificationLifecycleServiceTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/lifecycle/NotificationLifecycleServiceTests.kt index 3f50fed3d8..8b1838c5f2 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/lifecycle/NotificationLifecycleServiceTests.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/lifecycle/NotificationLifecycleServiceTests.kt @@ -5,6 +5,7 @@ import android.content.Context import android.content.Intent import androidx.test.core.app.ApplicationProvider import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.common.IDManager import com.onesignal.common.threading.withMain import com.onesignal.core.internal.application.impl.ApplicationService import com.onesignal.core.internal.device.IDeviceService @@ -41,7 +42,7 @@ import org.json.JSONObject import org.robolectric.Robolectric import org.robolectric.shadows.ShadowLooper -private class Mocks { +private class Mocks(onesignalId: String = "onesignalId") { val context = ApplicationProvider.getApplicationContext() val applicationService = run { @@ -58,6 +59,15 @@ private class Mocks { mockSubManager } + val mockBackend: INotificationBackendService = + mockk().apply { + coEvery { updateNotificationAsOpened(any(), any(), any(), any(), any()) } coAnswers { + // assume every updateNotificationAsOpened call takes 5 ms + delay(5) + Unit + } + } + val realNotificationLifecycleService = run { NotificationLifecycleService( @@ -68,16 +78,11 @@ private class Mocks { every { onDirectInfluenceFromNotification(any()) } returns Unit }, mockSubscriptionManager, + MockHelper.identityModelStore { it.onesignalId = onesignalId }, mockk().apply { every { deviceType } returns IDeviceService.DeviceType.Android }, - mockk().apply { - coEvery { updateNotificationAsOpened(any(), any(), any(), any()) } coAnswers { - // assume every updateNotificationAsOpened call takes 5 ms - delay(5) - Unit - } - }, + mockBackend, mockk(), mockk().apply { every { trackOpenedEvent(any(), any()) } returns Unit @@ -96,6 +101,13 @@ private class Mocks { } } +private fun openedPayload(notificationId: String): JSONArray = + JSONArray().put( + JSONObject() + .put("alert", "test message") + .put("custom", JSONObject().put("i", notificationId)), + ) + @RobolectricTest class NotificationLifecycleServiceTests : FunSpec({ beforeAny { @@ -132,6 +144,50 @@ class NotificationLifecycleServiceTests : FunSpec({ } } + test("notificationOpened reports the open with the backend onesignal id") { + // Given + val mocks = Mocks(onesignalId = "onesignalId") + val notificationLifecycleService = mocks.notificationLifecycleService + val activity = mocks.activity + + // When + notificationLifecycleService.notificationOpened(activity, openedPayload("notificationId")) + + // Then: the backend update runs on an IO coroutine, so allow it a moment to land. + coVerify(timeout = 1000) { + mocks.mockBackend.updateNotificationAsOpened( + MockHelper.DEFAULT_APP_ID, + "notificationId", + "UUID1", + IDeviceService.DeviceType.Android, + "onesignalId", + ) + } + } + + test("notificationOpened leaves the onesignal id out while the user has a local placeholder id") { + // Given: the tap landed right after login() or logout(), before the create user response + // replaced the local id. The server looks the user up by this field, so a placeholder must + // be dropped rather than sent. + val mocks = Mocks(onesignalId = IDManager.createLocalId()) + val notificationLifecycleService = mocks.notificationLifecycleService + val activity = mocks.activity + + // When + notificationLifecycleService.notificationOpened(activity, openedPayload("notificationId")) + + // Then + coVerify(timeout = 1000) { + mocks.mockBackend.updateNotificationAsOpened( + MockHelper.DEFAULT_APP_ID, + "notificationId", + "UUID1", + IDeviceService.DeviceType.Android, + null, + ) + } + } + test("queued click events are not refired when a second listener is added") { // Given val mocks = Mocks() diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/receivereceipt/ReceiveReceiptProcessorTests.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/receivereceipt/ReceiveReceiptProcessorTests.kt new file mode 100644 index 0000000000..8a587e4a73 --- /dev/null +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/receivereceipt/ReceiveReceiptProcessorTests.kt @@ -0,0 +1,82 @@ +package com.onesignal.notifications.internal.receivereceipt + +import com.onesignal.common.exceptions.BackendException +import com.onesignal.core.internal.device.IDeviceService +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.notifications.internal.backend.INotificationBackendService +import com.onesignal.notifications.internal.receivereceipt.impl.ReceiveReceiptProcessor +import io.kotest.assertions.throwables.shouldNotThrowAny +import io.kotest.core.spec.style.FunSpec +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk + +class ReceiveReceiptProcessorTests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + fun androidDevice() = + mockk().apply { + every { deviceType } returns IDeviceService.DeviceType.Android + } + + test("sendReceiveReceipt reports the receipt with the onesignal id captured at enqueue time") { + // Given + val mockBackend = mockk() + coEvery { mockBackend.updateNotificationAsReceived(any(), any(), any(), any(), any()) } returns Unit + val processor = ReceiveReceiptProcessor(androidDevice(), mockBackend) + + // When + processor.sendReceiveReceipt("appId", "subscriptionId", "notificationId", "onesignalId") + + // Then + coVerify(exactly = 1) { + mockBackend.updateNotificationAsReceived( + "appId", + "notificationId", + "subscriptionId", + IDeviceService.DeviceType.Android, + "onesignalId", + ) + } + } + + test("sendReceiveReceipt reports the receipt without a onesignal id when none was captured") { + // Given: the work manager enqueued the receipt while the user only had a local placeholder + // id, so it handed the worker null. The processor must pass that through unchanged rather + // than look the id up again, which could name a user who logged in after the receipt. + val mockBackend = mockk() + coEvery { mockBackend.updateNotificationAsReceived(any(), any(), any(), any(), any()) } returns Unit + val processor = ReceiveReceiptProcessor(androidDevice(), mockBackend) + + // When + processor.sendReceiveReceipt("appId", "subscriptionId", "notificationId", null) + + // Then + coVerify(exactly = 1) { + mockBackend.updateNotificationAsReceived( + "appId", + "notificationId", + "subscriptionId", + IDeviceService.DeviceType.Android, + null, + ) + } + } + + test("sendReceiveReceipt swallows a backend failure so the worker does not retry") { + // Given + val mockBackend = mockk() + coEvery { mockBackend.updateNotificationAsReceived(any(), any(), any(), any(), any()) } throws + BackendException(500, null) + val processor = ReceiveReceiptProcessor(androidDevice(), mockBackend) + + // When / Then + shouldNotThrowAny { + processor.sendReceiveReceipt("appId", "subscriptionId", "notificationId", "onesignalId") + } + } +})