From ade4c11de3e8c8075820767ca26065c9ce9e8f52 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Sun, 23 Dec 2018 10:19:20 +0100 Subject: [PATCH] Rename: fix renamed folder moved into renamed folder issue Issue #6694 --- src/libsync/discoveryphase.cpp | 10 ++++++++-- src/libsync/discoveryphase.h | 3 +++ src/libsync/owncloudpropagator.cpp | 6 ++++++ src/libsync/owncloudpropagator.h | 4 ++++ src/libsync/propagateremotemove.cpp | 8 +++++--- src/libsync/propagatorjobs.cpp | 3 ++- test/testsyncmove.cpp | 12 ++++++++++-- 7 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 157a8d844..f8ea51d29 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -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 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); } } diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 31dc9abea..cebc4181e 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -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 renamedItems, const QString original); } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 951d188ed..e8572790c 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -26,6 +26,7 @@ #include "common/utility.h" #include "account.h" #include "common/asserts.h" +#include "discoveryphase.h" #ifdef Q_OS_WIN #include @@ -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) diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 27a299682..6992ff584 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -503,6 +503,10 @@ public: bool createConflict(const SyncFileItemPtr &item, PropagatorCompositeJob *composite, QString *error); + + QMap _renamedDirectories; + QString adjustRenamedPath(const QString &original) const; + private slots: void abortTimeout() diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 65ad5d527..a79e59945 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -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; diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index b3548863b..4ee7381f5 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -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; diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp index 0c7a7f155..adfc74255 100644 --- a/test/testsyncmove.cpp +++ b/test/testsyncmove.cpp @@ -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); } }; -- 2.30.2