improve concurrency safety of Repository and Settings

core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/Repository.kt

@@ -16,51 +16,31 @@ class Repository(private val storage: Settings) {
     }
 
     var tcData: IABData
-        get() = storage.withLock {
-            keys
-                .filter { it.startsWith(TCF_PREFIX) }
-                .associateWith { this[it]!! }
-        }
-        set(value) {
-            storage.withLock {
-                removeKeysStartingWith(prefix = TCF_PREFIX)
-                value.entries.forEach { this[it.key] = it.value }
-            }
-        }
+        get() = storage.getKeysWithPrefix(TCF_PREFIX)
+        set(value) = storage.replaceKeysWithPrefix(TCF_PREFIX, value)
 
     var gppData: IABData
-        get() = storage.withLock {
-            keys
-                .filter { it.startsWith(GPP_PREFIX) }
-                .associateWith { this[it]!! }
-        }
-        set(value) {
-            storage.withLock {
-                removeKeysStartingWith(prefix = GPP_PREFIX)
-                value.entries.forEach { this[it.key] = it.value }
-            }
-        }
+        get() = storage.getKeysWithPrefix(GPP_PREFIX)
+        set(value) = storage.replaceKeysWithPrefix(GPP_PREFIX, value)
 
     var uspString: String?
-        get() = storage.withLock { this[USPSTRING_KEY] }
-        set(value) { storage.withLock { this[USPSTRING_KEY] = value } }
+        get() = storage.readStringOrNull(USPSTRING_KEY)
+        set(value) = storage.writeString(USPSTRING_KEY, value)
 
     var state: State?
         get() = runCatching {
-            storage.withLock {
-                Json.decodeFromString<State>(getString(SP_STATE_KEY, defaultValue = ""))
-            }
+            val json = storage.readString(SP_STATE_KEY, defaultValue = "")
+            if (json.isBlank()) null else Json.decodeFromString<State>(json)
         }.getOrNull()
         set(value) {
-            storage.withLock { this[SP_STATE_KEY] = Json.encodeToString(value) }
+            val json = value?.let { Json.encodeToString(it) }
+            storage.writeString(SP_STATE_KEY, json)
         }
 
     fun clear() {
-        storage.withLock {
-            removeKeysStartingWith(prefix = TCF_PREFIX)
-            removeKeysStartingWith(prefix = GPP_PREFIX)
-            remove(USPSTRING_KEY)
-            remove(SP_STATE_KEY)
-        }
+        storage.removeKeysStartingWith(TCF_PREFIX)
+        storage.removeKeysStartingWith(GPP_PREFIX)
+        storage.delete(USPSTRING_KEY)
+        storage.delete(SP_STATE_KEY)
     }
 }

core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/SettingsExt.kt

@@ -1,8 +1,7 @@
 package com.sourcepoint.mobile_core.storage
 
+import com.russhwolf.settings.MapSettings
 import com.russhwolf.settings.Settings
-import kotlinx.coroutines.runBlocking
-import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.InternalCoroutinesApi
 import kotlinx.coroutines.internal.SynchronizedObject
 import kotlinx.coroutines.internal.synchronized
@@ -37,6 +36,33 @@ internal fun Settings.removeKeysStartingWith(prefix: String) = withLock {
     toRemove.forEach { remove(it) }
 }
 
+internal fun Settings.getKeysWithPrefix(prefix: String): Map<String, JsonPrimitive> = withLock {
+    keys.filter { it.startsWith(prefix) }
+        .associateWith { getJsonPrimitive(it) }
+}
+
+internal fun Settings.replaceKeysWithPrefix(prefix: String, data: Map<String, JsonPrimitive>) = withLock {
+    val toRemove = keys.filter { it.startsWith(prefix) }
+    toRemove.forEach { remove(it) }
+    data.forEach { (key, value) -> putJsonPrimitive(key, value) }
+}
+
+internal fun Settings.readString(key: String, defaultValue: String = ""): String = withLock {
+    getString(key, defaultValue)
+}
+
+internal fun Settings.readStringOrNull(key: String): String? = withLock {
+    getStringOrNull(key)
+}
+
+internal fun Settings.writeString(key: String, value: String?) = withLock {
+    if (value == null) remove(key) else putString(key, value)
+}
+
+internal fun Settings.delete(key: String) = withLock {
+    remove(key)
+}
+
 internal operator fun Settings.set(key: String, value: JsonPrimitive) = putJsonPrimitive(key, value)
 
 internal fun Settings.putJsonPrimitive(key: String, value: JsonPrimitive) {

core/src/commonTest/kotlin/com/sourcepoint/mobile_core/RepositoryConcurrencyTest.kt

@@ -0,0 +1,177 @@
+package com.sourcepoint.mobile_core
+
+import com.russhwolf.settings.MapSettings
+import com.sourcepoint.mobile_core.models.consents.State
+import com.sourcepoint.mobile_core.storage.Repository
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.test.runTest
+import kotlinx.serialization.json.JsonPrimitive
+import com.sourcepoint.mobile_core.asserters.assertDoesNotContain
+import com.sourcepoint.mobile_core.asserters.assertIsEmpty
+import com.sourcepoint.mobile_core.asserters.assertNotEmpty
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
+import kotlin.time.Duration.Companion.milliseconds
+
+class RepositoryConcurrencyTest {
+
+    @Test
+    fun concurrentWritesToTcDataProduceSingleEntry() = runTest {
+        val repository = Repository(MapSettings())
+        repository.tcData = mapOf("IABTCF_initial" to JsonPrimitive("should_be_removed"))
+
+        val jobs = (0 until 10).map { threadId ->
+            async(Dispatchers.Default) {
+                repeat(100) { iteration ->
+                    repository.tcData = mapOf(
+                        "IABTCF_key1" to JsonPrimitive("thread${threadId}_iter${iteration}"),
+                        "IABTCF_key2" to JsonPrimitive("thread${threadId}_iter${iteration}")
+                    )
+                }
+            }
+        }
+        jobs.awaitAll()
+
+        val tcData = repository.tcData
+        assertEquals(2, tcData.size, "Expected exactly 2 keys after concurrent writes")
+        assertDoesNotContain(tcData.keys, "IABTCF_initial")
+        assertEquals(tcData["IABTCF_key1"], tcData["IABTCF_key2"])
+    }
+
+    @Test
+    fun concurrentWritesToGppDataProduceSingleEntry() = runTest {
+        val repository = Repository(MapSettings())
+        repository.gppData = mapOf("IABGPP_initial" to JsonPrimitive("should_be_removed"))
+
+        val jobs = (0 until 10).map { threadId ->
+            async(Dispatchers.Default) {
+                repeat(100) { iteration ->
+                    repository.gppData = mapOf(
+                        "IABGPP_key1" to JsonPrimitive("thread${threadId}_iter${iteration}"),
+                        "IABGPP_key2" to JsonPrimitive("thread${threadId}_iter${iteration}")
+                    )
+                }
+            }
+        }
+        jobs.awaitAll()
+
+        val gppData = repository.gppData
+        assertEquals(2, gppData.size, "Expected exactly 2 keys after concurrent writes")
+        assertDoesNotContain(gppData.keys, "IABGPP_initial")
+        assertEquals(gppData["IABGPP_key1"], gppData["IABGPP_key2"])
+    }
+
+    @Test
+    fun concurrentReadsNeverSeeInconsistentTcData() = runTest {
+        val repository = Repository(MapSettings())
+        repository.tcData = mapOf("IABTCF_initial" to JsonPrimitive("initial_value"))
+
+        val writeJobs = (0 until 50).map { writeId ->
+            async(Dispatchers.Default) {
+                repository.tcData = mapOf("IABTCF_key" to JsonPrimitive("value_$writeId"))
+            }
+        }
+
+        val readJobs = (0 until 50).map {
+            async(Dispatchers.Default) {
+                assertNotEmpty(repository.tcData)
+            }
+        }
+
+        (writeJobs + readJobs).awaitAll()
+        assertEquals(1, repository.tcData.size)
+    }
+
+    @Test
+    fun concurrentReadsNeverSeeInconsistentGppData() = runTest {
+        val repository = Repository(MapSettings())
+        repository.gppData = mapOf("IABGPP_initial" to JsonPrimitive("initial_value"))
+
+        val writeJobs = (0 until 50).map { writeId ->
+            async(Dispatchers.Default) {
+                repository.gppData = mapOf("IABGPP_key" to JsonPrimitive("value_$writeId"))
+            }
+        }
+
+        val readJobs = (0 until 50).map {
+            async(Dispatchers.Default) {
+                assertNotEmpty(repository.gppData)
+            }
+        }
+
+        (writeJobs + readJobs).awaitAll()
+        assertEquals(1, repository.gppData.size)
+    }
+
+    @Test
+    fun concurrentClearOperationsLeaveStorageEmpty() = runTest {
+        val storage = MapSettings()
+        val repository = Repository(storage)
+
+        val jobs = (0 until 100).map { iteration ->
+            async(Dispatchers.Default) {
+                repository.tcData = mapOf("IABTCF_key" to JsonPrimitive("value"))
+                repository.gppData = mapOf("IABGPP_key" to JsonPrimitive("value"))
+                repository.uspString = "uspString_$iteration"
+                delay(1.milliseconds)
+                repository.clear()
+            }
+        }
+        jobs.awaitAll()
+
+        assertIsEmpty(storage.keys)
+    }
+
+    @Test
+    fun multiKeyWritesAreAtomic() = runTest {
+        val repository = Repository(MapSettings())
+
+        val jobs = (0 until 100).map { id ->
+            async(Dispatchers.Default) {
+                repository.tcData = mapOf(
+                    "IABTCF_key1" to JsonPrimitive("value1_$id"),
+                    "IABTCF_key2" to JsonPrimitive("value2_$id"),
+                    "IABTCF_key3" to JsonPrimitive("value3_$id")
+                )
+            }
+        }
+        jobs.awaitAll()
+
+        val tcData = repository.tcData
+        assertEquals(3, tcData.size)
+
+        val ids = tcData.values.map { it.content.substringAfterLast("_") }.toSet()
+        assertEquals(1, ids.size, "All values must be from same write operation")
+    }
+
+    @Test
+    fun stateSerializationIsThreadSafe() = runTest {
+        val repository = Repository(MapSettings())
+
+        val writeJobs = (0 until 100).map { id ->
+            async(Dispatchers.Default) {
+                repository.state = State(accountId = id, propertyId = id * 2)
+            }
+        }
+
+        val readJobs = (0 until 100).map {
+            async(Dispatchers.Default) {
+                repository.state?.let { state ->
+                    assertNotNull(state.accountId)
+                    assertNotNull(state.propertyId)
+                    assertEquals(state.accountId * 2, state.propertyId)
+                }
+            }
+        }
+
+        (writeJobs + readJobs).awaitAll()
+
+        val finalState = repository.state
+        assertNotNull(finalState)
+        assertEquals(finalState.accountId * 2, finalState.propertyId)
+    }
+}