diff options
Diffstat (limited to 'tests/test_notmuchworker.cpp')
| -rw-r--r-- | tests/test_notmuchworker.cpp | 143 |
1 files changed, 140 insertions, 3 deletions
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 |