sync: sweep the local list as it is after the download
The full run captured locals once and gave the same list to both the download and the sweep. apply can re-point a row's href when a UID reappears under a new filename, and purge deletes by row id, so the sweep destroyed the row the download had just repaired: the task came back next cycle with sortOrder 0, no colour and no parent, and deletedLocally was reported for a task nobody deleted. Subtasks were un-parented for good, since parent_id is ON DELETE SET_NULL and clean children are never re-downloaded. The listing stays pre-write; only the local side is re-read.
This commit is contained in:
@@ -29,7 +29,8 @@ import kotlin.time.Instant
|
||||
* 3. **Downloads**, including everything the write phase decided we now need a
|
||||
* fresh copy of.
|
||||
* 4. **Sweep**, which is the only step that may delete a row it did not see fail
|
||||
* — and so it runs last, on a listing taken before any of the writes.
|
||||
* — and so it runs last, on a listing taken before any of the writes. Its
|
||||
* local side, unlike that listing, is read *after* the download.
|
||||
*
|
||||
* ⚠️ **A failed resource must not fail the collection**, the twin of "a failed
|
||||
* collection must not fail the account". Every per-resource outcome below either
|
||||
@@ -179,9 +180,10 @@ class CollectionSyncer(
|
||||
ref.href.toString() to ref.eTag?.takeIf { it.usable }?.value
|
||||
}
|
||||
|
||||
val locals = localResources()
|
||||
downloadPhase(locals, remoteETags)
|
||||
sweepPhase(locals, remoteETags.keys)
|
||||
downloadPhase(localResources(), remoteETags)
|
||||
// ⚠️ Re-read. The download just ran and may have re-pointed a row's
|
||||
// href; the pre-download list still names the vacated one.
|
||||
sweepPhase(localResources(), remoteETags.keys)
|
||||
|
||||
report = report.copy(reconciledInFull = true)
|
||||
// ⚠️ Adopted only now, and taken from the PROPFIND that *preceded*
|
||||
@@ -709,6 +711,12 @@ class CollectionSyncer(
|
||||
|
||||
// ------------------------------------------------------------ phase 4
|
||||
|
||||
/**
|
||||
* @param locals must be read *after* [downloadPhase]. [apply] can re-point
|
||||
* an existing row at a new href when the same UID reappears under a new
|
||||
* filename, and [purge] deletes by row id — so a pre-download list sweeps
|
||||
* away the row the download just repaired.
|
||||
*/
|
||||
private fun sweepPhase(locals: List<LocalResource>, remoteHrefs: Set<String>) {
|
||||
// ⚠️ An empty listing never sweeps. The sweep is the one phase that
|
||||
// deletes rows it did not see fail, and its evidence is a
|
||||
|
||||
@@ -317,6 +317,43 @@ class CollectionSyncerTest {
|
||||
assertThat(report.deletedLocally).isEqualTo(1)
|
||||
}
|
||||
|
||||
@Test fun `a task recreated under a new href is repointed, not swept`() {
|
||||
// Another client deleted one.ics and recreated the same UID as two.ics.
|
||||
remote.put("two.ics", vtodo("a", "Buy milk"))
|
||||
store.rows += task(id = 5, uid = "a", title = "Buy milk")
|
||||
.copy(
|
||||
href = "http://server/dav/tasks/one.ics",
|
||||
etag = "e-one.ics",
|
||||
sortOrder = 42,
|
||||
color = 0xFF0000,
|
||||
)
|
||||
|
||||
val report = sync()
|
||||
|
||||
// The download re-points row 5; a sweep reading the pre-download list
|
||||
// would purge it by id and lose the local-only columns for good.
|
||||
val row = store.rows.single()
|
||||
assertThat(row.id).isEqualTo(5)
|
||||
assertThat(row.href).isEqualTo("http://server/dav/tasks/two.ics")
|
||||
assertThat(row.sortOrder).isEqualTo(42)
|
||||
assertThat(row.color).isEqualTo(0xFF0000)
|
||||
assertThat(report.deletedLocally).isEqualTo(0)
|
||||
assertThat(report.downloaded).isEqualTo(1)
|
||||
}
|
||||
|
||||
@Test fun `the post-download sweep still removes a stale row`() {
|
||||
remote.put("one.ics", vtodo("a", "New"))
|
||||
store.rows += task(id = 1, uid = "b")
|
||||
.copy(href = "http://server/dav/tasks/gone.ics", etag = "e")
|
||||
|
||||
val report = sync()
|
||||
|
||||
// Reading locals after the download widens what the sweep sees; the row
|
||||
// just inserted must survive it, the stale one must not.
|
||||
assertThat(store.rows.map { it.uid }).containsExactly("a")
|
||||
assertThat(report.deletedLocally).isEqualTo(1)
|
||||
}
|
||||
|
||||
@Test fun `a create the server keeps refusing is eventually quarantined`() {
|
||||
store.rows += task(id = 1, uid = "a", title = "Never acceptable").copy(isDirty = true)
|
||||
remote.onPut = { PutOutcome.Rejected(415, "Unsupported Media Type") }
|
||||
|
||||
Reference in New Issue
Block a user