Skip to content

Commit 59fd1df

Browse files
committed
Add opRepoPostCreateDelay
This is a new behavior which wakes a specific amount of time after something is created (such as a User or Subscription) before make a network call to get or update it. The main motivation is that the OneSignal's server may return a 404 if you attempt to GET or PATCH on something that was just created. This is due fact that OneSignal's backend server replication sometimes has a delay under load. This may be considered a bug in the backend, but on the flip side the SDK is being inefficient if a follow up network request is made any way, which was the 2nd motivation for this change.
1 parent 83fa82b commit 59fd1df

22 files changed

+93
-9
lines changed

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/Operation.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ abstract class Operation(name: String) : Model() {
2020
this.name = name
2121
}
2222

23+
/**
24+
* This is a unique id that points to a record this operation will affect.
25+
* Example: If the operation is updating tags on a User this will be the onesignalId.
26+
*/
27+
abstract val applyToRecordId: String
28+
2329
/**
2430
* The key of this operation for when the starting operation has a [groupComparisonType]
2531
* of [GroupComparisonType.CREATE]

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.onesignal.core.internal.operations.impl
22

33
import com.onesignal.common.threading.WaiterWithValue
4-
import com.onesignal.common.threading.suspendifyOnThread
54
import com.onesignal.core.internal.config.ConfigModelStore
65
import com.onesignal.core.internal.operations.ExecutionResult
76
import com.onesignal.core.internal.operations.GroupComparisonType
@@ -12,7 +11,11 @@ import com.onesignal.core.internal.startup.IStartableService
1211
import com.onesignal.core.internal.time.ITime
1312
import com.onesignal.debug.LogLevel
1413
import com.onesignal.debug.internal.logging.Logging
14+
import com.onesignal.user.internal.operations.impl.states.NewRecordsState
15+
import kotlinx.coroutines.CoroutineScope
1516
import kotlinx.coroutines.delay
17+
import kotlinx.coroutines.launch
18+
import kotlinx.coroutines.newSingleThreadContext
1619
import kotlinx.coroutines.withTimeoutOrNull
1720
import java.util.UUID
1821
import kotlin.reflect.KClass
@@ -22,6 +25,7 @@ internal class OperationRepo(
2225
private val _operationModelStore: OperationModelStore,
2326
private val _configModelStore: ConfigModelStore,
2427
private val _time: ITime,
28+
private val _newRecordState: NewRecordsState,
2529
) : IOperationRepo, IStartableService {
2630
internal class OperationQueueItem(
2731
val operation: Operation,
@@ -38,6 +42,7 @@ internal class OperationRepo(
3842
private val queue = mutableListOf<OperationQueueItem>()
3943
private val waiter = WaiterWithValue<Boolean>()
4044
private var paused = false
45+
private var coroutineScope = CoroutineScope(newSingleThreadContext(name = "OpRepo"))
4146

4247
/** *** Buckets ***
4348
* Purpose: Bucketing is a pattern we are using to help save network
@@ -56,13 +61,11 @@ internal class OperationRepo(
5661
* network calls.
5762
*/
5863
private var enqueueIntoBucket = 0
59-
private val executeBucket: Int get() {
60-
return if (enqueueIntoBucket == 0) 0 else enqueueIntoBucket - 1
61-
}
64+
private val executeBucket get() =
65+
if (enqueueIntoBucket == 0) 0 else enqueueIntoBucket - 1
6266

6367
init {
6468
val executorsMap: MutableMap<String, IOperationExecutor> = mutableMapOf()
65-
6669
for (executor in executors) {
6770
for (operation in executor.operations) {
6871
executorsMap[operation] = executor
@@ -83,9 +86,7 @@ internal class OperationRepo(
8386

8487
override fun start() {
8588
paused = false
86-
suspendifyOnThread(name = "OpRepo") {
87-
processQueueForever()
88-
}
89+
coroutineScope.launch { processQueueForever() }
8990
}
9091

9192
override fun enqueue(
@@ -195,6 +196,18 @@ internal class OperationRepo(
195196
synchronized(queue) {
196197
queue.forEach { it.operation.translateIds(response.idTranslations) }
197198
}
199+
response.idTranslations.values.forEach { _newRecordState.add(it) }
200+
coroutineScope.launch {
201+
delay(_configModelStore.model.opRepoPostCreateDelay)
202+
synchronized(queue) {
203+
// NOTE: Even if the queue is not empty we may wake
204+
// when not needed, as those operations may not have
205+
// depended on these ids. This however should be very
206+
// rare and the side-effect is only a bit less
207+
// batching.
208+
if (queue.isNotEmpty()) waiter.wake(false)
209+
}
210+
}
198211
}
199212

200213
when (response.result) {
@@ -278,7 +291,9 @@ internal class OperationRepo(
278291
return synchronized(queue) {
279292
val startingOp =
280293
queue.firstOrNull {
281-
it.operation.canStartExecute && it.bucket <= bucketFilter
294+
it.operation.canStartExecute &&
295+
_newRecordState.canAccess(it.operation.applyToRecordId) &&
296+
it.bucket <= bucketFilter
282297
}
283298

284299
if (startingOp != null) {

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/UserModule.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import com.onesignal.user.internal.operations.impl.executors.UpdateUserOperation
2525
import com.onesignal.user.internal.operations.impl.listeners.IdentityModelStoreListener
2626
import com.onesignal.user.internal.operations.impl.listeners.PropertiesModelStoreListener
2727
import com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener
28+
import com.onesignal.user.internal.operations.impl.states.NewRecordsState
2829
import com.onesignal.user.internal.properties.PropertiesModelStore
2930
import com.onesignal.user.internal.service.UserRefreshService
3031
import com.onesignal.user.internal.subscriptions.ISubscriptionManager
@@ -68,5 +69,8 @@ internal class UserModule : IModule {
6869
builder.register<UserRefreshService>().provides<IStartableService>()
6970

7071
builder.register<RecoverFromDroppedLoginBug>().provides<IStartableService>()
72+
73+
// Shared state between Executors
74+
builder.register<NewRecordsState>().provides<NewRecordsState>()
7175
}
7276
}

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/CreateSubscriptionOperation.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ class CreateSubscriptionOperation() : Operation(SubscriptionOperationExecutor.CR
8686
override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Subscription.$subscriptionId"
8787
override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER
8888
override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId)
89+
override val applyToRecordId: String get() = onesignalId
8990

9091
constructor(appId: String, onesignalId: String, subscriptionId: String, type: SubscriptionType, enabled: Boolean, address: String, status: SubscriptionStatus) : this() {
9192
this.appId = appId

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteAliasOperation.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class DeleteAliasOperation() : Operation(IdentityOperationExecutor.DELETE_ALIAS)
4141
override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Alias.$label"
4242
override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE
4343
override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId)
44+
override val applyToRecordId: String get() = onesignalId
4445

4546
constructor(appId: String, onesignalId: String, label: String) : this() {
4647
this.appId = appId

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteSubscriptionOperation.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class DeleteSubscriptionOperation() : Operation(SubscriptionOperationExecutor.DE
4242
override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Subscription.$subscriptionId"
4343
override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE
4444
override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) && !IDManager.isLocalId(onesignalId)
45+
override val applyToRecordId: String get() = subscriptionId
4546

4647
constructor(appId: String, onesignalId: String, subscriptionId: String) : this() {
4748
this.appId = appId

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/DeleteTagOperation.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class DeleteTagOperation() : Operation(UpdateUserOperationExecutor.DELETE_TAG) {
4242
override val modifyComparisonKey: String get() = "$appId.User.$onesignalId"
4343
override val groupComparisonType: GroupComparisonType = GroupComparisonType.ALTER
4444
override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId)
45+
override val applyToRecordId: String get() = onesignalId
4546

4647
constructor(appId: String, onesignalId: String, key: String) : this() {
4748
this.appId = appId

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserFromSubscriptionOperation.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class LoginUserFromSubscriptionOperation() : Operation(LoginUserFromSubscription
4040
override val modifyComparisonKey: String get() = "$appId.Subscription.$subscriptionId.Login"
4141
override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE
4242
override val canStartExecute: Boolean = true
43+
override val applyToRecordId: String get() = subscriptionId
4344

4445
constructor(appId: String, onesignalId: String, subscriptionId: String) : this() {
4546
this.appId = appId

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ class LoginUserOperation() : Operation(LoginUserOperationExecutor.LOGIN_USER) {
5656
override val modifyComparisonKey: String = ""
5757
override val groupComparisonType: GroupComparisonType = GroupComparisonType.CREATE
5858
override val canStartExecute: Boolean get() = existingOnesignalId == null || !IDManager.isLocalId(existingOnesignalId!!)
59+
override val applyToRecordId: String get() = onesignalId
5960

6061
constructor(appId: String, onesignalId: String, externalId: String?, existingOneSignalId: String? = null) : this() {
6162
this.appId = appId

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/RefreshUserOperation.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class RefreshUserOperation() : Operation(RefreshUserOperationExecutor.REFRESH_US
3333
override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Refresh"
3434
override val groupComparisonType: GroupComparisonType = GroupComparisonType.CREATE
3535
override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId)
36+
override val applyToRecordId: String get() = onesignalId
3637

3738
constructor(appId: String, onesignalId: String) : this() {
3839
this.appId = appId

0 commit comments

Comments
 (0)