diff --git a/CHANGELOG.md b/CHANGELOG.md index 543bf84..c1c1940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- Reminders no longer depend on Android telling Calendula when they are due. + Calendula now works out each reminder's time itself and sets its own alarm for + it. On some phones — Samsung's among them — the system's calendar storage never + sends the signal a calendar app is meant to wake up on, and no amount of + battery or notification settings helps: the reminder is simply never announced. + None of that is visible from inside an app that waits to be told, which is why + it took a second pass to find ([#75]). + + Reminders also survive things that used to lose them quietly. After a restart + or an app update Calendula re-arms its alarms, and a reminder whose moment + passed while the phone was off still arrives, as long as the event has not + ended yet. + +- All-day reminders now arrive at the time you chose in **Settings → + Notifications**, on every occurrence. A yearly birthday could drift an hour + either way depending on daylight saving, and all-day reminders on calendars + from an account fired in the middle of the night instead of in the morning + ([#75]). + - Reminders now arrive for every calendar you have switched on. A calendar that was hidden at system level — switched off in another calendar app, or never switched on after being added — still showed its events and listed their diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 8e0df89..1db01ea 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -33,6 +33,14 @@ android:maxSdkVersion="32" /> + + + + - - + + + + diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt index ba988f6..55c2595 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt @@ -8,6 +8,8 @@ import de.jeanlucmakiola.calendula.data.calendar.CalendarVisibilityReconciler import de.jeanlucmakiola.calendula.data.backup.BackupWorker import de.jeanlucmakiola.calendula.data.contacts.SpecialDatesScheduler import de.jeanlucmakiola.calendula.data.contacts.SpecialDatesSyncWorker +import de.jeanlucmakiola.calendula.data.reminders.ReminderMaintenanceScheduler +import de.jeanlucmakiola.calendula.data.reminders.ReminderMaintenanceWorker import de.jeanlucmakiola.floret.crash.CrashConfig import de.jeanlucmakiola.floret.crash.CrashReporter import kotlinx.coroutines.CoroutineScope @@ -41,6 +43,25 @@ class CalendulaApp : Application() { reconcileAutoBackup() reconcileSpecialDates() reconcileCalendarVisibility() + startReminderDelivery() + } + + /** + * Bring reminder delivery up with the process (#75). The app plans and arms + * its own reminder alarms now, so launch is one of the moments that has to + * re-check them: a scan re-arms whatever the system dropped, posts anything + * a missed alarm still owes, and starts watching the provider so an edit + * re-plans without waiting for the next pass. The daily worker is the + * backstop for a device that drops the alarm with no reboot to announce it. + */ + private fun startReminderDelivery() { + val deps = EntryPointAccessors.fromApplication( + this, ReminderMaintenanceWorker.Deps::class.java, + ) + val scanner = deps.reminderScanner() + scanner.startWatchingProvider() + scanner.scanInBackground() + ReminderMaintenanceScheduler.apply(this) } /** diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/di/DataModule.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/di/DataModule.kt index 8b001d4..963f11c 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/di/DataModule.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/di/DataModule.kt @@ -18,8 +18,8 @@ import de.jeanlucmakiola.calendula.data.contacts.AndroidContactSpecialDatesDataS import de.jeanlucmakiola.calendula.data.contacts.AndroidSpecialDatesCalendarSpec import de.jeanlucmakiola.calendula.data.contacts.ContactSpecialDatesDataSource import de.jeanlucmakiola.calendula.data.contacts.SpecialDatesCalendarSpec -import de.jeanlucmakiola.calendula.data.reminders.AndroidReminderAlertStore -import de.jeanlucmakiola.calendula.data.reminders.ReminderAlertStore +import de.jeanlucmakiola.calendula.data.reminders.ProviderReminderInstanceSource +import de.jeanlucmakiola.calendula.data.reminders.ReminderInstanceSource import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import javax.inject.Singleton @@ -46,9 +46,9 @@ abstract class DataBindModule { @Binds @Singleton - abstract fun bindReminderAlertStore( - impl: AndroidReminderAlertStore, - ): ReminderAlertStore + abstract fun bindReminderInstanceSource( + impl: ProviderReminderInstanceSource, + ): ReminderInstanceSource @Binds @Singleton diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/ReminderStatePrefs.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/ReminderStatePrefs.kt new file mode 100644 index 0000000..053a2e5 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/ReminderStatePrefs.kt @@ -0,0 +1,42 @@ +package de.jeanlucmakiola.calendula.data.prefs + +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.longPreferencesKey +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map +import javax.inject.Inject +import javax.inject.Singleton + +/** + * How far reminder delivery has got. One number, and it replaces everything the + * provider's `CalendarAlerts.STATE` used to do for us (#75). + * + * A scan posts the reminders whose moment falls after this watermark and up to + * now, then moves it to now. That single rule gives both halves of what the + * retired path got from the provider: a scan that runs twice cannot post the + * same reminder again, and a scan that runs *late* — after a reboot, an app + * update or a doze window swallowed the alarm — still posts everything the + * missed alarm would have. + * + * Unset means "never scanned". It is deliberately not treated as zero: the first + * scan after an install or an upgrade would otherwise consider every reminder + * since the epoch overdue and bury the user in notifications. + */ +@Singleton +class ReminderStatePrefs @Inject constructor( + private val store: DataStore, +) { + + /** The watermark, or `null` before the first scan has ever run. */ + suspend fun lastScanMillis(): Long? = store.data.map { it[LAST_SCAN_KEY] }.first() + + suspend fun setLastScanMillis(millis: Long) { + store.edit { prefs -> prefs[LAST_SCAN_KEY] = millis } + } + + private companion object { + val LAST_SCAN_KEY = longPreferencesKey("reminder_last_scan_millis") + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/AlertHandling.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/AlertHandling.kt deleted file mode 100644 index 9d6cf57..0000000 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/AlertHandling.kt +++ /dev/null @@ -1,30 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -/** - * Still relevant while the event has not ended: a reminder for an event that is - * already over is pointless to re-surface. Falls back to the begin time when the - * end is unknown (0L). - */ -internal fun ReminderAlert.isRelevantAt(nowMillis: Long): Boolean = - (endMillis.takeIf { it > 0L } ?: beginMillis) >= nowMillis - -/** - * The alerts [EventReminderReceiver] may mark handled (`STATE_FIRED`): the ones - * it posted, plus the ones it silenced whose event is already over. - * - * A silenced alert for an event still ahead is deliberately left - * `STATE_SCHEDULED`. Silencing is not handling — the calendar is switched off in - * Calendula while the provider still holds `VISIBLE = 1` (a read-only install, - * or an upgrade whose flush hasn't landed), so switching it back on before the - * event must still be able to surface the reminder. [ReminderAlertStore.dueAlerts] - * only ever returns scheduled rows, so marking them here would lose them for - * good; leaving them makes the provider's own table the stash - * ([ReminderRecovery]). - */ -internal fun handledAlertIds( - due: List, - postedIds: Set, - nowMillis: Long, -): List = due - .filter { it.alertId in postedIds || !it.isRelevantAt(nowMillis) } - .map { it.alertId } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/EventReminderReceiver.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/EventReminderReceiver.kt deleted file mode 100644 index 95067e3..0000000 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/EventReminderReceiver.kt +++ /dev/null @@ -1,67 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -import android.Manifest -import android.content.BroadcastReceiver -import android.content.Context -import android.content.Intent -import android.content.pm.PackageManager -import android.provider.CalendarContract -import androidx.core.content.ContextCompat -import dagger.hilt.android.AndroidEntryPoint -import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.flow.first -import kotlinx.coroutines.launch -import javax.inject.Inject - -/** - * Becomes the app that turns the calendar provider's reminder alarms into - * visible notifications (the Etar model — the provider broadcasts - * `EVENT_REMINDER` at reminder time but posts nothing itself). - * - * The broadcast's data URI only carries the alarm time, so it is ignored: - * we query every still-scheduled, due `CalendarAlerts` row ourselves, post - * them, and mark them fired. Posting happens before marking — a crash in - * between re-posts silently (same tag) rather than losing the reminder. - * - * There is no per-calendar filtering here: a calendar switched off in - * Settings → Calendars has `Calendars.VISIBLE = 0`, and the provider creates no - * alert rows for it in the first place (#75). The one case that flag can't - * cover — a read-only install, which keeps its switches app-side — is gated in - * [ReminderNotifier.post], where the snoozed re-show passes too. What that gate - * silences is *not* marked fired while the event is still ahead, so switching - * the calendar back on can still surface it (see [handledAlertIds]). - */ -@AndroidEntryPoint -class EventReminderReceiver : BroadcastReceiver() { - - @Inject lateinit var alertStore: ReminderAlertStore - @Inject lateinit var notifier: ReminderNotifier - @Inject lateinit var settingsPrefs: SettingsPrefs - - override fun onReceive(context: Context, intent: Intent) { - if (intent.action != CalendarContract.ACTION_EVENT_REMINDER) return - val readGranted = ContextCompat.checkSelfPermission( - context, Manifest.permission.READ_CALENDAR, - ) == PackageManager.PERMISSION_GRANTED - if (!readGranted || !notifier.canPost()) return - - val pendingResult = goAsync() - CoroutineScope(SupervisorJob() + Dispatchers.IO).launch { - try { - if (settingsPrefs.remindersEnabled.first()) { - val now = System.currentTimeMillis() - val due = alertStore.dueAlerts(now) - val postedIds = due - .filter { notifier.post(it) } - .mapTo(mutableSetOf()) { it.alertId } - alertStore.markFired(handledAlertIds(due, postedIds, now), now) - } - } finally { - pendingResult.finish() - } - } - } -} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt index bbe8ea3..53c883b 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt @@ -18,14 +18,14 @@ import javax.inject.Inject * intents (notification action buttons and our own [ReminderSnoozeScheduler] * alarm), so the receiver is not exported. * - * - **Dismiss** just cancels the notification — the `CalendarAlerts` row is - * already fired, so nothing re-posts it. + * - **Dismiss** just cancels the notification — the scan's watermark has moved + * past this reminder, so nothing re-posts it. * - **Snooze** cancels the notification and schedules an exact alarm to re-show * it after the user's snooze delay. * - **Show** (the alarm) re-posts the same notification, so the user can snooze * or dismiss it again — unless the calendar was switched off during the - * snooze, which [ReminderNotifier.post] catches (this alarm is ours, so no - * provider alert row stands between it and the notification). + * snooze, which [ReminderNotifier.post] catches — this alarm is its own + * trigger, outside the ordinary scan. */ @AndroidEntryPoint class ReminderActionReceiver : BroadcastReceiver() { @@ -75,7 +75,14 @@ class ReminderActionReceiver : BroadcastReceiver() { const val ACTION_DISMISS = "de.jeanlucmakiola.calendula.reminders.DISMISS" const val ACTION_SHOW = "de.jeanlucmakiola.calendula.reminders.SHOW" - private const val EXTRA_ALERT_ID = "alert_id" + /** + * Not handled here — the notification body opens the detail screen + * directly. It only claims a slot in [requestCode] so that intent stays + * distinct from the three this receiver does handle. + */ + const val ACTION_OPEN = "de.jeanlucmakiola.calendula.reminders.OPEN" + + private const val EXTRA_ALERT_KEY = "alert_key" private const val EXTRA_EVENT_ID = "event_id" private const val EXTRA_CALENDAR_ID = "calendar_id" private const val EXTRA_BEGIN = "begin" @@ -88,7 +95,7 @@ class ReminderActionReceiver : BroadcastReceiver() { fun intent(context: Context, action: String, alert: ReminderAlert): Intent = Intent(context, ReminderActionReceiver::class.java).apply { this.action = action - putExtra(EXTRA_ALERT_ID, alert.alertId) + putExtra(EXTRA_ALERT_KEY, alert.key) putExtra(EXTRA_EVENT_ID, alert.eventId) putExtra(EXTRA_CALENDAR_ID, alert.calendarId) putExtra(EXTRA_BEGIN, alert.beginMillis) @@ -99,23 +106,29 @@ class ReminderActionReceiver : BroadcastReceiver() { } /** - * A stable request code per (alert, action) so the three PendingIntents - * of one notification stay distinct and don't clobber each other. + * A stable request code per (alert, action) so one notification's + * PendingIntents stay distinct and don't clobber each other. + * + * The key is a hash now rather than a small row id, so the shift is what + * keeps the action slot intact; the top three bits it drops only make two + * *different* reminders collide, which the intent extras then separate + * (PendingIntents compare their intents too, not just the request code). */ fun requestCode(alert: ReminderAlert, action: String): Int { val actionOffset = when (action) { ACTION_SNOOZE -> 1 ACTION_DISMISS -> 2 ACTION_SHOW -> 3 + ACTION_OPEN -> 4 else -> 0 } - return alert.alertId.toInt() * 8 + actionOffset + return (alert.key.toInt() shl 3) + actionOffset } private fun alertFrom(intent: Intent): ReminderAlert? { - if (!intent.hasExtra(EXTRA_ALERT_ID)) return null + if (!intent.hasExtra(EXTRA_ALERT_KEY)) return null return ReminderAlert( - alertId = intent.getLongExtra(EXTRA_ALERT_ID, 0L), + key = intent.getLongExtra(EXTRA_ALERT_KEY, 0L), eventId = intent.getLongExtra(EXTRA_EVENT_ID, 0L), calendarId = intent.getLongExtra(EXTRA_CALENDAR_ID, 0L), beginMillis = intent.getLongExtra(EXTRA_BEGIN, 0L), diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlarms.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlarms.kt new file mode 100644 index 0000000..89a4593 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlarms.kt @@ -0,0 +1,71 @@ +package de.jeanlucmakiola.calendula.data.reminders + +import android.app.AlarmManager +import android.app.PendingIntent +import android.content.Context +import android.content.Intent +import android.os.Build +import androidx.core.content.getSystemService +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject +import javax.inject.Singleton + +/** + * True on API < 31 (no restriction), and on 31+ when the exact-alarm capability + * is held — auto-granted via `USE_EXACT_ALARM` on API 33+ (Calendula is a + * calendar app), user-revocable on 31–32. + */ +internal fun AlarmManager.canScheduleExactCompat(): Boolean = + Build.VERSION.SDK_INT < Build.VERSION_CODES.S || canScheduleExactAlarms() + +/** + * Holds the app's own wake-up for the next reminder — the half of delivery that + * used to be the provider's (#75). + * + * Exactly **one** alarm exists at a time, for the earliest reminder still ahead. + * Every firing re-scans and re-arms, so a reminder added, moved or deleted in + * between is picked up on the next pass instead of needing an alarm per reminder + * to be kept in sync with the provider's tables. + * + * A reminder that lands late is a broken reminder, hence an *exact* alarm; the + * inexact allow-while-idle fallback only applies where the OS withholds the + * capability (API 31–32 with the user's permission revoked). + */ +@Singleton +class ReminderAlarmScheduler @Inject constructor( + @ApplicationContext private val context: Context, +) { + + fun scheduleScan(triggerAtMillis: Long) { + val alarmManager = context.getSystemService() ?: return + val pendingIntent = scanPendingIntent(PendingIntent.FLAG_UPDATE_CURRENT) + if (alarmManager.canScheduleExactCompat()) { + alarmManager.setExactAndAllowWhileIdle( + AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent, + ) + } else { + alarmManager.setAndAllowWhileIdle( + AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent, + ) + } + } + + /** Drop the pending wake-up — reminders are off, or there is nothing to wait for. */ + fun cancelScan() { + val alarmManager = context.getSystemService() ?: return + alarmManager.cancel(scanPendingIntent(PendingIntent.FLAG_UPDATE_CURRENT)) + } + + private fun scanPendingIntent(flags: Int): PendingIntent = PendingIntent.getBroadcast( + context, + SCAN_REQUEST_CODE, + Intent(context, ReminderScheduleReceiver::class.java) + .setAction(ReminderScheduleReceiver.ACTION_SCAN), + flags or PendingIntent.FLAG_IMMUTABLE, + ) + + private companion object { + // Fixed: there is only ever one scan alarm, and re-arming must replace it. + const val SCAN_REQUEST_CODE = 0x5CA1 + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlert.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlert.kt new file mode 100644 index 0000000..2d8c28b --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlert.kt @@ -0,0 +1,37 @@ +package de.jeanlucmakiola.calendula.data.reminders + +import de.jeanlucmakiola.calendula.domain.reminders.PlannedReminder + +/** + * One reminder as the notification layer needs it: what to show, and the stable + * [key] that identifies it across a reboot, a re-scan and a reinstall. + * + * [key] used to be the `CalendarAlerts` row id. It is now derived from the + * reminder itself (see [PlannedReminder.key]) because there is no row any more — + * in-house delivery reads `Instances` and `Reminders` and owns the alarm (#75). + * Everything downstream only ever needed it to be stable and unique, which it + * still is: it keys the notification tag, so a reminder posted twice replaces + * itself instead of stacking, and it keys the snooze/dismiss `PendingIntent`s. + */ +data class ReminderAlert( + val key: Long, + val eventId: Long, + val calendarId: Long, + val beginMillis: Long, + val endMillis: Long, + /** Raw event title; may be blank — the notifier substitutes "(no title)". */ + val title: String, + val location: String?, + val isAllDay: Boolean, +) + +fun PlannedReminder.toAlert(): ReminderAlert = ReminderAlert( + key = key, + eventId = instance.eventId, + calendarId = instance.calendarId, + beginMillis = instance.beginMillis, + endMillis = instance.endMillis, + title = instance.title, + location = instance.location, + isAllDay = instance.isAllDay, +) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlertStore.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlertStore.kt deleted file mode 100644 index 54f68dc..0000000 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlertStore.kt +++ /dev/null @@ -1,115 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -import android.content.ContentValues -import android.content.Context -import android.provider.CalendarContract -import android.util.Log -import dagger.hilt.android.qualifiers.ApplicationContext -import javax.inject.Inject -import javax.inject.Singleton - -/** - * One due row of the provider's `CalendarAlerts` table (a join with Events). - * Stays in the data layer: alerts feed the notification path only and never - * reach a screen, so there is no domain model for them. - */ -data class ReminderAlert( - val alertId: Long, - val eventId: Long, - val calendarId: Long, - val beginMillis: Long, - val endMillis: Long, - /** Raw event title; may be blank — the notifier substitutes "(no title)". */ - val title: String, - val location: String?, - val isAllDay: Boolean, -) - -/** - * Seam over the `CalendarAlerts` table so the receiver logic can be exercised - * without a ContentResolver. The provider creates these rows itself — only - * for `METHOD_ALERT` reminders (verified in AOSP `CalendarAlarmManager`), so - * email reminders never show up here. - */ -interface ReminderAlertStore { - - /** Alerts that are due (`ALARM_TIME` has passed) and still unhandled. */ - fun dueAlerts(nowMillis: Long): List - - /** - * Mark the given alerts handled (`STATE_FIRED`) so a later broadcast does - * not surface them again. Best effort: this write needs `WRITE_CALENDAR`, - * which the user may have declined — then re-broadcasts silently replace - * the already-posted notifications instead (same tag, alert-once). - */ - fun markFired(alertIds: List, nowMillis: Long) -} - -@Singleton -class AndroidReminderAlertStore @Inject constructor( - @ApplicationContext private val context: Context, -) : ReminderAlertStore { - - override fun dueAlerts(nowMillis: Long): List = context.contentResolver.query( - CalendarContract.CalendarAlerts.CONTENT_URI, - PROJECTION, - CalendarContract.CalendarAlerts.STATE + " = ? AND " + - CalendarContract.CalendarAlerts.ALARM_TIME + " <= ?", - arrayOf( - CalendarContract.CalendarAlerts.STATE_SCHEDULED.toString(), - nowMillis.toString(), - ), - CalendarContract.CalendarAlerts.BEGIN + " ASC", - )?.use { c -> - buildList { - while (c.moveToNext()) { - add( - ReminderAlert( - alertId = c.getLong(0), - eventId = c.getLong(1), - calendarId = c.getLong(2), - beginMillis = c.getLong(3), - endMillis = c.getLong(4), - title = c.getString(5).orEmpty(), - location = c.getString(6)?.takeIf { it.isNotBlank() }, - isAllDay = c.getInt(7) == 1, - ), - ) - } - } - } ?: emptyList() - - override fun markFired(alertIds: List, nowMillis: Long) { - if (alertIds.isEmpty()) return - val values = ContentValues().apply { - put(CalendarContract.CalendarAlerts.STATE, CalendarContract.CalendarAlerts.STATE_FIRED) - put(CalendarContract.CalendarAlerts.RECEIVED_TIME, nowMillis) - put(CalendarContract.CalendarAlerts.NOTIFY_TIME, nowMillis) - } - try { - context.contentResolver.update( - CalendarContract.CalendarAlerts.CONTENT_URI, - values, - CalendarContract.CalendarAlerts._ID + - " IN (" + alertIds.joinToString(",") + ")", - null, - ) - } catch (e: SecurityException) { - Log.w(TAG, "Cannot mark alerts fired without WRITE_CALENDAR", e) - } - } - - private companion object { - const val TAG = "ReminderAlertStore" - val PROJECTION = arrayOf( - CalendarContract.CalendarAlerts._ID, - CalendarContract.CalendarAlerts.EVENT_ID, - CalendarContract.CalendarAlerts.CALENDAR_ID, - CalendarContract.CalendarAlerts.BEGIN, - CalendarContract.CalendarAlerts.END, - CalendarContract.CalendarAlerts.TITLE, - CalendarContract.CalendarAlerts.EVENT_LOCATION, - CalendarContract.CalendarAlerts.ALL_DAY, - ) - } -} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderInstanceSource.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderInstanceSource.kt new file mode 100644 index 0000000..7ab93ee --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderInstanceSource.kt @@ -0,0 +1,128 @@ +package de.jeanlucmakiola.calendula.data.reminders + +import android.content.Context +import android.content.ContentUris +import android.provider.CalendarContract +import dagger.hilt.android.qualifiers.ApplicationContext +import de.jeanlucmakiola.calendula.domain.reminders.ReminderEventInstance +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The read side of in-house reminder delivery: occurrences and the reminder + * offsets hanging off them, straight out of the provider's own tables. + * + * Deliberately *not* `CalendarAlerts`. That table is the provider's own working + * copy of this same information, and the whole point of #75's second round is + * that we can no longer assume it gets written. `Instances` and `Reminders` are + * plain data the app already reads everywhere else. + * + * An interface so [ReminderScanner] can be exercised on the JVM without a + * ContentResolver, in the shape the rest of `data/calendar` uses. + */ +interface ReminderInstanceSource { + + /** Occurrences overlapping `[fromMillis, toMillis]`, of switched-on calendars. */ + fun occurrences(fromMillis: Long, toMillis: Long): List + + /** `METHOD_ALERT` reminder offsets per event id, for the given events. */ + fun reminderMinutes(eventIds: Collection): Map> + + /** + * The largest `METHOD_ALERT` offset anywhere in the table, so the query + * window can be stretched to cover it and a long-lead reminder is planned + * before it comes due rather than firing late. + */ + fun longestReminderMinutes(): Int +} + +@Singleton +class ProviderReminderInstanceSource @Inject constructor( + @ApplicationContext private val context: Context, +) : ReminderInstanceSource { + + override fun occurrences(fromMillis: Long, toMillis: Long): List { + val uri = CalendarContract.Instances.CONTENT_URI.buildUpon().apply { + ContentUris.appendId(this, fromMillis) + ContentUris.appendId(this, toMillis) + }.build() + // `visible` is the calendar's system flag, which the app's one visibility + // model writes (#75) — a switched-off calendar must not plan reminders. + // The status clause mirrors CalendarDataSource.instances: a cancelled + // single occurrence of a series is a real row, and NULL means "normal", + // so a bare `!= CANCELED` would drop every ordinary event. + val selection = "${CalendarContract.Calendars.VISIBLE} = 1 AND " + + "(${CalendarContract.Instances.STATUS} IS NULL OR " + + "${CalendarContract.Instances.STATUS} != ${CalendarContract.Events.STATUS_CANCELED})" + return context.contentResolver.query( + uri, OCCURRENCE_PROJECTION, selection, null, null, + )?.use { c -> + buildList { + while (c.moveToNext()) { + add( + ReminderEventInstance( + eventId = c.getLong(0), + calendarId = c.getLong(1), + beginMillis = c.getLong(2), + endMillis = if (c.isNull(3)) 0L else c.getLong(3), + title = c.getString(4).orEmpty(), + location = c.getString(5)?.takeIf { it.isNotBlank() }, + isAllDay = c.getInt(6) == 1, + ), + ) + } + } + } ?: emptyList() + } + + override fun reminderMinutes(eventIds: Collection): Map> { + if (eventIds.isEmpty()) return emptyMap() + val out = mutableMapOf>() + // Batched because the ids go into the selection literally; an unbounded + // `IN (...)` on a busy calendar would grow the SQL past what SQLite takes. + eventIds.distinct().chunked(EVENT_ID_BATCH).forEach { batch -> + context.contentResolver.query( + CalendarContract.Reminders.CONTENT_URI, + REMINDER_PROJECTION, + "${CalendarContract.Reminders.METHOD} = " + + "${CalendarContract.Reminders.METHOD_ALERT} AND " + + "${CalendarContract.Reminders.EVENT_ID} IN (${batch.joinToString(",")})", + null, + null, + )?.use { c -> + while (c.moveToNext()) { + out.getOrPut(c.getLong(0)) { mutableListOf() } += c.getInt(1) + } + } + } + return out + } + + override fun longestReminderMinutes(): Int = context.contentResolver.query( + CalendarContract.Reminders.CONTENT_URI, + arrayOf(CalendarContract.Reminders.MINUTES), + "${CalendarContract.Reminders.METHOD} = ${CalendarContract.Reminders.METHOD_ALERT}", + null, + // One row is enough: the provider passes the sort order to SQLite. + "${CalendarContract.Reminders.MINUTES} DESC", + )?.use { c -> if (c.moveToFirst()) c.getInt(0) else 0 } ?: 0 + + private companion object { + const val EVENT_ID_BATCH = 50 + + val OCCURRENCE_PROJECTION = arrayOf( + CalendarContract.Instances.EVENT_ID, + CalendarContract.Instances.CALENDAR_ID, + CalendarContract.Instances.BEGIN, + CalendarContract.Instances.END, + CalendarContract.Instances.TITLE, + CalendarContract.Instances.EVENT_LOCATION, + CalendarContract.Instances.ALL_DAY, + ) + + val REMINDER_PROJECTION = arrayOf( + CalendarContract.Reminders.EVENT_ID, + CalendarContract.Reminders.MINUTES, + ) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderMaintenanceWorker.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderMaintenanceWorker.kt new file mode 100644 index 0000000..7f04042 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderMaintenanceWorker.kt @@ -0,0 +1,67 @@ +package de.jeanlucmakiola.calendula.data.reminders + +import android.content.Context +import android.util.Log +import androidx.work.CoroutineWorker +import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.PeriodicWorkRequestBuilder +import androidx.work.WorkManager +import androidx.work.WorkerParameters +import dagger.hilt.EntryPoint +import dagger.hilt.InstallIn +import dagger.hilt.android.EntryPointAccessors +import dagger.hilt.components.SingletonComponent +import java.util.concurrent.TimeUnit + +/** + * The backstop under the alarm: a daily scan that runs whether or not the alarm + * survived. + * + * The scan alarm re-arms itself at most a day out, so in the steady state this + * finds nothing to do. It exists for the case the whole feature is about — a + * device that quietly drops the alarm without a reboot to announce it. The + * scheduling half of reminder delivery must not have a single point of failure, + * which is exactly what the provider's broadcast turned out to be. + */ +object ReminderMaintenanceScheduler { + + private const val WORK_NAME = "reminder-scan-maintenance" + + /** Enqueue the daily backstop; idempotent, so every launch may call it. */ + fun apply(context: Context) { + val request = PeriodicWorkRequestBuilder(1, TimeUnit.DAYS) + // The launch scan covers now; let the first periodic run wait. + .setInitialDelay(1, TimeUnit.DAYS) + .build() + WorkManager.getInstance(context) + .enqueueUniquePeriodicWork(WORK_NAME, ExistingPeriodicWorkPolicy.UPDATE, request) + } +} + +class ReminderMaintenanceWorker( + appContext: Context, + params: WorkerParameters, +) : CoroutineWorker(appContext, params) { + + @EntryPoint + @InstallIn(SingletonComponent::class) + interface Deps { + fun reminderScanner(): ReminderScanner + } + + override suspend fun doWork(): Result = try { + EntryPointAccessors.fromApplication(applicationContext, Deps::class.java) + .reminderScanner() + .scan() + Result.success() + } catch (e: Exception) { + // The scan swallows its own failures; anything reaching here is the + // entry point itself, which a retry will not mend. Never fail the chain. + Log.w(TAG, "Reminder maintenance scan failed", e) + Result.success() + } + + private companion object { + const val TAG = "ReminderMaintenance" + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt index 0b16714..7e512db 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt @@ -29,11 +29,13 @@ import javax.inject.Inject import javax.inject.Singleton /** - * Posts one notification per due reminder alert on a dedicated channel. - * Tapping opens the event's detail screen; the tag is the alert id, so a - * re-broadcast of an alert we couldn't mark fired replaces its notification - * silently ([NotificationCompat.Builder.setOnlyAlertOnce]) instead of - * duplicating it. + * Posts one notification per due reminder on a dedicated channel. Tapping opens + * the event's detail screen. + * + * The tag is the reminder's stable key, so a scan that posts the same reminder + * again — a catch-up pass overlapping the alarm that already fired — replaces + * its notification silently ([NotificationCompat.Builder.setOnlyAlertOnce]) + * instead of stacking a second one. */ @Singleton class ReminderNotifier @Inject constructor( @@ -52,13 +54,12 @@ class ReminderNotifier @Inject constructor( } /** - * The single choke point for "this calendar is switched off". The provider - * side needs no help — with `VISIBLE = 0` it creates no alert rows at all — - * but two paths reach [post] without one: a snooze we re-show from our own - * exact alarm, scheduled before the calendar was switched off, and a - * read-only install whose switch lives app-side ([CalendarPrefs]) because it - * may not write the flag. Both are covered here rather than in either - * receiver. + * The single choke point for "this calendar is switched off". The scan + * already filters on `Calendars.VISIBLE`, but two paths reach [post] around + * it: a snooze re-shown from its own alarm, armed before the calendar was + * switched off, and a read-only install whose switch lives app-side + * ([CalendarPrefs]) because it may not write the flag. Both are covered here + * rather than in either receiver. */ private suspend fun isSilenced(calendarId: Long): Boolean = calendarId in calendarPrefs.pendingDisabledCalendarIds.first() || @@ -66,8 +67,8 @@ class ReminderNotifier @Inject constructor( /** * Post [alert], unless its calendar is switched off. Returns whether the - * notification was put up: a silenced alert must stay unhandled so that - * switching the calendar back on can still surface it (see [handledAlertIds]). + * notification was put up, which the snooze re-show path uses to tell a + * silenced reminder from a delivered one. */ suspend fun post(alert: ReminderAlert): Boolean { if (isSilenced(alert.calendarId)) return false @@ -117,7 +118,7 @@ class ReminderNotifier @Inject constructor( .build() try { NotificationManagerCompat.from(context) - .notify(alert.alertId.toString(), NOTIFICATION_ID, notification) + .notify(alert.key.toString(), NOTIFICATION_ID, notification) } catch (e: SecurityException) { // POST_NOTIFICATIONS was revoked between canPost() and here. Log.w(TAG, "Could not post reminder for event ${alert.eventId}", e) @@ -128,7 +129,7 @@ class ReminderNotifier @Inject constructor( /** Remove a posted reminder (snooze re-shows it later; dismiss is final). */ fun cancel(alert: ReminderAlert) { - NotificationManagerCompat.from(context).cancel(alert.alertId.toString(), NOTIFICATION_ID) + NotificationManagerCompat.from(context).cancel(alert.key.toString(), NOTIFICATION_ID) } private fun actionIntent(alert: ReminderAlert, action: String): PendingIntent = @@ -141,7 +142,11 @@ class ReminderNotifier @Inject constructor( private fun detailIntent(alert: ReminderAlert): PendingIntent = PendingIntent.getActivity( context, - /* requestCode = */ alert.alertId.toInt(), + // Shares the per-(alert, action) request-code scheme with the buttons, so + // the key's wider value range can't collide one notification's intents. + /* requestCode = */ ReminderActionReceiver.requestCode( + alert, ReminderActionReceiver.ACTION_OPEN, + ), MainActivity.eventDetailIntent(context, alert.eventId, alert.beginMillis, alert.endMillis), PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderRecovery.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderRecovery.kt deleted file mode 100644 index 93bf846..0000000 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderRecovery.kt +++ /dev/null @@ -1,46 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -import de.jeanlucmakiola.calendula.data.di.IoDispatcher -import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.flow.first -import kotlinx.coroutines.withContext -import javax.inject.Inject -import javax.inject.Singleton - -/** - * Re-posts the reminders a switched-off calendar silenced, when it is switched - * back on while they still matter. - * - * Only the app-side switch needs this — a read-only install, or an upgrade the - * reconciler hasn't flushed yet. With `Calendars.VISIBLE = 0` the provider - * deletes the calendar's alert rows itself and re-creates them on the way back; - * app-side the rows stay, still `STATE_SCHEDULED`, because - * [EventReminderReceiver] deliberately leaves the ones it silenced unhandled - * (see [handledAlertIds]). So the provider's own table is the stash, and nothing - * is mirrored locally. - * - * Best effort at switch-on time: it mirrors the receiver's gates (reminders on, - * notifications postable) and there is no later re-scan, so an alert left - * unposted because those are closed is simply released. - */ -@Singleton -class ReminderRecovery @Inject constructor( - private val alertStore: ReminderAlertStore, - private val notifier: ReminderNotifier, - private val settingsPrefs: SettingsPrefs, - @IoDispatcher private val io: CoroutineDispatcher, -) { - - suspend fun rePostFor(calendarIds: Collection) = withContext(io) { - if (calendarIds.isEmpty()) return@withContext - if (!settingsPrefs.remindersEnabled.first() || !notifier.canPost()) return@withContext - val now = System.currentTimeMillis() - val ids = calendarIds.toSet() - val recovered = alertStore.dueAlerts(now) - .filter { it.calendarId in ids && it.isRelevantAt(now) } - if (recovered.isEmpty()) return@withContext - val postedIds = recovered.filter { notifier.post(it) }.map { it.alertId } - alertStore.markFired(postedIds, now) - } -} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt new file mode 100644 index 0000000..1522850 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt @@ -0,0 +1,170 @@ +package de.jeanlucmakiola.calendula.data.reminders + +import android.Manifest +import android.content.Context +import android.content.pm.PackageManager +import android.util.Log +import androidx.core.content.ContextCompat +import dagger.hilt.android.qualifiers.ApplicationContext +import de.jeanlucmakiola.calendula.data.calendar.CalendarDataSource +import de.jeanlucmakiola.calendula.data.di.IoDispatcher +import de.jeanlucmakiola.calendula.data.prefs.ReminderStatePrefs +import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs +import de.jeanlucmakiola.calendula.domain.reminders.planReminders +import de.jeanlucmakiola.calendula.domain.reminders.reminderQueryHorizon +import de.jeanlucmakiola.calendula.domain.reminders.reminderWatermark +import de.jeanlucmakiola.calendula.domain.reminders.scheduleReminders +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext +import java.time.ZoneId +import javax.inject.Inject +import javax.inject.Singleton + +/** + * One pass of in-house reminder delivery: read what is planned, post what has + * come due, and arm the next wake-up. + * + * This is the whole loop. Every trigger — the alarm firing, boot, a clock or + * timezone change, an edit landing in the provider, the app starting, the daily + * safety net — runs the same [scan], so there is a single path to reason about + * and no ordering between triggers to get wrong. Re-running it is always safe: + * the watermark in [ReminderStatePrefs] decides what is owed, not the trigger. + */ +@Singleton +class ReminderScanner @Inject constructor( + @ApplicationContext private val context: Context, + private val source: ReminderInstanceSource, + private val calendarDataSource: CalendarDataSource, + private val notifier: ReminderNotifier, + private val alarms: ReminderAlarmScheduler, + private val state: ReminderStatePrefs, + private val settingsPrefs: SettingsPrefs, + @IoDispatcher private val io: kotlinx.coroutines.CoroutineDispatcher, +) { + + // Triggers overlap freely (an alarm during a burst of edits); serialize so + // two passes can't both read the same watermark and post the same reminder. + private val scanLock = Mutex() + + private val scope = CoroutineScope(SupervisorJob() + io) + private val providerChanges = MutableSharedFlow( + replay = 0, extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST, + ) + private var watching = false + + suspend fun scan() = withContext(io) { + scanLock.withLock { + try { + runScan() + } catch (e: SecurityException) { + // The calendar permission was revoked mid-flight. Nothing to + // re-arm and nothing to recover from — the next grant re-scans. + Log.w(TAG, "Reminder scan lacks the calendar permission", e) + } catch (e: Exception) { + Log.w(TAG, "Reminder scan failed", e) + } + } + } + + private suspend fun runScan() { + val now = System.currentTimeMillis() + if (!hasReadCalendar()) return + + if (!settingsPrefs.remindersEnabled.first()) { + // Reminders off: drop the wake-up, but keep the watermark moving so + // switching them back on doesn't replay everything missed meanwhile. + alarms.cancelScan() + state.setLastScanMillis(now) + return + } + + val lookahead = reminderQueryHorizon(LOOKAHEAD_MILLIS, source.longestReminderMinutes()) + // Reach a little into the past as well: an event already under way can + // still owe a reminder (an all-day "at time of event" encodes to a + // negative offset, which fires after begin), and a catch-up pass needs + // to see the occurrences whose moment it missed. + val occurrences = source.occurrences(now - PAST_WINDOW_MILLIS, now + lookahead) + val planned = planReminders( + instances = occurrences, + minutesByEvent = source.reminderMinutes(occurrences.map { it.eventId }), + zone = ZoneId.systemDefault(), + allDayTimeMinutes = settingsPrefs.allDayReminderTimeMinutes.first(), + ) + + val schedule = scheduleReminders( + planned = planned, + lastFiredMillis = reminderWatermark(state.lastScanMillis(), now), + nowMillis = now, + horizonMillis = now + MAX_ALARM_INTERVAL_MILLIS, + ) + + if (notifier.canPost()) { + schedule.due.forEach { notifier.post(it.toAlert()) } + } + // Advance regardless of whether anything could be posted: a user who + // muted notifications is not owed a backlog when they unmute. + state.setLastScanMillis(now) + alarms.scheduleScan(schedule.nextAlarmMillis) + } + + private fun hasReadCalendar(): Boolean = ContextCompat.checkSelfPermission( + context, Manifest.permission.READ_CALENDAR, + ) == PackageManager.PERMISSION_GRANTED + + /** + * Re-scan when the provider changes, so an event saved or deleted in the app + * re-arms the alarm immediately rather than waiting for the next pass. + * + * Debounced: a single save writes the event, its reminders and its + * attendees, and each lands as its own notification. Only useful while the + * process is alive — every other trigger covers the rest, which is why + * nothing here needs to survive it. + */ + fun startWatchingProvider() { + if (watching) return + watching = true + providerChanges + .debounce(PROVIDER_CHANGE_DEBOUNCE_MILLIS) + .onEach { scan() } + .launchIn(scope) + calendarDataSource.registerChangeListener { providerChanges.tryEmit(Unit) } + } + + /** Fire-and-forget scan for callers that are not in a coroutine already. */ + fun scanInBackground() { + scope.launch(start = CoroutineStart.DEFAULT) { scan() } + } + + private companion object { + const val TAG = "ReminderScanner" + + /** + * How far ahead occurrences are read. Stretched further by the longest + * reminder offset in the table, so this is only the floor. + */ + const val LOOKAHEAD_MILLIS = 7L * 24 * 60 * 60 * 1000 + + /** How far back to look for occurrences that may still owe a reminder. */ + const val PAST_WINDOW_MILLIS = 24L * 60 * 60 * 1000 + + /** + * Never wait longer than a day for the next pass, even with nothing + * pending: it rolls the lookahead window forward and re-arms an alarm + * the system may have dropped. + */ + const val MAX_ALARM_INTERVAL_MILLIS = 24L * 60 * 60 * 1000 + + const val PROVIDER_CHANGE_DEBOUNCE_MILLIS = 2_000L + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScheduleReceiver.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScheduleReceiver.kt new file mode 100644 index 0000000..929b0d7 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScheduleReceiver.kt @@ -0,0 +1,63 @@ +package de.jeanlucmakiola.calendula.data.reminders + +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch +import javax.inject.Inject + +/** + * Every reason to re-run a reminder scan that arrives from outside the process. + * + * All of them do the same thing, because [ReminderScanner.scan] is idempotent + * and works out what is owed from its watermark rather than from why it was + * called: + * + * - **our own alarm** ([ACTION_SCAN]) — the ordinary case, a reminder is due; + * - **boot** and **package replaced** — both wipe pending alarms, so the app has + * to re-arm or reminders stop silently, which is the failure #75 is about; + * - **time and timezone changes** — they move every reminder relative to the + * armed alarm, and an all-day reminder's fire hour is recomposed in the + * current zone, so both need a fresh plan. + * + * Exported because the system broadcasts arrive from outside. [ACTION_SCAN] is + * ours and always sent as an explicit intent; another app triggering a scan + * early would only make it re-read the provider and re-arm, which is harmless. + */ +@AndroidEntryPoint +class ReminderScheduleReceiver : BroadcastReceiver() { + + @Inject lateinit var scanner: ReminderScanner + + override fun onReceive(context: Context, intent: Intent) { + // Every action here does the same thing, but the filter still has to be + // checked: the receiver is exported, and the system broadcasts it takes + // are protected, so an intent arriving with any other action did not + // come from where it claims to. + if (intent.action !in HANDLED_ACTIONS) return + val pendingResult = goAsync() + CoroutineScope(SupervisorJob() + Dispatchers.IO).launch { + try { + scanner.scan() + } finally { + pendingResult.finish() + } + } + } + + companion object { + const val ACTION_SCAN = "de.jeanlucmakiola.calendula.reminders.SCAN" + + private val HANDLED_ACTIONS = setOf( + ACTION_SCAN, + Intent.ACTION_BOOT_COMPLETED, + Intent.ACTION_MY_PACKAGE_REPLACED, + Intent.ACTION_TIME_CHANGED, + Intent.ACTION_TIMEZONE_CHANGED, + ) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt index 4bb78f5..ea1ad44 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt @@ -12,9 +12,12 @@ import javax.inject.Singleton /** * Schedules a one-off exact alarm that re-shows a snoozed reminder. * - * The app otherwise relies entirely on the calendar provider's `EVENT_REMINDER` - * broadcast (the Etar model), but a snoozed reminder has no provider backing — - * its `CalendarAlerts` row is already fired — so we must re-fire it ourselves. + * Separate from [ReminderAlarmScheduler]'s scan alarm, and deliberately so: a + * snooze is pinned to one reminder at a time the user chose, while the scan + * alarm is a single moving wake-up for whatever comes next. A re-show also has + * to outlive the scan's watermark moving past that reminder, so it carries the + * reminder in its own intent rather than re-deriving it. + * * A snooze that lands late is a broken snooze, hence an *exact* alarm; we fall * back to an inexact allow-while-idle alarm only if the OS withholds the * exact-alarm capability (API 31–32 where the user revoked it). 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 d0decb5..e098b3f 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 @@ -195,6 +195,20 @@ fun scheduleReminders( fun ReminderEventInstance.isStillRelevant(nowMillis: Long): Boolean = (endMillis.takeIf { it > 0L } ?: beginMillis) >= nowMillis +/** + * The watermark a scan at [nowMillis] should measure against, given what the + * last one recorded. + * + * A first-ever scan ([lastScanMillis] `null`) claims the present, so an install + * or an upgrade onto in-house delivery does not treat every reminder since the + * epoch as overdue and bury the user in notifications. A watermark in the + * *future* — the clock was moved back, or the user travelled across the date + * line — is clamped for the mirror-image reason: left alone it would silence + * every reminder until real time caught up with it. + */ +fun reminderWatermark(lastScanMillis: Long?, nowMillis: Long): Long = + lastScanMillis?.coerceAtMost(nowMillis) ?: nowMillis + /** * How far ahead instances must be queried for [scheduleReminders] to see every * reminder in time: the plain lookahead plus the longest offset any reminder row diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt index 1635054..1d50c78 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt @@ -13,7 +13,6 @@ import de.jeanlucmakiola.calendula.data.di.IoDispatcher import de.jeanlucmakiola.calendula.data.ics.IcsExporter import de.jeanlucmakiola.calendula.data.prefs.BackupStatus import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs -import de.jeanlucmakiola.calendula.data.reminders.ReminderRecovery import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.ics.IcsWriter import kotlinx.coroutines.CoroutineDispatcher @@ -44,7 +43,6 @@ class CalendarsViewModel @Inject constructor( private val repository: CalendarRepository, private val icsExporter: IcsExporter, private val settingsPrefs: SettingsPrefs, - private val reminderRecovery: ReminderRecovery, @IoDispatcher private val io: CoroutineDispatcher, ) : ViewModel() { @@ -128,24 +126,21 @@ class CalendarsViewModel @Inject constructor( * reminders. Nothing is patched by hand — the provider notifies and the * observer re-queries. * - * Switching one back on also re-posts the reminders it silenced while it was - * off and that are still relevant — those the app kept app-side because it - * may not write the flag ([ReminderRecovery]). + * Nothing has to be re-posted on the way back on: reminder delivery plans + * from `Instances` and `Reminders` on every scan (#75), and the provider + * change this write makes triggers one. */ fun setCalendarVisible(id: Long, visible: Boolean) = write { repository.setCalendarsVisible(listOf(id), visible) - if (visible) reminderRecovery.rePostFor(listOf(id)) } /** * Switch every calendar of one account on or off — the "toggle all" - * affordance on an account header. Each row is written on its own (the - * provider only re-arms reminder alarms for a single-id update), in one + * affordance on an account header. Each row is written on its own, in one * coroutine so the writes can't race each other. */ fun setAccountVisible(ids: Collection, visible: Boolean) = write { repository.setCalendarsVisible(ids, visible) - if (visible) reminderRecovery.rePostFor(ids) } // --- Automatic backup (issue #8) ------------------------------------ diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/AlertHandlingTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/AlertHandlingTest.kt deleted file mode 100644 index 9b377e2..0000000 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/AlertHandlingTest.kt +++ /dev/null @@ -1,66 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -import com.google.common.truth.Truth.assertThat -import org.junit.jupiter.api.Test - -/** - * What the reminder receiver may mark handled. The silenced-but-still-ahead case - * is the one that matters: those rows are the only copy of the reminder (#75). - */ -class AlertHandlingTest { - - private val now = 1_700_000_000_000L - - private fun alert( - id: Long, - calendarId: Long = 1L, - beginMillis: Long = now + 60_000L, - endMillis: Long = now + 3_600_000L, - ) = ReminderAlert( - alertId = id, - eventId = id * 10, - calendarId = calendarId, - beginMillis = beginMillis, - endMillis = endMillis, - title = "E $id", - location = null, - isAllDay = false, - ) - - @Test - fun `posted alerts are handled`() { - val due = listOf(alert(1L), alert(2L)) - - assertThat(handledAlertIds(due, postedIds = setOf(1L, 2L), nowMillis = now)) - .containsExactly(1L, 2L) - } - - @Test - fun `a silenced alert whose event is still ahead stays unhandled`() { - // Switching its calendar back on before the event has to bring it back, - // and dueAlerts only ever returns STATE_SCHEDULED rows. - val due = listOf(alert(1L), alert(2L)) - - assertThat(handledAlertIds(due, postedIds = setOf(1L), nowMillis = now)) - .containsExactly(1L) - } - - @Test - fun `a silenced alert whose event is over is handled`() { - // Nothing left to re-surface, so it must not linger as scheduled. - val over = alert(2L, beginMillis = now - 7_200_000L, endMillis = now - 3_600_000L) - val due = listOf(alert(1L), over) - - assertThat(handledAlertIds(due, postedIds = setOf(1L), nowMillis = now)) - .containsExactly(1L, 2L) - } - - @Test - fun `an unknown end time falls back to the begin time`() { - val started = alert(1L, beginMillis = now - 1L, endMillis = 0L) - val notYet = alert(2L, beginMillis = now + 1L, endMillis = 0L) - - assertThat(handledAlertIds(listOf(started, notYet), postedIds = emptySet(), nowMillis = now)) - .containsExactly(1L) - } -} 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 443bbcb..8dcd1c1 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 @@ -437,6 +437,26 @@ class ReminderPlanTest { .isEqualTo(7 * day + 14 * day) } + @Test + fun `a first-ever scan starts from now, not from the epoch`() { + // Otherwise an install — or the upgrade onto in-house delivery — treats + // every reminder ever set as overdue and posts the lot. + assertThat(reminderWatermark(lastScanMillis = null, nowMillis = now)).isEqualTo(now) + } + + @Test + fun `an ordinary watermark is used as recorded`() { + assertThat(reminderWatermark(lastScanMillis = now - day, nowMillis = now)) + .isEqualTo(now - day) + } + + @Test + fun `a watermark left in the future by a clock change is clamped`() { + // Left alone it would silence every reminder until real time caught up. + assertThat(reminderWatermark(lastScanMillis = now + 5 * day, nowMillis = now)) + .isEqualTo(now) + } + @Test fun `a negative longest offset does not shrink the query horizon`() { assertThat(reminderQueryHorizon(lookaheadMillis = 7 * day, maxReminderMinutes = -420)) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 629f57f..d84f2eb 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -33,7 +33,7 @@ flowchart TD Repo["CalendarRepository\n(interface + impl, Flow-based, io-dispatched)"] DS["CalendarDataSource\n(interface + AndroidCalendarDataSource)"] Prefs["SettingsPrefs / CalendarPrefs\n(DataStore)"] - Rem["reminders/\nReminderAlertStore + ReminderNotifier"] + Rem["reminders/\nReminderScanner + ReminderNotifier"] end Provider[("CalendarContract\n(system calendar provider)")] @@ -137,29 +137,56 @@ can't fake a conflict. ## Reminder delivery -The provider schedules reminder alarms (for `METHOD_ALERT` rows only) and -broadcasts `EVENT_REMINDER` — but posts no notification; a calendar app -must (the Etar model): +Calendula plans and fires its own reminders. It reads the offsets in +`Reminders` as data, works out when each occurrence's reminder is due, and holds +**one** exact alarm for the earliest one still ahead: ```mermaid sequenceDiagram - participant P as CalendarProvider - participant R as EventReminderReceiver - participant S as ReminderAlertStore + participant T as Trigger (alarm / boot / time change / edit / launch / daily worker) + participant Sc as ReminderScanner + participant Src as ReminderInstanceSource + participant P as ReminderPlan (pure) participant N as ReminderNotifier - P->>R: EVENT_REMINDER broadcast (manifest receiver, exported) - R->>S: dueAlerts(now) — CalendarAlerts: SCHEDULED, alarmTime ≤ now - S-->>R: due alerts - R->>N: post(alert) — one notification per alert, tag = alert id - R->>S: markFired(ids) — best effort, needs WRITE_CALENDAR + participant A as ReminderAlarmScheduler + T->>Sc: scan() + Sc->>Src: occurrences(window) + reminderMinutes(ids) + Src-->>Sc: Instances ⋈ Reminders + Sc->>P: planReminders / scheduleReminders(watermark, now) + P-->>Sc: due + next alarm + Sc->>N: post(alert) — tag = reminder key + Sc->>A: scheduleScan(next) ``` -Posting happens before marking: a crash in between re-posts silently (same -tag + `setOnlyAlertOnce`) rather than losing a reminder. Swiped -notifications never return because `FIRED` rows are never re-queried. +**Why not the provider's broadcast.** It used to schedule the alarms, write the +`CalendarAlerts` rows and broadcast `EVENT_REMINDER`, and the app only reacted. +That chain holds on stock Android and demonstrably not everywhere: AOSP's own +unbundled calendar carries three separate workarounds for OEM providers that +retarget the broadcast or only write the alert row at alert time. A reacting app +cannot tell "nothing was due" from "the broadcast never came" (#75) — and the +reporter's silent events were in a calendar Calendula created itself, so +`VISIBLE` was never the cause there. -**One visibility model.** The provider only schedules alarms for calendars with -`Calendars.VISIBLE = 1`, so that flag *is* the app's on/off switch: Settings → +**The watermark replaces `CalendarAlerts.STATE`.** A scan posts the reminders +whose moment falls in `(lastScan, now]`, then moves the mark +(`ReminderStatePrefs`). Half-open, so a scan that runs twice cannot post twice, +while a scan that runs *late* still posts what the missed alarm owed — a reboot, +an app update or a doze window costs nothing. A first-ever scan claims the +present rather than the epoch, and a watermark left in the future by a clock +change is clamped. Every trigger runs the same idempotent `scan()`, so there is +no ordering between them to get wrong; `BOOT_COMPLETED` and `MY_PACKAGE_REPLACED` +matter because both wipe pending alarms. + +**All-day reminders fire at the hour the setting names.** The stored offset is +not a plain lead time — `AllDayReminderEncoding` folds a wall-clock hour into it, +sampled against one date's UTC offset — so taking it at face value drifts by the +offset delta across a DST boundary, and rows from other apps carry no hour at +all. The offset is therefore read only for *which day* it means; the hour comes +from the global all-day reminder setting, recomposed against each occurrence's +own date. Timed reminders need none of this: `begin` is an absolute instant. + +**One visibility model.** The scan only plans occurrences of calendars with +`Calendars.VISIBLE = 1`, and that flag *is* the app's on/off switch: Settings → Calendars writes it (one calendar per update — `CalendarProvider2` skips its own `checkNextAlarm()` reschedule for any selection that isn't `_id=`), and every display predicate reads `CalendarSource.isVisibleInSystem`. The reconciliation @@ -171,16 +198,14 @@ the app has not been allowed to write yet (read-only permission grant, or a pre-permission launch); `CalendarVisibilityReconciler` drains it entry by entry, and until it does, the repository and `ReminderNotifier.post` honour it. That gate also covers a snooze re-shown from our own alarm after its calendar was -switched off. Silencing is not handling: an alert the gate drops keeps its -`SCHEDULED` state while its event is still ahead (`handledAlertIds`), so -switching the calendar back on re-posts it (`ReminderRecovery`) instead of -losing it — the provider's own table is the stash. The drawer's filter sheet -(`CalendarPrefs.hiddenCalendarIds`) is a separate in-app declutter that never -touches reminders. +switched off. The drawer's filter sheet (`CalendarPrefs.hiddenCalendarIds`) is a +separate in-app declutter that never touches reminders. -Deliberately absent until real devices prove it necessary: own alarm -scheduling, `BOOT_COMPLETED`, snooze/dismiss actions, battery-exemption -prompts. +Deliberately absent: a fallback to the provider's `EVENT_REMINDER` broadcast. +Keeping both would double-post wherever the provider works, and Etar's way out — +a latch that disables its own scheduling once a real broadcast arrives — cannot +be copied, because our failure mode includes a broadcast that arrives with no +alert row behind it. ## Testing