diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-19 11:10:51 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-19 11:10:51 +0200 |
| commit | 41d0b94dcd2b4208e5d3e7483c88ce9b664ce565 (patch) | |
| tree | 61aceb2bd653d4c0df463955cc610942f928f46e /tests/test_mainwindow.cpp | |
| parent | a36fff5617f16ac1d17c0c52f2112a20c4fa9336 (diff) | |
| download | qtmaildir-41d0b94dcd2b4208e5d3e7483c88ce9b664ce565.tar.gz qtmaildir-41d0b94dcd2b4208e5d3e7483c88ce9b664ce565.zip | |
fix(worker): give a moved message a fresh maildir name
mbsync's manual, under "the more efficient default UID mapping scheme": "it is
important that the MUA renames files when moving them between Maildir folders",
and "the general expectation is that a completely new filename is generated as
if the message was new".
qtmaildir is that MUA and did not rename. moveMessages() kept
QFileInfo(from).fileName() verbatim, `,U=<n>` included. That infix is mbsync's
per-folder IMAP UID, so carrying it across a folder boundary makes it a claim
about a folder the file is no longer in; moving a message out and back then
reinserts a UID the server has since reassigned. Reported by the user as
`Maildir error: duplicate UID 1`, and measured on the real Maildir: four
collisions in one folder, eight distinct messages, none lost.
freshMaildirName() regenerates the unique part and keeps ONLY the `:2,<flags>`
suffix. Keeping the flags is not a contradiction of "as if the message was new":
they record seen, flagged and replied, and maildir.synchronize_flags is true, so
dropping them would mark every deleted message unread and lose Important on the
way to the trash.
Two things fell out of the change and both were defects waiting to happen. The
already-in-the-destination guard compared full PATHS, which worked only because
the name was carried across; with a fresh name it can never be true, so a
message already in the destination would be renamed on every move. It compares
directories now. And test_mainwindow's folderHasMessageFile() matched on the
filename stem, so all fifty-odd assertions using it began reporting "the file is
not there" about files that were there. It reads the Message-ID out of each file
instead, which is what those assertions always meant.
Three mutations fail: the old name carried across, the flags dropped, and the
uniqueness counter frozen so two messages moved in one batch collide.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'tests/test_mainwindow.cpp')
| -rw-r--r-- | tests/test_mainwindow.cpp | 40 |
1 files changed, 38 insertions, 2 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 79e137a..0deaec3 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -8907,15 +8907,51 @@ static bool notmuchTag(const QString &configPath, const QStringList &args) return process.waitForFinished(15000) && process.exitCode() == 0; } +/// Whether `dir` holds the message whose id the fixture wrote as `stem`. +/// +/// Matches on the MESSAGE-ID INSIDE each file, never on the filename. It used +/// to compare filenames, which worked only while a move carried the name +/// across unchanged. It no longer does: mbsync requires an MUA to rename a +/// file when it moves it between folders, so `NotmuchWorker::moveMessages()` +/// generates a fresh name and this helper could never find a moved message +/// again. Every one of these assertions failed at once, each reporting "the +/// file is not there" about a file that was. +/// +/// `stem` stays in the fixture's `<local>.example.org` form so the fifty-odd +/// call sites did not have to change; it is turned back into +/// `<local@example.org>` here. static bool folderHasMessageFile(const QString &dir, const QString &stem) { QDir directory(dir); if (!directory.exists()) return false; + + // `del1.example.org` is the fixture's rendering of `del1@example.org`: + // it replaces the `@` to make a filename-safe stem. Only the LAST dot + // before the domain is the substituted one, so the split is on the first + // dot, which is where the local part ends for every id these tests use. + const int dot = stem.indexOf(QLatin1Char('.')); + if (dot < 0) + return false; + const QString messageId = QStringLiteral("<%1@%2>") + .arg(stem.left(dot), stem.mid(dot + 1)); + const QStringList entries = directory.entryList(QDir::Files); for (const QString &entry : entries) { - if (entry == stem || entry.startsWith(stem + QLatin1Char(':'))) - return true; + QFile file(directory.filePath(entry)); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) + continue; + // The header block only: a quoted id in a body must not count. + while (!file.atEnd()) { + const QByteArray line = file.readLine(); + if (line.trimmed().isEmpty()) + break; + if (line.startsWith("Message-ID:") || line.startsWith("Message-Id:")) { + if (QString::fromUtf8(line).contains(messageId)) + return true; + break; + } + } } return false; } |
