From cac58a98dbf9f042deadbb78bbcfc1e424666b02 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 17:39:38 +0200 Subject: [PATCH] fix: say why a restore failed in words that can be translated The snackbar appended the codec's or the exception's own English text, class names and all. Failures now carry a category (not a backup, from a newer version, invalid data, too large, unreadable) that the screen maps to a string resource; the detail stays in the result for tests. --- .../clockula/data/backup/BackupCodec.kt | 56 ++++++++++++++----- .../clockula/ui/settings/BackupScreen.kt | 16 +++++- .../clockula/ui/settings/BackupUiState.kt | 12 +++- .../clockula/ui/settings/BackupViewModel.kt | 11 ++-- app/src/main/res/values/strings.xml | 5 ++ .../ui/settings/BackupViewModelTest.kt | 5 +- 6 files changed, 78 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupCodec.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupCodec.kt index b1b2d60..c6daf96 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupCodec.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupCodec.kt @@ -19,10 +19,21 @@ import kotlinx.serialization.json.Json import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Instant -/** The result of [BackupCodec.decode]: either a validated [BackupContent], or why not. */ +/** + * Why a restore failed, in words a user can act on. The codec's own four, plus + * the two the caller meets around it: the file would not open, or the import + * itself failed after the file was valid. + */ +enum class BackupProblem { NOT_A_BACKUP, NEWER_VERSION, INVALID_DATA, TOO_LARGE, UNREADABLE, NOT_APPLIED } + +/** + * The result of [BackupCodec.decode]: either a validated [BackupContent], or + * why not — [problem] for the user, [reason] the specific detail, in English, + * for tests and logs. + */ sealed interface BackupResult { data class Success(val content: BackupContent) : BackupResult - data class Failure(val reason: String) : BackupResult + data class Failure(val problem: BackupProblem, val reason: String) : BackupResult } /** @@ -50,13 +61,15 @@ object BackupCodec { json.encodeToString(BackupDocument.serializer(), content.toDocument()) fun decode(text: String): BackupResult { - if (text.length > MAX_DOCUMENT_CHARS) return BackupResult.Failure("backup exceeds the 8 MiB limit") + if (text.length > MAX_DOCUMENT_CHARS) { + return BackupResult.Failure(BackupProblem.TOO_LARGE, "backup exceeds the 8 MiB limit") + } val document = try { json.decodeFromString(BackupDocument.serializer(), text) } catch (e: SerializationException) { - return BackupResult.Failure("not a valid backup file: ${e.message}") + return BackupResult.Failure(BackupProblem.NOT_A_BACKUP, "not a valid backup file: ${e.message}") } catch (e: IllegalArgumentException) { - return BackupResult.Failure("not a valid backup file: ${e.message}") + return BackupResult.Failure(BackupProblem.NOT_A_BACKUP, "not a valid backup file: ${e.message}") } return document.toContent() } @@ -120,21 +133,30 @@ object BackupCodec { // --- decode + validate --- private fun BackupDocument.toContent(): BackupResult { - if (format != BACKUP_FORMAT) return BackupResult.Failure("unknown format: '$format'") - if (version != BACKUP_VERSION) return BackupResult.Failure("unsupported backup version: $version") + if (format != BACKUP_FORMAT) { + return BackupResult.Failure(BackupProblem.NOT_A_BACKUP, "unknown format: '$format'") + } + if (version != BACKUP_VERSION) { + val problem = if (version > BACKUP_VERSION) BackupProblem.NEWER_VERSION else BackupProblem.INVALID_DATA + return BackupResult.Failure(problem, "unsupported backup version: $version") + } if (alarms.size > MAX_ALARMS || timers.size > MAX_TIMERS || worldClocks.size > MAX_WORLD_CLOCKS) { - return BackupResult.Failure("backup contains too many records") + return BackupResult.Failure(BackupProblem.TOO_LARGE, "backup contains too many records") } val alarms = mutableListOf() for ((index, dto) in this.alarms.withIndex()) { if (dto.hour !in 0..23 || dto.minute !in 0..59) { return BackupResult.Failure( + BackupProblem.INVALID_DATA, "alarms[$index]: hour/minute out of range (${dto.hour}:${dto.minute})", ) } if (dto.label.length > MAX_LABEL_CHARS || (dto.ringtoneUri?.length ?: 0) > MAX_URI_CHARS) { - return BackupResult.Failure("alarms[$index]: label or ringtone URI is too long") + return BackupResult.Failure( + BackupProblem.TOO_LARGE, + "alarms[$index]: label or ringtone URI is too long", + ) } alarms += BackupAlarm( time = TimeOfDay(dto.hour, dto.minute), @@ -157,10 +179,13 @@ object BackupCodec { val timers = mutableListOf() for ((index, dto) in this.timers.withIndex()) { if (dto.durationMillis < 0) { - return BackupResult.Failure("timers[$index]: negative duration") + return BackupResult.Failure(BackupProblem.INVALID_DATA, "timers[$index]: negative duration") } if (dto.label.length > MAX_LABEL_CHARS || (dto.ringtoneUri?.length ?: 0) > MAX_URI_CHARS) { - return BackupResult.Failure("timers[$index]: label or ringtone URI is too long") + return BackupResult.Failure( + BackupProblem.TOO_LARGE, + "timers[$index]: label or ringtone URI is too long", + ) } timers += BackupTimer( label = dto.label, @@ -174,10 +199,13 @@ object BackupCodec { val worldClocks = mutableListOf() for ((index, dto) in this.worldClocks.withIndex()) { if (!Zones.isValid(dto.zoneId)) { - return BackupResult.Failure("worldClocks[$index]: not an IANA zone id: '${dto.zoneId}'") + return BackupResult.Failure( + BackupProblem.INVALID_DATA, + "worldClocks[$index]: not an IANA zone id: '${dto.zoneId}'", + ) } if ((dto.label?.length ?: 0) > MAX_LABEL_CHARS) { - return BackupResult.Failure("worldClocks[$index]: label is too long") + return BackupResult.Failure(BackupProblem.TOO_LARGE, "worldClocks[$index]: label is too long") } worldClocks += BackupWorldClock( zoneId = dto.zoneId, @@ -191,7 +219,7 @@ object BackupCodec { (settingsDto.timerRingtoneUri?.length ?: 0) > MAX_URI_CHARS || (settingsDto.timerPresets?.size ?: 0) > 1_000 ) { - return BackupResult.Failure("settings contain oversized values") + return BackupResult.Failure(BackupProblem.TOO_LARGE, "settings contain oversized values") } val settingsContent = BackupSettings( themeMode = settingsDto.themeMode.toEnum(ThemeMode.SYSTEM), diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupScreen.kt index 0a1d029..3b67a02 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupScreen.kt @@ -32,6 +32,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.clockula.R +import de.jeanlucmakiola.clockula.data.backup.BackupProblem import de.jeanlucmakiola.floret.components.CollapsingScaffold import de.jeanlucmakiola.floret.components.GroupedListInset import de.jeanlucmakiola.floret.components.GroupedRow @@ -68,15 +69,15 @@ fun BackupScreen( val exportSucceeded = stringResource(R.string.settings_backup_export_succeeded) val exportFailed = stringResource(R.string.settings_backup_export_failed) val importSucceeded = stringResource(R.string.settings_backup_import_succeeded) - val importFailed = stringResource(R.string.settings_backup_import_failed) + val importFailed = BackupProblem.entries.associateWith { stringResource(it.messageRes()) } LaunchedEffect(viewModel) { viewModel.events.collect { event -> val message = when (event) { BackupEvent.ExportSucceeded -> exportSucceeded - is BackupEvent.ExportFailed -> "$exportFailed: ${event.reason}" + BackupEvent.ExportFailed -> exportFailed BackupEvent.ImportSucceeded -> importSucceeded - is BackupEvent.ImportFailed -> "$importFailed: ${event.reason}" + is BackupEvent.ImportFailed -> importFailed.getValue(event.problem) } scope.launch { snackbarHost.showSnackbar(message) } } @@ -140,3 +141,12 @@ fun BackupScreen( ) } } + +private fun BackupProblem.messageRes(): Int = when (this) { + BackupProblem.NOT_A_BACKUP -> R.string.settings_backup_import_failed_not_backup + BackupProblem.NEWER_VERSION -> R.string.settings_backup_import_failed_newer + BackupProblem.INVALID_DATA -> R.string.settings_backup_import_failed_invalid + BackupProblem.TOO_LARGE -> R.string.settings_backup_import_failed_too_large + BackupProblem.UNREADABLE -> R.string.settings_backup_import_failed_unreadable + BackupProblem.NOT_APPLIED -> R.string.settings_backup_import_failed +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupUiState.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupUiState.kt index ac23d5b..1984af4 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupUiState.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupUiState.kt @@ -1,9 +1,15 @@ package de.jeanlucmakiola.clockula.ui.settings -/** A one-off result the screen shows as a snackbar and then forgets. */ +import de.jeanlucmakiola.clockula.data.backup.BackupProblem + +/** + * A one-off result the screen shows as a snackbar and then forgets. A failure + * carries a category the screen can put into words, never an exception's own + * English message. + */ internal sealed interface BackupEvent { data object ExportSucceeded : BackupEvent - data class ExportFailed(val reason: String) : BackupEvent + data object ExportFailed : BackupEvent data object ImportSucceeded : BackupEvent - data class ImportFailed(val reason: String) : BackupEvent + data class ImportFailed(val problem: BackupProblem) : BackupEvent } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModel.kt index 4d3b555..e4ed324 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModel.kt @@ -6,6 +6,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.clockula.backup.BackupEngine import de.jeanlucmakiola.clockula.backup.BackupFiles import de.jeanlucmakiola.clockula.data.backup.BackupCodec +import de.jeanlucmakiola.clockula.data.backup.BackupProblem import de.jeanlucmakiola.clockula.data.backup.BackupResult import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.CancellationException @@ -47,14 +48,14 @@ class BackupViewModel @Inject internal constructor( onSuccess = { BackupEvent.ExportSucceeded }, onFailure = { if (it is CancellationException) throw it - BackupEvent.ExportFailed(it.message ?: "unknown error") + BackupEvent.ExportFailed }, ) _events.trySend(event) } catch (cancelled: CancellationException) { throw cancelled } catch (failure: Exception) { - _events.trySend(BackupEvent.ExportFailed(failure.message ?: "unknown error")) + _events.trySend(BackupEvent.ExportFailed) } finally { _busy.value = false } @@ -73,13 +74,13 @@ class BackupViewModel @Inject internal constructor( } else { val failure = result.exceptionOrNull() if (failure is CancellationException) throw failure - BackupEvent.ImportFailed(failure?.message ?: "unknown error") + BackupEvent.ImportFailed(BackupProblem.UNREADABLE) } _events.trySend(event) } catch (cancelled: CancellationException) { throw cancelled } catch (failure: Exception) { - _events.trySend(BackupEvent.ImportFailed(failure.message ?: "unknown error")) + _events.trySend(BackupEvent.ImportFailed(BackupProblem.NOT_APPLIED)) } finally { _busy.value = false } @@ -91,6 +92,6 @@ class BackupViewModel @Inject internal constructor( engine.import(decoded.content) BackupEvent.ImportSucceeded } - is BackupResult.Failure -> BackupEvent.ImportFailed(decoded.reason) + is BackupResult.Failure -> BackupEvent.ImportFailed(decoded.problem) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f11321f..ad0a66c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -329,6 +329,11 @@ Couldn\'t save the backup Backup restored Couldn\'t restore the backup + Couldn\'t restore: this isn\'t a Clockula backup + Couldn\'t restore: this backup is from a newer Clockula. Update the app first + Couldn\'t restore: the backup has invalid data in it + Couldn\'t restore: the backup is too large + Couldn\'t restore: the file couldn\'t be opened Replace everything? This replaces all your current alarms, timers, world clocks and settings with what\'s in the backup file. This can\'t be undone. Replace diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModelTest.kt index 0561264..e8333f5 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/BackupViewModelTest.kt @@ -3,6 +3,7 @@ package de.jeanlucmakiola.clockula.ui.settings import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.backup.BackupFiles import de.jeanlucmakiola.clockula.data.backup.BackupCodec +import de.jeanlucmakiola.clockula.data.backup.BackupProblem import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension import de.jeanlucmakiola.clockula.testing.backupHarness import kotlinx.coroutines.flow.first @@ -31,7 +32,7 @@ class BackupViewModelTest { viewModel.onExportFolderPicked("content://folder") - assertThat(viewModel.events.first()).isEqualTo(BackupEvent.ExportFailed("provider unavailable")) + assertThat(viewModel.events.first()).isEqualTo(BackupEvent.ExportFailed) assertThat(viewModel.busy.value).isFalse() } @@ -49,7 +50,7 @@ class BackupViewModelTest { viewModel.onImportFilePicked("content://backup") - assertThat(viewModel.events.first()).isEqualTo(BackupEvent.ImportFailed("database unavailable")) + assertThat(viewModel.events.first()).isEqualTo(BackupEvent.ImportFailed(BackupProblem.NOT_APPLIED)) assertThat(viewModel.busy.value).isFalse() } }