diff --git a/autotests/libs/itemsynctest.cpp b/autotests/libs/itemsynctest.cpp --- a/autotests/libs/itemsynctest.cpp +++ b/autotests/libs/itemsynctest.cpp @@ -42,15 +42,14 @@ using namespace Akonadi; Q_DECLARE_METATYPE(KJob *) -Q_DECLARE_METATYPE(ItemSync::TransactionMode) class ItemsyncTest : public QObject { Q_OBJECT private: Item::List fetchItems(const Collection &col) { - qDebug() << "fetching items from collection" << col.remoteId() << col.name(); + qDebug() << "fetching items from collection" << col.id(); ItemFetchJob *fetch = new ItemFetchJob(col, this); fetch->fetchScope().fetchFullPayload(); fetch->fetchScope().fetchAllAttributes(); @@ -80,7 +79,6 @@ Control::start(); AkonadiTest::setAllResourcesOffline(); qRegisterMetaType(); - qRegisterMetaType(); } static Item modifyItem(Item item) @@ -111,12 +109,8 @@ QVERIFY(changedSpy.isValid()); ItemSync *syncer = new ItemSync(col); - syncer->setTransactionMode(ItemSync::SingleTransaction); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); syncer->setFullSyncItems(origItems); AKVERIFYEXEC(syncer); - QCOMPARE(transactionSpy.count(), 1); Item::List resultItems = fetchItems(col); QCOMPARE(resultItems.count(), origItems.count()); @@ -128,18 +122,14 @@ void testFullStreamingSync_data() { - QTest::addColumn("transactionMode"); QTest::addColumn("goToEventLoopAfterAddingItems"); - QTest::newRow("single transaction, no eventloop") << ItemSync::SingleTransaction << false; - QTest::newRow("multi transaction, no eventloop") << ItemSync::MultipleTransactions << false; - QTest::newRow("single transaction, with eventloop") << ItemSync::SingleTransaction << true; - QTest::newRow("multi transaction, with eventloop") << ItemSync::MultipleTransactions << true; + QTest::newRow("no eventloop") << false; + QTest::newRow("with eventloop") << true; } void testFullStreamingSync() { - QFETCH(ItemSync::TransactionMode, transactionMode); QFETCH(bool, goToEventLoopAfterAddingItems); const Collection col = Collection(collectionIdFromPath(QStringLiteral("res1/foo"))); @@ -157,9 +147,6 @@ QVERIFY(changedSpy.isValid()); ItemSync *syncer = new ItemSync(col); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); - syncer->setTransactionMode(transactionMode); syncer->setBatchSize(1); syncer->setAutoDelete(false); syncer->setStreamingEnabled(true); @@ -186,12 +173,6 @@ KJob *job = spy.at(0).at(0).value(); QCOMPARE(job, syncer); QCOMPARE(job->error(), 0); - if (transactionMode == ItemSync::SingleTransaction) { - QCOMPARE(transactionSpy.count(), 1); - } - if (transactionMode == ItemSync::MultipleTransactions) { - QCOMPARE(transactionSpy.count(), origItems.count()); - } Item::List resultItems = fetchItems(col); QCOMPARE(resultItems.count(), origItems.count()); @@ -226,12 +207,8 @@ { ItemSync *syncer = new ItemSync(col); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); - syncer->setTransactionMode(ItemSync::SingleTransaction); syncer->setIncrementalSyncItems(origItems, Item::List()); AKVERIFYEXEC(syncer); - QCOMPARE(transactionSpy.count(), 1); } QTest::qWait(100); @@ -260,12 +237,8 @@ { ItemSync *syncer = new ItemSync(col); - syncer->setTransactionMode(ItemSync::SingleTransaction); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); - syncer->setIncrementalSyncItems(resultItems, delItems); + syncer->setIncrementalSyncItems({}, delItems); AKVERIFYEXEC(syncer); - QCOMPARE(transactionSpy.count(), 1); } Item::List resultItems2 = fetchItems(col); @@ -298,9 +271,6 @@ QVERIFY(changedSpy.isValid()); ItemSync *syncer = new ItemSync(col); - syncer->setTransactionMode(ItemSync::SingleTransaction); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); syncer->setAutoDelete(false); QSignalSpy spy(syncer, SIGNAL(result(KJob*))); QVERIFY(spy.isValid()); @@ -323,7 +293,6 @@ KJob *job = spy.at(0).at(0).value(); QCOMPARE(job, syncer); QCOMPARE(job->error(), 0); - QCOMPARE(transactionSpy.count(), 1); Item::List resultItems = fetchItems(col); QCOMPARE(resultItems.count(), origItems.count()); @@ -352,13 +321,8 @@ QVERIFY(changedSpy.isValid()); ItemSync *syncer = new ItemSync(col); - syncer->setTransactionMode(ItemSync::SingleTransaction); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); syncer->setIncrementalSyncItems(Item::List(), Item::List()); AKVERIFYEXEC(syncer); - //It would be better if we didn't have a transaction at all, but so far the transaction is still created - QCOMPARE(transactionSpy.count(), 1); Item::List resultItems = fetchItems(col); QCOMPARE(resultItems.count(), origItems.count()); @@ -385,12 +349,9 @@ QVERIFY(changedSpy.isValid()); ItemSync *syncer = new ItemSync(col); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); QSignalSpy spy(syncer, SIGNAL(result(KJob*))); QVERIFY(spy.isValid()); syncer->setStreamingEnabled(true); - syncer->setTransactionMode(ItemSync::MultipleTransactions); QTest::qWait(0); QCOMPARE(spy.count(), 0); @@ -407,7 +368,6 @@ QTest::qWait(100); //this should process one batch of batchSize() items QTRY_COMPARE(changedSpy.count(), syncer->batchSize()); - QCOMPARE(transactionSpy.count(), 1); //one per batch for (int i = syncer->batchSize(); i < origItems.count(); ++i) { Item::List l; @@ -422,7 +382,6 @@ syncer->deliveryDone(); QTRY_COMPARE(spy.count(), 1); - QCOMPARE(transactionSpy.count(), 2); //one per batch QTest::qWait(100); Item::List resultItems = fetchItems(col); @@ -459,7 +418,6 @@ modifiedItem.setPayload("payload2"); ItemSync *syncer = new ItemSync(col); - syncer->setTransactionMode(ItemSync::MultipleTransactions); syncer->setIncrementalSyncItems(Item::List() << modifiedItem, Item::List()); AKVERIFYEXEC(syncer); @@ -489,12 +447,9 @@ Item::List origItems = fetchItems(col); ItemSync *syncer = new ItemSync(col); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); QSignalSpy spy(syncer, SIGNAL(result(KJob*))); QVERIFY(spy.isValid()); syncer->setStreamingEnabled(true); - syncer->setTransactionMode(ItemSync::MultipleTransactions); QTest::qWait(0); QCOMPARE(spy.count(), 0); @@ -551,12 +506,9 @@ origItems = fetchItems(col); ItemSync *syncer = new ItemSync(col); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); QSignalSpy spy(syncer, SIGNAL(result(KJob*))); QVERIFY(spy.isValid()); syncer->setStreamingEnabled(true); - syncer->setTransactionMode(ItemSync::MultipleTransactions); QTest::qWait(0); QCOMPARE(spy.count(), 0); @@ -615,13 +567,9 @@ QBENCHMARK { ItemSync *syncer = new ItemSync(col); - syncer->setTransactionMode(ItemSync::SingleTransaction); - QSignalSpy transactionSpy(syncer, SIGNAL(transactionCommitted())); - QVERIFY(transactionSpy.isValid()); syncer->setFullSyncItems(origItems); AKVERIFYEXEC(syncer); - QCOMPARE(transactionSpy.count(), 1); } const Item::List resultItems = fetchItems(col); @@ -655,11 +603,10 @@ // and an ItemSync running ItemSync *syncer = new ItemSync(col); - syncer->setTransactionMode(ItemSync::SingleTransaction); syncer->setFullSyncItems(origItems); // When the user cancels the ItemSync - QTimer::singleShot(10, syncer, &ItemSync::rollback); + QTimer::singleShot(10, syncer, [syncer]() { syncer->kill(KJob::EmitResult); }); // Then the itemsync should finish at some point, and not crash QVERIFY(!syncer->exec()); diff --git a/autotests/shared/CMakeLists.txt b/autotests/shared/CMakeLists.txt --- a/autotests/shared/CMakeLists.txt +++ b/autotests/shared/CMakeLists.txt @@ -20,3 +20,4 @@ endmacro() add_unit_test(akrangestest.cpp) +add_unit_test(akscopeguardtest.cpp) diff --git a/src/server/storage/transaction.cpp b/autotests/shared/akscopeguardtest.cpp copy from src/server/storage/transaction.cpp copy to autotests/shared/akscopeguardtest.cpp --- a/src/server/storage/transaction.cpp +++ b/autotests/shared/akscopeguardtest.cpp @@ -1,5 +1,5 @@ /* - Copyright (c) 2006 Volker Krause + Copyright (c) 2018 Daniel Vrátil This library is free software; you can redistribute it and/or modify it under the terms of the GNU Library General Public License as published by @@ -17,35 +17,27 @@ 02110-1301, USA. */ -#include "transaction.h" -#include "storage/datastore.h" +#include "shared/akscopeguard.h" -using namespace Akonadi::Server; +#include +#include -Transaction::Transaction(DataStore *db, const QString &name, bool beginTransaction) - : mDb(db) - , mName(name) - , mCommitted(false) +class AkScopeGuardTest : public QObject { - if (beginTransaction) { - mDb->beginTransaction(mName); + Q_OBJECT +private Q_SLOTS: + void testGoingOutOfScope() + { + bool callbackCalled = false; + { + AkScopeGuard guard([&callbackCalled]() { + callbackCalled = true; + }); + } + QVERIFY(callbackCalled); } -} +}; -Transaction::~Transaction() -{ - if (!mCommitted) { - mDb->rollbackTransaction(); - } -} +QTEST_GUILESS_MAIN(AkScopeGuardTest) -bool Transaction::commit() -{ - mCommitted = true; - return mDb->commitTransaction(); -} - -void Transaction::begin() -{ - mDb->beginTransaction(mName); -} +#include "akscopeguardtest.moc" diff --git a/src/agentbase/resourcebase.h b/src/agentbase/resourcebase.h --- a/src/agentbase/resourcebase.h +++ b/src/agentbase/resourcebase.h @@ -574,15 +574,7 @@ * @see setTotalItems(int) */ void setItemStreamingEnabled(bool enable); - - /** - * Set transaction mode for item sync'ing. - * @param mode item transaction mode - * @see Akonadi::ItemSync::TransactionMode - * @since 4.6 - */ - void setItemTransactionMode(ItemSync::TransactionMode mode); - + /** * Set merge mode for item sync'ing. * diff --git a/src/agentbase/resourcebase.cpp b/src/agentbase/resourcebase.cpp --- a/src/agentbase/resourcebase.cpp +++ b/src/agentbase/resourcebase.cpp @@ -72,7 +72,6 @@ , scheduler(nullptr) , mItemSyncer(nullptr) , mItemSyncFetchScope(nullptr) - , mItemTransactionMode(ItemSync::SingleTransaction) , mItemMergeMode(ItemSync::RIDMerge) , mCollectionSyncer(nullptr) , mTagSyncer(nullptr) @@ -188,7 +187,6 @@ "createItemSyncInstance", "Calling items retrieval methods although no item retrieval is in progress"); if (!mItemSyncer) { mItemSyncer = new ItemSync(q->currentCollection()); - mItemSyncer->setTransactionMode(mItemTransactionMode); mItemSyncer->setBatchSize(mItemSyncBatchSize); mItemSyncer->setMergeMode(mItemMergeMode); if (mItemSyncFetchScope) { @@ -457,7 +455,6 @@ ResourceScheduler *scheduler = nullptr; ItemSync *mItemSyncer = nullptr; ItemFetchScope *mItemSyncFetchScope = nullptr; - ItemSync::TransactionMode mItemTransactionMode; ItemSync::MergeMode mItemMergeMode; CollectionSync *mCollectionSyncer = nullptr; TagSync *mTagSyncer = nullptr; @@ -1260,7 +1257,7 @@ break; case ResourceScheduler::SyncCollection: if (d->mItemSyncer) { - d->mItemSyncer->rollback(); + d->mItemSyncer->kill(); } else { d->scheduler->taskDone(); } @@ -1536,12 +1533,6 @@ { } -void ResourceBase::setItemTransactionMode(ItemSync::TransactionMode mode) -{ - Q_D(ResourceBase); - d->mItemTransactionMode = mode; -} - void ResourceBase::setItemSynchronizationFetchScope(const ItemFetchScope &fetchScope) { Q_D(ResourceBase); diff --git a/src/core/itemsync.h b/src/core/itemsync.h --- a/src/core/itemsync.h +++ b/src/core/itemsync.h @@ -154,32 +154,6 @@ */ ItemFetchScope &fetchScope(); - /** - * Aborts the sync process and rolls back all not yet committed transactions. - * Use this if an external error occurred during the sync process (such as the - * user canceling it). - * @since 4.5 - */ - void rollback(); - - /** - * Transaction mode used by ItemSync. - * @since 4.6 - */ - enum TransactionMode { - SingleTransaction, ///< Use a single transaction for the entire sync process (default), provides maximum consistency ("all or nothing") and best performance - MultipleTransactions, ///< Use one transaction per chunk of delivered items, good compromise between the other two when using streaming - NoTransaction ///< Use no transaction at all, provides highest responsiveness (might therefore feel faster even when actually taking slightly longer), no consistency guaranteed (can fail anywhere in the sync process) - }; - - /** - * Set the transaction mode to use for this sync. - * @note You must call this method before starting the sync, changes afterwards lead to undefined results. - * @param mode the transaction mode to use - * @since 4.6 - */ - void setTransactionMode(TransactionMode mode); - /** * Minimum number of items required to start processing in streaming mode. * When MultipleTransactions is used, one transaction per batch will be created. @@ -244,24 +218,16 @@ */ void readyForNextBatch(int remainingBatchSize); - /** - * @internal - * Emitted whenever a transaction is committed. This is for testing only. - * - * @since 4.14 - */ - void transactionCommitted(); - protected: void doStart() override; + bool doKill() override; void slotResult(KJob *job) override; private: //@cond PRIVATE Q_DECLARE_PRIVATE(ItemSync) Q_PRIVATE_SLOT(d_func(), void slotLocalListDone(KJob *)) - Q_PRIVATE_SLOT(d_func(), void slotTransactionResult(KJob *)) Q_PRIVATE_SLOT(d_func(), void slotItemsReceived(const Akonadi::Item::List &)) //@endcond }; diff --git a/src/core/itemsync.cpp b/src/core/itemsync.cpp --- a/src/core/itemsync.cpp +++ b/src/core/itemsync.cpp @@ -29,12 +29,11 @@ #include "itemdeletejob.h" #include "itemfetchjob.h" #include "itemmodifyjob.h" -#include "transactionsequence.h" #include "itemfetchscope.h" - #include "akonadicore_debug.h" +#include using namespace Akonadi; @@ -46,9 +45,6 @@ public: ItemSyncPrivate(ItemSync *parent) : JobPrivate(parent) - , mTransactionMode(ItemSync::SingleTransaction) - , mCurrentTransaction(nullptr) - , mTransactionJobs(0) , mPendingJobs(0) , mProgress(0) , mTotalItems(-1) @@ -73,26 +69,18 @@ void slotItemsReceived(const Item::List &items); void slotLocalListDone(KJob *job); void slotLocalDeleteDone(KJob *job); - void slotLocalChangeDone(KJob *job); void execute(); void processItems(); void processBatch(); void deleteItems(const Item::List &items); - void slotTransactionResult(KJob *job); - void requestTransaction(); - Job *subjobParent() const; void fetchLocalItemsToDelete(); QString jobDebuggingString() const override; bool allProcessed() const; Q_DECLARE_PUBLIC(ItemSync) Collection mSyncCollection; QSet mListedItems; - ItemSync::TransactionMode mTransactionMode; - TransactionSequence *mCurrentTransaction; - int mTransactionJobs; - // fetch scope for initial item listing ItemFetchScope mFetchScope; @@ -128,20 +116,31 @@ return; } mPendingJobs++; - ItemCreateJob *create = new ItemCreateJob(item, mSyncCollection, subjobParent()); + ItemCreateJob *create = new ItemCreateJob(item, mSyncCollection, q); ItemCreateJob::MergeOptions merge = ItemCreateJob::Silent; if (mMergeMode == ItemSync::GIDMerge && !item.gid().isEmpty()) { merge |= ItemCreateJob::GID; } else { merge |= ItemCreateJob::RID; } create->setMerge(merge); - q->connect(create, &ItemCreateJob::result, q, [this](KJob *job) {slotLocalChangeDone(job);}); + q->connect(create, &ItemCreateJob::result, q, [this, item](KJob *job) { + if (job->error() && job->error() != Job::KilledJobError) { + qCWarning(AKONADICORE_LOG) << "Creating/updating item" << item.remoteId() << "in the akonadi database failed:" << job->errorString(); + } + mPendingJobs--; + mProgress++; + + checkDone(); + }); } bool ItemSyncPrivate::allProcessed() const { - return mDeliveryDone && mCurrentBatchRemoteItems.isEmpty() && mRemoteItemQueue.isEmpty() && mRemovedRemoteItemQueue.isEmpty() && mCurrentBatchRemovedRemoteItems.isEmpty(); + Q_Q(const ItemSync); + return mDeliveryDone && !q->hasSubjobs() + && mCurrentBatchRemoteItems.isEmpty() && mRemoteItemQueue.isEmpty() + && mRemovedRemoteItemQueue.isEmpty() && mCurrentBatchRemovedRemoteItems.isEmpty(); } void ItemSyncPrivate::checkDone() @@ -151,22 +150,9 @@ if (mPendingJobs > 0) { return; } - - if (mTransactionJobs > 0) { - //Commit the current transaction if we're in batch processing mode or done - //and wait until the transaction is committed to process the next batch - if (mTransactionMode == ItemSync::MultipleTransactions || (mDeliveryDone && mRemoteItemQueue.isEmpty())) { - if (mCurrentTransaction) { - Q_EMIT q->transactionCommitted(); - mCurrentTransaction->commit(); - mCurrentTransaction = nullptr; - } - return; - } - } mProcessingBatch = false; - if (q->error() == Job::UserCanceled && mTransactionJobs == 0 && !mFinished) { + if (q->error() == Job::UserCanceled && !mFinished) { qCDebug(AKONADICORE_LOG) << "ItemSync of collection" << mSyncCollection.id() << "finished due to user cancelling"; mFinished = true; q->emitResult(); @@ -213,16 +199,20 @@ * * delete all superfluous items */ Q_D(ItemSync); + qCDebug(AKONADICORE_LOG) << "ItemSync for collection" << d->mSyncCollection.id() + << "received a non-incremental batch of" << items.size() + << "items. Already processed" << d->mTotalItemsProcessed + << "and expecting in total" << d->mTotalItems << "items"; + Q_ASSERT(!d->mIncremental); if (!d->mStreaming) { d->mDeliveryDone = true; } d->mRemoteItemQueue += items; d->mTotalItemsProcessed += items.count(); - qCDebug(AKONADICORE_LOG) << "Received batch: " << items.count() - << "Already processed: " << d->mTotalItemsProcessed - << "Expected total amount: " << d->mTotalItems; + if (!d->mDisableAutomaticDeliveryDone && (d->mTotalItemsProcessed == d->mTotalItems)) { + qCDebug(AKONADICORE_LOG) << "ItemSync for collection" << d->mSyncCollection.id() << "now has all items to sync."; d->mDeliveryDone = true; } d->execute(); @@ -259,15 +249,21 @@ * * removed items can be removed right away */ Q_D(ItemSync); + qCDebug(AKONADICORE_LOG) << "ItemSync for Collection ID" << d->mSyncCollection.id() + << "received incremental batch of" << changedItems.size() << "changed items and" + << removedItems.size() << " removed items. Already processed" + << d->mTotalItemsProcessed << "and expecting in total:" << d->mTotalItems << "items"; + d->mIncremental = true; if (!d->mStreaming) { d->mDeliveryDone = true; } d->mRemoteItemQueue += changedItems; d->mRemovedRemoteItemQueue += removedItems; d->mTotalItemsProcessed += changedItems.count() + removedItems.count(); - qCDebug(AKONADICORE_LOG) << "Received: " << changedItems.count() << "Removed: " << removedItems.count() << "In total: " << d->mTotalItemsProcessed << " Wanted: " << d->mTotalItems; - if (!d->mDisableAutomaticDeliveryDone && (d->mTotalItemsProcessed == d->mTotalItems)) { + if (!d->mDisableAutomaticDeliveryDone && (d->mTotalItemsProcessed == d->mTotalItems)) { + qDebug(AKONADICORE_LOG) << "ItemSync for Collection ID" << d->mSyncCollection.id() + << "now has all items to sync."; d->mDeliveryDone = true; } d->execute(); @@ -296,7 +292,7 @@ qFatal("This must not be called while in incremental mode"); return; } - ItemFetchJob *job = new ItemFetchJob(mSyncCollection, subjobParent()); + ItemFetchJob *job = new ItemFetchJob(mSyncCollection, q); job->fetchScope().setFetchRemoteIdentification(true); job->fetchScope().setFetchModificationTime(false); job->setDeliveryOption(ItemFetchJob::EmitItemsIndividually); @@ -380,9 +376,6 @@ return; } - //request a transaction, there are items that require processing - requestTransaction(); - processItems(); // removed @@ -426,17 +419,8 @@ } mPendingJobs++; - ItemDeleteJob *job = new ItemDeleteJob(itemsToDelete, subjobParent()); + ItemDeleteJob *job = new ItemDeleteJob(itemsToDelete, q); q->connect(job, &ItemDeleteJob::result, q, [this](KJob *job) { slotLocalDeleteDone(job); }); - - // It can happen that the groupware servers report us deleted items - // twice, in this case this item delete job will fail on the second try. - // To avoid a rollback of the complete transaction we gracefully allow the job - // to fail :) - TransactionSequence *transaction = qobject_cast(subjobParent()); - if (transaction) { - transaction->setIgnoreJobFailure(job); - } } void ItemSyncPrivate::slotLocalDeleteDone(KJob *job) @@ -450,48 +434,6 @@ checkDone(); } -void ItemSyncPrivate::slotLocalChangeDone(KJob *job) -{ - if (job->error() && job->error() != Job::KilledJobError) { - qCWarning(AKONADICORE_LOG) << "Creating/updating items from the akonadi database failed:" << job->errorString(); - } - mPendingJobs--; - mProgress++; - - checkDone(); -} - -void ItemSyncPrivate::slotTransactionResult(KJob *job) -{ - --mTransactionJobs; - if (mCurrentTransaction == job) { - mCurrentTransaction = nullptr; - } - - checkDone(); -} - -void ItemSyncPrivate::requestTransaction() -{ - Q_Q(ItemSync); - //we never want parallel transactions, single transaction just makes one big transaction, and multi transaction uses multiple transaction sequentially - if (!mCurrentTransaction) { - ++mTransactionJobs; - mCurrentTransaction = new TransactionSequence(q); - mCurrentTransaction->setAutomaticCommittingEnabled(false); - QObject::connect(mCurrentTransaction, &TransactionSequence::result, q, [this](KJob *job) { slotTransactionResult(job); }); - } -} - -Job *ItemSyncPrivate::subjobParent() const -{ - Q_Q(const ItemSync); - if (mCurrentTransaction && mTransactionMode != ItemSync::NoTransaction) { - return mCurrentTransaction; - } - return const_cast(q); -} - void ItemSync::setStreamingEnabled(bool enable) { Q_D(ItemSync); @@ -522,24 +464,24 @@ } } -void ItemSync::rollback() +bool ItemSync::doKill() { Q_D(ItemSync); - qCDebug(AKONADICORE_LOG) << "The item sync is being rolled-back."; - setError(UserCanceled); - if (d->mCurrentTransaction) { - d->mCurrentTransaction->rollback(); + + qCDebug(AKONADICORE_LOG) << "ItemSync for collection" << d->mSyncCollection.id() << "cancelled."; + + for (auto *subjob : subjobs()) { + subjob->kill(KJob::Quietly); + removeSubjob(subjob); } - d->mDeliveryDone = true; // user won't deliver more data - d->execute(); // end this in an ordered way, since we have an error set no real change will be done -} + d->mDeliveryDone = true; + setError(UserCanceled); + setErrorText(i18n("User canceled operation.")); -void ItemSync::setTransactionMode(ItemSync::TransactionMode mode) -{ - Q_D(ItemSync); - d->mTransactionMode = mode; + return true; } + int ItemSync::batchSize() const { Q_D(const ItemSync); diff --git a/src/core/jobs/job.cpp b/src/core/jobs/job.cpp --- a/src/core/jobs/job.cpp +++ b/src/core/jobs/job.cpp @@ -355,7 +355,11 @@ break; } if (!errorText().isEmpty()) { - str += QStringLiteral(" (%1)").arg(errorText()); + if (str.isEmpty()) { + str = errorText(); + } else { + str += QStringLiteral(" (%1)").arg(errorText()); + } } return str; } diff --git a/src/server/handler/collectionmodifyhandler.cpp b/src/server/handler/collectionmodifyhandler.cpp --- a/src/server/handler/collectionmodifyhandler.cpp +++ b/src/server/handler/collectionmodifyhandler.cpp @@ -32,6 +32,7 @@ #include "storage/collectionqueryhelper.h" #include "search/searchmanager.h" #include "akonadiserver_debug.h" +#include "shared/akscopeguard.h" using namespace Akonadi; using namespace Akonadi::Server; @@ -237,6 +238,16 @@ } } + AkScopeGuard collectionReferencingGuard([&]() noexcept { + // If transaction is not committed and we are going out of scope, then a rollback + // occurred and we need to undo the collection reference change, if any. + if (!transaction.isCommitted()) { + if (referencedChanged && changes.contains(AKONADI_PARAM_REFERENCED)) { + connection()->collectionReferenceManager()->referenceCollection(connection()->sessionId(), collection, false); + } + } + }); + if (cmd.modifiedParts() & Protocol::ModifyCollectionCommand::RemovedAttributes) { Q_FOREACH (const QByteArray &attr, cmd.removedAttributes()) { if (db->removeCollectionAttribute(collection, attr)) { diff --git a/src/server/storage/collectionstatistics.cpp b/src/server/storage/collectionstatistics.cpp --- a/src/server/storage/collectionstatistics.cpp +++ b/src/server/storage/collectionstatistics.cpp @@ -212,7 +212,7 @@ #undef FLAGS_COLUMN - return qb; + return std::move(qb); } CollectionStatistics::Statistics CollectionStatistics::calculateCollectionStatistics(const Collection &col) diff --git a/src/server/storage/transaction.h b/src/server/storage/transaction.h --- a/src/server/storage/transaction.h +++ b/src/server/storage/transaction.h @@ -55,6 +55,8 @@ */ ~Transaction(); + bool isCommitted() const; + /** Commits the transaction. Returns true on success. If a global transaction is used, nothing happens, global transactions have diff --git a/src/server/storage/transaction.cpp b/src/server/storage/transaction.cpp --- a/src/server/storage/transaction.cpp +++ b/src/server/storage/transaction.cpp @@ -39,6 +39,11 @@ } } +bool Transaction::isCommitted() const +{ + return mCommitted; +} + bool Transaction::commit() { mCommitted = true; diff --git a/src/server/storage/transaction.cpp b/src/shared/akscopeguard.h copy from src/server/storage/transaction.cpp copy to src/shared/akscopeguard.h --- a/src/server/storage/transaction.cpp +++ b/src/shared/akscopeguard.h @@ -1,5 +1,5 @@ /* - Copyright (c) 2006 Volker Krause + Copyright (C) 2019 Daniel Vrátil This library is free software; you can redistribute it and/or modify it under the terms of the GNU Library General Public License as published by @@ -17,35 +17,30 @@ 02110-1301, USA. */ -#include "transaction.h" -#include "storage/datastore.h" +#ifndef AKSCOPEGUARD_H_ +#define AKSCOPEGUARD_H_ -using namespace Akonadi::Server; +#include +#include -Transaction::Transaction(DataStore *db, const QString &name, bool beginTransaction) - : mDb(db) - , mName(name) - , mCommitted(false) +class AkScopeGuard { - if (beginTransaction) { - mDb->beginTransaction(mName); +public: + AkScopeGuard(std::function &&func) + : mFunc(func) + {} + + ~AkScopeGuard() noexcept + { + mFunc(); } -} -Transaction::~Transaction() -{ - if (!mCommitted) { - mDb->rollbackTransaction(); - } -} +private: + Q_DISABLE_COPY(AkScopeGuard); + AkScopeGuard(AkScopeGuard &&) = delete; + AkScopeGuard &operator=(AkScopeGuard &&) = delete; -bool Transaction::commit() -{ - mCommitted = true; - return mDb->commitTransaction(); -} + std::function mFunc; +}; -void Transaction::begin() -{ - mDb->beginTransaction(mName); -} +#endif