mirrors/nextcloud-desktop
1
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2024-11-26 15:06:08 +03:00
Code Issues Projects Releases Packages Wiki Activity

SyncJournalDB: Allow callers of getFileRecord if the query failed

Browse source 
The current implementation would return the same value whether the query failed
or if no row would be found. This is something that is currently checked by csync
and needs to be provided if we want to use SyncJournalDB there.

Adjusted all call sites to also check the return value even though they
could still just rely on rec.isValid(), but makes it more explicit as to what
happens for database errors in those cases, if we ever want to gracefully handle
them.
This commit is contained in:
Jocelyn Turcotte 2017-09-13 19:02:38 +02:00 committed by Roeland Jago Douma
parent d76ecf015c
commit c6610f6fbf
No known key found for this signature in database
GPG key ID: F941078878347C0C
13 changed files with 67 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
src/common/syncjournaldb.cpp
View file

@ -972,37 +972,42 @@ bool SyncJournalDb::deleteFileRecord(const QString &filename, bool recursively)
}
SyncJournalFileRecord SyncJournalDb::getFileRecord(const QString &filename)
bool SyncJournalDb::getFileRecord(const QString &filename, SyncJournalFileRecord *rec)
{
QMutexLocker locker(&_mutex);
qlonglong phash = getPHash(filename);
SyncJournalFileRecord rec;
// Reset the output var in case the caller is reusing it.
Q_ASSERT(rec);
rec->_path.clear();
Q_ASSERT(!rec->isValid());
if (!filename.isEmpty() && checkConnect()) {
if (!checkConnect())
return false;
if (!filename.isEmpty()) {
_getFileRecordQuery->reset_and_clear_bindings();
_getFileRecordQuery->bindValue(1, phash);
_getFileRecordQuery->bindValue(1, getPHash(filename));
if (!_getFileRecordQuery->exec()) {
locker.unlock();
close();
return rec;
return false;
}
if (_getFileRecordQuery->next()) {
rec._path = _getFileRecordQuery->stringValue(0);
rec._inode = _getFileRecordQuery->intValue(1);
//rec._uid = _getFileRecordQuery->value(2).toInt(&ok); Not Used
//rec._gid = _getFileRecordQuery->value(3).toInt(&ok); Not Used
//rec._mode = _getFileRecordQuery->intValue(4);
rec._modtime = Utility::qDateTimeFromTime_t(_getFileRecordQuery->int64Value(5));
rec._type = _getFileRecordQuery->intValue(6);
rec._etag = _getFileRecordQuery->baValue(7);
rec._fileId = _getFileRecordQuery->baValue(8);
rec._remotePerm = RemotePermissions(_getFileRecordQuery->baValue(9).constData());
rec._fileSize = _getFileRecordQuery->int64Value(10);
rec._serverHasIgnoredFiles = (_getFileRecordQuery->intValue(11) > 0);
rec._checksumHeader = _getFileRecordQuery->baValue(12);
rec->_path = _getFileRecordQuery->stringValue(0);
rec->_inode = _getFileRecordQuery->intValue(1);
//rec->_uid = _getFileRecordQuery->value(2).toInt(&ok); Not Used
//rec->_gid = _getFileRecordQuery->value(3).toInt(&ok); Not Used
//rec->_mode = _getFileRecordQuery->intValue(4);
rec->_modtime = Utility::qDateTimeFromTime_t(_getFileRecordQuery->int64Value(5));
rec->_type = _getFileRecordQuery->intValue(6);
rec->_etag = _getFileRecordQuery->baValue(7);
rec->_fileId = _getFileRecordQuery->baValue(8);
rec->_remotePerm = RemotePermissions(_getFileRecordQuery->baValue(9).constData());
rec->_fileSize = _getFileRecordQuery->int64Value(10);
rec->_serverHasIgnoredFiles = (_getFileRecordQuery->intValue(11) > 0);
rec->_checksumHeader = _getFileRecordQuery->baValue(12);
} else {
int errId = _getFileRecordQuery->errorId();
if (errId != SQLITE_DONE) { // only do this if the problem is different from SQLITE_DONE
@ -1014,7 +1019,7 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord(const QString &filename)
}
}
}
return rec;
return true;
}
bool SyncJournalDb::postSyncCleanup(const QSet<QString> &filepathsToKeep,
@ -1150,10 +1155,12 @@ bool SyncJournalDb::updateLocalMetadata(const QString &filename,
bool SyncJournalDb::setFileRecordMetadata(const SyncJournalFileRecord &record)
{
SyncJournalFileRecord existing = getFileRecord(record._path);
SyncJournalFileRecord existing;
if (!getFileRecord(record._path, &existing))
return false;
// If there's no existing record, just insert the new one.
if (existing._path.isEmpty()) {
if (!existing.isValid()) {
return setFileRecord(record);
}

5
src/common/syncjournaldb.h
View file

@ -53,9 +53,8 @@ public:
/// Migrate a csync_journal to the new path, if necessary. Returns false on error
static bool maybeMigrateDb(const QString &localPath, const QString &absoluteJournalPath);
// to verify that the record could be queried successfully check
// with SyncJournalFileRecord::isValid()
SyncJournalFileRecord getFileRecord(const QString &filename);
// To verify that the record could be found check with SyncJournalFileRecord::isValid()
bool getFileRecord(const QString &filename, SyncJournalFileRecord *rec);
bool setFileRecord(const SyncJournalFileRecord &record);
/// Like setFileRecord, but preserves checksums

6
src/gui/folder.cpp
View file

@ -467,8 +467,10 @@ void Folder::slotWatchedPathChanged(const QString &path)
// Check that the mtime actually changed.
if (path.startsWith(this->path())) {
auto relativePath = path.mid(this->path().size());
auto record = _journal.getFileRecord(relativePath);
if (record.isValid() && !FileSystem::fileChanged(path, record._fileSize, Utility::qDateTimeToTime_t(record._modtime))) {
SyncJournalFileRecord record;
if (_journal.getFileRecord(relativePath, &record)
&& record.isValid()
&& !FileSystem::fileChanged(path, record._fileSize, Utility::qDateTimeToTime_t(record._modtime))) {
qCInfo(lcFolder) << "Ignoring spurious notification for file" << relativePath;
return; // probably a spurious notification
}

4
src/gui/owncloudgui.cpp
View file

@ -1002,10 +1002,10 @@ void ownCloudGui::slotShowShareDialog(const QString &sharePath, const QString &l
const auto accountState = folder->accountState();
const QString file = localPath.mid(folder->cleanPath().length() + 1);
SyncJournalFileRecord fileRecord = folder->journalDb()->getFileRecord(file);
SyncJournalFileRecord fileRecord;
bool resharingAllowed = true; // lets assume the good
if (fileRecord.isValid()) {
if (folder->journalDb()->getFileRecord(file, &fileRecord) && fileRecord.isValid()) {
// check the permission: Is resharing allowed?
if (!fileRecord._remotePerm.isNull() && !fileRecord._remotePerm.hasPermission(RemotePermissions::CanReshare)) {
resharingAllowed = false;

4
src/gui/socketapi.cpp
View file

@ -504,8 +504,8 @@ void fetchPrivateLinkUrl(const QString &localFile, SocketApi *target, void (Sock
const QString file = localFileClean.mid(shareFolder->cleanPath().length() + 1);
// Generate private link ourselves: used as a fallback
const SyncJournalFileRecord rec = shareFolder->journalDb()->getFileRecord(file);
if (!rec.isValid())
SyncJournalFileRecord rec;
if (!shareFolder->journalDb()->getFileRecord(file, &rec) || !rec.isValid())
return;
const QString oldUrl =
shareFolder->accountState()->account()->deprecatedPrivateLinkUrl(rec.numericFileId()).toString(QUrl::FullyEncoded);

4
src/libsync/owncloudpropagator.cpp
View file

@ -964,8 +964,8 @@ void CleanupPollsJob::start()
auto info = _pollInfos.first();
_pollInfos.pop_front();
SyncJournalFileRecord record = _journal->getFileRecord(info._file);
if (record.isValid()) {
SyncJournalFileRecord record;
if (_journal->getFileRecord(info._file, &record) && record.isValid()) {
SyncFileItemPtr item = SyncFileItem::fromSyncJournalFileRecord(record);
PollJob *job = new PollJob(_account, info._url, item, _journal, _localPath, this);
connect(job, &PollJob::finishedSignal, this, &CleanupPollsJob::slotPollFinished);

4
src/libsync/propagatedownload.cpp
View file

@ -706,8 +706,8 @@ namespace { // Anonymous namespace for the recall feature
// Path of the recalled file in the local folder
QString localRecalledFile = recalledFile.mid(folderPath.size());
SyncJournalFileRecord record = journal.getFileRecord(localRecalledFile);
if (!record.isValid()) {
SyncJournalFileRecord record;
if (!journal.getFileRecord(localRecalledFile, &record) || !record.isValid()) {
qCWarning(lcPropagateDownload) << "No db entry for recall of" << localRecalledFile;
continue;
}

4
src/libsync/propagateremotemove.cpp
View file

@ -162,8 +162,8 @@ void PropagateRemoteMove::slotMoveJobFinished()
void PropagateRemoteMove::finalize()
{
SyncJournalFileRecord oldRecord =
propagator()->_journal->getFileRecord(_item->_originalFile);
SyncJournalFileRecord oldRecord;
propagator()->_journal->getFileRecord(_item->_originalFile, &oldRecord);
// if reading from db failed still continue hoping that deleteFileRecord
// reopens the db successfully.
// The db is only queried to transfer the content checksum from the old

4
src/libsync/propagatorjobs.cpp
View file

@ -231,8 +231,8 @@ void PropagateLocalRename::start()
}
}
SyncJournalFileRecord oldRecord =
propagator()->_journal->getFileRecord(_item->_originalFile);
SyncJournalFileRecord oldRecord;
propagator()->_journal->getFileRecord(_item->_originalFile, &oldRecord);
propagator()->_journal->deleteFileRecord(_item->_originalFile);
// store the rename file name in the item.

6
src/libsync/syncengine.cpp
View file

@ -600,8 +600,10 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other,
}
// If the 'W' remote permission changed, update the local filesystem
SyncJournalFileRecord prev = _journal->getFileRecord(item->_file);
if (prev.isValid() && prev._remotePerm.hasPermission(RemotePermissions::CanWrite) != item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) {
SyncJournalFileRecord prev;
if (_journal->getFileRecord(item->_file, &prev)
&& prev.isValid()
&& prev._remotePerm.hasPermission(RemotePermissions::CanWrite) != item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) {
const bool isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite);
FileSystem::setFileReadOnlyWeak(filePath, isReadOnly);
}

4
src/libsync/syncfilestatustracker.cpp
View file

@ -145,8 +145,8 @@ SyncFileStatus SyncFileStatusTracker::fileStatus(const QString &relativePath)
return SyncFileStatus::StatusSync;
// First look it up in the database to know if it's shared
SyncJournalFileRecord rec = _syncEngine->journal()->getFileRecord(relativePath);
if (rec.isValid()) {
SyncJournalFileRecord rec;
if (_syncEngine->journal()->getFileRecord(relativePath, &rec) && rec.isValid()) {
return resolveSyncAndErrorStatus(relativePath, rec._remotePerm.hasPermission(RemotePermissions::IsShared) ? Shared : NotShared);
}

3
test/testsyncengine.cpp
View file

@ -109,7 +109,8 @@ private slots:
fakeFolder.syncOnce();
auto getDbChecksum = [&](QString path) {
auto record = fakeFolder.syncJournal().getFileRecord(path);
SyncJournalFileRecord record;
fakeFolder.syncJournal().getFileRecord(path, &record);
return record._checksumHeader;
};

20
test/testsyncjournaldb.cpp
View file

@ -45,7 +45,8 @@ private slots:
void testFileRecord()
{
SyncJournalFileRecord record = _db.getFileRecord("nonexistant");
SyncJournalFileRecord record;
QVERIFY(_db.getFileRecord("nonexistant", &record));
QVERIFY(!record.isValid());
record._path = "foo";
@ -59,13 +60,14 @@ private slots:
record._checksumHeader = "MD5:mychecksum";
QVERIFY(_db.setFileRecord(record));
SyncJournalFileRecord storedRecord = _db.getFileRecord("foo");
SyncJournalFileRecord storedRecord;
QVERIFY(_db.getFileRecord("foo", &storedRecord));
QVERIFY(storedRecord == record);
// Update checksum
record._checksumHeader = "Adler32:newchecksum";
_db.updateFileRecordChecksum("foo", "newchecksum", "Adler32");
storedRecord = _db.getFileRecord("foo");
QVERIFY(_db.getFileRecord("foo", &storedRecord));
QVERIFY(storedRecord == record);
// Update metadata
@ -77,11 +79,11 @@ private slots:
record._remotePerm = RemotePermissions("NV");
record._fileSize = 289055;
_db.setFileRecordMetadata(record);
storedRecord = _db.getFileRecord("foo");
QVERIFY(_db.getFileRecord("foo", &storedRecord));
QVERIFY(storedRecord == record);
QVERIFY(_db.deleteFileRecord("foo"));
record = _db.getFileRecord("foo");
QVERIFY(_db.getFileRecord("foo", &record));
QVERIFY(!record.isValid());
}
@ -96,7 +98,8 @@ private slots:
record._modtime = QDateTime::currentDateTimeUtc();
QVERIFY(_db.setFileRecord(record));
SyncJournalFileRecord storedRecord = _db.getFileRecord("foo-checksum");
SyncJournalFileRecord storedRecord;
QVERIFY(_db.getFileRecord("foo-checksum", &storedRecord));
QVERIFY(storedRecord._path == record._path);
QVERIFY(storedRecord._remotePerm == record._remotePerm);
QVERIFY(storedRecord._checksumHeader == record._checksumHeader);
@ -112,11 +115,12 @@ private slots:
SyncJournalFileRecord record;
record._path = "foo-nochecksum";
record._remotePerm = RemotePermissions("RWN");
record._modtime = QDateTime::currentDateTimeUtc();
record._modtime = QDateTime::currentDateTimeUtc();
QVERIFY(_db.setFileRecord(record));
SyncJournalFileRecord storedRecord = _db.getFileRecord("foo-nochecksum");
SyncJournalFileRecord storedRecord;
QVERIFY(_db.getFileRecord("foo-nochecksum", &storedRecord));
QVERIFY(storedRecord == record);
}
}