diff --git a/src/application/adapters/caldav_adapter_test.rs b/src/application/adapters/caldav_adapter_test.rs index 5f7847c6..4f4284bc 100644 --- a/src/application/adapters/caldav_adapter_test.rs +++ b/src/application/adapters/caldav_adapter_test.rs @@ -35,6 +35,7 @@ mod tests { all_day: false, rrule: None, ical_uid: "uid-evt-001@oxicloud".to_string(), + recurrence_id: None, created_at: Utc.with_ymd_and_hms(2025, 1, 1, 0, 0, 0).unwrap(), updated_at: Utc.with_ymd_and_hms(2025, 1, 1, 0, 0, 0).unwrap(), } diff --git a/src/application/dtos/calendar_dto.rs b/src/application/dtos/calendar_dto.rs index 92fca841..de2e965d 100644 --- a/src/application/dtos/calendar_dto.rs +++ b/src/application/dtos/calendar_dto.rs @@ -89,6 +89,12 @@ pub struct CalendarEventDto { pub all_day: bool, pub rrule: Option, pub ical_uid: String, + /// RFC 5545 §3.8.4.4 RECURRENCE-ID. `None` on masters and on + /// non-recurring events; `Some` on per-instance exception + /// overrides. Two rows sharing (`calendar_id`, `ical_uid`) but + /// distinguished by this field represent a recurring master and + /// its modified occurrence(s) respectively (see #528). + pub recurrence_id: Option>, pub created_at: DateTime, pub updated_at: DateTime, } @@ -106,6 +112,7 @@ impl Default for CalendarEventDto { all_day: false, rrule: None, ical_uid: String::new(), + recurrence_id: None, created_at: Utc::now(), updated_at: Utc::now(), } @@ -125,6 +132,7 @@ impl From for CalendarEventDto { all_day: event.all_day(), rrule: event.rrule().map(|s| s.to_string()), ical_uid: event.ical_uid().to_string(), + recurrence_id: event.recurrence_id().copied(), created_at: *event.created_at(), updated_at: *event.updated_at(), } diff --git a/src/application/ports/calendar_ports.rs b/src/application/ports/calendar_ports.rs index 332da7ec..cd4781bf 100644 --- a/src/application/ports/calendar_ports.rs +++ b/src/application/ports/calendar_ports.rs @@ -6,6 +6,19 @@ use crate::common::errors::DomainError; use chrono::{DateTime, Utc}; use uuid::Uuid; +/// Result of a multi-VEVENT PUT (`upsert_ical_events`). See #528. +#[derive(Debug, Clone)] +pub struct UpsertEventsResult { + /// Every event that was persisted for this PUT. Ordered as they + /// appeared in the body — the master (if present) is typically + /// first, followed by exception overrides. + pub events: Vec, + /// True if at least one row was newly created; false if every + /// event replaced an existing row. Drives the handler's choice + /// between 201 Created and 204 No Content. + pub any_inserted: bool, +} + /// Port for external calendar storage mechanisms pub trait CalendarStoragePort: Send + Sync + 'static { // Calendar operations @@ -53,6 +66,23 @@ pub trait CalendarStoragePort: Send + Sync + 'static { &self, event: CreateEventICalDto, ) -> Result; + /// Upsert every VEVENT in an iCalendar body — one master and zero + /// or more per-instance exception overrides (RFC 5545 §3.8.4.4). + /// + /// Routing: an event whose `RECURRENCE-ID` is unset targets the + /// master row `(calendar_id, ical_uid) WHERE recurrence_id IS NULL`; + /// an event whose `RECURRENCE-ID` is set targets its own exception + /// row `(calendar_id, ical_uid, recurrence_id)` and never touches + /// the master. Existing rows are replaced (delete-then-insert to + /// stay compatible with the DB-level partial unique indexes and to + /// keep the ETag surface identical to the pre-#528 single-event + /// path). + /// + /// See AtalayaLabs/OxiCloud#528. + async fn upsert_ical_events( + &self, + event: CreateEventICalDto, + ) -> Result; async fn update_event( &self, event_id: &str, @@ -136,6 +166,15 @@ pub trait CalendarUseCase: Send + Sync + 'static { event: CreateEventICalDto, user_id: Uuid, ) -> Result; + /// Route a PUT'd iCalendar body containing one or more VEVENTs to + /// their per-instance rows. See `CalendarStoragePort::upsert_ical_events` + /// for the routing rules; this method just adds the `Permission::Create` + /// gate for the caller. + async fn upsert_ical_events( + &self, + event: CreateEventICalDto, + user_id: Uuid, + ) -> Result; async fn update_event( &self, event_id: &str, diff --git a/src/application/services/calendar_service.rs b/src/application/services/calendar_service.rs index aa563b5c..c8c2b093 100644 --- a/src/application/services/calendar_service.rs +++ b/src/application/services/calendar_service.rs @@ -8,7 +8,9 @@ use crate::application::dtos::calendar_dto::{ UpdateCalendarDto, UpdateEventDto, }; use crate::application::ports::authorization_ports::AuthorizationEngine; -use crate::application::ports::calendar_ports::{CalendarStoragePort, CalendarUseCase}; +use crate::application::ports::calendar_ports::{ + CalendarStoragePort, CalendarUseCase, UpsertEventsResult, +}; use crate::common::errors::{DomainError, ErrorKind}; use crate::domain::services::authorization::{Permission, Resource, Role, Subject}; use crate::infrastructure::adapters::calendar_storage_adapter::CalendarStorageAdapter; @@ -234,6 +236,21 @@ impl CalendarUseCase for CalendarService { self.calendar_storage.create_event_from_ical(event).await } + async fn upsert_ical_events( + &self, + event: CreateEventICalDto, + user_id: Uuid, + ) -> Result { + // Same gate as create_event_from_ical — a PUT to the collection + // is a write. `Permission::Create` matches the single-event + // path; per-instance exception updates ride on the same + // permission because from the ACL's perspective it's still + // a write to the calendar. + self.require_calendar_perm(&event.calendar_id, user_id, Permission::Create) + .await?; + self.calendar_storage.upsert_ical_events(event).await + } + async fn update_event( &self, event_id: &str, diff --git a/src/domain/entities/calendar_event.rs b/src/domain/entities/calendar_event.rs index eb0725ed..d0bc4e60 100644 --- a/src/domain/entities/calendar_event.rs +++ b/src/domain/entities/calendar_event.rs @@ -795,6 +795,86 @@ impl CalendarEvent { Some((value.trim().to_string(), params)) } + /// Parse a VCALENDAR body containing one or more VEVENT components + /// (typically a master + one or more per-instance exception + /// overrides in the same PUT — RFC 5545 §3.6.1), returning one + /// `CalendarEvent` per VEVENT. + /// + /// Splitting is done on the raw text so each returned entity's + /// `ical_data` remains a valid standalone iCalendar body (the GET + /// path serves it verbatim). Line-folding (§3.1) is preserved + /// because we forward every line as-is inside the extracted block; + /// the ical-crate parser inside `from_ical` unfolds when reading. + /// + /// Nested VALARM / VTODO sub-components inside a VEVENT are + /// carried through unchanged — the scanner only splits on + /// `BEGIN:VEVENT` / `END:VEVENT` at the outer level. + /// + /// Returns `InvalidInput` if the body contains zero VEVENTs — a + /// PUT with no events isn't a state we accept on the CalDAV surface. + pub fn parse_all_events(calendar_id: Uuid, ical_data: &str) -> Result> { + let blocks = Self::split_vevents(ical_data); + + if blocks.is_empty() { + return Err(DomainError::new( + ErrorKind::InvalidInput, + "CalendarEvent", + "No VEVENT components found in iCalendar body", + )); + } + + let mut out = Vec::with_capacity(blocks.len()); + for block in blocks { + // Wrap each VEVENT in a fresh VCALENDAR shell so the + // stored `ical_data` per row is self-describing (RFC 5545 + // §3.4 mandates VERSION + PRODID on any exported body). + let wrapped = format!( + "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//OxiCloud//NONSGML Calendar//EN\r\n{}END:VCALENDAR\r\n", + block, + ); + out.push(Self::from_ical(calendar_id, wrapped)?); + } + Ok(out) + } + + /// Extract each `BEGIN:VEVENT` … `END:VEVENT` block from the raw + /// body as its own String (CRLF-terminated). Component tags are + /// matched case-insensitively per RFC 5545 §3.1. Anything outside + /// a VEVENT (VTIMEZONE / VTODO / VJOURNAL / calendar-level + /// properties) is discarded — those aren't ours to persist. + fn split_vevents(ical_data: &str) -> Vec { + let mut blocks = Vec::new(); + let mut in_event = false; + let mut current = String::new(); + + for raw_line in ical_data.split('\n') { + let line = raw_line.trim_end_matches('\r'); + // Match the tag ignoring case, allowing surrounding + // whitespace (some clients emit a leading space on folded + // continuations — the raw-line scan sees those but they + // won't start with BEGIN/END so they slot through as + // in-event content, which is correct). + let upper = line.trim_start().to_ascii_uppercase(); + + if upper.starts_with("BEGIN:VEVENT") { + in_event = true; + current.clear(); + } + + if in_event { + current.push_str(line); + current.push_str("\r\n"); + } + + if in_event && upper.starts_with("END:VEVENT") { + blocks.push(std::mem::take(&mut current)); + in_event = false; + } + } + + blocks + } + /// Parse the raw iCalendar body and return the first VEVENT /// component's properties. Returns `None` on any parse failure or /// if the body carries zero events (e.g. a `VCALENDAR` with only @@ -1210,7 +1290,7 @@ END:VCALENDAR\r 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")); + assert_eq!(ev.rrule(), Some("FREQ=DAILY;COUNT=10")); } #[test] @@ -1276,6 +1356,193 @@ END:VCALENDAR\r ); } + // ───────────────────────────────────────────────────────────── + // Phase 3 — parse_all_events (multi-VEVENT splitter) + // ───────────────────────────────────────────────────────────── + + /// Timed daily recurring master + one timed exception override, + /// both inside a single VCALENDAR wrapper — the shape a CalDAV + /// client PUTs when it modifies one occurrence. + const MASTER_PLUS_TIMED_EXCEPTION: &str = "\ +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:20260101T090000Z\r +DTEND:20260101T093000Z\r +SUMMARY:Daily standup\r +RRULE:FREQ=DAILY;COUNT=10\r +END:VEVENT\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 +"; + + #[test] + fn parse_all_events_splits_master_and_exception() { + // Both VEVENTs must come back: master with recurrence_id=None, + // exception with recurrence_id=Some. UIDs match (that's what + // ties the exception to its master); it's the recurrence_id + // marker that distinguishes them. + let cal_id = Uuid::new_v4(); + let events = CalendarEvent::parse_all_events(cal_id, MASTER_PLUS_TIMED_EXCEPTION) + .expect("both VEVENTs must parse"); + + assert_eq!(events.len(), 2, "expected master + exception"); + assert_eq!(events[0].ical_uid(), "daily-1@oxicloud.test"); + assert_eq!(events[1].ical_uid(), "daily-1@oxicloud.test"); + + assert!( + events[0].recurrence_id().is_none(), + "first row must be the master (recurrence_id None)" + ); + let rid = events[1] + .recurrence_id() + .expect("second row must be the exception override"); + assert_eq!(rid.to_rfc3339(), "2026-01-03T09:00:00+00:00"); + + assert_eq!(events[0].rrule(), Some("FREQ=DAILY;COUNT=10")); + assert!( + events[1].rrule().is_none(), + "exception overrides do NOT carry RRULE" + ); + + // Each event's stored ical_data must be a self-contained + // VCALENDAR body so the GET path can serve it verbatim. + for e in &events { + assert!(e.ical_data().starts_with("BEGIN:VCALENDAR")); + assert!(e.ical_data().trim_end().ends_with("END:VCALENDAR")); + } + } + + #[test] + fn parse_all_events_lone_master_returns_single_event() { + // No RECURRENCE-ID exception in the body → one row, master. + let cal_id = Uuid::new_v4(); + let events = + CalendarEvent::parse_all_events(cal_id, TIMED_EVENT).expect("plain event must parse"); + assert_eq!(events.len(), 1); + assert!(events[0].recurrence_id().is_none()); + } + + #[test] + fn parse_all_events_all_day_master_plus_all_day_exception() { + // The #528 shape: DATE-form DTSTART on both, DATE-form + // RECURRENCE-ID on the exception. Pre-parser-rewrite this + // silently 500'd because the param-carrying property lines + // were invisible to the substring scanner. + let body = "\ +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:20260105\r +DTEND;VALUE=DATE:20260106\r +SUMMARY:Weekly review\r +RRULE:FREQ=WEEKLY;COUNT=4\r +END:VEVENT\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 review — rescheduled\r +RECURRENCE-ID;VALUE=DATE:20260112\r +END:VEVENT\r +END:VCALENDAR\r +"; + let cal_id = Uuid::new_v4(); + let events = CalendarEvent::parse_all_events(cal_id, body).expect("both must parse"); + assert_eq!(events.len(), 2); + assert!(events[0].all_day()); + assert!(events[1].all_day()); + assert!(events[0].recurrence_id().is_none()); + let rid = events[1].recurrence_id().unwrap(); + assert_eq!(rid.to_rfc3339(), "2026-01-12T00:00:00+00:00"); + } + + #[test] + fn parse_all_events_zero_vevents_is_invalid_input() { + // A VCALENDAR with only calendar-level properties (no events) + // is not a state the CalDAV surface accepts on PUT. + let body = "\ +BEGIN:VCALENDAR\r +VERSION:2.0\r +PRODID:-//test//EN\r +END:VCALENDAR\r +"; + let err = + CalendarEvent::parse_all_events(Uuid::new_v4(), body).expect_err("must reject empty"); + assert_eq!(err.kind, ErrorKind::InvalidInput); + } + + #[test] + fn parse_all_events_vtodo_is_ignored() { + // A body carrying only VTODOs (no VEVENTs) is treated as + // "zero events" — we don't persist tasks in the events table. + let body = "\ +BEGIN:VCALENDAR\r +VERSION:2.0\r +PRODID:-//test//EN\r +BEGIN:VTODO\r +UID:task-1@x\r +SUMMARY:buy milk\r +END:VTODO\r +END:VCALENDAR\r +"; + let err = CalendarEvent::parse_all_events(Uuid::new_v4(), body) + .expect_err("VTODO-only body must be rejected"); + assert_eq!(err.kind, ErrorKind::InvalidInput); + } + + #[test] + fn parse_all_events_preserves_valarm_inside_vevent() { + // VALARM lives INSIDE a VEVENT. The splitter must NOT be + // fooled by BEGIN:VALARM into thinking a new outer component + // has started — the whole VALARM block must ride along inside + // the parent VEVENT's stored ical_data. + let body = "\ +BEGIN:VCALENDAR\r +VERSION:2.0\r +PRODID:-//test//EN\r +BEGIN:VEVENT\r +UID:with-alarm@x\r +DTSTAMP:20260101T100000Z\r +DTSTART:20260101T090000Z\r +DTEND:20260101T093000Z\r +SUMMARY:Standup with alarm\r +BEGIN:VALARM\r +ACTION:DISPLAY\r +TRIGGER:-PT15M\r +DESCRIPTION:Standup soon\r +END:VALARM\r +END:VEVENT\r +END:VCALENDAR\r +"; + let events = CalendarEvent::parse_all_events(Uuid::new_v4(), body) + .expect("VEVENT with VALARM must parse"); + assert_eq!(events.len(), 1); + let stored = events[0].ical_data(); + assert!( + stored.contains("BEGIN:VALARM"), + "VALARM must survive the split into stored ical_data" + ); + assert!( + stored.contains("END:VALARM"), + "matching END:VALARM must survive too" + ); + } + #[test] fn set_recurrence_id_setter_round_trips() { // Repository rehydration path: `with_id` initialises diff --git a/src/domain/repositories/calendar_event_repository.rs b/src/domain/repositories/calendar_event_repository.rs index d08cc561..90b105e9 100644 --- a/src/domain/repositories/calendar_event_repository.rs +++ b/src/domain/repositories/calendar_event_repository.rs @@ -46,13 +46,35 @@ pub trait CalendarEventRepository: Send + Sync + 'static { end: &DateTime, ) -> CalendarEventRepositoryResult>; - /// Finds an event by its iCalendar UID in a specific calendar + /// Finds an event by its iCalendar UID in a specific calendar. + /// + /// **Master-only lookup.** Filters `recurrence_id IS NULL` so the + /// return value is unambiguous — the row that clients treat as + /// "the event with this UID" is the master. Per-instance override + /// rows share the UID but live under + /// `find_event_by_ical_uid_and_recurrence_id` (see #528). async fn find_event_by_ical_uid( &self, calendar_id: &Uuid, ical_uid: &str, ) -> CalendarEventRepositoryResult>; + /// Finds a specific per-instance exception override for a recurring + /// master (RFC 5545 §3.8.4.4). `recurrence_id` pinpoints which + /// occurrence of the master with the given UID is being targeted; + /// returns `None` if no override has been PUT for that instance + /// yet — which the PUT handler then uses to decide insert vs. + /// update. + /// + /// The row is guaranteed unique by the partial index + /// `idx_calendar_events_exception_unique`. + async fn find_event_by_ical_uid_and_recurrence_id( + &self, + calendar_id: &Uuid, + ical_uid: &str, + recurrence_id: &DateTime, + ) -> CalendarEventRepositoryResult>; + /// Finds the events matching any of the given iCalendar UIDs in one /// indexed query (`ical_uid = ANY(...)`). Used by CalDAV multiget so a /// request for a handful of events never pays for the whole calendar. diff --git a/src/infrastructure/adapters/calendar_storage_adapter.rs b/src/infrastructure/adapters/calendar_storage_adapter.rs index c8a45941..42552539 100644 --- a/src/infrastructure/adapters/calendar_storage_adapter.rs +++ b/src/infrastructure/adapters/calendar_storage_adapter.rs @@ -13,7 +13,7 @@ use crate::application::dtos::calendar_dto::{ CalendarDto, CalendarEventDto, CreateCalendarDto, CreateEventDto, CreateEventICalDto, UpdateCalendarDto, UpdateEventDto, }; -use crate::application::ports::calendar_ports::CalendarStoragePort; +use crate::application::ports::calendar_ports::{CalendarStoragePort, UpsertEventsResult}; use crate::common::errors::{DomainError, ErrorKind}; use crate::domain::entities::calendar::Calendar; use crate::domain::entities::calendar_event::CalendarEvent; @@ -262,6 +262,73 @@ impl CalendarStoragePort for CalendarStorageAdapter { Ok(CalendarEventDto::from(created)) } + async fn upsert_ical_events( + &self, + dto: CreateEventICalDto, + ) -> Result { + let calendar_id = Uuid::parse_str(&dto.calendar_id).map_err(|_| { + DomainError::new( + ErrorKind::InvalidInput, + "Event", + "Invalid calendar ID format", + ) + })?; + + // Verify calendar exists before touching the events table. + let _calendar = self + .calendar_repository + .find_calendar_by_id(&calendar_id) + .await?; + + // Split the body into one CalendarEvent per VEVENT. A body + // with zero VEVENTs (or only VTODOs / VJOURNALs) returns + // InvalidInput here — which the handler layer maps to 400. + let parsed = CalendarEvent::parse_all_events(calendar_id, &dto.ical_data)?; + + let mut out = Vec::with_capacity(parsed.len()); + let mut any_inserted = false; + + for event in parsed { + let ical_uid = event.ical_uid().to_string(); + + // Existing row lookup routes on the master/exception split. + // Master: (calendar_id, ical_uid) WHERE recurrence_id IS NULL + // Exception: (calendar_id, ical_uid, recurrence_id) + let existing = match event.recurrence_id().copied() { + Some(rid) => { + self.event_repository + .find_event_by_ical_uid_and_recurrence_id(&calendar_id, &ical_uid, &rid) + .await? + } + None => { + self.event_repository + .find_event_by_ical_uid(&calendar_id, &ical_uid) + .await? + } + }; + + // Delete-then-insert keeps the DB-level partial unique + // indexes happy and matches the pre-#528 update semantics + // of the single-event path (fresh row id per replace, + // ETag changes on update). + if let Some(existing_event) = existing { + self.event_repository + .delete_event(existing_event.id()) + .await?; + } else { + any_inserted = true; + } + + let created = self.event_repository.create_event(event).await?; + out.push(CalendarEventDto::from(created)); + } + + Ok(UpsertEventsResult { + events: out, + any_inserted, + }) + } + async fn update_event( &self, event_id: &str, diff --git a/src/infrastructure/repositories/pg/calendar_event_pg_repository.rs b/src/infrastructure/repositories/pg/calendar_event_pg_repository.rs index 17cf365d..ed560275 100644 --- a/src/infrastructure/repositories/pg/calendar_event_pg_repository.rs +++ b/src/infrastructure/repositories/pg/calendar_event_pg_repository.rs @@ -389,6 +389,66 @@ impl CalendarEventRepository for CalendarEventPgRepository { } } + async fn find_event_by_ical_uid_and_recurrence_id( + &self, + calendar_id: &Uuid, + ical_uid: &str, + recurrence_id: &DateTime, + ) -> CalendarEventRepositoryResult> { + // Uses idx_calendar_events_exception_unique — the partial + // unique index on (calendar_id, ical_uid, recurrence_id) + // WHERE recurrence_id IS NOT NULL — for the exact-match seek. + 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, recurrence_id + FROM caldav.calendar_events + WHERE calendar_id = $1 + AND ical_uid = $2 + AND recurrence_id = $3 + "#, + ) + .bind(calendar_id) + .bind(ical_uid) + .bind(recurrence_id) + .fetch_optional(&*self.pool) + .await + .map_err(|e| { + DomainError::database_error(format!( + "Failed to get calendar event exception by UID+RECURRENCE-ID: {}", + e + )) + })?; + + match row_opt { + Some(row) => { + let mut event = CalendarEvent::with_id( + row.get("id"), + row.get("calendar_id"), + row.get("summary"), + row.get::, _>("description"), + row.get::, _>("location"), + row.get("start_time"), + row.get("end_time"), + row.get("all_day"), + row.get::, _>("rrule"), + row.get("ical_uid"), + row.get("ical_data"), + row.get("created_at"), + row.get("updated_at"), + ) + .map_err(|e| { + DomainError::database_error(format!("Error creating calendar event: {}", e)) + })?; + event.set_recurrence_id(row.get::>, _>("recurrence_id")); + Ok(Some(event)) + } + None => Ok(None), + } + } + async fn find_events_by_ical_uids( &self, calendar_id: &Uuid, diff --git a/src/interfaces/api/handlers/caldav_handler.rs b/src/interfaces/api/handlers/caldav_handler.rs index ce2cbe06..485eaf71 100644 --- a/src/interfaces/api/handlers/caldav_handler.rs +++ b/src/interfaces/api/handlers/caldav_handler.rs @@ -627,87 +627,51 @@ async fn handle_put( let ical_data = String::from_utf8(body_bytes.to_vec()) .map_err(|e| AppError::bad_request(format!("Invalid UTF-8 in iCalendar data: {}", e)))?; - let ical_uid = extract_uid_from_ical(&ical_data); - - // Indexed single-row lookup — listing the whole calendar (every row - // with its ical_data) to find one UID made imports O(N²). - let existing = if let Some(ref uid) = ical_uid { - calendar_service - .get_event_by_ical_uid(calendar_id, uid, user.id) - .await - .unwrap_or_default() - } else { - None + // Route the PUT through `upsert_ical_events` so a body carrying a + // master + N per-instance overrides (RFC 5545 §3.8.4.4 — the + // Thunderbird / Apple Calendar / DAVx⁵ "modify one occurrence" + // shape) persists each VEVENT to its own row instead of the last + // one clobbering the master. See AtalayaLabs/OxiCloud#528. + // + // Kind-aware error mapping (`AppError::from(DomainError)`): + // * `InvalidInput` → 400 (malformed iCal / missing DTSTART) + // * `NotFound` → 404 (calendar doesn't exist / no perm) + // * `AccessDenied` → 403 (caller lacks Write on the calendar) + // * anything else → 500 (genuine server bug) + let create_dto = CreateEventICalDto { + calendar_id: calendar_id.to_string(), + ical_data, }; - if let Some(existing_event) = existing { - // Update existing event — re-create from iCal for full fidelity. - // Both calls use `AppError::from` — the delete propagates - // NotFound/AccessDenied as 404/403, and the recreate propagates - // InvalidInput on malformed iCalendar as 400 (see comment on - // create_event_from_ical below). - calendar_service - .delete_event(&existing_event.id, user.id) - .await - .map_err(AppError::from)?; + let result = calendar_service + .upsert_ical_events(create_dto, user.id) + .await + .map_err(AppError::from)?; - let create_dto = CreateEventICalDto { - calendar_id: calendar_id.to_string(), - ical_data, - }; - let event = calendar_service - .create_event_from_ical(create_dto, user.id) - .await - .map_err(AppError::from)?; + // The event surface still exposes a single object resource per + // UID, so we return an ETag anchored on the master row when + // present, otherwise the first exception's id. This matches the + // pre-#528 header contract for clients that only understand a + // single ETag per PUT. + let etag_source = result + .events + .iter() + .find(|e| e.recurrence_id.is_none()) + .or_else(|| result.events.first()) + .map(|e| e.id.to_string()) + .unwrap_or_default(); - Ok(Response::builder() - .status(StatusCode::NO_CONTENT) - .header(header::ETAG, format!("\"{}\"", event.id)) - .body(Body::empty()) - .unwrap()) + let status = if result.any_inserted { + StatusCode::CREATED } else { - let create_dto = CreateEventICalDto { - calendar_id: calendar_id.to_string(), - ical_data, - }; + StatusCode::NO_CONTENT + }; - // `AppError::from(DomainError)` (via the `From` impl in - // `interfaces/errors.rs`) maps the ErrorKind onto the correct - // HTTP status: - // * `InvalidInput` → 400 (e.g. "Missing DTSTART in iCalendar - // data" from `CalendarEvent::from_ical`) — this is the fix - // for AtalayaLabs/OxiCloud#545 comment from `funboytwo`. - // * `NotFound` → 404 (parent calendar doesn't exist) - // * `AccessDenied` → 403 (caller lacks Write on the calendar) - // * `DatabaseError`/`InternalError` → 500 (genuine server bug) - // - // The old `map_err(|e| AppError::internal_error(...))` was - // blanket-wrapping every case as 500, hiding client-input bugs - // as opaque server errors. Downstream monitoring (500 rate, - // pager alerts) took the false hit; users saw an unhelpful - // "Internal Server Error" for their own bad iCalendar. - let event = calendar_service - .create_event_from_ical(create_dto, user.id) - .await - .map_err(AppError::from)?; - - Ok(Response::builder() - .status(StatusCode::CREATED) - .header(header::ETAG, format!("\"{}\"", event.id)) - .body(Body::empty()) - .unwrap()) - } -} - -/// Extract UID from iCalendar data -fn extract_uid_from_ical(ical_data: &str) -> Option { - for line in ical_data.lines() { - let trimmed = line.trim(); - if let Some(stripped) = trimmed.strip_prefix("UID:") { - return Some(stripped.trim().to_string()); - } - } - None + Ok(Response::builder() + .status(status) + .header(header::ETAG, format!("\"{}\"", etag_source)) + .body(Body::empty()) + .unwrap()) } // ─── GET (.ics) ────────────────────────────────────────────────────── diff --git a/tests/api/caldav_recurring.hurl b/tests/api/caldav_recurring.hurl new file mode 100644 index 00000000..9caa1bf2 --- /dev/null +++ b/tests/api/caldav_recurring.hurl @@ -0,0 +1,280 @@ +# ============================================================= +# OxiCloud – CalDAV recurring events with RECURRENCE-ID overrides +# ============================================================= +# End-to-end regression for AtalayaLabs/OxiCloud#528. +# +# Pre-fix behaviour (all-day recurring event, one occurrence +# modified in Thunderbird/Apple Calendar/DAVx⁵/Gnome Calendar): +# * The client PUTs a VCALENDAR containing the master (with +# RRULE) + a per-instance override (RFC 5545 §3.8.4.4, +# `RECURRENCE-ID`). Pre-fix the substring-based parser +# could not read any property carrying parameters +# (`DTSTART;VALUE=DATE:...`, `RECURRENCE-ID;VALUE=DATE:...`), +# so all-day master modifications 500'd outright. +# * Even for timed events, the old create_event_from_ical +# read only the first VEVENT — a second PUT of just the +# exception would overwrite the master row entirely, +# silently corrupting the client's view of the series. +# +# Post-fix (this file's invariant): +# 1. Master PUT → 201 CREATED, one row (recurrence_id NULL). +# 2. PUT master + exception in one body → both persist to +# their own row keyed by (calendar_id, ical_uid, +# recurrence_id). Response is 201 CREATED because the +# exception was newly inserted. +# 3. PUT ONLY the exception with modified content → 204 +# No Content (in-place replace, no new rows). CRITICALLY, +# the MASTER row survives untouched — a GET on the .ics +# URL still returns the master's original RRULE + summary. +# 4. All-day master + all-day exception (the exact #528 shape) +# completes the same round-trip. +# +# Storage invariant enforced by two partial unique indexes on +# caldav.calendar_events (see migration 20260913000001): +# * idx_calendar_events_master_unique — at most one master +# per (calendar_id, ical_uid). +# * idx_calendar_events_exception_unique — at most one +# exception override per (calendar_id, ical_uid, +# recurrence_id). +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 – Admin logs in. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ + "username": "{{username}}", + "password": "{{password}}" +} + +HTTP 200 +[Captures] +admin_token: jsonpath "$.access_token" + + +# ───────────────────────────────────────────────────────────── +# Step 2 – MKCALENDAR: fresh calendar for #528 regression. +# ───────────────────────────────────────────────────────────── +MKCALENDAR {{base_url}}/caldav/recurring-528/ +Authorization: Bearer {{admin_token}} + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step 3 – PROPFIND to capture the server-assigned UUID for +# recurring-528. The (?s).* anchor greedy-matches to the LAST +# /caldav// in the body, which is our just-created +# calendar (default-provisioned calendars come first by +# created_at, this one is newest). +# ───────────────────────────────────────────────────────────── +PROPFIND {{base_url}}/caldav/ +Authorization: Bearer {{admin_token}} +Depth: 1 +Content-Type: application/xml +``` + + + + + + + +``` + +HTTP 207 +[Captures] +calendar_id: body regex "(?s).*/caldav/([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})/" +[Asserts] +body contains "recurring-528" + + +# ───────────────────────────────────────────────────────────── +# Step 4 – PUT the recurring master (timed, daily, 10 count). +# Expect 201 CREATED (fresh row) and a non-empty ETag. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics +Authorization: Bearer {{admin_token}} +Content-Type: text/calendar; charset=utf-8 +``` +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//OxiCloud e2e//EN +BEGIN:VEVENT +UID:daily-e2e-528 +DTSTAMP:20260101T100000Z +DTSTART:20260101T090000Z +DTEND:20260101T093000Z +SUMMARY:Daily standup +RRULE:FREQ=DAILY;COUNT=10 +END:VEVENT +END:VCALENDAR +``` + +HTTP 201 +[Asserts] +header "ETag" exists + + +# ───────────────────────────────────────────────────────────── +# Step 5 – GET the master. Body contains RRULE + original +# SUMMARY, confirming the master is stored and serves as-is. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics +Authorization: Bearer {{admin_token}} + +HTTP 200 +[Asserts] +body contains "FREQ=DAILY;COUNT=10" +body contains "SUMMARY:Daily standup" + + +# ───────────────────────────────────────────────────────────── +# Step 6 – The #528 heart: PUT master + per-instance override +# in a single body. This is what Thunderbird sends when the +# user modifies one occurrence of a recurring event. +# +# Expected: +# * 201 CREATED because the exception is newly inserted. +# (The master is replaced-in-place — any_inserted=true +# is decided by the NEW exception row, not the master.) +# * Both rows now exist in the DB. Verified in Step 7 via +# the master's GET still returning the master data. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics +Authorization: Bearer {{admin_token}} +Content-Type: text/calendar; charset=utf-8 +``` +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//OxiCloud e2e//EN +BEGIN:VEVENT +UID:daily-e2e-528 +DTSTAMP:20260101T100000Z +DTSTART:20260101T090000Z +DTEND:20260101T093000Z +SUMMARY:Daily standup +RRULE:FREQ=DAILY;COUNT=10 +END:VEVENT +BEGIN:VEVENT +UID:daily-e2e-528 +DTSTAMP:20260101T100000Z +DTSTART:20260103T110000Z +DTEND:20260103T120000Z +SUMMARY:Daily standup — rescheduled +RECURRENCE-ID:20260103T090000Z +END:VEVENT +END:VCALENDAR +``` + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step 7 – GET the master. It must STILL be the master (with +# RRULE + original SUMMARY). Pre-fix the exception would have +# clobbered this row and Step 7 would see the exception's +# SUMMARY ("… rescheduled") without the RRULE. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics +Authorization: Bearer {{admin_token}} + +HTTP 200 +[Asserts] +body contains "FREQ=DAILY;COUNT=10" +body contains "SUMMARY:Daily standup" +body not contains "SUMMARY:Daily standup — rescheduled" + + +# ───────────────────────────────────────────────────────────── +# Step 8 – PUT only the exception with a modified SUMMARY. +# Because the exception row already exists (from Step 6), +# no new row is inserted → 204 No Content. The MASTER is +# untouched (verified in Step 9). +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics +Authorization: Bearer {{admin_token}} +Content-Type: text/calendar; charset=utf-8 +``` +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//OxiCloud e2e//EN +BEGIN:VEVENT +UID:daily-e2e-528 +DTSTAMP:20260101T110000Z +DTSTART:20260103T120000Z +DTEND:20260103T130000Z +SUMMARY:Daily standup — rescheduled AGAIN +RECURRENCE-ID:20260103T090000Z +END:VEVENT +END:VCALENDAR +``` + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 9 – Master survives the exception update. Pre-fix this +# would fail: the old delete-by-UID-then-insert path would +# have removed the master when the exception-only PUT landed. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics +Authorization: Bearer {{admin_token}} + +HTTP 200 +[Asserts] +body contains "FREQ=DAILY;COUNT=10" +body contains "SUMMARY:Daily standup" +body not contains "SUMMARY:Daily standup — rescheduled" + + +# ───────────────────────────────────────────────────────────── +# Step 10 – The all-day flavour: master with DTSTART;VALUE=DATE +# + exception with RECURRENCE-ID;VALUE=DATE. Pre-parser-rewrite +# this 500'd because the param-carrying property lines were +# invisible to the substring scanner (root cause of #528). +# +# Uses a distinct UID so it doesn't collide with Step 4-8 rows +# under the master partial unique index. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/caldav/{{calendar_id}}/weekly-allday-528.ics +Authorization: Bearer {{admin_token}} +Content-Type: text/calendar; charset=utf-8 +``` +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//OxiCloud e2e//EN +BEGIN:VEVENT +UID:weekly-allday-528 +DTSTAMP:20260101T100000Z +DTSTART;VALUE=DATE:20260105 +DTEND;VALUE=DATE:20260106 +SUMMARY:Weekly review +RRULE:FREQ=WEEKLY;COUNT=4 +END:VEVENT +BEGIN:VEVENT +UID:weekly-allday-528 +DTSTAMP:20260101T100000Z +DTSTART;VALUE=DATE:20260113 +DTEND;VALUE=DATE:20260114 +SUMMARY:Weekly review — moved +RECURRENCE-ID;VALUE=DATE:20260112 +END:VEVENT +END:VCALENDAR +``` + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step 11 – Cleanup: delete the entire calendar (cascades to +# all events + exception rows in a single storage call). Keeps +# the shared Hurl DB uncluttered for downstream test files +# (per feedback_hurl_teardown_shared_db). +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/caldav/{{calendar_id}}/ +Authorization: Bearer {{admin_token}} + +HTTP 204 diff --git a/tests/api/run.sh b/tests/api/run.sh index a30d7e90..3fc6ff98 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -168,6 +168,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/dav_error_mapping.hurl" \ "$API_DIR/contacts.hurl" \ "$API_DIR/calendar.hurl" \ + "$API_DIR/caldav_recurring.hurl" \ "$API_DIR/playlists.hurl" \ "$API_DIR/public_shares.hurl" \ "$API_DIR/permissions.hurl" \