Don't merge collection sync queue items originating from different places

The aim of the overal idea of merging queued collection sync request
was to reduce the number of synchronization calls. These can be
long-lasting operations, which could queue-up quickly. Merging them
allows to reduce the effect of long sync queues.

The original implementation however took merging a step too far, as it
merged requests based on the collection id, regardless of their
origin, which currently can be either an explicit sync
request (ResourceBase::retrieveItems) or a background sync due to
server-side subscription trigger. This could lead to a scenario, where
the Akonadi-triggered sync would be merged away when encountering a
queued subscription sync. This would cause the start handler not to be
called resulting in a broken resource state machine (no call to
itemsRetrieved()).

Fix this by adding the type of sync request to the merge criteria.
Signed-off-by: Krzysztof Nowicki <krissn@op.pl>

resources/ews/ewsresource.cpp

@@ -321,7 +321,7 @@ void EwsResource::connectionError()
 
 void EwsResource::retrieveItems(const Collection &collection)
 {
-    queueFetchItemsJob(collection, [this](EwsFetchItemsJob *fetchJob) {
+    queueFetchItemsJob(collection, RetrieveItems, [this](EwsFetchItemsJob *fetchJob) {
         auto col = fetchJob->collection();
         if (fetchJob->error()) {
             qCWarningNC(EWSRES_LOG) << QStringLiteral("Item fetch error:") << fetchJob->errorString() << fetchJob->error() << fetchJob->ewsResponseCode();
@@ -354,9 +354,9 @@ void EwsResource::retrieveItems(const Collection &collection)
     });
 }
 
-void EwsResource::queueFetchItemsJob(const Akonadi::Collection &col, std::function<void(EwsFetchItemsJob *)> startFn)
+void EwsResource::queueFetchItemsJob(const Akonadi::Collection &col, QueuedFetchItemsJobType type, std::function<void(EwsFetchItemsJob *)> startFn)
 {
-    qCDebugNC(EWSRES_LOG) << QStringLiteral("Enqueuing sync for collection ") << col;
+    qCDebugNC(EWSRES_LOG) << QStringLiteral("Enqueuing sync for collection ") << col << col.id();
 
     const auto queueEmpty = mFetchItemsJobQueue.empty();
     if (mFetchItemsJobQueue.count() > 1) {
@@ -370,7 +370,9 @@ void EwsResource::queueFetchItemsJob(const Akonadi::Collection &col, std::functi
         }
     }
 
-    mFetchItemsJobQueue.enqueue({col, startFn});
+    mFetchItemsJobQueue.enqueue({col, type, startFn});
+
+    qCDebugNC(EWSRES_LOG) << QStringLiteral("Sync queue state: ") << dumpResourceToString().replace(QLatin1Char('\n'), QLatin1Char(' '));
 
     if (queueEmpty) {
         startFetchItemsJob(col, startFn);
@@ -379,7 +381,7 @@ void EwsResource::queueFetchItemsJob(const Akonadi::Collection &col, std::functi
 
 void EwsResource::dequeueFetchItemsJob()
 {
-    qCDebugNC(EWSRES_LOG) << QStringLiteral("Finished queued sync");
+    qCDebugNC(EWSRES_LOG) << QStringLiteral("Finished queued sync ") << mFetchItemsJobQueue.head().col << mFetchItemsJobQueue.head().col.id();
 
     mFetchItemsJobQueue.dequeue();
 
@@ -1084,7 +1086,7 @@ void EwsResource::foldersModifiedCollectionSyncFinished(KJob *job)
 
     auto fetchColJob = qobject_cast<CollectionFetchJob *>(job);
     const auto collection = fetchColJob->collections().at(0);
-    queueFetchItemsJob(collection, [this](EwsFetchItemsJob *fetchJob) {
+    queueFetchItemsJob(collection, SubscriptionSync, [this](EwsFetchItemsJob *fetchJob) {
         auto collection = fetchJob->collection();
         if (fetchJob->error()) {
             qCWarningNC(EWSRES_LOG) << QStringLiteral("Item fetch error:") << fetchJob->errorString() << fetchJob->error();
@@ -1464,7 +1466,7 @@ QString EwsResource::dumpResourceToString() const
 {
     QString dump = QStringLiteral("item sync queue (%1):\n").arg(mFetchItemsJobQueue.count());
 
-    for (const auto &item : std::as_const(mFetchItemsJobQueue)) {
+    for (const auto &item : qAsConst(mFetchItemsJobQueue)) {
         dump += QStringLiteral(" %1:%2\n").arg(item.col.id()).arg(item.type);
     }
 

resources/ews/ewsresource.h

@@ -128,6 +128,8 @@ private Q_SLOTS:
 private:
     enum AuthStage { AuthIdle, AuthRefreshToken, AuthAccessToken, AuthFailure };
 
+    enum QueuedFetchItemsJobType { RetrieveItems, SubscriptionSync };
+
     void finishItemsFetch(FetchItemState *state);
     void fetchSpecialFolders();
     void specialFoldersCollectionsRetrieved(const Akonadi::Collection::List &folders);
@@ -145,7 +147,7 @@ private:
     static QString getCollectionSyncState(const Akonadi::Collection &col);
     static void saveCollectionSyncState(Akonadi::Collection &col, const QString &state);
 
-    void queueFetchItemsJob(const Akonadi::Collection &col, std::function<void(EwsFetchItemsJob *)> startFn);
+    void queueFetchItemsJob(const Akonadi::Collection &col, QueuedFetchItemsJobType type, std::function<void(EwsFetchItemsJob *)> startFn);
     void startFetchItemsJob(const Akonadi::Collection &col, std::function<void(EwsFetchItemsJob *)> startFn);
     void dequeueFetchItemsJob();
 
@@ -170,6 +172,7 @@ private:
 
     struct QueuedFetchItemsJob {
         Akonadi::Collection col;
+        QueuedFetchItemsJobType type;
         std::function<void(EwsFetchItemsJob *)> startFn;
     };
     QQueue<QueuedFetchItemsJob> mFetchItemsJobQueue;