Files
agendula/docs/OWN-STORE.md
Jean-Luc Makiola 8c3cbcf928 docs: fix seven defects in the own-store plan
Reviewed the plan against the code it describes. Two design holes and
five errors.

Instance identity was the real one. The plan deleted the materialised
instances table without saying what replaces the instance row id, which
TasksRepositoryImpl.updateTask passes to updateInstance and which
ListsScreen keys a lazy list by. Two occurrences of one series can
appear in the same list, so taskId alone is not unique and a hash of
(taskId, start) can collide - as a Compose key that is a visible bug.
Task.id is dropped for occurrenceStart, updateInstance takes
(taskId, occurrenceStart, form), and External mode maps back to a real
instance row with one query. This is the single seam change, and the
plan's "TasksDataSource unchanged" claim was wrong.

Local lists had no account name. TaskList.accountName is non-null,
ListsViewModel groups by it and ListsScreen renders it as a section
header, so a null account_id must still report "Local".

The unique index was wrong: overrides share their master's UID, so
unique (list_id, uid) would reject the rows the recurrence design
depends on. It needs recurrence_id in the key.

Phase 0 broke background reminders. It dropped our authority from
ProviderChangeReceiver's manifest filter while the provider was still
the store, and renamed StorageMode.LOCAL to OWN four phases before OWN
meant Room. Both moved to phase 5.

Parity against the provider was overclaimed: the provider materialises
one occurrence, so multi-occurrence expansion has nothing to compare
against and is tested against RFC 5545 directly.

The phases sum to 6.5-7 weeks, not the 6-6.5 stated, and the difference
from STORAGE-DECISION.md's 4.5-6 is now explained rather than left as a
contradiction.

Gaps closed: WAL vs Auto Backup (checkpoint on ON_STOP, sidecars in the
backup rules, tested in phase 6), cascade rules for master_id and
parent_id, Instant type converters, a rollback path that re-runs the
import from tasks.db.imported, the release note for dropping the
authority and its permissions, and ICalendarWriter.uidFor's synthesis
branch becoming External-only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-13 15:30:37 +02:00

622 lines
29 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Agendula's own task store — architecture and plan
**Branch:** `feat/own-store`
**Decision:** taken. `docs/STORAGE-DECISION.md` costed it; this is the build.
**Supersedes:** the "keep `:provider`" position in `STORAGE-AND-SYNC.md` and the
"Settled" section of `SYNC.md`.
---
## The decision, in one paragraph
Agendula stops vendoring the dmfs OpenTasks provider. The `:provider` module —
14,555 lines of Java, 1.66× the size of the app itself — is **deleted**. In its
place the app gets its own Room database, designed for the two things Agendula
actually does: show tasks, and sync them over CalDAV. Support for *external*
providers (OpenTasks, tasks.org) **stays**, unchanged, as a user choice — so
anyone already syncing through DAVx5 keeps working exactly as they do today.
Agendula becomes a normal Android app with a normal database, plus an optional
compatibility path into somebody else's ContentProvider.
---
## What changes and what does not
```
BEFORE AFTER
UI / ViewModels UI / ViewModels
│ │
TasksRepository TasksRepository ← unchanged
│ │
TasksDataSource (interface) TasksDataSource ← one method
╲ changes
AndroidTasksDataSource RoomTasksDataSource AndroidTasksDataSource
│ │ │
ContentResolver Room / SQLite ContentResolver
│ │ │
┌────┴─────┐ our tables ┌──────┴──────┐
│ │ │ │
:provider OpenTasks OpenTasks tasks.org
(deleted) tasks.org (external, unchanged)
```
**Unchanged above the data layer.** Every ViewModel, every screen. Verified:
exactly one file outside `data/tasks` references `TasksContract`
(`domain/Models.kt`, for five constants), and it stops doing so in phase 0.
Navigation addresses tasks by `taskId` throughout (`Destinations.kt:59`,
`TaskDetailScreen.kt:202,384`) — never by the instance id — so the change below
does not reach the UI.
**One seam method changes.** `TasksDataSource.updateInstance(instanceId, form)`
becomes `updateInstance(taskId, occurrenceStart, form)`, and `Task` gains
`occurrenceStart: Instant?`. See *Instance identity* — this is the one place the
"nothing above the data layer changes" claim needed qualifying, and
`TasksRepositoryImpl.updateTask` is the only caller.
**Deleted.** The `:provider` Gradle module, its manifest `<provider>`, its two
custom permissions, its 84 Java files, its 13 translated string resources, and
its three dmfs runtime dependencies from `:provider`'s own build file.
**Kept for External mode.** `TasksContract.kt`, `ColumnReader.kt`,
`TaskMapper.kt`, `TaskWriteMapper.kt`, `AndroidTasksDataSource.kt`,
`ProviderResolver.kt`, `ProviderEnvironment.kt`, `TaskProjections.kt`. These
describe *somebody else's* schema and are exactly right for that job.
---
## Storage modes after the change
```kotlin
enum class StorageMode {
/** Agendula's own Room database. The default; always available. */
OWN,
/** A tasks provider app already installed — OpenTasks, tasks.org. */
EXTERNAL,
}
```
`LOCAL` (meaning "our bundled dmfs provider") is gone. `ProviderResolver.own`
and the `TaskProvider(isOwn = true)` case go with it: in `OWN` mode there is no
authority, no ContentResolver and no permission to grant.
> **This rename happens in phase 5, not phase 0.** Between phases 1 and 4 both
> stores exist, so the enum carries `LOCAL` (dmfs), `OWN` (Room) and `EXTERNAL`
> simultaneously. Renaming `LOCAL` → `OWN` up front would make `OWN` mean *dmfs*
> for four phases and *Room* afterwards, which is exactly the kind of thing that
> gets misread six weeks later.
`ProviderResolver` narrows to what it was always really for — **discovering
external providers** — and `ProviderStatus.READY` becomes unconditional in `OWN`
mode.
### Consequences worth stating plainly
- **No runtime permission is needed for the default path.** Today's permission
prompt only ever applied to External mode; now that is visibly true.
- **Third-party apps can no longer read Agendula's tasks.** We publish no
ContentProvider. Users who need interop pick External mode, or wait for a
possible read-only facade (explicitly out of scope — see *Deliberately not
doing*).
- **Local lists still report an account name.** `TaskList.accountName` is a
non-null String that `ListsViewModel.kt:98` groups by and
`ListsScreen.kt:222` renders as a section header. `RoomTasksDataSource` maps
`account_id IS NULL` to `accountName = "Local"`, `accountType =
"local"`, so the existing grouping and `TaskList.isLocal` keep working with no
UI change. (`isLocal` moves off `TasksContract.LOCAL_ACCOUNT_TYPE` in phase 0
and compares against a `domain` constant instead.)
Knock-on, benign: `TaskEditViewModel.kt:101` picks the first *non*-local list
as the edit form's default. In `OWN` mode with no account configured every
list is local, so it falls through to `firstOrNull()`. Same practical result,
worth knowing before someone reports it as a bug.
- **Auto Backup gets simpler and safer.** One Room file we control, with a
documented restore path, instead of a provider database whose `cleanUpLists`
routine could delete restored lists whose accounts no longer exist.
⚠️ With one caveat that has to be handled, not assumed away: **Room enables
write-ahead logging by default**, and Auto Backup copies files without
checkpointing. A `-wal` sidecar can hold writes the backed-up `.db` does not.
We checkpoint (`PRAGMA wal_checkpoint(TRUNCATE)`) on `ON_STOP` and include
`.db`, `-wal` and `-shm` together in the backup rules, so a restore is
consistent either way. Phase 6 tests this, because "our backup is safer" is
the kind of claim that is worth exactly as much as its test.
---
## The schema
Four tables. Designed from Agendula's actual reads and writes plus RFC 5545's
`VTODO`, not inherited from a 2013 schema.
### `task_lists`
| Column | Type | Notes |
|---|---|---|
| `id` | INTEGER PK | |
| `name` | TEXT NOT NULL | |
| `color` | INTEGER NOT NULL | ARGB |
| `account_id` | INTEGER NULL | FK → `accounts`, NULL = device-only |
| `is_visible` | INTEGER NOT NULL | default 1 |
| `is_synced` | INTEGER NOT NULL | default 1 |
| `owner` | TEXT NULL | CalDAV owner display name |
| `is_read_only` | INTEGER NOT NULL | **new** — the provider could not express this at all |
| `sort_order` | INTEGER NOT NULL | user ordering, which the provider also lacked |
| `href` | TEXT NULL | collection URL, relative to the account root |
| `ctag` | TEXT NULL | |
| `sync_token` | TEXT NULL | RFC 6578, **per collection** — not squatted into a shared slot |
| `is_dirty` | INTEGER NOT NULL | a real boolean, not dmfs's monotonic counter |
> `account_id` being nullable is the single most important schema change. In the
> dmfs provider `ACCOUNT_TYPE` is write-once and throws on change, which made
> "turn sync on" a full data migration. Here, attaching a local list to an
> account is `UPDATE task_lists SET account_id = ?`.
### `tasks`
Master rows *and* recurrence overrides live here. An override is a row with
`recurrence_id` set and `master_id` pointing at its series master.
> `master_id` and `parent_id` are different things and must not be conflated.
> **`parent_id`** is task hierarchy — a subtask's parent, the thing
> `RELATED-TO;RELTYPE=PARENT` carries. **`master_id`** is recurrence — which
> series an override belongs to. A row can have both: a subtask can itself
> recur.
| Group | Columns |
|---|---|
| identity | `id`, `list_id`, `uid` (NOT NULL, minted at creation), `href`, `etag` |
| content | `title`, `description`, `location`, `url`, `color` |
| state | `status`, `percent_complete`, `completed_at`, `priority`, `classification` |
| time | `dtstart`, `due`, `duration`, `is_all_day`, `timezone` |
| recurrence | `rrule`, `rdate`, `exdate`, `recurrence_id`, `master_id` |
| hierarchy | `parent_id`, `sort_order` |
| audit | `created_at`, `last_modified`, `sequence` |
| sync | `is_dirty`, `is_deleted`, `unknown_properties` |
Two entries deserve explanation.
**`uid` is NOT NULL and assigned at creation.** Every task gets a real
RFC 4122 UUID the moment it is inserted, in every mode, synced or not. This
closes the gap `ICalendarWriter.uidFor` currently papers over by synthesising
`agendula-<rowid>@…`, and it means any local task can later be pushed to a
server without duplicating. The provider never assigned one.
**`unknown_properties`** holds the raw unfolded iCalendar lines of every
property we do not model — `ATTENDEE`, `CATEGORIES`, `X-*`, `GEO`, and anything
a future RFC adds. On write we re-emit them verbatim after the properties we do
own. This is what makes an honest round-trip possible, and it replaces the
provider's `data0``data15` bag with something that cannot silently lose a field
it has no column for.
Indices: `(list_id, is_deleted)`, `(parent_id)`, `(master_id, recurrence_id)`,
`(is_dirty)`, and **unique on `(list_id, uid, recurrence_id)`**.
> The unique index deliberately includes `recurrence_id`. An override **shares
> its master's UID** — that is what makes it an override rather than a separate
> task — so a unique index on `(list_id, uid)` alone would reject the very rows
> the recurrence design depends on. With `recurrence_id` NULL on the master and
> set on each override, the constraint says the right thing: one master and at
> most one override per occurrence, per UID, per list.
**Cascades.** `master_id` is `ON DELETE CASCADE` — deleting a series deletes its
overrides, which would otherwise become unreachable rows that still sync.
`parent_id` is `ON DELETE SET NULL`: deleting a parent promotes its subtasks to
top level rather than destroying work the user did not ask to lose. `task_id` on
`task_alarms` cascades.
**Type converters.** Every time column is `kotlin.time.Instant` in the entity
and INTEGER epoch-millis in SQLite, via one `@TypeConverter` pair. `status` and
`priority` convert through the existing `domain` enums, so `statusFromInt` /
`toInt()` keep their single home.
### `task_alarms`
| Column | Notes |
|---|---|
| `id`, `task_id` | FK, `ON DELETE CASCADE` |
| `minutes_before` | positive = before the reference |
| `reference` | `DUE` or `START` |
| `message` | optional |
Replaces `AlarmHandler` (133 lines of Java) and the `dataN` slot convention.
Delete-and-reinsert stops being necessary — the provider's re-validate-everything
behaviour was the only reason `setAlarm` worked that way.
### `accounts`
| Column | Notes |
|---|---|
| `id`, `display_name`, `principal_url`, `home_set_url` |
| `username` | the app password is **not** here — Keystore only, per `SYNC.md` |
| `last_sync_at`, `last_sync_error` |
Not populated until `SYNC.md` phase 2, but the FK exists from v1 so enabling
sync never requires a schema migration.
---
## Recurrence: expand at read, not on write
The provider maintained a materialised `instances` table, recomputed by
`Instantiating.java` on every write — and still only ever materialised **one**
upcoming occurrence.
Agendula expands lazily instead:
```
tasks (masters + overrides) ──► RecurrenceExpander ──► List<Task>
rrule/rdate/exdate (lib-recur, in memory) occurrences
```
This is the right call here because **the repository already filters and sorts
in Kotlin, not SQL**. `TasksRepositoryImpl.loadTasks` reads the whole set,
applies `TaskFiltering.matches`, then `TaskSorting.DEFAULT`. Nothing depends on
the database being able to order by instance time, so nothing is lost — and a
materialised table's entire class of staleness bugs never exists.
- Bounded window: expansion is capped (default: 1 year back, 2 years forward,
hard ceiling of N occurrences per series) so an unbounded `RRULE` cannot hang
the UI.
- `lib-recur` **pinned at 0.12.2** — 0.16.0 removed `RecurrenceSet`. We pin
because we chose to, and the pin is now ours to lift on our own schedule.
- Client-side expansion is required for CalDAV regardless: server-side
`CALDAV:expand` on `VTODO` is broken on every server `SYNC.md` targets.
### Instance identity
Deleting the materialised `instances` table deletes the instance **row id**, and
two things use it today:
- `TasksRepositoryImpl.updateTask``dataSource.updateInstance(current.id, …)`
- `ListsScreen.kt:422``items(results, key = { it.id })`
The Compose key is the constraint that decides the design. Two occurrences of
one series can appear in the same list, so `taskId` alone is not unique, and a
hash of `(taskId, start)` folded into a `Long` can collide — which as a Compose
key is a visible bug, not a theoretical one.
So we address occurrences by what they actually are:
```kotlin
data class Task(
val taskId: Long, // the master row — unchanged, what navigation uses
val occurrenceStart: Instant?, // null for a non-recurring task
)
fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm)
```
`Task.id` is dropped; the Compose key becomes `"$taskId@${occurrenceStart}"`,
which is unique by construction and stable across reloads.
**External mode absorbs this without loss.** `AndroidTasksDataSource` maps
`(taskId, occurrenceStart)` back to a real instance row with one query —
`WHERE task_id = ? AND instance_start = ?` — before writing through the
instances URI. One extra query on an operation the user performs by hand, in
exchange for a seam that does not depend on a foreign table's row ids.
This is the **only** change to `TasksDataSource`, and
`TasksRepositoryImpl.updateTask` is its only caller.
### Completing one occurrence of a recurring task
The provider's `Detaching.java` implemented **model (d): detach the occurrence
as a brand-new task with its own UID**. We inherited that without ever choosing
it, and it is the model least compatible with CalDAV.
**We implement model (a): a `RECURRENCE-ID` override.** Completing one
occurrence writes a second `tasks` row with the same `uid`, a `recurrence_id`
naming the occurrence, `master_id` pointing at the series, and the completed
state. This is what RFC 5545 specifies and what every other CalDAV client
expects to receive.
This decision is now made explicitly, recorded here, and testable.
---
## Reactivity
`TasksDataSource.registerObserver(onChange: () -> Unit): AutoCloseable` **stays
as-is**. The Room implementation backs it with `InvalidationTracker.Observer`
over the four tables; the External implementation keeps its `ContentObserver`.
One interface, two mechanisms, `TasksRepositoryImpl.observing()` untouched.
Going Flow-native in the DAOs is a later, optional refinement. Doing it now
would change the interface and therefore the External path, for no user-visible
gain.
---
## Migrating existing users
Anyone on v0.3.x has their tasks inside the bundled provider's SQLite file at
`/data/data/de.jeanlucmakiola.agendula/databases/tasks.db` (dmfs schema
version 23). Removing the Gradle module does **not** remove that file — an app
update leaves the data directory intact.
So the migration reads the file directly, with no provider and no
ContentResolver involved:
```
OneShotImport
1. does databases/tasks.db exist? no → nothing to do, mark done
2. open SQLiteDatabase.OPEN_READONLY
3. read tasklists → task_lists (account_type LOCAL → account_id NULL)
4. read tasks → tasks (skip _deleted = 1; mint uid where NULL)
5. read properties → task_alarms (mimetype = …/alarm only)
6. verify counts, inside one Room transaction
7. record completion in DataStore
8. rename tasks.db → tasks.db.imported (kept one release, then deleted)
```
Rules that make this safe:
- **Read-only, single transaction, verified counts.** Either the whole import
lands or none of it does.
- **The source file is renamed, never deleted**, for one release. If the import
is wrong we can still recover from a user's device.
- **Idempotent.** Guarded by a DataStore flag *and* by the rename, so a crash
mid-import cannot double-import.
- **Runs before first UI read**, gated the same way `StorageModeHolder.awaitReady()`
already gates the launch reminder re-sync.
- Tasks that were in an *external* account inside our bundled provider (only
possible if the user had pointed DAVx5 at our authority) are imported as
local lists, with their `uid` preserved. Rare, but preserving the UID is what
lets them be re-attached to an account later.
`tasks.db.imported` is excluded from Auto Backup; the new Room database (with
its `-wal` and `-shm` sidecars) is included, which is the whole point of owning
it.
### If the import goes wrong in production
The rename is not just tidiness — it is the rollback. `tasks.db.imported` is a
complete, untouched dmfs database, so recovery does not need `:provider` to
still exist:
1. `OneShotImport` can be re-run against `tasks.db.imported` as well as
`tasks.db`; the DataStore flag is clearable by a targeted fix release.
2. Re-import truncates the Room tables first and re-runs in one transaction, so
a second attempt is not a merge and cannot duplicate.
3. Only after a release with no import defects reported does a subsequent
version delete `tasks.db.imported`.
This is the reason phase 5 (deleting `:provider`) ships *after* phase 4 rather
than with it — and the reason the deletion is its own release.
---
## Effects on the sync plan
`SYNC.md`'s phase list was written against the provider. Owning the store
deletes work from it outright:
| `SYNC.md` item | Fate |
|---|---|
| "Assign UIDs at creation" (phase 0) | **gone**`uid` is NOT NULL from v1 |
| Auto Backup / `cleanUpLists` data-loss guard (phase 0) | **gone** — no `cleanUpLists` |
| `lib-recur` pin rationale (phase 0) | reduced to a normal version choice |
| Local → Synced migration (phase 3) | **gone**`account_id` is a nullable FK |
| ETag / href / CTag squats into `SYNC1``SYNC8` | **gone** — real columns |
| Per-collection sync token (phase 3) | **gone** — real column |
| `_DIRTY` set-on-delete workaround (phase 3) | **gone** — tombstones are ours |
| `CALLER_IS_SYNCADAPTER` ignored by instances URI | **gone** — no URIs |
| `Moving` dual-UID collision (phase 4) | **gone** |
| Read-only collections (phase 4) | now *possible*`is_read_only` exists |
| Recurring-completion model | **decided here** — RECURRENCE-ID override |
| Byte-stable round-trip | improved — `unknown_properties` preserves the rest |
Everything platform-level and protocol-level in `SYNC.md` is untouched: the
`targetSdk 34` sync-framework gate, the stub sync-adapter pattern, credential
storage, Play compliance, discovery, conditional `PUT`, conflict policy, and
every per-server quirk in the server-reality table.
---
## Plan
### Phase 0 — Untangle (0.5 wk)
- `domain/Models.kt` stops importing `TasksContract`; the status, priority and
local-account constants move into `domain`. This is the last contract
reference above the data layer.
- `Task.id``Task.occurrenceStart`; `updateInstance(taskId, occurrenceStart,
form)`. `AndroidTasksDataSource` gains the lookup query, so the *existing*
provider path exercises the new signature before Room ever does.
- Add `StorageMode.OWN` as a **third** value alongside `LOCAL` and `EXTERNAL`.
- Add Room + `room.schemaLocation` to the version catalog (KSP is already
applied to `:app` for Hilt).
Deliberately **not** here — both were in an earlier draft and both were wrong:
- *Renaming `LOCAL` → `OWN`.* The provider is still the store until phase 4;
renaming now makes `OWN` mean dmfs for four phases and Room afterwards.
Phase 5.
- *Dropping our authority from `ProviderChangeReceiver`'s manifest filter.* That
receiver is what re-syncs reminders while the app is backgrounded
(`ProviderChangeReceiver.kt:47`). Removing the filter while the provider is
still live would silently stop background reminder updates. Phase 5.
**Done when:** the app builds and behaves identically, provider still present,
still default, and the seam change is proven on the provider path.
### Phase 1 — Schema and DAOs (1 wk)
- The four entities above, plus DAOs, plus `schemas/` exported for migration
testing (`room.schemaLocation`, committed).
- `RoomTasksDataSource` implementing all 14 `TasksDataSource` methods except the
recurrence-dependent ones, which throw until phase 2.
- `DataModule` binds by `StorageMode`.
**Done when:** a JVM test creates lists and non-recurring tasks through
`TasksDataSource` against an in-memory Room database and reads them back.
### Phase 2 — Recurrence (1.52 wk)
The hard phase. Budget accordingly.
- `RecurrenceExpander` over `lib-recur`: `RRULE`, `RDATE`, `EXDATE`, overrides,
all-day handling, bounded window, `distanceFromCurrent`.
- `RECURRENCE-ID` override creation on single-occurrence edit and completion.
- A test suite that is the deliverable, not an afterthought: daily/weekly/
monthly-by-day/yearly, `COUNT` and `UNTIL`, DST boundaries, all-day series,
a series with an override, a series with an exception, and an unbounded rule
hitting the window ceiling.
**Done when:** the expansion suite is green and single-occurrence editing forks
correctly.
⚠️ **Parity against the provider is only partly available, and the earlier draft
of this plan overclaimed it.** The provider materialises exactly one upcoming
occurrence, so there is no multi-occurrence behaviour to compare against. The
split:
| Behaviour | Reference |
|---|---|
| Multi-occurrence expansion | RFC 5545 §3.8.5 and `lib-recur` directly — **no provider parity exists** |
| The single next occurrence | provider parity, while it is still in-tree |
| Editing one occurrence (forking) | provider parity — *except* model (a) vs (d), enumerated as explicit difference tests |
| All-day and DST handling | provider parity |
That partial availability is still the reason deletion is phase 5 rather than
phase 0. It is just not the blanket safety net it was described as.
### Phase 3 — Semantics parity (1 wk)
- Completion coherence: `status` ↔ `percent_complete` ↔ `completed_at` ↔ closed,
replacing `AutoCompleting.java` and — importantly — the reopen asymmetry that
`TaskWriteMapper` currently works around in the app.
- Parent/child integrity: `parent_id` is `ON DELETE SET NULL`, so deleting a
parent promotes its subtasks rather than destroying them.
- Validation: `DUE` xor `DURATION`, `due >= dtstart`, all-day pinned to UTC
midnight, list must exist.
- Delete semantics: hard delete when `account_id IS NULL`, tombstone when set;
`master_id` cascades so a deleted series takes its overrides with it.
- `ICalendarWriter.uidFor`'s synthesis branch becomes dead on the Room path
(`uid` is NOT NULL). It stays for External, where UIDs really can be absent —
the KDoc gets updated to say which path each branch now serves.
**Done when:** `TaskWriteMapper`'s provider-quirk workarounds are demonstrably
unnecessary on the Room path (they stay for External).
### Phase 4 — Import and cutover (1 wk)
- `OneShotImport` per the rules above, with tests over a fixture `tasks.db`
captured from a real v0.3.x install.
- `OWN` becomes the default for new installs and for upgraders after import.
- Backup rules updated: include the Room database **and its `-wal`/`-shm`
sidecars**, exclude `tasks.db.imported` and the Keystore blob. WAL checkpoint
on `ON_STOP`.
**Done when:** an upgrade from a v0.3.2 APK with seeded data lands every task,
list and reminder in Room, verified by count and by content.
### Phase 5 — Delete `:provider` (0.5 wk)
- Remove the module, its `settings.gradle.kts` include, its `:app` dependency,
the three dmfs deps it pulled in, `provider/PROVENANCE.md`.
- Add `lib-recur` (and `rfc5545-datetime`) directly to `:app`.
- `StorageMode`: `LOCAL` is deleted, `OWN` is what remains beside `EXTERNAL`.
`ProviderResolver` loses `own` / `isOwn`.
- `ProviderChangeReceiver`'s manifest filter drops our own authority — safe
*now*, because nothing of ours broadcasts `ACTION_PROVIDER_CHANGED` any more.
In `OWN` mode the in-app `InvalidationTracker` observer covers foreground
changes, and until sync exists nothing outside the app can change our data.
**When `SYNC.md` phase 3 lands, the sync worker must call
`ReminderScheduler.sync()` itself** — that is the replacement for the
broadcast, and it belongs in the sync work, not here.
- Attribution screen: dmfs code is gone, but `lib-recur` stays and is
Apache-2.0. `PROVENANCE.md` is replaced by a short note in
`STORAGE-DECISION.md` recording that the fork existed and why it ended.
- **Release note, user-facing:** dropping the `<provider>` also drops the
`de.jeanlucmakiola.agendula.tasks` authority and both custom permissions.
Anyone who pointed DAVx5 or another app at that authority loses it silently —
it has to be called out in the release, with External mode as the answer.
**Done when:** `./gradlew build` is green with `:provider` absent, and the APK
declares no ContentProvider and no custom permissions.
### Phase 6 — Harden (1 wk)
- Room migration test infrastructure (`MigrationTestHelper`) wired up, so v1 →
v2 is cheap when sync adds columns.
- Restore-path test: Auto Backup restore into a fresh install, **including the
WAL case** — write, background, restore, verify the last write survived.
- Performance check at 5,000 tasks with 20 recurring series.
### Total
| Phase | | |
|---|---|---:|
| 0 | Untangle | 0.5 |
| 1 | Schema and DAOs | 1 |
| 2 | Recurrence | 1.52 |
| 3 | Semantics parity | 1 |
| 4 | Import and cutover | 1 |
| 5 | Delete `:provider` | 0.5 |
| 6 | Harden | 1 |
| | | **6.57 wk** |
Against which `SYNC.md`'s own estimate drops by 2.54 weeks, so the net cost of
owning the store is roughly **+2.5 to +4.5 weeks** — before counting the bugs
that stop being unfixable.
> This does not contradict `STORAGE-DECISION.md`'s 4.56 week figure; it
> supersedes it. That estimate costed only the new store (schema, expansion,
> semantics, import, tests) on the assumption `:provider` would be *kept*
> alongside it. This plan deletes the provider, which adds phase 0's untangling
> and phase 5's removal — work the earlier figure never had to include.
---
## Testing posture
| Layer | How |
|---|---|
| Entities, DAOs, migrations | Room in-memory + `MigrationTestHelper`, JVM |
| `RecurrenceExpander` | pure JVM, no Android — the largest suite |
| Semantics (completion, hierarchy, validation) | JVM through `TasksDataSource` |
| `OneShotImport` | fixture `tasks.db` committed as a test resource |
| External mode | unchanged; existing `TaskMapper` / `TaskWriteMapper` tests stay |
The 93 existing app tests must stay green throughout. The 56 provider tests
leave with the module in phase 5 — replaced, not abandoned: phases 2 and 3 owe
equivalent coverage of the behaviour those tests protected. Note the limit
recorded in phase 2: parity covers the single next occurrence, forking and
all-day/DST handling. Multi-occurrence expansion has no provider behaviour to
compare against and is tested against RFC 5545 and `lib-recur` directly.
---
## Risks
| Risk | Mitigation |
|---|---|
| **Recurrence is subtler than estimated** | The likeliest overrun, and the least mitigated — provider parity does not cover multi-occurrence expansion, so the reference is the RFC. Phase 2 is isolated and pure-JVM, so it can overrun without blocking phases 34. |
| **Import loses a user's data** | Read-only source, single transaction, count verification, source renamed not deleted, re-runnable from `tasks.db.imported`, fixture-based tests. |
| **Regression in a behaviour nobody documented** | Partial: phase 2 and 3 parity tests run against the provider while it is still present, for the behaviours where parity exists at all. That is why deletion is phase 5. |
| **A restore silently loses recent writes** | WAL checkpoint on `ON_STOP`, sidecars included in the backup rules, and a phase 6 test that exercises exactly this. |
| **Losing third-party interop** | External mode covers users who need it. Called out in the phase 5 release note. A read-only facade stays possible later; nothing here forecloses it. |
| **Room + KSP build cost** | KSP is already in the build for Hilt; Room adds one processor. |
---
## Deliberately not doing
- **An exported ContentProvider facade over Room.** Possible later (~11.5 wk),
not now. Shipping one would recreate the public-API surface whose validation
and URI plumbing is most of what we are deleting.
- **A domain-native schema.** The table shapes above stay recognisably close to
`TaskContract` where `TaskContract` was right, because it is a proven design
for `VTODO` and because it keeps a future facade cheap.
- **Flow-native DAOs.** Later refinement; changes the interface for no
user-visible gain today.
- **FTS / search.** The provider carried 798 lines of it. The app has never
called it. If search is wanted it is a feature request, designed on its own
terms.
- **Categories and attendees as first-class tables.** They round-trip through
`unknown_properties` until a feature actually needs them.