From 4fea176e28804f387c11a58ba5ebaf9c655c105a Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 13 Jul 2026 15:01:11 +0200 Subject: [PATCH] fix: drop occurrences via EXDATE on calendars with no _sync_id (#47) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cancelled-exception fix works on synced calendars but not local ones. A cancelled exception only attaches to its parent through ORIGINAL_SYNC_ID; a local event has no _sync_id, so the link never forms and the provider's expansion of the *parent* collapses — every other occurrence disappears, which is the original #47 corruption, just on a different calendar type. Verified on-device both ways: a DAVx5 series survives a single-occurrence delete, the same series on a LOCAL calendar vanishes entirely. deleteOccurrence now branches on _sync_id. Synced events keep the (verified) exception path. Events without one — local calendars, and synced events not yet pushed — add the occurrence to the master's EXDATE, which needs no parent link and is the canonical iCalendar way to drop one; a sync adapter carries it upstream unchanged if the calendar later syncs. Two provider quirks shape the write (both observed on a Pixel): - An EXDATE-only update is not treated as a recurrence change: the expanded Instances rows are left alone, so the occurrence stays visible. The time/recurrence set has to ride along to force re-expansion. - DTSTART alone is worse — the provider then recomputes lastDate as if the event were a single instance and collapses the series to its first occurrence. DTSTART + DURATION + RRULE + zone together re-expand it correctly. This path is reached in normal use: Calendula's own contact special-date calendars are local and hold all-day yearly series, so deleting one birthday occurrence went through it. All-day series take the VALUE=DATE EXDATE form. Adds pure buildOccurrenceExdateValues + JVM tests (timed, append, duplicate fold, all-day). Co-Authored-By: Claude Opus 4.8 --- .../data/calendar/CalendarDataSource.kt | 46 +++++++++--- .../data/calendar/EventWriteMapper.kt | 67 +++++++++++++++++ .../data/calendar/EventWriteMapperTest.kt | 71 +++++++++++++++++++ 3 files changed, 176 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt index 84a3e2a..110518e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt @@ -979,6 +979,8 @@ class AndroidCalendarDataSource @Inject constructor( CalendarContract.Events.EVENT_TIMEZONE, CalendarContract.Events.DURATION, CalendarContract.Events.ALL_DAY, + CalendarContract.Events._SYNC_ID, + CalendarContract.Events.EXDATE, ), null, null, null, )?.use { c -> @@ -989,6 +991,8 @@ class AndroidCalendarDataSource @Inject constructor( timezone = c.getString(2), duration = c.getString(3), allDay = c.getInt(4), + syncId = c.getString(5), + exdate = c.getString(6), ) } else { null @@ -1001,6 +1005,9 @@ class AndroidCalendarDataSource @Inject constructor( val timezone: String?, val duration: String?, val allDay: Int, + /** Null on a local calendar (and before a synced event's first push). */ + val syncId: String? = null, + val exdate: String? = null, ) { /** UNTIL cutoff for ending the series before the occurrence at [beginMillis]. */ fun truncationCutoff(beginMillis: Long): Long = previousLocalDayEndUtcMillis( @@ -1191,15 +1198,38 @@ class AndroidCalendarDataSource @Inject constructor( } override fun deleteOccurrence(eventId: Long, beginMillis: Long) { - // A cancelled exception row hides exactly this occurrence; the sync - // adapter turns it into an EXDATE/cancelled VEVENT upstream. It has to - // carry the full time set (DTSTART + DURATION + zone), not just STATUS: - // the provider only demotes the cloned exception to a single instance — - // clearing the inherited RRULE — when it can derive that instance from - // those columns. A STATUS-only cancel left the RRULE standing and - // cancelled the whole series, wiping every other occurrence (#47), the - // same trap the edit path documents (Codeberg #16). val row = querySeriesRow(eventId) + if (row.syncId == null) { + // No _sync_id — a local calendar, or a synced event not pushed yet. + // A cancelled exception can only attach to its parent through + // ORIGINAL_SYNC_ID, so with none the link never forms and the + // provider's expansion of the *parent* collapses, taking every other + // occurrence with it (#47 on a local calendar). EXDATE needs no link. + // Calendula's own contact special-date calendars are local and hold + // yearly series, so this path is reached in normal use. + val values = buildOccurrenceExdateValues( + existingExdate = row.exdate, + occurrenceMillis = beginMillis, + dtStartMillis = row.dtStartMillis, + rrule = row.rrule, + duration = row.duration, + timezone = row.timezone, + allDay = row.allDay, + ) + val updated = resolver.update( + ContentUris.withAppendedId(CalendarContract.Events.CONTENT_URI, eventId), + values.toContentValues(), null, null, + ) + if (updated == 0) { + throw WriteFailedException("exdate occurrence event id=$eventId begin=$beginMillis") + } + return + } + // A cancelled exception row hides exactly this occurrence; the sync + // adapter turns it into an EXDATE/cancelled VEVENT upstream. It carries + // the full time set (DTSTART + DURATION + zone) so the provider derives a + // single instance rather than cloning the master's RRULE — the same trap + // the edit path documents (Codeberg #16). val values = buildOccurrenceCancelValues( originalInstanceMillis = beginMillis, dtStartMillis = beginMillis, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt index e310c48..0768f55 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt @@ -209,6 +209,73 @@ internal fun buildOccurrenceCancelValues( put(CalendarContract.Events.STATUS, CalendarContract.Events.STATUS_CANCELED) } +/** + * The master-row columns that drop the occurrence at [occurrenceMillis] from a + * series by adding it to `EXDATE` — the path for events that have **no + * `_sync_id`** (a local calendar, or a synced event not yet pushed). + * + * A cancelled exception row (see [buildOccurrenceCancelValues]) only attaches to + * its parent through `ORIGINAL_SYNC_ID`. Without a `_sync_id` the link never + * forms, and the provider's expansion of the *parent* collapses — every other + * occurrence disappears (Codeberg #47, reproduced on a local calendar). EXDATE + * needs no link, and is the canonical iCalendar way to drop an occurrence, so a + * sync adapter carries it upstream unchanged if the calendar later syncs. + * + * The whole time/recurrence set is rewritten alongside it on purpose. The + * provider does **not** treat an EXDATE-only update as a recurrence change: it + * leaves the expanded `Instances` rows untouched, so the occurrence stays visible + * (and, symmetrically, un-excluding one leaves it hidden). Writing DTSTART with + * it forces the re-expansion — but DTSTART *alone* makes the provider recompute + * `lastDate` as if the event were a single instance, collapsing the series to its + * first occurrence. Passing DTSTART + DURATION + RRULE + zone together is what + * re-expands it correctly. All observed on a Pixel; see the #47 notes. + * + * EXDATE is a comma-separated list, so an existing one is appended to (a repeat + * of the same occurrence is folded away). All-day series take the `VALUE=DATE` + * form (`yyyyMMdd`), timed ones the UTC date-time form (`yyyyMMddTHHmmssZ`). + */ +internal fun buildOccurrenceExdateValues( + existingExdate: String?, + occurrenceMillis: Long, + dtStartMillis: Long, + rrule: String?, + duration: String?, + timezone: String?, + allDay: Int, +): Map { + val stamp = formatExdateStamp(occurrenceMillis, isAllDay = allDay != 0) + val existing = existingExdate?.split(',') + ?.map { it.trim() } + ?.filter { it.isNotEmpty() } + .orEmpty() + val merged = (existing + stamp).distinct().joinToString(",") + return mapOf( + CalendarContract.Events.EXDATE to merged, + CalendarContract.Events.DTSTART to dtStartMillis, + CalendarContract.Events.RRULE to rrule, + CalendarContract.Events.DURATION to duration, + CalendarContract.Events.EVENT_TIMEZONE to timezone, + CalendarContract.Events.ALL_DAY to allDay, + ) +} + +/** + * One EXDATE entry for the occurrence starting at [occurrenceMillis]. Both forms + * are UTC: the provider stores an all-day DTSTART at UTC midnight, so its date + * reads off the UTC calendar day. + */ +private fun formatExdateStamp(occurrenceMillis: Long, isAllDay: Boolean): String { + val utc = Instant.ofEpochMilli(occurrenceMillis).atZone(ZoneOffset.UTC) + return if (isAllDay) { + "%04d%02d%02d".format(utc.year, utc.monthValue, utc.dayOfMonth) + } else { + "%04d%02d%02dT%02d%02d%02dZ".format( + utc.year, utc.monthValue, utc.dayOfMonth, + utc.hour, utc.minute, utc.second, + ) + } +} + /** * The `EVENT_COLOR` / `EVENT_COLOR_KEY` columns for a colour selection. A * [colorKey] writes the key alone (the provider derives `EVENT_COLOR` from the diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt index 252ca37..7aa8d78 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt @@ -260,6 +260,77 @@ class EventWriteMapperTest { .isEqualTo(CalendarContract.Events.STATUS_CANCELED) } + // --- buildOccurrenceExdateValues ("delete only this event", no _sync_id) --- + + @Test + fun `exdate drop excludes the occurrence and rewrites the recurrence set`() { + // 2026-07-15T08:00:00Z. + val values = buildOccurrenceExdateValues( + existingExdate = null, + occurrenceMillis = 1_784_102_400_000L, + dtStartMillis = 1_783_929_600_000L, + rrule = "FREQ=DAILY;COUNT=5", + duration = "PT1H", + timezone = "Europe/Berlin", + allDay = 0, + ) + assertThat(values[CalendarContract.Events.EXDATE]).isEqualTo("20260715T080000Z") + // The whole time/recurrence set rides along: an EXDATE-only update is not + // treated as a recurrence change, so the provider would leave the expanded + // instances (and the occurrence) in place. DTSTART alone is worse — it + // makes the provider recompute lastDate as a single instance and collapse + // the series to its first occurrence. + assertThat(values[CalendarContract.Events.DTSTART]).isEqualTo(1_783_929_600_000L) + assertThat(values[CalendarContract.Events.RRULE]).isEqualTo("FREQ=DAILY;COUNT=5") + assertThat(values[CalendarContract.Events.DURATION]).isEqualTo("PT1H") + assertThat(values[CalendarContract.Events.EVENT_TIMEZONE]).isEqualTo("Europe/Berlin") + } + + @Test + fun `exdate drop appends to an existing exdate list`() { + val values = buildOccurrenceExdateValues( + existingExdate = "20260714T080000Z", + occurrenceMillis = 1_784_102_400_000L, + dtStartMillis = 1_783_929_600_000L, + rrule = "FREQ=DAILY;COUNT=5", + duration = "PT1H", + timezone = "Europe/Berlin", + allDay = 0, + ) + assertThat(values[CalendarContract.Events.EXDATE]) + .isEqualTo("20260714T080000Z,20260715T080000Z") + } + + @Test + fun `exdate drop folds away a repeated occurrence`() { + val values = buildOccurrenceExdateValues( + existingExdate = "20260715T080000Z", + occurrenceMillis = 1_784_102_400_000L, + dtStartMillis = 1_783_929_600_000L, + rrule = "FREQ=DAILY;COUNT=5", + duration = "PT1H", + timezone = "Europe/Berlin", + allDay = 0, + ) + assertThat(values[CalendarContract.Events.EXDATE]).isEqualTo("20260715T080000Z") + } + + @Test + fun `all-day exdate drop uses the date-only form`() { + // An all-day DTSTART sits at UTC midnight, so the date reads off UTC. + val values = buildOccurrenceExdateValues( + existingExdate = null, + occurrenceMillis = 1_784_073_600_000L, // 2026-07-15T00:00:00Z + dtStartMillis = 1_783_900_800_000L, + rrule = "FREQ=YEARLY", + duration = "P1D", + timezone = "UTC", + allDay = 1, + ) + assertThat(values[CalendarContract.Events.EXDATE]).isEqualTo("20260715") + assertThat(values[CalendarContract.Events.ALL_DAY]).isEqualTo(1) + } + // --- per-event colour --- @Test