Local discovery: always recurse within touched directory

If the file system watcher tells us a directory was modified, we should
recurse into it because it means it is probably a new directory

Issue #6804

src/libsync/syncengine.cpp

@@ -1642,6 +1642,22 @@ void SyncEngine::setLocalDiscoveryOptions(LocalDiscoveryStyle style, std::set<QB
 {
     _localDiscoveryStyle = style;
     _localDiscoveryPaths = std::move(paths);
+
+    // Normalize to make sure that no path is a contained in another.
+    // Note: for simplicity, this code consider anything less than '/' as a path separator, so for
+    // example, this will remove "foo.bar" if "foo" is in the list. This will mean we might have
+    // some false positive, but that's Ok.
+    // This invariant is used in SyncEngine::shouldDiscoverLocally
+    QByteArray prev;
+    auto it = _localDiscoveryPaths.begin();
+    while(it != _localDiscoveryPaths.end()) {
+        if (!prev.isNull() && it->startsWith(prev) && (prev.endsWith('/') || *it == prev || it->at(prev.size()) <= '/')) {
+            it = _localDiscoveryPaths.erase(it);
+        } else {
+            prev = *it;
+            ++it;
+        }
+    }
 }
 
 bool SyncEngine::shouldDiscoverLocally(const QByteArray &path) const
@@ -1649,14 +1665,28 @@ bool SyncEngine::shouldDiscoverLocally(const QByteArray &path) const
     if (_localDiscoveryStyle == LocalDiscoveryStyle::FilesystemOnly)
         return true;
 
+    // The intention is that if "A/X" is in _localDiscoveryPaths:
+    // - parent folders like "/", "A" will be discovered (to make sure the discovery reaches the
+    //   point where something new happened)
+    // - the folder itself "A/X" will be discovered
+    // - subfolders like "A/X/Y" will be discovered (so data inside a new or renamed folder will be
+    //   discovered in full)
+    // Check out TestLocalDiscovery::testLocalDiscoveryDecision()
+
     auto it = _localDiscoveryPaths.lower_bound(path);
-    if (it == _localDiscoveryPaths.end() || !it->startsWith(path))
+    if (it == _localDiscoveryPaths.end() || !it->startsWith(path)) {
+        // Maybe a subfolder of something in the list?
+        if (it != _localDiscoveryPaths.begin() && path.startsWith(*(--it))) {
+            return it->endsWith('/') || (path.size() > it->size() && path.at(it->size()) <= '/');
+        }
         return false;
+    }
 
     // maybe an exact match or an empty path?
     if (it->size() == path.size() || path.isEmpty())
         return true;
 
+    // Maybe a parent folder of something in the list?
     // check for a prefix + / match
     forever {
         if (it->size() > path.size() && it->at(path.size()) == '/')

test/testlocaldiscovery.cpp

@@ -76,7 +76,7 @@ private slots:
 
         fakeFolder.syncEngine().setLocalDiscoveryOptions(
             LocalDiscoveryStyle::DatabaseAndFilesystem,
-            { "A/X", "foo bar space/touch", "foo/", "zzz" });
+            { "A/X", "A/X space", "A/X/beta", "foo bar space/touch", "foo/", "zzz", "zzzz" });
 
         QVERIFY(engine.shouldDiscoverLocally(""));
         QVERIFY(engine.shouldDiscoverLocally("A"));
@@ -84,11 +84,23 @@ private slots:
         QVERIFY(!engine.shouldDiscoverLocally("B"));
         QVERIFY(!engine.shouldDiscoverLocally("A B"));
         QVERIFY(!engine.shouldDiscoverLocally("B/X"));
-        QVERIFY(!engine.shouldDiscoverLocally("A/X/Y"));
         QVERIFY(engine.shouldDiscoverLocally("foo bar space"));
         QVERIFY(engine.shouldDiscoverLocally("foo"));
         QVERIFY(!engine.shouldDiscoverLocally("foo bar"));
         QVERIFY(!engine.shouldDiscoverLocally("foo bar/touch"));
+        // These are within "A/X" so they should be discovered
+        QVERIFY(engine.shouldDiscoverLocally("A/X/alpha"));
+        QVERIFY(engine.shouldDiscoverLocally("A/X beta"));
+        QVERIFY(engine.shouldDiscoverLocally("A/X/Y"));
+        QVERIFY(engine.shouldDiscoverLocally("A/X space"));
+        QVERIFY(engine.shouldDiscoverLocally("A/X space/alpha"));
+        QVERIFY(!engine.shouldDiscoverLocally("A/Xylo/foo"));
+        QVERIFY(engine.shouldDiscoverLocally("zzzz/hello"));
+        QVERIFY(!engine.shouldDiscoverLocally("zzza/hello"));
+
+        QEXPECT_FAIL("", "There is a possibility of false positives if the set contains a path "
+            "which is a prefix, and that prefix is followed by a character less than '/'", Continue);
+        QVERIFY(!engine.shouldDiscoverLocally("A/X o"));
 
         fakeFolder.syncEngine().setLocalDiscoveryOptions(
             LocalDiscoveryStyle::DatabaseAndFilesystem,
@@ -149,6 +161,29 @@ private slots:
         QVERIFY(fakeFolder.currentRemoteState().find("A/a4"));
         QVERIFY(tracker.localDiscoveryPaths().empty());
     }
+
+    void testDirectoryAndSubDirectory()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+
+        fakeFolder.localModifier().mkdir("A/newDir");
+        fakeFolder.localModifier().mkdir("A/newDir/subDir");
+        fakeFolder.localModifier().insert("A/newDir/subDir/file", 10);
+
+        auto expectedState = fakeFolder.currentLocalState();
+
+        // Only "A" was modified according to the file system tracker
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(
+            LocalDiscoveryStyle::DatabaseAndFilesystem,
+            { "A" });
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(fakeFolder.currentLocalState(), expectedState);
+        QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
+    }
+
+
 };
 
 QTEST_GUILESS_MAIN(TestLocalDiscovery)