Keep deleted and edited occurrences when moving a series (#404)

Moving a recurring event to another calendar re-inserted its exception rows under the new event. That event has no `_sync_id` yet, so the rows never attached: deleted occurrences came back, and edited ones showed twice (original plus the edited copy).

The move now does what delete and edit-this-occurrence already do for unsynced series: cancelled occurrences become EXDATE entries on the moved series, edited ones become EXDATE plus a standalone event on the target calendar. The standalone copies are rolled back with the rest if the move fails.

`buildCopiedExceptionValues` is gone with its tests, and `ExceptionRowSnapshot`/`ExceptionProjection` only read the columns still used.

Adds `MoveEventExceptionsTest` (instrumented), which fails on the old code and passes now.

Co-authored-by: Jean-Luc Makiola <business@jeanlucmakiola.de>
Reviewed-on: https://codeberg.org/jlmakiola/calendula/pulls/404

app/src/androidTest/java/de/jeanlucmakiola/calendula/data/calendar/MoveEventExceptionsTest.kt

@@ -0,0 +1,91 @@
+package de.jeanlucmakiola.calendula.data.calendar
+
+import android.Manifest
+import android.content.ContentUris
+import android.content.ContentValues
+import android.provider.CalendarContract
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.test.rule.GrantPermissionRule
+import com.google.common.truth.Truth.assertThat
+import de.jeanlucmakiola.calendula.domain.toEditForm
+import kotlinx.datetime.TimeZone
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/**
+ * Moving a synced series keeps its deleted and edited occurrences, although the
+ * moved row has no `_sync_id` for exception rows to attach to.
+ */
+@RunWith(AndroidJUnit4::class)
+class MoveEventExceptionsTest {
+
+    @get:Rule
+    val permissionRule: GrantPermissionRule = GrantPermissionRule.grant(
+        Manifest.permission.READ_CALENDAR,
+        Manifest.permission.WRITE_CALENDAR,
+    )
+
+    private val context = InstrumentationRegistry.getInstrumentation().targetContext
+    private val resolver = context.contentResolver
+    private val ds = AndroidCalendarDataSource(context)
+    private val zone = TimeZone.currentSystemDefault()
+
+    private val day = 86_400_000L
+    private val start = 1_893_492_000_000L // 2030-01-01T10:00Z
+    private val hour = 3_600_000L
+
+    /** "day offset:title" of every instance on [calendarId]. */
+    private fun occurrences(calendarId: Long): List<String> =
+        ds.instances(start - day, start + 10 * day)
+            .filter { it.calendarId == calendarId }
+            .map { "${(it.start.toEpochMilliseconds() - start) / day}:${it.title}" }
+            .sorted()
+
+    @Test
+    fun movedSeriesKeepsCancelledAndModifiedOccurrences() {
+        val a = ds.createLocalCalendar("move-test-a", 0xFF336699.toInt(), null)
+        val b = ds.createLocalCalendar("move-test-b", 0xFF993366.toInt(), null)
+        try {
+            val src = insertSyncedSeries(a)
+            ds.deleteOccurrence(src, start + 2 * day)
+            val occurrence = ds.eventDetail(src, 540)!!.toEditForm(start + 3 * day, start + 3 * day + hour, zone)
+            ds.updateOccurrence(src, start + 3 * day, occurrence, occurrence.copy(title = "edited"), 540)
+            val expected = listOf("0:series", "1:series", "3:edited", "4:series")
+            assertThat(occurrences(a)).isEqualTo(expected)
+
+            val form = ds.eventDetail(src, 540)!!.toEditForm(start, start + hour, zone)
+            ds.moveEvent(src, b, form, form, 540)
+
+            assertThat(occurrences(a)).isEmpty()
+            assertThat(occurrences(b)).isEqualTo(expected)
+        } finally {
+            ds.deleteCalendar(a)
+            ds.deleteCalendar(b)
+        }
+    }
+
+    /** A daily series carrying a `_sync_id`, so its exceptions are real exception rows. */
+    private fun insertSyncedSeries(calendarId: Long): Long {
+        val accountName = resolver.query(
+            ContentUris.withAppendedId(CalendarContract.Calendars.CONTENT_URI, calendarId),
+            arrayOf(CalendarContract.Calendars.ACCOUNT_NAME), null, null, null,
+        )!!.use { it.moveToFirst(); it.getString(0) }
+        val uri = CalendarContract.Events.CONTENT_URI.buildUpon()
+            .appendQueryParameter(CalendarContract.CALLER_IS_SYNCADAPTER, "true")
+            .appendQueryParameter(CalendarContract.Calendars.ACCOUNT_NAME, accountName)
+            .appendQueryParameter(CalendarContract.Calendars.ACCOUNT_TYPE, CalendarContract.ACCOUNT_TYPE_LOCAL)
+            .build()
+        val values = ContentValues().apply {
+            put(CalendarContract.Events.CALENDAR_ID, calendarId)
+            put(CalendarContract.Events.TITLE, "series")
+            put(CalendarContract.Events.DTSTART, start)
+            put(CalendarContract.Events.DURATION, "P3600S")
+            put(CalendarContract.Events.RRULE, "FREQ=DAILY;COUNT=5")
+            put(CalendarContract.Events.EVENT_TIMEZONE, "UTC")
+            put(CalendarContract.Events._SYNC_ID, "move-test-series")
+        }
+        return ContentUris.parseId(resolver.insert(uri, values)!!)
+    }
+}

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt

@@ -36,7 +36,10 @@ import de.jeanlucmakiola.calendula.domain.contacts.SpecialDateType
 import de.jeanlucmakiola.calendula.domain.ics.IcsEvent
 import de.jeanlucmakiola.calendula.domain.ics.ParsedIcsEvent
 import de.jeanlucmakiola.calendula.domain.ics.semanticReminderMinutes
+import de.jeanlucmakiola.calendula.domain.ics.parseRfc2445DurationMillis
 import de.jeanlucmakiola.calendula.domain.rruleTruncatedAt
+import de.jeanlucmakiola.calendula.domain.toEditForm
+import kotlinx.datetime.TimeZone
 import kotlinx.datetime.toJavaLocalDate
 import java.time.Instant
 import java.time.LocalDate
@@ -238,8 +241,10 @@ interface CalendarDataSource {
      * and cancelled occurrences) to [targetCalendarId], returning the new
      * `Events._ID`. `CALENDAR_ID` is sync-adapter-owned and can't be updated in
      * place, so this is copy+delete: the row is re-inserted on the target (its
-     * `UID_2445` preserved), its exceptions, reminders and editable guests
-     * replayed, the [original]→[updated] field edits applied, then the source
+     * `UID_2445` preserved), its reminders and editable guests copied, its
+     * exceptions turned into EXDATEs (modified ones also into standalone events,
+     * as the new row has no `_sync_id` to attach them to), the
+     * [original]→[updated] field edits applied, then the source
      * deleted. The source is removed only once the copy fully succeeds; a failure
      * before that rolls the new event back, leaving the original untouched.
      * [allDayReminderTimeMinutes]: see [insertEvent].
@@ -1073,10 +1078,11 @@ class AndroidCalendarDataSource @Inject constructor(
             buildMovedMasterValues(master.event, targetCalendarId, uid).toContentValues(),
         )?.let(ContentUris::parseId)
             ?: throw WriteFailedException("insert moved event into calendar id=$targetCalendarId")
+        val detached = mutableListOf<Long>()
         try {
             copyReminderRows(fromEventId = eventId, toEventId = newEventId)
             insertAttendees(newEventId, editableAttendees(eventId))
-            copyExceptions(fromEventId = eventId, toEventId = newEventId)
+            copyExceptions(eventId, newEventId, targetCalendarId, allDayReminderTimeMinutes, detached)
             // Apply the user's field edits exactly as a same-calendar "all
             // events" save would: the moved master carries the source's values,
             // so the dirty diff against [original] writes only what changed
@@ -1086,6 +1092,7 @@ class AndroidCalendarDataSource @Inject constructor(
             // Undo the partial copy so a failed move leaves nothing behind; the
             // source is still intact (it's deleted only past this point).
             runCatching { deleteEvent(newEventId) }
+            detached.forEach(::rollBackDetached)
             throw t
         }
         deleteEvent(eventId)
@@ -1162,44 +1169,47 @@ class AndroidCalendarDataSource @Inject constructor(
         }
 
     /**
-     * Replay every exception of the series [fromEventId] against the moved series
-     * [toEventId]. The copy preserved the recurrence skeleton, so each occurrence
-     * time still resolves: a cancelled occurrence is re-hidden with a cancelled
-     * exception, a modified one is re-inserted with its fields (then its reminders
-     * reconciled and editable guests copied). No-op for a non-recurring event.
+     * Carry every exception of the series [fromEventId] over to the moved series
+     * [toEventId]. The moved row has no `_sync_id` yet, so exception rows would
+     * never attach to it ([deleteOccurrence], [detachOccurrence]): a cancelled
+     * occurrence becomes an EXDATE, a modified one an EXDATE plus a standalone
+     * event on [targetCalendarId], its id added to [detached] for rollback.
      */
-    private fun copyExceptions(fromEventId: Long, toEventId: Long) {
-        queryExceptionRows(fromEventId).forEach { ex ->
-            if (ex.isCancelled) {
-                val values = ContentValues().apply {
-                    put(CalendarContract.Events.ORIGINAL_INSTANCE_TIME, ex.originalInstanceMillis)
-                    put(CalendarContract.Events.STATUS, CalendarContract.Events.STATUS_CANCELED)
-                }
-                resolver.insert(
-                    ContentUris.withAppendedId(
-                        CalendarContract.Events.CONTENT_EXCEPTION_URI, toEventId,
-                    ),
-                    values,
-                ) ?: throw WriteFailedException("copy cancelled occurrence to event id=$toEventId")
-            } else {
-                val uri = resolver.insert(
-                    ContentUris.withAppendedId(
-                        CalendarContract.Events.CONTENT_EXCEPTION_URI, toEventId,
-                    ),
-                    buildCopiedExceptionValues(ex).toContentValues(),
-                ) ?: throw WriteFailedException("copy modified occurrence to event id=$toEventId")
-                val newExceptionId = ContentUris.parseId(uri)
-                // The provider may clone the parent's reminders onto the new
-                // exception; reconcile to the source exception's exact set so
-                // they neither double nor drop. Same DTSTART → same all-day
-                // encoding, so the stored offsets match directly.
-                reconcileReminders(
-                    newExceptionId,
-                    queryReminders(ex.exceptionEventId).map { it.minutes },
-                )
-                insertAttendees(newExceptionId, editableAttendees(ex.exceptionEventId))
-            }
+    private fun copyExceptions(
+        fromEventId: Long,
+        toEventId: Long,
+        targetCalendarId: Long,
+        allDayReminderTimeMinutes: Int,
+        detached: MutableList<Long>,
+    ) {
+        val exceptions = queryExceptionRows(fromEventId)
+        if (exceptions.isEmpty()) return
+        val zone = TimeZone.currentSystemDefault()
+        exceptions.filterNot { it.isCancelled }.forEach { ex ->
+            val detail = eventDetail(ex.exceptionEventId, allDayReminderTimeMinutes)
+                ?: throw WriteFailedException("read modified occurrence id=${ex.exceptionEventId}")
+            val endMillis = ex.dtEndMillis ?: (ex.dtStartMillis + parseRfc2445DurationMillis(ex.duration))
+            val form = detail.toEditForm(ex.dtStartMillis, endMillis, zone)
+                .copy(calendarId = targetCalendarId)
+                .toDetachedOccurrence()
+            detached += insertEvent(form, allDayReminderTimeMinutes)
         }
+        val row = querySeriesRow(toEventId)
+        val values = buildOccurrencesExdateValues(
+            existingExdate = row.exdate,
+            occurrences = exceptions.map { it.originalInstanceMillis },
+            dtStartMillis = row.dtStartMillis,
+            rrule = row.rrule,
+            rdate = row.rdate,
+            duration = row.duration,
+            timezone = row.timezone,
+            allDay = row.allDay,
+        )
+        val updated = resolver.update(
+            ContentUris.withAppendedId(CalendarContract.Events.CONTENT_URI, toEventId),
+            values.toContentValues(), null, null,
+        )
+        if (updated == 0) throw WriteFailedException("exdate moved exceptions event id=$toEventId")
     }
 
     /** The series' exception rows (modified + cancelled), oldest occurrence first. */
@@ -1218,18 +1228,10 @@ class AndroidCalendarDataSource @Inject constructor(
                 exceptionEventId = r.getLong(ExceptionProjection.IDX_ID),
                 originalInstanceMillis = r.getLong(ExceptionProjection.IDX_ORIGINAL_INSTANCE_TIME),
                 isCancelled = status == CalendarContract.Events.STATUS_CANCELED,
-                status = status,
-                title = r.getString(ExceptionProjection.IDX_TITLE).orEmpty(),
-                isAllDay = r.getInt(ExceptionProjection.IDX_ALL_DAY) != 0,
                 dtStartMillis = r.getLong(ExceptionProjection.IDX_DTSTART),
                 dtEndMillis = r.getLong(ExceptionProjection.IDX_DTEND)
                     .takeUnless { r.isNull(ExceptionProjection.IDX_DTEND) },
                 duration = r.getString(ExceptionProjection.IDX_DURATION),
-                timezone = r.getString(ExceptionProjection.IDX_EVENT_TIMEZONE),
-                availability = r.getInt(ExceptionProjection.IDX_AVAILABILITY),
-                accessLevel = r.getInt(ExceptionProjection.IDX_ACCESS_LEVEL),
-                location = r.getString(ExceptionProjection.IDX_LOCATION),
-                description = r.getString(ExceptionProjection.IDX_DESCRIPTION),
             )
         }
     } ?: emptyList()

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt

@@ -372,48 +372,11 @@ internal data class ExceptionRowSnapshot(
     val exceptionEventId: Long,
     val originalInstanceMillis: Long,
     val isCancelled: Boolean,
-    val status: Int?,
-    val title: String,
-    val isAllDay: Boolean,
     val dtStartMillis: Long,
     val dtEndMillis: Long?,
     val duration: String?,
-    val timezone: String?,
-    val availability: Int,
-    val accessLevel: Int,
-    val location: String?,
-    val description: String?,
 )
 
-/**
- * Column values replaying a *modified* occurrence [snapshot] against the moved
- * series via `CONTENT_EXCEPTION_URI`. Like [buildOccurrenceExceptionValues] the
- * length travels as DURATION (the provider rejects DTEND on an exception). A
- * cancelled occurrence is written separately (ORIGINAL_INSTANCE_TIME +
- * STATUS_CANCELED) — this builder is only for the modified case.
- */
-internal fun buildCopiedExceptionValues(snapshot: ExceptionRowSnapshot): Map<String, Any?> =
-    buildMap {
-        put(CalendarContract.Events.ORIGINAL_INSTANCE_TIME, snapshot.originalInstanceMillis)
-        put(CalendarContract.Events.TITLE, snapshot.title)
-        put(CalendarContract.Events.ALL_DAY, if (snapshot.isAllDay) 1 else 0)
-        put(CalendarContract.Events.DTSTART, snapshot.dtStartMillis)
-        put(
-            CalendarContract.Events.DURATION,
-            snapshot.duration?.takeIf { it.isNotBlank() }
-                ?: rfc2445Duration(
-                    (snapshot.dtEndMillis ?: snapshot.dtStartMillis) - snapshot.dtStartMillis,
-                    snapshot.isAllDay,
-                ),
-        )
-        put(CalendarContract.Events.EVENT_TIMEZONE, snapshot.timezone ?: "UTC")
-        put(CalendarContract.Events.AVAILABILITY, snapshot.availability)
-        put(CalendarContract.Events.ACCESS_LEVEL, snapshot.accessLevel)
-        put(CalendarContract.Events.EVENT_LOCATION, snapshot.location?.ifEmpty { null })
-        put(CalendarContract.Events.DESCRIPTION, snapshot.description?.ifEmpty { null })
-        snapshot.status?.let { put(CalendarContract.Events.STATUS, it) }
-    }
-
 /**
  * Column values for a *cancelled*-occurrence exception row ("delete only this
  * event"): inserting them at `Events.CONTENT_EXCEPTION_URI/<id>` makes the
@@ -479,9 +442,24 @@ internal fun buildOccurrenceExdateValues(
     duration: String?,
     timezone: String?,
     allDay: Int,
+): Map<String, Any?> = buildOccurrencesExdateValues(
+    existingExdate, listOf(occurrenceMillis), dtStartMillis, rrule, rdate, duration, timezone, allDay,
+)
+
+/** [buildOccurrenceExdateValues] excluding every one of [occurrences] in one write. */
+internal fun buildOccurrencesExdateValues(
+    existingExdate: String?,
+    occurrences: List<Long>,
+    dtStartMillis: Long,
+    rrule: String?,
+    rdate: String?,
+    duration: String?,
+    timezone: String?,
+    allDay: Int,
 ): Map<String, Any?> = mapOf(
-    CalendarContract.Events.EXDATE to
-        mergedExdate(existingExdate, occurrenceMillis, allDay != 0, timezone),
+    CalendarContract.Events.EXDATE to occurrences.fold(existingExdate) { exdate, millis ->
+        mergedExdate(exdate, millis, allDay != 0, timezone)
+    },
     CalendarContract.Events.DTSTART to dtStartMillis,
     CalendarContract.Events.RRULE to rrule,
     CalendarContract.Events.RDATE to rdate,

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt

@@ -251,40 +251,26 @@ internal object MoveMasterProjection {
 
 /**
  * The exception rows of a recurring series (`ORIGINAL_ID` = the series), read to
- * replay them against a moved copy (see [ExceptionRowSnapshot]). Both modified
+ * carry them over to a moved copy (see [ExceptionRowSnapshot]). Both modified
  * occurrences and cancellations (`STATUS_CANCELED`) are read; the query filters
- * `DELETED = 0` so provider tombstones aren't replayed.
+ * `DELETED = 0` so provider tombstones aren't carried over.
  */
 internal object ExceptionProjection {
     val COLUMNS: Array<String> = arrayOf(
         CalendarContract.Events._ID,
         CalendarContract.Events.ORIGINAL_INSTANCE_TIME,
         CalendarContract.Events.STATUS,
-        CalendarContract.Events.TITLE,
-        CalendarContract.Events.ALL_DAY,
         CalendarContract.Events.DTSTART,
         CalendarContract.Events.DTEND,
         CalendarContract.Events.DURATION,
-        CalendarContract.Events.EVENT_TIMEZONE,
-        CalendarContract.Events.AVAILABILITY,
-        CalendarContract.Events.ACCESS_LEVEL,
-        CalendarContract.Events.EVENT_LOCATION,
-        CalendarContract.Events.DESCRIPTION,
     )
 
     const val IDX_ID = 0
     const val IDX_ORIGINAL_INSTANCE_TIME = 1
     const val IDX_STATUS = 2
-    const val IDX_TITLE = 3
-    const val IDX_ALL_DAY = 4
-    const val IDX_DTSTART = 5
-    const val IDX_DTEND = 6
-    const val IDX_DURATION = 7
-    const val IDX_EVENT_TIMEZONE = 8
-    const val IDX_AVAILABILITY = 9
-    const val IDX_ACCESS_LEVEL = 10
-    const val IDX_LOCATION = 11
-    const val IDX_DESCRIPTION = 12
+    const val IDX_DTSTART = 3
+    const val IDX_DTEND = 4
+    const val IDX_DURATION = 5
 }
 
 internal object AttendeeProjection {

app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt

@@ -1179,47 +1179,21 @@ class EventWriteMapperTest {
         assertThat(values[CalendarContract.Events.DURATION]).isEqualTo("P5400S")
     }
 
-    // --- buildCopiedExceptionValues (calendar move: exception replay) ---
-
-    private fun exceptionSnapshot(
-        isCancelled: Boolean = false,
-        duration: String? = null,
-        // Occurrence starts one hour into the day and runs 30 minutes.
-        dtEndMillis: Long? = 1_781_164_800_000L + 3_600_000L + 1_800_000L,
-    ) = ExceptionRowSnapshot(
-        exceptionEventId = 42L,
-        originalInstanceMillis = 1_781_164_800_000L,
-        isCancelled = isCancelled,
-        status = if (isCancelled) CalendarContract.Events.STATUS_CANCELED else CalendarContract.Events.STATUS_CONFIRMED,
-        title = "Moved occurrence",
-        isAllDay = false,
-        dtStartMillis = 1_781_164_800_000L + 3_600_000L,
-        dtEndMillis = dtEndMillis,
-        duration = duration,
-        timezone = "Europe/Berlin",
-        availability = CalendarContract.Events.AVAILABILITY_BUSY,
-        accessLevel = CalendarContract.Events.ACCESS_DEFAULT,
-        location = "",
-        description = "Notes",
-    )
-
     @Test
-    fun `copied modified occurrence carries the original instance and DURATION not DTEND`() {
-        val values = buildCopiedExceptionValues(exceptionSnapshot())
-        assertThat(values[CalendarContract.Events.ORIGINAL_INSTANCE_TIME])
-            .isEqualTo(1_781_164_800_000L)
-        assertThat(values[CalendarContract.Events.TITLE]).isEqualTo("Moved occurrence")
-        assertThat(values[CalendarContract.Events.DTSTART]).isEqualTo(1_781_164_800_000L + 3_600_000L)
-        // 30-minute occurrence derived from DTEND, written as DURATION only.
-        assertThat(values[CalendarContract.Events.DURATION]).isEqualTo("P1800S")
-        assertThat(values).doesNotContainKey(CalendarContract.Events.DTEND)
-        assertThat(values).containsEntry(CalendarContract.Events.EVENT_LOCATION, null)
-    }
-
-    @Test
-    fun `copied modified occurrence keeps a stored duration verbatim`() {
-        val values = buildCopiedExceptionValues(exceptionSnapshot(duration = "P900S", dtEndMillis = null))
-        assertThat(values[CalendarContract.Events.DURATION]).isEqualTo("P900S")
+    fun `several occurrences are excluded in one exdate write`() {
+        val values = buildOccurrencesExdateValues(
+            existingExdate = "20260601T090000Z",
+            occurrences = listOf(1_781_168_400_000L, 1_781_254_800_000L, 1_781_168_400_000L),
+            dtStartMillis = 1_780_304_400_000L,
+            rrule = "FREQ=DAILY",
+            rdate = null,
+            duration = "P3600S",
+            timezone = "Europe/Berlin",
+            allDay = 0,
+        )
+        assertThat(values[CalendarContract.Events.EXDATE])
+            .isEqualTo("20260601T090000Z,20260611T090000Z,20260612T090000Z")
+        assertThat(values[CalendarContract.Events.RRULE]).isEqualTo("FREQ=DAILY")
     }
 
     @Test