Merge pull request #588 from EdouardVanbelle/fix/528-Recurrence-on-ical

This commit is contained in:
Dionisio Pozo
2026-07-14 23:10:19 +02:00
committed by GitHub
14 changed files with 1522 additions and 198 deletions
Generated
+10
View File
@@ -3155,6 +3155,15 @@ dependencies = [
"cc",
]
[[package]]
name = "ical"
version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9b7cab7543a8b7729a19e2c04309f902861293dcdae6558dfbeb634454d279f6"
dependencies = [
"thiserror 1.0.69",
]
[[package]]
name = "icu_collections"
version = "2.2.0"
@@ -4247,6 +4256,7 @@ dependencies = [
"http-body 1.0.1",
"http-body-util",
"http-range-header",
"ical",
"id3",
"idna",
"image",
+15
View File
@@ -19,6 +19,21 @@ flate2 = "1.1.9"
tracing = "0.1.44"
tracing-subscriber = { version = "0.3.23", features = ["env-filter"] }
chrono = { version = "0.4.45", features = ["serde"] }
# RFC 5545 iCalendar parser + emitter.
#
# Adopted 2026-07-14 to replace the hand-rolled property-scan in
# `src/domain/entities/calendar_event.rs::extract_ical_property`,
# which used a naive `format!("\n{}:", name)` substring search and
# refused ANY property carrying parameters (`DTSTART;VALUE=DATE:...`,
# `RECURRENCE-ID;VALUE=DATE:...`, `ATTENDEE;CN=…;PARTSTAT=…:mailto:…`).
# That broke all-day events and made the domain unaware of exception
# instances (see AtalayaLabs/OxiCloud#528).
#
# The crate is the widely-used Rust parser (~1500 SLOC, MIT/Apache),
# actively maintained by @Peltoche as `ical-rs` on GitHub. It handles
# line-folding, escaped characters, parameter maps, and every standard
# component. If a spec conformance gap is found, we contribute upstream.
ical = "0.11"
http-body = "1.0.1"
serde = { version = "1.0.228", features = ["derive"] }
serde_json = "1.0.150"
@@ -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;
@@ -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(),
}
+8
View File
@@ -89,6 +89,12 @@ pub struct CalendarEventDto {
pub all_day: bool,
pub rrule: Option<String>,
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<DateTime<Utc>>,
pub created_at: DateTime<Utc>,
pub updated_at: DateTime<Utc>,
}
@@ -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<CalendarEvent> 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(),
}
+39
View File
@@ -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<CalendarEventDto>,
/// 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<CalendarEventDto, DomainError>;
/// 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<UpsertEventsResult, DomainError>;
async fn update_event(
&self,
event_id: &str,
@@ -136,6 +166,15 @@ pub trait CalendarUseCase: Send + Sync + 'static {
event: CreateEventICalDto,
user_id: Uuid,
) -> Result<CalendarEventDto, DomainError>;
/// 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<UpsertEventsResult, DomainError>;
async fn update_event(
&self,
event_id: &str,
+18 -1
View File
@@ -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<UpsertEventsResult, DomainError> {
// 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,
File diff suppressed because it is too large Load Diff
@@ -46,13 +46,35 @@ pub trait CalendarEventRepository: Send + Sync + 'static {
end: &DateTime<Utc>,
) -> CalendarEventRepositoryResult<Vec<CalendarEvent>>;
/// 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<Option<CalendarEvent>>;
/// 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<Utc>,
) -> CalendarEventRepositoryResult<Option<CalendarEvent>>;
/// 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.
@@ -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<UpsertEventsResult, DomainError> {
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,
@@ -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,67 @@ 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),
}
}
async fn find_event_by_ical_uid_and_recurrence_id(
&self,
calendar_id: &Uuid,
ical_uid: &str,
recurrence_id: &DateTime<Utc>,
) -> CalendarEventRepositoryResult<Option<CalendarEvent>> {
// 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::<Option<String>, _>("description"),
row.get::<Option<String>, _>("location"),
row.get("start_time"),
row.get("end_time"),
row.get("all_day"),
row.get::<Option<String>, _>("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::<Option<DateTime<Utc>>, _>("recurrence_id"));
Ok(Some(event))
}
None => Ok(None),
@@ -375,12 +454,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 +481,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 +499,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);
}
+39 -75
View File
@@ -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<String> {
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) ──────────────────────────────────────────────────────
+280
View File
@@ -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/<uuid>/ 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
```
<?xml version="1.0" encoding="UTF-8"?>
<D:propfind xmlns:D="DAV:">
<D:prop>
<D:displayname/>
<D:resourcetype/>
</D:prop>
</D:propfind>
```
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
+1
View File
@@ -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" \