PinStates cleanup

- SyncJournalDB functions now behind internalPinStates() to avoid
accidental usage, when nearly everyone should go through Vfs.
- Rename Vfs::getPinState() to Vfs::pinState()
This commit is contained in:
Christian Kamm 2019-01-29 10:53:47 +01:00 committed by Kevin Ottens
parent 0eb4065197
commit 83a818678f
No known key found for this signature in database
GPG key ID: 074BBBCB8DECC9E2
10 changed files with 127 additions and 100 deletions

View file

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

View file

@ -246,58 +246,78 @@ public:
*/ */
void markVirtualFileForDownloadRecursively(const QByteArray &path); void markVirtualFileForDownloadRecursively(const QByteArray &path);
/** /** Grouping for all functions relating to pin states,
* Gets the PinState for the path without considering parents.
* *
* If a path has no explicit PinState "Inherited" is returned. * Use internalPinStates() to get at them.
*
* The path should not have a trailing slash.
* It's valid to use the root path "".
*
* Returns none on db error.
*/ */
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. * Gets the PinState for the path without considering parents.
* *
* If the exact path has no entry or has an Inherited state, * If a path has no explicit PinState "Inherited" is returned.
* the state of the closest parent path is returned. *
* * The path should not have a trailing slash.
* The path should not have a trailing slash. * It's valid to use the root path "".
* It's valid to use the root path "". *
* * Returns none on db error.
* Never returns PinState::Inherited. If the root is "Inherited" */
* or there's an error, "AlwaysLocal" is returned. Optional<PinState> rawForPath(const QByteArray &path);
*
* Returns none on db error.
*/
Optional<PinState> effectivePinStateForPath(const QByteArray &path);
/** /**
* Sets a path's pin state. * Gets the PinState for the path after inheriting from parents.
* *
* The path should not have a trailing slash. * If the exact path has no entry or has an Inherited state,
* It's valid to use the root path "". * the state of the closest parent path is returned.
*/ *
void setPinStateForPath(const QByteArray &path, PinState state); * 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. * Sets a path's pin state.
* *
* Used when the user asks a subtree to have a particular pin state. * The path should not have a trailing slash.
* The path should not have a trailing slash. * It's valid to use the root path "".
* The path "" wipes every entry. */
*/ void setForPath(const QByteArray &path, PinState state);
void wipePinStateForPathAndBelow(const QByteArray &path);
/** /**
* 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. * Important: Not all vfs plugins store the pin states in the database,
* Note that this will have an entry for "". * prefer to use Vfs::pinState() etc.
*/ */
Optional<QVector<QPair<QByteArray, PinState>>> rawPinStates(); PinStateInterface internalPinStates();
/** /**
* Only used for auto-test: * Only used for auto-test:

View file

@ -73,14 +73,14 @@ void VfsDefaults::start(const VfsSetupParams &params)
bool VfsDefaults::setPinState(const QString &folderPath, PinState state) bool VfsDefaults::setPinState(const QString &folderPath, PinState state)
{ {
auto path = folderPath.toUtf8(); auto path = folderPath.toUtf8();
_setupParams.journal->wipePinStateForPathAndBelow(path); _setupParams.journal->internalPinStates().wipeForPathAndBelow(path);
_setupParams.journal->setPinStateForPath(path, state); _setupParams.journal->internalPinStates().setForPath(path, state);
return true; 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) VfsOff::VfsOff(QObject *parent)

View file

@ -191,7 +191,7 @@ public:
* *
* folderPath is relative to the sync folder. * 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: public slots:
/** Update in-sync state based on SyncFileStatusTracker signal. /** Update in-sync state based on SyncFileStatusTracker signal.
@ -219,7 +219,7 @@ public:
// use the journal to back the pinstates // use the journal to back the pinstates
bool setPinState(const QString &folderPath, PinState state) override; 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 // access initial setup data
const VfsSetupParams &params() const { return _setupParams; } const VfsSetupParams &params() const { return _setupParams; }

View file

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

View file

@ -663,13 +663,15 @@ void Folder::setSupportsVirtualFiles(bool enabled)
bool Folder::newFilesAreVirtual() const bool Folder::newFilesAreVirtual() const
{ {
auto pinState = _journal.rawPinStateForPath(""); if (!supportsVirtualFiles())
return false;
auto pinState = _vfs->pinState(QString());
return pinState && *pinState == PinState::OnlineOnly; return pinState && *pinState == PinState::OnlineOnly;
} }
void Folder::setNewFilesAreVirtual(bool enabled) void Folder::setNewFilesAreVirtual(bool enabled)
{ {
_journal.setPinStateForPath("", enabled ? PinState::OnlineOnly : PinState::AlwaysLocal); _vfs->setPinState(QString(), enabled ? PinState::OnlineOnly : PinState::AlwaysLocal);
} }
bool Folder::supportsSelectiveSync() const bool Folder::supportsSelectiveSync() const

View file

@ -1019,7 +1019,7 @@ void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListe
for (const auto &file : files) { for (const auto &file : files) {
auto fileData = FileData::get(file); auto fileData = FileData::get(file);
auto path = fileData.folderRelativePathNoVfsSuffix(); auto path = fileData.folderRelativePathNoVfsSuffix();
auto pinState = syncFolder->vfs().getPinState(path); auto pinState = syncFolder->vfs().pinState(path);
if (!pinState) { if (!pinState) {
// db error // db error
hasAlwaysLocal = true; hasAlwaysLocal = true;

View file

@ -1360,7 +1360,7 @@ void ProcessDirectoryJob::computePinState(PinState parentState)
{ {
_pinState = parentState; _pinState = parentState;
if (_queryLocal != ParentDontExist) { 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; _pinState = *state;
} }
} }

View file

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

View file

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