From: Christian Kamm Date: Tue, 29 Jan 2019 09:53:47 +0000 (+0100) Subject: PinStates cleanup X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~303 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=83a818678f2d11cbd434eb9b942c6d964e1bcb92;p=nextcloud-desktop.git 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 04ecff58d..fb7e8ce32 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2086,16 +2086,16 @@ void SyncJournalDb::markVirtualFileForDownloadRecursively(const QByteArray &path query.exec(); } -Optional SyncJournalDb::rawPinStateForPath(const QByteArray &path) +Optional 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 SyncJournalDb::rawPinStateForPath(const QByteArray &path) return static_cast(query.intValue(0)); } -Optional SyncJournalDb::effectivePinStateForPath(const QByteArray &path) +Optional 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 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 SyncJournalDb::effectivePinStateForPath(const QByteArray &pat return static_cast(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(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>> SyncJournalDb::rawPinStates() +Optional>> +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> result; @@ -2183,6 +2184,11 @@ Optional>> 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 2062581c1..c1d96aced 100644 --- 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 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 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 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 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>> 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>> rawPinStates(); + PinStateInterface internalPinStates(); /** * Only used for auto-test: diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index 7e712a654..e9d4ca081 100644 --- a/src/common/vfs.cpp +++ b/src/common/vfs.cpp @@ -73,14 +73,14 @@ void VfsDefaults::start(const VfsSetupParams ¶ms) 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 VfsDefaults::getPinState(const QString &folderPath) +Optional 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 c57e9349f..5d3e993ff 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -191,7 +191,7 @@ public: * * folderPath is relative to the sync folder. */ - virtual Optional getPinState(const QString &folderPath) = 0; + virtual Optional 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 getPinState(const QString &folderPath) override; + Optional pinState(const QString &folderPath) override; // access initial setup data const VfsSetupParams ¶ms() const { return _setupParams; } diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index da939cf8d..1cff6c12f 100644 --- 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 = 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 9828a495c..916241778 100644 --- 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 e4f44f639..9b2de08d6 100644 --- 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 286c72fc5..28f2140b1 100644 --- 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 b8f63d9fc..b59de4376 100644 --- 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 c8e8c2d6a..afa9f426e 100644 --- a/test/testsyncvirtualfiles.cpp +++ b/test/testsyncvirtualfiles.cpp @@ -65,7 +65,7 @@ QSharedPointer 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");