diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-07 19:17:59 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-07 19:17:59 +0200 |
| commit | 01194fa2c48019f1dcbdfefac94f8fd5f8e38122 (patch) | |
| tree | e833545eaa42c44f1cb984bd52fbf7bb66d36102 /tests | |
| parent | 6bbaa1d796d3d7828c3fa6b82c7031f8aaae84bf (diff) | |
| download | qtmaildir-01194fa2c48019f1dcbdfefac94f8fd5f8e38122.tar.gz qtmaildir-01194fa2c48019f1dcbdfefac94f8fd5f8e38122.zip | |
feat(sync): sync only the accounts with unsynced edits
A sync ran mbsync -a regardless of what changed, so tagging mail in one
account fetched all of them. The account set was not a parameter anywhere
on the path: MailSync::start() took no arguments and the script hardcoded
-a, so nothing between a tag edit and mbsync carried which account changed.
Track which accounts have edits and pass their mbsync channels through to
the script, which now takes channel names and falls back to -a when given
none. An empty set means all accounts, per the request: a sync with nothing
pending is a fetch, and narrowing that to wherever the last edit landed
would quietly stop collecting mail everywhere else.
The channel is a new optional per-account key rather than the section key.
The two names genuinely diverge, because a QSettings section key may carry
dots that the channel does not, and mbsync treats an unknown channel as
fatal rather than skipping it, so key-as-channel would fail those accounts'
syncs outright rather than degrade. It defaults to the key, so accounts
whose two names already agree need no config change.
The edited-account set is deliberately not netted the way the pending-edit
map is: that map tracks the index, where a tag removed and re-added leaves
nothing outstanding, while this tracks the mail store, where both writes
have already renamed files that mbsync still has to propagate. It is also
snapshotted before flushHeldEdits(), which inserts into it synchronously
rather than on a queued reply, so a successful sync cannot clear accounts
whose edits it never carried.
Closes item 49.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_config.cpp | 33 | ||||
| -rw-r--r-- | tests/test_mailsync.cpp | 52 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 43 |
3 files changed, 128 insertions, 0 deletions
diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 4d92891..dc85968 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -51,6 +51,8 @@ private slots: void extraMimetypesAppendToBuiltins(); void extraMimetypeDescriptionMayContainComma(); void malformedExtraMimetypeIsSkipped(); + void syncChannelDefaultsToTheAccountKey(); + void syncChannelIsActuallyRead(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -524,5 +526,36 @@ void TestConfig::malformedExtraMimetypeIsSkipped() QVERIFY(!config.problems().isEmpty()); } +void TestConfig::syncChannelDefaultsToTheAccountKey() +{ + // Most accounts name their mbsync channel exactly as their section key, so + // the common case must need no config edit at all. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir = work\n"))); + + QCOMPARE(config.accounts().size(), 1); + QCOMPARE(config.accounts().at(0).syncChannel(), QStringLiteral("work")); +} + +void TestConfig::syncChannelIsActuallyRead() +{ + // The key exists because the two names genuinely diverge: a QSettings + // section key may carry dots that the mbsync channel does not, and passing + // the section key to mbsync would name a channel that does not exist. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.mail-first.last]\n" + "maildir = mail-first.last\n" + "channel = mail-firstlast\n"))); + + QCOMPARE(config.accounts().size(), 1); + QCOMPARE(config.accounts().at(0).syncChannel(), + QStringLiteral("mail-firstlast")); +} + QTEST_MAIN(TestConfig) #include "test_config.moc" diff --git a/tests/test_mailsync.cpp b/tests/test_mailsync.cpp index 6c93e8d..e91de3e 100644 --- a/tests/test_mailsync.cpp +++ b/tests/test_mailsync.cpp @@ -43,6 +43,9 @@ private slots: void missingBinaryReportsFailureNotSilence(); void startDoesNotBlock(); void argumentsAreNotShellInterpreted(); + void channelsAreAppendedToTheCommand(); + void noChannelsMeansNoExtraArguments(); + void channelNamesAreNotShellInterpreted(); void phaseStartsAsMbsync(); void notmuchLineSwitchesPhase(); @@ -262,6 +265,55 @@ void TestMailSync::argumentsAreNotShellInterpreted() QVERIFY(sync.log().contains(QStringLiteral("; touch"))); } +void TestMailSync::channelsAreAppendedToTheCommand() +{ + // Item 49: a sync that knows which accounts were touched passes their + // channel names, and they must reach the script as separate arguments + // after whatever the config line already carries. + const QString script = makeScript(QStringLiteral("chan.sh"), + QStringLiteral("echo \"[$@]\"")); + + MailSync sync(script); + QSignalSpy spy(&sync, &MailSync::finished); + QVERIFY(sync.start({ QStringLiteral("work"), QStringLiteral("personal") })); + QVERIFY(spy.wait(5000)); + + QVERIFY(sync.log().contains(QStringLiteral("[work personal]"))); +} + +void TestMailSync::noChannelsMeansNoExtraArguments() +{ + // Empty means "sync everything", which is the script's own default. It must + // not become an empty string argument: mbsync would read that as a channel + // named "" and fail the run. + const QString script = makeScript(QStringLiteral("nochan.sh"), + QStringLiteral("echo \"count=$#\"")); + + MailSync sync(script); + QSignalSpy spy(&sync, &MailSync::finished); + QVERIFY(sync.start()); + QVERIFY(spy.wait(5000)); + + QVERIFY(sync.log().contains(QStringLiteral("count=0"))); +} + +void TestMailSync::channelNamesAreNotShellInterpreted() +{ + // Channel names are derived from config, same trust boundary as the command + // itself, and reach the same QProcess argument list. The injection test + // above covers the command; this covers the half added for item 49. + const QString script = makeScript(QStringLiteral("chanargs.sh"), + QStringLiteral("echo \"$1\"")); + + MailSync sync(script); + QSignalSpy spy(&sync, &MailSync::finished); + QVERIFY(sync.start({ QStringLiteral("; touch %1/chanpwned") + .arg(m_dir.path()) })); + QVERIFY(spy.wait(5000)); + + QVERIFY(!QFile::exists(m_dir.filePath(QStringLiteral("chanpwned")))); +} + void TestMailSync::phaseStartsAsMbsync() { // A fresh tracker has nothing to report until it is fed, and a run is diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 82686e5..f84bbab 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -28,6 +28,9 @@ class TestThreadListModel : public QObject Q_OBJECT private slots: void startsEmpty(); + void accountKeysComeFromTheAccountTags(); + void accountKeysCoverAThreadSpanningTwoAccounts(); + void accountKeysAreEmptyForAnUnknownThread(); void appendsBatches(); void appendingEmptyBatchIsNoOp(); void clearResetsModel(); @@ -73,6 +76,46 @@ static ThreadSummary makeThread(const QString &id, const QString &subject) return t; } +void TestThreadListModel::accountKeysComeFromTheAccountTags() +{ + // Item 49 reads this to decide which mbsync channels a sync needs. Only + // account tags count: a functional tag names no mailbox. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("Hi")); + t.tags.append(TagColors::tagForAccountKey(QStringLiteral("work"))); + model.appendBatch({ t }); + + QCOMPARE(model.accountKeysForThread(QStringLiteral("t1")), + QStringList{ QStringLiteral("work") }); +} + +void TestThreadListModel::accountKeysCoverAThreadSpanningTwoAccounts() +{ + // The row shows one chip, but tagging this thread touches files under both + // mailboxes. Returning only the first would strand the other's edits. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("Hi")); + t.tags.append(TagColors::tagForAccountKey(QStringLiteral("work"))); + t.tags.append(TagColors::tagForAccountKey(QStringLiteral("personal"))); + model.appendBatch({ t }); + + QStringList keys = model.accountKeysForThread(QStringLiteral("t1")); + keys.sort(); + QCOMPARE(keys, (QStringList{ QStringLiteral("personal"), + QStringLiteral("work") })); +} + +void TestThreadListModel::accountKeysAreEmptyForAnUnknownThread() +{ + // A thread the model no longer holds must yield nothing rather than + // matching some other row. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), + QStringLiteral("Hi")) }); + + QVERIFY(model.accountKeysForThread(QStringLiteral("nope")).isEmpty()); +} + void TestThreadListModel::startsEmpty() { ThreadListModel model; |
