fix: [SDK-4474] self-heal SDK-4388-stuck push subscriptions on session start

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt

@@ -13,11 +13,14 @@ import com.onesignal.debug.LogLevel
 import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.user.internal.backend.IUserBackendService
 import com.onesignal.user.internal.backend.IdentityConstants
+import com.onesignal.user.internal.backend.SubscriptionObject
 import com.onesignal.user.internal.backend.SubscriptionObjectType
 import com.onesignal.user.internal.builduser.IRebuildUserService
 import com.onesignal.user.internal.identity.IdentityModel
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.operations.RefreshUserOperation
+import com.onesignal.user.internal.operations.UpdateSubscriptionOperation
+import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener
 import com.onesignal.user.internal.operations.impl.states.NewRecordsState
 import com.onesignal.user.internal.properties.PropertiesModel
 import com.onesignal.user.internal.properties.PropertiesModelStore
@@ -93,7 +96,13 @@ internal class RefreshUserOperationExecutor(
             // No longer hydrate timezone from remote, set locally
             propertiesModel.timezone = TimeUtils.getTimeZoneId()
 
+            // Retrieve the push subscription ID from the backend configuration model store. Used
+            // both for re-attaching the locally-cached push model below and for the SDK-4474
+            // self-heal divergence check inside the loop.
+            val pushSubscriptionIdFromConfig = _configModelStore.model.pushSubscriptionId
+
             val subscriptionModels = mutableListOf<SubscriptionModel>()
+            var pushSelfHealOperation: UpdateSubscriptionOperation? = null
             for (subscription in response.subscriptions) {
                 val subscriptionModel = SubscriptionModel()
                 subscriptionModel.id = subscription.id!!
@@ -121,10 +130,22 @@ internal class RefreshUserOperationExecutor(
                 // so we don't want to cache these subscriptions from the backend.
                 if (subscriptionModel.type != SubscriptionType.PUSH) {
                     subscriptionModels.add(subscriptionModel)
+                } else if (subscription.id == pushSubscriptionIdFromConfig && pushSelfHealOperation == null) {
+                    // SDK-4474 self-heal for SDK-4388 victims. The buggy
+                    // SubscriptionOperationExecutor in older SDK builds dispatched the merged
+                    // create-subscription + update-subscription(SUBSCRIBED) batch as a POST
+                    // /subscriptions carrying the already-existing server-side id; the server
+                    // responded 200 {} (no-op) and the local op queue was emptied, leaving the
+                    // server stuck at enabled:false / notification_types:0. Once that user
+                    // upgrades to a fixed SDK build, nothing in the queue triggers the fix path.
+                    //
+                    // Detect the divergence here (every session start runs RefreshUser) and
+                    // re-assert local truth via PATCH. "Device is the source of truth" is the
+                    // existing policy for push; this just enforces it across the wire when the
+                    // server has drifted out of sync.
+                    pushSelfHealOperation = buildPushSelfHealOperationForStuckSubscription(op, subscription, pushSubscriptionIdFromConfig)
                 }
             }
-            // 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
@@ -140,7 +161,10 @@ internal class RefreshUserOperationExecutor(
             _propertiesModelStore.replace(propertiesModel, ModelChangeTags.HYDRATE)
             _subscriptionsModelStore.replaceAll(subscriptionModels, ModelChangeTags.HYDRATE)
 
-            return ExecutionResponse(ExecutionResult.SUCCESS)
+            return ExecutionResponse(
+                ExecutionResult.SUCCESS,
+                operations = pushSelfHealOperation?.let { listOf<Operation>(it) },
+            )
         } catch (ex: BackendException) {
             val responseType = NetworkUtils.getResponseStatusType(ex.statusCode)
 
@@ -170,6 +194,49 @@ internal class RefreshUserOperationExecutor(
         }
     }
 
+    /**
+     * SDK-4474 self-heal builder. Returns an [UpdateSubscriptionOperation] iff the cached
+     * push subscription model resolves to enabled-and-opted-in but the server response
+     * recorded the same id as disabled. Returns null otherwise (no divergence — no-op).
+     *
+     * Caller has already verified that [serverSubscription.id] matches the cached
+     * `pushSubscriptionId`, so this method only inspects the local model + server flags.
+     */
+    private fun buildPushSelfHealOperationForStuckSubscription(
+        op: RefreshUserOperation,
+        serverSubscription: SubscriptionObject,
+        pushSubscriptionId: String,
+    ): UpdateSubscriptionOperation? {
+        val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionId)
+        if (cachedPushSubscriptionModel == null || cachedPushSubscriptionModel.type != SubscriptionType.PUSH) {
+            return null
+        }
+
+        val (localEnabled, localStatus) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(cachedPushSubscriptionModel)
+        val serverEnabled = (serverSubscription.enabled == true) && ((serverSubscription.notificationTypes ?: 0) > 0)
+        val divergent = localEnabled && !serverEnabled
+
+        return if (divergent) {
+            Logging.info(
+                "RefreshUserOperationExecutor: push subscription $pushSubscriptionId diverged from server " +
+                    "(server enabled=${serverSubscription.enabled} notificationTypes=${serverSubscription.notificationTypes}; " +
+                    "local opted-in and SUBSCRIBED). Enqueuing follow-up update-subscription op to re-assert local " +
+                    "truth via PATCH /subscriptions/{id}. See SDK-4388 / SDK-4474.",
+            )
+            UpdateSubscriptionOperation(
+                op.appId,
+                op.onesignalId,
+                pushSubscriptionId,
+                cachedPushSubscriptionModel.type,
+                localEnabled,
+                cachedPushSubscriptionModel.address,
+                localStatus,
+            )
+        } else {
+            null
+        }
+    }
+
     companion object {
         const val REFRESH_USER = "refresh-user"
     }

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt

@@ -5,6 +5,8 @@ import com.onesignal.common.exceptions.BackendException
 import com.onesignal.common.modeling.ModelChangeTags
 import com.onesignal.core.internal.operations.ExecutionResult
 import com.onesignal.core.internal.operations.Operation
+import com.onesignal.debug.LogLevel
+import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.mocks.MockHelper
 import com.onesignal.user.internal.backend.CreateUserResponse
 import com.onesignal.user.internal.backend.IUserBackendService
@@ -16,6 +18,7 @@ import com.onesignal.user.internal.builduser.IRebuildUserService
 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.operations.impl.executors.SubscriptionOperationExecutor
 import com.onesignal.user.internal.properties.PropertiesModel
 import com.onesignal.user.internal.subscriptions.SubscriptionModel
 import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
@@ -59,8 +62,10 @@ class RefreshUserOperationExecutorTests : FunSpec({
                 mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId, "aliasLabel1" to "aliasValue1"),
                 PropertiesObject(country = remoteCountry, language = remoteLanguage, timezoneId = remoteTimeZone, tags = remoteTags),
                 listOf(
-                    SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "on-backend-push-token"),
-                    SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken2"),
+                    // notificationTypes = 1 keeps server-side view healthy so the SDK-4474
+                    // self-heal divergence check is a no-op for this happy-path test.
+                    SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, notificationTypes = 1, token = "on-backend-push-token"),
+                    SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, notificationTypes = 1, token = "pushToken2"),
                     SubscriptionObject(remoteSubscriptionId2, SubscriptionObjectType.EMAIL, token = "name@company.com"),
                 ),
             )
@@ -314,6 +319,170 @@ class RefreshUserOperationExecutorTests : FunSpec({
         }
     }
 
+    // SDK-4474 self-heal divergence detection. Verifies that when the device-cached push
+    // subscription resolves to enabled-and-opted-in but the GET /users response returns the
+    // same subscription as disabled (the SDK-4388 stuck state), RefreshUserOperationExecutor
+    // emits a follow-up UpdateSubscriptionOperation to re-assert local truth via PATCH.
+    fun buildSelfHealHarness(
+        serverPushEnabled: Boolean,
+        serverNotificationTypes: Int?,
+        localOptedIn: Boolean,
+        localStatus: SubscriptionStatus,
+        localAddress: String,
+    ): Triple<RefreshUserOperationExecutor, SubscriptionModel, IUserBackendService> {
+        val mockUserBackendService = mockk<IUserBackendService>()
+        coEvery { mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) } returns
+            CreateUserResponse(
+                mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId),
+                PropertiesObject(),
+                listOf(
+                    SubscriptionObject(
+                        existingSubscriptionId1,
+                        SubscriptionObjectType.ANDROID_PUSH,
+                        enabled = serverPushEnabled,
+                        notificationTypes = serverNotificationTypes,
+                        token = "on-backend-push-token",
+                    ),
+                ),
+            )
+
+        val mockIdentityModelStore = MockHelper.identityModelStore()
+        val mockIdentityModel = IdentityModel()
+        mockIdentityModel.onesignalId = remoteOneSignalId
+        every { mockIdentityModelStore.model } returns mockIdentityModel
+        every { mockIdentityModelStore.replace(any(), any()) } just runs
+
+        val mockPropertiesModelStore = MockHelper.propertiesModelStore()
+        val mockPropertiesModel = PropertiesModel()
+        mockPropertiesModel.onesignalId = remoteOneSignalId
+        every { mockPropertiesModelStore.model } returns mockPropertiesModel
+        every { mockPropertiesModelStore.replace(any(), any()) } just runs
+
+        val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
+        every { mockSubscriptionsModelStore.replaceAll(any(), any()) } just runs
+
+        val cachedPushSubscriptionModel = SubscriptionModel()
+        cachedPushSubscriptionModel.id = existingSubscriptionId1
+        cachedPushSubscriptionModel.type = SubscriptionType.PUSH
+        cachedPushSubscriptionModel.address = localAddress
+        cachedPushSubscriptionModel.status = localStatus
+        cachedPushSubscriptionModel.optedIn = localOptedIn
+        every { mockSubscriptionsModelStore.get(existingSubscriptionId1) } returns cachedPushSubscriptionModel
+
+        val mockConfigModelStore =
+            MockHelper.configModelStore {
+                it.pushSubscriptionId = existingSubscriptionId1
+            }
+
+        val executor =
+            RefreshUserOperationExecutor(
+                mockUserBackendService,
+                mockIdentityModelStore,
+                mockPropertiesModelStore,
+                mockSubscriptionsModelStore,
+                mockConfigModelStore,
+                mockk<IRebuildUserService>(),
+                getNewRecordState(),
+            )
+
+        return Triple(executor, cachedPushSubscriptionModel, mockUserBackendService)
+    }
+
+    test("SDK-4474 self-heal: enqueues follow-up update-subscription op when server is stuck-disabled but local is enabled") {
+        // Given: server view says push is disabled (the SDK-4388 stuck state), local view says enabled
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = false,
+                serverNotificationTypes = 0,
+                localOptedIn = true,
+                localStatus = SubscriptionStatus.SUBSCRIBED,
+                localAddress = onDevicePushToken,
+            )
+
+        // Suppress logcat output for the self-heal WARN line so the unmocked android.util.Log
+        // in robolectric-free unit tests doesn't blow up. Restored in finally.
+        val originalLogLevel = Logging.logLevel
+        Logging.logLevel = LogLevel.NONE
+        try {
+            // When
+            val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId)))
+
+            // Then a single UpdateSubscriptionOperation is returned to the op repo
+            response.result shouldBe ExecutionResult.SUCCESS
+            response.operations?.count() shouldBe 1
+            val followup = response.operations!![0]
+            (followup is UpdateSubscriptionOperation) shouldBe true
+            followup as UpdateSubscriptionOperation
+            followup.name shouldBe SubscriptionOperationExecutor.UPDATE_SUBSCRIPTION
+            followup.appId shouldBe appId
+            followup.onesignalId shouldBe remoteOneSignalId
+            followup.subscriptionId shouldBe existingSubscriptionId1
+            followup.type shouldBe SubscriptionType.PUSH
+            followup.enabled shouldBe true
+            followup.address shouldBe onDevicePushToken
+            followup.status shouldBe SubscriptionStatus.SUBSCRIBED
+        } finally {
+            Logging.logLevel = originalLogLevel
+        }
+    }
+
+    test("SDK-4474 self-heal: does NOT enqueue follow-up op when server matches local (healthy push subscription)") {
+        // Given: server already enabled and notificationTypes=1, local also enabled
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = true,
+                serverNotificationTypes = 1,
+                localOptedIn = true,
+                localStatus = SubscriptionStatus.SUBSCRIBED,
+                localAddress = onDevicePushToken,
+            )
+
+        // When
+        val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId)))
+
+        // Then no follow-up ops emitted
+        response.result shouldBe ExecutionResult.SUCCESS
+        response.operations shouldBe null
+    }
+
+    test("SDK-4474 self-heal: does NOT enqueue follow-up op when local is opted out (UNSUBSCRIBE is intentional)") {
+        // Given: server is disabled, but so is local (user explicitly opted out)
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = false,
+                serverNotificationTypes = -2,
+                localOptedIn = false,
+                localStatus = SubscriptionStatus.SUBSCRIBED,
+                localAddress = onDevicePushToken,
+            )
+
+        // When
+        val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId)))
+
+        // Then no follow-up — opt-out is the user's intent, not divergence
+        response.result shouldBe ExecutionResult.SUCCESS
+        response.operations shouldBe null
+    }
+
+    test("SDK-4474 self-heal: does NOT enqueue follow-up op when local has NO_PERMISSION (OS-level disable is real)") {
+        // Given: server disabled, local also has no notification permission
+        val (executor, _, _) =
+            buildSelfHealHarness(
+                serverPushEnabled = false,
+                serverNotificationTypes = 0,
+                localOptedIn = true,
+                localStatus = SubscriptionStatus.NO_PERMISSION,
+                localAddress = onDevicePushToken,
+            )
+
+        // When
+        val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId)))
+
+        // Then no follow-up — OS denies notifications, server's view is correct
+        response.result shouldBe ExecutionResult.SUCCESS
+        response.operations shouldBe null
+    }
+
     test("refresh user is retried when backend returns MISSING, but isInMissingRetryWindow") {
         // Given
         val mockUserBackendService = mockk<IUserBackendService>()