Mailbox roles must be unique per archive (fixes #8)

This commit is contained in:
Maurus Decimus
2026-06-15 21:28:27 +02:00
parent 48e38f19f3
commit c1f3f742fe
11 changed files with 336 additions and 7 deletions
+13
View File
@@ -2,6 +2,19 @@
All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/).
## [1.0.3] - 2026-06-XX
### Added
### Changed
### Fixed
- Mailbox roles must be unique per archive (#8).
# Change Log
All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/).
## [1.0.2] - 2026-06-11 ## [1.0.2] - 2026-06-11
### Added ### Added
Generated
+1 -1
View File
@@ -2939,7 +2939,7 @@ dependencies = [
[[package]] [[package]]
name = "vandelay" name = "vandelay"
version = "1.0.2" version = "1.0.3"
dependencies = [ dependencies = [
"base64", "base64",
"blake3", "blake3",
+1 -1
View File
@@ -1,7 +1,7 @@
[package] [package]
name = "vandelay" name = "vandelay"
description = "JMAP account migration utility" description = "JMAP account migration utility"
version = "1.0.2" version = "1.0.3"
authors = ["Stalwart Labs LLC <[email protected]>"] authors = ["Stalwart Labs LLC <[email protected]>"]
license = "Apache-2.0 OR MIT" license = "Apache-2.0 OR MIT"
repository = "https://github.com/stalwartlabs/vandelay" repository = "https://github.com/stalwartlabs/vandelay"
+1
View File
@@ -14,6 +14,7 @@ pub mod imap_state;
pub mod init; pub mod init;
pub mod maildir_ids; pub mod maildir_ids;
pub mod managesieve_ids; pub mod managesieve_ids;
pub mod roles;
pub mod sources; pub mod sources;
pub mod sync_state_jmap; pub mod sync_state_jmap;
pub mod takeout_ids; pub mod takeout_ids;
+90
View File
@@ -0,0 +1,90 @@
/*
* SPDX-FileCopyrightText: 2020 Stalwart Labs LLC <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0 OR MIT
*/
use rusqlite::{Connection, params};
pub fn unique_role(
conn: &Connection,
role: Option<&str>,
exclude_id: Option<i64>,
) -> rusqlite::Result<Option<String>> {
let Some(r) = role else {
return Ok(None);
};
let taken: bool = conn.query_row(
"SELECT EXISTS(SELECT 1 FROM mailboxes WHERE role = ?1 AND (?2 IS NULL OR id != ?2))",
params![r, exclude_id],
|row| row.get(0),
)?;
Ok((!taken).then(|| r.to_owned()))
}
#[cfg(test)]
mod tests {
use super::*;
use crate::db::init;
fn mem() -> Connection {
let conn = Connection::open_in_memory().unwrap();
init::apply_schema(&conn).unwrap();
conn
}
fn insert(conn: &Connection, name: &str, role: Option<&str>) -> i64 {
conn.execute(
"INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed)
VALUES (?1, NULL, ?2, 0, 1)",
params![name, role],
)
.unwrap();
conn.last_insert_rowid()
}
#[test]
fn first_claimant_keeps_role_second_is_dropped() {
let conn = mem();
let first = unique_role(&conn, Some("sent"), None).unwrap();
assert_eq!(first.as_deref(), Some("sent"));
insert(&conn, "Sent", first.as_deref());
let second = unique_role(&conn, Some("sent"), None).unwrap();
assert_eq!(second, None);
}
#[test]
fn null_role_stays_null() {
let conn = mem();
assert_eq!(unique_role(&conn, None, None).unwrap(), None);
}
#[test]
fn distinct_roles_are_independent() {
let conn = mem();
insert(&conn, "Sent", Some("sent"));
assert_eq!(
unique_role(&conn, Some("trash"), None).unwrap().as_deref(),
Some("trash")
);
}
#[test]
fn update_excludes_own_row() {
let conn = mem();
let id = insert(&conn, "Sent", Some("sent"));
assert_eq!(
unique_role(&conn, Some("sent"), Some(id)).unwrap().as_deref(),
Some("sent")
);
}
#[test]
fn update_detects_other_holder() {
let conn = mem();
insert(&conn, "Sent", Some("sent"));
let other = insert(&conn, "Envoyés", None);
assert_eq!(unique_role(&conn, Some("sent"), Some(other)).unwrap(), None);
}
}
+18 -1
View File
@@ -160,6 +160,8 @@ pub fn reconcile(
.unwrap_or(0); .unwrap_or(0);
let mut uploader = Uploader::new(net, &ctx.conn); let mut uploader = Uploader::new(net, &ctx.conn);
let mut taken_roles: HashSet<String> =
targets.iter().filter_map(|t| t.role.clone()).collect();
let interleave = ty == ObjectType::FileNode; let interleave = ty == ObjectType::FileNode;
for d in 0..=max_depth { for d in 0..=max_depth {
let level: Vec<&LocalNode> = to_create let level: Vec<&LocalNode> = to_create
@@ -225,7 +227,22 @@ pub fn reconcile(
continue; continue;
} }
match build_create(ctx, ty, n.local, maps, &mut uploader) { match build_create(ctx, ty, n.local, maps, &mut uploader) {
Ok(obj) => { Ok(mut obj) => {
if ty == ObjectType::Mailbox
&& let Some(r) = n.role.as_deref()
{
if taken_roles.contains(r) {
if let Value::Object(m) = &mut obj {
m.remove("role");
}
logger.warn(&format!(
"Mailbox local {}: role '{r}' already present on target, creating as a plain folder",
n.local
));
} else {
taken_roles.insert(r.to_owned());
}
}
batch.push((format!("c{}", n.local), obj)); batch.push((format!("c{}", n.local), obj));
} }
Err(e) => { Err(e) => {
+4
View File
@@ -380,6 +380,8 @@ fn upsert_mailbox(
.map_err(|e| Error::Partial(e.to_string()))?; .map_err(|e| Error::Partial(e.to_string()))?;
let existing = ctx.local.get(&folder.folder.folder_id.id); let existing = ctx.local.get(&folder.folder.folder_id.id);
let local_id = if let Some(row) = existing { let local_id = if let Some(row) = existing {
let role = crate::db::roles::unique_role(&tx, role, Some(row.local_id))
.map_err(|e| Error::Partial(e.to_string()))?;
tx.execute( tx.execute(
"UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3 WHERE id = ?4", "UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3 WHERE id = ?4",
params![folder.folder.display_name, parent_local, role, row.local_id], params![folder.folder.display_name, parent_local, role, row.local_id],
@@ -396,6 +398,8 @@ fn upsert_mailbox(
ctx.counts.fetched += 1; ctx.counts.fetched += 1;
row.local_id row.local_id
} else { } else {
let role = crate::db::roles::unique_role(&tx, role, None)
.map_err(|e| Error::Partial(e.to_string()))?;
tx.execute( tx.execute(
"INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed) \ "INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed) \
VALUES (?1, ?2, ?3, 0, 1)", VALUES (?1, ?2, ?3, 0, 1)",
@@ -91,6 +91,7 @@ pub fn reconcile_mail(
server_ids.push(graph_id.to_owned()); server_ids.push(graph_id.to_owned());
let local_id = if let Some(id) = existing { let local_id = if let Some(id) = existing {
let role = crate::db::roles::unique_role(&tx, role, Some(id))?;
tx.execute( tx.execute(
"UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3, is_subscribed = ?4 "UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3, is_subscribed = ?4
WHERE id = ?5", WHERE id = ?5",
@@ -99,6 +100,7 @@ pub fn reconcile_mail(
counts.fetched += 1; counts.fetched += 1;
id id
} else { } else {
let role = crate::db::roles::unique_role(&tx, role, None)?;
tx.execute( tx.execute(
"INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed) "INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed)
VALUES (?1, ?2, ?3, 0, ?4)", VALUES (?1, ?2, ?3, 0, ?4)",
+4 -2
View File
@@ -564,26 +564,28 @@ fn upsert_mailboxes(
}; };
let existing = db::imap_ids::local_for_mailbox(&tx, source_id, &folder.name)?; let existing = db::imap_ids::local_for_mailbox(&tx, source_id, &folder.name)?;
let id = if let Some(id) = existing { let id = if let Some(id) = existing {
let role = db::roles::unique_role(&tx, folder.role, Some(id))?;
tx.execute( tx.execute(
"UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3, "UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3,
is_subscribed = ?4 WHERE id = ?5", is_subscribed = ?4 WHERE id = ?5",
params![ params![
folder.leaf, folder.leaf,
parent_local, parent_local,
folder.role, role,
folder.subscribed as i64, folder.subscribed as i64,
id id
], ],
)?; )?;
id id
} else { } else {
let role = db::roles::unique_role(&tx, folder.role, None)?;
tx.execute( tx.execute(
"INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed) "INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed)
VALUES (?1, ?2, ?3, 0, ?4)", VALUES (?1, ?2, ?3, 0, ?4)",
params![ params![
folder.leaf, folder.leaf,
parent_local, parent_local,
folder.role, role,
folder.subscribed as i64 folder.subscribed as i64
], ],
)?; )?;
+4 -2
View File
@@ -113,13 +113,14 @@ pub fn insert_mailbox(
resolver: &impl LocalResolver, resolver: &impl LocalResolver,
) -> Result<i64, JmapError> { ) -> Result<i64, JmapError> {
let parent = opt_parent(resolver, ObjectType::Mailbox, &wire.parent_id); let parent = opt_parent(resolver, ObjectType::Mailbox, &wire.parent_id);
let role = crate::db::roles::unique_role(conn, wire.role.as_deref(), None)?;
conn.execute( conn.execute(
"INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed) "INSERT INTO mailboxes (name, parent_id, role, sort_order, is_subscribed)
VALUES (?1, ?2, ?3, ?4, ?5)", VALUES (?1, ?2, ?3, ?4, ?5)",
params![ params![
wire.name, wire.name,
parent, parent,
wire.role, role,
wire.sort_order, wire.sort_order,
wire.is_subscribed as i64 wire.is_subscribed as i64
], ],
@@ -401,6 +402,7 @@ pub fn update_mailbox(
resolver: &impl LocalResolver, resolver: &impl LocalResolver,
) -> Result<bool, JmapError> { ) -> Result<bool, JmapError> {
let parent = opt_parent(resolver, ObjectType::Mailbox, &wire.parent_id); let parent = opt_parent(resolver, ObjectType::Mailbox, &wire.parent_id);
let role = crate::db::roles::unique_role(conn, wire.role.as_deref(), Some(local_id))?;
let n = conn.execute( let n = conn.execute(
"UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3, sort_order = ?4, "UPDATE mailboxes SET name = ?1, parent_id = ?2, role = ?3, sort_order = ?4,
is_subscribed = ?5 is_subscribed = ?5
@@ -409,7 +411,7 @@ pub fn update_mailbox(
params![ params![
wire.name, wire.name,
parent, parent,
wire.role, role,
wire.sort_order, wire.sort_order,
wire.is_subscribed as i64, wire.is_subscribed as i64,
local_id local_id
+198
View File
@@ -2347,3 +2347,201 @@ fn first_run_cursor_is_captured_up_front_not_from_the_fetch() {
); );
let _ = std::fs::remove_file(&archive); let _ = std::fs::remove_file(&archive);
} }
#[test]
fn export_duplicate_role_mailbox_created_as_plain_folder_keeping_subtree() {
let mut server = mockito::Server::new();
let base = server.url();
let api = "/jmap/api";
let archive = tmp();
{
let conn = db::init::open(&archive).unwrap();
conn.execute(
"INSERT INTO mailboxes (id,name,parent_id,role) VALUES (1,'Inbox',NULL,'inbox')",
[],
)
.unwrap();
conn.execute(
"INSERT INTO mailboxes (id,name,parent_id,role) VALUES (2,'Sent',NULL,'sent')",
[],
)
.unwrap();
conn.execute(
"INSERT INTO mailboxes (id,name,parent_id,role) VALUES (3,'Éléments envoyés',NULL,'sent')",
[],
)
.unwrap();
conn.execute(
"INSERT INTO mailboxes (id,name,parent_id,role) VALUES (4,'Brouillons locaux',3,NULL)",
[],
)
.unwrap();
}
let _root = server.mock("GET", "/").with_status(404).create();
let _wk = server
.mock("GET", "/.well-known/jmap")
.with_body(session_body_full(&base))
.expect_at_least(1)
.create();
let _mbterm = anchor_terminator(&mut server, api, "Mailbox");
let _mq = server
.mock("POST", api)
.match_body(Matcher::Regex("Mailbox/query".into()))
.with_body(
json!({"methodResponses":[["Mailbox/query",
{"accountId":"w","ids":["TI","TS"]},"q"]]})
.to_string(),
)
.expect_at_least(1)
.create();
let _mg = server
.mock("POST", api)
.match_body(Matcher::Regex("Mailbox/get".into()))
.with_body(
json!({"methodResponses":[["Mailbox/get",{"accountId":"w","list":[
{"id":"TI","name":"Inbox","role":"inbox","parentId":null,"myRights":{"mayDelete":true}},
{"id":"TS","name":"Sent","role":"sent","parentId":null,"myRights":{"mayDelete":true}}
],"notFound":[]},"g"]]})
.to_string(),
)
.expect_at_least(1)
.create();
let role_create = server
.mock("POST", api)
.match_body(Matcher::AllOf(vec![
Matcher::Regex("Mailbox/set".into()),
Matcher::Regex("envoy".into()),
Matcher::Regex("\"role\"".into()),
]))
.with_body(
json!({"methodResponses":[["Mailbox/set",{"accountId":"w",
"notCreated":{"c3":{"type":"invalidProperties","properties":["role"],
"description":"A mailbox with role 'sent' already exists."}}},"s"]]})
.to_string(),
)
.expect_at_most(1)
.create();
let folder_create = server
.mock("POST", api)
.match_body(Matcher::AllOf(vec![
Matcher::Regex("Mailbox/set".into()),
Matcher::Regex("envoy".into()),
]))
.with_body(
json!({"methodResponses":[["Mailbox/set",{"accountId":"w",
"created":{"c3":{"id":"M3"}}},"s"]]})
.to_string(),
)
.expect(1)
.create();
let child_create = server
.mock("POST", api)
.match_body(Matcher::AllOf(vec![
Matcher::Regex("Mailbox/set".into()),
Matcher::Regex("Brouillons".into()),
]))
.with_body(
json!({"methodResponses":[["Mailbox/set",{"accountId":"w",
"created":{"c4":{"id":"M4"}}},"s"]]})
.to_string(),
)
.expect(1)
.create();
let summary = sync::export::run(
common(&archive),
export_cfg_objects(&base, vec![ObjectType::Mailbox]),
)
.expect("export");
role_create.assert();
folder_create.assert();
child_create.assert();
let counts = summary
.per_type
.iter()
.find(|(t, _)| *t == "Mailbox")
.map(|(_, c)| c.clone())
.expect("mailbox counts");
assert_eq!(counts.skipped, 2, "Inbox and Sent match existing role mailboxes");
assert_eq!(
counts.created, 2,
"duplicate-role folder and its child are both created"
);
assert_eq!(counts.failed, 0, "no cascade skip of the subtree");
let _ = std::fs::remove_file(&archive);
}
#[test]
fn import_jmap_duplicate_role_is_deduplicated_to_single_mailbox() {
let mut server = mockito::Server::new();
let base = server.url();
let api = "/jmap/api";
let archive = tmp();
let _root = server.mock("GET", "/").with_status(404).create();
let _wk = server
.mock("GET", "/.well-known/jmap")
.with_body(session_body_full(&base))
.expect_at_least(1)
.create();
let _term = anchor_terminator(&mut server, api, "Mailbox");
let _q = server
.mock("POST", api)
.match_body(Matcher::Regex("Mailbox/query".into()))
.with_body(
json!({"methodResponses":[["Mailbox/query",
{"accountId":"w","ids":["A","B","C"]},"q"]]})
.to_string(),
)
.expect(1)
.create();
let _g = server
.mock("POST", api)
.match_body(Matcher::Regex("Mailbox/get".into()))
.with_body(
json!({"methodResponses":[["Mailbox/get",{"accountId":"w","state":"s1","list":[
{"id":"A","name":"Sent","parentId":null,"role":"sent","sortOrder":0,"isSubscribed":true},
{"id":"B","name":"Éléments envoyés","parentId":null,"role":"sent","sortOrder":0,"isSubscribed":true},
{"id":"C","name":"Sent Items","parentId":null,"role":"sent","sortOrder":0,"isSubscribed":true}
],"notFound":[]},"g"]]})
.to_string(),
)
.expect(1)
.create();
sync::import_jmap::run(
common(&archive),
import_cfg_objects(&base, vec![ObjectType::Mailbox]),
)
.expect("import");
let conn = rusqlite::Connection::open(&archive).unwrap();
let total: i64 = conn
.query_row("SELECT count(*) FROM mailboxes", [], |r| r.get(0))
.unwrap();
assert_eq!(total, 3, "all three folders are imported");
let with_role: i64 = conn
.query_row(
"SELECT count(*) FROM mailboxes WHERE role = 'sent'",
[],
|r| r.get(0),
)
.unwrap();
assert_eq!(with_role, 1, "exactly one mailbox keeps the sent role");
let null_roles: i64 = conn
.query_row(
"SELECT count(*) FROM mailboxes WHERE role IS NULL",
[],
|r| r.get(0),
)
.unwrap();
assert_eq!(null_roles, 2, "the surplus duplicates become plain folders");
drop(conn);
let _ = std::fs::remove_file(&archive);
}