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.

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 236eb34e18a70a6b123158c13569e1fdd73274cc..0a50488ef24c38d25eaf5695318eda74bcf151c3 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/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.

diff --git a/test/testsyncconflict.cpp b/test/testsyncconflict.cpp
index 48cf1bf0c01f03dbf4778c886c2538d65ad21743..96275e44ef280752f5d72c970d9cc393ea359270 100644 (file)
--- a/test/testsyncconflict.cpp
+++ b/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)