fix(alarms): spend a skip only after its fire grace
The resolver now keeps a skip armed until its occurrence is out of the fire grace too, so a reschedule in those two minutes cannot spend it early and let a fire already in flight ring the dismissed alarm; onFire refuses that fire by the armed watermark. The notifier also cancels notices it posted itself, which the platform may not list as active yet. Fixes #16
This commit is contained in:
1 parent
b4af164826
commit
a88e696d52
5 files changed
+63
-10
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user