Vfs: Clear up relationship between _type and pin state

The pin state is a per-item attribute that has an effect on _type:
AlwaysLocal dehydrated files will be marked for hydration and OnlineOnly
hydrated files will be marked for dehydration.

Where exactly this effect materializes depends on how the pin states are
stored. If they're stored in the db (suffix) the dbEntry._type is
changed during the discovery.

If the pin state is stored in the filesystem, the localEntry._type must
be adjusted by the plugin's stat callback.

This patch makes pin states behave more consistently between plugins.
Previously with suffix-vfs pin states only had an effect on new remote
files. Now the effect of pinning or unpinning files or directories is as
documented and similar to other plugins.

diff --git a/src/common/pinstate.h b/src/common/pinstate.h
index 5fb26716830f45583d13cb7c567cac5818265cd3..053e7062f0e3f1f2371509aaa34851da2245b700 100644 (file)
--- a/src/common/pinstate.h
+++ b/src/common/pinstate.h
@@ -58,7 +58,9 @@ enum class PinState {
      * Also known as "unpinned". Unpinned hydrated files shall be dehydrated
      * as soon as possible.
      *
-     * If a unpinned file becomes hydrated its pin state changes to unspecified.
+     * If a unpinned file becomes hydrated (such as due to an implicit hydration
+     * where the user requested access to the file's data) its pin state changes
+     * to Unspecified.
      */
     OnlineOnly = 2,
 

diff --git a/src/csync/csync.h b/src/csync/csync.h
index 13125cc8e97e49ce10fd60c9903215fd0c85e7b7..cc9e1c1a6ec037ed19cbd9d029d6a5be9a5458bc 100644 (file)
--- a/src/csync/csync.h
+++ b/src/csync/csync.h
@@ -142,16 +142,26 @@ enum ItemType {
 
     /** A ItemTypeVirtualFile that wants to be hydrated.
      *
-     * Actions may put this in the db as a request to a future sync.
+     * Actions may put this in the db as a request to a future sync, such as
+     * implicit hydration (when the user wants to access file data) when using
+     * suffix vfs. For pin-state driven hydrations changing the database is
+     * not necessary.
+     *
      * For some vfs plugins the placeholder files on disk may be marked for
-     * dehydration (like with a file attribute) and then the local discovery
+     * (de-)hydration (like with a file attribute) and then the local discovery
      * will return this item type.
+     *
+     * The discovery will also use this item type to mark entries for hydration
+     * if an item's pin state mandates it, such as when encountering a AlwaysLocal
+     * file that is dehydrated.
      */
     ItemTypeVirtualFileDownload = 5,
 
     /** A ItemTypeFile that wants to be dehydrated.
      *
-     * May exist in db or local files, similar to ItemTypeVirtualFileDownload.
+     * Similar to ItemTypeVirtualFileDownload, but there's currently no situation
+     * where it's stored in the database since there is no action that triggers a
+     * file dehydration without changing the pin state.
      */
     ItemTypeVirtualFileDehydration = 6,
 };

diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp
index a409b613c3bea882ca0b08e59ca041b61c702b3d..69ade78726d05122f78c1532c7359e64672b6e1c 100644 (file)
--- a/src/gui/accountsettings.cpp
+++ b/src/gui/accountsettings.cpp
@@ -762,14 +762,9 @@ void AccountSettings::slotSetCurrentFolderAvailability(PinState state)
     if (!selected.isValid() || !folder)
         return;
 
-    // similar to socket api: set pin state, wipe sub pin-states and sync
+    // similar to socket api: sets pin state recursively and sync
     folder->setNewFilesAreVirtual(state == PinState::OnlineOnly);
-
-    if (state == PinState::AlwaysLocal) {
-        folder->downloadVirtualFile("");
-    } else {
-        folder->dehydrateFile("");
-    }
+    folder->scheduleThisFolderSoon();
 }
 
 void AccountSettings::showConnectionLabel(const QString &message, QStringList errors)

diff --git a/src/gui/application.cpp b/src/gui/application.cpp
index 63edc4b0068c0a51facdbafac3915f92f6e1a8c6..5930096b5567ce061d51502bc0e8e8ddb7104018 100644 (file)
--- a/src/gui/application.cpp
+++ b/src/gui/application.cpp
@@ -793,7 +793,7 @@ void Application::openVirtualFile(const QString &filename)
         return;
     }
     QString relativePath = QDir::cleanPath(filename).mid(folder->cleanPath().length() + 1);
-    folder->downloadVirtualFile(relativePath);
+    folder->implicitlyHydrateFile(relativePath);
     QString normalName = filename.left(filename.size() - virtualFileExt.size());
     auto con = QSharedPointer<QMetaObject::Connection>::create();
     *con = QObject::connect(folder, &Folder::syncFinished, [con, normalName] {

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index 633cc92099fd6aa58e897d13198d59f314ca6e35..8ca941f28745437075016ef07cc403ffd33666a1 100644 (file)
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -590,58 +590,36 @@ void Folder::slotWatchedPathChanged(const QString &path)
     scheduleThisFolderSoon();
 }
 
-void Folder::downloadVirtualFile(const QString &_relativepath)
+void Folder::implicitlyHydrateFile(const QString &relativepath)
 {
-    qCInfo(lcFolder) << "Download virtual file: " << _relativepath;
-    auto relativepath = _relativepath.toUtf8();
+    qCInfo(lcFolder) << "Implicitly hydrate virtual file:" << relativepath;
 
     // Set in the database that we should download the file
     SyncJournalFileRecord record;
-    _journal.getFileRecord(relativepath, &record);
-    if (!record.isValid() && !relativepath.isEmpty())
+    _journal.getFileRecord(relativepath.toUtf8(), &record);
+    if (!record.isValid()) {
+        qCInfo(lcFolder) << "Did not find file in db";
         return;
-    if (record._type == ItemTypeVirtualFile) {
-        record._type = ItemTypeVirtualFileDownload;
-        _journal.setFileRecord(record);
-        // Make sure we go over that file during the discovery even if
-        // no actual remote discovery would be necessary
-        _journal.schedulePathForRemoteDiscovery(relativepath);
-    } else if (record._type == ItemTypeDirectory || relativepath.isEmpty()) {
-        _journal.markVirtualFileForDownloadRecursively(relativepath);
-    } else {
-        qCWarning(lcFolder) << "Invalid existing record " << record._type << " for file " << _relativepath;
     }
-
-    // Schedule a sync (Folder man will start the sync in a few ms)
-    slotScheduleThisFolder();
-}
-
-void Folder::dehydrateFile(const QString &_relativepath)
-{
-    qCInfo(lcFolder) << "Dehydrating file: " << _relativepath;
-    auto relativepath = _relativepath.toUtf8();
-
-    auto markForDehydration = [&](SyncJournalFileRecord rec) {
-        if (rec._type != ItemTypeFile)
-            return;
-        rec._type = ItemTypeVirtualFileDehydration;
-        _journal.setFileRecord(rec);
-        _localDiscoveryTracker->addTouchedPath(relativepath);
-    };
-
-    SyncJournalFileRecord record;
-    _journal.getFileRecord(relativepath, &record);
-    if (!record.isValid() && !relativepath.isEmpty())
+    if (!record.isVirtualFile()) {
+        qCInfo(lcFolder) << "The file is not virtual";
         return;
-    if (record._type == ItemTypeFile) {
-        markForDehydration(record);
-    } else if (record._type == ItemTypeDirectory || relativepath.isEmpty()) {
-        _journal.getFilesBelowPath(relativepath, markForDehydration);
-    } else {
-        qCWarning(lcFolder) << "Invalid existing record " << record._type << " for file " << _relativepath;
+    }
+    record._type = ItemTypeVirtualFileDownload;
+    _journal.setFileRecord(record);
+
+    // 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);
+    if (pin && *pin == PinState::OnlineOnly) {
+        _vfs->setPinState(pinPath, PinState::Unspecified);
     }
 
-    // Schedule a sync (Folder man will start the sync in a few ms)
+    // Add to local discovery
+    schedulePathForLocalDiscovery(relativepath);
     slotScheduleThisFolder();
 }
 
@@ -684,7 +662,12 @@ bool Folder::newFilesAreVirtual() const
 
 void Folder::setNewFilesAreVirtual(bool enabled)
 {
-    _vfs->setPinState(QString(), enabled ? PinState::OnlineOnly : PinState::AlwaysLocal);
+    const auto newPin = enabled ? PinState::OnlineOnly : PinState::AlwaysLocal;
+    _vfs->setPinState(QString(), newPin);
+
+    // We don't actually need discovery, but it's important to recurse
+    // into all folders, so the changes can be applied.
+    slotNextSyncFullLocalDiscovery();
 }
 
 bool Folder::supportsSelectiveSync() const

diff --git a/src/gui/folder.h b/src/gui/folder.h
index d836952de8a7779731a0b20b0138b4936bbb42ae..a88ead00d3a080f7d76a2ef083198d8de6523866 100644 (file)
--- a/src/gui/folder.h
+++ b/src/gui/folder.h
@@ -332,21 +332,21 @@ public slots:
     void slotWatchedPathChanged(const QString &path);
 
     /**
-     * Mark a virtual file as being ready for download, and start a sync.
-     * relativepath is the path to the file (including the extension)
-     * Passing a folder means that all contained virtual items shall be downloaded.
-     * A relative path of "" downloads everything.
-     */
-    void downloadVirtualFile(const QString &relativepath);
-
-    /**
-     * Turn a regular file into a dehydrated placeholder.
+     * Mark a virtual file as being requested for download, and start a sync.
+     *
+     * "implicit" here means that this download request comes from the user wanting
+     * to access the file's data. The user did not change the file's pin state.
+     * If the file is currently OnlineOnly its state will change to Unspecified.
+     *
+     * The download request is stored by setting ItemTypeVirtualFileDownload
+     * in the database. This is necessary since the hydration is not driven by
+     * the pin state.
+     *
+     * relativepath is the folder-relative path to the file (including the extension)
      *
-     * relativepath is the path to the file
-     * It's allowed to pass a path to a folder: all contained files will be dehydrated.
-     * A relative path of "" dehydrates everything.
+     * Note, passing directories is not supported. Files only.
      */
-    void dehydrateFile(const QString &relativepath);
+    void implicitlyHydrateFile(const QString &relativepath);
 
     /** Adds the path to the local discovery list
      *

diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp
index 3e62918157c1a234194ca55b4d87303f3b1c60ab..b96619d407a747aabe4f929fc22be7968c1d4e99 100644 (file)
--- a/src/gui/socketapi.cpp
+++ b/src/gui/socketapi.cpp
@@ -729,8 +729,9 @@ void SocketApi::command_MAKE_AVAILABLE_LOCALLY(const QString &filesArg, SocketLi
         auto pinPath = data.folderRelativePathNoVfsSuffix();
         data.folder->vfs().setPinState(pinPath, PinState::AlwaysLocal);
 
-        // Trigger the recursive download
-        data.folder->downloadVirtualFile(data.folderRelativePath);
+        // Trigger sync
+        data.folder->schedulePathForLocalDiscovery(data.folderRelativePath);
+        data.folder->scheduleThisFolderSoon();
     }
 }
 
@@ -748,8 +749,9 @@ void SocketApi::command_MAKE_ONLINE_ONLY(const QString &filesArg, SocketListener
         auto pinPath = data.folderRelativePathNoVfsSuffix();
         data.folder->vfs().setPinState(pinPath, PinState::OnlineOnly);
 
-        // Trigger recursive dehydration
-        data.folder->dehydrateFile(data.folderRelativePath);
+        // Trigger sync
+        data.folder->schedulePathForLocalDiscovery(data.folderRelativePath);
+        data.folder->scheduleThisFolderSoon();
     }
 }
 

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 20f566f0d33e2aad7b87861de191581116ad7f14..5cb62bbc1a179c04108ea81cd2be824452446ab6 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -88,7 +88,9 @@ void ProcessDirectoryJob::process()
             auto name = pathU8.isEmpty() ? rec._path : QString::fromUtf8(rec._path.constData() + (pathU8.size() + 1));
             if (rec.isVirtualFile() && isVfsWithSuffix())
                 chopVirtualFileSuffix(name);
-            entries[name].dbEntry = rec;
+            auto &dbEntry = entries[name].dbEntry;
+            dbEntry = rec;
+            setupDbPinStateActions(dbEntry);
         })) {
         dbError();
         return;
@@ -1456,4 +1458,30 @@ void ProcessDirectoryJob::computePinState(PinState parentState)
     }
 }
 
+void ProcessDirectoryJob::setupDbPinStateActions(SyncJournalFileRecord &record)
+{
+    // Only suffix-vfs uses the db for pin states.
+    // Other plugins will set localEntry._type according to the file's pin state.
+    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);
+    if (!pin || *pin == PinState::Inherited)
+        pin = _pinState;
+
+    // OnlineOnly hydrated files want to be dehydrated
+    if (record._type == ItemTypeFile && *pin == PinState::OnlineOnly)
+        record._type = ItemTypeVirtualFileDehydration;
+
+    // AlwaysLocal dehydrated files want to be hydrated
+    if (record._type == ItemTypeVirtualFile && *pin == PinState::AlwaysLocal)
+        record._type = ItemTypeVirtualFileDownload;
+}
+
 }

diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h
index 6cc059bbd94e56c4adf185c922bac77e33e6c3dc..20d27b73df480fe768c07aec8e4d4959c2d1f1a5 100644 (file)
--- a/src/libsync/discovery.h
+++ b/src/libsync/discovery.h
@@ -198,13 +198,25 @@ private:
       */
     bool runLocalQuery();
 
-    /** Sets _pinState
+    /** Sets _pinState, the directory's pin state
      *
      * If the folder exists locally its state is retrieved, otherwise the
      * parent's pin state is inherited.
      */
     void computePinState(PinState parentState);
 
+    /** Adjust record._type if the db pin state suggests it.
+     *
+     * If the pin state is stored in the database (suffix vfs only right now)
+     * its effects won't be seen in localEntry._type. Instead the effects
+     * should materialize in dbEntry._type.
+     *
+     * This function checks whether the combination of file type and pin
+     * state suggests a hydration or dehydration action and changes the
+     * _type field accordingly.
+     */
+    void setupDbPinStateActions(SyncJournalFileRecord &record);
+
     QueryMode _queryServer = QueryMode::NormalQuery;
     QueryMode _queryLocal = QueryMode::NormalQuery;
 
@@ -244,7 +256,7 @@ private:
     PathTuple _currentFolder;
     bool _childModified = false; // the directory contains modified item what would prevent deletion
     bool _childIgnored = false; // The directory contains ignored item that would prevent deletion
-    PinState _pinState = PinState::Unspecified; // The directory's pin-state, see setParentPinState()
+    PinState _pinState = PinState::Unspecified; // The directory's pin-state, see computePinState()
 
 signals:
     void finished();

diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp
index b32889505f0513f8077311fac4bc0a0ae59ad557..afb737c6ea3a0b627a08a5b501f398f62d3812bd 100644 (file)
--- a/test/testsyncvirtualfiles.cpp
+++ b/test/testsyncvirtualfiles.cpp
@@ -64,8 +64,9 @@ QSharedPointer<Vfs> setupVfs(FakeFolder &folder)
     auto suffixVfs = QSharedPointer<Vfs>(createVfsFromPlugin(Vfs::WithSuffix).release());
     folder.switchToVfs(suffixVfs);
 
-    // Using this directly doesn't recursively unpin everything
-    folder.syncJournal().internalPinStates().setForPath("", PinState::OnlineOnly);
+    // Using this directly doesn't recursively unpin everything and instead leaves
+    // the files in the hydration that that they start with
+    folder.syncJournal().internalPinStates().setForPath("", PinState::Unspecified);
 
     return suffixVfs;
 }
@@ -923,7 +924,7 @@ private slots:
         setPin("online", PinState::OnlineOnly);
         setPin("unspec", PinState::Unspecified);
 
-        // Test 1: root is OnlineOnly
+        // Test 1: root is Unspecified
         fakeFolder.remoteModifier().insert("file1");
         fakeFolder.remoteModifier().insert("online/file1");
         fakeFolder.remoteModifier().insert("local/file1");
@@ -935,7 +936,7 @@ private slots:
         QVERIFY(fakeFolder.currentLocalState().find("local/file1"));
         QVERIFY(fakeFolder.currentLocalState().find("unspec/file1.nextcloud"));
 
-        // Test 2: root is AlwaysLocal
+        // Test 2: change root to AlwaysLocal
         setPin("", PinState::AlwaysLocal);
 
         fakeFolder.remoteModifier().insert("file2");
@@ -949,8 +950,32 @@ private slots:
         QVERIFY(fakeFolder.currentLocalState().find("local/file2"));
         QVERIFY(fakeFolder.currentLocalState().find("unspec/file2.nextcloud"));
 
-        // file1 is unchanged
+        // root file1 was hydrated due to its new pin state
+        QVERIFY(fakeFolder.currentLocalState().find("file1"));
+
+        // file1 is unchanged in the explicitly pinned subfolders
+        QVERIFY(fakeFolder.currentLocalState().find("online/file1.nextcloud"));
+        QVERIFY(fakeFolder.currentLocalState().find("local/file1"));
+        QVERIFY(fakeFolder.currentLocalState().find("unspec/file1.nextcloud"));
+
+        // Test 3: change root to OnlineOnly
+        setPin("", PinState::OnlineOnly);
+
+        fakeFolder.remoteModifier().insert("file3");
+        fakeFolder.remoteModifier().insert("online/file3");
+        fakeFolder.remoteModifier().insert("local/file3");
+        fakeFolder.remoteModifier().insert("unspec/file3");
+        QVERIFY(fakeFolder.syncOnce());
+
+        QVERIFY(fakeFolder.currentLocalState().find("file3.nextcloud"));
+        QVERIFY(fakeFolder.currentLocalState().find("online/file3.nextcloud"));
+        QVERIFY(fakeFolder.currentLocalState().find("local/file3"));
+        QVERIFY(fakeFolder.currentLocalState().find("unspec/file3.nextcloud"));
+
+        // root file1 was dehydrated due to its new pin state
         QVERIFY(fakeFolder.currentLocalState().find("file1.nextcloud"));
+
+        // file1 is unchanged in the explicitly pinned subfolders
         QVERIFY(fakeFolder.currentLocalState().find("online/file1.nextcloud"));
         QVERIFY(fakeFolder.currentLocalState().find("local/file1"));
         QVERIFY(fakeFolder.currentLocalState().find("unspec/file1.nextcloud"));