From 19efb67740fcf301e934417c20a94c940b447272 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Sat, 12 Sep 2026 16:48:21 +0200 Subject: [PATCH] feat(data): timer writes that survive a clock change and a reboot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "+1 min" on a timer that has just rung resumes it with exactly a minute, in one transaction — the user asked for a minute more than zero, not a minute more than an anchor that has gone by. M2 left that branch paused with no test to pin it; this is the gesture a timer app is judged on. Every running row carries both anchors: the monotonic one it actually runs on, and a wall-clock fallback for a reboot. A system clock change re-derives the fallback from the monotonic remainder rather than leaving it stale, because a stale fallback is how a thirty-minute timer rings twenty-nine minutes early after a reboot. Presets are app-wide, sanitised on read as well as on write, and go through the store's update so two edits cannot swallow each other. --- .../clockula/data/prefs/SettingsPrefs.kt | 21 ++ .../clockula/data/prefs/TimerPrefs.kt | 44 ++++ .../clockula/data/timers/TimerRepository.kt | 33 ++- .../data/timers/TimerRepositoryImpl.kt | 75 ++++++- .../data/timers/TimerRingStateStore.kt | 25 +++ .../clockula/data/prefs/TimerPrefsTest.kt | 207 ++++++++++++++++++ 6 files changed, 397 insertions(+), 8 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefs.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRingStateStore.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefsTest.kt 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 ceb0206..4db715c 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 @@ -2,6 +2,7 @@ package de.jeanlucmakiola.clockula.data.prefs import de.jeanlucmakiola.clockula.domain.ClockDefaults import de.jeanlucmakiola.clockula.domain.DismissChallenge +import de.jeanlucmakiola.clockula.domain.timer.TimerPresets import de.jeanlucmakiola.floret.prefs.Appearance import de.jeanlucmakiola.floret.prefs.AppearancePrefs import de.jeanlucmakiola.floret.prefs.PrefStore @@ -93,6 +94,26 @@ class SettingsPrefs @Inject constructor(private val store: PrefStore) { suspend fun setHomeZoneId(zoneId: String?) = store.set(ClockPrefs.homeZoneId, zoneId) + // --- timer presets (M6) --- + + /** + * The app-wide quick-start durations, sanitised on read as well as on write + * (D13). Absent means [de.jeanlucmakiola.clockula.domain.timer.TimerPresets.BUILT_IN]; + * an explicitly empty value means the user removed them all and is honoured. + */ + val timerPresets: Flow> = store.flow(TimerPrefs.presets) + + suspend fun setTimerPresets(values: List) = store.set(TimerPrefs.presets, values) + + /** Through [PrefStore.update], so two concurrent edits cannot swallow each other. */ + suspend fun addTimerPreset(value: Duration) { + store.update(TimerPrefs.presets) { TimerPresets.plus(it, value) } + } + + suspend fun removeTimerPreset(value: Duration) { + store.update(TimerPrefs.presets) { TimerPresets.minus(it, value) } + } + // --- first-launch bookkeeping (M4) --- /** diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefs.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefs.kt new file mode 100644 index 0000000..e8182aa --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefs.kt @@ -0,0 +1,44 @@ +package de.jeanlucmakiola.clockula.data.prefs + +import de.jeanlucmakiola.clockula.domain.timer.TimerPresets +import de.jeanlucmakiola.floret.prefs.Pref +import de.jeanlucmakiola.floret.prefs.nullableLongPref +import de.jeanlucmakiola.floret.prefs.nullableStringPref +import kotlin.time.Duration +import kotlin.time.Instant + +/** + * The timer path's own on-disk keys. Both are single *records* rather than + * lists of rows, which is why docs/PLAN.md §5 sends them here instead of to + * Room — and it keeps the ring's volatile state out of M10's backup of a timer + * (slice plan D13, D14). + */ +object TimerPrefs { + + /** + * The app-wide quick-start durations, sanitised through [Pref.map] on read + * as well as on write — exactly as every bounded value in [ClockPrefs] is. + * An **absent** key means [TimerPresets.BUILT_IN]; an explicitly **empty** + * value means the user removed them all and is honoured. + */ + val presets: Pref> = nullableStringPref("timer_presets").map( + decode = { raw -> + if (raw == null) TimerPresets.BUILT_IN else TimerPresets.sanitise(TimerPresets.decode(raw)) + }, + encode = { values -> TimerPresets.encode(TimerPresets.sanitise(values)) }, + ) + + /** + * When the current ring session began *sounding*. Wall clock, deliberately: + * "how long has this been making noise" is a question a reboot must not + * reset, which is why `alarm_states.ringing_since` is wall clock too. §5's + * elapsed-realtime rule governs the countdown, not the ring's age. + * + * Null means no session, and writing null removes the key rather than + * storing a sentinel for it. + */ + val ringSoundingSince: Pref = nullableLongPref("timer_ring_sounding_since_millis").map( + decode = { millis -> millis?.let(Instant::fromEpochMilliseconds) }, + encode = { instant -> instant?.toEpochMilliseconds() }, + ) +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt index cfa22b5..b8f5012 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt @@ -20,14 +20,45 @@ interface TimerRepository { /** Back to IDLE with the full configured duration; clears every anchor. */ suspend fun reset(id: Long) - /** "+1 min": extends the remaining time only — [Timer.duration] is untouched. */ + /** + * "+1 min", measured from the tap: a RUNNING timer is rebased on what is + * actually left, a PAUSED one keeps its pause, an **EXPIRED one resumes with + * exactly [extra] left**, and an IDLE one is a silent no-op. + * [Timer.duration] is untouched on every branch, so [reset] still returns + * the timer to the length the user configured (M6 D12). + */ suspend fun addTime(id: Long, extra: Duration) /** The ringing service calls this when a timer reaches zero. */ suspend fun markExpired(id: Long) + /** + * No-op unless the timer is IDLE (D3): changing the configured length of a + * countdown that is already running is ambiguous, and every answer is a + * surprise. Sets `remaining` to match. Clamped at zero. + */ + suspend fun setDuration(id: Long, duration: Duration) + + /** Normalised through `Ringtones`; blank becomes null. Touches no anchor. */ + suspend fun setRingtoneUri(id: Long, uri: String?) + suspend fun reorder(idsInOrder: List) + /** + * Rewrites every RUNNING timer's wall-clock fallback from what its + * monotonic anchor says is left. Call on a clock or zone change: the + * elapsed-realtime anchors are authoritative and must not move, but + * `endsAtWallClock` is a wall-clock instant the change has just + * invalidated — and it is both the notification's chronometer base and the + * reboot fallback, so leaving it stale makes a running timer read finished + * and a reboot ring it at the wrong minute (M6 D4, D19). + * + * A row whose monotonic anchor is already stale is left alone: + * [repairAfterReboot] owns that case, and its remainder is read *from* the + * fallback, so re-deriving the fallback from it would be circular. + */ + suspend fun reanchorWallClocks() + /** * Rewrites every RUNNING timer's monotonic anchors from its wall-clock * fallback. Call once at the top of a new boot, before the new uptime can diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt index e83005e..1de689c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt @@ -1,5 +1,6 @@ package de.jeanlucmakiola.clockula.data.timers +import de.jeanlucmakiola.clockula.domain.Ringtones import de.jeanlucmakiola.clockula.domain.Timer import de.jeanlucmakiola.clockula.domain.TimerDraft import de.jeanlucmakiola.clockula.domain.TimerState @@ -71,21 +72,41 @@ class TimerRepositoryImpl @Inject constructor( it.copy(state = TimerState.IDLE, remaining = it.duration).cleared() } + /** + * One rule for all three live states: the extra is measured **from the + * tap**, never from an anchor that has gone by. [Timer.duration] is never + * moved, so `reset` still returns the timer to the length the user + * configured (M6 D12). + * + * | Before | After | + * |---|---| + * | RUNNING | still running, rebased on `snapshot.remaining + extra` | + * | PAUSED | still paused, `remaining + extra`. The user paused deliberately | + * | EXPIRED | **RUNNING with exactly `extra` left**, all three anchors written | + * | IDLE | nothing. A stale notification action is a silent no-op | + */ override suspend fun addTime(id: Long, extra: Duration) { val now = elapsedRealtimeClock.elapsedRealtime() val wall = wallClock.now() edit(id, wall) { timer -> - if (timer.state == TimerState.RUNNING) { + when (timer.state) { // Rebased on what is actually left, not on the old end anchor: // a timer whose anchor is already past has zero left, and the // user asked for `extra` more than zero — not `extra` more than // an anchor that has gone by. - timer.anchored(timer.snapshotAt(now, wall).remaining + extra, now, wall) - } else { - // An expired timer that gains time is paused, not running: it has - // no anchors, and the user still has to press start. - val state = if (timer.state == TimerState.EXPIRED) TimerState.PAUSED else timer.state - timer.copy(state = state, remaining = timer.remaining + extra) + TimerState.RUNNING -> + timer.anchored(timer.snapshotAt(now, wall).remaining + extra, now, wall) + + // The same sentence, with the anchor that has gone by being the + // expiry itself: "+1 min" on a timer that has just rung is the + // commonest gesture in a timer app, and a second tap on Start is + // a papercut. Resuming here is also the only way to avoid + // emitting an intermediate PAUSED frame to the pill and the row. + TimerState.EXPIRED -> timer.anchored(extra, now, wall) + + TimerState.PAUSED -> timer.copy(remaining = timer.remaining + extra) + + TimerState.IDLE -> null } } } @@ -94,12 +115,52 @@ class TimerRepositoryImpl @Inject constructor( it.copy(state = TimerState.EXPIRED, remaining = Duration.ZERO).cleared() } + override suspend fun setDuration(id: Long, duration: Duration) = edit(id) { timer -> + // Refused unless IDLE: changing the configured length of a countdown + // that is already running is ambiguous, and every answer is a surprise + // (M6 D3). The editor hides the keypad for a non-idle timer, so this is + // here for a stale caller. + if (timer.state != TimerState.IDLE) return@edit null + val length = duration.coerceAtLeast(Duration.ZERO) + timer.copy(duration = length, remaining = length) + } + + override suspend fun setRingtoneUri(id: Long, uri: String?) = edit(id) { + it.copy(ringtoneUri = Ringtones.normalise(uri)) + } + /** Ids the table no longer holds are dropped, so the survivors number 0..n-1. */ override suspend fun reorder(idsInOrder: List) { val known = dao.observeAll().first().mapTo(HashSet()) { it.id } dao.reorder(idsInOrder.filter { it in known }) } + /** + * A clock change invalidates the opposite anchor from a reboot: the + * monotonic ones still hold, and the wall-clock fallback — the notification's + * chronometer base and the reboot fallback — is the one that has moved. It + * is re-derived from what the monotonic anchor says is left, so the two + * agree again (M6 D4, D19). + */ + override suspend fun reanchorWallClocks() { + val now = elapsedRealtimeClock.elapsedRealtime() + val wall = wallClock.now() + for (entity in dao.running()) { + edit(entity.id, wall) { timer -> + val snapshot = timer.snapshotAt(now, wall) + // A stale anchor is a reboot `repairAfterReboot` has not run for + // yet, and its remainder is read *from* the wall-clock fallback: + // re-deriving the fallback from it would be circular. + if (snapshot.anchorIsStale || !snapshot.isRunning) return@edit null + val endsAt = wall + snapshot.remaining + // No clock change, no write — so a resync that changed nothing + // emits nothing to the row and the pill. + if (endsAt == timer.endsAtWallClock) return@edit null + timer.copy(endsAtWallClock = endsAt) + } + } + } + /** * A reboot invalidates every monotonic anchor, and the new boot's uptime * eventually climbs past the stored one — at which point a dead anchor looks diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRingStateStore.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRingStateStore.kt new file mode 100644 index 0000000..754e2cd --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRingStateStore.kt @@ -0,0 +1,25 @@ +package de.jeanlucmakiola.clockula.data.timers + +import de.jeanlucmakiola.clockula.data.prefs.TimerPrefs +import de.jeanlucmakiola.floret.prefs.PrefStore +import kotlinx.coroutines.flow.Flow +import kotlin.time.Instant +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The DataStore half of the timer ring: one nullable wall-clock instant saying + * when the current session began *sounding*. Null means no session. A single + * record, so docs/PLAN.md §5 sends it here rather than to a Room table — and + * volatile ring state then stays out of M10's backup of a timer (D14). + */ +@Singleton +class TimerRingStateStore @Inject constructor(private val store: PrefStore) { + + /** Built once, distinct-until-changed (ARCHITECTURE.md §6). */ + val soundingSince: Flow = store.flow(TimerPrefs.ringSoundingSince) + + suspend fun current(): Instant? = store.get(TimerPrefs.ringSoundingSince) + + suspend fun set(instant: Instant?) = store.set(TimerPrefs.ringSoundingSince, instant) +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefsTest.kt new file mode 100644 index 0000000..f8fcbd7 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerPrefsTest.kt @@ -0,0 +1,207 @@ +package de.jeanlucmakiola.clockula.data.prefs + +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.stringPreferencesKey +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.data.timers.TimerRingStateStore +import de.jeanlucmakiola.clockula.domain.timer.TimerPresets +import de.jeanlucmakiola.clockula.testing.T0 +import de.jeanlucmakiola.floret.prefs.PrefStore +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.job +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.nio.file.Path +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds + +/** + * §5.10 — 12 cases over a **real** DataStore on a `@TempDir`, as `ClockPrefsTest` + * does. The presets are sanitised on read as well as on write (D13), and the + * ring session's anchor is one nullable key whose absence *is* "no session" + * (D14). + */ +class TimerPrefsTest { + + private val presetsKey = stringPreferencesKey("timer_presets") + private val soundingSinceKey = "timer_ring_sounding_since_millis" + + /** + * One store's worth of lifetime, as `SettingsPrefsTest` documents: DataStore + * keeps a registry of the files it has open and refuses a second store over + * a live one, so a test that wants to *reopen* a file has to end the first + * scope first — the way a process ending is what really releases the file. + */ + private fun TestScope.newScope(): CoroutineScope = + CoroutineScope(UnconfinedTestDispatcher(testScheduler) + Job()) + + private fun newDataStore(tempDir: Path, scope: CoroutineScope): DataStore = + PreferenceDataStoreFactory.create( + scope = scope, + produceFile = { tempDir.resolve("clockula_prefs_test.preferences_pb").toFile() }, + ) + + private fun TestScope.newDataStore(tempDir: Path): DataStore = + newDataStore(tempDir, newScope()) + + private suspend fun DataStore.writeRawPresets(raw: String) { + updateData { it.toMutablePreferences().apply { this[presetsKey] = raw } } + } + + /** §5.10 #1 */ + @Test + fun `an absent key means the built-in set`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + + assertThat(prefs.timerPresets.first()).isEqualTo(TimerPresets.BUILT_IN) + } + + /** §5.10 #2 */ + @Test + fun `an explicitly empty set is the user's choice and is honoured`(@TempDir tempDir: Path) = + runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + + prefs.setTimerPresets(emptyList()) + + assertThat(prefs.timerPresets.first()).isEmpty() + } + + /** §5.10 #3 */ + @Test + fun `a written set round-trips`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + + prefs.setTimerPresets(listOf(90.seconds, 7.minutes)) + + assertThat(prefs.timerPresets.first()).containsExactly(90.seconds, 7.minutes).inOrder() + } + + /** §5.10 #4 */ + @Test + fun `a hand-edited nonsense value reads as nothing and throws nothing`(@TempDir tempDir: Path) = + runTest { + val dataStore = newDataStore(tempDir) + val prefs = SettingsPrefs(PrefStore(dataStore)) + + dataStore.writeRawPresets("nonsense") + + assertThat(prefs.timerPresets.first()).isEmpty() + } + + /** §5.10 #5 */ + @Test + fun `a stored entry below the minimum is dropped on read`(@TempDir tempDir: Path) = runTest { + val dataStore = newDataStore(tempDir) + val prefs = SettingsPrefs(PrefStore(dataStore)) + + dataStore.writeRawPresets("${100.milliseconds.inWholeMilliseconds},${1.minutes.inWholeMilliseconds}") + + assertThat(prefs.timerPresets.first()).containsExactly(1.minutes) + } + + /** §5.10 #6 */ + @Test + fun `a stored entry above the maximum is dropped, not clamped`(@TempDir tempDir: Path) = runTest { + val dataStore = newDataStore(tempDir) + val prefs = SettingsPrefs(PrefStore(dataStore)) + + dataStore.writeRawPresets("${48.hours.inWholeMilliseconds},${1.minutes.inWholeMilliseconds}") + + assertThat(prefs.timerPresets.first()).containsExactly(1.minutes) + } + + /** §5.10 #7 */ + @Test + fun `a stored duplicate reads back once`(@TempDir tempDir: Path) = runTest { + val dataStore = newDataStore(tempDir) + val prefs = SettingsPrefs(PrefStore(dataStore)) + + dataStore.writeRawPresets("300000,300000") + + assertThat(prefs.timerPresets.first()).containsExactly(5.minutes) + } + + /** §5.10 #8 */ + @Test + fun `a descending stored list reads back ascending`(@TempDir tempDir: Path) = runTest { + val dataStore = newDataStore(tempDir) + val prefs = SettingsPrefs(PrefStore(dataStore)) + + dataStore.writeRawPresets("600000,300000,60000") + + assertThat(prefs.timerPresets.first()) + .containsExactly(1.minutes, 5.minutes, 10.minutes) + .inOrder() + } + + /** §5.10 #9 */ + @Test + fun `two presets saved at once cannot swallow each other`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.setTimerPresets(emptyList()) + + coroutineScope { + launch { prefs.addTimerPreset(3.minutes) } + launch { prefs.addTimerPreset(7.minutes) } + } + + assertThat(prefs.timerPresets.first()).containsExactly(3.minutes, 7.minutes).inOrder() + } + + /** §5.10 #10 */ + @Test + fun `removing one preset leaves the rest in order`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.setTimerPresets(listOf(1.minutes, 5.minutes, 10.minutes)) + + prefs.removeTimerPreset(5.minutes) + + assertThat(prefs.timerPresets.first()).containsExactly(1.minutes, 10.minutes).inOrder() + } + + /** §5.10 #11 */ + @Test + fun `the session anchor survives process death`(@TempDir tempDir: Path) = runTest { + val firstProcess = newScope() + val store = TimerRingStateStore(PrefStore(newDataStore(tempDir, firstProcess))) + + store.set(T0) + val beforeDeath = store.current() + + // The process death: the store's scope dies with the process, and that + // is also what lets the file be opened again. A dead store cannot be + // read, so what it saw is captured while it is still alive. + firstProcess.coroutineContext.job.cancelAndJoin() + val reborn = TimerRingStateStore(PrefStore(newDataStore(tempDir, newScope()))) + + assertThat(beforeDeath to reborn.current()).isEqualTo(T0 to T0) + } + + /** §5.10 #12 */ + @Test + fun `closing the session removes the key rather than storing a sentinel`( + @TempDir tempDir: Path, + ) = runTest { + val dataStore = newDataStore(tempDir) + val store = TimerRingStateStore(PrefStore(dataStore)) + store.set(T0) + + store.set(null) + + assertThat(dataStore.data.first().asMap().keys.map { it.name }) + .doesNotContain(soundingSinceKey) + } +}