agendula/docs/STORAGE-DECISION.md

Storage: keep the vendored provider, or build our own?

Status: decided — build our own. See OWN-STORE.md for the architecture and plan; this document is the reasoning that got there.

The decision went further than the recommendation below: :provider is not kept alongside a Room store, it is deleted. External mode (OpenTasks, tasks.org) stays. The staged sequencing survives in a different form — the provider remains in-tree until OWN-STORE.md phase 5 so recurrence parity can be tested against it, then goes.

This reopens a question SYNC.md marked settled. It is reopened on purpose: the argument that settled it was "the provider hands us the sync bookkeeping for free", and the phase-1 audit found most of that bookkeeping broken, absent, or unusable for our purposes. A conclusion is only as good as its premise.

---

The three options

	What it means	Store	Exported provider
A	Keep :provider as-is	dmfs TaskProvider	yes, ours today
B	Room, same TaskContract shape	our Room DB	dropped, or a later facade
C	Room, clean domain schema, TaskContract only as an export format	our Room DB	no

External mode (talking to OpenTasks / tasks.org) is orthogonal and survives all three. It is the reason nothing below gets deleted.

---

What the swap actually touches — measured, not estimated

The seam is already there, and it is clean

TasksDataSource (data/tasks/TasksDataSource.kt, 58 lines) is a 14-method, domain-shaped interface. It takes and returns Task, TaskList, TaskForm — no Cursor, no Uri, no ContentValues.

Above it, 17 files import from data.tasks. What they import:

8 × TasksRepository      4 × TasksDataSource       3 × ProviderResolver
4 × recoveringFromProviderFailure                  2 × ProviderStatus
1 each: StorageMode, StorageModeHolder, ProviderEnvironment, TaskQuery, …

Exactly one file outside the data package touches TasksContract at all — domain/Models.kt, and only for four status integers, one priority constant and one account-type string. Ten lines. Nothing else above the data layer knows a ContentProvider exists.

The whole UI, all five milestones of it, is untouched by a storage swap. That is not luck — AndroidTasksDataSource's own KDoc says the seam exists so that "swapping the provider never reaches above this file." It holds.

Nothing gets deleted

File	Lines	Under a Room store
AndroidTasksDataSource.kt	220	kept — External mode still needs it
TasksContract.kt	178	kept — External mode speaks it
TasksRepositoryImpl.kt	151	unchanged
ProviderResolver.kt	143	unchanged
TaskWriteMapper.kt	118	kept for External
TaskMapper.kt	102	kept for External
TasksDataSource.kt	58	unchanged — it is the interface
TasksRepository.kt	53	unchanged
ProviderEnvironment.kt	51	unchanged
StorageModeHolder.kt	47	unchanged
ColumnReader.kt	40	kept for External
ProviderFlow.kt	31	unchanged
StorageMode.kt	30	one new constant
TaskProjections.kt	24	kept for External
Failures.kt	16	unchanged
	1,262	0 removed

The work is additive: a second TasksDataSource implementation, a third StorageMode, and one @Binds becoming a dispatcher. DataModule.kt has a single binding to change.

This reframes the question. It is not rewrite vs. keep. It is write a second backend behind an interface that exists for exactly this purpose, and run both until one wins.

---

What the new backend has to do

Room entities and DAOs for the ~50 columns the app actually uses across four tables are mechanical. The real work is the behaviour the provider's processors perform. Measured against :provider's Java:

Behaviour	Provider	Notes
Instance expansion	~1,070	Instantiating + instancedata + iterables. The hard one.
Recurring-instance edit	337	Detaching — this is the recurrence-model decision
Completion coherence	210	AutoCompleting: status ↔ percent ↔ completed ↔ is_closed
Validation	601	three processors, mostly defending a public API
Parent / child	269	we use parent_id only
Alarm property rows	133	one Room entity
	~2,620

And what we would not write, of the 14,555 vendored lines:

Not needed	Lines	Why
TaskDatabaseHelper	895	23 migrations from a 2013 schema. We start at v1.
FTSDatabaseHelper + ngrams	798	the app never searches the provider — verified, zero call sites
model/adapters	1,581	a type-safe layer over ContentValues. Room entities delete the problem.
model	1,811	cursor ↔ entity adaptation. Room's job.
TaskProvider + SQLiteContentProvider	1,772	URI matching, permissions, batch ops — for a public API
CategoryHandler + RelationHandler	553	unused
utils (most)	~800	dmfs jems idiom → Kotlin stdlib
≈ 8,200 lines we would simply not have

Two things make instance expansion less frightening than its line count:

1. We need client-side recurrence expansion regardless. Server-side CALDAV:expand on VTODO is broken on every server we target (SYNC.md), so lib-recur is in the build either way.
2. We would use the same eight lib-recur classes the provider does — RecurrenceRule, RecurrenceSet, RecurrenceSetIterator, RecurrenceList, RecurrenceRuleAdapter, DateTime, Duration, InvalidRecurrenceRuleException. The algorithm is in the library, not in the provider.
3. And the provider's expansion materialises only one upcoming occurrence anyway — it is not the complete implementation its size suggests.

---

The cost, both directions

Building it

Schema, entities, DAOs	1 wk
Instance expansion on lib-recur, with a real test suite	1.5–2 wk
Completion / parent / validation semantics	1 wk
Recurring-edit model — shared cost, phase 1 either way	(0.5–1 wk)
Migrating existing users' local data out of the provider	0.5 wk
Tests to parity with the current 93 + 56	1 wk
Net additional	4.5–6 wk

⚠️ Superseded by OWN-STORE.md's 6.5–7 wk. The figure above costed the new store only, on the assumption :provider would be kept beside it. The decision taken was to delete the provider, which adds the untangling (phase 0) and the removal (phase 5) that this estimate never had to include. The costing logic stands; the total does not.

What it removes from the sync plan

Roughly sixteen of the phase-1 audit's storage findings are provider-imposed — they exist only because we run dmfs's implementation, and vanish when we own the store:

• _DIRTY not set on delete, and defaulting to 1
• TaskLists._DIRTY as a monotonic counter, not a flag
• the instances URI ignoring CALLER_IS_SYNCADAPTER
• no home for a per-collection sync token, href, ETag or CTag — all four squat into generic SYNC1–SYNC8 slots
• read-only collections cannot be represented at all (ACCESS_LEVEL inert)
• sync-adapter delete ignoring the account parameters it forces you to supply
• Moving leaving a dual-UID collision
• ACCOUNT_TYPE write-once → enabling sync is a full data migration
• Auto Backup restore arming cleanUpLists → silent task loss
• Detaching deciding the recurring-completion model for us
• the lib-recur version trap (0.16.0 removed RecurrenceSet) — we pin because the provider does, not because we want to

Conservatively that is 2.5–4 weeks off phases 0, 3 and 4 of the 11.5–15 week sync plan, plus a class of bug that is currently unfixable without patching vendored Java.

Net

≈ +1 to +3.5 weeks, for a store we control, in exchange for two real losses.

---

The honest case for keeping it (Option A)

Not nothing, and it should not be waved away:

• It works, and it has 56 passing JVM tests over recurrence, reparenting, instances and observers. A Room reimplementation is new code with new bugs, in the layer that holds the user's only copy of their data. That risk is real and it points at A.
• Tombstones actually work. Soft delete for account rows, hard delete for sync adapters, hidden from normal queries, undelete refused. Of all the sync bookkeeping, this is the piece that held up under audit.
• The exported provider under our own authority — third-party apps can read Agendula's tasks, and asking DAVx5 to sync us stays possible.
• Eleven local modification sites, all marked AGENDULA CHANGE, all documented in provider/PROVENANCE.md. The fork is under control today.

And the case against keeping it:

• 14,555 lines of Java — 1.66× the entire app (8,783 lines of Kotlin). We carry, build, lint, translate and ship all of it to use maybe a third.
• Upstream is effectively dormant; every future targetSdk bump and every Android SQLite behaviour change lands on us, in someone else's code, in a language the rest of the app does not use.
• It makes behavioural decisions on our behalf (Detaching, AutoCompleting) that we then have to reverse-engineer before we can honour them over CalDAV.

---

Recommendation

Option B — build our own store on Room, keeping the TaskContract shape as the internal model — and keep :provider in-tree while we do.

Three reasons, in order of weight:

1. The seam already exists and the work is additive. Nothing is deleted, nothing above data/tasks changes, and both backends can ship side by side behind StorageMode. The "big rewrite" this decision was originally weighed against does not exist.
2. The premise that settled it is gone. The provider was kept for sync bookkeeping we have since measured as broken. Sixteen findings deep, keeping it is now a cost to the sync plan, not a saving.
3. The window is now. After phase 1 the mapper and engine are written against whichever store won, and this stops being a two-file change.

Keeping the TaskContract shape rather than going domain-native (Option C) is deliberate: it is a proven schema for exactly this problem, other engines understand it, and it keeps a future exported facade cheap — without obliging us to run a 2015 Java implementation of it.

Sequencing that keeps the risk low

1. Add StorageMode.OWN and a Room TasksDataSource. Both backends live.
2. Ship it behind a setting; the vendored provider stays the default.
3. Run the sync engine against Room only.
4. Once Room has real production mileage, decide whether the exported ContentProvider is worth re-implementing as a thin facade (~1–1.5 wk) or whether External mode already covers everyone who wanted it.

Step 4 is a genuinely open question and does not need answering now. That is the point of sequencing it last.

---

Open

• Is an exported provider worth keeping at all? It matters only if third parties should read our tasks, or if we want DAVx5 to sync our store. External mode arguably already serves the second. Undecided.
• The Room estimate is mine, not measured. Instance expansion is the item that could overrun; everything else is well-bounded.

---

Postscript: the fork existed, and how it ended

provider/PROVENANCE.md recorded the vendored dmfs task provider in detail. Both are gone; this is what is worth keeping.

The module was opentasks-provider plus opentasks-contract from dmfs/opentasks 1.4.2, commit 49ebf80b1eeee52a611e5a22f24f849852a6255f (2021-03-21), Apache-2.0, database version 23. It was vendored in-tree rather than pulled as an artifact because the permission names are hardcoded in the upstream AAR's manifest, and shipping under dmfs's own names would have made Agendula and OpenTasks mutually uninstallable (INSTALL_FAILED_DUPLICATE_PERMISSION). In-tree also satisfied F-Droid's from-source requirement.

It was deleted in the feat/own-store work (docs/OWN-STORE.md phase 5) once Room was the default and every v0.3.x install had been imported. 14,555 lines of Java left with it.

What survives, and why. lib-recur (Apache-2.0, dmfs) is still a direct dependency — it is what expands recurrences — so dmfs's attribution is still owed, now through an ordinary third-party dependency rather than vendored source. TasksContract.kt and the External-mode mappers also stay: they describe somebody else's schema, which is exactly what they were always right for.

One detail the fork's provenance file carried that still binds us. DB 23 is the first to have is_recurring; tasks.org's fork is DB 22 and lacks it. That is why TaskMapper.task derives recurrence from rrule/rdate rather than trusting that column, and it must keep doing so for as long as External mode supports tasks.org.