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 bc2703c..2783ab7 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 @@ -68,7 +68,11 @@ class AndroidAlarmNotifier @Inject constructor( .filter { it.tag == UpcomingNotifications.TAG } .map { it.id } .toSet() - onScreen.filterNot { it in wantedIds }.forEach { notifications.cancel(UpcomingNotifications.TAG, it) } + // Also what this process posted: a notify is asynchronous, so one + // posted a moment ago may not be listed as active yet. + (onScreen + lastPosted.keys.map(UpcomingNotifications::notificationId)) + .filterNot { it in wantedIds } + .forEach { notifications.cancel(UpcomingNotifications.TAG, it) } lastPosted.keys.retainAll(notices.map { it.alarm.id }.toSet()) // A swipe is the user's word for that occurrence; the record keeps diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt index 4adf88b..a13bc60 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt @@ -113,7 +113,10 @@ object AlarmResolver { } val watermark = working.skippedOccurrence ?: candidates(alarm, working, now, zone, fireGrace).first() - val consumed = watermark <= now + // Spent only once the skipped occurrence is out of the fire grace too: + // a fire for it can still be accepted until then, and the engine refuses + // that fire by this very watermark (#16). + val consumed = watermark + fireGrace <= now return AlarmResolution( alarmId = alarm.id, // Always the candidate *after* the watermark, so a skipped occurrence 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 c2ae9a4..65b062e 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -15,6 +15,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds /** * The upcoming-alarm notification (#16). The harness starts at 05:00 Berlin, so @@ -189,8 +190,12 @@ class AlarmEngineUpcomingTest { assertThat(outcome).isEqualTo(FireOutcome.Ignored) assertThat(harness.ring.events).isEmpty() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) - // The skip is spent on the one occurrence, not on tomorrow's. + // The skip is spent on the one occurrence once its grace is over, + // not on tomorrow's. + harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE + harness.engine.onUpcomingWindow() assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) } @Test @@ -216,6 +221,24 @@ class AlarmEngineUpcomingTest { assertThat(harness.engine.dismissUpcomingOccurrence(1L, sixThirty)).isEqualTo(DismissOutcome.NotFound) } + @Test + fun `a fire for the dismissed occurrence is refused even after another reschedule`(@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 inside the fire grace, before the + // fire already on its way is processed: the skip is not spent yet. + harness.wallClock.instant = sixThirty + 30.seconds + harness.engine.reschedule() + val outcome = harness.engine.onFire(1L, sixThirty) + + assertThat(outcome).isEqualTo(FireOutcome.Ignored) + assertThat(harness.ring.events).isEmpty() + } + @Test fun `a handled occurrence a clock moved backwards re-armed still rings`(@TempDir tempDir: Path) = runTest { val harness = alarmEngineHarness(tempDir) diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt index 6d89e0e..2d3d87b 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt @@ -250,7 +250,11 @@ class AlarmResolverTest { val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) val watermark = offsetInstant("2026-06-10T07:00+02:00") - val resolution = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) + val resolution = resolve( + alarm, + ringState(skippedOccurrence = watermark), + now = watermark + AlarmRing.FIRE_GRACE + 30.seconds, + ) assertThat(resolution.clearSkipFlag).isTrue() assertThat(resolution.state.skippedOccurrence).isNull() @@ -261,19 +265,36 @@ class AlarmResolverTest { fun `a consumed skip cannot ring on the next pass through the fire grace`() { val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) val watermark = offsetInstant("2026-06-10T07:00+02:00") - val first = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) + val first = resolve( + alarm, + ringState(skippedOccurrence = watermark), + now = watermark + AlarmRing.FIRE_GRACE + 30.seconds, + ) // The engine persists the first pass: the flag cleared, the state saved. val second = resolve( alarm.copy(skipNextOccurrence = false), first.state, - now = watermark + 40.seconds, + now = watermark + AlarmRing.FIRE_GRACE + 40.seconds, ) assertThat(first.state.handledOccurrence).isEqualTo(watermark) assertThat(second.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) } + @Test + fun `a skip stays armed through its occurrence's fire grace`() { + val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) + val watermark = offsetInstant("2026-06-10T07:00+02:00") + + val resolution = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) + + // A fire for it can still arrive; the engine refuses it by this watermark. + assertThat(resolution.clearSkipFlag).isFalse() + assertThat(resolution.state.skippedOccurrence).isEqualTo(watermark) + assertThat(resolution.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) + } + @Test fun `skipping a one-shot alarm disables it`() { val alarm = alarmAt(hour = 7, skipNextOccurrence = true) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6ad2880..008afb1 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -250,10 +250,12 @@ so the skip eats a second alarm. With the watermark: one after; - watermark still in the future ⇒ fire the first candidate after it; write nothing; -- watermark at or before now ⇒ consumed: clear the flag and the watermark, - record the skipped occurrence as the handled one, and *still* return the - first candidate after it, so the skipped occurrence cannot ring on its way - out through the grace window, on this pass or the next (#16). +- watermark and its fire grace both behind now ⇒ consumed: clear the flag and + the watermark, record the skipped occurrence as the handled one, and *still* + return the first candidate after it, so the skipped occurrence cannot ring on + this pass or the next. Not spent during the grace itself (#16): a late fire + for the skipped occurrence can still be accepted until then, and `onFire` + refuses it by the armed watermark. Arming is deferred while a snooze is pending, and a **non-repeating** alarm with the flag set is *disabled* rather than skipped: a one-shot has exactly one