diff options
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_mainwindow.cpp | 40 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 143 |
2 files changed, 178 insertions, 5 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; } diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index bf1c23c..b5da31a 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -90,6 +90,9 @@ private slots: void moveMessagesReindexesAtTheNewPath(); void moveMessagesKeepsTheMessagesTags(); void moveMessagesReportsOnlyWhatMoved(); + void moveMessagesGivesTheFileAFreshMaildirName(); + void moveMessagesKeepsTheMaildirFlags(); + void twoMessagesMovedTogetherGetDistinctNames(); private: /// Adds one read message in `folder` and reindexes, for the move tests. @@ -1160,9 +1163,19 @@ void TestNotmuchWorker::moveMessagesRelocatesTheFile() // cur/, never new/: a file in new/ is re-announced as fresh mail by every // reader of the Maildir. - const QString expected = m_fixture.maildirPath() + QStringLiteral("/trash/cur/") - + QFileInfo(before).fileName(); - QVERIFY2(QFile::exists(expected), qPrintable(expected)); + // + // Asserted on the DIRECTORY, not on the full path. The filename is + // deliberately regenerated by the move (see + // moveMessagesGivesTheFileAFreshMaildirName), so an assertion naming the + // old filename here encoded the very bug that item fixes: it required the + // name to be carried across, which is what produced duplicate mbsync UIDs + // on real mail. + const QString expectedDir = + m_fixture.maildirPath() + QStringLiteral("/trash/cur"); + const QString after = fileOf(id); + QVERIFY2(!after.isEmpty(), "the message is not in the database after the move"); + QCOMPARE(QFileInfo(after).absolutePath(), expectedDir); + QVERIFY2(QFile::exists(after), qPrintable(after)); QVERIFY(!QFile::exists(before)); } @@ -1217,6 +1230,130 @@ void TestNotmuchWorker::moveMessagesKeepsTheMessagesTags() .arg(after.join(QLatin1Char(' '))))); } +void TestNotmuchWorker::moveMessagesGivesTheFileAFreshMaildirName() +{ + // 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. The `,U=<n>` infix is mbsync's + // per-folder IMAP UID, so carrying it into another folder 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, and mbsync reports + // `Maildir error: duplicate UID`. + // + // Measured on the user's real Maildir on 2026-08-19: four collisions in one + // folder, eight distinct messages, from a move-out and restore made with + // 0.26.0. + const QString id = QStringLiteral("move5@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + // A realistic mbsync name, which the fixture does not produce on its own: + // <unique>,U=<n>:2,<flags>. + const QString before = fileOf(id); + QVERIFY(!before.isEmpty()); + const QString uidName = QFileInfo(before).absolutePath() + + QStringLiteral("/move5.example.org,U=42:2,S"); + QVERIFY2(QFile::rename(before, uidName), "could not stage a ,U= filename"); + QVERIFY(m_fixture.index()); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.moveMessages({ id }, QStringLiteral("trash")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QString after = fileOf(id); + QVERIFY2(!after.isEmpty(), "the message is not in the database after the move"); + QVERIFY(QFile::exists(after)); + + const QString name = QFileInfo(after).fileName(); + // The point of the item: no UID infix survives the move. + QVERIFY2(!name.contains(QStringLiteral(",U=")), + qPrintable(QStringLiteral("the moved file kept a UID infix: %1") + .arg(name))); + // And it is a genuinely new name rather than the old one with the infix + // cut out, which is what mbsync calls the expectation. + QVERIFY2(name != QStringLiteral("move5.example.org:2,S"), + qPrintable(QStringLiteral("the name was only stripped, not " + "regenerated: %1").arg(name))); +} + +void TestNotmuchWorker::moveMessagesKeepsTheMaildirFlags() +{ + // A fresh name must NOT mean fresh state. The `:2,<flags>` suffix carries + // seen, flagged and replied, and notmuch's maildir.synchronize_flags is + // true on the user's setup, so dropping it would mark read mail unread and + // lose Important on every message the user deletes. + // + // Asserted on the FLAGS rather than on the whole name, since the unique + // part is expected to change and the flags are expected not to. + const QString id = QStringLiteral("move6@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + const QString before = fileOf(id); + QVERIFY(!before.isEmpty()); + // Seen and Flagged, so a suffix that is dropped or truncated shows up. + const QString staged = QFileInfo(before).absolutePath() + + QStringLiteral("/move6.example.org,U=7:2,FS"); + QVERIFY2(QFile::rename(before, staged), "could not stage a flagged filename"); + QVERIFY(m_fixture.index()); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.moveMessages({ id }, QStringLiteral("trash")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QString after = fileOf(id); + QVERIFY2(!after.isEmpty(), "the message is not in the database after the move"); + const QString name = QFileInfo(after).fileName(); + + QVERIFY2(name.endsWith(QStringLiteral(":2,FS")), + qPrintable(QStringLiteral("the move lost the maildir flags: %1") + .arg(name))); + QVERIFY(!name.contains(QStringLiteral(",U="))); +} + +void TestNotmuchWorker::twoMessagesMovedTogetherGetDistinctNames() +{ + // The generated name must be unique, since a collision is the entire class + // of bug this change exists to remove: two files landing on one name means + // one message silently overwrites the other. + // + // Two messages in ONE batch, which is the case a timestamp alone does not + // cover: both are moved in the same second, so only the per-process counter + // separates them. A generator using time and pid alone passes every other + // test here and fails this one. + const QString first = QStringLiteral("move7@example.org"); + const QString second = QStringLiteral("move8@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), first), + qPrintable(m_fixture.error())); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), second), + qPrintable(m_fixture.error())); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.moveMessages({ first, second }, QStringLiteral("trash")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QString a = fileOf(first); + const QString b = fileOf(second); + QVERIFY2(!a.isEmpty() && !b.isEmpty(), + "a message is missing from the database after the move"); + + // Distinct names... + QVERIFY2(QFileInfo(a).fileName() != QFileInfo(b).fileName(), + qPrintable(QStringLiteral("both messages were named %1") + .arg(QFileInfo(a).fileName()))); + // ...and both files really are on disk, which is what a collision would + // have destroyed. The name check alone would pass against one file that + // overwrote the other if the database still named two paths. + QVERIFY(QFile::exists(a)); + QVERIFY(QFile::exists(b)); +} + void TestNotmuchWorker::moveMessagesReportsOnlyWhatMoved() { // A stale id must not abort the batch, and must not be reported as moved |