cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(528): pass2 add recurrence_id field

Browse Source
This commit is contained in:
Edouard Vanbelle
2026-07-14 17:27:16 +02:00
parent 184b9dfab6
commit 02f67f7a5c
3 changed files with 318 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
migrations/20260913000001_calendar_events_recurrence_id.sql
+70
View File
@@ -0,0 +1,70 @@
-- ════════════════════════════════════════════════════════════════════════════
-- caldav.calendar_events — add RECURRENCE-ID column for exception instances
-- ════════════════════════════════════════════════════════════════════════════
-- Motivation: AtalayaLabs/OxiCloud#528 — CalDAV clients (Thunderbird, Apple
-- Calendar, Gnome Calendar, DAVx⁵) modify a single occurrence of a recurring
-- event by PUTting a separate VEVENT that shares the master's UID and adds
-- a RECURRENCE-ID identifying which occurrence is overridden (RFC 5545
-- §3.8.4.4).
--
-- Pre-#528 behaviour: modifications either hit a UID collision (silent
-- 500 or corrupt state) or overwrote the master. Post-#528 the exception
-- override lives as its own row keyed by
-- (calendar_id, ical_uid, recurrence_id), with the master identified by
-- `recurrence_id IS NULL`.
--
-- Related but distinct from parser Phase 1 (rewrite of extract_ical_property
-- on top of the `ical` crate) — that landed in the same branch to enable
-- parsing RECURRENCE-ID at all. This migration is the storage half.
--
-- No backfill needed — pre-migration events all become masters (NULL). No
-- existing exception rows existed because the parser couldn't read them.
-- ════════════════════════════════════════════════════════════════════════════
BEGIN;
-- Column: nullable. NULL = master, non-NULL = exception instance whose
-- value pinpoints which occurrence of the recurring master is being
-- overridden. TIMESTAMPTZ so both timed (DATE-TIME) and all-day (DATE)
-- RECURRENCE-IDs fit — the domain-side `parse_ical_datetime` normalises
-- both into `DateTime<Utc>` (all-day → midnight UTC of the target date).
ALTER TABLE caldav.calendar_events
ADD COLUMN recurrence_id TIMESTAMP WITH TIME ZONE NULL;
COMMENT ON COLUMN caldav.calendar_events.recurrence_id IS
'RFC 5545 §3.8.4.4 RECURRENCE-ID. NULL on the master, non-NULL on '
'per-instance exception overrides. Keyed with (calendar_id, ical_uid) '
'via the two partial unique indexes below.';
-- Partial unique index: at most one master row per (calendar_id, ical_uid).
--
-- Without this a client that re-uses a UID across calendar events (e.g. a
-- pre-2026-08 import that didn't dedupe) could produce two masters — the
-- lookup by (calendar_id, ical_uid) WHERE recurrence_id IS NULL would then
-- be ambiguous and the exception-routing logic would either overwrite the
-- wrong master or refuse to insert. Pre-migration duplicates would fail
-- this index creation; if that happens, the reconciliation is out of scope
-- for this migration (dedup script would go here — but the existing
-- codebase generates fresh UIDs on ambiguity so it shouldn't fire in
-- practice).
CREATE UNIQUE INDEX idx_calendar_events_master_unique
ON caldav.calendar_events (calendar_id, ical_uid)
WHERE recurrence_id IS NULL;
-- Partial unique index: at most one exception override per
-- (calendar_id, ical_uid, recurrence_id). Prevents two rows both claiming
-- to override the same instance of the same master — which would confuse
-- the client on next PROPFIND.
CREATE UNIQUE INDEX idx_calendar_events_exception_unique
ON caldav.calendar_events (calendar_id, ical_uid, recurrence_id)
WHERE recurrence_id IS NOT NULL;
-- Read-path index for the "give me the master + all its exceptions"
-- query the PROPFIND handler will run. Covered by the two unique indexes
-- above only partially — this covering index reads the full
-- (calendar_id, ical_uid) pair in one seek regardless of which side of
-- the master/exception split.
CREATE INDEX idx_calendar_events_uid_lookup
ON caldav.calendar_events (calendar_id, ical_uid);
COMMIT;
src/domain/entities/calendar_event.rs
+178 -7
View File
@@ -51,6 +51,29 @@ pub struct CalendarEvent {
/// Recurrence rule in iCalendar RRULE format (optional)
rrule: Option<String>,
/// RECURRENCE-ID (RFC 5545 §3.8.4.4) — non-NULL on exception
/// instances of a recurring event, NULL on the master.
///
/// When a client (Thunderbird, Apple Calendar, Gnome Calendar, …)
/// modifies a SINGLE occurrence of a recurring event, it sends
/// a separate VEVENT that shares the master's UID and carries
/// a `RECURRENCE-ID` identifying which occurrence is being
/// overridden. That per-instance override lives as its own row
/// in `caldav.calendar_events`; the master row keeps NULL here.
///
/// Lookup key is `(calendar_id, ical_uid, recurrence_id)` —
/// enforced at the DB layer by two partial unique indexes:
///
/// * `(calendar_id, ical_uid) WHERE recurrence_id IS NULL` —
/// at most one master per UID per calendar.
/// * `(calendar_id, ical_uid, recurrence_id) WHERE
/// recurrence_id IS NOT NULL` — at most one override for a
/// given (master, instance) pair.
///
/// See AtalayaLabs/OxiCloud#528 for the ticket that motivated
/// this field, and `docs/plan/` (future) for the full model.
recurrence_id: Option<DateTime<Utc>>,
/// Unique identifier in iCalendar format (used for CalDAV sync)
ical_uid: String,
@@ -140,6 +163,7 @@ impl CalendarEvent {
end_time,
all_day,
rrule,
recurrence_id: None,
ical_uid: Uuid::new_v4().to_string(),
ical_data,
created_at: now,
@@ -209,6 +233,7 @@ impl CalendarEvent {
end_time,
all_day,
rrule,
recurrence_id: None,
ical_uid,
ical_data,
created_at,
@@ -268,10 +293,7 @@ impl CalendarEvent {
// and anything else means timed.
let all_day = dtstart_params
.get("VALUE")
.map(|vs| {
vs.iter()
.any(|v| v.eq_ignore_ascii_case("DATE"))
})
.map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE")))
.unwrap_or(false);
let start_time = Self::parse_ical_datetime(&dtstart_value, all_day).map_err(|e| {
@@ -299,6 +321,26 @@ impl CalendarEvent {
let ical_uid = Self::extract_ical_property(&ical_data, "UID")
.unwrap_or_else(|| Uuid::new_v4().to_string());
// RECURRENCE-ID (RFC 5545 §3.8.4.4). When present, this VEVENT
// is an override for a specific occurrence of a recurring
// master with the same UID. The parameter tells us whether the
// value is a date (all-day master) or datetime (timed master).
// A parse failure here downgrades to `None` — the VEVENT still
// gets stored, just as a plain event (worst case a client sync
// treats it as a new master, which the DB uniqueness will
// refuse; better a persistence error than a silent split).
let recurrence_id =
match Self::extract_ical_property_with_params(&ical_data, "RECURRENCE-ID") {
Some((value, params)) => {
let is_date = params
.get("VALUE")
.map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE")))
.unwrap_or(false);
Self::parse_ical_datetime(&value, is_date).ok()
}
None => None,
};
let now = Utc::now();
Ok(Self {
@@ -311,6 +353,7 @@ impl CalendarEvent {
end_time,
all_day,
rrule,
recurrence_id,
ical_uid,
ical_data,
created_at: now,
@@ -366,6 +409,24 @@ impl CalendarEvent {
}
/// Returns the event's iCalendar UID
/// Returns the RECURRENCE-ID for this event, if any. `None` on
/// masters and standalone (non-recurring) events; `Some` on
/// exception overrides that target a specific occurrence of a
/// recurring master with the same `ical_uid`.
pub fn recurrence_id(&self) -> Option<&DateTime<Utc>> {
self.recurrence_id.as_ref()
}
/// Set the RECURRENCE-ID on this event. Used by the repository
/// layer when reconstructing an entity from a stored row (the
/// column is read straight into the field — no re-parse of the
/// ical_data body). Passing `None` clears the marker, promoting
/// an exception back to a plain event.
pub fn set_recurrence_id(&mut self, recurrence_id: Option<DateTime<Utc>>) {
self.recurrence_id = recurrence_id;
self.updated_at = Utc::now();
}
pub fn ical_uid(&self) -> &str {
&self.ical_uid
}
@@ -591,8 +652,7 @@ impl CalendarEvent {
self.start_time = start_time;
}
if let Some((value, _params)) =
Self::extract_ical_property_with_params(&ical_data, "DTEND")
if let Some((value, _params)) = Self::extract_ical_property_with_params(&ical_data, "DTEND")
&& let Ok(end_time) = Self::parse_ical_datetime(&value, all_day)
{
self.end_time = end_time;
@@ -1059,7 +1119,7 @@ END:VCALENDAR\r
let ev = parse_ok(RECURRING_WITH_EXCEPTION);
assert_eq!(ev.summary(), "Daily standup");
assert_eq!(ev.ical_uid(), "daily-1@oxicloud.test");
assert_eq!(ev.rrule().as_deref(), Some("FREQ=DAILY;COUNT=10"));
assert_eq!(ev.rrule(), Some("FREQ=DAILY;COUNT=10"));
}
#[test]
@@ -1121,4 +1181,115 @@ END:VCALENDAR\r
let v = CalendarEvent::extract_ical_property(TIMED_EVENT, "dtstart");
assert_eq!(v.as_deref(), Some("20260101T120000Z"));
}
// ─────────────────────────────────────────────────────────────
// Phase 2 — RECURRENCE-ID extraction into the entity
// ─────────────────────────────────────────────────────────────
#[test]
fn master_event_has_no_recurrence_id() {
// A plain VEVENT (no RECURRENCE-ID line) should carry a NULL
// recurrence_id — that's what marks it as a master in the DB.
let ev = parse_ok(TIMED_EVENT);
assert!(
ev.recurrence_id().is_none(),
"master should have recurrence_id = None"
);
}
#[test]
fn recurring_master_has_no_recurrence_id_even_with_rrule() {
// The presence of RRULE on the master does not by itself
// populate recurrence_id — only RECURRENCE-ID does. The
// exception-instance VEVENT in the same VCALENDAR carries
// RECURRENCE-ID; `parse_first_vevent` returns the master, so
// we get `None` here. Phase 3 will introduce a `parse_all_events`
// helper to surface the exceptions.
let ev = parse_ok(RECURRING_WITH_EXCEPTION);
assert!(
ev.recurrence_id().is_none(),
"master with RRULE should still have recurrence_id = None"
);
assert_eq!(ev.rrule().as_deref(), Some("FREQ=DAILY;COUNT=10"));
}
#[test]
fn timed_exception_populates_recurrence_id() {
// A standalone exception-override VEVENT (as sent by a client
// that's already synced the master and is now modifying one
// instance) parses with recurrence_id = the RECURRENCE-ID's
// timestamp. This is the phase-2 half of #528 — the value is
// preserved through the domain model; phase 3 will use it to
// route inserts to their own row.
let exception = "\
BEGIN:VCALENDAR\r
VERSION:2.0\r
PRODID:-//OxiCloud test//EN\r
BEGIN:VEVENT\r
UID:daily-1@oxicloud.test\r
DTSTAMP:20260101T100000Z\r
DTSTART:20260103T110000Z\r
DTEND:20260103T120000Z\r
SUMMARY:Daily standup — rescheduled\r
RECURRENCE-ID:20260103T090000Z\r
END:VEVENT\r
END:VCALENDAR\r
";
let ev = parse_ok(exception);
let rid = ev
.recurrence_id()
.expect("exception must have recurrence_id set");
assert_eq!(
rid.to_rfc3339(),
"2026-01-03T09:00:00+00:00",
"RECURRENCE-ID must parse to the timed override timestamp"
);
}
#[test]
fn all_day_exception_populates_recurrence_id_at_midnight() {
// RECURRENCE-ID;VALUE=DATE:20260112 — the exact shape #528
// flagged. Domain normalises the DATE form to midnight UTC on
// the given day so the field's type stays `DateTime<Utc>`.
let exception = "\
BEGIN:VCALENDAR\r
VERSION:2.0\r
PRODID:-//OxiCloud test//EN\r
BEGIN:VEVENT\r
UID:weekly-allday@oxicloud.test\r
DTSTAMP:20260101T100000Z\r
DTSTART;VALUE=DATE:20260113\r
DTEND;VALUE=DATE:20260114\r
SUMMARY:Weekly all-day — rescheduled\r
RECURRENCE-ID;VALUE=DATE:20260112\r
END:VEVENT\r
END:VCALENDAR\r
";
let ev = parse_ok(exception);
let rid = ev
.recurrence_id()
.expect("all-day exception must have recurrence_id set");
assert_eq!(
rid.to_rfc3339(),
"2026-01-12T00:00:00+00:00",
"all-day RECURRENCE-ID must normalise to 00:00:00 UTC of the target date"
);
}
#[test]
fn set_recurrence_id_setter_round_trips() {
// Repository rehydration path: `with_id` initialises
// recurrence_id to None; the repo calls `set_recurrence_id`
// with the DB column value. Prove both branches survive the
// setter cleanly.
let mut ev = parse_ok(TIMED_EVENT);
assert!(ev.recurrence_id().is_none());
let target = Utc.with_ymd_and_hms(2026, 3, 15, 12, 0, 0).unwrap();
ev.set_recurrence_id(Some(target));
assert_eq!(ev.recurrence_id(), Some(&target));
ev.set_recurrence_id(None);
assert!(ev.recurrence_id().is_none());
}
}
src/infrastructure/repositories/pg/calendar_event_pg_repository.rs
+70 -44
View File
@@ -30,10 +30,11 @@ impl CalendarEventRepository for CalendarEventPgRepository {
sqlx::query(
r#"
INSERT INTO caldav.calendar_events (
id, calendar_id, summary, description, location, start_time, end_time,
all_day, rrule, created_at, updated_at, ical_uid, ical_data
id, calendar_id, summary, description, location, start_time, end_time,
all_day, rrule, created_at, updated_at, ical_uid, ical_data,
recurrence_id
)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)
"#,
)
.bind(event.id())
@@ -49,6 +50,10 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.bind(event.updated_at())
.bind(event.ical_uid())
.bind(event.ical_data())
// NULL on masters, non-NULL on exception overrides — see the
// `20260913000001_calendar_events_recurrence_id.sql` migration
// and `docs/architecture/rebac-authorization.md` follow-up doc.
.bind(event.recurrence_id().copied())
.execute(&*self.pool)
.await
.map_err(|e| {
@@ -68,16 +73,17 @@ impl CalendarEventRepository for CalendarEventPgRepository {
sqlx::query(
r#"
UPDATE caldav.calendar_events
SET summary = $1,
description = $2,
location = $3,
start_time = $4,
end_time = $5,
all_day = $6,
SET summary = $1,
description = $2,
location = $3,
start_time = $4,
end_time = $5,
all_day = $6,
rrule = $7,
ical_data = $8,
updated_at = $9
WHERE id = $10
recurrence_id = $9,
updated_at = $10
WHERE id = $11
"#,
)
.bind(event.summary())
@@ -88,6 +94,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.bind(event.all_day())
.bind(event.rrule())
.bind(event.ical_data())
.bind(event.recurrence_id().copied())
.bind(now)
.bind(event.id())
.execute(&*self.pool)
@@ -126,12 +133,12 @@ impl CalendarEventRepository for CalendarEventPgRepository {
) -> CalendarEventRepositoryResult<Vec<CalendarEvent>> {
let rows = sqlx::query(
r#"
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data, recurrence_id
FROM caldav.calendar_events
WHERE calendar_id = $1
WHERE calendar_id = $1
AND (
(start_time >= $2 AND start_time < $3) OR
(end_time > $2 AND end_time <= $3) OR
@@ -152,7 +159,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
let mut events = Vec::new();
for row in rows {
let event = CalendarEvent::with_id(
let mut event = CalendarEvent::with_id(
row.get("id"),
row.get("calendar_id"),
row.get("summary"),
@@ -170,6 +177,11 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.map_err(|e| {
DomainError::database_error(format!("Error creating calendar event: {}", e))
})?;
// Rehydrate the RECURRENCE-ID after entity construction —
// `with_id` initialises to `None` because the field predates
// the rest of the constructor signature (#528). Keeping
// `with_id` unchanged avoids ripple-changing every caller.
event.set_recurrence_id(row.get::<Option<DateTime<Utc>>, _>("recurrence_id"));
events.push(event);
}
@@ -179,10 +191,10 @@ impl CalendarEventRepository for CalendarEventPgRepository {
async fn find_event_by_id(&self, id: &Uuid) -> CalendarEventRepositoryResult<CalendarEvent> {
let row = sqlx::query(
r#"
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data, recurrence_id
FROM caldav.calendar_events
WHERE id = $1
"#,
@@ -195,11 +207,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
})?
.ok_or_else(|| DomainError::not_found("Calendar Event", id.to_string()))?;
// In a real implementation, we would build a complete CalendarEvent object
// For simplicity, we create an object with default values to
// demonstrate the approach without macros
let event = CalendarEvent::with_id(
let mut event = CalendarEvent::with_id(
row.get("id"),
row.get("calendar_id"),
row.get("summary"),
@@ -217,6 +225,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.map_err(|e| {
DomainError::database_error(format!("Error creating calendar event: {}", e))
})?;
event.set_recurrence_id(row.get::<Option<DateTime<Utc>>, _>("recurrence_id"));
Ok(event)
}
@@ -227,10 +236,10 @@ impl CalendarEventRepository for CalendarEventPgRepository {
) -> CalendarEventRepositoryResult<Vec<CalendarEvent>> {
let rows = sqlx::query(
r#"
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data, recurrence_id
FROM caldav.calendar_events
WHERE calendar_id = $1
ORDER BY start_time
@@ -245,7 +254,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
let mut events = Vec::new();
for row in rows {
let event = CalendarEvent::with_id(
let mut event = CalendarEvent::with_id(
row.get("id"),
row.get("calendar_id"),
row.get("summary"),
@@ -263,6 +272,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.map_err(|e| {
DomainError::database_error(format!("Error creating calendar event: {}", e))
})?;
event.set_recurrence_id(row.get::<Option<DateTime<Utc>>, _>("recurrence_id"));
events.push(event);
}
@@ -278,10 +288,10 @@ impl CalendarEventRepository for CalendarEventPgRepository {
let rows = sqlx::query(
r#"
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data, recurrence_id
FROM caldav.calendar_events
WHERE calendar_id = $1 AND summary ILIKE $2
ORDER BY start_time
@@ -297,7 +307,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
let mut events = Vec::new();
for row in rows {
let event = CalendarEvent::with_id(
let mut event = CalendarEvent::with_id(
row.get("id"),
row.get("calendar_id"),
row.get("summary"),
@@ -315,6 +325,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.map_err(|e| {
DomainError::database_error(format!("Error creating calendar event: {}", e))
})?;
event.set_recurrence_id(row.get::<Option<DateTime<Utc>>, _>("recurrence_id"));
events.push(event);
}
@@ -326,14 +337,21 @@ impl CalendarEventRepository for CalendarEventPgRepository {
calendar_id: &Uuid,
ical_uid: &str,
) -> CalendarEventRepositoryResult<Option<CalendarEvent>> {
// Phase 2 note: this method looks up "an event with this UID"
// — the SELECT still isn't filtered on `recurrence_id IS NULL`
// because the phase-3 handler routing (which will distinguish
// master vs. exception override at PUT time) is where the
// filter actually needs to live. For phase 2 the invariant is
// enforced only at INSERT time via the two partial unique
// indexes; reads see whatever's there.
let row_opt = sqlx::query(
r#"
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data, recurrence_id
FROM caldav.calendar_events
WHERE calendar_id = $1 AND ical_uid = $2
WHERE calendar_id = $1 AND ical_uid = $2 AND recurrence_id IS NULL
"#,
)
.bind(calendar_id)
@@ -346,7 +364,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
match row_opt {
Some(row) => {
let event = CalendarEvent::with_id(
let mut event = CalendarEvent::with_id(
row.get("id"),
row.get("calendar_id"),
row.get("summary"),
@@ -364,6 +382,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.map_err(|e| {
DomainError::database_error(format!("Error creating calendar event: {}", e))
})?;
event.set_recurrence_id(row.get::<Option<DateTime<Utc>>, _>("recurrence_id"));
Ok(Some(event))
}
None => Ok(None),
@@ -375,12 +394,18 @@ impl CalendarEventRepository for CalendarEventPgRepository {
calendar_id: &Uuid,
ical_uids: &[String],
) -> CalendarEventRepositoryResult<Vec<CalendarEvent>> {
// Batch UID lookup returns ALL rows for the given UIDs, both
// masters and exception overrides. Callers that want just
// masters filter downstream. Same phase-2 policy as the
// single-UID variant — read-side filtering is a phase-3
// concern; the DB unique indexes are what guarantee at most
// one master + N distinct exceptions per (calendar, UID).
let rows = sqlx::query(
r#"
SELECT
id, calendar_id, summary, description, location,
start_time, end_time, all_day, rrule,
created_at, updated_at, ical_uid, ical_data
created_at, updated_at, ical_uid, ical_data, recurrence_id
FROM caldav.calendar_events
WHERE calendar_id = $1 AND ical_uid = ANY($2)
ORDER BY start_time
@@ -396,7 +421,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
let mut events = Vec::new();
for row in rows {
let event = CalendarEvent::with_id(
let mut event = CalendarEvent::with_id(
row.get("id"),
row.get("calendar_id"),
row.get("summary"),
@@ -414,6 +439,7 @@ impl CalendarEventRepository for CalendarEventPgRepository {
.map_err(|e| {
DomainError::database_error(format!("Error creating calendar event: {}", e))
})?;
event.set_recurrence_id(row.get::<Option<DateTime<Utc>>, _>("recurrence_id"));
events.push(event);
}