From 0c0558c24c000c73b41640ea313d97446aa5bd27 Mon Sep 17 00:00:00 2001 From: randogoth Date: Thu, 10 Sep 2026 13:29:02 +0300 Subject: [PATCH] Let an event choose which source it is filed under MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An event created in an aggregate went to the configured default_sink and nowhere else, so with several writable sources there was no way to say which calendar a new event belonged in. A line reading @endpoint-id in the description now picks the source, and a matching CATEGORIES value does too. The description rather than the title because every calendar client exposes a notes field and editing it does not disfigure the event's name; CATEGORIES as well because that is the field iCalendar intends, even though many mobile clients hide it. The marker is stripped before the event reaches the calendar, being calcalist's bookkeeping rather than content. A marker naming something that is not a writable source of that aggregate is refused and reported, not redirected to the default: a typo should not quietly file an event in the wrong calendar. A bare address in prose is not a marker either, since a marker must be a line of its own. Also covers the shapes beyond many-into-one: a source feeding several aggregates, several aggregates sharing one target, a cycle between two aggregates, a delete cascading across aggregates, and competing edits arriving through two aggregates at once — the last being caught by the existing conflict detection rather than silently overwriting. Error display no longer repeats itself; thiserror already prints the cause chain. Verified live against real accounts: two Google calendars aggregating into a Posteo calendar, an edit in the aggregate reaching the originating Google calendar, an event routed to a chosen source by its description marker, and a deletion propagating from the aggregate through to Google. 129 tests. Co-Authored-By: Claude Opus 5 --- SPECS.md | 2 +- TODO.md | 13 ++- src/main.rs | 6 ++ src/mirror.rs | 82 ++++++++++++++ src/reconcile.rs | 140 +++++++++++++++++++++++- src/sync.rs | 275 ++++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 506 insertions(+), 12 deletions(-) diff --git a/SPECS.md b/SPECS.md index 69c2557..7300e38 100644 --- a/SPECS.md +++ b/SPECS.md @@ -20,7 +20,7 @@ Every endpoint — sources and aggregate targets alike — is mirrored to a loca | Question | Behaviour | |---|---| | Provenance | Aggregate UIDs derived as `blake3(aggregate_id, source_id, source_uid)`; the state file is a cache, not a single point of failure | -| Routing new events | To the aggregate's configured `default_sink`; refused if none is set | +| Routing new events | A `@endpoint-id` line in the description, or a matching category, picks the source; otherwise the aggregate's `default_sink`. A hint naming an invalid sink is refused, never redirected to the default | | Conflicts | Source wins — the origin calendar is authoritative | | Deletion | Propagates to the source, guarded by a mass-deletion threshold | | Attendees (mirroring) | Kept verbatim on a Google target; demoted to inert data on a CalDAV target | diff --git a/TODO.md b/TODO.md index 936fb77..6c47de7 100644 --- a/TODO.md +++ b/TODO.md @@ -63,15 +63,22 @@ Tests: - [x] Retarget: drift makes `sync` exit non-zero having written nothing and losing no source event; purge is bounded by the derivation; an unrouted creation reaches a sink first +### Verified live, end to end + +Two Google calendars aggregating into a Posteo CalDAV calendar, against real accounts: +fan-in from both sources with provenance intact; an edit in the aggregate reaching the +originating Google calendar; an event created in the aggregate routed to a chosen +source by a description marker; deletion propagating from the aggregate through to +Google; and the mass-deletion guard refusing a 100% removal until `--force`. + ### Known gaps carried out of M1 - [ ] A recurring series' *exceptions* are not pushed to Google. Google models them as separate events against an already existing series, so they need `events.instances` plus a patch per exception. Reported per sync rather than dropped silently. -- [ ] `push` deleting an event remotely is exercised only when the reconciler removes - a mirror mid-cycle; it has no end-to-end test yet, because a pull legitimately - resurrects anything deleted from a vdir before the cycle runs. +- [x] `push` deleting an event remotely — verified live: deleting a mirror in the + CalDAV aggregate removed the origin event from Google. - [ ] A failed Google pull aborts the whole cycle, including the CalDAV side. Safe — reconciling against a stale snapshot could read as mass deletion — but it means a lapsed token stops everything. diff --git a/src/main.rs b/src/main.rs index 80d187a..1aee63a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -199,6 +199,12 @@ fn describe(skipped: &reconcile::Skipped) -> String { } => format!( "`{aggregate_uid}` was edited, but its source `{source_id}` is a read-only feed" ), + reconcile::Skipped::UnknownSink { + aggregate_uid, + requested, + } => format!( + "`{aggregate_uid}` asked to be filed under `{requested}`, which is not a writable source of this aggregate; it was left where it is" + ), } } diff --git a/src/mirror.rs b/src/mirror.rs index 2fd4e7f..f846344 100644 --- a/src/mirror.rs +++ b/src/mirror.rs @@ -106,6 +106,88 @@ fn append_guests_to_description(calendar: &mut Calendar, guests: &[String]) { calendar.set_property("VEVENT", "DESCRIPTION", &combined); } +/// Marks which source an event created in the aggregate should be filed under. +/// +/// A line of its own in the description, `@endpoint-id`. The description is used +/// rather than the title because every calendar client exposes a notes field and +/// editing it does not disfigure the event's name, and rather than CATEGORIES +/// because many mobile clients do not surface categories at all — though a +/// matching category is honoured too, that being the field iCalendar intends. +pub const ROUTE_MARKER: char = '@'; + +/// The source an event asks to be filed under, if it names one. +pub fn routing_hint(calendar: &Calendar) -> Option { + if let Some(description) = calendar + .properties("VEVENT", "DESCRIPTION") + .next() + .map(|property| property.value) + && let Some(hint) = description_hint(description) + { + return Some(hint); + } + calendar + .properties("VEVENT", "CATEGORIES") + .flat_map(|property| { + property + .value + .split(',') + .map(|category| category.trim().to_string()) + .collect::>() + }) + .find(|category| !category.is_empty()) +} + +/// Finds a line consisting only of `@name`. Requiring the whole line keeps an +/// address written in prose from being mistaken for a routing instruction. +fn description_hint(description: &str) -> Option { + description + .split("\\n") + .map(str::trim) + .find_map(|line| line.strip_prefix(ROUTE_MARKER)) + .filter(|name| !name.is_empty() && !name.contains(char::is_whitespace)) + .map(str::to_string) +} + +/// Removes the routing instruction, which is calcalist's own bookkeeping and has +/// no business appearing in the calendar the event lands in. +pub fn strip_routing_hint(calendar: &mut Calendar, hint: &str) { + let categories: Vec = calendar + .properties("VEVENT", "CATEGORIES") + .map(|property| property.value.to_string()) + .collect(); + if !categories.is_empty() { + calendar.remove_properties("VEVENT", &["CATEGORIES"]); + let kept: Vec = categories + .iter() + .flat_map(|value| value.split(',')) + .map(str::trim) + .filter(|category| !category.eq_ignore_ascii_case(hint) && !category.is_empty()) + .map(str::to_string) + .collect(); + if !kept.is_empty() { + calendar.add_property("VEVENT", "CATEGORIES", &kept.join(",")); + } + } + + let Some(description) = calendar + .properties("VEVENT", "DESCRIPTION") + .next() + .map(|property| property.value.to_string()) + else { + return; + }; + let marker = format!("{ROUTE_MARKER}{hint}"); + let kept: Vec<&str> = description + .split("\\n") + .filter(|line| line.trim() != marker) + .collect(); + if kept.iter().all(|line| line.trim().is_empty()) { + calendar.remove_properties("VEVENT", &["DESCRIPTION"]); + } else { + calendar.set_property("VEVENT", "DESCRIPTION", &kept.join("\\n")); + } +} + /// Rebuilds a source-side event from an edited aggregate copy. /// /// `donor` is the event as it currently stands in the source calendar, when diff --git a/src/reconcile.rs b/src/reconcile.rs index 53e0201..6ac7a07 100644 --- a/src/reconcile.rs +++ b/src/reconcile.rs @@ -91,6 +91,13 @@ pub enum Skipped { aggregate_uid: String, source_id: String, }, + /// The event asked to be filed under a source that cannot take it. Refused + /// rather than sent to the default, since a typo should not quietly put the + /// event in the wrong calendar. + UnknownSink { + aggregate_uid: String, + requested: String, + }, } #[derive(Debug)] @@ -348,20 +355,34 @@ fn route_new_events( .filter(|(uid, _)| !provenance::is_derived(uid)); for (uid, item) in user_created { - let sink = policy - .default_sink + // An event may name the source it belongs in; otherwise the aggregate's + // configured sink takes it. + let requested = mirror::routing_hint(&item.calendar); + let wanted = requested.as_deref().or(policy.default_sink); + let sink = wanted .and_then(|sink| sources.iter().find(|source| source.id == sink)) .filter(|sink| sink.writable); + let Some(sink) = sink else { - outcome.skipped.push(Skipped::NoSink { - aggregate_uid: uid.clone(), + outcome.skipped.push(match requested { + Some(requested) => Skipped::UnknownSink { + aggregate_uid: uid.clone(), + requested, + }, + None => Skipped::NoSink { + aggregate_uid: uid.clone(), + }, }); continue; }; // Attendees are kept here deliberately: the user is organising a meeting, // and the sink server inviting the guests is the intended behaviour. - let routed = mirror::to_source(&item.calendar, None, uid); + let mut prepared = item.calendar.clone(); + if let Some(hint) = &requested { + mirror::strip_routing_hint(&mut prepared, hint); + } + let routed = mirror::to_source(&prepared, None, uid); let source_hash = routed.content_hash(); let aggregate_uid = derive_uid(policy.aggregate_id, sink.id, uid).to_string(); let canonical = mirror::to_aggregate( @@ -730,6 +751,115 @@ mod tests { assert!(outcome.skipped.is_empty()); } + /// A note in the description picks the calendar the event lands in. + #[test] + fn a_description_marker_chooses_the_sink() { + let sources = map(vec![]); + let other = map(vec![]); + let with_hint = Calendar::parse( + "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nBEGIN:VEVENT\r\nUID:hand@phone\r\nDTSTART:20260910T090000Z\r\nSUMMARY:Dentist\r\nDESCRIPTION:Remember the card\\n@other\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n", + ) + .expect("parse"); + let aggregate = map(vec![Item { + uid: "hand@phone".into(), + path: Path::new("/tmp/x.ics").into(), + hash: with_hint.content_hash(), + calendar: with_hint, + }]); + + let views = [ + view(&sources, true), + SourceView { + id: "other", + writable: true, + owner: None, + items: &other, + }, + ]; + let policy = Policy { + default_sink: Some(SRC), + ..policy() + }; + let outcome = reconcile(policy, &views, &aggregate, None).expect("ok"); + + let routed = outcome + .actions + .iter() + .find_map(|action| match action { + Action::WriteSource { + source_id, + calendar, + .. + } => Some((source_id.clone(), calendar.clone())), + _ => None, + }) + .expect("an event should be routed"); + assert_eq!(routed.0, "other", "the marker should win over default_sink"); + // The instruction is bookkeeping and must not reach the calendar. + let ics = routed.1.to_ics(); + assert!(!ics.contains("@other"), "{ics}"); + assert!(ics.contains("Remember the card"), "{ics}"); + } + + /// A typo must not quietly file the event in the wrong calendar. + #[test] + fn an_unknown_marker_is_refused_rather_than_guessed() { + let sources = map(vec![]); + let with_typo = Calendar::parse( + "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nBEGIN:VEVENT\r\nUID:hand@phone\r\nDTSTART:20260910T090000Z\r\nSUMMARY:Dentist\r\nDESCRIPTION:@nosuchplace\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n", + ) + .expect("parse"); + let aggregate = map(vec![Item { + uid: "hand@phone".into(), + path: Path::new("/tmp/x.ics").into(), + hash: with_typo.content_hash(), + calendar: with_typo, + }]); + let policy = Policy { + default_sink: Some(SRC), + ..policy() + }; + let outcome = reconcile(policy, &[view(&sources, true)], &aggregate, None).expect("ok"); + assert!(outcome.actions.is_empty(), "{:?}", outcome.actions); + assert_eq!( + outcome.skipped, + vec![Skipped::UnknownSink { + aggregate_uid: "hand@phone".into(), + requested: "nosuchplace".into(), + }] + ); + } + + /// An address in prose is not a routing instruction. + #[test] + fn an_email_address_in_the_notes_is_not_a_marker() { + let sources = map(vec![]); + let prose = Calendar::parse( + "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nBEGIN:VEVENT\r\nUID:hand@phone\r\nDTSTART:20260910T090000Z\r\nSUMMARY:Dentist\r\nDESCRIPTION:ask me@example.com about it\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n", + ) + .expect("parse"); + let aggregate = map(vec![Item { + uid: "hand@phone".into(), + path: Path::new("/tmp/x.ics").into(), + hash: prose.content_hash(), + calendar: prose, + }]); + let policy = Policy { + default_sink: Some(SRC), + ..policy() + }; + let outcome = reconcile(policy, &[view(&sources, true)], &aggregate, None).expect("ok"); + // Falls through to the configured sink, unbothered by the address. + assert!( + outcome.actions.iter().any(|action| matches!( + action, + Action::WriteSource { source_id, .. } if source_id == SRC + )), + "{:?}", + outcome.actions + ); + } + /// Guessing a sink would put the event in the wrong calendar, so refuse. #[test] fn without_a_sink_a_user_created_event_is_left_alone() { diff --git a/src/sync.rs b/src/sync.rs index 4eee8bb..b6e1f1d 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -26,13 +26,13 @@ pub enum SyncError { State(#[from] crate::state::StateError), #[error(transparent)] Pimsync(#[from] PimsyncError), - #[error("google endpoint `{endpoint}`: {source}")] + #[error("google endpoint `{endpoint}` could not be synchronised")] Google { endpoint: String, #[source] source: ApiError, }, - #[error("google endpoint `{endpoint}`: {source}")] + #[error("google endpoint `{endpoint}` could not be authorised")] GoogleAuth { endpoint: String, #[source] @@ -44,7 +44,7 @@ pub enum SyncError { #[source] source: std::io::Error, }, - #[error("aggregate `{aggregate}`: {source}")] + #[error("aggregate `{aggregate}` could not be reconciled")] Reconcile { aggregate: String, #[source] @@ -437,3 +437,272 @@ pub fn vdir_root(state_dir: &Path) -> PathBuf { pub fn vdir_path(state_dir: &Path, endpoint_id: &str) -> PathBuf { vdir_root(state_dir).join(endpoint_id) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::state::State; + use std::fs; + + /// Endpoints are Google-typed so nothing here touches pimsync. The tests call + /// `sync_aggregate` directly rather than `run`, so no network is involved. + const CONFIG: &str = r#" +version = 1 + +[[endpoint]] +id = "work" +type = "google" +calendar_id = "work@example.com" +client_id = "x.apps.googleusercontent.com" + +[[endpoint]] +id = "home" +type = "google" +calendar_id = "home@example.com" +client_id = "x.apps.googleusercontent.com" + +[[endpoint]] +id = "gcal" +type = "google" +calendar_id = "gcal@example.com" +client_id = "x.apps.googleusercontent.com" + +[[endpoint]] +id = "posteo" +type = "google" +calendar_id = "posteo@example.com" +client_id = "x.apps.googleusercontent.com" +"#; + + fn event(uid: &str, summary: &str) -> String { + format!( + "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nBEGIN:VEVENT\r\nUID:{uid}\r\nDTSTART:20260910T090000Z\r\nDTSTAMP:20260101T000000Z\r\nSUMMARY:{summary}\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n" + ) + } + + fn seed(dir: &Path, endpoint: &str, uid: &str, summary: &str) { + let vdir = vdir_path(dir, endpoint); + fs::create_dir_all(&vdir).expect("create vdir"); + fs::write(vdir.join(format!("{uid}.ics")), event(uid, summary)).expect("write"); + } + + fn settle(config: &Config, dir: &Path, aggregate_id: &str) -> AggregateReport { + let aggregate = config + .aggregates + .iter() + .find(|candidate| candidate.id == aggregate_id) + .expect("aggregate"); + let target = config + .endpoint(&aggregate.target) + .expect("target endpoint") + .clone(); + let state_path = dir.join(crate::state::FILE_NAME); + let mut state = State::load(&state_path).expect("state"); + let report = sync_aggregate(config, aggregate, &target, dir, &mut state, false, false) + .expect("reconcile"); + state.save(&state_path).expect("save"); + report + } + + fn uids(dir: &Path, endpoint: &str) -> Vec { + vdir::read(&vdir_path(dir, endpoint)) + .expect("read") + .keys() + .cloned() + .collect() + } + + /// One calendar published into two different targets at once. + #[test] + fn a_source_can_feed_several_aggregates() { + let config: Config = toml::from_str(&format!( + r#"{CONFIG} +[[aggregate]] +id = "to-google" +target = "gcal" +sources = ["work"] + +[[aggregate]] +id = "to-posteo" +target = "posteo" +sources = ["work"] +"# + )) + .expect("config"); + let dir = tempfile::tempdir().expect("temp dir"); + seed(dir.path(), "work", "e1@example.com", "Standup"); + + settle(&config, dir.path(), "to-google"); + settle(&config, dir.path(), "to-posteo"); + + let google = uids(dir.path(), "gcal"); + let posteo = uids(dir.path(), "posteo"); + assert_eq!(google.len(), 1); + assert_eq!(posteo.len(), 1); + // The aggregate id is part of the derivation, so the two copies are + // distinct events rather than one event in two places. + assert_ne!(google[0], posteo[0]); + } + + /// Two aggregates publishing into the same calendar must not fight over it. + #[test] + fn two_aggregates_can_share_one_target() { + let config: Config = toml::from_str(&format!( + r#"{CONFIG} +[[aggregate]] +id = "from-work" +target = "gcal" +sources = ["work"] + +[[aggregate]] +id = "from-home" +target = "gcal" +sources = ["home"] +"# + )) + .expect("config"); + let dir = tempfile::tempdir().expect("temp dir"); + seed(dir.path(), "work", "w1@example.com", "Standup"); + seed(dir.path(), "home", "h1@example.com", "Dentist"); + + settle(&config, dir.path(), "from-work"); + settle(&config, dir.path(), "from-home"); + // Reconciling the first again must not disturb the second's events. + let again = settle(&config, dir.path(), "from-work"); + + assert_eq!(uids(dir.path(), "gcal").len(), 2); + assert_eq!(again.deleted_from_aggregate, 0, "{again:?}"); + assert_eq!(again.written_back, 0, "{again:?}"); + assert_eq!(again.mirrored, 0, "{again:?}"); + } + + fn fan_out_config() -> Config { + toml::from_str(&format!( + r#"{CONFIG} +[[aggregate]] +id = "to-google" +target = "gcal" +sources = ["work"] + +[[aggregate]] +id = "to-posteo" +target = "posteo" +sources = ["work"] +"# + )) + .expect("config") + } + + /// Deleting a mirror deletes the origin, and the origin's disappearance then + /// retires every other aggregate's copy. Correct, and worth knowing: a delete + /// in one aggregate is not local to it. + #[test] + fn deleting_a_mirror_removes_the_event_from_every_aggregate() { + let config = fan_out_config(); + let dir = tempfile::tempdir().expect("temp dir"); + seed(dir.path(), "work", "e1@example.com", "Standup"); + settle(&config, dir.path(), "to-google"); + settle(&config, dir.path(), "to-posteo"); + assert_eq!(uids(dir.path(), "posteo").len(), 1); + + // Remove it from just one of the two aggregates. + let mirror = vdir::read(&vdir_path(dir.path(), "gcal")).expect("read"); + let path = mirror.values().next().expect("a mirror").path.clone(); + vdir::remove(&path).expect("remove"); + + let first = settle(&config, dir.path(), "to-google"); + assert_eq!(first.deleted_from_sources, 1, "the origin should go"); + assert!(uids(dir.path(), "work").is_empty()); + + let second = settle(&config, dir.path(), "to-posteo"); + assert_eq!(second.deleted_from_aggregate, 1, "the other copy follows"); + assert!(uids(dir.path(), "posteo").is_empty()); + } + + /// Editing the same event through two aggregates in one pass: the first edit + /// reaches the origin, and the second is then a genuine both-sides-changed + /// conflict, so it is reported rather than silently overwriting. + #[test] + fn competing_edits_through_two_aggregates_are_reported_as_conflicts() { + let config = fan_out_config(); + let dir = tempfile::tempdir().expect("temp dir"); + seed(dir.path(), "work", "e1@example.com", "Standup"); + settle(&config, dir.path(), "to-google"); + settle(&config, dir.path(), "to-posteo"); + + for (endpoint, summary) in [ + ("gcal", "Edited via Google"), + ("posteo", "Edited via Posteo"), + ] { + let held = vdir::read(&vdir_path(dir.path(), endpoint)).expect("read"); + let (uid, item) = held.iter().next().expect("a mirror"); + let edited = item + .calendar + .to_ics() + .replace("SUMMARY:Standup", &format!("SUMMARY:{summary}")); + let calendar = crate::ical::Calendar::parse(&edited).expect("parse"); + vdir::write( + &vdir_path(dir.path(), endpoint), + uid, + &calendar, + Some(&item.path), + ) + .expect("write"); + } + + let first = settle(&config, dir.path(), "to-google"); + assert_eq!(first.written_back, 1, "the first edit reaches the origin"); + + let second = settle(&config, dir.path(), "to-posteo"); + assert_eq!( + second.conflicts.len(), + 1, + "the second is a conflict: {second:?}" + ); + // Source wins, so the origin keeps the edit that got there first. + let work = vdir::read(&vdir_path(dir.path(), "work")).expect("read"); + let ics = work.values().next().expect("origin").calendar.to_ics(); + assert!(ics.contains("Edited via Google"), "{ics}"); + } + + /// A mirror that lands back in a source must not be mirrored onward, or a + /// pair of aggregates pointing at each other would breed events forever. + #[test] + fn a_cycle_between_two_aggregates_does_not_multiply_events() { + let config: Config = toml::from_str(&format!( + r#"{CONFIG} +[[aggregate]] +id = "forward" +target = "gcal" +sources = ["work"] + +[[aggregate]] +id = "backward" +target = "work" +sources = ["gcal"] +"# + )) + .expect("config"); + let dir = tempfile::tempdir().expect("temp dir"); + seed(dir.path(), "work", "e1@example.com", "Standup"); + + for _ in 0..3 { + settle(&config, dir.path(), "forward"); + settle(&config, dir.path(), "backward"); + } + + // One original, one mirror of it. The mirror is never re-ingested. + assert_eq!( + uids(dir.path(), "gcal").len(), + 1, + "{:?}", + uids(dir.path(), "gcal") + ); + assert_eq!( + uids(dir.path(), "work").len(), + 1, + "{:?}", + uids(dir.path(), "work") + ); + } +}