Branch 'kolab/integration/4.13.0' - 6 commits - resources/imap
Christian Mollekopf
mollekopf at kolabsys.com
Mon Apr 28 02:16:29 CEST 2014
resources/imap/resourcestate.cpp | 13 ++++++
resources/imap/resourcestate.h | 2 +
resources/imap/resourcestateinterface.h | 1
resources/imap/resourcetask.cpp | 6 +++
resources/imap/resourcetask.h | 1
resources/imap/retrieveitemstask.cpp | 17 ++------
resources/imap/retrieveitemstask.h | 2 -
resources/imap/tests/dummyresourcestate.cpp | 5 ++
resources/imap/tests/dummyresourcestate.h | 1
resources/imap/tests/testretrieveitemstask.cpp | 49 ++++++++++++++++++++++++-
10 files changed, 83 insertions(+), 14 deletions(-)
New commits:
commit d06a50a5cf5fc76991b333a605c84e00528374fd
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Fri Apr 25 18:08:17 2014 +0200
IMAP-Resource: simplified the code slightly.
commit 51d0dadf4100dbaf53f5b3c2353e1b2394e55d98
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Fri Apr 25 10:16:53 2014 +0200
IMAP-Resource: Trigger a new sync to fix the cache if broken.
The recovery codepath relies on up-to-date attributes of the collection,
but if a task is deferred the collection is not refetched.
We work around this now by providing a function to cancel the task and schedule a new sync.
commit 226f8313c9df2d93ca0dc5f20e41c3e90c1912cd
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Fri Apr 25 09:58:43 2014 +0200
IMAP-Resource: Also refetch the mailbox if uidvalidity was not set
If we don't have the attribute this means uidvalidity could have changed,
so we must refetch. This also makes the recovery path from an inconsitent
cache work (almost).
Tests were adjusted accordingly.
commit 0af363bcc9b07f3e9f6eb741bdab5c17256c0dbb
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Fri Apr 25 18:14:38 2014 +0200
IMAP-Resource: Test uidvalidity and adjusted to requirement of the attribute.
diff --git a/resources/imap/tests/testretrieveitemstask.cpp b/resources/imap/tests/testretrieveitemstask.cpp
index 31064f3..4b8e93b 100644
--- a/resources/imap/tests/testretrieveitemstask.cpp
+++ b/resources/imap/tests/testretrieveitemstask.cpp
@@ -22,6 +22,7 @@
#include "retrieveitemstask.h"
#include "uidnextattribute.h"
#include <highestmodseqattribute.h>
+#include <uidvalidityattribute.h>
#include <akonadi/cachepolicy.h>
#include <akonadi/collectionstatistics.h>
@@ -43,6 +44,7 @@ private slots:
QStringList callNames;
collection = createCollectionChain( QLatin1String("/INBOX/Foo") );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(1149151135);
scenario.clear();
scenario << defaultPoolConnectionScenario()
@@ -82,6 +84,7 @@ private slots:
<< Akonadi::MessagePart::Body );
collection = createCollectionChain( QLatin1String("/INBOX/Foo") );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(1149151135);
collection.setCachePolicy( policy );
scenario.clear();
@@ -121,6 +124,7 @@ private slots:
stats.setCount( 1 );
collection = createCollectionChain( QLatin1String("/INBOX/Foo") );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(1149151135);
collection.setCachePolicy( policy );
collection.setStatistics( stats );
@@ -150,6 +154,7 @@ private slots:
collection = createCollectionChain( QLatin1String("/INBOX/Foo") );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(1149151135);
collection.setCachePolicy( policy );
collection.setStatistics( stats );
scenario.clear();
@@ -220,6 +225,7 @@ private slots:
QTest::newRow( "uidnext mismatch with recovery attempt" ) << collection << scenario << callNames;
collection = createCollectionChain(QLatin1String("/INBOX/Foo") );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(1149151135);
collection.setCachePolicy( policy );
collection.attribute<UidNextAttribute>( Akonadi::Collection::AddIfMissing )->setUidNext( 2471 );
collection.attribute<HighestModSeqAttribute>( Akonadi::Entity::AddIfMissing )->setHighestModSeq( 123456788 );
@@ -250,6 +256,7 @@ private slots:
collection = createCollectionChain(QLatin1String("/INBOX/Foo") );
collection.setCachePolicy( policy );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(1149151135);
collection.attribute<UidNextAttribute>( Akonadi::Collection::AddIfMissing )->setUidNext( 2471 );
collection.attribute<HighestModSeqAttribute>( Akonadi::Entity::AddIfMissing )->setHighestModSeq( 123456788 );
stats.setCount( 5 );
@@ -279,6 +286,7 @@ private slots:
collection = createCollectionChain(QLatin1String("/INBOX/Foo") );
collection.setCachePolicy( policy );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(1149151135);
collection.attribute<UidNextAttribute>( Akonadi::Collection::AddIfMissing )->setUidNext( 2471 );
collection.attribute<HighestModSeqAttribute>( Akonadi::Entity::AddIfMissing )->setHighestModSeq( 123456788 );
stats.setCount( 3 );
@@ -323,6 +331,45 @@ private slots:
// We miss one item in the local collection, and therefore fetch the wrong items based
// the item count of the collection.
QTest::newRow( "Inconsistent local cache" ) << collection << scenario << callNames;
+
+
+ collection = createCollectionChain( QLatin1String("/INBOX/Foo") );
+ collection.attribute<UidValidityAttribute>(Akonadi::Entity::AddIfMissing)->setUidValidity(3);
+ collection.setCachePolicy( policy );
+ stats.setCount( 1 );
+ collection.setStatistics( stats );
+
+ scenario.clear();
+ scenario << defaultPoolConnectionScenario()
+ << "C: A000003 SELECT \"INBOX/Foo\""
+ << "S: A000003 OK select done"
+ << "C: A000004 EXPUNGE"
+ << "S: A000004 OK expunge done"
+ << "C: A000005 SELECT \"INBOX/Foo\""
+ << "S: * FLAGS (\\Answered \\Flagged \\Draft \\Deleted \\Seen)"
+ << "S: * OK [ PERMANENTFLAGS (\\Answered \\Flagged \\Draft \\Deleted \\Seen) ]"
+ << "S: * 1 EXISTS"
+ << "S: * 0 RECENT"
+ << "S: * OK [ UIDVALIDITY 1149151135 ]"
+ << "S: * OK [ UIDNEXT 2471 ]"
+ << "S: A000005 OK select done"
+ << "C: A000006 FETCH 1 (RFC822.SIZE INTERNALDATE BODY.PEEK[] FLAGS UID)"
+ << "S: * 1 FETCH ( FLAGS (\\Seen) UID 2321 )"
+ << "S: * 1 FETCH ( FLAGS (\\Seen) UID 2321 INTERNALDATE \"29-Jun-2010 15:26:42 +0200\" "
+ "RFC822.SIZE 75 BODY[] {75}\r\n"
+ "From: Foo <foo at kde.org>\r\n"
+ "To: Bar <bar at kde.org>\r\n"
+ "Subject: Test Mail\r\n"
+ "\r\n"
+ "Test\r\n"
+ " )"
+ << "S: A000006 OK fetch done";
+
+ callNames.clear();
+ callNames << "itemsRetrieved" << "applyCollectionChanges" << "itemsRetrievalDone";
+
+ QTest::newRow( "uidvalidity" ) << collection << scenario << callNames;
+
}
void shouldIntrospectCollection()
commit 47b878cf29ff4b5980005d42156aaec03ba54752
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Fri Apr 25 18:08:17 2014 +0200
IMAP-Resource: simplified the code slightly.
diff --git a/resources/imap/retrieveitemstask.cpp b/resources/imap/retrieveitemstask.cpp
index ef3d126..22ab2af 100644
--- a/resources/imap/retrieveitemstask.cpp
+++ b/resources/imap/retrieveitemstask.cpp
@@ -541,16 +541,7 @@ void RetrieveItemsTask::onFinalSelectDone( KJob *job )
setTotalItems(m_messageUidsMissingBody.size());
KIMAP::ImapSet imapSet;
imapSet.add( m_messageUidsMissingBody );
-
- m_batchFetcher = new BatchFetcher(imapSet, scope, batchSize(), m_session);
- m_batchFetcher->setUidBased(true);
- m_incremental = true;
- m_batchFetcher->setProperty("alreadyFetched", 1);
- connect(m_batchFetcher, SIGNAL(itemsRetrieved(Akonadi::Item::List)),
- this, SLOT(onItemsRetrieved(Akonadi::Item::List)));
- connect(m_batchFetcher, SIGNAL(result(KJob*)),
- this, SLOT(onRetrievalDone(KJob*)));
- m_batchFetcher->start();
+ retrieveItems(imapSet, scope, true, true);
} else if ( messageCount > 0 ) {
if ( messageCount < realMessageCount ) {
// Some messages were removed, list all flags to find out which messages
@@ -575,13 +566,14 @@ void RetrieveItemsTask::onFinalSelectDone( KJob *job )
}
}
-void RetrieveItemsTask::retrieveItems(const KIMAP::ImapSet& set, const KIMAP::FetchJob::FetchScope &scope, bool incremental)
+void RetrieveItemsTask::retrieveItems(const KIMAP::ImapSet& set, const KIMAP::FetchJob::FetchScope &scope, bool incremental, bool uidBased)
{
Q_ASSERT(set.intervals().size() == 1);
m_incremental = incremental;
m_batchFetcher = new BatchFetcher(set, scope, batchSize(), m_session);
+ m_batchFetcher->setUidBased(uidBased);
m_batchFetcher->setProperty("alreadyFetched", set.intervals().first().begin());
connect(m_batchFetcher, SIGNAL(itemsRetrieved(Akonadi::Item::List)),
this, SLOT(onItemsRetrieved(Akonadi::Item::List)));
diff --git a/resources/imap/retrieveitemstask.h b/resources/imap/retrieveitemstask.h
index a3f1ef8..41787bf 100644
--- a/resources/imap/retrieveitemstask.h
+++ b/resources/imap/retrieveitemstask.h
@@ -68,7 +68,7 @@ private:
void triggerPreExpungeSelect( const QString &mailBox );
void triggerExpunge( const QString &mailBox );
void triggerFinalSelect( const QString &mailBox );
- void retrieveItems(const KIMAP::ImapSet& set, const KIMAP::FetchJob::FetchScope &scope, bool incremental = false);
+ void retrieveItems(const KIMAP::ImapSet& set, const KIMAP::FetchJob::FetchScope &scope, bool incremental = false, bool uidBased = false);
void listFlagsForImapSet( const KIMAP::ImapSet& set );
void taskComplete();
commit dc4b79d19cab8ecd61350c7bef009f57cb6d2a3a
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Fri Apr 25 18:07:38 2014 +0200
IMAP-Resource: Fixed recovery from invalid index state.
The recovery codepath relie on up-to-date annotaitons on the collection,
but if a task is deferred the collection is not refetched.
We work around this now by providing a function to cancel a task and schedule a new sync.
diff --git a/resources/imap/resourcestate.cpp b/resources/imap/resourcestate.cpp
index 2575cd0..12b7907 100644
--- a/resources/imap/resourcestate.cpp
+++ b/resources/imap/resourcestate.cpp
@@ -303,6 +303,15 @@ void ResourceState::deferTask()
m_resource->deferTask();
}
+void ResourceState::restartItemRetrieval(Akonadi::Collection::Id col)
+{
+ //This ensures the collection fetch job is rerun (it isn't when using deferTask)
+ //The task will be appended
+ //TODO: deferTask should rerun the collectionfetchjob
+ m_resource->synchronizeCollection(col);
+ cancelTask("Restarting item retrieval.");
+}
+
void ResourceState::taskDone()
{
m_resource->taskDone();
@@ -323,6 +332,10 @@ void ResourceState::emitPercent( int percent )
emit m_resource->percent( percent );
}
+void ResourceState::synchronizeCollection(Akonadi::Entity::Id id)
+{
+ m_resource->synchronizeCollection(id);
+}
void ResourceState::synchronizeCollectionTree()
{
diff --git a/resources/imap/resourcestate.h b/resources/imap/resourcestate.h
index 65fc9f8..04bc0c6 100644
--- a/resources/imap/resourcestate.h
+++ b/resources/imap/resourcestate.h
@@ -107,6 +107,7 @@ public:
virtual void cancelTask( const QString &errorString );
virtual void deferTask();
+ virtual void restartItemRetrieval(Akonadi::Collection::Id col);
virtual void taskDone();
virtual void emitError( const QString &message );
@@ -114,6 +115,7 @@ public:
virtual void emitPercent( int percent );
+ virtual void synchronizeCollection(Akonadi::Collection::Id);
virtual void synchronizeCollectionTree();
virtual void scheduleConnectionAttempt();
diff --git a/resources/imap/resourcestateinterface.h b/resources/imap/resourcestateinterface.h
index 6cdb4d6..145ba10 100644
--- a/resources/imap/resourcestateinterface.h
+++ b/resources/imap/resourcestateinterface.h
@@ -90,6 +90,7 @@ public:
virtual void cancelTask( const QString &errorString ) = 0;
virtual void deferTask() = 0;
+ virtual void restartItemRetrieval(Akonadi::Collection::Id col) = 0;
virtual void taskDone() = 0;
virtual void emitError( const QString &message ) = 0;
diff --git a/resources/imap/resourcetask.cpp b/resources/imap/resourcetask.cpp
index b4a4ee1..eca857b 100644
--- a/resources/imap/resourcetask.cpp
+++ b/resources/imap/resourcetask.cpp
@@ -328,6 +328,12 @@ void ResourceTask::deferTask()
deleteLater();
}
+void ResourceTask::restartItemRetrieval(Akonadi::Entity::Id col)
+{
+ m_resource->restartItemRetrieval(col);
+ deleteLater();
+}
+
void ResourceTask::taskDone()
{
m_resource->taskDone();
diff --git a/resources/imap/resourcetask.h b/resources/imap/resourcetask.h
index d143757..2c2589a 100644
--- a/resources/imap/resourcetask.h
+++ b/resources/imap/resourcetask.h
@@ -120,6 +120,7 @@ protected:
void cancelTask( const QString &errorString );
void deferTask();
+ void restartItemRetrieval(Akonadi::Collection::Id col);
void taskDone();
void emitPercent( int percent );
void emitError( const QString &message );
diff --git a/resources/imap/retrieveitemstask.cpp b/resources/imap/retrieveitemstask.cpp
index 199470e..ef3d126 100644
--- a/resources/imap/retrieveitemstask.cpp
+++ b/resources/imap/retrieveitemstask.cpp
@@ -633,7 +633,8 @@ void RetrieveItemsTask::onRetrievalDone( KJob *job )
col.removeAttribute("uidvalidity");
applyCollectionChanges(col);
kWarning() << "Inconsistent cache, retrying sync." << batchFetcher->lowestUidFetched() << m_lowestExpectedUid;
- deferTask();
+ //We cannot use deferTask because that doesn't refetch the collection and we rely on up-to date annotations
+ restartItemRetrieval(col.id());
return;
}
diff --git a/resources/imap/tests/dummyresourcestate.cpp b/resources/imap/tests/dummyresourcestate.cpp
index 1ee3e6c..e6abe25 100644
--- a/resources/imap/tests/dummyresourcestate.cpp
+++ b/resources/imap/tests/dummyresourcestate.cpp
@@ -280,6 +280,11 @@ void DummyResourceState::deferTask()
recordCall( "deferTask" );
}
+void DummyResourceState::restartItemRetrieval(Akonadi::Entity::Id col)
+{
+ recordCall( "restartItemRetrieval", QVariant::fromValue(col) );
+}
+
void DummyResourceState::taskDone()
{
recordCall( "taskDone" );
diff --git a/resources/imap/tests/dummyresourcestate.h b/resources/imap/tests/dummyresourcestate.h
index 56a02ea..ff47b32 100644
--- a/resources/imap/tests/dummyresourcestate.h
+++ b/resources/imap/tests/dummyresourcestate.h
@@ -106,6 +106,7 @@ public:
virtual void cancelTask( const QString &errorString );
virtual void deferTask();
+ virtual void restartItemRetrieval(Akonadi::Collection::Id col);
virtual void taskDone();
virtual void emitError( const QString &message );
diff --git a/resources/imap/tests/testretrieveitemstask.cpp b/resources/imap/tests/testretrieveitemstask.cpp
index cefbf44..31064f3 100644
--- a/resources/imap/tests/testretrieveitemstask.cpp
+++ b/resources/imap/tests/testretrieveitemstask.cpp
@@ -318,7 +318,7 @@ private slots:
<< "S: A000006 OK fetch done";
callNames.clear();
- callNames << "itemsRetrievedIncremental" << "applyCollectionChanges" << "cancelTask";
+ callNames << "itemsRetrievedIncremental" << "applyCollectionChanges" << "restartItemRetrieval";
// We miss one item in the local collection, and therefore fetch the wrong items based
// the item count of the collection.
More information about the commits
mailing list