From 01bb8ecf12e1be55b21d2ed491f18533966cbe81 Mon Sep 17 00:00:00 2001 From: Krzysztof Borowy Date: Tue, 13 Dec 2022 21:54:16 +0100 Subject: [PATCH 1/3] fix: use lock for storage auth --- data/persistence/build.gradle.kts | 1 + .../localstorage/DodoAuthStorage.kt | 2 +- .../localstorage/DodoAuthStorageImpl.kt | 22 +++++++++++-------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/data/persistence/build.gradle.kts b/data/persistence/build.gradle.kts index 7f93a650..07202d9f 100644 --- a/data/persistence/build.gradle.kts +++ b/data/persistence/build.gradle.kts @@ -52,6 +52,7 @@ kotlin { val commonMain by getting { dependencies { implementation(libs.multiplatform.settings) + implementation(libs.kotlinx.coroutines.core) implementation(libs.kotlinx.serialization.json) implementation(libs.io.insert.koin.core) } diff --git a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt index 7c253e0e..37a5c2c9 100644 --- a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt +++ b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt @@ -29,5 +29,5 @@ interface DodoAuthStorage { /** * Get the Access token for @param server */ - fun getAccessToken(server: String): String? + suspend fun getAccessToken(server: String): String? } diff --git a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt index ae081b8b..b536eb29 100644 --- a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt +++ b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt @@ -12,6 +12,8 @@ package social.androiddev.common.persistence.localstorage import com.russhwolf.settings.Settings import com.russhwolf.settings.get import com.russhwolf.settings.set +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.serialization.Serializable import kotlinx.serialization.builtins.ListSerializer import kotlinx.serialization.json.Json @@ -20,7 +22,7 @@ internal class DodoAuthStorageImpl( private val settings: Settings, private val json: Json ) : DodoAuthStorage { - + private val lock = Mutex() override var currentDomain: String? get() = settings[KEY_DOMAIN_CACHE] set(value) { @@ -31,29 +33,31 @@ internal class DodoAuthStorageImpl( * The user can set up multiple accounts on their device. So we * key the AccessToken by the unique server/domain */ - private var diskCache: Map + private var diskCache: LinkedHashMap get() { return settings .getStringOrNull(KEY_ACCESS_TOKENS_CACHE) ?.let { str -> json.decodeFromString(ListSerializer(AccessToken.serializer()), str) - .associateBy { it.server } + .associateByTo(linkedMapOf()) { it.server } } - ?: mutableMapOf() + ?: linkedMapOf() } set(value) { val list = value.map { it.value } settings[KEY_ACCESS_TOKENS_CACHE] = json.encodeToString(ListSerializer(AccessToken.serializer()), list) } + private val memCache: LinkedHashMap by lazy { diskCache } - private val memCache: MutableMap by lazy { diskCache.toMutableMap() } - - override fun getAccessToken(server: String): String? = memCache[server]?.token + override suspend fun getAccessToken(server: String): String? = + lock.withLock { memCache[server]?.token } override suspend fun saveAccessToken(server: String, token: String) { - memCache[server] = AccessToken(token = token, server = server) - diskCache = memCache + lock.withLock { + memCache[server] = AccessToken(token = token, server = server) + diskCache = memCache + } } private companion object { From 84746436bf441d4b75988d3a69084a01ad4d45b9 Mon Sep 17 00:00:00 2001 From: Krzysztof Borowy Date: Fri, 16 Dec 2022 13:44:00 +0100 Subject: [PATCH 2/3] move to constructor, inline default --- .../common/persistence/localstorage/DodoAuthStorageImpl.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt index b536eb29..50ac7db1 100644 --- a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt +++ b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt @@ -20,9 +20,9 @@ import kotlinx.serialization.json.Json internal class DodoAuthStorageImpl( private val settings: Settings, - private val json: Json + private val json: Json, + private val lock: Mutex = Mutex() ) : DodoAuthStorage { - private val lock = Mutex() override var currentDomain: String? get() = settings[KEY_DOMAIN_CACHE] set(value) { From dcc0509237f241905c9bc9b274523137c4450c5f Mon Sep 17 00:00:00 2001 From: Krzysztof Borowy Date: Tue, 20 Dec 2022 13:27:27 +0100 Subject: [PATCH 3/3] use reentrant lock --- build.gradle.kts | 1 + data/persistence/build.gradle.kts | 2 +- .../common/persistence/localstorage/DodoAuthStorage.kt | 2 +- .../persistence/localstorage/DodoAuthStorageImpl.kt | 9 +++++---- gradle/libs.versions.toml | 10 +++++++--- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index a8d4fbd7..a00272b0 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -16,6 +16,7 @@ buildscript { classpath(libs.org.jetbrains.kotlin.serialization.plugin) classpath(libs.com.squareup.sqldelight.gradle.plugin) classpath(libs.com.google.osdetector.gradle.plugin) + classpath(libs.org.jetbrains.kotlinx.atomicfu.plugin) } } diff --git a/data/persistence/build.gradle.kts b/data/persistence/build.gradle.kts index 07202d9f..8913f70c 100644 --- a/data/persistence/build.gradle.kts +++ b/data/persistence/build.gradle.kts @@ -4,6 +4,7 @@ plugins { id("com.squareup.sqldelight") id("social.androiddev.code-quality") kotlin("plugin.serialization") + id("kotlinx-atomicfu") } @@ -52,7 +53,6 @@ kotlin { val commonMain by getting { dependencies { implementation(libs.multiplatform.settings) - implementation(libs.kotlinx.coroutines.core) implementation(libs.kotlinx.serialization.json) implementation(libs.io.insert.koin.core) } diff --git a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt index 37a5c2c9..7c253e0e 100644 --- a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt +++ b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorage.kt @@ -29,5 +29,5 @@ interface DodoAuthStorage { /** * Get the Access token for @param server */ - suspend fun getAccessToken(server: String): String? + fun getAccessToken(server: String): String? } diff --git a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt index 50ac7db1..bcda58f6 100644 --- a/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt +++ b/data/persistence/src/commonMain/kotlin/social/androiddev/common/persistence/localstorage/DodoAuthStorageImpl.kt @@ -12,8 +12,9 @@ package social.androiddev.common.persistence.localstorage import com.russhwolf.settings.Settings import com.russhwolf.settings.get import com.russhwolf.settings.set -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock +import kotlinx.atomicfu.locks.ReentrantLock +import kotlinx.atomicfu.locks.reentrantLock +import kotlinx.atomicfu.locks.withLock import kotlinx.serialization.Serializable import kotlinx.serialization.builtins.ListSerializer import kotlinx.serialization.json.Json @@ -21,7 +22,7 @@ import kotlinx.serialization.json.Json internal class DodoAuthStorageImpl( private val settings: Settings, private val json: Json, - private val lock: Mutex = Mutex() + private val lock: ReentrantLock = reentrantLock() ) : DodoAuthStorage { override var currentDomain: String? get() = settings[KEY_DOMAIN_CACHE] @@ -50,7 +51,7 @@ internal class DodoAuthStorageImpl( } private val memCache: LinkedHashMap by lazy { diskCache } - override suspend fun getAccessToken(server: String): String? = + override fun getAccessToken(server: String): String? = lock.withLock { memCache[server]?.token } override suspend fun saveAccessToken(server: String, token: String) { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index d01aeb71..71286a3c 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -2,7 +2,10 @@ # plugins org-jetbrains-compose = "1.2.1" org-jetbrains-kotlin = "1.7.20" -org-jetbrains-kotlinx-coroutines = "1.6.4" + +# Need to match with kotlin version used: https://github.com/Kotlin/kotlinx-atomicfu/blob/0.18.5/gradle.properties +org-jetbrains-kotlinx-atomicfu = "0.18.5" +org-jetbrains-kotlinx-serialization = "1.4.1" com-android-tools-build = "7.2.2" com-squareup-sqldelight = "1.5.3" com-diffplug-spotless = "6.11.0" @@ -10,7 +13,7 @@ com-google-osdetector = "1.7.1" # libraries io-ktor = "2.1.3" com-google-truth = "1.1.3" -org-jetbrains-kotlinx = "1.4.1" +org-jetbrains-kotlinx-coroutines = "1.6.4" androidx-core = "1.3.1" androidx-appcompat = "1.5.1" androidx-activity = "1.6.1" @@ -28,6 +31,7 @@ org-jetbrains-compose-gradle-plugin = { module = "org.jetbrains.compose:compose- org-jetbrains-kotlin-gradle-plugin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "org-jetbrains-kotlin" } com-android-tools-build-gradle = { module = "com.android.tools.build:gradle", version.ref = "com-android-tools-build" } org-jetbrains-kotlin-serialization-plugin = { module = "org.jetbrains.kotlin:kotlin-serialization", version.ref = "org-jetbrains-kotlin" } +org-jetbrains-kotlinx-atomicfu-plugin = { module = "org.jetbrains.kotlinx:atomicfu-gradle-plugin", version.ref = "org-jetbrains-kotlinx-atomicfu" } com-squareup-sqldelight-gradle-plugin = { module = "com.squareup.sqldelight:gradle-plugin", version.ref = "com-squareup-sqldelight" } com-diffplug-spotless-gradle-plugin = { module = "com.diffplug.spotless:spotless-plugin-gradle", version.ref = "com-diffplug-spotless" } com-google-osdetector-gradle-plugin = { module = "com.google.gradle:osdetector-gradle-plugin", version.ref = "com-google-osdetector" } @@ -49,7 +53,7 @@ io-insert-koin-android = { module = "io.insert-koin:koin-android", version.ref = kotlinx-coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "org-jetbrains-kotlinx-coroutines" } kotlinx-coroutines-javafx = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-javafx", version.ref = "org-jetbrains-kotlinx-coroutines" } org-jetbrains-kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "org-jetbrains-kotlinx-coroutines" } -org-jetbrains-kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "org-jetbrains-kotlinx" } +org-jetbrains-kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "org-jetbrains-kotlinx-serialization" } org-jetbrains-kotlin-test-junit = { module = "org.jetbrains.kotlin:kotlin-test-junit", version.ref = "org-jetbrains-kotlin" } org-jetbrains-kotlin-test-common = { module = "org.jetbrains.kotlin:kotlin-test-common", version.ref = "org-jetbrains-kotlin" }