fix renaming of folders with a deep hierarchy inside them

src/libsync/discovery.cpp

@@ -37,6 +37,28 @@ namespace OCC {
 
 Q_LOGGING_CATEGORY(lcDisco, "nextcloud.sync.discovery", QtInfoMsg)
 
+ProcessDirectoryJob::ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState, qint64 lastSyncTimestamp, QObject *parent)
+    : QObject(parent)
+    , _lastSyncTimestamp(lastSyncTimestamp)
+    , _discoveryData(data)
+{
+    qCDebug(lcDisco) << data;
+    computePinState(basePinState);
+}
+
+ProcessDirectoryJob::ProcessDirectoryJob(const PathTuple &path, const SyncFileItemPtr &dirItem, QueryMode queryLocal, QueryMode queryServer, qint64 lastSyncTimestamp, ProcessDirectoryJob *parent)
+    : QObject(parent)
+    , _dirItem(dirItem)
+    , _lastSyncTimestamp(lastSyncTimestamp)
+    , _queryServer(queryServer)
+    , _queryLocal(queryLocal)
+    , _discoveryData(parent->_discoveryData)
+    , _currentFolder(path)
+{
+    qCDebug(lcDisco) << path._server << queryServer << path._local << queryLocal << lastSyncTimestamp;
+    computePinState(parent->_pinState);
+}
+
 void ProcessDirectoryJob::start()
 {
     qCInfo(lcDisco) << "STARTING" << _currentFolder._server << _queryServer << _currentFolder._local << _queryLocal;
@@ -52,6 +74,7 @@ void ProcessDirectoryJob::start()
         if (!_discoveryData->_shouldDiscoverLocaly(_currentFolder._local)
             && (_currentFolder._local == _currentFolder._original || !_discoveryData->_shouldDiscoverLocaly(_currentFolder._original))) {
             _queryLocal = ParentNotChanged;
+            qCDebug(lcDisco) << "adjusted discovery policy" << _currentFolder._server << _queryServer << _currentFolder._local << _queryLocal;
         }
     }
 

src/libsync/discovery.h

@@ -64,28 +64,12 @@ class ProcessDirectoryJob : public QObject
      * The base pin state is used if the root dir's pin state can't be retrieved.
      */
     explicit ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState,
-        qint64 lastSyncTimestamp, QObject *parent)
-        : QObject(parent)
-        , _lastSyncTimestamp(lastSyncTimestamp)
-        , _discoveryData(data)
-    {
-        computePinState(basePinState);
-    }
+        qint64 lastSyncTimestamp, QObject *parent);
 
     /// For creating subjobs
     explicit ProcessDirectoryJob(const PathTuple &path, const SyncFileItemPtr &dirItem,
         QueryMode queryLocal, QueryMode queryServer, qint64 lastSyncTimestamp,
-        ProcessDirectoryJob *parent)
-        : QObject(parent)
-        , _dirItem(dirItem)
-        , _lastSyncTimestamp(lastSyncTimestamp)
-        , _queryServer(queryServer)
-        , _queryLocal(queryLocal)
-        , _discoveryData(parent->_discoveryData)
-        , _currentFolder(path)
-    {
-        computePinState(parent->_pinState);
-    }
+        ProcessDirectoryJob *parent);
 
     void start();
     /** Start up to nbJobs, return the number of job started; emit finished() when done */

src/libsync/discoveryphase.h

@@ -32,11 +32,20 @@ class ExcludedFiles;
 
 namespace OCC {
 
+namespace LocalDiscoveryEnums {
+
+OCSYNC_EXPORT Q_NAMESPACE
+
 enum class LocalDiscoveryStyle {
     FilesystemOnly, //< read all local data from the filesystem
     DatabaseAndFilesystem, //< read from the db, except for listed paths
 };
 
+Q_ENUM_NS(LocalDiscoveryStyle)
+
+}
+
+using OCC::LocalDiscoveryEnums::LocalDiscoveryStyle;
 
 class Account;
 class SyncJournalDb;

src/libsync/propagatorjobs.cpp

@@ -211,16 +211,26 @@ void PropagateLocalMkdir::startLocalMkdir()
     done(resultStatus);
 }
 
+PropagateLocalRename::PropagateLocalRename(OwncloudPropagator *propagator, const SyncFileItemPtr &item)
+    : PropagateItemJob(propagator, item)
+{
+    qCDebug(lcPropagateLocalRename) << _item->_file << _item->_renameTarget << _item->_originalFile;
+}
+
 void PropagateLocalRename::start()
 {
     if (propagator()->_abortRequested)
         return;
 
-    QString existingFile = propagator()->fullLocalPath(propagator()->adjustRenamedPath(_item->_file));
-    QString targetFile = propagator()->fullLocalPath(_item->_renameTarget);
+    const auto previousNameInDb = propagator()->adjustRenamedPath(_item->_file);
+    const auto existingFile = propagator()->fullLocalPath(propagator()->adjustRenamedPath(_item->_file));
+    const auto targetFile = propagator()->fullLocalPath(_item->_renameTarget);
+
+    const auto fileAlreadyMoved = !QFileInfo::exists(propagator()->fullLocalPath(_item->_originalFile));
 
     // 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");
     if (_item->_file != _item->_renameTarget) {
         propagator()->reportProgress(*_item, 0);
         qCDebug(lcPropagateLocalRename) << "MOVE " << existingFile << " => " << targetFile;
@@ -234,8 +244,7 @@ void PropagateLocalRename::start()
             // it would have to come out the localFileNameClash function
             done(SyncFileItem::NormalError,
                 tr("File %1 cannot be renamed to %2 because of a local file name clash")
-                    .arg(QDir::toNativeSeparators(_item->_file))
-                    .arg(QDir::toNativeSeparators(_item->_renameTarget)));
+                    .arg(QDir::toNativeSeparators(_item->_file), QDir::toNativeSeparators(_item->_renameTarget)));
             return;
         }
 
@@ -249,15 +258,37 @@ void PropagateLocalRename::start()
     }
 
     SyncJournalFileRecord oldRecord;
-    if (!propagator()->_journal->getFileRecord(_item->_originalFile, &oldRecord)) {
+    if (!propagator()->_journal->getFileRecord(fileAlreadyMoved ? previousNameInDb : _item->_originalFile, &oldRecord)) {
         qCWarning(lcPropagateLocalRename) << "could not get file from local DB" << _item->_originalFile;
         done(SyncFileItem::NormalError, tr("could not get file %1 from local DB").arg(_item->_originalFile));
         return;
     }
-    if (!propagator()->_journal->deleteFileRecord(_item->_originalFile)) {
-        qCWarning(lcPropagateLocalRename) << "could not delete file from local DB" << _item->_originalFile;
-        done(SyncFileItem::NormalError, tr("Could not delete file record %1 from local DB").arg(_item->_originalFile));
-        return;
+
+    const auto deleteOldRecord = [this] (const QString &fileName) -> bool
+    {
+        SyncJournalFileRecord oldRecord;
+        if (!propagator()->_journal->getFileRecord(fileName, &oldRecord)) {
+            qCWarning(lcPropagateLocalRename) << "could not get file from local DB" << fileName;
+            done(SyncFileItem::NormalError, tr("could not get file %1 from local DB").arg(fileName));
+            return false;
+        }
+        if (!propagator()->_journal->deleteFileRecord(fileName)) {
+            qCWarning(lcPropagateLocalRename) << "could not delete file from local DB" << fileName;
+            done(SyncFileItem::NormalError, tr("Could not delete file record %1 from local DB").arg(fileName));
+            return false;
+        }
+
+        return true;
+    };
+
+    if (fileAlreadyMoved) {
+        if (!deleteOldRecord(previousNameInDb)) {
+            return;
+        }
+    } else {
+        if (!deleteOldRecord(_item->_originalFile)) {
+            return;
+        }
     }
 
     auto &vfs = propagator()->syncOptions()._vfs;
@@ -282,6 +313,40 @@ void PropagateLocalRename::start()
             return;
         }
     } else {
+        auto dbQueryResult = propagator()->_journal->getFilesBelowPath(oldFile.toUtf8(), [oldFile, this] (const SyncJournalFileRecord &record) -> void {
+            const auto oldFileName = record._path;
+            const auto oldFileNameString = QString::fromUtf8(oldFileName);
+            auto newFileNameString = oldFileNameString;
+            newFileNameString.replace(0, oldFile.length(), _item->_renameTarget);
+
+            if (oldFileNameString == newFileNameString) {
+                return;
+            }
+
+            SyncJournalFileRecord oldRecord;
+            if (!propagator()->_journal->getFileRecord(oldFileName, &oldRecord)) {
+                qCWarning(lcPropagateLocalRename) << "could not get file from local DB" << oldFileName;
+                done(SyncFileItem::NormalError, tr("could not get file %1 from local DB").arg(oldFileNameString));
+                return;
+            }
+            if (!propagator()->_journal->deleteFileRecord(oldFileName)) {
+                qCWarning(lcPropagateLocalRename) << "could not delete file from local DB" << oldFileName;
+                done(SyncFileItem::NormalError, tr("Could not delete file record %1 from local DB").arg(oldFileNameString));
+                return;
+            }
+
+            auto newItem = SyncFileItem::fromSyncJournalFileRecord(oldRecord);
+            newItem->_file = newFileNameString;
+            const auto result = propagator()->updateMetadata(*newItem);
+            if (!result) {
+                done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error()));
+                return;
+            }
+        });
+        if (!dbQueryResult) {
+            done(SyncFileItem::FatalError, tr("Failed to propagate directory rename in hierarchy"));
+            return;
+        }
         propagator()->_renamedDirectories.insert(oldFile, _item->_renameTarget);
         if (!PropagateRemoteMove::adjustSelectiveSync(propagator()->_journal, oldFile, _item->_renameTarget)) {
             done(SyncFileItem::FatalError, tr("Failed to rename file"));

src/libsync/propagatorjobs.h

@@ -86,10 +86,7 @@ class PropagateLocalRename : public PropagateItemJob
 {
     Q_OBJECT
 public:
-    PropagateLocalRename(OwncloudPropagator *propagator, const SyncFileItemPtr &item)
-        : PropagateItemJob(propagator, item)
-    {
-    }
+    PropagateLocalRename(OwncloudPropagator *propagator, const SyncFileItemPtr &item);
     void start() override;
     JobParallelism parallelism() override { return _item->isDirectory() ? WaitForFinished : FullParallelism; }
 };

src/libsync/syncengine.cpp

@@ -586,7 +586,11 @@ void SyncEngine::startSync()
     if (!_discoveryPhase->_remoteFolder.endsWith('/'))
         _discoveryPhase->_remoteFolder+='/';
     _discoveryPhase->_syncOptions = _syncOptions;
-    _discoveryPhase->_shouldDiscoverLocaly = [this](const QString &s) { return shouldDiscoverLocally(s); };
+    _discoveryPhase->_shouldDiscoverLocaly = [this](const QString &s) {
+        const auto result = shouldDiscoverLocally(s);
+        qCInfo(lcEngine) << "shouldDiscoverLocaly" << s << (result ? "true" : "false");
+        return result;
+    };
     _discoveryPhase->setSelectiveSyncBlackList(selectiveSyncBlackList);
     _discoveryPhase->setSelectiveSyncWhiteList(_journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncWhiteList, &ok));
     if (!ok) {

test/testsyncmove.cpp

@@ -992,6 +992,56 @@ private slots:
         QVERIFY(!fakeFolder.currentRemoteState().find(dest));
     }
 
+    void testRenameDeepHierarchy()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        fakeFolder.remoteModifier().mkdir("FolA");
+        fakeFolder.remoteModifier().mkdir("FolA/FolB2");
+        fakeFolder.remoteModifier().mkdir("FolA/FolB");
+        fakeFolder.remoteModifier().mkdir("FolA/FolB/FolC");
+        fakeFolder.remoteModifier().mkdir("FolA/FolB/FolC/FolD");
+        fakeFolder.remoteModifier().mkdir("FolA/FolB/FolC/FolD/FolE");
+        fakeFolder.remoteModifier().insert("FolA/FileA.txt");
+        auto fileA = fakeFolder.remoteModifier().find("FolA/FileA.txt");
+        QVERIFY(fileA);
+        fileA->extraDavProperties = "<oc:checksums><checksum>SHA1:22596363b3de40b06f981fb85d82312e8c0ed511</checksum></oc:checksums>";
+        fakeFolder.remoteModifier().insert("FolA/FolB/FileB.txt");
+        auto fileB = fakeFolder.remoteModifier().find("FolA/FolB/FileB.txt");
+        QVERIFY(fileB);
+        fileB->extraDavProperties = "<oc:checksums><checksum>SHA1:22596363b3de40b06f981fb85d82312e8c0ed511</checksum></oc:checksums>";
+        fakeFolder.remoteModifier().insert("FolA/FolB/FolC/FileC.txt");
+        auto fileC = fakeFolder.remoteModifier().find("FolA/FolB/FolC/FileC.txt");
+        QVERIFY(fileC);
+        fileC->extraDavProperties = "<oc:checksums><checksum>SHA1:22596363b3de40b06f981fb85d82312e8c0ed511</checksum></oc:checksums>";
+        fakeFolder.remoteModifier().insert("FolA/FolB/FolC/FolD/FileD.txt");
+        auto fileD = fakeFolder.remoteModifier().find("FolA/FolB/FolC/FolD/FileD.txt");
+        QVERIFY(fileD);
+        fileD->extraDavProperties = "<oc:checksums><checksum>SHA1:22596363b3de40b06f981fb85d82312e8c0ed511</checksum></oc:checksums>";
+        fakeFolder.remoteModifier().insert("FolA/FolB/FolC/FolD/FolE/FileE.txt");
+        auto fileE = fakeFolder.remoteModifier().find("FolA/FolB/FolC/FolD/FolE/FileE.txt");
+        QVERIFY(fileE);
+        fileE->extraDavProperties = "<oc:checksums><checksum>SHA1:22596363b3de40b06f981fb85d82312e8c0ed511</checksum></oc:checksums>";
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem);
+        QVERIFY(fakeFolder.syncOnce());
+
+        fakeFolder.remoteModifier().rename("FolA/FolB", "FolA/FolB2/FolB");
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem);
+        QVERIFY(fakeFolder.syncOnce());
+
+        fakeFolder.remoteModifier().insert("FolA/FolB2/FolB/FolC/FolD/FileD2.txt");
+        auto fileD2 = fakeFolder.remoteModifier().find("FolA/FolB2/FolB/FolC/FolD/FileD2.txt");
+        QVERIFY(fileD2);
+        fileD2->extraDavProperties = "<oc:checksums><checksum>SHA1:22596363b3de40b06f981fb85d82312e8c0ed511</checksum></oc:checksums>";
+        fakeFolder.remoteModifier().appendByte("FolA/FolB2/FolB/FolC/FolD/FileD.txt");
+        auto fileDMoved = fakeFolder.remoteModifier().find("FolA/FolB2/FolB/FolC/FolD/FileD.txt");
+        QVERIFY(fileDMoved);
+        fileDMoved->extraDavProperties = "<oc:checksums><checksum>SHA1:22596363b3de40b06f981fb85d82312e8c0ed522</checksum></oc:checksums>";
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::FilesystemOnly);
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncMove)