From db36ceabeb698b1988f7f6ecfe694c226eb156d7 Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Fri, 2 Mar 2018 11:58:18 +0100 Subject: Fixed synchronization with new mail notifications --- examples/imapresource/imapresource.cpp | 44 +++++++++++++++--------- examples/imapresource/tests/imapmailsynctest.cpp | 25 ++++++++++++-- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/examples/imapresource/imapresource.cpp b/examples/imapresource/imapresource.cpp index f87c5ff..811e560 100644 --- a/examples/imapresource/imapresource.cpp +++ b/examples/imapresource/imapresource.cpp @@ -267,13 +267,13 @@ public: }) // //First we fetch flag changes for all messages. Since we don't know which messages are locally available we just get everything and only apply to what we have. .then([=] { - auto uidNext = syncStore().readValue(folderRemoteId, "uidnext").toLongLong(); + auto lastSeenUid = syncStore().readValue(folderRemoteId, "uidnext").toLongLong(); bool ok = false; const auto changedsince = syncStore().readValue(folderRemoteId, "changedsince").toLongLong(&ok); SinkLogCtx(mLogCtx) << "About to update flags" << folder.path() << "changedsince: " << changedsince; //If we have any mails so far we start off by updating any changed flags using changedsince if (ok) { - return imap->fetchFlags(folder, KIMAP2::ImapSet(1, qMax(uidNext, qint64(1))), changedsince, [=](const Message &message) { + return imap->fetchFlags(folder, KIMAP2::ImapSet(1, qMax(lastSeenUid, qint64(1))), changedsince, [=](const Message &message) { const auto folderLocalId = syncStore().resolveRemoteId(ENTITY_TYPE_FOLDER, folderRemoteId); const auto remoteId = assembleMailRid(folderLocalId, message.uid); @@ -315,12 +315,12 @@ public: return job.then([=](const QVector &uidsToFetch) { SinkTraceCtx(mLogCtx) << "Received result set " << uidsToFetch; SinkTraceCtx(mLogCtx) << "About to fetch mail" << folder.path(); - const auto uidNext = syncStore().readValue(folderRemoteId, "uidnext").toLongLong(); + const auto lastSeenUid = syncStore().readValue(folderRemoteId, "uidnext").toLongLong(); - //Make sure the uids are sorted in reverse order and drop everything below uidNext (so we don't refetch what we already have + //Make sure the uids are sorted in reverse order and drop everything below lastSeenUid (so we don't refetch what we already have QVector filteredAndSorted = uidsToFetch; qSort(filteredAndSorted.begin(), filteredAndSorted.end(), qGreater()); - auto lowerBound = qLowerBound(filteredAndSorted.begin(), filteredAndSorted.end(), uidNext, qGreater()); + auto lowerBound = qLowerBound(filteredAndSorted.begin(), filteredAndSorted.end(), lastSeenUid, qGreater()); if (lowerBound != filteredAndSorted.end()) { filteredAndSorted.erase(lowerBound, filteredAndSorted.end()); } @@ -348,13 +348,14 @@ public: } }) .then([=] { - SinkLogCtx(mLogCtx) << "UIDMAX: " << *maxUid << folder.path(); + SinkLogCtx(mLogCtx) << "Highest found uid: " << *maxUid << folder.path(); if (*maxUid > 0) { syncStore().writeValue(folderRemoteId, "uidnext", QByteArray::number(*maxUid)); } else { if (serverUidNext) { + SinkLogCtx(mLogCtx) << "Storing the server side uidnext: " << serverUidNext << folder.path(); //If we don't receive a mail we should still record the updated uidnext value. - syncStore().writeValue(folderRemoteId, "uidnext", QByteArray::number(serverUidNext)); + syncStore().writeValue(folderRemoteId, "uidnext", QByteArray::number(serverUidNext - 1)); } } syncStore().writeValue(folderRemoteId, "fullsetLowerbound", QByteArray::number(lowerBoundUid)); @@ -567,17 +568,26 @@ public: synchronizeFolders(*folderList); return *folderList; }) + //The rest is only to check for new messages. .each([=](const Imap::Folder &folder) { - //TODO examine instead - return imap->select(folder) - .then([=](const SelectResult &result) { - const auto folderRemoteId = folderRid(folder); - auto localUidNext = syncStore().readValue(folderRemoteId, "uidnext").toLongLong(); - if (result.uidNext > localUidNext) { - const auto folderLocalId = syncStore().resolveRemoteId(ENTITY_TYPE_FOLDER, folderRemoteId); - emitNotification(Notification::Info, ApplicationDomain::NewContentAvailable, {}, {}, {{folderLocalId}}); - } - }); + if (!folder.noselect && folder.subscribed) { + return imap->examine(folder) + .then([=](const SelectResult &result) { + const auto folderRemoteId = folderRid(folder); + auto lastSeenUid = syncStore().readValue(folderRemoteId, "uidnext").toLongLong(); + SinkTraceCtx(mLogCtx) << "Checking for new messages." << folderRemoteId << " Last seen uid: " << lastSeenUid << " Uidnext: " << result.uidNext; + if (result.uidNext > (lastSeenUid + 1)) { + const auto folderLocalId = syncStore().resolveRemoteId(ENTITY_TYPE_FOLDER, folderRemoteId); + emitNotification(Notification::Info, ApplicationDomain::NewContentAvailable, {}, {}, {{folderLocalId}}); + } + }).then([=] (const KAsync::Error &error) { + if (error) { + //Ignore the error because we don't want to fail the synchronization here + SinkWarningCtx(mLogCtx) << "Examine failed: " << error.errorMessage; + } + }); + } + return KAsync::null(); }); }) .then([=] (const KAsync::Error &error) { diff --git a/examples/imapresource/tests/imapmailsynctest.cpp b/examples/imapresource/tests/imapmailsynctest.cpp index 1d62a80..06369f3 100644 --- a/examples/imapresource/tests/imapmailsynctest.cpp +++ b/examples/imapresource/tests/imapmailsynctest.cpp @@ -135,12 +135,15 @@ protected: private slots: void testNewMailNotification() { + createFolder(QStringList() << "testNewMailNotification"); + createMessage(QStringList() << "testNewMailNotification", newMessage("Foobar")); + const auto syncFolders = Sink::SyncScope{ApplicationDomain::getTypeName()}.resourceFilter(mResourceInstanceIdentifier); //Fetch folders initially VERIFYEXEC(Store::synchronize(syncFolders)); VERIFYEXEC(ResourceControl::flushMessageQueue(mResourceInstanceIdentifier)); - auto folder = Store::readOne(Sink::Query{}.resourceFilter(mResourceInstanceIdentifier).filter("test")); + auto folder = Store::readOne(Sink::Query{}.resourceFilter(mResourceInstanceIdentifier).filter("testNewMailNotification")); Q_ASSERT(!folder.identifier().isEmpty()); const auto syncTestMails = Sink::SyncScope{ApplicationDomain::getTypeName()}.resourceFilter(mResourceInstanceIdentifier).filter(QVariant::fromValue(folder.identifier())); @@ -172,7 +175,7 @@ private slots: QVERIFY(!notificationReceived); //Create message and retry - createMessage(QStringList() << "test", newMessage("This is a Subject.")); + createMessage(QStringList() << "testNewMailNotification", newMessage("This is a Subject.")); //Should result in change notification VERIFYEXEC(Store::synchronize(syncFolders)); @@ -180,6 +183,24 @@ private slots: QTRY_VERIFY(notificationReceived); } + + void testSyncFolderBeforeFetchingNewMessages() + { + const auto syncScope = Sink::Query{}.resourceFilter(mResourceInstanceIdentifier); + + createFolder(QStringList() << "test3"); + + VERIFYEXEC(Store::synchronize(syncScope)); + VERIFYEXEC(ResourceControl::flushMessageQueue(mResourceInstanceIdentifier)); + + createMessage(QStringList() << "test3", newMessage("Foobar")); + + VERIFYEXEC(Store::synchronize(syncScope)); + VERIFYEXEC(ResourceControl::flushMessageQueue(mResourceInstanceIdentifier)); + + auto mailQuery = Sink::Query{}.resourceFilter(mResourceInstanceIdentifier).request().filter(Sink::Query{}.filter("test3")); + QCOMPARE(Store::read(mailQuery).size(), 1); + } }; QTEST_MAIN(ImapMailSyncTest) -- cgit v1.2.3