diff options
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/maildirname.cpp | 80 | ||||
| -rw-r--r-- | src/maildirname.h | 41 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 64 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_maildirname.cpp | 93 |
6 files changed, 219 insertions, 61 deletions
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6108696..12168a5 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -11,6 +11,7 @@ add_library(qtmaildir_lib STATIC marks.cpp carddelegate.cpp notmuchworker.cpp + maildirname.cpp tagchip.cpp tagcolors.cpp savequerydialog.cpp diff --git a/src/maildirname.cpp b/src/maildirname.cpp new file mode 100644 index 0000000..6263aec --- /dev/null +++ b/src/maildirname.cpp @@ -0,0 +1,80 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "maildirname.h" + +#include <QCoreApplication> +#include <QDateTime> +#include <QHostInfo> + +namespace MaildirName { + +/// A fresh Maildir filename for a message being moved between folders, +/// preserving only its `:2,<flags>` suffix. +/// +/// mbsync's manual is explicit about why this exists, 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". +/// +/// The `,U=<n>` infix mbsync writes is its per-folder IMAP UID. 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 refuses the folder with `Maildir error: +/// duplicate UID`. Measured on real mail, four collisions in one folder from +/// a single move-and-restore. +/// +/// The FLAGS are kept, deliberately, and that is not a contradiction of +/// "as if the message was new". They record seen, flagged and replied, and +/// `maildir.synchronize_flags` is true, so notmuch reads them back as tags: +/// dropping them would mark every deleted message unread and lose Important +/// on the way to the trash. Only the unique part is regenerated. +QString fresh(const QString &oldName) +{ + // The `:2,` suffix, when there is one. `info` is everything from the + // separator on, so an empty-flag `:2,` is preserved as faithfully as + // `:2,FS`. + QString info; + const int sep = oldName.indexOf(QStringLiteral(":2,")); + if (sep >= 0) + info = oldName.mid(sep); + + // The conventional left-to-right unique part: time, a per-process counter, + // the pid, the host. The counter is what makes two messages moved in the + // same second distinct, which a timestamp alone does not guarantee. + static quint64 counter = 0; + const qint64 now = QDateTime::currentSecsSinceEpoch(); + const QString host = QHostInfo::localHostName().isEmpty() + ? QStringLiteral("localhost") + : QHostInfo::localHostName(); + + return QStringLiteral("%1.M%2P%3Q%4.%5%6") + .arg(now) + .arg(QDateTime::currentMSecsSinceEpoch() % 1000) + .arg(QCoreApplication::applicationPid()) + .arg(++counter) + // A `/` or a `:` in a hostname would break the path or the flag + // separator. Neither is legal in a hostname, so this is belt and + // braces rather than a known case. + .arg(QString(host).replace(QLatin1Char('/'), QLatin1Char('_')) + .replace(QLatin1Char(':'), QLatin1Char('_'))) + .arg(info); +} + +} // namespace MaildirName diff --git a/src/maildirname.h b/src/maildirname.h new file mode 100644 index 0000000..f24bc71 --- /dev/null +++ b/src/maildirname.h @@ -0,0 +1,41 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include <QString> + +/// Maildir filename generation, shared by every path that writes a message +/// file: NotmuchWorker::moveMessages() and DraftStore. +/// +/// A namespace rather than a class; there is no state beyond a counter. +namespace MaildirName { + +/// A fresh, unique Maildir filename, preserving \p oldName's flag suffix. +/// +/// A FRESH name, never a reuse. mbsync writes a `,U=<n>` infix that is +/// meaningful only within one folder, and carrying it across a folder +/// boundary produced "Maildir error: duplicate UID" on real mail. Only the +/// `:2,` flag suffix is carried, because the flags describe the message +/// rather than its position. +/// +/// Pass an empty string for a message that has no previous name, which is +/// what a newly composed draft is. +QString fresh(const QString &oldName); + +} // namespace MaildirName diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index d0274cd..8c28ec5 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -20,16 +20,15 @@ #include <notmuch.h> -#include <QCoreApplication> #include <QDateTime> #include <QDir> #include <QDirIterator> #include <QFileInfo> -#include <QHostInfo> #include <QSet> #include <cstdlib> +#include "maildirname.h" #include "mimeparser.h" #include "nmraii.h" @@ -687,63 +686,6 @@ void NotmuchWorker::applyTags(const TagChange &change) emit tagsApplied(change); } -namespace { - -/// A fresh Maildir filename for a message being moved between folders, -/// preserving only its `:2,<flags>` suffix. -/// -/// mbsync's manual is explicit about why this exists, 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". -/// -/// The `,U=<n>` infix mbsync writes is its per-folder IMAP UID. 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 refuses the folder with `Maildir error: -/// duplicate UID`. Measured on real mail, four collisions in one folder from -/// a single move-and-restore. -/// -/// The FLAGS are kept, deliberately, and that is not a contradiction of -/// "as if the message was new". They record seen, flagged and replied, and -/// `maildir.synchronize_flags` is true, so notmuch reads them back as tags: -/// dropping them would mark every deleted message unread and lose Important -/// on the way to the trash. Only the unique part is regenerated. -QString freshMaildirName(const QString &oldName) -{ - // The `:2,` suffix, when there is one. `info` is everything from the - // separator on, so an empty-flag `:2,` is preserved as faithfully as - // `:2,FS`. - QString info; - const int sep = oldName.indexOf(QStringLiteral(":2,")); - if (sep >= 0) - info = oldName.mid(sep); - - // The conventional left-to-right unique part: time, a per-process counter, - // the pid, the host. The counter is what makes two messages moved in the - // same second distinct, which a timestamp alone does not guarantee. - static quint64 counter = 0; - const qint64 now = QDateTime::currentSecsSinceEpoch(); - const QString host = QHostInfo::localHostName().isEmpty() - ? QStringLiteral("localhost") - : QHostInfo::localHostName(); - - return QStringLiteral("%1.M%2P%3Q%4.%5%6") - .arg(now) - .arg(QDateTime::currentMSecsSinceEpoch() % 1000) - .arg(QCoreApplication::applicationPid()) - .arg(++counter) - // A `/` or a `:` in a hostname would break the path or the flag - // separator. Neither is legal in a hostname, so this is belt and - // braces rather than a known case. - .arg(QString(host).replace(QLatin1Char('/'), QLatin1Char('_')) - .replace(QLatin1Char(':'), QLatin1Char('_'))) - .arg(info); -} - -} // namespace - void NotmuchWorker::moveMessages(const QStringList &messageIds, const QString &destFolder) { @@ -822,11 +764,11 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds, continue; } - // A FRESH name, never the old one. See freshMaildirName(): carrying + // A FRESH name, never the old one. See MaildirName::fresh(): carrying // the `,U=` infix across a folder boundary is what produced // `Maildir error: duplicate UID` on real mail. const QString to = destDir + QLatin1Char('/') - + freshMaildirName(QFileInfo(from).fileName()); + + MaildirName::fresh(QFileInfo(from).fileName()); if (!QFile::rename(from, to)) { emit errorOccurred(QStringLiteral("Cannot move %1 to %2") diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 58d5659..15f9955 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -69,6 +69,7 @@ add_qtmaildir_test(busyindicator) add_qtmaildir_test(tagstrip) add_qtmaildir_test(messagedetailsdialog) add_qtmaildir_test(markdownrenderer) +add_qtmaildir_test(maildirname) add_qtmaildir_test(translations) # Asserts on the tracked .ts rather than the generated .qm: an untranslated # string is dropped by lrelease, so it is invisible in the .qm and shows up diff --git a/tests/test_maildirname.cpp b/tests/test_maildirname.cpp new file mode 100644 index 0000000..dcc8fab --- /dev/null +++ b/tests/test_maildirname.cpp @@ -0,0 +1,93 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "maildirname.h" + +#include <QSet> +#include <QTest> + +class TestMaildirName : public QObject +{ + Q_OBJECT + +private slots: + void aFreshNameIsUniquePerCall(); + void theFlagSuffixIsPreserved(); + void anEmptyFlagSuffixIsPreserved(); + void aNameWithNoSuffixGetsNone(); + void theUidInfixIsNotCarriedAcross(); +}; + +// Two messages written in the same second must not collide, which a +// timestamp alone does not guarantee, and that is what the counter is for. +void TestMaildirName::aFreshNameIsUniquePerCall() +{ + QSet<QString> names; + for (int i = 0; i < 100; ++i) + names.insert(MaildirName::fresh(QStringLiteral("1234.M1P1Q1.host"))); + + QVERIFY2(names.size() == 100, + qPrintable(QStringLiteral("expected 100 unique names, got %1") + .arg(names.size()))); +} + +// The flags say whether a message is read, flagged or draft, and losing them +// on a move silently marks mail unread again. +void TestMaildirName::theFlagSuffixIsPreserved() +{ + const QString name = MaildirName::fresh(QStringLiteral("1234.M1P1Q1.host:2,FS")); + QVERIFY2(name.endsWith(QStringLiteral(":2,FS")), + qPrintable(QStringLiteral("generated name did not preserve flags: %1") + .arg(name))); +} + +// `:2,` with no flags is not the same as no suffix at all, it says the flags +// are known and empty. +void TestMaildirName::anEmptyFlagSuffixIsPreserved() +{ + const QString name = MaildirName::fresh(QStringLiteral("1234.M1P1Q1.host:2,")); + QVERIFY2(name.endsWith(QStringLiteral(":2,")), + qPrintable(QStringLiteral("generated name did not preserve empty flag suffix: %1") + .arg(name))); +} + +// A suffix must not be invented. +void TestMaildirName::aNameWithNoSuffixGetsNone() +{ + const QString name = MaildirName::fresh(QStringLiteral("1234.M1P1Q1.host")); + QVERIFY2(!name.contains(QStringLiteral(":2,")), + qPrintable(QStringLiteral("generated name invented a flag suffix: %1") + .arg(name))); +} + +// This is the reason the function exists; carrying mbsync's `,U=` infix +// across a folder boundary produced "Maildir error: duplicate UID" on real +// mail. +void TestMaildirName::theUidInfixIsNotCarriedAcross() +{ + const QString name = MaildirName::fresh(QStringLiteral("1234.M1P1Q1.host,U=42:2,S")); + QVERIFY2(!name.contains(QStringLiteral("U=42")), + qPrintable(QStringLiteral("generated name carried the UID infix across: %1") + .arg(name))); + QVERIFY2(name.endsWith(QStringLiteral(":2,S")), + qPrintable(QStringLiteral("generated name did not preserve flags: %1") + .arg(name))); +} + +QTEST_MAIN(TestMaildirName) +#include "test_maildirname.moc" |