From f926d098394cc7b4a69f30851f18e9ce699b76a7 Mon Sep 17 00:00:00 2001 From: James Maes Date: Wed, 29 Jul 2026 17:19:15 -0500 Subject: [PATCH 1/3] feat(calendar): add Calendar gRPC vertical (proto, daemon, CLI, E2E) (#214) Wire the orphaned nuncio-cal engine into nunciod as a real, authenticated gRPC vertical mirroring Mail: add service Calendar (Sync/ListEvents/GetEvent) to nuncio.proto with a regenerated descriptor golden, connect_calendar in nuncio-proto's client, calendar_sync::sync_with_backend plus CalendarEngineOverrides/CalendarGrpcService mounted behind its own BearerAuthInterceptor in nunciod::grpc, and real `cal sync`/`cal list` gRPC clients in nuncio-cli replacing the fabricated CLI output. ListEvents/GetEvent always read genuinely persisted data; production Sync returns an honest error since there is no persisted per-account CalDAV configuration yet, and only an injected MockCalendarBackend exercises a working Sync. Adds unit, daemon integration, and offline full-daemon E2E tests, including an unauthenticated-call-rejected assertion for the new service. Refs #185 Co-authored-by: James Maes --- crates/nuncio-cli/src/args.rs | 49 +- crates/nuncio-cli/src/runner.rs | 348 +++++++++-- crates/nuncio-cli/tests/cli_system_test.rs | 56 +- .../proto/nuncio/v1/descriptor.bin | Bin 7726 -> 8772 bytes .../nuncio-proto/proto/nuncio/v1/nuncio.proto | 91 +++ crates/nuncio-proto/src/client.rs | 43 ++ crates/nunciod/src/calendar_sync.rs | 147 +++++ crates/nunciod/src/grpc.rs | 543 +++++++++++++++++- crates/nunciod/src/lib.rs | 1 + crates/nunciod/tests/calendar_e2e_test.rs | 193 +++++++ crates/nunciod/tests/spine_e2e_test.rs | 5 +- 11 files changed, 1408 insertions(+), 68 deletions(-) create mode 100644 crates/nunciod/src/calendar_sync.rs create mode 100644 crates/nunciod/tests/calendar_e2e_test.rs diff --git a/crates/nuncio-cli/src/args.rs b/crates/nuncio-cli/src/args.rs index 5ddd11d..8cb0153 100644 --- a/crates/nuncio-cli/src/args.rs +++ b/crates/nuncio-cli/src/args.rs @@ -335,13 +335,56 @@ pub enum FolderSubcommand { } /// Calendar subcommands (`nuncio cal `). +/// +/// `account`/`calendar` currently have no persisted CalDAV account/collection +/// concept to look up defaults from (see `nunciod::calendar_sync`'s doc +/// comment), so both are plain flags rather than resolved from configured +/// account state; `start`/`end` default to a window spanning "everything" +/// (`0` .. `i64::MAX` unix seconds) so `list`/`sync` are usable with no flags +/// at all once an account/calendar id is supplied. #[derive(Subcommand, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub enum CalSubcommand { - /// List upcoming calendar events. - List, + /// List calendar events for an account within an optional time window. + List { + /// Account identifier owning the calendar. + #[arg(short, long, help = "Account identifier")] + account: String, + /// Calendar collection identifier. + #[arg( + short, + long, + default_value = "default", + help = "Calendar collection identifier" + )] + calendar: String, + /// Window start (unix seconds, inclusive). + #[arg(long, default_value_t = 0, help = "Window start (unix seconds)")] + start: i64, + /// Window end (unix seconds, inclusive). + #[arg(long, default_value_t = i64::MAX, help = "Window end (unix seconds)")] + end: i64, + }, /// Synchronize local calendar cache with remote CalDAV server. - Sync, + Sync { + /// Account identifier owning the calendar. + #[arg(short, long, help = "Account identifier")] + account: String, + /// Calendar collection identifier. + #[arg( + short, + long, + default_value = "default", + help = "Calendar collection identifier" + )] + calendar: String, + /// Window start (unix seconds, inclusive). + #[arg(long, default_value_t = 0, help = "Window start (unix seconds)")] + start: i64, + /// Window end (unix seconds, inclusive). + #[arg(long, default_value_t = i64::MAX, help = "Window end (unix seconds)")] + end: i64, + }, } /// System subcommands (`nuncio system `). diff --git a/crates/nuncio-cli/src/runner.rs b/crates/nuncio-cli/src/runner.rs index d9ad7f1..1339c18 100644 --- a/crates/nuncio-cli/src/runner.rs +++ b/crates/nuncio-cli/src/runner.rs @@ -1,6 +1,6 @@ //! Headless engine runner executing CLI commands against core services. -use nuncio_core::{CoreCommand, EventBus}; +use nuncio_core::EventBus; use nuncio_store::vault::{SecretManager, GRPC_TOKEN_ACCOUNT}; use nuncio_store::{DatabaseEngine, DatabaseError}; use serde_json::json; @@ -57,6 +57,22 @@ fn message_proto_to_json(message: &nuncio_proto::v1::Message) -> serde_json::Val }) } +/// Renders a `nuncio.v1.CalendarEvent` (as returned by the daemon's +/// `Calendar` gRPC service) into the JSON shape used by `cal list`'s +/// `--json` output. +fn calendar_event_proto_to_json(event: &nuncio_proto::v1::CalendarEvent) -> serde_json::Value { + json!({ + "id": event.id, + "account_id": event.account_id, + "calendar_id": event.calendar_id, + "summary": event.summary, + "start_time": event.start_time, + "end_time": event.end_time, + "rrule": event.rrule, + "location": event.location, + }) +} + /// Maps a `nuncio_core::export::ExportFormat` onto its wire-format /// `nuncio.v1.ExportFormat` enum value, mirroring `nunciod::grpc`'s /// server-side mapping. @@ -348,20 +364,23 @@ impl HeadlessRunner { FolderSubcommand::List => self.handle_folders_list(json_mode).await, }, Commands::Cal { action } => match action { - CalSubcommand::List => { - if json_mode { - format_json(&json!({ "events": [] })) - } else { - "Calendar Events: 0 events found".to_string() - } + CalSubcommand::List { + account, + calendar, + start, + end, + } => { + self.handle_cal_list(account, calendar, *start, *end, json_mode) + .await } - CalSubcommand::Sync => { - self.event_bus.process_command(CoreCommand::SyncAll); - if json_mode { - format_json(&json!({ "status": "calendar_sync_started" })) - } else { - "Calendar synchronization started.".to_string() - } + CalSubcommand::Sync { + account, + calendar, + start, + end, + } => { + self.handle_cal_sync(account, calendar, *start, *end, json_mode) + .await } }, Commands::System { action } => match action { @@ -1585,6 +1604,115 @@ impl HeadlessRunner { .map_err(|e| format!("nunciod daemon unreachable at {}: {e}", self.grpc_addr)) } + /// Resolves the gRPC bearer token from the injected vault and dials the + /// `nuncio.v1.Calendar` service at `self.grpc_addr`, shared by + /// [`Self::handle_cal_list`] and [`Self::handle_cal_sync`]. + async fn connect_calendar_client( + &self, + ) -> Result { + let token_bytes = self + .secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; + let token = hex::encode(token_bytes); + + nuncio_proto::client::connect_calendar(&self.grpc_addr, &token) + .await + .map_err(|e| format!("nunciod daemon unreachable at {}: {e}", self.grpc_addr)) + } + + /// `cal list`: a real thin gRPC client of the running `nunciod` daemon's + /// `nuncio.v1.Calendar/ListEvents` API. Returns only genuinely persisted + /// events from the daemon's real store -- never a fabricated + /// "0 events found". + async fn handle_cal_list( + &self, + account: &str, + calendar: &str, + start: i64, + end: i64, + json_mode: bool, + ) -> String { + let mut client = match self.connect_calendar_client().await { + Ok(client) => client, + Err(e) => return Self::render_error(&e, json_mode), + }; + + match client + .list_events(nuncio_proto::v1::ListEventsRequest { + account_id: account.to_string(), + calendar_id: calendar.to_string(), + start_window: start, + end_window: end, + }) + .await + { + Ok(response) => { + let events = response.into_inner().events; + if json_mode { + let events_json: Vec = + events.iter().map(calendar_event_proto_to_json).collect(); + format_json(&json!({ + "account": account, + "calendar": calendar, + "events": events_json + })) + } else { + format!("Calendar Events: {} event(s) found", events.len()) + } + } + Err(status) => Self::render_error( + &format!("nunciod daemon rejected list_events: {status}"), + json_mode, + ), + } + } + + /// `cal sync`: a real thin gRPC client of the running `nunciod` daemon's + /// `nuncio.v1.Calendar/Sync` API. With no persisted per-account CalDAV + /// configuration, the daemon's production `Sync` path returns an honest + /// error -- this NEVER prints a fabricated "Calendar synchronization + /// started" the way this command used to. + async fn handle_cal_sync( + &self, + account: &str, + calendar: &str, + start: i64, + end: i64, + json_mode: bool, + ) -> String { + let mut client = match self.connect_calendar_client().await { + Ok(client) => client, + Err(e) => return Self::render_error(&e, json_mode), + }; + + match client + .sync(nuncio_proto::v1::CalendarSyncRequest { + account_id: account.to_string(), + calendar_id: calendar.to_string(), + start_window: start, + end_window: end, + }) + .await + { + Ok(response) => { + let synced_count = response.into_inner().synced_count; + if json_mode { + format_json(&json!({ + "status": "calendar_sync_complete", + "synced_count": synced_count, + })) + } else { + format!("Calendar synchronization complete: {synced_count} event(s) synced") + } + } + Err(status) => Self::render_error( + &format!("nunciod daemon rejected calendar sync: {status}"), + json_mode, + ), + } + } + /// Formats an error message consistently for both JSON and human-text /// output modes. fn render_error(message: &str, json_mode: bool) -> String { @@ -1781,26 +1909,14 @@ mod tests { // `SecretManager::mock()` against a live stub `Mail` gRPC server // (`mail_rpcs_round_trip_over_grpc_to_a_stub_daemon`). - // Cal Noun Commands - let cal_list = runner - .execute_command( - &Commands::Cal { - action: CalSubcommand::List, - }, - true, - ) - .await; - assert!(cal_list.contains(r#""events":[]"#)); - - let cal_sync = runner - .execute_command( - &Commands::Cal { - action: CalSubcommand::Sync, - }, - true, - ) - .await; - assert!(cal_sync.contains(r#""status":"calendar_sync_started""#)); + // Cal Noun Commands: `List`/`Sync` are real gRPC clients of the + // `nunciod` daemon's `Calendar` API -- exactly like the Mail Noun + // Commands above, they must never run against this + // `ephemeral()`-constructed runner's production `SecretManager` or + // its (unreachable in CI) default gRPC address. They are exercised + // separately below via `ephemeral_with` + `SecretManager::mock()` + // against a live stub `Calendar` gRPC server + // (`calendar_rpcs_round_trip_over_grpc_to_a_stub_daemon`). // System Noun Commands are exercised separately below via // `ephemeral_with` + `SecretManager::mock()`: `system status` is a @@ -2426,6 +2542,170 @@ mod tests { assert_eq!(recorded_send.body_text, "Let's discuss the roadmap."); } + /// Reference-client proof: boots a stub `nuncio.v1.Calendar` gRPC server + /// (mirroring the `Mail` stub pattern above) and drives the real + /// `HeadlessRunner`'s `cal list`/`cal sync` gRPC client paths against it. + /// + /// Real persistence and the honest-error-when-no-backend-injected + /// behavior are proven by `nunciod`'s own `grpc::tests`; this test exists + /// purely to prove the CLI's connect + call + JSON-format happy path, + /// and that the exact account/calendar/window it was given reaches the + /// daemon over the wire. + #[tokio::test] + async fn calendar_rpcs_round_trip_over_grpc_to_a_stub_daemon() { + use nuncio_proto::v1::calendar_server::{Calendar as CalendarService, CalendarServer}; + use nuncio_proto::v1::{ + CalendarEvent as CalendarEventProto, CalendarSyncRequest, CalendarSyncResponse, + GetEventRequest, GetEventResponse, ListEventsRequest, ListEventsResponse, + }; + use std::sync::Mutex; + + fn stub_event() -> CalendarEventProto { + CalendarEventProto { + id: "evt-stub-1".to_string(), + account_id: "acct-stub-1".to_string(), + calendar_id: "cal-stub".to_string(), + summary: "Stub Meeting".to_string(), + start_time: 1_700_000_000, + end_time: 1_700_003_600, + rrule: None, + location: Some("Room 1".to_string()), + } + } + + /// Minimal test-only stub of `nuncio.v1.Calendar`: records the last + /// `ListEventsRequest`/`CalendarSyncRequest` it received (so this + /// test can assert on exactly what the CLI sent over the wire) and + /// otherwise returns fixed responses. + #[derive(Default)] + struct StubCalendar { + last_list_events: Arc>>, + last_sync: Arc>>, + } + + #[tonic::async_trait] + impl CalendarService for StubCalendar { + async fn sync( + &self, + request: tonic::Request, + ) -> Result, tonic::Status> { + let req = request.into_inner(); + *self.last_sync.lock().unwrap_or_else(|e| e.into_inner()) = Some(req); + Ok(tonic::Response::new(CalendarSyncResponse { + synced_count: 2, + })) + } + + async fn list_events( + &self, + request: tonic::Request, + ) -> Result, tonic::Status> { + let req = request.into_inner(); + *self + .last_list_events + .lock() + .unwrap_or_else(|e| e.into_inner()) = Some(req); + Ok(tonic::Response::new(ListEventsResponse { + events: vec![stub_event()], + })) + } + + async fn get_event( + &self, + request: tonic::Request, + ) -> Result, tonic::Status> { + let req = request.into_inner(); + if req.event_id == "evt-stub-1" { + Ok(tonic::Response::new(GetEventResponse { + event: Some(stub_event()), + })) + } else { + Err(tonic::Status::not_found(format!( + "event '{}' not found", + req.event_id + ))) + } + } + } + + let stub = StubCalendar::default(); + let list_probe = stub.last_list_events.clone(); + let sync_probe = stub.last_sync.clone(); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind ephemeral loopback port"); + let addr = listener.local_addr().expect("listener has local addr"); + tokio::spawn(async move { + let _ = tonic::transport::Server::builder() + .add_service(CalendarServer::new(stub)) + .serve_with_incoming(tokio_stream::wrappers::TcpListenerStream::new(listener)) + .await; + }); + + let runner = + HeadlessRunner::ephemeral_with(Arc::new(SecretManager::mock()), addr.to_string()) + .await + .expect("ephemeral runner initializes"); + + // `cal list`: proves the CLI is a real gRPC client of + // `Calendar/ListEvents` -- the exact account/calendar/window + // supplied reaches the daemon over the wire, and the daemon's real + // returned events (NOT a fabricated "0 events found") appear in the + // CLI's output. + let cal_list = runner + .execute_command( + &Commands::Cal { + action: CalSubcommand::List { + account: "acct-stub-1".to_string(), + calendar: "cal-stub".to_string(), + start: 0, + end: i64::MAX, + }, + }, + true, + ) + .await; + assert!(cal_list.contains("evt-stub-1")); + assert!(cal_list.contains("Stub Meeting")); + let recorded_list = list_probe + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone() + .expect("stub daemon received a list_events request"); + assert_eq!(recorded_list.account_id, "acct-stub-1"); + assert_eq!(recorded_list.calendar_id, "cal-stub"); + assert_eq!(recorded_list.start_window, 0); + assert_eq!(recorded_list.end_window, i64::MAX); + + // `cal sync`: proves the CLI is a real gRPC client of + // `Calendar/Sync` and reports the daemon's real `synced_count`, + // rather than fabricating a "calendar_sync_started" status by only + // flipping this runner's own throwaway local `EventBus`. + let cal_sync = runner + .execute_command( + &Commands::Cal { + action: CalSubcommand::Sync { + account: "acct-stub-1".to_string(), + calendar: "cal-stub".to_string(), + start: 0, + end: i64::MAX, + }, + }, + true, + ) + .await; + assert!(cal_sync.contains(r#""status":"calendar_sync_complete""#)); + assert!(cal_sync.contains(r#""synced_count":2"#)); + let recorded_sync = sync_probe + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone() + .expect("stub daemon received a sync request"); + assert_eq!(recorded_sync.account_id, "acct-stub-1"); + assert_eq!(recorded_sync.calendar_id, "cal-stub"); + } + /// Reference-client proof: boots a stub `nuncio.v1.Filters` gRPC server /// (mirroring the `Accounts`/`Mail` stub pattern above) and drives the /// real `HeadlessRunner`'s diff --git a/crates/nuncio-cli/tests/cli_system_test.rs b/crates/nuncio-cli/tests/cli_system_test.rs index 6246cef..a691156 100644 --- a/crates/nuncio-cli/tests/cli_system_test.rs +++ b/crates/nuncio-cli/tests/cli_system_test.rs @@ -42,17 +42,11 @@ async fn system_test_cli_noun_verb_execution_matrix() { // separately below via `ephemeral_with` + `SecretManager::mock()` // rather than through this `ephemeral()`-constructed runner. - // 5. Calendar list & sync - let out: String = runner - .execute_command( - &Commands::Cal { - action: CalSubcommand::List, - }, - true, - ) - .await; - let json: Value = serde_json::from_str(&out).expect("valid json"); - assert_eq!(json["status"], "ok"); + // 5. Calendar list & sync are real gRPC clients of the `nunciod` + // daemon's `Calendar` API, so -- exactly like Folder/Mail above -- they + // are exercised separately below via `ephemeral_with` + + // `SecretManager::mock()` rather than through this + // `ephemeral()`-constructed runner. } /// `system status` is a real gRPC client of the `nunciod` daemon. With no @@ -144,10 +138,10 @@ async fn account_add_and_list_report_honest_errors_when_daemon_unreachable() { assert!(!add_out.contains("irrelevant-unreachable-daemon")); } -/// `folder list`, `mail list`, `mail read`, `mail search`, and `mail mark` -/// are real gRPC clients of the `nunciod` daemon's `Mail` API. With no -/// daemon reachable, all of them must report a clear, honest error rather -/// than fabricating an empty result. +/// `folder list`, `mail list`, `mail read`, `mail search`, `mail mark`, and +/// `cal list`/`cal sync` are real gRPC clients of the `nunciod` daemon's +/// `Mail`/`Calendar` APIs. With no daemon reachable, all of them must report +/// a clear, honest error rather than fabricating an empty result. #[tokio::test] async fn mail_and_folder_report_honest_errors_when_daemon_unreachable() { let listener = tokio::net::TcpListener::bind("127.0.0.1:0") @@ -233,4 +227,36 @@ async fn mail_and_folder_report_honest_errors_when_daemon_unreachable() { ) .await, ); + + assert_honest_error( + runner + .execute_command( + &Commands::Cal { + action: CalSubcommand::List { + account: "acct-1".to_string(), + calendar: "default".to_string(), + start: 0, + end: i64::MAX, + }, + }, + true, + ) + .await, + ); + + assert_honest_error( + runner + .execute_command( + &Commands::Cal { + action: CalSubcommand::Sync { + account: "acct-1".to_string(), + calendar: "default".to_string(), + start: 0, + end: i64::MAX, + }, + }, + true, + ) + .await, + ); } diff --git a/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin b/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin index ef5989bbd7c82c2b327b3e4c4bda1f8179eb3e9f..5a4b5ad575acfc32c30137c1e29cb8ad29503650 100644 GIT binary patch delta 978 zcmb_ayH3ME5X3kSdjop_35l&_~@cHiw`;=9qoB_?#ZXKtmd zQ1T?$4SY*q)9R)J3hMaCf?}NX`)+h)@BH$6eT@u6;odFXv)dnH=2f4!RUpaOM{SDhF~@46Jdg;0fnyk-7JCGRa*N_ zg9R1uzDRIPxl7mc!h}$sPvaFGQz9KdW&I`wFf9b*16YTdEao5}emD>#(w&mP z&IH7J@TgM2$HXSVkc_r>p)%<#J`YKZ5vx43=ho6&4a&zsOvP5>zZw5W-|3IW4*`w) zlD4;?fcgEzY?^w#v|}|$LcAbsTy8g1P7%eCI(q}=+Bl1+((&uyMc7hK>; +/// The authenticated `nuncio.v1.Calendar` client type returned by +/// [`connect_calendar`]. +pub type AuthenticatedCalendarClient = + CalendarClient>; + /// Dials `addr` (a `host:port` pair, e.g. `127.0.0.1:9420`) over plain HTTP /// (the loopback gRPC transport is never TLS-wrapped; auth is via bearer /// token instead) and returns the connected [`Channel`], shared by every @@ -238,6 +244,26 @@ pub async fn connect_audit( Ok(AuditClient::with_interceptor(channel, interceptor)) } +/// Dials the `nuncio.v1.Calendar` gRPC endpoint at `addr` and returns a +/// client that injects `authorization: Bearer ` metadata on every +/// call. +/// +/// `Calendar` is guarded by the exact same `BearerAuthInterceptor` as every +/// other service on the server side (see +/// `nunciod::grpc::serve_on_listener_with_overrides`), so this shares +/// [`BearerTokenInterceptor`] and [`dial`] with [`connect_system`] / +/// [`connect_accounts`] / [`connect_mail`] / [`connect_filters`] / +/// [`connect_export`] / [`connect_audit`] rather than hand-rolling yet +/// another auth handshake. +pub async fn connect_calendar( + addr: &str, + token: &str, +) -> Result { + let interceptor = BearerTokenInterceptor::new(token)?; + let channel = dial(addr).await?; + Ok(CalendarClient::with_interceptor(channel, interceptor)) +} + /// Opens the `nuncio.v1.System/Subscribe` server-streaming RPC on an already /// authenticated client (as returned by [`connect_system`]), reusing the /// bearer-token interceptor already attached to `client` rather than @@ -396,6 +422,23 @@ mod tests { assert!(err.to_string().contains("127.0.0.1:1")); } + #[tokio::test] + async fn connect_calendar_fails_closed_on_invalid_token() { + let err = connect_calendar("127.0.0.1:0", "tok\ntoken") + .await + .expect_err("invalid token must fail before dialing"); + assert!(matches!(err, ConnectError::InvalidToken(_))); + } + + #[tokio::test] + async fn connect_calendar_reports_transport_error_when_daemon_unreachable() { + let err = connect_calendar("127.0.0.1:1", "abc123") + .await + .expect_err("connecting to an unreachable daemon must fail"); + assert!(matches!(err, ConnectError::Transport { .. })); + assert!(err.to_string().contains("127.0.0.1:1")); + } + #[tokio::test] async fn connect_system_reports_invalid_endpoint_for_a_malformed_address() { // An embedded space is not a legal URI authority character, so diff --git a/crates/nunciod/src/calendar_sync.rs b/crates/nunciod/src/calendar_sync.rs new file mode 100644 index 0000000..29618ac --- /dev/null +++ b/crates/nunciod/src/calendar_sync.rs @@ -0,0 +1,147 @@ +//! Real inbound calendar synchronization routine. +//! +//! Mirrors `nunciod::sync`'s shape for the calendar engine: [`sync_with_backend`] +//! fetches events from a [`CalendarBackend`] for a single calendar collection and +//! time window, then persists each event via [`DatabaseEngine::save_calendar_event`]. +//! +//! # No production entry point yet +//! +//! Unlike `sync.rs`'s `run_account_sync`/`run_all_accounts_sync`, this module +//! deliberately has NO production entry point that resolves a real account's +//! CalDAV collection URL and credential and builds a real `CalDavClient` from +//! them: `nuncio_core::AccountConfig` carries no CalDAV fields today, so there +//! is nothing persisted to build one from. [`sync_with_backend`] is the only +//! function here; it is exercised directly in tests with `MockCalendarBackend`, +//! and it is the same seam a future story's production entry point will call +//! once per-account CalDAV configuration exists. +use nuncio_cal::CalendarBackend; +use nuncio_store::db::DatabaseError; +use thiserror::Error; + +/// Errors that can occur while synchronizing a single calendar collection's events. +#[derive(Debug, Error)] +pub enum CalendarSyncError { + /// The calendar backend (CalDAV/mock) reported an error. + #[error("calendar backend error: {0}")] + Backend(#[from] nuncio_cal::CalendarError), + /// A database persistence error occurred while saving synced events. + #[error("database error: {0}")] + Store(#[from] DatabaseError), +} + +/// Fetch every event in `calendar_id` whose window overlaps `[start_window, +/// end_window]` from `backend`, persisting each via +/// [`DatabaseEngine::save_calendar_event`]. Returns the number of events +/// processed. +/// +/// Pure fetch-and-persist, unit-testable directly with a +/// [`nuncio_cal::MockCalendarBackend`] -- no event-bus or credential logic, +/// mirroring `nunciod::sync::fetch_and_persist`. +pub async fn sync_with_backend( + db: &nuncio_store::db::DatabaseEngine, + backend: &dyn CalendarBackend, + calendar_id: &str, + start_window: i64, + end_window: i64, +) -> Result { + let events = backend + .fetch_events(calendar_id, start_window, end_window) + .await?; + let mut synced = 0usize; + for event in events { + db.save_calendar_event(&event).await?; + synced += 1; + } + Ok(synced) +} + +#[cfg(test)] +mod tests { + use super::*; + use nuncio_cal::MockCalendarBackend; + use nuncio_core::model::CalendarEvent; + use nuncio_store::db::DatabaseEngine; + + fn mock_event(id: &str, calendar_id: &str, start: i64, end: i64) -> CalendarEvent { + CalendarEvent { + id: id.to_string(), + account_id: "acct-cal-1".to_string(), + calendar_id: calendar_id.to_string(), + summary: format!("Event {id}"), + start_time: start, + end_time: end, + rrule: None, + location: None, + } + } + + #[tokio::test] + async fn sync_with_backend_persists_mock_events() { + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("ephemeral db"); + + let mock = MockCalendarBackend::new(); + mock.add_event(mock_event( + "evt-1", + "cal-work", + 1_700_000_000, + 1_700_003_600, + )); + mock.add_event(mock_event( + "evt-2", + "cal-work", + 1_700_010_000, + 1_700_013_600, + )); + // Outside the requested window entirely -- must not be returned or persisted. + mock.add_event(mock_event( + "evt-out-of-window", + "cal-work", + 1_800_000_000, + 1_800_003_600, + )); + + let synced = sync_with_backend(&db, &mock, "cal-work", 1_699_999_000, 1_700_020_000) + .await + .expect("sync succeeds"); + assert_eq!(synced, 2); + + let persisted = db + .list_calendar_events("acct-cal-1", 1_699_999_000, 1_700_020_000) + .await + .expect("list persisted events"); + assert_eq!(persisted.len(), 2); + let ids: Vec<&str> = persisted.iter().map(|e| e.id.as_str()).collect(); + assert!(ids.contains(&"evt-1")); + assert!(ids.contains(&"evt-2")); + } + + #[tokio::test] + async fn sync_with_backend_returns_zero_for_empty_backend() { + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("ephemeral db"); + let mock = MockCalendarBackend::new(); + + let synced = sync_with_backend(&db, &mock, "cal-empty", 0, i64::MAX) + .await + .expect("sync succeeds"); + assert_eq!(synced, 0); + } + + #[tokio::test] + async fn sync_with_backend_surfaces_backend_failure() { + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("ephemeral db"); + let mock = MockCalendarBackend::new(); + mock.set_should_fail(true); + + let err = sync_with_backend(&db, &mock, "cal-work", 0, i64::MAX) + .await + .expect_err("sync fails"); + assert!(matches!(err, CalendarSyncError::Backend(_))); + assert!(err.to_string().contains("calendar backend error")); + } +} diff --git a/crates/nunciod/src/grpc.rs b/crates/nunciod/src/grpc.rs index a9a7a64..a8a2654 100644 --- a/crates/nunciod/src/grpc.rs +++ b/crates/nunciod/src/grpc.rs @@ -5,11 +5,13 @@ //! //! This is the daemon's sole client-facing transport. +use nuncio_cal::CalendarBackend; use nuncio_core::{CoreCommand, CoreEvent, EventBus}; use nuncio_filter::{FilterEngine, NsqlParser, NsqlValidator, ValidationOptions}; use nuncio_mail::{MailBackend, MessageSender}; use nuncio_proto::v1::accounts_server::{Accounts, AccountsServer}; use nuncio_proto::v1::audit_server::{Audit, AuditServer}; +use nuncio_proto::v1::calendar_server::{Calendar, CalendarServer}; use nuncio_proto::v1::event::Kind; use nuncio_proto::v1::export_server::{Export, ExportServer}; use nuncio_proto::v1::filters_server::{Filters, FiltersServer}; @@ -18,11 +20,13 @@ use nuncio_proto::v1::system_server::{System, SystemServer}; use nuncio_proto::v1::{ export_request, AccountConfig as AccountConfigProto, AccountProtocol as AccountProtocolProto, AddAccountRequest, AddAccountResponse, Attachment as AttachmentProto, - AuditRecord as AuditRecordProto, BatchFilterProgress, CreateRuleRequest, CreateRuleResponse, + AuditRecord as AuditRecordProto, BatchFilterProgress, CalendarEvent as CalendarEventProto, + CalendarSyncRequest, CalendarSyncResponse, CreateRuleRequest, CreateRuleResponse, DatabaseRecovered, DeleteRuleRequest, DeleteRuleResponse, Event, EventError, ExportFormat as ExportFormatProto, ExportRequest, ExportResponse, FilterExecuted, - FilterRule as FilterRuleProto, Folder as FolderProto, GetMessageRequest, GetMessageResponse, - GetStatusRequest, GetStatusResponse, ListAccountsRequest, ListAccountsResponse, + FilterRule as FilterRuleProto, Folder as FolderProto, GetEventRequest, GetEventResponse, + GetMessageRequest, GetMessageResponse, GetStatusRequest, GetStatusResponse, + ListAccountsRequest, ListAccountsResponse, ListEventsRequest, ListEventsResponse, ListFoldersRequest, ListFoldersResponse, ListMessagesRequest, ListMessagesResponse, ListRecordsRequest, ListRecordsResponse, ListRulesRequest, ListRulesResponse, MarkReadRequest, MarkReadResponse, Message as MessageProto, MessageFlagsChanged, MessageSearchHit, @@ -418,6 +422,141 @@ fn map_folder_to_proto(folder: nuncio_core::model::Folder) -> FolderProto { } } +/// Maps a `nuncio_core::model::CalendarEvent` onto its wire-format +/// `nuncio.v1.CalendarEvent` representation. Field-for-field, no +/// decryption/derivation to do here -- calendar events are never encrypted +/// at rest (see `DatabaseEngine::save_calendar_event`'s doc comment). +fn map_calendar_event_to_proto(event: nuncio_core::model::CalendarEvent) -> CalendarEventProto { + CalendarEventProto { + id: event.id, + account_id: event.account_id, + calendar_id: event.calendar_id, + summary: event.summary, + start_time: event.start_time, + end_time: event.end_time, + rrule: event.rrule, + location: event.location, + } +} + +/// Test-only injection point for the daemon's calendar sync backend, +/// mirroring [`MailEngineOverrides`]'s shape. +/// +/// Production (every existing caller of [`serve`] / [`serve_on_listener`]) +/// never constructs a non-default instance: [`Default`] yields +/// `calendar_backend: None`, which routes `Calendar/Sync` to an honest +/// error, since there is currently no persisted per-account CalDAV +/// configuration to build a real backend from. Only a full-daemon E2E test +/// supplies `Some(..)`, standing in a [`nuncio_cal::MockCalendarBackend`] +/// for real network I/O. +#[derive(Clone, Default)] +pub struct CalendarEngineOverrides { + /// When `Some`, `Calendar/Sync` fetches from this backend instead of + /// returning an honest "no CalDAV configuration" error. + pub calendar_backend: Option>, +} + +impl std::fmt::Debug for CalendarEngineOverrides { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CalendarEngineOverrides") + .field("calendar_backend", &self.calendar_backend.is_some()) + .finish() + } +} + +/// `nuncio.v1.Calendar` gRPC service implementation backed by the daemon's +/// live [`DatabaseEngine`] calendar-event read/write methods. +/// +/// `ListEvents`/`GetEvent` always read genuinely persisted data. `Sync` +/// fetches from [`CalendarEngineOverrides::calendar_backend`] when injected; +/// otherwise it returns an honest error, since there is no persisted +/// per-account CalDAV configuration to build a real backend from yet (see +/// `nunciod::calendar_sync`'s doc comment). +struct CalendarGrpcService { + db: Arc, + overrides: CalendarEngineOverrides, +} + +#[tonic::async_trait] +impl Calendar for CalendarGrpcService { + async fn sync( + &self, + request: Request, + ) -> Result, Status> { + let req = request.into_inner(); + + let synced = match &self.overrides.calendar_backend { + Some(backend) => crate::calendar_sync::sync_with_backend( + &self.db, + backend.as_ref(), + &req.calendar_id, + req.start_window, + req.end_window, + ) + .await + .map_err(|e| Status::internal(format!("calendar sync failed: {e}")))?, + None => { + return Err(Status::internal(format!( + "no CalDAV configuration exists for account '{}'; per-account CalDAV \ + configuration is not yet implemented", + req.account_id + ))); + } + }; + + Ok(Response::new(CalendarSyncResponse { + synced_count: synced as u64, + })) + } + + async fn list_events( + &self, + request: Request, + ) -> Result, Status> { + let req = request.into_inner(); + if req.account_id.is_empty() { + return Err(Status::invalid_argument("account_id is required")); + } + // `DatabaseEngine::list_calendar_events` filters by account_id and + // time window only -- there is no per-calendar-collection filter in + // the store today, so `req.calendar_id` is not applied here. This + // never fabricates data; it genuinely returns every persisted event + // for the account in the requested window, same as `req.calendar_id` + // being absent from the query would. + + let events = self + .db + .list_calendar_events(&req.account_id, req.start_window, req.end_window) + .await + .map_err(|e| Status::internal(format!("failed to list calendar events: {e}")))? + .into_iter() + .map(map_calendar_event_to_proto) + .collect(); + + Ok(Response::new(ListEventsResponse { events })) + } + + async fn get_event( + &self, + request: Request, + ) -> Result, Status> { + let req = request.into_inner(); + if req.event_id.is_empty() { + return Err(Status::invalid_argument("event_id is required")); + } + + let event = self + .db + .get_calendar_event(&req.event_id) + .await + .map_err(|e| Status::not_found(format!("event '{}' not found: {e}", req.event_id)))?; + + Ok(Response::new(GetEventResponse { + event: Some(map_calendar_event_to_proto(event)), + })) + } +} + /// Test-only injection point for the daemon's inbound sync and outbound /// send engines, threaded through the daemon's runtime wiring for the /// full-daemon offline spine E2E test. @@ -1104,8 +1243,9 @@ impl tonic::service::Interceptor for BearerAuthInterceptor { /// Binds a loopback TCP listener at `addr` and serves the `nuncio.v1.System`, /// `nuncio.v1.Accounts`, `nuncio.v1.Mail`, `nuncio.v1.Filters`, -/// `nuncio.v1.Export`, and `nuncio.v1.Audit` gRPC services on it, all -/// authenticated by `token`, until the transport server errors. +/// `nuncio.v1.Export`, `nuncio.v1.Audit`, and `nuncio.v1.Calendar` gRPC +/// services on it, all authenticated by `token`, until the transport server +/// errors. /// /// `addr` MUST be a loopback address (e.g. `127.0.0.1:PORT`); callers are /// responsible for passing loopback-only addresses (see @@ -1128,8 +1268,8 @@ pub async fn serve( } /// Serves the `nuncio.v1.System`, `nuncio.v1.Accounts`, `nuncio.v1.Mail`, -/// `nuncio.v1.Filters`, `nuncio.v1.Export`, and `nuncio.v1.Audit` gRPC -/// services on an already-bound [`TcpListener`]. +/// `nuncio.v1.Filters`, `nuncio.v1.Export`, `nuncio.v1.Audit`, and +/// `nuncio.v1.Calendar` gRPC services on an already-bound [`TcpListener`]. /// /// # Security /// @@ -1161,17 +1301,21 @@ pub async fn serve_on_listener( secrets, token, MailEngineOverrides::default(), + CalendarEngineOverrides::default(), ) .await } /// Identical to [`serve_on_listener`], except the `nuncio.v1.Mail` service's -/// inbound sync and outbound send engines can be overridden. -/// [`serve_on_listener`] is simply this function called -/// with `MailEngineOverrides::default()` (i.e. every field `None`, which is -/// production's exact prior behavior); this function exists so a -/// full-daemon offline E2E test can supply `Some(..)` without changing -/// [`serve_on_listener`]'s signature for its many existing callers. +/// inbound sync and outbound send engines, and the `nuncio.v1.Calendar` +/// service's sync backend, can be overridden. +/// [`serve_on_listener`] is simply this function called with +/// `MailEngineOverrides::default()` / `CalendarEngineOverrides::default()` +/// (i.e. every field `None`, which is production's exact prior behavior); +/// this function exists so a full-daemon offline E2E test can supply +/// `Some(..)` without changing [`serve_on_listener`]'s signature for its many +/// existing callers. +#[allow(clippy::too_many_arguments)] pub async fn serve_on_listener_with_overrides( listener: TcpListener, event_bus: Arc, @@ -1180,6 +1324,7 @@ pub async fn serve_on_listener_with_overrides( secrets: Arc, token: impl Into>, overrides: MailEngineOverrides, + calendar_overrides: CalendarEngineOverrides, ) -> Result<(), GrpcServeError> { let token: Arc = token.into(); @@ -1228,10 +1373,20 @@ pub async fn serve_on_listener_with_overrides( // Audit: mounted behind its own `BearerAuthInterceptor`, exactly like // every other service above -- see the hard invariant documented on // this function's doc comment. - let audit_service = AuditGrpcService { db }; - let audit_interceptor = BearerAuthInterceptor::new(token); + let audit_service = AuditGrpcService { db: db.clone() }; + let audit_interceptor = BearerAuthInterceptor::new(token.clone()); let audit_svc = AuditServer::with_interceptor(audit_service, audit_interceptor); + // Calendar: mounted behind its own `BearerAuthInterceptor`, exactly like + // every other service above -- see the hard invariant documented on + // this function's doc comment. + let calendar_service = CalendarGrpcService { + db, + overrides: calendar_overrides, + }; + let calendar_interceptor = BearerAuthInterceptor::new(token); + let calendar_svc = CalendarServer::with_interceptor(calendar_service, calendar_interceptor); + Server::builder() .add_service(system_svc) .add_service(accounts_svc) @@ -1239,6 +1394,7 @@ pub async fn serve_on_listener_with_overrides( .add_service(filters_svc) .add_service(export_svc) .add_service(audit_svc) + .add_service(calendar_svc) .serve_with_incoming(TcpListenerStream::new(listener)) .await .map_err(GrpcServeError::Transport) @@ -1320,6 +1476,32 @@ mod tests { secrets: Arc, token: &str, overrides: MailEngineOverrides, + ) -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) { + spawn_test_server_with_all_overrides( + event_bus, + db, + filter_engine, + secrets, + token, + overrides, + CalendarEngineOverrides::default(), + ) + .await + } + + /// Spawns a test server on an ephemeral loopback port backed by the + /// given `db`/`filter_engine`/`secrets`/`overrides`/`calendar_overrides`: + /// the helper every test that injects a + /// [`CalendarEngineOverrides::calendar_backend`] uses to prove `Sync` + /// drives an injected test double over the real authenticated gRPC API. + async fn spawn_test_server_with_all_overrides( + event_bus: Arc, + db: Arc, + filter_engine: Arc, + secrets: Arc, + token: &str, + overrides: MailEngineOverrides, + calendar_overrides: CalendarEngineOverrides, ) -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) { let listener = TcpListener::bind("127.0.0.1:0") .await @@ -1335,6 +1517,7 @@ mod tests { secrets, token, overrides, + calendar_overrides, ) .await; }); @@ -2022,6 +2205,52 @@ mod tests { assert_eq!(err.code(), Code::Unauthenticated); } + #[tokio::test] + async fn calendar_rpcs_reject_missing_bearer_token() { + // Confirms `Calendar` is mounted behind its own + // `BearerAuthInterceptor` exactly like every other service (no + // un-intercepted service). + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::{CalendarSyncRequest, GetEventRequest, ListEventsRequest}; + + let event_bus = Arc::new(EventBus::new()); + let (addr, _handle, _dir) = spawn_test_server(event_bus, "correct-token").await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + + let err = client + .list_events(ListEventsRequest { + account_id: "acct-1".to_string(), + calendar_id: "cal-1".to_string(), + start_window: 0, + end_window: i64::MAX, + }) + .await + .expect_err("missing bearer token must be rejected"); + assert_eq!(err.code(), Code::Unauthenticated); + + let err = client + .get_event(GetEventRequest { + event_id: "evt-1".to_string(), + }) + .await + .expect_err("missing bearer token must be rejected"); + assert_eq!(err.code(), Code::Unauthenticated); + + let err = client + .sync(CalendarSyncRequest { + account_id: "acct-1".to_string(), + calendar_id: "cal-1".to_string(), + start_window: 0, + end_window: i64::MAX, + }) + .await + .expect_err("missing bearer token must be rejected"); + assert_eq!(err.code(), Code::Unauthenticated); + } + #[tokio::test] async fn get_message_reports_not_found_for_unknown_message_id() { let event_bus = Arc::new(EventBus::new()); @@ -3395,4 +3624,288 @@ mod tests { assert_eq!(response.record_count, 1); assert_eq!(response.first_broken_seq, Some(1)); } + + // ---- Calendar ---- + + fn sample_calendar_event( + id: &str, + account_id: &str, + calendar_id: &str, + start: i64, + end: i64, + ) -> nuncio_core::model::CalendarEvent { + nuncio_core::model::CalendarEvent { + id: id.to_string(), + account_id: account_id.to_string(), + calendar_id: calendar_id.to_string(), + summary: format!("Summary for {id}"), + start_time: start, + end_time: end, + rrule: None, + location: None, + } + } + + /// `ListEvents` reads only genuinely persisted events via + /// `DatabaseEngine::list_calendar_events` -- never fabricated data. + #[tokio::test] + async fn list_events_returns_only_persisted_events_in_window() { + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::ListEventsRequest; + + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("connect ephemeral test db"); + db.save_calendar_event(&sample_calendar_event( + "evt-in-window", + "acct-cal-1", + "cal-work", + 1_700_000_000, + 1_700_003_600, + )) + .await + .expect("save event in window"); + db.save_calendar_event(&sample_calendar_event( + "evt-out-of-window", + "acct-cal-1", + "cal-work", + 1_800_000_000, + 1_800_003_600, + )) + .await + .expect("save event out of window"); + + let event_bus = Arc::new(EventBus::new()); + let secrets = Arc::new(SecretManager::mock()); + let filter_engine = Arc::new(FilterEngine::new(Vec::new()).expect("empty rule set")); + let (addr, _handle) = spawn_test_server_with( + event_bus, + Arc::new(db), + filter_engine, + secrets, + "correct-token", + ) + .await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + let response = client + .list_events(authed_bearer_request(ListEventsRequest { + account_id: "acct-cal-1".to_string(), + calendar_id: "cal-work".to_string(), + start_window: 1_699_999_000, + end_window: 1_700_004_000, + })) + .await + .expect("list_events succeeds") + .into_inner(); + + assert_eq!(response.events.len(), 1); + assert_eq!(response.events[0].id, "evt-in-window"); + } + + #[tokio::test] + async fn list_events_rejects_empty_account_id() { + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::ListEventsRequest; + + let event_bus = Arc::new(EventBus::new()); + let (addr, _handle, _dir) = spawn_test_server(event_bus, "correct-token").await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + let err = client + .list_events(authed_bearer_request(ListEventsRequest { + account_id: String::new(), + calendar_id: "cal-work".to_string(), + start_window: 0, + end_window: i64::MAX, + })) + .await + .expect_err("empty account_id must be rejected"); + assert_eq!(err.code(), Code::InvalidArgument); + } + + #[tokio::test] + async fn get_event_returns_persisted_event() { + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::GetEventRequest; + + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("connect ephemeral test db"); + db.save_calendar_event(&sample_calendar_event( + "evt-get-1", + "acct-cal-1", + "cal-work", + 1_700_000_000, + 1_700_003_600, + )) + .await + .expect("save event"); + + let event_bus = Arc::new(EventBus::new()); + let secrets = Arc::new(SecretManager::mock()); + let filter_engine = Arc::new(FilterEngine::new(Vec::new()).expect("empty rule set")); + let (addr, _handle) = spawn_test_server_with( + event_bus, + Arc::new(db), + filter_engine, + secrets, + "correct-token", + ) + .await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + let response = client + .get_event(authed_bearer_request(GetEventRequest { + event_id: "evt-get-1".to_string(), + })) + .await + .expect("get_event succeeds") + .into_inner(); + + let event = response.event.expect("event present in response"); + assert_eq!(event.id, "evt-get-1"); + assert_eq!(event.summary, "Summary for evt-get-1"); + } + + #[tokio::test] + async fn get_event_reports_not_found_for_unknown_event_id() { + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::GetEventRequest; + + let event_bus = Arc::new(EventBus::new()); + let (addr, _handle, _dir) = spawn_test_server(event_bus, "correct-token").await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + let err = client + .get_event(authed_bearer_request(GetEventRequest { + event_id: "does-not-exist".to_string(), + })) + .await + .expect_err("unknown event id must be rejected"); + assert_eq!(err.code(), Code::NotFound); + } + + /// With no `CalendarEngineOverrides::calendar_backend` injected, `Sync` + /// returns an honest error -- there is no persisted per-account CalDAV + /// configuration to build a real backend from -- never a fabricated + /// `synced_count`. + #[tokio::test] + async fn calendar_sync_without_override_reports_honest_error() { + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::CalendarSyncRequest; + + let event_bus = Arc::new(EventBus::new()); + let (addr, _handle, _dir) = spawn_test_server(event_bus, "correct-token").await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + let err = client + .sync(authed_bearer_request(CalendarSyncRequest { + account_id: "acct-cal-no-config".to_string(), + calendar_id: "cal-work".to_string(), + start_window: 0, + end_window: i64::MAX, + })) + .await + .expect_err("sync with no injected backend and no CalDAV config must fail"); + assert_eq!(err.code(), Code::Internal); + assert!(err.message().contains("acct-cal-no-config")); + assert!(err.message().contains("no CalDAV configuration")); + } + + /// With a [`CalendarEngineOverrides::calendar_backend`] injected, `Sync` + /// genuinely fetches from it (via `calendar_sync::sync_with_backend`) + /// and awaits full completion before returning, so the synced events are + /// immediately visible to `ListEvents`/`GetEvent` -- exactly the + /// mechanism a full-daemon offline E2E test uses. + #[tokio::test] + async fn calendar_sync_with_injected_backend_persists_and_is_immediately_visible() { + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::{CalendarSyncRequest, GetEventRequest, ListEventsRequest}; + + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("connect ephemeral test db"); + let db = Arc::new(db); + let secrets = Arc::new(SecretManager::mock()); + + let mock_backend = nuncio_cal::MockCalendarBackend::new(); + mock_backend.add_event(sample_calendar_event( + "evt-sync-injected-1", + "acct-cal-injected", + "cal-work", + 1_700_000_000, + 1_700_003_600, + )); + + let calendar_overrides = CalendarEngineOverrides { + calendar_backend: Some(Arc::new(mock_backend)), + }; + let filter_engine = Arc::new(FilterEngine::new(Vec::new()).expect("empty rule set")); + let (addr, _handle) = spawn_test_server_with_all_overrides( + Arc::new(EventBus::new()), + db, + filter_engine, + secrets, + "correct-token", + MailEngineOverrides::default(), + calendar_overrides, + ) + .await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + + let response = client + .sync(authed_bearer_request(CalendarSyncRequest { + account_id: "acct-cal-injected".to_string(), + calendar_id: "cal-work".to_string(), + start_window: 1_699_999_000, + end_window: 1_700_004_000, + })) + .await + .expect("sync succeeds") + .into_inner(); + assert_eq!(response.synced_count, 1); + + // Immediately visible, no fixed sleep. + let list_response = client + .list_events(authed_bearer_request(ListEventsRequest { + account_id: "acct-cal-injected".to_string(), + calendar_id: "cal-work".to_string(), + start_window: 1_699_999_000, + end_window: 1_700_004_000, + })) + .await + .expect("list_events succeeds") + .into_inner(); + assert_eq!(list_response.events.len(), 1); + assert_eq!(list_response.events[0].id, "evt-sync-injected-1"); + + let get_response = client + .get_event(authed_bearer_request(GetEventRequest { + event_id: "evt-sync-injected-1".to_string(), + })) + .await + .expect("get_event succeeds") + .into_inner(); + assert_eq!( + get_response + .event + .expect("event present in response") + .summary, + "Summary for evt-sync-injected-1" + ); + } } diff --git a/crates/nunciod/src/lib.rs b/crates/nunciod/src/lib.rs index b6d74f6..11bf971 100644 --- a/crates/nunciod/src/lib.rs +++ b/crates/nunciod/src/lib.rs @@ -2,6 +2,7 @@ #![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used, clippy::panic))] +pub mod calendar_sync; pub mod grpc; pub mod orchestrator; pub mod send; diff --git a/crates/nunciod/tests/calendar_e2e_test.rs b/crates/nunciod/tests/calendar_e2e_test.rs new file mode 100644 index 0000000..98aaac7 --- /dev/null +++ b/crates/nunciod/tests/calendar_e2e_test.rs @@ -0,0 +1,193 @@ +//! Full-daemon offline end-to-end test for the calendar vertical. +//! +//! Boots a real `nunciod` daemon gRPC server +//! (`nunciod::grpc::serve_on_listener_with_overrides`) on an ephemeral +//! loopback port, backed by a real temp file-backed [`DatabaseEngine`] and a +//! [`SecretManager::mock`] vault (never the real OS keyring), with an +//! injected [`nuncio_cal::MockCalendarBackend`] standing in for real network +//! I/O (via the [`nunciod::grpc::CalendarEngineOverrides`] injection seam). +//! +//! Driving ONLY the authenticated `nuncio-proto` gRPC client +//! (`connect_calendar`) with the shared bearer token -- never reaching into +//! daemon internals -- this proves: +//! a. `Calendar/Sync` fetches from the injected mock backend and persists +//! the result, awaiting full completion before returning, so the +//! synced events are immediately visible to `ListEvents`/`GetEvent`. +//! b. An unauthenticated call to `Calendar` is rejected with +//! `Code::Unauthenticated`, proving it is mounted behind its own +//! `BearerAuthInterceptor` exactly like every other service. +//! +//! Deterministic and fully offline: no fixed sleeps, no real network, no +//! real OS keyring. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use nuncio_cal::MockCalendarBackend; +use nuncio_core::model::CalendarEvent; +use nuncio_core::EventBus; +use nuncio_filter::FilterEngine; +use nuncio_proto::v1::{CalendarSyncRequest, GetEventRequest, ListEventsRequest}; +use nuncio_store::db::DatabaseEngine; +use nuncio_store::vault::{SecretManager, GRPC_TOKEN_ACCOUNT}; +use nunciod::grpc::{ + serve_on_listener_with_overrides, CalendarEngineOverrides, MailEngineOverrides, +}; +use std::sync::Arc; +use tokio::net::TcpListener; +use tonic::Code; + +const ACCOUNT_ID: &str = "acct-cal-e2e-1"; +const CALENDAR_ID: &str = "cal-e2e-work"; + +fn mock_event(id: &str, start: i64, end: i64) -> CalendarEvent { + CalendarEvent { + id: id.to_string(), + account_id: ACCOUNT_ID.to_string(), + calendar_id: CALENDAR_ID.to_string(), + summary: format!("E2E Event {id}"), + start_time: start, + end_time: end, + rrule: None, + location: Some("Conference Room".to_string()), + } +} + +/// Boots the real daemon gRPC server, injecting `calendar_overrides`, on an +/// ephemeral loopback port backed by a real temp file-backed +/// [`DatabaseEngine`]. Returns the server address and the minted bearer +/// token; the `TempDir` guard is returned too so the caller controls its +/// lifetime for the whole test. +async fn boot_daemon( + secrets: Arc, + calendar_overrides: CalendarEngineOverrides, +) -> (std::net::SocketAddr, String, tempfile::TempDir) { + let dir = tempfile::tempdir().expect("create temp dir for file-backed db"); + let db_path = dir.path().join("calendar_e2e.db"); + let db = Arc::new( + DatabaseEngine::connect_file(&db_path, &secrets) + .await + .expect("open temp file-backed database"), + ); + + let token_bytes = secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .expect("mint gRPC bearer token from mock vault"); + let token = hex::encode(token_bytes); + + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind ephemeral loopback port"); + let addr = listener.local_addr().expect("listener has local addr"); + + let event_bus = Arc::new(EventBus::new()); + let server_secrets = secrets.clone(); + let server_token = token.clone(); + let server_filter_engine = Arc::new(FilterEngine::new(Vec::new()).expect("empty rule set")); + tokio::spawn(async move { + let _ = serve_on_listener_with_overrides( + listener, + event_bus, + db, + server_filter_engine, + server_secrets, + server_token, + MailEngineOverrides::default(), + calendar_overrides, + ) + .await; + }); + + (addr, token, dir) +} + +/// The full offline, deterministic end-to-end proof of the calendar vertical: +/// `Sync` against an injected mock backend, then `ListEvents`/`GetEvent` +/// immediately reflect the persisted result -- ALL over the authenticated +/// `nuncio.v1.Calendar` gRPC API against a live in-process daemon, with +/// protocol I/O fully mocked. +#[tokio::test] +async fn calendar_sync_then_list_and_get_round_trip_over_authenticated_grpc() { + let secrets = Arc::new(SecretManager::mock()); + + let mock_backend = MockCalendarBackend::new(); + mock_backend.add_event(mock_event("evt-e2e-1", 1_700_000_000, 1_700_003_600)); + mock_backend.add_event(mock_event("evt-e2e-2", 1_700_010_000, 1_700_013_600)); + // Outside the sync window this test requests -- must not be synced. + mock_backend.add_event(mock_event( + "evt-e2e-out-of-window", + 1_900_000_000, + 1_900_003_600, + )); + + let calendar_overrides = CalendarEngineOverrides { + calendar_backend: Some(Arc::new(mock_backend)), + }; + + let (addr, token, _db_dir) = boot_daemon(secrets, calendar_overrides).await; + let addr_str = addr.to_string(); + + let mut calendar_client = nuncio_proto::client::connect_calendar(&addr_str, &token) + .await + .expect("calendar client connects"); + + // ---- (a) Sync fetches from the injected mock backend and persists ---- + let sync_response = calendar_client + .sync(CalendarSyncRequest { + account_id: ACCOUNT_ID.to_string(), + calendar_id: CALENDAR_ID.to_string(), + start_window: 1_699_999_000, + end_window: 1_700_020_000, + }) + .await + .expect("sync succeeds") + .into_inner(); + assert_eq!(sync_response.synced_count, 2); + + // Immediately visible, no fixed sleep: `Sync` only returns once the + // fetch-and-persist work has fully completed. + let list_response = calendar_client + .list_events(ListEventsRequest { + account_id: ACCOUNT_ID.to_string(), + calendar_id: CALENDAR_ID.to_string(), + start_window: 1_699_999_000, + end_window: 1_700_020_000, + }) + .await + .expect("list_events succeeds") + .into_inner(); + assert_eq!(list_response.events.len(), 2); + let synced_ids: Vec<&str> = list_response.events.iter().map(|e| e.id.as_str()).collect(); + assert!(synced_ids.contains(&"evt-e2e-1")); + assert!(synced_ids.contains(&"evt-e2e-2")); + assert!(!synced_ids.contains(&"evt-e2e-out-of-window")); + + let get_response = calendar_client + .get_event(GetEventRequest { + event_id: "evt-e2e-1".to_string(), + }) + .await + .expect("get_event succeeds") + .into_inner(); + let event = get_response.event.expect("event present in response"); + assert_eq!(event.summary, "E2E Event evt-e2e-1"); + assert_eq!(event.location.as_deref(), Some("Conference Room")); + + // ---- (b) An unauthenticated call is rejected ---- + // Uses the raw generated client (bypassing `connect_calendar`'s + // bearer-token interceptor) to prove `Calendar` is mounted behind its + // own `BearerAuthInterceptor`, exactly like every other service. + let mut unauthenticated_client = + nuncio_proto::v1::calendar_client::CalendarClient::connect(format!("http://{addr_str}")) + .await + .expect("client connects at the transport level without any auth"); + let err = unauthenticated_client + .list_events(ListEventsRequest { + account_id: ACCOUNT_ID.to_string(), + calendar_id: CALENDAR_ID.to_string(), + start_window: 0, + end_window: i64::MAX, + }) + .await + .expect_err("missing bearer token must be rejected"); + assert_eq!(err.code(), Code::Unauthenticated); +} diff --git a/crates/nunciod/tests/spine_e2e_test.rs b/crates/nunciod/tests/spine_e2e_test.rs index bfc8f48..ac7e161 100644 --- a/crates/nunciod/tests/spine_e2e_test.rs +++ b/crates/nunciod/tests/spine_e2e_test.rs @@ -42,7 +42,9 @@ use nuncio_proto::v1::{ }; use nuncio_store::db::DatabaseEngine; use nuncio_store::vault::{SecretManager, GRPC_TOKEN_ACCOUNT}; -use nunciod::grpc::{serve_on_listener_with_overrides, MailEngineOverrides}; +use nunciod::grpc::{ + serve_on_listener_with_overrides, CalendarEngineOverrides, MailEngineOverrides, +}; use std::sync::Arc; use tokio::net::TcpListener; @@ -125,6 +127,7 @@ async fn boot_daemon( server_secrets, server_token, overrides, + CalendarEngineOverrides::default(), ) .await; }); -- 2.54.0 From 9496d63eaa2b1f20afd1997eca115666772bf4db Mon Sep 17 00:00:00 2001 From: James Maes Date: Wed, 29 Jul 2026 17:35:44 -0500 Subject: [PATCH 2/3] M1: Wire RRULE recurrence expansion into calendar event queries (#215) * feat(calendar): expand RRULE recurrence in ListEvents and filter by calendar_id Wire the previously-dead nuncio_cal::RecurrenceEngine into CalendarGrpcService::list_events: non-recurring events pass through unchanged, while recurring masters are expanded into real occurrences for the requested window via a new store method (list_recurring_calendar_events, no time filter since a master's own timestamps can precede the window while its occurrences fall inside it) and a pure expand_events_for_window merge helper that keeps recurring rows sourced only from expansion, never double-counting a master that also overlaps the window in the raw query. Fix expand_occurrences to stop trusting an empty match as "return the raw master": that fabricated an out-of-window result whenever a window fell entirely outside a bounded (UNTIL/COUNT) series, and could hang scanning an unbounded series for a match that would never come. skip_while/take_while over the RRuleSet's monotonic iterator bounds the walk and returns a genuinely empty result instead. Also apply the calendar_id filter list_events accepted but ignored, closing the gap between it and the mock backend's own filtering. Refs #186 * docs(nunciod): reword stale comment referencing removed rrule fallback --------- Co-authored-by: James Maes --- crates/nuncio-cal/src/rrule.rs | 50 ++++++- crates/nuncio-store/src/db.rs | 91 ++++++++++++ crates/nunciod/src/grpc.rs | 249 +++++++++++++++++++++++++++++++-- 3 files changed, 375 insertions(+), 15 deletions(-) diff --git a/crates/nuncio-cal/src/rrule.rs b/crates/nuncio-cal/src/rrule.rs index 2fb763b..7148521 100644 --- a/crates/nuncio-cal/src/rrule.rs +++ b/crates/nuncio-cal/src/rrule.rs @@ -47,9 +47,15 @@ impl RecurrenceEngine { let duration = event.end_time - event.start_time; + // `RRuleSet`'s iterator yields occurrences in strictly increasing date order, so once + // an occurrence passes `window_end_dt` every later one will too -- `take_while` lets + // us stop walking the (possibly unbounded, no-UNTIL/COUNT) series there instead of + // scanning forever for a match that will never come once the series has moved past + // the window. `skip_while` similarly avoids materializing pre-window occurrences. let results = rrule_set .into_iter() - .filter(|dt| *dt >= window_start_dt && *dt <= window_end_dt) + .skip_while(|dt| *dt < window_start_dt) + .take_while(|dt| *dt <= window_end_dt) .take(Self::MAX_EXPANSION_COUNT as usize); let mut occurrences = Vec::new(); @@ -62,12 +68,11 @@ impl RecurrenceEngine { occurrences.push(instance); } - if occurrences.is_empty() { - // Return original if no occurrences fall strictly inside window - Ok(vec![event.clone()]) - } else { - Ok(occurrences) - } + // No fallback to the raw master here: an empty result genuinely means no occurrence + // of this recurring event falls inside the requested window (e.g. the window is past + // an `UNTIL`/`COUNT` bound), and returning the master's own out-of-window timestamps + // instead would fabricate a result the caller never asked for. + Ok(occurrences) } } @@ -110,6 +115,37 @@ mod tests { assert!(results[0].id.contains("evt-weekly_occ_")); } + #[test] + fn recurring_event_with_no_occurrences_in_window_returns_empty() { + let event = sample_recurring_event(); + // Window is entirely before the series' own DTSTART -- no occurrence can fall + // inside it, so this must return no events at all, not the out-of-window master. + let results = RecurrenceEngine::expand_occurrences(&event, 1_600_000_000, 1_600_003_600) + .expect("expansion succeeds"); + assert!(results.is_empty()); + } + + #[test] + fn until_bound_stops_occurrences_past_the_bound() { + let mut event = sample_recurring_event(); + // Weekly series ends after its third occurrence (2024-01-15). + event.rrule = Some("FREQ=WEEKLY;INTERVAL=1;UNTIL=20240115T000000Z".to_string()); + + // Querying a window entirely after the UNTIL bound must produce zero occurrences. + let past_bound = RecurrenceEngine::expand_occurrences(&event, 1_707_350_400, 1_709_251_200) + .expect("expansion succeeds"); + assert!(past_bound.is_empty()); + + // Querying a window spanning the bound only returns occurrences up to and + // including it, never past it. + let spanning_bound = RecurrenceEngine::expand_occurrences(&event, 1704067200, 1706745600) + .expect("expansion succeeds"); + assert_eq!(spanning_bound.len(), 3); + for occurrence in &spanning_bound { + assert!(occurrence.start_time <= 1_705_276_800); // 2024-01-15T00:00:00Z + } + } + #[test] fn invalid_rrule_returns_parse_error() { let mut event = sample_recurring_event(); diff --git a/crates/nuncio-store/src/db.rs b/crates/nuncio-store/src/db.rs index 79066c2..dc1759a 100644 --- a/crates/nuncio-store/src/db.rs +++ b/crates/nuncio-store/src/db.rs @@ -995,6 +995,58 @@ impl DatabaseEngine { .collect()) } + /// Query every persisted recurring calendar event (non-null, non-empty `rrule`) for + /// `account_id`, with **no** time-window filter -- a recurring master's own stored + /// `start_time`/`end_time` can validly sit long before (or after) any given query + /// window while its expanded occurrences still fall inside it, so callers that need + /// to materialize occurrences for a window must expand every one of the account's + /// recurring masters rather than pre-filtering by the master row's own timestamps. + #[allow(clippy::type_complexity)] + pub async fn list_recurring_calendar_events( + &self, + account_id: &str, + ) -> Result, DatabaseError> { + let rows: Vec<( + String, + String, + String, + String, + i64, + i64, + Option, + Option, + )> = sqlx::query_as( + r#" + SELECT id, account_id, calendar_id, summary, start_time, end_time, rrule, location + FROM calendar_events + WHERE account_id = ? AND rrule IS NOT NULL AND rrule != '' + ORDER BY start_time ASC + "#, + ) + .bind(account_id) + .fetch_all(&self.pool) + .await + .map_err(DatabaseError::Query)?; + + Ok(rows + .into_iter() + .map( + |(id, account_id, calendar_id, summary, start_time, end_time, rrule, location)| { + nuncio_core::model::CalendarEvent { + id, + account_id, + calendar_id, + summary, + start_time, + end_time, + rrule, + location, + } + }, + ) + .collect()) + } + /// Retrieve a single calendar event by ID. #[allow(clippy::type_complexity)] pub async fn get_calendar_event( @@ -1981,6 +2033,45 @@ mod tests { assert_eq!(hits[0].id, "evt-db-100"); } + /// `list_recurring_calendar_events` returns only rows with a non-empty `rrule`, for the + /// requested account only, and applies no time-window filter at all -- a master whose own + /// stored `start_time` is far outside any given query window must still come back, since + /// its expanded occurrences may fall inside that window. + #[tokio::test] + async fn list_recurring_calendar_events_filters_by_account_and_rrule_presence() { + let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap(); + + let mut recurring = sample_calendar_event("evt-recurring", "acct-1"); + recurring.rrule = Some("FREQ=WEEKLY;INTERVAL=1".to_string()); + recurring.start_time = 1_000; // far outside any window used below + recurring.end_time = 4_600; + engine.save_calendar_event(&recurring).await.unwrap(); + + let non_recurring = sample_calendar_event("evt-single", "acct-1"); + engine.save_calendar_event(&non_recurring).await.unwrap(); + + let mut other_account_recurring = sample_calendar_event("evt-other-acct", "acct-2"); + other_account_recurring.rrule = Some("FREQ=DAILY".to_string()); + engine + .save_calendar_event(&other_account_recurring) + .await + .unwrap(); + + let recurring_for_acct1 = engine + .list_recurring_calendar_events("acct-1") + .await + .expect("list recurring calendar events succeeds"); + assert_eq!(recurring_for_acct1.len(), 1); + assert_eq!(recurring_for_acct1[0].id, "evt-recurring"); + + let recurring_for_acct2 = engine + .list_recurring_calendar_events("acct-2") + .await + .expect("list recurring calendar events succeeds"); + assert_eq!(recurring_for_acct2.len(), 1); + assert_eq!(recurring_for_acct2[0].id, "evt-other-acct"); + } + /// Re-saving an event with the same ID (an update, not a fresh insert) replaces both the /// row and its FTS entry rather than duplicating either. #[tokio::test] diff --git a/crates/nunciod/src/grpc.rs b/crates/nunciod/src/grpc.rs index a8a2654..a33c0f7 100644 --- a/crates/nunciod/src/grpc.rs +++ b/crates/nunciod/src/grpc.rs @@ -439,6 +439,47 @@ fn map_calendar_event_to_proto(event: nuncio_core::model::CalendarEvent) -> Cale } } +/// Combines an account's in-window events with its recurring masters into the final event +/// list for a `ListEvents` query window, expanding each recurring master into its real +/// occurrences via [`nuncio_cal::RecurrenceEngine::expand_occurrences`] rather than trusting +/// the master row's own stored `start_time`/`end_time` to represent every future instance. +/// +/// `recurring` events are never sourced from `all_in_window` for the final result -- only +/// `e.rrule.is_none()` rows from `all_in_window` are kept verbatim -- so a recurring master +/// that also happens to overlap the window in `all_in_window` is never emitted a second time +/// alongside its expanded occurrences. +/// +/// A recurring event whose `rrule` fails to parse degrades to the raw stored master (a genuine persisted row -- just unexpanded, not fabricated data) with a warning +/// logged, rather than failing the whole query for one malformed rule among many. +fn expand_events_for_window( + all_in_window: Vec, + recurring: Vec, + start_window: i64, + end_window: i64, +) -> Vec { + let mut events: Vec = all_in_window + .into_iter() + .filter(|e| e.rrule.is_none()) + .collect(); + + for event in recurring { + match nuncio_cal::RecurrenceEngine::expand_occurrences(&event, start_window, end_window) { + Ok(occurrences) => events.extend(occurrences), + Err(e) => { + tracing::warn!( + "Calendar: failed to expand rrule for event '{}', falling back to raw \ + stored event: {}", + event.id, + e + ); + events.push(event); + } + } + } + + events +} + /// Test-only injection point for the daemon's calendar sync backend, /// mirroring [`MailEngineOverrides`]'s shape. /// @@ -517,18 +558,25 @@ impl Calendar for CalendarGrpcService { if req.account_id.is_empty() { return Err(Status::invalid_argument("account_id is required")); } - // `DatabaseEngine::list_calendar_events` filters by account_id and - // time window only -- there is no per-calendar-collection filter in - // the store today, so `req.calendar_id` is not applied here. This - // never fabricates data; it genuinely returns every persisted event - // for the account in the requested window, same as `req.calendar_id` - // being absent from the query would. - let events = self + let all_in_window = self .db .list_calendar_events(&req.account_id, req.start_window, req.end_window) .await - .map_err(|e| Status::internal(format!("failed to list calendar events: {e}")))? + .map_err(|e| Status::internal(format!("failed to list calendar events: {e}")))?; + + let recurring = self + .db + .list_recurring_calendar_events(&req.account_id) + .await + .map_err(|e| Status::internal(format!("failed to list recurring events: {e}")))?; + + let mut events = + expand_events_for_window(all_in_window, recurring, req.start_window, req.end_window); + + events.retain(|e| req.calendar_id.is_empty() || e.calendar_id == req.calendar_id); + + let events = events .into_iter() .map(map_calendar_event_to_proto) .collect(); @@ -3705,6 +3753,65 @@ mod tests { assert_eq!(response.events[0].id, "evt-in-window"); } + /// `ListEvents` filters by `calendar_id` when it's non-empty: an event that overlaps the + /// window but belongs to a different calendar collection must not appear in the response. + #[tokio::test] + async fn list_events_excludes_events_from_a_different_calendar_id() { + use nuncio_proto::v1::calendar_client::CalendarClient; + use nuncio_proto::v1::ListEventsRequest; + + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("connect ephemeral test db"); + db.save_calendar_event(&sample_calendar_event( + "evt-work-cal", + "acct-cal-1", + "cal-work", + 1_700_000_000, + 1_700_003_600, + )) + .await + .expect("save work-calendar event"); + db.save_calendar_event(&sample_calendar_event( + "evt-personal-cal", + "acct-cal-1", + "cal-personal", + 1_700_000_000, + 1_700_003_600, + )) + .await + .expect("save personal-calendar event"); + + let event_bus = Arc::new(EventBus::new()); + let secrets = Arc::new(SecretManager::mock()); + let filter_engine = Arc::new(FilterEngine::new(Vec::new()).expect("empty rule set")); + let (addr, _handle) = spawn_test_server_with( + event_bus, + Arc::new(db), + filter_engine, + secrets, + "correct-token", + ) + .await; + + let mut client = CalendarClient::connect(format!("http://{addr}")) + .await + .expect("client connects"); + let response = client + .list_events(authed_bearer_request(ListEventsRequest { + account_id: "acct-cal-1".to_string(), + calendar_id: "cal-work".to_string(), + start_window: 1_699_999_000, + end_window: 1_700_004_000, + })) + .await + .expect("list_events succeeds") + .into_inner(); + + assert_eq!(response.events.len(), 1); + assert_eq!(response.events[0].id, "evt-work-cal"); + } + #[tokio::test] async fn list_events_rejects_empty_account_id() { use nuncio_proto::v1::calendar_client::CalendarClient; @@ -3908,4 +4015,130 @@ mod tests { "Summary for evt-sync-injected-1" ); } + + // ---- expand_events_for_window (pure, no DB/gRPC) ---- + + fn recurring_event( + id: &str, + rrule: &str, + start: i64, + end: i64, + ) -> nuncio_core::model::CalendarEvent { + let mut event = sample_calendar_event(id, "acct-cal-1", "cal-work", start, end); + event.rrule = Some(rrule.to_string()); + event + } + + /// A weekly event's later occurrence (well past the stored master's own start/end) is + /// returned when the query window covers only that later occurrence -- proving the merge + /// helper materializes real occurrences rather than trusting the master row's own window + /// membership. + #[test] + fn expand_events_for_window_finds_later_occurrence_of_a_recurring_event() { + let master = recurring_event( + "evt-weekly", + "FREQ=WEEKLY;INTERVAL=1", + 1_704_067_200, // 2024-01-01T00:00:00Z + 1_704_070_800, + ); + + // Query a window 4 weeks later, which does not overlap the master's own + // start/end at all. + let start_window = 1_706_400_000; // 2024-01-28 + let end_window = 1_706_500_000; + + let result = expand_events_for_window(Vec::new(), vec![master], start_window, end_window); + + assert_eq!(result.len(), 1); + assert!(result[0].start_time >= start_window && result[0].start_time <= end_window); + assert!(result[0].id.starts_with("evt-weekly_occ_")); + } + + /// A non-recurring event passed through `all_in_window` is returned unchanged. + #[test] + fn expand_events_for_window_passes_non_recurring_events_through_unchanged() { + let event = sample_calendar_event( + "evt-single", + "acct-cal-1", + "cal-work", + 1_700_000_000, + 1_700_003_600, + ); + + let result = expand_events_for_window( + vec![event.clone()], + Vec::new(), + 1_699_999_000, + 1_700_004_000, + ); + + assert_eq!(result, vec![event]); + } + + /// A recurring master whose own stored start/end also overlaps the query window must + /// never appear twice -- once as the raw row from `all_in_window` and again as an + /// expanded occurrence. + #[test] + fn expand_events_for_window_never_double_counts_a_recurring_master() { + let master = recurring_event( + "evt-weekly", + "FREQ=WEEKLY;INTERVAL=1", + 1_704_067_200, + 1_704_070_800, + ); + + // The master's own row also overlaps the window (as `list_calendar_events` + // would genuinely return it), simulating the exact double-count risk. + let result = expand_events_for_window( + vec![master.clone()], + vec![master], + 1_704_067_200, + 1_704_070_800, + ); + + assert_eq!(result.len(), 1); + assert!(result[0].id.starts_with("evt-weekly_occ_")); + } + + /// A `COUNT`/`UNTIL`-bounded recurrence produces no events for a window entirely past + /// its bound. + #[test] + fn expand_events_for_window_honors_until_bound() { + let master = recurring_event( + "evt-bounded", + "FREQ=WEEKLY;INTERVAL=1;UNTIL=20240115T000000Z", + 1_704_067_200, + 1_704_070_800, + ); + + let result = expand_events_for_window( + Vec::new(), + vec![master], + 1_709_251_200, // 2024-03-01, well past the UNTIL bound + 1_709_337_600, + ); + + assert!(result.is_empty()); + } + + /// An event with an unparseable `rrule` degrades to the raw stored master rather than + /// vanishing or failing the whole merge. + #[test] + fn expand_events_for_window_degrades_unparseable_rrule_to_raw_master() { + let master = recurring_event( + "evt-bad-rrule", + "NOT_A_VALID_RRULE", + 1_700_000_000, + 1_700_003_600, + ); + + let result = expand_events_for_window( + Vec::new(), + vec![master.clone()], + 1_699_999_000, + 1_700_004_000, + ); + + assert_eq!(result, vec![master]); + } } -- 2.54.0 From e3e3a01fe53fe981e47de5067ce06aa5ad3872be Mon Sep 17 00:00:00 2001 From: James Maes Date: Wed, 29 Jul 2026 17:48:48 -0500 Subject: [PATCH 3/3] feat(contacts): build a real CardDAV engine, replacing the fabricated client Replace CardDavClient::fetch_remote_vcards's hardcoded "Google CardDAV Sync Contact" return with a real reqwest REPORT/addressbook-query client that parses a genuine multistatus response. Add a real vCard (RFC 6350) parser -- the crate previously only had a generator -- and prove round-trip fidelity through Contact::to_vcard(). Add the ContactsBackend trait and MockContactsBackend, mirroring nuncio_cal::CalendarBackend/MockCalendarBackend. Distinguish transport failures from parse failures via CardDavError::TransportFailed/ ParseFailed rather than ever silently succeeding with fabricated data. Give nuncio-store real, tested save_contact/list_contacts/get_contact CRUD backed by a genuine SQLite table + trigram FTS mirror, so contacts persist through the daemon's own store instead of nuncio-contacts's disconnected ContactsDatabase pool. This takes nuncio-store as a narrow, deliberate dependency on nuncio-contacts::Contact. Opt nuncio-contacts into the workspace lint gate ([lints] workspace, #![forbid(unsafe_code)]) for parity with every other crate. Wiremock-only, offline: no live network calls in tests. --- Cargo.lock | 2 + crates/nuncio-contacts/Cargo.toml | 5 + crates/nuncio-contacts/src/backend.rs | 18 ++ crates/nuncio-contacts/src/carddav.rs | 199 ++++++++++-- crates/nuncio-contacts/src/lib.rs | 10 + crates/nuncio-contacts/src/mock.rs | 116 +++++++ crates/nuncio-contacts/src/parser.rs | 247 +++++++++++++++ .../tests/wiremock_carddav_test.rs | 156 ++++++++++ crates/nuncio-store/Cargo.toml | 1 + crates/nuncio-store/src/db.rs | 289 ++++++++++++++++++ 10 files changed, 1024 insertions(+), 19 deletions(-) create mode 100644 crates/nuncio-contacts/src/backend.rs create mode 100644 crates/nuncio-contacts/src/mock.rs create mode 100644 crates/nuncio-contacts/src/parser.rs create mode 100644 crates/nuncio-contacts/tests/wiremock_carddav_test.rs diff --git a/Cargo.lock b/Cargo.lock index 0a4dd07..19a0b75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2513,6 +2513,7 @@ dependencies = [ "thiserror 1.0.69", "tokio", "tracing", + "wiremock", ] [[package]] @@ -2611,6 +2612,7 @@ dependencies = [ "hex", "hmac", "keyring", + "nuncio-contacts", "nuncio-core", "nuncio-filter", "serde", diff --git a/crates/nuncio-contacts/Cargo.toml b/crates/nuncio-contacts/Cargo.toml index 2cc1465..c91f4c9 100644 --- a/crates/nuncio-contacts/Cargo.toml +++ b/crates/nuncio-contacts/Cargo.toml @@ -4,6 +4,10 @@ version.workspace = true edition.workspace = true authors.workspace = true license.workspace = true +repository.workspace = true + +[lints] +workspace = true [dependencies] nuncio-core = { path = "../nuncio-core" } @@ -21,3 +25,4 @@ async-trait = { workspace = true } [dev-dependencies] tempfile = { workspace = true } +wiremock = { workspace = true } diff --git a/crates/nuncio-contacts/src/backend.rs b/crates/nuncio-contacts/src/backend.rs new file mode 100644 index 0000000..75a9af3 --- /dev/null +++ b/crates/nuncio-contacts/src/backend.rs @@ -0,0 +1,18 @@ +//! Protocol-agnostic async contacts backend trait definitions. + +use async_trait::async_trait; + +use crate::carddav::CardDavError; +use crate::models::Contact; + +/// Protocol-agnostic contacts backend engine trait implemented by CardDAV and mock engines. +/// +/// Mirrors `nuncio_cal::CalendarBackend`'s shape: `Send + Sync` so implementations can be held +/// behind an `Arc` and shared across daemon tasks, and returning the crate's single +/// [`CardDavError`] type so callers do not need to know whether a given implementation talks +/// to a real server or an in-memory fixture. +#[async_trait] +pub trait ContactsBackend: Send + Sync { + /// Fetch contacts belonging to `account_id`. + async fn fetch_contacts(&self, account_id: &str) -> Result, CardDavError>; +} diff --git a/crates/nuncio-contacts/src/carddav.rs b/crates/nuncio-contacts/src/carddav.rs index f150f21..0fe496b 100644 --- a/crates/nuncio-contacts/src/carddav.rs +++ b/crates/nuncio-contacts/src/carddav.rs @@ -1,35 +1,56 @@ -use crate::models::Contact; -use reqwest::Client; +//! CardDAV (RFC 6352) `REPORT` client and multistatus response parser. + +use async_trait::async_trait; use serde::{Deserialize, Serialize}; use thiserror::Error; -use tracing::info; -#[derive(Error, Debug)] +use crate::backend::ContactsBackend; +use crate::models::Contact; +use crate::parser::VCardParserAdapter; + +/// Errors returned by the CardDAV client engine. +#[derive(Error, Debug, PartialEq, Eq)] pub enum CardDavError { - #[error("HTTP error: {0}")] - Http(#[from] reqwest::Error), - #[error("CardDAV sync error: {0}")] - SyncError(String), + /// Failed to parse an RFC 6350 vCard payload embedded in a CardDAV response. + #[error("failed to parse vCard payload: {0}")] + ParseFailed(String), + + /// CardDAV network/transport-layer failure (connection, TLS, or an unexpected HTTP + /// status) distinct from a payload that connected fine but failed to parse. + #[error("CardDAV transport failure: {0}")] + TransportFailed(String), } +/// Configuration for a specific CardDAV address book collection endpoint, mirroring +/// `nuncio_cal::CalDavAccountConfig`. +/// +/// `carddav_url` must already resolve to a specific address book collection (e.g. +/// `https://carddav.example.com/dav/addressbooks/user/jmaes/contacts/`) -- PROPFIND-based +/// `addressbook-home-set` auto-discovery is out of scope for this client. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CardDavAccountConfig { + /// Nuncio account identifier this address book collection belongs to. pub account_id: String, + /// Fully-qualified URL of the CardDAV address book collection to query. pub carddav_url: String, + /// Basic-auth username (or app-specific username, per provider). pub username: String, + /// Basic-auth secret (password or app-specific token) resolved from the OS keyring by the + /// caller -- never stored anywhere else in plaintext. pub auth_token: String, } +/// CardDAV client protocol engine managing address book `REPORT` queries against a real server. pub struct CardDavClient { - #[allow(dead_code)] - client: Client, config: CardDavAccountConfig, + http: reqwest::Client, } impl CardDavClient { + /// Create a new `CardDavClient` bound to a specific CardDAV address book collection. pub fn new(config: CardDavAccountConfig) -> Self { Self { - client: Client::builder() + http: reqwest::Client::builder() .user_agent("Nuncio-Contacts-CardDAV/1.0") .build() .unwrap_or_default(), @@ -37,13 +58,153 @@ impl CardDavClient { } } - pub async fn fetch_remote_vcards(&self) -> Result, CardDavError> { - info!( - "Initiating CardDAV PROPFIND fetch from {}", - self.config.carddav_url - ); - // Returns mock or parsed remote vcards for CardDAV endpoint - let sample_contact = Contact::new("Google CardDAV Sync Contact", "carddav.sync@nuncio.mx"); - Ok(vec![sample_contact]) + /// Construct a standard CardDAV `` XML payload (RFC 6352 Section + /// 8.6) requesting the full `address-data` of every card in the collection. + pub fn build_report_query() -> String { + r#" + + + + + +"# + .to_string() + } + + /// Parse a CardDAV WebDAV XML `` response containing embedded vCard data. + pub fn parse_multistatus_response( + &self, + account_id: &str, + raw_xml: &str, + ) -> Result, CardDavError> { + let mut contacts = Vec::new(); + + for block in raw_xml.split("") { + if let Some((vcard_data, _)) = block.split_once("") { + let clean_vcard = vcard_data.trim(); + if !clean_vcard.is_empty() { + let contact_id = format!("carddav-{account_id}-{}", contacts.len() + 1); + let contact = + VCardParserAdapter::parse_vcard(&contact_id, account_id, clean_vcard)?; + contacts.push(contact); + } + } + } + + Ok(contacts) + } + + /// Issue a live CardDAV `REPORT` addressbook-query (RFC 6352 Section 8.6) against + /// `self.config.carddav_url` and parse the `multistatus` response into domain contacts. + /// + /// This performs a real network request -- no canned or fabricated data is ever + /// returned. A server or network failure surfaces as [`CardDavError::TransportFailed`]; a + /// malformed vCard inside a returned response surfaces as [`CardDavError::ParseFailed`]. + pub async fn fetch_remote_vcards( + &self, + account_id: &str, + ) -> Result, CardDavError> { + let body = Self::build_report_query(); + + // "REPORT" is a fixed, always-valid HTTP token; `from_bytes` cannot fail for it, but + // the error is still propagated rather than unwrapped so no code path here can panic. + let report_method = reqwest::Method::from_bytes(b"REPORT") + .map_err(|e| CardDavError::TransportFailed(format!("invalid HTTP method: {e}")))?; + + let response = self + .http + .request(report_method, &self.config.carddav_url) + .header("Content-Type", "application/xml; charset=utf-8") + .header("Depth", "1") + .basic_auth(&self.config.username, Some(&self.config.auth_token)) + .body(body) + .send() + .await + .map_err(|e| CardDavError::TransportFailed(e.to_string()))?; + + let status = response.status(); + // RFC 6352 REPORT responses are conventionally 207 Multi-Status; some servers reply + // 200 OK for a single-collection result, so any 2xx/207 status is accepted. + if status.as_u16() != 207 && !status.is_success() { + return Err(CardDavError::TransportFailed(format!( + "CardDAV server returned unexpected status {status}" + ))); + } + + let raw_xml = response + .text() + .await + .map_err(|e| CardDavError::TransportFailed(e.to_string()))?; + + self.parse_multistatus_response(account_id, &raw_xml) + } +} + +#[async_trait] +impl ContactsBackend for CardDavClient { + async fn fetch_contacts(&self, account_id: &str) -> Result, CardDavError> { + self.fetch_remote_vcards(account_id).await + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn test_config() -> CardDavAccountConfig { + CardDavAccountConfig { + account_id: "acct-1".to_string(), + carddav_url: "https://carddav.example.com/addressbooks/contacts/".to_string(), + username: "jmaes".to_string(), + auth_token: "app-token-secret".to_string(), + } + } + + #[test] + fn build_report_query_targets_addressbook_query() { + let query = CardDavClient::build_report_query(); + assert!(query.contains(" + + + /addressbooks/contacts/alice.vcf + + + BEGIN:VCARD +VERSION:4.0 +FN:Alice Dev +EMAIL:alice@nuncio.mx +END:VCARD + + + + "#; + + let contacts = client + .parse_multistatus_response("acct-1", xml_response) + .expect("parse succeeds"); + + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0].display_name, "Alice Dev"); + assert_eq!(contacts[0].emails[0].email, "alice@nuncio.mx"); + assert_eq!(contacts[0].account_id.as_deref(), Some("acct-1")); + } + + #[test] + fn parse_multistatus_response_with_no_cards_yields_empty_not_fabricated() { + let client = CardDavClient::new(test_config()); + let xml_response = r#" + "#; + + let contacts = client + .parse_multistatus_response("acct-1", xml_response) + .expect("parse succeeds"); + assert!(contacts.is_empty()); } } diff --git a/crates/nuncio-contacts/src/lib.rs b/crates/nuncio-contacts/src/lib.rs index f935b72..3911bfc 100644 --- a/crates/nuncio-contacts/src/lib.rs +++ b/crates/nuncio-contacts/src/lib.rs @@ -1,12 +1,22 @@ +//! Contacts (CardDAV / RFC 6350 vCard) engine: a real CardDAV `REPORT` client, a vCard +//! parser/generator round trip, and a protocol-agnostic [`ContactsBackend`] trait. + +#![forbid(unsafe_code)] #![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used, clippy::panic))] +pub mod backend; pub mod carddav; pub mod db; +pub mod mock; pub mod models; +pub mod parser; +pub use backend::ContactsBackend; pub use carddav::{CardDavAccountConfig, CardDavClient, CardDavError}; pub use db::{ContactsDatabase, ContactsStoreError}; +pub use mock::MockContactsBackend; pub use models::{Contact, ContactEmail, ContactGroup, ContactPhone}; +pub use parser::VCardParserAdapter; #[cfg(test)] mod tests { diff --git a/crates/nuncio-contacts/src/mock.rs b/crates/nuncio-contacts/src/mock.rs new file mode 100644 index 0000000..8e07537 --- /dev/null +++ b/crates/nuncio-contacts/src/mock.rs @@ -0,0 +1,116 @@ +//! Deterministic mock contacts backend for offline testing and integration verification. + +use async_trait::async_trait; +use std::sync::{Arc, Mutex}; + +use crate::backend::ContactsBackend; +use crate::carddav::CardDavError; +use crate::models::Contact; + +/// Thread-safe mock contacts backend for offline testing. +#[derive(Debug, Clone, Default)] +pub struct MockContactsBackend { + contacts: Arc>>, + should_fail: Arc>, +} + +impl MockContactsBackend { + /// Create a new `MockContactsBackend`. + pub fn new() -> Self { + Self::default() + } + + /// Configure the mock to simulate CardDAV network failure errors. + pub fn set_should_fail(&self, fail: bool) { + if let Ok(mut flag) = self.should_fail.lock() { + *flag = fail; + } + } + + /// Add a mock contact to storage. + pub fn add_contact(&self, contact: Contact) { + if let Ok(mut guard) = self.contacts.lock() { + guard.push(contact); + } + } + + /// Retrieve contacts belonging to a specific account. + pub fn list_contacts(&self, account_id: &str) -> Result, CardDavError> { + let should_fail = self + .should_fail + .lock() + .map_err(|e| CardDavError::ParseFailed(e.to_string()))?; + if *should_fail { + return Err(CardDavError::TransportFailed( + "simulated CardDAV network failure".to_string(), + )); + } + + let guard = self + .contacts + .lock() + .map_err(|e| CardDavError::ParseFailed(e.to_string()))?; + let matches = guard + .iter() + .filter(|c| c.account_id.as_deref() == Some(account_id)) + .cloned() + .collect(); + + Ok(matches) + } +} + +#[async_trait] +impl ContactsBackend for MockContactsBackend { + async fn fetch_contacts(&self, account_id: &str) -> Result, CardDavError> { + self.list_contacts(account_id) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::Contact as ModelContact; + + fn sample_contact(account_id: &str, display_name: &str) -> ModelContact { + let mut contact = ModelContact::new(display_name, format!("{display_name}@nuncio.mx")); + contact.account_id = Some(account_id.to_string()); + contact + } + + #[test] + fn mock_contacts_backend_operations() { + let mock = MockContactsBackend::new(); + mock.add_contact(sample_contact("acct-1", "Mock Contact")); + + let contacts = mock + .list_contacts("acct-1") + .expect("list contacts succeeds"); + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0].display_name, "Mock Contact"); + + assert!(mock + .list_contacts("acct-other") + .expect("list succeeds") + .is_empty()); + + mock.set_should_fail(true); + assert!(mock.list_contacts("acct-1").is_err()); + } + + /// Proves `MockContactsBackend` is usable behind the `ContactsBackend` trait object, the + /// same way the daemon will inject either it or the real CardDAV client. + #[tokio::test] + async fn mock_contacts_backend_is_usable_as_a_trait_object() { + let mock = MockContactsBackend::new(); + mock.add_contact(sample_contact("acct-1", "Trait Object Contact")); + + let backend: Arc = Arc::new(mock); + let contacts = backend + .fetch_contacts("acct-1") + .await + .expect("fetch contacts succeeds"); + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0].display_name, "Trait Object Contact"); + } +} diff --git a/crates/nuncio-contacts/src/parser.rs b/crates/nuncio-contacts/src/parser.rs new file mode 100644 index 0000000..ca83878 --- /dev/null +++ b/crates/nuncio-contacts/src/parser.rs @@ -0,0 +1,247 @@ +//! vCard (RFC 6350) parser adapter converting raw vCard payloads into Nuncio [`Contact`] +//! domain entities -- the read half of the round trip whose write half is +//! [`Contact::to_vcard`]. + +use chrono::Utc; + +use crate::carddav::CardDavError; +use crate::models::{Contact, ContactEmail, ContactPhone}; + +/// vCard parser adapter converting raw `.vcf`/CardDAV `address-data` payloads into Nuncio +/// [`Contact`] domain entities. +pub struct VCardParserAdapter; + +impl VCardParserAdapter { + /// Maximum allowed single-vCard payload size (1MB -- generous for a single contact + /// record, small enough to reject a runaway/malicious response body outright). + pub const MAX_PAYLOAD_BYTES: usize = 1024 * 1024; + + /// Parse a raw RFC 6350 vCard string into a [`Contact`] entity. + /// + /// `id` is assigned by the caller (CardDAV responses carry no stable contact ID of their + /// own outside the enclosing WebDAV `href`); `account_id` tags which Nuncio account this + /// contact was fetched for. Never fabricates a placeholder name or email: a vCard with + /// neither `FN` nor any `EMAIL` is a parse failure, not an empty/default contact. + pub fn parse_vcard( + id: &str, + account_id: &str, + raw_vcard: &str, + ) -> Result { + if raw_vcard.len() > Self::MAX_PAYLOAD_BYTES { + return Err(CardDavError::ParseFailed( + "vCard payload exceeds maximum allowed limit of 1MB".to_string(), + )); + } + if !raw_vcard.to_ascii_uppercase().contains("BEGIN:VCARD") { + return Err(CardDavError::ParseFailed( + "vCard payload is missing BEGIN:VCARD".to_string(), + )); + } + + let unfolded = unfold_lines(raw_vcard); + + let mut display_name: Option = None; + let mut given_name: Option = None; + let mut family_name: Option = None; + let mut organization: Option = None; + let mut job_title: Option = None; + let mut emails: Vec = Vec::new(); + let mut phones: Vec = Vec::new(); + + for line in unfolded.lines() { + let line = line.trim(); + if line.is_empty() { + continue; + } + let upper = line.to_ascii_uppercase(); + if upper == "BEGIN:VCARD" || upper == "END:VCARD" || upper.starts_with("VERSION:") { + continue; + } + + let Some((prop, value)) = line.split_once(':') else { + continue; + }; + let mut segments = prop.split(';'); + let name = segments.next().unwrap_or("").to_ascii_uppercase(); + let params: Vec<&str> = segments.collect(); + + match name.as_str() { + "FN" => display_name = Some(value.trim().to_string()), + "N" => { + let comps: Vec<&str> = value.split(';').collect(); + if let Some(family) = comps.first().map(|s| s.trim()).filter(|s| !s.is_empty()) + { + family_name = Some(family.to_string()); + } + if let Some(given) = comps.get(1).map(|s| s.trim()).filter(|s| !s.is_empty()) { + given_name = Some(given.to_string()); + } + } + "ORG" => organization = Some(value.trim().to_string()), + "TITLE" => job_title = Some(value.trim().to_string()), + "EMAIL" => { + let label = extract_type(¶ms).unwrap_or_else(|| "other".to_string()); + let is_primary = emails.is_empty(); + emails.push(ContactEmail { + email: value.trim().to_string(), + label: label.to_ascii_lowercase(), + is_primary, + }); + } + "TEL" => { + let label = extract_type(¶ms).unwrap_or_else(|| "other".to_string()); + let is_primary = phones.is_empty(); + phones.push(ContactPhone { + phone: value.trim().to_string(), + label: label.to_ascii_lowercase(), + is_primary, + }); + } + _ => {} + } + } + + let display_name = display_name + .or_else(|| emails.first().map(|e| e.email.clone())) + .ok_or_else(|| { + CardDavError::ParseFailed( + "vCard has neither FN nor EMAIL to derive a display name".to_string(), + ) + })?; + + let now = Utc::now(); + Ok(Contact { + id: id.to_string(), + account_id: Some(account_id.to_string()), + display_name, + given_name, + family_name, + organization, + job_title, + notes: None, + avatar_url: None, + emails, + phones, + is_favorite: false, + interaction_count: 0, + last_interacted_at: None, + created_at: now, + updated_at: now, + }) + } +} + +/// Extract a `TYPE=...` parameter value from a vCard property's parameter list. +fn extract_type(params: &[&str]) -> Option { + params.iter().find_map(|param| { + param + .to_ascii_uppercase() + .strip_prefix("TYPE=") + .map(|v| v.to_string()) + }) +} + +/// Unfold RFC 6350 line continuations (a line starting with a space/tab is a continuation of +/// the previous line) into single logical lines, tolerating both CRLF and bare LF input. +fn unfold_lines(raw: &str) -> String { + let mut result = String::new(); + for line in raw.split("\r\n").flat_map(|l| l.split('\n')) { + if (line.starts_with(' ') || line.starts_with('\t')) && !result.is_empty() { + result.push_str(line.trim_start()); + } else { + if !result.is_empty() { + result.push('\n'); + } + result.push_str(line); + } + } + result +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_vcard_with_all_supported_fields() { + let vcard = "BEGIN:VCARD\r\n\ + VERSION:4.0\r\n\ + FN:James Maes\r\n\ + N:Maes;James;;;\r\n\ + ORG:KofTwentyTwo\r\n\ + TITLE:Engineer\r\n\ + EMAIL;TYPE=WORK:james.maes@kof22.com\r\n\ + TEL;TYPE=MOBILE:+1-555-0199\r\n\ + END:VCARD"; + + let contact = + VCardParserAdapter::parse_vcard("c-1", "acct-1", vcard).expect("parse succeeds"); + + assert_eq!(contact.display_name, "James Maes"); + assert_eq!(contact.given_name.as_deref(), Some("James")); + assert_eq!(contact.family_name.as_deref(), Some("Maes")); + assert_eq!(contact.organization.as_deref(), Some("KofTwentyTwo")); + assert_eq!(contact.job_title.as_deref(), Some("Engineer")); + assert_eq!(contact.emails.len(), 1); + assert_eq!(contact.emails[0].email, "james.maes@kof22.com"); + assert_eq!(contact.emails[0].label, "work"); + assert!(contact.emails[0].is_primary); + assert_eq!(contact.phones.len(), 1); + assert_eq!(contact.phones[0].phone, "+1-555-0199"); + assert_eq!(contact.phones[0].label, "mobile"); + } + + #[test] + fn parse_vcard_without_fn_falls_back_to_primary_email_as_display_name() { + let vcard = "BEGIN:VCARD\r\nVERSION:4.0\r\nEMAIL:alice@nuncio.mx\r\nEND:VCARD"; + let contact = + VCardParserAdapter::parse_vcard("c-2", "acct-1", vcard).expect("parse succeeds"); + assert_eq!(contact.display_name, "alice@nuncio.mx"); + } + + #[test] + fn parse_vcard_missing_begin_marker_is_a_real_error() { + let err = VCardParserAdapter::parse_vcard("c-3", "acct-1", "FN:No Envelope") + .expect_err("must reject a payload with no BEGIN:VCARD"); + assert!(matches!(err, CardDavError::ParseFailed(_))); + } + + #[test] + fn parse_vcard_with_no_name_or_email_is_a_real_error_not_a_fabricated_contact() { + let vcard = "BEGIN:VCARD\r\nVERSION:4.0\r\nORG:Ghost Corp\r\nEND:VCARD"; + let err = VCardParserAdapter::parse_vcard("c-4", "acct-1", vcard) + .expect_err("a vCard with no FN and no EMAIL must never become a fabricated contact"); + assert!(matches!(err, CardDavError::ParseFailed(_))); + } + + /// Proves the parser is the true inverse of [`Contact::to_vcard`]: generating a vCard from + /// a rich domain [`Contact`] and parsing it back must reproduce the same + /// FN/N/ORG/TITLE/primary-EMAIL/primary-TEL fields. + #[test] + fn vcard_round_trip_preserves_core_fields() { + let mut original = Contact::new("Alice Developer", "alice@kof22.com"); + original.given_name = Some("Alice".to_string()); + original.family_name = Some("Developer".to_string()); + original.organization = Some("KofTwentyTwo".to_string()); + original.job_title = Some("Staff Engineer".to_string()); + original.phones.push(ContactPhone { + phone: "+1-555-0100".to_string(), + label: "mobile".to_string(), + is_primary: true, + }); + + let vcard = original.to_vcard(); + let round_tripped = VCardParserAdapter::parse_vcard("c-rt-1", "acct-1", &vcard) + .expect("round-trip parse succeeds"); + + assert_eq!(round_tripped.display_name, original.display_name); + assert_eq!(round_tripped.given_name, original.given_name); + assert_eq!(round_tripped.family_name, original.family_name); + assert_eq!(round_tripped.organization, original.organization); + assert_eq!(round_tripped.job_title, original.job_title); + assert_eq!(round_tripped.emails[0].email, original.emails[0].email); + assert_eq!(round_tripped.emails[0].label, original.emails[0].label); + assert_eq!(round_tripped.phones[0].phone, original.phones[0].phone); + assert_eq!(round_tripped.phones[0].label, original.phones[0].label); + } +} diff --git a/crates/nuncio-contacts/tests/wiremock_carddav_test.rs b/crates/nuncio-contacts/tests/wiremock_carddav_test.rs new file mode 100644 index 0000000..a9c3ee4 --- /dev/null +++ b/crates/nuncio-contacts/tests/wiremock_carddav_test.rs @@ -0,0 +1,156 @@ +//! WireMock integration test suite for the real CardDAV `REPORT` transport. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use nuncio_contacts::{CardDavAccountConfig, CardDavClient, CardDavError, ContactsBackend}; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +/// A canned CardDAV `multistatus` response containing two real vCards, exercising the real +/// HTTP transport together with the vCard parser. +fn multistatus_body() -> String { + r#" + + + /addressbooks/contacts/james.vcf + + + BEGIN:VCARD +VERSION:4.0 +FN:James Maes +N:Maes;James;;; +ORG:KofTwentyTwo +TITLE:Founder +EMAIL;TYPE=WORK:james.maes@kof22.com +TEL;TYPE=MOBILE:+1-555-0199 +END:VCARD + + + + + /addressbooks/contacts/alice.vcf + + + BEGIN:VCARD +VERSION:4.0 +FN:Alice Dev +EMAIL:alice@nuncio.mx +END:VCARD + + + +"# + .to_string() +} + +#[tokio::test] +async fn wiremock_carddav_report_fetches_and_parses_vcards() { + let mock_server = MockServer::start().await; + + Mock::given(method("REPORT")) + .and(path("/addressbooks/contacts/")) + .respond_with(ResponseTemplate::new(207).set_body_string(multistatus_body())) + .mount(&mock_server) + .await; + + let config = CardDavAccountConfig { + account_id: "acct-wm-1".to_string(), + carddav_url: format!("{}/addressbooks/contacts/", mock_server.uri()), + username: "james.maes".to_string(), + auth_token: "wiremock-app-token".to_string(), + }; + let client = CardDavClient::new(config); + + // Drive the real client through the `ContactsBackend` trait, exactly how the daemon will + // invoke either this real client or `MockContactsBackend` behind the same seam. + let contacts: Vec<_> = ContactsBackend::fetch_contacts(&client, "acct-wm-1") + .await + .expect("fetch_contacts succeeds against the wiremock server"); + + assert_eq!(contacts.len(), 2); + + let james = contacts + .iter() + .find(|c| c.display_name == "James Maes") + .expect("James Maes contact present"); + assert_eq!(james.account_id.as_deref(), Some("acct-wm-1")); + assert_eq!(james.given_name.as_deref(), Some("James")); + assert_eq!(james.family_name.as_deref(), Some("Maes")); + assert_eq!(james.organization.as_deref(), Some("KofTwentyTwo")); + assert_eq!(james.job_title.as_deref(), Some("Founder")); + assert_eq!(james.emails[0].email, "james.maes@kof22.com"); + assert_eq!(james.phones[0].phone, "+1-555-0199"); + + let alice = contacts + .iter() + .find(|c| c.display_name == "Alice Dev") + .expect("Alice Dev contact present"); + assert_eq!(alice.emails[0].email, "alice@nuncio.mx"); +} + +#[tokio::test] +async fn wiremock_carddav_report_server_error_surfaces_as_transport_failure() { + let mock_server = MockServer::start().await; + + Mock::given(method("REPORT")) + .and(path("/addressbooks/broken/")) + .respond_with(ResponseTemplate::new(500)) + .mount(&mock_server) + .await; + + let config = CardDavAccountConfig { + account_id: "acct-wm-2".to_string(), + carddav_url: format!("{}/addressbooks/broken/", mock_server.uri()), + username: "james.maes".to_string(), + auth_token: "wiremock-app-token".to_string(), + }; + let client = CardDavClient::new(config); + + let err = client + .fetch_remote_vcards("acct-wm-2") + .await + .expect_err("a 500 response must be a real error, not fabricated empty success"); + + assert!(matches!(err, CardDavError::TransportFailed(_))); +} + +#[tokio::test] +async fn wiremock_carddav_report_malformed_vcard_surfaces_as_parse_failure() { + let mock_server = MockServer::start().await; + + let malformed_body = r#" + + + /addressbooks/malformed/ghost.vcf + + + BEGIN:VCARD +VERSION:4.0 +ORG:No Name Or Email +END:VCARD + + + +"#; + + Mock::given(method("REPORT")) + .and(path("/addressbooks/malformed/")) + .respond_with(ResponseTemplate::new(207).set_body_string(malformed_body)) + .mount(&mock_server) + .await; + + let config = CardDavAccountConfig { + account_id: "acct-wm-3".to_string(), + carddav_url: format!("{}/addressbooks/malformed/", mock_server.uri()), + username: "james.maes".to_string(), + auth_token: "wiremock-app-token".to_string(), + }; + let client = CardDavClient::new(config); + + let err = client + .fetch_remote_vcards("acct-wm-3") + .await + .expect_err("a vCard with neither FN nor EMAIL must be a real parse error"); + + assert!(matches!(err, CardDavError::ParseFailed(_))); +} diff --git a/crates/nuncio-store/Cargo.toml b/crates/nuncio-store/Cargo.toml index 347f5c9..d60e957 100644 --- a/crates/nuncio-store/Cargo.toml +++ b/crates/nuncio-store/Cargo.toml @@ -12,6 +12,7 @@ workspace = true [dependencies] nuncio-core = { path = "../nuncio-core" } nuncio-filter = { path = "../nuncio-filter" } +nuncio-contacts = { path = "../nuncio-contacts" } tokio.workspace = true sqlx.workspace = true serde.workspace = true diff --git a/crates/nuncio-store/src/db.rs b/crates/nuncio-store/src/db.rs index 79066c2..50fd1cc 100644 --- a/crates/nuncio-store/src/db.rs +++ b/crates/nuncio-store/src/db.rs @@ -115,6 +115,52 @@ fn resolve_engine_keys( Ok((storage_key, worm_key, ledger_key)) } +/// Reconstruct a [`nuncio_contacts::Contact`] from a `contacts` table row. +/// +/// `created_at`/`updated_at`/`last_interacted_at` are stored as RFC 3339 strings written by +/// [`DatabaseEngine::save_contact`] itself, so a parse failure here would indicate corrupted +/// storage rather than bad input; falling back to "now" for that edge case (rather than +/// propagating a parse error through every read) mirrors the tolerant `unwrap_or_default` +/// JSON-decode style already used for `emails_json`/`phones_json` below. +fn contact_from_row(row: &sqlx::sqlite::SqliteRow) -> nuncio_contacts::Contact { + let emails_json: String = row.get("emails_json"); + let phones_json: String = row.get("phones_json"); + let emails: Vec = + serde_json::from_str(&emails_json).unwrap_or_default(); + let phones: Vec = + serde_json::from_str(&phones_json).unwrap_or_default(); + let is_favorite: i64 = row.get("is_favorite"); + let interaction_count: i64 = row.get("interaction_count"); + let last_interacted_at: Option = row.get("last_interacted_at"); + let created_at: String = row.get("created_at"); + let updated_at: String = row.get("updated_at"); + + let parse_rfc3339 = |s: &str| -> chrono::DateTime { + chrono::DateTime::parse_from_rfc3339(s) + .map(|dt| dt.with_timezone(&chrono::Utc)) + .unwrap_or_else(|_| chrono::Utc::now()) + }; + + nuncio_contacts::Contact { + id: row.get("id"), + account_id: row.get("account_id"), + display_name: row.get("display_name"), + given_name: row.get("given_name"), + family_name: row.get("family_name"), + organization: row.get("organization"), + job_title: row.get("job_title"), + notes: row.get("notes"), + avatar_url: row.get("avatar_url"), + emails, + phones, + is_favorite: is_favorite != 0, + interaction_count: interaction_count as u64, + last_interacted_at: last_interacted_at.as_deref().map(parse_rfc3339), + created_at: parse_rfc3339(&created_at), + updated_at: parse_rfc3339(&updated_at), + } +} + /// Structured result of re-verifying the entire persisted WORM audit /// ledger's hash chain. See /// [`DatabaseEngine::verify_worm_audit_chain_report`]. @@ -519,6 +565,54 @@ impl DatabaseEngine { INSERT INTO events_fts(id, summary, location) VALUES (new.id, new.summary, COALESCE(new.location, '')); END; + + -- Contacts (nuncio_contacts::Contact). Emails/phones are stored as JSON columns + -- (mirroring the schema already proven in nuncio-contacts's own, now-superseded + -- ContactsDatabase) rather than normalized child tables, since a contact's email/ + -- phone list is always read and written as a whole with its parent record. Like + -- calendar summary/location, none of this is encrypted at rest, so trigger-based + -- FTS mirroring introduces no confidentiality regression. + CREATE TABLE IF NOT EXISTS contacts ( + id TEXT PRIMARY KEY NOT NULL, + account_id TEXT, + display_name TEXT NOT NULL, + given_name TEXT, + family_name TEXT, + organization TEXT, + job_title TEXT, + notes TEXT, + avatar_url TEXT, + emails_json TEXT NOT NULL DEFAULT '[]', + phones_json TEXT NOT NULL DEFAULT '[]', + is_favorite INTEGER NOT NULL DEFAULT 0, + interaction_count INTEGER NOT NULL DEFAULT 0, + last_interacted_at TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL + ); + + CREATE VIRTUAL TABLE IF NOT EXISTS contacts_fts USING fts5( + id UNINDEXED, + display_name, + organization, + emails_json, + tokenize = 'trigram' + ); + + CREATE TRIGGER IF NOT EXISTS contacts_ai AFTER INSERT ON contacts BEGIN + INSERT INTO contacts_fts(id, display_name, organization, emails_json) + VALUES (new.id, new.display_name, COALESCE(new.organization, ''), new.emails_json); + END; + + CREATE TRIGGER IF NOT EXISTS contacts_ad AFTER DELETE ON contacts BEGIN + DELETE FROM contacts_fts WHERE id = old.id; + END; + + CREATE TRIGGER IF NOT EXISTS contacts_au AFTER UPDATE ON contacts BEGIN + DELETE FROM contacts_fts WHERE id = old.id; + INSERT INTO contacts_fts(id, display_name, organization, emails_json) + VALUES (new.id, new.display_name, COALESCE(new.organization, ''), new.emails_json); + END; "#, ) .execute(&self.pool) @@ -1034,6 +1128,103 @@ impl DatabaseEngine { }) } + /// Save a [`nuncio_contacts::Contact`] to SQLite (INSERT OR REPLACE). + /// + /// `nuncio-store` is otherwise limited to depending only on `nuncio-core` domain types; + /// this method (and [`Self::list_contacts`]/[`Self::get_contact`]) is a deliberate, + /// narrow exception, taking a direct dependency on `nuncio_contacts::Contact` because that + /// type has not (yet) been promoted to `nuncio-core` the way `CalendarEvent`/`Email` have. + pub async fn save_contact( + &self, + contact: &nuncio_contacts::Contact, + ) -> Result<(), DatabaseError> { + let emails_json = + serde_json::to_string(&contact.emails).unwrap_or_else(|_| "[]".to_string()); + let phones_json = + serde_json::to_string(&contact.phones).unwrap_or_else(|_| "[]".to_string()); + let last_interacted_str = contact.last_interacted_at.map(|t| t.to_rfc3339()); + + sqlx::query( + r#" + INSERT OR REPLACE INTO contacts + (id, account_id, display_name, given_name, family_name, organization, job_title, + notes, avatar_url, emails_json, phones_json, is_favorite, interaction_count, + last_interacted_at, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + "#, + ) + .bind(&contact.id) + .bind(&contact.account_id) + .bind(&contact.display_name) + .bind(&contact.given_name) + .bind(&contact.family_name) + .bind(&contact.organization) + .bind(&contact.job_title) + .bind(&contact.notes) + .bind(&contact.avatar_url) + .bind(&emails_json) + .bind(&phones_json) + .bind(contact.is_favorite) + .bind(contact.interaction_count as i64) + .bind(&last_interacted_str) + .bind(contact.created_at.to_rfc3339()) + .bind(contact.updated_at.to_rfc3339()) + .execute(&self.pool) + .await + .map_err(DatabaseError::Query)?; + + Ok(()) + } + + /// Query persisted contacts belonging to `account_id`, most-recently-interacted first. + /// + /// See the confidentiality/dependency note on [`Self::save_contact`]. + pub async fn list_contacts( + &self, + account_id: &str, + ) -> Result, DatabaseError> { + let rows = sqlx::query( + r#" + SELECT id, account_id, display_name, given_name, family_name, organization, + job_title, notes, avatar_url, emails_json, phones_json, is_favorite, + interaction_count, last_interacted_at, created_at, updated_at + FROM contacts + WHERE account_id = ? + ORDER BY interaction_count DESC, display_name ASC + "#, + ) + .bind(account_id) + .fetch_all(&self.pool) + .await + .map_err(DatabaseError::Query)?; + + Ok(rows.iter().map(contact_from_row).collect()) + } + + /// Retrieve a single contact by ID. + /// + /// See the confidentiality/dependency note on [`Self::save_contact`]. + pub async fn get_contact( + &self, + contact_id: &str, + ) -> Result { + let row = sqlx::query( + r#" + SELECT id, account_id, display_name, given_name, family_name, organization, + job_title, notes, avatar_url, emails_json, phones_json, is_favorite, + interaction_count, last_interacted_at, created_at, updated_at + FROM contacts + WHERE id = ? + "#, + ) + .bind(contact_id) + .fetch_one(&self.pool) + .await + .map_err(DatabaseError::Query)?; + + Ok(contact_from_row(&row)) + } + /// Update a single message's read/unread flag in place. /// /// Returns `DatabaseError::Query(sqlx::Error::RowNotFound)` if no message @@ -2026,6 +2217,104 @@ mod tests { )); } + fn sample_contact(id: &str, account_id: &str) -> nuncio_contacts::Contact { + let mut contact = nuncio_contacts::Contact::new("Architecture Contact", "arch@nuncio.mx"); + contact.id = id.to_string(); + contact.account_id = Some(account_id.to_string()); + contact.organization = Some("KofTwentyTwo".to_string()); + contact + } + + /// Count rows in `contacts_fts` matching a trigram search term, proving the real write + /// path (not a raw-SQL test insert) keeps the trigger-populated FTS index in sync -- the + /// same proof pattern as `save_get_list_and_search_calendar_event`, without depending on + /// `SearchEngine` gaining a contacts-specific method (out of scope for this story). + async fn contacts_fts_hit_count(engine: &DatabaseEngine, term: &str) -> i64 { + let pattern = format!("%{term}%"); + let row: (i64,) = + sqlx::query_as("SELECT COUNT(*) FROM contacts_fts WHERE display_name LIKE ?") + .bind(&pattern) + .fetch_one(engine.pool()) + .await + .unwrap(); + row.0 + } + + /// A saved contact is retrievable by both `get_contact` and `list_contacts`, and its FTS + /// mirror is kept in sync by the real write path (not a raw-SQL test insert). + #[tokio::test] + async fn save_get_list_and_search_contact() { + let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap(); + + let contact = sample_contact("ct-db-100", "acct-1"); + engine + .save_contact(&contact) + .await + .expect("save contact succeeds"); + + let fetched = engine + .get_contact("ct-db-100") + .await + .expect("get contact succeeds"); + assert_eq!(fetched.display_name, contact.display_name); + assert_eq!(fetched.account_id, contact.account_id); + assert_eq!(fetched.emails[0].email, contact.emails[0].email); + + let listed = engine + .list_contacts("acct-1") + .await + .expect("list contacts succeeds"); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].id, "ct-db-100"); + + // Different account -> no match. + let other_account = engine + .list_contacts("acct-other") + .await + .expect("list contacts succeeds"); + assert!(other_account.is_empty()); + + assert_eq!(contacts_fts_hit_count(&engine, "Architecture").await, 1); + } + + /// Re-saving a contact with the same ID (an update, not a fresh insert) replaces both the + /// row and its FTS entry rather than duplicating either. + #[tokio::test] + async fn save_contact_upserts_existing_row_and_fts_entry() { + let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap(); + + let mut contact = sample_contact("ct-db-200", "acct-1"); + engine.save_contact(&contact).await.unwrap(); + + contact.display_name = "Renamed Contact".to_string(); + engine.save_contact(&contact).await.unwrap(); + + let fetched = engine.get_contact("ct-db-200").await.unwrap(); + assert_eq!(fetched.display_name, "Renamed Contact"); + + assert_eq!( + contacts_fts_hit_count(&engine, "Architecture Contact").await, + 0 + ); + assert_eq!(contacts_fts_hit_count(&engine, "Renamed").await, 1); + } + + /// `get_contact` for an ID that was never saved reports `sqlx::Error::RowNotFound` rather + /// than silently fabricating an empty/default contact. + #[tokio::test] + async fn get_contact_reports_not_found_for_missing_contact() { + let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap(); + + let err = engine + .get_contact("ct-does-not-exist") + .await + .expect_err("missing contact must be a real error"); + assert!(matches!( + err, + DatabaseError::Query(sqlx::Error::RowNotFound) + )); + } + /// `set_message_read` flips the persisted /// `read_flag` in place (both directions), and reports /// `sqlx::Error::RowNotFound` for a message ID that was never saved, -- 2.54.0