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,29 +244,28 @@ 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;
  }
 
  emit propagator()->touchedFile(existingFile);
  emit propagator()->touchedFile(targetFile);
- QString renameError;
- if (!FileSystem::rename(existingFile, targetFile, &renameError)) {
+ if (QString renameError; !FileSystem::rename(existingFile, targetFile, &renameError)) {
  done(SyncFileItem::NormalError, renameError);
  return;
  }
  }
 
  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));
+
+ if (fileAlreadyMoved && !deleteOldDbRecord(previousNameInDb)) {
+ return;
+ } else if (!deleteOldDbRecord(_item->_originalFile)) {
  return;
  }
 
@@ -269,7 +278,7 @@ void PropagateLocalRename::start()
  const auto oldFile = _item->_file;
 
  if (!_item->isDirectory()) { // Directories are saved at the end
- SyncFileItem newItem(*_item);
+ auto newItem(*_item);
  if (oldRecord.isValid()) {
  newItem._checksumHeader = oldRecord._checksumHeader;
  }
@@ -282,6 +291,40 @@ void PropagateLocalRename::start()
  return;
  }
  } else {
+ const 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;
+ }
+
+ const 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"));
@@ -298,4 +341,20 @@ void PropagateLocalRename::start()
 
  done(SyncFileItem::Success);
 }
+
+bool PropagateLocalRename::deleteOldDbRecord(const QString &fileName)
+{
+ if (SyncJournalFileRecord oldRecord; !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;
+}
 }

src/libsync/propagatorjobs.h

@@ -86,11 +86,12 @@ 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; }
+
+private:
+ bool deleteOldDbRecord(const QString &fileName);
+
 };
 }

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 &path) {
+ const auto result = shouldDiscoverLocally(path);
+ qCInfo(lcEngine) << "shouldDiscoverLocaly" << path << (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)