fix(reminders): arm reminders again in our own store
Regression from deleting the provider. sync() gated on providerResolver.resolve() != null, and OWN resolves to no provider by design — so from that commit no due reminder was ever armed in what had just become the default mode, and clearAll() cancelled any that survived the upgrade. The gate is now ProviderResolver.canReadStore(): OWN is always readable, and only EXTERNAL can fail, for the two reasons it ever could. Putting the decision on the resolver rather than inside the scheduler is what makes it testable at all — ReminderScheduler needs Context and AlarmManager, which is why nothing caught this. Also brings ARCHITECTURE.md and ROADMAP.md in line with the branch: one module, OWN/EXTERNAL, the four Room tables, expansion at read time, the import and startup gate, and the manifest surface that no longer declares a provider or any permission of its own.
This commit is contained in:
@@ -17,10 +17,11 @@ import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
/**
|
||||
* The self-scheduled due-reminder engine. Tasks providers don't deliver
|
||||
* reminders, so Agendula reads upcoming due tasks and arms one exact [AlarmManager]
|
||||
* alarm each, within a rolling window. Re-run on app start, boot and provider
|
||||
* change; it diffs against [ScheduledReminderStore] so only changed alarms move.
|
||||
* The self-scheduled due-reminder engine. Nothing else delivers task reminders —
|
||||
* not the platform, not a tasks provider — so Agendula reads upcoming due tasks
|
||||
* and arms one exact [AlarmManager] alarm each, within a rolling window. Re-run
|
||||
* on app start, on boot, and on an external provider change; it diffs against
|
||||
* [ScheduledReminderStore] so only changed alarms move.
|
||||
*/
|
||||
@Singleton
|
||||
class ReminderScheduler @Inject constructor(
|
||||
@@ -32,9 +33,11 @@ class ReminderScheduler @Inject constructor(
|
||||
@IoDispatcher private val io: CoroutineDispatcher,
|
||||
) {
|
||||
suspend fun sync() = withContext(io) {
|
||||
val provider = providerResolver.resolve()
|
||||
val settings = settingsPrefs.settings.first()
|
||||
if (provider == null || !providerResolver.hasPermission(provider) || !settings.remindersEnabled) {
|
||||
// Gate on whether the store is readable, not on whether a provider
|
||||
// resolves: our own store deliberately resolves to no provider, so the
|
||||
// latter clears every reminder in the default mode.
|
||||
if (!settings.remindersEnabled || !providerResolver.canReadStore()) {
|
||||
clearAll()
|
||||
return@withContext
|
||||
}
|
||||
@@ -43,12 +46,11 @@ class ReminderScheduler @Inject constructor(
|
||||
val tasks = runCatching { dataSource.tasks(TaskQuery(includeCompleted = false)) }
|
||||
.getOrElse { return@withContext }
|
||||
|
||||
// One reminder per *occurrence*: the instances view yields a row per
|
||||
// One reminder per *occurrence*: a recurring series yields a row per
|
||||
// occurrence, all sharing a taskId, so this is a Set rather than a
|
||||
// taskId-keyed Map — keying by task would collapse a daily recurring task
|
||||
// down to one arbitrary reminder (the query is unsorted, so which one
|
||||
// survived was provider-defined).
|
||||
// Per-task leads, stored as Alarm property rows. One query for all of them.
|
||||
// down to one arbitrary reminder.
|
||||
// Per-task leads. One query for all of them.
|
||||
val perTask = runCatching { dataSource.alarms() }.getOrElse { emptyMap() }
|
||||
|
||||
val desired = tasks
|
||||
|
||||
@@ -89,6 +89,19 @@ class ProviderResolver @Inject constructor(
|
||||
fun hasPermission(provider: TaskProvider): Boolean =
|
||||
environment.isGranted(provider.readPermission) && environment.isGranted(provider.writePermission)
|
||||
|
||||
/**
|
||||
* Whether the active store can be read at all.
|
||||
*
|
||||
* [StorageMode.OWN] always can — it is our own database, with nothing to
|
||||
* install and nothing to grant. Only [StorageMode.EXTERNAL] can be
|
||||
* unreadable. Callers that gate on `resolve() != null` instead get this wrong
|
||||
* the moment OWN is active, because OWN resolves to no provider by design.
|
||||
*/
|
||||
fun canReadStore(): Boolean = when (mode()) {
|
||||
StorageMode.OWN -> true
|
||||
StorageMode.EXTERNAL -> resolveExternal()?.let(::hasPermission) == true
|
||||
}
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* Verified on-device: tasks.org exposes `org.tasks.opentasks` backed by
|
||||
|
||||
@@ -39,6 +39,14 @@ class ProviderResolverTest {
|
||||
@Nested
|
||||
inner class OwnStore {
|
||||
|
||||
@Test
|
||||
fun `is readable without anything installed or granted`() {
|
||||
// Guards a real regression: the reminder engine used to gate on
|
||||
// resolve() != null, which is exactly what OWN returns, so every
|
||||
// reminder was cleared the moment our own store became the default.
|
||||
assertThat(resolver(mode = StorageMode.OWN).canReadStore()).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `resolves to no provider at all`() {
|
||||
// Room has no authority and no ContentResolver, so there is nothing
|
||||
@@ -108,6 +116,21 @@ class ProviderResolverTest {
|
||||
assertThat(resolver(mode = StorageMode.EXTERNAL).resolve()).isNull()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `external is unreadable until a provider is installed and granted`() {
|
||||
assertThat(resolver(mode = StorageMode.EXTERNAL).canReadStore()).isFalse()
|
||||
assertThat(
|
||||
resolver(installed = openTasksInstalled, mode = StorageMode.EXTERNAL).canReadStore(),
|
||||
).isFalse()
|
||||
assertThat(
|
||||
resolver(
|
||||
installed = openTasksInstalled,
|
||||
granted = openTasksGranted,
|
||||
mode = StorageMode.EXTERNAL,
|
||||
).canReadStore(),
|
||||
).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `external still requires the runtime permission`() {
|
||||
val resolver = resolver(installed = openTasksInstalled, mode = StorageMode.EXTERNAL)
|
||||
|
||||
Reference in New Issue
Block a user