From: Olivier Goffart <ogoffart@woboq.com>
Date: Tue, 16 Oct 2018 14:00:59 +0000 (+0200)
Subject: Discovery: Remove stale DB entries
X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~483
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=f666511a4b8c220763c119ed165ce0ded4e0ec81;p=nextcloud-desktop.git

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 236eb34e1..0a50488ef 100644
--- 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 48cf1bf0c..96275e44e 100644
--- 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)