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.
This commit is contained in:
1 parent
3003f75333
commit
cac58a98db
6 files changed
+78
-27
No files matched your search
@@ -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<BackupAlarm>()
|
||||
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<BackupTimer>()
|
||||
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<BackupWorldClock>()
|
||||
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),
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -329,6 +329,11 @@
|
||||
<string name="settings_backup_export_failed">Couldn\'t save the backup</string>
|
||||
<string name="settings_backup_import_succeeded">Backup restored</string>
|
||||
<string name="settings_backup_import_failed">Couldn\'t restore the backup</string>
|
||||
<string name="settings_backup_import_failed_not_backup">Couldn\'t restore: this isn\'t a Clockula backup</string>
|
||||
<string name="settings_backup_import_failed_newer">Couldn\'t restore: this backup is from a newer Clockula. Update the app first</string>
|
||||
<string name="settings_backup_import_failed_invalid">Couldn\'t restore: the backup has invalid data in it</string>
|
||||
<string name="settings_backup_import_failed_too_large">Couldn\'t restore: the backup is too large</string>
|
||||
<string name="settings_backup_import_failed_unreadable">Couldn\'t restore: the file couldn\'t be opened</string>
|
||||
<string name="settings_backup_import_confirm_title">Replace everything?</string>
|
||||
<string name="settings_backup_import_confirm_message">This replaces all your current alarms, timers, world clocks and settings with what\'s in the backup file. This can\'t be undone.</string>
|
||||
<string name="settings_backup_import_confirm_action">Replace</string>
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user