Merge pull request #2354 from OneSignal/fix/multiple_create_subscription_ops_without_token

nan-li · web-flow · commit 5d39fa853ebc · 2025-09-02T09:24:32.000-07:00
fix(subs): Fix when multiple create subscription operations exist without token
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/SubscriptionBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/SubscriptionBackendService.kt
@@ -21,7 +21,6 @@ internal class SubscriptionBackendService(
         subscription: SubscriptionObject,
     ): Pair<String, RywData?>? {
         val jsonSubscription = JSONConverter.convertToJSON(subscription)
-        jsonSubscription.remove("id")
         val requestJSON = JSONObject().put("subscription", jsonSubscription)
 
         val response = _httpClient.post("apps/$appId/users/by/$aliasLabel/$aliasValue/subscriptions", requestJSON)
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/CreateSubscriptionOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/CreateSubscriptionOperation.kt
@@ -99,8 +99,7 @@ class CreateSubscriptionOperation() : Operation(SubscriptionOperationExecutor.CR
     }
 
     override fun translateIds(map: Map<String, String>) {
-        if (map.containsKey(onesignalId)) {
-            onesignalId = map[onesignalId]!!
-        }
+        map[onesignalId]?.let { onesignalId = it }
+        map[subscriptionId]?.let { subscriptionId = it }
     }
 }
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt
@@ -3,6 +3,7 @@ package com.onesignal.user.internal.operations.impl.executors
 import android.os.Build
 import com.onesignal.common.AndroidUtils
 import com.onesignal.common.DeviceUtils
+import com.onesignal.common.IDManager
 import com.onesignal.common.NetworkUtils
 import com.onesignal.common.OneSignalUtils
 import com.onesignal.common.RootToolsInternalMethods
@@ -287,9 +288,10 @@ internal class LoginUserOperationExecutor(
                     SubscriptionObjectType.fromDeviceType(_deviceService.deviceType)
                 }
             }
+        val subId = if (!IDManager.isLocalId(operation.subscriptionId)) operation.subscriptionId else null
         mutableSubscriptions[operation.subscriptionId] =
             SubscriptionObject(
-                id = null,
+                id = subId,
                 subscriptionType,
                 operation.address,
                 operation.enabled,
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/SubscriptionOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/SubscriptionOperationExecutor.kt
@@ -3,6 +3,7 @@ package com.onesignal.user.internal.operations.impl.executors
 import android.os.Build
 import com.onesignal.common.AndroidUtils
 import com.onesignal.common.DeviceUtils
+import com.onesignal.common.IDManager
 import com.onesignal.common.NetworkUtils
 import com.onesignal.common.OneSignalUtils
 import com.onesignal.common.RootToolsInternalMethods
@@ -87,11 +88,12 @@ internal class SubscriptionOperationExecutor(
         val enabled = lastUpdateOperation?.enabled ?: createOperation.enabled
         val address = lastUpdateOperation?.address ?: createOperation.address
         val status = lastUpdateOperation?.status ?: createOperation.status
+        val subId = if (!IDManager.isLocalId(createOperation.subscriptionId)) createOperation.subscriptionId else null
 
         try {
             val subscription =
                 SubscriptionObject(
-                    id = null,
+                    id = subId,
                     convert(createOperation.type),
                     address,
                     enabled,
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/backend/SubscriptionBackendServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/backend/SubscriptionBackendServiceTests.kt
@@ -33,7 +33,7 @@ class SubscriptionBackendServiceTests : FunSpec({
         // When
         val subscription =
             SubscriptionObject(
-                "no-id",
+                "sub-id",
                 SubscriptionObjectType.ANDROID_PUSH,
                 "pushToken",
                 true,
@@ -49,7 +49,7 @@ class SubscriptionBackendServiceTests : FunSpec({
                 "apps/appId/users/by/$aliasLabel/$aliasValue/subscriptions",
                 withArg {
                     val sub = it.getJSONObject("subscription")
-                    sub.has("id") shouldBe false
+                    sub.getString("id") shouldBe "sub-id"
                     sub.getString("type") shouldBe "AndroidPush"
                     sub.getString("token") shouldBe "pushToken"
                     sub.getBoolean("enabled") shouldBe true
@@ -70,7 +70,7 @@ class SubscriptionBackendServiceTests : FunSpec({
         // When
         val subscription =
             SubscriptionObject(
-                "no-id",
+                "sub-id",
                 SubscriptionObjectType.ANDROID_PUSH,
                 "pushToken",
                 true,
@@ -94,7 +94,7 @@ class SubscriptionBackendServiceTests : FunSpec({
                 "apps/appId/users/by/$aliasLabel/$aliasValue/subscriptions",
                 withArg {
                     val sub = it.getJSONObject("subscription")
-                    sub.has("id") shouldBe false
+                    sub.getString("id") shouldBe "sub-id"
                     sub.getString("type") shouldBe "AndroidPush"
                     sub.getString("token") shouldBe "pushToken"
                     sub.getBoolean("enabled") shouldBe true
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/SubscriptionOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/SubscriptionOperationExecutorTests.kt
@@ -45,7 +45,7 @@ class SubscriptionOperationExecutorTests :
             coEvery { mockConsistencyManager.setRywData(any(), any(), any()) } just runs
         }
 
-        test("create subscription successfully creates subscription") {
+        test("create subscription successfully creates subscription, with local ID") {
             // Given
             val mockSubscriptionBackendService = mockk<ISubscriptionBackendService>()
             coEvery { mockSubscriptionBackendService.createSubscription(any(), any(), any(), any()) } returns
@@ -95,6 +95,7 @@ class SubscriptionOperationExecutorTests :
                     IdentityConstants.ONESIGNAL_ID,
                     remoteOneSignalId,
                     withArg {
+                        it.id shouldBe null
                         it.type shouldBe SubscriptionObjectType.ANDROID_PUSH
                         it.enabled shouldBe true
                         it.token shouldBe "pushToken"
@@ -104,6 +105,60 @@ class SubscriptionOperationExecutorTests :
             }
         }
 
+        test("create subscription includes the subscription ID if non-local") {
+            // Given
+            val mockSubscriptionBackendService = mockk<ISubscriptionBackendService>()
+            coEvery { mockSubscriptionBackendService.createSubscription(any(), any(), any(), any()) } returns
+                Pair(remoteSubscriptionId, rywData)
+
+            val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
+            val subscriptionModel = SubscriptionModel()
+            subscriptionModel.id = remoteSubscriptionId
+            every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns subscriptionModel
+
+            val mockBuildUserService = mockk<IRebuildUserService>()
+
+            val subscriptionOperationExecutor =
+                SubscriptionOperationExecutor(
+                    mockSubscriptionBackendService,
+                    MockHelper.deviceService(),
+                    AndroidMockHelper.applicationService(),
+                    mockSubscriptionsModelStore,
+                    MockHelper.configModelStore(),
+                    mockBuildUserService,
+                    getNewRecordState(),
+                    mockConsistencyManager,
+                )
+
+            val operations =
+                listOf<Operation>(
+                    CreateSubscriptionOperation(
+                        appId,
+                        remoteOneSignalId,
+                        remoteSubscriptionId,
+                        SubscriptionType.PUSH,
+                        true,
+                        "pushToken",
+                        SubscriptionStatus.SUBSCRIBED,
+                    ),
+                )
+
+            // When
+            subscriptionOperationExecutor.execute(operations)
+
+            // Then
+            coVerify(exactly = 1) {
+                mockSubscriptionBackendService.createSubscription(
+                    appId,
+                    IdentityConstants.ONESIGNAL_ID,
+                    remoteOneSignalId,
+                    withArg {
+                        it.id shouldBe remoteSubscriptionId
+                    },
+                )
+            }
+        }
+
         test("create subscription fails with retry when there is a network condition") {
             // Given
             val mockSubscriptionBackendService = mockk<ISubscriptionBackendService>()

Original file line number	Diff line number	Diff line change
`@@ -99,8 +99,7 @@ class CreateSubscriptionOperation() : Operation(SubscriptionOperationExecutor.CR`
`99`	`99`	`}`
`100`	`100`
`101`	`101`	`override fun translateIds(map: Map<String, String>) {`
`102`		`- if (map.containsKey(onesignalId)) {`
`103`		`- onesignalId = map[onesignalId]!!`
`104`		`- }`
	`102`	`+ map[onesignalId]?.let { onesignalId = it }`
	`103`	`+ map[subscriptionId]?.let { subscriptionId = it }`
`105`	`104`	`}`
`106`	`105`	`}`