From a36ed56f01b3bf3e54f0eaf7308bc1e2f4e86c85 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 6 Aug 2018 12:41:59 +0200 Subject: [PATCH] New Discovery algorithm: Refactor a bit the way the signal are emited --- src/libsync/discovery.cpp | 25 ++++++++----------------- src/libsync/discovery.h | 2 -- src/libsync/discoveryphase.cpp | 17 +++++++++++++++++ src/libsync/discoveryphase.h | 8 ++++++-- src/libsync/syncengine.cpp | 31 ++++++++----------------------- src/libsync/syncengine.h | 1 - 6 files changed, 39 insertions(+), 45 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 13caaa977..4d877485a 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -139,7 +139,7 @@ void ProcessDirectoryJob::start() item->_instruction = CSYNC_INSTRUCTION_IGNORE; item->_status = SyncFileItem::NormalError; item->_errorString = tr("Filename encoding is not valid"); - emit itemDiscovered(item); + emit _discoveryData->itemDiscovered(item); continue; } i.modtime = dirent->modtime; @@ -351,7 +351,7 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, bool isDirectory, } _childIgnored = true; - emit itemDiscovered(item); + emit _discoveryData->itemDiscovered(item); return true; } @@ -558,11 +558,10 @@ void ProcessDirectoryJob::processFile(PathTuple path, item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _discoveryData, this); job->_currentFolder = path; - connect(job, &ProcessDirectoryJob::itemDiscovered, this, &ProcessDirectoryJob::itemDiscovered); connect(job, &ProcessDirectoryJob::finished, this, &ProcessDirectoryJob::subJobFinished); _queuedJobs.push_back(job); } else { - emit itemDiscovered(item); + emit _discoveryData->itemDiscovered(item); } _pendingAsyncJobs--; progress(); @@ -815,7 +814,7 @@ void ProcessDirectoryJob::processFile(PathTuple path, isMove = false; } } - if (auto deleteJob = static_cast(_discoveryData->_queuedDeletedDirectories.value(originalPath).data())) { + if (auto deleteJob = _discoveryData->_queuedDeletedDirectories.value(originalPath)) { oldEtag = deleteJob->_dirItem->_etag; delete _discoveryData->_queuedDeletedDirectories.take(originalPath); } @@ -866,11 +865,10 @@ void ProcessDirectoryJob::processFile(PathTuple path, if (recurse && item->isDirectory()) { auto job = new ProcessDirectoryJob(item, recurseQueryServer, NormalQuery, _discoveryData, this); job->_currentFolder = path; - connect(job, &ProcessDirectoryJob::itemDiscovered, this, &ProcessDirectoryJob::itemDiscovered); connect(job, &ProcessDirectoryJob::finished, this, &ProcessDirectoryJob::subJobFinished); _queuedJobs.push_back(job); } else { - emit itemDiscovered(item); + emit _discoveryData->itemDiscovered(item); } _pendingAsyncJobs--; progress(); @@ -964,7 +962,6 @@ void ProcessDirectoryJob::processFile(PathTuple path, job->setParent(_discoveryData); _discoveryData->_queuedDeletedDirectories[path._original] = job; } else { - connect(job, &ProcessDirectoryJob::itemDiscovered, this, &ProcessDirectoryJob::itemDiscovered); connect(job, &ProcessDirectoryJob::finished, this, &ProcessDirectoryJob::subJobFinished); _queuedJobs.push_back(job); } @@ -972,7 +969,7 @@ void ProcessDirectoryJob::processFile(PathTuple path, if (item->_instruction == CSYNC_INSTRUCTION_REMOVE) { _discoveryData->_deletedItem[path._original] = item; } - emit itemDiscovered(item); + emit _discoveryData->itemDiscovered(item); } } @@ -1000,11 +997,10 @@ void ProcessDirectoryJob::processBlacklisted(const PathTuple &path, const OCC::L if (item->isDirectory() && item->_instruction != CSYNC_INSTRUCTION_IGNORE) { auto job = new ProcessDirectoryJob(item, InBlackList, NormalQuery, _discoveryData, this); job->_currentFolder = path; - connect(job, &ProcessDirectoryJob::itemDiscovered, this, &ProcessDirectoryJob::itemDiscovered); connect(job, &ProcessDirectoryJob::finished, this, &ProcessDirectoryJob::subJobFinished); _queuedJobs.push_back(job); } else { - emit itemDiscovered(item); + emit _discoveryData->itemDiscovered(item); } } @@ -1089,7 +1085,7 @@ void ProcessDirectoryJob::subJobFinished() _childModified |= job->_childModified; if (job->_dirItem) - emit itemDiscovered(job->_dirItem); + emit _discoveryData->itemDiscovered(job->_dirItem); int count = _runningJobs.removeAll(job); ASSERT(count == 1); @@ -1131,11 +1127,6 @@ void ProcessDirectoryJob::progress() emit finished(); } } -void ProcessDirectoryJob::abort() -{ - // This should delete all the sub jobs, and so abort everything - deleteLater(); -} void ProcessDirectoryJob::dbError() { diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index e12085e3a..67a0b8067 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -46,7 +46,6 @@ public: { } void start(); - void abort(); SyncFileItemPtr _dirItem; @@ -102,7 +101,6 @@ private: bool _childIgnored = false; // The directory contains ignored item that would prevent deletion signals: - void itemDiscovered(const SyncFileItemPtr &item); void finished(); }; } diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 6d58829ff..e4cd5b9a5 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -13,6 +13,7 @@ */ #include "discoveryphase.h" +#include "discovery.h" #include "account.h" #include "common/asserts.h" @@ -145,6 +146,22 @@ QString DiscoveryPhase::adjustRenamedPath(const QString &original) const return original; } +void DiscoveryPhase::startJob(ProcessDirectoryJob *job) +{ + connect(job, &ProcessDirectoryJob::finished, this, [this, job] { + if (job->_dirItem) + emit itemDiscovered(job->_dirItem); + job->deleteLater(); + if (!_queuedDeletedDirectories.isEmpty()) { + auto nextJob = _queuedDeletedDirectories.take(_queuedDeletedDirectories.firstKey()); + startJob(nextJob); + } else { + emit finished(); + } + }); + job->start(); +} + /* FIXME (used to be called every time we were doing a propfind) void DiscoveryJob::update_job_update_callback(bool local, const char *dirUrl, diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index ee6bb9485..4c1217a62 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -39,6 +39,7 @@ enum class LocalDiscoveryStyle { class Account; class SyncJournalDb; +class ProcessDirectoryJob; struct RemoteInfo @@ -140,13 +141,16 @@ public: bool checkSelectiveSyncNewFolder(const QString &path, RemotePermissions rp); QMap _deletedItem; - QMap> _queuedDeletedDirectories; + QMap _queuedDeletedDirectories; QMap _renamedItems; // map source -> destinations QString adjustRenamedPath(const QString &original) const; + void startJob(ProcessDirectoryJob *); + signals: void fatalError(const QString &errorString); - void folderDiscovered(bool local, QString folderUrl); + void itemDiscovered(const SyncFileItemPtr &item); + void finished(); // A new folder was discovered and was not synced because of the confirmation feature void newBigFolder(const QString &folder, bool isExternal); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 58383630a..2b4a36336 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -602,33 +602,17 @@ void SyncEngine::slotStartDiscovery() _discoveryPhase->_invalidFilenamePattern = invalidFilenamePattern; _discoveryPhase->_ignoreHiddenFiles = ignoreHiddenFiles(); - connect(_discoveryPhase.data(), &DiscoveryPhase::folderDiscovered, this, &SyncEngine::slotFolderDiscovered); + connect(_discoveryPhase.data(), &DiscoveryPhase::itemDiscovered, this, &SyncEngine::slotItemDiscovered); connect(_discoveryPhase.data(), &DiscoveryPhase::newBigFolder, this, &SyncEngine::newBigFolder); connect(_discoveryPhase.data(), &DiscoveryPhase::fatalError, this, [this](const QString &errorString) { syncError(errorString); finalize(false); }); + connect(_discoveryPhase.data(), &DiscoveryPhase::finished, this, &SyncEngine::slotDiscoveryJobFinished); - _discoveryJob = new ProcessDirectoryJob(SyncFileItemPtr(), ProcessDirectoryJob::NormalQuery, ProcessDirectoryJob::NormalQuery, - _discoveryPhase.data(), this); - // FIXME! this sucks - auto runQueuedJob = [this](ProcessDirectoryJob *job, const auto &runQueuedJob) -> void { - connect(job, &ProcessDirectoryJob::finished, this, [this, job, runQueuedJob] { - if (job->_dirItem) - job->itemDiscovered(job->_dirItem); - sender()->deleteLater(); - if (!_discoveryPhase->_queuedDeletedDirectories.isEmpty()) { - auto job = qobject_cast(_discoveryPhase->_queuedDeletedDirectories.take(_discoveryPhase->_queuedDeletedDirectories.firstKey()).data()); - ASSERT(job); - runQueuedJob(job, runQueuedJob); - } else { - slotDiscoveryJobFinished(); - } - }); - connect(job, &ProcessDirectoryJob::itemDiscovered, this, &SyncEngine::slotItemDiscovered); - job->start(); - }; - runQueuedJob(_discoveryJob.data(), runQueuedJob); + auto discoveryJob = new ProcessDirectoryJob(SyncFileItemPtr(), ProcessDirectoryJob::NormalQuery, ProcessDirectoryJob::NormalQuery, + _discoveryPhase.data(), _discoveryPhase.data()); + _discoveryPhase->startJob(discoveryJob); /* * FIXME @@ -997,8 +981,9 @@ void SyncEngine::abort() qCInfo(lcEngine) << "Aborting sync"; // Aborts the discovery phase job - if (_discoveryJob) { - _discoveryJob->abort(); + if (_discoveryPhase) { + // Should take care to delete all children jobs + _discoveryPhase.take()->deleteLater(); } // For the propagator if (_propagator) { diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 477efe62c..90f00a8aa 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -228,7 +228,6 @@ private: QString _remotePath; QString _remoteRootEtag; SyncJournalDb *_journal; - QPointer _discoveryJob; QScopedPointer _discoveryPhase; QSharedPointer _propagator; -- 2.30.2