sync: three recurrence defects the review found in the iCal layer

A DATE-valued UNTIL lost the last day of every series. The floating-UNTIL
repair rebuilt the bound from hours/minutes/seconds, all zero for a DATE,
so FREQ=DAILY;UNTIL=20250109 on a 09:00 Berlin series yielded five
occurrences ending on the 8th instead of six ending on the 9th. §3.3.10
pairs a DATE UNTIL with a DATE DTSTART; Google and Apple emit it beside a
timed one anyway, and only a device away from UTC ever saw it.

A RELATED-TO that is not a PARENT was destroyed on the next PUT. None of
them is ever claimed, so they all live in the residue, and the
contradiction test ignored RELTYPE — reading a CHILD link as a PARENT
link that disagreed with the column. It is scoped now, exactly as the
read side is.

RDATE and EXDATE lost their parameters permanently, and kept only their
first line. Both are cardinality-many — Apple writes one line per
excluded occurrence — and reading only the first left the rest out of the
column the expander works from, so a deleted occurrence reappeared in
the list and in its reminders while round-tripping to the server
perfectly out of the residue, which is what made it invisible. They are
merged now, and claimed only when no copy carries a parameter: the
column is the bare value, so claiming a TZID- or VALUE=DATE-qualified
property dropped that qualifier for good, leaving a floating EXDATE that
matched nothing and a bare eight-digit RDATE that §3.3.5 reads as a
malformed DATE-TIME — a permanent 415 under the Prefer: handling=strict
this client sends. When we do change the list ourselves, the authored
line takes the parameters the residue gives up, checked against the
value rather than copied across.

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt

@@ -79,6 +79,9 @@ object VTodoMapper {
         // SEQUENCE, so the duplicate is unconditional — and under
         // `Prefer: handling=strict` sabre will not quietly repair it.
         "SEQUENCE", "PRIORITY", "PERCENT-COMPLETE", "CLASS",
+        // Parameterised copies stay in the residue verbatim; authoring the
+        // column beside them would emit the same exclusions twice.
+        "RDATE", "EXDATE",
     )
 
     /** What a VTODO yields. Row identity ([TaskEntity.id], `listId`) is the caller's. */
@@ -109,6 +112,31 @@ object VTodoMapper {
         fun <T> take(property: ICalProperty?, value: T?): T? =
             value?.also { claim(property) }
 
+        /**
+         * Every `RDATE` / `EXDATE` copy's values, merged.
+         *
+         * ⚠️ Both are cardinality-**many**, and Apple Calendar writes one line
+         * per excluded occurrence. Reading only the first left the rest out of
+         * the column the expander works from, so a deleted occurrence
+         * reappeared in the list and in its reminders — while round-tripping
+         * back to the server perfectly out of the residue, which is what made it
+         * invisible.
+         *
+         * ⚠️ Claimed only when no copy carries a parameter. The column is the
+         * bare value, so claiming a `TZID=`- or `VALUE=DATE`-qualified property
+         * would drop that qualifier for good: the property never reaches the
+         * residue, and `write` re-emits it naked. A floating EXDATE stops
+         * matching the instance it excluded, and a bare eight-digit RDATE is
+         * read as a malformed DATE-TIME (§3.3.5) — a permanent 415 under the
+         * `Prefer: handling=strict` this client sends.
+         */
+        fun dateList(vtodo: ICalComponent, name: String, claim: (ICalProperty?) -> Unit): String? {
+            val copies = vtodo.properties(name)
+            if (copies.isEmpty()) return null
+            if (copies.all { it.params.isEmpty() }) copies.forEach(claim)
+            return copies.joinToString(",") { it.value.trim() }.takeIf { it.isNotEmpty() }
+        }
+
         val uidProperty = vtodo.property("UID")
         // Claimed even when empty: an empty UID must not reach the residue, or
         // write would emit the caller's minted UID alongside the empty one.
@@ -211,8 +239,8 @@ object VTodoMapper {
             isAllDay = allDay,
             timezone = timezone,
             rrule = take(vtodo.property("RRULE"), vtodo.property("RRULE")?.value?.trim()),
-            rdate = take(vtodo.property("RDATE"), vtodo.property("RDATE")?.value?.trim()),
-            exdate = take(vtodo.property("EXDATE"), vtodo.property("EXDATE")?.value?.trim()),
+            rdate = dateList(vtodo, "RDATE", ::claim),
+            exdate = dateList(vtodo, "EXDATE", ::claim),
             recurrenceId = recurrenceId.instant,
             createdAt = created.instant,
             lastModified = lastModified.instant,
@@ -257,7 +285,8 @@ object VTodoMapper {
         now: Instant = Clock.System.now(),
     ): ICalComponent {
         val residue = parseResidue(entity.unknownProperties)
-        val keptResidue = residue.properties.filterNot { contradictsResidue(it, entity, parentUid) }
+        val keptResidue =
+            residue.properties.filterNot { contradictsResidue(it, residue, entity, parentUid) }
         val suppressed = keptResidue
             .map { it.name.uppercase() }
             .filterTo(mutableSetOf()) { it in SUPPRESSED_BY_RESIDUE }
@@ -292,8 +321,14 @@ object VTodoMapper {
         add("DURATION", entity.duration)
 
         add("RRULE", entity.rrule)
-        add("RDATE", entity.rdate)
-        add("EXDATE", entity.exdate)
+        // ⚠️ Authored only when the residue's copies contradicted the column —
+        // i.e. we changed the exclusions ourselves. The parameters come from the
+        // copy the residue is giving up, so an authored value keeps the TZID the
+        // list was written in; with no copy to take them from, a date-shaped
+        // value still needs its VALUE=DATE, since §3.3.5's default is DATE-TIME
+        // and a bare eight-digit value is malformed.
+        dateList("RDATE", entity.rdate, residue, suppressed, properties)
+        dateList("EXDATE", entity.exdate, residue, suppressed, properties)
         addTime("RECURRENCE-ID", entity.recurrenceId)
 
         add("CREATED", entity.createdAt?.let { ICalValues.formatDateTime(it, null) })
@@ -310,6 +345,40 @@ object VTodoMapper {
         )
     }
 
+    private fun dateList(
+        name: String,
+        value: String?,
+        residue: ICalComponent,
+        suppressed: Set<String>,
+        into: MutableList<ICalProperty>,
+    ) {
+        if (value == null || name in suppressed) return
+        val inherited = residue.properties(name).firstOrNull { it.params.isNotEmpty() }?.params
+        into += ICalProperty(name, paramsFor(value, inherited.orEmpty()), value)
+    }
+
+    /**
+     * The parameters an authored list may carry, given what is in it.
+     *
+     * ⚠️ Checked against the value rather than copied across. §3.3.5 forbids
+     * `TZID` on a `…Z` value and sabre answers 415 for the pair, and a
+     * date-shaped value needs `VALUE=DATE` or it is read as a malformed
+     * DATE-TIME. A `TZID` inherited from a residue copy therefore only survives
+     * where the value is still a local wall time.
+     *
+     * What this cannot repair is an `EXDATE` whose zone differs from `DTSTART`'s:
+     * one `timezone` column describes the task, so a value appended here is
+     * written in *that* zone. Legal, and vanishingly rare — every client that
+     * writes both writes them alike.
+     */
+    private fun paramsFor(value: String, inherited: List<ICalParam>): List<ICalParam> {
+        val values = value.split(',').map(String::trim).filter(String::isNotEmpty)
+        if (values.isEmpty()) return emptyList()
+        if (values.none { it.contains('T') }) return listOf(ICalParam("VALUE", "DATE"))
+        if (values.any { it.endsWith("Z") }) return emptyList()
+        return inherited.filter { it.name.equals("TZID", ignoreCase = true) }
+    }
+
     /**
      * True when a suppressing residue property no longer describes what its
      * column holds — i.e. the user has edited that field since it was imported,
@@ -317,9 +386,20 @@ object VTodoMapper {
      */
     private fun contradictsResidue(
         property: ICalProperty,
+        residue: ICalComponent,
         entity: TaskEntity,
         parentUid: String?,
     ): Boolean {
+        // Cardinality-many and list-valued: the column carries every copy's
+        // values merged, so it contradicts only when that whole set has changed
+        // — which is what deleting an occurrence does, and nothing else here
+        // does. All the copies are then dropped together and one authored line
+        // carries the lot, parameters included.
+        fun listChanged(column: String?): Boolean {
+            fun split(text: String?) =
+                text.orEmpty().split(',').map(String::trim).filter(String::isNotEmpty)
+            return split(column) != residue.properties(property.name).flatMap { split(it.value) }
+        }
         fun sameInstant(column: Instant?) =
             ICalValues.readInstant(ICalValues.parseTime(property)) == column
 
@@ -328,6 +408,8 @@ object VTodoMapper {
             "DUE" -> !sameInstant(entity.due)
             "COMPLETED" -> !sameInstant(entity.completedAt)
             "RECURRENCE-ID" -> !sameInstant(entity.recurrenceId)
+            "RDATE" -> listChanged(entity.rdate)
+            "EXDATE" -> listChanged(entity.exdate)
             "CREATED" -> !sameInstant(entity.createdAt)
             "LAST-MODIFIED" -> !sameInstant(entity.lastModified)
             // The residue only ever holds a STATUS we could not read, which left
@@ -337,7 +419,15 @@ object VTodoMapper {
             // A null parentUid is "the parent is not in this store", not "there is
             // no parent" — dropping the link there would destroy a relationship
             // over a row we simply have not fetched.
-            "RELATED-TO" -> parentUid != null && property.value.trim() != parentUid
+            //
+            // ⚠️ Scoped to RELTYPE=PARENT, exactly as the read side is. Nothing
+            // here models CHILD or SIBLING, and no RELATED-TO is ever claimed,
+            // so every one of them lives in the residue — where an unscoped
+            // comparison read a CHILD link as "a PARENT link that disagrees with
+            // the column" and dropped it on the next PUT.
+            "RELATED-TO" ->
+                (property.param("RELTYPE") ?: "PARENT").equals("PARENT", ignoreCase = true) &&
+                    parentUid != null && property.value.trim() != parentUid
             // Each of these reaches the residue only when it could not be parsed,
             // which left the column at its fallback. A column that has since moved
             // off that fallback is the user's edit, and it wins.

app/src/main/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpander.kt

@@ -159,14 +159,24 @@ object RecurrenceExpander {
             // anyway. Re-read the UNTIL's local fields in the series zone.
             val until = rule.until
             if (until != null && until.isFloating) {
+                // ⚠️ A DATE-valued UNTIL bounds the whole of its last day, not
+                // midnight on it. Its hour, minute and second fields are all
+                // zero, so rebuilding from them silently dropped every
+                // occurrence on the final day of a series anchored at any other
+                // time — five daily occurrences where there should be six, only
+                // away from UTC, which is to say for almost everyone. §3.3.10
+                // pairs a DATE UNTIL with a DATE DTSTART; Google and Apple emit
+                // it beside a timed one anyway, and end-of-day is the only
+                // reading of that which keeps the day the user can see.
+                val date = until.isAllDay
                 rule.until = DateTime(
                     zone,
                     until.year,
                     until.month,
                     until.dayOfMonth,
-                    until.hours,
-                    until.minutes,
-                    until.seconds,
+                    if (date) 23 else until.hours,
+                    if (date) 59 else until.minutes,
+                    if (date) 59 else until.seconds,
                 )
             }
         }

app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt

@@ -127,11 +127,93 @@ class VTodoMapperTest {
             assertThat(mapped.entity.isAllDay).isTrue()
             assertThat(mapped.entity.due).isNotNull()
         }
+        @Test
+        fun `every EXDATE line reaches the column the expander reads`() {
+            // ⚠️ Cardinality-many, and Apple Calendar writes one line per
+            // excluded occurrence. Reading only the first left the rest out of
+            // expansion, so a deleted occurrence came back in the list and in
+            // its reminders — while round-tripping to the server perfectly out
+            // of the residue, which is what made it invisible.
+            val mapped = VTodoMapper.read(
+                vtodo(
+                    "DTSTART:20260101T090000Z",
+                    "RRULE:FREQ=DAILY",
+                    "EXDATE:20260102T090000Z",
+                    "EXDATE:20260103T090000Z",
+                ),
+            )
+            assertThat(mapped.entity.exdate)
+                .isEqualTo("20260102T090000Z,20260103T090000Z")
+        }
+
+        @Test
+        fun `a parameterised EXDATE keeps its parameters by staying in the residue`() {
+            // Claiming it would drop the TZID for good: the property never
+            // reaches the residue and write re-emits it naked, so the exclusion
+            // stops matching the instance it excluded.
+            val mapped = VTodoMapper.read(
+                vtodo(
+                    "DTSTART;TZID=Europe/Berlin:20260101T090000",
+                    "RRULE:FREQ=DAILY",
+                    "EXDATE;TZID=Europe/Berlin:20260102T090000",
+                ),
+            )
+            assertThat(mapped.entity.exdate).isEqualTo("20260102T090000")
+            assertThat(mapped.entity.unknownProperties)
+                .contains("EXDATE;TZID=Europe/Berlin:20260102T090000")
+        }
     }
 
     @Nested
     inner class Writing {
 
+        @Test
+        fun `an unchanged parameterised EXDATE is written once, from the residue`() {
+            val mapped = VTodoMapper.read(
+                vtodo(
+                    "DTSTART;TZID=Europe/Berlin:20260101T090000",
+                    "RRULE:FREQ=DAILY",
+                    "EXDATE;TZID=Europe/Berlin:20260102T090000",
+                ),
+            )
+
+            val written = VTodoMapper.write(mapped.entity)
+
+            val exdates = written.properties("EXDATE")
+            assertThat(exdates).hasSize(1)
+            assertThat(exdates.single().param("TZID")).isEqualTo("Europe/Berlin")
+        }
+
+        @Test
+        fun `a changed EXDATE is authored with the parameters the residue gives up`() {
+            val mapped = VTodoMapper.read(
+                vtodo(
+                    "DTSTART;TZID=Europe/Berlin:20260101T090000",
+                    "RRULE:FREQ=DAILY",
+                    "EXDATE;TZID=Europe/Berlin:20260102T090000",
+                ),
+            )
+            // What deleting a second occurrence leaves behind.
+            val excepted = mapped.entity.copy(
+                exdate = "${mapped.entity.exdate},20260103T090000",
+            )
+
+            val written = VTodoMapper.write(excepted)
+
+            val exdate = written.properties("EXDATE").single()
+            assertThat(exdate.value).isEqualTo("20260102T090000,20260103T090000")
+            assertThat(exdate.param("TZID")).isEqualTo("Europe/Berlin")
+        }
+
+        @Test
+        fun `a date-shaped EXDATE we author carries VALUE=DATE`() {
+            // §3.3.5's default is DATE-TIME, so a bare eight-digit value is
+            // malformed — a permanent 415 under Prefer: handling=strict.
+            val entity = entity().copy(isAllDay = true, exdate = "20260102")
+            val exdate = VTodoMapper.write(entity).properties("EXDATE").single()
+            assertThat(exdate.param("VALUE")).isEqualTo("DATE")
+        }
+
         @Test
         fun `SEQUENCE is preserved verbatim and never bumped`() {
             // It is the Organizer's revision counter (RFC 5545 section 3.8.7.4);

app/src/test/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpanderTest.kt

@@ -154,6 +154,35 @@ class RecurrenceExpanderTest {
         ).inOrder()
     }
 
+    @Test
+    fun `a date-valued UNTIL covers the whole of its last day`() {
+        // ⚠️ A DATE-valued UNTIL is midnight, and the repair below rebuilt it
+        // from hours/minutes/seconds — all zero for a DATE — so a series anchored
+        // at 09:00 Berlin lost every occurrence on the final day. RFC 5545
+        // §3.3.10 puts the bound at the end of that date.
+        val result = expand(
+            spec(rrule = "FREQ=DAILY;UNTIL=20250109", anchor = "2025-01-07T08:00:00Z"),
+        )
+        assertThat(result).containsExactly(
+            "2025-01-07T08:00:00Z",
+            "2025-01-08T08:00:00Z",
+            "2025-01-09T08:00:00Z",
+        ).inOrder()
+    }
+
+    @Test
+    fun `an all-day series with a date-valued UNTIL keeps its last day`() {
+        val result = expand(
+            spec(
+                rrule = "FREQ=DAILY;UNTIL=20250109",
+                anchor = "2025-01-07T00:00:00Z",
+                isAllDay = true,
+                timeZone = null,
+            ),
+        )
+        assertThat(result).hasSize(3)
+    }
+
     @Test
     fun `an unbounded rule stops at the occurrence ceiling`() {
         val result = expand(