Move: add more test and fix move within moves

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 9aadc430754fef0286f530479b3b959522c21b24..3c74e6a08c378b89871599a96169dd46a724abd6 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -282,7 +282,7 @@ void ProcessDirectoryJob::processFile(PathTuple path,
                               << " | fileid: " << dbEntry._fileId << "//" << serverEntry.fileId
                               << " | inode: " << dbEntry._inode << "/" << localEntry.inode << "/";
 
-    if (_discoveryData->_renamedItems.contains(path._original)) {
+    if (_discoveryData->isRenamed(path._original)) {
         qCDebug(lcDisco) << "Ignoring renamed";
         return; // Ignore this.
     }
@@ -489,7 +489,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
         // Now we know there is a sane rename candidate.
         QString originalPath = QString::fromUtf8(base._path);
 
-        if (_discoveryData->_renamedItems.contains(originalPath)) {
+        if (_discoveryData->isRenamed(originalPath)) {
             qCInfo(lcDisco, "folder already has a rename entry, skipping");
             return;
         }
@@ -530,8 +530,8 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
         bool wasDeletedOnServer = _discoveryData->findAndCancelDeletedJob(originalPath).first;
 
         auto postProcessRename = [this, item, base, originalPath](PathTuple &path) {
-            auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath);
-            _discoveryData->_renamedItems.insert(originalPath, path._target);
+            auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath, SyncFileItem::Up);
+            _discoveryData->_renamedItemsRemote.insert(originalPath, path._target);
             item->_modtime = base._modtime;
             item->_inode = base._inode;
             item->_instruction = CSYNC_INSTRUCTION_RENAME;
@@ -556,7 +556,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
                 QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs);
                 if (etag || etag.error().code != 404 ||
                     // Somehow another item claimed this original path, consider as if it existed
-                    _discoveryData->_renamedItems.contains(originalPath)) {
+                    _discoveryData->isRenamed(originalPath)) {
                     // If the file exist or if there is another error, consider it is a new file.
                     postProcessServerNew();
                     return;
@@ -818,7 +818,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
         }
     }
     auto originalPath = QString::fromUtf8(base._path);
-    if (isMove && _discoveryData->_renamedItems.contains(originalPath))
+    if (isMove && _discoveryData->isRenamed(originalPath))
         isMove = false;
 
     //Check local permission if we are allowed to put move the file here
@@ -833,8 +833,8 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
         auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath);
 
         auto processRename = [item, originalPath, base, this](PathTuple &path) {
-            auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath);
-            _discoveryData->_renamedItems.insert(originalPath, path._target);
+            auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath, SyncFileItem::Down);
+            _discoveryData->_renamedItemsLocal.insert(originalPath, path._target);
             item->_renameTarget = path._target;
             path._server = adjustedOriginalPath;
             item->_file = path._server;
@@ -861,7 +861,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
                 chopVirtualFileSuffix(serverOriginalPath);
             auto job = new RequestEtagJob(_discoveryData->_account, serverOriginalPath, this);
             connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult<QString> &etag) mutable {
-                if (!etag || (*etag != base._etag && !item->isDirectory()) || _discoveryData->_renamedItems.contains(originalPath)) {
+                if (!etag || (*etag != base._etag && !item->isDirectory()) || _discoveryData->isRenamed(originalPath)) {
                     qCInfo(lcDisco) << "Can't rename because the etag has changed or the directory is gone" << originalPath;
                     // Can't be a rename, leave it as a new.
                     postProcessLocalNew();

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index f8ea51d29891fe14ef1d43ffa6c879fd0cf35106..bb80a9a53c5c33be8604931686b88536f8a50ee4 100644 (file)
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -132,11 +132,10 @@ void DiscoveryPhase::checkSelectiveSyncNewFolder(const QString &path, RemotePerm
     propfindJob->start();
 }
 
-
 /* Given a path on the remote, give the path as it is when the rename is done */
-QString DiscoveryPhase::adjustRenamedPath(const QString &original) const
+QString DiscoveryPhase::adjustRenamedPath(const QString &original, SyncFileItem::Direction d) const
 {
-    return OCC::adjustRenamedPath(_renamedItems, original);
+    return OCC::adjustRenamedPath(d == SyncFileItem::Down ? _renamedItemsRemote : _renamedItemsLocal, original);
 }
 
 QString adjustRenamedPath(const QMap<QString, QString> renamedItems, const QString original)

diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
index cebc4181ea286d8e5ca789080f4e944e954b82df..8e90fbb75d8f43678bf9e11732924d41d936945c 100644 (file)
--- a/src/libsync/discoveryphase.h
+++ b/src/libsync/discoveryphase.h
@@ -132,7 +132,10 @@ class DiscoveryPhase : public QObject
     friend class ProcessDirectoryJob;
     QMap<QString, SyncFileItemPtr> _deletedItem;
     QMap<QString, ProcessDirectoryJob *> _queuedDeletedDirectories;
-    QMap<QString, QString> _renamedItems; // map source -> destinations
+    // map source -> destinations
+    QMap<QString, QString> _renamedItemsRemote;
+    QMap<QString, QString> _renamedItemsLocal;
+    bool isRenamed(const QString &p) { return _renamedItemsLocal.contains(p) || _renamedItemsRemote.contains(p); }
     int _currentlyActiveJobs = 0;
 
     // both must contain a sorted list
@@ -153,7 +156,7 @@ class DiscoveryPhase : public QObject
      * Note that it only considers parent directory renames. So if A/B got renamed to C/D,
      * checking A/B/file would yield C/D/file, but checking A/B would yield A/B.
      */
-    QString adjustRenamedPath(const QString &original) const;
+    QString adjustRenamedPath(const QString &original, SyncFileItem::Direction) const;
 
     /**
      * Check if there is already a job to delete that item.

diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp
index adfc742554d78d9c82941da0bbfd7a2a034a5402..dee9ac0b6eaafe14147856169756826ae7987bde 100644 (file)
--- a/test/testsyncmove.cpp
+++ b/test/testsyncmove.cpp
@@ -670,6 +670,48 @@ private slots:
         QCOMPARE(fakeFolder.currentLocalState(), expectedState);
         QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
     }
+
+    void testDeepHierarchy_data()
+    {
+        QTest::addColumn<bool>("local");
+        QTest::newRow("remote") << false;
+        QTest::newRow("local") << true;
+    }
+
+    void testDeepHierarchy()
+    {
+        QFETCH(bool, local);
+        FakeFolder fakeFolder { FileInfo::A12_B12_C12_S12() };
+        auto &modifier = local ? fakeFolder.localModifier() : fakeFolder.remoteModifier();
+
+        modifier.mkdir("FolA");
+        modifier.mkdir("FolA/FolB");
+        modifier.mkdir("FolA/FolB/FolC");
+        modifier.mkdir("FolA/FolB/FolC/FolD");
+        modifier.mkdir("FolA/FolB/FolC/FolD/FolE");
+        modifier.insert("FolA/FileA.txt");
+        modifier.insert("FolA/FolB/FileB.txt");
+        modifier.insert("FolA/FolB/FolC/FileC.txt");
+        modifier.insert("FolA/FolB/FolC/FolD/FileD.txt");
+        modifier.insert("FolA/FolB/FolC/FolD/FolE/FileE.txt");
+        QVERIFY(fakeFolder.syncOnce());
+
+        modifier.insert("FolA/FileA2.txt");
+        modifier.insert("FolA/FolB/FileB2.txt");
+        modifier.insert("FolA/FolB/FolC/FileC2.txt");
+        modifier.insert("FolA/FolB/FolC/FolD/FileD2.txt");
+        modifier.insert("FolA/FolB/FolC/FolD/FolE/FileE2.txt");
+        modifier.rename("FolA", "FolA_Renamed");
+        modifier.rename("FolA_Renamed/FolB", "FolB_Renamed");
+        modifier.rename("FolB_Renamed/FolC", "FolA");
+        modifier.rename("FolA/FolD", "FolA/FolD_Renamed");
+        modifier.mkdir("FolB_Renamed/New");
+        modifier.rename("FolA/FolD_Renamed/FolE", "FolB_Renamed/New/FolE");
+        auto expected = local ? fakeFolder.currentLocalState() : fakeFolder.currentRemoteState();
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), expected);
+        QCOMPARE(fakeFolder.currentRemoteState(), expected);
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncMove)