WebDAV import materialised the account root collection as a directory named after the account displayname (fixes #18)
This commit is contained in:
@@ -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
|
||||
|
||||
Generated
+1
-1
@@ -2901,7 +2901,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "vandelay"
|
||||
version = "1.0.6"
|
||||
version = "1.0.7"
|
||||
dependencies = [
|
||||
"base64",
|
||||
"blake3",
|
||||
|
||||
+1
-1
@@ -1,7 +1,7 @@
|
||||
[package]
|
||||
name = "vandelay"
|
||||
description = "JMAP account migration utility"
|
||||
version = "1.0.6"
|
||||
version = "1.0.7"
|
||||
authors = ["Stalwart Labs LLC <[email protected]>"]
|
||||
license = "Apache-2.0 OR MIT"
|
||||
repository = "https://github.com/stalwartlabs/vandelay"
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -270,48 +270,6 @@ fn parse_tzid_from_vtimezone(vt: Option<&str>) -> Option<String> {
|
||||
None
|
||||
}
|
||||
|
||||
pub fn upsert_root_directory(
|
||||
conn: &Connection,
|
||||
source_id: i64,
|
||||
root: &DiscoveredCollection,
|
||||
) -> Result<i64, Error> {
|
||||
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::*;
|
||||
|
||||
@@ -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<i64>,
|
||||
existing_local: Option<i64>,
|
||||
propfind_etag: String,
|
||||
propfind_content_type: Option<String>,
|
||||
@@ -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<String> = 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<i64>)> = VecDeque::new();
|
||||
queue.push_back((absolute_root, root.href.as_str().to_owned(), None));
|
||||
|
||||
let mut file_plans: Vec<FilePlan> = 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<i64>,
|
||||
}
|
||||
|
||||
struct WalkState<'a> {
|
||||
@@ -385,7 +384,7 @@ fn walk_one(
|
||||
pos: WalkPos<'_>,
|
||||
state: WalkState<'_>,
|
||||
logger: Logger,
|
||||
) -> Result<Vec<(String, String, i64)>, Error> {
|
||||
) -> Result<Vec<(String, String, Option<i64>)>, 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<i64>,
|
||||
parent_href: &str,
|
||||
) -> Result<Option<FilePlan>, 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<i64>,
|
||||
counts: &mut TypeCounts,
|
||||
) -> Result<i64, Error> {
|
||||
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,
|
||||
|
||||
@@ -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#"<?xml version="1.0" encoding="utf-8"?>
|
||||
<d:mkcol xmlns:d="DAV:" xmlns:c="urn:ietf:params:xml:ns:carddav">
|
||||
@@ -373,7 +376,9 @@ fn baikal_carddav_preserves_apple_item_labels() {
|
||||
<d:displayname>AB Labels</d:displayname>
|
||||
</d:prop></d:set>
|
||||
</d:mkcol>"#;
|
||||
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}"
|
||||
|
||||
@@ -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<i64> = 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<&str>> = Vec::new();
|
||||
|
||||
@@ -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#"<?xml version="1.0"?>
|
||||
<d:multistatus xmlns:d="DAV:">
|
||||
<d:response>
|
||||
<d:href>{url}/dav/file/u/</d:href>
|
||||
<d:propstat>
|
||||
<d:prop><d:resourcetype><d:collection/></d:resourcetype><d:displayname>System administrator</d:displayname></d:prop>
|
||||
<d:status>HTTP/1.1 200 OK</d:status>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
<d:response>
|
||||
<d:href>{url}/dav/file/u/sub/</d:href>
|
||||
<d:propstat>
|
||||
<d:prop><d:resourcetype><d:collection/></d:resourcetype><d:displayname>sub</d:displayname></d:prop>
|
||||
<d:status>HTTP/1.1 200 OK</d:status>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
<d:response>
|
||||
<d:href>{url}/dav/file/u/top.txt</d:href>
|
||||
<d:propstat>
|
||||
<d:prop><d:resourcetype/><d:getcontenttype>text/plain</d:getcontenttype><d:getetag>"t1"</d:getetag></d:prop>
|
||||
<d:status>HTTP/1.1 200 OK</d:status>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
</d:multistatus>"#
|
||||
);
|
||||
let sub_body = format!(
|
||||
r#"<?xml version="1.0"?>
|
||||
<d:multistatus xmlns:d="DAV:">
|
||||
<d:response>
|
||||
<d:href>{url}/dav/file/u/sub/</d:href>
|
||||
<d:propstat>
|
||||
<d:prop><d:resourcetype><d:collection/></d:resourcetype><d:displayname>sub</d:displayname></d:prop>
|
||||
<d:status>HTTP/1.1 200 OK</d:status>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
<d:response>
|
||||
<d:href>{url}/dav/file/u/sub/inner.txt</d:href>
|
||||
<d:propstat>
|
||||
<d:prop><d:resourcetype/><d:getcontenttype>text/plain</d:getcontenttype><d:getetag>"i1"</d:getetag></d:prop>
|
||||
<d:status>HTTP/1.1 200 OK</d:status>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
</d:multistatus>"#
|
||||
);
|
||||
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<i64> = 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<i64>) = 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<i64> = 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();
|
||||
|
||||
Reference in New Issue
Block a user