Discovery phase: refactor some code in DiscoveryPhase::findAndCancelDeletedJob

Less code duplication

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 99c86d5dd801537adf660ec04fca027cec94123e..602f960820e4b844bc1255f5094c210faabf42df 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -533,18 +533,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
                     }
                 }
 
-                bool wasDeletedOnServer = false;
-                auto it = _discoveryData->_deletedItem.find(originalPath);
-                if (it != _discoveryData->_deletedItem.end()) {
-                    ASSERT((*it)->_instruction == CSYNC_INSTRUCTION_REMOVE);
-                    (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
-                    wasDeletedOnServer = true;
-                }
-                auto otherJob = _discoveryData->_queuedDeletedDirectories.take(originalPath);
-                if (otherJob) {
-                    delete otherJob;
-                    wasDeletedOnServer = true;
-                }
+                bool wasDeletedOnServer = _discoveryData->findAndCancelDeletedJob(originalPath).first;
 
                 auto postProcessRename = [this, item, base, originalPath](PathTuple &path) {
                     auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath);
@@ -582,12 +571,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
                         // The file do not exist, it is a rename
 
                         // In case the deleted item was discovered in parallel
-                        auto it = _discoveryData->_deletedItem.find(originalPath);
-                        if (it != _discoveryData->_deletedItem.end()) {
-                            ASSERT((*it)->_instruction == CSYNC_INSTRUCTION_REMOVE);
-                            (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
-                        }
-                        delete _discoveryData->_queuedDeletedDirectories.take(originalPath);
+                        _discoveryData->findAndCancelDeletedJob(originalPath);
 
                         postProcessRename(path);
                         processFileFinalize(item, path, item->isDirectory(), item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _queryServer);
@@ -877,22 +861,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
             }
 
             if (isMove) {
-                QByteArray oldEtag;
-                auto it = _discoveryData->_deletedItem.find(originalPath);
-                if (it != _discoveryData->_deletedItem.end()) {
-                    if ((*it)->_instruction != CSYNC_INSTRUCTION_REMOVE && (*it)->_type != ItemTypeVirtualFile)
-                        isMove = false;
-                    else
-                        (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
-                    oldEtag = (*it)->_etag;
-                    if (!item->isDirectory() && oldEtag != base._etag) {
-                        isMove = false;
-                    }
-                }
-                if (auto deleteJob = _discoveryData->_queuedDeletedDirectories.value(originalPath)) {
-                    oldEtag = deleteJob->_dirItem->_etag;
-                    delete _discoveryData->_queuedDeletedDirectories.take(originalPath);
-                }
+                auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath);
 
                 auto processRename = [item, originalPath, base, this](PathTuple &path) {
                     auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath);
@@ -912,10 +881,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
                     path._server = adjustedOriginalPath;
                     qCInfo(lcDisco) << "Rename detected (up) " << item->_file << " -> " << item->_renameTarget;
                 };
-                if (isMove && !oldEtag.isEmpty()) {
-                    recurseQueryServer = oldEtag == base._etag ? ParentNotChanged : NormalQuery;
+                if (wasDeletedOnClient.first) {
+                    recurseQueryServer = wasDeletedOnClient.second == base._etag ? ParentNotChanged : NormalQuery;
                     processRename(path);
-                } else if (isMove) {
+                } else {
                     // We must query the server to know if the etag has not changed
                     _pendingAsyncJobs++;
                     QString serverOriginalPath = originalPath;
@@ -929,13 +898,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
                             postProcessLocalNew();
                         } else {
                             // In case the deleted item was discovered in parallel
-                            auto it = _discoveryData->_deletedItem.find(originalPath);
-                            if (it != _discoveryData->_deletedItem.end()) {
-                                ASSERT((*it)->_instruction == CSYNC_INSTRUCTION_REMOVE);
-                                (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
-                            }
-                            delete _discoveryData->_queuedDeletedDirectories.take(originalPath);
-
+                            _discoveryData->findAndCancelDeletedJob(originalPath);
                             processRename(path);
                             recurseQueryServer = *etag == base._etag ? ParentNotChanged : NormalQuery;
                         }
@@ -945,8 +908,6 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
                     });
                     job->start();
                     return;
-                } else {
-                    postProcessLocalNew();
                 }
             } else {
                 postProcessLocalNew();

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index d2ff2acafd7793003b287c663c6e2d5ecdb04a40..17ec8fee2a73334516c5f06a08f42f282f1bd618 100644 (file)
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -145,6 +145,27 @@ QString DiscoveryPhase::adjustRenamedPath(const QString &original) const
     return original;
 }
 
+QPair<bool, QByteArray> DiscoveryPhase::findAndCancelDeletedJob(const QString &originalPath)
+{
+    bool result = false;
+    QByteArray oldEtag;
+    auto it = _deletedItem.find(originalPath);
+    if (it != _deletedItem.end()) {
+        ENFORCE((*it)->_instruction == CSYNC_INSTRUCTION_REMOVE
+            // re-creation of virtual files count as a delete
+             || ((*it)->_type == ItemTypeVirtualFile && (*it)->_instruction == CSYNC_INSTRUCTION_NEW));
+        (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
+        result = true;
+        oldEtag = (*it)->_etag;
+    }
+    if (auto *otherJob = _queuedDeletedDirectories.take(originalPath)) {
+        oldEtag = otherJob->_dirItem->_etag;
+        delete otherJob;
+        result = true;
+    }
+    return { result, oldEtag };
+}
+
 void DiscoveryPhase::startJob(ProcessDirectoryJob *job)
 {
     ENFORCE(!_currentRootJob);

diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
index 06e7dd299cfdcc0f4e2a46d449b2190550043e6e..72fce7e3cdc14688d0c383607c98ac8825053fdc 100644 (file)
--- a/src/libsync/discoveryphase.h
+++ b/src/libsync/discoveryphase.h
@@ -162,6 +162,14 @@ public:
      */
     QString adjustRenamedPath(const QString &original) const;
 
+    /**
+     * Check if there is already a job to delete that item.
+     * If that's not the case, return { false, QByteArray() }.
+     * If there is such a job, cancel that job and return true and the old etag
+     * This is useful to detect if a file has been renamed to something else.
+     */
+    QPair<bool, QByteArray> findAndCancelDeletedJob(const QString &originalPath);
+
     void startJob(ProcessDirectoryJob *);
 
     QByteArray _dataFingerprint;