summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Vrátil <dvratil@kde.org>2016-10-22 22:48:42 (GMT)
committerDaniel Vrátil <dvratil@kde.org>2016-10-22 22:48:42 (GMT)
commit78ea1208a3f008aca0cf52bc6fefb722547edae2 (patch)
treeacbfb0ca274e4b0e8e26c09471e34a1684ee02c8
parent7b5421adbd97e6e3341534e46a649216fbe5a426 (diff)
Use the new KIMAP::MoveJob to move emails when server supports it
We fall back to COPY + STORE \Deleted + EXPUNGE if the server does not support MOVE. In both cases we fall back to SEARCH to get new UIDs when the server does not support UIDPLUS.
-rw-r--r--CMakeLists.txt2
-rw-r--r--resources/imap/autotests/testmoveitemstask.cpp194
-rw-r--r--resources/imap/moveitemstask.cpp61
-rw-r--r--resources/imap/moveitemstask.h3
4 files changed, 244 insertions, 16 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt
index bc91431..1ab917d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -64,7 +64,7 @@ set(KCALENDARCORE_LIB_VERSION "5.3.40")
set(IDENTITYMANAGEMENT_LIB_VERSION "5.3.40")
set(KMAILTRANSPORT_LIB_VERSION "5.3.40")
set(CALENDARUTILS_LIB_VERSION "5.3.40")
-set(KIMAP_LIB_VERSION "5.3.40")
+set(KIMAP_LIB_VERSION "5.3.41")
set(KMBOX_LIB_VERSION "5.3.40")
set(AKONADICALENDAR_LIB_VERSION "5.3.40")
set(KONTACTINTERFACE_LIB_VERSION "5.3.40")
diff --git a/resources/imap/autotests/testmoveitemstask.cpp b/resources/imap/autotests/testmoveitemstask.cpp
index 8fedaa7..5d414fd 100644
--- a/resources/imap/autotests/testmoveitemstask.cpp
+++ b/resources/imap/autotests/testmoveitemstask.cpp
@@ -247,6 +247,200 @@ private Q_SLOTS:
server.quit();
}
+
+ void shouldMoveMessage_data()
+ {
+ QTest::addColumn<Akonadi::Item>("item");
+ QTest::addColumn<Akonadi::Collection>("source");
+ QTest::addColumn<Akonadi::Collection>("target");
+ QTest::addColumn< QList<QByteArray> >("scenario");
+ QTest::addColumn<QStringList>("callNames");
+
+ Akonadi::Item item;
+ Akonadi::Collection inbox;
+ Akonadi::Collection source;
+ Akonadi::Collection target;
+ QList<QByteArray> scenario;
+ QStringList callNames;
+
+ item = Akonadi::Item(1);
+ item.setRemoteId(QStringLiteral("5"));
+
+ KMime::Message::Ptr message(new KMime::Message);
+
+ QString messageContent = QStringLiteral("From: ervin\nTo: someone\nSubject: foo\n\nSpeechless...");
+
+ message->setContent(messageContent.toUtf8());
+ message->parse();
+ item.setPayload(message);
+
+ inbox = createCollectionChain(QStringLiteral("/INBOX"));
+ source = Akonadi::Collection(3);
+ source.setRemoteId(QStringLiteral("/Foo"));
+ source.setParentCollection(inbox);
+ target = Akonadi::Collection(4);
+ target.setRemoteId(QStringLiteral("/Bar"));
+ target.setParentCollection(inbox);
+
+ scenario.clear();
+ scenario << defaultPoolConnectionScenario({ "MOVE" })
+ << "C: A000003 SELECT \"INBOX/Foo\""
+ << "S: A000003 OK select done"
+ << "C: A000004 UID MOVE 5 \"INBOX/Bar\""
+ << "S: A000004 OK move [ COPYUID 1239890035 5 65 ]";
+
+ callNames.clear();
+ callNames << QStringLiteral("itemsChangesCommitted");
+
+ QTest::newRow("moving mail") << item << source << target << scenario << callNames;
+
+
+ item = Akonadi::Item(1);
+ item.setRemoteId(QStringLiteral("5"));
+
+ message = KMime::Message::Ptr(new KMime::Message);
+
+ messageContent = QStringLiteral("From: ervin\nTo: someone\nSubject: foo\nMessage-ID: <42.4242.foo@bar.org>\n\nSpeechless...");
+
+ message->setContent(messageContent.toUtf8());
+ message->parse();
+ item.setPayload(message);
+
+ source = Akonadi::Collection(3);
+ source.setRemoteId(QStringLiteral("/Foo"));
+ source.setParentCollection(inbox);
+ source.addAttribute(new UidNextAttribute(42));
+ target = Akonadi::Collection(3);
+ target.setRemoteId(QStringLiteral("/Bar"));
+ target.setParentCollection(inbox);
+ target.addAttribute(new UidNextAttribute(65));
+
+ scenario.clear();
+ scenario << defaultPoolConnectionScenario({ "MOVE" })
+ << "C: A000003 SELECT \"INBOX/Foo\""
+ << "S: A000003 OK select done"
+ << "C: A000004 UID MOVE 5 \"INBOX/Bar\""
+ << "S: A000004 OK MOVE done"
+ << "C: A000005 SELECT \"INBOX/Bar\""
+ << "S: A000005 OK select done"
+ << "C: A000006 UID SEARCH (HEADER Message-ID <42.4242.foo@bar.org>)"
+ << "S: * SEARCH 65"
+ << "S: A000006 OK search done";
+
+ callNames.clear();
+ callNames << QStringLiteral("itemsChangesCommitted") << QStringLiteral("applyCollectionChanges");
+
+ QTest::newRow("moving mail, no COPYUID, message had Message-ID") << item << source << target << scenario << callNames;
+
+ item = Akonadi::Item(1);
+ item.setRemoteId(QStringLiteral("5"));
+
+ message = KMime::Message::Ptr(new KMime::Message);
+
+ messageContent = QStringLiteral("From: ervin\nTo: someone\nSubject: foo\n\nSpeechless...");
+
+ message->setContent(messageContent.toUtf8());
+ message->parse();
+ item.setPayload(message);
+
+ source = Akonadi::Collection(3);
+ source.setRemoteId(QStringLiteral("/Foo"));
+ source.setParentCollection(inbox);
+ source.addAttribute(new UidNextAttribute(42));
+ target = Akonadi::Collection(4);
+ target.setRemoteId(QStringLiteral("/Bar"));
+ target.setParentCollection(inbox);
+ target.addAttribute(new UidNextAttribute(65));
+
+ scenario.clear();
+ scenario << defaultPoolConnectionScenario({ "MOVE" })
+ << "C: A000003 SELECT \"INBOX/Foo\""
+ << "S: A000003 OK select done"
+ << "C: A000004 UID MOVE 5 \"INBOX/Bar\""
+ << "S: A000004 OK MOVE done"
+ << "C: A000005 SELECT \"INBOX/Bar\""
+ << "S: A000005 OK select done"
+ << "C: A000006 UID SEARCH NEW UID 65:*"
+ << "S: * SEARCH 65"
+ << "S: A000006 OK search done";
+
+ callNames.clear();
+ callNames << QStringLiteral("itemsChangesCommitted") << QStringLiteral("applyCollectionChanges");
+
+ QTest::newRow("moving mail, no COPYUID, message didn't have Message-ID") << item << source << target << scenario << callNames;
+
+ item = Akonadi::Item(1);
+ item.setRemoteId(QStringLiteral("5"));
+ message = KMime::Message::Ptr(new KMime::Message);
+ messageContent = QStringLiteral("From: ervin\nTo: someone\nSubject: foo\nMessage-ID: <42.4242.foo@bar.org>\n\nSpeechless...");
+ message->setContent(messageContent.toUtf8());
+ message->parse();
+ item.setPayload(message);
+
+ scenario.clear();
+ scenario << defaultPoolConnectionScenario({ "MOVE" })
+ << "C: A000003 SELECT \"INBOX/Foo\""
+ << "S: A000003 OK select done"
+ << "C: A000004 UID MOVE 5 \"INBOX/Bar\""
+ << "S: A000004 OK MOVE done"
+ << "C: A000005 SELECT \"INBOX/Bar\""
+ << "S: A000005 OK select done"
+ << "C: A000006 UID SEARCH (HEADER Message-ID <42.4242.foo@bar.org>)"
+ << "S: * SEARCH 61 65"
+ << "S: A000006 OK search done";
+
+ callNames.clear();
+ callNames << QStringLiteral("itemsChangesCommitted") << QStringLiteral("applyCollectionChanges");
+
+ QTest::newRow("moving mail, no COPYUID, message didn't have unique Message-ID, but last one matches old uidnext") << item << source << target << scenario << callNames;
+ }
+
+ void shouldMoveMessage()
+ {
+ QFETCH(Akonadi::Item, item);
+ QFETCH(Akonadi::Collection, source);
+ QFETCH(Akonadi::Collection, target);
+ QFETCH(QList<QByteArray>, scenario);
+ QFETCH(QStringList, callNames);
+
+ FakeServer server;
+ server.setScenario(scenario);
+ server.startAndWait();
+
+ SessionPool pool(1);
+
+ pool.setPasswordRequester(createDefaultRequester());
+ QVERIFY(pool.connect(createDefaultAccount()));
+ QVERIFY(waitForSignal(&pool, SIGNAL(connectDone(int,QString))));
+
+ DummyResourceState::Ptr state = DummyResourceState::Ptr(new DummyResourceState);
+ state->setItem(item);
+ state->setSourceCollection(source);
+ state->setTargetCollection(target);
+ state->setServerCapabilities({ QStringLiteral("MOVE") });
+ MoveItemsTask *task = new MoveItemsTask(state);
+ task->start(&pool);
+
+ QTRY_COMPARE(state->calls().count(), callNames.size());
+ for (int i = 0; i < callNames.size(); i++) {
+ QString command = QString::fromUtf8(state->calls().at(i).first);
+ QVariant parameter = state->calls().at(i).second;
+
+ if (command == QLatin1String("cancelTask") && callNames[i] != QLatin1String("cancelTask")) {
+ qDebug() << "Got a cancel:" << parameter.toString();
+ }
+
+ QCOMPARE(command, callNames[i]);
+
+ if (command == QLatin1String("cancelTask")) {
+ QVERIFY(!parameter.toString().isEmpty());
+ }
+ }
+
+ QVERIFY(server.isAllScenarioDone());
+
+ server.quit();
+ }
};
QTEST_GUILESS_MAIN(TestMoveItemsTask)
diff --git a/resources/imap/moveitemstask.cpp b/resources/imap/moveitemstask.cpp
index a9d73cc..405adf6 100644
--- a/resources/imap/moveitemstask.cpp
+++ b/resources/imap/moveitemstask.cpp
@@ -31,6 +31,7 @@
#include <kimap/selectjob.h>
#include <kimap/session.h>
#include <kimap/storejob.h>
+#include <kimap/movejob.h>
#include <kmime/kmime_message.h>
@@ -89,7 +90,7 @@ void MoveItemsTask::doStart(KIMAP::Session *session)
select->start();
} else {
- triggerCopyJob(session);
+ startMove(session);
}
}
@@ -101,11 +102,11 @@ void MoveItemsTask::onSelectDone(KJob *job)
} else {
KIMAP::SelectJob *select = static_cast<KIMAP::SelectJob *>(job);
- triggerCopyJob(select->session());
+ startMove(select->session());
}
}
-void MoveItemsTask::triggerCopyJob(KIMAP::Session *session)
+void MoveItemsTask::startMove(KIMAP::Session *session)
{
const QString newMailBox = mailBoxForCollection(targetCollection());
@@ -123,22 +124,28 @@ void MoveItemsTask::triggerCopyJob(KIMAP::Session *session)
set.add(item.remoteId().toLong());
} catch (const Akonadi::PayloadException &e) {
- qCWarning(IMAPRESOURCE_LOG) << "Copy failed, payload exception " << item.id() << item.remoteId();
- cancelTask(i18n("Failed to copy item, it has no message payload. Remote id: %1", item.remoteId()));
+ qCWarning(IMAPRESOURCE_LOG) << "Move failed, payload exception " << item.id() << item.remoteId();
+ cancelTask(i18n("Failed to move item, it has no message payload. Remote id: %1", item.remoteId()));
return;
}
}
- KIMAP::CopyJob *copy = new KIMAP::CopyJob(session);
-
- copy->setUidBased(true);
- copy->setSequenceSet(set);
- copy->setMailBox(newMailBox);
-
- connect(copy, &KIMAP::CopyJob::result, this, &MoveItemsTask::onCopyDone);
-
- copy->start();
-
+ const bool canMove = serverCapabilities().contains(QStringLiteral("MOVE"), Qt::CaseInsensitive);
+ if (canMove) {
+ auto job = new KIMAP::MoveJob(session);
+ job->setUidBased(true);
+ job->setSequenceSet(set);
+ job->setMailBox(newMailBox);
+ connect(job, &KIMAP::Job::result, this, &MoveItemsTask::onMoveDone);
+ job->start();
+ } else {
+ auto job = new KIMAP::CopyJob(session);
+ job->setUidBased(true);
+ job->setSequenceSet(set);
+ job->setMailBox(newMailBox);
+ connect(job, &KIMAP::Job ::result, this, &MoveItemsTask::onCopyDone);
+ job->start();
+ }
m_oldSet = set;
}
@@ -167,6 +174,30 @@ void MoveItemsTask::onCopyDone(KJob *job)
}
}
+void MoveItemsTask::onMoveDone(KJob *job)
+{
+ if (job->error()) {
+ qCWarning(IMAPRESOURCE_LOG) << job->errorString();
+ cancelTask(job->errorString());
+ } else {
+ KIMAP::MoveJob *move = static_cast<KIMAP::MoveJob *>(job);
+
+ m_newUids = imapSetToList(move->resultingUids());
+
+ if (!m_newUids.isEmpty()) {
+ recordNewUid();
+ } else {
+ // Let's go for a search to find a new UID :-)
+
+ // We did a move, so we're very likely not in the right mailbox
+ KIMAP::SelectJob *select = new KIMAP::SelectJob(move->session());
+ select->setMailBox(mailBoxForCollection(targetCollection()));
+ connect(select, &KIMAP::SelectJob::result, this, &MoveItemsTask::onPreSearchSelectDone);
+ select->start();
+ }
+ }
+}
+
void MoveItemsTask::onStoreFlagsDone(KJob *job)
{
if (job->error()) {
diff --git a/resources/imap/moveitemstask.h b/resources/imap/moveitemstask.h
index 5368040..0605362 100644
--- a/resources/imap/moveitemstask.h
+++ b/resources/imap/moveitemstask.h
@@ -38,6 +38,7 @@ private Q_SLOTS:
void onSelectDone(KJob *job);
void onCopyDone(KJob *job);
void onStoreFlagsDone(KJob *job);
+ void onMoveDone(KJob *job);
void onPreSearchSelectDone(KJob *job);
void onSearchDone(KJob *job);
@@ -46,6 +47,8 @@ protected:
void doStart(KIMAP::Session *session) Q_DECL_OVERRIDE;
private:
+ void startMove(KIMAP::Session *session);
+ void triggerMoveJob(KIMAP::Session *session);
void triggerCopyJob(KIMAP::Session *session);
void recordNewUid();
QVector<qint64> imapSetToList(const KIMAP::ImapSet &set);