Revert "Refactor SessionPool"

This reverts commit 50057f59.

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.41")
+set(KIMAP_LIB_VERSION "5.15.40")
 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,7 +24,6 @@ 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,11 +49,6 @@ 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,7 +111,6 @@ 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,13 +524,6 @@ 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,7 +122,6 @@ 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,12 +13,13 @@
 #include "imapresource_debug.h"
 #include <KLocalizedString>
 
+#include <kimap/capabilitiesjob.h>
 #include <kimap/logoutjob.h>
-#include <kimap/enablejob.h>
+#include <kimap/namespacejob.h>
+#include <kimap/idjob.h>
 
 #include "imapaccount.h"
 #include "passwordrequesterinterface.h"
-#include "preparesessionjob.h"
 
 qint64 SessionPool::m_requestCounter = 0;
 
@@ -111,7 +112,6 @@ void SessionPool::disconnect(SessionTermination termination)
     m_account = nullptr;
     m_namespaces.clear();
     m_capabilities.clear();
-    m_effectiveCapabilities.clear();
 
     m_initialConnectDone = false;
     Q_EMIT disconnectDone();
@@ -159,11 +159,6 @@ QStringList SessionPool::serverCapabilities() const
     return m_capabilities;
 }
 
-QStringList SessionPool::effectiveServerCapabilities() const
-{
-    return m_effectiveCapabilities;
-}
-
 QList<KIMAP::MailBoxDescriptor> SessionPool::serverNamespaces() const
 {
     return m_namespaces;
@@ -399,12 +394,12 @@ void SessionPool::onLoginDone(KJob *job)
 
     if (job->error() == 0) {
         if (m_initialConnectDone) {
-            enableSessionCapabilities(login->session());
+            declareSessionReady(login->session());
         } else {
-            // 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();
+            // 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();
         }
     } else {
         if (job->error() == KIMAP::LoginJob::ERR_COULD_NOT_CONNECT) {
@@ -429,50 +424,33 @@ void SessionPool::onLoginDone(KJob *job)
     }
 }
 
-void SessionPool::onPrepareSessionDone(KJob *job)
+void SessionPool::onCapabilitiesTestDone(KJob *job)
 {
-    auto *prepareJob = qobject_cast<PrepareSessionJob *>(job);
-
+    KIMAP::CapabilitiesJob *capJob = qobject_cast<KIMAP::CapabilitiesJob *>(job);
     // Can happen if we disconnected meanwhile
-    if (!m_connectingPool.contains(prepareJob->session())) {
+    if (!m_connectingPool.contains(capJob->session())) {
         Q_EMIT connectDone(CancelledError, i18n("Disconnected from server during login."));
         return;
     }
 
     if (job->error()) {
-        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();
+        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()));
         }
-
-        cancelSessionCreation(prepareJob->session(), code, msg);
         return;
     }
 
-    m_capabilities = prepareJob->capabilities();
-    m_effectiveCapabilities = prepareJob->capabilities();
-    m_namespaces = prepareJob->namespaces();
-    m_personalNamespaces = prepareJob->personalNamespaces();
-    m_userNamespaces = prepareJob->userNamespaces();
-    m_sharedNamespaces = prepareJob->sharedNamespaces();
+    m_capabilities = capJob->capabilities();
 
     QStringList missing;
     const QStringList expected = {QStringLiteral("IMAP4REV1")};
@@ -483,7 +461,7 @@ void SessionPool::onPrepareSessionDone(KJob *job)
     }
 
     if (!missing.isEmpty()) {
-        cancelSessionCreation(prepareJob->session(),
+        cancelSessionCreation(capJob->session(),
                               IncompatibleServerError,
                               i18n("Cannot use the IMAP server %1, "
                                    "some mandatory capabilities are missing: %2. "
@@ -493,50 +471,76 @@ void SessionPool::onPrepareSessionDone(KJob *job)
         return;
     }
 
-    enableSessionCapabilities(prepareJob->session());
-}
-
-void SessionPool::enableSessionCapabilities(KIMAP::Session *session)
-{
-    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();
+    // 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(session);
+        declareSessionReady(capJob->session());
     }
 }
 
-void SessionPool::onEnableDone(KJob *job)
+void SessionPool::setClientId(const QByteArray &clientId)
 {
-    auto *enableJob = qobject_cast<KIMAP::EnableJob *>(job);
+    m_clientId = clientId;
+}
 
-    // Can happen if we disconnected meanwhile
-    if (!m_connectingPool.contains(enableJob->session())) {
+void SessionPool::onNamespacesTestDone(KJob *job)
+{
+    KIMAP::NamespaceJob *nsJob = qobject_cast<KIMAP::NamespaceJob *>(job);
+    // Can happen if we disconnect meanwhile
+    if (!m_connectingPool.contains(nsJob->session())) {
         Q_EMIT connectDone(CancelledError, i18n("Disconnected from server during login."));
         return;
     }
 
-    if (!enableJob->enabledCapabilities().contains(QStringView{u"QRESYNC"})) {
-        qCWarning(IMAPRESOURCE_LOG) << "Failed to enable QRESYNC!";
-        m_effectiveCapabilities.removeOne(QStringLiteral("QRESYNC"));
+    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();
     }
 
-    declareSessionReady(enableJob->session());
+    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());
+    }
 }
 
-void SessionPool::setClientId(const QByteArray &clientId)
+void SessionPool::onIdDone(KJob *job)
 {
-    m_clientId = 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());
 }
 
 void SessionPool::onConnectionLost()
@@ -553,7 +557,6 @@ void SessionPool::onConnectionLost()
         m_account = nullptr;
         m_namespaces.clear();
         m_capabilities.clear();
-        m_effectiveCapabilities.clear();
 
         m_initialConnectDone = false;
     }

resources/imap/sessionpool.h

@@ -68,7 +68,6 @@ public:
 
     ImapAccount *account() const;
     QStringList serverCapabilities() const;
-    QStringList effectiveServerCapabilities() const;
     QList<KIMAP::MailBoxDescriptor> serverNamespaces() const;
     enum Namespace {
         Personal,
@@ -89,19 +88,18 @@ private Q_SLOTS:
 
     void onPasswordRequestDone(int resultType, const QString &password);
     void onLoginDone(KJob *job);
-    void onPrepareSessionDone(KJob *job);
-    void onEnableDone(KJob *job);
+    void onCapabilitiesTestDone(KJob *job);
+    void onNamespacesTestDone(KJob *job);
+    void onIdDone(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;
 
@@ -119,7 +117,6 @@ 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;