diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index 5e3a23c..6c95c1e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -290,6 +290,10 @@ class AlarmEngine @Inject constructor( */ private suspend fun dismissComingLocked(alarm: Alarm): DismissOutcome { if (alarm.repeatDays.isRepeating) { + // Persist the resolution first: an earlier skip that has gone by but + // not yet been spent in storage would otherwise absorb this one, + // and the occurrence just dismissed would ring. + rescheduleLocked() alarms.setSkipNextOccurrence(alarm.id, true) rescheduleLocked() return DismissOutcome.SkippedNext(alarm.id) @@ -309,12 +313,16 @@ class AlarmEngine @Inject constructor( // delivered late (after a reboot, or by Doze) still rings. Rejecting it // here would only re-register the same past instant and fire again. val lateBy = if (isSnoozeFire) AlarmRing.AUTO_SILENCE_AFTER else AlarmRing.FIRE_GRACE - // A skip armed on this very occurrence after its fire was already on its - // way: the user dismissed it in advance (#16), so it must not ring. - val skipped = !isSnoozeFire && state.skippedOccurrence == scheduledFor - // Deleted, switched off, skipped, or a PendingIntent so stale the moment - // has gone: re-resolve the slot and stay quiet. - if (alarm == null || !alarm.enabled || skipped || now - scheduledFor > lateBy) { + // An occurrence already accounted for: skipped in advance (#16) — armed or, + // once a reschedule spent the skip, recorded as handled — or a duplicate + // of one that already rang. A fire already on its way when that was + // written must not ring it. Equality only: a watermark ahead of a clock + // moved backwards must never silence an earlier occurrence. + val accountedFor = !isSnoozeFire && + (scheduledFor == state.skippedOccurrence || scheduledFor == state.handledOccurrence) + // Deleted, switched off, accounted for, or a PendingIntent so stale the + // moment has gone: re-resolve the slot and stay quiet. + if (alarm == null || !alarm.enabled || accountedFor || now - scheduledFor > lateBy) { rescheduleLocked() return FireOutcome.Ignored } @@ -343,10 +351,13 @@ class AlarmEngine @Inject constructor( val upcoming = mutableListOf() rescheduleLocked(deferUpcomingTo = upcoming) - states.save(states.state(alarmId).copy(ringingSince = now)) - ring.startRinging(alarmId) - scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId) - syncUpcomingLocked(upcoming, now) + try { + states.save(states.state(alarmId).copy(ringingSince = now)) + ring.startRinging(alarmId) + scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId) + } finally { + syncUpcomingLocked(upcoming, now) + } return FireOutcome.Ringing(alarmId) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt index 3c0ace1..bc2703c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt @@ -73,9 +73,10 @@ class AndroidAlarmNotifier @Inject constructor( // A swipe is the user's word for that occurrence; the record keeps // only the occurrences still noticed. - val swiped = UpcomingNotifications.swipedKeys(context) - val stillSwiped = swiped.intersect(notices.map(UpcomingNotifications::key).toSet()) - if (stillSwiped != swiped) UpcomingNotifications.retainSwiped(context, stillSwiped) + val stillSwiped = UpcomingNotifications.retainSwiped( + context, + notices.map(UpcomingNotifications::key).toSet(), + ) // POST_NOTIFICATIONS is a runtime permission from API 33. Denied, the // user loses the notice and keeps the alarm and the row's Dismiss. diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt index 637e583..812e193 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt @@ -13,8 +13,9 @@ import kotlin.time.Instant import javax.inject.Inject /** - * The AlarmManager slots arriving: the fire, the backstop and the upcoming check. Thin by design: every decision the engine - * could make is the engine's, so the receiver only unpacks and dispatches. + * The AlarmManager slots arriving: the fire, the backstop and the upcoming + * check. Thin by design: every decision the engine could make is the + * engine's, so the receiver only unpacks and dispatches. */ @AndroidEntryPoint class AlarmFireReceiver : HiltBroadcastReceiver() { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt index c7d431e..c4f182b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt @@ -81,19 +81,28 @@ object UpcomingNotifications { * a new process, never brings back a notice the user got rid of, and a * notice the system took away does come back. */ - fun swipedKeys(context: Context): Set = - prefs(context).getStringSet(SWIPED_KEY, emptySet()).orEmpty() - + @Synchronized fun recordSwiped(context: Context, alarmId: Long, fireAtMillis: Long) { - val keys = swipedKeys(context) + key(alarmId, fireAtMillis) + val keys = read(context) + key(alarmId, fireAtMillis) prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply() } - /** Drops the keys for occurrences no longer noticed, so the record stays small. */ - fun retainSwiped(context: Context, keys: Set) { - prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply() + /** + * Drops the keys for occurrences no longer in [noticed], so the record stays + * small, and returns what is left. One synchronised read-modify-write, so a + * swipe recorded from the receiver meanwhile is never overwritten. + */ + @Synchronized + fun retainSwiped(context: Context, noticed: Set): Set { + val swiped = read(context) + val kept = swiped.intersect(noticed) + if (kept != swiped) prefs(context).edit().putStringSet(SWIPED_KEY, kept).apply() + return kept } + private fun read(context: Context): Set = + prefs(context).getStringSet(SWIPED_KEY, emptySet()).orEmpty().toSet() + fun key(notice: UpcomingNotice): String = key(notice.alarm.id, notice.fireAt.toEpochMilliseconds()) private fun key(alarmId: Long, fireAtMillis: Long): String = "$alarmId@$fireAtMillis" @@ -138,6 +147,7 @@ object UpcomingNotifications { PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) + /** Excluded from backup and device transfer: it describes this device's shade. */ private const val RECORD_FILE = "alarm_upcoming_notices" private const val SWIPED_KEY = "swiped" diff --git a/app/src/main/res/xml/backup_rules.xml b/app/src/main/res/xml/backup_rules.xml index a407ce6..7177e8b 100644 --- a/app/src/main/res/xml/backup_rules.xml +++ b/app/src/main/res/xml/backup_rules.xml @@ -5,4 +5,6 @@ otherwise prompt again after a restore. Keep in step with data_extraction_rules.xml. --> + + diff --git a/app/src/main/res/xml/data_extraction_rules.xml b/app/src/main/res/xml/data_extraction_rules.xml index a8a7cba..55a8382 100644 --- a/app/src/main/res/xml/data_extraction_rules.xml +++ b/app/src/main/res/xml/data_extraction_rules.xml @@ -1,10 +1,13 @@ - + + + diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index 718a488..c6e4fba 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -8,6 +8,7 @@ import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotices import de.jeanlucmakiola.clockula.testing.RingEvent import de.jeanlucmakiola.clockula.testing.alarmEngineHarness import de.jeanlucmakiola.clockula.testing.offsetInstant +import de.jeanlucmakiola.clockula.testing.ringState import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir @@ -207,4 +208,41 @@ class AlarmEngineUpcomingTest { assertThat(harness.engine.dismissUpcomingOccurrence(1L, sixThirty)).isEqualTo(DismissOutcome.NotFound) } + + @Test + fun `a fire that lands after the dismissed skip was spent still does not ring`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + // Another entry point reschedules first and spends the skip. + harness.wallClock.instant = sixThirty + harness.engine.reschedule() + val outcome = harness.engine.onFire(1L, sixThirty) + + assertThat(outcome).isEqualTo(FireOutcome.Ignored) + assertThat(harness.ring.events).isEmpty() + } + + @Test + fun `an old skip gone by but not yet spent does not swallow the new dismissal`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm( + id = 1L, + hour = 6, + minute = 30, + repeatDays = RepeatDays.EVERY_DAY, + skipNextOccurrence = true, + ) + // Yesterday's skip, never spent because nothing rescheduled since. + harness.states.save(ringState(alarmId = 1L, skippedOccurrence = offsetInstant("2026-06-09T06:30+02:00"))) + + val outcome = harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + assertThat(outcome).isEqualTo(DismissOutcome.SkippedNext(1L)) + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7ab3848..cfda3ee 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1250,8 +1250,12 @@ the alarm's row for whoever opens the app instead. round its challenge. When it matches, it takes `dismissUpcoming`'s own branches: a repeating alarm skips that occurrence, a one-shot is switched off (or deleted, if transient). A fire broadcast already in flight when a skip - was armed is `Ignored` by `onFire`, which checks the skip watermark against - the instant it carries. + was armed is `Ignored` by `onFire`: a natural fire whose instant *equals* + the skip or the handled-occurrence watermark is already accounted for. + Equality, not order, so a watermark left ahead of a clock moved backwards + never silences an earlier occurrence. A repeating alarm's resolution is + persisted before the skip is armed, so an old skip that has gone by but was + never spent cannot absorb the new one. - **No dismiss challenge before the ring**, as in Google Clock. The challenge proves you are awake while an alarm sounds; before it, skipping is already one unchallenged switch away in the editor. The notification's Dismiss does ask