feat: make LocalStorageRepository#insertEvents thread safe.

By using mutual exclusion (`Mutex`). Also, remove the `persistEvent` method, as it's no longer needed actually.
Parsely · Nov 7, 2023 · 0487719 · 0487719
1 parent 71cc691
commit 0487719
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 22 deletions.
diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt
@@ -5,8 +5,14 @@ import java.io.EOFException
 import java.io.FileNotFoundException
 import java.io.ObjectInputStream
 import java.io.ObjectOutputStream
+import kotlinx.coroutines.currentCoroutineContext
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 
 internal open class LocalStorageRepository(private val context: Context) {
+
+    private val mutex = Mutex()
+
     /**
      * Persist an object to storage.
      *
@@ -73,19 +79,13 @@ internal open class LocalStorageRepository(private val context: Context) {
         storedQueue.removeAt(0)
     }
 
-    open fun persistEvent(event: Map<String, Any?>) {
-        val storedQueue = getStoredQueue()
-        ParselyTracker.PLog("Persisting event queue. Current size: ${storedQueue.size}")
-        persistObject(ArrayList(storedQueue.plus(event).distinct()))
-    }
-
     /**
      * Save the event queue to persistent storage.
      */
-    @Synchronized
-    open fun persistQueue(inMemoryQueue: List<Map<String, Any?>?>) {
+    open suspend fun insertEvents(toInsert: List<Map<String, Any?>?>) = mutex.withLock {
+        println("Test: ${currentCoroutineContext()}")
         ParselyTracker.PLog("Persisting event queue")
-        persistObject((inMemoryQueue + getStoredQueue()).distinct())
+        persistObject(ArrayList((toInsert + getStoredQueue()).distinct()))
     }
 
     companion object {

diff --git a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt
@@ -3,12 +3,16 @@ package com.parsely.parselyandroid
 import android.content.Context
 import androidx.test.core.app.ApplicationProvider
 import java.io.File
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.runCurrent
+import kotlinx.coroutines.test.runTest
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 
+@OptIn(ExperimentalCoroutinesApi::class)
 @RunWith(RobolectricTestRunner::class)
 class LocalStorageRepositoryTest {
 
@@ -21,11 +25,10 @@ class LocalStorageRepositoryTest {
     }
 
     @Test
-    fun `when expelling stored event, then assert that it has no effect`() {
+    fun `when expelling stored event, then assert that it has no effect`() = runTest {
         // given
-        ((1..100).map { mapOf("index" to it) }).forEach {
-            sut.persistEvent(it)
-        }
+        sut.insertEvents(((1..100).map { mapOf("index" to it) }))
+        runCurrent()
 
         // when
         sut.expelStoredEvent()
@@ -35,14 +38,13 @@ class LocalStorageRepositoryTest {
     }
 
     @Test
-    fun `given the list of events, when persisting the list, then querying the list returns the same result`() {
+    fun `given the list of events, when persisting the list, then querying the list returns the same result`() = runTest {
         // given
         val eventsList = (1..10).map { mapOf("index" to it) }
 
         // when
-        eventsList.forEach {
-            sut.persistEvent(it)
-        }
+        sut.insertEvents(eventsList)
+        runCurrent()
 
         // then
         assertThat(sut.getStoredQueue()).hasSize(10).containsExactlyInAnyOrderElementsOf(eventsList)
@@ -54,25 +56,28 @@ class LocalStorageRepositoryTest {
     }
 
     @Test
-    fun `given stored queue with some elements, when persisting an event, then assert there'll be no duplicates`() {
+    fun `given stored queue with some elements, when persisting an event, then assert there'll be no duplicates`() = runTest {
         // given
         val storedQueue = (1..5).map { mapOf("index" to it) }
         val newEvents = (3..10).map { mapOf("index" to it) }
-        storedQueue.forEach { sut.persistEvent(it) }
+        sut.insertEvents(storedQueue)
+        runCurrent()
 
         // when
-        newEvents.forEach { sut.persistEvent(it) }
+        sut.insertEvents(newEvents)
+        runCurrent()
 
         // then
         val expectedQueue = (1..10).map { mapOf("index" to it) }
         assertThat(sut.getStoredQueue()).hasSize(10).containsExactlyInAnyOrderElementsOf(expectedQueue)
     }
 
     @Test
-    fun `given stored queue, when removing some events, then assert queue is doesn't contain removed events and contains not removed events`() {
+    fun `given stored queue, when removing some events, then assert queue is doesn't contain removed events and contains not removed events`() = runTest {
         // given
         val initialList = (1..10).map { mapOf("index" to it) }
-        initialList.forEach { sut.persistEvent(it) }
+        sut.insertEvents(initialList)
+        runCurrent()
         val eventsToRemove = initialList.slice(0..5)
         val eventsToKeep = initialList.slice(6..9)