sync(chunk 2a): vendor dav4jvm 2.2.1 as :dav
JitPack-only publishing conflicts with FAIL_ON_PROJECT_REPOS, artifacts are unsigned and not immutable, and upstream shipped two breaking majors 19 days apart. 2.2.1 is the last OkHttp release: 3.0.0 moved to Ktor and 4.x needs Java 21, and SYNC.md's whole auth section is written in OkHttp terms, so 4.x would have invalidated it. A plain JVM module, not an Android library — the tree has no Android imports and keeping it that way keeps the floret-kit extraction a file move. Four changes from upstream, all in dav/PROVENANCE.md: - commons-lang3 dropped (one import, one call). The replacement requires the whole string to be consumed, as DateUtils did: pattern 1 ends in a literal 'GMT', so SimpleDateFormat would match "...GMT+02:00" as a prefix and throw the offset away. - HTTP dates were parsed and formatted in the device's local zone — the GMT in the format string is a quoted literal and timeZone was never set, so every getlastmodified was out by the local UTC offset. Upstream tests dates nowhere. The formatter is also no longer shared and mutable. - dav4jvm#209: permanent redirects now reach the caller via DavResource.permanentLocation, cleared per request so it never describes an earlier one. - xpp3 is compileOnly; Android supplies org.xmlpull.v1. SYNC.md's other claimed defect, "does not follow 303", is not true of 2.2.1 — pinned by a test rather than removed from the notes. CI names :dav:test explicitly; testDebugUnitTest is Android-only and would have run none of the vendored suite. lintDebug added to the per-chunk checks after it caught a literal byte-order mark left by chunk 1.
This commit is contained in:
+42
-6
@@ -56,7 +56,8 @@ Nextcloud/Radicale/Baïkal in chunk 5.
|
||||
| # | Chunk | Rests on | The thing it must not get wrong |
|
||||
|---|---|---|---|
|
||||
| 1 | **Mapper** — VTODO ↔ Room | nothing | Unknown properties survive a read-modify-write cycle |
|
||||
| 2 | **Auth, discovery, account** | 1 (nothing hard) | Never send a credential into an unvalidated redirect chain |
|
||||
| 2a | **Vendored DAV foundation** | nothing | Vendor a tree we can actually maintain, and know which of its defects are real |
|
||||
| 2b | **Auth, discovery, account** | 2a | Never send a credential into an unvalidated redirect chain |
|
||||
| 3 | **Engine core** — read, write, conflicts | 1, 2 | A failed resource must not fail the collection |
|
||||
| 4 | **Incremental sync + scheduling** | 3 | Never persist a sync token before the bodies it covers are applied |
|
||||
| 5 | **Hardening, compliance, real servers** | 1–4 | Ship no licence violation, and no retry loop on a dead app password |
|
||||
@@ -172,7 +173,34 @@ and no fixture loses an unknown property.
|
||||
|
||||
---
|
||||
|
||||
## Chunk 2 — auth, discovery, account
|
||||
## Chunk 2a — the vendored DAV foundation
|
||||
|
||||
⚠️ **Split out of chunk 2 while building it.** Vendoring is a self-contained
|
||||
piece with its own verification — upstream's suite has to pass unmodified before
|
||||
anything is built on top — and bundling it with discovery and auth would have
|
||||
produced one commit nobody could review.
|
||||
|
||||
**Version:** dav4jvm **2.2.1**, the last OkHttp release. 3.0.0 deleted the OkHttp
|
||||
package for Ktor and 4.x requires Java 21, and `SYNC.md`'s entire auth section is
|
||||
written in OkHttp terms — preemptive Basic via an `Interceptor`, OkHttp stripping
|
||||
`Authorization` on cross-host redirects, `BasicDigestAuthHandler` because OkHttp
|
||||
has no Digest. Taking 4.x would have invalidated all of it. A plain JVM module,
|
||||
not an Android library: the tree has zero Android imports and must keep it that
|
||||
way, or the floret-kit extraction stops being a file move.
|
||||
|
||||
Full record in [`dav/PROVENANCE.md`](../dav/PROVENANCE.md). Of the two defects
|
||||
`SYNC.md` said we would inherit, **one was not real** — 2.2.1 does follow 303 —
|
||||
and the other (`dav4jvm#209`, permanent redirects never reaching the caller) is
|
||||
fixed. A third, which the audit did not have, was found by writing the tests
|
||||
upstream never wrote: **HTTP dates were parsed and formatted in the device's
|
||||
local zone**, putting every `getlastmodified` out by the local UTC offset.
|
||||
|
||||
**Done when:** upstream's 63 tests pass unmodified, `commons-lang3` and the xpp3
|
||||
runtime dependency are gone, and `:app` links against the module.
|
||||
|
||||
---
|
||||
|
||||
## Chunk 2b — auth, discovery, account
|
||||
|
||||
**Goal:** the app can add a CalDAV account and list its VTODO collections. No
|
||||
syncing yet.
|
||||
@@ -438,10 +466,18 @@ attribution screen ships, and a `releaseTest` build syncs on a real device.
|
||||
## Definition of done, every chunk
|
||||
|
||||
1. It builds: `./gradlew :app:assembleDebug`.
|
||||
2. Unit tests pass: `./gradlew :app:testDebugUnitTest`.
|
||||
3. `/code-review high` on the chunk's diff, with the findings either fixed or
|
||||
answered in the commit message.
|
||||
4. One commit per chunk, referencing the chunk number. No release, no tag.
|
||||
2. Unit tests pass: `./gradlew :app:testDebugUnitTest :dav:test`. ⚠️ Name
|
||||
`:dav:test` explicitly — `testDebugUnitTest` is an Android-variant task and
|
||||
does not exist on a plain JVM module, so the vendored suite would otherwise be
|
||||
compiled by nobody and run by nobody.
|
||||
3. Lint passes: `./gradlew :app:lintDebug`. Added after chunk 1 shipped a literal
|
||||
byte-order mark that builds and tests both accepted and CI's lint step would
|
||||
have rejected.
|
||||
4. `/code-review high` on the chunk's diff, with the findings either fixed or
|
||||
answered in the commit message. For a chunk that vendors code, scope the
|
||||
review to the files we wrote — reviewing thousands of lines of verbatim
|
||||
upstream is noise, and its defects belong in `PROVENANCE.md`.
|
||||
5. One commit per chunk, referencing the chunk number. No release, no tag.
|
||||
|
||||
Chunk 2 additionally does not count as done until the account-add UI has had an
|
||||
**on-device review with an explicit go-ahead** — `CLAUDE.md`'s rule, and this is
|
||||
|
||||
Reference in New Issue
Block a user