Merge pull request #2125 from OneSignal/fix/fcm-push-token-bug

Fix/fcm push token bug
OneSignal · Jun 17, 2024 · d3b50f1 · d3b50f1
2 parents 91fc398 + c832f1b
commit d3b50f1
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 11 deletions.
diff --git a/...ava/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt b/...ava/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)

diff --git a/...src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt b/...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<SubscriptionModelStore>()
  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,
  )

diff --git a/...rc/main/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListener.kt b/...rc/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(