fix(timers): tell a fresh install by its boot id and repair a garbled timer vibration

An empty store was a racy sign of a fresh install: the UI can write the store (the notification-permission flag) before the startup settle runs, and a fresh install would then vibrate its timers. The boot id is written only by the engines' pass, which runs after the settle in the same coroutine, and an Auto Backup restore brings the old device's.

Once settled, the startup pass costs one read and no package-manager call. A garbled stored value is replaced rather than left following the alarm default.

Refs #22

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
+     * Ahead of the engines' pass, because the boot gate writes the boot id, and
-     * "nothing written yet" is half of how a fresh install is told apart. Never
+     * 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
+            // and by then the boot id is there, so even a fresh install settles
-            // settles on the alarm default: vibrating, the safe side to err on.
+            // on the alarm default: vibrating, the safe side to err on.
         }
     }
 

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
+     * later no longer drags the timers along. Only a fresh install starts with
-     * [freshInstall] package *and* a store nothing has written yet — starts
+     * it off: a [freshInstall] package *and* a store without a boot id, which
-     * with it off. Either signal alone mistakes something for fresh (an Auto
+     * the engines write on every launch after this runs. Either signal alone
-     * Backup restore; a store replaced after corruption), and that mistake
+     * mistakes something for fresh (an Auto Backup restore brings the old
-     * would silently stop someone's timers buzzing.
+     * 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
+            val stored = prefs[stringPreferencesKey(ClockPrefs.timerVibrate.name)]
-            prefs[ClockPrefs.timerVibrate] = if (prefs.asMap().isEmpty() && freshInstall) {
+            // 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

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())

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.
+        // Auto Backup restored the old device's store, boot id and all.
-        val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir)))
+        val store = PrefStore(newDataStore(tempDir))
-        prefs.setSnoozeMinutes(7)
+        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)))

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
+package never updated since install *and* a store without `last_boot_id`
-get `false`; anything else (an update, an Auto Backup restore, a store replaced
+(the engines write it on every launch, after the settle) count as fresh and
-after corruption) gets the alarm default copied. It is deliberately not a
+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.