Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 5 additions & 12 deletions OneSignalSDK/detekt/detekt-baseline-core.xml
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@
<ID>LongMethod:TrackGooglePurchase.kt$TrackGooglePurchase$private fun queryBoughtItems()</ID>
<ID>LongMethod:TrackGooglePurchase.kt$TrackGooglePurchase$private fun sendPurchases( skusToAdd: ArrayList&lt;String>, newPurchaseTokens: ArrayList&lt;String>, )</ID>
<ID>LongMethod:UpdateUserOperationExecutor.kt$UpdateUserOperationExecutor$override suspend fun execute(operations: List&lt;Operation>): ExecutionResponse</ID>
<ID>LongParameterList:CrashDirCleanup.kt$( label: String, path: String, entries: List&lt;CrashDirEntry>, nowMs: Long, maxSample: Int, ownedSuffix: String = CRASH_OWNED_SUFFIX, )</ID>
<ID>LongParameterList:CreateSubscriptionOperation.kt$CreateSubscriptionOperation$(appId: String, onesignalId: String, externalId: String?, subscriptionId: String, type: SubscriptionType, enabled: Boolean, address: String, status: SubscriptionStatus)</ID>
<ID>LongParameterList:ICustomEventBackendService.kt$ICustomEventBackendService$( appId: String, onesignalId: String, externalId: String?, timestamp: Long, eventName: String, eventProperties: String?, metadata: CustomEventMetadata, jwt: String? = null, )</ID>
<ID>LongParameterList:IDatabase.kt$IDatabase$( table: String, columns: Array&lt;String>? = null, whereClause: String? = null, whereArgs: Array&lt;String>? = null, groupBy: String? = null, having: String? = null, orderBy: String? = null, limit: String? = null, action: (ICursor) -> Unit, )</ID>
Expand Down Expand Up @@ -257,7 +258,7 @@
<ID>MagicNumber:PermissionsActivity.kt$PermissionsActivity$23</ID>
<ID>MagicNumber:RefreshUserOperationExecutor.kt$RefreshUserOperationExecutor$404</ID>
<ID>MagicNumber:SessionListener.kt$SessionListener$1000</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.DISABLED_FROM_REST_API_DEFAULT_REASON$30</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.DISABLED_FROM_REST_API$31</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.ERROR$9999</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_AUTHENTICATION_FAILED$29</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_OTHER$11</ID>
Expand All @@ -268,6 +269,7 @@
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.HMS_ARGUMENTS_INVALID$26</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.HMS_TOKEN_TIMEOUT$25</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.INVALID_FCM_SENDER_ID$6</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MANUALLY_UNSUBSCRIBED$22</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MISSING_FIREBASE_FCM_LIBRARY$4</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MISSING_HMS_PUSHKIT_LIBRARY$28</ID>
<ID>MagicNumber:SubscriptionModel.kt$SubscriptionStatus.MISSING_JETPACK_LIBRARY$3</ID>
Expand Down Expand Up @@ -308,7 +310,7 @@
<ID>ReturnCount:ConfigModel.kt$ConfigModel$override fun createModelForProperty( property: String, jsonObject: JSONObject, ): Model?</ID>
<ID>ReturnCount:ExecutorsIvExtensions.kt$internal fun resolveIvBackendParams( op: Operation, onesignalId: String, jwtTokenStore: JwtTokenStore, ivBehaviorActive: Boolean, ): IvBackendParams</ID>
<ID>ReturnCount:ExecutorsIvExtensions.kt$internal fun resolveIvJwt( op: Operation, jwtTokenStore: JwtTokenStore, ivBehaviorActive: Boolean, ): String?</ID>
<ID>ReturnCount:FeatureFlagsBackendService.kt$FeatureFlagsBackendService$override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome</ID>
<ID>ReturnCount:FeatureFlagsRefreshService.kt$FeatureFlagsRefreshService$private suspend fun fetchAndApply(appId: String)</ID>
<ID>ReturnCount:HttpClient.kt$HttpClient$private suspend fun makeRequest( url: String, method: String?, jsonBody: JSONObject?, timeout: Int, headers: OptionalHeaders?, ): HttpResponse</ID>
<ID>ReturnCount:IdentityOperationExecutor.kt$IdentityOperationExecutor$override suspend fun execute(operations: List&lt;Operation>): ExecutionResponse</ID>
<ID>ReturnCount:JSONUtils.kt$JSONUtils$fun compareJSONArrays( jsonArray1: JSONArray?, jsonArray2: JSONArray?, ): Boolean</ID>
Expand All @@ -321,7 +323,6 @@
<ID>ReturnCount:Model.kt$Model$protected fun getOptIntProperty( name: String, create: (() -> Int?)? = null, ): Int?</ID>
<ID>ReturnCount:Model.kt$Model$protected fun getOptLongProperty( name: String, create: (() -> Long?)? = null, ): Long?</ID>
<ID>ReturnCount:Model.kt$Model$protected inline fun &lt;reified T : Enum&lt;T>> getOptEnumProperty(name: String): T?</ID>
<ID>ReturnCount:OneSignalImp.kt$OneSignalImp$private fun internalInit( context: Context, appId: String?, ): Boolean</ID>
<ID>ReturnCount:OperationModelStore.kt$OperationModelStore$override fun create(jsonObject: JSONObject?): Operation?</ID>
<ID>ReturnCount:OperationModelStore.kt$OperationModelStore$private fun isValidOperation(jsonObject: JSONObject): Boolean</ID>
<ID>ReturnCount:OperationRepo.kt$OperationRepo$private fun shouldSuppressAnonymousOp(op: Operation): Boolean</ID>
Expand Down Expand Up @@ -354,7 +355,6 @@
<ID>SwallowedException:PreferencesService.kt$PreferencesService$t: Throwable</ID>
<ID>SwallowedException:SyncJobService.kt$SyncJobService$e: Exception</ID>
<ID>SwallowedException:TrackGooglePurchase.kt$TrackGooglePurchase.Companion$t: Throwable</ID>
<ID>ThrowsCount:OneSignalImp.kt$OneSignalImp$private suspend fun waitUntilInitInternal(operationName: String? = null)</ID>
<ID>TooGenericExceptionCaught:AndroidUtils.kt$AndroidUtils$e: Throwable</ID>
<ID>TooGenericExceptionCaught:DeviceUtils.kt$DeviceUtils$t: Throwable</ID>
<ID>TooGenericExceptionCaught:FeatureFlagsRefreshService.kt$FeatureFlagsRefreshService$e: Exception</ID>
Expand All @@ -363,6 +363,7 @@
<ID>TooGenericExceptionCaught:JSONUtils.kt$JSONUtils$t: Throwable</ID>
<ID>TooGenericExceptionCaught:Logging.kt$Logging$t: Throwable</ID>
<ID>TooGenericExceptionCaught:OneSignalDispatchers.kt$OneSignalDispatchers$e: Exception</ID>
<ID>TooGenericExceptionCaught:OneSignalDispatchers.kt$OneSignalDispatchers.Pools$e: Exception</ID>
<ID>TooGenericExceptionCaught:OperationRepo.kt$OperationRepo$e: Throwable</ID>
<ID>TooGenericExceptionCaught:PreferenceStoreFix.kt$PreferenceStoreFix$e: Throwable</ID>
<ID>TooGenericExceptionCaught:PreferencesService.kt$PreferencesService$e: Throwable</ID>
Expand Down Expand Up @@ -632,15 +633,7 @@
<ID>UnusedPrivateMember:JSONUtils.kt$JSONUtils$`object`: Any</ID>
<ID>UnusedPrivateMember:OSDatabase.kt$OSDatabase.Companion$private const val FLOAT_TYPE = " FLOAT"</ID>
<ID>UnusedPrivateMember:OperationRepo.kt$OperationRepo$private val _time: ITime</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("'initWithContext failed' before 'login'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("'initWithContext failed' before 'logout'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("'initWithContext failed' before 'updateUserJwt'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'addUserJwtInvalidatedListener'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'login'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'logout'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'removeUserJwtInvalidatedListener'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before 'updateUserJwt'")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("Must call 'initWithContext' before use")</ID>
<ID>UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw initFailureException ?: IllegalStateException("Initialization failed. Cannot proceed.")</ID>
</CurrentIssues>
</SmellBaseline>
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(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<T>().firstOrNull { it.name == value }
return value as T
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,21 @@ internal open class PushSubscription(
get() = model.address

override val optedIn: Boolean
get() = model.optedIn && model.status != SubscriptionStatus.NO_PERMISSION
// A remote disable suppresses delivery just as surely as a missing permission or an
// opt-out, so it belongs in the same answer. Reported through the model's recorded reason
// rather than its status, which stays device-owned.
get() = model.optedIn &&
model.status != SubscriptionStatus.NO_PERMISSION &&
model.remoteDisabledReason == 0

override fun optIn() {
// A deliberate opt-in overrides a remote disable; clearing it with a NORMAL-tagged
// change drives a subscription update that re-enables it on the server. The flag keeps
// a fetch that started before that update went out from recording the disable again.
model.remoteDisableClearedByUser = true
if (model.remoteDisabledReason != 0) {
model.remoteDisabledReason = 0
}
Comment thread
nan-li marked this conversation as resolved.
// 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ class UserSwitcher(
optedIn = currentPushSubscription?.optedIn ?: true
address = currentPushSubscription?.address ?: ""
status = currentPushSubscription?.status ?: SubscriptionStatus.NO_PERMISSION
remoteDisabledReason = currentPushSubscription?.remoteDisabledReason ?: 0
// In memory and not a model property, so it does not travel with the copy above.
remoteDisableClearedByUser = currentPushSubscription?.remoteDisableClearedByUser ?: false
sdk = oneSignalUtils.sdkVersion
deviceOS = this@UserSwitcher.deviceOS ?: ""
carrier = carrierName ?: ""
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 remote
// 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(
Comment thread
nan-li marked this conversation as resolved.
SubscriptionModel::remoteDisabledReason.name,
0,
ModelChangeTags.HYDRATE,
)
pushSubscription.remoteDisabledReason = 0
val (enabled, status) = SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus(pushSubscription)
return CreateSubscriptionOperation(
appId,
onesignalId,
externalId,
pushSubscription.id,
pushSubscription.type,
enabled,
pushSubscription.address,
status,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<SubscriptionStatus>(::status.name) ?: SubscriptionStatus.SUBSCRIBED
private set(value) {
setEnumProperty(::status.name, value)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<SubscriptionStatus>(::status.name) ?: SubscriptionStatus.SUBSCRIBED
private set(value) {
setEnumProperty(::status.name, value)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
!SubscriptionStatus.isRemoteDisable(subscriptionModel.status.value)
subscriptionModel.sdk = subscription.sdk ?: ""
subscriptionModel.deviceOS = subscription.deviceOS ?: ""
subscriptionModel.carrier = subscription.carrier ?: ""
Expand All @@ -136,6 +137,7 @@ internal class RefreshUserOperationExecutor(
if (subscriptionModel.type != SubscriptionType.PUSH) {
subscriptionModels.add(subscriptionModel)
} else if (subscription.id == pushSubscriptionIdFromConfig && pushSelfHealOperationForStuckSubscription == null) {
hydrateRemoteDisableState(subscription, pushSubscriptionIdFromConfig)
// Self-heal for users stuck at "Never Subscribed". Older SDK builds dispatched
// the merged create-subscription + update-subscription(SUBSCRIBED) batch as a
// POST /subscriptions carrying the already-existing server-side id; the server
Expand Down Expand Up @@ -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 remote disable is deliberate suppression, not the stuck-subscription drift this
// self-heal exists for; leave it in place.
val serverDisabledRemotely = SubscriptionStatus.isRemoteDisable(serverSubscription.notificationTypes)
val divergent = localEnabled && !serverEnabled && !serverDisabledRemotely

return if (divergent) {
Logging.info(
Expand All @@ -242,6 +247,48 @@ internal class RefreshUserOperationExecutor(
}
}

/**
* Records or clears the server's remote disable state on the cached push model. Only that
* state is server-owned; the device stays the source of truth for the rest of the push model,
* which is why push subscriptions are otherwise not hydrated from the backend. An opt-in whose
* update has not reached the server yet outranks a fetch that still reports the disable it cleared.
*/
private fun hydrateRemoteDisableState(
serverSubscription: SubscriptionObject,
pushSubscriptionId: String,
) {
val cachedPushSubscriptionModel = _subscriptionsModelStore.get(pushSubscriptionId)
val serverTypes = serverSubscription.notificationTypes
if (cachedPushSubscriptionModel == null || serverTypes == null) return
// The recorded reason mirrors the server's field verbatim, so -22 and -31 stay
// distinguishable; any other reported value clears.
val target = if (SubscriptionStatus.isRemoteDisable(serverTypes)) serverTypes else 0
if (target == 0) {
cachedPushSubscriptionModel.remoteDisableClearedByUser = false
} else if (cachedPushSubscriptionModel.remoteDisableClearedByUser) {
// This fetch predates the opt-in's update, so it reports the state that update replaces.
Logging.debug("RefreshUserOperationExecutor: keeping an opt-in over a stale remote disable report")
return
}
if (cachedPushSubscriptionModel.remoteDisabledReason != target) {
Logging.debug(
if (target != 0) {
"RefreshUserOperationExecutor: recording remote disable $target for push subscription $pushSubscriptionId"
} else {
"RefreshUserOperationExecutor: clearing remote disable ${cachedPushSubscriptionModel.remoteDisabledReason} for push subscription $pushSubscriptionId"
},
)
// NORMAL so this generates a subscription update of its own. An update queued before
// this fetch still carries the enabled it was built with. Without a correction it
// re-enables the subscription, and the next fetch clears this record to match.
cachedPushSubscriptionModel.setIntProperty(
SubscriptionModel::remoteDisabledReason.name,
target,
ModelChangeTags.NORMAL,
)
}
}

companion object {
Comment thread
nan-li marked this conversation as resolved.
const val REFRESH_USER = "refresh-user"
}
Expand Down
Loading