PinStates cleanup

- SyncJournalDB functions now behind internalPinStates() to avoid
accidental usage, when nearly everyone should go through Vfs.
- Rename Vfs::getPinState() to Vfs::pinState()

diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp
index 04ecff58de16c343866f0f2f8109d4ad61f9426a..fb7e8ce32c65f4a83a292baf6522469cee975edb 100644 (file)
--- a/src/common/syncjournaldb.cpp
+++ b/src/common/syncjournaldb.cpp
@@ -2086,16 +2086,16 @@ void SyncJournalDb::markVirtualFileForDownloadRecursively(const QByteArray &path
     query.exec();
 }
 
-Optional<PinState> SyncJournalDb::rawPinStateForPath(const QByteArray &path)
+Optional<PinState> SyncJournalDb::PinStateInterface::rawForPath(const QByteArray &path)
 {
-    QMutexLocker lock(&_mutex);
-    if (!checkConnect())
+    QMutexLocker lock(&_db->_mutex);
+    if (!_db->checkConnect())
         return {};
 
-    auto &query = _getRawPinStateQuery;
+    auto &query = _db->_getRawPinStateQuery;
     ASSERT(query.initOrReset(QByteArrayLiteral(
             "SELECT pinState FROM flags WHERE path == ?1;"),
-        _db));
+        _db->_db));
     query.bindValue(1, path);
     query.exec();
 
@@ -2106,13 +2106,13 @@ Optional<PinState> SyncJournalDb::rawPinStateForPath(const QByteArray &path)
     return static_cast<PinState>(query.intValue(0));
 }
 
-Optional<PinState> SyncJournalDb::effectivePinStateForPath(const QByteArray &path)
+Optional<PinState> SyncJournalDb::PinStateInterface::effectiveForPath(const QByteArray &path)
 {
-    QMutexLocker lock(&_mutex);
-    if (!checkConnect())
+    QMutexLocker lock(&_db->_mutex);
+    if (!_db->checkConnect())
         return {};
 
-    auto &query = _getEffectivePinStateQuery;
+    auto &query = _db->_getEffectivePinStateQuery;
     ASSERT(query.initOrReset(QByteArrayLiteral(
             "SELECT pinState FROM flags WHERE"
             // explicitly allow "" to represent the root path
@@ -2120,7 +2120,7 @@ Optional<PinState> SyncJournalDb::effectivePinStateForPath(const QByteArray &pat
             " (" IS_PREFIX_PATH_OR_EQUAL("path", "?1") " OR path == '')"
             " AND pinState is not null AND pinState != 0"
             " ORDER BY length(path) DESC;"),
-        _db));
+        _db->_db));
     query.bindValue(1, path);
     query.exec();
 
@@ -2131,13 +2131,13 @@ Optional<PinState> SyncJournalDb::effectivePinStateForPath(const QByteArray &pat
     return static_cast<PinState>(query.intValue(0));
 }
 
-void SyncJournalDb::setPinStateForPath(const QByteArray &path, PinState state)
+void SyncJournalDb::PinStateInterface::setForPath(const QByteArray &path, PinState state)
 {
-    QMutexLocker lock(&_mutex);
-    if (!checkConnect())
+    QMutexLocker lock(&_db->_mutex);
+    if (!_db->checkConnect())
         return;
 
-    auto &query = _setPinStateQuery;
+    auto &query = _db->_setPinStateQuery;
     ASSERT(query.initOrReset(QByteArrayLiteral(
             // If we had sqlite >=3.24.0 everywhere this could be an upsert,
             // making further flags columns easy
@@ -2145,35 +2145,36 @@ void SyncJournalDb::setPinStateForPath(const QByteArray &path, PinState state)
             //" ON CONFLICT(path) DO UPDATE SET pinState=?2;"),
             // Simple version that doesn't work nicely with multiple columns:
             "INSERT OR REPLACE INTO flags(path, pinState) VALUES(?1, ?2);"),
-        _db));
+        _db->_db));
     query.bindValue(1, path);
     query.bindValue(2, static_cast<int>(state));
     query.exec();
 }
 
-void SyncJournalDb::wipePinStateForPathAndBelow(const QByteArray &path)
+void SyncJournalDb::PinStateInterface::wipeForPathAndBelow(const QByteArray &path)
 {
-    QMutexLocker lock(&_mutex);
-    if (!checkConnect())
+    QMutexLocker lock(&_db->_mutex);
+    if (!_db->checkConnect())
         return;
 
-    auto &query = _wipePinStateQuery;
+    auto &query = _db->_wipePinStateQuery;
     ASSERT(query.initOrReset(QByteArrayLiteral(
             "DELETE FROM flags WHERE "
             // Allow "" to delete everything
             " (" IS_PREFIX_PATH_OR_EQUAL("?1", "path") " OR ?1 == '');"),
-        _db));
+        _db->_db));
     query.bindValue(1, path);
     query.exec();
 }
 
-Optional<QVector<QPair<QByteArray, PinState>>> SyncJournalDb::rawPinStates()
+Optional<QVector<QPair<QByteArray, PinState>>>
+SyncJournalDb::PinStateInterface::rawList()
 {
-    QMutexLocker lock(&_mutex);
-    if (!checkConnect())
+    QMutexLocker lock(&_db->_mutex);
+    if (!_db->checkConnect())
         return {};
 
-    SqlQuery query("SELECT path, pinState FROM flags;", _db);
+    SqlQuery query("SELECT path, pinState FROM flags;", _db->_db);
     query.exec();
 
     QVector<QPair<QByteArray, PinState>> result;
@@ -2183,6 +2184,11 @@ Optional<QVector<QPair<QByteArray, PinState>>> SyncJournalDb::rawPinStates()
     return result;
 }
 
+SyncJournalDb::PinStateInterface SyncJournalDb::internalPinStates()
+{
+    return {this};
+}
+
 void SyncJournalDb::commit(const QString &context, bool startTrans)
 {
     QMutexLocker lock(&_mutex);

diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h
index 2062581c1c1afdf786ad8c462f212da95aebc168..c1d96aced950db5c0b2991ac3810763f9fdabda6 100644 (file)
--- a/src/common/syncjournaldb.h
+++ b/src/common/syncjournaldb.h
@@ -246,58 +246,78 @@ public:
      */
     void markVirtualFileForDownloadRecursively(const QByteArray &path);
 
-    /**
-     * Gets the PinState for the path without considering parents.
-     *
-     * If a path has no explicit PinState "Inherited" is returned.
+    /** Grouping for all functions relating to pin states,
      *
-     * The path should not have a trailing slash.
-     * It's valid to use the root path "".
-     *
-     * Returns none on db error.
+     * Use internalPinStates() to get at them.
      */
-    Optional<PinState> rawPinStateForPath(const QByteArray &path);
+    struct OCSYNC_EXPORT PinStateInterface
+    {
+        PinStateInterface(const PinStateInterface &) = delete;
+        PinStateInterface(PinStateInterface &&) = delete;
 
-    /**
-     * Gets the PinState for the path after inheriting from parents.
-     *
-     * If the exact path has no entry or has an Inherited state,
-     * the state of the closest parent path is returned.
-     *
-     * The path should not have a trailing slash.
-     * It's valid to use the root path "".
-     *
-     * Never returns PinState::Inherited. If the root is "Inherited"
-     * or there's an error, "AlwaysLocal" is returned.
-     *
-     * Returns none on db error.
-     */
-    Optional<PinState> effectivePinStateForPath(const QByteArray &path);
+        /**
+         * Gets the PinState for the path without considering parents.
+         *
+         * If a path has no explicit PinState "Inherited" is returned.
+         *
+         * The path should not have a trailing slash.
+         * It's valid to use the root path "".
+         *
+         * Returns none on db error.
+         */
+        Optional<PinState> rawForPath(const QByteArray &path);
 
-    /**
-     * Sets a path's pin state.
-     *
-     * The path should not have a trailing slash.
-     * It's valid to use the root path "".
-     */
-    void setPinStateForPath(const QByteArray &path, PinState state);
+        /**
+         * Gets the PinState for the path after inheriting from parents.
+         *
+         * If the exact path has no entry or has an Inherited state,
+         * the state of the closest parent path is returned.
+         *
+         * The path should not have a trailing slash.
+         * It's valid to use the root path "".
+         *
+         * Never returns PinState::Inherited. If the root is "Inherited"
+         * or there's an error, "AlwaysLocal" is returned.
+         *
+         * Returns none on db error.
+         */
+        Optional<PinState> effectiveForPath(const QByteArray &path);
 
-    /**
-     * Wipes pin states for a path and below.
-     *
-     * Used when the user asks a subtree to have a particular pin state.
-     * The path should not have a trailing slash.
-     * The path "" wipes every entry.
-     */
-    void wipePinStateForPathAndBelow(const QByteArray &path);
+        /**
+         * Sets a path's pin state.
+         *
+         * The path should not have a trailing slash.
+         * It's valid to use the root path "".
+         */
+        void setForPath(const QByteArray &path, PinState state);
 
-    /**
-     * Returns list of all paths with their pin state as in the db.
+        /**
+         * Wipes pin states for a path and below.
+         *
+         * Used when the user asks a subtree to have a particular pin state.
+         * The path should not have a trailing slash.
+         * The path "" wipes every entry.
+         */
+        void wipeForPathAndBelow(const QByteArray &path);
+
+        /**
+         * Returns list of all paths with their pin state as in the db.
+         *
+         * Returns nothing on db error.
+         * Note that this will have an entry for "".
+         */
+        Optional<QVector<QPair<QByteArray, PinState>>> rawList();
+
+        SyncJournalDb *_db;
+    };
+    friend struct PinStateInterface;
+
+    /** Access to PinStates stored in the database.
      *
-     * Returns nothing on db error.
-     * Note that this will have an entry for "".
+     * Important: Not all vfs plugins store the pin states in the database,
+     * prefer to use Vfs::pinState() etc.
      */
-    Optional<QVector<QPair<QByteArray, PinState>>> rawPinStates();
+    PinStateInterface internalPinStates();
 
     /**
      * Only used for auto-test:

diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp
index 7e712a65472db2bad0dddf671ff5f6b7d901eb82..e9d4ca081ea04c6580805b88ab0fd67128b64c2c 100644 (file)
--- a/src/common/vfs.cpp
+++ b/src/common/vfs.cpp
@@ -73,14 +73,14 @@ void VfsDefaults::start(const VfsSetupParams &params)
 bool VfsDefaults::setPinState(const QString &folderPath, PinState state)
 {
     auto path = folderPath.toUtf8();
-    _setupParams.journal->wipePinStateForPathAndBelow(path);
-    _setupParams.journal->setPinStateForPath(path, state);
+    _setupParams.journal->internalPinStates().wipeForPathAndBelow(path);
+    _setupParams.journal->internalPinStates().setForPath(path, state);
     return true;
 }
 
-Optional<PinState> VfsDefaults::getPinState(const QString &folderPath)
+Optional<PinState> VfsDefaults::pinState(const QString &folderPath)
 {
-    return _setupParams.journal->effectivePinStateForPath(folderPath.toUtf8());
+    return _setupParams.journal->internalPinStates().effectiveForPath(folderPath.toUtf8());
 }
 
 VfsOff::VfsOff(QObject *parent)

diff --git a/src/common/vfs.h b/src/common/vfs.h
index c57e9349f30af5c731f7721a3344c99776444bc3..5d3e993ff7e788381961420fa87f133d121f1b07 100644 (file)
--- a/src/common/vfs.h
+++ b/src/common/vfs.h
@@ -191,7 +191,7 @@ public:
      *
      * folderPath is relative to the sync folder.
      */
-    virtual Optional<PinState> getPinState(const QString &folderPath) = 0;
+    virtual Optional<PinState> pinState(const QString &folderPath) = 0;
 
 public slots:
     /** Update in-sync state based on SyncFileStatusTracker signal.
@@ -219,7 +219,7 @@ public:
 
     // use the journal to back the pinstates
     bool setPinState(const QString &folderPath, PinState state) override;
-    Optional<PinState> getPinState(const QString &folderPath) override;
+    Optional<PinState> pinState(const QString &folderPath) override;
 
     // access initial setup data
     const VfsSetupParams &params() const { return _setupParams; }

diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp
index da939cf8df2ce3995312f38d1b923d91829af1ae..1cff6c12fbd872ef55df205920ec2b2768293f22 100644 (file)
--- a/src/gui/accountsettings.cpp
+++ b/src/gui/accountsettings.cpp
@@ -669,9 +669,8 @@ void AccountSettings::slotEnableVfsCurrentFolder()
             folder->setSupportsVirtualFiles(true);
             folder->setVfsOnOffSwitchPending(false);
 
-            // Wipe pin states to be sure
-            folder->journalDb()->wipePinStateForPathAndBelow("");
-            folder->journalDb()->setPinStateForPath("", PinState::OnlineOnly);
+            // Sets pin states to OnlineOnly everywhere
+            folder->setNewFilesAreVirtual(true);
 
             FolderMan::instance()->scheduleFolder(folder);
 
@@ -727,8 +726,7 @@ void AccountSettings::slotDisableVfsCurrentFolder()
             folder->setVfsOnOffSwitchPending(false);
 
             // Wipe pin states and selective sync db
-            folder->journalDb()->wipePinStateForPathAndBelow("");
-            folder->journalDb()->setPinStateForPath("", PinState::AlwaysLocal);
+            folder->setNewFilesAreVirtual(false);
             folder->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, {});
 
             FolderMan::instance()->scheduleFolder(folder);
@@ -750,6 +748,8 @@ void AccountSettings::slotDisableVfsCurrentFolder()
 
 void AccountSettings::slotSetCurrentFolderAvailability(PinState state)
 {
+    ASSERT(state == PinState::OnlineOnly || state == PinState::AlwaysLocal);
+
     FolderMan *folderMan = FolderMan::instance();
     QPointer<Folder> folder = folderMan->folder(selectedFolderAlias());
     QModelIndex selected = _ui->_folderList->selectionModel()->currentIndex();
@@ -757,8 +757,7 @@ void AccountSettings::slotSetCurrentFolderAvailability(PinState state)
         return;
 
     // similar to socket api: set pin state, wipe sub pin-states and sync
-    folder->journalDb()->wipePinStateForPathAndBelow("");
-    folder->journalDb()->setPinStateForPath("", state);
+    folder->setNewFilesAreVirtual(state == PinState::OnlineOnly);
 
     if (state == PinState::AlwaysLocal) {
         folder->downloadVirtualFile("");

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index 9828a495c9dd8a3acd87d0baab1503de59da433d..916241778d40f2945cb58bc6272d03669fba8db8 100644 (file)
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -663,13 +663,15 @@ void Folder::setSupportsVirtualFiles(bool enabled)
 
 bool Folder::newFilesAreVirtual() const
 {
-    auto pinState = _journal.rawPinStateForPath("");
+    if (!supportsVirtualFiles())
+        return false;
+    auto pinState = _vfs->pinState(QString());
     return pinState && *pinState == PinState::OnlineOnly;
 }
 
 void Folder::setNewFilesAreVirtual(bool enabled)
 {
-    _journal.setPinStateForPath("", enabled ? PinState::OnlineOnly : PinState::AlwaysLocal);
+    _vfs->setPinState(QString(), enabled ? PinState::OnlineOnly : PinState::AlwaysLocal);
 }
 
 bool Folder::supportsSelectiveSync() const

diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp
index e4f44f639b408d9bf9017454c279b544b5f93822..9b2de08d688a25e8fec93ab014578b035c301821 100644 (file)
--- a/src/gui/socketapi.cpp
+++ b/src/gui/socketapi.cpp
@@ -1019,7 +1019,7 @@ void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListe
         for (const auto &file : files) {
             auto fileData = FileData::get(file);
             auto path = fileData.folderRelativePathNoVfsSuffix();
-            auto pinState = syncFolder->vfs().getPinState(path);
+            auto pinState = syncFolder->vfs().pinState(path);
             if (!pinState) {
                 // db error
                 hasAlwaysLocal = true;

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 286c72fc5eca439a9360517e7aac1738c9e57220..28f2140b1c70df9903d9809b2fe1e4fb2c68c761 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -1360,7 +1360,7 @@ void ProcessDirectoryJob::computePinState(PinState parentState)
 {
     _pinState = parentState;
     if (_queryLocal != ParentDontExist) {
-        if (auto state = _discoveryData->_syncOptions._vfs->getPinState(_currentFolder._local)) // ouch! pin local or original?
+        if (auto state = _discoveryData->_syncOptions._vfs->pinState(_currentFolder._local)) // ouch! pin local or original?
             _pinState = *state;
     }
 }

diff --git a/test/testsyncjournaldb.cpp b/test/testsyncjournaldb.cpp
index b8f63d9fc0307f2c52f271ae10c3b400a5a9f7eb..b59de4376310d5cc6f55926fb935e8421cc20150 100644 (file)
--- a/test/testsyncjournaldb.cpp
+++ b/test/testsyncjournaldb.cpp
@@ -323,13 +323,13 @@ private slots:
     void testPinState()
     {
         auto make = [&](const QByteArray &path, PinState state) {
-            _db.setPinStateForPath(path, state);
-            auto pinState = _db.rawPinStateForPath(path);
+            _db.internalPinStates().setForPath(path, state);
+            auto pinState = _db.internalPinStates().rawForPath(path);
             QVERIFY(pinState);
             QCOMPARE(*pinState, state);
         };
         auto get = [&](const QByteArray &path) -> PinState {
-            auto state = _db.effectivePinStateForPath(path);
+            auto state = _db.internalPinStates().effectiveForPath(path);
             if (!state) {
                 QTest::qFail("couldn't read pin state", __FILE__, __LINE__);
                 return PinState::Inherited;
@@ -337,7 +337,7 @@ private slots:
             return *state;
         };
         auto getRaw = [&](const QByteArray &path) -> PinState {
-            auto state = _db.rawPinStateForPath(path);
+            auto state = _db.internalPinStates().rawForPath(path);
             if (!state) {
                 QTest::qFail("couldn't read pin state", __FILE__, __LINE__);
                 return PinState::Inherited;
@@ -345,8 +345,8 @@ private slots:
             return *state;
         };
 
-        _db.wipePinStateForPathAndBelow("");
-        auto list = _db.rawPinStates();
+        _db.internalPinStates().wipeForPathAndBelow("");
+        auto list = _db.internalPinStates().rawList();
         QCOMPARE(list->size(), 0);
 
         // Make a thrice-nested setup
@@ -366,7 +366,7 @@ private slots:
             }
         }
 
-        list = _db.rawPinStates();
+        list = _db.internalPinStates().rawList();
         QCOMPARE(list->size(), 4 + 9 + 27);
 
         // Baseline direct checks (the fallback for unset root pinstate is AlwaysLocal)
@@ -413,20 +413,20 @@ private slots:
 
         // Wiping
         QCOMPARE(getRaw("local/local"), PinState::AlwaysLocal);
-        _db.wipePinStateForPathAndBelow("local/local");
+        _db.internalPinStates().wipeForPathAndBelow("local/local");
         QCOMPARE(getRaw("local"), PinState::AlwaysLocal);
         QCOMPARE(getRaw("local/local"), PinState::Inherited);
         QCOMPARE(getRaw("local/local/local"), PinState::Inherited);
         QCOMPARE(getRaw("local/local/online"), PinState::Inherited);
-        list = _db.rawPinStates();
+        list = _db.internalPinStates().rawList();
         QCOMPARE(list->size(), 4 + 9 + 27 - 4);
 
         // Wiping everything
-        _db.wipePinStateForPathAndBelow("");
+        _db.internalPinStates().wipeForPathAndBelow("");
         QCOMPARE(getRaw(""), PinState::Inherited);
         QCOMPARE(getRaw("local"), PinState::Inherited);
         QCOMPARE(getRaw("online"), PinState::Inherited);
-        list = _db.rawPinStates();
+        list = _db.internalPinStates().rawList();
         QCOMPARE(list->size(), 0);
     }
 

diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp
index c8e8c2d6aedf8860c96260d5d742b6a7debedf47..afa9f426e39cc2bfdd6ec3e8bea5cb2b6a89b870 100644 (file)
--- a/test/testsyncvirtualfiles.cpp
+++ b/test/testsyncvirtualfiles.cpp
@@ -65,7 +65,7 @@ QSharedPointer<Vfs> setupVfs(FakeFolder &folder)
     folder.switchToVfs(suffixVfs);
 
     // Using this directly doesn't recursively unpin everything
-    folder.syncJournal().setPinStateForPath("", PinState::OnlineOnly);
+    folder.syncJournal().internalPinStates().setForPath("", PinState::OnlineOnly);
 
     return suffixVfs;
 }
@@ -433,7 +433,7 @@ private slots:
         QVERIFY(fakeFolder.syncOnce());
         QVERIFY(fakeFolder.currentLocalState().find("A/a1.nextcloud"));
 
-        fakeFolder.syncJournal().setPinStateForPath("", PinState::AlwaysLocal);
+        fakeFolder.syncJournal().internalPinStates().setForPath("", PinState::AlwaysLocal);
 
         // Create a new remote file, it'll not be virtual
         fakeFolder.remoteModifier().insert("A/a2");
@@ -748,7 +748,7 @@ private slots:
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
 
         auto setPin = [&] (const QByteArray &path, PinState state) {
-            fakeFolder.syncJournal().setPinStateForPath(path, state);
+            fakeFolder.syncJournal().internalPinStates().setForPath(path, state);
         };
 
         fakeFolder.remoteModifier().mkdir("local");
@@ -774,7 +774,7 @@ private slots:
         QVERIFY(fakeFolder.currentLocalState().find("unspec/file1.nextcloud"));
 
         // Test 2: root is AlwaysLocal
-        fakeFolder.syncJournal().setPinStateForPath("", PinState::AlwaysLocal);
+        setPin("", PinState::AlwaysLocal);
 
         fakeFolder.remoteModifier().insert("file2");
         fakeFolder.remoteModifier().insert("online/file2");