Refactor SessionPool

Split preparation of initial session into its own subjob. Enable
QRESYNC if it's available.

CMakeLists.txt

@@ -76,7 +76,7 @@ set(AKONADI_VERSION "5.15.40")
 set(IDENTITYMANAGEMENT_LIB_VERSION "5.15.40")
 set(KMAILTRANSPORT_LIB_VERSION "5.15.40")
 set(CALENDARUTILS_LIB_VERSION "5.15.40")
-set(KIMAP_LIB_VERSION "5.15.40")
+set(KIMAP_LIB_VERSION "5.15.41")
 set(KMBOX_LIB_VERSION "5.15.40")
 set(AKONADICALENDAR_LIB_VERSION "5.15.40")
 set(KONTACTINTERFACE_LIB_VERSION "5.15.40")

resources/imap/CMakeLists.txt

@@ -24,6 +24,7 @@ set( imapresource_LIB_SRCS
   noselectattribute.cpp
   noinferiorsattribute.cpp
   passwordrequesterinterface.cpp
+  preparesessionjob.cpp
   removecollectionrecursivetask.cpp
   resourcestateinterface.cpp
   resourcetask.cpp

resources/imap/preparesessionjob.cpp

+/*
+    SPDX-FileCopyrightText: 2020 Daniel Vrátil <dvratil@kde.org>
+
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#include "preparesessionjob.h"
+
+#include <KIMAP/CapabilitiesJob>
+#include <KIMAP/IdJob>
+#include <KIMAP/NamespaceJob>
+#include <KIMAP/EnableJob>
+
+PrepareSessionJob::PrepareSessionJob(KIMAP::Session *session, const QByteArray &clientId)
+    : m_session(session)
+    , m_clientId(clientId)
+{}
+
+void PrepareSessionJob::start()
+{
+    // First we need to retrieve list of capabilities
+    auto *job = new KIMAP::CapabilitiesJob(m_session);
+    addSubjob(job);
+    job->start();
+}
+
+void PrepareSessionJob::slotResult(KJob *job)
+{
+    if (qobject_cast<KIMAP::CapabilitiesJob *>(job)) {
+        capabilitiesJobDone(job);
+    }
+
+    removeSubjob(job);
+
+    if (!hasSubjobs()) {
+        emitResult();
+    }
+}
+
+bool PrepareSessionJob::handleError(KJob *job, Error error)
+{
+    if (job->error()) {
+        setError(error);
+        setErrorText(job->errorString());
+        return true;
+    }
+
+    return false;
+}
+
+void PrepareSessionJob::capabilitiesJobDone(KJob *job)
+{
+    if (handleError(job, CapabilitiesTestError)) {
+        return;
+    }
+
+    auto *capsJob = qobject_cast<KIMAP::CapabilitiesJob *>(job);
+    m_capabilities = capsJob->capabilities();
+
+    for (const auto &cap : m_capabilities) {
+        if (cap == QStringView{u"NAMESPACE"}) {
+            auto *job = new KIMAP::NamespaceJob(m_session);
+            connect(job, &KJob::result, this, &PrepareSessionJob::namespaceJobDone);
+            addSubjob(job);
+            job->start();
+        } else if (cap == QStringView{u"ID"}) {
+            auto *job = new KIMAP::IdJob(m_session);
+            job->setField("name", m_clientId);
+            connect(job, &KJob::result, this, &PrepareSessionJob::idJobDone);
+            addSubjob(job);
+            job->start();
+        }
+    }
+}
+
+void PrepareSessionJob::namespaceJobDone(KJob *job)
+{
+    if (handleError(job, NamespaceFetchError)) {
+        return;
+    }
+
+    auto *nsJob = qobject_cast<KIMAP::NamespaceJob *>(job);
+
+    m_personalNamespaces = nsJob->personalNamespaces();
+    m_userNamespaces = nsJob->userNamespaces();
+    m_sharedNamespaces = nsJob->sharedNamespaces();
+
+    if (nsJob->containsEmptyNamespace()) {
+        // When we got the empty namespace here, we assume that the other
+        // ones can be freely ignored and that the server will give us all
+        // the mailboxes if we list from the empty namespace itself...
+
+        m_namespaces.clear();
+    } else {
+        // ... otherwise we assume that we have to list explicitly each
+        // namespace
+
+        m_namespaces = nsJob->personalNamespaces()
+                       +nsJob->userNamespaces()
+                       +nsJob->sharedNamespaces();
+    }
+}
+
+void PrepareSessionJob::idJobDone(KJob *job)
+{
+    handleError(job, IdentificationError);
+}
+

resources/imap/preparesessionjob.h

+/*
+    SPDX-FileCopyrightText: 2020 Daniel Vrátil <dvratil@kde.org>
+
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#include <KCompositeJob>
+
+#include <KIMAP/ListJob>
+
+namespace KIMAP {
+class Session;
+}
+
+class PrepareSessionJob : public KCompositeJob
+{
+    Q_OBJECT
+public:
+    enum Error {
+        CapabilitiesTestError = KJob::UserDefinedError,
+        NamespaceFetchError,
+        IdentificationError
+    };
+
+    explicit PrepareSessionJob(KIMAP::Session *session, const QByteArray &clientId);
+
+    void start() override;
+
+    KIMAP::Session *session() const { return m_session; }
+    QStringList capabilities() const { return m_capabilities; }
+    QList<KIMAP::MailBoxDescriptor> namespaces() const { return m_namespaces; }
+    QList<KIMAP::MailBoxDescriptor> personalNamespaces() const { return m_personalNamespaces; }
+    QList<KIMAP::MailBoxDescriptor> userNamespaces() const { return m_userNamespaces; }
+    QList<KIMAP::MailBoxDescriptor> sharedNamespaces() const { return m_sharedNamespaces; }
+
+protected:
+    void slotResult(KJob *job) override;
+
+private:
+    bool handleError(KJob *job, Error error);
+
+    void capabilitiesJobDone(KJob *job);
+    void namespaceJobDone(KJob *job);
+    void idJobDone(KJob *job);
+
+private:
+    KIMAP::Session *m_session;
+    QByteArray m_clientId;
+
+    QStringList m_capabilities;
+
+    QList<KIMAP::MailBoxDescriptor> m_namespaces;
+    QList<KIMAP::MailBoxDescriptor> m_personalNamespaces;
+    QList<KIMAP::MailBoxDescriptor> m_userNamespaces;
+    QList<KIMAP::MailBoxDescriptor> m_sharedNamespaces;
+};

resources/imap/resourcestate.cpp

@@ -49,6 +49,11 @@ QStringList ResourceState::serverCapabilities() const
     return m_resource->m_pool->serverCapabilities();
 }
 
+QStringList ResourceState::effectiveServerCapabilities() const
+{
+    return m_resource->m_pool->effectiveServerCapabilities();
+}
+
 QList<KIMAP::MailBoxDescriptor> ResourceState::serverNamespaces() const
 {
     return m_resource->m_pool->serverNamespaces();

resources/imap/resourcestate.h

@@ -111,6 +111,7 @@ public:
     QString resourceName() const override;
     QString resourceIdentifier() const override;
     QStringList serverCapabilities() const override;
+    QStringList effectiveServerCapabilities() const override;
     QList<KIMAP::MailBoxDescriptor> serverNamespaces() const override;
     QList<KIMAP::MailBoxDescriptor> personalNamespaces() const override;
     QList<KIMAP::MailBoxDescriptor> userNamespaces() const override;

resources/imap/resourcetask.cpp

@@ -524,6 +524,13 @@ bool ResourceTask::serverSupportsCondstore() const
            && !serverCapabilities().contains(QLatin1String("X-GM-EXT-1"));
 }
 
+bool ResourceTask::isQResyncEnabled() const
+{
+    // Check support for QRESYNC (RFC5162).
+    // QRESYNC must be enabled on each session, so check that has happened.
+    return effectiveServerCapabilities().contains(QLatin1String("QRESYNC"));
+}
+
 int ResourceTask::batchSize() const
 {
     return m_resource->batchSize();

resources/imap/resourcetask.h

@@ -122,6 +122,7 @@ protected:
 
     virtual bool serverSupportsAnnotations() const;
     virtual bool serverSupportsCondstore() const;
+    virtual bool serverSupportsQResync() const;
 
     int batchSize() const;
     void setItemMergingMode(Akonadi::ItemSync::MergeMode mode);

resources/imap/sessionpool.cpp

@@ -13,13 +13,12 @@
 #include "imapresource_debug.h"
 #include <KLocalizedString>
 
-#include <kimap/capabilitiesjob.h>
 #include <kimap/logoutjob.h>
-#include <kimap/namespacejob.h>
-#include <kimap/idjob.h>
+#include <kimap/enablejob.h>
 
 #include "imapaccount.h"
 #include "passwordrequesterinterface.h"
+#include "preparesessionjob.h"
 
 qint64 SessionPool::m_requestCounter = 0;
 
@@ -112,6 +111,7 @@ void SessionPool::disconnect(SessionTermination termination)
     m_account = nullptr;
     m_namespaces.clear();
     m_capabilities.clear();
+    m_effectiveCapabilities.clear();
 
     m_initialConnectDone = false;
     Q_EMIT disconnectDone();
@@ -159,6 +159,11 @@ QStringList SessionPool::serverCapabilities() const
     return m_capabilities;
 }
 
+QStringList SessionPool::effectiveServerCapabilities() const
+{
+    return m_effectiveCapabilities;
+}
+
 QList<KIMAP::MailBoxDescriptor> SessionPool::serverNamespaces() const
 {
     return m_namespaces;
@@ -394,12 +399,12 @@ void SessionPool::onLoginDone(KJob *job)
 
     if (job->error() == 0) {
         if (m_initialConnectDone) {
-            declareSessionReady(login->session());
+            enableSessionCapabilities(login->session());
         } else {
-            // On initial connection we ask for capabilities
-            KIMAP::CapabilitiesJob *capJob = new KIMAP::CapabilitiesJob(login->session());
-            QObject::connect(capJob, &KIMAP::CapabilitiesJob::result, this, &SessionPool::onCapabilitiesTestDone);
-            capJob->start();
+            // On initial connection we need to prepare the session (and ourselves)
+            auto *prepJob = new PrepareSessionJob(login->session(), m_clientId);
+            QObject::connect(prepJob, &KJob::result, this, &SessionPool::onPrepareSessionDone);
+            prepJob->start();
         }
     } else {
         if (job->error() == KIMAP::LoginJob::ERR_COULD_NOT_CONNECT) {
@@ -424,33 +429,50 @@ void SessionPool::onLoginDone(KJob *job)
     }
 }
 
-void SessionPool::onCapabilitiesTestDone(KJob *job)
+void SessionPool::onPrepareSessionDone(KJob *job)
 {
-    KIMAP::CapabilitiesJob *capJob = qobject_cast<KIMAP::CapabilitiesJob *>(job);
+    auto *prepareJob = qobject_cast<PrepareSessionJob *>(job);
+
     // Can happen if we disconnected meanwhile
-    if (!m_connectingPool.contains(capJob->session())) {
+    if (!m_connectingPool.contains(prepareJob->session())) {
         Q_EMIT connectDone(CancelledError, i18n("Disconnected from server during login."));
         return;
     }
 
     if (job->error()) {
-        if (m_account) {
-            cancelSessionCreation(capJob->session(),
-                                  CapabilitiesTestError,
-                                  i18n("Could not test the capabilities supported by the "
-                                       "IMAP server %1.\n%2",
-                                       m_account->server(), job->errorString()));
-        } else {
-            // Can happen when we lose all ready connections while trying to check capabilities.
-            cancelSessionCreation(capJob->session(),
-                                  CapabilitiesTestError,
-                                  i18n("Could not test the capabilities supported by the "
-                                       "IMAP server.\n%1", job->errorString()));
+        QString msg;
+        int code = KJob::NoError;
+        switch (job->error()) {
+        case PrepareSessionJob::CapabilitiesTestError:
+            code = CapabilitiesTestError;
+            msg = m_account ? i18n("Could not test the capabilities supported by the IMAP server %1.\n%2", m_account->server(), job->errorString())
+                            : i18n("Could not test the capabilities supported by the IMAP server.\n%1", job->errorString());
+            break;
+        case PrepareSessionJob::NamespaceFetchError:
+            code = job->error();
+            msg = m_account ? i18n("Could not retrieve namespaces from the IMAP server %1.\n%2", m_account->server(), job->errorString())
+                            : i18n("Could not retrieve namespaces from the IMAP server.\n%1", job->errorString());
+            break;
+        case PrepareSessionJob::IdentificationError:
+            code = job->error();
+            msg = m_account ? i18n("Could not send client ID to the IMAP server %1.\n%2", m_account->server(), job->errorString())
+                            : i18n("Could not send client ID to the IMAP server.\n%1", job->errorString());
+            break;
+        default:
+            code = job->error();
+            msg = job->errorString();
         }
+
+        cancelSessionCreation(prepareJob->session(), code, msg);
         return;
     }
 
-    m_capabilities = capJob->capabilities();
+    m_capabilities = prepareJob->capabilities();
+    m_effectiveCapabilities = prepareJob->capabilities();
+    m_namespaces = prepareJob->namespaces();
+    m_personalNamespaces = prepareJob->personalNamespaces();
+    m_userNamespaces = prepareJob->userNamespaces();
+    m_sharedNamespaces = prepareJob->sharedNamespaces();
 
     QStringList missing;
     const QStringList expected = {QStringLiteral("IMAP4REV1")};
@@ -461,7 +483,7 @@ void SessionPool::onCapabilitiesTestDone(KJob *job)
     }
 
     if (!missing.isEmpty()) {
-        cancelSessionCreation(capJob->session(),
+        cancelSessionCreation(prepareJob->session(),
                               IncompatibleServerError,
                               i18n("Cannot use the IMAP server %1, "
                                    "some mandatory capabilities are missing: %2. "
@@ -471,76 +493,50 @@ void SessionPool::onCapabilitiesTestDone(KJob *job)
         return;
     }
 
-    // If the extension is supported, grab the namespaces from the server
-    if (m_capabilities.contains(QLatin1String("NAMESPACE"))) {
-        KIMAP::NamespaceJob *nsJob = new KIMAP::NamespaceJob(capJob->session());
-        QObject::connect(nsJob, &KIMAP::NamespaceJob::result, this, &SessionPool::onNamespacesTestDone);
-        nsJob->start();
-        return;
-    } else if (m_capabilities.contains(QLatin1String("ID"))) {
-        KIMAP::IdJob *idJob = new KIMAP::IdJob(capJob->session());
-        idJob->setField("name", m_clientId);
-        QObject::connect(idJob, &KIMAP::IdJob::result, this, &SessionPool::onIdDone);
-        idJob->start();
-        return;
-    } else {
-        declareSessionReady(capJob->session());
-    }
+    enableSessionCapabilities(prepareJob->session());
 }
 
-void SessionPool::setClientId(const QByteArray &clientId)
+void SessionPool::enableSessionCapabilities(KIMAP::Session *session)
 {
-    m_clientId = clientId;
+    QStringList capsToEnable;
+
+    if (m_capabilities.contains(QStringView{u"CONDSTORE"})
+        && m_capabilities.contains(QStringView{u"QRESYNC"})) {
+        capsToEnable.push_back(QStringLiteral("QRESYNC"));
+    }
+
+    if (!capsToEnable.empty()) {
+        qCDebug(IMAPRESOURCE_LOG) << "Capabilities to ENABLE on the server:" << capsToEnable;
+        auto *job = new KIMAP::EnableJob(session);
+        job->setCapabilities(capsToEnable);
+        QObject::connect(job, &KJob::result, this, &SessionPool::onEnableDone);
+        job->start();
+    } else {
+        declareSessionReady(session);
+    }
 }
 
-void SessionPool::onNamespacesTestDone(KJob *job)
+void SessionPool::onEnableDone(KJob *job)
 {
-    KIMAP::NamespaceJob *nsJob = qobject_cast<KIMAP::NamespaceJob *>(job);
-    // Can happen if we disconnect meanwhile
-    if (!m_connectingPool.contains(nsJob->session())) {
+    auto *enableJob = qobject_cast<KIMAP::EnableJob *>(job);
+
+    // Can happen if we disconnected meanwhile
+    if (!m_connectingPool.contains(enableJob->session())) {
         Q_EMIT connectDone(CancelledError, i18n("Disconnected from server during login."));
         return;
     }
 
-    m_personalNamespaces = nsJob->personalNamespaces();
-    m_userNamespaces = nsJob->userNamespaces();
-    m_sharedNamespaces = nsJob->sharedNamespaces();
-
-    if (nsJob->containsEmptyNamespace()) {
-        // When we got the empty namespace here, we assume that the other
-        // ones can be freely ignored and that the server will give us all
-        // the mailboxes if we list from the empty namespace itself...
-
-        m_namespaces.clear();
-    } else {
-        // ... otherwise we assume that we have to list explicitly each
-        // namespace
-
-        m_namespaces = nsJob->personalNamespaces()
-                       +nsJob->userNamespaces()
-                       +nsJob->sharedNamespaces();
+    if (!enableJob->enabledCapabilities().contains(QStringView{u"QRESYNC"})) {
+        qCWarning(IMAPRESOURCE_LOG) << "Failed to enable QRESYNC!";
+        m_effectiveCapabilities.removeOne(QStringLiteral("QRESYNC"));
     }
 
-    if (m_capabilities.contains(QLatin1String("ID"))) {
-        KIMAP::IdJob *idJob = new KIMAP::IdJob(nsJob->session());
-        idJob->setField("name", m_clientId);
-        QObject::connect(idJob, &KIMAP::IdJob::result, this, &SessionPool::onIdDone);
-        idJob->start();
-        return;
-    } else {
-        declareSessionReady(nsJob->session());
-    }
+    declareSessionReady(enableJob->session());
 }
 
-void SessionPool::onIdDone(KJob *job)
+void SessionPool::setClientId(const QByteArray &clientId)
 {
-    KIMAP::IdJob *idJob = qobject_cast<KIMAP::IdJob *>(job);
-    // Can happen if we disconnected meanwhile
-    if (!m_connectingPool.contains(idJob->session())) {
-        Q_EMIT connectDone(CancelledError, i18n("Disconnected during login."));
-        return;
-    }
-    declareSessionReady(idJob->session());
+    m_clientId = clientId;
 }
 
 void SessionPool::onConnectionLost()
@@ -557,6 +553,7 @@ void SessionPool::onConnectionLost()
         m_account = nullptr;
         m_namespaces.clear();
         m_capabilities.clear();
+        m_effectiveCapabilities.clear();
 
         m_initialConnectDone = false;
     }

resources/imap/sessionpool.h

@@ -68,6 +68,7 @@ public:
 
     ImapAccount *account() const;
     QStringList serverCapabilities() const;
+    QStringList effectiveServerCapabilities() const;
     QList<KIMAP::MailBoxDescriptor> serverNamespaces() const;
     enum Namespace {
         Personal,
@@ -88,18 +89,19 @@ private Q_SLOTS:
 
     void onPasswordRequestDone(int resultType, const QString &password);
     void onLoginDone(KJob *job);
-    void onCapabilitiesTestDone(KJob *job);
-    void onNamespacesTestDone(KJob *job);
-    void onIdDone(KJob *job);
+    void onPrepareSessionDone(KJob *job);
+    void onEnableDone(KJob *job);
 
     void onSessionDestroyed(QObject *);
 
 private:
     void onConnectionLost();
     void killSession(KIMAP::Session *session, SessionTermination termination);
+    void enableSessionCapabilities(KIMAP::Session *session);
     void declareSessionReady(KIMAP::Session *session);
     void cancelSessionCreation(KIMAP::Session *session, int errorCode, const QString &errorString);
     void requestPassword();
+    void enableQResync(KIMAP::Session *session);
 
     static qint64 m_requestCounter;
 
@@ -117,6 +119,7 @@ private:
     QList<KIMAP::Session *> m_reservedPool;  // currently used
 
     QStringList m_capabilities;
+    QStringList m_effectiveCapabilities;
     QList<KIMAP::MailBoxDescriptor> m_namespaces;
     QList<KIMAP::MailBoxDescriptor> m_personalNamespaces;
     QList<KIMAP::MailBoxDescriptor> m_userNamespaces;