From a30114efbba42564d2cdc3470d801a6fe4c24ccf Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 4 Sep 2026 16:45:03 +0200 Subject: [PATCH] sync(chunk 1): VTODO mapper with an unknown-property round-trip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hand-rolled content-line model instead of ical4j: a raw (name, params, value) tree is what the round-trip requirement wants, and a typed model normalises away exactly what has to survive. lib-recur already does RRULE and java.time is native at minSdk 29, so the 2.2 MB of zone data and the registry shims buy nothing. Deviates from SYNC.md's library table — see SYNC-PLAN.md decision 4. - domain/ical: parser, serialiser, value codecs. No Android, no data types, so the floret-kit extraction stays a file move. - data/tasks/ical/VTodoMapper: VTODO <-> TaskEntity. Claims a property only when it can reproduce it exactly; everything else round-trips verbatim through TaskEntity.unknown_properties, which already existed at v1 — no migration needed, and BEGIN/END lines carry the nesting the plan thought needed a second table. - 19 fixtures as the specification, with the canonical comparison harness from SYNC.md: no property lost, modulo the enumerated allowlist. - ICalendarWriter now delegates folding and escaping rather than carrying its own copy. VALARM ownership settled: neither side writes the other's alarms. VALARMs round-trip in the residue, local reminders stay in task_alarms. The setAlarm collision was the provider's; the two stores are now disjoint. --- .../agendula/data/tasks/ical/VTodoMapper.kt | 442 ++++++++++++++++++ .../agendula/domain/export/ICalendarWriter.kt | 57 +-- .../agendula/domain/ical/ICalModel.kt | 60 +++ .../agendula/domain/ical/ICalParser.kt | 149 ++++++ .../agendula/domain/ical/ICalSerializer.kt | 100 ++++ .../agendula/domain/ical/ICalValues.kt | 135 ++++++ .../agendula/data/tasks/ical/ICalCanonical.kt | 79 ++++ .../data/tasks/ical/VTodoMapperTest.kt | 304 ++++++++++++ .../data/tasks/ical/VTodoRoundTripTest.kt | 161 +++++++ .../agendula/domain/ical/ICalParserTest.kt | 154 ++++++ .../domain/ical/ICalSerializerTest.kt | 64 +++ app/src/test/resources/vtodo/README.md | 32 ++ .../test/resources/vtodo/apple-sort-order.ics | 11 + .../resources/vtodo/completion-model-a.ics | 22 + .../resources/vtodo/completion-model-b.ics | 12 + .../resources/vtodo/completion-model-c.ics | 14 + .../resources/vtodo/completion-model-d.ics | 20 + .../test/resources/vtodo/floating-time.ics | 11 + app/src/test/resources/vtodo/folded-utf8.ics | 12 + .../resources/vtodo/malformed-scalars.ics | 14 + .../resources/vtodo/mixed-value-types.ics | 11 + .../test/resources/vtodo/moz-alarm-props.ics | 18 + .../vtodo/nextcloud-shared-confidential.ics | 12 + .../resources/vtodo/quoted-param-colon.ics | 11 + .../vtodo/recurring-master-overrides.ics | 33 ++ .../resources/vtodo/rrule-due-no-dtstart.ics | 12 + .../test/resources/vtodo/two-timezones.ics | 11 + .../resources/vtodo/uid-special-chars.ics | 9 + .../resources/vtodo/unknown-component.ics | 13 + app/src/test/resources/vtodo/unknown-tzid.ics | 19 + .../resources/vtodo/valarm-unknown-nested.ics | 18 + docs/SYNC-PLAN.md | 38 +- 32 files changed, 2010 insertions(+), 48 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalModel.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalParser.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializer.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ICalCanonical.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalParserTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializerTest.kt create mode 100644 app/src/test/resources/vtodo/README.md create mode 100644 app/src/test/resources/vtodo/apple-sort-order.ics create mode 100644 app/src/test/resources/vtodo/completion-model-a.ics create mode 100644 app/src/test/resources/vtodo/completion-model-b.ics create mode 100644 app/src/test/resources/vtodo/completion-model-c.ics create mode 100644 app/src/test/resources/vtodo/completion-model-d.ics create mode 100644 app/src/test/resources/vtodo/floating-time.ics create mode 100644 app/src/test/resources/vtodo/folded-utf8.ics create mode 100644 app/src/test/resources/vtodo/malformed-scalars.ics create mode 100644 app/src/test/resources/vtodo/mixed-value-types.ics create mode 100644 app/src/test/resources/vtodo/moz-alarm-props.ics create mode 100644 app/src/test/resources/vtodo/nextcloud-shared-confidential.ics create mode 100644 app/src/test/resources/vtodo/quoted-param-colon.ics create mode 100644 app/src/test/resources/vtodo/recurring-master-overrides.ics create mode 100644 app/src/test/resources/vtodo/rrule-due-no-dtstart.ics create mode 100644 app/src/test/resources/vtodo/two-timezones.ics create mode 100644 app/src/test/resources/vtodo/uid-special-chars.ics create mode 100644 app/src/test/resources/vtodo/unknown-component.ics create mode 100644 app/src/test/resources/vtodo/unknown-tzid.ics create mode 100644 app/src/test/resources/vtodo/valarm-unknown-nested.ics diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt new file mode 100644 index 0000000..6d22260 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt @@ -0,0 +1,442 @@ +package de.jeanlucmakiola.agendula.data.tasks.ical + +import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity +import de.jeanlucmakiola.agendula.domain.PRIORITY_NONE +import de.jeanlucmakiola.agendula.domain.TaskStatus +import de.jeanlucmakiola.agendula.domain.ical.ICalComponent +import de.jeanlucmakiola.agendula.domain.ical.ICalParam +import de.jeanlucmakiola.agendula.domain.ical.ICalParser +import de.jeanlucmakiola.agendula.domain.ical.ICalProperty +import de.jeanlucmakiola.agendula.domain.ical.ICalSerializer +import de.jeanlucmakiola.agendula.domain.ical.ICalValues +import java.time.ZoneId +import kotlin.time.Clock +import kotlin.time.Instant + +/** + * VTODO ↔ [TaskEntity]. + * + * ## The residue + * + * Everything the mapper does not claim — unknown properties, unknown parameters, + * `VALARM`s, whole unknown sub-components — is serialised verbatim into + * [TaskEntity.unknownProperties] and re-emitted on write. RFC 5545 §3.1 requires + * it ("Applications MUST preserve the value data for x-name and iana-token + * values that they don't recognize"), and failing it destroys other people's + * data invisibly — invisible in our own UI precisely because we are the client + * that does not understand the property. + * + * ## What "claimed" means, and why it is narrow + * + * The mapper claims a property only when it can **reproduce it exactly** from + * its columns. Everything else stays in the residue and round-trips untouched: + * + * - a value it cannot parse or that is out of range (`PRIORITY:11`, + * `PERCENT-COMPLETE:abc`, `STATUS:X-DEFERRED`, `SEQUENCE:x`) — clamping or + * defaulting these would be a silent rewrite of somebody's data; + * - a time it can read but not reproduce — a floating stamp, or a `TZID` this + * device's tzdb has never heard of. The column still gets a best-effort + * instant so the UI has something to show; + * - a `DTSTART`/`DUE` pair that disagrees on value type or timezone, where + * authoring both from one `is_all_day` flag and one `timezone` column would + * destroy the odd one out. + * + * ## Residue eviction + * + * A suppressed property is only suppressed while it still *agrees* with its + * column. [contradictsResidue] compares the two on write: if the user has since + * edited that field, the stale residue copy is evicted and the column is + * authored. Without this, editing the due date of a task imported with a + * floating `DUE` would silently do nothing on the server. + * + * ## Alarms + * + * `VALARM`s round-trip in the residue and are never authored here. Local + * reminders live in `task_alarms` and are never serialised. The two stores are + * disjoint, so neither can destroy the other — which is what `docs/SYNC.md`'s + * `setAlarm` collision was actually about, and it does not survive into the + * own-store world. Merging them is a later decision, not a silent one. + */ +object VTodoMapper { + + /** + * DAVx5's limit, and ours for the same two reasons: Android's `CursorWindow` + * row cap, and `CALDAV:max-resource-size`, whose violation is a failed PUT. + */ + const val MAX_RESIDUE_BYTES = 25 * 1024 + + /** + * Cardinality-one properties the mapper authors from a column, and therefore + * must not author while the residue still holds the original. + */ + private val SUPPRESSED_BY_RESIDUE = setOf( + "DTSTART", "DUE", "COMPLETED", "RECURRENCE-ID", "CREATED", "LAST-MODIFIED", + "STATUS", "RELATED-TO", + ) + + /** What a VTODO yields. Row identity ([TaskEntity.id], `listId`) is the caller's. */ + data class Mapped( + val entity: TaskEntity, + /** `RELATED-TO;RELTYPE=PARENT`, for the caller to resolve to a row id. */ + val parentUid: String?, + /** Set when the source carried no `UID` and the caller must mint one. */ + val uidWasMissing: Boolean, + /** Residue that exceeded [MAX_RESIDUE_BYTES] and had to be dropped. */ + val droppedResidue: Boolean, + ) + + // ---------------------------------------------------------------- read + + fun read(vtodo: ICalComponent, listId: Long = 0L): Mapped { + // Claims are tracked by index, not by value: two identical content lines + // are two pieces of data, and claiming one must not swallow the other on + // the way into the residue. + val claimed = mutableSetOf() + fun claim(property: ICalProperty?) { + property?.let { p -> + vtodo.properties.indexOfFirst { it === p }.takeIf { it >= 0 }?.let(claimed::add) + } + } + + /** Claims [property] only if [value] could be read from it. */ + fun take(property: ICalProperty?, value: T?): T? = + value?.also { claim(property) } + + 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. + val uid = take(uidProperty, uidProperty?.value?.trim()).orEmpty() + + val dtstart = readTime(vtodo, "DTSTART") + val due = readTime(vtodo, "DUE") + + // A task's primary stamp is DUE; DTSTART only decides when there is no + // DUE. Deriving one all-day flag from "either is a DATE" turns a + // date/date-time pair into two DATEs and destroys the time half. + val allDay = if (due.present) due.isDate else dtstart.isDate + val timezone = dtstart.tzid ?: due.tzid + + // Claim a stamp only when the single is_all_day flag and the single + // timezone column can reproduce it. Otherwise it stays in the residue and + // is re-emitted exactly as it arrived. + if (dtstart.reproducible(allDay, timezone)) claim(dtstart.property) + if (due.reproducible(allDay, timezone)) claim(due.property) + + val recurrenceId = readTime(vtodo, "RECURRENCE-ID") + if (recurrenceId.reproducible(allDay, timezone)) claim(recurrenceId.property) + + // These three are UTC-only on the way out, so a zoned or date-valued one + // is not reproducible. + val completed = readTime(vtodo, "COMPLETED") + if (completed.isUtcDateTime) claim(completed.property) + val created = readTime(vtodo, "CREATED") + if (created.isUtcDateTime) claim(created.property) + val lastModified = readTime(vtodo, "LAST-MODIFIED") + if (lastModified.isUtcDateTime) claim(lastModified.property) + + val statusProperty = vtodo.property("STATUS") + val status = take( + statusProperty, + when (statusProperty?.value?.trim()?.uppercase()) { + "NEEDS-ACTION" -> TaskStatus.NEEDS_ACTION + "IN-PROCESS" -> TaskStatus.IN_PROCESS + "COMPLETED" -> TaskStatus.COMPLETED + "CANCELLED" -> TaskStatus.CANCELLED + else -> null + }, + ) ?: TaskStatus.NEEDS_ACTION + + val percentProperty = vtodo.property("PERCENT-COMPLETE") + val percent = take( + percentProperty, + percentProperty?.value?.trim()?.toIntOrNull()?.takeIf { it in 0..100 }, + ) + + // 0 = undefined, 1 = highest, 9 = lowest. Stored raw: bucketing on the way + // in would rewrite a server's PRIORITY:3 as 1 and lose it on write-back. + val priorityProperty = vtodo.property("PRIORITY") + val priority = take( + priorityProperty, + priorityProperty?.value?.trim()?.toIntOrNull()?.takeIf { it in 0..9 }, + ) ?: PRIORITY_NONE + + val classProperty = vtodo.property("CLASS") + val classification = take( + classProperty, + CLASS_NAMES.indexOf(classProperty?.value?.trim()?.uppercase()).takeIf { it >= 0 }, + ) + + val sequenceProperty = vtodo.property("SEQUENCE") + val sequence = take( + sequenceProperty, + sequenceProperty?.value?.trim()?.toIntOrNull()?.takeIf { it >= 0 }, + ) ?: 0 + + // RELTYPE defaults to PARENT (§3.2.15), and §3.8.4.5 reads backwards to + // most implementers: the *referencing* component is the subordinate one, + // so this points at our parent. + // + // Deliberately **not** claimed. TaskEntity holds only a local `parent_id`, + // so a parent that is not in this store — not fetched yet, in another + // collection, deleted locally — would leave nothing to write back and the + // relationship would be destroyed. Keeping it in the residue means the + // link survives even when the parent row does not. + val parentUid = vtodo.properties("RELATED-TO") + .firstOrNull { (it.param("RELTYPE") ?: "PARENT").equals("PARENT", ignoreCase = true) } + ?.value?.trim()?.takeIf { it.isNotEmpty() } + + val entity = TaskEntity( + listId = listId, + uid = uid, + title = take(vtodo.property("SUMMARY"), vtodo.property("SUMMARY")?.text()), + description = take(vtodo.property("DESCRIPTION"), vtodo.property("DESCRIPTION")?.text()), + location = take(vtodo.property("LOCATION"), vtodo.property("LOCATION")?.text()), + // URI, not TEXT — escaping it would corrupt a query string. + url = take(vtodo.property("URL"), vtodo.property("URL")?.value?.trim()), + status = status, + percentComplete = percent, + completedAt = completed.instant, + priority = priority, + classification = classification, + dtstart = dtstart.instant, + due = due.instant, + duration = take(vtodo.property("DURATION"), vtodo.property("DURATION")?.value?.trim()), + 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()), + recurrenceId = recurrenceId.instant, + createdAt = created.instant, + lastModified = lastModified.instant, + // The Organizer's revision counter (§3.8.7.4) — preserved verbatim, + // never ours to bump. + sequence = sequence, + ) + + // DTSTAMP is regenerated on every serialisation and carries no state, so + // it is dropped rather than stored. LAST-MODIFIED, which does carry + // state, is a column above — conflating the two makes every sync look + // like an edit. + val residueProperties = vtodo.properties + .filterIndexed { index, _ -> index !in claimed } + .filterNot { it.name.equals("DTSTAMP", ignoreCase = true) } + + val residueText = ICalSerializer.serializeProperties(residueProperties) + + ICalSerializer.serializeAll(vtodo.components) + val tooLarge = residueText.toByteArray(Charsets.UTF_8).size > MAX_RESIDUE_BYTES + + return Mapped( + entity = entity.copy( + unknownProperties = residueText.takeIf { it.isNotEmpty() && !tooLarge }, + ), + parentUid = parentUid, + uidWasMissing = uid.isEmpty(), + droppedResidue = tooLarge, + ) + } + + // --------------------------------------------------------------- write + + /** + * Serialises [entity] back to a VTODO. + * + * [parentUid] is the parent's `UID`, which the entity holds only as a row id. + * [now] is the `DTSTAMP`. + */ + fun write( + entity: TaskEntity, + parentUid: String? = null, + now: Instant = Clock.System.now(), + ): ICalComponent { + val residue = parseResidue(entity.unknownProperties) + val keptResidue = residue.properties.filterNot { contradictsResidue(it, entity, parentUid) } + val suppressed = keptResidue + .map { it.name.uppercase() } + .filterTo(mutableSetOf()) { it in SUPPRESSED_BY_RESIDUE } + + val properties = mutableListOf() + fun add(name: String, value: String?, vararg params: ICalParam) { + if (value == null || name.uppercase() in suppressed) return + properties += ICalProperty(name, params.toList(), value) + } + fun addTime(name: String, instant: Instant?) { + if (instant == null || name in suppressed) return + properties += timeProperty(name, instant, entity) + } + + add("UID", entity.uid) + add("DTSTAMP", ICalValues.formatDateTime(now, null)) + add("SEQUENCE", entity.sequence.toString()) + add("SUMMARY", entity.title?.let(ICalValues::escapeText)) + add("DESCRIPTION", entity.description?.let(ICalValues::escapeText)) + add("LOCATION", entity.location?.let(ICalValues::escapeText)) + add("URL", entity.url) + + add("STATUS", entity.status.toICalName()) + add("PERCENT-COMPLETE", entity.percentComplete?.toString()) + // §3.8.2.1: COMPLETED MUST be UTC — no TZID, no floating, no DATE. + add("COMPLETED", entity.completedAt?.let { ICalValues.formatDateTime(it, null) }) + add("PRIORITY", entity.priority.takeIf { it != PRIORITY_NONE }?.toString()) + add("CLASS", entity.classification?.let { CLASS_NAMES.getOrNull(it) }) + + addTime("DTSTART", entity.dtstart) + addTime("DUE", entity.due) + add("DURATION", entity.duration) + + add("RRULE", entity.rrule) + add("RDATE", entity.rdate) + add("EXDATE", entity.exdate) + addTime("RECURRENCE-ID", entity.recurrenceId) + + add("CREATED", entity.createdAt?.let { ICalValues.formatDateTime(it, null) }) + add("LAST-MODIFIED", entity.lastModified?.let { ICalValues.formatDateTime(it, null) }) + + if (parentUid != null && "RELATED-TO" !in suppressed) { + properties += ICalProperty("RELATED-TO", listOf(ICalParam("RELTYPE", "PARENT")), parentUid) + } + + return ICalComponent( + name = "VTODO", + properties = properties + keptResidue, + components = residue.components, + ) + } + + /** + * 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, + * so the stale copy must go and the column must be authored instead. + */ + private fun contradictsResidue( + property: ICalProperty, + entity: TaskEntity, + parentUid: String?, + ): Boolean { + fun sameInstant(column: Instant?) = + ICalValues.readInstant(ICalValues.parseTime(property)) == column + + return when (property.name.uppercase()) { + "DTSTART" -> !sameInstant(entity.dtstart) + "DUE" -> !sameInstant(entity.due) + "COMPLETED" -> !sameInstant(entity.completedAt) + "RECURRENCE-ID" -> !sameInstant(entity.recurrenceId) + "CREATED" -> !sameInstant(entity.createdAt) + "LAST-MODIFIED" -> !sameInstant(entity.lastModified) + // The residue only ever holds a STATUS we could not read, which left + // the column at its NEEDS-ACTION fallback. Anything else means the + // user has since set a real status. + "STATUS" -> entity.status != TaskStatus.NEEDS_ACTION + // 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 + else -> false + } + } + + /** + * A `VALARM` with `TRIGGER;RELATED=END` needs `DUE`, or `DTSTART` plus + * `DURATION` (§3.8.6.3). Clearing the due date on a task that has an + * end-relative reminder produces a resource the server rejects permanently — + * and it is reachable from ordinary UI actions, so it is checked before PUT + * rather than discovered as a 415. + */ + fun validate(vtodo: ICalComponent): List { + val problems = mutableListOf() + val due = vtodo.property("DUE") + val dtstart = vtodo.property("DTSTART") + val duration = vtodo.property("DURATION") + + if (due != null && dtstart != null) { + val start = ICalValues.readInstant(ICalValues.parseTime(dtstart)) + val end = ICalValues.readInstant(ICalValues.parseTime(due)) + // sabre answers 415 for both of these, not a 4xx that names them. + if (start != null && end != null && end < start) problems += "DUE precedes DTSTART" + if (isDateValue(dtstart) != isDateValue(due)) { + problems += "DTSTART and DUE disagree on value type" + } + } + + if (due == null && (dtstart == null || duration == null)) { + val endRelative = vtodo.components("VALARM").any { alarm -> + alarm.property("TRIGGER")?.param("RELATED").equals("END", ignoreCase = true) + } + if (endRelative) problems += "TRIGGER;RELATED=END with neither DUE nor DTSTART+DURATION" + } + + if (vtodo.property("METHOD") != null) problems += "METHOD is not allowed on a stored resource" + return problems + } + + // -------------------------------------------------------------- helpers + + private val CLASS_NAMES = listOf("PUBLIC", "PRIVATE", "CONFIDENTIAL") + + private class TimeRead( + val property: ICalProperty?, + val instant: Instant?, + val tzid: String?, + val isDate: Boolean, + val representable: Boolean, + ) { + val present get() = property != null + + /** True when one `is_all_day` flag and one `timezone` column reproduce it. */ + fun reproducible(allDay: Boolean, timezone: String?) = + present && representable && isDate == allDay && tzid == timezone + + /** True when it is a UTC date-time — the only form we author for these. */ + val isUtcDateTime get() = present && representable && !isDate && tzid == null + } + + private fun readTime(vtodo: ICalComponent, name: String): TimeRead { + val property = vtodo.property(name) ?: return TimeRead(null, null, null, false, false) + return when (val value = ICalValues.parseTime(property)) { + is ICalValues.TimeValue.Date -> TimeRead(property, value.instant, null, true, true) + is ICalValues.TimeValue.Timed -> TimeRead(property, value.instant, value.tzid, false, true) + // Readable but not reproducible: the column gets the best-effort + // instant, the property round-trips from the residue verbatim. + is ICalValues.TimeValue.Unrepresentable -> + TimeRead(property, value.instant, null, false, false) + } + } + + private fun timeProperty(name: String, instant: Instant, entity: TaskEntity): ICalProperty { + if (entity.isAllDay) { + return ICalProperty( + name, + listOf(ICalParam("VALUE", "DATE")), + ICalValues.formatDate(instant), + ) + } + // Resolve the zone once. Letting the parameter and the value each decide + // separately produces TZID on a `…Z` value, which §3.3.5 forbids and + // sabre answers 415 for. + val tzid = entity.timezone?.takeIf { runCatching { ZoneId.of(it) }.isSuccess } + val params = if (tzid == null) emptyList() else listOf(ICalParam("TZID", tzid)) + return ICalProperty(name, params, ICalValues.formatDateTime(instant, tzid)) + } + + /** The same DATE test [ICalValues.parseTime] applies, so the two cannot disagree. */ + private fun isDateValue(property: ICalProperty): Boolean = + ICalValues.parseTime(property) is ICalValues.TimeValue.Date + + private fun ICalProperty.text(): String = ICalValues.unescapeText(value) + + private fun parseResidue(text: String?): ICalComponent { + if (text.isNullOrEmpty()) return ICalComponent("VTODO") + // The residue is stored as bare properties followed by whole sub-component + // blocks, so it parses as the body of a VTODO with the wrapper restored. + return runCatching { + ICalParser.parse("BEGIN:VTODO\r\n$text\r\nEND:VTODO\r\n") + }.getOrElse { ICalComponent("VTODO") } + } + + private fun TaskStatus.toICalName(): String = when (this) { + TaskStatus.NEEDS_ACTION -> "NEEDS-ACTION" + TaskStatus.IN_PROCESS -> "IN-PROCESS" + TaskStatus.COMPLETED -> "COMPLETED" + TaskStatus.CANCELLED -> "CANCELLED" + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt index 520fa7a..1f6afe2 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt @@ -3,6 +3,8 @@ package de.jeanlucmakiola.agendula.domain.export import de.jeanlucmakiola.agendula.domain.Priority import de.jeanlucmakiola.agendula.domain.TaskStatus import de.jeanlucmakiola.agendula.domain.calendarDate +import de.jeanlucmakiola.agendula.domain.ical.ICalSerializer +import de.jeanlucmakiola.agendula.domain.ical.ICalValues import de.jeanlucmakiola.agendula.domain.toICal import java.time.ZoneOffset import java.time.format.DateTimeFormatter @@ -30,9 +32,6 @@ object ICalendarWriter { private const val PRODUCT_ID = "-//Jean-Luc Makiola//Agendula//EN" - /** RFC 5545 caps a content line at 75 octets, excluding the CRLF. */ - private const val MAX_LINE_OCTETS = 75 - private val DATE = DateTimeFormatter.ofPattern("yyyyMMdd") private val DATE_TIME_UTC = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'") @@ -145,53 +144,17 @@ object ICalendarWriter { } /** - * Escapes a TEXT value per RFC 5545 §3.3.11. Backslash first, or it would - * double the backslashes introduced by the later replacements. - */ - internal fun escapeText(value: String): String = value - .replace("\\", "\\\\") - .replace(";", "\\;") - .replace(",", "\\,") - .replace("\r\n", "\\n") - .replace("\n", "\\n") - .replace("\r", "\\n") - - /** - * Folds a content line to at most [MAX_LINE_OCTETS] octets, continuing with - * CRLF + a single space. + * TEXT escaping and folding are the same operations the sync mapper needs, + * and having two implementations of either is how the two halves drift. + * These delegate; the implementations live in `domain/ical/`. * - * Counted in **octets, not characters** — the limit is defined that way, and an - * emoji in a task title is four of them. Splits are kept on character - * boundaries so folding can never cut a UTF-8 sequence in half and corrupt the - * text; an importer unfolds by removing CRLF + leading whitespace, recovering - * the original exactly. + * This writer itself stays separate from `VTodoMapper` on purpose: it + * serialises **domain** export models, which exist in both storage modes, + * whereas the mapper serialises Room entities, which exist only in one. */ - internal fun fold(content: String): String { - if (content.utf8Size() <= MAX_LINE_OCTETS) return content + internal fun escapeText(value: String): String = ICalValues.escapeText(value) - val out = StringBuilder() - var octets = 0 - // First line takes the full budget; every continuation loses one octet to - // the leading space. - var budget = MAX_LINE_OCTETS - var index = 0 - while (index < content.length) { - val codePoint = content.codePointAt(index) - val charCount = Character.charCount(codePoint) - val size = String(Character.toChars(codePoint)).utf8Size() - if (octets + size > budget) { - out.append(CRLF).append(' ') - octets = 0 - budget = MAX_LINE_OCTETS - 1 - } - out.append(content, index, index + charCount) - octets += size - index += charCount - } - return out.toString() - } - - private fun String.utf8Size(): Int = toByteArray(Charsets.UTF_8).size + internal fun fold(content: String): String = ICalSerializer.fold(content) private fun TaskStatus.toICalName(): String = when (this) { TaskStatus.NEEDS_ACTION -> "NEEDS-ACTION" diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalModel.kt new file mode 100644 index 0000000..6653bf1 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalModel.kt @@ -0,0 +1,60 @@ +package de.jeanlucmakiola.agendula.domain.ical + +/** + * A parameter on a content line: `TZID=Europe/Berlin`, `MEMBER="a","b"`. + * + * Values are held **unquoted**. Quoting is optional in RFC 5545 and carries no + * meaning, so it is normalised away on parse and reapplied on serialise only + * where the grammar forces it — `docs/SYNC-PLAN.md` puts parameter quoting on + * the round-trip allowlist for exactly this reason. + */ +data class ICalParam(val name: String, val values: List) { + constructor(name: String, value: String) : this(name, listOf(value)) +} + +/** + * One content line. + * + * [value] is kept **exactly as it arrived**, unfolded but still escaped. That is + * deliberate and it is the whole reason this model exists instead of a typed + * one: a property we do not model is re-emitted from this string verbatim, so it + * cannot be normalised, reordered inside itself, or lost. Decoding happens in + * the mapper, for the properties the mapper actually claims. + */ +data class ICalProperty( + val name: String, + val params: List = emptyList(), + val value: String, +) { + /** First value of [name], unquoted, or `null`. */ + fun param(name: String): String? = + params.firstOrNull { it.name.equals(name, ignoreCase = true) }?.values?.firstOrNull() +} + +/** A `BEGIN:`/`END:` block — `VCALENDAR`, `VTODO`, `VALARM`, `VTIMEZONE`, or one we don't know. */ +data class ICalComponent( + val name: String, + val properties: List = emptyList(), + val components: List = emptyList(), +) { + fun property(name: String): ICalProperty? = + properties.firstOrNull { it.name.equals(name, ignoreCase = true) } + + fun properties(name: String): List = + properties.filter { it.name.equals(name, ignoreCase = true) } + + fun components(name: String): List = + components.filter { it.name.equals(name, ignoreCase = true) } + + /** This component with every property in [names] removed. Sub-components are untouched. */ + fun without(names: Set): ICalComponent { + val upper = names.map { it.uppercase() }.toSet() + return copy(properties = properties.filterNot { it.name.uppercase() in upper }) + } + + /** True when nothing survives — no properties and no sub-components worth keeping. */ + fun isEmpty(): Boolean = properties.isEmpty() && components.isEmpty() +} + +/** Thrown when input is not recoverable as iCalendar at all. */ +class ICalParseException(message: String) : Exception(message) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalParser.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalParser.kt new file mode 100644 index 0000000..ba09c20 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalParser.kt @@ -0,0 +1,149 @@ +package de.jeanlucmakiola.agendula.domain.ical + +/** + * Parses RFC 5545 text into an [ICalComponent] tree. + * + * Lexical only: it splits content lines into name, parameters and value and + * nests `BEGIN`/`END` blocks. It does not interpret a single value — that is the + * mapper's job, and keeping the two apart is what lets an unrecognised property + * survive a read-modify-write cycle untouched. + * + * Deliberately tolerant, because servers and other clients are not careful: + * unknown components nest like any other, a stray `END` without its `BEGIN` is + * ignored rather than fatal, and a line with no colon is skipped. Only a missing + * outer component is fatal. + */ +object ICalParser { + + fun parse(text: String): ICalComponent = + parseAll(text).firstOrNull() ?: throw ICalParseException("no component found") + + /** Every top-level component in [text]. Normally one `VCALENDAR`. */ + fun parseAll(text: String): List { + val roots = mutableListOf() + val stack = ArrayDeque() + + for (line in unfold(text)) { + val property = parseLine(line) ?: continue + when { + property.name.equals("BEGIN", ignoreCase = true) -> + stack.addLast(Builder(property.value.trim().uppercase())) + + property.name.equals("END", ignoreCase = true) -> { + // Close by *name*, not by position. A resource with a missing + // END:VTODO would otherwise have its END:VCALENDAR close the + // VTODO, leave VCALENDAR open, and end with no component at + // all — discarding a whole multiget response over one + // malformed task. + val name = property.value.trim().uppercase() + val depth = stack.indexOfLast { it.name == name } + if (depth < 0) continue + repeat(stack.size - depth) { close(stack, roots) } + } + + // A property before any BEGIN is malformed; dropping it is the only + // option that doesn't invent a component to hang it on. + else -> stack.lastOrNull()?.properties?.add(property) + } + } + // Anything still open at end of input is missing its END. Keeping it is + // strictly better than dropping it. + while (stack.isNotEmpty()) close(stack, roots) + return roots + } + + private fun close(stack: ArrayDeque, roots: MutableList) { + val component = stack.removeLast().build() + val parent = stack.lastOrNull() + if (parent == null) roots += component else parent.components += component + } + + /** + * Splits [text] into unfolded content lines. + * + * RFC 5545 §3.1: a CRLF followed by a single space or tab is a fold and both + * are removed. Bare LF is accepted because plenty of real files carry it, and + * a leading BOM is stripped. + */ + internal fun unfold(text: String): List { + val lines = mutableListOf() + val normalised = text.removePrefix("") + for (raw in normalised.split("\r\n", "\n", "\r")) { + if (raw.isEmpty()) continue + val continuation = raw[0] == ' ' || raw[0] == '\t' + if (continuation && lines.isNotEmpty()) { + lines.last().append(raw, 1, raw.length) + } else { + lines += StringBuilder(raw) + } + } + return lines.map { it.toString() } + } + + /** + * Splits one unfolded line into name, parameters and value. + * + * The value separator is the first colon **outside a quoted parameter value** + * — `ATTENDEE;CN="Smith, J:r":mailto:x` has three colons and only the third + * one ends the parameter list. + */ + internal fun parseLine(line: String): ICalProperty? { + var quoted = false + var colon = -1 + for (i in line.indices) { + val c = line[i] + if (c == '"') quoted = !quoted + else if (c == ':' && !quoted) { colon = i; break } + } + // An unbalanced quote in the parameter section leaves the scan stuck + // inside a quoted string forever. Falling back to the first colon keeps + // the line instead of dropping it whole. + if (colon < 0) colon = line.indexOf(':') + if (colon < 0) return null + + val head = line.substring(0, colon) + val value = line.substring(colon + 1) + + val segments = splitUnquoted(head, ';') + val name = segments.firstOrNull()?.trim().orEmpty() + if (name.isEmpty()) return null + + val params = segments.drop(1).mapNotNull { segment -> + val eq = segment.indexOf('=') + // A parameter with no '=' is non-conformant. Keep it as a valueless + // parameter rather than dropping it — it is still someone's data. + if (eq < 0) return@mapNotNull ICalParam(segment.trim(), emptyList()) + val paramName = segment.substring(0, eq).trim() + if (paramName.isEmpty()) return@mapNotNull null + val values = splitUnquoted(segment.substring(eq + 1), ',').map { it.unquote() } + ICalParam(paramName, values) + } + + return ICalProperty(name, params, value) + } + + /** Splits on [delimiter], ignoring delimiters inside double quotes. */ + private fun splitUnquoted(text: String, delimiter: Char): List { + val out = mutableListOf() + val current = StringBuilder() + var quoted = false + for (c in text) { + when { + c == '"' -> { quoted = !quoted; current.append(c) } + c == delimiter && !quoted -> { out += current.toString(); current.clear() } + else -> current.append(c) + } + } + out += current.toString() + return out + } + + private fun String.unquote(): String = + if (length >= 2 && startsWith('"') && endsWith('"')) substring(1, length - 1) else this + + private class Builder(val name: String) { + val properties = mutableListOf() + val components = mutableListOf() + fun build() = ICalComponent(name, properties.toList(), components.toList()) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializer.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializer.kt new file mode 100644 index 0000000..c16b8db --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializer.kt @@ -0,0 +1,100 @@ +package de.jeanlucmakiola.agendula.domain.ical + +/** + * Serialises an [ICalComponent] tree back to RFC 5545 text. + * + * The inverse of [ICalParser] for everything the parser preserves: property + * order within a component, parameter order, and property values byte-for-byte. + * What it does *not* preserve is fold position and parameter quoting, neither of + * which carries information — both are on the round-trip allowlist. + */ +object ICalSerializer { + + private const val CRLF = "\r\n" + + /** RFC 5545 §3.1 caps a content line at 75 octets, excluding the CRLF. */ + private const val MAX_LINE_OCTETS = 75 + + fun serialize(component: ICalComponent): String = buildString { + write(component) + } + + /** Serialises [components] one after another — the form the residue is stored in. */ + fun serializeAll(components: List): String = buildString { + components.forEach { write(it) } + } + + /** Serialises bare properties with no enclosing component. */ + fun serializeProperties(properties: List): String = buildString { + properties.forEach { line(render(it)) } + } + + private fun StringBuilder.write(component: ICalComponent) { + line("BEGIN:${component.name}") + component.properties.forEach { line(render(it)) } + component.components.forEach { write(it) } + line("END:${component.name}") + } + + private fun StringBuilder.line(content: String) { + append(fold(content)).append(CRLF) + } + + internal fun render(property: ICalProperty): String = buildString { + append(property.name) + for (param in property.params) { + append(';').append(param.name) + if (param.values.isNotEmpty()) { + append('=') + append(param.values.joinToString(",") { quoteIfNeeded(it) }) + } + } + append(':').append(property.value) + } + + /** + * A parameter value is quoted only when the grammar forces it — it may not + * contain a colon, semicolon or comma unquoted. Any embedded double quote is + * dropped, because RFC 5545 gives it no escape and emitting one produces a + * line no parser can read back. + */ + private fun quoteIfNeeded(value: String): String { + if (value.none { it == ':' || it == ';' || it == ',' }) return value + return "\"" + value.replace("\"", "") + "\"" + } + + /** + * Folds a content line to at most [MAX_LINE_OCTETS] octets, continuing with + * CRLF + a single space. + * + * Counted in **octets, not characters** — the limit is defined that way, and + * an emoji in a task title is four of them. Splits stay on character + * boundaries so a fold can never cut a UTF-8 sequence in half. + */ + internal fun fold(content: String): String { + if (content.utf8Size() <= MAX_LINE_OCTETS) return content + + val out = StringBuilder() + var octets = 0 + // The first line takes the full budget; every continuation loses one octet + // to its leading space. + var budget = MAX_LINE_OCTETS + var index = 0 + while (index < content.length) { + val codePoint = content.codePointAt(index) + val charCount = Character.charCount(codePoint) + val size = String(Character.toChars(codePoint)).utf8Size() + if (octets + size > budget) { + out.append(CRLF).append(' ') + octets = 0 + budget = MAX_LINE_OCTETS - 1 + } + out.append(content, index, index + charCount) + octets += size + index += charCount + } + return out.toString() + } + + private fun String.utf8Size(): Int = toByteArray(Charsets.UTF_8).size +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt new file mode 100644 index 0000000..037eb5e --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt @@ -0,0 +1,135 @@ +package de.jeanlucmakiola.agendula.domain.ical + +import de.jeanlucmakiola.agendula.domain.allDayInstantOf +import java.time.LocalDate +import java.time.LocalDateTime +import java.time.ZoneId +import java.time.ZoneOffset +import java.time.format.DateTimeFormatter +import kotlin.time.Instant + +/** Value-level codecs: RFC 5545 TEXT escaping and the DATE / DATE-TIME forms. */ +object ICalValues { + + private val DATE = DateTimeFormatter.ofPattern("yyyyMMdd") + private val DATE_TIME = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss") + + /** + * RFC 5545 §3.3.11. Note the asymmetry with [unescapeText]: a literal colon + * needs no escape in a property value, and escaping it is a common bug that + * other clients then have to undo. + */ + fun escapeText(value: String): String = value + .replace("\\", "\\\\") + .replace(";", "\\;") + .replace(",", "\\,") + .replace("\r\n", "\\n") + .replace("\n", "\\n") + .replace("\r", "\\n") + + fun unescapeText(value: String): String { + val out = StringBuilder(value.length) + var i = 0 + while (i < value.length) { + val c = value[i] + if (c == '\\' && i + 1 < value.length) { + when (val next = value[i + 1]) { + 'n', 'N' -> out.append('\n') + '\\', ';', ',' -> out.append(next) + // An unknown escape is left as it was found; inventing a + // meaning for it would corrupt the value on write-back. + else -> out.append(c).append(next) + } + i += 2 + } else { + out.append(c) + i++ + } + } + return out.toString() + } + + /** How a DATE / DATE-TIME property was written, and whether we can reproduce it. */ + sealed interface TimeValue { + /** `VALUE=DATE` — date-only. */ + data class Date(val instant: Instant) : TimeValue + + /** A UTC instant (`…Z`), or a zoned one whose `TZID` the device knows. */ + data class Timed(val instant: Instant, val tzid: String?) : TimeValue + + /** + * A form we can read but not reproduce: a floating time (no `Z`, no + * `TZID`), or a `TZID` absent from the device's tzdb. [instant] is a + * best-effort reading for the UI; the property itself stays in the + * residue and is re-emitted verbatim, so nothing is lost on write-back. + */ + data class Unrepresentable(val instant: Instant?, val reason: String) : TimeValue + } + + /** + * Reads a DATE or DATE-TIME property. + * + * The unknown-`TZID` case is the one that matters: RFC 5545 lets a file + * carry its own `VTIMEZONE` for a zone the device has never heard of, and + * guessing UTC there silently moves the user's task by hours. It is reported + * as [TimeValue.Unrepresentable] instead. + */ + fun parseTime(property: ICalProperty): TimeValue { + val raw = property.value.trim() + val isDate = property.param("VALUE").equals("DATE", ignoreCase = true) || + (raw.length == 8 && !raw.contains('T')) + + if (isDate) { + val date = runCatching { LocalDate.parse(raw, DATE) }.getOrNull() + ?: return TimeValue.Unrepresentable(null, "unparseable DATE") + return TimeValue.Date(allDayInstantOf(date)) + } + + val utc = raw.endsWith("Z") + val local = runCatching { LocalDateTime.parse(raw.removeSuffix("Z"), DATE_TIME) }.getOrNull() + ?: return TimeValue.Unrepresentable(null, "unparseable DATE-TIME") + + if (utc) return TimeValue.Timed(local.toInstant(ZoneOffset.UTC).toKotlin(), null) + + val tzid = property.param("TZID") + ?: return TimeValue.Unrepresentable( + local.toInstant(ZoneOffset.UTC).toKotlin(), + "floating time", + ) + + val zone = runCatching { ZoneId.of(tzid) }.getOrNull() + ?: return TimeValue.Unrepresentable( + local.toInstant(ZoneOffset.UTC).toKotlin(), + "unknown TZID $tzid", + ) + + return TimeValue.Timed(local.atZone(zone).toInstant().toKotlin(), tzid) + } + + /** The instant this value denotes, however well it could be read. */ + fun readInstant(value: TimeValue): Instant? = when (value) { + is TimeValue.Date -> value.instant + is TimeValue.Timed -> value.instant + is TimeValue.Unrepresentable -> value.instant + } + + /** `20260904` — the all-day form. */ + fun formatDate(instant: Instant): String = + java.time.Instant.ofEpochMilli(instant.toEpochMilliseconds()) + .atZone(ZoneOffset.UTC) + .format(DATE) + + /** `20260904T080000Z`, or the local form when [tzid] names a zone we know. */ + fun formatDateTime(instant: Instant, tzid: String?): String { + val moment = java.time.Instant.ofEpochMilli(instant.toEpochMilliseconds()) + val zone = tzid?.let { runCatching { ZoneId.of(it) }.getOrNull() } + return if (zone == null) { + moment.atZone(ZoneOffset.UTC).format(DATE_TIME) + "Z" + } else { + moment.atZone(zone).format(DATE_TIME) + } + } + + private fun java.time.Instant.toKotlin(): Instant = + Instant.fromEpochMilliseconds(toEpochMilli()) +} diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ICalCanonical.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ICalCanonical.kt new file mode 100644 index 0000000..72b4dd9 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ICalCanonical.kt @@ -0,0 +1,79 @@ +package de.jeanlucmakiola.agendula.data.tasks.ical + +import de.jeanlucmakiola.agendula.domain.ical.ICalComponent +import de.jeanlucmakiola.agendula.domain.ical.ICalValues + +/** + * Reduces a component tree to the multiset of facts a faithful round-trip must + * preserve, so two serialisations can be compared for *meaning* rather than for + * bytes. + * + * `docs/SYNC.md` is explicit that byte-stability is unachievable and that + * asserting it produces a corpus which gets normalised until it tests nothing. + * Six things legitimately differ across a round-trip — `PRODID` must change, + * fold position carries no information, parameter quoting is optional, property + * order within a component is unconstrained, `DTSTAMP` is regenerated, and + * `VTIMEZONE` is re-emitted — so those are enumerated here and **nothing else + * may differ**. In particular the unfolded value octets of every property are + * compared exactly. + */ +object ICalCanonical { + + /** Properties whose value is allowed to differ across a round-trip. */ + private val ALLOWED_TO_DIFFER = setOf("PRODID", "DTSTAMP", "LAST-MODIFIED", "SEQUENCE") + + /** Re-emitted from its own rules rather than preserved verbatim. */ + private val OPAQUE_COMPONENTS = setOf("VTIMEZONE") + + /** + * The one value-level equivalence, and it is an equivalence rather than a + * concession: RFC 5545 section 3.8.1.11 gives a to-do no default `STATUS`, + * and an absent one is universally read as needing action. Normalising + * between the two directions loses nothing, so both are dropped before + * comparison. A `STATUS` that is *not* `NEEDS-ACTION` is compared normally, + * so losing a completion still fails. + */ + private fun isNeutralStatus(name: String, value: String) = + name == "STATUS" && value == "NEEDS-ACTION" + + data class Fact( + val path: String, + val name: String, + val params: Map>, + val value: String, + ) + + fun facts(component: ICalComponent, path: String = ""): List { + if (component.name in OPAQUE_COMPONENTS) return emptyList() + val here = if (path.isEmpty()) component.name else "$path/${component.name}" + val own = component.properties + .filterNot { it.name.uppercase() in ALLOWED_TO_DIFFER } + .filterNot { isNeutralStatus(it.name.uppercase(), it.value.trim()) } + .map { property -> + Fact( + path = here, + name = property.name.uppercase(), + params = property.params + .associate { it.name.uppercase() to it.values } + .toSortedMap() + .toMap(), + // The comparison unit `docs/SYNC.md` specifies: the unfolded, + // **unescaped** value. `\N` and `\n` are the same escape, and a + // bare comma in a TEXT value is malformed input that we repair + // on write — both are equivalences, not losses. Anything that + // actually changes the value still fails. + value = ICalValues.unescapeText(property.value), + ) + } + return own + component.components.flatMap { facts(it, here) } + } + + /** Facts present in [before] but missing from [after] — i.e. what was lost. */ + fun lost(before: ICalComponent, after: ICalComponent): List { + val remaining = facts(after).toMutableList() + return facts(before).filterNot { remaining.remove(it) } + } + + /** Facts [after] gained that [before] never had. */ + fun invented(before: ICalComponent, after: ICalComponent): List = lost(after, before) +} diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt new file mode 100644 index 0000000..dd6fd28 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt @@ -0,0 +1,304 @@ +package de.jeanlucmakiola.agendula.data.tasks.ical + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity +import de.jeanlucmakiola.agendula.domain.PRIORITY_NONE +import de.jeanlucmakiola.agendula.domain.TaskStatus +import de.jeanlucmakiola.agendula.domain.ical.ICalComponent +import de.jeanlucmakiola.agendula.domain.ical.ICalParser +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import kotlin.time.Instant + +class VTodoMapperTest { + + private fun vtodo(vararg lines: String): ICalComponent = + ICalParser.parse((listOf("BEGIN:VTODO", "UID:t-1") + lines + "END:VTODO").joinToString("\r\n")) + + @Nested + inner class Reading { + + @Test + fun `PRIORITY is stored raw, because bucketing on the way in loses it`() { + // Priority.HIGH folds 1-4 into one bucket, so rewriting a server's + // PRIORITY:3 as 1 would be a silent edit on the next write-back. + assertThat(VTodoMapper.read(vtodo("PRIORITY:3")).entity.priority).isEqualTo(3) + } + + @Test + fun `an out-of-range PRIORITY falls back to none`() { + assertThat(VTodoMapper.read(vtodo("PRIORITY:42")).entity.priority).isEqualTo(PRIORITY_NONE) + } + + @Test + fun `an unrecognised STATUS is not flattened to NEEDS-ACTION`() { + val mapped = VTodoMapper.read(vtodo("STATUS:X-DEFERRED")) + assertThat(mapped.entity.status).isEqualTo(TaskStatus.NEEDS_ACTION) + // It survives in the residue instead of being rewritten. + assertThat(mapped.entity.unknownProperties).contains("STATUS:X-DEFERRED") + } + + @Test + fun `RELATED-TO points up - the referencing component is the subordinate one`() { + val mapped = VTodoMapper.read(vtodo("RELATED-TO;RELTYPE=PARENT:parent-uid")) + assertThat(mapped.parentUid).isEqualTo("parent-uid") + } + + @Test + fun `RELTYPE defaults to PARENT when absent`() { + assertThat(VTodoMapper.read(vtodo("RELATED-TO:parent-uid")).parentUid).isEqualTo("parent-uid") + } + + @Test + fun `a RELATED-TO with another RELTYPE is somebody else's relationship`() { + val mapped = VTodoMapper.read(vtodo("RELATED-TO;RELTYPE=SIBLING:other-uid")) + assertThat(mapped.parentUid).isNull() + assertThat(mapped.entity.unknownProperties).contains("RELTYPE=SIBLING") + } + + @Test + fun `a parent link is reported but left in the residue`() { + // TaskEntity holds only a local parent_id, so a parent that is not in + // this store would leave nothing to write back. Keeping the property + // means the relationship survives a parent we have not fetched. + val mapped = VTodoMapper.read( + vtodo("RELATED-TO:parent-uid", "RELATED-TO;RELTYPE=SIBLING:other-uid"), + ) + assertThat(mapped.parentUid).isEqualTo("parent-uid") + assertThat(mapped.entity.unknownProperties).contains("parent-uid") + assertThat(mapped.entity.unknownProperties).contains("other-uid") + } + + @Test + fun `a malformed scalar stays in the residue instead of being clamped`() { + val mapped = VTodoMapper.read( + vtodo("PRIORITY:11", "PERCENT-COMPLETE:150", "SEQUENCE:x"), + ) + assertThat(mapped.entity.priority).isEqualTo(PRIORITY_NONE) + assertThat(mapped.entity.percentComplete).isNull() + assertThat(mapped.entity.sequence).isEqualTo(0) + val residue = mapped.entity.unknownProperties.orEmpty() + assertThat(residue).contains("PRIORITY:11") + assertThat(residue).contains("PERCENT-COMPLETE:150") + assertThat(residue).contains("SEQUENCE:x") + } + + @Test + fun `a mixed value-type pair keeps the odd one out in the residue`() { + // One is_all_day flag cannot author a DATE start and a timed due, and + // flattening both to dates destroys the 09:00. + val mapped = VTodoMapper.read( + vtodo("DTSTART;VALUE=DATE:20260901", "DUE;TZID=Europe/Berlin:20260901T090000"), + ) + assertThat(mapped.entity.isAllDay).isFalse() + assertThat(mapped.entity.unknownProperties).contains("DTSTART;VALUE=DATE:20260901") + } + + @Test + fun `a second timezone stays in the residue rather than being collapsed`() { + val mapped = VTodoMapper.read( + vtodo( + "DTSTART;TZID=Europe/Berlin:20260901T080000", + "DUE;TZID=America/New_York:20260901T140000", + ), + ) + assertThat(mapped.entity.timezone).isEqualTo("Europe/Berlin") + assertThat(mapped.entity.unknownProperties).contains("America/New_York") + } + + @Test + fun `DTSTAMP is dropped, LAST-MODIFIED is kept`() { + val mapped = VTodoMapper.read( + vtodo("DTSTAMP:20260901T120000Z", "LAST-MODIFIED:20260801T090000Z"), + ) + assertThat(mapped.entity.unknownProperties.orEmpty()).doesNotContain("DTSTAMP") + assertThat(mapped.entity.lastModified).isNotNull() + } + + @Test + fun `a missing UID is reported rather than invented`() { + val bare = ICalParser.parse("BEGIN:VTODO\r\nSUMMARY:no uid\r\nEND:VTODO") + assertThat(VTodoMapper.read(bare).uidWasMissing).isTrue() + } + + @Test + fun `a DUE date with no DTSTART still reads as all-day`() { + val mapped = VTodoMapper.read(vtodo("DUE;VALUE=DATE:20260901")) + assertThat(mapped.entity.isAllDay).isTrue() + assertThat(mapped.entity.due).isNotNull() + } + } + + @Nested + inner class Writing { + + @Test + fun `SEQUENCE is preserved verbatim and never bumped`() { + // It is the Organizer's revision counter (RFC 5545 section 3.8.7.4); + // a client that increments it confuses scheduling-aware peers. + val entity = entity(sequence = 7) + assertThat(VTodoMapper.write(entity).property("SEQUENCE")?.value).isEqualTo("7") + } + + @Test + fun `COMPLETED is written in UTC, with no TZID and no DATE form`() { + val entity = entity( + completedAt = Instant.parse("2026-09-01T19:00:00Z"), + isAllDay = true, + timezone = "Europe/Berlin", + ) + val completed = VTodoMapper.write(entity).property("COMPLETED") + assertThat(completed?.value).isEqualTo("20260901T190000Z") + assertThat(completed?.params).isEmpty() + } + + @Test + fun `an all-day task writes VALUE=DATE`() { + val entity = entity(due = Instant.parse("2026-09-01T00:00:00Z"), isAllDay = true) + val due = VTodoMapper.write(entity).property("DUE") + assertThat(due?.param("VALUE")).isEqualTo("DATE") + assertThat(due?.value).isEqualTo("20260901") + } + + @Test + fun `an unedited residue property suppresses the column it came from`() { + val entity = entity( + // What read() would have stored: the best-effort reading of the + // unknown-zone value it could not reproduce. + due = Instant.parse("2026-09-10T17:00:00Z"), + unknownProperties = "DUE;TZID=Custom/HQ:20260910T170000\r\n", + ) + val written = VTodoMapper.write(entity) + assertThat(written.properties("DUE")).hasSize(1) + assertThat(written.property("DUE")?.param("TZID")).isEqualTo("Custom/HQ") + } + + @Test + fun `an edited column evicts the stale residue copy`() { + // Without eviction, changing the due date of a task imported with an + // unreproducible stamp would silently do nothing on the server. + val entity = entity( + due = Instant.parse("2026-09-20T17:00:00Z"), + unknownProperties = "DUE;TZID=Custom/HQ:20260910T170000\r\n", + ) + val written = VTodoMapper.write(entity) + assertThat(written.properties("DUE")).hasSize(1) + assertThat(written.property("DUE")?.value).isEqualTo("20260920T170000Z") + } + + @Test + fun `an unreadable STATUS in the residue is not doubled by the default`() { + // STATUS has cardinality one; authoring NEEDS-ACTION next to the + // residue copy produces a resource with two of them. + val entity = entity(unknownProperties = "STATUS:X-DEFERRED\r\n") + val written = VTodoMapper.write(entity) + assertThat(written.properties("STATUS")).hasSize(1) + assertThat(written.property("STATUS")?.value).isEqualTo("X-DEFERRED") + } + + @Test + fun `a locally set status evicts the unreadable one`() { + val entity = entity(unknownProperties = "STATUS:X-DEFERRED\r\n") + .copy(status = TaskStatus.COMPLETED) + val written = VTodoMapper.write(entity) + assertThat(written.properties("STATUS")).hasSize(1) + assertThat(written.property("STATUS")?.value).isEqualTo("COMPLETED") + } + + @Test + fun `a timezone the device cannot resolve never becomes TZID on a UTC value`() { + // RFC 5545 section 3.3.5 forbids TZID on a `...Z` value, and sabre + // answers 415 for it. + val entity = entity(due = Instant.parse("2026-09-05T05:30:00Z"), timezone = "Custom/Nope") + val due = VTodoMapper.write(entity).property("DUE") + assertThat(due?.params).isEmpty() + assertThat(due?.value).isEqualTo("20260905T053000Z") + } + + @Test + fun `PRIORITY 0 is omitted, because 0 means undefined`() { + assertThat(VTodoMapper.write(entity()).property("PRIORITY")).isNull() + } + } + + @Nested + inner class Validation { + + @Test + fun `DUE before DTSTART is caught here, not as a 415 from sabre`() { + val problems = VTodoMapper.validate( + vtodo("DTSTART:20260910T100000Z", "DUE:20260901T100000Z"), + ) + assertThat(problems).contains("DUE precedes DTSTART") + } + + @Test + fun `DTSTART and DUE must agree on value type`() { + val problems = VTodoMapper.validate( + vtodo("DTSTART;VALUE=DATE:20260901", "DUE:20260910T100000Z"), + ) + assertThat(problems).contains("DTSTART and DUE disagree on value type") + } + + @Test + fun `an end-relative alarm with no DUE is invalid, and reachable from the UI`() { + // Clearing the due date on a task that has an end-relative reminder is + // an ordinary UI action that produces a permanently rejected resource. + val withAlarm = ICalParser.parse( + """ + BEGIN:VTODO + UID:t-1 + SUMMARY:x + BEGIN:VALARM + ACTION:DISPLAY + TRIGGER;RELATED=END:-PT15M + END:VALARM + END:VTODO + """.trimIndent(), + ) + assertThat(VTodoMapper.validate(withAlarm)) + .contains("TRIGGER;RELATED=END with neither DUE nor DTSTART+DURATION") + } + + @Test + fun `METHOD is rejected, because sabre answers 415 for it`() { + assertThat(VTodoMapper.validate(vtodo("METHOD:REQUEST"))) + .contains("METHOD is not allowed on a stored resource") + } + + @Test + fun `an implicit DATE is recognised as one, so value types do not falsely disagree`() { + // A bare 8-digit value with no T is a DATE whether or not VALUE=DATE + // says so. Two different date tests would block a legitimate PUT. + assertThat( + VTodoMapper.validate(vtodo("DTSTART:20260101", "DUE;VALUE=DATE:20260102")), + ).isEmpty() + } + + @Test + fun `a well-formed task has no problems`() { + assertThat( + VTodoMapper.validate(vtodo("DTSTART:20260901T100000Z", "DUE:20260910T100000Z")), + ).isEmpty() + } + } + + private fun entity( + sequence: Int = 0, + completedAt: Instant? = null, + due: Instant? = null, + isAllDay: Boolean = false, + timezone: String? = null, + unknownProperties: String? = null, + ) = TaskEntity( + listId = 1L, + uid = "t-1", + title = "A task", + sequence = sequence, + completedAt = completedAt, + due = due, + isAllDay = isAllDay, + timezone = timezone, + unknownProperties = unknownProperties, + ) +} diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt new file mode 100644 index 0000000..48b0895 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt @@ -0,0 +1,161 @@ +package de.jeanlucmakiola.agendula.data.tasks.ical + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.domain.ical.ICalComponent +import de.jeanlucmakiola.agendula.domain.ical.ICalParser +import de.jeanlucmakiola.agendula.domain.ical.ICalSerializer +import org.junit.jupiter.api.DynamicTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestFactory +import kotlin.time.Instant + +/** + * The corpus in `app/src/test/resources/vtodo/` is the mapper's specification. + * + * The requirement being encoded is RFC 5545 §3.1: *"Applications MUST preserve + * the value data for x-name and iana-token values that they don't recognize."* + * Failing it destroys other people's data invisibly — invisible in our own UI + * precisely because we are the client that does not understand the property. + */ +class VTodoRoundTripTest { + + private val fixtures = listOf( + "nextcloud-shared-confidential", + "recurring-master-overrides", + "valarm-unknown-nested", + "unknown-component", + "unknown-tzid", + "uid-special-chars", + "rrule-due-no-dtstart", + "moz-alarm-props", + "apple-sort-order", + "floating-time", + "completion-model-a", + "completion-model-b", + "completion-model-c", + "completion-model-d", + "quoted-param-colon", + "folded-utf8", + "mixed-value-types", + "malformed-scalars", + "two-timezones", + ) + + @TestFactory + fun `every fixture round-trips without losing a property`(): List = + fixtures.map { name -> + DynamicTest.dynamicTest(name) { + val todos = vtodosOf(name) + assertThat(todos).isNotEmpty() + todos.forEach { original -> + val roundTripped = roundTrip(original) + assertThat(ICalCanonical.lost(original, roundTripped)).isEmpty() + assertThat(ICalCanonical.invented(original, roundTripped)).isEmpty() + } + } + } + + @Test + fun `an unknown property nested inside a VALARM survives`() { + val original = vtodosOf("valarm-unknown-nested").single() + val roundTripped = roundTrip(original) + + val alarm = roundTripped.components("VALARM").single() + assertThat(alarm.property("X-WR-ALARMUID")?.value) + .isEqualTo("8E6C4A1E-0000-4C1B-9B1A-3F5F4C2D9A11") + // RFC 9074's alarm properties are what other clients now write; a flat + // property-per-row model would have dropped this one with the nesting. + assertThat(alarm.property("ACKNOWLEDGED")?.value).isEqualTo("20260901T113000Z") + } + + @Test + fun `an entirely unknown sub-component survives`() { + val roundTripped = roundTrip(vtodosOf("unknown-component").single()) + val vendor = roundTripped.components("X-VENDOR-METADATA").single() + assertThat(vendor.property("X-VENDOR-KEY")?.value).isEqualTo("project-alpha") + } + + @Test + fun `a TZID the device does not know is re-emitted verbatim, not guessed`() { + val original = vtodosOf("unknown-tzid").single() + val mapped = VTodoMapper.read(original) + val roundTripped = roundTrip(original) + + val due = roundTripped.property("DUE") + assertThat(due?.param("TZID")).isEqualTo("Custom/Company-HQ") + assertThat(due?.value).isEqualTo("20260910T170000") + // Exactly one DUE: the residue owns it, so the mapper must not also + // author one from its column. + assertThat(roundTripped.properties("DUE")).hasSize(1) + // The column still gets a best-effort instant so the UI has something. + assertThat(mapped.entity.due).isNotNull() + } + + @Test + fun `a floating time is re-emitted verbatim, not pinned to a zone`() { + val roundTripped = roundTrip(vtodosOf("floating-time").single()) + assertThat(roundTripped.property("DTSTART")?.value).isEqualTo("20260905T070000") + assertThat(roundTripped.property("DTSTART")?.params).isEmpty() + assertThat(roundTripped.properties("DTSTART")).hasSize(1) + } + + @Test + fun `X-MOZ alarm bookkeeping survives, because dropping it causes alarm storms`() { + val roundTripped = roundTrip(vtodosOf("moz-alarm-props").single()) + assertThat(roundTripped.property("X-MOZ-LASTACK")?.value).isEqualTo("20260904T090000Z") + assertThat(roundTripped.property("X-MOZ-SNOOZE-TIME")?.value).isEqualTo("20260905T093000Z") + } + + @Test + fun `X-APPLE-SORT-ORDER is preserved and never interpreted`() { + val original = vtodosOf("apple-sort-order").single() + val mapped = VTodoMapper.read(original) + assertThat(roundTrip(original).property("X-APPLE-SORT-ORDER")?.value).isEqualTo("734829163") + // Ours is local ordering and has nothing to do with Apple's. + assertThat(mapped.entity.sortOrder).isEqualTo(0) + } + + @Test + fun `a quoted parameter containing a colon and a comma survives`() { + val roundTripped = roundTrip(vtodosOf("quoted-param-colon").single()) + val attendee = roundTripped.property("ATTENDEE") + assertThat(attendee?.param("CN")).isEqualTo("Smith, J:r") + assertThat(attendee?.value).isEqualTo("mailto:jr@example.com") + // And it is re-quoted on the way out, or nothing can read it back. + assertThat(ICalSerializer.render(attendee!!)).contains("CN=\"Smith, J:r\"") + } + + @Test + fun `residue past the size cap is dropped loudly rather than truncated`() { + val huge = "X-BULK:" + "a".repeat(VTodoMapper.MAX_RESIDUE_BYTES + 1) + val vtodo = ICalParser.parse( + "BEGIN:VTODO\r\nUID:big-001\r\nSUMMARY:Big\r\n$huge\r\nEND:VTODO\r\n", + ) + val mapped = VTodoMapper.read(vtodo) + + assertThat(mapped.droppedResidue).isTrue() + assertThat(mapped.entity.unknownProperties).isNull() + } + + private fun roundTrip(vtodo: ICalComponent): ICalComponent { + val mapped = VTodoMapper.read(vtodo) + val written = VTodoMapper.write( + entity = mapped.entity, + parentUid = mapped.parentUid, + now = Instant.fromEpochMilliseconds(0), + ) + // Serialise and re-parse rather than comparing the tree directly: folding + // and parameter quoting are where a serialiser loses data, and skipping + // the text would skip exactly that. + return ICalParser.parse( + "BEGIN:VCALENDAR\r\n" + ICalSerializer.serialize(written) + "END:VCALENDAR\r\n", + ).components("VTODO").single() + } + + private fun vtodosOf(name: String): List { + val text = checkNotNull(javaClass.getResourceAsStream("/vtodo/$name.ics")) { + "missing fixture $name.ics" + }.bufferedReader().readText() + return ICalParser.parse(text).components("VTODO") + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalParserTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalParserTest.kt new file mode 100644 index 0000000..a95f744 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalParserTest.kt @@ -0,0 +1,154 @@ +package de.jeanlucmakiola.agendula.domain.ical + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +/** + * The lexer. Worth testing closely because every correctness guarantee above it + * assumes a content line was split where RFC 5545 says it splits — and the + * places it does not split are where other clients' data goes missing. + */ +class ICalParserTest { + + @Nested + inner class Unfolding { + + @Test + fun `unfolding removes the CRLF and the whitespace, not just the CRLF`() { + // RFC 5545 section 3.1: a fold is CRLF + one WSP and *both* go. A + // folder that keeps the space silently inserts one into every long + // value it ever wrote. + val lines = ICalParser.unfold("SUMMARY:Hello\r\n world\r\n") + assertThat(lines).containsExactly("SUMMARY:Helloworld") + } + + @Test + fun `a tab folds too`() { + assertThat(ICalParser.unfold("SUMMARY:Hello\r\n\tworld")).containsExactly("SUMMARY:Helloworld") + } + + @Test + fun `bare LF is accepted, because real files carry it`() { + assertThat(ICalParser.unfold("A:1\nB:2")).containsExactly("A:1", "B:2") + } + + @Test + fun `a leading BOM is stripped rather than glued onto the first name`() { + val component = ICalParser.parse("BEGIN:VTODO\r\nUID:x\r\nEND:VTODO\r\n") + assertThat(component.name).isEqualTo("VTODO") + } + } + + @Nested + inner class ContentLines { + + @Test + fun `the value starts at the first colon outside a quoted parameter`() { + val property = ICalParser.parseLine("""ATTENDEE;CN="Smith, J:r":mailto:jr@example.com""") + assertThat(property?.name).isEqualTo("ATTENDEE") + assertThat(property?.param("CN")).isEqualTo("Smith, J:r") + assertThat(property?.value).isEqualTo("mailto:jr@example.com") + } + + @Test + fun `a multi-valued parameter splits only on unquoted commas`() { + val property = ICalParser.parseLine("""X-THING;MEMBER="a,b",c:v""") + assertThat(property?.params?.single()?.values).containsExactly("a,b", "c").inOrder() + } + + @Test + fun `a value may contain colons`() { + assertThat(ICalParser.parseLine("URL:https://example.com/a:b")?.value) + .isEqualTo("https://example.com/a:b") + } + + @Test + fun `the value is kept escaped, because that is what makes it reproducible`() { + assertThat(ICalParser.parseLine("""DESCRIPTION:a\, b\; c""")?.value) + .isEqualTo("""a\, b\; c""") + } + + @Test + fun `a parameter with no equals sign is kept rather than dropped`() { + val property = ICalParser.parseLine("X-THING;BROKEN:v") + assertThat(property?.params?.single()?.name).isEqualTo("BROKEN") + assertThat(property?.params?.single()?.values).isEmpty() + } + + @Test + fun `a line with no colon is not a content line`() { + assertThat(ICalParser.parseLine("GARBAGE")).isNull() + } + } + + @Nested + inner class Structure { + + @Test + fun `unknown components nest like any other`() { + val component = ICalParser.parse( + """ + BEGIN:VTODO + UID:x + BEGIN:X-VENDOR + X-KEY:v + END:X-VENDOR + END:VTODO + """.trimIndent(), + ) + assertThat(component.components("X-VENDOR").single().property("X-KEY")?.value).isEqualTo("v") + } + + @Test + fun `a stray END is ignored rather than fatal`() { + val component = ICalParser.parse("END:VALARM\nBEGIN:VTODO\nUID:x\nEND:VTODO\n") + assertThat(component.name).isEqualTo("VTODO") + assertThat(component.property("UID")?.value).isEqualTo("x") + } + + @Test + fun `component names are upper-cased so lookups are not case-dependent`() { + assertThat(ICalParser.parse("begin:vtodo\nuid:x\nend:vtodo\n").name).isEqualTo("VTODO") + } + + @Test + fun `text with no component at all is fatal`() { + runCatching { ICalParser.parse("SUMMARY:orphan\r\n") } + .onSuccess { error("expected ICalParseException") } + .onFailure { assertThat(it).isInstanceOf(ICalParseException::class.java) } + } + + @Test + fun `a missing END does not swallow the enclosing component`() { + // Closing by position would let END:VCALENDAR close the VTODO, leave + // VCALENDAR open, and end with no component at all — discarding a + // whole multiget response over one malformed task. + val roots = ICalParser.parseAll( + "BEGIN:VCALENDAR\r\nBEGIN:VTODO\r\nUID:x\r\nEND:VCALENDAR\r\n", + ) + val calendar = roots.single() + assertThat(calendar.name).isEqualTo("VCALENDAR") + assertThat(calendar.components("VTODO").single().property("UID")?.value).isEqualTo("x") + } + + @Test + fun `a component left open at end of input is still returned`() { + val calendar = ICalParser.parse("BEGIN:VCALENDAR\r\nBEGIN:VTODO\r\nUID:x\r\n") + assertThat(calendar.components("VTODO").single().property("UID")?.value).isEqualTo("x") + } + + @Test + fun `an unbalanced quote does not swallow the whole line`() { + val property = ICalParser.parseLine("""X-PROP;CN="unclosed:value""") + assertThat(property?.name).isEqualTo("X-PROP") + assertThat(property?.value).isEqualTo("value") + } + + @Test + fun `duplicate properties are both kept`() { + val component = ICalParser.parse("BEGIN:VTODO\nCATEGORIES:a\nCATEGORIES:a\nEND:VTODO\n") + assertThat(component.properties("CATEGORIES")).hasSize(2) + } + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializerTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializerTest.kt new file mode 100644 index 0000000..f0b4280 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalSerializerTest.kt @@ -0,0 +1,64 @@ +package de.jeanlucmakiola.agendula.domain.ical + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +class ICalSerializerTest { + + @Test + fun `a line is folded at 75 octets, not 75 characters`() { + // Each 'ä' is two octets, so 40 of them is 80 — over the limit at 40 + // characters, which a character-counting folder would let through. + val property = ICalProperty("SUMMARY", value = "ä".repeat(40)) + val folded = ICalSerializer.fold(ICalSerializer.render(property)) + + assertThat(folded).contains("\r\n ") + folded.split("\r\n").forEach { + assertThat(it.toByteArray(Charsets.UTF_8).size).isAtMost(75) + } + } + + @Test + fun `folding never splits a multi-byte character`() { + val value = "🌱".repeat(30) + val folded = ICalSerializer.fold("SUMMARY:$value") + val unfolded = ICalParser.unfold(folded + "\r\n").single() + assertThat(unfolded).isEqualTo("SUMMARY:$value") + } + + @Test + fun `a parameter value is quoted only when the grammar forces it`() { + assertThat(ICalSerializer.render(ICalProperty("X", listOf(ICalParam("P", "plain")), "v"))) + .isEqualTo("X;P=plain:v") + assertThat(ICalSerializer.render(ICalProperty("X", listOf(ICalParam("P", "a:b")), "v"))) + .isEqualTo("""X;P="a:b":v""") + assertThat(ICalSerializer.render(ICalProperty("X", listOf(ICalParam("P", "a,b")), "v"))) + .isEqualTo("""X;P="a,b":v""") + } + + @Test + fun `an embedded double quote survives when the value need not be quoted`() { + // The parser only unquotes a matched leading/trailing pair, so stripping + // unconditionally would lose two characters from a value it never quoted. + assertThat(ICalSerializer.render(ICalProperty("X", listOf(ICalParam("P", """a"b""")), "v"))) + .isEqualTo("""X;P=a"b:v""") + } + + @Test + fun `an embedded double quote is dropped only when quoting is forced`() { + assertThat(ICalSerializer.render(ICalProperty("X", listOf(ICalParam("P", """a"b,c""")), "v"))) + .isEqualTo("""X;P="ab,c":v""") + } + + @Test + fun `components round-trip through serialise and parse`() { + val original = ICalComponent( + name = "VTODO", + properties = listOf(ICalProperty("UID", value = "x")), + components = listOf( + ICalComponent("VALARM", listOf(ICalProperty("ACTION", value = "DISPLAY"))), + ), + ) + assertThat(ICalParser.parse(ICalSerializer.serialize(original))).isEqualTo(original) + } +} diff --git a/app/src/test/resources/vtodo/README.md b/app/src/test/resources/vtodo/README.md new file mode 100644 index 0000000..622f914 --- /dev/null +++ b/app/src/test/resources/vtodo/README.md @@ -0,0 +1,32 @@ +# VTODO fixture corpus + +The mapper's **specification**, not its regression net. Every file here is a +shape that was found in the wild and that a plausible implementation gets wrong. +A clean, server-generated VTODO catches nothing and is deliberately absent. + +`RoundTripTest` asserts every fixture survives `parse → map → write → serialise` +with no property lost, compared as a canonical multiset modulo the allowlist in +`docs/SYNC-PLAN.md` § chunk 1 (`PRODID`, `DTSTAMP`, `LAST-MODIFIED`, `SEQUENCE`, +VTIMEZONE bodies, fold positions, parameter quoting). + +| File | What it is for | +|---|---| +| `nextcloud-shared-confidential.ics` | A `CLASS:CONFIDENTIAL` task as Nextcloud rewrites it on GET from a **shared** calendar — `DUE`, `STATUS`, `COMPLETED`, `PERCENT-COMPLETE`, `PRIORITY` and `RELATED-TO` already stripped, ETag left untouched. Re-PUTting this destroys the owner's task | +| `recurring-master-overrides.ics` | Master plus two `RECURRENCE-ID` overrides in one resource, sharing a UID | +| `valarm-unknown-nested.ics` | Unknown properties **inside** a `VALARM` — the case a flat property-per-row model cannot represent | +| `unknown-component.ics` | An entirely unknown sub-component | +| `unknown-tzid.ics` | A `TZID` no device tzdb knows, with its own `VTIMEZONE` | +| `uid-special-chars.ics` | A UID containing `/` and `@` — the href sanitiser must not assume it is filename-safe | +| `rrule-due-no-dtstart.ics` | `RRULE` + `DUE` and no `DTSTART`: no well-defined `RECURRENCE-ID`, undefined in RFC 5545, ubiquitous in the wild | +| `moz-alarm-props.ics` | `X-MOZ-LASTACK` / `X-MOZ-SNOOZE-TIME`. Dropping them causes documented Thunderbird alarm storms | +| `apple-sort-order.ics` | `X-APPLE-SORT-ORDER` — preserve, never interpret | +| `floating-time.ics` | `DTSTART` with neither `Z` nor `TZID`. Readable, not reproducible: it must round-trip from the residue, not be rewritten as a guess | +| `completion-model-a.ics` | Completion as a `RECURRENCE-ID` override, master stays open — jtx Board, Thunderbird. **The model we write** | +| `completion-model-b.ics` | Master's `DUE` advanced in place, completion cleared — tasks.org, Evolution, Nextcloud in practice | +| `completion-model-c.ics` | `STATUS:COMPLETED` on the master, killing the series — Nextcloud Tasks ≤ 0.17. **Always a bug**; we read it, we never write it | +| `completion-model-d.ics` | The completed occurrence detached as a new task with a new UID — OpenTasks | +| `quoted-param-colon.ics` | A quoted parameter value containing a colon and a comma. The value separator is the first colon *outside* quotes | +| `mixed-value-types.ics` | `DTSTART;VALUE=DATE` with a timed `DUE`. One `is_all_day` flag cannot author both, so the odd one out must stay in the residue rather than be flattened to a date | +| `malformed-scalars.ics` | `PRIORITY:11`, `PERCENT-COMPLETE:150`, `SEQUENCE:x`, `STATUS:X-DEFERRED`, `CLASS:X-INTERNAL` — values no spec allows. Clamping or defaulting any of them is a silent rewrite of somebody's data | +| `two-timezones.ics` | `DTSTART` and `DUE` in different zones. One `timezone` column cannot author both | +| `folded-utf8.ics` | A line folded mid-emoji-adjacent, to prove folding counts octets and splits on character boundaries | diff --git a/app/src/test/resources/vtodo/apple-sort-order.ics b/app/src/test/resources/vtodo/apple-sort-order.ics new file mode 100644 index 0000000..fb05b64 --- /dev/null +++ b/app/src/test/resources/vtodo/apple-sort-order.ics @@ -0,0 +1,11 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Apple Inc.//macOS 15.3//EN +BEGIN:VTODO +UID:apple-sort-001 +DTSTAMP:20260901T120000Z +SUMMARY:Buy milk +X-APPLE-SORT-ORDER:734829163 +PRIORITY:3 +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/completion-model-a.ics b/app/src/test/resources/vtodo/completion-model-a.ics new file mode 100644 index 0000000..518e460 --- /dev/null +++ b/app/src/test/resources/vtodo/completion-model-a.ics @@ -0,0 +1,22 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//jtx Board//EN +BEGIN:VTODO +UID:model-a-001 +DTSTAMP:20260901T120000Z +SUMMARY:Take out the bins +DTSTART;VALUE=DATE:20260901 +RRULE:FREQ=WEEKLY;BYDAY=TU +STATUS:NEEDS-ACTION +END:VTODO +BEGIN:VTODO +UID:model-a-001 +RECURRENCE-ID;VALUE=DATE:20260901 +DTSTAMP:20260901T190000Z +SUMMARY:Take out the bins +DTSTART;VALUE=DATE:20260901 +STATUS:COMPLETED +COMPLETED:20260901T190000Z +PERCENT-COMPLETE:100 +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/completion-model-b.ics b/app/src/test/resources/vtodo/completion-model-b.ics new file mode 100644 index 0000000..8827482 --- /dev/null +++ b/app/src/test/resources/vtodo/completion-model-b.ics @@ -0,0 +1,12 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//tasks.org//EN +BEGIN:VTODO +UID:model-b-001 +DTSTAMP:20260908T190000Z +SUMMARY:Take out the bins +DUE;VALUE=DATE:20260908 +RRULE:FREQ=WEEKLY;BYDAY=TU +STATUS:NEEDS-ACTION +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/completion-model-c.ics b/app/src/test/resources/vtodo/completion-model-c.ics new file mode 100644 index 0000000..9dbd0a6 --- /dev/null +++ b/app/src/test/resources/vtodo/completion-model-c.ics @@ -0,0 +1,14 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Nextcloud Tasks 0.17//EN +BEGIN:VTODO +UID:model-c-001 +DTSTAMP:20260901T190000Z +SUMMARY:Take out the bins +DUE;VALUE=DATE:20260901 +RRULE:FREQ=WEEKLY;BYDAY=TU +STATUS:COMPLETED +COMPLETED:20260901T190000Z +PERCENT-COMPLETE:100 +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/completion-model-d.ics b/app/src/test/resources/vtodo/completion-model-d.ics new file mode 100644 index 0000000..1fb3da2 --- /dev/null +++ b/app/src/test/resources/vtodo/completion-model-d.ics @@ -0,0 +1,20 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//dmfs.org/OpenTasks//EN +BEGIN:VTODO +UID:model-d-001 +DTSTAMP:20260908T190000Z +SUMMARY:Take out the bins +DUE;VALUE=DATE:20260908 +RRULE:FREQ=WEEKLY;BYDAY=TU +STATUS:NEEDS-ACTION +END:VTODO +BEGIN:VTODO +UID:model-d-detached-001 +DTSTAMP:20260901T190000Z +SUMMARY:Take out the bins +DUE;VALUE=DATE:20260901 +STATUS:COMPLETED +COMPLETED:20260901T190000Z +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/floating-time.ics b/app/src/test/resources/vtodo/floating-time.ics new file mode 100644 index 0000000..5f30078 --- /dev/null +++ b/app/src/test/resources/vtodo/floating-time.ics @@ -0,0 +1,11 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Radicale//NONSGML Radicale Server//EN +BEGIN:VTODO +UID:floating-001 +DTSTAMP:20260901T120000Z +SUMMARY:Wake up wherever you are +DTSTART:20260905T070000 +DUE:20260905T073000 +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/folded-utf8.ics b/app/src/test/resources/vtodo/folded-utf8.ics new file mode 100644 index 0000000..6122257 --- /dev/null +++ b/app/src/test/resources/vtodo/folded-utf8.ics @@ -0,0 +1,12 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Agendula//Test//EN +BEGIN:VTODO +UID:folded-utf8-001 +DTSTAMP:20260901T120000Z +SUMMARY:Gießkanne füllen und die Pflanzen im Wintergarten gießen — 🌱🌿🪴 + jede Woche montags +DESCRIPTION:Zeile eins\nZeile zwei mit Komma\, Semikolon\; und Backslash\\ dr + in +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/malformed-scalars.ics b/app/src/test/resources/vtodo/malformed-scalars.ics new file mode 100644 index 0000000..67aab8c --- /dev/null +++ b/app/src/test/resources/vtodo/malformed-scalars.ics @@ -0,0 +1,14 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Careless Client//EN +BEGIN:VTODO +UID:malformed-scalars-001 +DTSTAMP:20260901T120000Z +SUMMARY:Values no spec allows +PRIORITY:11 +PERCENT-COMPLETE:150 +SEQUENCE:x +STATUS:X-DEFERRED +CLASS:X-INTERNAL +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/mixed-value-types.ics b/app/src/test/resources/vtodo/mixed-value-types.ics new file mode 100644 index 0000000..4e4d202 --- /dev/null +++ b/app/src/test/resources/vtodo/mixed-value-types.ics @@ -0,0 +1,11 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Mixed//EN +BEGIN:VTODO +UID:mixed-value-001 +DTSTAMP:20260901T120000Z +SUMMARY:Starts on a day, due at a time +DTSTART;VALUE=DATE:20260901 +DUE;TZID=Europe/Berlin:20260901T090000 +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/moz-alarm-props.ics b/app/src/test/resources/vtodo/moz-alarm-props.ics new file mode 100644 index 0000000..08f695e --- /dev/null +++ b/app/src/test/resources/vtodo/moz-alarm-props.ics @@ -0,0 +1,18 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +BEGIN:VTODO +UID:moz-alarm-001 +DTSTAMP:20260901T120000Z +SUMMARY:Call the dentist +DUE;TZID=Europe/Berlin:20260905T100000 +X-MOZ-LASTACK:20260904T090000Z +X-MOZ-SNOOZE-TIME:20260905T093000Z +X-MOZ-GENERATION:3 +BEGIN:VALARM +ACTION:DISPLAY +DESCRIPTION:Call the dentist +TRIGGER;RELATED=END:-PT15M +END:VALARM +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/nextcloud-shared-confidential.ics b/app/src/test/resources/vtodo/nextcloud-shared-confidential.ics new file mode 100644 index 0000000..22288df --- /dev/null +++ b/app/src/test/resources/vtodo/nextcloud-shared-confidential.ics @@ -0,0 +1,12 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Nextcloud calendar v4.7.0 +BEGIN:VTODO +UID:conf-shared-001 +DTSTAMP:20260901T120000Z +SUMMARY:Quarterly filing +CLASS:CONFIDENTIAL +CREATED:20260801T090000Z +LAST-MODIFIED:20260901T115900Z +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/quoted-param-colon.ics b/app/src/test/resources/vtodo/quoted-param-colon.ics new file mode 100644 index 0000000..a7af75d --- /dev/null +++ b/app/src/test/resources/vtodo/quoted-param-colon.ics @@ -0,0 +1,11 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//SOGo//EN +BEGIN:VTODO +UID:quoted-param-001 +DTSTAMP:20260901T120000Z +SUMMARY:Review with the team +ATTENDEE;CN="Smith, J:r";PARTSTAT=NEEDS-ACTION:mailto:jr@example.com +ORGANIZER;CN=Alice:mailto:alice@example.com +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/recurring-master-overrides.ics b/app/src/test/resources/vtodo/recurring-master-overrides.ics new file mode 100644 index 0000000..4664b14 --- /dev/null +++ b/app/src/test/resources/vtodo/recurring-master-overrides.ics @@ -0,0 +1,33 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +BEGIN:VTODO +UID:series-weekly-001 +DTSTAMP:20260901T120000Z +SUMMARY:Water the plants +DTSTART;TZID=Europe/Berlin:20260901T080000 +DUE;TZID=Europe/Berlin:20260901T090000 +RRULE:FREQ=WEEKLY;BYDAY=MO +STATUS:NEEDS-ACTION +END:VTODO +BEGIN:VTODO +UID:series-weekly-001 +RECURRENCE-ID;TZID=Europe/Berlin:20260908T080000 +DTSTAMP:20260908T081500Z +SUMMARY:Water the plants +DTSTART;TZID=Europe/Berlin:20260908T080000 +DUE;TZID=Europe/Berlin:20260908T090000 +STATUS:COMPLETED +PERCENT-COMPLETE:100 +COMPLETED:20260908T081500Z +END:VTODO +BEGIN:VTODO +UID:series-weekly-001 +RECURRENCE-ID;TZID=Europe/Berlin:20260915T080000 +DTSTAMP:20260915T070000Z +SUMMARY:Water the plants (away — ask neighbour) +DTSTART;TZID=Europe/Berlin:20260915T080000 +DUE;TZID=Europe/Berlin:20260915T090000 +STATUS:NEEDS-ACTION +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/rrule-due-no-dtstart.ics b/app/src/test/resources/vtodo/rrule-due-no-dtstart.ics new file mode 100644 index 0000000..9f84cab --- /dev/null +++ b/app/src/test/resources/vtodo/rrule-due-no-dtstart.ics @@ -0,0 +1,12 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Nextcloud Tasks//EN +BEGIN:VTODO +UID:rrule-no-dtstart-001 +DTSTAMP:20260901T120000Z +SUMMARY:Pay rent +DUE;VALUE=DATE:20260901 +RRULE:FREQ=MONTHLY;BYMONTHDAY=1 +STATUS:NEEDS-ACTION +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/two-timezones.ics b/app/src/test/resources/vtodo/two-timezones.ics new file mode 100644 index 0000000..f8a44b0 --- /dev/null +++ b/app/src/test/resources/vtodo/two-timezones.ics @@ -0,0 +1,11 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Traveller//EN +BEGIN:VTODO +UID:two-zones-001 +DTSTAMP:20260901T120000Z +SUMMARY:Leave Berlin, land in New York +DTSTART;TZID=Europe/Berlin:20260901T080000 +DUE;TZID=America/New_York:20260901T140000 +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/uid-special-chars.ics b/app/src/test/resources/vtodo/uid-special-chars.ics new file mode 100644 index 0000000..65c9e43 --- /dev/null +++ b/app/src/test/resources/vtodo/uid-special-chars.ics @@ -0,0 +1,9 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Example Corp//CalDAV Client//EN +BEGIN:VTODO +UID:20260901T120000Z/task@example.com +DTSTAMP:20260901T120000Z +SUMMARY:UID that is not filename-safe +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/unknown-component.ics b/app/src/test/resources/vtodo/unknown-component.ics new file mode 100644 index 0000000..a53e13c --- /dev/null +++ b/app/src/test/resources/vtodo/unknown-component.ics @@ -0,0 +1,13 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Some Other Client//EN +BEGIN:VTODO +UID:unknown-comp-001 +DTSTAMP:20260901T120000Z +SUMMARY:Task with a component we have never seen +BEGIN:X-VENDOR-METADATA +X-VENDOR-KEY:project-alpha +X-VENDOR-WEIGHT:7 +END:X-VENDOR-METADATA +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/unknown-tzid.ics b/app/src/test/resources/vtodo/unknown-tzid.ics new file mode 100644 index 0000000..07292b7 --- /dev/null +++ b/app/src/test/resources/vtodo/unknown-tzid.ics @@ -0,0 +1,19 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Legacy Groupware//EN +BEGIN:VTIMEZONE +TZID:Custom/Company-HQ +BEGIN:STANDARD +DTSTART:19700101T000000 +TZOFFSETFROM:+0130 +TZOFFSETTO:+0130 +TZNAME:HQ +END:STANDARD +END:VTIMEZONE +BEGIN:VTODO +UID:unknown-tzid-001 +DTSTAMP:20260901T120000Z +SUMMARY:Board pack due +DUE;TZID=Custom/Company-HQ:20260910T170000 +END:VTODO +END:VCALENDAR diff --git a/app/src/test/resources/vtodo/valarm-unknown-nested.ics b/app/src/test/resources/vtodo/valarm-unknown-nested.ics new file mode 100644 index 0000000..15dd127 --- /dev/null +++ b/app/src/test/resources/vtodo/valarm-unknown-nested.ics @@ -0,0 +1,18 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Apple Inc.//iOS 18.2//EN +BEGIN:VTODO +UID:alarm-nested-001 +DTSTAMP:20260901T120000Z +SUMMARY:Renew passport +DUE;TZID=Europe/Berlin:20261001T170000 +BEGIN:VALARM +ACTION:DISPLAY +DESCRIPTION:Reminder +TRIGGER;RELATED=START:-PT30M +ACKNOWLEDGED:20260901T113000Z +X-WR-ALARMUID:8E6C4A1E-0000-4C1B-9B1A-3F5F4C2D9A11 +X-APPLE-DEFAULT-ALARM:TRUE +END:VALARM +END:VTODO +END:VCALENDAR diff --git a/docs/SYNC-PLAN.md b/docs/SYNC-PLAN.md index 29bd081..0530391 100644 --- a/docs/SYNC-PLAN.md +++ b/docs/SYNC-PLAN.md @@ -24,7 +24,7 @@ and says so rather than quietly picking the other branch. | 1 | dav4jvm + cert4android distribution, and the Java-21 wall | **Vendor a known-good tree**, recompiled at our Java 17 target | `FAIL_ON_PROJECT_REPOS` stays; no Ktor/guava/xpp3 tail; MPL-2.0 headers retained and the attribution screen becomes mandatory, not optional. Freezes the API churn that shipped two breaking majors nineteen days apart | | 2 | Conflict policy on 412 | **Server wins, local edit discarded** | DAVx5's policy. Terminates, always. The discarded edit is *surfaced* — a sync report the user can read — because silence is what makes this policy feel like data loss. The draft's "preserve as a duplicate" stays withdrawn: RFC 4791 §4.1 makes it unuploadable | | 3 | External mode's future | **Kept, unchanged** | The mapper and the UI stay dual-capable. Retiring it is a separate decision that does not block sync, and it ships and works today | -| 4 | iCalendar library | **ical4j 4.3.0**, with the shims `SYNC.md` § *Libraries* enumerates | BSD-3 (attribution screen again). Costs the 2.2 MB of duplicated zone data, the mandatory `ical4j.properties` + `MapTimeZoneCache` + registry shim, and R8 keep rules. biweekly's missing tz database is the disqualifier for CalDAV round-tripping | +| 4 | iCalendar library | **None — a hand-rolled content-line model.** ⚠️ Overturned during chunk 1; `SYNC.md` § *Libraries* assumed a typed library was the only option | A raw `(name, params, value)` content-line tree is what the unknown-property requirement actually wants: a typed model normalises away exactly what must survive. `ICalendarWriter` already folds in 204 lines, `lib-recur` is already a direct `:app` dependency and does `RRULE`, and `java.time` is native at minSdk 29 — so ical4j's 2.2 MB of duplicated zone data, its production-exhausting `ZoneRulesProvider`, its mandatory properties/cache/registry shims and its release-only R8 failure buy nothing. VTIMEZONE bodies are already on the round-trip allowlist, so they round-trip opaquely. **Cost accepted: we own the lexer.** biweekly was rejected on the same ground as before | --- @@ -134,6 +134,42 @@ local reminder edit destroys every server-side alarm on that task. **Done when:** the corpus round-trips, the migration test passes both directions, and no fixture loses an unknown property. +### Settled while building it + +- ⚠️ **No schema migration was needed.** `TaskEntity.unknown_properties` already + exists at v1, reserved for exactly this and written by nothing. The residue + fills it. It holds leftover properties followed by whole sub-component blocks — + `BEGIN`/`END` lines are content lines like any other, so nesting and unknown + sub-components fall out of the same representation, which is what the plan + thought needed a second table. +- **`VALARM` ownership: neither side writes the other's alarms.** `VALARM`s + round-trip in the residue and are never authored; local reminders live in + `task_alarms` and are never serialised. The collision `SYNC.md` describes was + `AndroidTasksDataSource.setAlarm` bulk-deleting *provider property rows* — in + the own-store world the two stores are disjoint and neither can destroy the + other. Merging them is a later decision; the point is that it is now a + decision, not an accident. +- **The mapper claims a property only when it can reproduce it exactly.** + Everything else stays in the residue: a value it cannot parse or that is out of + range (`PRIORITY:11`, `PERCENT-COMPLETE:150`, `SEQUENCE:x`, `STATUS:X-DEFERRED`), + a time it can read but not reproduce (floating, or a `TZID` this device's tzdb + lacks), and a `DTSTART`/`DUE` pair that disagrees on value type or timezone — + one `is_all_day` flag and one `timezone` column cannot author both, and + flattening the odd one out destroys it. Clamping or defaulting any of these + would be a silent rewrite of somebody's data. `SUPPRESSED_BY_RESIDUE` makes + "present in the residue" mean "do not author this". +- ⚠️ **Suppression is conditional, or editing breaks.** A residue property + suppresses its column only while the two still agree. If the user edits a field + whose stamp was unreproducible, the stale residue copy is evicted and the + column is authored — otherwise changing the due date of an imported task would + silently do nothing on the server. `RELATED-TO` is the deliberate exception: a + parent we have not fetched is not the same as no parent, so an unresolvable + link is kept rather than destroyed. +- **One normalisation is allowed beyond the stated allowlist**, and it is an + equivalence rather than a concession: an absent `STATUS` and + `STATUS:NEEDS-ACTION` mean the same thing for a to-do. Any other `STATUS` + compares exactly, so losing a completion still fails the corpus. + --- ## Chunk 2 — auth, discovery, account