From 8d847ae4ea2dc26158919259789b2c871d79b1a6 Mon Sep 17 00:00:00 2001 From: Wally Hackenslacker Date: Wed, 18 Mar 2026 10:04:22 -0400 Subject: [PATCH] fix: persist settings api key through api key store --- .../app/auth/SettingsApplyCoordinator.kt | 59 ++++++++--- .../app/auth/SettingsApplyCoordinatorTest.kt | 100 ++++++++++++++++-- 2 files changed, 138 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinator.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinator.kt index cc81ece..5c31c78 100644 --- 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, ) } } diff --git a/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinatorTest.kt b/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinatorTest.kt index d7fbe85..bdfeca9 100644 --- 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() + + val getCalls = mutableListOf() + val saveCalls = mutableListOf() + val invalidateCalls = mutableListOf() + + override suspend fun saveApiKey(baseUrl: String, apiKey: String): Result { + saveCalls += SaveCall(baseUrl = baseUrl, apiKey = apiKey) + keysByBaseUrl[baseUrl] = apiKey + return Result.success(Unit) + } + + override suspend fun getApiKey(baseUrl: String): Result { + getCalls += baseUrl + return Result.success(keysByBaseUrl[baseUrl]) + } + + override suspend fun invalidateApiKey(baseUrl: String): Result { + 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, + ) }