diff --git a/autotests/server/fakedatastore.h b/autotests/server/fakedatastore.h --- a/autotests/server/fakedatastore.h +++ b/autotests/server/fakedatastore.h @@ -104,7 +104,7 @@ const QString &gid, PimItem &pimItem) override; - bool cleanupPimItems(const PimItem::List &items) override; + bool cleanupPimItems(const PimItem::List &items, bool silent = false) override; bool unhidePimItem(PimItem &pimItem) override; bool unhideAllPimItems() override; diff --git a/autotests/server/fakedatastore.cpp b/autotests/server/fakedatastore.cpp --- a/autotests/server/fakedatastore.cpp +++ b/autotests/server/fakedatastore.cpp @@ -259,11 +259,11 @@ remote_id, remoteRevision, gid, pimItem); } -bool FakeDataStore::cleanupPimItems(const PimItem::List &items) +bool FakeDataStore::cleanupPimItems(const PimItem::List &items, bool silent) { mChanges.insert(QStringLiteral("cleanupPimItems"), - QVariantList() << QVariant::fromValue(items)); - return DataStore::cleanupPimItems(items); + QVariantList() << QVariant::fromValue(items) << silent); + return DataStore::cleanupPimItems(items, silent); } bool FakeDataStore::unhidePimItem(PimItem &pimItem) diff --git a/src/server/handler/itemcreatehandler.h b/src/server/handler/itemcreatehandler.h --- a/src/server/handler/itemcreatehandler.h +++ b/src/server/handler/itemcreatehandler.h @@ -28,6 +28,8 @@ namespace Server { +class Transaction; + /** @ingroup akonadi_server_handler @@ -62,6 +64,8 @@ bool notify(const PimItem &item, bool seen, const Collection &collection); bool notify(const PimItem &item, const Collection &collection, const QSet &changedParts); + + void recoverFromMultipleMergeCandidates(const PimItem::List &items, const Collection &collection); }; } // namespace Server diff --git a/src/server/handler/itemcreatehandler.cpp b/src/server/handler/itemcreatehandler.cpp --- a/src/server/handler/itemcreatehandler.cpp +++ b/src/server/handler/itemcreatehandler.cpp @@ -30,8 +30,12 @@ #include "storage/partstreamer.h" #include "storage/parthelper.h" #include "storage/selectquerybuilder.h" +#include "storage/itemretrievalmanager.h" #include +#include "shared/akranges.h" +#include "shared/akscopeguard.h" + #include //std::accumulate using namespace Akonadi; @@ -353,6 +357,60 @@ return true; } +void ItemCreateHandler::recoverFromMultipleMergeCandidates(const PimItem::List &items, const Collection &collection) +{ + // HACK HACK HACK: When this happens within ItemSync, we are running inside a client-side + // transaction, so just calling commit here won't have any effect, since this handler will + // ultimately fail and the client will rollback the transaction. To circumvent this, we + // will forcibly commit the transaction, do our changes here within a new transaction and + // then we open a new transaction so that the client won't notice. + + int transactionDepth = 0; + while (storageBackend()->inTransaction()) { + ++transactionDepth; + storageBackend()->commitTransaction(); + } + const AkScopeGuard restoreTransaction([&]() { + for (int i = 0; i < transactionDepth; ++i) { + storageBackend()->beginTransaction(QStringLiteral("RestoredTransactionAfterMMCRecovery")); + } + }); + + Transaction transaction(storageBackend(), QStringLiteral("MMC Recovery Transaction")); + + // If any of the conflicting items is dirty or does not have a remote ID, we don't want to remove + // them as it would cause data loss. There's a chance next changeReplay will fix this, so + // next time the ItemSync hits this multiple merge candidates, all changes will be committed + // and this check will succeed + if (items | any([](const auto &item) { return item.dirty() || item.remoteId().isEmpty(); })) { + qCWarning(AKONADISERVER_LOG) << "Automatic multiple merge candidates recovery failed: at least one of the candidates has uncommitted changes!"; + return; + } + + // This cannot happen with ItemSync, but in theory could happen during individual GID merge. + if (items | any([collection](const auto &item) { return item.collectionId() != collection.id(); })) { + qCWarning(AKONADISERVER_LOG) << "Automatic multiple merge candidates recovery failed: all candidates do not belong to the same collection."; + return; + } + + storageBackend()->cleanupPimItems(items, DataStore::Silent); + if (!transaction.commit()) { + qCWarning(AKONADISERVER_LOG) << "Automatic multiple merge candidates recovery failed: failed to commit database transaction."; + return; + } + + + // Schedule a new sync of the collection, one that will succeed + const auto resource = collection.resource().name(); + QMetaObject::invokeMethod(ItemRetrievalManager::instance(), "triggerCollectionSync", + Qt::QueuedConnection, + Q_ARG(QString, resource), Q_ARG(qint64, collection.id())); + + qCInfo(AKONADISERVER_LOG) << "Automatic multiple merge candidates recovery successful: conflicting items" << (items | transform([](const auto &i) { return i.id(); }) | toQVector) + << "in collection" << collection.name() << "(ID:" << collection.id() << ") were removed and a new sync was scheduled in the resource" + << resource; +} + bool ItemCreateHandler::parseStream() { const auto &cmd = Protocol::cmdCast(m_command); @@ -370,7 +428,6 @@ return false; } - if (cmd.mergeModes() == Protocol::CreateItemCommand::None) { if (!insertItem(cmd, item, parentCol)) { return false; @@ -434,15 +491,19 @@ } storageTrx.commit(); } else { - qCWarning(AKONADISERVER_LOG) << "Multiple merge candidates:"; + qCWarning(AKONADISERVER_LOG) << "Multiple merge candidates, will attempt to recover:"; for (const PimItem &item : result) { qCWarning(AKONADISERVER_LOG) << "\tID:" << item.id() << ", RID:" << item.remoteId() << ", GID:" << item.gid() << ", Collection:" << item.collection().name() << "(" << item.collectionId() << ")" << ", Resource:" << item.collection().resource().name() << "(" << item.collection().resourceId() << ")"; } - // Nor GID or RID are guaranteed to be unique, so make sure we don't merge - // something we don't want + + transaction.commit(); // commit the current transaction, before we attempt MMC recovery + recoverFromMultipleMergeCandidates(result, parentCol); + + // Even if the recovery was successful, indicate error to force the client to abort the + // sync, since we've interfered with the overall state. return failureResponse(QStringLiteral("Multiple merge candidates in collection '%1', aborting").arg(item.collection().name())); } } diff --git a/src/server/storage/datastore.h b/src/server/storage/datastore.h --- a/src/server/storage/datastore.h +++ b/src/server/storage/datastore.h @@ -103,6 +103,8 @@ { Q_OBJECT public: + const constexpr static bool Silent = true; + /** Closes the database connection and destroys the DataStore object. */ @@ -199,7 +201,7 @@ /** * Removes the pim item and all referenced data ( e.g. flags ) */ - virtual bool cleanupPimItems(const PimItem::List &items); + virtual bool cleanupPimItems(const PimItem::List &items, bool silent = false); /** * Unhides the specified PimItem. Emits the itemAdded() notification as diff --git a/src/server/storage/datastore.cpp b/src/server/storage/datastore.cpp --- a/src/server/storage/datastore.cpp +++ b/src/server/storage/datastore.cpp @@ -1217,29 +1217,31 @@ return false; } -bool DataStore::cleanupPimItems(const PimItem::List &items) +bool DataStore::cleanupPimItems(const PimItem::List &items, bool silent) { // generate relation removed notifications - for (const PimItem &item : items) { - SelectQueryBuilder relationQuery; - relationQuery.addValueCondition(Relation::leftIdFullColumnName(), Query::Equals, item.id()); - relationQuery.addValueCondition(Relation::rightIdFullColumnName(), Query::Equals, item.id()); - relationQuery.setSubQueryMode(Query::Or); + if (!silent) { + for (const PimItem &item : items) { + SelectQueryBuilder relationQuery; + relationQuery.addValueCondition(Relation::leftIdFullColumnName(), Query::Equals, item.id()); + relationQuery.addValueCondition(Relation::rightIdFullColumnName(), Query::Equals, item.id()); + relationQuery.setSubQueryMode(Query::Or); - if (!relationQuery.exec()) { - throw HandlerException("Failed to obtain relations"); - } - const Relation::List relations = relationQuery.result(); - for (const Relation &relation : relations) { - notificationCollector()->relationRemoved(relation); + if (!relationQuery.exec()) { + throw HandlerException("Failed to obtain relations"); + } + const Relation::List relations = relationQuery.result(); + for (const Relation &relation : relations) { + notificationCollector()->relationRemoved(relation); + } } - } - // generate the notification before actually removing the data - notificationCollector()->itemsRemoved(items); + // generate the notification before actually removing the data + notificationCollector()->itemsRemoved(items); + } // FIXME: Create a single query to do this - Q_FOREACH (const PimItem &item, items) { + for (const auto &item : items) { if (!item.clearFlags()) { qCWarning(AKONADISERVER_LOG) << "Failed to clean up flags from PimItem" << item.id(); return false;