Revert "[imap] Restore QRESYNC support"

This reverts commit 8f95c296.

There are still issues with syncing (uidvalidity problems).

CCMAIL: dvratil@kde.org

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
@@ -32,7 +31,6 @@ set( imapresource_LIB_SRCS
   retrievecollectionstask.cpp
   retrieveitemtask.cpp
   retrieveitemstask.cpp
-  retrieveitemstask_qresync.cpp
   searchtask.cpp
   sessionpool.cpp
   uidvalidityattribute.cpp

resources/imap/autotests/CMakeLists.txt

@@ -41,5 +41,4 @@ IMAP_RESOURCE_UNIT_TESTS(
   testretrievecollectionstask
   testretrieveitemtask
   testretrieveitemstask
-  testretrieveitemsqresynctask
 )

resources/imap/autotests/dummyresourcestate.cpp

@@ -67,16 +67,6 @@ QStringList DummyResourceState::serverCapabilities() const
     return m_capabilities;
 }
 
-void DummyResourceState::setEffectiveServerCapabilities(const QStringList &capabilities)
-{
-    m_effectiveCapabilities = capabilities;
-}
-
-QStringList DummyResourceState::effectiveServerCapabilities() const
-{
-    return m_effectiveCapabilities;
-}
-
 void DummyResourceState::setServerNamespaces(const QList<KIMAP::MailBoxDescriptor> &namespaces)
 {
     m_namespaces = namespaces;

resources/imap/autotests/dummyresourcestate.h

@@ -35,9 +35,6 @@ public:
     void setServerCapabilities(const QStringList &capabilities);
     QStringList serverCapabilities() const override;
 
-    void setEffectiveServerCapabilities(const QStringList &capabilities);
-    QStringList effectiveServerCapabilities() const override;
-
     void setServerNamespaces(const QList<KIMAP::MailBoxDescriptor> &namespaces);
     QList<KIMAP::MailBoxDescriptor> serverNamespaces() const override;
     QList<KIMAP::MailBoxDescriptor> personalNamespaces() const override;
@@ -145,7 +142,6 @@ private:
     QString m_resourceName;
     QString m_resourceIdentifier;
     QStringList m_capabilities;
-    QStringList m_effectiveCapabilities;
     QList<KIMAP::MailBoxDescriptor> m_namespaces;
 
     bool m_automaticExpunge;

resources/imap/imapresourcebase.cpp

@@ -60,7 +60,6 @@
 #include "retrievecollectionstask.h"
 #include "retrieveitemtask.h"
 #include "retrieveitemstask.h"
-#include "retrieveitemstask_qresync.h"
 #include "searchtask.h"
 
 #include "settingspasswordrequester.h"
@@ -417,18 +416,9 @@ void ImapResourceBase::retrieveItems(const Collection &col)
 
     setItemStreamingEnabled(true);
 
-    ResourceTask *task;
-    if (m_pool->effectiveServerCapabilities().contains(QStringView{u"QRESYNC"})) {
-        auto *t = new RetrieveItemsTaskQResync(createResourceState(TaskArguments(col)), this);
-        connect(this, &ResourceBase::retrieveNextItemSyncBatch, t, &RetrieveItemsTaskQResync::onReadyForNextBatch);
-        task = t;
-    } else {
-        auto t = new RetrieveItemsTask(createResourceState(TaskArguments(col)), this);
-        connect(this, &ResourceBase::retrieveNextItemSyncBatch, t, &RetrieveItemsTask::onReadyForNextBatch);
-        task = t;
-    }
-
+    RetrieveItemsTask *task = new RetrieveItemsTask(createResourceState(TaskArguments(col)), this);
     connect(task, SIGNAL(status(int,QString)), SIGNAL(status(int,QString)));
+    connect(this, &ResourceBase::retrieveNextItemSyncBatch, task, &RetrieveItemsTask::onReadyForNextBatch);
     startTask(task);
 }
 

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 * const m_session;
-    const 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/resourcestateinterface.h

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

resources/imap/resourcetask.cpp

@@ -144,11 +144,6 @@ QStringList ResourceTask::serverCapabilities() const
     return m_resource->serverCapabilities();
 }
 
-QStringList ResourceTask::effectiveServerCapabilities() const
-{
-    return m_resource->effectiveServerCapabilities();
-}
-
 QList<KIMAP::MailBoxDescriptor> ResourceTask::serverNamespaces() const
 {
     return m_resource->serverNamespaces();
@@ -528,13 +523,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();
@@ -545,7 +533,7 @@ ResourceStateInterface::Ptr ResourceTask::resourceState()
     return m_resource;
 }
 
-KIMAP::Acl::Rights ResourceTask::myRights(const Akonadi::Collection &col) const
+KIMAP::Acl::Rights ResourceTask::myRights(const Akonadi::Collection &col)
 {
     const auto *aclAttribute = col.attribute<Akonadi::ImapAclAttribute>();
     if (aclAttribute) {

resources/imap/resourcetask.h

@@ -56,7 +56,6 @@ protected:
     QString userName() const;
     QString resourceName() const;
     QStringList serverCapabilities() const;
-    QStringList effectiveServerCapabilities() const;
     QList<KIMAP::MailBoxDescriptor> serverNamespaces() const;
 
     bool isAutomaticExpungeEnabled() const;
@@ -123,14 +122,13 @@ protected:
 
     virtual bool serverSupportsAnnotations() const;
     virtual bool serverSupportsCondstore() const;
-    virtual bool isQResyncEnabled() const;
 
     int batchSize() const;
     void setItemMergingMode(Akonadi::ItemSync::MergeMode mode);
 
     ResourceStateInterface::Ptr resourceState();
 
-    KIMAP::Acl::Rights myRights(const Akonadi::Collection &) const;
+    KIMAP::Acl::Rights myRights(const Akonadi::Collection &);
 
 private:
     void abortTask(const QString &errorString);

resources/imap/retrieveitemstask.cpp

@@ -51,8 +51,6 @@ void RetrieveItemsTask::setFetchMissingItemBodies(bool enabled)
 
 void RetrieveItemsTask::doStart(KIMAP::Session *session)
 {
-    m_time.start();
-
     emitPercent(0);
     // Prevent fetching items from noselect folders.
     if (collection().hasAttribute("noselect")) {
@@ -133,6 +131,7 @@ void RetrieveItemsTask::startRetrievalTasks()
 {
     const QString mailBox = mailBoxForCollection(collection());
     qCDebug(IMAPRESOURCE_LOG) << "Starting retrieval for " << mailBox;
+    m_time.start();
 
     // Now is the right time to expunge the messages marked \\Deleted from this mailbox.
     const bool hasACL = serverCapabilities().contains(QLatin1String("ACL"));

resources/imap/retrieveitemstask_qresync.cpp

-/*
-    SPDX-FileCopyrightText: 2020  Daniel Vrátil <dvratil@kde.org>
-
-    SPDX-License-Identifier: LGPL-2.0-or-later
-*/
-
-#include "retrieveitemstask_qresync.h"
-#include "imapresource_debug.h"
-#include "uidvalidityattribute.h"
-#include "highestmodseqattribute.h"
-#include "uidnextattribute.h"
-#include "noselectattribute.h"
-#include "collectionflagsattribute.h"
-#include "messagehelper.h"
-#include "batchfetcher.h"
-
-#include <Akonadi/KMime/MessageParts>
-#include <AkonadiCore/CachePolicy>
-
-#include <KIMAP/Session>
-#include <KIMAP/SelectJob>
-#include <KIMAP/ExpungeJob>
-#include <KIMAP/FetchJob>
-#include <KIMAP/ImapSet>
-#include <KIMAP/CloseJob>
-
-#include <KMime/Message>
-
-#include <KLocalizedString>
-
-namespace {
-
-qint64 getUidValidity(const Akonadi::Collection &col)
-{
-    if (col.hasAttribute<UidValidityAttribute>()) {
-        return col.attribute<UidValidityAttribute>()->uidValidity();
-    }
-
-    return -1;
-}
-
-quint64 getHighestModSeq(const Akonadi::Collection &col)
-{
-    if (col.hasAttribute<HighestModSeqAttribute>()) {
-        return col.attribute<HighestModSeqAttribute>()->highestModSequence();
-    }
-
-    return 0;
-}
-
-int getUidNext(const Akonadi::Collection &col)
-{
-    if (col.hasAttribute<UidNextAttribute>()) {
-        return col.attribute<UidNextAttribute>()->uidNext();
-    }
-
-    return -1;
-}
-
-QList<QByteArray> getFlags(const Akonadi::Collection &col)
-{
-    if (col.hasAttribute<Akonadi::CollectionFlagsAttribute>()) {
-        return col.attribute<Akonadi::CollectionFlagsAttribute>()->flags();
-    }
-
-    return {};
-}
-
-bool isNoSelect(const Akonadi::Collection &col)
-{
-    return col.hasAttribute<NoSelectAttribute>()
-            && col.attribute<NoSelectAttribute>()->noSelect();
-}
-
-bool shouldFetchFullPayload(const Akonadi::Collection &col)
-{
-    return col.cachePolicy().localParts().contains(QLatin1String(Akonadi::MessagePart::Body));
-}
-
-template<typename T, typename Getter, typename Setter, typename Val>
-bool updateAttr(Akonadi::Collection &col, Getter getter, Setter setter, const Val &val)
-{
-    if (col.hasAttribute<T>()) {
-        if ((col.attribute<T>()->*getter)() == val) {
-            return false;
-        }
-    }
-
-    auto *attr = col.attribute<T>(Akonadi::Collection::AddIfMissing);
-    (attr->*setter)(val);
-    return true;
-}
-
-} // namespace
-
-
-
-
-
-RetrieveItemsTaskQResync::RetrieveItemsTaskQResync(const ResourceStateInterface::Ptr &resource, QObject *parent)
-    : ResourceTask(CancelIfNoSession, resource, parent)
-{
-    setItemMergingMode(Akonadi::ItemSync::RIDMerge);
-}
-
-RetrieveItemsTaskQResync::~RetrieveItemsTaskQResync() = default;
-
-bool RetrieveItemsTaskQResync::shouldExpunge(const Akonadi::Collection &col, bool readOnly) const
-{
-    const bool hasACL = serverCapabilities().contains(QLatin1String("ACL"));
-    const auto rights = myRights(col);
-    return !readOnly
-            && isAutomaticExpungeEnabled()
-            && (!hasACL || (rights &KIMAP::Acl::Expunge) || (rights & KIMAP::Acl::Delete));
-}
-
-void RetrieveItemsTaskQResync::doStart(KIMAP::Session *session)
-{
-    m_stats.timer.start();
-
-    emitPercent(0);
-
-    // The flow is as follows
-    // CLOSE (optionally)
-    //  - closes the mailbox (if it needed)
-    // SELECT mailbox (QRESYNC)
-    //  - reports vanished emails
-    //  - reports changed emails
-    // UID EXPUNGE
-    //  - reports vanished emails
-    // FETCH (lastNextUid:*)
-    //  - fetches all new messages
-
-    const auto col = collection();
-    m_localState = MailBoxState {
-        getUidValidity(col),
-        getUidNext(col),
-        getHighestModSeq(col),
-        getFlags(col),
-        -1, // messageCount (reported only by server)
-        -1, // recentCount (reported only by server)
-        -1  // firstUnseenIndex (reported only by server)
-    };
-
-    const auto mailbox = mailBoxForCollection(collection());
-    qCInfo(IMAPRESOURCE_LOG) << "Starting sync of mailbox" << mailbox << "(col" << col.id() << ")";
-    qCDebug(IMAPRESOURCE_LOG) << "Local cache state:";
-    qCDebug(IMAPRESOURCE_LOG) << " UidValidity=" << m_localState.uidValidity;
-    qCDebug(IMAPRESOURCE_LOG) << " UidNext=" << m_localState.nextUid;
-    qCDebug(IMAPRESOURCE_LOG) << " HighestModSeq=" << m_localState.highestModSeq;
-    qCDebug(IMAPRESOURCE_LOG) << " Flags=" << m_localState.flags;
-
-    // Prevent fetching items from noselect folders.
-    if (isNoSelect(collection())) {
-        qCDebug(IMAPRESOURCE_LOG) << "Mailbox" << mailbox << "(col" << col.id() << ") is no-select, not synchronizing.";
-        finishSync();
-        return;
-    }
-
-    // If the mailbox is already opened we need to re-open it in order to get all the
-    // metadata.
-    if (session->selectedMailBox() == mailbox) {
-        qCDebug(IMAPRESOURCE_LOG) << "Mailbox" << mailbox << "already selected, re-opening it.";
-
-        auto *close = new KIMAP::CloseJob(session);
-        connect(close, &KJob::result, this, [this, session, mailbox](KJob *job) {
-            if (job->error()) {
-                qCWarning(IMAPRESOURCE_LOG) << "Failed to close current mailbox" << mailbox << ":" << job->errorString();
-                cancelTask(job->errorString());
-                return;
-            }
-
-            selectMailbox(session);
-        });
-        close->start();
-    } else {
-        selectMailbox(session);
-    }
-}
-
-void RetrieveItemsTaskQResync::selectMailbox(KIMAP::Session *session)
-{
-    auto *select = new KIMAP::SelectJob(session);
-    select->setMailBox(mailBoxForCollection(collection()));
-    select->setCondstoreEnabled(true);
-    if (m_localState.uidValidity > -1 && m_localState.highestModSeq > 0) {
-        select->setQResync(m_localState.uidValidity, m_localState.highestModSeq);
-    }
-    connect(select, &KIMAP::SelectJob::vanished, this, &RetrieveItemsTaskQResync::removeLocalMessages);
-    connect(select, &KIMAP::SelectJob::modified, this, &RetrieveItemsTaskQResync::updateLocalMessages);
-    connect(select, &KJob::result, this, [this, select](KJob * /*job*/) {
-        if (select->error()) {
-            qCWarning(IMAPRESOURCE_LOG) << "Failed to select mailbox" << mailBoxForCollection(collection()) << ":" << select->errorString();
-            cancelTask(select->errorString());
-            return;
-        }
-
-        m_serverState = MailBoxState{
-            select->uidValidity(),
-            select->nextUid(),
-            select->highestModSequence(),
-            select->flags(),
-            select->messageCount(),
-            select->recentCount(),
-            select->firstUnseenIndex()
-        };
-        qCDebug(IMAPRESOURCE_LOG) << "Server state reported by SELECT command:";
-        qCDebug(IMAPRESOURCE_LOG) << "  UidValidity=" << m_serverState.uidValidity;
-        qCDebug(IMAPRESOURCE_LOG) << "  NextUid=" << m_serverState.nextUid;
-        qCDebug(IMAPRESOURCE_LOG) << "  HighestModSeq=" << m_serverState.highestModSeq;
-        qCDebug(IMAPRESOURCE_LOG) << "  Flags=" << m_serverState.flags;
-        qCDebug(IMAPRESOURCE_LOG) << "  MessageCount=" << m_serverState.messageCount;
-        qCDebug(IMAPRESOURCE_LOG) << "  RecentCount=" << m_serverState.recentCount;
-        qCDebug(IMAPRESOURCE_LOG) << "  FirstUnseenIndex=" << m_serverState.firstUnseen;
-
-        if (m_serverState.nextUid < 0) {
-            qCInfo(IMAPRESOURCE_LOG) << "Server did not report UIDNEXT, server is broken.";
-            cancelTask(i18n("Server has not reported UIDNEXT."));
-            return;
-        } else if (m_serverState.uidValidity != m_localState.uidValidity || m_localState.nextUid <= 0) {
-            // Check UIDVALIDITY matches. If not, we must do a full re-sync
-            qCInfo(IMAPRESOURCE_LOG) << "UidValidity mismatch for mailbox" << mailBoxForCollection(collection()) << ", forcing full resync.";
-            m_syncMode = SyncMode::Full;
-            m_localState.highestModSeq = 0;
-            m_localState.nextUid = 1; // We want to re-fetch all messages
-            setTotalItems(m_serverState.messageCount);
-        } else if (m_serverState.highestModSeq == m_localState.highestModSeq) {
-            // Optimization: there's nothing to sync!
-            qCInfo(IMAPRESOURCE_LOG) << "Mailbox" << mailBoxForCollection(collection()) << "already up to date";
-            finishSync();
-            return;
-        }
-
-        // At this point, we have synchronized: