fix: persist settings api key through api key store

2026-03-18 10:04:22 -04:00
parent 24fccc4d7e
commit 8d847ae4ea
2 changed files with 138 additions and 21 deletions
--- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinator.kt
+++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinator.kt
@@ -12,9 +12,14 @@ sealed interface SettingsApplyResult {
 class SettingsApplyCoordinator(
    private val sessionStore: SessionStore,
    private val apiClient: KanbnApiClient,
+    private val apiKeyStore: ApiKeyStore,
 ) {
    suspend fun apply(): SettingsApplyResult {
-        val snapshot = LastKnownGoodSnapshot.capture(sessionStore)
+        val snapshot = try {
+            LastKnownGoodSnapshot.capture(sessionStore, apiKeyStore)
+        } catch (_: Exception) {
+            return SettingsApplyResult.NetworkError("Failed to apply settings changes.")
+        }

        val draftTheme = sessionStore.getDraftThemeMode().trim().ifBlank { "system" }
        val draftBaseUrlRaw = sessionStore.getDraftBaseUrl().orEmpty()
@@ -40,8 +45,9 @@ class SettingsApplyCoordinator(
        }

        val themeChanged = draftTheme != snapshot.committedThemeMode
+        val committedApiKey = snapshot.committedApiStoreKey.orEmpty()
        val credentialChanged = normalizedDraftBaseUrl != snapshot.committedBaseUrl ||
-            draftApiKey != snapshot.committedApiKey
+            draftApiKey != committedApiKey
        if (!themeChanged && !credentialChanged) {
            return SettingsApplyResult.NoChanges
        }
@@ -50,7 +56,7 @@ class SettingsApplyCoordinator(
            when (val auth = apiClient.healthCheck(normalizedDraftBaseUrl, draftApiKey)) {
                is AuthResult.Success -> Unit
                is AuthResult.Failure -> {
-                    snapshot.restore(sessionStore)
+                    rollback(snapshot, normalizedDraftBaseUrl)
                    return when (auth.reason) {
                        AuthFailureReason.Authentication -> SettingsApplyResult.AuthError(auth.message)
                        AuthFailureReason.Connectivity,
@@ -66,9 +72,17 @@ class SettingsApplyCoordinator(
            sessionStore.saveDraftThemeMode(draftTheme)
            sessionStore.saveDraftBaseUrl(normalizedDraftBaseUrl)
            sessionStore.saveDraftApiKey(draftApiKey)
+
+            if (credentialChanged) {
+                apiKeyStore.saveApiKey(normalizedDraftBaseUrl, draftApiKey).getOrThrow()
+                if (normalizedDraftBaseUrl != snapshot.committedBaseUrl) {
+                    apiKeyStore.invalidateApiKey(snapshot.committedBaseUrl).getOrThrow()
+                }
+            }
+
            sessionStore.syncDraftsToCommitted()
-        } catch (_: Throwable) {
-            snapshot.restore(sessionStore)
+        } catch (_: Exception) {
+            rollback(snapshot, normalizedDraftBaseUrl)
            return SettingsApplyResult.NetworkError("Failed to apply settings changes.")
        }

@@ -79,26 +93,47 @@ class SettingsApplyCoordinator(
        }
    }

+    private suspend fun rollback(snapshot: LastKnownGoodSnapshot, candidateBaseUrl: String) {
+        snapshot.restoreSession(sessionStore)
+        snapshot.restoreApiKeys(apiKeyStore, candidateBaseUrl)
+    }
+
    private data class LastKnownGoodSnapshot(
        val committedThemeMode: String,
        val committedBaseUrl: String,
-        val committedApiKey: String,
+        val committedSessionApiKey: String,
+        val committedApiStoreKey: String?,
    ) {
-        fun restore(sessionStore: SessionStore) {
+        fun restoreSession(sessionStore: SessionStore) {
            sessionStore.saveThemeMode(committedThemeMode)
            sessionStore.saveBaseUrl(committedBaseUrl)
-            sessionStore.saveApiKey(committedApiKey)
+            sessionStore.saveApiKey(committedSessionApiKey)
            sessionStore.saveDraftThemeMode(committedThemeMode)
            sessionStore.saveDraftBaseUrl(committedBaseUrl)
-            sessionStore.saveDraftApiKey(committedApiKey)
+            sessionStore.saveDraftApiKey(committedSessionApiKey)
+        }
+
+        suspend fun restoreApiKeys(apiKeyStore: ApiKeyStore, candidateBaseUrl: String) {
+            if (candidateBaseUrl != committedBaseUrl) {
+                apiKeyStore.invalidateApiKey(candidateBaseUrl)
+            }
+
+            if (committedApiStoreKey == null) {
+                apiKeyStore.invalidateApiKey(committedBaseUrl)
+            } else {
+                apiKeyStore.saveApiKey(committedBaseUrl, committedApiStoreKey)
+            }
        }

        companion object {
-            fun capture(sessionStore: SessionStore): LastKnownGoodSnapshot {
+            suspend fun capture(sessionStore: SessionStore, apiKeyStore: ApiKeyStore): LastKnownGoodSnapshot {
+                val committedBaseUrl = sessionStore.getBaseUrl().orEmpty()
+                val committedApiStoreKey = apiKeyStore.getApiKey(committedBaseUrl).getOrThrow()
                return LastKnownGoodSnapshot(
                    committedThemeMode = sessionStore.getThemeMode(),
-                    committedBaseUrl = sessionStore.getBaseUrl().orEmpty(),
-                    committedApiKey = sessionStore.getApiKey().orEmpty(),
+                    committedBaseUrl = committedBaseUrl,
+                    committedSessionApiKey = sessionStore.getApiKey().orEmpty(),
+                    committedApiStoreKey = committedApiStoreKey,
                )
            }
        }
--- a/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinatorTest.kt
+++ b/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinatorTest.kt
@@ -9,14 +9,23 @@ class SettingsApplyCoordinatorTest {

    @Test
    fun applyNoChangesReturnsNoChangesWithoutAuthCall() = runTest {
-        val sessionStore = FakeSessionStore()
+        val sessionStore = FakeSessionStore().apply {
+            saveDraftApiKey("truth-key")
+            saveApiKey("session-key")
+        }
        val apiClient = FakeKanbnApiClient()
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "truth-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

        assertEquals(SettingsApplyResult.NoChanges, result)
        assertEquals(0, apiClient.healthCheckCalls.size)
+        assertEquals(1, apiKeyStore.getCalls.size)
+        assertTrue(apiKeyStore.saveCalls.isEmpty())
+        assertTrue(apiKeyStore.invalidateCalls.isEmpty())
    }

    @Test
@@ -28,7 +37,10 @@ class SettingsApplyCoordinatorTest {
        val apiClient = FakeKanbnApiClient(
            healthCheckResult = AuthResult.Success,
        )
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "existing-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

@@ -40,6 +52,10 @@ class SettingsApplyCoordinatorTest {
        assertEquals("next-key", sessionStore.getApiKey())
        assertEquals(sessionStore.getBaseUrl(), sessionStore.getDraftBaseUrl())
        assertEquals(sessionStore.getApiKey(), sessionStore.getDraftApiKey())
+        assertEquals("next-key", apiKeyStore.peek("https://next.kan.bn/"))
+        assertEquals(null, apiKeyStore.peek("https://kan.bn/"))
+        assertEquals(1, apiKeyStore.saveCalls.size)
+        assertEquals(1, apiKeyStore.invalidateCalls.size)
    }

    @Test
@@ -54,7 +70,10 @@ class SettingsApplyCoordinatorTest {
                reason = AuthFailureReason.Authentication,
            ),
        )
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "existing-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

@@ -66,6 +85,8 @@ class SettingsApplyCoordinatorTest {
        assertEquals("existing-key", sessionStore.getApiKey())
        assertEquals("https://kan.bn/", sessionStore.getDraftBaseUrl())
        assertEquals("existing-key", sessionStore.getDraftApiKey())
+        assertEquals("existing-key", apiKeyStore.peek("https://kan.bn/"))
+        assertEquals(null, apiKeyStore.peek("https://broken.kan.bn/"))
    }

    @Test
@@ -79,7 +100,10 @@ class SettingsApplyCoordinatorTest {
        val apiClient = FakeKanbnApiClient(
            healthCheckResult = AuthResult.Success,
        )
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "existing-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

@@ -90,6 +114,8 @@ class SettingsApplyCoordinatorTest {
        assertEquals("existing-key", sessionStore.getApiKey())
        assertEquals("https://kan.bn/", sessionStore.getDraftBaseUrl())
        assertEquals("existing-key", sessionStore.getDraftApiKey())
+        assertEquals("existing-key", apiKeyStore.peek("https://kan.bn/"))
+        assertEquals(null, apiKeyStore.peek("https://next.kan.bn/"))
    }

    @Test
@@ -98,7 +124,10 @@ class SettingsApplyCoordinatorTest {
            saveDraftBaseUrl("ftp://kan.bn")
        }
        val apiClient = FakeKanbnApiClient()
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "existing-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

@@ -112,6 +141,8 @@ class SettingsApplyCoordinatorTest {
        assertEquals(0, apiClient.healthCheckCalls.size)
        assertEquals("https://kan.bn/", sessionStore.getDraftBaseUrl())
        assertEquals("existing-key", sessionStore.getDraftApiKey())
+        assertTrue(apiKeyStore.saveCalls.isEmpty())
+        assertTrue(apiKeyStore.invalidateCalls.isEmpty())
    }

    @Test
@@ -120,7 +151,10 @@ class SettingsApplyCoordinatorTest {
            saveDraftApiKey("   ")
        }
        val apiClient = FakeKanbnApiClient()
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "existing-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

@@ -133,6 +167,8 @@ class SettingsApplyCoordinatorTest {
        )
        assertEquals(0, apiClient.healthCheckCalls.size)
        assertEquals("existing-key", sessionStore.getDraftApiKey())
+        assertTrue(apiKeyStore.saveCalls.isEmpty())
+        assertTrue(apiKeyStore.invalidateCalls.isEmpty())
    }

    @Test
@@ -148,7 +184,10 @@ class SettingsApplyCoordinatorTest {
                reason = AuthFailureReason.Connectivity,
            ),
        )
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "existing-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

@@ -158,6 +197,8 @@ class SettingsApplyCoordinatorTest {
        )
        assertEquals("system", sessionStore.getThemeMode())
        assertEquals("system", sessionStore.getDraftThemeMode())
+        assertEquals("existing-key", apiKeyStore.peek("https://kan.bn/"))
+        assertEquals(null, apiKeyStore.peek("https://next.kan.bn/"))
    }

    @Test
@@ -166,7 +207,10 @@ class SettingsApplyCoordinatorTest {
            saveDraftThemeMode("dark")
        }
        val apiClient = FakeKanbnApiClient()
-        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient)
+        val apiKeyStore = FakeApiKeyStore().apply {
+            setKey("https://kan.bn/", "existing-key")
+        }
+        val coordinator = SettingsApplyCoordinator(sessionStore, apiClient, apiKeyStore)

        val result = coordinator.apply()

@@ -174,6 +218,8 @@ class SettingsApplyCoordinatorTest {
        assertEquals(0, apiClient.healthCheckCalls.size)
        assertEquals("dark", sessionStore.getThemeMode())
        assertEquals("dark", sessionStore.getDraftThemeMode())
+        assertTrue(apiKeyStore.saveCalls.isEmpty())
+        assertTrue(apiKeyStore.invalidateCalls.isEmpty())
    }

    private class FakeSessionStore(
@@ -274,4 +320,40 @@ class SettingsApplyCoordinatorTest {
        val baseUrl: String,
        val apiKey: String,
    )
+
+    private class FakeApiKeyStore : ApiKeyStore {
+        private val keysByBaseUrl = mutableMapOf<String, String>()
+
+        val getCalls = mutableListOf<String>()
+        val saveCalls = mutableListOf<SaveCall>()
+        val invalidateCalls = mutableListOf<String>()
+
+        override suspend fun saveApiKey(baseUrl: String, apiKey: String): Result<Unit> {
+            saveCalls += SaveCall(baseUrl = baseUrl, apiKey = apiKey)
+            keysByBaseUrl[baseUrl] = apiKey
+            return Result.success(Unit)
+        }
+
+        override suspend fun getApiKey(baseUrl: String): Result<String?> {
+            getCalls += baseUrl
+            return Result.success(keysByBaseUrl[baseUrl])
+        }
+
+        override suspend fun invalidateApiKey(baseUrl: String): Result<Unit> {
+            invalidateCalls += baseUrl
+            keysByBaseUrl.remove(baseUrl)
+            return Result.success(Unit)
+        }
+
+        fun setKey(baseUrl: String, apiKey: String) {
+            keysByBaseUrl[baseUrl] = apiKey
+        }
+
+        fun peek(baseUrl: String): String? = keysByBaseUrl[baseUrl]
+    }
+
+    private data class SaveCall(
+        val baseUrl: String,
+        val apiKey: String,
+    )
 }