Discovery: Remove stale DB entries

And test the Remove/Remove case.

This means we need to always query the database for all the entries.
This showed another small bug in the test in which sync item for virtual
files at the root could have a slash in front of them.

src/libsync/discovery.cpp

@@ -200,23 +200,22 @@ void ProcessDirectoryJob::process()
     }
     _localEntries.clear();
 
-    if (_queryServer == ParentNotChanged || _queryLocal == ParentNotChanged) {
-        // fetch all the name from the DB
-        auto pathU8 = _currentFolder._original.toUtf8();
-        // FIXME do that better (a query that do not get stuff recursively ?)
-        if (!_discoveryData->_statedb->getFilesBelowPath(pathU8, [&](const SyncJournalFileRecord &rec) {
-                if (rec._path.indexOf("/", pathU8.size() + 1) > 0)
-                    return;
-                auto name = pathU8.isEmpty() ? rec._path : QString::fromUtf8(rec._path.mid(pathU8.size() + 1));
-                if (rec._type == ItemTypeVirtualFile || rec._type == ItemTypeVirtualFileDownload) {
-                    name.chop(_discoveryData->_syncOptions._virtualFileSuffix.size());
-                }
-                entriesNames.insert(name);
-                dbEntriesHash[name] = rec;
-            })) {
-            dbError();
-            return;
-        }
+
+    // fetch all the name from the DB
+    auto pathU8 = _currentFolder._original.toUtf8();
+    // FIXME do that better (a query that do not get stuff recursively ?)
+    if (!_discoveryData->_statedb->getFilesBelowPath(pathU8, [&](const SyncJournalFileRecord &rec) {
+            if (rec._path.indexOf("/", pathU8.size() + 1) > 0)
+                return;
+            auto name = pathU8.isEmpty() ? rec._path : QString::fromUtf8(rec._path.mid(pathU8.size() + 1));
+            if (rec._type == ItemTypeVirtualFile || rec._type == ItemTypeVirtualFileDownload) {
+                name.chop(_discoveryData->_syncOptions._virtualFileSuffix.size());
+            }
+            entriesNames.insert(name);
+            dbEntriesHash[name] = rec;
+        })) {
+        dbError();
+        return;
     }
 
 
@@ -246,10 +245,6 @@ void ProcessDirectoryJob::process()
         if (handleExcluded(path._target, localEntry.isDirectory || serverEntry.isDirectory, isHidden, localEntry.isSymLink))
             continue;
 
-        if (_queryServer != ParentNotChanged && _queryLocal != ParentNotChanged && !_discoveryData->_statedb->getFileRecord(path._original, &record)) {
-            dbError();
-            return;
-        }
         if (_queryServer == InBlackList || _discoveryData->isInSelectiveSyncBlackList(path._original)) {
             processBlacklisted(path, localEntry, record);
             continue;
@@ -604,9 +599,10 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
     } else if (dbEntry._type == ItemTypeVirtualFileDownload) {
         item->_direction = SyncFileItem::Down;
         item->_instruction = CSYNC_INSTRUCTION_NEW;
-        // (path contains the suffix)
-        item->_file = _currentFolder._target + QLatin1Char('/') + serverEntry.name;
         item->_type = ItemTypeVirtualFileDownload;
+        item->_file = path._target;
+        if (item->_file.endsWith(_discoveryData->_syncOptions._virtualFileSuffix))
+            item->_file.chop(_discoveryData->_syncOptions._virtualFileSuffix.size());
     } else if (dbEntry._etag != serverEntry.etag) {
         item->_direction = SyncFileItem::Down;
         item->_modtime = serverEntry.modtime;
@@ -908,6 +904,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
     } else if (noServerEntry) {
         // Not locally, not on the server. The entry is stale!
         qCInfo(lcDisco) << "Stale DB entry";
+        _discoveryData->_statedb->deleteFileRecord(path._original, true);
         return;
     } else if (dbEntry._type == ItemTypeVirtualFile) {
         // If the virtual file is removed, recreate it.

test/testsyncconflict.cpp

@@ -64,6 +64,13 @@ bool expectAndWipeConflict(FileModifier &local, FileInfo state, const QString pa
     return false;
 }
 
+SyncJournalFileRecord dbRecord(FakeFolder &folder, const QString &path)
+{
+    SyncJournalFileRecord record;
+    folder.syncJournal().getFileRecord(path, &record);
+    return record;
+}
+
 class TestSyncConflict : public QObject
 {
     Q_OBJECT
@@ -600,6 +607,30 @@ private slots:
         QVERIFY(fakeFolder.syncOnce());
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }
+
+    // Test what happens if we remove entries both on the server, and locally
+    void testRemoveRemove()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+        fakeFolder.remoteModifier().remove("A");
+        fakeFolder.localModifier().remove("A");
+        fakeFolder.remoteModifier().remove("B/b1");
+        fakeFolder.localModifier().remove("B/b1");
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        auto expectedState = fakeFolder.currentLocalState();
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(fakeFolder.currentLocalState(), expectedState);
+        QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
+
+        QVERIFY(dbRecord(fakeFolder, "B/b2").isValid());
+
+        QVERIFY(!dbRecord(fakeFolder, "B/b1").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a1").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A").isValid());
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncConflict)