Rename: fix renamed folder moved into renamed folder issue
authorOlivier Goffart <ogoffart@woboq.com>
Sun, 23 Dec 2018 09:19:20 +0000 (10:19 +0100)
committerKevin Ottens <kevin.ottens@nextcloud.com>
Tue, 15 Dec 2020 09:58:35 +0000 (10:58 +0100)
Issue #6694

src/libsync/discoveryphase.cpp
src/libsync/discoveryphase.h
src/libsync/owncloudpropagator.cpp
src/libsync/owncloudpropagator.h
src/libsync/propagateremotemove.cpp
src/libsync/propagatorjobs.cpp
test/testsyncmove.cpp

index 157a8d8444937dbd3b36527cf782f3a7c392414c..f8ea51d29891fe14ef1d43ffa6c879fd0cf35106 100644 (file)
@@ -132,13 +132,19 @@ 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
+{
+    return OCC::adjustRenamedPath(_renamedItems, original);
+}
+
+QString adjustRenamedPath(const QMap<QString, QString> renamedItems, const QString original)
 {
     int slashPos = original.size();
     while ((slashPos = original.lastIndexOf('/', slashPos - 1)) > 0) {
-        auto it = _renamedItems.constFind(original.left(slashPos));
-        if (it != _renamedItems.constEnd()) {
+        auto it = renamedItems.constFind(original.left(slashPos));
+        if (it != renamedItems.constEnd()) {
             return *it + original.mid(slashPos);
         }
     }
index 31dc9abeac39b002363782544b81f5b6db87a152..cebc4181ea286d8e5ca789080f4e944e954b82df 100644 (file)
@@ -192,4 +192,7 @@ signals:
     // A new folder was discovered and was not synced because of the confirmation feature
     void newBigFolder(const QString &folder, bool isExternal);
 };
+
+/// Implementation of DiscoveryPhase::adjustRenamedPath
+QString adjustRenamedPath(const QMap<QString, QString> renamedItems, const QString original);
 }
index 951d188edee211fe8d135e7ba4e3b5d67e6c77dc..e8572790c70a2d3a645d11108fdef6c75f68c308 100644 (file)
@@ -26,6 +26,7 @@
 #include "common/utility.h"
 #include "account.h"
 #include "common/asserts.h"
+#include "discoveryphase.h"
 
 #ifdef Q_OS_WIN
 #include <windef.h>
@@ -718,6 +719,11 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item,
     return true;
 }
 
+QString OwncloudPropagator::adjustRenamedPath(const QString &original) const
+{
+    return OCC::adjustRenamedPath(_renamedDirectories, original);
+}
+
 // ================================================================================
 
 PropagatorJob::PropagatorJob(OwncloudPropagator *propagator)
index 27a299682d8c07cfe180a6cafe77acc1e8f9d373..6992ff5843a462fc1a22d022efdb022cc8d3ba25 100644 (file)
@@ -503,6 +503,10 @@ public:
     bool createConflict(const SyncFileItemPtr &item,
         PropagatorCompositeJob *composite, QString *error);
 
+
+    QMap<QString, QString> _renamedDirectories;
+    QString adjustRenamedPath(const QString &original) const;
+
 private slots:
 
     void abortTimeout()
index 65ad5d52764aec08abb75e2d3db9623741871f79..a79e59945c01747fa77b16f93ac9fd9fbcdbdea5 100644 (file)
@@ -78,17 +78,18 @@ void PropagateRemoteMove::start()
     if (propagator()->_abortRequested.fetchAndAddRelaxed(0))
         return;
 
-    qCDebug(lcPropagateRemoteMove) << _item->_file << _item->_renameTarget;
+    QString origin = propagator()->adjustRenamedPath(_item->_file);
+    qCDebug(lcPropagateRemoteMove) << origin << _item->_renameTarget;
 
     QString targetFile(propagator()->getFilePath(_item->_renameTarget));
 
-    if (_item->_file == _item->_renameTarget) {
+    if (origin == _item->_renameTarget) {
         // The parent has been renamed already so there is nothing more to do.
         finalize();
         return;
     }
 
-    QString source = propagator()->_remoteFolder + _item->_file;
+    QString source = propagator()->_remoteFolder + origin;
     QString destination = QDir::cleanPath(propagator()->account()->davUrl().path() + propagator()->_remoteFolder + _item->_renameTarget);
     auto &vfs = propagator()->syncOptions()._vfs;
     if (vfs->mode() == Vfs::WithSuffix
@@ -179,6 +180,7 @@ void PropagateRemoteMove::finalize()
     }
 
     if (_item->isDirectory()) {
+        propagator()->_renamedDirectories.insert(_item->_file, _item->_renameTarget);
         if (!adjustSelectiveSync(propagator()->_journal, _item->_file, _item->_renameTarget)) {
             done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
             return;
index b3548863b2f14123bd11f7cc0a2c02a6006c3cb6..4ee7381f539408968bacf5333d15a45627ca1be5 100644 (file)
@@ -247,7 +247,7 @@ void PropagateLocalRename::start()
     if (propagator()->_abortRequested.fetchAndAddRelaxed(0))
         return;
 
-    QString existingFile = propagator()->getFilePath(_item->_file);
+    QString existingFile = propagator()->getFilePath(propagator()->adjustRenamedPath(_item->_file));
     QString targetFile = propagator()->getFilePath(_item->_renameTarget);
 
     // if the file is a file underneath a moved dir, the _item->file is equal
@@ -299,6 +299,7 @@ void PropagateLocalRename::start()
             return;
         }
     } else {
+        propagator()->_renamedDirectories.insert(oldFile, _item->_renameTarget);
         if (!PropagateRemoteMove::adjustSelectiveSync(propagator()->_journal, oldFile, _item->_renameTarget)) {
             done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
             return;
index 0c7a7f1555658356e2ddfed051ab860f57c466f2..adfc742554d78d9c82941da0bbfd7a2a034a5402 100644 (file)
@@ -650,7 +650,6 @@ private slots:
         QCOMPARE(fakeFolder.currentLocalState(), expectedState);
         QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
 
-        /* FIXME - likely addressed by ogoffart's sync code refactor
         // Now, the revert, but "crossed"
         fakeFolder.localModifier().rename("Empty/A", "A");
         fakeFolder.localModifier().rename("AllEmpty/C", "C");
@@ -660,7 +659,16 @@ private slots:
         QVERIFY(fakeFolder.syncOnce());
         QCOMPARE(fakeFolder.currentLocalState(), expectedState);
         QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
-        */
+
+        // Reverse on remote
+        fakeFolder.remoteModifier().rename("A/AllEmpty", "AllEmpty");
+        fakeFolder.remoteModifier().rename("C/Empty", "Empty");
+        fakeFolder.remoteModifier().rename("C", "AllEmpty/C");
+        fakeFolder.remoteModifier().rename("A", "Empty/A");
+        expectedState = fakeFolder.currentRemoteState();
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), expectedState);
+        QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
     }
 };