From e125d970aa2f4ad6cd494e5f410ba1c5e53f5308 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 17 Aug 2026 20:04:05 +0200 Subject: feat(worker): move messages between maildir folders The first mutation here that is not a notmuch tag. Indexes the new path before dropping the old one, since removing the last filename for a message id deletes the database entry and every tag on it. --- src/notmuchworker.cpp | 106 +++++++++++++++++++++++++++++++ src/notmuchworker.h | 18 ++++++ tests/test_notmuchworker.cpp | 144 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+) diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index b152830..6c41839 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -614,6 +614,112 @@ void NotmuchWorker::applyTags(const TagChange &change) emit tagsApplied(change); } +void NotmuchWorker::moveMessages(const QStringList &messageIds, + const QString &destFolder) +{ + if (messageIds.isEmpty() || destFolder.isEmpty()) + return; + + // The read-only handle must be closed first: notmuch allows only one open + // handle per process. Same ordering as applyTags, for the same reason. + close(); + + const QByteArray configPath = configPathArg(); + notmuch_database_t *db = nullptr; + char *error = nullptr; + const notmuch_status_t status = notmuch_database_open_with_config( + nullptr, + NOTMUCH_DATABASE_MODE_READ_WRITE, + configPath.isEmpty() ? nullptr : configPath.constData(), + nullptr, + &db, + &error); + + if (status != NOTMUCH_STATUS_SUCCESS) { + emit errorOccurred( + QStringLiteral("Cannot open database for writing: %1") + .arg(QString::fromUtf8(error ? error + : notmuch_status_to_string(status)))); + free(error); + return; + } + + const QString root = QString::fromUtf8(notmuch_database_get_path(db)); + const QString destDir = + root + QLatin1Char('/') + destFolder + QStringLiteral("/cur"); + + QStringList moved; + for (const QString &id : messageIds) { + notmuch_message_t *raw = nullptr; + // find_message reports SUCCESS with a null message when the id is not + // in the database, so both have to be checked. A stale id must not + // abort the batch: the live ids alongside it still need moving. + if (notmuch_database_find_message(db, id.toUtf8().constData(), &raw) + != NOTMUCH_STATUS_SUCCESS || !raw) { + continue; + } + NmMessage message(raw); + + const char *rawName = notmuch_message_get_filename(message.get()); + if (!rawName) + continue; + const QString from = QString::fromUtf8(rawName); + // The handle is released before the file moves under it. + message.reset(); + + // cur/, never new/. A file dropped in new/ is re-announced as fresh + // mail by every reader of the Maildir. + if (!QDir().mkpath(destDir)) { + emit errorOccurred(QStringLiteral("Cannot create folder %1") + .arg(destDir)); + continue; + } + + const QString to = destDir + QLatin1Char('/') + QFileInfo(from).fileName(); + if (from == to) { + // Already where it was asked to go. Reported as moved, since the + // caller's request is satisfied. + moved.append(id); + continue; + } + + if (!QFile::rename(from, to)) { + emit errorOccurred(QStringLiteral("Cannot move %1 to %2") + .arg(QFileInfo(from).fileName(), destFolder)); + continue; + } + + // Index the NEW path BEFORE dropping the old one. The reverse order + // removes the last filename for this message id, which deletes the + // database entry and every tag on it; the file then reindexes as a + // brand new message with default tags, silently. + notmuch_message_t *indexed = nullptr; + const notmuch_status_t added = notmuch_database_index_file( + db, to.toUtf8().constData(), nullptr, &indexed); + if (indexed) + notmuch_message_destroy(indexed); + + // DUPLICATE_MESSAGE_ID is success here: it means the id was already + // known, which is exactly the case for a file this just moved. + if (added != NOTMUCH_STATUS_SUCCESS + && added != NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) { + QFile::rename(to, from); + emit errorOccurred(QStringLiteral("Cannot index %1 at its new path: %2") + .arg(id, QString::fromUtf8( + notmuch_status_to_string(added)))); + continue; + } + + notmuch_database_remove_message(db, from.toUtf8().constData()); + moved.append(id); + } + + notmuch_database_close(db); + notmuch_database_destroy(db); + + emit messagesMoved(moved, destFolder); +} + void NotmuchWorker::requestAllTags(quint64 generation) { if (!openReadOnly()) diff --git a/src/notmuchworker.h b/src/notmuchworker.h index f07e563..d8d8ff8 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -120,6 +120,19 @@ public slots: /// it would block the user's cron `notmuch new`. void applyTags(const TagChange &change); + /// Moves messages into `destFolder`, relative to the database path. + /// + /// A folder NAME rather than a "move to trash" call, because v2's Send + /// needs exactly this operation for Drafts and Sent. Nothing + /// trash-specific belongs here. + /// + /// The first mutation in this class that is not a notmuch tag: a rename on + /// disk plus a reindex. Ordering is rename, index the new path, drop the + /// old one. Indexing first is required, not stylistic: removing the last + /// filename for a message id deletes the database entry and every tag on + /// it, so removing before indexing loses the message's tags. + void moveMessages(const QStringList &messageIds, const QString &destFolder); + /// Batch tagging over whole threads. The UI holds thread ids, not message /// ids, for rows it has not opened, so the resolution happens here where /// the database handle lives. This is the path the archive/flag/delete @@ -184,6 +197,11 @@ signals: quint64 generation); void messageLoaded(const QVector &messages, quint64 generation); void tagsApplied(const TagChange &change); + + /// Carries the ids that ACTUALLY moved, which may be fewer than requested. + /// A stale id, a missing folder or a failed rename drops out here rather + /// than aborting the batch. + void messagesMoved(const QStringList &messageIds, const QString &destFolder); void allTagsReady(const QStringList &tags, quint64 generation); /// One entry per requested query, in the order they were asked for. A query diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 899fd11..bf1c23c 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -86,7 +86,20 @@ private slots: void requestFoldersListsEveryMaildirFolder(); void requestFoldersOnUnreadableConfigEmitsError(); + void moveMessagesRelocatesTheFile(); + void moveMessagesReindexesAtTheNewPath(); + void moveMessagesKeepsTheMessagesTags(); + void moveMessagesReportsOnlyWhatMoved(); + private: + /// Adds one read message in `folder` and reindexes, for the move tests. + /// Each of those takes its own message, because a move is destructive and + /// the fixture database is shared by every test in this class. + bool addMovableMessage(const QString &folder, const QString &messageId); + /// The single file backing `messageId`, or an empty string when the + /// database does not know the id. + QString fileOf(const QString &messageId); + /// Tags of one message, read back through a fresh worker query. QStringList tagsOf(const QString &messageId); QVector messagesOfThread(const QString &threadId, @@ -181,6 +194,34 @@ QString TestNotmuchWorker::threadIdOf(const QString &subject) return QString(); } +bool TestNotmuchWorker::addMovableMessage(const QString &folder, + const QString &messageId) +{ + if (!m_fixture.addMessage(folder, messageId, + QStringLiteral("Movable %1").arg(messageId), + QStringLiteral("Erin "), + QStringLiteral("Sun, 7 Jun 2026 10:00:00 +0000"), + QStringLiteral("movable body"), false)) { + return false; + } + return m_fixture.index(); +} + +QString TestNotmuchWorker::fileOf(const QString &messageId) +{ + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy loaded(&worker, &NotmuchWorker::threadLoaded); + worker.loadThread(QStringLiteral("{id:%1}").arg(messageId), QString(), 1); + if (loaded.isEmpty()) + return {}; + const auto messages = loaded.first().at(0).value>(); + for (const MessageRef &m : messages) { + if (m.messageId == messageId) + return m.filePath; + } + return {}; +} + QVector TestNotmuchWorker::messagesOfThread(const QString &threadId, const QString &matchQuery, bool matchedOnly) @@ -1096,5 +1137,108 @@ void TestNotmuchWorker::requestFoldersOnUnreadableConfigEmitsError() QVERIFY(ready.isEmpty()); } +void TestNotmuchWorker::moveMessagesRelocatesTheFile() +{ + const QString id = QStringLiteral("move1@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + const QString before = fileOf(id); + QVERIFY(!before.isEmpty()); + QVERIFY(QFile::exists(before)); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.moveMessages({ id }, QStringLiteral("trash")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QCOMPARE(moved.size(), 1); + QCOMPARE(moved.first().at(0).toStringList(), QStringList{ id }); + QCOMPARE(moved.first().at(1).toString(), QStringLiteral("trash")); + + // 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)); + QVERIFY(!QFile::exists(before)); +} + +void TestNotmuchWorker::moveMessagesReindexesAtTheNewPath() +{ + // The half a filesystem check cannot see. A moved file with a stale index + // entry sits correctly on disk and is invisible to every query. + const QString id = QStringLiteral("move2@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + 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 QVector inTrash = + runQuery(QStringLiteral("path:\"trash/**\" and id:%1").arg(id)); + QCOMPARE(inTrash.size(), 1); + + const QVector inInbox = + runQuery(QStringLiteral("path:\"inbox/**\" and id:%1").arg(id)); + QCOMPARE(inInbox.size(), 0); +} + +void TestNotmuchWorker::moveMessagesKeepsTheMessagesTags() +{ + // The ordering test. notmuch_database_remove_message() removes the LAST + // filename for a message id by deleting the whole database entry, tags + // included, so the new path must be indexed before the old one is dropped. + // The reverse order leaves the file correctly placed, findable by query, + // and stripped of every tag the user ever put on it. + const QString id = QStringLiteral("move3@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + NotmuchWorker tagger(m_fixture.configPath()); + tagger.applyTags(TagChange{ { id }, + { QStringLiteral("keepme") }, + {}, + QStringLiteral("Tag before moving") }); + QVERIFY(tagsOf(id).contains(QStringLiteral("keepme"))); + + 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 QStringList after = tagsOf(id); + QVERIFY2(after.contains(QStringLiteral("keepme")), + qPrintable(QStringLiteral("tags after the move: %1") + .arg(after.join(QLatin1Char(' '))))); +} + +void TestNotmuchWorker::moveMessagesReportsOnlyWhatMoved() +{ + // A stale id must not abort the batch, and must not be reported as moved + // either: a caller that assumed the request succeeded would show a delete + // that never happened. + const QString id = QStringLiteral("move4@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + + worker.moveMessages({ QStringLiteral("nosuchmessage@example.org"), id }, + QStringLiteral("trash")); + + QCOMPARE(moved.size(), 1); + QCOMPARE(moved.first().at(0).toStringList(), QStringList{ id }); + + const QVector inTrash = + runQuery(QStringLiteral("path:\"trash/**\" and id:%1").arg(id)); + QCOMPARE(inTrash.size(), 1); +} + QTEST_MAIN(TestNotmuchWorker) #include "test_notmuchworker.moc" -- cgit v1.2.3