diff --git a/src/application/adapters/caldav_adapter.rs b/src/application/adapters/caldav_adapter.rs index 7b500695..ab93fe12 100644 --- a/src/application/adapters/caldav_adapter.rs +++ b/src/application/adapters/caldav_adapter.rs @@ -47,6 +47,106 @@ fn parse_caldav_datetime(value: &str) -> Option> { }) } +/// Extract the `BEGIN:VEVENT` ... `END:VEVENT` slice from a +/// stored `ical_data` body (as returned by the storage layer — +/// one full VCALENDAR per row). +/// +/// Case-insensitive on the tag names per RFC 5545 §3.1. Includes +/// the `BEGIN:VEVENT` and `END:VEVENT` lines themselves. Returns +/// `None` if either tag is missing (malformed body) so callers +/// can fall back safely. +pub(crate) fn extract_vevent_chunk(ical_data: &str) -> Option<&str> { + let upper = ical_data.to_ascii_uppercase(); + let begin = upper.find("BEGIN:VEVENT")?; + // End marker: the line-start of END:VEVENT after `begin`, plus + // the length of "END:VEVENT" itself, then find the next CRLF/LF + // to include the terminator line. + let after_begin = &upper[begin..]; + let rel_end = after_begin.find("END:VEVENT")?; + let end_tag_end = begin + rel_end + "END:VEVENT".len(); + // Include any immediate line terminator so the chunk stays a + // well-formed line even when the caller concatenates. + let mut end = end_tag_end; + if ical_data[end..].starts_with('\r') { + end += 1; + } + if ical_data[end..].starts_with('\n') { + end += 1; + } + Some(&ical_data[begin..end]) +} + +/// Group a slice of events by `ical_uid`, preserving the order of +/// first appearance for the groups themselves, and placing the +/// master (`recurrence_id.is_none()`) first within each group per +/// RFC 5545 §3.6.1 convention. Ties among exceptions preserve the +/// original slice order. +/// +/// Used by the read-side emitters to fold master + per-instance +/// override rows into a single calendar-object-resource, matching +/// the "one URL per UID" contract of RFC 4791 §4.1. +pub(crate) fn group_events_by_uid<'a>( + events: &'a [CalendarEventDto], +) -> Vec> { + let mut order: Vec = Vec::new(); + let mut buckets: std::collections::HashMap> = + std::collections::HashMap::new(); + + for event in events { + let key = event.ical_uid.clone(); + if !buckets.contains_key(&key) { + order.push(key.clone()); + } + buckets.entry(key).or_default().push(event); + } + + let mut out = Vec::with_capacity(order.len()); + for uid in order { + let mut bucket = buckets.remove(&uid).unwrap_or_default(); + // Master first (recurrence_id None), exceptions in insertion order. + bucket.sort_by_key(|e| e.recurrence_id.is_some()); + out.push(bucket); + } + out +} + +/// Build the calendar-object-resource body for a bundle (master + +/// N exception overrides sharing the same UID). Serves each row's +/// stored `ical_data` verbatim, extracting the VEVENT chunk and +/// wrapping the concatenation in a single VCALENDAR shell. +/// +/// This is the fix for the phase-4 read-side gap: the pre-fix +/// emitter regenerated the body from DTO fields, which (a) lost +/// every property outside UID / SUMMARY / DTSTART / DTEND / +/// DESCRIPTION / LOCATION / RRULE (so ATTENDEE, VALARM, CATEGORIES, +/// STATUS, X-* all silently dropped) and (b) never emitted +/// RECURRENCE-ID so exception rows were invisible in the bundled +/// GET body. Serving stored bytes verbatim closes both. +/// +/// If any row's `ical_data` is malformed (no VEVENT tag pair), +/// that row is skipped — the bundle survives the rest. An empty +/// input bundle yields a minimal VCALENDAR with no VEVENTs (the +/// caller decides whether to treat that as 404 upstream). +pub(crate) fn bundle_to_calendar_body(bundle: &[&CalendarEventDto]) -> String { + let mut buf = String::with_capacity(256 + bundle.len() * 320); + buf.push_str("BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//OxiCloud//NONSGML Calendar//EN\r\n"); + for event in bundle { + if let Some(chunk) = extract_vevent_chunk(&event.ical_data) { + // The chunk already carries its own trailing line + // terminator (see extract_vevent_chunk). Append as-is. + buf.push_str(chunk); + // Defensive: guarantee a line separator between VEVENTs + // even if the extracted chunk didn't include a trailing + // newline (some stored bodies lack the terminator). + if !buf.ends_with('\n') { + buf.push_str("\r\n"); + } + } + } + buf.push_str("END:VCALENDAR\r\n"); + buf +} + /// Returns whether `caller_id` owns `calendar`. /// /// CalDAV clients (DAVx5, Apple Calendar, Thunderbird) only mount a collection @@ -940,13 +1040,27 @@ impl CalDavAdapter { // Write the calendar collection itself Self::write_calendar_response(&mut xml_writer, calendar, request, base_href, caller_id)?; - // If depth > 0, include event resources + // If depth > 0, include event resources — folded per UID + // so a recurring event's master + per-instance exception + // overrides share ONE D:response (RFC 4791 §4.1 + RFC + // 5545 §3.6.1). Pre-fix this loop emitted one D:response + // per DB row, and since master + exception share the + // same href (base + uid.ics) clients saw a duplicate + // href and deduped — the exception appeared to have + // vanished. if depth != "0" { - for event in events { - // Write a basic DAV response for each event - xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; + for bundle in group_events_by_uid(events) { + // The master (sorted first by group_events_by_uid) + // supplies the ETag anchor + getlastmodified. If + // the bundle is all exceptions (no master row), + // fall back to the first exception. + let anchor = match bundle.first() { + Some(e) => *e, + None => continue, + }; + let event_href = format!("{}{}.ics", base_href, anchor.ical_uid); - let event_href = format!("{}{}.ics", base_href, event.ical_uid); + xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; xml_writer.write_event(Event::Start(BytesStart::new("D:href")))?; xml_writer.write_event(Event::Text(BytesText::new(&event_href)))?; xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; @@ -957,10 +1071,10 @@ impl CalDavAdapter { // resourcetype (empty for non-collection) xml_writer.write_event(Event::Empty(BytesStart::new("D:resourcetype")))?; - // getetag + // getetag — anchor row's id xml_writer.write_event(Event::Start(BytesStart::new("D:getetag")))?; xml_writer - .write_event(Event::Text(BytesText::new(&format!("\"{}\"", event.id))))?; + .write_event(Event::Text(BytesText::new(&format!("\"{}\"", anchor.id))))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; // getcontenttype @@ -970,10 +1084,10 @@ impl CalDavAdapter { )))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getcontenttype")))?; - // getlastmodified + // getlastmodified — anchor row's updated_at xml_writer.write_event(Event::Start(BytesStart::new("D:getlastmodified")))?; xml_writer - .write_event(Event::Text(BytesText::new(&event.updated_at.to_rfc2822())))?; + .write_event(Event::Text(BytesText::new(&anchor.updated_at.to_rfc2822())))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getlastmodified")))?; xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; @@ -1016,13 +1130,20 @@ impl CalDavAdapter { CalDavReportType::SyncCollection { props, .. } => props.clone(), }; - // Add responses for events - for event in events { - // Create the event href based on its UID - let href = format!("{}{}.ics", base_href, event.ical_uid); - - // Write event response - Self::write_event_response(&mut xml_writer, event, &props, &href)?; + // Add responses for events — folded per UID so a + // recurring master + per-instance exception overrides + // share ONE D:response with all VEVENTs concatenated + // into the calendar-data payload (RFC 4791 §4.1). Pre- + // fix this loop emitted one D:response per DB row, so + // master + exception carried duplicate hrefs and clients + // deduped, hiding the exception from the resulting sync. + for bundle in group_events_by_uid(events) { + let anchor = match bundle.first() { + Some(e) => *e, + None => continue, + }; + let href = format!("{}{}.ics", base_href, anchor.ical_uid); + Self::write_event_response(&mut xml_writer, &bundle, &props, &href)?; } // End multistatus @@ -1031,13 +1152,22 @@ impl CalDavAdapter { Ok(()) } - /// Write event properties as a response + /// Write a bundle (master + exception overrides sharing a + /// UID) as one D:response. The bundle is emitted at one + /// href (base + uid.ics); ETag + getlastmodified anchor on + /// the first bundle entry (which `group_events_by_uid` puts + /// the master at); calendar-data contains every VEVENT. fn write_event_response( xml_writer: &mut Writer, - event: &CalendarEventDto, + bundle: &[&CalendarEventDto], props: &[QualifiedName], href: &str, ) -> Result<()> { + let anchor = bundle + .first() + .copied() + .expect("write_event_response: bundle must be non-empty (caller guards)"); + // Start response element xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; @@ -1054,10 +1184,10 @@ impl CalDavAdapter { // If no specific props requested, return all common ones if props.is_empty() { - Self::write_event_standard_props(xml_writer, event)?; + Self::write_event_standard_props(xml_writer, anchor, bundle)?; } else { // Write specifically requested properties - Self::write_event_requested_props(xml_writer, event, props)?; + Self::write_event_requested_props(xml_writer, anchor, bundle, props)?; } // End prop @@ -1077,19 +1207,24 @@ impl CalDavAdapter { Ok(()) } - /// Write standard event properties + /// Write standard event properties for a UID bundle. + /// `anchor` supplies metadata (ETag, updated_at); `bundle` + /// supplies the full calendar-data payload (master + all + /// exceptions concatenated into one VCALENDAR). fn write_event_standard_props( xml_writer: &mut Writer, - event: &CalendarEventDto, + anchor: &CalendarEventDto, + bundle: &[&CalendarEventDto], ) -> Result<()> { // Common WebDAV properties // Resource type (empty for non-collection) xml_writer.write_event(Event::Empty(BytesStart::new("D:resourcetype")))?; - // ETag based on updated_at timestamp + // ETag anchored on the master (or first exception in + // a master-less bundle — pathological state today). xml_writer.write_event(Event::Start(BytesStart::new("D:getetag")))?; - xml_writer.write_event(Event::Text(BytesText::new(&format!("\"{}\"", event.id))))?; + xml_writer.write_event(Event::Text(BytesText::new(&format!("\"{}\"", anchor.id))))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; // Content type @@ -1101,38 +1236,17 @@ impl CalDavAdapter { // Last modified xml_writer.write_event(Event::Start(BytesStart::new("D:getlastmodified")))?; - xml_writer.write_event(Event::Text(BytesText::new(&event.updated_at.to_rfc2822())))?; + xml_writer.write_event(Event::Text(BytesText::new(&anchor.updated_at.to_rfc2822())))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getlastmodified")))?; - // CalDAV specific properties - - // Calendar data (iCalendar format) + // CalDAV calendar-data — the whole bundle emitted as one + // VCALENDAR by extracting each row's stored VEVENT chunk + // verbatim. Every property (ATTENDEE / VALARM / CATEGORIES + // / STATUS / X-* / RECURRENCE-ID on exception rows) + // survives because we no longer regenerate from DTO + // fields. xml_writer.write_event(Event::Start(BytesStart::new("C:calendar-data")))?; - // In a full implementation, we would generate a complete iCalendar component here - // For now, we'll just provide a basic example - let ical_data = format!( - "BEGIN:VCALENDAR\r\n\ - VERSION:2.0\r\n\ - PRODID:-//OxiCloud//NONSGML Calendar//EN\r\n\ - BEGIN:VEVENT\r\n\ - UID:{}\r\n\ - SUMMARY:{}\r\n\ - DTSTART:{}\r\n\ - DTEND:{}\r\n\ - {}\ - DTSTAMP:{}\r\n\ - END:VEVENT\r\n\ - END:VCALENDAR\r\n", - event.ical_uid, - event.summary.replace("\n", "\\n"), - event.start_time.format("%Y%m%dT%H%M%SZ"), - event.end_time.format("%Y%m%dT%H%M%SZ"), - event - .rrule - .as_ref() - .map_or("".to_string(), |r| format!("RRULE:{}\r\n", r)), - event.updated_at.format("%Y%m%dT%H%M%SZ"), - ); + let ical_data = bundle_to_calendar_body(bundle); xml_writer.write_event(Event::Text(BytesText::new(&ical_data)))?; xml_writer.write_event(Event::End(BytesEnd::new("C:calendar-data")))?; @@ -1142,7 +1256,8 @@ impl CalDavAdapter { /// Write requested event properties fn write_event_requested_props( xml_writer: &mut Writer, - event: &CalendarEventDto, + anchor: &CalendarEventDto, + bundle: &[&CalendarEventDto], props: &[QualifiedName], ) -> Result<()> { for prop in props { @@ -1154,7 +1269,7 @@ impl CalDavAdapter { ("DAV:", "getetag") => { xml_writer.write_event(Event::Start(BytesStart::new("D:getetag")))?; xml_writer - .write_event(Event::Text(BytesText::new(&format!("\"{}\"", event.id))))?; + .write_event(Event::Text(BytesText::new(&format!("\"{}\"", anchor.id))))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; } ("DAV:", "getcontenttype") => { @@ -1166,39 +1281,18 @@ impl CalDavAdapter { } ("DAV:", "getlastmodified") => { xml_writer.write_event(Event::Start(BytesStart::new("D:getlastmodified")))?; - xml_writer - .write_event(Event::Text(BytesText::new(&event.updated_at.to_rfc2822())))?; + xml_writer.write_event(Event::Text(BytesText::new( + &anchor.updated_at.to_rfc2822(), + )))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getlastmodified")))?; } - // CalDAV namespace properties + // CalDAV namespace properties — calendar-data is + // the whole bundle, master + exceptions in one + // VCALENDAR served from stored ical_data. ("urn:ietf:params:xml:ns:caldav", "calendar-data") => { xml_writer.write_event(Event::Start(BytesStart::new("C:calendar-data")))?; - // In a full implementation, we would generate a complete iCalendar component here - // For now, we'll just provide a basic example - let ical_data = format!( - "BEGIN:VCALENDAR\r\n\ - VERSION:2.0\r\n\ - PRODID:-//OxiCloud//NONSGML Calendar//EN\r\n\ - BEGIN:VEVENT\r\n\ - UID:{}\r\n\ - SUMMARY:{}\r\n\ - DTSTART:{}\r\n\ - DTEND:{}\r\n\ - {}\ - DTSTAMP:{}\r\n\ - END:VEVENT\r\n\ - END:VCALENDAR\r\n", - event.ical_uid, - event.summary.replace("\n", "\\n"), - event.start_time.format("%Y%m%dT%H%M%SZ"), - event.end_time.format("%Y%m%dT%H%M%SZ"), - event - .rrule - .as_ref() - .map_or("".to_string(), |r| format!("RRULE:{}\r\n", r)), - event.updated_at.format("%Y%m%dT%H%M%SZ"), - ); + let ical_data = bundle_to_calendar_body(bundle); xml_writer.write_event(Event::Text(BytesText::new(&ical_data)))?; xml_writer.write_event(Event::End(BytesEnd::new("C:calendar-data")))?; } @@ -1328,6 +1422,201 @@ impl CalDavAdapter { // Tests // ───────────────────────────────────────────────────────────── +#[cfg(test)] +mod bundle_helper_tests { + use super::*; + + /// One DTO builder for all tests in this module — carries + /// enough state (uid, recurrence_id, ical_data) for both the + /// grouping tests and the bundle-body tests. + fn dto(uid: &str, is_exception: bool, ical: &str) -> CalendarEventDto { + use chrono::Utc; + CalendarEventDto { + id: "row-".to_string() + uid, + calendar_id: "cal".to_string(), + summary: "s".to_string(), + description: None, + location: None, + start_time: Utc::now(), + end_time: Utc::now(), + all_day: false, + rrule: None, + ical_uid: uid.to_string(), + recurrence_id: if is_exception { Some(Utc::now()) } else { None }, + ical_data: ical.to_string(), + created_at: Utc::now(), + updated_at: Utc::now(), + } + } + + // ── extract_vevent_chunk ────────────────────────────────── + + #[test] + fn extract_vevent_finds_the_block_inside_vcalendar() { + let body = "\ +BEGIN:VCALENDAR\r +VERSION:2.0\r +BEGIN:VEVENT\r +UID:x\r +DTSTART:20260101T090000Z\r +END:VEVENT\r +END:VCALENDAR\r +"; + let chunk = extract_vevent_chunk(body).expect("VEVENT present"); + assert!(chunk.starts_with("BEGIN:VEVENT")); + assert!(chunk.contains("UID:x")); + assert!(chunk.trim_end().ends_with("END:VEVENT")); + } + + #[test] + fn extract_vevent_case_insensitive_tags() { + // RFC 5545 §3.1: component names are case-insensitive on + // read. Real client output is nearly always uppercase but + // a lowercase or mixed-case tag mustn't confuse the + // splitter. + let body = "begin:vcalendar\nbegin:vevent\nuid:x\nend:vevent\nend:vcalendar\n"; + let chunk = extract_vevent_chunk(body).expect("case-insensitive lookup"); + assert!(chunk.to_ascii_lowercase().contains("uid:x")); + } + + #[test] + fn extract_vevent_missing_returns_none() { + // A body with only VTIMEZONE (no VEVENT) → None. Caller + // uses this to skip malformed rows without crashing the + // bundle emitter. + let body = "BEGIN:VCALENDAR\r\nBEGIN:VTIMEZONE\r\nEND:VTIMEZONE\r\nEND:VCALENDAR\r\n"; + assert!(extract_vevent_chunk(body).is_none()); + } + + #[test] + fn extract_vevent_includes_trailing_line_terminator() { + // The chunk should end with CRLF so bundle concatenation + // produces valid line-separated iCalendar body. + let body = "BEGIN:VEVENT\r\nUID:x\r\nEND:VEVENT\r\n"; + let chunk = extract_vevent_chunk(body).unwrap(); + assert!( + chunk.ends_with("\r\n"), + "chunk must retain trailing CRLF for safe concatenation, got {:?}", + chunk + ); + } + + // ── group_events_by_uid ─────────────────────────────────── + + #[test] + fn group_places_master_first_within_each_uid() { + // Mixed order: exception first, then master, then a + // second exception. Result: [master, exception1, exception2]. + let ex1 = dto("u1", true, ""); + let master = dto("u1", false, ""); + let ex2 = dto("u1", true, ""); + let events = vec![ex1, master, ex2]; + + let grouped = group_events_by_uid(&events); + assert_eq!(grouped.len(), 1); + assert_eq!(grouped[0].len(), 3); + assert!( + grouped[0][0].recurrence_id.is_none(), + "master (recurrence_id None) must be first per RFC 5545 §3.6.1 convention" + ); + assert!(grouped[0][1].recurrence_id.is_some()); + assert!(grouped[0][2].recurrence_id.is_some()); + } + + #[test] + fn group_preserves_uid_order_of_first_appearance() { + // If the input has UIDs in order [A, B, A], the output's + // group order is [A, B] — first-appearance wins. + let a1 = dto("A", false, ""); + let b = dto("B", false, ""); + let a2 = dto("A", true, ""); + let events = vec![a1, b, a2]; + + let grouped = group_events_by_uid(&events); + assert_eq!(grouped.len(), 2); + assert_eq!(grouped[0][0].ical_uid, "A"); + assert_eq!(grouped[0].len(), 2); + assert_eq!(grouped[1][0].ical_uid, "B"); + assert_eq!(grouped[1].len(), 1); + } + + #[test] + fn group_empty_input_yields_empty_output() { + let events: Vec = vec![]; + assert!(group_events_by_uid(&events).is_empty()); + } + + // ── bundle_to_calendar_body ─────────────────────────────── + + #[test] + fn bundle_body_wraps_all_vevents_in_one_vcalendar() { + let master = dto( + "u", + false, + "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nBEGIN:VEVENT\r\nUID:u\r\nSUMMARY:Master\r\nRRULE:FREQ=DAILY;COUNT=3\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n", + ); + let exception = dto( + "u", + true, + "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nBEGIN:VEVENT\r\nUID:u\r\nSUMMARY:Override\r\nRECURRENCE-ID:20260103T090000Z\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n", + ); + let bundle: Vec<&CalendarEventDto> = vec![&master, &exception]; + + let body = bundle_to_calendar_body(&bundle); + assert!(body.starts_with("BEGIN:VCALENDAR")); + assert!(body.trim_end().ends_with("END:VCALENDAR")); + assert_eq!( + body.matches("BEGIN:VEVENT").count(), + 2, + "bundle must produce one VEVENT per bundle member" + ); + assert!(body.contains("SUMMARY:Master")); + assert!(body.contains("SUMMARY:Override")); + assert!( + body.contains("RECURRENCE-ID:20260103T090000Z"), + "exception RECURRENCE-ID must survive verbatim from stored ical_data" + ); + assert!( + body.contains("RRULE:FREQ=DAILY;COUNT=3"), + "master RRULE must survive verbatim from stored ical_data" + ); + } + + #[test] + fn bundle_body_skips_rows_with_malformed_ical_data() { + // Real world defense: a row whose stored ical_data is + // corrupt (no VEVENT tag) shouldn't kill the bundle. + // Emit the good rows; skip the bad one. + let good = dto( + "u", + false, + "BEGIN:VCALENDAR\r\nBEGIN:VEVENT\r\nUID:u\r\nSUMMARY:OK\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n", + ); + let bad = dto("u", true, "not-an-ical-body"); + let bundle: Vec<&CalendarEventDto> = vec![&good, &bad]; + + let body = bundle_to_calendar_body(&bundle); + assert_eq!(body.matches("BEGIN:VEVENT").count(), 1); + assert!(body.contains("SUMMARY:OK")); + } + + #[test] + fn bundle_body_of_single_row_still_wraps_in_vcalendar() { + // A non-recurring event is a bundle of one — output shape + // must remain a valid VCALENDAR body. + let single = dto( + "u", + false, + "BEGIN:VCALENDAR\r\nBEGIN:VEVENT\r\nUID:u\r\nSUMMARY:Lone\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n", + ); + let bundle: Vec<&CalendarEventDto> = vec![&single]; + let body = bundle_to_calendar_body(&bundle); + assert!(body.starts_with("BEGIN:VCALENDAR")); + assert!(body.contains("SUMMARY:Lone")); + assert_eq!(body.matches("BEGIN:VEVENT").count(), 1); + } +} + #[cfg(test)] mod time_range_parser_tests { use super::*; diff --git a/src/application/adapters/caldav_adapter_test.rs b/src/application/adapters/caldav_adapter_test.rs index 4f4284bc..3c3d4692 100644 --- a/src/application/adapters/caldav_adapter_test.rs +++ b/src/application/adapters/caldav_adapter_test.rs @@ -36,6 +36,25 @@ mod tests { rrule: None, ical_uid: "uid-evt-001@oxicloud".to_string(), recurrence_id: None, + // Post-phase-4 the emitter serves stored ical_data + // verbatim (folded per UID) instead of regenerating + // from DTO fields. The fixture must therefore carry + // a valid single-VEVENT VCALENDAR body — this is what + // create_event_from_ical stores per row. + ical_data: "BEGIN:VCALENDAR\r\n\ + VERSION:2.0\r\n\ + PRODID:-//OxiCloud test//EN\r\n\ + BEGIN:VEVENT\r\n\ + UID:uid-evt-001@oxicloud\r\n\ + DTSTAMP:20250601T090000Z\r\n\ + DTSTART:20250615T100000Z\r\n\ + DTEND:20250615T110000Z\r\n\ + SUMMARY:Team Meeting\r\n\ + DESCRIPTION:Weekly team sync\r\n\ + LOCATION:Conference Room A\r\n\ + END:VEVENT\r\n\ + END:VCALENDAR\r\n" + .to_string(), 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 de2e965d..261ae6e7 100644 --- a/src/application/dtos/calendar_dto.rs +++ b/src/application/dtos/calendar_dto.rs @@ -95,6 +95,16 @@ pub struct CalendarEventDto { /// distinguished by this field represent a recurring master and /// its modified occurrence(s) respectively (see #528). pub recurrence_id: Option>, + /// Full stored iCalendar body for this row — one VCALENDAR + /// containing exactly one VEVENT. Populated at every read + /// path from the entity's `ical_data()`. The CalDAV read + /// emitters serve this verbatim (extracted + bundled per + /// UID) instead of regenerating from the other DTO fields, + /// so properties beyond the structured columns + /// (ATTENDEE, VALARM, CATEGORIES, RECURRENCE-ID, X-*) + /// survive PUT → GET round-trips. See phase-4 read-side + /// unification. + pub ical_data: String, pub created_at: DateTime, pub updated_at: DateTime, } @@ -113,6 +123,7 @@ impl Default for CalendarEventDto { rrule: None, ical_uid: String::new(), recurrence_id: None, + ical_data: String::new(), created_at: Utc::now(), updated_at: Utc::now(), } @@ -133,6 +144,7 @@ impl From for CalendarEventDto { rrule: event.rrule().map(|s| s.to_string()), ical_uid: event.ical_uid().to_string(), recurrence_id: event.recurrence_id().copied(), + ical_data: event.ical_data().to_string(), created_at: *event.created_at(), updated_at: *event.updated_at(), } diff --git a/src/interfaces/api/handlers/caldav_handler.rs b/src/interfaces/api/handlers/caldav_handler.rs index e668acd3..fefc666f 100644 --- a/src/interfaces/api/handlers/caldav_handler.rs +++ b/src/interfaces/api/handlers/caldav_handler.rs @@ -26,7 +26,10 @@ use percent_encoding::percent_decode_str; use std::fmt::Write; use std::sync::Arc; -use crate::application::adapters::caldav_adapter::{CalDavAdapter, CalDavReportType}; +use crate::application::adapters::caldav_adapter::{ + CalDavAdapter, CalDavReportType, bundle_to_calendar_body, extract_vevent_chunk, + group_events_by_uid, +}; use crate::application::adapters::uid_from_multiget_href; use crate::application::adapters::webdav_adapter::{PropFindRequest, PropFindType}; use crate::application::dtos::calendar_dto::{ @@ -683,7 +686,13 @@ async fn handle_get( let calendar_id = parts[0]; if parts.len() < 2 { - // GET on calendar collection + // GET on calendar collection — return all events, folded + // per UID so master + exception overrides live in ONE + // VCALENDAR body per resource (RFC 4791 §4.1 + RFC 5545 + // §3.6.1). Serves each row's stored `ical_data` verbatim + // via `bundle_to_calendar_body`; VTIMEZONE / VALARM / + // ATTENDEE / CATEGORIES / X-* survive because we no + // longer regenerate the body from DTO fields. let events = calendar_service .list_events(calendar_id, None, None, user.id) .await @@ -703,83 +712,87 @@ async fn handle_get( .body(Body::from(ical)) .unwrap()) } else { - // GET on individual event — indexed lookup by iCalendar UID. + // GET on individual event resource — fetch ALL rows for + // this UID (master + any exception overrides) and emit + // ONE calendar-object-resource containing every VEVENT. + // This is the phase-4 fix: `get_event_by_ical_uid` is + // master-only; using it here made exceptions invisible + // to clients and their next-PUT would silently drop the + // stored exception rows. let event_file = parts[1]; let ical_uid = event_file.trim_end_matches(".ics"); - let event = calendar_service - .get_event_by_ical_uid(calendar_id, ical_uid, user.id) + let bundle = calendar_service + .get_events_by_ical_uids(calendar_id, &[ical_uid.to_string()], user.id) .await - .map_err(AppError::from)? - .ok_or_else(|| AppError::not_found(format!("Event not found: {}", ical_uid)))?; + .map_err(AppError::from)?; - let ical = generate_event_ical(&event); + if bundle.is_empty() { + return Err(AppError::not_found(format!( + "Event not found: {}", + ical_uid + ))); + } + + // Group so the master (recurrence_id None) sits first, + // then flatten for the bundle emitter. ETag anchors on + // the first row of the first group — that's the master + // for a recurring event, or the sole row for a + // non-recurring one. Stable across bundle contents so + // If-Match on subsequent PUTs keys off the master's id. + let grouped = group_events_by_uid(&bundle); + let flat: Vec<&_> = grouped.into_iter().flatten().collect(); + let etag_source = flat.first().map(|e| e.id.clone()).unwrap_or_default(); + let ical = bundle_to_calendar_body(&flat); Ok(Response::builder() .status(StatusCode::OK) .header(header::CONTENT_TYPE, "text/calendar; charset=utf-8") - .header(header::ETAG, format!("\"{}\"", event.id)) + .header(header::ETAG, format!("\"{}\"", etag_source)) .body(Body::from(ical)) .unwrap()) } } +/// Emit a full VCALENDAR body for the entire calendar, with rows +/// grouped by UID so each recurring event's master + exception +/// overrides live under one iCalendar resource. Each row's stored +/// `ical_data` VEVENT chunk is served verbatim. fn generate_full_calendar_ical( calendar_name: &str, events: &[crate::application::dtos::calendar_dto::CalendarEventDto], ) -> String { - // Pre-estimate: ~200 bytes header + ~320 bytes per event + // Pre-estimate: ~200 bytes header + ~320 bytes per event. let mut buf = String::with_capacity(256 + events.len() * 320); let _ = write!( buf, "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//OxiCloud//NONSGML Calendar//EN\r\nX-WR-CALNAME:{}\r\n", calendar_name ); - for event in events { - write_vevent(&mut buf, event); + // Group + append each row's stored VEVENT chunk. Malformed + // rows are silently skipped (defensive) — the bulk-GET body + // survives the rest. + for group in group_events_by_uid(events) { + for event in group { + if let Some(chunk) = extract_vevent_chunk(&event.ical_data) { + buf.push_str(chunk); + if !buf.ends_with('\n') { + buf.push_str("\r\n"); + } + } + } } buf.push_str("END:VCALENDAR\r\n"); buf } -fn generate_event_ical(event: &crate::application::dtos::calendar_dto::CalendarEventDto) -> String { - let mut buf = String::with_capacity(512); - buf.push_str("BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//OxiCloud//NONSGML Calendar//EN\r\n"); - write_vevent(&mut buf, event); - buf.push_str("END:VCALENDAR\r\n"); - buf -} - -/// Writes a VEVENT block directly into `buf` — zero intermediate allocations. -fn write_vevent( - buf: &mut String, - event: &crate::application::dtos::calendar_dto::CalendarEventDto, -) { - let _ = write!( - buf, - "BEGIN:VEVENT\r\nUID:{}\r\nSUMMARY:{}\r\nDTSTART:{}\r\nDTEND:{}\r\n", - event.ical_uid, - event.summary.replace('\n', "\\n"), - event.start_time.format("%Y%m%dT%H%M%SZ"), - event.end_time.format("%Y%m%dT%H%M%SZ"), - ); - if let Some(ref desc) = event.description { - let _ = write!(buf, "DESCRIPTION:{}\r\n", desc.replace('\n', "\\n")); - } - if let Some(ref loc) = event.location { - let _ = write!(buf, "LOCATION:{}\r\n", loc); - } - if let Some(ref rrule) = event.rrule { - let _ = write!(buf, "RRULE:{}\r\n", rrule); - } - let _ = write!( - buf, - "DTSTAMP:{}\r\nCREATED:{}\r\nLAST-MODIFIED:{}\r\nEND:VEVENT\r\n", - event.updated_at.format("%Y%m%dT%H%M%SZ"), - event.created_at.format("%Y%m%dT%H%M%SZ"), - event.updated_at.format("%Y%m%dT%H%M%SZ"), - ); -} +// NOTE: the pre-phase-4 `generate_event_ical` + `write_vevent` +// helpers were removed. They regenerated the response body from +// DTO fields, which (a) silently dropped every property outside +// the DTO surface (ATTENDEE, VALARM, CATEGORIES, STATUS, X-*) +// and (b) never emitted RECURRENCE-ID on exception rows. The +// `bundle_to_calendar_body` path replaces both by serving each +// row's stored `ical_data` verbatim. // ─── DELETE ────────────────────────────────────────────────────────── diff --git a/tests/api/caldav_recurring.hurl b/tests/api/caldav_recurring.hurl index 9caa1bf2..5422868c 100644 --- a/tests/api/caldav_recurring.hurl +++ b/tests/api/caldav_recurring.hurl @@ -173,10 +173,16 @@ 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. +# Step 7 – GET the URL — must return the FULL calendar-object- +# resource: master VEVENT (with RRULE + original SUMMARY) AND +# the exception VEVENT (with RECURRENCE-ID + rescheduled +# SUMMARY) concatenated in ONE VCALENDAR body. This is the +# phase-4 read-side contract per RFC 4791 §4.1 + RFC 5545 +# §3.6.1 — one URL per UID, one VCALENDAR containing every +# component. +# +# Pre-phase-4 this GET returned ONLY the master and clients +# never saw the exception, so their next-PUT dropped it. # ───────────────────────────────────────────────────────────── GET {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics Authorization: Bearer {{admin_token}} @@ -185,7 +191,8 @@ HTTP 200 [Asserts] body contains "FREQ=DAILY;COUNT=10" body contains "SUMMARY:Daily standup" -body not contains "SUMMARY:Daily standup — rescheduled" +body contains "SUMMARY:Daily standup — rescheduled" +body contains "RECURRENCE-ID:20260103T090000Z" # ───────────────────────────────────────────────────────────── @@ -216,9 +223,15 @@ 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. +# Step 9 – After the exception-only PUT: bundled GET returns +# the master (unchanged, still carries RRULE + original +# SUMMARY) AND the newly-updated exception (SUMMARY now +# "rescheduled AGAIN" from Step 8). +# +# Pre-phase-3 the exception-only PUT wiped the master row. +# Pre-phase-4 the master survived but the exception was +# invisible in the GET body. +# Post-phase-4: both survive, both visible. # ───────────────────────────────────────────────────────────── GET {{base_url}}/caldav/{{calendar_id}}/daily-e2e-528.ics Authorization: Bearer {{admin_token}} @@ -227,7 +240,8 @@ HTTP 200 [Asserts] body contains "FREQ=DAILY;COUNT=10" body contains "SUMMARY:Daily standup" -body not contains "SUMMARY:Daily standup — rescheduled" +body contains "SUMMARY:Daily standup — rescheduled AGAIN" +body contains "RECURRENCE-ID:20260103T090000Z" # ───────────────────────────────────────────────────────────── diff --git a/tests/caldav/test_ical_coverage.py b/tests/caldav/test_ical_coverage.py index 2d011a99..e546f340 100644 --- a/tests/caldav/test_ical_coverage.py +++ b/tests/caldav/test_ical_coverage.py @@ -12,16 +12,13 @@ DTSTART / DTEND / DESCRIPTION / LOCATION / RRULE / DTSTAMP / CREATED / LAST-MODIFIED). Anything not in that list is silently dropped even though the original `ical_data` is stored intact. -Tests split into two groups: - - * **Sanity** — properties the server emits on GET; they must - round-trip. Regressions here would be genuine server bugs. - - * **xfail (documented gaps)** — properties the server currently - drops. `@pytest.mark.xfail(strict=False)` lets the suite stay - green while making the gap visible in the pytest summary. If - a future server fix makes one of these survive, pytest - reports it as `XPASS` — an alert to remove the marker. +Every test is a strict round-trip pin: PUT a vCalendar body +carrying the property, GET the URL, assert the property is +present in the response. Post-phase-4 the emitter serves each +row's stored `ical_data` verbatim (folded per UID), so a +regression on any property here means either the storage +layer stopped preserving ical_data OR the emitter reverted +to DTO-field regeneration. """ from __future__ import annotations @@ -30,7 +27,6 @@ import textwrap import uuid import caldav -import pytest # ───────────────────────────────────────────────────────────── @@ -151,24 +147,16 @@ def test_uid_and_dtstamp_are_preserved(fresh_calendar: caldav.Calendar) -> None: # ───────────────────────────────────────────────────────────── -# Documented gaps — properties the server currently drops on -# GET. `xfail(strict=False)` means "expected to fail; don't fail -# the suite, but flag XPASS if it starts passing". When the -# read-side fix lands, remove the marker. +# Extended round-trips — properties beyond the DTO-structured +# columns. Post-phase-4 the emitter serves each row's stored +# `ical_data` verbatim (folded per UID), so ATTENDEE, ORGANIZER, +# CATEGORIES, STATUS+TRANSP, VALARM (nested), custom X-* all +# survive PUT → GET. A regression on any of these means either +# storage stopped preserving ical_data OR the emitter reverted +# to DTO regeneration. # ───────────────────────────────────────────────────────────── -_EMITTER_GAP_REASON = ( - "GET regenerates the body from DTO fields via write_vevent " - "(caldav_handler.rs:~770) which only emits UID / SUMMARY / " - "DTSTART / DTEND / DESCRIPTION / LOCATION / RRULE / DTSTAMP / " - "CREATED / LAST-MODIFIED. Every other iCal property is stored " - "in ical_data on the row but silently dropped on read. " - "Fix path: either serve ical_data verbatim on GET, or extend " - "the DTO to carry the full property set." -) - -@pytest.mark.xfail(reason=_EMITTER_GAP_REASON, strict=False) def test_attendee_survives_round_trip(fresh_calendar: caldav.Calendar) -> None: uid = f"cov-attendee-{uuid.uuid4().hex[:8]}" body = _minimal_event( @@ -185,7 +173,6 @@ def test_attendee_survives_round_trip(fresh_calendar: caldav.Calendar) -> None: assert "alice@example.com" in fetched -@pytest.mark.xfail(reason=_EMITTER_GAP_REASON, strict=False) def test_organizer_survives_round_trip(fresh_calendar: caldav.Calendar) -> None: uid = f"cov-organizer-{uuid.uuid4().hex[:8]}" body = _minimal_event( @@ -199,7 +186,6 @@ def test_organizer_survives_round_trip(fresh_calendar: caldav.Calendar) -> None: assert "bob@example.com" in fetched -@pytest.mark.xfail(reason=_EMITTER_GAP_REASON, strict=False) def test_categories_survive_round_trip(fresh_calendar: caldav.Calendar) -> None: uid = f"cov-cats-{uuid.uuid4().hex[:8]}" body = _minimal_event( @@ -213,7 +199,6 @@ def test_categories_survive_round_trip(fresh_calendar: caldav.Calendar) -> None: assert "ENGINEERING" in fetched -@pytest.mark.xfail(reason=_EMITTER_GAP_REASON, strict=False) def test_status_and_transp_survive_round_trip( fresh_calendar: caldav.Calendar, ) -> None: @@ -233,7 +218,6 @@ def test_status_and_transp_survive_round_trip( assert "TRANSP:TRANSPARENT" in fetched -@pytest.mark.xfail(reason=_EMITTER_GAP_REASON, strict=False) def test_valarm_survives_round_trip(fresh_calendar: caldav.Calendar) -> None: """VALARM is a nested sub-component of VEVENT (RFC 5545 §3.6.6) and drives every "remind me 15 min before" popup. It lives @@ -268,7 +252,6 @@ def test_valarm_survives_round_trip(fresh_calendar: caldav.Calendar) -> None: assert "TRIGGER:-PT15M" in fetched -@pytest.mark.xfail(reason=_EMITTER_GAP_REASON, strict=False) def test_custom_x_property_survives_round_trip( fresh_calendar: caldav.Calendar, ) -> None: diff --git a/tests/caldav/test_recurring.py b/tests/caldav/test_recurring.py index dd3fe31a..d6b66912 100644 --- a/tests/caldav/test_recurring.py +++ b/tests/caldav/test_recurring.py @@ -182,19 +182,25 @@ def test_recurring_master_plus_exception_preserves_master( body = _get_master_ical(fresh_calendar, uid) assert "RRULE:FREQ=DAILY;COUNT=10" in body, ( "Master row lost its RRULE — the exception overwrote the master. " - "This is the exact regression from #528.\nMaster body: " + body + "This is the exact regression from #528.\nBundle body: " + body ) assert "SUMMARY:Daily standup" in body - - # NOTE: not asserting the exception row is client-visible here. - # RFC 4791 §4.1 + RFC 5545 §3.8.4.4 model a recurring event with - # per-instance overrides as ONE calendar-object-resource whose - # VCALENDAR contains the master VEVENT + all exception VEVENTs. - # OxiCloud currently persists them as separate rows but the - # GET/PROPFIND emitter returns only the master (see phase-4 - # follow-up on branch feat/caldav-read-side). Once phase 4 - # lands, add: assert "RECURRENCE-ID" in body and - # assert "rescheduled" in body. + # Phase-4 read-side unification: the GET response is the + # WHOLE calendar-object-resource — master + all exception + # VEVENTs concatenated in one VCALENDAR per RFC 4791 §4.1 + + # RFC 5545 §3.6.1. The exception's SUMMARY and its + # RECURRENCE-ID must therefore appear alongside the master's + # RRULE. Pre-phase-4 the emitter served only the master row + # and clients silently dropped the exception on next-PUT. + assert "SUMMARY:Daily standup — rescheduled" in body, ( + "Exception VEVENT missing from bundled GET body — phase-4 " + "read-side regression.\nBundle body: " + body + ) + assert "RECURRENCE-ID" in body, ( + "Exception RECURRENCE-ID missing from bundled GET body — " + "clients need it to correlate the override with the master.\n" + "Bundle body: " + body + ) def test_exception_only_put_does_not_wipe_master( @@ -255,24 +261,29 @@ def test_exception_only_put_does_not_wipe_master( ), ) - # Master URL GET must still return the master. Pre-fix the - # exception-only PUT would have replaced the master (keyed by - # UID with no recurrence_id filter) — this is the data-loss - # half of #528. + # Bundled GET returns the WHOLE calendar-object-resource: + # master row (unchanged since Step 1 seed) + the updated + # exception row (SUMMARY "rescheduled AGAIN" from the + # exception-only PUT above). + # Pre-phase-3 the exception-only PUT wiped the master. + # Pre-phase-4 the master survived but the exception was + # invisible in the GET body. + # Post-phase-4: both survive AND both are visible. body = _get_master_ical(fresh_calendar, uid) - assert "RRULE:FREQ=DAILY;COUNT=10" in body - assert "SUMMARY:Daily standup" in body - assert "rescheduled" not in body, ( - "GET on the master URL returned the exception's data — the " - "master was clobbered by the exception-only PUT." + assert "RRULE:FREQ=DAILY;COUNT=10" in body, ( + "Master row lost its RRULE — data-loss regression from #528.\n" + "Bundle body: " + body + ) + assert "SUMMARY:Daily standup" in body, ( + "Master's original SUMMARY missing from bundle body — the " + "master row was clobbered by the exception-only PUT.\n" + "Bundle body: " + body + ) + assert "SUMMARY:Daily standup — rescheduled AGAIN" in body, ( + "Updated exception SUMMARY missing — the second exception-only " + "PUT either failed to update or the emitter dropped the exception " + "row from the bundle.\nBundle body: " + body ) - - # NOTE: exception-row survival is not asserted client-side - # today — the emitter only surfaces the master. Phase 4 - # (feat/caldav-read-side) will fold master + exceptions into a - # single VCALENDAR body; once landed, add an assertion that the - # updated exception's SUMMARY ("rescheduled AGAIN") is present - # in the same GET body as the master's RRULE. # ───────────────────────────────────────────────────────────── @@ -322,7 +333,16 @@ def test_all_day_recurring_master_plus_exception( f"Master body: {data}" ) assert "SUMMARY:Weekly review" in data - - # NOTE: exception row is stored server-side but not yet visible - # in the GET body. Phase 4 will fold it in — assertion to add - # once that lands: assert "RECURRENCE-ID;VALUE=DATE:20260112" in data. + # Phase-4 bundle: exception row visible in the GET body. + # DATE-form RECURRENCE-ID (with the `;VALUE=DATE` parameter) + # survives verbatim because we serve stored ical_data + # instead of regenerating. + assert "SUMMARY:Weekly review — moved" in data, ( + "All-day exception SUMMARY missing from bundled GET body:\n" + + data + ) + assert "RECURRENCE-ID;VALUE=DATE:20260112" in data, ( + "DATE-form RECURRENCE-ID lost — either the exception row " + "isn't in the bundle or the emitter mangled the property " + "parameter.\nBundle body: " + data + )