sync: match a multiget reply by the resource, not by its spelling

The request href is ours, written into the REPORT body verbatim; the
response href is the server's own spelling of it. Compared as raw
encodedPath, a server answering /my@dav.ics for a requested
/my%40dav.ics -- the case UrlUtils.equals exists for -- puts one
resource in unsolicited *and* in missing, drops its intact body, and
after three such runs quarantines it out of the download for good,
because nothing on that side can refund the count. Our own names never
carry an @, so this is about resources other clients created, which is
most of an existing collection.

The key is now scheme, host, port and the *decoded* path segments. A
list rather than a joined string, so /a%2Fb does not collide with /a/b;
with the host, so a same-path href from elsewhere cannot supply a body
for our row. The trailing slash still distinguishes, as UrlUtils.equals
also refuses to normalise it.

The syncer stops spending an unrefundable count on a guess: when the
server both omitted hrefs we asked for and volunteered hrefs we did not,
"omitted" and "we did not recognise its spelling" are indistinguishable,
so those are skipped rather than failed. A wasteful re-request is
recoverable; a permanently dropped task is not.

app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt

@@ -655,7 +655,22 @@ class CollectionSyncer(
                 // `apply`, so nothing counts it — without this it is re-requested
                 // on every sync for ever, which is the loop quarantine exists to
                 // break.
-                fetched.missing.forEach { fail(it.toString(), "listed but not returned") }
+                // ⚠️ When the server both omitted hrefs we asked for and
+                // volunteered hrefs we did not, "omitted" and "we failed to
+                // recognise its spelling" are indistinguishable — and only one
+                // of the two verdicts is reversible. A download-side count has
+                // no refund: at THRESHOLD the href is stripped before the fetch,
+                // so `apply` can never run to clear it. A wasteful re-request is
+                // recoverable; a permanent silent drop of a good task is not.
+                val unmatched = fetched.unsolicited.isNotEmpty()
+                fetched.missing.forEach {
+                    val href = it.toString()
+                    if (unmatched) {
+                        skip(href, "listed but not returned, among hrefs we did not ask for")
+                    } else {
+                        fail(href, "listed but not returned")
+                    }
+                }
                 fetched.failed.forEach { failure ->
                     // ⚠️ Quarantining here also silences the upload phase and the
                     // sweep, because a row's quarantine key *is* its href. So the

app/src/test/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncerTest.kt

@@ -516,6 +516,50 @@ class CollectionSyncerTest {
         assertThat(report.quarantined.single().failures).isEqualTo(0)
     }
 
+    @Test fun `a missing href is not counted when the server also sent strays`() {
+        remote.put("one.ics", vtodo("a", "Fine"))
+        remote.onFetch = { hrefs ->
+            Result.success(
+                FetchResult(
+                    resources = emptyList(),
+                    missing = hrefs,
+                    failed = emptyList(),
+                    // The shape an href we failed to recognise takes: one
+                    // resource in both buckets.
+                    unsolicited = listOf("http://server/dav/tasks/one.ics".toHttpUrl()),
+                ),
+            )
+        }
+
+        val report = sync()
+
+        // ⚠️ Counting here is irreversible — at THRESHOLD the href is stripped
+        // before the fetch, so nothing can ever clear it again.
+        assertThat(quarantine).isEmpty()
+        assertThat(report.quarantined.single().failures).isEqualTo(0)
+    }
+
+    @Test fun `a missing href is still counted when nothing was unmatched`() {
+        remote.put("one.ics", vtodo("a", "Fine"))
+        remote.onFetch = { hrefs ->
+            Result.success(
+                FetchResult(
+                    resources = emptyList(),
+                    missing = hrefs,
+                    failed = emptyList(),
+                    unsolicited = emptyList(),
+                ),
+            )
+        }
+
+        val report = sync()
+
+        // A plain omission repeats identically every run and nothing else breaks
+        // the loop, which is what the counter is for.
+        assertThat(quarantine.values.single()).isEqualTo(1)
+        assertThat(report.quarantined.single().failures).isEqualTo(1)
+    }
+
     @Test fun `a resource the server keeps refusing stops being requested`() {
         remote.put("two.ics", vtodo("b", "Refused"))
         remote.onFetch = { hrefs -> refuseTwoIcs(hrefs, code = 403) }

caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalendarCollection.kt

@@ -178,6 +178,26 @@ class CalendarCollection(
      */
     override fun fetch(hrefs: List<HttpUrl>): Result<FetchResult> = fetch(hrefs, MULTIGET_BATCH)
 
+    /**
+     * What makes two hrefs the same resource.
+     *
+     * ⚠️ Not the raw `encodedPath`. The request href is *ours*, written into the
+     * REPORT body verbatim, and the response href is the server's own spelling
+     * of it — a server that answers `/my@dav.ics` for a requested `/my%40dav.ics`
+     * is the reason `UrlUtils.equals` exists at all. Compared raw, that resource
+     * lands in `unsolicited` *and* in `missing`, its intact body is dropped, and
+     * three such runs quarantine it out of the download for good, since nothing
+     * on this side can refund the count.
+     *
+     * `pathSegments` is decoded, so both spellings agree. Keeping it a list, not
+     * a joined string, stops `/a%2Fb` colliding with `/a/b`. Scheme, host and
+     * port ride along because a path-only match would accept a body from another
+     * host entirely. The trailing slash still distinguishes, exactly as
+     * `UrlUtils.equals` refuses to normalise it.
+     */
+    private fun identityOf(url: HttpUrl): List<String> =
+        listOf(url.scheme, url.host, url.port.toString()) + url.pathSegments
+
     /** [batchSize] is a seam for tests; production always uses [MULTIGET_BATCH]. */
     internal fun fetch(hrefs: List<HttpUrl>, batchSize: Int): Result<FetchResult> =
         runCatching {
@@ -187,10 +207,10 @@ class CalendarCollection(
             val seen = mutableSetOf<HttpUrl>()
 
             hrefs.chunked(batchSize).forEach { batch ->
-                val wanted = batch.associateBy { it.encodedPath }
+                val wanted = batch.associateBy { identityOf(it) }
                 dav.multiget(batch, MIME_ICALENDAR, ICALENDAR_VERSION) { response, relation ->
                     if (relation == Response.HrefRelation.SELF) return@multiget
-                    val asked = wanted[response.href.encodedPath]
+                    val asked = wanted[identityOf(response.href)]
                     if (asked == null) {
                         unsolicited += response.href
                         return@multiget

caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalendarCollectionTest.kt

@@ -101,6 +101,106 @@ END:VCALENDAR</C:calendar-data>
         assertThat(result.missing.map { it.encodedPath }).containsExactly("/dav/tasks/two.ics")
     }
 
+    @Test fun `a server that respells the path is still matched`() {
+        server.enqueue(
+            multistatus(
+                """
+                <response>
+                  <href>/dav/tasks/my%40dav.ics</href>
+                  <propstat><prop>
+                    <getetag>"e1"</getetag>
+                    <C:calendar-data xmlns:C="urn:ietf:params:xml:ns:caldav">BEGIN:VCALENDAR
+END:VCALENDAR</C:calendar-data>
+                  </prop><status>HTTP/1.1 200 OK</status></propstat>
+                </response>
+                """,
+            ),
+        )
+
+        val asked = href("my@dav.ics")
+        val result = collection.fetch(listOf(asked)).getOrThrow()
+
+        // ⚠️ Compared raw, one resource lands in unsolicited *and* missing, its
+        // body is dropped, and three such runs quarantine it out of the download
+        // for good — nothing on that side can refund the count.
+        assertThat(result.resources).hasSize(1)
+        assertThat(result.missing).isEmpty()
+        assertThat(result.unsolicited).isEmpty()
+        assertThat(result.failed).isEmpty()
+        // The href we asked for, not the server's spelling: apply is keyed on
+        // the local row's href.
+        assertThat(result.resources.single().href).isEqualTo(asked)
+    }
+
+    @Test fun `the same path on another host is not our resource`() {
+        server.enqueue(
+            multistatus(
+                """
+                <response>
+                  <href>https://evil.example.com/dav/tasks/one.ics</href>
+                  <propstat><prop>
+                    <getetag>"e1"</getetag>
+                    <C:calendar-data xmlns:C="urn:ietf:params:xml:ns:caldav">BEGIN:VCALENDAR
+END:VCALENDAR</C:calendar-data>
+                  </prop><status>HTTP/1.1 200 OK</status></propstat>
+                </response>
+                """,
+            ),
+        )
+
+        val result = collection.fetch(listOf(href("one.ics"))).getOrThrow()
+
+        // Matching on the path alone would apply one host's body to another
+        // host's row.
+        assertThat(result.resources).isEmpty()
+        assertThat(result.unsolicited.map { it.host }).containsExactly("evil.example.com")
+        assertThat(result.missing).hasSize(1)
+    }
+
+    @Test fun `a trailing slash is still a different resource`() {
+        server.enqueue(
+            multistatus(
+                """
+                <response>
+                  <href>/dav/tasks/one.ics/</href>
+                  <propstat><prop><getetag>"e1"</getetag></prop><status>HTTP/1.1 200 OK</status></propstat>
+                </response>
+                """,
+            ),
+        )
+
+        val result = collection.fetch(listOf(href("one.ics"))).getOrThrow()
+
+        // Deliberately not normalised, as UrlUtils.equals also refuses to.
+        assertThat(result.resources).isEmpty()
+        assertThat(result.missing).hasSize(1)
+    }
+
+    @Test fun `an encoded slash is not the same as a real one`() {
+        server.enqueue(
+            multistatus(
+                """
+                <response>
+                  <href>/dav/tasks/a/b.ics</href>
+                  <propstat><prop>
+                    <getetag>"e1"</getetag>
+                    <C:calendar-data xmlns:C="urn:ietf:params:xml:ns:caldav">BEGIN:VCALENDAR
+END:VCALENDAR</C:calendar-data>
+                  </prop><status>HTTP/1.1 200 OK</status></propstat>
+                </response>
+                """,
+            ),
+        )
+
+        val asked = server.url("/dav/tasks/a%2Fb.ics")
+        val result = collection.fetch(listOf(asked)).getOrThrow()
+
+        // One segment "a/b" is not two segments "a" and "b" — which is why the
+        // key stays a list rather than a joined string.
+        assertThat(result.resources).isEmpty()
+        assertThat(result.missing).containsExactly(asked)
+    }
+
     @Test fun `a refused resource is reported as failed, not as missing`() {
         server.enqueue(
             multistatus(