From ff2e868ff741c1f15d8d3f032677916271463401 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Tue, 6 Jun 2017 16:00:41 +0200 Subject: [PATCH] SyncEngine: SelectiveSync: Remove local files of undelected folder despite other modified files Issue #5783 When the directry that should be removed by selective sync contains changes, we ignore the whole sub tree instead of only ignoreing new files. We cannot ignore the whole directory, we need to ignore only the directory that do not have files to remove --- src/libsync/syncengine.cpp | 34 ++++++++++++++++++++++++++++++++++ test/testsyncengine.cpp | 24 +++++++++++++++++++++--- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 12d6c879cd4..00a795976f9 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -1159,6 +1159,7 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) bool selectiveListOk; auto selectiveSyncBlackList = _journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &selectiveListOk); std::sort(selectiveSyncBlackList.begin(), selectiveSyncBlackList.end()); + SyncFileItemPtr needle; for (SyncFileItemVector::iterator it = syncItems.begin(); it != syncItems.end(); ++it) { if ((*it)->_direction != SyncFileItem::Up) { @@ -1177,8 +1178,41 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) (*it)->_errorString = tr("Ignored because of the \"choose what to sync\" blacklist"); if ((*it)->_isDirectory) { + auto it_base = it; for (SyncFileItemVector::iterator it_next = it + 1; it_next != syncItems.end() && (*it_next)->_file.startsWith(path); ++it_next) { it = it_next; + // We want to ignore almost all instructions for items inside selective-sync excluded folders. + //The exception are DOWN/REMOVE actions that remove local files and folders that are + //guaranteed to be up-to-date with their server copies. + if ((*it)->_direction == SyncFileItem::Down && (*it)->_instruction == CSYNC_INSTRUCTION_REMOVE) { + // We need to keep the "delete" items. So we need to un-ignore parent directories + QString parentDir = (*it)->_file; + do { + parentDir = QFileInfo(parentDir).path(); + if (parentDir.isEmpty() || !parentDir.startsWith((*it_base)->destination())) { + break; + } + // Find the parent directory in the syncItems vector. Since the vector + // is sorted we can use a lower_bound, but we need a fake + // SyncFileItemPtr needle to compare against + if (!needle) + needle = SyncFileItemPtr::create(); + needle->_file = parentDir; + auto parent_it = std::lower_bound(it_base, it, needle); + if (parent_it == syncItems.end() || (*parent_it)->destination() != parentDir) { + break; + } + ASSERT((*parent_it)->_isDirectory); + if ((*parent_it)->_instruction != CSYNC_INSTRUCTION_IGNORE) { + break; // already changed + } + (*parent_it)->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + (*parent_it)->_status = SyncFileItem::NoStatus; + (*parent_it)->_errorString.clear(); + + } while (true); + continue; + } (*it)->_instruction = CSYNC_INSTRUCTION_IGNORE; (*it)->_status = SyncFileItem::FileIgnored; (*it)->_errorString = tr("Ignored because of the \"choose what to sync\" blacklist"); diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 242d5e438ba..4200d5e1b3d 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -220,7 +220,18 @@ private slots: FileInfo { QStringLiteral("parentFolder"), { FileInfo{ QStringLiteral("subFolder"), { { QStringLiteral("fileA.txt"), 400 }, - { QStringLiteral("fileB.txt"), 400, 'o' } + { QStringLiteral("fileB.txt"), 400, 'o' }, + FileInfo { QStringLiteral("subsubFolder"), { + { QStringLiteral("fileC.txt"), 400 }, + { QStringLiteral("fileD.txt"), 400, 'o' } + }}, + FileInfo{ QStringLiteral("anotherFolder"), { + FileInfo { QStringLiteral("emptyFolder"), { } }, + FileInfo { QStringLiteral("subsubFolder"), { + { QStringLiteral("fileE.txt"), 400 }, + { QStringLiteral("fileF.txt"), 400, 'o' } + }} + }} }} }} }}}; @@ -233,9 +244,11 @@ private slots: {"parentFolder/subFolder/"}); fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync("parentFolder/subFolder/"); - // But touch a local file before the next sync, such that the local folder + // But touch local file before the next sync, such that the local folder // can't be removed fakeFolder.localModifier().setContents("parentFolder/subFolder/fileB.txt", 'n'); + fakeFolder.localModifier().setContents("parentFolder/subFolder/subsubFolder/fileD.txt", 'n'); + fakeFolder.localModifier().setContents("parentFolder/subFolder/anotherFolder/subsubFolder/fileF.txt", 'n'); // Several follow-up syncs don't change the state at all, // in particular the remote state doesn't change and fileB.txt @@ -250,8 +263,13 @@ private slots: // The local state should still have subFolderA auto local = fakeFolder.currentLocalState(); QVERIFY(local.find("parentFolder/subFolder")); - QVERIFY(local.find("parentFolder/subFolder/fileA.txt")); + QVERIFY(!local.find("parentFolder/subFolder/fileA.txt")); QVERIFY(local.find("parentFolder/subFolder/fileB.txt")); + QVERIFY(!local.find("parentFolder/subFolder/subsubFolder/fileC.txt")); + QVERIFY(local.find("parentFolder/subFolder/subsubFolder/fileD.txt")); + QVERIFY(!local.find("parentFolder/subFolder/anotherFolder/subsubFolder/fileE.txt")); + QVERIFY(local.find("parentFolder/subFolder/anotherFolder/subsubFolder/fileF.txt")); + QVERIFY(!local.find("parentFolder/subFolder/anotherFolder/emptyFolder")); } } }