From 2b571113dd789d8289e1d3c11a6d9100cd0a6a86 Mon Sep 17 00:00:00 2001 From: Rodrigo Gomez Palacio Date: Mon, 3 Jun 2024 16:37:48 -0500 Subject: [PATCH 1/3] Remove unnecessary retrievePushToken call Motivation: since we're replacing the config model every time, this is already being called. We don't need to call it again in `start` --- .../internal/listeners/DeviceRegistrationListener.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListener.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListener.kt index 65052baee5..abb7f5630e 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListener.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListener.kt @@ -37,8 +37,6 @@ internal class DeviceRegistrationListener( _configModelStore.subscribe(this) _notificationsManager.addPermissionObserver(this) _subscriptionManager.subscribe(this) - - retrievePushTokenAndUpdateSubscription() } override fun onModelReplaced( From 1531d9ce7a3b07674665057d1c79ae72075e616c Mon Sep 17 00:00:00 2001 From: Rodrigo Gomez Palacio Date: Thu, 13 Jun 2024 16:01:14 -0500 Subject: [PATCH 2/3] Only replace local models with non-push subscriptions Motivation: the backend should sync its push token state with the client state (truth) since it's the client that gets the token directly when subscribing -- as opposed to the client syncing with whatever comes in from the backend. --- .../executors/RefreshUserOperationExecutor.kt | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt index 11afd77254..cec48b05a9 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt @@ -117,11 +117,24 @@ internal class RefreshUserOperationExecutor( subscriptionModel.carrier = subscription.carrier ?: "" subscriptionModel.appVersion = subscription.appVersion ?: "" - // We only add a push subscription if it is this device's push subscription. - if (subscriptionModel.type != SubscriptionType.PUSH || subscriptionModel.id == _configModelStore.model.pushSubscriptionId) { + // We only add a non-push subscriptions. For push, the device is the source of truth + // so we don't want to cache these subscriptions from the backend. + if (subscriptionModel.type != SubscriptionType.PUSH) { subscriptionModels.add(subscriptionModel) } } + // Retrieve the push subscription ID from the backend configuration model store + val pushSubscriptionIdFromConfig = _configModelStore.model.pushSubscriptionId + + if (pushSubscriptionIdFromConfig != null) { + // Retrieve the push subscription model from the model store + val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionIdFromConfig) + + // If non-null, the cached push subscription matches the ID coming from backend config + if (cachedPushSubscriptionModel != null) { + subscriptionModels.add(cachedPushSubscriptionModel) + } + } _identityModelStore.replace(identityModel, ModelChangeTags.HYDRATE) _propertiesModelStore.replace(propertiesModel, ModelChangeTags.HYDRATE) From c832f1b963ddfe0e37c8fa88a73928c1b8744a0e Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 17 Jun 2024 12:49:45 -0400 Subject: [PATCH 3/3] Update RefreshUserOperationExecutorTests to account for device's push subscription not being refreshed from backend. --- .../RefreshUserOperationExecutorTests.kt | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt index 5179905cbc..07acdd60cb 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt @@ -16,7 +16,9 @@ import com.onesignal.user.internal.identity.IdentityModel import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState import com.onesignal.user.internal.operations.impl.executors.RefreshUserOperationExecutor import com.onesignal.user.internal.properties.PropertiesModel +import com.onesignal.user.internal.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore +import com.onesignal.user.internal.subscriptions.SubscriptionStatus import com.onesignal.user.internal.subscriptions.SubscriptionType import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe @@ -30,6 +32,7 @@ import io.mockk.runs class RefreshUserOperationExecutorTests : FunSpec({ val appId = "appId" val existingSubscriptionId1 = "existing-subscriptionId1" + val onDevicePushToken = "on-device-push-token" val remoteOneSignalId = "remote-onesignalId" val remoteSubscriptionId1 = "remote-subscriptionId1" val remoteSubscriptionId2 = "remote-subscriptionId2" @@ -42,7 +45,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId, "aliasLabel1" to "aliasValue1"), PropertiesObject(country = "US"), listOf( - SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken1"), + SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "on-backend-push-token"), SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken2"), SubscriptionObject(remoteSubscriptionId2, SubscriptionObjectType.EMAIL, token = "name@company.com"), ), @@ -66,6 +69,14 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockSubscriptionsModelStore = mockk() every { mockSubscriptionsModelStore.replaceAll(any(), any()) } just runs + val mockPushSubscriptionModel = SubscriptionModel() + mockPushSubscriptionModel.id = existingSubscriptionId1 + mockPushSubscriptionModel.type = SubscriptionType.PUSH + mockPushSubscriptionModel.address = onDevicePushToken + mockPushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED + mockPushSubscriptionModel.optedIn = true + every { mockSubscriptionsModelStore.get(existingSubscriptionId1) } returns mockPushSubscriptionModel + val mockConfigModelStore = MockHelper.configModelStore { it.pushSubscriptionId = existingSubscriptionId1 @@ -109,14 +120,14 @@ class RefreshUserOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore.replaceAll( withArg { it.count() shouldBe 2 - it[0].id shouldBe existingSubscriptionId1 - it[0].type shouldBe SubscriptionType.PUSH + it[0].id shouldBe remoteSubscriptionId2 + it[0].type shouldBe SubscriptionType.EMAIL it[0].optedIn shouldBe true - it[0].address shouldBe "pushToken1" - it[1].id shouldBe remoteSubscriptionId2 - it[1].type shouldBe SubscriptionType.EMAIL + it[0].address shouldBe "name@company.com" + it[1].id shouldBe existingSubscriptionId1 + it[1].type shouldBe SubscriptionType.PUSH it[1].optedIn shouldBe true - it[1].address shouldBe "name@company.com" + it[1].address shouldBe onDevicePushToken }, ModelChangeTags.HYDRATE, )