stall OperationRepo by opRepoPostCreateDelay

The OperationRepo can't juggle two different users correctly, so stall the whole queue by opRepoPostCreateDelay. We plan to address this limitation in a future PR.
OneSignal · May 16, 2024 · 9f8afc0 · 9f8afc0
1 parent 5d5c837
commit 9f8afc0
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 44 deletions.
diff --git a/...onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt b/...onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt
@@ -235,12 +235,17 @@ internal class OperationRepo(
                     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))
-                    }
+                // Stall processing the queue so the backend's DB has to time
+                // reflect the change before we do any other operations to it.
+                // NOTE: Future: We could run this logic in a
+                // coroutineScope.launch() block so other operations not
+                // effecting this these id's can still be done in parallel,
+                // however other parts of the system don't currently account
+                // for this so this is not safe to do.
+                val waitTime = _configModelStore.model.opRepoPostCreateDelay
+                delay(waitTime)
+                synchronized(queue) {
+                    if (queue.isNotEmpty()) waiter.wake(LoopWaiterMessage(false, waitTime))
                 }
             }
 

diff --git a/...onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt b/...onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt
@@ -546,31 +546,15 @@ class OperationRepoTests : FunSpec({
         mocks.operationRepo.start()
         mocks.operationRepo.enqueue(operation1)
         val job = launch { mocks.operationRepo.enqueueAndWait(operation2) }.also { yield() }
-        mocks.operationRepo.enqueue(operation3)
+        mocks.operationRepo.enqueueAndWait(operation3)
         job.join()
 
         // Then
         coVerifyOrder {
-            mocks.executor.execute(
-                withArg {
-                    it.count() shouldBe 1
-                    it[0] shouldBe operation1
-                },
-            )
+            mocks.executor.execute(listOf(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
-                },
-            )
+            mocks.executor.execute(listOf(operation2))
+            mocks.executor.execute(listOf(operation3))
         }
     }
 
@@ -595,25 +579,9 @@ class OperationRepoTests : FunSpec({
 
         // Then
         coVerifyOrder {
-            mocks.executor.execute(
-                withArg {
-                    it.count() shouldBe 1
-                    it[0] shouldBe operation1
-                },
-            )
+            mocks.executor.execute(listOf(operation1))
             operation2.translateIds(mapOf("local-id1" to "id2"))
-            mocks.executor.execute(
-                withArg {
-                    it.count() shouldBe 1
-                    it[0] shouldBe operation2
-                },
-            )
-            mocks.executor.execute(
-                withArg {
-                    it.count() shouldBe 1
-                    it[0] shouldBe operation3
-                },
-            )
+            mocks.executor.execute(listOf(operation2, operation3))
         }
     }