Make DatabaseLoginsStorage async

There hasn't been agreement on a grand vision for async Rust in
app-services (#6549).  I think part of the reason is that ADR is
discussing too many things.  This is my attempt to take a small change
from that ADR and see if we agree.

This change takes the async wrapping code that's currently in
android-components and moves it to our app-services wrapper.  This
improves the API boundary IMO. The SYNC sync and the credentials
management teams are responsible for threading issues.  For example,
we're the one's thinking through how to avoid blocking too many threads
when we're waiting on the primary password dialog.  Since we're
responsible for threading issues, we should own the code

On a practical level: I have some ideas on how to improve our threading
strategy and it's simpler to think about those changes if all the code
is in one repo.

Here's the corresponding android-components change:
bendk/firefox@d038ba6

If we agree on this, then I think we can do something similar for iOS.

What do others think?  Is this a good idea?  Do we need/want an ADR for this?

Author: bendk

components/logins/android/build.gradle

@@ -19,10 +19,12 @@ dependencies {
     api project(':init_rust_components')
     api project(':sync15')
 
+    implementation libs.kotlin.coroutines
     implementation libs.mozilla.glean
     implementation project(':init_rust_components')
 
     testImplementation libs.mozilla.glean.forUnitTests
+    testImplementation libs.kotlin.coroutines.test
     testImplementation libs.androidx.test.core
     testImplementation libs.androidx.work.testing
     testImplementation project(":syncmanager")

components/logins/android/src/main/java/mozilla/appservices/logins/DatabaseLoginsStorage.kt

@@ -3,25 +3,23 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package mozilla.appservices.logins
-
-/**
- * Import some private Glean types, so that we can use them in type declarations.
- *
- * I do not like importing these private classes, but I do like the nice generic
- * code they allow me to write! By agreement with the Glean team, we must not
- * instantiate anything from these classes, and it's on us to fix any bustage
- * on version updates.
- */
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 import mozilla.telemetry.glean.private.CounterMetricType
 import mozilla.telemetry.glean.private.LabeledMetricType
+import kotlin.coroutines.CoroutineContext
 import org.mozilla.appservices.logins.GleanMetrics.LoginsStore as LoginsStoreMetrics
 
 /**
  * An artifact of the uniffi conversion - a thin-ish wrapper around a
  * LoginStore.
  */
 
-class DatabaseLoginsStorage(dbPath: String, keyManager: KeyManager) : AutoCloseable {
+class DatabaseLoginsStorage(
+    dbPath: String,
+    keyManager: KeyManager,
+    private val coroutineContext: CoroutineContext = Dispatchers.IO,
+) : AutoCloseable {
     private var store: LoginStore
 
     init {
@@ -30,94 +28,94 @@ class DatabaseLoginsStorage(dbPath: String, keyManager: KeyManager) : AutoClosea
     }
 
     @Throws(LoginsApiException::class)
-    fun reset() {
-        this.store.reset()
+    suspend fun reset(): Unit = withContext(coroutineContext) {
+        store.reset()
     }
 
     @Throws(LoginsApiException::class)
-    fun wipeLocal() {
-        this.store.wipeLocal()
+    suspend fun wipeLocal(): Unit = withContext(coroutineContext) {
+        store.wipeLocal()
     }
 
     @Throws(LoginsApiException::class)
-    fun delete(id: String): Boolean {
-        return writeQueryCounters.measure {
+    suspend fun delete(id: String): Boolean = withContext(coroutineContext) {
+        writeQueryCounters.measure {
             store.delete(id)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun get(id: String): Login? {
-        return readQueryCounters.measure {
+    suspend fun get(id: String): Login? = withContext(coroutineContext) {
+        readQueryCounters.measure {
             store.get(id)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun touch(id: String) {
+    suspend fun touch(id: String): Unit = withContext(coroutineContext) {
         writeQueryCounters.measure {
             store.touch(id)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun isEmpty(): Boolean {
-        return readQueryCounters.measure {
+    suspend fun isEmpty(): Boolean = withContext(coroutineContext) {
+        readQueryCounters.measure {
             store.isEmpty()
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun list(): List<Login> {
-        return readQueryCounters.measure {
+    suspend fun list(): List<Login> = withContext(coroutineContext) {
+        readQueryCounters.measure {
             store.list()
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun hasLoginsByBaseDomain(baseDomain: String): Boolean {
-        return readQueryCounters.measure {
+    suspend fun hasLoginsByBaseDomain(baseDomain: String): Boolean = withContext(coroutineContext) {
+        readQueryCounters.measure {
             store.hasLoginsByBaseDomain(baseDomain)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun getByBaseDomain(baseDomain: String): List<Login> {
-        return readQueryCounters.measure {
+    suspend fun getByBaseDomain(baseDomain: String): List<Login> = withContext(coroutineContext) {
+        readQueryCounters.measure {
             store.getByBaseDomain(baseDomain)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun findLoginToUpdate(look: LoginEntry): Login? {
-        return readQueryCounters.measure {
+    suspend fun findLoginToUpdate(look: LoginEntry): Login? = withContext(coroutineContext) {
+        readQueryCounters.measure {
             store.findLoginToUpdate(look)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun add(entry: LoginEntry): Login {
-        return writeQueryCounters.measure {
+    suspend fun add(entry: LoginEntry): Login = withContext(coroutineContext) {
+        writeQueryCounters.measure {
             store.add(entry)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun update(id: String, entry: LoginEntry): Login {
-        return writeQueryCounters.measure {
+    suspend fun update(id: String, entry: LoginEntry): Login = withContext(coroutineContext) {
+        writeQueryCounters.measure {
             store.update(id, entry)
         }
     }
 
     @Throws(LoginsApiException::class)
-    fun addOrUpdate(entry: LoginEntry): Login {
-        return writeQueryCounters.measure {
+    suspend fun addOrUpdate(entry: LoginEntry): Login = withContext(coroutineContext) {
+        writeQueryCounters.measure {
             store.addOrUpdate(entry)
         }
     }
 
     fun registerWithSyncManager() {
-        return store.registerWithSyncManager()
+        store.registerWithSyncManager()
     }
 
     @Synchronized

components/logins/android/src/test/java/mozilla/appservices/logins/DatabaseLoginsStorageTest.kt

@@ -4,14 +4,14 @@
 package mozilla.appservices.logins
 
 import androidx.test.core.app.ApplicationProvider
+import kotlinx.coroutines.test.runTest
 import mozilla.appservices.RustComponentsInitializer
 import mozilla.appservices.syncmanager.SyncManager
 import mozilla.telemetry.glean.testing.GleanTestRule
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
-import org.junit.Assert.assertThrows
 import org.junit.Assert.assertTrue
 import org.junit.Assert.fail
 import org.junit.Rule
@@ -41,7 +41,7 @@ class DatabaseLoginsStorageTest {
         return DatabaseLoginsStorage(dbPath = dbPath.absolutePath, keyManager = keyManager)
     }
 
-    protected fun getTestStore(): DatabaseLoginsStorage {
+    protected suspend fun getTestStore(): DatabaseLoginsStorage {
         val store = createTestStore()
 
         store.add(
@@ -77,7 +77,7 @@ class DatabaseLoginsStorageTest {
     }
 
     @Test
-    fun testMetricsGathering() {
+    fun testMetricsGathering() = runTest {
         val store = createTestStore()
 
         assertNull(LoginsStoreMetrics.writeQueryCount.testGetValue())
@@ -132,7 +132,7 @@ class DatabaseLoginsStorageTest {
     }
 
     @Test
-    fun testTouch() {
+    fun testTouch() = runTest {
         val store = getTestStore()
         val login = store.list()[0]
         // Wait 100ms so that touch is certain to change timeLastUsed.
@@ -145,13 +145,17 @@ class DatabaseLoginsStorageTest {
         assertEquals(login.timesUsed + 1, updatedLogin!!.timesUsed)
         assert(updatedLogin.timeLastUsed > login.timeLastUsed)
 
-        assertThrows(LoginsApiException.NoSuchRecord::class.java) { store.touch("abcdabcdabcd") }
+        try {
+            store.touch("abcdabcdabcd")
+        } catch (e: LoginsApiException.NoSuchRecord) {
+            // Expected error
+        }
 
         finishAndClose(store)
     }
 
     @Test
-    fun testDelete() {
+    fun testDelete() = runTest {
         val store = getTestStore()
         val login = store.list()[0]
 
@@ -165,7 +169,7 @@ class DatabaseLoginsStorageTest {
     }
 
     @Test
-    fun testWipeLocal() {
+    fun testWipeLocal() = runTest {
         val test = getTestStore()
         val logins = test.list()
         assertEquals(2, logins.size)

gradle/libs.versions.toml

@@ -14,6 +14,7 @@ protobuf-plugin = "0.9.5"
 # Kotlin
 kotlin-compiler = "2.1.21"
 kotlin-coroutines = "1.10.2"
+kotlin-coroutines-test = "1.10.2"
 
 # Mozilla
 android-components = "139.0.20250417022706"
@@ -58,6 +59,7 @@ protobuf-compiler = { group = "com.google.protobuf", name = "protoc", version.re
 # Kotlin
 kotlin-gradle-plugin = { group = "org.jetbrains.kotlin", name = "kotlin-gradle-plugin", version.ref = "kotlin-compiler" }
 kotlin-coroutines = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-android", version.ref = "kotlin-coroutines" }
+kotlin-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "kotlin-coroutines-test" }
 
 # Mozilla
 mozilla-concept-fetch = { group = "org.mozilla.components", name = "concept-fetch", version.ref = "android-components" }