diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index b3fedf6da4c35..ec86e7f4e3363 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -573,8 +573,8 @@ void ProcessDirectoryJob::processFile(PathTuple path, const auto hasLocal = localEntry.isValid() ? "true" : _queryLocal == ParentNotChanged ? "db" : "false"; const auto serverFileIsLocked = (serverEntry.isValid() ? (serverEntry.locked == SyncFileItem::LockStatus::LockedItem ? "locked" : "not locked") : ""); const auto localFileIsLocked = dbEntry._lockstate._locked ? "locked" : "not locked"; - const auto serverFileLockType = serverEntry.isValid() ? QString::number(static_cast(serverEntry.lockOwnerType)) : QStringLiteral(""); - const auto localFileLockType = dbEntry._lockstate._locked ? QString::number(static_cast(dbEntry._lockstate._lockOwnerType)) : QStringLiteral(""); + const auto serverFileLockType = serverEntry.isValid() ? QString::number(static_cast(serverEntry.lockOwnerType)) : QString{}; + const auto localFileLockType = dbEntry._lockstate._locked ? QString::number(static_cast(dbEntry._lockstate._lockOwnerType)) : QString{}; QString processingLog; QDebug deleteLogger{&processingLog}; @@ -587,6 +587,7 @@ void ProcessDirectoryJob::processFile(PathTuple path, << " | checksum: " << dbEntry._checksumHeader << "//" << serverEntry.checksumHeader << " | perm: " << dbEntry._remotePerm << "//" << serverEntry.remotePerm << " | fileid: " << dbEntry._fileId << "//" << serverEntry.fileId + << " | inode: " << dbEntry._inode << "/" << localEntry.inode << "/" << " | type: " << dbEntry._type << "/" << localEntry.type << "/" << (serverEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile) << " | e2ee: " << dbEntry.isE2eEncrypted() << "/" << serverEntry.isE2eEncrypted() << " | e2eeMangledName: " << dbEntry.e2eMangledName() << "/" << serverEntry.e2eMangledName diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 002480c82705d..6711685a31a3f 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -302,8 +302,9 @@ void PropagateLocalRename::start() const auto previousNameInDb = propagator()->adjustRenamedPath(_item->_file); const auto existingFile = propagator()->fullLocalPath(previousNameInDb); const auto targetFile = propagator()->fullLocalPath(_item->_renameTarget); + const auto originalFile = propagator()->fullLocalPath(_item->_originalFile); - const auto fileAlreadyMoved = !FileSystem::fileExists(propagator()->fullLocalPath(_item->_originalFile)) && FileSystem::fileExists(existingFile); + const auto fileAlreadyMoved = (!FileSystem::fileExists(originalFile) || !FileSystem::fileExists(existingFile))&& FileSystem::fileExists(targetFile); auto pinState = OCC::PinState::Unspecified; if (!fileAlreadyMoved) { auto pinStateResult = vfs->pinState(propagator()->adjustRenamedPath(_item->_file)); @@ -315,6 +316,8 @@ 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"); + qCDebug(lcPropagateLocalRename()) << (FileSystem::fileExists(originalFile) ? "original file exists" : "orignal file is missing") << originalFile << _item->_originalFile; + qCDebug(lcPropagateLocalRename()) << (FileSystem::fileExists(existingFile) ? "existing file exists" : "existing file is missing") << existingFile << previousNameInDb; Q_ASSERT(FileSystem::fileExists(propagator()->fullLocalPath(_item->_originalFile)) || FileSystem::fileExists(existingFile)); if (_item->_file != _item->_renameTarget) { propagator()->reportProgress(*_item, 0); @@ -444,7 +447,7 @@ void PropagateLocalRename::start() if (fileAlreadyMoved && !deleteOldDbRecord(previousNameInDb)) { return; - } else if (!deleteOldDbRecord(_item->_originalFile)) { + } else if (!deleteOldDbRecord(previousNameInDb)) { qCWarning(lcPropagateLocalRename) << "Could not delete file from local DB" << _item->_originalFile; return; } @@ -470,26 +473,27 @@ void PropagateLocalRename::start() done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(newItem._file), ErrorCategory::GenericError); 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); + } else if (!fileAlreadyMoved) { + qCDebug(lcPropagateLocalRename) << "propagate child items after move from" << existingFile << "to" << targetFile; + const auto dbQueryResult = propagator()->_journal->getFilesBelowPath(previousNameInDb.toUtf8(), [previousNameInDb, this] (const SyncJournalFileRecord &record) -> void { + const auto oldFileNameString = propagator()->adjustRenamedPath(QString::fromUtf8(record._path)); auto newFileNameString = oldFileNameString; - newFileNameString.replace(0, oldFile.length(), _item->_renameTarget); + newFileNameString.replace(0, previousNameInDb.length(), _item->_renameTarget); + + qCDebug(lcPropagateLocalRename) << "child rename from" << oldFileNameString << "to" << newFileNameString; if (oldFileNameString == newFileNameString) { + Q_ASSERT(false); 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), OCC::ErrorCategory::GenericError); + if (!propagator()->_journal->getFileRecord(oldFileNameString, &oldRecord)) { + qCWarning(lcPropagateLocalRename) << "Could not get file from local DB" << 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), OCC::ErrorCategory::GenericError); + if (!propagator()->_journal->deleteFileRecord(oldFileNameString)) { + qCWarning(lcPropagateLocalRename) << "could not delete file from local DB" << oldFileNameString; return; } @@ -497,7 +501,6 @@ void PropagateLocalRename::start() newItem->_file = newFileNameString; const auto result = propagator()->updateMetadata(*newItem); if (!result) { - done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error()), OCC::ErrorCategory::GenericError); return; } }); diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp index d1ee8a06f3a96..d671389414dab 100644 --- a/src/libsync/syncfilestatustracker.cpp +++ b/src/libsync/syncfilestatustracker.cpp @@ -236,7 +236,11 @@ void SyncFileStatusTracker::slotAboutToPropagate(SyncFileItemVector &items) std::swap(_syncProblems, oldProblems); for (const auto &item : std::as_const(items)) { - qCInfo(lcStatusTracker) << "Investigating" << item->destination() << item->_status << item->_instruction << item->_direction; + if (item->_instruction == CSyncEnums::CSYNC_INSTRUCTION_RENAME) { + qCInfo(lcStatusTracker) << "Investigating" << item->destination() << item->_status << item->_instruction << item->_direction << item->_file << item->_originalFile << item->_renameTarget; + } else { + qCInfo(lcStatusTracker) << "Investigating" << item->destination() << item->_status << item->_instruction << item->_direction; + } _dirtyPaths.remove(item->destination()); if (hasErrorStatus(*item)) { diff --git a/test/testsynccfapi.cpp b/test/testsynccfapi.cpp index 20f509cb64aa9..b34b480c0c14d 100644 --- a/test/testsynccfapi.cpp +++ b/test/testsynccfapi.cpp @@ -1486,6 +1486,35 @@ private slots: QVERIFY(itemInstruction(completeSpy, odtFile, CSYNC_INSTRUCTION_UPDATE_METADATA)); QCOMPARE(*vfs->pinState(odtFile), PinState::Unspecified); } + + void renameOnBothSides() + { + FakeFolder fakeFolder { FileInfo::A12_B12_C12_S12() }; + auto vfs = setupVfs(fakeFolder); + + // Test that renaming a file within a directory that was renamed on the other side actually do a rename. + + // 1) move the folder alphabetically before + fakeFolder.remoteModifier().rename("A/a1", "A/a1m"); + fakeFolder.localModifier().rename("A", "_A"); + fakeFolder.localModifier().rename("B/b1", "B/b1m"); + fakeFolder.remoteModifier().rename("B", "_B"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentRemoteState(), fakeFolder.currentRemoteState()); + QVERIFY(fakeFolder.currentRemoteState().find("_A/a1m")); + QVERIFY(fakeFolder.currentRemoteState().find("_B/b1m")); + + // 2) move alphabetically after + fakeFolder.remoteModifier().rename("_A/a2", "_A/a2m"); + fakeFolder.localModifier().rename("_B/b2", "_B/b2m"); + fakeFolder.localModifier().rename("_A", "S/A"); + fakeFolder.remoteModifier().rename("_B", "S/B"); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentRemoteState(), fakeFolder.currentRemoteState()); + QVERIFY(fakeFolder.currentRemoteState().find("S/A/a2m")); + QVERIFY(fakeFolder.currentRemoteState().find("S/B/b2m")); + } }; QTEST_GUILESS_MAIN(TestSyncCfApi) diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp index 4f775c7fc9fd6..246d34882f787 100644 --- a/test/testsyncmove.cpp +++ b/test/testsyncmove.cpp @@ -1317,6 +1317,61 @@ private slots: QCOMPARE(counter.nMKCOL, 0); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + + void testRenameComplexScenarioNoRecordLeak() + { + FakeFolder fakeFolder{FileInfo{}}; + + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + fakeFolder.remoteModifier().mkdir("0"); + fakeFolder.remoteModifier().mkdir("0/00 without file"); + fakeFolder.remoteModifier().mkdir("0/00 without file/project"); + fakeFolder.remoteModifier().mkdir("0/00 without file/project/a"); + fakeFolder.remoteModifier().mkdir("0/00 without file/project/a/a with file"); + fakeFolder.remoteModifier().mkdir("0/00 without file/project/a/a without file"); + fakeFolder.remoteModifier().mkdir("0/00 without file/project/a/aa with file"); + fakeFolder.remoteModifier().mkdir("0/00 without file/project/00 with file"); + fakeFolder.remoteModifier().mkdir("0/00 without file/project/new without file"); + fakeFolder.remoteModifier().insert("0/00 without file/project/00 with file/test.md"); + fakeFolder.remoteModifier().insert("0/00 without file/project/a/a with file/test.md"); + fakeFolder.remoteModifier().insert("0/00 without file/project/a/aa with file/test.md"); + + QVERIFY(fakeFolder.syncOnce()); + + auto itemsCounter = 0; + auto dbResult = fakeFolder.syncJournal().getFilesBelowPath("", [&itemsCounter] (const SyncJournalFileRecord&) -> void { ++itemsCounter; }); + + QVERIFY(dbResult); + QCOMPARE(itemsCounter, 12); + + fakeFolder.remoteModifier().rename("0/00 without file/project", "project tests"); + fakeFolder.remoteModifier().rename("project tests/a", "project tests/a empty"); + fakeFolder.remoteModifier().rename("project tests/a empty/a with file", "project tests/a with file"); + fakeFolder.remoteModifier().rename("project tests/a empty/a without file", "project tests/a without file"); + fakeFolder.remoteModifier().rename("project tests/a empty/aa with file", "project tests/aa with file"); + fakeFolder.remoteModifier().rename("project tests/new without file", "project tests/new without file"); + fakeFolder.remoteModifier().rename("0/00 without file", "project tests/00 without file"); + fakeFolder.remoteModifier().rename("0", "project tests/z 0 empty"); + + connect(&fakeFolder.syncEngine(), &OCC::SyncEngine::itemCompleted, this, [&fakeFolder] () { + auto itemsCounter = 0; + auto dbResult = fakeFolder.syncJournal().getFilesBelowPath("", [&itemsCounter] (const SyncJournalFileRecord&) -> void { ++itemsCounter; }); + + QVERIFY(dbResult); + if (itemsCounter > 12) { + QVERIFY(itemsCounter <= 12); + } + }); + + QVERIFY(fakeFolder.syncOnce()); + + itemsCounter = 0; + dbResult = fakeFolder.syncJournal().getFilesBelowPath("", [&itemsCounter] (const SyncJournalFileRecord&) -> void { ++itemsCounter; }); + + QVERIFY(dbResult); + QCOMPARE(itemsCounter, 12); + } }; QTEST_GUILESS_MAIN(TestSyncMove)