Branch 'kolab/integration/4.13.0' - 6 commits - akonadi/collectionfetchjob.cpp akonadi/collectionfetchscope.cpp akonadi/collectionfetchscope.h akonadi/collectionsync.cpp akonadi/itemmodifyjob.cpp akonadi/protocolhelper.cpp akonadi/resourcebase.cpp akonadi/tagsync.cpp akonadi/tests

Christian Mollekopf mollekopf at kolabsys.com
Fri Oct 31 10:09:08 CET 2014


 akonadi/collectionfetchjob.cpp      |   48 +++++++++++++++++++++++----
 akonadi/collectionfetchscope.cpp    |   13 +++++++
 akonadi/collectionfetchscope.h      |   18 ++++++++++
 akonadi/collectionsync.cpp          |   24 ++++++++++---
 akonadi/itemmodifyjob.cpp           |    8 +---
 akonadi/protocolhelper.cpp          |    5 --
 akonadi/resourcebase.cpp            |    1 
 akonadi/tagsync.cpp                 |   63 ++++++++++++++++++++----------------
 akonadi/tests/collectionjobtest.cpp |   18 ++++++++++
 akonadi/tests/collectionjobtest.h   |    1 
 10 files changed, 148 insertions(+), 51 deletions(-)

New commits:
commit 88794d8ad451c55a4c570b94f3500cf90b4213ea
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Fri Oct 31 09:22:02 2014 +0100

    CollectionSync: Prevent double result emission

diff --git a/akonadi/collectionsync.cpp b/akonadi/collectionsync.cpp
index 17aa83d..d8621c3 100644
--- a/akonadi/collectionsync.cpp
+++ b/akonadi/collectionsync.cpp
@@ -145,6 +145,7 @@ public:
         , localListDone(false)
         , deliveryDone(false)
         , akonadiRootCollection(Collection::root())
+        , resultEmitted(false)
     {
     }
 
@@ -500,7 +501,8 @@ public:
                 currentTransaction->rollback();
                 q->setError(Unknown);
                 q->setErrorText(i18n("Found unresolved orphan collections"));
-                q->emitResult();
+                kWarning() << "found unresolved orphan collection";
+                emitResult();
                 return;
             }
 
@@ -515,7 +517,7 @@ public:
             currentTransaction->rollback();
             q->setError(Unknown);
             q->setErrorText(i18n("Incomplete collection tree"));
-            q->emitResult();
+            emitResult();
             return;
         }
         */
@@ -629,6 +631,7 @@ public:
     void done()
     {
         if (currentTransaction) {
+            //This can trigger a direct call of transactionSequenceResult
             currentTransaction->commit();
             currentTransaction = 0;
         }
@@ -637,7 +640,17 @@ public:
             q->setError(Unknown);
             q->setErrorText(i18n("Found unresolved orphan collections"));
         }
-        q->emitResult();
+        emitResult();
+    }
+
+    void emitResult()
+    {
+        //Prevent double result emission
+        Q_ASSERT(!resultEmitted);
+        if (!resultEmitted) {
+            resultEmitted = true;
+            q->emitResult();
+        }
     }
 
     void createTransaction()
@@ -659,7 +672,6 @@ public:
         // a new transaction in the queue already
         if (job == currentTransaction) {
             currentTransaction = 0;
-            q->emitResult();
         }
     }
 
@@ -691,7 +703,7 @@ public:
             // There's nothing to do after local listing -> we are done!
             if (remoteCollectionsToCreate.isEmpty() && remoteCollectionsToUpdate.isEmpty() && localCollectionsToRemove.isEmpty()) {
                 kDebug() << "Nothing to do";
-                q->emitResult();
+                emitResult();
                 return;
             }
             // Ok, there's some work to do, so create a transaction we can use
@@ -735,6 +747,8 @@ public:
     // access
     Collection akonadiRootCollection;
 
+    bool resultEmitted;
+
 
 };
 


commit 3be0925dbf75df4fa3a7bb648cfaeb4f2801fdd6
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Thu Oct 30 15:37:47 2014 +0100

    TagSync: handle gid item references.

diff --git a/akonadi/tagsync.cpp b/akonadi/tagsync.cpp
index f534dba..5709dc1 100644
--- a/akonadi/tagsync.cpp
+++ b/akonadi/tagsync.cpp
@@ -20,8 +20,6 @@ namespace Akonadi {
     class Item;
 }
 
-unsigned int qHash(const Akonadi::Item &item);
-
 #include "tagsync.h"
 
 #include <akonadi/itemfetchjob.h>
@@ -33,16 +31,6 @@ unsigned int qHash(const Akonadi::Item &item);
 
 using namespace Akonadi;
 
-//We want to compare items by remoteId and not by id
-uint qHash(const Item &item)
-{
-    if (item.isValid()) {
-        return qHash(item.id());
-    }
-    Q_ASSERT(!item.remoteId().isEmpty());
-    return qHash(item.remoteId());
-}
-
 bool operator==(const Item &left, const Item &right)
 {
     if (left.isValid() && right.isValid() && (left.id() == right.id())) {
@@ -51,6 +39,9 @@ bool operator==(const Item &left, const Item &right)
     if (!left.remoteId().isEmpty() && !right.remoteId().isEmpty() && (left.remoteId() == right.remoteId())) {
         return true;
     }
+    if (!left.gid().isEmpty() && !right.gid().isEmpty() && (left.gid() == right.gid())) {
+        return true;
+    }
     return false;
 }
 
@@ -123,6 +114,7 @@ void TagSync::diffTags()
             ItemFetchJob *itemFetch = new ItemFetchJob(tag, this);
             itemFetch->setProperty("tag", QVariant::fromValue(tag));
             itemFetch->setProperty("merge", false);
+            itemFetch->fetchScope().setFetchGid(true);
             connect(itemFetch, SIGNAL(result(KJob*)), this, SLOT(onTagItemsFetchDone(KJob*)));
             connect(itemFetch, SIGNAL(result(KJob*)), this, SLOT(onJobDone(KJob*)));
             tagById.remove(tagByRid.value(remoteTag.remoteId()).id());
@@ -134,6 +126,7 @@ void TagSync::diffTags()
             ItemFetchJob *itemFetch = new ItemFetchJob(tag, this);
             itemFetch->setProperty("tag", QVariant::fromValue(tag));
             itemFetch->setProperty("merge", true);
+            itemFetch->fetchScope().setFetchGid(true);
             connect(itemFetch, SIGNAL(result(KJob*)), this, SLOT(onTagItemsFetchDone(KJob*)));
             connect(itemFetch, SIGNAL(result(KJob*)), this, SLOT(onJobDone(KJob*)));
             tagById.remove(tagByGid.value(remoteTag.gid()).id());
@@ -143,7 +136,6 @@ void TagSync::diffTags()
             createJob->setMergeIfExisting(true);
             connect(createJob, SIGNAL(result(KJob*)), this, SLOT(onCreateTagDone(KJob*)));
             connect(createJob, SIGNAL(result(KJob*)), this, SLOT(onJobDone(KJob*)));
-            //TODO add tags
         }
     }
     Q_FOREACH (const Akonadi::Tag::Id &removedTag, tagById.keys()) {
@@ -156,20 +148,10 @@ void TagSync::diffTags()
     checkDone();
 }
 
-static QSet<QString> ridSet(const Akonadi::Item::List &list)
-{
-    QSet<QString> set;
-    Q_FOREACH (const Akonadi::Item &item, list) {
-        set << item.remoteId();
-    }
-    return set;
-}
-
 void TagSync::onCreateTagDone(KJob *job)
 {
     if (job->error()) {
         kWarning() << "ItemFetch failed: " << job->errorString();
-        // cancelTask(job->errorString());
         return;
     }
 
@@ -183,21 +165,46 @@ void TagSync::onCreateTagDone(KJob *job)
     }
 }
 
+static bool containsByGidOrRid(const Item::List &items, const Item &key)
+{
+    Q_FOREACH(const Item &item, items) {
+        if ((!item.gid().isEmpty() && !key.gid().isEmpty()) && (item.gid() == key.gid())) {
+            return true;
+        } else if (item.remoteId() == key.remoteId()) {
+            return true;
+        }
+    }
+    return false;
+}
+
 void TagSync::onTagItemsFetchDone(KJob *job)
 {
     if (job->error()) {
         kWarning() << "ItemFetch failed: " << job->errorString();
-        // cancelTask(job->errorString());
         return;
     }
 
     const Akonadi::Item::List items = static_cast<Akonadi::ItemFetchJob*>(job)->items();
     const Akonadi::Tag tag = job->property("tag").value<Akonadi::Tag>();
     const bool merge = job->property("merge").toBool();
-    const QSet<Item> localMembers = items.toSet();
-    const QSet<Item> remoteMembers = mRidMemberMap.value(QString::fromLatin1(tag.remoteId())).toSet();
-    const QSet<Item> toAdd = remoteMembers - localMembers;
-    const QSet<Item> toRemove = localMembers - remoteMembers;
+    const Item::List remoteMembers = mRidMemberMap.value(QString::fromLatin1(tag.remoteId()));
+
+    //add = remote - local
+    Item::List toAdd;
+    Q_FOREACH(const Item &remote, remoteMembers) {
+        if (!containsByGidOrRid(items, remote)) {
+            toAdd << remote;
+        }
+    }
+
+    //remove = local - remote
+    Item::List toRemove;
+    Q_FOREACH(const Item &local, items) {
+        if (!containsByGidOrRid(remoteMembers, local)) {
+            toRemove << local;
+        }
+    }
+
     if (!merge) {
         Q_FOREACH (Item item, toRemove) {
             item.clearTag(tag);


commit 5a2b834d5b7466c3e466d935b427ba485a8a3beb
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Thu Oct 30 12:36:16 2014 +0100

    ItemModifyJob: Deal with empty tag sets.
    
    Required to clear tags.

diff --git a/akonadi/itemmodifyjob.cpp b/akonadi/itemmodifyjob.cpp
index 4d52cdf..5b6fc5d 100644
--- a/akonadi/itemmodifyjob.cpp
+++ b/akonadi/itemmodifyjob.cpp
@@ -119,7 +119,7 @@ void ItemModifyJobPrivate::setSilent( bool silent )
 QByteArray ItemModifyJobPrivate::tagsToCommandParameter(const Tag::List &tags) const
 {
     QByteArray c;
-    if (tags.first().id() >= 0) {
+    if (tags.isEmpty() || tags.first().id() >= 0) {
         c += "TAGS";
         c += ' ' + ProtocolHelper::tagSetToImapSequenceSet(tags);
     } else if (std::find_if(tags.constBegin(), tags.constEnd(),
diff --git a/akonadi/protocolhelper.cpp b/akonadi/protocolhelper.cpp
index 3cd4197..94dabe7 100644
--- a/akonadi/protocolhelper.cpp
+++ b/akonadi/protocolhelper.cpp
@@ -326,13 +326,10 @@ QByteArray ProtocolHelper::entitySetToByteArray( const QList<Item> &_objects, co
 
 QByteArray ProtocolHelper::tagSetToImapSequenceSet( const Akonadi::Tag::List &_objects )
 {
-  if ( _objects.isEmpty() )
-    throw Exception( "No objects specified" );
-
   Tag::List objects( _objects );
 
   std::sort( objects.begin(), objects.end(), boost::bind( &Tag::id, _1 ) < boost::bind( &Tag::id, _2 ) );
-  if ( !objects.first().isValid() ) {
+  if ( !objects.isEmpty() && !objects.first().isValid() ) {
     throw Exception( "Not all tags have a uid" );
   }
   // all items have a uid set


commit 7ecba87ad89a7c63a53e1bcee083d6a5391da11a
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Thu Oct 30 12:33:37 2014 +0100

    Revert "ItemModifyJob: support an empty set of tags."
    
    This reverts commit c9b49056dd276762d924545dcb2a85ef96eb19a1.

diff --git a/akonadi/itemmodifyjob.cpp b/akonadi/itemmodifyjob.cpp
index 075aade..4d52cdf 100644
--- a/akonadi/itemmodifyjob.cpp
+++ b/akonadi/itemmodifyjob.cpp
@@ -119,13 +119,9 @@ void ItemModifyJobPrivate::setSilent( bool silent )
 QByteArray ItemModifyJobPrivate::tagsToCommandParameter(const Tag::List &tags) const
 {
     QByteArray c;
-    if (tags.isEmpty() || (tags.first().id() >= 0)) {
+    if (tags.first().id() >= 0) {
         c += "TAGS";
-        if (tags.isEmpty()) {
-            c += " ()";
-        } else {
-            c += ' ' + ProtocolHelper::tagSetToImapSequenceSet(tags);
-        }
+        c += ' ' + ProtocolHelper::tagSetToImapSequenceSet(tags);
     } else if (std::find_if(tags.constBegin(), tags.constEnd(),
                         boost::bind(&QByteArray::isEmpty, boost::bind(&Tag::remoteId, _1)))
         == tags.constEnd()) {


commit cedd9a151b53bfb0628720966f9994acccaee678
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Wed Oct 29 21:14:43 2014 +0100

    Starting CollectionFetchJob should not fail with invalid collections
    
    If running CollectionFetchJob with a list of collections, where
    invalid collection ids. CollectionFetchJob should run all subjobs to
    return a list of valid collections.
    
    KOLAB: 3829
    
    Patch by Sandro Knauss

diff --git a/akonadi/collectionfetchjob.cpp b/akonadi/collectionfetchjob.cpp
index c2c1261..1cd9d80 100644
--- a/akonadi/collectionfetchjob.cpp
+++ b/akonadi/collectionfetchjob.cpp
@@ -103,6 +103,27 @@ public:
             return QString::fromLatin1("Collection RemoteId %1").arg(mBase.remoteId());
         }
     }
+
+    bool jobFailed(KJob *job)
+    {
+        Q_Q(CollectionFetchJob);
+        if (mScope.ignoreRetrievalErrors()) {
+           int error = job->error();
+           if (error && !q->error()) {
+               q->setError(error);
+               q->setErrorText(job->errorText());
+           }
+
+           if (error == Job::ConnectionFailed ||
+               error == Job::ProtocolVersionMismatch ||
+               error == Job::UserCanceled) {
+               return true;
+           }
+           return false;
+        } else {
+            return job->error();
+        }
+    }
 };
 
 CollectionFetchJob::CollectionFetchJob(const Collection &collection, Type type, QObject *parent)
@@ -411,10 +432,27 @@ void CollectionFetchJob::slotResult(KJob *job)
 
     CollectionFetchJob *list = qobject_cast<CollectionFetchJob *>(job);
     Q_ASSERT(job);
+
+    if (d->mType == NonOverlappingRoots) {
+        d->mPrefetchList += list->collections();
+    } else if (!d->mBasePrefetch) {
+        d->mCollections += list->collections();
+    }
+
+    if (d_ptr->mCurrentSubJob == job && !d->jobFailed(job)) {
+        if (job->error()) {
+            kWarning() << "Error during CollectionFetchJob: " << job->errorString();
+        }
+        d_ptr->mCurrentSubJob = 0;
+        removeSubjob(job);
+        QTimer::singleShot(0, this, SLOT(startNext()));
+    } else {
+        Job::slotResult(job);
+    }
+
     if (d->mBasePrefetch) {
         d->mBasePrefetch = false;
         const Collection::List roots = list->collections();
-        Job::slotResult(job);
         Q_ASSERT(!hasSubjobs());
         if (!job->error()) {
             foreach (const Collection &col, roots) {
@@ -425,18 +463,14 @@ void CollectionFetchJob::slotResult(KJob *job)
         }
         // No result yet.
     } else if (d->mType == NonOverlappingRoots) {
-        d->mPrefetchList += list->collections();
-        Job::slotResult(job);
-        if (!job->error() && !hasSubjobs()) {
+        if (!d->jobFailed(job) && !hasSubjobs()) {
             const Collection::List result = filterDescendants(d->mPrefetchList);
             d->mPendingCollections += result;
             d->mCollections = result;
             d->delayedEmitResult();
         }
     } else {
-        d->mCollections += list->collections();
-        Job::slotResult(job);
-        if (!job->error() && !hasSubjobs()) {
+        if (!d->jobFailed(job) && !hasSubjobs()) {
             d->delayedEmitResult();
         }
     }
diff --git a/akonadi/collectionfetchscope.cpp b/akonadi/collectionfetchscope.cpp
index 8265d3c..64f1756 100644
--- a/akonadi/collectionfetchscope.cpp
+++ b/akonadi/collectionfetchscope.cpp
@@ -34,6 +34,7 @@ public:
         , listFilter(CollectionFetchScope::Enabled)
         , fetchAllAttributes(false)
         , fetchIdOnly(true)
+        , mIgnoreRetrievalErrors(false)
     {
     }
 
@@ -54,6 +55,7 @@ public:
         }
         fetchAllAttributes = other.fetchAllAttributes;
         fetchIdOnly = other.fetchIdOnly;
+        mIgnoreRetrievalErrors = other.mIgnoreRetrievalErrors;
     }
 
 public:
@@ -66,6 +68,7 @@ public:
     QScopedPointer<CollectionFetchScope> ancestorFetchScope;
     bool fetchAllAttributes;
     bool fetchIdOnly;
+    bool mIgnoreRetrievalErrors;
 };
 
 CollectionFetchScope::CollectionFetchScope()
@@ -190,6 +193,16 @@ bool CollectionFetchScope::fetchIdOnly() const
     return d->fetchIdOnly;
 }
 
+void CollectionFetchScope::setIgnoreRetrievalErrors(bool enable)
+{
+    d->mIgnoreRetrievalErrors = enable;
+}
+
+bool CollectionFetchScope::ignoreRetrievalErrors() const
+{
+    return d->mIgnoreRetrievalErrors;
+}
+
 void CollectionFetchScope::setAncestorFetchScope(const CollectionFetchScope &scope)
 {
     *d->ancestorFetchScope = scope;
diff --git a/akonadi/collectionfetchscope.h b/akonadi/collectionfetchscope.h
index 7ab7781..8fb443f 100644
--- a/akonadi/collectionfetchscope.h
+++ b/akonadi/collectionfetchscope.h
@@ -294,6 +294,24 @@ public:
     bool fetchIdOnly() const;
 
     /**
+     * Ignore retrieval errors while fetching collections, and always deliver what is available.
+     *
+     * This flag is useful to fetch a list of collections, where some might no longer be available.
+     *
+     * @since KF5
+     */
+    void setIgnoreRetrievalErrors(bool enabled);
+
+    /**
+     * Returns whether retrieval errors should be ignored.
+     *
+     * @see setIgnoreRetrievalErrors()
+     * @since KF5
+     */
+    bool ignoreRetrievalErrors() const;
+
+
+    /**
      * Returns @c true if there is nothing to fetch.
      */
     bool isEmpty() const;
diff --git a/akonadi/tests/collectionjobtest.cpp b/akonadi/tests/collectionjobtest.cpp
index 7cb0ffb..c218b8d 100644
--- a/akonadi/tests/collectionjobtest.cpp
+++ b/akonadi/tests/collectionjobtest.cpp
@@ -603,6 +603,24 @@ void CollectionJobTest::testMultiList()
   compareLists( res, req );
 }
 
+void CollectionJobTest::testMultiListInvalid()
+{
+  Collection::List req;
+  req << Collection( res1ColId ) << Collection (1234567) << Collection( res2ColId );
+  CollectionFetchJob* job = new CollectionFetchJob( req, this );
+  QVERIFY( !job->exec() );
+  // not all available collections are fetched
+  QVERIFY( job->collections().count() != 2 );
+
+  job = new CollectionFetchJob( req, this );
+  job->fetchScope().setIgnoreRetrievalErrors(true);
+  QVERIFY( !job->exec() );
+  Collection::List res;
+  res = job->collections();
+  req = Collection::List() << Collection( res1ColId ) << Collection( res2ColId );
+  compareLists( res, req );
+}
+
 void CollectionJobTest::testRecursiveMultiList()
 {
   Akonadi::Collection::List toFetch;
diff --git a/akonadi/tests/collectionjobtest.h b/akonadi/tests/collectionjobtest.h
index cd71d3c..56b4205 100644
--- a/akonadi/tests/collectionjobtest.h
+++ b/akonadi/tests/collectionjobtest.h
@@ -45,6 +45,7 @@ class CollectionJobTest : public QObject
     void testUtf8CollectionName_data();
     void testUtf8CollectionName();
     void testMultiList();
+    void testMultiListInvalid();
     void testRecursiveMultiList();
     void testNonOverlappingRootList();
     void testSelect();


commit 5c356776af99525252a71a803b42553ab276b700
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Wed Oct 29 16:31:29 2014 +0100

    Manual revert of 7859c10a7511b96a67e7466c69a3c6a3ad575988.
    
    Resource don't fetch the GID by default, resulting in GID always getting cleared
    when the resource replayed the change and sets the RID. The change was
    introduced for the google resource.

diff --git a/akonadi/resourcebase.cpp b/akonadi/resourcebase.cpp
index 5b10981..042275e 100644
--- a/akonadi/resourcebase.cpp
+++ b/akonadi/resourcebase.cpp
@@ -716,7 +716,6 @@ void ResourceBase::changesCommitted(const Item::List &items)
         job->d_func()->setClean();
         job->disableRevisionCheck(); // TODO: remove, but where/how do we handle the error?
         job->setIgnorePayload(true);   // we only want to reset the dirty flag and update the remote id
-        job->setUpdateGid(true);   // allow resources to update GID too
     }
 }
 




More information about the commits mailing list