Skip to content

Commit 0988977

Browse files
committed
create test to prove operOp's enqueue doesn't wait
This commit simply adds a test to prove the following issue with the current implementation of OperationRepo. Once processQueueForever starts processing operations it will execute them back-to-back until it can't process any more. This is done intently as the idea is sync all changes to the backend quickly, after waiting a set amount of time for batching. However nothing is in place to account for something continually adding new operations to the repo while it's in this mode. Some misbehaving app could be adding operations in a tight loop as fast or faster than the queue could exclude them.
1 parent 8898cb8 commit 0988977

File tree

2 files changed

+33
-1
lines changed

2 files changed

+33
-1
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ internal class OperationRepo(
151151
}
152152
}
153153

154-
private suspend fun executeOperations(ops: List<OperationQueueItem>) {
154+
internal suspend fun executeOperations(ops: List<OperationQueueItem>) {
155155
try {
156156
val startingOp = ops.first()
157157
val executor =

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import io.kotest.core.spec.style.FunSpec
1111
import io.kotest.matchers.shouldBe
1212
import io.mockk.CapturingSlot
1313
import io.mockk.coEvery
14+
import io.mockk.coVerify
1415
import io.mockk.coVerifyOrder
1516
import io.mockk.every
1617
import io.mockk.just
@@ -370,6 +371,37 @@ class OperationRepoTests : FunSpec({
370371
}
371372
response shouldBe true
372373
}
374+
375+
// This ensures a misbehaving app can't add operations (such as addTag())
376+
// in a tight loop and cause a number of back-to-back operations without delay.
377+
test("operations enqueued while repo is executing should be executed only after the next opRepoExecutionInterval") {
378+
// Given
379+
val mocks = Mocks()
380+
mocks.configModelStore.model.opRepoExecutionInterval = 100
381+
val enqueueAndWaitMaxTime = mocks.configModelStore.model.opRepoExecutionInterval / 2
382+
val opRepo = mocks.operationRepo
383+
384+
val executeOperationsCall = Waiter()
385+
coEvery { opRepo.executeOperations(any()) } coAnswers {
386+
executeOperationsCall.wake()
387+
delay(10)
388+
}
389+
390+
// When
391+
opRepo.start()
392+
opRepo.enqueue(mockOperation(groupComparisonType = GroupComparisonType.NONE))
393+
executeOperationsCall.waitForWake()
394+
val secondEnqueueResult =
395+
withTimeoutOrNull(enqueueAndWaitMaxTime) {
396+
opRepo.enqueueAndWait(mockOperation(groupComparisonType = GroupComparisonType.NONE))
397+
}
398+
399+
// Then
400+
secondEnqueueResult shouldBe null
401+
coVerify(exactly = 1) {
402+
opRepo.executeOperations(any())
403+
}
404+
}
373405
}) {
374406
companion object {
375407
private fun mockOperation(

0 commit comments

Comments
 (0)