VfsSuffix: Wipe stale pin states #7273

Previously the pin states of deleted files stayed in the 'flags'
database and could be inadvertently reused when a new file with the same
name appeared. Now they are deleted.

To make this work right, the meaning of the 'path' column in the 'flags'
table was changed: Previously it never had the .owncloud file suffix.
Now it's the same as in metadata.path.

This takes the safe parts from #7274 for inclusion in 2.6. The more
elaborate database schema changes (why use 'path' the join the two
tables in the first place?) shall go into master.

diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp
index 0645ad4633aa6f2542ce891af365aca44cb64e2f..c301ddccfed71a6adbec5c09f2ced8a6df36a00d 100644 (file)
--- a/src/common/syncjournaldb.cpp
+++ b/src/common/syncjournaldb.cpp
@@ -1656,6 +1656,16 @@ bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet<QString> &keep)
     return deleteBatch(delQuery, superfluousPaths, "blacklist");
 }
 
+void SyncJournalDb::deleteStaleFlagsEntries()
+{
+    QMutexLocker locker(&_mutex);
+    if (!checkConnect())
+        return;
+
+    SqlQuery delQuery("DELETE FROM flags WHERE path != '' AND path NOT IN (SELECT path from metadata);", _db);
+    delQuery.exec();
+}
+
 int SyncJournalDb::errorBlackListEntryCount()
 {
     int re = 0;

diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h
index 9ef1e7d72e1da91e90acbcf4f3230c236dbebe73..aa4b615ba8a3988da260cdc75ec5c52fe59a5566 100644 (file)
--- a/src/common/syncjournaldb.h
+++ b/src/common/syncjournaldb.h
@@ -141,6 +141,9 @@ public:
     SyncJournalErrorBlacklistRecord errorBlacklistEntry(const QString &);
     bool deleteStaleErrorBlacklistEntries(const QSet<QString> &keep);
 
+    /// Delete flags table entries that have no metadata correspondent
+    void deleteStaleFlagsEntries();
+
     void avoidRenamesOnNextSync(const QString &path) { avoidRenamesOnNextSync(path.toUtf8()); }
     void avoidRenamesOnNextSync(const QByteArray &path);
     void setPollInfo(const PollInfo &);

diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp
index 745f66a3bedabf3b88dd3a017803669f20745451..efec96639508f8afc4a4dfd46000d7fa53188d3b 100644 (file)
--- a/src/common/vfs.cpp
+++ b/src/common/vfs.cpp
@@ -77,14 +77,16 @@ bool Vfs::setPinStateInDb(const QString &folderPath, PinState state)
 
 Optional<PinState> Vfs::pinStateInDb(const QString &folderPath)
 {
-    return _setupParams.journal->internalPinStates().effectiveForPath(folderPath.toUtf8());
+    auto pin = _setupParams.journal->internalPinStates().effectiveForPath(folderPath.toUtf8());
+    return pin;
 }
 
-Vfs::AvailabilityResult Vfs::availabilityInDb(const QString &folderPath, const QString &pinPath)
+Vfs::AvailabilityResult Vfs::availabilityInDb(const QString &folderPath)
 {
-    auto pin = _setupParams.journal->internalPinStates().effectiveForPathRecursive(pinPath.toUtf8());
+    auto path = folderPath.toUtf8();
+    auto pin = _setupParams.journal->internalPinStates().effectiveForPathRecursive(path);
     // not being able to retrieve the pin state isn't too bad
-    auto hydrationStatus = _setupParams.journal->hasHydratedOrDehydratedFiles(folderPath.toUtf8());
+    auto hydrationStatus = _setupParams.journal->hasHydratedOrDehydratedFiles(path);
     if (!hydrationStatus)
         return AvailabilityError::DbError;
 

diff --git a/src/common/vfs.h b/src/common/vfs.h
index 916c009a6b827cda80b5079abcb4960371fc5036..e587efd36cd696874ae3cce2662a50e18051362d 100644 (file)
--- a/src/common/vfs.h
+++ b/src/common/vfs.h
@@ -257,8 +257,7 @@ protected:
     // Db-backed pin state handling. Derived classes may use it to implement pin states.
     bool setPinStateInDb(const QString &folderPath, PinState state);
     Optional<PinState> pinStateInDb(const QString &folderPath);
-    // sadly for virtual files the path in the metadata table can differ from path in 'flags'
-    AvailabilityResult availabilityInDb(const QString &folderPath, const QString &pinPath);
+    AvailabilityResult availabilityInDb(const QString &folderPath);
 
     // the parameters passed to start()
     VfsSetupParams _setupParams;

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index 5b5feb9d820cb871726fd462295ddc2fc973deed..88a1e59de491a4575a27fbf8e088ca4795bd444a 100644 (file)
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -617,12 +617,9 @@ void Folder::implicitlyHydrateFile(const QString &relativepath)
 
     // Change the file's pin state if it's contradictory to being hydrated
     // (suffix-virtual file's pin state is stored at the hydrated path)
-    QString pinPath = relativepath;
-    if (_vfs->mode() == Vfs::WithSuffix && pinPath.endsWith(_vfs->fileSuffix()))
-        pinPath.chop(_vfs->fileSuffix().size());
-    const auto pin = _vfs->pinState(pinPath);
+    const auto pin = _vfs->pinState(relativepath);
     if (pin && *pin == PinState::OnlineOnly) {
-        _vfs->setPinState(pinPath, PinState::Unspecified);
+        _vfs->setPinState(relativepath, PinState::Unspecified);
     }
 
     // Add to local discovery

diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp
index 6f3b534e907efe8d3e55e9a6cfe0871dbacfefb2..32f295ea306225fdcb686f0e244f2fd05d143d23 100644 (file)
--- a/src/gui/socketapi.cpp
+++ b/src/gui/socketapi.cpp
@@ -726,8 +726,7 @@ void SocketApi::command_MAKE_AVAILABLE_LOCALLY(const QString &filesArg, SocketLi
             continue;
 
         // Update the pin state on all items
-        auto pinPath = data.folderRelativePathNoVfsSuffix();
-        data.folder->vfs().setPinState(pinPath, PinState::AlwaysLocal);
+        data.folder->vfs().setPinState(data.folderRelativePath, PinState::AlwaysLocal);
 
         // Trigger sync
         data.folder->schedulePathForLocalDiscovery(data.folderRelativePath);
@@ -746,8 +745,7 @@ void SocketApi::command_MAKE_ONLINE_ONLY(const QString &filesArg, SocketListener
             continue;
 
         // Update the pin state on all items
-        auto pinPath = data.folderRelativePathNoVfsSuffix();
-        data.folder->vfs().setPinState(pinPath, PinState::OnlineOnly);
+        data.folder->vfs().setPinState(data.folderRelativePath, PinState::OnlineOnly);
 
         // Trigger sync
         data.folder->schedulePathForLocalDiscovery(data.folderRelativePath);

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 1d75772dd78180aba0425d7615a7bbcd899bcb40..e891c666fb22475583e7bf2161cbd59f16a59ac9 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -1468,13 +1468,7 @@ void ProcessDirectoryJob::setupDbPinStateActions(SyncJournalFileRecord &record)
     if (!isVfsWithSuffix())
         return;
 
-    QByteArray pinPath = record._path;
-    if (record.isVirtualFile()) {
-        const auto suffix = _discoveryData->_syncOptions._vfs->fileSuffix().toUtf8();
-        if (pinPath.endsWith(suffix))
-            pinPath.chop(suffix.size());
-    }
-    auto pin = _discoveryData->_statedb->internalPinStates().rawForPath(pinPath);
+    auto pin = _discoveryData->_statedb->internalPinStates().rawForPath(record._path);
     if (!pin || *pin == PinState::Inherited)
         pin = _pinState;
 

diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 92fa83951349480be53c220662ffa729a4d5b273..5bd245c490b1a41be32b6aeaa64f2f966808488f 100644 (file)
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -1027,6 +1027,13 @@ void PropagateDownloadFile::downloadFinished()
             qCDebug(lcPropagateDownload) << "Download of previous virtual file finished" << fn;
             QFile::remove(fn);
             propagator()->_journal->deleteFileRecord(virtualFile);
+
+            // Move the pin state to the new location
+            auto pin = propagator()->_journal->internalPinStates().rawForPath(_item->_file.toUtf8());
+            if (pin && *pin != PinState::Inherited) {
+                vfs->setPinState(virtualFile, *pin);
+                vfs->setPinState(_item->_file, PinState::Inherited);
+            }
         }
     }
 

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index d5bcd0cc60f4edd72879d13e20d6e06bf5ccefa0..a552cf22310cda01e73dafc04adf17221fb88fc1 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -842,6 +842,7 @@ void SyncEngine::slotPropagationFinished(bool success)
 
     conflictRecordMaintenance();
 
+    _journal->deleteStaleFlagsEntries();
     _journal->commit("All Finished.", false);
 
     // Send final progress information even if no

diff --git a/src/libsync/vfs/suffix/vfs_suffix.cpp b/src/libsync/vfs/suffix/vfs_suffix.cpp
index 5a2d829c85e7298014cc3b29e00a83cc388499fb..474e7b77ee3b07eb7c688bd79262d0454ca13ff0 100644 (file)
--- a/src/libsync/vfs/suffix/vfs_suffix.cpp
+++ b/src/libsync/vfs/suffix/vfs_suffix.cpp
@@ -18,6 +18,7 @@
 
 #include "syncfileitem.h"
 #include "filesystem.h"
+#include "common/syncjournaldb.h"
 
 namespace OCC {
 
@@ -81,6 +82,13 @@ void VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
     SyncFileItem virtualItem(item);
     virtualItem._file = item._renameTarget;
     createPlaceholder(virtualItem);
+
+    // Move the item's pin state
+    auto pin = _setupParams.journal->internalPinStates().rawForPath(item._file.toUtf8());
+    if (pin && *pin != PinState::Inherited) {
+        setPinState(item._renameTarget, *pin);
+        setPinState(item._file, PinState::Inherited);
+    }
 }
 
 void VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
@@ -107,11 +115,7 @@ bool VfsSuffix::statTypeVirtualFile(csync_file_stat_t *stat, void *)
 
 Vfs::AvailabilityResult VfsSuffix::availability(const QString &folderPath)
 {
-    const auto suffix = fileSuffix();
-    QString pinPath = folderPath;
-    if (pinPath.endsWith(suffix))
-        pinPath.chop(suffix.size());
-    return availabilityInDb(folderPath, pinPath);
+    return availabilityInDb(folderPath);
 }
 
 } // namespace OCC

diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp
index d4efc9304429f7694ba5ad937caed8b0188eda46..48e8c0e5dc1c8cf977ca96cf4dd1bd65b4ec56b0 100644 (file)
--- a/test/testsyncvirtualfiles.cpp
+++ b/test/testsyncvirtualfiles.cpp
@@ -1072,7 +1072,7 @@ private slots:
 
         triggerDownload(fakeFolder, "unspec/file1");
         setPin("local/file2", PinState::OnlineOnly);
-        setPin("online/file2", PinState::AlwaysLocal);
+        setPin("online/file2.owncloud", PinState::AlwaysLocal);
         QVERIFY(fakeFolder.syncOnce());
 
         QCOMPARE(*vfs->availability("unspec"), VfsItemAvailability::AllHydrated);
@@ -1120,7 +1120,7 @@ private slots:
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
 
         // root is unspecified
-        QCOMPARE(*vfs->pinState("file1"), PinState::Unspecified);
+        QCOMPARE(*vfs->pinState("file1.owncloud"), PinState::Unspecified);
         QCOMPARE(*vfs->pinState("local/file1"), PinState::AlwaysLocal);
         QCOMPARE(*vfs->pinState("online/file1"), PinState::Unspecified);
         QCOMPARE(*vfs->pinState("unspec/file1"), PinState::Unspecified);
@@ -1148,6 +1148,17 @@ private slots:
         QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::Unspecified);
 
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        // When a file is deleted and later a new file has the same name, the old pin
+        // state isn't preserved.
+        QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::Unspecified);
+        fakeFolder.remoteModifier().remove("onlinerenamed2/file1rename");
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::OnlineOnly);
+        fakeFolder.remoteModifier().insert("onlinerenamed2/file1rename");
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::OnlineOnly);
+        QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename.owncloud"), PinState::OnlineOnly);
     }
 };