diff --git a/src/exchange_ews/contact_map.rs b/src/exchange_ews/contact_map.rs index 5b7aa67..632c3e7 100644 --- a/src/exchange_ews/contact_map.rs +++ b/src/exchange_ews/contact_map.rs @@ -92,35 +92,52 @@ pub fn to_jscontact(raw: &ContactItemRaw) -> Value { Value::Object(map_singleton("1", json!({"@type": "Title", "name": t}))), ); } - if !raw.emails.is_empty() { + { let mut map = Map::new(); - for (i, (key, addr)) in raw.emails.iter().enumerate() { - let entry_id = format!("{}", i + 1); + for (i, (key, addr)) in raw + .emails + .iter() + .filter(|(_, v)| !v.trim().is_empty()) + .enumerate() + { let mut entry = Map::new(); entry.insert("@type".to_owned(), Value::String("EmailAddress".to_owned())); entry.insert("address".to_owned(), Value::String(addr.clone())); entry.insert("contexts".to_owned(), email_contexts(key)); - map.insert(entry_id, Value::Object(entry)); + map.insert((i + 1).to_string(), Value::Object(entry)); + } + if !map.is_empty() { + card.insert("emails".to_owned(), Value::Object(map)); } - card.insert("emails".to_owned(), Value::Object(map)); } - if !raw.phones.is_empty() { + { let mut map = Map::new(); - for (i, (key, num)) in raw.phones.iter().enumerate() { - let entry_id = format!("{}", i + 1); + for (i, (key, num)) in raw + .phones + .iter() + .filter(|(_, v)| !v.trim().is_empty()) + .enumerate() + { let mut entry = Map::new(); entry.insert("@type".to_owned(), Value::String("Phone".to_owned())); entry.insert("number".to_owned(), Value::String(num.clone())); entry.insert("contexts".to_owned(), phone_contexts(key)); entry.insert("features".to_owned(), phone_features(key)); - map.insert(entry_id, Value::Object(entry)); + map.insert((i + 1).to_string(), Value::Object(entry)); + } + if !map.is_empty() { + card.insert("phones".to_owned(), Value::Object(map)); } - card.insert("phones".to_owned(), Value::Object(map)); } - if !raw.ims.is_empty() { + { let mut map = Map::new(); - for (i, (key, addr)) in raw.ims.iter().enumerate() { - let entry_id = format!("{}", i + 1); + for (i, (key, addr)) in raw + .ims + .iter() + .filter(|(_, v)| !v.trim().is_empty()) + .enumerate() + { + let entry_id = (i + 1).to_string(); let mut entry = Map::new(); entry.insert( "@type".to_owned(), @@ -132,7 +149,9 @@ pub fn to_jscontact(raw: &ContactItemRaw) -> Value { } map.insert(entry_id, Value::Object(entry)); } - card.insert("onlineServices".to_owned(), Value::Object(map)); + if !map.is_empty() { + card.insert("onlineServices".to_owned(), Value::Object(map)); + } } if !raw.addresses.is_empty() { let pref_key = raw @@ -185,7 +204,7 @@ pub fn to_jscontact(raw: &ContactItemRaw) -> Value { } card.insert("keywords".to_owned(), Value::Object(keywords)); } - if let Some(notes) = raw.notes.as_ref() { + if let Some(notes) = raw.notes.as_ref().filter(|n| !n.trim().is_empty()) { card.insert( "notes".to_owned(), Value::Object(map_singleton("1", json!({"@type": "Note", "note": notes}))), @@ -341,6 +360,29 @@ mod tests { assert_eq!(v["phones"]["1"]["contexts"]["work"], true); } + #[test] + fn empty_body_and_empty_im_entry_are_omitted() { + let raw = ContactItemRaw { + display_name: Some("Random Person".to_owned()), + notes: Some(String::new()), + ims: vec![("ImAddress1".to_owned(), String::new())], + emails: vec![("EmailAddress1".to_owned(), " ".to_owned())], + phones: vec![("MobilePhone".to_owned(), "+17801234567".to_owned())], + ..ContactItemRaw::default() + }; + let v = to_jscontact(&raw); + assert!(v.get("notes").is_none(), "empty body must not emit notes"); + assert!( + v.get("onlineServices").is_none(), + "empty IM entry must not emit onlineServices" + ); + assert!( + v.get("emails").is_none(), + "whitespace-only email must not emit emails" + ); + assert_eq!(v["phones"]["1"]["number"], "+17801234567"); + } + #[test] fn synthetic_uid_is_stable() { let u1 = synthetic_uid("ITEM-1"); diff --git a/src/exchange_ews/parse.rs b/src/exchange_ews/parse.rs index 2eb6108..96ac129 100644 --- a/src/exchange_ews/parse.rs +++ b/src/exchange_ews/parse.rs @@ -956,6 +956,7 @@ pub fn parse_message_item(inner_xml: &str) -> Result { let mut mime_charset: Option = None; let mut category_collecting = false; let mut in_flag = false; + let mut in_flagstatus_ext = false; let mut cur = String::new(); loop { buf.clear(); @@ -999,6 +1000,13 @@ pub fn parse_message_item(inner_xml: &str) -> Result { in_flag = true; } else if in_flag && local.eq_ignore_ascii_case(b"FlagStatus") { current = Some("flagStatus"); + } else if local.eq_ignore_ascii_case(b"ExtendedFieldURI") { + if let Some(tag) = attr_value(e, b"PropertyTag") { + let t = tag.trim().to_ascii_lowercase(); + in_flagstatus_ext = t == "0x1090" || t == "4240"; + } + } else if in_flagstatus_ext && local.eq_ignore_ascii_case(b"Value") { + current = Some("flagStatusExt"); } else { current = None; } @@ -1027,6 +1035,9 @@ pub fn parse_message_item(inner_xml: &str) -> Result { } "category" => item.categories.push(text), "flagStatus" => item.flag_status = Some(text), + "flagStatusExt" if text.trim() == "2" => { + item.flag_status = Some("Flagged".to_owned()); + } _ => {} } } @@ -1035,6 +1046,8 @@ pub fn parse_message_item(inner_xml: &str) -> Result { category_collecting = false; } else if lower == b"flag" { in_flag = false; + } else if lower == b"extendedproperty" { + in_flagstatus_ext = false; } } Event::Text(ref t) => { @@ -2248,6 +2261,64 @@ mod tests { assert!(matches!(r[1].response_code, ResponseCode::ItemNotFound)); } + #[test] + fn pid_tag_flag_status_extended_property_marks_message_flagged() { + fn parse_one(message_xml: &str) -> MessageItem { + let body = format!( + "\ + \ + NoError\ + {message_xml}\ + " + ); + let r = parse_response_messages(body.as_bytes(), b"GetItemResponseMessage").unwrap(); + parse_message_item(&r[0].inner_xml).unwrap() + } + + let flagged = parse_one( + "\ + Flagged\ + \ + \ + 2\ + \ + true", + ); + assert_eq!(flagged.flag_status.as_deref(), Some("Flagged")); + assert_eq!(flagged.is_read, Some(true)); + + let cleared = parse_one( + "\ + \ + \ + 0\ + ", + ); + assert_eq!(cleared.flag_status, None); + } + + #[test] + fn native_item_flag_still_parses_on_exchange_2013_plus() { + let body = format!( + "\ + \ + NoError\ + \ + Native\ + Flagged\ + false\ + " + ); + let r = parse_response_messages(body.as_bytes(), b"GetItemResponseMessage").unwrap(); + let p = parse_message_item(&r[0].inner_xml).unwrap(); + assert_eq!( + p.flag_status.as_deref(), + Some("Flagged"), + "the 2013+ native item:Flag path must be unaffected by the pre-2013 extended-property fallback" + ); + assert_eq!(p.is_read, Some(false)); + } + #[test] fn sync_folder_items_emits_create_delete_readflag() { let body = format!( diff --git a/src/exchange_ews/recurrence.rs b/src/exchange_ews/recurrence.rs index 17ea253..a76929f 100644 --- a/src/exchange_ews/recurrence.rs +++ b/src/exchange_ews/recurrence.rs @@ -92,10 +92,11 @@ pub fn to_jscalendar_rule(raw: &RawRecurrence) -> Option { match raw.range.as_ref() { Some(RecurrenceRange::NoEnd { .. }) | None => {} Some(RecurrenceRange::EndDate { end_date, .. }) => { - let local = if end_date.contains('T') { - end_date.clone() + let trimmed = end_date.trim().trim_end_matches('Z'); + let local = if trimmed.contains('T') { + trimmed.to_owned() } else { - format!("{end_date}T23:59:59") + format!("{trimmed}T23:59:59") }; rule.insert("until".to_owned(), Value::String(local)); } @@ -345,6 +346,19 @@ mod tests { assert_eq!(rule["bySetPosition"], json!([-1])); } + #[test] + fn end_date_with_trailing_z_yields_valid_local_until() { + let raw = RawRecurrence { + pattern: Some(RecurrencePattern::Daily { interval: 2 }), + range: Some(RecurrenceRange::EndDate { + start_date: "2026-07-01Z".to_owned(), + end_date: "2026-08-01Z".to_owned(), + }), + }; + let rule = to_jscalendar_rule(&raw).unwrap(); + assert_eq!(rule["until"], "2026-08-01T23:59:59"); + } + #[test] fn weekly_every_weekday_expands() { let raw = RawRecurrence { diff --git a/src/exchange_ews/types.rs b/src/exchange_ews/types.rs index 736668d..679eeb8 100644 --- a/src/exchange_ews/types.rs +++ b/src/exchange_ews/types.rs @@ -148,6 +148,23 @@ impl ServerVersion { } } +const SKIPPED_CONTAINER_CLASSES: [&str; 8] = [ + "ipf.task", + "ipf.journal", + "ipf.stickynote", + "ipf.configuration", + "ipf.storeitem", + "ipf.skypeteams", + "ipf.files", + "ipf.note.outlookhomepage", +]; + +fn class_matches(lower_class: &str, lower_prefix: &str) -> bool { + lower_class == lower_prefix + || (lower_class.starts_with(lower_prefix) + && lower_class[lower_prefix.len()..].starts_with('.')) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum FolderClass { Mail, @@ -158,18 +175,25 @@ pub enum FolderClass { impl FolderClass { pub fn from_ipf(class: &str) -> FolderClass { - if class.eq_ignore_ascii_case("IPF.Note") { - FolderClass::Mail - } else if class.eq_ignore_ascii_case("IPF.Appointment") - || class.starts_with("IPF.Appointment.") - { + let lower = class.trim().to_ascii_lowercase(); + if class_matches(&lower, "ipf.appointment") { FolderClass::Calendar - } else if class.eq_ignore_ascii_case("IPF.Contact") || class.starts_with("IPF.Contact.") { + } else if class_matches(&lower, "ipf.contact") { FolderClass::Contacts - } else { + } else if SKIPPED_CONTAINER_CLASSES + .iter() + .any(|p| class_matches(&lower, p)) + { FolderClass::Skipped + } else { + FolderClass::Mail } } + + pub fn is_mail_fallback(class: &str) -> bool { + let lower = class.trim().to_ascii_lowercase(); + FolderClass::from_ipf(class) == FolderClass::Mail && !class_matches(&lower, "ipf.note") + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -300,6 +324,34 @@ mod tests { assert_eq!(FolderClass::from_ipf("IPF.Task"), FolderClass::Skipped); } + #[test] + fn absent_or_unknown_folder_class_falls_back_to_mail() { + assert_eq!(FolderClass::from_ipf(""), FolderClass::Mail); + assert_eq!(FolderClass::from_ipf(" "), FolderClass::Mail); + assert_eq!(FolderClass::from_ipf("IPF.SomethingNew"), FolderClass::Mail); + assert!(FolderClass::is_mail_fallback("")); + assert!(FolderClass::is_mail_fallback("IPF.SomethingNew")); + assert!(!FolderClass::is_mail_fallback("IPF.Note")); + assert!(!FolderClass::is_mail_fallback("IPF.Note.OutlookHomepage")); + assert!(!FolderClass::is_mail_fallback("IPF.Task")); + } + + #[test] + fn internal_and_out_of_scope_classes_are_skipped() { + for c in [ + "IPF.Task", + "IPF.Journal", + "IPF.StickyNote", + "IPF.Configuration", + "IPF.StoreItem.PdpProfileV2Secured", + "IPF.SkypeTeams.Message", + "IPF.Files", + "IPF.Note.OutlookHomepage", + ] { + assert_eq!(FolderClass::from_ipf(c), FolderClass::Skipped, "{c}"); + } + } + #[test] fn server_version_from_build() { assert_eq!( diff --git a/src/exchange_ews/xml.rs b/src/exchange_ews/xml.rs index d89b077..a19e931 100644 --- a/src/exchange_ews/xml.rs +++ b/src/exchange_ews/xml.rs @@ -147,6 +147,10 @@ pub fn get_item_body(shape: ItemShape, ids: &[ItemId], version: ServerVersion) - out.push_str(""); if version >= ServerVersion::Exchange2013 { out.push_str(""); + } else { + out.push_str( + "", + ); } out.push_str(""); out.push_str(""); @@ -329,6 +333,14 @@ mod tests { !legacy.contains("item:Flag"), "item:Flag is an Exchange 2013 schema addition; must be omitted on 2010" ); + assert!( + legacy.contains("PropertyTag=\"0x1090\""), + "on pre-2013 the follow-up flag must be fetched via the PidTagFlagStatus extended property" + ); + assert!( + !modern.contains("0x1090"), + "the extended-property fallback is only used when item:Flag is unavailable" + ); assert!(legacy.contains("item:DateTimeReceived")); assert!(legacy.contains("message:IsReadReceiptRequested")); assert!( diff --git a/src/sync/import_exchange_ews/coordinator.rs b/src/sync/import_exchange_ews/coordinator.rs index 1272ad0..4ee25cd 100644 --- a/src/sync/import_exchange_ews/coordinator.rs +++ b/src/sync/import_exchange_ews/coordinator.rs @@ -126,9 +126,14 @@ pub fn run(common: CommonConfig, config: EwsImportConfig) -> Result { mail.push(ClassifiedFolder { @@ -163,13 +177,26 @@ const WELL_KNOWN_ROLES: &[(DistinguishedFolderId, Option<&'static str>)] = &[ (DistinguishedFolderId::ConversationHistory, None), ]; +struct DeleteTarget<'a> { + type_name: &'a str, + table: &'a str, + counts: &'a mut TypeCounts, +} + +pub struct ReconcileCounts<'a> { + pub mailbox: &'a mut TypeCounts, + pub calendar: &'a mut TypeCounts, + pub addressbook: &'a mut TypeCounts, + pub email: &'a mut TypeCounts, + pub calendar_event: &'a mut TypeCounts, + pub contact: &'a mut TypeCounts, +} + pub fn reconcile( conn: &mut Connection, source_id: i64, plan: &FolderPlan, - mailbox_counts: &mut TypeCounts, - calendar_counts: &mut TypeCounts, - addressbook_counts: &mut TypeCounts, + counts: &mut ReconcileCounts<'_>, _logger: Logger, ) -> Result<(), Error> { let local_mailbox: HashMap = @@ -210,7 +237,7 @@ pub fn reconcile( source_id, id_to_local: &mut id_to_local, local: &local_mailbox, - counts: mailbox_counts, + counts: &mut *counts.mailbox, root_folder_id: &plan.root_folder_id, }; upsert_mailbox(&mut ctx, folder, role)?; @@ -222,7 +249,7 @@ pub fn reconcile( source_id, id_to_local: &mut id_to_local, local: &local_calendar, - counts: calendar_counts, + counts: &mut *counts.calendar, root_folder_id: &plan.root_folder_id, }; upsert_calendar(&mut ctx, folder)?; @@ -234,7 +261,7 @@ pub fn reconcile( source_id, id_to_local: &mut id_to_local, local: &local_addressbook, - counts: addressbook_counts, + counts: &mut *counts.addressbook, root_folder_id: &plan.root_folder_id, }; upsert_address_book(&mut ctx, folder)?; @@ -243,29 +270,50 @@ pub fn reconcile( delete_vanished( conn, source_id, - exchange_ews_ids::MAILBOX, - "mailboxes", &local_mailbox, &server_ids_mail, - mailbox_counts, + DeleteTarget { + type_name: exchange_ews_ids::MAILBOX, + table: "mailboxes", + counts: counts.mailbox, + }, + Some(DeleteTarget { + type_name: exchange_ews_ids::EMAIL, + table: "emails", + counts: counts.email, + }), )?; delete_vanished( conn, source_id, - exchange_ews_ids::CALENDAR, - "calendars", &local_calendar, &server_ids_calendar, - calendar_counts, + DeleteTarget { + type_name: exchange_ews_ids::CALENDAR, + table: "calendars", + counts: counts.calendar, + }, + Some(DeleteTarget { + type_name: exchange_ews_ids::CALENDAR_EVENT, + table: "calendar_events", + counts: counts.calendar_event, + }), )?; delete_vanished( conn, source_id, - exchange_ews_ids::ADDRESS_BOOK, - "address_books", &local_addressbook, &server_ids_addressbook, - addressbook_counts, + DeleteTarget { + type_name: exchange_ews_ids::ADDRESS_BOOK, + table: "address_books", + counts: counts.addressbook, + }, + Some(DeleteTarget { + type_name: exchange_ews_ids::CONTACT_CARD, + table: "contact_cards", + counts: counts.contact, + }), )?; Ok(()) } @@ -487,11 +535,10 @@ fn parent_local_id( fn delete_vanished( conn: &mut Connection, source_id: i64, - type_name: &str, - table: &str, local: &HashMap, server_ids: &[String], - counts: &mut TypeCounts, + target: DeleteTarget<'_>, + mut child: Option>, ) -> Result<(), Error> { let server_set: std::collections::HashSet<&str> = server_ids.iter().map(String::as_str).collect(); @@ -500,27 +547,62 @@ fn delete_vanished( .filter(|(id, _)| !server_set.contains(id.as_str())) .map(|(id, row)| (id.as_str(), row)) .collect(); - if type_name == exchange_ews_ids::MAILBOX { + if target.type_name == exchange_ews_ids::MAILBOX { vanished.sort_by_key(|(_, row)| std::cmp::Reverse(folder_depth(row, local))); } for (item_id, row) in vanished { + if let Some(c) = child.as_mut() { + delete_folder_children(conn, source_id, item_id, c)?; + } let tx = conn .unchecked_transaction() .map_err(|e| Error::Partial(e.to_string()))?; let result = tx.execute( - &format!("DELETE FROM {table} WHERE id = ?1"), + &format!("DELETE FROM {} WHERE id = ?1", target.table), params![row.local_id], ); match result { Ok(_) => { - exchange_ews_ids::delete_item(&tx, source_id, type_name, item_id) + exchange_ews_ids::delete_item(&tx, source_id, target.type_name, item_id) .map_err(|e| Error::Partial(e.to_string()))?; tx.commit().map_err(|e| Error::Partial(e.to_string()))?; - counts.deleted += 1; + target.counts.deleted += 1; } Err(_) => { let _ = tx.rollback(); - counts.failed += 1; + target.counts.failed += 1; + } + } + } + Ok(()) +} + +fn delete_folder_children( + conn: &mut Connection, + source_id: i64, + folder_item_id: &str, + child: &mut DeleteTarget<'_>, +) -> Result<(), Error> { + let kids = exchange_ews_ids::items_in_folder(conn, source_id, child.type_name, folder_item_id) + .map_err(|e| Error::Partial(e.to_string()))?; + for kid in kids { + let tx = conn + .unchecked_transaction() + .map_err(|e| Error::Partial(e.to_string()))?; + let deleted = tx.execute( + &format!("DELETE FROM {} WHERE id = ?1", child.table), + params![kid.local_id], + ); + match deleted { + Ok(_) => { + exchange_ews_ids::delete_item(&tx, source_id, child.type_name, &kid.item_id) + .map_err(|e| Error::Partial(e.to_string()))?; + tx.commit().map_err(|e| Error::Partial(e.to_string()))?; + child.counts.deleted += 1; + } + Err(_) => { + let _ = tx.rollback(); + child.counts.failed += 1; } } } @@ -550,6 +632,98 @@ fn folder_depth( mod tests { use super::*; + #[test] + fn vanished_folder_cascades_deletion_of_its_child_items() { + let conn = rusqlite::Connection::open_in_memory().unwrap(); + crate::db::init::apply_schema(&conn).unwrap(); + let sid = crate::db::sources::upsert_source( + &conn, + &crate::db::sources::SourceKey { + kind: "exchange_ews".to_owned(), + session_url: "https://x/EWS/Exchange.asmx".to_owned(), + account_id: "u@d".to_owned(), + }, + Some("u"), + "u@d", + ) + .unwrap(); + conn.execute( + "INSERT INTO blobs (id, hash, data) VALUES (1, X'00', X'01')", + [], + ) + .unwrap(); + conn.execute("INSERT INTO mailboxes (id, name) VALUES (10, 'Tasks')", []) + .unwrap(); + conn.execute( + "INSERT INTO emails (id, blob_id, received_at, mailbox_ids) VALUES (20, 1, '', '[10]')", + [], + ) + .unwrap(); + exchange_ews_ids::insert( + &conn, + sid, + exchange_ews_ids::MAILBOX, + "", + "FOLDER", + "CK", + 10, + ) + .unwrap(); + exchange_ews_ids::insert( + &conn, + sid, + exchange_ews_ids::EMAIL, + "FOLDER", + "MAIL", + "CK", + 20, + ) + .unwrap(); + + let mut conn = conn; + let local = + exchange_ews_ids::folders_of_type(&conn, sid, exchange_ews_ids::MAILBOX).unwrap(); + let mut folder_counts = TypeCounts::default(); + let mut email_counts = TypeCounts::default(); + delete_vanished( + &mut conn, + sid, + &local, + &[], + DeleteTarget { + type_name: exchange_ews_ids::MAILBOX, + table: "mailboxes", + counts: &mut folder_counts, + }, + Some(DeleteTarget { + type_name: exchange_ews_ids::EMAIL, + table: "emails", + counts: &mut email_counts, + }), + ) + .unwrap(); + + assert_eq!(folder_counts.deleted, 1); + assert_eq!( + email_counts.deleted, 1, + "child email must be cascade-deleted" + ); + let mailboxes: i64 = conn + .query_row("SELECT count(*) FROM mailboxes", [], |r| r.get(0)) + .unwrap(); + let emails: i64 = conn + .query_row("SELECT count(*) FROM emails", [], |r| r.get(0)) + .unwrap(); + let orphan_idmap: i64 = conn + .query_row("SELECT count(*) FROM sync_id_exchange_ews", [], |r| { + r.get(0) + }) + .unwrap(); + assert_eq!(mailboxes, 0); + assert_eq!(emails, 0, "no orphaned email row may remain"); + assert_eq!(orphan_idmap, 0, "both idmap rows must be removed"); + } + fn row(id: &str, parent_ews_id: &str, local_id: i64) -> exchange_ews_ids::FolderRow { exchange_ews_ids::FolderRow { item_id: id.to_owned(),