New Discovery algorithm: Refactor a bit the way the signal are emited
authorOlivier Goffart <ogoffart@woboq.com>
Mon, 6 Aug 2018 10:41:59 +0000 (12:41 +0200)
committerKevin Ottens <kevin.ottens@nextcloud.com>
Tue, 15 Dec 2020 09:58:02 +0000 (10:58 +0100)
src/libsync/discovery.cpp
src/libsync/discovery.h
src/libsync/discoveryphase.cpp
src/libsync/discoveryphase.h
src/libsync/syncengine.cpp
src/libsync/syncengine.h

index 13caaa977bd78d717302a97c0afdc860c03680de..4d877485a694d74f976178d38cde90b7254bf446 100644 (file)
@@ -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<ProcessDirectoryJob *>(_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()
 {
index e12085e3ade949666c6f54ed41f4f724f2aa1fed..67a0b80672d9721246821d83c847c78f989c0682 100644 (file)
@@ -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();
 };
 }
index 6d58829ffb54805e2a9d2c65a0022f60aa924c95..e4cd5b9a5dd2a9018beab8df04bea584c8ad8c62 100644 (file)
@@ -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,
index ee6bb948595e6f4e2812ff7eb461fb0d3b66a1a7..4c1217a62d6a5038e6444457b643a6a506554dcf 100644 (file)
@@ -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<QString, SyncFileItemPtr> _deletedItem;
-    QMap<QString, QPointer<QObject>> _queuedDeletedDirectories;
+    QMap<QString, ProcessDirectoryJob *> _queuedDeletedDirectories;
     QMap<QString, QString> _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);
index 58383630ac1e4752b9164f1a8f534862e4f4dd34..2b4a3633643a4530f7fa906cd6bc49278d7b999b 100644 (file)
@@ -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<ProcessDirectoryJob *>(_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) {
index 477efe62c18ddf72251f6f439e3541af06008b0b..90f00a8aaa9f4417baa4080880d60ce8e599ef50 100644 (file)
@@ -228,7 +228,6 @@ private:
     QString _remotePath;
     QString _remoteRootEtag;
     SyncJournalDb *_journal;
-    QPointer<ProcessDirectoryJob> _discoveryJob;
     QScopedPointer<DiscoveryPhase> _discoveryPhase;
     QSharedPointer<OwncloudPropagator> _propagator;