aboutsummaryrefslogtreecommitdiffstats
path: root/tests/test_mainwindow.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-19 11:10:51 +0200
committerDanilo M. <danix@danix.xyz>2026-08-19 11:10:51 +0200
commit41d0b94dcd2b4208e5d3e7483c88ce9b664ce565 (patch)
tree61aceb2bd653d4c0df463955cc610942f928f46e /tests/test_mainwindow.cpp
parenta36fff5617f16ac1d17c0c52f2112a20c4fa9336 (diff)
downloadqtmaildir-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.cpp40
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;
}