From 24fccc4d7e6dc7be33ff1e99c4c72185e261bab4 Mon Sep 17 00:00:00 2001 From: Wally Hackenslacker Date: Wed, 18 Mar 2026 09:57:17 -0400 Subject: [PATCH] feat: implement settings apply coordinator with rollback --- .../app/auth/SessionPreferences.kt | 62 ++++ .../kanbn4droid/app/auth/SessionStore.kt | 31 ++ .../app/auth/SettingsApplyCoordinator.kt | 111 +++++++ .../app/auth/SettingsApplyCoordinatorTest.kt | 277 ++++++++++++++++++ 4 files changed, 481 insertions(+) create mode 100644 app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinator.kt create mode 100644 app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinatorTest.kt diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionPreferences.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionPreferences.kt index a590c2d..60be748 100644 --- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionPreferences.kt +++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionPreferences.kt @@ -7,12 +7,28 @@ class SessionPreferences(context: Context) : SessionStore { private val preferences: SharedPreferences = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) + override fun getThemeMode(): String = preferences.getString(KEY_THEME_MODE, THEME_MODE_SYSTEM) ?: THEME_MODE_SYSTEM + + override fun saveThemeMode(themeMode: String) { + preferences.edit().putString(KEY_THEME_MODE, themeMode).apply() + } + override fun getBaseUrl(): String? = preferences.getString(KEY_BASE_URL, null) override fun saveBaseUrl(url: String) { preferences.edit().putString(KEY_BASE_URL, url).apply() } + override fun getApiKey(): String? = preferences.getString(KEY_API_KEY, null) + + override fun saveApiKey(apiKey: String) { + preferences.edit().putString(KEY_API_KEY, apiKey).apply() + } + + override fun clearApiKey() { + preferences.edit().remove(KEY_API_KEY).apply() + } + override fun getWorkspaceId(): String? = preferences.getString(KEY_WORKSPACE_ID, null) override fun saveWorkspaceId(workspaceId: String) { @@ -27,9 +43,55 @@ class SessionPreferences(context: Context) : SessionStore { preferences.edit().remove(KEY_WORKSPACE_ID).apply() } + override fun initializeDraftsFromCommitted() { + val editor = preferences.edit() + editor.putString(KEY_DRAFT_THEME_MODE, getThemeMode()) + editor.putString(KEY_DRAFT_BASE_URL, getBaseUrl()) + editor.putString(KEY_DRAFT_API_KEY, getApiKey()) + editor.apply() + } + + override fun getDraftThemeMode(): String { + return preferences.getString(KEY_DRAFT_THEME_MODE, getThemeMode()) ?: getThemeMode() + } + + override fun saveDraftThemeMode(themeMode: String) { + preferences.edit().putString(KEY_DRAFT_THEME_MODE, themeMode).apply() + } + + override fun getDraftBaseUrl(): String? = preferences.getString(KEY_DRAFT_BASE_URL, getBaseUrl()) + + override fun saveDraftBaseUrl(url: String) { + preferences.edit().putString(KEY_DRAFT_BASE_URL, url).apply() + } + + override fun getDraftApiKey(): String? = preferences.getString(KEY_DRAFT_API_KEY, getApiKey()) + + override fun saveDraftApiKey(apiKey: String) { + preferences.edit().putString(KEY_DRAFT_API_KEY, apiKey).apply() + } + + override fun resetDraftsFromCommitted() { + initializeDraftsFromCommitted() + } + + override fun syncDraftsToCommitted() { + val editor = preferences.edit() + editor.putString(KEY_THEME_MODE, getDraftThemeMode()) + editor.putString(KEY_BASE_URL, getDraftBaseUrl()) + editor.putString(KEY_API_KEY, getDraftApiKey()) + editor.apply() + } + private companion object { private const val PREFS_NAME = "kanbn_session" + private const val THEME_MODE_SYSTEM = "system" + private const val KEY_THEME_MODE = "theme_mode" private const val KEY_BASE_URL = "base_url" + private const val KEY_API_KEY = "api_key" private const val KEY_WORKSPACE_ID = "workspace_id" + private const val KEY_DRAFT_THEME_MODE = "draft_theme_mode" + private const val KEY_DRAFT_BASE_URL = "draft_base_url" + private const val KEY_DRAFT_API_KEY = "draft_api_key" } } diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionStore.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionStore.kt index 19316ba..4e55e8a 100644 --- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionStore.kt +++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SessionStore.kt @@ -1,10 +1,41 @@ package space.hackenslacker.kanbn4droid.app.auth interface SessionStore { + fun getThemeMode(): String = "system" + fun saveThemeMode(themeMode: String) {} fun getBaseUrl(): String? fun saveBaseUrl(url: String) + fun getApiKey(): String? = null + fun saveApiKey(apiKey: String) {} + fun clearApiKey() {} fun getWorkspaceId(): String? fun saveWorkspaceId(workspaceId: String) fun clearBaseUrl() fun clearWorkspaceId() + + fun initializeDraftsFromCommitted() {} + fun getDraftThemeMode(): String = getThemeMode() + fun saveDraftThemeMode(themeMode: String) { + saveThemeMode(themeMode) + } + + fun getDraftBaseUrl(): String? = getBaseUrl() + fun saveDraftBaseUrl(url: String) { + saveBaseUrl(url) + } + + fun getDraftApiKey(): String? = getApiKey() + fun saveDraftApiKey(apiKey: String) { + saveApiKey(apiKey) + } + + fun resetDraftsFromCommitted() { + initializeDraftsFromCommitted() + } + + fun syncDraftsToCommitted() { + saveThemeMode(getDraftThemeMode()) + getDraftBaseUrl()?.let(::saveBaseUrl) ?: clearBaseUrl() + getDraftApiKey()?.let(::saveApiKey) ?: clearApiKey() + } } 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 new file mode 100644 index 0000000..cc81ece --- /dev/null +++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinator.kt @@ -0,0 +1,111 @@ +package space.hackenslacker.kanbn4droid.app.auth + +sealed interface SettingsApplyResult { + data object NoChanges : SettingsApplyResult + data class SuccessNoCredentialChange(val themeChanged: Boolean) : SettingsApplyResult + data object SuccessCredentialChange : SettingsApplyResult + data class ValidationError(val field: String, val message: String) : SettingsApplyResult + data class AuthError(val message: String) : SettingsApplyResult + data class NetworkError(val message: String) : SettingsApplyResult +} + +class SettingsApplyCoordinator( + private val sessionStore: SessionStore, + private val apiClient: KanbnApiClient, +) { + suspend fun apply(): SettingsApplyResult { + val snapshot = LastKnownGoodSnapshot.capture(sessionStore) + + val draftTheme = sessionStore.getDraftThemeMode().trim().ifBlank { "system" } + val draftBaseUrlRaw = sessionStore.getDraftBaseUrl().orEmpty() + val draftApiKey = sessionStore.getDraftApiKey().orEmpty().trim() + + val normalizedDraftBaseUrl = when (val normalized = UrlNormalizer.normalize(draftBaseUrlRaw)) { + is UrlValidationResult.Valid -> normalized.normalizedUrl + is UrlValidationResult.Invalid -> { + sessionStore.resetDraftsFromCommitted() + return SettingsApplyResult.ValidationError( + field = FIELD_BASE_URL, + message = normalized.message, + ) + } + } + + if (draftApiKey.isBlank()) { + sessionStore.resetDraftsFromCommitted() + return SettingsApplyResult.ValidationError( + field = FIELD_API_KEY, + message = "API key is required", + ) + } + + val themeChanged = draftTheme != snapshot.committedThemeMode + val credentialChanged = normalizedDraftBaseUrl != snapshot.committedBaseUrl || + draftApiKey != snapshot.committedApiKey + if (!themeChanged && !credentialChanged) { + return SettingsApplyResult.NoChanges + } + + if (credentialChanged) { + when (val auth = apiClient.healthCheck(normalizedDraftBaseUrl, draftApiKey)) { + is AuthResult.Success -> Unit + is AuthResult.Failure -> { + snapshot.restore(sessionStore) + return when (auth.reason) { + AuthFailureReason.Authentication -> SettingsApplyResult.AuthError(auth.message) + AuthFailureReason.Connectivity, + AuthFailureReason.Server, + AuthFailureReason.Unexpected, + -> SettingsApplyResult.NetworkError(auth.message) + } + } + } + } + + try { + sessionStore.saveDraftThemeMode(draftTheme) + sessionStore.saveDraftBaseUrl(normalizedDraftBaseUrl) + sessionStore.saveDraftApiKey(draftApiKey) + sessionStore.syncDraftsToCommitted() + } catch (_: Throwable) { + snapshot.restore(sessionStore) + return SettingsApplyResult.NetworkError("Failed to apply settings changes.") + } + + return if (credentialChanged) { + SettingsApplyResult.SuccessCredentialChange + } else { + SettingsApplyResult.SuccessNoCredentialChange(themeChanged = themeChanged) + } + } + + private data class LastKnownGoodSnapshot( + val committedThemeMode: String, + val committedBaseUrl: String, + val committedApiKey: String, + ) { + fun restore(sessionStore: SessionStore) { + sessionStore.saveThemeMode(committedThemeMode) + sessionStore.saveBaseUrl(committedBaseUrl) + sessionStore.saveApiKey(committedApiKey) + sessionStore.saveDraftThemeMode(committedThemeMode) + sessionStore.saveDraftBaseUrl(committedBaseUrl) + sessionStore.saveDraftApiKey(committedApiKey) + } + + companion object { + fun capture(sessionStore: SessionStore): LastKnownGoodSnapshot { + return LastKnownGoodSnapshot( + committedThemeMode = sessionStore.getThemeMode(), + committedBaseUrl = sessionStore.getBaseUrl().orEmpty(), + committedApiKey = sessionStore.getApiKey().orEmpty(), + ) + } + } + } + + private companion object { + private const val FIELD_BASE_URL = "baseUrl" + private const val FIELD_API_KEY = "apiKey" + } +} 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 new file mode 100644 index 0000000..d7fbe85 --- /dev/null +++ b/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/SettingsApplyCoordinatorTest.kt @@ -0,0 +1,277 @@ +package space.hackenslacker.kanbn4droid.app.auth + +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class SettingsApplyCoordinatorTest { + + @Test + fun applyNoChangesReturnsNoChangesWithoutAuthCall() = runTest { + val sessionStore = FakeSessionStore() + val apiClient = FakeKanbnApiClient() + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertEquals(SettingsApplyResult.NoChanges, result) + assertEquals(0, apiClient.healthCheckCalls.size) + } + + @Test + fun applyCredentialChangeSuccessPersistsAndReturnsSuccessCredentialChange() = runTest { + val sessionStore = FakeSessionStore().apply { + saveDraftBaseUrl("https://next.kan.bn") + saveDraftApiKey("next-key") + } + val apiClient = FakeKanbnApiClient( + healthCheckResult = AuthResult.Success, + ) + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertEquals(SettingsApplyResult.SuccessCredentialChange, result) + assertEquals(1, apiClient.healthCheckCalls.size) + assertEquals("https://next.kan.bn/", apiClient.healthCheckCalls.single().baseUrl) + assertEquals("next-key", apiClient.healthCheckCalls.single().apiKey) + assertEquals("https://next.kan.bn/", sessionStore.getBaseUrl()) + assertEquals("next-key", sessionStore.getApiKey()) + assertEquals(sessionStore.getBaseUrl(), sessionStore.getDraftBaseUrl()) + assertEquals(sessionStore.getApiKey(), sessionStore.getDraftApiKey()) + } + + @Test + fun applyAuthFailureRollsBackCommittedAndDraftValues() = runTest { + val sessionStore = FakeSessionStore().apply { + saveDraftBaseUrl("https://broken.kan.bn") + saveDraftApiKey("bad-key") + } + val apiClient = FakeKanbnApiClient( + healthCheckResult = AuthResult.Failure( + message = "Authentication failed. Check your API key.", + reason = AuthFailureReason.Authentication, + ), + ) + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertEquals( + SettingsApplyResult.AuthError("Authentication failed. Check your API key."), + result, + ) + assertEquals("https://kan.bn/", sessionStore.getBaseUrl()) + assertEquals("existing-key", sessionStore.getApiKey()) + assertEquals("https://kan.bn/", sessionStore.getDraftBaseUrl()) + assertEquals("existing-key", sessionStore.getDraftApiKey()) + } + + @Test + fun applyPersistFailureAfterAuthRollsBack() = runTest { + val sessionStore = FakeSessionStore( + failSyncToCommitted = true, + ).apply { + saveDraftBaseUrl("https://next.kan.bn") + saveDraftApiKey("next-key") + } + val apiClient = FakeKanbnApiClient( + healthCheckResult = AuthResult.Success, + ) + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertTrue(result is SettingsApplyResult.NetworkError) + result as SettingsApplyResult.NetworkError + assertTrue(result.message.contains("apply", ignoreCase = true)) + assertEquals("https://kan.bn/", sessionStore.getBaseUrl()) + assertEquals("existing-key", sessionStore.getApiKey()) + assertEquals("https://kan.bn/", sessionStore.getDraftBaseUrl()) + assertEquals("existing-key", sessionStore.getDraftApiKey()) + } + + @Test + fun applyInvalidUrlReturnsValidationError() = runTest { + val sessionStore = FakeSessionStore().apply { + saveDraftBaseUrl("ftp://kan.bn") + } + val apiClient = FakeKanbnApiClient() + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertEquals( + SettingsApplyResult.ValidationError( + field = "baseUrl", + message = "Base URL must start with http:// or https://", + ), + result, + ) + assertEquals(0, apiClient.healthCheckCalls.size) + assertEquals("https://kan.bn/", sessionStore.getDraftBaseUrl()) + assertEquals("existing-key", sessionStore.getDraftApiKey()) + } + + @Test + fun applyBlankApiKeyReturnsValidationErrorForApiKey() = runTest { + val sessionStore = FakeSessionStore().apply { + saveDraftApiKey(" ") + } + val apiClient = FakeKanbnApiClient() + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertEquals( + SettingsApplyResult.ValidationError( + field = "apiKey", + message = "API key is required", + ), + result, + ) + assertEquals(0, apiClient.healthCheckCalls.size) + assertEquals("existing-key", sessionStore.getDraftApiKey()) + } + + @Test + fun applyThemeAndCredentialsFailureRollsBackThemeDraft() = runTest { + val sessionStore = FakeSessionStore().apply { + saveDraftThemeMode("dark") + saveDraftBaseUrl("https://next.kan.bn") + saveDraftApiKey("next-key") + } + val apiClient = FakeKanbnApiClient( + healthCheckResult = AuthResult.Failure( + message = "Cannot reach server. Check your connection and URL.", + reason = AuthFailureReason.Connectivity, + ), + ) + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertEquals( + SettingsApplyResult.NetworkError("Cannot reach server. Check your connection and URL."), + result, + ) + assertEquals("system", sessionStore.getThemeMode()) + assertEquals("system", sessionStore.getDraftThemeMode()) + } + + @Test + fun applyThemeChangeSuccessSignalsThemeMode() = runTest { + val sessionStore = FakeSessionStore().apply { + saveDraftThemeMode("dark") + } + val apiClient = FakeKanbnApiClient() + val coordinator = SettingsApplyCoordinator(sessionStore, apiClient) + + val result = coordinator.apply() + + assertEquals(SettingsApplyResult.SuccessNoCredentialChange(themeChanged = true), result) + assertEquals(0, apiClient.healthCheckCalls.size) + assertEquals("dark", sessionStore.getThemeMode()) + assertEquals("dark", sessionStore.getDraftThemeMode()) + } + + private class FakeSessionStore( + private val failSyncToCommitted: Boolean = false, + ) : SessionStore { + private var committedThemeMode: String = "system" + private var committedBaseUrl: String? = "https://kan.bn/" + private var committedApiKey: String? = "existing-key" + + private var draftThemeMode: String = committedThemeMode + private var draftBaseUrl: String? = committedBaseUrl + private var draftApiKey: String? = committedApiKey + + override fun getThemeMode(): String = committedThemeMode + + override fun saveThemeMode(themeMode: String) { + committedThemeMode = themeMode + } + + override fun getBaseUrl(): String? = committedBaseUrl + + override fun saveBaseUrl(url: String) { + committedBaseUrl = url + } + + override fun getApiKey(): String? = committedApiKey + + override fun saveApiKey(apiKey: String) { + committedApiKey = apiKey + } + + override fun clearApiKey() { + committedApiKey = null + } + + override fun getWorkspaceId(): String? = null + + override fun saveWorkspaceId(workspaceId: String) { + } + + override fun clearBaseUrl() { + committedBaseUrl = null + } + + override fun clearWorkspaceId() { + } + + override fun initializeDraftsFromCommitted() { + draftThemeMode = committedThemeMode + draftBaseUrl = committedBaseUrl + draftApiKey = committedApiKey + } + + override fun getDraftThemeMode(): String = draftThemeMode + + override fun saveDraftThemeMode(themeMode: String) { + draftThemeMode = themeMode + } + + override fun getDraftBaseUrl(): String? = draftBaseUrl + + override fun saveDraftBaseUrl(url: String) { + draftBaseUrl = url + } + + override fun getDraftApiKey(): String? = draftApiKey + + override fun saveDraftApiKey(apiKey: String) { + draftApiKey = apiKey + } + + override fun resetDraftsFromCommitted() { + initializeDraftsFromCommitted() + } + + override fun syncDraftsToCommitted() { + committedThemeMode = draftThemeMode + committedBaseUrl = draftBaseUrl + committedApiKey = draftApiKey + if (failSyncToCommitted) { + throw IllegalStateException("sync failed") + } + } + } + + private class FakeKanbnApiClient( + private val healthCheckResult: AuthResult = AuthResult.Success, + ) : KanbnApiClient { + val healthCheckCalls = mutableListOf() + + override suspend fun healthCheck(baseUrl: String, apiKey: String): AuthResult { + healthCheckCalls += HealthCheckCall(baseUrl = baseUrl, apiKey = apiKey) + return healthCheckResult + } + } + + private data class HealthCheckCall( + val baseUrl: String, + val apiKey: String, + ) +}