diff --git a/CHANGELOG.md b/CHANGELOG.md index 74a5523..502df3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [1.0.7] - 2026-07-XX + +### Added + +### Changed + +### Fixed +- WebDAV import materialised the account root collection as a directory named after the account displayname (#18). + ## [1.0.6] - 2026-07-12 ### Added diff --git a/Cargo.lock b/Cargo.lock index 6a223b9..4fd1104 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2901,7 +2901,7 @@ dependencies = [ [[package]] name = "vandelay" -version = "1.0.6" +version = "1.0.7" dependencies = [ "base64", "blake3", diff --git a/Cargo.toml b/Cargo.toml index 46d391b..e137a88 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "vandelay" description = "JMAP account migration utility" -version = "1.0.6" +version = "1.0.7" authors = ["Stalwart Labs LLC "] license = "Apache-2.0 OR MIT" repository = "https://github.com/stalwartlabs/vandelay" diff --git a/src/sync/export/tree.rs b/src/sync/export/tree.rs index b219dfa..272de1c 100644 --- a/src/sync/export/tree.rs +++ b/src/sync/export/tree.rs @@ -224,7 +224,8 @@ pub fn reconcile( } }, }; - if let Some(tid) = find_name_collision(&targets, parent_target.as_deref(), &n.name) { + if let Some(tid) = find_name_collision(&targets, parent_target.as_deref(), &n.name) + { record_merge( ty, n.local, diff --git a/src/sync/import_dav/collections.rs b/src/sync/import_dav/collections.rs index 4261308..9507468 100644 --- a/src/sync/import_dav/collections.rs +++ b/src/sync/import_dav/collections.rs @@ -270,48 +270,6 @@ fn parse_tzid_from_vtimezone(vt: Option<&str>) -> Option { None } -pub fn upsert_root_directory( - conn: &Connection, - source_id: i64, - root: &DiscoveredCollection, -) -> Result { - let collection_href = root.href.as_str().to_owned(); - let name = display_or_fallback(&root.props.displayname, &root.href); - if let Some(local) = - dav_ids::local_for_item(conn, source_id, dav_ids::FILE_NODE, &collection_href) - .map_err(|e| Error::Partial(e.to_string()))? - { - conn.execute( - "UPDATE file_nodes SET name = ?1 WHERE id = ?2", - params![name, local], - ) - .map_err(|e| Error::Partial(e.to_string()))?; - return Ok(local); - } - let now = time::OffsetDateTime::now_utc() - .format(&time::format_description::well_known::Rfc3339) - .map_err(|e| Error::Partial(format!("clock: {e}")))?; - conn.execute( - "INSERT INTO file_nodes (parent_id, node_type, blob_id, target, name, media_type, - created, modified, is_subscribed, role) - VALUES (NULL, 'directory', NULL, NULL, ?1, NULL, ?2, NULL, 1, NULL)", - params![name, now], - ) - .map_err(|e| Error::Partial(e.to_string()))?; - let local_id = conn.last_insert_rowid(); - dav_ids::insert( - conn, - source_id, - dav_ids::FILE_NODE, - &collection_href, - &collection_href, - "", - local_id, - ) - .map_err(|e| Error::Partial(e.to_string()))?; - Ok(local_id) -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/sync/import_dav/tree.rs b/src/sync/import_dav/tree.rs index 4aaa35f..e2a230d 100644 --- a/src/sync/import_dav/tree.rs +++ b/src/sync/import_dav/tree.rs @@ -26,7 +26,7 @@ use crate::sync::import_jmap::pool::Pool; struct FilePlan { item_href: String, parent_href: String, - parent_local: i64, + parent_local: Option, existing_local: Option, propfind_etag: String, propfind_content_type: Option, @@ -73,7 +73,6 @@ pub fn reconcile_filenodes( let dav_connections = ctx.dav_connections; let logger = ctx.logger; let absolute_root = absolute(base_url, root.href.as_str())?; - let root_local = super::collections::upsert_root_directory(conn, source_id, root)?; let known: Vec<(String, i64)> = dav_ids::collections_of_type(conn, source_id, dav_ids::FILE_NODE) @@ -85,8 +84,8 @@ pub fn reconcile_filenodes( let mut visited: HashSet = HashSet::new(); visited.insert(root.href.as_str().to_owned()); - let mut queue: VecDeque<(String, String, i64)> = VecDeque::new(); - queue.push_back((absolute_root, root.href.as_str().to_owned(), root_local)); + let mut queue: VecDeque<(String, String, Option)> = VecDeque::new(); + queue.push_back((absolute_root, root.href.as_str().to_owned(), None)); let mut file_plans: Vec = Vec::new(); @@ -369,7 +368,7 @@ fn delete_vanished( struct WalkPos<'a> { url: &'a str, parent_href: &'a str, - parent_local: i64, + parent_local: Option, } struct WalkState<'a> { @@ -385,7 +384,7 @@ fn walk_one( pos: WalkPos<'_>, state: WalkState<'_>, logger: Logger, -) -> Result, Error> { +) -> Result)>, Error> { let url = pos.url; let parent_href = pos.parent_href; let parent_local = pos.parent_local; @@ -411,7 +410,7 @@ fn walk_one( if r.props.is_collection { let local = upsert_directory(conn, source_id, &r, parent_local, counts)?; let abs = absolute(url, r.href.as_str())?; - children.push((abs, r.href.as_str().to_owned(), local)); + children.push((abs, r.href.as_str().to_owned(), Some(local))); } else { match plan_file(conn, source_id, &r, url, parent_local, parent_href) { Ok(Some(plan)) => file_plans.push(plan), @@ -439,7 +438,7 @@ fn plan_file( source_id: i64, response: &DavResponse, collection_url: &str, - parent_local: i64, + parent_local: Option, parent_href: &str, ) -> Result, Error> { let item_href = response.href.as_str().to_owned(); @@ -496,7 +495,7 @@ fn upsert_directory( conn: &mut Connection, source_id: i64, response: &DavResponse, - parent_local: i64, + parent_local: Option, counts: &mut TypeCounts, ) -> Result { let item_href = response.href.as_str().to_owned(); @@ -625,7 +624,7 @@ mod tests { FilePlan { item_href: href.to_owned(), parent_href: "/dav/file/u/".to_owned(), - parent_local: 1, + parent_local: Some(1), existing_local: None, propfind_etag: String::new(), propfind_content_type: None, diff --git a/tests/integration_baikal.rs b/tests/integration_baikal.rs index 063084e..1cc929f 100644 --- a/tests/integration_baikal.rs +++ b/tests/integration_baikal.rs @@ -363,8 +363,11 @@ fn baikal_carddav_preserves_apple_item_labels() { let account = &b.accounts[0]; let dav_root = b.dav_root(); - let client = - integration::dav_client::DavSeed::new(dav_root.clone(), &account.username, &account.password); + let client = integration::dav_client::DavSeed::new( + dav_root.clone(), + &account.username, + &account.password, + ); let book = format!("/addressbooks/{}/ablabels/", account.username); let mkbook = r#" @@ -373,7 +376,9 @@ fn baikal_carddav_preserves_apple_item_labels() { AB Labels "#; - client.mkcol(&book, Some(mkbook)).expect("mkcol addressbook"); + client + .mkcol(&book, Some(mkbook)) + .expect("mkcol addressbook"); let uid = "apple-item-labels-1"; let vcard = format!( @@ -425,7 +430,14 @@ fn baikal_carddav_preserves_apple_item_labels() { drop(conn); eprintln!("archived JSContact:\n{data}"); - for needle in ["x-abdate", "x-ablabel", "20171111", "20111111", "Name1", "Name2"] { + for needle in [ + "x-abdate", + "x-ablabel", + "20171111", + "20111111", + "Name1", + "Name2", + ] { assert!( data.to_lowercase().contains(&needle.to_lowercase()), "Baikal CardDAV import dropped {needle:?}; stored: {data}" diff --git a/tests/integration_webdav.rs b/tests/integration_webdav.rs index 7af3120..dd4a0f5 100644 --- a/tests/integration_webdav.rs +++ b/tests/integration_webdav.rs @@ -89,8 +89,8 @@ fn webdav_starts_seeds_and_imports() { |r| r.get(0), ) .unwrap(); - let expected_dirs = account.layout.files.iter().filter(|s| s.directory).count() + 1; - let expected_files = account.layout.files.len() - (expected_dirs - 1); + let expected_dirs = account.layout.files.iter().filter(|s| s.directory).count(); + let expected_files = account.layout.files.len() - expected_dirs; assert_eq!( files as usize, expected_files, @@ -99,16 +99,43 @@ fn webdav_starts_seeds_and_imports() { ); assert_eq!( dirs as usize, expected_dirs, - "{}: directory count mismatch (seeded layout dirs + account root = {expected_dirs}, imported {dirs})", + "{}: directory count mismatch (seeded layout dirs = {expected_dirs}, imported {dirs}); the account root is a virtual mount point, not a node (issue #18)", seed.username ); assert_eq!( nodes, - account.layout.files.len() + 1, - "{}: total file_nodes mismatch (layout + account root)", + account.layout.files.len(), + "{}: total file_nodes mismatch (layout only, no synthetic account-root node)", seed.username ); + let admin_root_nodes: i64 = conn + .query_row( + "SELECT count(*) FROM file_nodes WHERE name = ?1", + [&account.username], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + admin_root_nodes, 0, + "{}: the account root must not be materialised as a directory named after the account (issue #18)", + seed.username + ); + for spec in account.layout.files.iter().filter(|s| s.parent.is_none()) { + let parent_id: Option = conn + .query_row( + "SELECT parent_id FROM file_nodes WHERE name = ?1", + [spec.name], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + parent_id, None, + "{}: top-level node {} must map to the target's implicit root (NULL parent), not nest under an account-root directory (issue #18)", + seed.username, spec.name + ); + } + for spec in account.layout.files { let segments = layout_segments(account.layout.files, spec.key); let mut probes: Vec> = Vec::new(); diff --git a/tests/mock_dav.rs b/tests/mock_dav.rs index 269d161..41e0581 100644 --- a/tests/mock_dav.rs +++ b/tests/mock_dav.rs @@ -984,6 +984,173 @@ fn webdav_discovery_keeps_only_self_row_as_root() { assert!(disc.collections[0].href.as_str().ends_with("/files/u/")); } +#[test] +fn webdav_root_collection_is_not_materialised_children_map_to_target_root() { + use rusqlite::Connection; + use vandelay::dav::discover::DiscoveredCollection; + use vandelay::dav::href::Href; + use vandelay::dav::parse::ResourceProps; + use vandelay::db; + use vandelay::db::sources::SourceKey; + use vandelay::logging::Logger; + use vandelay::sync::TypeCounts; + use vandelay::sync::import_dav::tree::{WebDavCtx, reconcile_filenodes}; + + let mut server = mockito::Server::new(); + let url = server.url(); + + let root_body = format!( + r#" + + + {url}/dav/file/u/ + + System administrator + HTTP/1.1 200 OK + + + + {url}/dav/file/u/sub/ + + sub + HTTP/1.1 200 OK + + + + {url}/dav/file/u/top.txt + + text/plain"t1" + HTTP/1.1 200 OK + + +"# + ); + let sub_body = format!( + r#" + + + {url}/dav/file/u/sub/ + + sub + HTTP/1.1 200 OK + + + + {url}/dav/file/u/sub/inner.txt + + text/plain"i1" + HTTP/1.1 200 OK + + +"# + ); + let _root = multistatus_response(&mut server, "PROPFIND", "/dav/file/u/", &root_body); + let _sub = multistatus_response(&mut server, "PROPFIND", "/dav/file/u/sub/", &sub_body); + let _f1 = server + .mock("GET", "/dav/file/u/top.txt") + .with_status(200) + .with_header("content-type", "text/plain") + .with_body("toplevel\n") + .create(); + let _f2 = server + .mock("GET", "/dav/file/u/sub/inner.txt") + .with_status(200) + .with_header("content-type", "text/plain") + .with_body("inner\n") + .create(); + + let mut conn = Connection::open_in_memory().unwrap(); + db::init::apply_schema(&conn).unwrap(); + let source_id = db::sources::upsert_source( + &conn, + &SourceKey { + kind: "webdav".to_owned(), + session_url: url.clone(), + account_id: format!("{url}/dav/file/u/"), + }, + Some("u"), + "u", + ) + .unwrap(); + + let root = DiscoveredCollection { + url: format!("{url}/dav/file/u/"), + href: Href::from_normalised("/dav/file/u/".to_owned()), + props: ResourceProps { + is_collection: true, + displayname: Some("System administrator".to_owned()), + ..Default::default() + }, + }; + let c = client(0); + let ctx = WebDavCtx { + client: &c, + source_id, + base_url: &url, + dav_connections: 2, + logger: Logger::from_flags(false, 0), + }; + let mut counts = TypeCounts::default(); + reconcile_filenodes(&mut conn, &ctx, &root, &mut counts).expect("reconcile"); + + let total: i64 = conn + .query_row("SELECT count(*) FROM file_nodes", [], |r| r.get(0)) + .unwrap(); + assert_eq!( + total, 3, + "only sub/, top.txt and inner.txt land; the root collection is a virtual mount point, not a node" + ); + + let admin_named: i64 = conn + .query_row( + "SELECT count(*) FROM file_nodes WHERE name = 'System administrator'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + admin_named, 0, + "the account display name must not become a directory (issue #18)" + ); + + let top_parent: Option = conn + .query_row( + "SELECT parent_id FROM file_nodes WHERE name = 'top.txt'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + top_parent, None, + "a file at the root maps to the target's implicit root (NULL parent)" + ); + + let (sub_id, sub_parent): (i64, Option) = conn + .query_row( + "SELECT id, parent_id FROM file_nodes WHERE name = 'sub'", + [], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .unwrap(); + assert_eq!( + sub_parent, None, + "a directory at the root maps to the target's implicit root (NULL parent)" + ); + + let inner_parent: Option = conn + .query_row( + "SELECT parent_id FROM file_nodes WHERE name = 'inner.txt'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + inner_parent, + Some(sub_id), + "a nested file keeps its real parent directory" + ); +} + #[test] fn streaming_propfind_strips_control_chars_before_parse() { let mut server = mockito::Server::new();