Merge pull request #6463 from nextcloud/bugfix/multipleMovesOfASingle…

…Fileid when moving a file, checks that it exists at origin or destination
nextcloud · Mar 5, 2024 · 903e313 · 903e313
2 parents 7a0b03a + b7c1a95
commit 903e313
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 4 deletions.
diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
@@ -1666,6 +1666,9 @@ void ProcessDirectoryJob::processFileFinalize(
         ASSERT(_dirItem && _dirItem->_instruction == CSYNC_INSTRUCTION_RENAME);
         // This is because otherwise subitems are not updated!  (ideally renaming a directory could
         // update the database for all items!  See PropagateDirectory::slotSubJobsFinished)
+        const auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(path._original, SyncFileItem::Down);
+        Q_UNUSED(adjustedOriginalPath)
+        _discoveryData->_renamedItemsLocal.insert(path._original, path._target);
         item->_instruction = CSYNC_INSTRUCTION_RENAME;
         item->_renameTarget = path._target;
         item->_direction = _dirItem->_direction;

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
@@ -261,6 +261,11 @@ void DiscoveryPhase::setSelectiveSyncWhiteList(const QStringList &list)
     _selectiveSyncWhiteList.sort();
 }
 
+bool DiscoveryPhase::isRenamed(const QString &p) const
+{
+    return _renamedItemsLocal.contains(p) || _renamedItemsRemote.contains(p);
+}
+
 void DiscoveryPhase::scheduleMoreJobs()
 {
     auto limit = qMax(1, _syncOptions._parallelNetworkJobs);

diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
@@ -256,7 +256,7 @@ class DiscoveryPhase : public QObject
      * Useful for avoiding processing of items that have already been claimed in
      * a rename (would otherwise be discovered as deletions).
      */
-    [[nodiscard]] bool isRenamed(const QString &p) const { return _renamedItemsLocal.contains(p) || _renamedItemsRemote.contains(p); }
+    [[nodiscard]] bool isRenamed(const QString &p) const;
 
     int _currentlyActiveJobs = 0;
 

diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
@@ -224,10 +224,10 @@ void PropagateLocalRename::start()
 
     auto &vfs = propagator()->syncOptions()._vfs;
     const auto previousNameInDb = propagator()->adjustRenamedPath(_item->_file);
-    const auto existingFile = propagator()->fullLocalPath(propagator()->adjustRenamedPath(_item->_file));
+    const auto existingFile = propagator()->fullLocalPath(previousNameInDb);
     const auto targetFile = propagator()->fullLocalPath(_item->_renameTarget);
 
-    const auto fileAlreadyMoved = !QFileInfo::exists(propagator()->fullLocalPath(_item->_originalFile));
+    const auto fileAlreadyMoved = !QFileInfo::exists(propagator()->fullLocalPath(_item->_originalFile)) && QFileInfo::exists(existingFile);
     auto pinState = OCC::PinState::Unspecified;
     if (!fileAlreadyMoved) {
         auto pinStateResult = vfs->pinState(propagator()->adjustRenamedPath(_item->_file));
@@ -239,6 +239,7 @@ void PropagateLocalRename::start()
     // if the file is a file underneath a moved dir, the _item->file is equal
     // to _item->renameTarget and the file is not moved as a result.
     qCDebug(lcPropagateLocalRename) << _item->_file << _item->_renameTarget << _item->_originalFile << previousNameInDb << (fileAlreadyMoved ? "original file has already moved" : "original file is still there");
+    Q_ASSERT(QFileInfo::exists(propagator()->fullLocalPath(_item->_originalFile)) || QFileInfo::exists(existingFile));
     if (_item->_file != _item->_renameTarget) {
         propagator()->reportProgress(*_item, 0);
         qCDebug(lcPropagateLocalRename) << "MOVE " << existingFile << " => " << targetFile;

diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp
@@ -84,6 +84,12 @@ class TestSyncMove : public QObject
     Q_OBJECT
 
 private slots:
+    void initTestCase()
+    {
+        Logger::instance()->setLogFlush(true);
+        Logger::instance()->setLogDebug(true);
+    }
+
     void testMoveCustomRemoteRoot()
     {
         FileInfo subFolder(QStringLiteral("AS"), { { QStringLiteral("f1"), 4 } });
@@ -140,7 +146,22 @@ private slots:
     void testSelectiveSyncMovedFolder()
     {
         // issue #5224
-        FakeFolder fakeFolder{ FileInfo{ QString(), { FileInfo{ QStringLiteral("parentFolder"), { FileInfo{ QStringLiteral("subFolderA"), { { QStringLiteral("fileA.txt"), 400 } } }, FileInfo{ QStringLiteral("subFolderB"), { { QStringLiteral("fileB.txt"), 400 } } } } } } } };
+        FakeFolder fakeFolder{
+                              FileInfo{QString(), {
+                                      FileInfo{QStringLiteral("parentFolder"), {
+                                              FileInfo{QStringLiteral("subFolderA"), {
+                                                      {QStringLiteral("fileA.txt"), 400}
+                                                  }
+                                              },
+                                              FileInfo{QStringLiteral("subFolderB"), {
+                                                      {QStringLiteral("fileB.txt"), 400}
+                                                  }
+                                              }
+                                          }
+                                      }
+                                  }
+                              }
+        };
 
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
         auto expectedServerState = fakeFolder.currentRemoteState();
@@ -1042,6 +1063,66 @@ private slots:
 
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }
+
+    void testRenameSameFileInMultiplePaths()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        fakeFolder.remoteModifier().mkdir("FolderA");
+        fakeFolder.remoteModifier().mkdir("FolderA/folderParent");
+        fakeFolder.remoteModifier().mkdir("FolderB");
+        fakeFolder.remoteModifier().mkdir("FolderB/folderChild");
+        fakeFolder.remoteModifier().insert("FolderB/folderChild/FileA.txt");
+        fakeFolder.remoteModifier().mkdir("FolderC");
+
+        const auto folderParentFileInfo = fakeFolder.remoteModifier().find("FolderA/folderParent");
+        const auto folderParentSharedFolderFileId = folderParentFileInfo->fileId;
+        const auto folderParentSharedFolderEtag = folderParentFileInfo->etag;
+        const auto folderChildFileInfo = fakeFolder.remoteModifier().find("FolderB/folderChild");
+        const auto folderChildInFolderAFolderFileId = folderChildFileInfo->fileId;
+        const auto folderChildInFolderAEtag = folderChildFileInfo->etag;
+        const auto fileAFileInfo = fakeFolder.remoteModifier().find("FolderB/folderChild/FileA.txt");
+        const auto fileAInFolderAFolderFileId = fileAFileInfo->fileId;
+        const auto fileAInFolderAEtag = fileAFileInfo->etag;
+
+        auto folderCFileInfo = fakeFolder.remoteModifier().find("FolderC");
+        folderCFileInfo->fileId = folderParentSharedFolderFileId;
+        folderCFileInfo->etag = folderParentSharedFolderEtag;
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        fakeFolder.remoteModifier().remove("FolderB/folderChild");
+        fakeFolder.remoteModifier().mkdir("FolderA/folderParent/folderChild");
+        fakeFolder.remoteModifier().insert("FolderA/folderParent/folderChild/FileA.txt");
+        fakeFolder.remoteModifier().mkdir("FolderC/folderChild");
+        fakeFolder.remoteModifier().insert("FolderC/folderChild/FileA.txt");
+
+        auto folderChildInFolderParentFileInfo = fakeFolder.remoteModifier().find("FolderA/folderParent/folderChild");
+        folderChildInFolderParentFileInfo->fileId = folderChildInFolderAFolderFileId;
+        folderChildInFolderParentFileInfo->etag = folderChildInFolderAEtag;
+
+        auto fileAInFolderParentFileInfo = fakeFolder.remoteModifier().find("FolderA/folderParent/folderChild/FileA.txt");
+        fileAInFolderParentFileInfo->fileId = fileAInFolderAFolderFileId;
+        fileAInFolderParentFileInfo->etag = fileAInFolderAEtag;
+
+        auto folderChildInFolderCFileInfo = fakeFolder.remoteModifier().find("FolderC/folderChild");
+        folderChildInFolderCFileInfo->fileId = folderChildInFolderAFolderFileId;
+        folderChildInFolderCFileInfo->etag = folderChildInFolderAEtag;
+
+        auto fileAInFolderCFileInfo = fakeFolder.remoteModifier().find("FolderC/folderChild/FileA.txt");
+        fileAInFolderCFileInfo->fileId = fileAInFolderAFolderFileId;
+        fileAInFolderCFileInfo->etag = fileAInFolderAEtag;
+
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::FilesystemOnly);
+        QVERIFY(fakeFolder.syncOnce());
+
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::FilesystemOnly);
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncMove)