From 60e30a5e2d76ea2c4912731ec7d5d1780ee59763 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 26 Aug 2026 19:21:05 -0700 Subject: [PATCH 1/2] 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 2/2] 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 + } +})