diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt index 22b3297931..81328efb54 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt @@ -141,6 +141,32 @@ class ConfigModel : Model() { setLongProperty(::opRepoPostWakeDelay.name, value) } + /** + * The number of milliseconds to delay after an operation completes + * that creates or changes ids. + * This is a "cold down" period to avoid a caveat with OneSignal's backend + * replication, where you may incorrectly get a 404 when attempting a GET + * or PATCH REST API call on something just after it is created. + */ + var opRepoPostCreateDelay: Long + get() = getLongProperty(::opRepoPostCreateDelay.name) { 5_000 } + set(value) { + setLongProperty(::opRepoPostCreateDelay.name, value) + } + + /** + * The number of milliseconds to retry operations for new models. + * This is a fallback to opRepoPostCreateDelay, where it's delay may + * not be enough. The server may be unusually overloaded so we will + * retry these (back-off rules apply to all retries) as we only want + * to re-create records as a last resort. + */ + var opRepoPostCreateRetryUpTo: Long + get() = getLongProperty(::opRepoPostCreateRetryUpTo.name) { 60_000 } + set(value) { + setLongProperty(::opRepoPostCreateRetryUpTo.name, value) + } + /** * The minimum number of milliseconds required to pass to allow the fetching of IAM to occur. */ diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/Operation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/Operation.kt index 42fdd62374..76f51994ab 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/Operation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/Operation.kt @@ -20,6 +20,12 @@ abstract class Operation(name: String) : Model() { this.name = name } + /** + * This is a unique id that points to a record this operation will affect. + * Example: If the operation is updating tags on a User this will be the onesignalId. + */ + abstract val applyToRecordId: String + /** * The key of this operation for when the starting operation has a [groupComparisonType] * of [GroupComparisonType.CREATE] diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt index e94de1be20..70c09ce50f 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt @@ -1,7 +1,6 @@ package com.onesignal.core.internal.operations.impl import com.onesignal.common.threading.WaiterWithValue -import com.onesignal.common.threading.suspendifyOnThread import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.operations.ExecutionResult import com.onesignal.core.internal.operations.GroupComparisonType @@ -12,7 +11,11 @@ import com.onesignal.core.internal.startup.IStartableService import com.onesignal.core.internal.time.ITime import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging +import com.onesignal.user.internal.operations.impl.states.NewRecordsState +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import kotlinx.coroutines.newSingleThreadContext import kotlinx.coroutines.withTimeoutOrNull import java.util.UUID import kotlin.reflect.KClass @@ -22,6 +25,7 @@ internal class OperationRepo( private val _operationModelStore: OperationModelStore, private val _configModelStore: ConfigModelStore, private val _time: ITime, + private val _newRecordState: NewRecordsState, ) : IOperationRepo, IStartableService { internal class OperationQueueItem( val operation: Operation, @@ -34,10 +38,16 @@ internal class OperationRepo( } } + internal class LoopWaiterMessage( + val force: Boolean, + val previousWaitedTime: Long = 0, + ) + private val executorsMap: Map private val queue = mutableListOf() - private val waiter = WaiterWithValue() + private val waiter = WaiterWithValue() private var paused = false + private var coroutineScope = CoroutineScope(newSingleThreadContext(name = "OpRepo")) /** *** Buckets *** * Purpose: Bucketing is a pattern we are using to help save network @@ -56,13 +66,11 @@ internal class OperationRepo( * network calls. */ private var enqueueIntoBucket = 0 - private val executeBucket: Int get() { - return if (enqueueIntoBucket == 0) 0 else enqueueIntoBucket - 1 - } + private val executeBucket get() = + if (enqueueIntoBucket == 0) 0 else enqueueIntoBucket - 1 init { val executorsMap: MutableMap = mutableMapOf() - for (executor in executors) { for (operation in executor.operations) { executorsMap[operation] = executor @@ -83,9 +91,7 @@ internal class OperationRepo( override fun start() { paused = false - suspendifyOnThread(name = "OpRepo") { - processQueueForever() - } + coroutineScope.launch { processQueueForever() } } override fun enqueue( @@ -123,7 +129,7 @@ internal class OperationRepo( } } - waiter.wake(flush) + waiter.wake(LoopWaiterMessage(flush, 0)) } /** @@ -160,16 +166,16 @@ internal class OperationRepo( */ private suspend fun waitForNewOperationAndExecutionInterval() { // 1. Wait for an operation to be enqueued - var force = waiter.waitForWake() + var wakeMessage = waiter.waitForWake() // 2. Wait at least the time defined in opRepoExecutionInterval // so operations can be grouped, unless one of them used // flush=true (AKA force) var lastTime = _time.currentTimeMillis - var remainingTime = _configModelStore.model.opRepoExecutionInterval - while (!force && remainingTime > 0) { + var remainingTime = _configModelStore.model.opRepoExecutionInterval - wakeMessage.previousWaitedTime + while (!wakeMessage.force && remainingTime > 0) { withTimeoutOrNull(remainingTime) { - force = waiter.waitForWake() + wakeMessage = waiter.waitForWake() } remainingTime -= _time.currentTimeMillis - lastTime lastTime = _time.currentTimeMillis @@ -195,6 +201,14 @@ internal class OperationRepo( synchronized(queue) { queue.forEach { it.operation.translateIds(response.idTranslations) } } + response.idTranslations.values.forEach { _newRecordState.add(it) } + coroutineScope.launch { + val waitTime = _configModelStore.model.opRepoPostCreateDelay + delay(waitTime) + synchronized(queue) { + if (queue.isNotEmpty()) waiter.wake(LoopWaiterMessage(false, waitTime)) + } + } } when (response.result) { @@ -278,7 +292,9 @@ internal class OperationRepo( return synchronized(queue) { val startingOp = queue.firstOrNull { - it.operation.canStartExecute && it.bucket <= bucketFilter + it.operation.canStartExecute && + _newRecordState.canAccess(it.operation.applyToRecordId) && + it.bucket <= bucketFilter } if (startingOp != null) { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/UserModule.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/UserModule.kt index c1b463286e..7e2c5daacb 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/UserModule.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/UserModule.kt @@ -25,6 +25,7 @@ import com.onesignal.user.internal.operations.impl.executors.UpdateUserOperation import com.onesignal.user.internal.operations.impl.listeners.IdentityModelStoreListener import com.onesignal.user.internal.operations.impl.listeners.PropertiesModelStoreListener import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener +import com.onesignal.user.internal.operations.impl.states.NewRecordsState import com.onesignal.user.internal.properties.PropertiesModelStore import com.onesignal.user.internal.service.UserRefreshService import com.onesignal.user.internal.subscriptions.ISubscriptionManager @@ -68,5 +69,8 @@ internal class UserModule : IModule { builder.register().provides() builder.register().provides() + + // Shared state between Executors + builder.register().provides() } } 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 59c463b7f8..3fe1b718b2 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 @@ -86,6 +86,7 @@ class CreateSubscriptionOperation() : Operation(SubscriptionOperationExecutor.CR override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Subscription.$subscriptionId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, subscriptionId: String, type: SubscriptionType, enabled: Boolean, address: String, status: SubscriptionStatus) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteAliasOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteAliasOperation.kt index c2ae81f5bb..1595a6de2b 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteAliasOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteAliasOperation.kt @@ -41,6 +41,7 @@ class DeleteAliasOperation() : Operation(IdentityOperationExecutor.DELETE_ALIAS) override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Alias.$label" override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, label: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteSubscriptionOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteSubscriptionOperation.kt index b7724a7404..40ee092d8b 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteSubscriptionOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteSubscriptionOperation.kt @@ -42,6 +42,7 @@ class DeleteSubscriptionOperation() : Operation(SubscriptionOperationExecutor.DE override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Subscription.$subscriptionId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) && !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = subscriptionId constructor(appId: String, onesignalId: String, subscriptionId: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteTagOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteTagOperation.kt index 023187cbac..f88ae3c568 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteTagOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteTagOperation.kt @@ -42,6 +42,7 @@ class DeleteTagOperation() : Operation(UpdateUserOperationExecutor.DELETE_TAG) { override val modifyComparisonKey: String get() = "$appId.User.$onesignalId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, key: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserFromSubscriptionOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserFromSubscriptionOperation.kt index a8aa8af7e4..9597283f75 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserFromSubscriptionOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserFromSubscriptionOperation.kt @@ -40,6 +40,7 @@ class LoginUserFromSubscriptionOperation() : Operation(LoginUserFromSubscription override val modifyComparisonKey: String get() = "$appId.Subscription.$subscriptionId.Login" override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE override val canStartExecute: Boolean = true + override val applyToRecordId: String get() = subscriptionId constructor(appId: String, onesignalId: String, subscriptionId: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt index 67eb10ea9a..b283cc3da0 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt @@ -56,6 +56,7 @@ class LoginUserOperation() : Operation(LoginUserOperationExecutor.LOGIN_USER) { override val modifyComparisonKey: String = "" override val groupComparisonType: GroupComparisonType = GroupComparisonType.CREATE override val canStartExecute: Boolean get() = existingOnesignalId == null || !IDManager.isLocalId(existingOnesignalId!!) + override val applyToRecordId: String get() = existingOnesignalId ?: onesignalId constructor(appId: String, onesignalId: String, externalId: String?, existingOneSignalId: String? = null) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/RefreshUserOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/RefreshUserOperation.kt index bba5e1708c..953cbe7b9c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/RefreshUserOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/RefreshUserOperation.kt @@ -33,6 +33,7 @@ class RefreshUserOperation() : Operation(RefreshUserOperationExecutor.REFRESH_US override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Refresh" override val groupComparisonType: GroupComparisonType = GroupComparisonType.CREATE override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetAliasOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetAliasOperation.kt index 206c6d6556..c88c23e46f 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetAliasOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetAliasOperation.kt @@ -51,6 +51,7 @@ class SetAliasOperation() : Operation(IdentityOperationExecutor.SET_ALIAS) { override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Identity.$label" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, label: String, value: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetPropertyOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetPropertyOperation.kt index 94dc51fbff..2aff9c174a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetPropertyOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetPropertyOperation.kt @@ -50,6 +50,7 @@ class SetPropertyOperation() : Operation(UpdateUserOperationExecutor.SET_PROPERT override val modifyComparisonKey: String get() = "$appId.User.$onesignalId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, property: String, value: Any?) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetTagOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetTagOperation.kt index 25826bf7f3..88bfa06eda 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetTagOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/SetTagOperation.kt @@ -51,6 +51,7 @@ class SetTagOperation() : Operation(UpdateUserOperationExecutor.SET_TAG) { override val modifyComparisonKey: String get() = "$appId.User.$onesignalId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, key: String, value: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackPurchaseOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackPurchaseOperation.kt index 1d7dafbfb1..78da8cfb0a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackPurchaseOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackPurchaseOperation.kt @@ -63,6 +63,7 @@ class TrackPurchaseOperation() : Operation(UpdateUserOperationExecutor.TRACK_PUR override val modifyComparisonKey: String get() = "$appId.User.$onesignalId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, treatNewAsExisting: Boolean, amountSpent: BigDecimal, purchases: List) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionEndOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionEndOperation.kt index 7b5ef70442..940051e91b 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionEndOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionEndOperation.kt @@ -41,6 +41,7 @@ class TrackSessionEndOperation() : Operation(UpdateUserOperationExecutor.TRACK_S override val modifyComparisonKey: String get() = "$appId.User.$onesignalId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String, sessionTime: Long) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionStartOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionStartOperation.kt index 4512857491..e5b9e0f29c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionStartOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackSessionStartOperation.kt @@ -32,6 +32,7 @@ class TrackSessionStartOperation() : Operation(UpdateUserOperationExecutor.TRACK override val modifyComparisonKey: String get() = "$appId.User.$onesignalId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = onesignalId constructor(appId: String, onesignalId: String) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TransferSubscriptionOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TransferSubscriptionOperation.kt index fce653c323..0122bf8729 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TransferSubscriptionOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TransferSubscriptionOperation.kt @@ -42,6 +42,7 @@ class TransferSubscriptionOperation() : Operation(SubscriptionOperationExecutor. override val modifyComparisonKey: String get() = "$appId.Subscription.$subscriptionId.Transfer" override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) && !IDManager.isLocalId(subscriptionId) + override val applyToRecordId: String get() = subscriptionId constructor(appId: String, subscriptionId: String, onesignalId: String) : this() { this.appId = appId 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 38f3fa2108..59a9e1c1b7 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 @@ -85,6 +85,7 @@ class UpdateSubscriptionOperation() : Operation(SubscriptionOperationExecutor.UP override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Subscription.$subscriptionId" override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) && !IDManager.isLocalId(onesignalId) + override val applyToRecordId: String get() = subscriptionId constructor(appId: String, onesignalId: String, subscriptionId: String, type: SubscriptionType, enabled: Boolean, address: String, status: SubscriptionStatus) : this() { this.appId = appId diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt index 93b90faaa1..fa55a55757 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt @@ -14,11 +14,13 @@ import com.onesignal.user.internal.builduser.IRebuildUserService import com.onesignal.user.internal.identity.IdentityModelStore import com.onesignal.user.internal.operations.DeleteAliasOperation import com.onesignal.user.internal.operations.SetAliasOperation +import com.onesignal.user.internal.operations.impl.states.NewRecordsState internal class IdentityOperationExecutor( private val _identityBackend: IIdentityBackendService, private val _identityModelStore: IdentityModelStore, private val _buildUserService: IRebuildUserService, + private val _newRecordState: NewRecordsState, ) : IOperationExecutor { override val operations: List get() = listOf(SET_ALIAS, DELETE_ALIAS) @@ -67,11 +69,15 @@ internal class IdentityOperationExecutor( NetworkUtils.ResponseStatusType.UNAUTHORIZED -> ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) NetworkUtils.ResponseStatusType.MISSING -> { - val operations = _buildUserService.getRebuildOperationsIfCurrentUser(lastOperation.appId, lastOperation.onesignalId) - if (operations == null) { + if (ex.statusCode == 404 && _newRecordState.isInMissingRetryWindow(lastOperation.onesignalId)) { + return ExecutionResponse(ExecutionResult.FAIL_RETRY) + } + + val rebuildOps = _buildUserService.getRebuildOperationsIfCurrentUser(lastOperation.appId, lastOperation.onesignalId) + if (rebuildOps == null) { return ExecutionResponse(ExecutionResult.FAIL_NORETRY) } else { - return ExecutionResponse(ExecutionResult.FAIL_RETRY, operations = operations) + return ExecutionResponse(ExecutionResult.FAIL_RETRY, operations = rebuildOps) } } } @@ -103,10 +109,14 @@ internal class IdentityOperationExecutor( NetworkUtils.ResponseStatusType.UNAUTHORIZED -> ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) NetworkUtils.ResponseStatusType.MISSING -> { - // This means either the User or the Alias was already - // deleted, either way the end state is the same, the - // alias no longer exists on that User. - ExecutionResponse(ExecutionResult.SUCCESS) + return if (ex.statusCode == 404 && _newRecordState.isInMissingRetryWindow(lastOperation.onesignalId)) { + ExecutionResponse(ExecutionResult.FAIL_RETRY) + } else { + // This means either the User or the Alias was already + // deleted, either way the end state is the same, the + // alias no longer exists on that User. + ExecutionResponse(ExecutionResult.SUCCESS) + } } } } 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 4266ab9037..36e1650dc9 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 @@ -17,6 +17,7 @@ import com.onesignal.user.internal.builduser.IRebuildUserService import com.onesignal.user.internal.identity.IdentityModel import com.onesignal.user.internal.identity.IdentityModelStore import com.onesignal.user.internal.operations.RefreshUserOperation +import com.onesignal.user.internal.operations.impl.states.NewRecordsState import com.onesignal.user.internal.properties.PropertiesModel import com.onesignal.user.internal.properties.PropertiesModelStore import com.onesignal.user.internal.subscriptions.SubscriptionModel @@ -31,6 +32,7 @@ internal class RefreshUserOperationExecutor( private val _subscriptionsModelStore: SubscriptionModelStore, private val _configModelStore: ConfigModelStore, private val _buildUserService: IRebuildUserService, + private val _newRecordState: NewRecordsState, ) : IOperationExecutor { override val operations: List get() = listOf(REFRESH_USER) @@ -135,6 +137,9 @@ internal class RefreshUserOperationExecutor( NetworkUtils.ResponseStatusType.UNAUTHORIZED -> ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) NetworkUtils.ResponseStatusType.MISSING -> { + if (ex.statusCode == 404 && _newRecordState.isInMissingRetryWindow(op.onesignalId)) { + return ExecutionResponse(ExecutionResult.FAIL_RETRY) + } val operations = _buildUserService.getRebuildOperationsIfCurrentUser(op.appId, op.onesignalId) if (operations == null) { return ExecutionResponse(ExecutionResult.FAIL_NORETRY) 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 a9cdf9fe97..08bfd48be0 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 @@ -26,6 +26,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.states.NewRecordsState import com.onesignal.user.internal.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore import com.onesignal.user.internal.subscriptions.SubscriptionType @@ -37,6 +38,7 @@ internal class SubscriptionOperationExecutor( private val _subscriptionModelStore: SubscriptionModelStore, private val _configModelStore: ConfigModelStore, private val _buildUserService: IRebuildUserService, + private val _newRecordState: NewRecordsState, ) : IOperationExecutor { override val operations: List get() = listOf(CREATE_SUBSCRIPTION, UPDATE_SUBSCRIPTION, DELETE_SUBSCRIPTION, TRANSFER_SUBSCRIPTION) @@ -136,6 +138,9 @@ internal class SubscriptionOperationExecutor( NetworkUtils.ResponseStatusType.UNAUTHORIZED -> ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) NetworkUtils.ResponseStatusType.MISSING -> { + if (ex.statusCode == 404 && _newRecordState.isInMissingRetryWindow(createOperation.onesignalId)) { + return ExecutionResponse(ExecutionResult.FAIL_RETRY) + } val operations = _buildUserService.getRebuildOperationsIfCurrentUser(createOperation.appId, createOperation.onesignalId) if (operations == null) { return ExecutionResponse(ExecutionResult.FAIL_NORETRY) @@ -177,7 +182,15 @@ internal class SubscriptionOperationExecutor( return when (responseType) { NetworkUtils.ResponseStatusType.RETRYABLE -> ExecutionResponse(ExecutionResult.FAIL_RETRY) - NetworkUtils.ResponseStatusType.MISSING -> + NetworkUtils.ResponseStatusType.MISSING -> { + if (ex.statusCode == 404 && + listOf( + lastOperation.onesignalId, + lastOperation.subscriptionId, + ).any { _newRecordState.isInMissingRetryWindow(it) } + ) { + return ExecutionResponse(ExecutionResult.FAIL_RETRY) + } // toss this, but create an identical CreateSubscriptionOperation to re-create the subscription being updated. ExecutionResponse( ExecutionResult.FAIL_NORETRY, @@ -194,6 +207,7 @@ internal class SubscriptionOperationExecutor( ), ), ) + } else -> ExecutionResponse(ExecutionResult.FAIL_NORETRY) } @@ -248,9 +262,19 @@ internal class SubscriptionOperationExecutor( val responseType = NetworkUtils.getResponseStatusType(ex.statusCode) return when (responseType) { - NetworkUtils.ResponseStatusType.MISSING -> - // if the subscription is missing, we are good! - ExecutionResponse(ExecutionResult.SUCCESS) + NetworkUtils.ResponseStatusType.MISSING -> { + if (ex.statusCode == 404 && + listOf( + op.onesignalId, + op.subscriptionId, + ).any { _newRecordState.isInMissingRetryWindow(it) } + ) { + ExecutionResponse(ExecutionResult.FAIL_RETRY) + } else { + // if the subscription is missing, we are good! + ExecutionResponse(ExecutionResult.SUCCESS) + } + } NetworkUtils.ResponseStatusType.RETRYABLE -> ExecutionResponse(ExecutionResult.FAIL_RETRY) else -> diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/UpdateUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/UpdateUserOperationExecutor.kt index 1002ee1f0f..a512c8756f 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/UpdateUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/UpdateUserOperationExecutor.kt @@ -22,6 +22,7 @@ import com.onesignal.user.internal.operations.SetTagOperation import com.onesignal.user.internal.operations.TrackPurchaseOperation import com.onesignal.user.internal.operations.TrackSessionEndOperation import com.onesignal.user.internal.operations.TrackSessionStartOperation +import com.onesignal.user.internal.operations.impl.states.NewRecordsState import com.onesignal.user.internal.properties.PropertiesModelStore internal class UpdateUserOperationExecutor( @@ -29,6 +30,7 @@ internal class UpdateUserOperationExecutor( private val _identityModelStore: IdentityModelStore, private val _propertiesModelStore: PropertiesModelStore, private val _buildUserService: IRebuildUserService, + private val _newRecordState: NewRecordsState, ) : IOperationExecutor { override val operations: List get() = listOf(SET_TAG, DELETE_TAG, SET_PROPERTY, TRACK_SESSION_START, TRACK_SESSION_END, TRACK_PURCHASE) @@ -163,6 +165,9 @@ internal class UpdateUserOperationExecutor( NetworkUtils.ResponseStatusType.UNAUTHORIZED -> ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) NetworkUtils.ResponseStatusType.MISSING -> { + if (ex.statusCode == 404 && _newRecordState.isInMissingRetryWindow(onesignalId)) { + return ExecutionResponse(ExecutionResult.FAIL_RETRY) + } val operations = _buildUserService.getRebuildOperationsIfCurrentUser(appId, onesignalId) if (operations == null) { return ExecutionResponse(ExecutionResult.FAIL_NORETRY) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/states/NewRecordsState.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/states/NewRecordsState.kt new file mode 100644 index 0000000000..e884fc7f6d --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/states/NewRecordsState.kt @@ -0,0 +1,33 @@ +package com.onesignal.user.internal.operations.impl.states + +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.time.ITime + +/** + * Purpose: Keeps track of ids that were just created on the backend. + * This list gets used to delay network calls to ensure upcoming + * requests are ready to be accepted by the backend. Also used for retries + * as a fallback if the server is under extra load. + */ +class NewRecordsState( + private val _time: ITime, + private val _configModelStore: ConfigModelStore, +) { + // Key = a string id + // Value = A Timestamp in ms of when the id was created + private val records: MutableMap = mutableMapOf() + + fun add(key: String) { + records[key] = _time.currentTimeMillis + } + + fun canAccess(key: String): Boolean { + val timeLastMovedOrCreated = records[key] ?: return true + return _time.currentTimeMillis - timeLastMovedOrCreated > _configModelStore.model.opRepoPostCreateDelay + } + + fun isInMissingRetryWindow(key: String): Boolean { + val timeLastMovedOrCreated = records[key] ?: return false + return _time.currentTimeMillis - timeLastMovedOrCreated <= _configModelStore.model.opRepoPostCreateRetryUpTo + } +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt index 243e539622..7a05f270ee 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt @@ -9,6 +9,7 @@ import com.onesignal.core.internal.time.impl.Time import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.mocks.MockHelper +import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.CapturingSlot @@ -23,7 +24,9 @@ import io.mockk.slot import io.mockk.spyk import io.mockk.verify import kotlinx.coroutines.delay +import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeoutOrNull +import kotlinx.coroutines.yield // Mocks used by every test in this file private class Mocks { @@ -53,6 +56,7 @@ private class Mocks { operationModelStore, configModelStore, Time(), + getNewRecordState(configModelStore), ), ) } @@ -73,6 +77,7 @@ class OperationRepoTests : FunSpec({ override val modifyComparisonKey = "" override val groupComparisonType = GroupComparisonType.NONE override val canStartExecute = false + override val applyToRecordId = "" } class MyOperation2 : MyOperation() @@ -485,6 +490,54 @@ class OperationRepoTests : FunSpec({ executor.execute(withArg { it[0] shouldBe secondOp }) } } + + // This is to account for the case where we create a User or Subscription + // and attempt to immediately access it (via GET or PATCH). A delay is + // needed as the backend may incorrectly 404 otherwise, due to a small + // delay in it's server replication. + // A cold down period like this also helps improve batching as well. + test("execution of an operation with translation IDs delays follow up operations") { + // Given + val mocks = Mocks() + mocks.configModelStore.model.opRepoPostCreateDelay = 100 + val operation1 = mockOperation(groupComparisonType = GroupComparisonType.NONE) + val operation2 = mockOperation(groupComparisonType = GroupComparisonType.NONE, applyToRecordId = "id2") + val operation3 = mockOperation(groupComparisonType = GroupComparisonType.NONE) + coEvery { + mocks.executor.execute(listOf(operation1)) + } returns ExecutionResponse(ExecutionResult.SUCCESS, mapOf("local-id1" to "id2")) + + // When + mocks.operationRepo.start() + mocks.operationRepo.enqueue(operation1) + val job = launch { mocks.operationRepo.enqueueAndWait(operation2) }.also { yield() } + mocks.operationRepo.enqueue(operation3) + job.join() + + // Then + coVerifyOrder { + mocks.executor.execute( + withArg { + it.count() shouldBe 1 + it[0] shouldBe operation1 + }, + ) + operation2.translateIds(mapOf("local-id1" to "id2")) + mocks.executor.execute( + withArg { + it.count() shouldBe 1 + it[0] shouldBe operation3 + }, + ) + // Ensure operation2 runs after operation3 as it has to wait for the create delay + mocks.executor.execute( + withArg { + it.count() shouldBe 1 + it[0] shouldBe operation2 + }, + ) + } + } }) { companion object { private fun mockOperation( @@ -495,6 +548,7 @@ class OperationRepoTests : FunSpec({ createComparisonKey: String = "create-key", modifyComparisonKey: String = "modify-key", operationIdSlot: CapturingSlot? = null, + applyToRecordId: String = "", ): Operation { val operation = mockk() val opIdSlot = operationIdSlot ?: slot() @@ -507,6 +561,7 @@ class OperationRepoTests : FunSpec({ every { operation.createComparisonKey } returns createComparisonKey every { operation.modifyComparisonKey } returns modifyComparisonKey every { operation.translateIds(any()) } just runs + every { operation.applyToRecordId } returns applyToRecordId return operation } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/ExecutorMocks.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/ExecutorMocks.kt new file mode 100644 index 0000000000..9890563333 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/ExecutorMocks.kt @@ -0,0 +1,12 @@ +package com.onesignal.user.internal.operations + +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.time.impl.Time +import com.onesignal.mocks.MockHelper +import com.onesignal.user.internal.operations.impl.states.NewRecordsState + +class ExecutorMocks { + companion object { + fun getNewRecordState(configModelStore: ConfigModelStore = MockHelper.configModelStore()) = NewRecordsState(Time(), configModelStore) + } +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt index 4721945a8c..0a9e53603f 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt @@ -10,6 +10,7 @@ import com.onesignal.user.internal.backend.IdentityConstants import com.onesignal.user.internal.builduser.IRebuildUserService import com.onesignal.user.internal.identity.IdentityModel import com.onesignal.user.internal.identity.IdentityModelStore +import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState import com.onesignal.user.internal.operations.impl.executors.IdentityOperationExecutor import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe @@ -37,7 +38,8 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() - val identityOperationExecutor = IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService) + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -59,7 +61,8 @@ class IdentityOperationExecutorTests : FunSpec({ val mockIdentityModelStore = MockHelper.identityModelStore() val mockBuildUserService = mockk() - val identityOperationExecutor = IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService) + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -78,7 +81,8 @@ class IdentityOperationExecutorTests : FunSpec({ val mockIdentityModelStore = MockHelper.identityModelStore() val mockBuildUserService = mockk() - val identityOperationExecutor = IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService) + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -89,6 +93,50 @@ class IdentityOperationExecutorTests : FunSpec({ response.result shouldBe ExecutionResult.FAIL_NORETRY } + test("execution of set alias operation with MISSING error") { + // Given + val mockIdentityBackendService = mockk() + coEvery { mockIdentityBackendService.setAlias(any(), any(), any(), any()) } throws BackendException(404) + + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockBuildUserService = mockk() + every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } returns null + + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) + + // When + + val response = identityOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_NORETRY + } + + test("execution of set alias operation with MISSING error, but isInMissingRetryWindow") { + // Given + val mockIdentityBackendService = mockk() + coEvery { mockIdentityBackendService.setAlias(any(), any(), any(), any()) } throws BackendException(404) + + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockBuildUserService = mockk() + every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } returns null + + val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } + val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add("onesignalId") } + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, newRecordState) + val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) + + // When + + val response = identityOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + } + test("execution of delete alias operation") { // Given val mockIdentityBackendService = mockk() @@ -103,7 +151,8 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() - val identityOperationExecutor = IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService) + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When @@ -125,7 +174,8 @@ class IdentityOperationExecutorTests : FunSpec({ val mockIdentityModelStore = MockHelper.identityModelStore() val mockBuildUserService = mockk() - val identityOperationExecutor = IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService) + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When @@ -144,7 +194,8 @@ class IdentityOperationExecutorTests : FunSpec({ val mockIdentityModelStore = MockHelper.identityModelStore() val mockBuildUserService = mockk() - val identityOperationExecutor = IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService) + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When @@ -154,4 +205,50 @@ class IdentityOperationExecutorTests : FunSpec({ // Then response.result shouldBe ExecutionResult.FAIL_NORETRY } + + // If we get a 404 then either the Alias and/or the User doesn't exist, + // ether way that Alias doesn't exist on the User currently so consider it as successful + test("execution of delete alias operation with MISSING error") { + // Given + val mockIdentityBackendService = mockk() + coEvery { mockIdentityBackendService.deleteAlias(any(), any(), any(), any()) } throws BackendException(404) + + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockBuildUserService = mockk() + + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) + + // When + + val response = identityOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.SUCCESS + } + + // The server may return a 404 incorrectly if we just created the User + // and it's replication is behind. We should retry if we are in the + // uncertainty window. + test("execution of delete alias operation with MISSING error, but isInMissingRetryWindow") { + // Given + val mockIdentityBackendService = mockk() + coEvery { mockIdentityBackendService.deleteAlias(any(), any(), any(), any()) } throws BackendException(404) + + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockBuildUserService = mockk() + + val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } + val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add("onesignalId") } + val identityOperationExecutor = + IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, newRecordState) + val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) + + // When + val response = identityOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + } }) 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 95ebac44e5..18681f4a10 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 @@ -13,6 +13,7 @@ import com.onesignal.user.internal.backend.SubscriptionObject import com.onesignal.user.internal.backend.SubscriptionObjectType import com.onesignal.user.internal.builduser.IRebuildUserService import com.onesignal.user.internal.identity.IdentityModel +import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState import com.onesignal.user.internal.operations.impl.executors.RefreshUserOperationExecutor import com.onesignal.user.internal.properties.PropertiesModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore @@ -40,7 +41,11 @@ class RefreshUserOperationExecutorTests : FunSpec({ CreateUserResponse( mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId, "aliasLabel1" to "aliasValue1"), PropertiesObject(country = "US"), - listOf(SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken1"), SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken2"), SubscriptionObject(remoteSubscriptionId2, SubscriptionObjectType.EMAIL, token = "name@company.com")), + listOf( + SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken1"), + SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken2"), + SubscriptionObject(remoteSubscriptionId2, SubscriptionObjectType.EMAIL, token = "name@company.com"), + ), ) // Given @@ -76,6 +81,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, mockConfigModelStore, mockBuildUserService, + getNewRecordState(), ) val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) @@ -150,6 +156,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) @@ -186,6 +193,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) @@ -219,6 +227,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) @@ -232,4 +241,76 @@ class RefreshUserOperationExecutorTests : FunSpec({ mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) } } + + test("refresh user fails without retry when backend returns MISSING") { + // Given + val mockUserBackendService = mockk() + coEvery { mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) } throws BackendException(404) + + // Given + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockPropertiesModelStore = MockHelper.propertiesModelStore() + val mockSubscriptionsModelStore = mockk() + val mockBuildUserService = mockk() + every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } returns null + + val loginUserOperationExecutor = + RefreshUserOperationExecutor( + mockUserBackendService, + mockIdentityModelStore, + mockPropertiesModelStore, + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) + + val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) + + // When + val response = loginUserOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_NORETRY + coVerify(exactly = 1) { + mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) + } + } + + test("refresh user is retried when backend returns MISSING, but isInMissingRetryWindow") { + // Given + val mockUserBackendService = mockk() + coEvery { mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) } throws BackendException(404) + + // Given + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockPropertiesModelStore = MockHelper.propertiesModelStore() + val mockSubscriptionsModelStore = mockk() + val mockBuildUserService = mockk() + + val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } + val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add(remoteOneSignalId) } + + val loginUserOperationExecutor = + RefreshUserOperationExecutor( + mockUserBackendService, + mockIdentityModelStore, + mockPropertiesModelStore, + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + newRecordState, + ) + + val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) + + // When + val response = loginUserOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + coVerify(exactly = 1) { + mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) + } + } }) 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 471e69f6fb..2d4f044ff1 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 @@ -10,6 +10,7 @@ import com.onesignal.user.internal.backend.ISubscriptionBackendService import com.onesignal.user.internal.backend.IdentityConstants import com.onesignal.user.internal.backend.SubscriptionObjectType import com.onesignal.user.internal.builduser.IRebuildUserService +import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState import com.onesignal.user.internal.operations.impl.executors.SubscriptionOperationExecutor import com.onesignal.user.internal.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore @@ -52,6 +53,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -104,6 +106,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -156,6 +159,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -191,6 +195,47 @@ class SubscriptionOperationExecutorTests : FunSpec({ } } + test("create subscription fails with retry when the backend returns MISSING, when isInMissingRetryWindow") { + // Given + val mockSubscriptionBackendService = mockk() + coEvery { mockSubscriptionBackendService.createSubscription(any(), any(), any(), any()) } throws BackendException(404) + + val mockSubscriptionsModelStore = mockk() + val mockBuildUserService = mockk() + val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } + val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add(remoteOneSignalId) } + + val subscriptionOperationExecutor = + SubscriptionOperationExecutor( + mockSubscriptionBackendService, + MockHelper.deviceService(), + AndroidMockHelper.applicationService(), + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + newRecordState, + ) + + val operations = + listOf( + CreateSubscriptionOperation( + appId, + remoteOneSignalId, + localSubscriptionId, + SubscriptionType.PUSH, + true, + "pushToken", + SubscriptionStatus.SUBSCRIBED, + ), + ) + + // When + val response = subscriptionOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + } + test("create subscription then delete subscription is a successful no-op") { // Given val mockSubscriptionBackendService = mockk() @@ -210,6 +255,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -253,6 +299,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -319,6 +366,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -378,6 +426,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -428,6 +477,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -462,6 +512,47 @@ class SubscriptionOperationExecutorTests : FunSpec({ } } + test("update subscription fails with retry when the backend returns MISSING, when isInMissingRetryWindow") { + // Given + val mockSubscriptionBackendService = mockk() + coEvery { mockSubscriptionBackendService.updateSubscription(any(), any(), any()) } throws BackendException(404) + + val mockSubscriptionsModelStore = mockk() + val mockBuildUserService = mockk() + val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } + val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add(remoteOneSignalId) } + + val subscriptionOperationExecutor = + SubscriptionOperationExecutor( + mockSubscriptionBackendService, + MockHelper.deviceService(), + AndroidMockHelper.applicationService(), + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + newRecordState, + ) + + val operations = + listOf( + UpdateSubscriptionOperation( + appId, + remoteOneSignalId, + remoteSubscriptionId, + SubscriptionType.PUSH, + true, + "pushToken2", + SubscriptionStatus.SUBSCRIBED, + ), + ) + + // When + val response = subscriptionOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + } + test("delete subscription successfully deletes subscription") { // Given val mockSubscriptionBackendService = mockk() @@ -480,6 +571,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -512,6 +604,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -545,6 +638,7 @@ class SubscriptionOperationExecutorTests : FunSpec({ mockSubscriptionsModelStore, MockHelper.configModelStore(), mockBuildUserService, + getNewRecordState(), ) val operations = @@ -559,4 +653,37 @@ class SubscriptionOperationExecutorTests : FunSpec({ response.result shouldBe ExecutionResult.SUCCESS coVerify(exactly = 1) { mockSubscriptionBackendService.deleteSubscription(appId, remoteSubscriptionId) } } + + test("delete subscription fails with retry when the backend returns MISSING, when isInMissingRetryWindow") { + // Given + val mockSubscriptionBackendService = mockk() + coEvery { mockSubscriptionBackendService.deleteSubscription(any(), any()) } throws BackendException(404) + + val mockSubscriptionsModelStore = mockk() + val mockBuildUserService = mockk() + val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } + val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add(remoteOneSignalId) } + + val subscriptionOperationExecutor = + SubscriptionOperationExecutor( + mockSubscriptionBackendService, + MockHelper.deviceService(), + AndroidMockHelper.applicationService(), + mockSubscriptionsModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + newRecordState, + ) + + val operations = + listOf( + DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId), + ) + + // When + val response = subscriptionOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/UpdateUserOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/UpdateUserOperationExecutorTests.kt index bad53be5d8..b2f916bf04 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/UpdateUserOperationExecutorTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/UpdateUserOperationExecutorTests.kt @@ -1,11 +1,13 @@ package com.onesignal.user.internal.operations +import com.onesignal.common.exceptions.BackendException import com.onesignal.core.internal.operations.ExecutionResult import com.onesignal.core.internal.operations.Operation import com.onesignal.mocks.MockHelper import com.onesignal.user.internal.backend.IUserBackendService import com.onesignal.user.internal.backend.IdentityConstants import com.onesignal.user.internal.builduser.IRebuildUserService +import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState import com.onesignal.user.internal.operations.impl.executors.UpdateUserOperationExecutor import com.onesignal.user.internal.properties.PropertiesModel import io.kotest.core.spec.style.FunSpec @@ -13,6 +15,7 @@ import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.runs @@ -39,6 +42,7 @@ class UpdateUserOperationExecutorTests : FunSpec({ mockIdentityModelStore, mockPropertiesModelStore, mockBuildUserService, + getNewRecordState(), ) val operations = listOf(SetTagOperation(appId, remoteOneSignalId, "tagKey1", "tagValue1")) @@ -77,6 +81,7 @@ class UpdateUserOperationExecutorTests : FunSpec({ mockIdentityModelStore, mockPropertiesModelStore, mockBuildUserService, + getNewRecordState(), ) val operations = listOf( @@ -137,6 +142,7 @@ class UpdateUserOperationExecutorTests : FunSpec({ mockIdentityModelStore, mockPropertiesModelStore, mockBuildUserService, + getNewRecordState(), ) val operations = listOf( @@ -180,6 +186,7 @@ class UpdateUserOperationExecutorTests : FunSpec({ mockIdentityModelStore, mockPropertiesModelStore, mockBuildUserService, + getNewRecordState(), ) val operations = listOf( @@ -243,6 +250,7 @@ class UpdateUserOperationExecutorTests : FunSpec({ mockIdentityModelStore, mockPropertiesModelStore, mockBuildUserService, + getNewRecordState(), ) val operations = listOf( @@ -271,4 +279,63 @@ class UpdateUserOperationExecutorTests : FunSpec({ ) } } + + test("update user single operation fails with MISSING") { + // Given + val mockUserBackendService = mockk() + coEvery { mockUserBackendService.updateUser(any(), any(), any(), any(), any(), any()) } throws BackendException(404) + + // Given + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockPropertiesModelStore = MockHelper.propertiesModelStore() + val mockBuildUserService = mockk() + every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } returns null + + val loginUserOperationExecutor = + UpdateUserOperationExecutor( + mockUserBackendService, + mockIdentityModelStore, + mockPropertiesModelStore, + mockBuildUserService, + getNewRecordState(), + ) + val operations = listOf(SetTagOperation(appId, remoteOneSignalId, "tagKey1", "tagValue1")) + + // When + val response = loginUserOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_NORETRY + } + + test("update user single operation fails with MISSING, but isInMissingRetryWindow") { + // Given + val mockUserBackendService = mockk() + coEvery { mockUserBackendService.updateUser(any(), any(), any(), any(), any(), any()) } throws BackendException(404) + + // Given + val mockIdentityModelStore = MockHelper.identityModelStore() + val mockPropertiesModelStore = MockHelper.propertiesModelStore() + val mockBuildUserService = mockk() + every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } returns null + + val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } + val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add(remoteOneSignalId) } + + val loginUserOperationExecutor = + UpdateUserOperationExecutor( + mockUserBackendService, + mockIdentityModelStore, + mockPropertiesModelStore, + mockBuildUserService, + newRecordState, + ) + val operations = listOf(SetTagOperation(appId, remoteOneSignalId, "tagKey1", "tagValue1")) + + // When + val response = loginUserOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.FAIL_RETRY + } }) diff --git a/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/MockHelper.kt b/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/MockHelper.kt index 27901d92bd..a29345a5f8 100644 --- a/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/MockHelper.kt +++ b/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/MockHelper.kt @@ -46,6 +46,8 @@ object MockHelper { configModel.opRepoExecutionInterval = 1 configModel.opRepoPostWakeDelay = 1 + configModel.opRepoPostCreateDelay = 1 + configModel.opRepoPostCreateRetryUpTo = 1 configModel.appId = DEFAULT_APP_ID