From 60e30a5e2d76ea2c4912731ec7d5d1780ee59763 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 26 Aug 2026 19:21:05 -0700 Subject: [PATCH 01/11] fix: respect REST API-disabled push subscriptions A push subscription disabled through the REST API (notification_types -31) was re-enabled by the SDK: RefreshUser discarded the server's disable state for push, the session-start self-heal re-asserted local truth over it, and every subscription payload recomputed enabled from device state. Mirror the server's disable code on the push model when RefreshUser reports it, report it back in subscription payloads instead of the device-derived values, skip the stuck-subscription self-heal for it, and carry it across the login/logout user switch. The mirror clears when the server reports any other state and on an explicit optIn(). The 404 recovery paths (user rebuild and update-404 re-create) treat the dead record's disable as gone and recreate from device truth. Also remove the mislabeled DISABLED_FROM_REST_API_DEFAULT_REASON(-30) enum case; no OneSignal API has ever written -30 as a REST disable. Enum-name persistence now parses leniently in the shared model accessor, so models and queued operations persisted under an unknown enum name read as SUBSCRIBED instead of throwing on upgrade. --- .../com/onesignal/common/modeling/Model.kt | 4 +- .../user/internal/PushSubscription.kt | 5 + .../onesignal/user/internal/UserSwitcher.kt | 1 + .../builduser/impl/RebuildUserService.kt | 42 +++++--- .../operations/CreateSubscriptionOperation.kt | 3 +- .../operations/UpdateSubscriptionOperation.kt | 3 +- .../executors/RefreshUserOperationExecutor.kt | 31 +++++- .../SubscriptionOperationExecutor.kt | 18 +++- .../SubscriptionModelStoreListener.kt | 7 +- .../subscriptions/SubscriptionModel.kt | 29 +++++- .../user/internal/UserSwitcherTests.kt | 26 +++++ .../RefreshUserOperationExecutorTests.kt | 95 +++++++++++++++++++ .../SubscriptionOperationExecutorTests.kt | 58 +++++++++++ .../subscriptions/SubscriptionManagerTests.kt | 66 ++++++++++++- 14 files changed, 362 insertions(+), 26 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt index b5c226ba38..b2a060d24c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt @@ -544,7 +544,9 @@ open class Model( val value = getOptAnyProperty(name) ?: return null if (value is T) return value - if (value is String) return enumValueOf(value) + // Enum properties persist by name; a name this build's enum lacks (a removed case, or a + // downgrade from a newer SDK) must read as null rather than throw at model load. + if (value is String) return enumValues().firstOrNull { it.name == value } return value as T } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt index 3e5dac63f4..0a380bf0ff 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt @@ -22,6 +22,11 @@ internal open class PushSubscription( get() = model.optedIn && model.status != SubscriptionStatus.NO_PERMISSION override fun optIn() { + // A deliberate opt-in overrides a REST API disable; clearing it with a NORMAL-tagged + // change drives a subscription update that re-enables it on the server. + if (model.restApiDisabledReason != 0) { + model.restApiDisabledReason = 0 + } // we set `optedIn` using the lower level method so we can set `forceChange=true`, which // will result in *always* driving change notification. model.setBooleanProperty(SubscriptionModel::optedIn.name, true, forceChange = true) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt index f1401e031a..3e1b7effae 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt @@ -68,6 +68,7 @@ class UserSwitcher( optedIn = currentPushSubscription?.optedIn ?: true address = currentPushSubscription?.address ?: "" status = currentPushSubscription?.status ?: SubscriptionStatus.NO_PERMISSION + restApiDisabledReason = currentPushSubscription?.restApiDisabledReason ?: 0 sdk = oneSignalUtils.sdkVersion deviceOS = this@UserSwitcher.deviceOS ?: "" carrier = carrierName ?: "" diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt index 22442072e5..9a0881a378 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt @@ -1,5 +1,6 @@ package com.onesignal.user.internal.builduser.impl +import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.operations.Operation import com.onesignal.user.internal.builduser.IRebuildUserService @@ -8,6 +9,7 @@ import com.onesignal.user.internal.identity.IdentityModelStore import com.onesignal.user.internal.operations.CreateSubscriptionOperation import com.onesignal.user.internal.operations.LoginUserOperation import com.onesignal.user.internal.operations.RefreshUserOperation +import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener import com.onesignal.user.internal.properties.PropertiesModel import com.onesignal.user.internal.properties.PropertiesModelStore import com.onesignal.user.internal.subscriptions.SubscriptionModel @@ -48,20 +50,36 @@ class RebuildUserService( operations.add(LoginUserOperation(appId, onesignalId, identityModel.externalId)) val pushSubscription = subscriptionModels.firstOrNull { it.id == _configModelStore.model.pushSubscriptionId } if (pushSubscription != null) { - operations.add( - CreateSubscriptionOperation( - appId, - onesignalId, - identityModel.externalId, - pushSubscription.id, - pushSubscription.type, - pushSubscription.optedIn, - pushSubscription.address, - pushSubscription.status, - ), - ) + operations.add(buildPushRecoveryOperation(appId, onesignalId, identityModel.externalId, pushSubscription)) } operations.add(RefreshUserOperation(appId, onesignalId, identityModel.externalId)) return operations } + + // The server records this rebuild recreates no longer exist, so a recorded REST API + // disable died with them; clear it and recreate from device truth. + private fun buildPushRecoveryOperation( + appId: String, + onesignalId: String, + externalId: String?, + pushSubscription: SubscriptionModel, + ): CreateSubscriptionOperation { + _subscriptionsModelStore.get(pushSubscription.id)?.setIntProperty( + SubscriptionModel::restApiDisabledReason.name, + 0, + ModelChangeTags.HYDRATE, + ) + pushSubscription.restApiDisabledReason = 0 + val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscription) + return CreateSubscriptionOperation( + appId, + onesignalId, + externalId, + pushSubscription.id, + pushSubscription.type, + enabled, + pushSubscription.address, + status, + ) + } } 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 index c1335a1b25..c094e63132 100644 --- 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 @@ -77,7 +77,8 @@ class CreateSubscriptionOperation() : Operation(SubscriptionOperationExecutor.CR * The status of this subscription. */ var status: SubscriptionStatus - get() = getEnumProperty(::status.name) + // A persisted name this build's enum lacks reads as SUBSCRIBED instead of dropping the op batch. + get() = getOptEnumProperty(::status.name) ?: SubscriptionStatus.SUBSCRIBED private set(value) { setEnumProperty(::status.name, value) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/UpdateSubscriptionOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/UpdateSubscriptionOperation.kt index 51ea11282b..8f8bcef53a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/UpdateSubscriptionOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/UpdateSubscriptionOperation.kt @@ -76,7 +76,8 @@ class UpdateSubscriptionOperation() : Operation(SubscriptionOperationExecutor.UP * The status of this subscription. */ var status: SubscriptionStatus - get() = getEnumProperty(::status.name) + // A persisted name this build's enum lacks reads as SUBSCRIBED instead of dropping the op batch. + get() = getOptEnumProperty(::status.name) ?: SubscriptionStatus.SUBSCRIBED private set(value) { setEnumProperty(::status.name, value) } 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 7ab631f961..0ce35222eb 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 @@ -125,7 +125,8 @@ internal class RefreshUserOperationExecutor( SubscriptionType.PUSH } } - subscriptionModel.optedIn = subscriptionModel.status != SubscriptionStatus.UNSUBSCRIBE && subscriptionModel.status != SubscriptionStatus.DISABLED_FROM_REST_API_DEFAULT_REASON + subscriptionModel.optedIn = subscriptionModel.status != SubscriptionStatus.UNSUBSCRIBE && + subscriptionModel.status != SubscriptionStatus.DISABLED_FROM_REST_API subscriptionModel.sdk = subscription.sdk ?: "" subscriptionModel.deviceOS = subscription.deviceOS ?: "" subscriptionModel.carrier = subscription.carrier ?: "" @@ -136,6 +137,7 @@ internal class RefreshUserOperationExecutor( if (subscriptionModel.type != SubscriptionType.PUSH) { subscriptionModels.add(subscriptionModel) } else if (subscription.id == pushSubscriptionIdFromConfig && pushSelfHealOperationForStuckSubscription == null) { + hydrateRestApiDisableState(subscription, pushSubscriptionIdFromConfig) // Self-heal for users stuck at "Never Subscribed". 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 @@ -218,7 +220,10 @@ internal class RefreshUserOperationExecutor( val (localEnabled, localStatus) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(cachedPushSubscriptionModel) val serverEnabled = (serverSubscription.enabled == true) && ((serverSubscription.notificationTypes ?: 0) > 0) - val divergent = localEnabled && !serverEnabled + // A REST API disable is deliberate suppression, not the stuck-subscription drift this + // self-heal exists for; leave it in place. + val serverDisabledViaRestApi = SubscriptionStatus.isRestApiDisable(serverSubscription.notificationTypes) + val divergent = localEnabled && !serverEnabled && !serverDisabledViaRestApi return if (divergent) { Logging.info( @@ -242,6 +247,28 @@ internal class RefreshUserOperationExecutor( } } + /** + * Records or clears the server's REST API disable state on the cached push model. Only that + * state is server-owned; the device stays the source of truth for the rest of the push model, + * which is why push subscriptions are otherwise not hydrated from the backend. + */ + private fun hydrateRestApiDisableState( + serverSubscription: SubscriptionObject, + pushSubscriptionId: String, + ) { + val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionId) ?: return + val serverTypes = serverSubscription.notificationTypes ?: return + // The recorded reason mirrors the server's field: -31 records, any other reported value clears. + val target = if (SubscriptionStatus.isRestApiDisable(serverTypes)) serverTypes else 0 + if (cachedPushSubscriptionModel.restApiDisabledReason != target) { + cachedPushSubscriptionModel.setIntProperty( + SubscriptionModel::restApiDisabledReason.name, + target, + ModelChangeTags.HYDRATE, + ) + } + } + companion object { const val REFRESH_USER = "refresh-user" } 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 index 6db548a206..693c318773 100644 --- 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 @@ -31,6 +31,7 @@ import com.onesignal.user.internal.operations.CreateSubscriptionOperation import com.onesignal.user.internal.operations.DeleteSubscriptionOperation import com.onesignal.user.internal.operations.TransferSubscriptionOperation 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.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore @@ -265,11 +266,22 @@ internal class SubscriptionOperationExecutor( // emitting Creates with the same subscriptionId, so they dedupe instead // of producing two POST /users subscription rows. HYDRATE prevents the // SubscriptionModelStoreListener from enqueuing follow-on operations. - _subscriptionModelStore.get(staleSubscriptionId)?.setStringProperty( + val recoveryModel = _subscriptionModelStore.get(staleSubscriptionId) + recoveryModel?.setStringProperty( SubscriptionModel::id.name, recoveryLocalId, ModelChangeTags.HYDRATE, ) + // The stale record died with any recorded REST API disable; recreate from + // device truth rather than the values frozen on the failed operation. + recoveryModel?.setIntProperty( + SubscriptionModel::restApiDisabledReason.name, + 0, + ModelChangeTags.HYDRATE, + ) + val (recoveryEnabled, recoveryStatus) = + recoveryModel?.let { SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(it) } + ?: Pair(lastOperation.enabled, lastOperation.status) if (_configModelStore.model.pushSubscriptionId == staleSubscriptionId) { _configModelStore.model.pushSubscriptionId = recoveryLocalId } @@ -284,9 +296,9 @@ internal class SubscriptionOperationExecutor( lastOperation.externalId, recoveryLocalId, lastOperation.type, - lastOperation.enabled, + recoveryEnabled, lastOperation.address, - lastOperation.status, + recoveryStatus, ), ), ) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt index c210193e69..4b979f8ea0 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt @@ -73,7 +73,12 @@ internal class SubscriptionModelStoreListener( val status: SubscriptionStatus val enabled: Boolean - if (model.optedIn && model.status == SubscriptionStatus.SUBSCRIBED && model.address.isNotEmpty()) { + // A REST API disable is server-owned; report it back rather than the device state so + // subscription payloads don't re-enable a suppressed subscription. + if (SubscriptionStatus.isRestApiDisable(model.restApiDisabledReason)) { + enabled = false + status = SubscriptionStatus.DISABLED_FROM_REST_API + } else if (model.optedIn && model.status == SubscriptionStatus.SUBSCRIBED && model.address.isNotEmpty()) { enabled = true status = SubscriptionStatus.SUBSCRIBED } else { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt index 84825dfda3..3bf0f5c4a4 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt @@ -67,8 +67,8 @@ enum class SubscriptionStatus(val value: Int) { /** The subscription is not enabled due to an FCM authentication failed IOException, this can be retried */ FIREBASE_FCM_ERROR_IOEXCEPTION_AUTHENTICATION_FAILED(-29), - /** The subscription is not enabled because the app has disabled the subscription via API */ - DISABLED_FROM_REST_API_DEFAULT_REASON(-30), + /** The subscription is not enabled because it was disabled through the REST API */ + DISABLED_FROM_REST_API(-31), /** The subscription is not enabled due to some other (unknown locally) error */ ERROR(9999), @@ -101,6 +101,15 @@ enum class SubscriptionStatus(val value: Int) { FIREBASE_FCM_ERROR_IOEXCEPTION_AUTHENTICATION_FAILED, // -29 ) + /** + * True when [value] is the code the server uses for a subscription disabled through the + * REST API, which is only -31. The SDK never derives it from device state, and + * server-reported error codes stay device-recoverable. + */ + fun isRestApiDisable(value: Int?): Boolean { + return value == DISABLED_FROM_REST_API.value + } + fun fromInt(value: Int): SubscriptionStatus? { return SubscriptionStatus.values().firstOrNull { it.value == value } } @@ -135,6 +144,19 @@ class SubscriptionModel : Model() { setBooleanProperty(::isDisabledInternally.name, value) } + /** + * The server's REST API disable code (-31), or 0 when the server has not disabled this + * subscription. Hydrated by RefreshUser and never derived from device state; while set, + * [SubscriptionModelStoreListener] reports `enabled = false` with this status so subscription + * payloads don't re-enable a suppressed subscription. Cleared when the server reports any + * other state, or by [IPushSubscription.optIn]. + */ + var restApiDisabledReason: Int + get() = getIntProperty(::restApiDisabledReason.name) { 0 } + set(value) { + setIntProperty(::restApiDisabledReason.name, value) + } + var type: SubscriptionType get() = getEnumProperty(::type.name) set(value) { @@ -160,7 +182,8 @@ class SubscriptionModel : Model() { setEnumProperty(::status.name, SubscriptionStatus.SUBSCRIBED) } - return getEnumProperty(::status.name) + // A persisted name this build's enum lacks reads as SUBSCRIBED instead of throwing. + return getOptEnumProperty(::status.name) ?: SubscriptionStatus.SUBSCRIBED } set(value) { setEnumProperty(::status.name, value) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt index 18c4c53ea2..67be2bca48 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt @@ -20,6 +20,7 @@ import com.onesignal.user.internal.identity.IdentityModel import com.onesignal.user.internal.identity.IdentityModelStore import com.onesignal.user.internal.operations.LoginUserFromSubscriptionOperation import com.onesignal.user.internal.operations.LoginUserOperation +import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener import com.onesignal.user.internal.properties.PropertiesModelStore import com.onesignal.user.internal.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore @@ -257,6 +258,31 @@ class UserSwitcherTests : FunSpec({ verify(exactly = 1) { mockSubscriptionModelStore.add(any(), ModelChangeTags.NO_PROPOGATE) } } + test("createAndSwitchToNewUser carries a REST API disable onto the new push model") { + // Given + val mocks = Mocks() + val userSwitcher = mocks.createUserSwitcher() + val disabledPushModel = + SubscriptionModel().apply { + id = mocks.testSubscriptionId + type = SubscriptionType.PUSH + address = "test-token" + optedIn = true + restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + } + mocks.subscriptionModelStore!!.add(disabledPushModel, ModelChangeTags.NO_PROPOGATE) + + // When + userSwitcher.createAndSwitchToNewUser() + + // Then the login create for the new user still reports the subscription disabled + val newPushModel = mocks.subscriptionModelStore!!.list().first { it.type == SubscriptionType.PUSH } + newPushModel.restApiDisabledReason shouldBe SubscriptionStatus.DISABLED_FROM_REST_API.value + val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(newPushModel) + enabled shouldBe false + status shouldBe SubscriptionStatus.DISABLED_FROM_REST_API + } + test("initUser with forceCreateUser creates new user") { // Given val mocks = Mocks() 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 072a6f2213..13b4af7187 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 @@ -529,4 +529,99 @@ class RefreshUserOperationExecutorTests : FunSpec({ mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) } } + + test("push self-heal: does NOT enqueue follow-up op when server was disabled through the REST API") { + // Given: server says push is disabled with the REST API code, local view says enabled + val (executor, cachedPushSubscriptionModel, _) = + buildSelfHealHarness( + serverPushEnabled = false, + serverNotificationTypes = SubscriptionStatus.DISABLED_FROM_REST_API.value, + localOptedIn = true, + localStatus = SubscriptionStatus.SUBSCRIBED, + localAddress = onDevicePushToken, + ) + + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + + // Then no follow-up op, and the disable is recorded on the cached push model + response.result shouldBe ExecutionResult.SUCCESS + response.operations shouldBe null + cachedPushSubscriptionModel.restApiDisabledReason shouldBe SubscriptionStatus.DISABLED_FROM_REST_API.value + } + + test("push self-heal: still re-asserts local truth when the server reports another disabled code") { + // Any disabled code other than -31 stays device-recoverable + val (executor, cachedPushSubscriptionModel, _) = + buildSelfHealHarness( + serverPushEnabled = false, + serverNotificationTypes = -2, + localOptedIn = true, + localStatus = SubscriptionStatus.SUBSCRIBED, + localAddress = onDevicePushToken, + ) + + val originalLogLevel = Logging.logLevel + Logging.logLevel = LogLevel.NONE + try { + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + + // Then the self-heal op is emitted and nothing is recorded as a REST API disable + response.result shouldBe ExecutionResult.SUCCESS + response.operations?.count() shouldBe 1 + (response.operations!![0] is UpdateSubscriptionOperation) shouldBe true + cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 + } finally { + Logging.logLevel = originalLogLevel + } + } + + test("push refresh: clears a recorded REST API disable when the server reports another code") { + // Given: -31 recorded locally, server now reports a different code + val (executor, cachedPushSubscriptionModel, _) = + buildSelfHealHarness( + serverPushEnabled = false, + serverNotificationTypes = -2, + localOptedIn = true, + localStatus = SubscriptionStatus.SUBSCRIBED, + localAddress = onDevicePushToken, + ) + cachedPushSubscriptionModel.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + + val originalLogLevel = Logging.logLevel + Logging.logLevel = LogLevel.NONE + try { + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + + // Then the mirror clears and the self-heal still re-asserts local truth + response.result shouldBe ExecutionResult.SUCCESS + cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 + response.operations?.count() shouldBe 1 + } finally { + Logging.logLevel = originalLogLevel + } + } + + test("push refresh: clears a recorded REST API disable when the server reports enabled again") { + // Given: a locally recorded REST API disable, server now reports the subscription enabled + val (executor, cachedPushSubscriptionModel, _) = + buildSelfHealHarness( + serverPushEnabled = true, + serverNotificationTypes = 1, + localOptedIn = true, + localStatus = SubscriptionStatus.SUBSCRIBED, + localAddress = onDevicePushToken, + ) + cachedPushSubscriptionModel.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + + // Then + response.result shouldBe ExecutionResult.SUCCESS + response.operations shouldBe null + cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 + } }) 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 index 77c58b109f..0ce0ff48ed 100644 --- 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 @@ -715,6 +715,64 @@ class SubscriptionOperationExecutorTests : configModelStore.model.pushSubscriptionId shouldBe recovery.subscriptionId } + test("update subscription 404 recovery recreates from device truth, not the dead record's REST API disable") { + // Given: the cached model carries a recorded REST API disable for the record that 404s + val mockSubscriptionBackendService = mockk() + coEvery { mockSubscriptionBackendService.updateSubscription(any(), any(), any()) } throws BackendException(404) + + val mockSubscriptionsModelStore = mockk() + val cachedSubscriptionModel = + SubscriptionModel().apply { + id = remoteSubscriptionId + type = SubscriptionType.PUSH + address = "pushToken2" + optedIn = true + restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + } + every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns cachedSubscriptionModel + + val configModelStore = MockHelper.configModelStore().also { it.model.pushSubscriptionId = remoteSubscriptionId } + val mockBuildUserService = mockk() + + val subscriptionOperationExecutor = + SubscriptionOperationExecutor( + mockSubscriptionBackendService, + MockHelper.deviceService(), + AndroidMockHelper.applicationService(), + mockSubscriptionsModelStore, + configModelStore, + mockBuildUserService, + getNewRecordState(), + mockConsistencyManager, + getJwtTokenStore(), getIdentityVerificationService(), + ) + + // The queued op is the -31 echo for the now-deleted record + val operations = + listOf( + UpdateSubscriptionOperation( + appId, + remoteOneSignalId, + "ext-1", + remoteSubscriptionId, + SubscriptionType.PUSH, + false, + "pushToken2", + SubscriptionStatus.DISABLED_FROM_REST_API, + ), + ) + + // When + val response = subscriptionOperationExecutor.execute(operations) + + // Then the recovery create is born from device truth and the dead record's disable is gone + response.result shouldBe ExecutionResult.FAIL_NORETRY + val recovery = response.operations!!.first() as CreateSubscriptionOperation + recovery.enabled shouldBe true + recovery.status shouldBe SubscriptionStatus.SUBSCRIBED + cachedSubscriptionModel.restApiDisabledReason shouldBe 0 + } + test("update subscription fails with retry when the backend returns MISSING, when isInMissingRetryWindow") { // Given val mockSubscriptionBackendService = mockk() diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt index a127a250e9..762da795a9 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt @@ -9,7 +9,10 @@ import com.onesignal.core.internal.application.IApplicationService import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.session.internal.session.ISessionService +import com.onesignal.user.internal.PushSubscription import com.onesignal.user.internal.Subscription +import com.onesignal.user.internal.operations.UpdateSubscriptionOperation +import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager import com.onesignal.user.subscriptions.ISmsSubscription import io.kotest.core.spec.style.FunSpec @@ -24,6 +27,7 @@ import io.mockk.mockk import io.mockk.runs import io.mockk.spyk import io.mockk.verify +import org.json.JSONObject class SubscriptionManagerTests : FunSpec({ @@ -681,7 +685,6 @@ class SubscriptionManagerTests : FunSpec({ listOf( SubscriptionStatus.NO_PERMISSION, SubscriptionStatus.UNSUBSCRIBE, - SubscriptionStatus.DISABLED_FROM_REST_API_DEFAULT_REASON, ) for (status in nonRetryableStatuses) { @@ -799,8 +802,67 @@ class SubscriptionManagerTests : FunSpec({ SubscriptionStatus.INVALID_FCM_SENDER_ID, SubscriptionStatus.OUTDATED_GOOGLE_PLAY_SERVICES_APP, SubscriptionStatus.HMS_ARGUMENTS_INVALID, - SubscriptionStatus.DISABLED_FROM_REST_API_DEFAULT_REASON, + SubscriptionStatus.DISABLED_FROM_REST_API, SubscriptionStatus.ERROR, ).forEach { it.isRetryableTokenError shouldBe false } } + + test("status persisted under an unknown enum name reads as SUBSCRIBED instead of throwing") { + // Models persist enum properties by name; a cached model written under an enum case this + // version does not have must still load. + val model = SubscriptionModel() + model.initializeFromJson( + JSONObject() + .put("id", "subscription1") + .put("status", "STATUS_UNKNOWN_TO_THIS_VERSION"), + ) + + model.status shouldBe SubscriptionStatus.SUBSCRIBED + } + + test("operation status persisted under an unknown enum name reads as SUBSCRIBED instead of throwing") { + // Operation batches persist by enum name like models; an unknown name must not drop the batch. + val operation = UpdateSubscriptionOperation() + operation.initializeFromJson(JSONObject().put("status", "STATUS_UNKNOWN_TO_THIS_VERSION")) + + operation.status shouldBe SubscriptionStatus.SUBSCRIBED + } + + test("getSubscriptionEnabledAndStatus reports a REST API disable back to the server") { + // Given a push subscription the server disabled through the REST API + val pushSubscription = SubscriptionModel() + pushSubscription.id = "subscription1" + pushSubscription.type = SubscriptionType.PUSH + pushSubscription.address = "pushToken" + pushSubscription.status = SubscriptionStatus.SUBSCRIBED + pushSubscription.optedIn = true + pushSubscription.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + + // When + val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscription) + + // Then + enabled shouldBe false + status shouldBe SubscriptionStatus.DISABLED_FROM_REST_API + } + + test("optIn clears a REST API disable so the update re-enables the subscription") { + // Given a push subscription the server disabled through the REST API + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = "subscription1" + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.address = "pushToken" + pushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED + pushSubscriptionModel.optedIn = true + pushSubscriptionModel.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + + // When + PushSubscription(pushSubscriptionModel).optIn() + + // Then + pushSubscriptionModel.restApiDisabledReason shouldBe 0 + val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscriptionModel) + enabled shouldBe true + status shouldBe SubscriptionStatus.SUBSCRIBED + } }) From 9807ac1c5d9ab799b0e2cb18d955a0b195d45073 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 2 Sep 2026 09:34:14 -0700 Subject: [PATCH 02/11] fix: start subscription recovery fresh and document optedIn semantics The update-404 recovery now starts from device truth even when the cached model is missing, replaceAll carries restApiDisabledReason across the push model copy, and the user-404 rebuild is covered by tests. IPushSubscription.optedIn documents that it reflects the user's preference and permission rather than a server-side disable, and the detekt baseline is regenerated. A session-start device-metadata write that precedes RefreshUser can still send enabled=true once before the server's disable is learned; that bounded window is accepted, matching iOS. --- OneSignalSDK/detekt/detekt-baseline-core.xml | 17 +---- .../SubscriptionOperationExecutor.kt | 12 ++- .../subscriptions/SubscriptionModelStore.kt | 1 + .../user/subscriptions/IPushSubscription.kt | 3 +- .../builduser/RebuildUserServiceTests.kt | 75 +++++++++++++++++++ 5 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt diff --git a/OneSignalSDK/detekt/detekt-baseline-core.xml b/OneSignalSDK/detekt/detekt-baseline-core.xml index d80e7a8aac..946f84ba53 100644 --- a/OneSignalSDK/detekt/detekt-baseline-core.xml +++ b/OneSignalSDK/detekt/detekt-baseline-core.xml @@ -187,7 +187,6 @@ InstanceOfCheckForException:HttpClient.kt$HttpClient$t is UnknownHostException LongMethod:ApplicationService.kt$ApplicationService$override suspend fun waitUntilSystemConditionsAvailable(): Boolean LongMethod:ConfigModelStoreListener.kt$ConfigModelStoreListener$private fun fetchParams() - LongMethod:FeatureFlagsBackendService.kt$FeatureFlagsBackendService$override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome LongMethod:HttpClient.kt$HttpClient$private suspend fun makeRequestIODispatcher( url: String, method: String?, jsonBody: JSONObject?, timeout: Int, headers: OptionalHeaders?, ): HttpResponse LongMethod:IdentityOperationExecutor.kt$IdentityOperationExecutor$override suspend fun execute(operations: List<Operation>): ExecutionResponse LongMethod:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun createUser( createUserOperation: LoginUserOperation, operations: List<Operation>, ): ExecutionResponse @@ -207,6 +206,7 @@ LongMethod:TrackGooglePurchase.kt$TrackGooglePurchase$private fun queryBoughtItems() LongMethod:TrackGooglePurchase.kt$TrackGooglePurchase$private fun sendPurchases( skusToAdd: ArrayList<String>, newPurchaseTokens: ArrayList<String>, ) LongMethod:UpdateUserOperationExecutor.kt$UpdateUserOperationExecutor$override suspend fun execute(operations: List<Operation>): ExecutionResponse + LongParameterList:CrashDirCleanup.kt$( label: String, path: String, entries: List<CrashDirEntry>, nowMs: Long, maxSample: Int, ownedSuffix: String = CRASH_OWNED_SUFFIX, ) LongParameterList:CreateSubscriptionOperation.kt$CreateSubscriptionOperation$(appId: String, onesignalId: String, externalId: String?, subscriptionId: String, type: SubscriptionType, enabled: Boolean, address: String, status: SubscriptionStatus) LongParameterList:ICustomEventBackendService.kt$ICustomEventBackendService$( appId: String, onesignalId: String, externalId: String?, timestamp: Long, eventName: String, eventProperties: String?, metadata: CustomEventMetadata, jwt: String? = null, ) LongParameterList:IDatabase.kt$IDatabase$( table: String, columns: Array<String>? = null, whereClause: String? = null, whereArgs: Array<String>? = null, groupBy: String? = null, having: String? = null, orderBy: String? = null, limit: String? = null, action: (ICursor) -> Unit, ) @@ -253,7 +253,7 @@ MagicNumber:PermissionsActivity.kt$PermissionsActivity$23 MagicNumber:RefreshUserOperationExecutor.kt$RefreshUserOperationExecutor$404 MagicNumber:SessionListener.kt$SessionListener$1000 - MagicNumber:SubscriptionModel.kt$SubscriptionStatus.DISABLED_FROM_REST_API_DEFAULT_REASON$30 + MagicNumber:SubscriptionModel.kt$SubscriptionStatus.DISABLED_FROM_REST_API$31 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.ERROR$9999 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_AUTHENTICATION_FAILED$29 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_OTHER$11 @@ -304,7 +304,7 @@ ReturnCount:ConfigModel.kt$ConfigModel$override fun createModelForProperty( property: String, jsonObject: JSONObject, ): Model? ReturnCount:ExecutorsIvExtensions.kt$internal fun resolveIvBackendParams( op: Operation, onesignalId: String, jwtTokenStore: JwtTokenStore, ivBehaviorActive: Boolean, ): IvBackendParams ReturnCount:ExecutorsIvExtensions.kt$internal fun resolveIvJwt( op: Operation, jwtTokenStore: JwtTokenStore, ivBehaviorActive: Boolean, ): String? - ReturnCount:FeatureFlagsBackendService.kt$FeatureFlagsBackendService$override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome + ReturnCount:FeatureFlagsRefreshService.kt$FeatureFlagsRefreshService$private suspend fun fetchAndApply(appId: String) ReturnCount:HttpClient.kt$HttpClient$private suspend fun makeRequest( url: String, method: String?, jsonBody: JSONObject?, timeout: Int, headers: OptionalHeaders?, ): HttpResponse ReturnCount:IdentityOperationExecutor.kt$IdentityOperationExecutor$override suspend fun execute(operations: List<Operation>): ExecutionResponse ReturnCount:JSONUtils.kt$JSONUtils$fun compareJSONArrays( jsonArray1: JSONArray?, jsonArray2: JSONArray?, ): Boolean @@ -317,7 +317,6 @@ ReturnCount:Model.kt$Model$protected fun getOptIntProperty( name: String, create: (() -> Int?)? = null, ): Int? ReturnCount:Model.kt$Model$protected fun getOptLongProperty( name: String, create: (() -> Long?)? = null, ): Long? ReturnCount:Model.kt$Model$protected inline fun <reified T : Enum<T>> getOptEnumProperty(name: String): T? - ReturnCount:OneSignalImp.kt$OneSignalImp$private fun internalInit( context: Context, appId: String?, ): Boolean ReturnCount:OperationModelStore.kt$OperationModelStore$override fun create(jsonObject: JSONObject?): Operation? ReturnCount:OperationModelStore.kt$OperationModelStore$private fun isValidOperation(jsonObject: JSONObject): Boolean ReturnCount:OperationRepo.kt$OperationRepo$private fun shouldSuppressAnonymousOp(op: Operation): Boolean @@ -350,7 +349,6 @@ SwallowedException:PreferencesService.kt$PreferencesService$t: Throwable SwallowedException:SyncJobService.kt$SyncJobService$e: Exception SwallowedException:TrackGooglePurchase.kt$TrackGooglePurchase.Companion$t: Throwable - ThrowsCount:OneSignalImp.kt$OneSignalImp$private suspend fun waitUntilInitInternal(operationName: String? = null) TooGenericExceptionCaught:AndroidUtils.kt$AndroidUtils$e: Throwable TooGenericExceptionCaught:DeviceUtils.kt$DeviceUtils$t: Throwable TooGenericExceptionCaught:FeatureFlagsRefreshService.kt$FeatureFlagsRefreshService$e: Exception @@ -359,6 +357,7 @@ TooGenericExceptionCaught:JSONUtils.kt$JSONUtils$t: Throwable TooGenericExceptionCaught:Logging.kt$Logging$t: Throwable TooGenericExceptionCaught:OneSignalDispatchers.kt$OneSignalDispatchers$e: Exception + TooGenericExceptionCaught:OneSignalDispatchers.kt$OneSignalDispatchers.Pools$e: Exception TooGenericExceptionCaught:OperationRepo.kt$OperationRepo$e: Throwable TooGenericExceptionCaught:PreferenceStoreFix.kt$PreferenceStoreFix$e: Throwable TooGenericExceptionCaught:PreferencesService.kt$PreferencesService$e: Throwable @@ -628,15 +627,7 @@ UnusedPrivateMember:JSONUtils.kt$JSONUtils$`object`: Any UnusedPrivateMember:OSDatabase.kt$OSDatabase.Companion$private const val FLOAT_TYPE = " FLOAT" UnusedPrivateMember:OperationRepo.kt$OperationRepo$private val _time: ITime - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("'initWithContext failed' before 'login'") - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("'initWithContext failed' before 'logout'") UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("'initWithContext failed' before 'updateUserJwt'") - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'addUserJwtInvalidatedListener'") - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'login'") - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'logout'") - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'removeUserJwtInvalidatedListener'") - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'updateUserJwt'") - UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before use") UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw initFailureException ?: IllegalStateException("Initialization failed. Cannot proceed.") 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 index 693c318773..87f545f14b 100644 --- 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 @@ -35,6 +35,7 @@ import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelSt import com.onesignal.user.internal.operations.impl.states.NewRecordsState 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 internal class SubscriptionOperationExecutor( @@ -281,7 +282,7 @@ internal class SubscriptionOperationExecutor( ) val (recoveryEnabled, recoveryStatus) = recoveryModel?.let { SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(it) } - ?: Pair(lastOperation.enabled, lastOperation.status) + ?: freshStartWithoutDeadDisable(lastOperation) if (_configModelStore.model.pushSubscriptionId == staleSubscriptionId) { _configModelStore.model.pushSubscriptionId = recoveryLocalId } @@ -388,6 +389,15 @@ internal class SubscriptionOperationExecutor( return ExecutionResponse(ExecutionResult.SUCCESS) } + /** The failed op's enabled/status, minus a REST API disable that belonged to the dead record. */ + private fun freshStartWithoutDeadDisable(operation: UpdateSubscriptionOperation): Pair { + return if (operation.status == SubscriptionStatus.DISABLED_FROM_REST_API) { + Pair(true, SubscriptionStatus.SUBSCRIBED) + } else { + Pair(operation.enabled, operation.status) + } + } + companion object { const val CREATE_SUBSCRIPTION = "create-subscription" const val UPDATE_SUBSCRIPTION = "update-subscription" diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt index 089d2bc881..a46b0ad68b 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt @@ -27,6 +27,7 @@ open class SubscriptionModelStore(prefs: IPreferencesService) : SimpleModelStore model.carrier = existingPushModel.carrier model.appVersion = existingPushModel.appVersion model.status = existingPushModel.status + model.restApiDisabledReason = existingPushModel.restApiDisabledReason } break } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt index 1430141f96..8b505f5688 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt @@ -15,7 +15,8 @@ interface IPushSubscription : ISubscription { * Whether the user of this subscription is opted-in to received notifications. When true, * the user is able to receive notifications through this subscription. Otherwise, the * user will not receive notifications through this subscription (even when the user has - * granted app permission). + * granted app permission). This reflects the user's preference and app permission only; a + * subscription the app owner disabled through the REST API still reports true here. */ val optedIn: Boolean diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt new file mode 100644 index 0000000000..ae42bc911f --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt @@ -0,0 +1,75 @@ +package com.onesignal.user.internal.builduser + +import com.onesignal.mocks.MockHelper +import com.onesignal.user.internal.builduser.impl.RebuildUserService +import com.onesignal.user.internal.operations.CreateSubscriptionOperation +import com.onesignal.user.internal.operations.LoginUserOperation +import com.onesignal.user.internal.operations.RefreshUserOperation +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 +import io.mockk.every +import io.mockk.mockk + +class RebuildUserServiceTests : FunSpec({ + val appId = "appId" + val onesignalId = "onesignalId" + val subscriptionId = "subscriptionId" + + fun buildService(pushModel: SubscriptionModel?): RebuildUserService { + val subscriptionModelStore = mockk() + every { subscriptionModelStore.list() } returns listOfNotNull(pushModel) + every { subscriptionModelStore.get(any()) } returns pushModel + return RebuildUserService( + MockHelper.identityModelStore { it.onesignalId = onesignalId }, + MockHelper.propertiesModelStore { it.onesignalId = onesignalId }, + subscriptionModelStore, + MockHelper.configModelStore { it.pushSubscriptionId = subscriptionId }, + ) + } + + test("rebuild recreates a REST-API-disabled push subscription from device truth") { + // Given: the records being rebuilt are gone, so the recorded disable goes with them + val pushModel = + SubscriptionModel().apply { + id = subscriptionId + type = SubscriptionType.PUSH + address = "pushToken" + optedIn = true + status = SubscriptionStatus.SUBSCRIBED + restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + } + val service = buildService(pushModel) + + // When + val operations = service.getRebuildOperationsIfCurrentUser(appId, onesignalId)!! + + // Then + (operations[0] is LoginUserOperation) shouldBe true + val create = operations[1] as CreateSubscriptionOperation + create.subscriptionId shouldBe subscriptionId + create.enabled shouldBe true + create.status shouldBe SubscriptionStatus.SUBSCRIBED + (operations[2] is RefreshUserOperation) shouldBe true + pushModel.restApiDisabledReason shouldBe 0 + } + + test("rebuild without a push subscription emits only the login and refresh") { + val service = buildService(null) + + val operations = service.getRebuildOperationsIfCurrentUser(appId, onesignalId)!! + + operations.size shouldBe 2 + (operations[0] is LoginUserOperation) shouldBe true + (operations[1] is RefreshUserOperation) shouldBe true + } + + test("rebuild returns null when the current user is no longer the one that needs rebuilding") { + val service = buildService(null) + + service.getRebuildOperationsIfCurrentUser(appId, "otherOnesignalId") shouldBe null + } +}) From 91a2c572b666379daada59260dd1e69db42c2a5a Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 4 Sep 2026 09:36:44 -0700 Subject: [PATCH 03/11] fix: keep an opt-in over a stale REST API disable report optIn() always sends a subscription update, but a RefreshUser fetch queued before that update still reports the disable the opt-in cleared. Recording that stale -31 again meant the next device update re-sent it and the next fetch read it back, silently undoing the opt-in. The push model now carries an in-memory flag set by optIn() and cleared when the server reports any state other than a REST API disable. While it is set, RefreshUser leaves the recorded reason alone. The flag arms on every opt-in rather than only when a disable was already recorded, because the same race exists on the first fetch after the customer disables the subscription. --- .../user/internal/PushSubscription.kt | 4 ++- .../executors/RefreshUserOperationExecutor.kt | 15 ++++++++--- .../subscriptions/SubscriptionModel.kt | 10 +++++++ .../RefreshUserOperationExecutorTests.kt | 26 ++++++++++++++++++- .../subscriptions/SubscriptionManagerTests.kt | 17 ++++++++++++ 5 files changed, 67 insertions(+), 5 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt index 0a380bf0ff..69a967789c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt @@ -23,7 +23,9 @@ internal open class PushSubscription( override fun optIn() { // A deliberate opt-in overrides a REST API disable; clearing it with a NORMAL-tagged - // change drives a subscription update that re-enables it on the server. + // change drives a subscription update that re-enables it on the server. The flag keeps + // a fetch that started before that update went out from recording the disable again. + model.restApiDisableClearedByUser = true if (model.restApiDisabledReason != 0) { model.restApiDisabledReason = 0 } 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 0ce35222eb..b2254ed082 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 @@ -250,16 +250,25 @@ internal class RefreshUserOperationExecutor( /** * Records or clears the server's REST API disable state on the cached push model. Only that * state is server-owned; the device stays the source of truth for the rest of the push model, - * which is why push subscriptions are otherwise not hydrated from the backend. + * which is why push subscriptions are otherwise not hydrated from the backend. An opt-in whose + * update has not reached the server yet outranks a fetch that still reports the disable it cleared. */ private fun hydrateRestApiDisableState( serverSubscription: SubscriptionObject, pushSubscriptionId: String, ) { - val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionId) ?: return - val serverTypes = serverSubscription.notificationTypes ?: return + val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionId) + val serverTypes = serverSubscription.notificationTypes + if (cachedPushSubscriptionModel == null || serverTypes == null) return // The recorded reason mirrors the server's field: -31 records, any other reported value clears. val target = if (SubscriptionStatus.isRestApiDisable(serverTypes)) serverTypes else 0 + if (target == 0) { + cachedPushSubscriptionModel.restApiDisableClearedByUser = false + } else if (cachedPushSubscriptionModel.restApiDisableClearedByUser) { + // This fetch predates the opt-in's update, so it reports the state that update replaces. + Logging.debug("RefreshUserOperationExecutor: keeping an opt-in over a stale REST API disable report") + return + } if (cachedPushSubscriptionModel.restApiDisabledReason != target) { cachedPushSubscriptionModel.setIntProperty( SubscriptionModel::restApiDisabledReason.name, diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt index 3bf0f5c4a4..c7a9b887e2 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt @@ -157,6 +157,16 @@ class SubscriptionModel : Model() { setIntProperty(::restApiDisabledReason.name, value) } + /** + * True from [IPushSubscription.optIn] until the server reports this subscription in any state + * other than a REST API disable. Every opt-in sends a subscription update, and a fetch that + * started before that update went out still reports the disable the opt-in cleared; while + * this is set, RefreshUser leaves [restApiDisabledReason] alone instead of recording that + * stale answer. Memory only, since a fresh process has no update in flight to protect. + */ + @Volatile + var restApiDisableClearedByUser: Boolean = false + var type: SubscriptionType get() = getEnumProperty(::type.name) set(value) { 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 13b4af7187..abce1fd27e 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 @@ -615,13 +615,37 @@ class RefreshUserOperationExecutorTests : FunSpec({ localAddress = onDevicePushToken, ) cachedPushSubscriptionModel.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + cachedPushSubscriptionModel.restApiDisableClearedByUser = true // When val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) - // Then + // Then the mirror clears and the opt-in's precedence over stale reports ends + response.result shouldBe ExecutionResult.SUCCESS + response.operations shouldBe null + cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 + cachedPushSubscriptionModel.restApiDisableClearedByUser shouldBe false + } + + test("push refresh: keeps an opt-in over a fetch that still reports the REST API disable it cleared") { + // Given: optIn() ran while this fetch was pending, so the server still reports -31 + val (executor, cachedPushSubscriptionModel, _) = + buildSelfHealHarness( + serverPushEnabled = false, + serverNotificationTypes = SubscriptionStatus.DISABLED_FROM_REST_API.value, + localOptedIn = true, + localStatus = SubscriptionStatus.SUBSCRIBED, + localAddress = onDevicePushToken, + ) + cachedPushSubscriptionModel.restApiDisableClearedByUser = true + + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + + // Then the stale disable is not recorded, the flag stays, and no self-heal fires response.result shouldBe ExecutionResult.SUCCESS response.operations shouldBe null cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 + cachedPushSubscriptionModel.restApiDisableClearedByUser shouldBe true } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt index 762da795a9..24b1b1c606 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt @@ -861,8 +861,25 @@ class SubscriptionManagerTests : FunSpec({ // Then pushSubscriptionModel.restApiDisabledReason shouldBe 0 + pushSubscriptionModel.restApiDisableClearedByUser shouldBe true val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscriptionModel) enabled shouldBe true status shouldBe SubscriptionStatus.SUBSCRIBED } + + test("optIn takes precedence over a pending fetch even when no REST API disable was recorded") { + // Given a push subscription with no recorded REST API disable + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = "subscription1" + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.address = "pushToken" + pushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED + pushSubscriptionModel.optedIn = true + + // When + PushSubscription(pushSubscriptionModel).optIn() + + // Then the flag is set, since every opt-in sends an update a pending fetch may predate + pushSubscriptionModel.restApiDisableClearedByUser shouldBe true + } }) From 3ff9a2641a3ec63c169ce48a9da9f76b3e8db556 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 9 Sep 2026 12:00:58 -0700 Subject: [PATCH 04/11] feat: treat a dashboard unsubscribe (-22) as a remote disable The server reports notification_types -22 when someone turns a subscription off by hand from the dashboard. That means the same thing as -31, disabled through the REST API, so both codes now suppress outgoing subscription payloads the same way and neither is derived from device state. The two codes stay distinct. SubscriptionModel.remoteDisabledReason records whichever one the server sent, and SubscriptionModelStoreListener maps it back to its own status rather than reporting every remote disable as -31. The widened check also covers the optedIn derivation on refresh, the stuck-subscription self-heal guard, and the dead-record recovery in SubscriptionOperationExecutor. Renamed isRestApiDisable to isRemoteDisable and restApiDisabledReason to remoteDisabledReason, since "REST API" no longer describes the concept. The property name doubles as the persistence key, but no release has written the old name, so this needs no migration. --- OneSignalSDK/detekt/detekt-baseline-core.xml | 1 + .../user/internal/PushSubscription.kt | 8 +- .../onesignal/user/internal/UserSwitcher.kt | 2 +- .../builduser/impl/RebuildUserService.kt | 6 +- .../executors/RefreshUserOperationExecutor.kt | 29 +++--- .../SubscriptionOperationExecutor.kt | 8 +- .../SubscriptionModelStoreListener.kt | 10 +- .../subscriptions/SubscriptionModel.kt | 59 ++++++++---- .../subscriptions/SubscriptionModelStore.kt | 2 +- .../user/subscriptions/IPushSubscription.kt | 3 +- .../user/internal/UserSwitcherTests.kt | 10 +- .../builduser/RebuildUserServiceTests.kt | 6 +- .../RefreshUserOperationExecutorTests.kt | 72 ++++++++------- .../SubscriptionOperationExecutorTests.kt | 8 +- .../subscriptions/SubscriptionManagerTests.kt | 92 ++++++++++++------- 15 files changed, 189 insertions(+), 127 deletions(-) diff --git a/OneSignalSDK/detekt/detekt-baseline-core.xml b/OneSignalSDK/detekt/detekt-baseline-core.xml index 649921551d..743ae6ca39 100644 --- a/OneSignalSDK/detekt/detekt-baseline-core.xml +++ b/OneSignalSDK/detekt/detekt-baseline-core.xml @@ -269,6 +269,7 @@ MagicNumber:SubscriptionModel.kt$SubscriptionStatus.HMS_ARGUMENTS_INVALID$26 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.HMS_TOKEN_TIMEOUT$25 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.INVALID_FCM_SENDER_ID$6 + MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MANUALLY_UNSUBSCRIBED$22 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MISSING_FIREBASE_FCM_LIBRARY$4 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MISSING_HMS_PUSHKIT_LIBRARY$28 MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MISSING_JETPACK_LIBRARY$3 diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt index 69a967789c..037648f51e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt @@ -22,12 +22,12 @@ internal open class PushSubscription( get() = model.optedIn && model.status != SubscriptionStatus.NO_PERMISSION override fun optIn() { - // A deliberate opt-in overrides a REST API disable; clearing it with a NORMAL-tagged + // A deliberate opt-in overrides a remote disable; clearing it with a NORMAL-tagged // change drives a subscription update that re-enables it on the server. The flag keeps // a fetch that started before that update went out from recording the disable again. - model.restApiDisableClearedByUser = true - if (model.restApiDisabledReason != 0) { - model.restApiDisabledReason = 0 + model.remoteDisableClearedByUser = true + if (model.remoteDisabledReason != 0) { + model.remoteDisabledReason = 0 } // we set `optedIn` using the lower level method so we can set `forceChange=true`, which // will result in *always* driving change notification. diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt index 3e1b7effae..5cf46983ef 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt @@ -68,7 +68,7 @@ class UserSwitcher( optedIn = currentPushSubscription?.optedIn ?: true address = currentPushSubscription?.address ?: "" status = currentPushSubscription?.status ?: SubscriptionStatus.NO_PERMISSION - restApiDisabledReason = currentPushSubscription?.restApiDisabledReason ?: 0 + remoteDisabledReason = currentPushSubscription?.remoteDisabledReason ?: 0 sdk = oneSignalUtils.sdkVersion deviceOS = this@UserSwitcher.deviceOS ?: "" carrier = carrierName ?: "" diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt index 9a0881a378..c88f0197b6 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/builduser/impl/RebuildUserService.kt @@ -56,7 +56,7 @@ class RebuildUserService( return operations } - // The server records this rebuild recreates no longer exist, so a recorded REST API + // The server records this rebuild recreates no longer exist, so a recorded remote // disable died with them; clear it and recreate from device truth. private fun buildPushRecoveryOperation( appId: String, @@ -65,11 +65,11 @@ class RebuildUserService( pushSubscription: SubscriptionModel, ): CreateSubscriptionOperation { _subscriptionsModelStore.get(pushSubscription.id)?.setIntProperty( - SubscriptionModel::restApiDisabledReason.name, + SubscriptionModel::remoteDisabledReason.name, 0, ModelChangeTags.HYDRATE, ) - pushSubscription.restApiDisabledReason = 0 + pushSubscription.remoteDisabledReason = 0 val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscription) return CreateSubscriptionOperation( appId, 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 b2254ed082..6fbd462abe 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 @@ -126,7 +126,7 @@ internal class RefreshUserOperationExecutor( } } subscriptionModel.optedIn = subscriptionModel.status != SubscriptionStatus.UNSUBSCRIBE && - subscriptionModel.status != SubscriptionStatus.DISABLED_FROM_REST_API + !SubscriptionStatus.isRemoteDisable(subscriptionModel.status.value) subscriptionModel.sdk = subscription.sdk ?: "" subscriptionModel.deviceOS = subscription.deviceOS ?: "" subscriptionModel.carrier = subscription.carrier ?: "" @@ -137,7 +137,7 @@ internal class RefreshUserOperationExecutor( if (subscriptionModel.type != SubscriptionType.PUSH) { subscriptionModels.add(subscriptionModel) } else if (subscription.id == pushSubscriptionIdFromConfig && pushSelfHealOperationForStuckSubscription == null) { - hydrateRestApiDisableState(subscription, pushSubscriptionIdFromConfig) + hydrateRemoteDisableState(subscription, pushSubscriptionIdFromConfig) // Self-heal for users stuck at "Never Subscribed". 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 @@ -220,10 +220,10 @@ internal class RefreshUserOperationExecutor( val (localEnabled, localStatus) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(cachedPushSubscriptionModel) val serverEnabled = (serverSubscription.enabled == true) && ((serverSubscription.notificationTypes ?: 0) > 0) - // A REST API disable is deliberate suppression, not the stuck-subscription drift this + // A remote disable is deliberate suppression, not the stuck-subscription drift this // self-heal exists for; leave it in place. - val serverDisabledViaRestApi = SubscriptionStatus.isRestApiDisable(serverSubscription.notificationTypes) - val divergent = localEnabled && !serverEnabled && !serverDisabledViaRestApi + val serverDisabledRemotely = SubscriptionStatus.isRemoteDisable(serverSubscription.notificationTypes) + val divergent = localEnabled && !serverEnabled && !serverDisabledRemotely return if (divergent) { Logging.info( @@ -248,30 +248,31 @@ internal class RefreshUserOperationExecutor( } /** - * Records or clears the server's REST API disable state on the cached push model. Only that + * Records or clears the server's remote disable state on the cached push model. Only that * state is server-owned; the device stays the source of truth for the rest of the push model, * which is why push subscriptions are otherwise not hydrated from the backend. An opt-in whose * update has not reached the server yet outranks a fetch that still reports the disable it cleared. */ - private fun hydrateRestApiDisableState( + private fun hydrateRemoteDisableState( serverSubscription: SubscriptionObject, pushSubscriptionId: String, ) { val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionId) val serverTypes = serverSubscription.notificationTypes if (cachedPushSubscriptionModel == null || serverTypes == null) return - // The recorded reason mirrors the server's field: -31 records, any other reported value clears. - val target = if (SubscriptionStatus.isRestApiDisable(serverTypes)) serverTypes else 0 + // The recorded reason mirrors the server's field verbatim, so -22 and -31 stay + // distinguishable; any other reported value clears. + val target = if (SubscriptionStatus.isRemoteDisable(serverTypes)) serverTypes else 0 if (target == 0) { - cachedPushSubscriptionModel.restApiDisableClearedByUser = false - } else if (cachedPushSubscriptionModel.restApiDisableClearedByUser) { + cachedPushSubscriptionModel.remoteDisableClearedByUser = false + } else if (cachedPushSubscriptionModel.remoteDisableClearedByUser) { // This fetch predates the opt-in's update, so it reports the state that update replaces. - Logging.debug("RefreshUserOperationExecutor: keeping an opt-in over a stale REST API disable report") + Logging.debug("RefreshUserOperationExecutor: keeping an opt-in over a stale remote disable report") return } - if (cachedPushSubscriptionModel.restApiDisabledReason != target) { + if (cachedPushSubscriptionModel.remoteDisabledReason != target) { cachedPushSubscriptionModel.setIntProperty( - SubscriptionModel::restApiDisabledReason.name, + SubscriptionModel::remoteDisabledReason.name, target, ModelChangeTags.HYDRATE, ) 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 index 87f545f14b..7e76e3ca81 100644 --- 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 @@ -273,10 +273,10 @@ internal class SubscriptionOperationExecutor( recoveryLocalId, ModelChangeTags.HYDRATE, ) - // The stale record died with any recorded REST API disable; recreate from + // The stale record died with any recorded remote disable; recreate from // device truth rather than the values frozen on the failed operation. recoveryModel?.setIntProperty( - SubscriptionModel::restApiDisabledReason.name, + SubscriptionModel::remoteDisabledReason.name, 0, ModelChangeTags.HYDRATE, ) @@ -389,9 +389,9 @@ internal class SubscriptionOperationExecutor( return ExecutionResponse(ExecutionResult.SUCCESS) } - /** The failed op's enabled/status, minus a REST API disable that belonged to the dead record. */ + /** The failed op's enabled/status, minus a remote disable that belonged to the dead record. */ private fun freshStartWithoutDeadDisable(operation: UpdateSubscriptionOperation): Pair { - return if (operation.status == SubscriptionStatus.DISABLED_FROM_REST_API) { + return if (SubscriptionStatus.isRemoteDisable(operation.status.value)) { Pair(true, SubscriptionStatus.SUBSCRIBED) } else { Pair(operation.enabled, operation.status) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt index 4b979f8ea0..8a22cedd8b 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt @@ -73,11 +73,13 @@ internal class SubscriptionModelStoreListener( val status: SubscriptionStatus val enabled: Boolean - // A REST API disable is server-owned; report it back rather than the device state so - // subscription payloads don't re-enable a suppressed subscription. - if (SubscriptionStatus.isRestApiDisable(model.restApiDisabledReason)) { + // A remote disable is server-owned; report the recorded code back rather than the + // device state so subscription payloads don't re-enable a suppressed subscription. + val remoteDisabledStatus = SubscriptionStatus.remoteDisableStatus(model.remoteDisabledReason) + + if (remoteDisabledStatus != null) { enabled = false - status = SubscriptionStatus.DISABLED_FROM_REST_API + status = remoteDisabledStatus } else if (model.optedIn && model.status == SubscriptionStatus.SUBSCRIBED && model.address.isNotEmpty()) { enabled = true status = SubscriptionStatus.SUBSCRIBED diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt index c7a9b887e2..6bd5a6f8c0 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt @@ -47,7 +47,10 @@ enum class SubscriptionStatus(val value: Int) { /** The subscription is not enabled due to any other FCM Exception, this can be retried */ FIREBASE_FCM_ERROR_MISC_EXCEPTION(-12), - // -13 to -24 reserved for other platforms + // -13 to -21, -23, and -24 reserved for other platforms + + /** The subscription is not enabled because it was unsubscribed by hand from the dashboard */ + MANUALLY_UNSUBSCRIBED(-22), /** The subscription is not enabled due the an HMS timeout, this can be retried */ HMS_TOKEN_TIMEOUT(-25), @@ -102,12 +105,31 @@ enum class SubscriptionStatus(val value: Int) { ) /** - * True when [value] is the code the server uses for a subscription disabled through the - * REST API, which is only -31. The SDK never derives it from device state, and - * server-reported error codes stay device-recoverable. + * The codes the server owns, meaning the app owner turned this subscription off remotely. + * The SDK never derives either from device state, and every other server-reported error + * code stays device-recoverable. + */ + private val REMOTE_DISABLES = + setOf( + MANUALLY_UNSUBSCRIBED, // -22 + DISABLED_FROM_REST_API, // -31 + ) + + /** + * The status for a remote-disable code, or `null` when [value] is not one. The two codes + * stay distinct in [SubscriptionModel.remoteDisabledReason] and on the wire, so callers + * report back the exact code the server sent rather than collapsing them. + */ + fun remoteDisableStatus(value: Int?): SubscriptionStatus? { + return REMOTE_DISABLES.firstOrNull { it.value == value } + } + + /** + * True when [value] is one of the codes for a subscription the app owner disabled + * remotely. The SDK treats them the same because both mean the server turned this off. */ - fun isRestApiDisable(value: Int?): Boolean { - return value == DISABLED_FROM_REST_API.value + fun isRemoteDisable(value: Int?): Boolean { + return remoteDisableStatus(value) != null } fun fromInt(value: Int): SubscriptionStatus? { @@ -145,27 +167,30 @@ class SubscriptionModel : Model() { } /** - * The server's REST API disable code (-31), or 0 when the server has not disabled this - * subscription. Hydrated by RefreshUser and never derived from device state; while set, - * [SubscriptionModelStoreListener] reports `enabled = false` with this status so subscription - * payloads don't re-enable a suppressed subscription. Cleared when the server reports any - * other state, or by [IPushSubscription.optIn]. + * The code for a subscription the app owner turned off remotely, either by hand from the + * dashboard ([SubscriptionStatus.MANUALLY_UNSUBSCRIBED], -22) or through the REST API + * ([SubscriptionStatus.DISABLED_FROM_REST_API], -31), or 0 when the server has not disabled + * this subscription. Both codes mean the same thing to the SDK but are recorded separately, so + * payloads echo back the one the server sent. Hydrated by RefreshUser and never derived from + * device state; while set, [SubscriptionModelStoreListener] reports `enabled = false` with the + * matching status so subscription payloads don't re-enable a suppressed subscription. Cleared + * when the server reports any other state, or by [IPushSubscription.optIn]. */ - var restApiDisabledReason: Int - get() = getIntProperty(::restApiDisabledReason.name) { 0 } + var remoteDisabledReason: Int + get() = getIntProperty(::remoteDisabledReason.name) { 0 } set(value) { - setIntProperty(::restApiDisabledReason.name, value) + setIntProperty(::remoteDisabledReason.name, value) } /** * True from [IPushSubscription.optIn] until the server reports this subscription in any state - * other than a REST API disable. Every opt-in sends a subscription update, and a fetch that + * other than a remote disable. Every opt-in sends a subscription update, and a fetch that * started before that update went out still reports the disable the opt-in cleared; while - * this is set, RefreshUser leaves [restApiDisabledReason] alone instead of recording that + * this is set, RefreshUser leaves [remoteDisabledReason] alone instead of recording that * stale answer. Memory only, since a fresh process has no update in flight to protect. */ @Volatile - var restApiDisableClearedByUser: Boolean = false + var remoteDisableClearedByUser: Boolean = false var type: SubscriptionType get() = getEnumProperty(::type.name) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt index a46b0ad68b..60294042b2 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt @@ -27,7 +27,7 @@ open class SubscriptionModelStore(prefs: IPreferencesService) : SimpleModelStore model.carrier = existingPushModel.carrier model.appVersion = existingPushModel.appVersion model.status = existingPushModel.status - model.restApiDisabledReason = existingPushModel.restApiDisabledReason + model.remoteDisabledReason = existingPushModel.remoteDisabledReason } break } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt index 8b505f5688..053b75816c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt @@ -16,7 +16,8 @@ interface IPushSubscription : ISubscription { * the user is able to receive notifications through this subscription. Otherwise, the * user will not receive notifications through this subscription (even when the user has * granted app permission). This reflects the user's preference and app permission only; a - * subscription the app owner disabled through the REST API still reports true here. + * subscription the app owner disabled remotely, from the dashboard or the REST API, still + * reports true here. */ val optedIn: Boolean diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt index 67be2bca48..f73c8caa53 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt @@ -258,7 +258,9 @@ class UserSwitcherTests : FunSpec({ verify(exactly = 1) { mockSubscriptionModelStore.add(any(), ModelChangeTags.NO_PROPOGATE) } } - test("createAndSwitchToNewUser carries a REST API disable onto the new push model") { + test("createAndSwitchToNewUser carries a remote disable onto the new push model") { + // Uses -22 rather than -31 so the assertions below also prove the exact recorded code + // survives the switch instead of every remote disable collapsing to one status. // Given val mocks = Mocks() val userSwitcher = mocks.createUserSwitcher() @@ -268,7 +270,7 @@ class UserSwitcherTests : FunSpec({ type = SubscriptionType.PUSH address = "test-token" optedIn = true - restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + remoteDisabledReason = SubscriptionStatus.MANUALLY_UNSUBSCRIBED.value } mocks.subscriptionModelStore!!.add(disabledPushModel, ModelChangeTags.NO_PROPOGATE) @@ -277,10 +279,10 @@ class UserSwitcherTests : FunSpec({ // Then the login create for the new user still reports the subscription disabled val newPushModel = mocks.subscriptionModelStore!!.list().first { it.type == SubscriptionType.PUSH } - newPushModel.restApiDisabledReason shouldBe SubscriptionStatus.DISABLED_FROM_REST_API.value + newPushModel.remoteDisabledReason shouldBe SubscriptionStatus.MANUALLY_UNSUBSCRIBED.value val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(newPushModel) enabled shouldBe false - status shouldBe SubscriptionStatus.DISABLED_FROM_REST_API + status shouldBe SubscriptionStatus.MANUALLY_UNSUBSCRIBED } test("initUser with forceCreateUser creates new user") { diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt index ae42bc911f..399f37b4dc 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/builduser/RebuildUserServiceTests.kt @@ -31,7 +31,7 @@ class RebuildUserServiceTests : FunSpec({ ) } - test("rebuild recreates a REST-API-disabled push subscription from device truth") { + test("rebuild recreates a remotely disabled push subscription from device truth") { // Given: the records being rebuilt are gone, so the recorded disable goes with them val pushModel = SubscriptionModel().apply { @@ -40,7 +40,7 @@ class RebuildUserServiceTests : FunSpec({ address = "pushToken" optedIn = true status = SubscriptionStatus.SUBSCRIBED - restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + remoteDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value } val service = buildService(pushModel) @@ -54,7 +54,7 @@ class RebuildUserServiceTests : FunSpec({ create.enabled shouldBe true create.status shouldBe SubscriptionStatus.SUBSCRIBED (operations[2] is RefreshUserOperation) shouldBe true - pushModel.restApiDisabledReason shouldBe 0 + pushModel.remoteDisabledReason shouldBe 0 } test("rebuild without a push subscription emits only the login and refresh") { 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 abce1fd27e..d4dbf73c87 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 @@ -530,28 +530,36 @@ class RefreshUserOperationExecutorTests : FunSpec({ } } - test("push self-heal: does NOT enqueue follow-up op when server was disabled through the REST API") { - // Given: server says push is disabled with the REST API code, local view says enabled - val (executor, cachedPushSubscriptionModel, _) = - buildSelfHealHarness( - serverPushEnabled = false, - serverNotificationTypes = SubscriptionStatus.DISABLED_FROM_REST_API.value, - localOptedIn = true, - localStatus = SubscriptionStatus.SUBSCRIBED, - localAddress = onDevicePushToken, - ) + // Both remote-disable codes mean "the app owner turned this off", so both suppress the + // self-heal, and each is recorded verbatim so the payload echoes back the code the server sent + // rather than a single collapsed one. + listOf( + SubscriptionStatus.MANUALLY_UNSUBSCRIBED, + SubscriptionStatus.DISABLED_FROM_REST_API, + ).forEach { remoteDisable -> + test("push self-heal: does NOT enqueue follow-up op when the server reports ${remoteDisable.value}") { + // Given: server says push is disabled with a remote-disable code, local view says enabled + val (executor, cachedPushSubscriptionModel, _) = + buildSelfHealHarness( + serverPushEnabled = false, + serverNotificationTypes = remoteDisable.value, + localOptedIn = true, + localStatus = SubscriptionStatus.SUBSCRIBED, + localAddress = onDevicePushToken, + ) - // When - val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) - // Then no follow-up op, and the disable is recorded on the cached push model - response.result shouldBe ExecutionResult.SUCCESS - response.operations shouldBe null - cachedPushSubscriptionModel.restApiDisabledReason shouldBe SubscriptionStatus.DISABLED_FROM_REST_API.value + // Then no follow-up op, and that exact code is recorded on the cached push model + response.result shouldBe ExecutionResult.SUCCESS + response.operations shouldBe null + cachedPushSubscriptionModel.remoteDisabledReason shouldBe remoteDisable.value + } } test("push self-heal: still re-asserts local truth when the server reports another disabled code") { - // Any disabled code other than -31 stays device-recoverable + // Any disabled code other than the remote-disable codes (-22, -31) stays device-recoverable val (executor, cachedPushSubscriptionModel, _) = buildSelfHealHarness( serverPushEnabled = false, @@ -567,17 +575,17 @@ class RefreshUserOperationExecutorTests : FunSpec({ // When val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) - // Then the self-heal op is emitted and nothing is recorded as a REST API disable + // Then the self-heal op is emitted and nothing is recorded as a remote disable response.result shouldBe ExecutionResult.SUCCESS response.operations?.count() shouldBe 1 (response.operations!![0] is UpdateSubscriptionOperation) shouldBe true - cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 + cachedPushSubscriptionModel.remoteDisabledReason shouldBe 0 } finally { Logging.logLevel = originalLogLevel } } - test("push refresh: clears a recorded REST API disable when the server reports another code") { + test("push refresh: clears a recorded remote disable when the server reports another code") { // Given: -31 recorded locally, server now reports a different code val (executor, cachedPushSubscriptionModel, _) = buildSelfHealHarness( @@ -587,7 +595,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ localStatus = SubscriptionStatus.SUBSCRIBED, localAddress = onDevicePushToken, ) - cachedPushSubscriptionModel.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + cachedPushSubscriptionModel.remoteDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value val originalLogLevel = Logging.logLevel Logging.logLevel = LogLevel.NONE @@ -597,15 +605,15 @@ class RefreshUserOperationExecutorTests : FunSpec({ // Then the mirror clears and the self-heal still re-asserts local truth response.result shouldBe ExecutionResult.SUCCESS - cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 + cachedPushSubscriptionModel.remoteDisabledReason shouldBe 0 response.operations?.count() shouldBe 1 } finally { Logging.logLevel = originalLogLevel } } - test("push refresh: clears a recorded REST API disable when the server reports enabled again") { - // Given: a locally recorded REST API disable, server now reports the subscription enabled + test("push refresh: clears a recorded remote disable when the server reports enabled again") { + // Given: a locally recorded remote disable, server now reports the subscription enabled val (executor, cachedPushSubscriptionModel, _) = buildSelfHealHarness( serverPushEnabled = true, @@ -614,8 +622,8 @@ class RefreshUserOperationExecutorTests : FunSpec({ localStatus = SubscriptionStatus.SUBSCRIBED, localAddress = onDevicePushToken, ) - cachedPushSubscriptionModel.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value - cachedPushSubscriptionModel.restApiDisableClearedByUser = true + cachedPushSubscriptionModel.remoteDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + cachedPushSubscriptionModel.remoteDisableClearedByUser = true // When val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) @@ -623,11 +631,11 @@ class RefreshUserOperationExecutorTests : FunSpec({ // Then the mirror clears and the opt-in's precedence over stale reports ends response.result shouldBe ExecutionResult.SUCCESS response.operations shouldBe null - cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 - cachedPushSubscriptionModel.restApiDisableClearedByUser shouldBe false + cachedPushSubscriptionModel.remoteDisabledReason shouldBe 0 + cachedPushSubscriptionModel.remoteDisableClearedByUser shouldBe false } - test("push refresh: keeps an opt-in over a fetch that still reports the REST API disable it cleared") { + test("push refresh: keeps an opt-in over a fetch that still reports the remote disable it cleared") { // Given: optIn() ran while this fetch was pending, so the server still reports -31 val (executor, cachedPushSubscriptionModel, _) = buildSelfHealHarness( @@ -637,7 +645,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ localStatus = SubscriptionStatus.SUBSCRIBED, localAddress = onDevicePushToken, ) - cachedPushSubscriptionModel.restApiDisableClearedByUser = true + cachedPushSubscriptionModel.remoteDisableClearedByUser = true // When val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) @@ -645,7 +653,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ // Then the stale disable is not recorded, the flag stays, and no self-heal fires response.result shouldBe ExecutionResult.SUCCESS response.operations shouldBe null - cachedPushSubscriptionModel.restApiDisabledReason shouldBe 0 - cachedPushSubscriptionModel.restApiDisableClearedByUser shouldBe true + cachedPushSubscriptionModel.remoteDisabledReason shouldBe 0 + cachedPushSubscriptionModel.remoteDisableClearedByUser 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 index 0ce0ff48ed..48ae3eda4d 100644 --- 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 @@ -715,8 +715,8 @@ class SubscriptionOperationExecutorTests : configModelStore.model.pushSubscriptionId shouldBe recovery.subscriptionId } - test("update subscription 404 recovery recreates from device truth, not the dead record's REST API disable") { - // Given: the cached model carries a recorded REST API disable for the record that 404s + test("update subscription 404 recovery recreates from device truth, not the dead record's remote disable") { + // Given: the cached model carries a recorded remote disable for the record that 404s val mockSubscriptionBackendService = mockk() coEvery { mockSubscriptionBackendService.updateSubscription(any(), any(), any()) } throws BackendException(404) @@ -727,7 +727,7 @@ class SubscriptionOperationExecutorTests : type = SubscriptionType.PUSH address = "pushToken2" optedIn = true - restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + remoteDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value } every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns cachedSubscriptionModel @@ -770,7 +770,7 @@ class SubscriptionOperationExecutorTests : val recovery = response.operations!!.first() as CreateSubscriptionOperation recovery.enabled shouldBe true recovery.status shouldBe SubscriptionStatus.SUBSCRIBED - cachedSubscriptionModel.restApiDisabledReason shouldBe 0 + cachedSubscriptionModel.remoteDisabledReason shouldBe 0 } test("update subscription fails with retry when the backend returns MISSING, when isInMissingRetryWindow") { diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt index 24b1b1c606..1c8c7a57c4 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt @@ -802,6 +802,7 @@ class SubscriptionManagerTests : FunSpec({ SubscriptionStatus.INVALID_FCM_SENDER_ID, SubscriptionStatus.OUTDATED_GOOGLE_PLAY_SERVICES_APP, SubscriptionStatus.HMS_ARGUMENTS_INVALID, + SubscriptionStatus.MANUALLY_UNSUBSCRIBED, SubscriptionStatus.DISABLED_FROM_REST_API, SubscriptionStatus.ERROR, ).forEach { it.isRetryableTokenError shouldBe false } @@ -828,47 +829,68 @@ class SubscriptionManagerTests : FunSpec({ operation.status shouldBe SubscriptionStatus.SUBSCRIBED } - test("getSubscriptionEnabledAndStatus reports a REST API disable back to the server") { - // Given a push subscription the server disabled through the REST API - val pushSubscription = SubscriptionModel() - pushSubscription.id = "subscription1" - pushSubscription.type = SubscriptionType.PUSH - pushSubscription.address = "pushToken" - pushSubscription.status = SubscriptionStatus.SUBSCRIBED - pushSubscription.optedIn = true - pushSubscription.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + test("SubscriptionStatus.isRemoteDisable is true only for the two server-owned disable codes") { + // -22 (unsubscribed by hand from the dashboard) and -31 (disabled through the REST API) are + // the only codes the app owner sets remotely. Every other negative code describes a device + // or delivery problem the device recovers from by re-asserting its own state, so widening + // this predicate would make the SDK stop re-enabling those subscriptions. + SubscriptionStatus.values().forEach { + SubscriptionStatus.isRemoteDisable(it.value) shouldBe + (it == SubscriptionStatus.MANUALLY_UNSUBSCRIBED || it == SubscriptionStatus.DISABLED_FROM_REST_API) + } + // 0 is the "nothing recorded" sentinel for remoteDisabledReason, not a disable. + SubscriptionStatus.isRemoteDisable(0) shouldBe false + SubscriptionStatus.isRemoteDisable(null) shouldBe false + } - // When - val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscription) + // Both codes are treated the same but recorded separately, so the payload reports back the + // exact code the server sent instead of collapsing -22 into -31. + listOf( + SubscriptionStatus.MANUALLY_UNSUBSCRIBED, + SubscriptionStatus.DISABLED_FROM_REST_API, + ).forEach { remoteDisable -> + test("getSubscriptionEnabledAndStatus reports a ${remoteDisable.value} disable back to the server") { + // Given a push subscription the app owner disabled remotely + val pushSubscription = SubscriptionModel() + pushSubscription.id = "subscription1" + pushSubscription.type = SubscriptionType.PUSH + pushSubscription.address = "pushToken" + pushSubscription.status = SubscriptionStatus.SUBSCRIBED + pushSubscription.optedIn = true + pushSubscription.remoteDisabledReason = remoteDisable.value - // Then - enabled shouldBe false - status shouldBe SubscriptionStatus.DISABLED_FROM_REST_API - } + // When + val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscription) - test("optIn clears a REST API disable so the update re-enables the subscription") { - // Given a push subscription the server disabled through the REST API - val pushSubscriptionModel = SubscriptionModel() - pushSubscriptionModel.id = "subscription1" - pushSubscriptionModel.type = SubscriptionType.PUSH - pushSubscriptionModel.address = "pushToken" - pushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED - pushSubscriptionModel.optedIn = true - pushSubscriptionModel.restApiDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value + // Then the recorded code round-trips rather than being reported as the other one + enabled shouldBe false + status shouldBe remoteDisable + } - // When - PushSubscription(pushSubscriptionModel).optIn() + test("optIn clears a ${remoteDisable.value} disable so the update re-enables the subscription") { + // Given a push subscription the app owner disabled remotely + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = "subscription1" + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.address = "pushToken" + pushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED + pushSubscriptionModel.optedIn = true + pushSubscriptionModel.remoteDisabledReason = remoteDisable.value - // Then - pushSubscriptionModel.restApiDisabledReason shouldBe 0 - pushSubscriptionModel.restApiDisableClearedByUser shouldBe true - val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscriptionModel) - enabled shouldBe true - status shouldBe SubscriptionStatus.SUBSCRIBED + // When + PushSubscription(pushSubscriptionModel).optIn() + + // Then + pushSubscriptionModel.remoteDisabledReason shouldBe 0 + pushSubscriptionModel.remoteDisableClearedByUser shouldBe true + val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscriptionModel) + enabled shouldBe true + status shouldBe SubscriptionStatus.SUBSCRIBED + } } - test("optIn takes precedence over a pending fetch even when no REST API disable was recorded") { - // Given a push subscription with no recorded REST API disable + test("optIn takes precedence over a pending fetch even when no remote disable was recorded") { + // Given a push subscription with no recorded remote disable val pushSubscriptionModel = SubscriptionModel() pushSubscriptionModel.id = "subscription1" pushSubscriptionModel.type = SubscriptionType.PUSH @@ -880,6 +902,6 @@ class SubscriptionManagerTests : FunSpec({ PushSubscription(pushSubscriptionModel).optIn() // Then the flag is set, since every opt-in sends an update a pending fetch may predate - pushSubscriptionModel.restApiDisableClearedByUser shouldBe true + pushSubscriptionModel.remoteDisableClearedByUser shouldBe true } }) From e880f2d3d250f03985156ea79c48294b87b5c513 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 9 Sep 2026 12:18:38 -0700 Subject: [PATCH 05/11] log when a remote disable code is recorded or cleared hydrateRemoteDisableState changed the model silently, so parsing -22 or -31 off the wire left no trace at any log level short of the raw HTTP body. Add a DEBUG line at the point the value actually changes. --- .../impl/executors/RefreshUserOperationExecutor.kt | 7 +++++++ 1 file changed, 7 insertions(+) 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 6fbd462abe..19b335b5ac 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 @@ -271,6 +271,13 @@ internal class RefreshUserOperationExecutor( return } if (cachedPushSubscriptionModel.remoteDisabledReason != target) { + Logging.debug( + if (target != 0) { + "RefreshUserOperationExecutor: recording remote disable $target for push subscription $pushSubscriptionId" + } else { + "RefreshUserOperationExecutor: clearing remote disable ${cachedPushSubscriptionModel.remoteDisabledReason} for push subscription $pushSubscriptionId" + }, + ) cachedPushSubscriptionModel.setIntProperty( SubscriptionModel::remoteDisabledReason.name, target, From 36a6e2ffd8f14b0bf35be68857c57c4f1a7d144f Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 9 Sep 2026 13:36:20 -0700 Subject: [PATCH 06/11] incorporate emote disable through optedIn optedIn documented that "the user is able to receive notifications through this subscription", and the public reference says it returns true when the subscription status is subscribed. Neither holds once the SDK respects a remote disable, because the disable now sticks instead of being flipped back on by the next routine update, and nothing else in the public API reveals it. An app syncing preferences through the REST API would read "subscribed" forever on a device receiving nothing. The disable is read from the model's recorded reason rather than its status, which stays device-owned, so a device-recoverable error code still reports opted in. optIn() already clears the reason, so a preference-center toggle reading false is not a dead end. The push observer already fires on the hydration write, so its payload now carries a real optedIn transition instead of an unchanged pair. --- .../user/internal/PushSubscription.kt | 7 ++- .../user/subscriptions/IPushSubscription.kt | 5 +- .../subscriptions/PushSubscriptionState.kt | 3 +- .../subscriptions/SubscriptionManagerTests.kt | 59 +++++++++++++++++++ 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt index 037648f51e..1b33b5de12 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/PushSubscription.kt @@ -19,7 +19,12 @@ internal open class PushSubscription( get() = model.address override val optedIn: Boolean - get() = model.optedIn && model.status != SubscriptionStatus.NO_PERMISSION + // A remote disable suppresses delivery just as surely as a missing permission or an + // opt-out, so it belongs in the same answer. Reported through the model's recorded reason + // rather than its status, which stays device-owned. + get() = model.optedIn && + model.status != SubscriptionStatus.NO_PERMISSION && + model.remoteDisabledReason == 0 override fun optIn() { // A deliberate opt-in overrides a remote disable; clearing it with a NORMAL-tagged diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt index 053b75816c..34b310982d 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/IPushSubscription.kt @@ -15,9 +15,8 @@ interface IPushSubscription : ISubscription { * Whether the user of this subscription is opted-in to received notifications. When true, * the user is able to receive notifications through this subscription. Otherwise, the * user will not receive notifications through this subscription (even when the user has - * granted app permission). This reflects the user's preference and app permission only; a - * subscription the app owner disabled remotely, from the dashboard or the REST API, still - * reports true here. + * granted app permission). This is false while the app owner has the subscription disabled + * remotely, from the dashboard or the REST API; [optIn] clears that suppression. */ val optedIn: Boolean diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/PushSubscriptionState.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/PushSubscriptionState.kt index f43dbdb8b6..f60dba08db 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/PushSubscriptionState.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/subscriptions/PushSubscriptionState.kt @@ -22,7 +22,8 @@ class PushSubscriptionState( * Whether the user of this subscription is opted-in to received notifications. When true, * the user is able to receive notifications through this subscription. Otherwise, the * user will not receive notifications through this subscription (even when the user has - * granted app permission). + * granted app permission). This is false while the app owner has the subscription disabled + * remotely, from the dashboard or the REST API. */ val optedIn: Boolean, ) { diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt index 1c8c7a57c4..acee1c0d08 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt @@ -867,6 +867,49 @@ class SubscriptionManagerTests : FunSpec({ status shouldBe remoteDisable } + test("optedIn reports false while a ${remoteDisable.value} disable is recorded") { + // A remote disable suppresses delivery, so the property clients read to decide whether + // push works must say so. Before this, a preference center showed "subscribed" on a + // device the app owner had turned off, and nothing in the public API revealed why. + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = "subscription1" + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.address = "pushToken" + pushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED + pushSubscriptionModel.optedIn = true + + val pushSubscription = PushSubscription(pushSubscriptionModel) + pushSubscription.optedIn shouldBe true + + // When the server's disable is recorded + pushSubscriptionModel.remoteDisabledReason = remoteDisable.value + + // Then + pushSubscription.optedIn shouldBe false + } + + test("refreshState carries a ${remoteDisable.value} disable into the observer payload") { + // The observer already fires on the hydration write; this pins the payload it carries, + // since the previous/current pair is built from refreshState. + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = "subscription1" + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.address = "pushToken" + pushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED + pushSubscriptionModel.optedIn = true + + val pushSubscription = PushSubscription(pushSubscriptionModel) + val previousState = pushSubscription.savedState + + // When + pushSubscriptionModel.remoteDisabledReason = remoteDisable.value + val currentState = pushSubscription.refreshState() + + // Then the observer sees a real transition rather than an unchanged pair + previousState.optedIn shouldBe true + currentState.optedIn shouldBe false + } + test("optIn clears a ${remoteDisable.value} disable so the update re-enables the subscription") { // Given a push subscription the app owner disabled remotely val pushSubscriptionModel = SubscriptionModel() @@ -883,12 +926,28 @@ class SubscriptionManagerTests : FunSpec({ // Then pushSubscriptionModel.remoteDisabledReason shouldBe 0 pushSubscriptionModel.remoteDisableClearedByUser shouldBe true + // The toggle a client drives off is not a dead end: opting in reports true again. + PushSubscription(pushSubscriptionModel).optedIn shouldBe true val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscriptionModel) enabled shouldBe true status shouldBe SubscriptionStatus.SUBSCRIBED } } + test("optedIn ignores a device-recoverable error status") { + // Only the two server-owned codes reach optedIn, and they arrive through + // remoteDisabledReason rather than status. A device-side delivery error is recoverable by + // re-asserting local state, so it must not read as an opt-out to the app. + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = "subscription1" + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.address = "pushToken" + pushSubscriptionModel.optedIn = true + pushSubscriptionModel.status = SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE + + PushSubscription(pushSubscriptionModel).optedIn shouldBe true + } + test("optIn takes precedence over a pending fetch even when no remote disable was recorded") { // Given a push subscription with no recorded remote disable val pushSubscriptionModel = SubscriptionModel() From a420653caa68abd6e117b21034faa68c13c1536b Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 9 Sep 2026 13:44:45 -0700 Subject: [PATCH 07/11] test: cover the push observer end to end for a hydrated remote disable The observer callback goes out through Dispatchers.Main, so nothing in this repo covered an IPushSubscriptionObserver receiving a change. That left the delivery half of the remote disable untested: the payload was pinned through refreshState, but not the path from a model write to the app's callback. This installs a test main dispatcher in its own spec, wires a real SubscriptionModelStore to a SubscriptionManager, and asserts that a HYDRATE-tagged write of the server's disable code reaches an attached observer as optedIn true to false. Kept separate from SubscriptionManagerTests so the dispatcher swap does not touch the other specs in that file. --- .../PushSubscriptionObserverTests.kt | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/PushSubscriptionObserverTests.kt diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/PushSubscriptionObserverTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/PushSubscriptionObserverTests.kt new file mode 100644 index 0000000000..99264c13f6 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/PushSubscriptionObserverTests.kt @@ -0,0 +1,111 @@ +package com.onesignal.user.internal.subscriptions + +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.core.internal.application.IApplicationService +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.mocks.MockPreferencesService +import com.onesignal.session.internal.session.ISessionService +import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager +import com.onesignal.user.subscriptions.IPushSubscriptionObserver +import com.onesignal.user.subscriptions.PushSubscriptionChangedState +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.mockk +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +/** + * End-to-end coverage for the push subscription observer, kept in its own spec because + * [IPushSubscriptionObserver] callbacks are delivered through `Dispatchers.Main` and the rest of the + * subscription specs run without a main dispatcher installed. + * + * The chain under test is the real one: a property write on a model held by a [SubscriptionModelStore] + * notifies the store, which re-broadcasts to [SubscriptionManager], which builds the changed state + * from the push subscription and hands it to the app's observer. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class PushSubscriptionObserverTests : FunSpec({ + + beforeTest { + Logging.logLevel = LogLevel.NONE + // Observer callbacks go out via suspendifyOnMain, so tests need a main dispatcher. + Dispatchers.setMain(UnconfinedTestDispatcher()) + } + + afterTest { + Dispatchers.resetMain() + } + + /** + * A store holding one opted-in push subscription, with [SubscriptionManager] subscribed to it and + * an observer attached. Returns the model to mutate and a latch plus recorded states to assert on. + */ + fun attachObserverToPushSubscription(): Triple> { + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = "subscription1" + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.address = "pushToken" + pushSubscriptionModel.status = SubscriptionStatus.SUBSCRIBED + pushSubscriptionModel.optedIn = true + + val subscriptionModelStore = SubscriptionModelStore(MockPreferencesService()) + subscriptionModelStore.add(pushSubscriptionModel) + + // Constructing the manager subscribes it to the store and builds its subscription list. + val subscriptionManager = + SubscriptionManager( + mockk(), + mockk(relaxed = true), + subscriptionModelStore, + ) + + val observedStates = mutableListOf() + val observerCalled = CountDownLatch(1) + subscriptionManager.subscriptions.push.addObserver( + object : IPushSubscriptionObserver { + override fun onPushSubscriptionChange(state: PushSubscriptionChangedState) { + observedStates.add(state) + observerCalled.countDown() + } + }, + ) + + return Triple(pushSubscriptionModel, observerCalled, observedStates) + } + + // Both codes take the same path, so both are pinned end to end rather than only the one the + // unit-level tests happen to exercise. + listOf( + SubscriptionStatus.MANUALLY_UNSUBSCRIBED, + SubscriptionStatus.DISABLED_FROM_REST_API, + ).forEach { remoteDisable -> + test("hydrating a ${remoteDisable.value} disable reports optedIn false to the app's observer") { + // Given an opted-in push subscription with an observer attached + val (pushSubscriptionModel, observerCalled, observedStates) = attachObserverToPushSubscription() + + // When RefreshUser records the server's disable, which it writes with the HYDRATE tag. + // Nothing on the path from the model to the observer filters on that tag, which is why + // the app hears about a disable it never asked for locally. + pushSubscriptionModel.setIntProperty( + SubscriptionModel::remoteDisabledReason.name, + remoteDisable.value, + ModelChangeTags.HYDRATE, + ) + + // Then the observer sees a real transition, not an unchanged pair + observerCalled.await(5, TimeUnit.SECONDS) shouldBe true + observedStates.size shouldBe 1 + observedStates[0].previous.optedIn shouldBe true + observedStates[0].current.optedIn shouldBe false + // The rest of the state is untouched: only the opt-in answer moved. + observedStates[0].current.id shouldBe "subscription1" + observedStates[0].current.token shouldBe "pushToken" + } + } +}) From 1d941b27bdd0417f925fae9b0efa457497699b20 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 9 Sep 2026 17:27:05 -0700 Subject: [PATCH 08/11] chore: demo app log onPushSubscriptionChange So we can see what info changed --- .../src/main/java/com/onesignal/example/ui/main/MainViewModel.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/demo/app/src/main/java/com/onesignal/example/ui/main/MainViewModel.kt b/examples/demo/app/src/main/java/com/onesignal/example/ui/main/MainViewModel.kt index 2d6bd010fc..07dbd8789c 100644 --- a/examples/demo/app/src/main/java/com/onesignal/example/ui/main/MainViewModel.kt +++ b/examples/demo/app/src/main/java/com/onesignal/example/ui/main/MainViewModel.kt @@ -695,6 +695,7 @@ class MainViewModel(application: Application) : AndroidViewModel(application), I private fun logDebug(message: String) = DemoLog.d(TAG, message) override fun onPushSubscriptionChange(state: PushSubscriptionChangedState) { + DemoLog.i(TAG, "onPushSubscriptionChange: ${state.toJSONObject()}") _pushSubscriptionId.postValue(state.current.id) _pushEnabled.postValue(state.current.optedIn) } From c931c81b8fc2618b884e6aff42fe72e332ecbd7e Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 10 Sep 2026 07:40:59 -0700 Subject: [PATCH 09/11] fix: re-apply a remote disable over a stale enabled Device metadata written at session start enqueues a subscription update before RefreshUser knows about a disable, and the operation freezes its enabled at that point. Recording the disable under HYDRATE generated no operation of its own, so the queued update re-enabled the subscription and the next fetch cleared the local record to match. A dashboard unsubscribe never recovered from that on its own, unlike a REST API disable that the integration re-sends on its next sync. Record under NORMAL so the write produces its own update. Executors read the last operation in a merged batch, so the correction replaces the stale one while it is still queued, and re-disables the subscription when it already went out. Clearing the record now generates an update too. That is the first chance to send an opt-out made while the disable was suppressing every payload. --- .../executors/RefreshUserOperationExecutor.kt | 5 +- .../RefreshUserOperationExecutorTests.kt | 131 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) 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 19b335b5ac..c7eb3f73f7 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 @@ -278,10 +278,13 @@ internal class RefreshUserOperationExecutor( "RefreshUserOperationExecutor: clearing remote disable ${cachedPushSubscriptionModel.remoteDisabledReason} for push subscription $pushSubscriptionId" }, ) + // NORMAL so this generates a subscription update of its own. An update queued before + // this fetch still carries the enabled it was built with. Without a correction it + // re-enables the subscription, and the next fetch clears this record to match. cachedPushSubscriptionModel.setIntProperty( SubscriptionModel::remoteDisabledReason.name, target, - ModelChangeTags.HYDRATE, + ModelChangeTags.NORMAL, ) } } 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 d4dbf73c87..d8b5eed04f 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 @@ -4,10 +4,12 @@ import com.onesignal.common.TimeUtils 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.IOperationRepo 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.mocks.MockPreferencesService import com.onesignal.user.internal.backend.CreateUserResponse import com.onesignal.user.internal.backend.IUserBackendService import com.onesignal.user.internal.backend.IdentityConstants @@ -21,6 +23,7 @@ import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getJwtToke 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.operations.impl.listeners.SubscriptionModelStoreListener import com.onesignal.user.internal.properties.PropertiesModel import com.onesignal.user.internal.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore @@ -397,6 +400,86 @@ class RefreshUserOperationExecutorTests : FunSpec({ return Triple(executor, cachedPushSubscriptionModel, mockUserBackendService) } + /** + * The same fetch as [buildSelfHealHarness], but against a real store with the real model store + * listener attached, so a test sees the operations a hydration actually produces rather than + * only the model it leaves behind. + */ + fun buildCorrectiveUpdateHarness( + serverPushEnabled: Boolean, + serverNotificationTypes: Int?, + localOptedIn: Boolean, + localRemoteDisabledReason: Int, + ): Triple> { + val mockUserBackendService = mockk() + 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 identityModel = IdentityModel() + identityModel.onesignalId = remoteOneSignalId + every { mockIdentityModelStore.model } returns identityModel + every { mockIdentityModelStore.replace(any(), any()) } just runs + + val mockPropertiesModelStore = MockHelper.propertiesModelStore() + val propertiesModel = PropertiesModel() + propertiesModel.onesignalId = remoteOneSignalId + every { mockPropertiesModelStore.model } returns propertiesModel + every { mockPropertiesModelStore.replace(any(), any()) } just runs + + val subscriptionModelStore = SubscriptionModelStore(MockPreferencesService()) + val cachedPushSubscriptionModel = + SubscriptionModel().apply { + id = existingSubscriptionId1 + type = SubscriptionType.PUSH + address = onDevicePushToken + status = SubscriptionStatus.SUBSCRIBED + optedIn = localOptedIn + remoteDisabledReason = localRemoteDisabledReason + } + // NO_PROPOGATE so seeding the store does not enqueue a create. + subscriptionModelStore.add(cachedPushSubscriptionModel, ModelChangeTags.NO_PROPOGATE) + + val enqueued = mutableListOf() + val mockOpRepo = mockk(relaxed = true) + every { mockOpRepo.enqueue(capture(enqueued), any()) } just runs + + val configModelStore = MockHelper.configModelStore { it.pushSubscriptionId = existingSubscriptionId1 } + + SubscriptionModelStoreListener( + subscriptionModelStore, + mockOpRepo, + mockIdentityModelStore, + configModelStore, + ).bootstrap() + + val executor = + RefreshUserOperationExecutor( + mockUserBackendService, + mockIdentityModelStore, + mockPropertiesModelStore, + subscriptionModelStore, + configModelStore, + mockk(), + getNewRecordState(), + getJwtTokenStore(), getIdentityVerificationService(), + ) + + return Triple(executor, cachedPushSubscriptionModel, enqueued) + } + test("push 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 stuck state), local view says enabled val (executor, _, _) = @@ -656,4 +739,52 @@ class RefreshUserOperationExecutorTests : FunSpec({ cachedPushSubscriptionModel.remoteDisabledReason shouldBe 0 cachedPushSubscriptionModel.remoteDisableClearedByUser shouldBe true } + + test("push refresh: recording a remote disable enqueues the update that re-applies it") { + // A device-metadata update queued earlier in the session carries the enabled it was built + // with, from before the disable was known. On its own it re-enables the subscription, and + // the next fetch then reports it as enabled and clears the local record, so neither the + // device nor the server is left holding the disable. The update this hydration produces is + // what replaces the stale one, or puts the state back if it already went out. + val (executor, cachedPushSubscriptionModel, enqueued) = + buildCorrectiveUpdateHarness( + serverPushEnabled = false, + serverNotificationTypes = SubscriptionStatus.MANUALLY_UNSUBSCRIBED.value, + localOptedIn = true, + localRemoteDisabledReason = 0, + ) + + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + + // Then + response.result shouldBe ExecutionResult.SUCCESS + cachedPushSubscriptionModel.remoteDisabledReason shouldBe SubscriptionStatus.MANUALLY_UNSUBSCRIBED.value + val corrective = enqueued.filterIsInstance().last() + corrective.subscriptionId shouldBe existingSubscriptionId1 + corrective.enabled shouldBe false + corrective.status shouldBe SubscriptionStatus.MANUALLY_UNSUBSCRIBED + } + + test("push refresh: clearing a remote disable sends the opt-out the device could not send") { + // While a disable is recorded every payload reports it, so an opt-out made during the + // suppression never reaches the server. Clearing the record is the first chance to send it. + val (executor, cachedPushSubscriptionModel, enqueued) = + buildCorrectiveUpdateHarness( + serverPushEnabled = true, + serverNotificationTypes = 1, + localOptedIn = false, + localRemoteDisabledReason = SubscriptionStatus.DISABLED_FROM_REST_API.value, + ) + + // When + val response = executor.execute(listOf(RefreshUserOperation(appId, remoteOneSignalId, null))) + + // Then + response.result shouldBe ExecutionResult.SUCCESS + cachedPushSubscriptionModel.remoteDisabledReason shouldBe 0 + val corrective = enqueued.filterIsInstance().last() + corrective.enabled shouldBe false + corrective.status shouldBe SubscriptionStatus.UNSUBSCRIBE + } }) From e0907818b9938bfbd48ece164d2413cd365d4ad9 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 10 Sep 2026 07:41:04 -0700 Subject: [PATCH 10/11] fix: settle the opt-in guard once its write goes out remoteDisableClearedByUser armed on every optIn() and only came down when a fetch reported a non-disabled state. If the app owner disabled the subscription again before that arrived, every later -22 and -31 was discarded for the rest of the process. Clear it once the opt-in's write has been sent. The operation repo runs one batch at a time, so a fetch issued after that point reports current state and the disable it carries has to be recorded. A retry or an auth failure means the write is still coming, so the guard stays armed. Also covers the merge precedence the previous commit depends on. Nothing asserted which values a batch holding both a stale enabled and a corrective disable actually sends. --- .../SubscriptionOperationExecutor.kt | 41 +++- .../SubscriptionOperationExecutorTests.kt | 195 ++++++++++++++++++ 2 files changed, 234 insertions(+), 2 deletions(-) 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 index 7e76e3ca81..ea6d4599b4 100644 --- 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 @@ -54,9 +54,46 @@ internal class SubscriptionOperationExecutor( get() = listOf(CREATE_SUBSCRIPTION, UPDATE_SUBSCRIPTION, DELETE_SUBSCRIPTION, TRANSFER_SUBSCRIPTION) override suspend fun execute(operations: List): ExecutionResponse { - Logging.log(LogLevel.DEBUG, "SubscriptionOperationExecutor(operations: $operations)") - val startingOp = operations.first() + return executeSubscriptionOperation(startingOp, operations) + .also { settleOptInGuard(startingOp, it) } + } + + /** + * Clears [SubscriptionModel.remoteDisableClearedByUser] once the opt-in's write has gone out. + * The operation repo runs one batch at a time, so a fetch after that point was issued after the + * write, and a disable it reports is current. A queued result means the write is still coming. + */ + private fun settleOptInGuard( + operation: Operation, + response: ExecutionResponse, + ) { + val operationSubscriptionId = + when (operation) { + is CreateSubscriptionOperation -> operation.subscriptionId + is UpdateSubscriptionOperation -> operation.subscriptionId + else -> return + } + + val writeStillPending = + response.result == ExecutionResult.FAIL_RETRY || + response.result == ExecutionResult.FAIL_UNAUTHORIZED || + response.result == ExecutionResult.FAIL_PAUSE_OPREPO + if (writeStillPending) return + + // A successful create moved the model to the id the backend assigned. + val subscriptionId = response.idTranslations?.get(operationSubscriptionId) ?: operationSubscriptionId + _subscriptionModelStore.get(subscriptionId)?.remoteDisableClearedByUser = false + } + + // execute() wraps this so every exit settles the opt-in guard. The throws are guard clauses + // for operation combinations that should never reach here. + @Suppress("ThrowsCount") + private suspend fun executeSubscriptionOperation( + startingOp: Operation, + operations: List, + ): ExecutionResponse { + Logging.log(LogLevel.DEBUG, "SubscriptionOperationExecutor(operations: $operations)") return if (startingOp is CreateSubscriptionOperation) { // If the subscription already exists on the backend (non-local id), POSTing 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 index 48ae3eda4d..cff33041bc 100644 --- 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 @@ -59,6 +59,8 @@ class SubscriptionOperationExecutorTests : val subscriptionModel1 = SubscriptionModel() subscriptionModel1.id = localSubscriptionId every { mockSubscriptionsModelStore.get(localSubscriptionId) } returns subscriptionModel1 + // A successful create moves the model to the backend's id, where the guard settles. + every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns subscriptionModel1 val mockBuildUserService = mockk() @@ -239,6 +241,8 @@ class SubscriptionOperationExecutorTests : coEvery { mockSubscriptionBackendService.createSubscription(any(), any(), any(), any()) } throws BackendException(404) val mockSubscriptionsModelStore = mockk() + // This store holds no model, so the guard settle finds nothing to clear. + every { mockSubscriptionsModelStore.get(localSubscriptionId) } returns null val mockBuildUserService = mockk() every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } answers { null } @@ -389,6 +393,8 @@ class SubscriptionOperationExecutorTests : val subscriptionModel1 = SubscriptionModel() subscriptionModel1.id = localSubscriptionId every { mockSubscriptionsModelStore.get(localSubscriptionId) } returns subscriptionModel1 + // A successful create moves the model to the backend's id, where the guard settles. + every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns subscriptionModel1 val mockBuildUserService = mockk() @@ -773,6 +779,195 @@ class SubscriptionOperationExecutorTests : cachedSubscriptionModel.remoteDisabledReason shouldBe 0 } + test("update subscription sends the corrective remote disable over the stale enabled it follows") { + // This is the batch the session-start race produces. A device-metadata update was + // queued before the fetch reported the disable, so it carries enabled=true, and the + // hydration that recorded the disable then enqueued a corrective update behind it. + // Both share modifyComparisonKey for this subscription, so the op repo merges them and + // one PATCH goes out. What that PATCH carries is the whole point of recording a remote + // disable under NORMAL rather than HYDRATE, and nothing else asserts it: the executor + // reads operations.last(), so a change to the merge order or to the enqueue order would + // re-open the hole while every other test still passed. + // Given + val mockSubscriptionBackendService = mockk() + coEvery { mockSubscriptionBackendService.updateSubscription(any(), any(), any()) } returns rywData + + val mockSubscriptionsModelStore = mockk() + val subscriptionModel = + SubscriptionModel().apply { + id = remoteSubscriptionId + type = SubscriptionType.PUSH + address = "pushToken1" + optedIn = true + remoteDisabledReason = SubscriptionStatus.MANUALLY_UNSUBSCRIBED.value + } + every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns subscriptionModel + + val subscriptionOperationExecutor = + SubscriptionOperationExecutor( + mockSubscriptionBackendService, + MockHelper.deviceService(), + AndroidMockHelper.applicationService(), + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockk(), + getNewRecordState(), + mockConsistencyManager, + getJwtTokenStore(), getIdentityVerificationService(), + ) + + val operations = + listOf( + // Queued at session start, built while the disable was still unknown. + UpdateSubscriptionOperation( + appId, + remoteOneSignalId, + null, + remoteSubscriptionId, + SubscriptionType.PUSH, + true, + "pushToken1", + SubscriptionStatus.SUBSCRIBED, + ), + // Enqueued by the hydration that recorded the disable. + UpdateSubscriptionOperation( + appId, + remoteOneSignalId, + null, + remoteSubscriptionId, + SubscriptionType.PUSH, + false, + "pushToken1", + SubscriptionStatus.MANUALLY_UNSUBSCRIBED, + ), + ) + + // When + val response = subscriptionOperationExecutor.execute(operations) + + // Then one PATCH goes out and it leaves the subscription disabled, not re-enabled + response.result shouldBe ExecutionResult.SUCCESS + coVerify(exactly = 1) { + mockSubscriptionBackendService.updateSubscription( + appId, + remoteSubscriptionId, + withArg { + it.enabled shouldBe false + it.notificationTypes shouldBe SubscriptionStatus.MANUALLY_UNSUBSCRIBED.value + }, + ) + } + } + + test("update subscription settles the opt-in guard once the write reaches the server") { + // The guard exists for one race, a fetch issued before the opt-in's write reached the + // server, which still reports the disable that write clears. Once the write has gone + // out, a later fetch was issued after it, so a disable it reports is current and has to + // be recorded. A guard that never comes down ignores every disable for the rest of the + // process instead. + // Given + val mockSubscriptionBackendService = mockk() + coEvery { mockSubscriptionBackendService.updateSubscription(any(), any(), any()) } returns rywData + + val mockSubscriptionsModelStore = mockk() + val subscriptionModel = + SubscriptionModel().apply { + id = remoteSubscriptionId + type = SubscriptionType.PUSH + address = "pushToken1" + optedIn = true + remoteDisableClearedByUser = true + } + every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns subscriptionModel + + val subscriptionOperationExecutor = + SubscriptionOperationExecutor( + mockSubscriptionBackendService, + MockHelper.deviceService(), + AndroidMockHelper.applicationService(), + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockk(), + getNewRecordState(), + mockConsistencyManager, + getJwtTokenStore(), getIdentityVerificationService(), + ) + + val operations = + listOf( + UpdateSubscriptionOperation( + appId, + remoteOneSignalId, + null, + remoteSubscriptionId, + SubscriptionType.PUSH, + true, + "pushToken1", + SubscriptionStatus.SUBSCRIBED, + ), + ) + + // When + val response = subscriptionOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.SUCCESS + subscriptionModel.remoteDisableClearedByUser shouldBe false + } + + test("update subscription keeps the opt-in guard armed while the write is still queued") { + // A retry means the server has not been told yet, so a fetch can still report the + // disable the opt-in cleared and the guard has to stay. + // Given + val mockSubscriptionBackendService = mockk() + coEvery { mockSubscriptionBackendService.updateSubscription(any(), any(), any()) } throws BackendException(500) + + val mockSubscriptionsModelStore = mockk() + val subscriptionModel = + SubscriptionModel().apply { + id = remoteSubscriptionId + type = SubscriptionType.PUSH + address = "pushToken1" + optedIn = true + remoteDisableClearedByUser = true + } + every { mockSubscriptionsModelStore.get(remoteSubscriptionId) } returns subscriptionModel + + val subscriptionOperationExecutor = + SubscriptionOperationExecutor( + mockSubscriptionBackendService, + MockHelper.deviceService(), + AndroidMockHelper.applicationService(), + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockk(), + getNewRecordState(), + mockConsistencyManager, + getJwtTokenStore(), getIdentityVerificationService(), + ) + + val operations = + listOf( + UpdateSubscriptionOperation( + appId, + remoteOneSignalId, + null, + remoteSubscriptionId, + SubscriptionType.PUSH, + true, + "pushToken1", + SubscriptionStatus.SUBSCRIBED, + ), + ) + + // When + val response = subscriptionOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + subscriptionModel.remoteDisableClearedByUser shouldBe true + } + test("update subscription fails with retry when the backend returns MISSING, when isInMissingRetryWindow") { // Given val mockSubscriptionBackendService = mockk() From df931382139662916070d71ae53cc4a84894ae82 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 10 Sep 2026 07:41:09 -0700 Subject: [PATCH 11/11] fix: carry the opt-in guard across a user switch createAndSwitchToNewUser copied remoteDisabledReason but not remoteDisableClearedByUser, which lives in memory rather than as a model property and so does not travel with the copy beside it. A login or logout landing between an opt-in and its write handed the replacement model a cleared guard, letting an older fetch record the disable the opt-in had just cleared. --- .../onesignal/user/internal/UserSwitcher.kt | 2 ++ .../user/internal/UserSwitcherTests.kt | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt index 5cf46983ef..e3470f0633 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserSwitcher.kt @@ -69,6 +69,8 @@ class UserSwitcher( address = currentPushSubscription?.address ?: "" status = currentPushSubscription?.status ?: SubscriptionStatus.NO_PERMISSION remoteDisabledReason = currentPushSubscription?.remoteDisabledReason ?: 0 + // In memory and not a model property, so it does not travel with the copy above. + remoteDisableClearedByUser = currentPushSubscription?.remoteDisableClearedByUser ?: false sdk = oneSignalUtils.sdkVersion deviceOS = this@UserSwitcher.deviceOS ?: "" carrier = carrierName ?: "" diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt index f73c8caa53..d1ab52dc1f 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserSwitcherTests.kt @@ -285,6 +285,31 @@ class UserSwitcherTests : FunSpec({ status shouldBe SubscriptionStatus.MANUALLY_UNSUBSCRIBED } + test("createAndSwitchToNewUser carries the opt-in guard onto the new push model") { + // The guard lives in memory rather than as a model property, so it is not carried by the + // copy of the reason beside it. Without it, a login landing between an opt-in and its write + // lets a fetch issued before that write record the disable the opt-in just cleared. + // Given + val mocks = Mocks() + val userSwitcher = mocks.createUserSwitcher() + val optedInPushModel = + SubscriptionModel().apply { + id = mocks.testSubscriptionId + type = SubscriptionType.PUSH + address = "test-token" + optedIn = true + remoteDisableClearedByUser = true + } + mocks.subscriptionModelStore!!.add(optedInPushModel, ModelChangeTags.NO_PROPOGATE) + + // When + userSwitcher.createAndSwitchToNewUser() + + // Then + val newPushModel = mocks.subscriptionModelStore!!.list().first { it.type == SubscriptionType.PUSH } + newPushModel.remoteDisableClearedByUser shouldBe true + } + test("initUser with forceCreateUser creates new user") { // Given val mocks = Mocks()