fix(store): deterministic message surrogate identity (C1) #337
No files matched your search
@@ -185,6 +185,8 @@ mod tests {
|
||||
id: "msg-100".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "INBOX".to_string(),
|
||||
remote_id: "100".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Test Subject 1".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -198,6 +200,8 @@ mod tests {
|
||||
id: "msg-101".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "INBOX".to_string(),
|
||||
remote_id: "101".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Test Subject 2".to_string(),
|
||||
sender: "charlie@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use bytes::Bytes;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use sha2::{Digest, Sha256};
|
||||
|
||||
/// Real-time daemon health, performance metrics, and telemetry payload.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
@@ -63,12 +64,30 @@ pub struct Attachment {
|
||||
/// Email message domain entity owned by Nuncio core.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct Email {
|
||||
/// Unique message identifier.
|
||||
/// Opaque, deterministic surrogate identifier for the message.
|
||||
///
|
||||
/// Derived from [`Email::surrogate_id`] over the account, folder,
|
||||
/// UIDVALIDITY, and protocol-native id. It carries no decodable internal
|
||||
/// encoding (callers must treat it as opaque) and is stable across
|
||||
/// re-syncs of the same message, so persisting it upserts the same row
|
||||
/// rather than duplicating. Distinct messages that happen to share a
|
||||
/// protocol id across folders or accounts hash to distinct surrogates, so
|
||||
/// one can never silently overwrite another.
|
||||
pub id: String,
|
||||
/// Account identifier owning the message.
|
||||
pub account_id: String,
|
||||
/// Mailbox folder identifier (e.g. "inbox").
|
||||
pub folder_id: String,
|
||||
/// Protocol-native message id used to address the message on its server:
|
||||
/// the IMAP UID as a decimal string, or the JMAP Email object id. This is
|
||||
/// what a remote mutation must use to identify the message on the wire --
|
||||
/// never the opaque [`Email::id`].
|
||||
pub remote_id: String,
|
||||
/// The addressing scope the `remote_id` is only meaningful within: the IMAP
|
||||
/// folder UIDVALIDITY as a decimal string. Protocols without a UIDVALIDITY
|
||||
/// (JMAP) carry a stable sentinel instead, so the surrogate id stays
|
||||
/// deterministic.
|
||||
pub uid_validity: String,
|
||||
/// Subject line.
|
||||
pub subject: String,
|
||||
/// Sender address (e.g. "alice@nuncio.mx").
|
||||
@@ -87,6 +106,33 @@ pub struct Email {
|
||||
pub attachments: Vec<Attachment>,
|
||||
}
|
||||
|
||||
impl Email {
|
||||
/// Compute the deterministic, collision-resistant surrogate id for a
|
||||
/// message from its addressing coordinates.
|
||||
///
|
||||
/// The four coordinates together uniquely identify a stored message: a
|
||||
/// protocol-native `remote_id` (IMAP UID / JMAP object id) is only unique
|
||||
/// within one `uid_validity` scope of one `folder_id` of one `account_id`,
|
||||
/// so all four must participate. Each field is fed length-prefixed so that
|
||||
/// no two distinct coordinate tuples can serialize to the same byte stream
|
||||
/// (e.g. `("a", "bc")` and `("ab", "c")` must not collide). The SHA-256
|
||||
/// digest is hex-encoded, yielding a stable opaque token that leaks none of
|
||||
/// its inputs' encoding on the wire.
|
||||
pub fn surrogate_id(
|
||||
account_id: &str,
|
||||
folder_id: &str,
|
||||
uid_validity: &str,
|
||||
remote_id: &str,
|
||||
) -> String {
|
||||
let mut hasher = Sha256::new();
|
||||
for field in [account_id, folder_id, uid_validity, remote_id] {
|
||||
hasher.update((field.len() as u64).to_le_bytes());
|
||||
hasher.update(field.as_bytes());
|
||||
}
|
||||
hex::encode(hasher.finalize())
|
||||
}
|
||||
}
|
||||
|
||||
/// Mailbox folder entity.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct Folder {
|
||||
@@ -168,4 +214,37 @@ mod tests {
|
||||
assert_eq!(parsed.status, "HEALTHY");
|
||||
assert_eq!(parsed.total_emails, 15000);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn surrogate_id_is_deterministic_and_opaque() {
|
||||
let a = Email::surrogate_id("acct-1", "INBOX", "42", "5");
|
||||
let b = Email::surrogate_id("acct-1", "INBOX", "42", "5");
|
||||
assert_eq!(a, b, "identical coordinates must hash identically");
|
||||
// A 32-byte SHA-256 digest hex-encodes to 64 chars, and the token must
|
||||
// not leak the protocol-native id it was built from.
|
||||
assert_eq!(a.len(), 64);
|
||||
assert!(a.chars().all(|c| c.is_ascii_hexdigit()));
|
||||
assert!(!a.contains("imap-uid-"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn surrogate_id_distinguishes_the_cross_folder_and_cross_account_collision() {
|
||||
// The same protocol UID (5) in two different folders of one account, or
|
||||
// in two different accounts, must NOT collapse to the same surrogate.
|
||||
let inbox = Email::surrogate_id("acct-1", "INBOX", "42", "5");
|
||||
let sent = Email::surrogate_id("acct-1", "Sent", "7", "5");
|
||||
let other_account = Email::surrogate_id("acct-2", "INBOX", "42", "5");
|
||||
assert_ne!(inbox, sent, "same UID in two folders must not collide");
|
||||
assert_ne!(
|
||||
inbox, other_account,
|
||||
"same UID in two accounts must not collide"
|
||||
);
|
||||
|
||||
// Field boundaries are unambiguous: shifting a delimiter-like split
|
||||
// across two adjacent fields yields a different id.
|
||||
assert_ne!(
|
||||
Email::surrogate_id("a", "bc", "1", "1"),
|
||||
Email::surrogate_id("ab", "c", "1", "1"),
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -373,6 +373,8 @@ mod tests {
|
||||
id: "msg-1".to_string(),
|
||||
account_id: account_id.to_string(),
|
||||
folder_id: folder_id.to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -394,6 +396,8 @@ mod tests {
|
||||
id: "msg-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Urgent Meeting".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
|
||||
@@ -36,25 +36,31 @@ pub enum RemoteMutationKind {
|
||||
Delete,
|
||||
}
|
||||
|
||||
/// A fully-addressed remote mutation for a single message: which message
|
||||
/// (protocol-native id), the folder it currently lives in, that folder's
|
||||
/// stored sync checkpoint (carrying IMAP UIDVALIDITY), and the change to
|
||||
/// apply.
|
||||
/// A fully-addressed remote mutation for a single message: the protocol-native
|
||||
/// id to address it on the server, the folder it currently lives in, the
|
||||
/// UIDVALIDITY scope that id was captured under, and the change to apply.
|
||||
///
|
||||
/// `folder_checkpoint` is the value persisted by a prior sync
|
||||
/// (`"{uidvalidity}:{uidnext}"` for IMAP). The IMAP backend uses it to refuse
|
||||
/// to act when the mailbox's current UIDVALIDITY no longer matches -- a
|
||||
/// renumbered mailbox reassigns UIDs, so the stored UID would otherwise
|
||||
/// address the wrong message. Protocols that do not use UIDVALIDITY (JMAP)
|
||||
/// ignore it.
|
||||
/// Addressing is driven by `remote_id` (the IMAP UID / JMAP object id), NOT by
|
||||
/// the opaque `message_id` -- so remote-mutation targeting never depends on the
|
||||
/// shape of the surrogate id. `uid_validity` is the mailbox UIDVALIDITY the
|
||||
/// `remote_id` was captured under; the IMAP backend refuses to act unless the
|
||||
/// mailbox's current UIDVALIDITY still matches it, because a renumbered mailbox
|
||||
/// (RFC 3501 s2.3.1.1) reassigns UIDs and the stored UID would otherwise
|
||||
/// address the wrong message. Protocols without a UIDVALIDITY (JMAP) carry a
|
||||
/// stable sentinel and ignore this guard.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct RemoteMutationSpec {
|
||||
/// Protocol-native message id (IMAP `imap-uid-{uid}`, JMAP object id).
|
||||
/// Opaque surrogate id of the target message. Diagnostic only -- it names
|
||||
/// the message in errors/logs and is never used to address it on the wire.
|
||||
pub message_id: String,
|
||||
/// Protocol-native id used to address the message on its server: the IMAP
|
||||
/// UID as a decimal string, or the JMAP Email object id.
|
||||
pub remote_id: String,
|
||||
/// Folder the message currently lives in.
|
||||
pub folder_id: String,
|
||||
/// The source folder's stored sync checkpoint, if any.
|
||||
pub folder_checkpoint: Option<String>,
|
||||
/// The UIDVALIDITY the `remote_id` was captured under (IMAP; decimal
|
||||
/// string), or a protocol sentinel for servers without UIDVALIDITY (JMAP).
|
||||
pub uid_validity: String,
|
||||
/// The change to apply.
|
||||
pub kind: RemoteMutationKind,
|
||||
}
|
||||
|
||||
@@ -34,31 +34,21 @@ const FETCH_ITEM_TIMEOUT: Duration = Duration::from_secs(60);
|
||||
use crate::backend::{MailBackend, RemoteMutationKind, RemoteMutationSpec};
|
||||
use crate::parser::{MailError, MimeParserAdapter};
|
||||
|
||||
/// Prefix an IMAP [`Email::id`] carries in front of its raw numeric UID (see
|
||||
/// [`ImapEngine::sync_folder_messages_with_session`], which formats every id as
|
||||
/// `"imap-uid-{uid}"`). A remote mutation must recover the UID from the id to
|
||||
/// address the message, so the prefix is defined once here.
|
||||
const IMAP_UID_ID_PREFIX: &str = "imap-uid-";
|
||||
|
||||
/// Recover the raw IMAP UID from an [`Email::id`] of the form
|
||||
/// `"imap-uid-{uid}"`. Returns `None` for any id that does not carry the
|
||||
/// expected prefix and a valid non-zero `u32` UID, so a mutation against a
|
||||
/// malformed/foreign id fails honestly rather than addressing UID 0.
|
||||
fn parse_imap_uid(message_id: &str) -> Option<u32> {
|
||||
let raw = message_id.strip_prefix(IMAP_UID_ID_PREFIX)?;
|
||||
let uid = raw.trim().parse::<u32>().ok()?;
|
||||
/// Recover a raw IMAP UID from a [`RemoteMutationSpec::remote_id`] (a decimal
|
||||
/// string). Returns `None` for anything that is not a valid non-zero `u32`, so
|
||||
/// a mutation against a malformed/foreign remote id fails honestly rather than
|
||||
/// addressing UID 0.
|
||||
fn parse_imap_uid(remote_id: &str) -> Option<u32> {
|
||||
let uid = remote_id.trim().parse::<u32>().ok()?;
|
||||
(uid >= 1).then_some(uid)
|
||||
}
|
||||
|
||||
/// Extract the UIDVALIDITY component of a stored folder checkpoint
|
||||
/// (`"{uidvalidity}:{uidnext}"`). Returns `None` for a missing, malformed, or
|
||||
/// legacy bare-number checkpoint -- the caller treats an absent stored
|
||||
/// UIDVALIDITY as "cannot prove the mailbox has not been renumbered" and
|
||||
/// refuses to act.
|
||||
fn checkpoint_uid_validity(checkpoint: Option<&str>) -> Option<u32> {
|
||||
let raw = checkpoint?;
|
||||
let (validity, _uid) = raw.split_once(CHECKPOINT_DELIM)?;
|
||||
validity.trim().parse::<u32>().ok()
|
||||
/// Parse a stored [`RemoteMutationSpec::uid_validity`] (a decimal string) into
|
||||
/// a `u32`. Returns `None` for a missing or non-numeric value -- the caller
|
||||
/// treats an unparseable UIDVALIDITY as "cannot prove the mailbox has not been
|
||||
/// renumbered" and refuses to act.
|
||||
fn parse_uid_validity(uid_validity: &str) -> Option<u32> {
|
||||
uid_validity.trim().parse::<u32>().ok()
|
||||
}
|
||||
|
||||
/// Delimiter between the two components of a folder sync checkpoint. The
|
||||
@@ -652,7 +642,16 @@ impl ImapEngine {
|
||||
|
||||
let uid_num = fetch_data.uid.unwrap_or(0);
|
||||
max_uid_seen = max_uid_seen.max(uid_num);
|
||||
let email_id = format!("imap-uid-{}", uid_num);
|
||||
// The message is addressed on the wire by its UID within the
|
||||
// folder's UIDVALIDITY scope; the persisted id is an opaque
|
||||
// surrogate hashed over both plus the account and folder, so the
|
||||
// same UID in another folder/account can never collide.
|
||||
let remote_id = uid_num.to_string();
|
||||
let uid_validity = server_uid_validity
|
||||
.map(|v| v.to_string())
|
||||
.unwrap_or_default();
|
||||
let email_id =
|
||||
Email::surrogate_id(&self.account_id, folder_id, &uid_validity, &remote_id);
|
||||
|
||||
let is_read = fetch_data
|
||||
.flags()
|
||||
@@ -665,6 +664,8 @@ impl ImapEngine {
|
||||
folder_id,
|
||||
raw_bytes,
|
||||
)?;
|
||||
email.remote_id = remote_id;
|
||||
email.uid_validity = uid_validity;
|
||||
email.read = is_read;
|
||||
email
|
||||
} else {
|
||||
@@ -721,6 +722,8 @@ impl ImapEngine {
|
||||
id: email_id,
|
||||
account_id: self.account_id.clone(),
|
||||
folder_id: folder_id.to_string(),
|
||||
remote_id,
|
||||
uid_validity,
|
||||
subject,
|
||||
sender,
|
||||
recipient,
|
||||
@@ -815,10 +818,10 @@ impl ImapEngine {
|
||||
where
|
||||
S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static,
|
||||
{
|
||||
let uid = parse_imap_uid(&spec.message_id).ok_or_else(|| {
|
||||
let uid = parse_imap_uid(&spec.remote_id).ok_or_else(|| {
|
||||
MailError::ImapError(format!(
|
||||
"cannot parse an IMAP UID from message id '{}'",
|
||||
spec.message_id
|
||||
"cannot parse an IMAP UID from remote id '{}' (message '{}')",
|
||||
spec.remote_id, spec.message_id
|
||||
))
|
||||
})?;
|
||||
|
||||
@@ -831,7 +834,7 @@ impl ImapEngine {
|
||||
|
||||
// Refuse to act unless the mailbox's current UIDVALIDITY provably
|
||||
// matches the one the stored UID was captured under.
|
||||
let stored_validity = checkpoint_uid_validity(spec.folder_checkpoint.as_deref());
|
||||
let stored_validity = parse_uid_validity(&spec.uid_validity);
|
||||
match (stored_validity, mailbox.uid_validity) {
|
||||
(Some(stored), Some(current)) if stored == current => {}
|
||||
(stored, current) => {
|
||||
@@ -1575,23 +1578,24 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_imap_uid_recovers_uid_and_rejects_malformed_ids() {
|
||||
assert_eq!(parse_imap_uid("imap-uid-42"), Some(42));
|
||||
assert_eq!(parse_imap_uid("imap-uid-1"), Some(1));
|
||||
// UID 0 is not a valid IMAP UID; a bare/foreign id yields nothing.
|
||||
assert_eq!(parse_imap_uid("imap-uid-0"), None);
|
||||
assert_eq!(parse_imap_uid("42"), None);
|
||||
fn parse_imap_uid_recovers_uid_and_rejects_non_numeric_ids() {
|
||||
assert_eq!(parse_imap_uid("42"), Some(42));
|
||||
assert_eq!(parse_imap_uid(" 42 "), Some(42));
|
||||
assert_eq!(parse_imap_uid("1"), Some(1));
|
||||
// UID 0 is not a valid IMAP UID; a non-numeric/foreign id yields nothing.
|
||||
assert_eq!(parse_imap_uid("0"), None);
|
||||
assert_eq!(parse_imap_uid("imap-uid-42"), None);
|
||||
assert_eq!(parse_imap_uid("jmap-object-id"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn checkpoint_uid_validity_extracts_only_a_well_formed_validity() {
|
||||
assert_eq!(checkpoint_uid_validity(Some("7:105")), Some(7));
|
||||
assert_eq!(checkpoint_uid_validity(Some(" 7 : 105 ")), Some(7));
|
||||
assert_eq!(checkpoint_uid_validity(None), None);
|
||||
// A legacy bare-number checkpoint carries no validity to compare.
|
||||
assert_eq!(checkpoint_uid_validity(Some("105")), None);
|
||||
assert_eq!(checkpoint_uid_validity(Some("x:105")), None);
|
||||
fn parse_uid_validity_accepts_only_a_decimal_value() {
|
||||
assert_eq!(parse_uid_validity("7"), Some(7));
|
||||
assert_eq!(parse_uid_validity(" 7 "), Some(7));
|
||||
assert_eq!(parse_uid_validity(""), None);
|
||||
// The JMAP sentinel and a full checkpoint string are not a bare validity.
|
||||
assert_eq!(parse_uid_validity("jmap"), None);
|
||||
assert_eq!(parse_uid_validity("7:105"), None);
|
||||
}
|
||||
|
||||
/// Drive a mutation over a scripted IMAP server (in-memory duplex),
|
||||
@@ -1697,10 +1701,18 @@ mod tests {
|
||||
}
|
||||
|
||||
fn imap_spec(kind: RemoteMutationKind, checkpoint: Option<&str>) -> RemoteMutationSpec {
|
||||
// The tests express the captured scope as a "{uidvalidity}:{uidnext}"
|
||||
// checkpoint string (mirroring how a sync stores it); the spec now
|
||||
// carries just the UIDVALIDITY component the guard compares against.
|
||||
let uid_validity = checkpoint
|
||||
.and_then(|c| c.split(':').next())
|
||||
.unwrap_or_default()
|
||||
.to_string();
|
||||
RemoteMutationSpec {
|
||||
message_id: "imap-uid-42".to_string(),
|
||||
message_id: "surrogate-42".to_string(),
|
||||
remote_id: "42".to_string(),
|
||||
folder_id: "INBOX".to_string(),
|
||||
folder_checkpoint: checkpoint.map(str::to_string),
|
||||
uid_validity,
|
||||
kind,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,14 @@ use serde_json::{json, Value};
|
||||
use crate::backend::{MailBackend, RemoteMutationKind, RemoteMutationSpec};
|
||||
use crate::parser::MailError;
|
||||
|
||||
/// Stand-in for the UIDVALIDITY scope on JMAP, which has no such concept: a
|
||||
/// JMAP Email object id is already globally stable within its account (RFC
|
||||
/// 8621), so a fixed sentinel keeps the surrogate-id computation uniform with
|
||||
/// IMAP without implying a UIDVALIDITY guard. The IMAP UIDVALIDITY guard treats
|
||||
/// this non-numeric value as "no validity to compare" and is never engaged for
|
||||
/// a JMAP mutation, which addresses messages by object id.
|
||||
const JMAP_UID_VALIDITY_SENTINEL: &str = "jmap";
|
||||
|
||||
/// JMAP Session Object (RFC 8620 Section 2).
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
@@ -214,7 +222,9 @@ impl JmapEngine {
|
||||
/// `$flagged` keyword (`true` to set, `null` to clear), the JMAP equivalent
|
||||
/// of IMAP `\Flagged`.
|
||||
pub fn build_email_set_request(account_id: &str, spec: &RemoteMutationSpec) -> Value {
|
||||
let id = &spec.message_id;
|
||||
// Address the message by its protocol-native JMAP object id, never the
|
||||
// opaque surrogate `message_id`.
|
||||
let id = &spec.remote_id;
|
||||
let (update, destroy) = match &spec.kind {
|
||||
RemoteMutationKind::SetFlagged { value } => {
|
||||
let keyword = if *value { json!(true) } else { Value::Null };
|
||||
@@ -383,10 +393,23 @@ impl JmapEngine {
|
||||
.map(|a| a.email.clone())
|
||||
.unwrap_or_else(|| "me@nuncio.mx".to_string());
|
||||
|
||||
let folder_id = "inbox".to_string();
|
||||
// The JMAP object id is the protocol-native id; the persisted
|
||||
// id is an opaque surrogate hashed over it plus the account,
|
||||
// folder, and (sentinel) scope so it stays uniform with IMAP.
|
||||
let remote_id = item.id;
|
||||
let id = Email::surrogate_id(
|
||||
&self.account_id,
|
||||
&folder_id,
|
||||
JMAP_UID_VALIDITY_SENTINEL,
|
||||
&remote_id,
|
||||
);
|
||||
Email {
|
||||
id: item.id,
|
||||
id,
|
||||
account_id: self.account_id.clone(),
|
||||
folder_id: "inbox".to_string(),
|
||||
folder_id,
|
||||
remote_id,
|
||||
uid_validity: JMAP_UID_VALIDITY_SENTINEL.to_string(),
|
||||
subject: item.subject.unwrap_or_else(|| "No Subject".to_string()),
|
||||
sender,
|
||||
recipient,
|
||||
@@ -558,7 +581,7 @@ impl MailBackend for JmapEngine {
|
||||
let request = Self::build_email_set_request(&account_id, spec);
|
||||
let raw = self.post_jmap(&session.api_url, &request).await?;
|
||||
let expect_destroy = matches!(spec.kind, RemoteMutationKind::Delete);
|
||||
Self::confirm_email_set_applied(&raw, &spec.message_id, expect_destroy)
|
||||
Self::confirm_email_set_applied(&raw, &spec.remote_id, expect_destroy)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -689,9 +712,10 @@ mod tests {
|
||||
|
||||
fn spec(kind: RemoteMutationKind) -> RemoteMutationSpec {
|
||||
RemoteMutationSpec {
|
||||
message_id: "m-1".to_string(),
|
||||
message_id: "surrogate-m-1".to_string(),
|
||||
remote_id: "m-1".to_string(),
|
||||
folder_id: "mb-inbox".to_string(),
|
||||
folder_checkpoint: None,
|
||||
uid_validity: JMAP_UID_VALIDITY_SENTINEL.to_string(),
|
||||
kind,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -244,6 +244,8 @@ mod tests {
|
||||
id: "msg-mock-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Mock Test".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
|
||||
@@ -129,6 +129,10 @@ impl MimeParserAdapter {
|
||||
id: id.to_string(),
|
||||
account_id: account_id.to_string(),
|
||||
folder_id: folder_id.to_string(),
|
||||
// The MIME body carries no protocol addressing; the caller (the
|
||||
// IMAP sync) fills these from the transport before persisting.
|
||||
remote_id: String::new(),
|
||||
uid_validity: String::new(),
|
||||
subject,
|
||||
sender,
|
||||
recipient,
|
||||
|
||||
@@ -329,6 +329,8 @@ mod tests {
|
||||
id: "msg-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "outbox".to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Status Update".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
|
||||
@@ -84,7 +84,11 @@ async fn wiremock_jmap_session_discovery_and_email_get_sync() {
|
||||
.expect("parse email get");
|
||||
|
||||
assert_eq!(emails.len(), 1);
|
||||
assert_eq!(emails[0].id, "msg-wm-100");
|
||||
// The JMAP object id is preserved as the protocol-native `remote_id`, while
|
||||
// the persisted id is an opaque 64-char surrogate hash (never the raw id).
|
||||
assert_eq!(emails[0].remote_id, "msg-wm-100");
|
||||
assert_ne!(emails[0].id, "msg-wm-100");
|
||||
assert_eq!(emails[0].id.len(), 64);
|
||||
assert_eq!(emails[0].subject, "WireMock JMAP E2E Test");
|
||||
assert_eq!(emails[0].sender, "sender@nuncio.mx");
|
||||
assert_eq!(new_state, "sync-state-500");
|
||||
@@ -200,7 +204,9 @@ async fn jmap_engine_sync_folders_and_messages_through_mail_backend_trait() {
|
||||
.await
|
||||
.expect("sync_messages succeeds");
|
||||
assert_eq!(emails.len(), 1);
|
||||
assert_eq!(emails[0].id, "msg-wm-200");
|
||||
assert_eq!(emails[0].remote_id, "msg-wm-200");
|
||||
assert_ne!(emails[0].id, "msg-wm-200");
|
||||
assert_eq!(emails[0].id.len(), 64);
|
||||
assert_eq!(emails[0].subject, "Trait-Level JMAP Sync");
|
||||
assert_eq!(emails[0].sender, "sender@nuncio.mx");
|
||||
assert_eq!(emails[0].received_at, 1700002000);
|
||||
@@ -264,9 +270,10 @@ fn jmap_engine(mock_server: &MockServer) -> JmapEngine {
|
||||
|
||||
fn spec(kind: RemoteMutationKind) -> RemoteMutationSpec {
|
||||
RemoteMutationSpec {
|
||||
message_id: "jmap-msg-1".to_string(),
|
||||
message_id: "surrogate-jmap-msg-1".to_string(),
|
||||
remote_id: "jmap-msg-1".to_string(),
|
||||
folder_id: "mb-inbox".to_string(),
|
||||
folder_checkpoint: None,
|
||||
uid_validity: "jmap".to_string(),
|
||||
kind,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -471,6 +471,8 @@ impl DatabaseEngine {
|
||||
id TEXT PRIMARY KEY NOT NULL,
|
||||
account_id TEXT NOT NULL,
|
||||
folder_id TEXT NOT NULL,
|
||||
remote_id TEXT NOT NULL DEFAULT '',
|
||||
uid_validity TEXT NOT NULL DEFAULT '',
|
||||
subject TEXT NOT NULL,
|
||||
sender TEXT NOT NULL,
|
||||
recipient TEXT NOT NULL,
|
||||
@@ -711,11 +713,84 @@ impl DatabaseEngine {
|
||||
self.ensure_accounts_smtp_columns().await?;
|
||||
self.ensure_accounts_tls_mode_columns().await?;
|
||||
self.ensure_accounts_dav_columns().await?;
|
||||
self.ensure_messages_identity_columns().await?;
|
||||
self.backfill_message_fts().await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Additive, backfill-safe migration that adds the `remote_id` /
|
||||
/// `uid_validity` message-identity columns to a pre-existing `messages`
|
||||
/// table that predated the opaque surrogate id, and installs the
|
||||
/// `UNIQUE(account_id, folder_id, uid_validity, remote_id)` index that is
|
||||
/// the real correctness guard against a shared protocol id (e.g. an IMAP
|
||||
/// UID reused across folders/accounts) overwriting an unrelated message.
|
||||
///
|
||||
/// A fresh database already gets both columns from `CREATE TABLE IF NOT
|
||||
/// EXISTS messages` above, so on a fresh database the `PRAGMA table_info`
|
||||
/// check finds them present and the backfill matches no rows -- only the
|
||||
/// index is created. For a pre-existing database file the columns are added
|
||||
/// with an empty-string default and every legacy row is backfilled to
|
||||
/// `remote_id = id` (the old primary key, which was globally unique) so the
|
||||
/// UNIQUE index can be built without collisions. SQLite has no `ADD COLUMN
|
||||
/// IF NOT EXISTS`, so column presence is checked explicitly first, making
|
||||
/// this safe to run on every daemon startup.
|
||||
///
|
||||
/// The whole step runs in a single transaction, and the backfill is
|
||||
/// unconditional (idempotent `WHERE remote_id = ''`) rather than gated on
|
||||
/// having just added the column. Both properties make a partial run
|
||||
/// self-healing: a crash mid-migration rolls back atomically, and even a
|
||||
/// committed intermediate state (column present but a legacy row still
|
||||
/// carrying the empty default) is completed by the next startup's backfill
|
||||
/// before the index is built -- so the daemon can never wedge on an index
|
||||
/// collision it could have avoided.
|
||||
async fn ensure_messages_identity_columns(&self) -> Result<(), DatabaseError> {
|
||||
let existing_columns: Vec<String> = sqlx::query("PRAGMA table_info(messages)")
|
||||
.fetch_all(&self.pool)
|
||||
.await
|
||||
.map_err(DatabaseError::Query)?
|
||||
.iter()
|
||||
.map(|row| row.get::<String, _>("name"))
|
||||
.collect();
|
||||
|
||||
let mut tx = self.pool.begin().await.map_err(DatabaseError::Query)?;
|
||||
|
||||
if !existing_columns.iter().any(|c| c == "remote_id") {
|
||||
sqlx::query("ALTER TABLE messages ADD COLUMN remote_id TEXT NOT NULL DEFAULT ''")
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(DatabaseError::Query)?;
|
||||
}
|
||||
if !existing_columns.iter().any(|c| c == "uid_validity") {
|
||||
sqlx::query("ALTER TABLE messages ADD COLUMN uid_validity TEXT NOT NULL DEFAULT ''")
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(DatabaseError::Query)?;
|
||||
}
|
||||
|
||||
// Seed any legacy row still carrying the empty-string default with a
|
||||
// unique, non-empty remote id (the old primary key) so the UNIQUE
|
||||
// identity index is satisfiable. Unconditional so a partial prior run
|
||||
// is always completed before the index is (re)built; a no-op on a fresh
|
||||
// or already-migrated database.
|
||||
sqlx::query("UPDATE messages SET remote_id = id WHERE remote_id = ''")
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(DatabaseError::Query)?;
|
||||
|
||||
sqlx::query(
|
||||
"CREATE UNIQUE INDEX IF NOT EXISTS idx_messages_identity \
|
||||
ON messages (account_id, folder_id, uid_validity, remote_id)",
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(DatabaseError::Query)?;
|
||||
|
||||
tx.commit().await.map_err(DatabaseError::Query)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Additive, backfill-safe migration that adds the `imap_tls_mode` /
|
||||
/// `smtp_tls_mode` columns to a pre-existing `accounts` table that
|
||||
/// predates persisted per-protocol TLS transport modes.
|
||||
@@ -1081,13 +1156,15 @@ impl DatabaseEngine {
|
||||
sqlx::query(
|
||||
r#"
|
||||
INSERT OR REPLACE INTO messages
|
||||
(id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
(id, account_id, folder_id, remote_id, uid_validity, subject, sender, recipient, received_at, read_flag, body_plain, body_html)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
"#,
|
||||
)
|
||||
.bind(&email.id)
|
||||
.bind(&email.account_id)
|
||||
.bind(&email.folder_id)
|
||||
.bind(&email.remote_id)
|
||||
.bind(&email.uid_validity)
|
||||
.bind(&email.subject)
|
||||
.bind(&email.sender)
|
||||
.bind(&email.recipient)
|
||||
@@ -1143,9 +1220,11 @@ impl DatabaseEngine {
|
||||
i64,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
String,
|
||||
String,
|
||||
)> = sqlx::query_as(
|
||||
r#"
|
||||
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html
|
||||
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity
|
||||
FROM messages
|
||||
WHERE folder_id = ?
|
||||
ORDER BY received_at DESC
|
||||
@@ -1171,6 +1250,8 @@ impl DatabaseEngine {
|
||||
read_flag,
|
||||
body_plain,
|
||||
body_html,
|
||||
remote_id,
|
||||
uid_validity,
|
||||
)| {
|
||||
let dec_plain = body_plain
|
||||
.map(|p| {
|
||||
@@ -1194,6 +1275,8 @@ impl DatabaseEngine {
|
||||
id,
|
||||
account_id,
|
||||
folder_id,
|
||||
remote_id,
|
||||
uid_validity,
|
||||
subject,
|
||||
sender,
|
||||
recipient,
|
||||
@@ -1225,9 +1308,11 @@ impl DatabaseEngine {
|
||||
i64,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
String,
|
||||
String,
|
||||
) = sqlx::query_as(
|
||||
r#"
|
||||
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html
|
||||
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity
|
||||
FROM messages
|
||||
WHERE id = ?
|
||||
"#,
|
||||
@@ -1252,6 +1337,8 @@ impl DatabaseEngine {
|
||||
id: row.0,
|
||||
account_id: row.1,
|
||||
folder_id: row.2,
|
||||
remote_id: row.10,
|
||||
uid_validity: row.11,
|
||||
subject: row.3,
|
||||
sender: row.4,
|
||||
recipient: row.5,
|
||||
@@ -1902,7 +1989,7 @@ impl DatabaseEngine {
|
||||
limit: usize,
|
||||
) -> Result<Vec<nuncio_core::model::Email>, DatabaseError> {
|
||||
let mut builder: sqlx::QueryBuilder<'_, sqlx::Sqlite> = sqlx::QueryBuilder::new(
|
||||
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html FROM messages "
|
||||
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity FROM messages "
|
||||
);
|
||||
if !last_id.is_empty() {
|
||||
builder.push("WHERE id > ");
|
||||
@@ -1922,6 +2009,8 @@ impl DatabaseEngine {
|
||||
i64,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
String,
|
||||
String,
|
||||
)>();
|
||||
|
||||
let rows = query
|
||||
@@ -1947,6 +2036,8 @@ impl DatabaseEngine {
|
||||
id: r.0,
|
||||
account_id: r.1,
|
||||
folder_id: r.2,
|
||||
remote_id: r.10,
|
||||
uid_validity: r.11,
|
||||
subject: r.3,
|
||||
sender: r.4,
|
||||
recipient: r.5,
|
||||
@@ -1972,7 +2063,7 @@ impl DatabaseEngine {
|
||||
folder_id: Option<&str>,
|
||||
) -> Result<Vec<nuncio_core::model::Email>, DatabaseError> {
|
||||
let mut builder: sqlx::QueryBuilder<'_, sqlx::Sqlite> = sqlx::QueryBuilder::new(
|
||||
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html FROM messages"
|
||||
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity FROM messages"
|
||||
);
|
||||
|
||||
let mut has_filter = false;
|
||||
@@ -2002,6 +2093,8 @@ impl DatabaseEngine {
|
||||
i64,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
String,
|
||||
String,
|
||||
)>();
|
||||
|
||||
let rows = query
|
||||
@@ -2027,6 +2120,8 @@ impl DatabaseEngine {
|
||||
id: r.0,
|
||||
account_id: r.1,
|
||||
folder_id: r.2,
|
||||
remote_id: r.10,
|
||||
uid_validity: r.11,
|
||||
subject: r.3,
|
||||
sender: r.4,
|
||||
recipient: r.5,
|
||||
@@ -2483,6 +2578,8 @@ mod tests {
|
||||
id: "msg-db-100".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "INBOX".to_string(),
|
||||
remote_id: "100".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Database Sync Test".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -2530,6 +2627,8 @@ mod tests {
|
||||
id: "msg-db-corrupt".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "INBOX".to_string(),
|
||||
remote_id: "corrupt".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Corrupted At Rest".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -2832,6 +2931,8 @@ mod tests {
|
||||
id: "msg-mark-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "INBOX".to_string(),
|
||||
remote_id: "mark-1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Mark Read Test".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -3362,6 +3463,8 @@ mod tests {
|
||||
id: format!("msg-{i:03}"),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: format!("{i}"),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: format!("Subject {i}"),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -3469,11 +3572,153 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Build a message exactly as a real sync would: an opaque surrogate id
|
||||
/// hashed over its addressing coordinates, with the protocol-native id and
|
||||
/// UIDVALIDITY scope carried in their own columns.
|
||||
fn synced_email(
|
||||
account_id: &str,
|
||||
folder_id: &str,
|
||||
uid_validity: &str,
|
||||
remote_id: &str,
|
||||
) -> nuncio_core::model::Email {
|
||||
nuncio_core::model::Email {
|
||||
id: nuncio_core::model::Email::surrogate_id(
|
||||
account_id,
|
||||
folder_id,
|
||||
uid_validity,
|
||||
remote_id,
|
||||
),
|
||||
account_id: account_id.to_string(),
|
||||
folder_id: folder_id.to_string(),
|
||||
remote_id: remote_id.to_string(),
|
||||
uid_validity: uid_validity.to_string(),
|
||||
subject: format!("{folder_id}/{remote_id}"),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
received_at: 1_700_000_000,
|
||||
read: false,
|
||||
body_plain: Some("body".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// The C1 regression: the same protocol UID reused across folders or
|
||||
/// accounts must persist as DISTINCT rows (never silently overwrite), and a
|
||||
/// re-sync of the SAME message must upsert the one row it already owns.
|
||||
#[tokio::test]
|
||||
async fn message_identity_prevents_cross_folder_and_cross_account_collision() {
|
||||
let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap();
|
||||
|
||||
// Same UID (5) in two folders of one account, and again in a second
|
||||
// account -- the exact shapes that used to collapse to `imap-uid-5`.
|
||||
let inbox = synced_email("acct-1", "INBOX", "42", "5");
|
||||
let sent = synced_email("acct-1", "Sent", "7", "5");
|
||||
let other_account = synced_email("acct-2", "INBOX", "99", "5");
|
||||
|
||||
// Every surrogate id is distinct, so none can overwrite another.
|
||||
assert_ne!(inbox.id, sent.id);
|
||||
assert_ne!(inbox.id, other_account.id);
|
||||
assert_ne!(sent.id, other_account.id);
|
||||
|
||||
engine.save_email(&inbox).await.expect("save inbox");
|
||||
engine.save_email(&sent).await.expect("save sent");
|
||||
engine
|
||||
.save_email(&other_account)
|
||||
.await
|
||||
.expect("save other account");
|
||||
|
||||
// All three coexist as separate rows, addressed by their own folder.
|
||||
assert_eq!(engine.list_messages("INBOX", 10).await.unwrap().len(), 2);
|
||||
assert_eq!(engine.list_messages("Sent", 10).await.unwrap().len(), 1);
|
||||
|
||||
// Re-syncing the SAME message recomputes the SAME id, so the upsert
|
||||
// updates the one row rather than duplicating it.
|
||||
let inbox_resynced = synced_email("acct-1", "INBOX", "42", "5");
|
||||
assert_eq!(inbox_resynced.id, inbox.id);
|
||||
engine
|
||||
.save_email(&inbox_resynced)
|
||||
.await
|
||||
.expect("re-sync upserts");
|
||||
assert_eq!(
|
||||
engine.list_messages("INBOX", 10).await.unwrap().len(),
|
||||
2,
|
||||
"a re-sync must upsert, not duplicate"
|
||||
);
|
||||
|
||||
// The recovered row round-trips its protocol addressing columns.
|
||||
let fetched = engine.get_message(&inbox.id).await.expect("get message");
|
||||
assert_eq!(fetched.remote_id, "5");
|
||||
assert_eq!(fetched.uid_validity, "42");
|
||||
assert_eq!(fetched.folder_id, "INBOX");
|
||||
}
|
||||
|
||||
/// A crash between `ADD COLUMN remote_id` and its backfill can commit an
|
||||
/// intermediate state -- the column present but a legacy row still carrying
|
||||
/// the empty-string default -- with the UNIQUE index not yet built. The
|
||||
/// migration must complete that backfill on the next open and build the
|
||||
/// index without a collision, rather than wedging the daemon.
|
||||
#[tokio::test]
|
||||
async fn messages_identity_migration_completes_from_a_partial_run() {
|
||||
let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap();
|
||||
|
||||
// Reproduce the partial state: drop the identity index, then insert two
|
||||
// rows in the same folder that both still hold the empty default. With
|
||||
// the index gone this is permitted, and the two would collide on
|
||||
// (account, folder, uid_validity='', remote_id='') if it were rebuilt now.
|
||||
sqlx::query("DROP INDEX IF EXISTS idx_messages_identity")
|
||||
.execute(engine.pool())
|
||||
.await
|
||||
.unwrap();
|
||||
for id in ["legacy-a", "legacy-b"] {
|
||||
sqlx::query(
|
||||
"INSERT INTO messages \
|
||||
(id, account_id, folder_id, remote_id, uid_validity, subject, sender, recipient, received_at, read_flag) \
|
||||
VALUES (?, 'acct-1', 'INBOX', '', '', 'subj', 'a@nuncio.mx', 'b@nuncio.mx', 0, 0)",
|
||||
)
|
||||
.bind(id)
|
||||
.execute(engine.pool())
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
// Re-running the migration must finish the backfill and build the index.
|
||||
engine
|
||||
.ensure_messages_identity_columns()
|
||||
.await
|
||||
.expect("a partial migration must complete on the next open");
|
||||
|
||||
// Both legacy rows survive with a unique, non-empty backfilled remote id.
|
||||
assert_eq!(
|
||||
engine.get_message("legacy-a").await.unwrap().remote_id,
|
||||
"legacy-a"
|
||||
);
|
||||
assert_eq!(
|
||||
engine.get_message("legacy-b").await.unwrap().remote_id,
|
||||
"legacy-b"
|
||||
);
|
||||
|
||||
// The unique identity index is now present, and re-running is idempotent.
|
||||
let index: Option<(String,)> = sqlx::query_as(
|
||||
"SELECT name FROM sqlite_master WHERE type = 'index' AND name = 'idx_messages_identity'",
|
||||
)
|
||||
.fetch_optional(engine.pool())
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(index.is_some(), "the unique identity index must be built");
|
||||
engine
|
||||
.ensure_messages_identity_columns()
|
||||
.await
|
||||
.expect("re-running an already-complete migration is a no-op");
|
||||
}
|
||||
|
||||
fn export_test_email(id: &str, account_id: &str, folder_id: &str) -> nuncio_core::model::Email {
|
||||
nuncio_core::model::Email {
|
||||
id: id.to_string(),
|
||||
account_id: account_id.to_string(),
|
||||
folder_id: folder_id.to_string(),
|
||||
remote_id: id.to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: format!("Subject {id}"),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
|
||||
@@ -105,6 +105,8 @@ mod tests {
|
||||
id: id.to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: id.to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
|
||||
@@ -1592,6 +1592,9 @@ fn synthetic_preview_email(id: &str) -> nuncio_core::model::Email {
|
||||
id: id.to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
// Synthetic preview only -- never persisted or mutated remotely.
|
||||
remote_id: id.to_string(),
|
||||
uid_validity: String::new(),
|
||||
subject: "Test Subject".to_string(),
|
||||
sender: "test@nuncio.mx".to_string(),
|
||||
recipient: "me@nuncio.mx".to_string(),
|
||||
@@ -3434,6 +3437,8 @@ mod tests {
|
||||
id: id.to_string(),
|
||||
account_id: "acct-mail-1".to_string(),
|
||||
folder_id: folder_id.to_string(),
|
||||
remote_id: id.to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -4320,6 +4325,8 @@ mod tests {
|
||||
id: "msg-filters-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -4420,6 +4427,8 @@ mod tests {
|
||||
id: "msg-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "anything".to_string(),
|
||||
sender: "a@b.com".to_string(),
|
||||
recipient: "c@d.com".to_string(),
|
||||
@@ -4562,6 +4571,8 @@ mod tests {
|
||||
id: "msg-preview-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: "preview-1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Urgent Board Meeting".to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -4728,6 +4739,8 @@ mod tests {
|
||||
id: "msg-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Urgent Meeting".to_string(),
|
||||
sender: "a@b.com".to_string(),
|
||||
recipient: "c@d.com".to_string(),
|
||||
@@ -4882,6 +4895,8 @@ mod tests {
|
||||
id: "msg-1".to_string(),
|
||||
account_id: "acct-1".to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: "1".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: "Urgent Meeting".to_string(),
|
||||
sender: "a@b.com".to_string(),
|
||||
recipient: "c@d.com".to_string(),
|
||||
|
||||
@@ -204,7 +204,7 @@ async fn execute_one(
|
||||
|
||||
match mutation.mutation_type.as_str() {
|
||||
"MOVE" | "COPY" | "FLAG" | "UNFLAG" | "DELETE" => {
|
||||
dispatch_mailbox_mutation(db, env, mutation, &email, &payload).await
|
||||
dispatch_mailbox_mutation(env, mutation, &email, &payload).await
|
||||
}
|
||||
"FORWARD" => dispatch_forward(db, env, &email, &payload).await,
|
||||
"WEBHOOK" => dispatch_webhook(env, mutation, &email, &payload).await,
|
||||
@@ -213,10 +213,10 @@ async fn execute_one(
|
||||
}
|
||||
|
||||
/// Execute a mailbox-affecting mutation (move/copy/flag/unflag/delete) against
|
||||
/// the account's mail backend, threading the source folder's stored checkpoint
|
||||
/// so the backend can enforce its UIDVALIDITY guard.
|
||||
/// the account's mail backend, recovering the remote addressing (protocol id +
|
||||
/// UIDVALIDITY scope) from the stored message row so the backend can enforce
|
||||
/// its UIDVALIDITY guard.
|
||||
async fn dispatch_mailbox_mutation(
|
||||
db: &DatabaseEngine,
|
||||
env: &dyn RemoteExecutionEnv,
|
||||
mutation: &PendingRemoteMutation,
|
||||
email: &nuncio_core::model::Email,
|
||||
@@ -245,23 +245,20 @@ async fn dispatch_mailbox_mutation(
|
||||
other => return Disposition::Permanent(format!("unexpected mailbox mutation '{other}'")),
|
||||
};
|
||||
|
||||
let folder_checkpoint = match db
|
||||
.get_folder_sync_state(&email.account_id, &email.folder_id)
|
||||
.await
|
||||
{
|
||||
Ok(checkpoint) => checkpoint,
|
||||
Err(e) => return Disposition::Retry(format!("failed to read folder checkpoint: {e}")),
|
||||
};
|
||||
|
||||
let backend = match env.mail_backend(&email.account_id).await {
|
||||
Ok(backend) => backend,
|
||||
Err(e) => return Disposition::Retry(format!("failed to build mail backend: {e}")),
|
||||
};
|
||||
|
||||
// Recover the remote addressing from the stored message row -- the
|
||||
// protocol-native id and the UIDVALIDITY scope it was captured under --
|
||||
// rather than parsing it out of the opaque surrogate id. The backend still
|
||||
// SELECT-verifies the UIDVALIDITY before mutating.
|
||||
let spec = RemoteMutationSpec {
|
||||
message_id: email.id.clone(),
|
||||
remote_id: email.remote_id.clone(),
|
||||
folder_id: email.folder_id.clone(),
|
||||
folder_checkpoint,
|
||||
uid_validity: email.uid_validity.clone(),
|
||||
kind,
|
||||
};
|
||||
|
||||
|
||||
@@ -628,6 +628,8 @@ mod tests {
|
||||
id: id.to_string(),
|
||||
account_id: "acct-mock-1".to_string(),
|
||||
folder_id: folder_id.to_string(),
|
||||
remote_id: id.to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "bob@nuncio.mx".to_string(),
|
||||
@@ -864,11 +866,16 @@ mod tests {
|
||||
// The wiremock stubs advertise 1 folder containing 1 message.
|
||||
assert_eq!(synced, 1);
|
||||
|
||||
// The message persists under an opaque surrogate id; it is recovered by
|
||||
// folder, and its protocol-native JMAP object id round-trips in
|
||||
// `remote_id`.
|
||||
let persisted = db
|
||||
.get_message("jmap-msg-1")
|
||||
.list_messages("inbox", 10)
|
||||
.await
|
||||
.expect("jmap message persisted");
|
||||
assert_eq!(persisted.subject, "Welcome to JMAP Sync");
|
||||
assert_eq!(persisted.len(), 1);
|
||||
assert_eq!(persisted[0].subject, "Welcome to JMAP Sync");
|
||||
assert_eq!(persisted[0].remote_id, "jmap-msg-1");
|
||||
|
||||
assert_eq!(
|
||||
events.recv().await.expect("start event"),
|
||||
|
||||
@@ -43,6 +43,8 @@ fn sample_email(id: &str, subject: &str) -> Email {
|
||||
id: id.to_string(),
|
||||
account_id: ACCOUNT_ID.to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: id.to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "triage-e2e@nuncio.mx".to_string(),
|
||||
|
||||
@@ -103,9 +103,11 @@ fn sample_account() -> nuncio_core::AccountConfig {
|
||||
|
||||
fn sample_email(subject: &str) -> Email {
|
||||
Email {
|
||||
id: "imap-uid-42".to_string(),
|
||||
id: "surrogate-inbox-42".to_string(),
|
||||
account_id: ACCOUNT_ID.to_string(),
|
||||
folder_id: FOLDER_ID.to_string(),
|
||||
remote_id: "42".to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: "owner@nuncio.mx".to_string(),
|
||||
@@ -161,11 +163,13 @@ async fn move_rule_executes_against_backend_and_completes_with_uidvalidity_check
|
||||
|
||||
let applied = env.backend.applied_mutations();
|
||||
assert_eq!(applied.len(), 1, "the backend op must genuinely run");
|
||||
assert_eq!(applied[0].message_id, "imap-uid-42");
|
||||
// Addressing is recovered from the stored row's columns, not by parsing the
|
||||
// opaque surrogate id: the protocol-native UID and the UIDVALIDITY scope it
|
||||
// was captured under, so the backend can enforce its guard.
|
||||
assert_eq!(applied[0].message_id, "surrogate-inbox-42");
|
||||
assert_eq!(applied[0].remote_id, "42");
|
||||
assert_eq!(applied[0].folder_id, FOLDER_ID);
|
||||
// The source folder's stored checkpoint (carrying UIDVALIDITY) is threaded
|
||||
// through so the backend can enforce its guard.
|
||||
assert_eq!(applied[0].folder_checkpoint.as_deref(), Some(CHECKPOINT));
|
||||
assert_eq!(applied[0].uid_validity, "1");
|
||||
assert_eq!(
|
||||
applied[0].kind,
|
||||
RemoteMutationKind::Move {
|
||||
|
||||
@@ -77,6 +77,8 @@ fn mock_inbound_email(id: &str, subject: &str, body: &str) -> Email {
|
||||
id: id.to_string(),
|
||||
account_id: ACCOUNT_ID.to_string(),
|
||||
folder_id: "inbox".to_string(),
|
||||
remote_id: id.to_string(),
|
||||
uid_validity: "1".to_string(),
|
||||
subject: subject.to_string(),
|
||||
sender: "alice@nuncio.mx".to_string(),
|
||||
recipient: ACCOUNT_EMAIL.to_string(),
|
||||
|
||||
Reference in new issue
Block a user