From 7902b7871e2be28ae53464a0662257cd9ff58f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Herculano?= <228650+andresilveirah@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:40:03 +0200 Subject: [PATCH 1/2] fix test reliance on hardcoded uuid --- .../mobile_core/network/SourcepointClientTest.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/network/SourcepointClientTest.kt b/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/network/SourcepointClientTest.kt index 385e864..2e403f3 100644 --- a/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/network/SourcepointClientTest.kt +++ b/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/network/SourcepointClientTest.kt @@ -89,7 +89,7 @@ class SourcepointClientTest { assertNotNull(response.preferences?.additionsChangeDate) assertNotNull(response.globalcmp) - response.globalcmp?.apply { + response.globalcmp.apply { assertNotEmpty(vendorListId) assertNotEmpty(applicableSections) assertTrue(applies) @@ -244,7 +244,7 @@ class SourcepointClientTest { response.campaigns.forEach { campaign -> assertNotNull(campaign.url, "Empty url for ${campaign.type}") assertNotNull(campaign.message, "Empty message for ${campaign.type}") - assertNotEmpty(campaign.message?.messageJson, "Empty message_json for ${campaign.type}") + assertNotEmpty(campaign.message.messageJson, "Empty message_json for ${campaign.type}") assertNotNull(campaign.messageMetaData, "Empty messageMetaData for ${campaign.type}") assertCampaignConsentsFromMessages(campaign) } @@ -294,7 +294,10 @@ class SourcepointClientTest { val customVendorId = "5ff4d000a228633ac048be41" val categoryId1 = "608bad95d08d3112188e0e36" val categoryId2 = "608bad95d08d3112188e0e2f" - val consentUUID = "uuid_36" // this uuid needs to exist in the backend, i.e. consent services + val consentUUID = api.postChoiceGDPRAction( + actionType = SPActionType.RejectAll, + request = GDPRChoiceRequest(sendPVData = false, propertyId = propertyId, sampleRate = 1.0f) + ).uuid val responseCustomConsent = api.customConsentGDPR( consentUUID = consentUUID, propertyId = propertyId, @@ -376,7 +379,6 @@ class SourcepointClientTest { val response = api.postChoiceGDPRAction( actionType = SPActionType.SaveAndExit, request = GDPRChoiceRequest( - uuid = "uuid_36", sendPVData = true, propertyId = propertyId, pmSaveAndExitVariables = """{ @@ -421,7 +423,6 @@ class SourcepointClientTest { val response = api.postChoiceCCPAAction( actionType = SPActionType.SaveAndExit, request = CCPAChoiceRequest( - uuid = "uuid_36", pmSaveAndExitVariables = """{"rejectedCategories":["608bae685461ff11a2c2865d"],"rejectedVendors":[],"privacyManagerId":"509688","lan":"EN"}""".encodeToJsonObject()!!, sendPVData = true, @@ -457,7 +458,6 @@ class SourcepointClientTest { val response = api.postChoiceUSNatAction( actionType = SPActionType.SaveAndExit, request = USNatChoiceRequest( - uuid = "uuid_36", vendorListId = "65a01016e17a3c7a831ec515", pmSaveAndExitVariables = """{"categories":["648c9c48e17a3c7a82360c54"],"lan":"EN","privacyManagerId":"943886","shownCategories":["648c9c48e17a3c7a82360c54"],"vendors":[]}""".encodeToJsonObject()!!, From c62017fc090ed257a1a8c295fe1a72705c87edeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Herculano?= <228650+andresilveirah@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:40:29 +0200 Subject: [PATCH 2/2] improve concurrency safety of Repository and Settings --- .../mobile_core/storage/Repository.kt | 48 ++-- .../mobile_core/storage/SettingsExt.kt | 30 ++- .../mobile_core/RepositoryConcurrencyTest.kt | 177 ++++++++++++++ .../mobile_core/SettingsExtTest.kt | 227 ++++++++++++++++++ 4 files changed, 446 insertions(+), 36 deletions(-) create mode 100644 core/src/commonTest/kotlin/com/sourcepoint/mobile_core/RepositoryConcurrencyTest.kt diff --git a/core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/Repository.kt b/core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/Repository.kt index 662216a..a7f46ff 100644 --- a/core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/Repository.kt +++ b/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(getString(SP_STATE_KEY, defaultValue = "")) - } + val json = storage.readString(SP_STATE_KEY, defaultValue = "") + if (json.isBlank()) null else Json.decodeFromString(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) } } diff --git a/core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/SettingsExt.kt b/core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/SettingsExt.kt index 75a7093..6081696 100644 --- a/core/src/commonMain/kotlin/com/sourcepoint/mobile_core/storage/SettingsExt.kt +++ b/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 = withLock { + keys.filter { it.startsWith(prefix) } + .associateWith { getJsonPrimitive(it) } +} + +internal fun Settings.replaceKeysWithPrefix(prefix: String, data: Map) = 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) { diff --git a/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/RepositoryConcurrencyTest.kt b/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/RepositoryConcurrencyTest.kt new file mode 100644 index 0000000..affe342 --- /dev/null +++ b/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) + } +} diff --git a/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/SettingsExtTest.kt b/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/SettingsExtTest.kt index 7241c21..fef8b6f 100644 --- a/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/SettingsExtTest.kt +++ b/core/src/commonTest/kotlin/com/sourcepoint/mobile_core/SettingsExtTest.kt @@ -1,11 +1,27 @@ package com.sourcepoint.mobile_core import com.russhwolf.settings.MapSettings +import com.sourcepoint.mobile_core.asserters.assertContains +import com.sourcepoint.mobile_core.asserters.assertDoesNotContain +import com.sourcepoint.mobile_core.asserters.assertIsEmpty +import com.sourcepoint.mobile_core.asserters.assertTrue +import com.sourcepoint.mobile_core.storage.delete import com.sourcepoint.mobile_core.storage.get +import com.sourcepoint.mobile_core.storage.getKeysWithPrefix +import com.sourcepoint.mobile_core.storage.readString +import com.sourcepoint.mobile_core.storage.readStringOrNull +import com.sourcepoint.mobile_core.storage.removeKeysStartingWith +import com.sourcepoint.mobile_core.storage.replaceKeysWithPrefix import com.sourcepoint.mobile_core.storage.set +import com.sourcepoint.mobile_core.storage.writeString +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.test.runTest import kotlinx.serialization.json.JsonPrimitive import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull class SettingsExtTest { @Test @@ -55,4 +71,215 @@ class SettingsExtTest { assertEquals(JsonPrimitive(1234567890123456789L), storage["foo"]) assertEquals(1234567890123456789L, storage["foo"]) } + + @Test + fun removesKeysWithPrefix() { + val settings = MapSettings( + "PREFIX_key" to "value", + "OTHER_key" to "other" + ) + + settings.removeKeysStartingWith("PREFIX_") + + assertDoesNotContain(settings.keys, "PREFIX_key") + assertContains(settings.keys, "OTHER_key") + } + + @Test + fun getsKeysWithPrefixReturnsCorrectMap() { + val settings = MapSettings( + "PREFIX_key" to "value", + "OTHER_key" to "other" + ) + + val result = settings.getKeysWithPrefix("PREFIX_") + + assertEquals(1, result.size) + assertContains(result.keys, "PREFIX_key") + assertDoesNotContain(result.keys, "OTHER_key") + assertEquals(JsonPrimitive("value"), result["PREFIX_key"]) + } + + @Test + fun replaceKeysWithPrefixRemovesOldAndAddsNew() { + val settings = MapSettings( + "PREFIX_old" to "old_value", + "OTHER_key" to "keep_me" + ) + + settings.replaceKeysWithPrefix( + "PREFIX_", + mapOf("PREFIX_new" to JsonPrimitive("new_value")) + ) + + val prefixKeys = settings.keys.filter { it.startsWith("PREFIX_") } + assertEquals(1, prefixKeys.size) + assertContains(prefixKeys, "PREFIX_new") + assertDoesNotContain(prefixKeys, "PREFIX_old") + assertContains(settings.keys, "OTHER_key") + } + + @Test + fun readWriteStringOperations() { + val settings = MapSettings() + + settings.writeString("key", "value") + assertEquals("value", settings.readString("key")) + assertEquals("value", settings.readStringOrNull("key")) + + settings.writeString("key", null) + assertDoesNotContain(settings.keys, "key") + assertNull(settings.readStringOrNull("key")) + } + + @Test + fun deleteRemovesKey() { + val settings = MapSettings("key" to "value") + + settings.delete("key") + + assertNull(settings.readStringOrNull("key")) + assertDoesNotContain(settings.keys, "key") + } + + @Test + fun concurrentPrefixRemovalIsAtomic() = runTest { + val settings = MapSettings( + "PREFIX_A_key" to "value_a", + "PREFIX_B_key" to "value_b" + ) + + val jobs = (0 until 100).map { i -> + async(Dispatchers.Default) { + val prefix = if (i % 2 == 0) "PREFIX_A_" else "PREFIX_B_" + settings.removeKeysStartingWith(prefix) + settings.replaceKeysWithPrefix( + prefix, + mapOf("${prefix}new" to JsonPrimitive("new_value")) + ) + } + } + jobs.awaitAll() + + val keysA = settings.keys.filter { it.startsWith("PREFIX_A_") } + val keysB = settings.keys.filter { it.startsWith("PREFIX_B_") } + assertTrue(keysA.isEmpty() || keysA.size == 1) + assertTrue(keysB.isEmpty() || keysB.size == 1) + } + + @Test + fun concurrentReplaceKeysWithPrefixIsAtomic() = runTest { + val settings = MapSettings() + + val jobs = (0 until 100).map { id -> + async(Dispatchers.Default) { + settings.replaceKeysWithPrefix( + "PREFIX_", + mapOf( + "PREFIX_key1" to JsonPrimitive("value1_$id"), + "PREFIX_key2" to JsonPrimitive("value2_$id"), + "PREFIX_key3" to JsonPrimitive("value3_$id") + ) + ) + } + } + jobs.awaitAll() + + val prefixKeys = settings.keys.filter { it.startsWith("PREFIX_") } + assertEquals(3, prefixKeys.size) + + val ids = prefixKeys.map { + settings.getKeysWithPrefix("PREFIX_")[it]?.content?.substringAfterLast("_") + }.toSet() + assertEquals(1, ids.size, "All keys must be from same atomic operation") + } + + @Test + fun concurrentGetKeysWithPrefixReturnsConsistentSnapshots() = runTest { + val settings = MapSettings( + "PREFIX_a" to "initial", + "PREFIX_b" to "initial", + "PREFIX_c" to "initial" + ) + + val writeJobs = (0 until 50).map { id -> + async(Dispatchers.Default) { + settings.replaceKeysWithPrefix( + "PREFIX_", + mapOf( + "PREFIX_a" to JsonPrimitive("$id"), + "PREFIX_b" to JsonPrimitive("$id"), + "PREFIX_c" to JsonPrimitive("$id") + ) + ) + } + } + + val readJobs = (0 until 100).map { + async(Dispatchers.Default) { + val data = settings.getKeysWithPrefix("PREFIX_") + if (data.isNotEmpty()) { + val values = data.values.map { it.content }.toSet() + assertEquals(1, values.size, "All values in snapshot must be the same") + } + } + } + + (writeJobs + readJobs).awaitAll() + } + + @Test + fun concurrentReadWriteStringOperations() = runTest { + val settings = MapSettings("counter" to "0") + + val jobs = (0 until 100).map { + async(Dispatchers.Default) { + val current = settings.readString("counter", "0").toInt() + settings.writeString("counter", "${current + 1}") + } + } + jobs.awaitAll() + + val finalValue = settings.readString("counter", "0").toInt() + assertTrue(finalValue > 0) + } + + @Test + fun concurrentDeleteOperations() = runTest { + val settings = MapSettings() + repeat(50) { i -> + settings.putString("key_$i", "value") + } + + val jobs = (0 until 50).map { i -> + async(Dispatchers.Default) { + settings.delete("key_$i") + } + } + jobs.awaitAll() + + assertIsEmpty(settings.keys) + } + + @Test + fun replaceKeysWithPrefixHandlesEmptyData() { + val settings = MapSettings( + "PREFIX_key1" to "value1", + "PREFIX_key2" to "value2" + ) + + settings.replaceKeysWithPrefix("PREFIX_", emptyMap()) + + assertDoesNotContain(settings.keys, "PREFIX_key1") + assertDoesNotContain(settings.keys, "PREFIX_key2") + } + + @Test + fun getKeysWithPrefixReturnsEmptyForNonExistentPrefix() { + val settings = MapSettings("key" to "value") + + val result = settings.getKeysWithPrefix("NONEXISTENT_") + + assertIsEmpty(result) + } }