diff --git a/OneSignalSDK/detekt/detekt-baseline-core.xml b/OneSignalSDK/detekt/detekt-baseline-core.xml index 9beb98a826..649921551d 100644 --- a/OneSignalSDK/detekt/detekt-baseline-core.xml +++ b/OneSignalSDK/detekt/detekt-baseline-core.xml @@ -211,6 +211,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, ) @@ -257,7 +258,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 @@ -308,7 +309,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 @@ -321,7 +322,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 @@ -354,7 +354,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 @@ -363,6 +362,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 @@ -632,15 +632,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/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..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 @@ -31,9 +31,11 @@ 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 +import com.onesignal.user.internal.subscriptions.SubscriptionStatus import com.onesignal.user.internal.subscriptions.SubscriptionType internal class SubscriptionOperationExecutor( @@ -265,11 +267,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) } + ?: freshStartWithoutDeadDisable(lastOperation) if (_configModelStore.model.pushSubscriptionId == staleSubscriptionId) { _configModelStore.model.pushSubscriptionId = recoveryLocalId } @@ -284,9 +297,9 @@ internal class SubscriptionOperationExecutor( lastOperation.externalId, recoveryLocalId, lastOperation.type, - lastOperation.enabled, + recoveryEnabled, lastOperation.address, - lastOperation.status, + recoveryStatus, ), ), ) @@ -376,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/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/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/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/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 + } +}) 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 + } })