fix(alarms): spend a skip at its occurrence again
Keeping a skip armed through its fire grace let the editor's switch or another dismissal inside those two minutes ring the skipped alarm late, or be absorbed. The resolver goes back to spending the skip at its occurrence and recording it as handled, and the upcoming check wakes at an armed skip's occurrence so the list stops saying "skipping next" on time. Fixes #16
This commit is contained in:
1 parent
a88e696d52
commit
fdb725e64c
6 files changed
+25
-72
No files matched your search
@@ -113,10 +113,7 @@ object AlarmResolver {
|
||||
}
|
||||
|
||||
val watermark = working.skippedOccurrence ?: candidates(alarm, working, now, zone, fireGrace).first()
|
||||
// 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
|
||||
val consumed = watermark <= now
|
||||
return AlarmResolution(
|
||||
alarmId = alarm.id,
|
||||
// Always the candidate *after* the watermark, so a skipped occurrence
|
||||
|
||||
@@ -51,12 +51,13 @@ object UpcomingNotices {
|
||||
* - a window opening, so its notice goes up;
|
||||
* - a noticed occurrence's fire grace running out, so a notice whose fire
|
||||
* came late (Doze, no exact alarms) does not outlive its alarm;
|
||||
* - an armed skip's ([pendingSkips]) fire grace running out, so a skip
|
||||
* dismissed in advance is spent on time and the list stops saying
|
||||
* "skipping next" about an occurrence that has gone by.
|
||||
* - an armed skip's ([pendingSkips]) occurrence, so a skip dismissed in
|
||||
* advance is spent on time and the list stops saying "skipping next"
|
||||
* about an occurrence that has gone by. Nothing is registered to fire
|
||||
* at a skipped occurrence, so there is no fire to race.
|
||||
*
|
||||
* The last two wait out the grace rather than land on the alarm's own
|
||||
* instant, so they never race its fire.
|
||||
* The closing waits out the grace rather than land on the alarm's own
|
||||
* instant, so it never races that alarm's fire.
|
||||
*/
|
||||
fun plan(
|
||||
upcoming: List<UpcomingAlarm>,
|
||||
@@ -73,7 +74,7 @@ object UpcomingNotices {
|
||||
.filter { it.source == FireSource.OCCURRENCE }
|
||||
.mapNotNull { entry -> entry.nextFire?.let { it - lead } }
|
||||
val closings = notices.map { it.fireAt + fireGrace }
|
||||
val skipsSpent = pendingSkips.map { it + fireGrace }
|
||||
val skipsSpent = pendingSkips
|
||||
val nextCheckAt = (openings + closings + skipsSpent).filter { it > now }.minOrNull()
|
||||
return UpcomingNoticePlan(notices, nextCheckAt)
|
||||
}
|
||||
|
||||
@@ -15,7 +15,6 @@ 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
|
||||
@@ -72,9 +71,9 @@ class AlarmEngineUpcomingTest {
|
||||
assertThat(alarm.skipNextOccurrence).isTrue()
|
||||
assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow)
|
||||
assertThat(harness.notifier.upcoming).isEmpty()
|
||||
// Back once the skipped occurrence has gone by, to spend the skip.
|
||||
assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE)
|
||||
harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE
|
||||
// Back at the skipped occurrence, to spend the skip.
|
||||
assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty)
|
||||
harness.wallClock.instant = sixThirty
|
||||
harness.engine.onUpcomingWindow()
|
||||
assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse()
|
||||
assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow)
|
||||
@@ -190,10 +189,7 @@ 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 once its grace is over,
|
||||
// not on tomorrow's.
|
||||
harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE
|
||||
harness.engine.onUpcomingWindow()
|
||||
// The skip is spent on the one occurrence, not on tomorrow's.
|
||||
assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse()
|
||||
assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow)
|
||||
}
|
||||
@@ -221,24 +217,6 @@ 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)
|
||||
|
||||
@@ -250,11 +250,7 @@ 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 + AlarmRing.FIRE_GRACE + 30.seconds,
|
||||
)
|
||||
val resolution = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds)
|
||||
|
||||
assertThat(resolution.clearSkipFlag).isTrue()
|
||||
assertThat(resolution.state.skippedOccurrence).isNull()
|
||||
@@ -265,36 +261,19 @@ 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 + AlarmRing.FIRE_GRACE + 30.seconds,
|
||||
)
|
||||
val first = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 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 + AlarmRing.FIRE_GRACE + 40.seconds,
|
||||
now = watermark + 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)
|
||||
|
||||
@@ -103,7 +103,7 @@ class UpcomingNoticesTest {
|
||||
)
|
||||
|
||||
// So the skip is spent then, not at tomorrow's window.
|
||||
assertThat(plan.nextCheckAt).isEqualTo(skipped + AlarmRing.FIRE_GRACE)
|
||||
assertThat(plan.nextCheckAt).isEqualTo(skipped)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
+9
-11
@@ -250,12 +250,10 @@ 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 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.
|
||||
- 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).
|
||||
|
||||
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
|
||||
@@ -1241,11 +1239,11 @@ the alarm's row for whoever opens the app instead.
|
||||
`rescheduleLocked`, so that is where the notices are synced and the third
|
||||
slot is armed for the earliest window still to open, *after* the real
|
||||
registration. The check also wakes once a noticed occurrence's fire grace has
|
||||
run out (so a notice cannot outlive a fire that came late) and once an armed
|
||||
skip's has (so a skip dismissed in advance is spent on time, and the list
|
||||
stops saying "skipping next" about an occurrence already gone). Both wait out
|
||||
the grace rather than land on the alarm's own instant, so they never race its
|
||||
fire. The sync is wrapped so a notification failure costs nothing,
|
||||
run out (so a notice cannot outlive a fire that came late; waiting out the
|
||||
grace keeps it from racing that fire) and at an armed skip's occurrence (so a
|
||||
skip dismissed in advance is spent on time, and the list stops saying
|
||||
"skipping next" about an occurrence already gone; nothing is registered to
|
||||
fire then, so there is nothing to race). The sync is wrapped so a notification failure costs nothing,
|
||||
and on the fire path it is deferred until the ring has started, so its binder
|
||||
calls never stand between the fire and the sound. The check's receiver just
|
||||
calls `onUpcomingWindow()`, which is `reschedule()`.
|
||||
|
||||
Reference in new issue
Block a user