diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 1046e55871..85bcb0b77d 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -913,6 +913,14 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it // Unknown in db: new file on the server Q_ASSERT(!dbEntry.isValid()); + if (_discoveryData->recursiveCheckForDeletedParents(item->_file)) { + qCWarning(lcDisco) << "Removing local file inside a remotely deleted folder" << item->_file; + item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->_direction = SyncFileItem::Down; + emit _discoveryData->itemDiscovered(item); + return; + } + item->_instruction = CSYNC_INSTRUCTION_NEW; item->_direction = SyncFileItem::Down; item->_modtime = serverEntry.modtime; @@ -1425,6 +1433,14 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( return; } + if (_discoveryData->recursiveCheckForDeletedParents(item->_file)) { + qCWarning(lcDisco) << "Removing local file inside a remotely deleted folder" << item->_file; + item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->_direction = SyncFileItem::Down; + emit _discoveryData->itemDiscovered(item); + return; + } + // New local file or rename item->_instruction = CSYNC_INSTRUCTION_NEW; item->_direction = SyncFileItem::Up; @@ -2155,25 +2171,6 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs) if (_queuedJobs.empty() && _runningJobs.empty() && _pendingAsyncJobs == 0) { _pendingAsyncJobs = -1; // We're finished, we don't want to emit finished again if (_dirItem) { - if (_childModified && _dirItem->_instruction == CSYNC_INSTRUCTION_REMOVE) { - // re-create directory that has modified contents - _dirItem->_instruction = CSYNC_INSTRUCTION_NEW; - - const auto perms = !_rootPermissions.isNull() ? _rootPermissions - : _dirParentItem ? _dirParentItem->_remotePerm : _rootPermissions; - - if (perms.isNull()) { - // No permissions set - } else if (_dirItem->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddSubDirectories)) { - qCWarning(lcDisco) << "checkForPermission: Not allowed because you don't have permission to add subfolders to that folder: " << _dirItem->_file; - _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE; - _dirItem->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder"); - const auto localPath = QString{_discoveryData->_localDir + _dirItem->_file}; - emit _discoveryData->remnantReadOnlyFolderDiscovered(_dirItem); - } - - _dirItem->_direction = _dirItem->_direction == SyncFileItem::Up ? SyncFileItem::Down : SyncFileItem::Up; - } if (_childModified && _dirItem->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE && !_dirItem->isDirectory()) { // Replacing a directory by a file is a conflict, if the directory had modified children _dirItem->_instruction = CSYNC_INSTRUCTION_CONFLICT; diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 4863eb13db..e50fd758ea 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -220,6 +221,33 @@ void DiscoveryPhase::enqueueDirectoryToDelete(const QString &path, ProcessDirect } } +bool DiscoveryPhase::recursiveCheckForDeletedParents(const QString &itemPath) const +{ + const auto &allKeys = _deletedItem.keys(); + qCDebug(lcDiscovery()) << allKeys.join(", "); + + auto result = false; + const auto &pathElements = itemPath.split('/'); + auto currentParentFolder = QString{}; + for (const auto &onePathComponent : pathElements) { + if (!currentParentFolder.isEmpty()) { + currentParentFolder += '/'; + } + currentParentFolder += onePathComponent; + + qCDebug(lcDiscovery()) << "checks" << currentParentFolder << "for" << allKeys.join(", "); + if (_deletedItem.find(currentParentFolder) == _deletedItem.end()) { + continue; + } + + qCDebug(lcDiscovery()) << "deleted parent found"; + result = true; + break; + } + + return result; +} + void DiscoveryPhase::markPermanentDeletionRequests() { // since we don't know in advance which files/directories need to be permanently deleted, diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index bf59d7ff13..13765725df 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -272,6 +272,8 @@ class DiscoveryPhase : public QObject void enqueueDirectoryToDelete(const QString &path, ProcessDirectoryJob* const directoryJob); + bool recursiveCheckForDeletedParents(const QString &itemPath) const; + /// contains files/folder names that are requested to be deleted permanently QSet _permanentDeletionRequests; diff --git a/test/testsynccfapi.cpp b/test/testsynccfapi.cpp index df42c72a3a..a07253e7bf 100644 --- a/test/testsynccfapi.cpp +++ b/test/testsynccfapi.cpp @@ -1553,6 +1553,33 @@ private slots: QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentRemoteState(), fakeFolder.currentRemoteState()); } + + void testSyncFolderNewDeleteConflictExpectDeletion() + { + FakeFolder fakeFolder{FileInfo{}}; + setupVfs(fakeFolder); + + fakeFolder.remoteModifier().mkdir("directory"); + fakeFolder.remoteModifier().mkdir("directory/subdir"); + fakeFolder.remoteModifier().insert("directory/file1"); + fakeFolder.remoteModifier().insert("directory/file2"); + fakeFolder.remoteModifier().insert("directory/file3"); + fakeFolder.remoteModifier().insert("directory/subdir/fileTxt1.txt"); + fakeFolder.remoteModifier().insert("directory/subdir/fileTxt2.txt"); + fakeFolder.remoteModifier().insert("directory/subdir/fileTxt3.txt"); + + // perform an initial sync to ensure local and remote have the same state + QVERIFY(fakeFolder.syncOnce()); + + fakeFolder.remoteModifier().remove("directory"); + fakeFolder.localModifier().mkdir("directory/subFolder"); + fakeFolder.localModifier().insert("directory/file4"); + fakeFolder.localModifier().insert("directory/subdir/fileTxt4.txt"); + + QVERIFY(fakeFolder.syncOnce()); + const auto directoryItem = fakeFolder.remoteModifier().find("directory"); + QCOMPARE(directoryItem, nullptr); + } }; QTEST_GUILESS_MAIN(TestSyncCfApi) diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 4779103a24..e7f075bd89 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -43,11 +43,11 @@ private slots: // A/a1 must be gone because the directory was removed on the server, but hello.txt must be there QVERIFY(!fakeFolder.currentRemoteState().find("A/a1")); - QVERIFY(fakeFolder.currentRemoteState().find("A/hello.txt")); + QVERIFY(!fakeFolder.currentRemoteState().find("A/hello.txt")); // Symmetry QVERIFY(!fakeFolder.currentRemoteState().find("B/b1")); - QVERIFY(fakeFolder.currentRemoteState().find("B/hello.txt")); + QVERIFY(!fakeFolder.currentRemoteState().find("B/hello.txt")); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index c7be002186..503454e4d7 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -259,59 +259,6 @@ private slots: QTest::newRow("delete") << false; } - void testLocalDeleteWithReuploadForNewLocalFiles() - { - QFETCH(bool, moveToTrashEnabled); - - FakeFolder fakeFolder{FileInfo{}}; - - auto syncOptions = fakeFolder.syncEngine().syncOptions(); - syncOptions._moveFilesToTrash = moveToTrashEnabled; - fakeFolder.syncEngine().setSyncOptions(syncOptions); - - // create folders hierarchy with some nested dirs and files - fakeFolder.localModifier().mkdir("A"); - fakeFolder.localModifier().insert("A/existingfile_A.txt", 100); - fakeFolder.localModifier().mkdir("A/B"); - fakeFolder.localModifier().insert("A/B/existingfile_B.data", 100); - fakeFolder.localModifier().mkdir("A/B/C"); - fakeFolder.localModifier().mkdir("A/B/C/c1"); - fakeFolder.localModifier().mkdir("A/B/C/c1/c2"); - fakeFolder.localModifier().insert("A/B/C/c1/c2/existingfile_C2.md", 100); - - QVERIFY(fakeFolder.syncOnce()); - - // make sure everything is uploaded - QVERIFY(fakeFolder.currentRemoteState().find("A/B/C/c1/c2")); - QVERIFY(fakeFolder.currentRemoteState().find("A/existingfile_A.txt")); - QVERIFY(fakeFolder.currentRemoteState().find("A/B/existingfile_B.data")); - QVERIFY(fakeFolder.currentRemoteState().find("A/B/C/c1/c2/existingfile_C2.md")); - QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - - // remove a folder "A" on the server - fakeFolder.remoteModifier().remove("A"); - - // put new files and folders into a local folder "A" - fakeFolder.localModifier().insert("A/B/C/c1/c2/newfile.txt", 100); - fakeFolder.localModifier().insert("A/B/C/c1/c2/Readme.data", 100); - fakeFolder.localModifier().mkdir("A/B/C/c1/c2/newfiles"); - fakeFolder.localModifier().insert("A/B/C/c1/c2/newfiles/newfile.txt", 100); - fakeFolder.localModifier().insert("A/B/C/c1/c2/newfiles/Readme.data", 100); - - QVERIFY(fakeFolder.syncOnce()); - // make sure new files and folders are uploaded (restored) - QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2")); - QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2/Readme.data")); - QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2/newfiles/newfile.txt")); - QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2/newfiles/Readme.data")); - // and the old files are removed - QVERIFY(!fakeFolder.currentLocalState().find("A/existingfile_A.txt")); - QVERIFY(!fakeFolder.currentLocalState().find("A/B/existingfile_B.data")); - QVERIFY(!fakeFolder.currentLocalState().find("A/B/C/c1/c2/existingfile_C2.md")); - - QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - } - void testEmlLocalChecksum() { FakeFolder fakeFolder{FileInfo{}}; fakeFolder.localModifier().insert("a1.eml", 64, 'A'); @@ -2489,6 +2436,31 @@ private slots: // last sync without changes, expect no files to be touched syncAndExpectNoTouchedFiles(); } + + void testSyncFolderNewDeleteConflictExpectDeletion() + { + FakeFolder fakeFolder{FileInfo{}}; + fakeFolder.remoteModifier().mkdir("directory"); + fakeFolder.remoteModifier().mkdir("directory/subdir"); + fakeFolder.remoteModifier().insert("directory/file1"); + fakeFolder.remoteModifier().insert("directory/file2"); + fakeFolder.remoteModifier().insert("directory/file3"); + fakeFolder.remoteModifier().insert("directory/subdir/fileTxt1.txt"); + fakeFolder.remoteModifier().insert("directory/subdir/fileTxt2.txt"); + fakeFolder.remoteModifier().insert("directory/subdir/fileTxt3.txt"); + + // perform an initial sync to ensure local and remote have the same state + QVERIFY(fakeFolder.syncOnce()); + + fakeFolder.remoteModifier().remove("directory"); + fakeFolder.localModifier().mkdir("directory/subFolder"); + fakeFolder.localModifier().insert("directory/file4"); + fakeFolder.localModifier().insert("directory/subdir/fileTxt4.txt"); + + QVERIFY(fakeFolder.syncOnce()); + const auto directoryItem = fakeFolder.remoteModifier().find("directory"); + QCOMPARE(directoryItem, nullptr); + } }; QTEST_GUILESS_MAIN(TestSyncEngine)