diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt index 5945f12..30c4d1f 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt @@ -94,20 +94,20 @@ class ClockulaApp : Application() { } /** - * Ahead of the engines' pass, because the boot gate writes the store and - * "nothing written yet" is half of how a fresh install is told apart. Never + * Ahead of the engines' pass, because the boot gate writes the boot id, and + * its absence is half of how a fresh install is told apart. Never * in the way of that pass: a failure here leaves the setting unsettled, * which reads as the old behaviour (#22). */ private suspend fun settleTimerVibrate() { try { - settings.settleTimerVibrate(freshInstall = isFreshInstall()) + settings.settleTimerVibrate(freshInstall = ::isFreshInstall) } catch (cancelled: CancellationException) { throw cancelled } catch (_: Exception) { // Unsettled reads as the old behaviour. The next launch tries again, - // and by then the store is no longer empty, so even a fresh install - // settles on the alarm default: vibrating, the safe side to err on. + // and by then the boot id is there, so even a fresh install settles + // on the alarm default: vibrating, the safe side to err on. } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt index bed61f8..c1f81da 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt @@ -127,22 +127,29 @@ class SettingsPrefs @Inject constructor(private val store: PrefStore) { * Gives the timers' vibration a value of its own, once (#22). Until then a * timer vibrated exactly when the alarm default said so, and an install * from before keeps that: the alarm default is copied, so changing it - * later no longer drags the timers along. Only a fresh install — a - * [freshInstall] package *and* a store nothing has written yet — starts - * with it off. Either signal alone mistakes something for fresh (an Auto - * Backup restore; a store replaced after corruption), and that mistake - * would silently stop someone's timers buzzing. + * later no longer drags the timers along. Only a fresh install starts with + * it off: a [freshInstall] package *and* a store without a boot id, which + * the engines write on every launch after this runs. Either signal alone + * mistakes something for fresh (an Auto Backup restore brings the old + * device's boot id; a store replaced after corruption is not a fresh + * package), and that mistake would silently stop someone's timers buzzing. + * The boot id rather than "empty" because the UI may write the store first. * * Best-effort by design: until it lands, and if it never does, the read * side's fallback keeps the old behaviour, so a failed write costs nothing. + * Once settled it is one read; [freshInstall] is asked only when needed. */ - suspend fun settleTimerVibrate(freshInstall: Boolean) { + suspend fun settleTimerVibrate(freshInstall: () -> Boolean) { + if (store.get(ClockPrefs.timerVibrate) != null) return // One transaction, so nothing written meanwhile can make the snapshot // it decides on stale. Raw keys for the reads: the kit reads a Pref // only from a store, not from a transaction's preferences. store.edit { prefs -> - if (stringPreferencesKey(ClockPrefs.timerVibrate.name) in prefs) return@edit - prefs[ClockPrefs.timerVibrate] = if (prefs.asMap().isEmpty() && freshInstall) { + val stored = prefs[stringPreferencesKey(ClockPrefs.timerVibrate.name)] + // Settled meanwhile; a garbled value falls through and is replaced. + if (stored?.toBooleanStrictOrNull() != null) return@edit + val neverRan = stringPreferencesKey(SystemPrefs.lastBootId.name) !in prefs + prefs[ClockPrefs.timerVibrate] = if (neverRan && freshInstall()) { false } else { prefs[booleanPreferencesKey(ClockPrefs.vibrate.name)] ?: ClockPrefs.vibrate.default diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefsTest.kt index 8401734..fc833b0 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefsTest.kt @@ -38,7 +38,7 @@ class ClockPrefsTest { fun `a fresh install reads the documented defaults`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) // As ClockulaApp's startup pass does on a fresh install. - prefs.settleTimerVibrate(freshInstall = true) + prefs.settleTimerVibrate(freshInstall = { true }) assertThat(prefs.defaults.first()).isEqualTo(ClockDefaults()) } @@ -77,7 +77,7 @@ class ClockPrefsTest { @Test fun `the defaults do not re-emit when the same value is written again`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) - prefs.settleTimerVibrate(freshInstall = true) + prefs.settleTimerVibrate(freshInstall = { true }) prefs.defaults.test { assertThat(awaitItem()).isEqualTo(ClockDefaults()) diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt index bdffca6..c44f501 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt @@ -185,7 +185,7 @@ class SettingsPrefsTest { fun `a fresh install settles timer vibration off`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) - prefs.settleTimerVibrate(freshInstall = true) + prefs.settleTimerVibrate(freshInstall = { true }) assertThat(prefs.currentDefaults().timerVibrate).isFalse() assertThat(prefs.currentDefaults().vibrate).isTrue() @@ -196,7 +196,7 @@ class SettingsPrefsTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) prefs.setVibrate(false) - prefs.settleTimerVibrate(freshInstall = false) + prefs.settleTimerVibrate(freshInstall = { false }) // Settled, so the two now move apart. prefs.setVibrate(true) @@ -210,7 +210,7 @@ class SettingsPrefsTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) prefs.setSnoozeMinutes(7) - prefs.settleTimerVibrate(freshInstall = false) + prefs.settleTimerVibrate(freshInstall = { false }) prefs.setVibrate(false) assertThat(prefs.currentDefaults().timerVibrate).isTrue() @@ -221,28 +221,49 @@ class SettingsPrefsTest { // A store replaced after corruption, or never written: empty, but not new. val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) - prefs.settleTimerVibrate(freshInstall = false) + prefs.settleTimerVibrate(freshInstall = { false }) assertThat(prefs.currentDefaults().timerVibrate).isTrue() } @Test fun `a new package with a restored store is not taken for fresh`(@TempDir tempDir: Path) = runTest { - // Auto Backup restored the old device's store before the first launch. - val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) - prefs.setSnoozeMinutes(7) + // Auto Backup restored the old device's store, boot id and all. + val store = PrefStore(newDataStore(tempDir)) + val prefs = SettingsPrefs(store) + store.set(SystemPrefs.lastBootId, "old-device-boot") - prefs.settleTimerVibrate(freshInstall = true) + prefs.settleTimerVibrate(freshInstall = { true }) assertThat(prefs.currentDefaults().timerVibrate).isTrue() } + @Test + fun `a fresh install is still fresh when the UI wrote the store first`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.markNotificationPermissionAsked() + + prefs.settleTimerVibrate(freshInstall = { true }) + + assertThat(prefs.currentDefaults().timerVibrate).isFalse() + } + + @Test + fun `once settled, the package is not asked again`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.settleTimerVibrate(freshInstall = { true }) + + prefs.settleTimerVibrate(freshInstall = { error("asked again") }) + + assertThat(prefs.currentDefaults().timerVibrate).isFalse() + } + @Test fun `settling never overwrites a choice`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) prefs.setTimerVibrate(true) - prefs.settleTimerVibrate(freshInstall = true) + prefs.settleTimerVibrate(freshInstall = { true }) assertThat(prefs.currentDefaults().timerVibrate).isTrue() } @@ -258,6 +279,21 @@ class SettingsPrefsTest { assertThat(prefs.currentDefaults().timerVibrate).isTrue() } + @Test + fun `a garbled timer vibration is settled like an absent one`(@TempDir tempDir: Path) = runTest { + val dataStore = newDataStore(tempDir) + val prefs = SettingsPrefs(PrefStore(dataStore)) + prefs.setVibrate(false) + dataStore.updateData { stored -> + stored.toMutablePreferences().apply { this[stringPreferencesKey("default_timer_vibrate")] = "maybe" } + } + + prefs.settleTimerVibrate(freshInstall = { false }) + prefs.setVibrate(true) + + assertThat(prefs.currentDefaults().timerVibrate).isFalse() + } + @Test fun `the timer ramp is clamped like the alarm ramp`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f87ee1d..7f5db0a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1913,9 +1913,11 @@ the setting existed had its timers vibrate exactly when the alarm default did, and keeps that. The key is nullable: absent reads as the alarm default, and `SettingsPrefs.settleTimerVibrate` writes a value of its own once, at the head of `ClockulaApp`'s startup pass (before the boot gate writes the store). Only a -package never updated since install *and* an empty store count as fresh and -get `false`; anything else (an update, an Auto Backup restore, a store replaced -after corruption) gets the alarm default copied. It is deliberately not a +package never updated since install *and* a store without `last_boot_id` +(the engines write it on every launch, after the settle) count as fresh and +get `false`; anything else (an update, an Auto Backup restore, which brings the +old boot id, a store replaced after corruption) gets the alarm default copied. +The boot id rather than an empty store, because the UI may write first. It is deliberately not a DataStore migration: a migration's write runs inside the store's initialisation, and a failed one would fail every read, the alarm path's included. A failed settle only leaves the fallback in place.