diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt index 9eedc95..d0decb5 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt @@ -1,5 +1,12 @@ package de.jeanlucmakiola.calendula.domain.reminders +import java.time.Instant +import java.time.LocalDate +import java.time.LocalTime +import java.time.ZoneId +import java.time.ZoneOffset +import java.time.temporal.ChronoUnit + /** * Works out *when* each reminder has to fire and *which* ones are due, with no * provider and no clock of its own — the whole decision layer of in-house @@ -63,15 +70,30 @@ data class ReminderSchedule( ) private const val MILLIS_PER_MINUTE = 60_000L +private const val MINUTES_PER_DAY = 1_440 /** * Pair every instance with each of its event's reminder offsets. * - * The fire instant is `begin − minutes` for timed **and** all-day occurrences - * alike. That is not an oversight: an all-day instance's `begin` is UTC midnight, - * and `AllDayReminderEncoding` deliberately encodes the wanted wall-clock time - * *into* the stored offset measured from exactly that point. Applying a second, - * timezone-dependent correction here would undo it. + * A **timed** occurrence is trivial: `begin` is an absolute instant, so + * `begin − minutes` is exact by construction, in any timezone, across any DST + * boundary. + * + * An **all-day** occurrence is not, and taking the offset at face value is what + * makes reminders land at the wrong hour. Its `begin` is UTC midnight, and the + * stored offset is not a plain lead time — `AllDayReminderEncoding` folds the + * wanted wall-clock hour into it, sampled against *one* date's UTC offset. Fire + * at `begin − minutes` and every occurrence in a different DST phase than the one + * that was sampled drifts by the offset delta, an hour early in one direction and + * an hour late in the other. Rows written by other apps carry no wall-clock at + * all — a conventional `1440` fires at UTC midnight, which is 01:00 or 02:00 + * local in Berlin and the wrong day west of UTC. + * + * So the offset is only read for *which day* it means, via [allDayLeadDays], and + * the hour comes from [allDayTimeMinutes] — the one global "show all-day + * reminders at" setting — recomposed against each occurrence's own date in + * [zone]. 09:00 Berlin is then 09:00 Berlin on every occurrence, whatever the + * offset was when the row was written. * * [minutesByEvent] may hold duplicate offsets (two identical reminder rows on one * event); they collapse, because they would otherwise fight over one notification. @@ -79,16 +101,58 @@ private const val MILLIS_PER_MINUTE = 60_000L fun planReminders( instances: List, minutesByEvent: Map>, + zone: ZoneId, + allDayTimeMinutes: Int, ): List = instances.flatMap { instance -> minutesByEvent[instance.eventId].orEmpty().distinct().map { minutes -> PlannedReminder( instance = instance, minutes = minutes, - alarmMillis = instance.beginMillis - minutes * MILLIS_PER_MINUTE, + alarmMillis = if (instance.isAllDay) { + allDayAlarmMillis(instance.beginMillis, minutes, zone, allDayTimeMinutes) + } else { + instance.beginMillis - minutes * MILLIS_PER_MINUTE + }, ) } } +/** UTC midnight of an all-day occurrence, as the calendar date it stands for. */ +private fun allDayDate(beginMillis: Long): LocalDate = + Instant.ofEpochMilli(beginMillis).atZone(ZoneOffset.UTC).toLocalDate() + +/** + * How many whole days before its occurrence a raw all-day offset means. + * + * A plain multiple of 1440 is read at face value. That covers rows from other + * calendar apps, which carry no encoded hour — and it stays right for our own + * rows that happen to land on a multiple, because those encode a wall-clock hour + * equal to the sampled UTC offset, so the day count is the same either way. + * + * Anything else is one of ours, with an hour folded in: recover the day count the + * way [de.jeanlucmakiola.calendula.data.calendar.fromProviderAllDayMinutes] + * does for display, by asking which local date the encoded instant falls on. + * Keeping the two in step is what makes the notification arrive on the day the + * event screen says it will. + */ +internal fun allDayLeadDays(rawMinutes: Int, beginMillis: Long, zone: ZoneId): Long { + if (rawMinutes % MINUTES_PER_DAY == 0) return (rawMinutes / MINUTES_PER_DAY).toLong() + val encoded = Instant.ofEpochMilli(beginMillis - rawMinutes * MILLIS_PER_MINUTE) + return ChronoUnit.DAYS.between(encoded.atZone(zone).toLocalDate(), allDayDate(beginMillis)) +} + +private fun allDayAlarmMillis( + beginMillis: Long, + rawMinutes: Int, + zone: ZoneId, + allDayTimeMinutes: Int, +): Long = allDayDate(beginMillis) + .minusDays(allDayLeadDays(rawMinutes, beginMillis, zone)) + .atTime(LocalTime.of(allDayTimeMinutes / 60, allDayTimeMinutes % 60)) + .atZone(zone) + .toInstant() + .toEpochMilli() + /** * Split [planned] into what is due now and when to wake up next. * diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt index c59e4ad..443bbcb 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt @@ -2,6 +2,11 @@ package de.jeanlucmakiola.calendula.domain.reminders import com.google.common.truth.Truth.assertThat import org.junit.jupiter.api.Test +import java.time.LocalDate +import java.time.LocalTime +import java.time.ZoneId +import java.time.ZoneOffset +import java.time.ZonedDateTime /** * The decision layer of in-house reminder delivery (#75). The watermark rules are @@ -30,10 +35,18 @@ class ReminderPlanTest { isAllDay = isAllDay, ) + /** [planReminders] with the fixtures' zone and all-day hour filled in. */ + private fun plan( + instances: List, + minutesByEvent: Map>, + zone: ZoneId = berlin, + allDayTimeMinutes: Int = nineAm, + ) = planReminders(instances, minutesByEvent, zone, allDayTimeMinutes) + @Test fun `a reminder fires its offset before the occurrence begins`() { val begin = now + 30 * minute - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L, beginMillis = begin)), minutesByEvent = mapOf(1L to listOf(10)), ) @@ -41,35 +54,159 @@ class ReminderPlanTest { assertThat(planned.map { it.alarmMillis }).containsExactly(begin - 10 * minute) } - @Test - fun `an all-day offset is measured from the raw begin, not corrected again`() { - // An all-day instance begins at UTC midnight and AllDayReminderEncoding - // already folded the wanted wall-clock time into the stored offset, so a - // second timezone correction here would move the reminder off it. - val utcMidnight = 1_700_006_400_000L - val planned = planReminders( - instances = listOf(instance(1L, beginMillis = utcMidnight, isAllDay = true)), - minutesByEvent = mapOf(1L to listOf(1_020)), - ) + // --- all-day: the hour the user picked, on every occurrence ------------- - assertThat(planned.single().alarmMillis).isEqualTo(utcMidnight - 1_020 * minute) + private val berlin = ZoneId.of("Europe/Berlin") + private val nineAm = 540 + + /** UTC midnight of [date] — how the provider stores an all-day occurrence. */ + private fun allDayBegin(date: String): Long = + LocalDate.parse(date).atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() + + private fun firedAt(alarmMillis: Long, zone: ZoneId = berlin): ZonedDateTime = + java.time.Instant.ofEpochMilli(alarmMillis).atZone(zone) + + /** + * The offset AllDayReminderEncoding would store for "[days] before, at + * [timeOfDayMinutes]" when sampled against [eventDate] — i.e. exactly the row + * the app writes today. + */ + private fun encodedAllDayMinutes( + eventDate: String, + days: Long, + timeOfDayMinutes: Int = nineAm, + zone: ZoneId = berlin, + ): Int { + val date = LocalDate.parse(eventDate) + val utcMidnight = date.atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() + val fire = date.minusDays(days) + .atTime(LocalTime.of(timeOfDayMinutes / 60, timeOfDayMinutes % 60)) + .atZone(zone).toInstant().toEpochMilli() + return ((utcMidnight - fire) / minute).toInt() + } + + private fun allDayAlarm(eventDate: String, rawMinutes: Int, zone: ZoneId = berlin): Long = + plan( + instances = listOf( + instance(1L, beginMillis = allDayBegin(eventDate), isAllDay = true), + ), + minutesByEvent = mapOf(1L to listOf(rawMinutes)), + zone = zone, + allDayTimeMinutes = nineAm, + ).single().alarmMillis + + @Test + fun `an all-day reminder fires at the hour the setting names`() { + // Winter: Berlin is UTC+1. "1 day before" on the 15th means the 14th, 09:00. + val alarm = allDayAlarm("2026-01-15", encodedAllDayMinutes("2026-01-15", days = 1)) + + assertThat(firedAt(alarm).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-01-14").atTime(9, 0)) } @Test - fun `a negative offset fires after the occurrence begins`() { + fun `a summer occurrence of a row written in winter is not an hour early`() { + // The drift this replaces: the offset was sampled at UTC+1, the occurrence + // falls at UTC+2, and firing at `begin - minutes` would land at 08:00. + val winterRow = encodedAllDayMinutes("2026-01-15", days = 1) + + val alarm = allDayAlarm("2026-07-15", winterRow) + + assertThat(firedAt(alarm).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0)) + } + + @Test + fun `a winter occurrence of a row written in summer is not an hour late`() { + // The same drift in the other direction: sampled at UTC+2, fires at UTC+1. + val summerRow = encodedAllDayMinutes("2026-07-15", days = 1) + + val alarm = allDayAlarm("2026-01-15", summerRow) + + assertThat(firedAt(alarm).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-01-14").atTime(9, 0)) + } + + @Test + fun `every occurrence of a yearly all-day series fires at the same wall clock`() { + // A birthday's offset is sampled once; the series must not walk off it. + val row = encodedAllDayMinutes("2026-07-15", days = 1) + val fired = listOf("2026-07-15", "2027-01-15", "2027-07-15", "2028-01-15") + .map { firedAt(allDayAlarm(it, row)).toLocalTime() } + + assertThat(fired.toSet()).containsExactly(LocalTime.of(9, 0)) + } + + @Test + fun `an all-day reminder on the day itself fires that morning`() { // "At time of event" on an all-day event encodes to a negative offset. - val begin = now + 30 * minute - val planned = planReminders( - instances = listOf(instance(1L, beginMillis = begin, isAllDay = true)), - minutesByEvent = mapOf(1L to listOf(-420)), + val sameDay = encodedAllDayMinutes("2026-07-15", days = 0) + assertThat(sameDay).isLessThan(0) + + val alarm = allDayAlarm("2026-07-15", sameDay) + + assertThat(firedAt(alarm).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-07-15").atTime(9, 0)) + } + + @Test + fun `a plain 1440 row from another calendar app means one day before`() { + // Foreign rows carry no encoded hour. Read at face value they would fire + // at UTC midnight — 02:00 local in summer Berlin. + val alarm = allDayAlarm("2026-07-15", rawMinutes = 1_440) + + assertThat(firedAt(alarm).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0)) + } + + @Test + fun `a plain 1440 row west of UTC still means one day before`() { + // UTC midnight of the 15th is the evening of the 14th in New York, so a + // local-date reading of the offset would put this two days out. + val newYork = ZoneId.of("America/New_York") + + val alarm = allDayAlarm("2026-07-15", rawMinutes = 1_440, zone = newYork) + + assertThat(firedAt(alarm, newYork).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0)) + } + + @Test + fun `an encoded row west of UTC fires at the named hour`() { + val newYork = ZoneId.of("America/New_York") + val row = encodedAllDayMinutes("2026-07-15", days = 1, zone = newYork) + + val alarm = allDayAlarm("2026-07-15", row, zone = newYork) + + assertThat(firedAt(alarm, newYork).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0)) + } + + @Test + fun `a zero-day row fires on the event's own date`() { + assertThat(allDayLeadDays(rawMinutes = 0, beginMillis = allDayBegin("2026-07-15"), zone = berlin)) + .isEqualTo(0L) + } + + @Test + fun `a timed reminder is exact across a DST boundary`() { + // Nothing to re-anchor: begin is an absolute instant either way. + val begin = LocalDate.parse("2026-03-29").atTime(14, 0) + .atZone(berlin).toInstant().toEpochMilli() + val planned = plan( + instances = listOf(instance(1L, beginMillis = begin, isAllDay = false)), + minutesByEvent = mapOf(1L to listOf(30)), + zone = berlin, + allDayTimeMinutes = nineAm, ) - assertThat(planned.single().alarmMillis).isEqualTo(begin + 420 * minute) + assertThat(firedAt(planned.single().alarmMillis).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-03-29").atTime(13, 30)) } @Test fun `every occurrence of a series gets its own reminder`() { - val planned = planReminders( + val planned = plan( instances = listOf( instance(1L, beginMillis = now + day), instance(1L, beginMillis = now + 2 * day), @@ -84,7 +221,7 @@ class ReminderPlanTest { @Test fun `duplicate reminder rows collapse to one`() { - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L)), minutesByEvent = mapOf(1L to listOf(10, 10)), ) @@ -94,7 +231,7 @@ class ReminderPlanTest { @Test fun `an event with no reminders plans nothing`() { - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L)), minutesByEvent = emptyMap(), ) @@ -105,7 +242,7 @@ class ReminderPlanTest { @Test fun `the same reminder keeps its key across scans`() { val plan = { - planReminders(listOf(instance(1L)), mapOf(1L to listOf(10))).single().key + plan(listOf(instance(1L)), mapOf(1L to listOf(10))).single().key } assertThat(plan()).isEqualTo(plan()) @@ -113,7 +250,7 @@ class ReminderPlanTest { @Test fun `occurrences of one series get different keys`() { - val planned = planReminders( + val planned = plan( instances = listOf( instance(1L, beginMillis = now + day), instance(1L, beginMillis = now + 2 * day), @@ -126,7 +263,7 @@ class ReminderPlanTest { @Test fun `two reminders on one occurrence get different keys`() { - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L)), minutesByEvent = mapOf(1L to listOf(10, 30)), ) @@ -136,7 +273,7 @@ class ReminderPlanTest { @Test fun `a reminder whose moment has passed since the last scan is due`() { - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L, beginMillis = now + 5 * minute)), minutesByEvent = mapOf(1L to listOf(10)), ) @@ -153,7 +290,7 @@ class ReminderPlanTest { fun `a reminder already covered by the watermark does not fire twice`() { // The scan runs again (a provider change, a reboot) after the alarm that // already posted this one. Nothing records "fired" but the watermark. - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L, beginMillis = now + 5 * minute)), minutesByEvent = mapOf(1L to listOf(10)), ) @@ -169,7 +306,7 @@ class ReminderPlanTest { @Test fun `a reminder exactly on the watermark does not fire again`() { val begin = now + 5 * minute - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L, beginMillis = begin)), minutesByEvent = mapOf(1L to listOf(10)), ) @@ -186,7 +323,7 @@ class ReminderPlanTest { fun `a missed alarm is caught up by a much later scan`() { // The device was off over the reminder; the scan on boot must still post it // while the event is ahead. This is what the provider path could never do. - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L, beginMillis = now + 5 * minute)), minutesByEvent = mapOf(1L to listOf(60)), ) @@ -201,7 +338,7 @@ class ReminderPlanTest { @Test fun `a reminder for an occurrence that already ended is dropped`() { - val planned = planReminders( + val planned = plan( instances = listOf( instance(1L, beginMillis = now - 3 * 60 * minute, endMillis = now - 2 * 60 * minute), ), @@ -218,7 +355,7 @@ class ReminderPlanTest { @Test fun `an occurrence with no end falls back to its begin for relevance`() { - val planned = planReminders( + val planned = plan( instances = listOf( instance(1L, beginMillis = now - minute, endMillis = 0L), ), @@ -234,7 +371,7 @@ class ReminderPlanTest { @Test fun `due reminders come out in occurrence order`() { - val planned = planReminders( + val planned = plan( instances = listOf( instance(1L, beginMillis = now + 20 * minute), instance(2L, beginMillis = now + 5 * minute), @@ -251,7 +388,7 @@ class ReminderPlanTest { @Test fun `the next wake-up is the earliest reminder still ahead`() { - val planned = planReminders( + val planned = plan( instances = listOf( instance(1L, beginMillis = now + 20 * minute), instance(2L, beginMillis = now + 90 * minute), @@ -280,7 +417,7 @@ class ReminderPlanTest { fun `a reminder beyond the horizon waits for the next scan`() { // Capping keeps the rolling window honest: the far-off reminder is picked // up by a later scan rather than pinned to an alarm we may never re-check. - val planned = planReminders( + val planned = plan( instances = listOf(instance(1L, beginMillis = now + 30 * day)), minutesByEvent = mapOf(1L to listOf(5)), )