SyncEngine: SelectiveSync: Remove local files of undelected folder despite other...

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

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index 12d6c879cd4006e310eb5f7de353649f2ab9d902..00a795976f98b9f215a985cb2d2f2043306a681d 100644 (file)
--- 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 242d5e438ba8ee6305f4d470dae24759bacba451..4200d5e1b3d4e17b8c841b582e5d0b1979e3904f 100644 (file)
--- 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"));
             }
         }
     }