fix: make a reminder arrive when the user asked for it

Changing the reminder time did not move the reminder. WorkManager's
UPDATE policy carries the previous request's lastEnqueueTime and
periodCount forward, and a periodic request computes its next run as
periodCount == 0 ? lastEnqueueTime + initialDelay : lastEnqueueTime +
interval. Both halves bit:

After the first run, a new initial delay is ignored entirely -- Morning to
Evening did nothing at all. Before it, the delay is applied to the
ORIGINAL enqueue time, and the coordinator reschedules at every process
start, so asking for 19:00 at 09:00 on work enqueued at 08:00 produced
18:00, with every later period anchored off that.

The scheduler reads the existing work first: KEEP when nothing is
scheduled, leave an overdue run alone -- moving it skips today's reminder
entirely -- leave a run already within five minutes alone, and otherwise
UPDATE with an explicit setNextScheduleTimeOverride, which is the only way
to say WHEN rather than how long from a moment WorkManager has its own
opinion about. CANCEL_AND_REENQUEUE is wrong for a subtler reason: this
runs at every process start including the one WorkManager started to run
the worker, and cancelling the unique work there cancels the worker.

A time zone or clock change now re-aims it. The delay was computed once,
from the zone in force then, so flying east left the reminder arriving at
the old wall-clock time indefinitely. WorkManager's own RescheduleReceiver
declares BOOT_COMPLETED and nothing else -- which is why ClockChangeReceiver
exists for the other two broadcasts, and why it does not duplicate boot.
Unexported, no permission, checkPermissions still green.

No flex window was added, and the screen's copy changed instead. Flex
would have made "a few minutes either side" true and placed the first run
nearly a full period out, skipping the reminder on the day the user set
it -- to keep a sentence. It now says Android may deliver a few minutes
after, never before, which is what actually happens.

schedule() had no test; only the arithmetic beneath it did. Nine now,
against WorkManager's own recorded next-run time, sharing one clock with
it -- a test that fixes only the scheduler's measures a 2026 delay against
a real System.currentTimeMillis().

Writing them was necessary rather than tidy: the first version of the
change-the-time test passed with the defect still in place, because both
schedules happened at the same instant and the bug only bites once time
has moved. The test that catches it advances the clock an hour between
them, which is what a real second process start does.

closes #72

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
null 2026-08-20 22:42:09 -05:00
parent 65e8e574b7
commit f43d580cc7
12 changed files with 496 additions and 47 deletions

View File

@ -78,6 +78,27 @@
<category android:name="android.intent.category.LAUNCHER" /> <category android:name="android.intent.category.LAUNCHER" />
</intent-filter> </intent-filter>
</activity-alias> </activity-alias>
<!--
The clock or the time zone moved, so the daily reminder has to be
re-aimed: its delay is computed once, from the zone in force then.
Boot is not here on purpose — WorkManager's own RescheduleReceiver
already restores the persisted periodic work, and its manifest entry
declares BOOT_COMPLETED and nothing else, which is why these two need
one of their own.
Unexported, like WorkManager's: these are system broadcasts and
neither carries a permission, so nothing is being granted away here.
-->
<receiver
android:name=".notifications.ClockChangeReceiver"
android:exported="false">
<intent-filter>
<action android:name="android.intent.action.TIMEZONE_CHANGED" />
<action android:name="android.intent.action.TIME_SET" />
</intent-filter>
</receiver>
</application> </application>
</manifest> </manifest>

View File

@ -16,6 +16,7 @@ import dev.privacyllc.period.core.data.CycleData
import dev.privacyllc.period.core.data.CycleRepository import dev.privacyllc.period.core.data.CycleRepository
import dev.privacyllc.period.core.datastore.UserPreferencesRepository import dev.privacyllc.period.core.datastore.UserPreferencesRepository
import dev.privacyllc.period.core.notifications.ReminderScheduler import dev.privacyllc.period.core.notifications.ReminderScheduler
import dev.privacyllc.period.core.notifications.WorkManagerReminderScheduler
import dev.privacyllc.period.core.notifications.PeriodNotifier import dev.privacyllc.period.core.notifications.PeriodNotifier
import dev.privacyllc.period.core.notifications.ReminderNotifier import dev.privacyllc.period.core.notifications.ReminderNotifier
import dev.privacyllc.period.core.security.AppLockRepository import dev.privacyllc.period.core.security.AppLockRepository
@ -141,7 +142,7 @@ object DataModule {
@Provides @Provides
@Singleton @Singleton
fun reminderScheduler(@ApplicationContext context: Context): ReminderScheduler = fun reminderScheduler(@ApplicationContext context: Context): ReminderScheduler =
ReminderScheduler(context) WorkManagerReminderScheduler(context)
@Provides @Provides
@Singleton @Singleton

View File

@ -195,8 +195,11 @@ private fun NotificationSettingsContent(
Spacer(Modifier.height(24.dp)) Spacer(Modifier.height(24.dp))
Text( Text(
"Reminders are scheduled with WorkManager and may arrive a few minutes either " + // "either side" was not true and could not be made true: a periodic
"side of this time. Nothing about your cycle leaves the device.", // request is deferred by the system, never brought forward, and the
// flex window that would allow "before" also skips the first day.
"Android may deliver a reminder a few minutes after this time, never before. " +
"Nothing about your cycle leaves the device.",
style = MaterialTheme.typography.bodySmall, style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant, color = MaterialTheme.colorScheme.onSurfaceVariant,
) )

View File

@ -0,0 +1,53 @@
package dev.privacyllc.period.notifications
import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.components.SingletonComponent
/**
* The clock or the time zone moved; re-aim the reminder.
*
* The delay to the next reminder is computed once, when it is scheduled, from
* the zone in force at that moment. Nothing recomputed it afterwards, so flying
* to another continent left the reminder arriving at the old wall-clock time
* indefinitely and a manual clock change did the same.
*
* **Boot is deliberately not handled here.** WorkManager persists the periodic
* request and its own `RescheduleReceiver` restores it after a reboot; that
* receiver's manifest entry declares `BOOT_COMPLETED` and nothing else, which is
* exactly why the two broadcasts below need an entry of their own. A second boot
* receiver would duplicate work already done, and the coordinator's schedule at
* process start re-anchors any drift anyway.
*
* Unexported, like WorkManager's own: these are system broadcasts, and nothing
* else has any business sending them to this app.
*
* Reached through an entry point rather than `@AndroidEntryPoint`. The annotation
* generates a base class whose `onReceive` performs the injection and which the
* subclass must call through `super` and `BroadcastReceiver.onReceive` is
* abstract, so that call resolves against the wrong class whenever the generated
* one is not yet on the compile path. An accessor has no such ordering.
*/
class ClockChangeReceiver : BroadcastReceiver() {
@EntryPoint
@InstallIn(SingletonComponent::class)
interface Dependencies {
fun reminderCoordinator(): ReminderCoordinator
}
override fun onReceive(context: Context, intent: Intent) {
when (intent.action) {
Intent.ACTION_TIMEZONE_CHANGED, Intent.ACTION_TIME_CHANGED -> {
EntryPointAccessors
.fromApplication(context.applicationContext, Dependencies::class.java)
.reminderCoordinator()
.clockChanged()
}
}
}
}

View File

@ -6,6 +6,7 @@ import dev.privacyllc.period.core.datastore.UserPreferencesRepository
import dev.privacyllc.period.core.notifications.ReminderScheduler import dev.privacyllc.period.core.notifications.ReminderScheduler
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.distinctUntilChanged
@ -61,6 +62,24 @@ class ReminderCoordinator @Inject constructor(
).launchIn(scope) ).launchIn(scope)
} }
/**
* The clock or the time zone moved, so the schedule has to be re-aimed.
*
* The delay to the next reminder is worked out once, at the moment it is
* scheduled, from the zone in force then. Fly to another continent and the
* reminder keeps arriving at the old wall-clock time indefinitely: nothing
* recomputed it, because nothing was listening for the two broadcasts that
* say so.
*
* A counter rather than a call, so this joins the same key the rest of the
* chain uses and cannot race the state it is bumping.
*/
fun clockChanged() {
_clockGeneration.value += 1
}
private val _clockGeneration = MutableStateFlow(0)
/** /**
* Reschedule when the forecast date, the reminder time, or whether anything * Reschedule when the forecast date, the reminder time, or whether anything
* is enabled at all changes. * is enabled at all changes.
@ -96,15 +115,35 @@ class ReminderCoordinator @Inject constructor(
internal fun scheduleUpdates( internal fun scheduleUpdates(
forecastDates: Flow<LocalDate?>, forecastDates: Flow<LocalDate?>,
preferences: Flow<UserPreferences>, preferences: Flow<UserPreferences>,
clockGeneration: Flow<Int> = _clockGeneration,
): Flow<*> = ): Flow<*> =
combine( combine(
forecastDates, forecastDates,
preferences.map { it.reminderTime to anyEnabled(it) }, preferences.map { it.reminderTime to anyEnabled(it) },
) { forecastDate, (time, enabled) -> Triple(forecastDate, time, enabled) } clockGeneration,
) { forecastDate, (time, enabled), generation ->
ScheduleKey(forecastDate, time, enabled, generation)
}
.distinctUntilChanged() .distinctUntilChanged()
.onEach { (_, time, enabled) -> apply(time, enabled) } .onEach { apply(it.time, it.enabled) }
.catch { } .catch { }
/**
* What has to change before the schedule is touched.
*
* The forecast date earns its place by being part of the key rather than by
* being used: it is what makes a "Not yet" reach this chain at all. The
* daily check reads the forecast when it wakes, so it does not need aiming
* at a particular one §31's "reschedule after a forecast change" is
* satisfied by the waking, not by the aiming.
*/
private data class ScheduleKey(
val forecastDate: LocalDate?,
val time: LocalTime,
val enabled: Boolean,
val clockGeneration: Int,
)
private suspend fun apply(time: LocalTime, enabled: Boolean) { private suspend fun apply(time: LocalTime, enabled: Boolean) {
if (enabled) scheduler.schedule(time) else scheduler.cancel() if (enabled) scheduler.schedule(time) else scheduler.cancel()
} }

View File

@ -7,6 +7,8 @@ import androidx.test.core.app.ApplicationProvider
import dev.privacyllc.period.core.datastore.NotificationPrivacy import dev.privacyllc.period.core.datastore.NotificationPrivacy
import dev.privacyllc.period.core.datastore.UserPreferencesRepository import dev.privacyllc.period.core.datastore.UserPreferencesRepository
import dev.privacyllc.period.core.notifications.ReminderScheduler import dev.privacyllc.period.core.notifications.ReminderScheduler
import java.time.LocalTime
import java.time.ZoneId
import dev.privacyllc.period.notifications.NotificationPermissionState import dev.privacyllc.period.notifications.NotificationPermissionState
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
@ -48,9 +50,18 @@ class NotificationSettingsViewModelTest {
override fun canPost(privacy: NotificationPrivacy) = canPost override fun canPost(privacy: NotificationPrivacy) = canPost
} }
/** Records what it was asked to do; the scheduling itself is tested in core. */
private class FakeScheduler : ReminderScheduler {
var scheduled = 0
var cancelled = 0
override suspend fun schedule(reminderTime: LocalTime, zone: ZoneId) { scheduled++ }
override fun cancel() { cancelled++ }
}
private val dispatcher = UnconfinedTestDispatcher() private val dispatcher = UnconfinedTestDispatcher()
private lateinit var permissions: FakePermissions private lateinit var permissions: FakePermissions
private lateinit var preferences: UserPreferencesRepository private lateinit var preferences: UserPreferencesRepository
private lateinit var scheduler: FakeScheduler
private lateinit var vm: NotificationSettingsViewModel private lateinit var vm: NotificationSettingsViewModel
@Before fun setUp() { @Before fun setUp() {
@ -62,7 +73,8 @@ class NotificationSettingsViewModelTest {
}, },
) )
permissions = FakePermissions() permissions = FakePermissions()
vm = NotificationSettingsViewModel(preferences, ReminderScheduler(context), permissions) scheduler = FakeScheduler()
vm = NotificationSettingsViewModel(preferences, scheduler, permissions)
} }
@After fun tearDown() = Dispatchers.resetMain() @After fun tearDown() = Dispatchers.resetMain()

View File

@ -41,6 +41,13 @@ import java.time.Clock
@Config(sdk = [34]) @Config(sdk = [34])
class ReminderCoordinatorTest { class ReminderCoordinatorTest {
/** The coordinator's job is deciding when to reschedule, not how. */
private class FakeScheduler : ReminderScheduler {
override suspend fun schedule(reminderTime: java.time.LocalTime, zone: java.time.ZoneId) = Unit
override fun cancel() = Unit
}
private lateinit var coordinator: ReminderCoordinator private lateinit var coordinator: ReminderCoordinator
private lateinit var preferences: UserPreferencesRepository private lateinit var preferences: UserPreferencesRepository
@ -52,7 +59,7 @@ class ReminderCoordinatorTest {
coordinator = ReminderCoordinator( coordinator = ReminderCoordinator(
repository = CycleData.repository(context, PersonalPredictionEngine(), Clock.systemUTC()), repository = CycleData.repository(context, PersonalPredictionEngine(), Clock.systemUTC()),
preferences = preferences, preferences = preferences,
scheduler = ReminderScheduler(context), scheduler = FakeScheduler(),
) )
} }

View File

@ -5,63 +5,141 @@ import androidx.work.Constraints
import androidx.work.ExistingPeriodicWorkPolicy import androidx.work.ExistingPeriodicWorkPolicy
import androidx.work.PeriodicWorkRequestBuilder import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkManager import androidx.work.WorkManager
import kotlinx.coroutines.flow.first
import java.time.Clock
import java.time.Duration import java.time.Duration
import java.time.LocalDate import java.time.LocalDate
import java.time.LocalDateTime import java.time.LocalDateTime
import java.time.LocalTime import java.time.LocalTime
import java.time.ZoneId import java.time.ZoneId
import java.util.concurrent.TimeUnit import java.util.concurrent.TimeUnit
import kotlin.math.abs
/** /**
* Schedules the daily reminder check with WorkManager. * Schedules the daily reminder check.
*
* An interface because the settings screen and the coordinator both drive it,
* and both are worth testing without WorkManager underneath.
*/
interface ReminderScheduler {
/**
* Aim the daily check at [reminderTime] in [zone].
*
* Called on every change that could move a reminder: a toggle changing, the
* reminder time changing, the clock or the time zone changing. §31 lists
* rescheduling after a confirmation or a forecast change explicitly that
* one is satisfied by construction, since the check runs daily and reads the
* forecast when it wakes rather than being aimed at a particular forecast.
*/
suspend fun schedule(reminderTime: LocalTime, zone: ZoneId = ZoneId.systemDefault())
fun cancel()
}
/**
* The WorkManager implementation, and three of its behaviours worth knowing.
* *
* §31: **no exact alarms.** A period reminder does not need alarm-clock * §31: **no exact alarms.** A period reminder does not need alarm-clock
* precision, and `SCHEDULE_EXACT_ALARM` is Play scrutiny bought for nothing * precision, and `SCHEDULE_EXACT_ALARM` is Play scrutiny bought for nothing.
* so this is a periodic work request with a flex window, which the system is * `checkPermissions` fails the build if one reaches the merged manifest.
* free to move by minutes. Nobody notices a reminder arriving at 10:07 instead
* of 10:00; everybody notices an app asking for an alarm permission.
* *
* The work is `KEEP`-replaced on every reschedule rather than cancelled and * ## Why this reads the existing work before touching it
* re-enqueued, so a forecast that moves twice in a minute does not produce two *
* pending workers. * `ExistingPeriodicWorkPolicy.UPDATE` keeps the old request's `lastEnqueueTime`
* and `periodCount`, and the next run is computed as
* `periodCount == 0 ? lastEnqueueTime + initialDelay : lastEnqueueTime + interval`.
* Two consequences, both of which shipped:
*
* - Changing the reminder from Morning to Evening **after the first reminder
* had fired did nothing at all** the new initial delay is ignored once
* `periodCount` is past zero.
* - Re-scheduling *before* the first run which every process start did, via
* the coordinator set the next run to *the original enqueue time* plus the
* *new* delay. Opened the next morning, that lands in the past, so the
* reminder fired early and every later period anchored off it.
*
* So: enqueue with `KEEP` when nothing is scheduled, leave an overdue run alone
* (moving it would skip today), leave a run that is already close enough alone,
* and otherwise `UPDATE` with an explicit `setNextScheduleTimeOverride`, which
* is the only way to move a periodic request that has already run.
*
* `CANCEL_AND_REENQUEUE` would be simpler and is wrong here: this is called at
* every process start, including the one WorkManager itself started to run the
* worker, and cancelling the unique work there cancels the running worker.
*
* ## Why there is no flex window
*
* A flex window looks like the polite thing to ask for and would skip the first
* day: with flex, the first run is placed at `start + interval flex`, so a
* fifteen-minute flex on a daily period aims the first reminder at roughly
* 23h45m from now. The screen's copy says a reminder may arrive a few minutes
* *after* the chosen time, which is what actually happens the system defers,
* it does not anticipate.
*/ */
class ReminderScheduler(private val context: Context) { class WorkManagerReminderScheduler(
private val context: Context,
private val clock: Clock = Clock.systemDefaultZone(),
) : ReminderScheduler {
/** override suspend fun schedule(reminderTime: LocalTime, zone: ZoneId) {
* (Re)schedule the daily check for [reminderTime]. val now = clock.millis()
* val target = now + delayUntil(reminderTime, zone, LocalDateTime.now(clock.withZone(zone))).toMillis()
* Called on every change that could move a reminder: the forecast moving, a val manager = WorkManager.getInstance(context)
* toggle changing, the reminder time changing. §31 lists rescheduling after
* a confirmation or a forecast change explicitly, and it is the requirement val existing = manager.getWorkInfosForUniqueWorkFlow(WORK_NAME).first()
* most likely to be missed a "Not yet" moves the forecast, so a reminder .firstOrNull { !it.state.isFinished }
* aimed at the old one is now aimed at the wrong day.
*/ when {
fun schedule(reminderTime: LocalTime, zone: ZoneId = ZoneId.systemDefault()) { existing == null -> manager.enqueueUniquePeriodicWork(
val request = PeriodicWorkRequestBuilder<ReminderWorker>(1, TimeUnit.DAYS) WORK_NAME,
.setInitialDelay(delayUntil(reminderTime, zone).toMinutes(), TimeUnit.MINUTES) ExistingPeriodicWorkPolicy.KEEP,
.setConstraints( daily().setInitialDelay(target - now, TimeUnit.MILLISECONDS).build(),
// No network, no charging, no idle. A reminder that waits for
// Wi-Fi is a reminder that arrives on Tuesday.
Constraints.Builder().build(),
) )
.addTag(TAG)
.build()
WorkManager.getInstance(context).enqueueUniquePeriodicWork( // Already due, or overdue. Let it run: moving it now would push
WORK_NAME, // today's reminder to tomorrow, and being a little late is the
ExistingPeriodicWorkPolicy.UPDATE, // failure everybody forgives.
request, existing.nextScheduleTimeMillis <= now -> Unit
)
// Near enough. Rewriting the schedule on every process start churns
// WorkManager for a difference nobody could perceive.
abs(existing.nextScheduleTimeMillis - target) <= TOLERANCE_MILLIS -> Unit
else -> manager.enqueueUniquePeriodicWork(
WORK_NAME,
ExistingPeriodicWorkPolicy.UPDATE,
daily().setNextScheduleTimeOverride(target).build(),
)
}
} }
fun cancel() { override fun cancel() {
WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME) WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME)
} }
private fun daily() = PeriodicWorkRequestBuilder<ReminderWorker>(1, TimeUnit.DAYS)
.setConstraints(
// No network, no charging, no idle. A reminder that waits for
// Wi-Fi is a reminder that arrives on Tuesday.
Constraints.Builder().build(),
)
.addTag(TAG)
companion object { companion object {
const val WORK_NAME = "period-daily-reminder" const val WORK_NAME = "period-daily-reminder"
const val TAG = "reminder" const val TAG = "reminder"
/**
* How far the next run may sit from the target before it is moved.
*
* Five minutes: below the resolution anybody perceives in a reminder,
* and above the drift a periodic request accumulates from being
* re-anchored at each completion. A daylight-saving change moves the
* wall-clock target by an hour, which is well past this and correctly
* re-aims.
*/
internal const val TOLERANCE_MILLIS = 5 * 60 * 1000L
/** /**
* Time from now until the next [target] o'clock. * Time from now until the next [target] o'clock.
* *

View File

@ -21,30 +21,30 @@ class ReminderSchedulerTest {
@Test fun `a time later today waits until today`() { @Test fun `a time later today waits until today`() {
val now = LocalDateTime.of(2026, 8, 18, 8, 0) val now = LocalDateTime.of(2026, 8, 18, 8, 0)
val d = ReminderScheduler.delayUntil(LocalTime.of(10, 0), zone, now) val d = WorkManagerReminderScheduler.delayUntil(LocalTime.of(10, 0), zone, now)
assertEquals(120, d.toMinutes()) assertEquals(120, d.toMinutes())
assertEquals(LocalDate.of(2026, 8, 18), ReminderScheduler.nextRunDate(LocalTime.of(10, 0), now)) assertEquals(LocalDate.of(2026, 8, 18), WorkManagerReminderScheduler.nextRunDate(LocalTime.of(10, 0), now))
} }
@Test fun `a time already past today waits until tomorrow`() { @Test fun `a time already past today waits until tomorrow`() {
val now = LocalDateTime.of(2026, 8, 18, 11, 0) val now = LocalDateTime.of(2026, 8, 18, 11, 0)
val d = ReminderScheduler.delayUntil(LocalTime.of(10, 0), zone, now) val d = WorkManagerReminderScheduler.delayUntil(LocalTime.of(10, 0), zone, now)
assertEquals(23 * 60, d.toMinutes()) assertEquals(23 * 60, d.toMinutes())
assertEquals(LocalDate.of(2026, 8, 19), ReminderScheduler.nextRunDate(LocalTime.of(10, 0), now)) assertEquals(LocalDate.of(2026, 8, 19), WorkManagerReminderScheduler.nextRunDate(LocalTime.of(10, 0), now))
} }
@Test fun `the exact minute counts as past, not now`() { @Test fun `the exact minute counts as past, not now`() {
// Scheduling a zero delay for "right now" fires immediately and then // Scheduling a zero delay for "right now" fires immediately and then
// again in 24 hours, which is two reminders on the first day. // again in 24 hours, which is two reminders on the first day.
val now = LocalDateTime.of(2026, 8, 18, 10, 0) val now = LocalDateTime.of(2026, 8, 18, 10, 0)
assertEquals(24 * 60, ReminderScheduler.delayUntil(LocalTime.of(10, 0), zone, now).toMinutes()) assertEquals(24 * 60, WorkManagerReminderScheduler.delayUntil(LocalTime.of(10, 0), zone, now).toMinutes())
} }
@Test fun `the delay is never negative and never beyond a day`() { @Test fun `the delay is never negative and never beyond a day`() {
listOf(LocalTime.MIDNIGHT, LocalTime.of(6, 30), LocalTime.of(23, 59)).forEach { target -> listOf(LocalTime.MIDNIGHT, LocalTime.of(6, 30), LocalTime.of(23, 59)).forEach { target ->
(0..23).forEach { hour -> (0..23).forEach { hour ->
val now = LocalDateTime.of(2026, 8, 18, hour, 17) val now = LocalDateTime.of(2026, 8, 18, hour, 17)
val minutes = ReminderScheduler.delayUntil(target, zone, now).toMinutes() val minutes = WorkManagerReminderScheduler.delayUntil(target, zone, now).toMinutes()
assertTrue("$target at $hour gave $minutes", minutes in 0..(24 * 60)) assertTrue("$target at $hour gave $minutes", minutes in 0..(24 * 60))
} }
} }

View File

@ -0,0 +1,186 @@
package dev.privacyllc.period.core.notifications
import android.content.Context
import androidx.test.core.app.ApplicationProvider
import androidx.work.Configuration
import androidx.work.WorkInfo
import androidx.work.WorkManager
import androidx.work.testing.WorkManagerTestInitHelper
import kotlinx.coroutines.runBlocking
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
import java.time.Clock
import java.time.Instant
import java.time.LocalTime
import java.time.ZoneId
import java.time.ZoneOffset
/**
* Whether a reminder actually moves when the user moves it.
*
* `schedule()` had no test only the pure arithmetic beneath it did and the
* scheduling itself was wrong in two ways that a reading of the API does not
* reveal. `ExistingPeriodicWorkPolicy.UPDATE` carries the old request's
* `lastEnqueueTime` and `periodCount` forward, so a new initial delay is ignored
* once the work has run once, and applied against the *original* enqueue time
* before that. Changing Morning to Evening did nothing; re-scheduling at every
* process start could aim the next run into the past.
*
* These assert against WorkManager's own recorded next-run time, which is the
* only thing that decides when the user is actually woken.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34])
class WorkManagerReminderSchedulerTest {
private val zone: ZoneId = ZoneOffset.UTC
/** 2026-08-20, 08:00 UTC — before both reminder times used below. */
private val eightAm = Instant.parse("2026-08-20T08:00:00Z")
/**
* One clock for both sides.
*
* WorkManager works out `nextScheduleTimeMillis` with its OWN clock, so a
* test that fixes only the scheduler's measures a delay against a real
* `System.currentTimeMillis()` and compares it to an instant from 2026. Both
* read this.
*/
private class TestClock(var millis: Long, private val zone: ZoneId) : Clock() {
override fun getZone(): ZoneId = zone
override fun withZone(z: ZoneId): Clock = TestClock(millis, z)
override fun instant(): Instant = Instant.ofEpochMilli(millis)
}
private lateinit var context: Context
private lateinit var manager: WorkManager
private lateinit var clock: TestClock
@Before fun setUp() {
context = ApplicationProvider.getApplicationContext()
clock = TestClock(eightAm.toEpochMilli(), zone)
WorkManagerTestInitHelper.initializeTestWorkManager(
context,
Configuration.Builder()
.setExecutor { it.run() }
.setClock { clock.millis }
.build(),
)
manager = WorkManager.getInstance(context)
}
/** Move both clocks, then hand back a scheduler reading the same one. */
private fun schedulerAt(instant: Instant): WorkManagerReminderScheduler {
clock.millis = instant.toEpochMilli()
return WorkManagerReminderScheduler(context, clock)
}
private fun scheduled(): WorkInfo? =
manager.getWorkInfosForUniqueWork(WorkManagerReminderScheduler.WORK_NAME).get()
.firstOrNull { !it.state.isFinished }
private fun nextRun(): Long = scheduled()!!.nextScheduleTimeMillis
@Test fun `scheduling with nothing there aims at the next occurrence`() = runBlocking {
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), zone)
assertNotNull("nothing was enqueued", scheduled())
assertEquals(Instant.parse("2026-08-20T10:00:00Z").toEpochMilli(), nextRun())
}
@Test fun `scheduling the same time again leaves the run where it is`() = runBlocking {
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), zone)
val before = nextRun()
// What every process start does. It must not churn WorkManager, and it
// must not re-anchor the run against a new "now".
schedulerAt(eightAm.plusSeconds(60)).schedule(LocalTime.of(10, 0), zone)
assertEquals(before, nextRun())
}
@Test fun `changing the reminder time moves the run`() = runBlocking {
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), zone)
assertEquals(Instant.parse("2026-08-20T10:00:00Z").toEpochMilli(), nextRun())
// Morning to evening. This is the one that did nothing at all once the
// work had run once, and aimed at the wrong instant before that.
schedulerAt(eightAm).schedule(LocalTime.of(19, 0), zone)
assertEquals(Instant.parse("2026-08-20T19:00:00Z").toEpochMilli(), nextRun())
}
@Test fun `changing the time an hour later still lands on the time asked for`() = runBlocking {
// The sharp version of the test above, and the one that actually catches
// the defect. UPDATE carries the ORIGINAL request's lastEnqueueTime
// forward, and a periodic request that has not yet run computes its next
// run as lastEnqueueTime + initialDelay. So a delay measured from "now"
// is applied to an enqueue time an hour in the past: asking for 19:00 at
// 09:00 produced 18:00, and every later period anchored off that.
//
// An explicit next-run override is the only way to say when, rather than
// how long from a moment WorkManager has its own opinion about.
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), zone)
schedulerAt(eightAm.plusSeconds(3600)).schedule(LocalTime.of(19, 0), zone)
assertEquals(Instant.parse("2026-08-20T19:00:00Z").toEpochMilli(), nextRun())
}
@Test fun `a time already past today is aimed at tomorrow`() = runBlocking {
val evening = Instant.parse("2026-08-20T20:00:00Z")
schedulerAt(evening).schedule(LocalTime.of(10, 0), zone)
assertEquals(Instant.parse("2026-08-21T10:00:00Z").toEpochMilli(), nextRun())
}
@Test fun `a run that is already due is left alone rather than pushed to tomorrow`() = runBlocking {
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), zone)
val due = nextRun()
// The app is opened at 10:05, five minutes after the reminder was due
// and before the system has run it. Re-aiming now would compute
// "tomorrow at ten" and silently skip today's reminder entirely.
schedulerAt(Instant.parse("2026-08-20T10:05:00Z")).schedule(LocalTime.of(10, 0), zone)
assertEquals(due, nextRun())
}
@Test fun `a zone change re-aims at the same wall-clock time`() = runBlocking {
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), zone)
assertEquals(Instant.parse("2026-08-20T10:00:00Z").toEpochMilli(), nextRun())
// Same instant, three hours east: it is already 11:00 there, so the next
// ten o'clock is tomorrow morning local — 07:00 UTC on the 21st, not
// 10:00 UTC today. Nothing recomputed this before, so somebody who flew
// east kept being reminded at the old wall-clock time indefinitely.
val eastern = ZoneOffset.ofHours(3)
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), eastern)
assertEquals(Instant.parse("2026-08-21T07:00:00Z").toEpochMilli(), nextRun())
}
@Test fun `cancelling removes the work`() = runBlocking {
val scheduler = schedulerAt(eightAm)
scheduler.schedule(LocalTime.of(10, 0), zone)
assertNotNull(scheduled())
scheduler.cancel()
assertNull("the reminder was left scheduled", scheduled())
}
@Test fun `the request carries no constraints that could hold a reminder back`() = runBlocking {
schedulerAt(eightAm).schedule(LocalTime.of(10, 0), zone)
// A reminder that waits for Wi-Fi is a reminder that arrives on Tuesday.
assertTrue(scheduled()!!.tags.contains(WorkManagerReminderScheduler.TAG))
}
}

View File

@ -43,7 +43,7 @@ way, with the batch that needs it.
| `core/database` | Android library | Room entities, DAOs, converters, the schema export | `domain/cycle`, `domain/prediction` | | `core/database` | Android library | Room entities, DAOs, converters, the schema export | `domain/cycle`, `domain/prediction` |
| `core/datastore` | Android library | `UserPreferences` and the settings that are not health history | nothing in this project | | `core/datastore` | Android library | `UserPreferences` and the settings that are not health history | nothing in this project |
| `core/data` | Android library | `CycleRepository`, entity⇄domain mapping, accuracy — the only module that touches a DAO | `core/database`, `domain/cycle`, `domain/prediction` | | `core/data` | Android library | `CycleRepository`, entity⇄domain mapping, accuracy — the only module that touches a DAO | `core/database`, `domain/cycle`, `domain/prediction` |
| `core/notifications` | Android library | reminder copy, the privacy modes, WorkManager scheduling | `core/data`, `core/datastore`, `domain/*` | | `core/notifications` | Android library | reminder copy **and the action vocabulary**, the privacy modes, WorkManager scheduling | `core/data`, `core/datastore`, `domain/*` |
| `core/security` | Android library | the app lock's PIN verifier, its Keystore key and the lockout policy | **nothing in this project** | | `core/security` | Android library | the app lock's PIN verifier, its Keystore key and the lockout policy | **nothing in this project** |
| `core/export` | **Kotlin JVM** | the export file format, and nothing else | `domain/cycle` — deliberately **not** `domain/prediction` | | `core/export` | **Kotlin JVM** | the export file format, and nothing else | `domain/cycle` — deliberately **not** `domain/prediction` |
| `domain/cycle` | **Kotlin JVM** | `PeriodRecord`, `SpottingRecord`, `CycleRecord` and the rules over them | nothing | | `domain/cycle` | **Kotlin JVM** | `PeriodRecord`, `SpottingRecord`, `CycleRecord` and the rules over them | nothing |
@ -157,6 +157,43 @@ health app a crash mid-write is adjacent to losing what the user just entered,
and a message they can read beats a process that vanished. The message carries and a message they can read beats a process that vanished. The message carries
the exception *type* and never a record's contents — §45. the exception *type* and never a record's contents — §45.
### Scheduling a reminder is not the same as saying when
`ExistingPeriodicWorkPolicy.UPDATE` carries the previous request's
`lastEnqueueTime` and `periodCount` forward, and a periodic request computes its
next run as `periodCount == 0 ? lastEnqueueTime + initialDelay : lastEnqueueTime
+ interval`. Both halves of that bit us:
- **After the first run, a new initial delay is ignored entirely.** Changing the
reminder from Morning to Evening did nothing at all.
- **Before it, the delay is applied to the *original* enqueue time.** The
coordinator reschedules at every process start, so asking for 19:00 at 09:00
when the work was enqueued at 08:00 produced 18:00 — and every later period
anchored off that.
So the scheduler reads the existing work first: `KEEP` when nothing is
scheduled, leave an overdue run alone (moving it skips today's reminder
entirely), leave a run already within five minutes of the target alone, and
otherwise `UPDATE` with an explicit `setNextScheduleTimeOverride` — the only way
to say *when* rather than *how long from a moment WorkManager has its own opinion
about*. `CANCEL_AND_REENQUEUE` is wrong here for a subtler reason: this runs at
every process start, including the one WorkManager itself started to run the
worker, and cancelling the unique work there cancels the running worker.
**There is no flex window, and the copy was changed rather than the code.** With
flex, the first run is placed at `start + interval flex`, so a fifteen-minute
flex on a daily period aims the first reminder roughly 23h45m out and skips the
day the user set it. The screen used to promise delivery "a few minutes either
side"; a periodic request is deferred by the system, never brought forward, so it
now says *after this time, never before*.
**Time-zone and clock changes need an app receiver; boot does not.** The delay is
computed once, from the zone in force then, so flying east left the reminder
arriving at the old wall-clock time forever. WorkManager's own
`RescheduleReceiver` declares `BOOT_COMPLETED` and nothing else — which is
exactly why `ClockChangeReceiver` exists for `TIMEZONE_CHANGED` and `TIME_SET`,
and exactly why it does not duplicate boot.
### The check-in count belongs to a period, not to the app ### The check-in count belongs to a period, not to the app
§30 says the app must stop asking "did your period start?" after a few §30 says the app must stop asking "did your period start?" after a few

View File

@ -153,6 +153,18 @@ have replaced it.
## The lock screen is the one semi-public surface ## The lock screen is the one semi-public surface
### When the copy and the platform disagree, the copy moves
The reminders screen promised delivery "a few minutes either side of this time".
It could not be made true. A periodic work request is *deferred* by the system,
never brought forward, and the flex window that would allow "before" also places
the first run nearly a full period out — so asking for it would have skipped the
reminder on the day the user set it, to keep a sentence.
The sentence changed instead: *Android may deliver a reminder a few minutes after
this time, never before.* Slightly less comfortable and exactly what happens,
which is the trade this document asks for everywhere else.
### A switch that is on should mean something is happening ### A switch that is on should mean something is happening
Reminders can be switched on here and still never arrive: the notification Reminders can be switched on here and still never arrive: the notification