Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/syncjournaldb handle errors #4819

Merged
merged 16 commits into from
Sep 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
cmake_minimum_required(VERSION 3.16)
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD 17)
cmake_policy(SET CMP0071 NEW) # Enable use of QtQuick compiler/generated code

project(client)
Expand Down
55 changes: 43 additions & 12 deletions src/common/syncjournaldb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,9 @@ void SyncJournalDb::deleteStaleFlagsEntries()
return;

SqlQuery delQuery("DELETE FROM flags WHERE path != '' AND path NOT IN (SELECT path from metadata);", _db);
delQuery.exec();
if (!delQuery.exec()) {
sqlFail(QStringLiteral("deleteStaleFlagsEntries"), delQuery);
}
}

int SyncJournalDb::errorBlackListEntryCount()
Expand Down Expand Up @@ -1862,14 +1864,18 @@ void SyncJournalDb::setPollInfo(const SyncJournalDb::PollInfo &info)
qCDebug(lcDb) << "Deleting Poll job" << info._file;
SqlQuery query("DELETE FROM async_poll WHERE path=?", _db);
query.bindValue(1, info._file);
query.exec();
if (!query.exec()) {
sqlFail(QStringLiteral("setPollInfo DELETE FROM async_poll"), query);
}
} else {
SqlQuery query("INSERT OR REPLACE INTO async_poll (path, modtime, filesize, pollpath) VALUES( ? , ? , ? , ? )", _db);
query.bindValue(1, info._file);
query.bindValue(2, info._modtime);
query.bindValue(3, info._fileSize);
query.bindValue(4, info._url);
query.exec();
if (!query.exec()) {
sqlFail(QStringLiteral("setPollInfo INSERT OR REPLACE INTO async_poll"), query);
}
}
}

Expand Down Expand Up @@ -1955,7 +1961,10 @@ void SyncJournalDb::avoidRenamesOnNextSync(const QByteArray &path)
SqlQuery query(_db);
query.prepare("UPDATE metadata SET fileid = '', inode = '0' WHERE " IS_PREFIX_PATH_OR_EQUAL("?1", "path"));
query.bindValue(1, path);
query.exec();

if (!query.exec()) {
sqlFail(QStringLiteral("avoidRenamesOnNextSync path: %1").arg(QString::fromUtf8(path)), query);
}

// We also need to remove the ETags so the update phase refreshes the directory paths
// on the next sync
Expand All @@ -1980,7 +1989,10 @@ void SyncJournalDb::schedulePathForRemoteDiscovery(const QByteArray &fileName)
// Note: CSYNC_FTW_TYPE_DIR == 2
query.prepare("UPDATE metadata SET md5='_invalid_' WHERE " IS_PREFIX_PATH_OR_EQUAL("path", "?1") " AND type == 2;");
query.bindValue(1, argument);
query.exec();

if (!query.exec()) {
sqlFail(QStringLiteral("schedulePathForRemoteDiscovery path: %1").arg(QString::fromUtf8(fileName)), query);
}

// Prevent future overwrite of the etags of this folder and all
// parent folders for this sync
Expand Down Expand Up @@ -2009,7 +2021,10 @@ void SyncJournalDb::forceRemoteDiscoveryNextSyncLocked()
qCInfo(lcDb) << "Forcing remote re-discovery by deleting folder Etags";
SqlQuery deleteRemoteFolderEtagsQuery(_db);
deleteRemoteFolderEtagsQuery.prepare("UPDATE metadata SET md5='_invalid_' WHERE type=2;");
deleteRemoteFolderEtagsQuery.exec();

if (!deleteRemoteFolderEtagsQuery.exec()) {
sqlFail(QStringLiteral("forceRemoteDiscoveryNextSyncLocked"), deleteRemoteFolderEtagsQuery);
}
}


Expand Down Expand Up @@ -2197,10 +2212,12 @@ QByteArray SyncJournalDb::conflictFileBaseName(const QByteArray &conflictName)
auto conflict = conflictRecord(conflictName);
QByteArray result;
if (conflict.isValid()) {
getFileRecordsByFileId(conflict.baseFileId, [&result](const SyncJournalFileRecord &record) {
if (!getFileRecordsByFileId(conflict.baseFileId, [&result](const SyncJournalFileRecord &record) {
if (!record._path.isEmpty())
result = record._path;
});
})) {
qCWarning(lcDb) << "conflictFileBaseName failed to getFileRecordsByFileId: " << conflictName;
}
}

if (result.isEmpty()) {
Expand All @@ -2214,7 +2231,10 @@ void SyncJournalDb::clearFileTable()
QMutexLocker lock(&_mutex);
SqlQuery query(_db);
query.prepare("DELETE FROM metadata;");
query.exec();

if (!query.exec()) {
sqlFail(QStringLiteral("clearFileTable"), query);
}
}

void SyncJournalDb::markVirtualFileForDownloadRecursively(const QByteArray &path)
Expand All @@ -2228,15 +2248,21 @@ void SyncJournalDb::markVirtualFileForDownloadRecursively(const QByteArray &path
"(" IS_PREFIX_PATH_OF("?1", "path") " OR ?1 == '') "
"AND type=4;", _db);
query.bindValue(1, path);
query.exec();

if (!query.exec()) {
sqlFail(QStringLiteral("markVirtualFileForDownloadRecursively UPDATE metadata SET type=5 path: %1").arg(QString::fromUtf8(path)), query);
}

// We also must make sure we do not read the files from the database (same logic as in schedulePathForRemoteDiscovery)
// This includes all the parents up to the root, but also all the directory within the selected dir.
static_assert(ItemTypeDirectory == 2, "");
query.prepare("UPDATE metadata SET md5='_invalid_' WHERE "
"(" IS_PREFIX_PATH_OF("?1", "path") " OR ?1 == '' OR " IS_PREFIX_PATH_OR_EQUAL("path", "?1") ") AND type == 2;");
query.bindValue(1, path);
query.exec();

if (!query.exec()) {
sqlFail(QStringLiteral("markVirtualFileForDownloadRecursively UPDATE metadata SET md5='_invalid_' path: %1").arg(QString::fromUtf8(path)), query);
}
}

Optional<PinState> SyncJournalDb::PinStateInterface::rawForPath(const QByteArray &path)
Expand Down Expand Up @@ -2366,7 +2392,12 @@ SyncJournalDb::PinStateInterface::rawList()
return {};

SqlQuery query("SELECT path, pinState FROM flags;", _db->_db);
query.exec();

if (!query.exec()) {
qCWarning(lcDb) << "SQL Error" << "PinStateInterface::rawList" << query.error();
_db->close();
ASSERT(false);
}

QVector<QPair<QByteArray, PinState>> result;
forever {
Expand Down
37 changes: 19 additions & 18 deletions src/common/syncjournaldb.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,25 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
static bool maybeMigrateDb(const QString &localPath, const QString &absoluteJournalPath);

// To verify that the record could be found check with SyncJournalFileRecord::isValid()
bool getFileRecord(const QString &filename, SyncJournalFileRecord *rec) { return getFileRecord(filename.toUtf8(), rec); }
bool getFileRecord(const QByteArray &filename, SyncJournalFileRecord *rec);
bool getFileRecordByE2eMangledName(const QString &mangledName, SyncJournalFileRecord *rec);
bool getFileRecordByInode(quint64 inode, SyncJournalFileRecord *rec);
bool getFileRecordsByFileId(const QByteArray &fileId, const std::function<void(const SyncJournalFileRecord &)> &rowCallback);
bool getFilesBelowPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
bool listFilesInPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
Result<void, QString> setFileRecord(const SyncJournalFileRecord &record);
[[nodiscard]] bool getFileRecord(const QString &filename, SyncJournalFileRecord *rec) { return getFileRecord(filename.toUtf8(), rec); }
[[nodiscard]] bool getFileRecord(const QByteArray &filename, SyncJournalFileRecord *rec);
[[nodiscard]] bool getFileRecordByE2eMangledName(const QString &mangledName, SyncJournalFileRecord *rec);
[[nodiscard]] bool getFileRecordByInode(quint64 inode, SyncJournalFileRecord *rec);
[[nodiscard]] bool getFileRecordsByFileId(const QByteArray &fileId, const std::function<void(const SyncJournalFileRecord &)> &rowCallback);
[[nodiscard]] bool getFilesBelowPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
[[nodiscard]] bool listFilesInPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
[[nodiscard]] Result<void, QString> setFileRecord(const SyncJournalFileRecord &record);

void keyValueStoreSet(const QString &key, QVariant value);
qint64 keyValueStoreGetInt(const QString &key, qint64 defaultValue);
[[nodiscard]] qint64 keyValueStoreGetInt(const QString &key, qint64 defaultValue);
void keyValueStoreDelete(const QString &key);

bool deleteFileRecord(const QString &filename, bool recursively = false);
bool updateFileRecordChecksum(const QString &filename,
[[nodiscard]] bool deleteFileRecord(const QString &filename, bool recursively = false);
[[nodiscard]] bool updateFileRecordChecksum(
const QString &filename,
const QByteArray &contentChecksum,
const QByteArray &contentChecksumType);
bool updateLocalMetadata(const QString &filename,
[[nodiscard]] bool updateLocalMetadata(const QString &filename,
qint64 modtime, qint64 size, quint64 inode);

/// Return value for hasHydratedOrDehydratedFiles()
Expand All @@ -99,7 +100,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
void setErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord &item);
void wipeErrorBlacklistEntry(const QString &file);
void wipeErrorBlacklistCategory(SyncJournalErrorBlacklistRecord::Category category);
int wipeErrorBlacklist();
[[nodiscard]] int wipeErrorBlacklist();
int errorBlackListEntryCount();

struct DownloadInfo
Expand Down Expand Up @@ -145,7 +146,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
QVector<uint> deleteStaleUploadInfos(const QSet<QString> &keep);

SyncJournalErrorBlacklistRecord errorBlacklistEntry(const QString &);
bool deleteStaleErrorBlacklistEntries(const QSet<QString> &keep);
[[nodiscard]] bool deleteStaleErrorBlacklistEntries(const QSet<QString> &keep);

/// Delete flags table entries that have no metadata correspondent
void deleteStaleFlagsEntries();
Expand Down Expand Up @@ -372,9 +373,9 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject

private:
int getFileRecordCount();
bool updateDatabaseStructure();
bool updateMetadataTableStructure();
bool updateErrorBlacklistTableStructure();
[[nodiscard]] bool updateDatabaseStructure();
[[nodiscard]] bool updateMetadataTableStructure();
[[nodiscard]] bool updateErrorBlacklistTableStructure();
bool sqlFail(const QString &log, const SqlQuery &query);
void commitInternal(const QString &context, bool startTrans = true);
void startTransaction();
Expand All @@ -388,7 +389,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
// Returns the integer id of the checksum type
//
// Returns 0 on failure and for empty checksum types.
int mapChecksumType(const QByteArray &checksumType);
[[nodiscard]] int mapChecksumType(const QByteArray &checksumType);

SqlDatabase _db;
QString _dbFile;
Expand Down
4 changes: 3 additions & 1 deletion src/gui/accountsettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,9 @@ void AccountSettings::slotSubfolderContextMenuRequested(const QModelIndex& index
// It might be an E2EE mangled path, so let's try to demangle it
const auto journal = folder->journalDb();
SyncJournalFileRecord rec;
journal->getFileRecordByE2eMangledName(remotePath, &rec);
if (!journal->getFileRecordByE2eMangledName(remotePath, &rec)) {
qCWarning(lcFolderStatus) << "Could not get file record by E2E Mangled Name from local DB" << remotePath;
}

const auto path = rec.isValid() ? rec._path : remotePath;

Expand Down
2 changes: 1 addition & 1 deletion src/gui/connectionvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ bool ConnectionValidator::setAndCheckServerVersion(const QString &version)
if (auto job = qobject_cast<AbstractNetworkJob *>(sender())) {
if (auto reply = job->reply()) {
_account->setHttp2Supported(
reply->attribute(QNetworkRequest::HTTP2WasUsedAttribute).toBool());
reply->attribute(QNetworkRequest::Http2WasUsedAttribute).toBool());
}
}
#endif
Expand Down
18 changes: 15 additions & 3 deletions src/gui/folder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,9 @@ void Folder::slotWatchedPathChanged(const QString &path, ChangeReason reason)


SyncJournalFileRecord record;
_journal.getFileRecord(relativePathBytes, &record);
if (!_journal.getFileRecord(relativePathBytes, &record)) {
qCWarning(lcFolder) << "could not get file from local DB" << relativePathBytes;
}
if (reason != ChangeReason::UnLock) {
// Check that the mtime/size actually changed or there was
// an attribute change (pin state) that caused the notification
Expand Down Expand Up @@ -613,7 +615,11 @@ void Folder::implicitlyHydrateFile(const QString &relativepath)

// Set in the database that we should download the file
SyncJournalFileRecord record;
_journal.getFileRecord(relativepath.toUtf8(), &record);
;
if (!_journal.getFileRecord(relativepath.toUtf8(), &record)) {
qCWarning(lcFolder) << "could not get file from local DB" << relativepath;
return;
}
if (!record.isValid()) {
qCInfo(lcFolder) << "Did not find file in db";
return;
Expand All @@ -622,8 +628,14 @@ void Folder::implicitlyHydrateFile(const QString &relativepath)
qCInfo(lcFolder) << "The file is not virtual";
return;
}

record._type = ItemTypeVirtualFileDownload;
_journal.setFileRecord(record);

const auto result = _journal.setFileRecord(record);
if (!result) {
qCWarning(lcFolder) << "Error when setting the file record to the database" << record._path << result.error();
return;
}

// 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)
Expand Down
2 changes: 1 addition & 1 deletion src/gui/folderstatusdelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ QRect FolderStatusDelegate::optionsButtonRect(QRect within, Qt::LayoutDirection
opt.rect.setSize(QSize(e,e));
QSize size = QApplication::style()->sizeFromContents(QStyle::CT_ToolButton, &opt, opt.rect.size()).expandedTo(QApplication::globalStrut());

int margin = QApplication::style()->pixelMetric(QStyle::PM_DefaultLayoutSpacing);
int margin = QApplication::style()->pixelMetric(QStyle::PM_LayoutHorizontalSpacing);
QRect r(QPoint(within.right() - size.width() - margin,
within.top() + within.height() / 2 - size.height() / 2),
size);
Expand Down
4 changes: 3 additions & 1 deletion src/gui/folderstatusmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,9 @@ void FolderStatusModel::slotUpdateDirectories(const QStringList &list)
newInfo._path = relativePath;

SyncJournalFileRecord rec;
parentInfo->_folder->journalDb()->getFileRecordByE2eMangledName(removeTrailingSlash(relativePath), &rec);
if (!parentInfo->_folder->journalDb()->getFileRecordByE2eMangledName(removeTrailingSlash(relativePath), &rec)) {
qCWarning(lcFolderStatus) << "Could not get file record by E2E Mangled Name from local DB" << removeTrailingSlash(relativePath);
}
if (rec.isValid()) {
newInfo._name = removeTrailingSlash(rec._path).split('/').last();
if (rec._isE2eEncrypted && !rec._e2eMangledName.isEmpty()) {
Expand Down
4 changes: 3 additions & 1 deletion src/gui/socketapi/socketapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1149,7 +1149,9 @@ SyncJournalFileRecord SocketApi::FileData::journalRecord() const
SyncJournalFileRecord record;
if (!folder)
return record;
folder->journalDb()->getFileRecord(folderRelativePath, &record);
if (!folder->journalDb()->getFileRecord(folderRelativePath, &record)) {
qCWarning(lcSocketApi) << "Failed to get journal record for path" << folderRelativePath;
}
return record;
}

Expand Down
4 changes: 3 additions & 1 deletion src/gui/tray/activitylistmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,9 @@ QVariant ActivityListModel::data(const QModelIndex &index, int role) const
// If this is an E2EE file or folder, pretend we got no path, hiding the share button which is what we want
if (folder) {
SyncJournalFileRecord rec;
folder->journalDb()->getFileRecord(fileName.mid(1), &rec);
if (!folder->journalDb()->getFileRecord(fileName.mid(1), &rec)) {
qCWarning(lcActivity) << "could not get file from local DB" << fileName.mid(1);
}
if (rec.isValid() && (rec._isE2eEncrypted || !rec._e2eMangledName.isEmpty())) {
return QString();
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/abstractnetworkjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ void AbstractNetworkJob::slotFinished()
const auto maxHttp2Resends = 3;
QByteArray verb = HttpLogger::requestVerb(*reply());
if (_reply->error() == QNetworkReply::ContentReSendError
&& _reply->attribute(QNetworkRequest::HTTP2WasUsedAttribute).toBool()) {
&& _reply->attribute(QNetworkRequest::Http2WasUsedAttribute).toBool()) {

if ((_requestBody && !_requestBody->isSequential()) || verb.isEmpty()) {
qCWarning(lcNetworkJob) << "Can't resend HTTP2 request, verb or body not suitable"
Expand Down
4 changes: 3 additions & 1 deletion src/libsync/abstractpropagateremotedeleteencrypted.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ void AbstractPropagateRemoteDeleteEncrypted::slotDeleteRemoteItemFinished()
return;
}

_propagator->_journal->deleteFileRecord(_item->_originalFile, _item->isDirectory());
if (!_propagator->_journal->deleteFileRecord(_item->_originalFile, _item->isDirectory())) {
qCWarning(ABSTRACT_PROPAGATE_REMOVE_ENCRYPTED) << "Failed to delete file record from local DB" << _item->_originalFile;
}
_propagator->_journal->commit("Remote Remove");

unlockFolder();
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/accessmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ QNetworkReply *AccessManager::createRequest(QNetworkAccessManager::Operation op,
// http2 seems to cause issues, as with our recommended server setup we don't support http2, disable it by default for now
static const bool http2EnabledEnv = qEnvironmentVariableIntValue("OWNCLOUD_HTTP2_ENABLED") == 1;

newRequest.setAttribute(QNetworkRequest::HTTP2AllowedAttribute, http2EnabledEnv);
newRequest.setAttribute(QNetworkRequest::Http2AllowedAttribute, http2EnabledEnv);
}
#endif

Expand Down
14 changes: 11 additions & 3 deletions src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
} else if (noServerEntry) {
// Not locally, not on the server. The entry is stale!
qCInfo(lcDisco) << "Stale DB entry";
_discoveryData->_statedb->deleteFileRecord(path._original, true);
if (!_discoveryData->_statedb->deleteFileRecord(path._original, true)) {
_discoveryData->fatalError(tr("Error while deleting file record %1 from the database").arg(path._original));
qCWarning(lcDisco) << "Failed to delete a file record from the local DB" << path._original;
}
return;
} else if (dbEntry._type == ItemTypeVirtualFile && isVfsWithSuffix()) {
// If the virtual file is removed, recreate it.
Expand Down Expand Up @@ -1270,7 +1273,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
if (wasDeletedOnClient.first) {
// More complicated. The REMOVE is canceled. Restore will happen next sync.
qCInfo(lcDisco) << "Undid remove instruction on source" << originalPath;
_discoveryData->_statedb->deleteFileRecord(originalPath, true);
if (!_discoveryData->_statedb->deleteFileRecord(originalPath, true)) {
qCWarning(lcDisco) << "Failed to delete a file record from the local DB" << originalPath;
}
_discoveryData->_statedb->schedulePathForRemoteDiscovery(originalPath);
_discoveryData->_anotherSyncNeeded = true;
} else {
Expand Down Expand Up @@ -1423,7 +1428,10 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
rec._fileSize = serverEntry.size;
rec._remotePerm = serverEntry.remotePerm;
rec._checksumHeader = serverEntry.checksumHeader;
_discoveryData->_statedb->setFileRecord(rec);
const auto result = _discoveryData->_statedb->setFileRecord(rec);
if (!result) {
qCWarning(lcDisco) << "Error when setting the file record to the database" << rec._path << result.error();
}
}
return;
}
Expand Down
Loading