From de2d11125b6a7b31b9f80fdf0881603b928db4de Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Mon, 18 Jan 2021 14:36:33 +0100 Subject: [PATCH 01/33] Move Prepared sql queries to seperate class to manage access --- src/common/common.cmake | 1 + src/common/ownsql.cpp | 24 ---- src/common/ownsql.h | 35 +---- src/common/preparedsqlquerymanager.cpp | 56 ++++++++ src/common/preparedsqlquerymanager.h | 119 +++++++++++++++ src/common/syncjournaldb.cpp | 191 ++++++++++++------------- src/common/syncjournaldb.h | 43 +----- test/testownsql.cpp | 2 - 8 files changed, 275 insertions(+), 196 deletions(-) create mode 100644 src/common/preparedsqlquerymanager.cpp create mode 100644 src/common/preparedsqlquerymanager.h diff --git a/src/common/common.cmake b/src/common/common.cmake index 5c7cd52c90..ebe69f5652 100644 --- a/src/common/common.cmake +++ b/src/common/common.cmake @@ -5,6 +5,7 @@ set(common_SOURCES ${CMAKE_CURRENT_LIST_DIR}/checksums.cpp ${CMAKE_CURRENT_LIST_DIR}/filesystembase.cpp ${CMAKE_CURRENT_LIST_DIR}/ownsql.cpp + ${CMAKE_CURRENT_LIST_DIR}/preparedsqlquerymanager.cpp ${CMAKE_CURRENT_LIST_DIR}/syncjournaldb.cpp ${CMAKE_CURRENT_LIST_DIR}/syncjournalfilerecord.cpp ${CMAKE_CURRENT_LIST_DIR}/utility.cpp diff --git a/src/common/ownsql.cpp b/src/common/ownsql.cpp index 46cf96e86e..736f7f03f2 100644 --- a/src/common/ownsql.cpp +++ b/src/common/ownsql.cpp @@ -490,28 +490,4 @@ void SqlQuery::reset_and_clear_bindings() } } -PreparedSqlQueryRAII::PreparedSqlQueryRAII(SqlQuery *query) - : _query(query) -{ - Q_ASSERT(!sqlite3_stmt_busy(_query->_stmt)); -} - -PreparedSqlQueryRAII::PreparedSqlQueryRAII(SqlQuery *query, const QByteArray &sql, SqlDatabase &db) - : _query(query) -{ - Q_ASSERT(!sqlite3_stmt_busy(_query->_stmt)); - ENFORCE(!query->_sqldb || &db == query->_sqldb) - query->_sqldb = &db; - query->_db = db.sqliteDb(); - if (!query->_stmt) { - _ok = query->prepare(sql) == 0; - } -} - -PreparedSqlQueryRAII::~PreparedSqlQueryRAII() -{ - _query->reset_and_clear_bindings(); -} - - } // namespace OCC diff --git a/src/common/ownsql.h b/src/common/ownsql.h index e0d340e882..d409dc8279 100644 --- a/src/common/ownsql.h +++ b/src/common/ownsql.h @@ -168,42 +168,9 @@ private: QByteArray _sql; friend class SqlDatabase; - friend class PreparedSqlQueryRAII; + friend class PreparedSqlQueryManager; }; -class OCSYNC_EXPORT PreparedSqlQueryRAII -{ -public: - /** - * Simple Guard which allow reuse of prepared querys. - * The queries are reset in the destructor to prevent wal locks - */ - PreparedSqlQueryRAII(SqlQuery *query); - /** - * Prepare the SqlQuery if it was not prepared yet. - */ - PreparedSqlQueryRAII(SqlQuery *query, const QByteArray &sql, SqlDatabase &db); - ~PreparedSqlQueryRAII(); - - explicit operator bool() const { return _ok; } - - SqlQuery *operator->() const - { - Q_ASSERT(_ok); - return _query; - } - - SqlQuery &operator*() const & - { - Q_ASSERT(_ok); - return *_query; - } - -private: - SqlQuery *const _query; - bool _ok = true; - Q_DISABLE_COPY(PreparedSqlQueryRAII); -}; } // namespace OCC #endif // OWNSQL_H diff --git a/src/common/preparedsqlquerymanager.cpp b/src/common/preparedsqlquerymanager.cpp new file mode 100644 index 0000000000..4c748e58b3 --- /dev/null +++ b/src/common/preparedsqlquerymanager.cpp @@ -0,0 +1,56 @@ +/* + * Copyright (C) by Hannah von Reth + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + + +#include "preparedsqlquerymanager.h" + +#include + +using namespace OCC; + +PreparedSqlQuery::PreparedSqlQuery(SqlQuery *query, bool ok) + : _query(query) + , _ok(ok) +{ +} + +PreparedSqlQuery::~PreparedSqlQuery() +{ + _query->reset_and_clear_bindings(); +} + +const PreparedSqlQuery PreparedSqlQueryManager::get(PreparedSqlQueryManager::Key key) +{ + auto &query = _queries[key]; + ENFORCE(query._stmt) + Q_ASSERT(!sqlite3_stmt_busy(query._stmt)); + return { &query }; +} + +const PreparedSqlQuery PreparedSqlQueryManager::get(PreparedSqlQueryManager::Key key, const QByteArray &sql, SqlDatabase &db) +{ + auto &query = _queries[key]; + Q_ASSERT(!sqlite3_stmt_busy(query._stmt)); + ENFORCE(!query._sqldb || &db == query._sqldb) + if (!query._stmt) { + query._sqldb = &db; + query._db = db.sqliteDb(); + return { &query, query.prepare(sql) == 0 }; + } + return { &query }; +} diff --git a/src/common/preparedsqlquerymanager.h b/src/common/preparedsqlquerymanager.h new file mode 100644 index 0000000000..fa3cb4a4ae --- /dev/null +++ b/src/common/preparedsqlquerymanager.h @@ -0,0 +1,119 @@ +/* + * Copyright (C) by Hannah von Reth + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#pragma once + +#include "ocsynclib.h" +#include "ownsql.h" +#include "common/asserts.h" + +namespace OCC { + +class OCSYNC_EXPORT PreparedSqlQuery +{ +public: + ~PreparedSqlQuery(); + + explicit operator bool() const { return _ok; } + + SqlQuery *operator->() const + { + Q_ASSERT(_ok); + return _query; + } + + SqlQuery &operator*() const & + { + Q_ASSERT(_ok); + return *_query; + } + +private: + PreparedSqlQuery(SqlQuery *query, bool ok = true); + + SqlQuery *_query; + bool _ok; + + friend class PreparedSqlQueryManager; +}; + +/** + * @brief Manage PreparedSqlQuery + */ +class OCSYNC_EXPORT PreparedSqlQueryManager +{ +public: + enum Key { + GetFileRecordQuery, + GetFileRecordQueryByMangledName, + GetFileRecordQueryByInode, + GetFileRecordQueryByFileId, + GetFilesBelowPathQuery, + GetAllFilesQuery, + ListFilesInPathQuery, + SetFileRecordQuery, + SetFileRecordChecksumQuery, + SetFileRecordLocalMetadataQuery, + GetDownloadInfoQuery, + SetDownloadInfoQuery, + DeleteDownloadInfoQuery, + GetUploadInfoQuery, + SetUploadInfoQuery, + DeleteUploadInfoQuery, + DeleteFileRecordPhash, + DeleteFileRecordRecursively, + GetErrorBlacklistQuery, + SetErrorBlacklistQuery, + GetSelectiveSyncListQuery, + GetChecksumTypeIdQuery, + GetChecksumTypeQuery, + InsertChecksumTypeQuery, + GetDataFingerprintQuery, + SetDataFingerprintQuery1, + SetDataFingerprintQuery2, + SetKeyValueStoreQuery, + GetKeyValueStoreQuery, + DeleteKeyValueStoreQuery, + GetConflictRecordQuery, + SetConflictRecordQuery, + DeleteConflictRecordQuery, + GetRawPinStateQuery, + GetEffectivePinStateQuery, + GetSubPinsQuery, + CountDehydratedFilesQuery, + SetPinStateQuery, + WipePinStateQuery, + + PreparedQueryCount + }; + PreparedSqlQueryManager() = default; + /** + * The queries are reset in the destructor to prevent wal locks + */ + const PreparedSqlQuery get(Key key); + /** + * Prepare the SqlQuery if it was not prepared yet. + */ + const PreparedSqlQuery get(Key key, const QByteArray &sql, SqlDatabase &db); + +private: + SqlQuery _queries[PreparedQueryCount]; + Q_DISABLE_COPY(PreparedSqlQueryManager); +}; + +} diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index a065520898..1984447ace 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -31,6 +31,7 @@ #include "filesystembase.h" #include "common/asserts.h" #include "common/checksums.h" +#include "common/preparedsqlquerymanager.h" #include "common/c_jhash.h" @@ -586,16 +587,15 @@ bool SyncJournalDb::checkConnect() if (forceRemoteDiscovery) { forceRemoteDiscoveryNextSyncLocked(); } - - const PreparedSqlQueryRAII deleteDownloadInfo(&_deleteDownloadInfoQuery, QByteArrayLiteral("DELETE FROM downloadinfo WHERE path=?1"), _db); + const auto deleteDownloadInfo = _queryManager.get(PreparedSqlQueryManager::DeleteDownloadInfoQuery, QByteArrayLiteral("DELETE FROM downloadinfo WHERE path=?1"), _db); if (!deleteDownloadInfo) { - return sqlFail(QStringLiteral("prepare _deleteDownloadInfoQuery"), _deleteDownloadInfoQuery); + return sqlFail(QStringLiteral("prepare _deleteDownloadInfoQuery"), *deleteDownloadInfo); } - const PreparedSqlQueryRAII deleteUploadInfoQuery(&_deleteUploadInfoQuery, QByteArrayLiteral("DELETE FROM uploadinfo WHERE path=?1"), _db); + const auto deleteUploadInfoQuery = _queryManager.get(PreparedSqlQueryManager::DeleteUploadInfoQuery, QByteArrayLiteral("DELETE FROM uploadinfo WHERE path=?1"), _db); if (!deleteUploadInfoQuery) { - return sqlFail(QStringLiteral("prepare _deleteUploadInfoQuery"), _deleteUploadInfoQuery); + return sqlFail(QStringLiteral("prepare _deleteUploadInfoQuery"), *deleteUploadInfoQuery); } QByteArray sql("SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory, requestId " @@ -605,7 +605,7 @@ bool SyncJournalDb::checkConnect() // case insensitively sql += " COLLATE NOCASE"; } - const PreparedSqlQueryRAII getErrorBlacklistQuery(&_getErrorBlacklistQuery, sql, _db); + const auto getErrorBlacklistQuery = _queryManager.get(PreparedSqlQueryManager::GetErrorBlacklistQuery, sql, _db); if (!getErrorBlacklistQuery) { return sqlFail(QStringLiteral("prepare _getErrorBlacklistQuery"), *getErrorBlacklistQuery); } @@ -937,10 +937,10 @@ Result SyncJournalDb::setFileRecord(const SyncJournalFileRecord & parseChecksumHeader(record._checksumHeader, &checksumType, &checksum); int contentChecksumTypeId = mapChecksumType(checksumType); - const PreparedSqlQueryRAII query(&_setFileRecordQuery, QByteArrayLiteral("INSERT OR REPLACE INTO metadata " - "(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId, e2eMangledName, isE2eEncrypted) " - "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18);"), - _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::SetFileRecordQuery, QByteArrayLiteral("INSERT OR REPLACE INTO metadata " + "(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId, e2eMangledName, isE2eEncrypted) " + "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18);"), + _db); if (!query) { return query->error(); } @@ -985,7 +985,7 @@ void SyncJournalDb::keyValueStoreSet(const QString &key, QVariant value) return; } - const PreparedSqlQueryRAII query(&_setKeyValueStoreQuery, QByteArrayLiteral("INSERT OR REPLACE INTO key_value_store (key, value) VALUES(?1, ?2);"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::SetKeyValueStoreQuery, QByteArrayLiteral("INSERT OR REPLACE INTO key_value_store (key, value) VALUES(?1, ?2);"), _db); if (!query) { return; } @@ -1002,7 +1002,7 @@ qint64 SyncJournalDb::keyValueStoreGetInt(const QString &key, qint64 defaultValu return defaultValue; } - const PreparedSqlQueryRAII query(&_getKeyValueStoreQuery, QByteArrayLiteral("SELECT value FROM key_value_store WHERE key = ?1;"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetKeyValueStoreQuery, QByteArrayLiteral("SELECT value FROM key_value_store WHERE key = ?1;"), _db); if (!query) { return defaultValue; } @@ -1024,7 +1024,7 @@ QVariant SyncJournalDb::keyValueStoreGet(const QString &key, QVariant defaultVal return defaultValue; } - const PreparedSqlQueryRAII query(&_getKeyValueStoreQuery, QByteArrayLiteral("SELECT value FROM key_value_store WHERE key = ?1;"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetKeyValueStoreQuery, QByteArrayLiteral("SELECT value FROM key_value_store WHERE key = ?1;"), _db); if (!query) { return defaultValue; } @@ -1041,7 +1041,7 @@ QVariant SyncJournalDb::keyValueStoreGet(const QString &key, QVariant defaultVal void SyncJournalDb::keyValueStoreDelete(const QString &key) { - const PreparedSqlQueryRAII query(&_deleteKeyValueStoreQuery, QByteArrayLiteral("DELETE FROM key_value_store WHERE key=?1;"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteKeyValueStoreQuery, QByteArrayLiteral("DELETE FROM key_value_store WHERE key=?1;"), _db); if (!query) { qCWarning(lcDb) << "Failed to initOrReset _deleteKeyValueStoreQuery"; Q_ASSERT(false); @@ -1063,7 +1063,7 @@ bool SyncJournalDb::deleteFileRecord(const QString &filename, bool recursively) // always delete the actual file. { - const PreparedSqlQueryRAII query(&_deleteFileRecordPhash, QByteArrayLiteral("DELETE FROM metadata WHERE phash=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteFileRecordPhash, QByteArrayLiteral("DELETE FROM metadata WHERE phash=?1"), _db); if (!query) { return false; } @@ -1077,7 +1077,7 @@ bool SyncJournalDb::deleteFileRecord(const QString &filename, bool recursively) } if (recursively) { - const PreparedSqlQueryRAII query(&_deleteFileRecordRecursively, QByteArrayLiteral("DELETE FROM metadata WHERE " IS_PREFIX_PATH_OF("?1", "path")), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteFileRecordRecursively, QByteArrayLiteral("DELETE FROM metadata WHERE " IS_PREFIX_PATH_OF("?1", "path")), _db); if (!query) return false; query->bindValue(1, filename); @@ -1109,7 +1109,7 @@ bool SyncJournalDb::getFileRecord(const QByteArray &filename, SyncJournalFileRec return false; if (!filename.isEmpty()) { - const PreparedSqlQueryRAII query(&_getFileRecordQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE phash=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetFileRecordQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE phash=?1"), _db); if (!query) { return false; } @@ -1153,7 +1153,7 @@ bool SyncJournalDb::getFileRecordByE2eMangledName(const QString &mangledName, Sy } if (!mangledName.isEmpty()) { - const PreparedSqlQueryRAII query(&_getFileRecordQueryByMangledName, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE e2eMangledName=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetFileRecordQueryByMangledName, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE e2eMangledName=?1"), _db); if (!query) { return false; } @@ -1193,7 +1193,7 @@ bool SyncJournalDb::getFileRecordByInode(quint64 inode, SyncJournalFileRecord *r if (!checkConnect()) return false; - const PreparedSqlQueryRAII query(&_getFileRecordQueryByInode, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE inode=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetFileRecordQueryByInode, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE inode=?1"), _db); if (!query) return false; @@ -1221,7 +1221,7 @@ bool SyncJournalDb::getFileRecordsByFileId(const QByteArray &fileId, const std:: if (!checkConnect()) return false; - const PreparedSqlQueryRAII query(&_getFileRecordQueryByFileId, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE fileid=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetFileRecordQueryByFileId, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE fileid=?1"), _db); if (!query) { return false; } @@ -1281,7 +1281,7 @@ bool SyncJournalDb::getFilesBelowPath(const QByteArray &path, const std::functio // and find nothing. So, unfortunately, we have to use a different query for // retrieving the whole tree. - const PreparedSqlQueryRAII query(&_getAllFilesQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " ORDER BY path||'/' ASC"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetAllFilesQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " ORDER BY path||'/' ASC"), _db); if (!query) { return false; } @@ -1289,15 +1289,15 @@ bool SyncJournalDb::getFilesBelowPath(const QByteArray &path, const std::functio } else { // This query is used to skip discovery and fill the tree from the // database instead - const PreparedSqlQueryRAII query(&_getFilesBelowPathQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE " IS_PREFIX_PATH_OF("?1", "path") - " OR " IS_PREFIX_PATH_OF("?1", "e2eMangledName") - // We want to ensure that the contents of a directory are sorted - // directly behind the directory itself. Without this ORDER BY - // an ordering like foo, foo-2, foo/file would be returned. - // With the trailing /, we get foo-2, foo, foo/file. This property - // is used in fill_tree_from_db(). - " ORDER BY path||'/' ASC"), - _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetFilesBelowPathQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE " IS_PREFIX_PATH_OF("?1", "path") + " OR " IS_PREFIX_PATH_OF("?1", "e2eMangledName") + // We want to ensure that the contents of a directory are sorted + // directly behind the directory itself. Without this ORDER BY + // an ordering like foo, foo-2, foo/file would be returned. + // With the trailing /, we get foo-2, foo, foo/file. This property + // is used in fill_tree_from_db(). + " ORDER BY path||'/' ASC"), + _db); if (!query) { return false; } @@ -1317,7 +1317,7 @@ bool SyncJournalDb::listFilesInPath(const QByteArray& path, if (!checkConnect()) return false; - const PreparedSqlQueryRAII query(&_listFilesInPathQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE parent_hash(path) = ?1 ORDER BY path||'/' ASC"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::ListFilesInPathQuery, QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE parent_hash(path) = ?1 ORDER BY path||'/' ASC"), _db); if (!query) { return false; } @@ -1380,9 +1380,9 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString &filename, int checksumTypeId = mapChecksumType(contentChecksumType); - const PreparedSqlQueryRAII query(&_setFileRecordChecksumQuery, QByteArrayLiteral("UPDATE metadata" - " SET contentChecksum = ?2, contentChecksumTypeId = ?3" - " WHERE phash == ?1;"), + const auto query = _queryManager.get(PreparedSqlQueryManager::SetFileRecordChecksumQuery, QByteArrayLiteral("UPDATE metadata" + " SET contentChecksum = ?2, contentChecksumTypeId = ?3" + " WHERE phash == ?1;"), _db); if (!query) { return false; @@ -1407,9 +1407,9 @@ bool SyncJournalDb::updateLocalMetadata(const QString &filename, return false; } - const PreparedSqlQueryRAII query(&_setFileRecordLocalMetadataQuery, QByteArrayLiteral("UPDATE metadata" - " SET inode=?2, modtime=?3, filesize=?4" - " WHERE phash == ?1;"), + const auto query = _queryManager.get(PreparedSqlQueryManager::SetFileRecordLocalMetadataQuery, QByteArrayLiteral("UPDATE metadata" + " SET inode=?2, modtime=?3, filesize=?4" + " WHERE phash == ?1;"), _db); if (!query) { return false; @@ -1428,8 +1428,8 @@ Optional SyncJournalDb::hasHydratedOrDehyd if (!checkConnect()) return {}; - const PreparedSqlQueryRAII query(&_countDehydratedFilesQuery, QByteArrayLiteral("SELECT DISTINCT type FROM metadata" - " WHERE (" IS_PREFIX_PATH_OR_EQUAL("?1", "path") " OR ?1 == '');"), + const auto query = _queryManager.get(PreparedSqlQueryManager::CountDehydratedFilesQuery, QByteArrayLiteral("SELECT DISTINCT type FROM metadata" + " WHERE (" IS_PREFIX_PATH_OR_EQUAL("?1", "path") " OR ?1 == '');"), _db); if (!query) { return {}; @@ -1490,7 +1490,7 @@ SyncJournalDb::DownloadInfo SyncJournalDb::getDownloadInfo(const QString &file) DownloadInfo res; if (checkConnect()) { - const PreparedSqlQueryRAII query(&_getDownloadInfoQuery, QByteArrayLiteral("SELECT tmpfile, etag, errorcount FROM downloadinfo WHERE path=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetDownloadInfoQuery, QByteArrayLiteral("SELECT tmpfile, etag, errorcount FROM downloadinfo WHERE path=?1"), _db); if (!query) { return res; } @@ -1518,9 +1518,9 @@ void SyncJournalDb::setDownloadInfo(const QString &file, const SyncJournalDb::Do if (i._valid) { - const PreparedSqlQueryRAII query(&_setDownloadInfoQuery, QByteArrayLiteral("INSERT OR REPLACE INTO downloadinfo " - "(path, tmpfile, etag, errorcount) " - "VALUES ( ?1 , ?2, ?3, ?4 )"), + const auto query = _queryManager.get(PreparedSqlQueryManager::SetDownloadInfoQuery, QByteArrayLiteral("INSERT OR REPLACE INTO downloadinfo " + "(path, tmpfile, etag, errorcount) " + "VALUES ( ?1 , ?2, ?3, ?4 )"), _db); if (!query) { return; @@ -1531,7 +1531,7 @@ void SyncJournalDb::setDownloadInfo(const QString &file, const SyncJournalDb::Do query->bindValue(4, i._errorCount); query->exec(); } else { - const PreparedSqlQueryRAII query(&_deleteDownloadInfoQuery); + const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteDownloadInfoQuery); query->bindValue(1, file); query->exec(); } @@ -1568,7 +1568,7 @@ QVector SyncJournalDb::getAndDeleteStaleDownloadInf } { - const PreparedSqlQueryRAII query(&_deleteDownloadInfoQuery); + const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteDownloadInfoQuery); if (!deleteBatch(*query, superfluousPaths, QStringLiteral("downloadinfo"))) { return empty_result; } @@ -1602,8 +1602,8 @@ SyncJournalDb::UploadInfo SyncJournalDb::getUploadInfo(const QString &file) UploadInfo res; if (checkConnect()) { - const PreparedSqlQueryRAII query(&_getUploadInfoQuery, QByteArrayLiteral("SELECT chunk, transferid, errorcount, size, modtime, contentChecksum FROM " - "uploadinfo WHERE path=?1"), + const auto query = _queryManager.get(PreparedSqlQueryManager::GetUploadInfoQuery, QByteArrayLiteral("SELECT chunk, transferid, errorcount, size, modtime, contentChecksum FROM " + "uploadinfo WHERE path=?1"), _db); if (!query) { return res; @@ -1637,9 +1637,9 @@ void SyncJournalDb::setUploadInfo(const QString &file, const SyncJournalDb::Uplo } if (i._valid) { - const PreparedSqlQueryRAII query(&_setUploadInfoQuery, QByteArrayLiteral("INSERT OR REPLACE INTO uploadinfo " - "(path, chunk, transferid, errorcount, size, modtime, contentChecksum) " - "VALUES ( ?1 , ?2, ?3 , ?4 , ?5, ?6 , ?7 )"), + const auto query = _queryManager.get(PreparedSqlQueryManager::SetUploadInfoQuery, QByteArrayLiteral("INSERT OR REPLACE INTO uploadinfo " + "(path, chunk, transferid, errorcount, size, modtime, contentChecksum) " + "VALUES ( ?1 , ?2, ?3 , ?4 , ?5, ?6 , ?7 )"), _db); if (!query) { return; @@ -1657,7 +1657,7 @@ void SyncJournalDb::setUploadInfo(const QString &file, const SyncJournalDb::Uplo return; } } else { - const PreparedSqlQueryRAII query(&_deleteUploadInfoQuery); + const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteUploadInfoQuery); query->bindValue(1, file); if (!query->exec()) { @@ -1692,7 +1692,7 @@ QVector SyncJournalDb::deleteStaleUploadInfos(const QSet &keep) } } - const PreparedSqlQueryRAII deleteUploadInfoQuery(&_deleteUploadInfoQuery); + const auto deleteUploadInfoQuery = _queryManager.get(PreparedSqlQueryManager::DeleteUploadInfoQuery); deleteBatch(*deleteUploadInfoQuery, superfluousPaths, QStringLiteral("uploadinfo")); return ids; } @@ -1706,7 +1706,7 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry(const QString return entry; if (checkConnect()) { - const PreparedSqlQueryRAII query(&_getErrorBlacklistQuery); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetErrorBlacklistQuery); query->bindValue(1, file); if (query->exec()) { if (query->next().hasData) { @@ -1847,9 +1847,9 @@ void SyncJournalDb::setErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord return; } - const PreparedSqlQueryRAII query(&_setErrorBlacklistQuery, QByteArrayLiteral("INSERT OR REPLACE INTO blacklist " - "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory, requestId) " - "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)"), + const auto query = _queryManager.get(PreparedSqlQueryManager::SetErrorBlacklistQuery, QByteArrayLiteral("INSERT OR REPLACE INTO blacklist " + "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory, requestId) " + "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)"), _db); if (!query) { return; @@ -1927,7 +1927,7 @@ QStringList SyncJournalDb::getSelectiveSyncList(SyncJournalDb::SelectiveSyncList return result; } - const PreparedSqlQueryRAII query(&_getSelectiveSyncListQuery, QByteArrayLiteral("SELECT path FROM selectivesync WHERE type=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetSelectiveSyncListQuery, QByteArrayLiteral("SELECT path FROM selectivesync WHERE type=?1"), _db); if (!query) { *ok = false; return result; @@ -2064,7 +2064,7 @@ QByteArray SyncJournalDb::getChecksumType(int checksumTypeId) } // Retrieve the id - const PreparedSqlQueryRAII query(&_getChecksumTypeQuery, QByteArrayLiteral("SELECT name FROM checksumtype WHERE id=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetChecksumTypeQuery, QByteArrayLiteral("SELECT name FROM checksumtype WHERE id=?1"), _db); if (!query) { return {}; } @@ -2092,7 +2092,7 @@ int SyncJournalDb::mapChecksumType(const QByteArray &checksumType) // Ensure the checksum type is in the db { - const PreparedSqlQueryRAII query(&_insertChecksumTypeQuery, QByteArrayLiteral("INSERT OR IGNORE INTO checksumtype (name) VALUES (?1)"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::InsertChecksumTypeQuery, QByteArrayLiteral("INSERT OR IGNORE INTO checksumtype (name) VALUES (?1)"), _db); if (!query) { return 0; } @@ -2104,7 +2104,7 @@ int SyncJournalDb::mapChecksumType(const QByteArray &checksumType) // Retrieve the id { - const PreparedSqlQueryRAII query(&_getChecksumTypeIdQuery, QByteArrayLiteral("SELECT id FROM checksumtype WHERE name=?1"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetChecksumTypeIdQuery, QByteArrayLiteral("SELECT id FROM checksumtype WHERE name=?1"), _db); if (!query) { return 0; } @@ -2130,7 +2130,7 @@ QByteArray SyncJournalDb::dataFingerprint() return QByteArray(); } - const PreparedSqlQueryRAII query(&_getDataFingerprintQuery, QByteArrayLiteral("SELECT fingerprint FROM datafingerprint"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetDataFingerprintQuery, QByteArrayLiteral("SELECT fingerprint FROM datafingerprint"), _db); if (!query) { return QByteArray(); } @@ -2152,16 +2152,16 @@ void SyncJournalDb::setDataFingerprint(const QByteArray &dataFingerprint) return; } - const PreparedSqlQueryRAII setDataFingerprintQuery1(&_setDataFingerprintQuery1, QByteArrayLiteral("DELETE FROM datafingerprint;"), _db); - const PreparedSqlQueryRAII setDataFingerprintQuery2(&_setDataFingerprintQuery2, QByteArrayLiteral("INSERT INTO datafingerprint (fingerprint) VALUES (?1);"), _db); + const auto setDataFingerprintQuery1 = _queryManager.get(PreparedSqlQueryManager::SetDataFingerprintQuery1, QByteArrayLiteral("DELETE FROM datafingerprint;"), _db); + const auto setDataFingerprintQuery2 = _queryManager.get(PreparedSqlQueryManager::SetDataFingerprintQuery2, QByteArrayLiteral("INSERT INTO datafingerprint (fingerprint) VALUES (?1);"), _db); if (!setDataFingerprintQuery1 || !setDataFingerprintQuery2) { return; } - _setDataFingerprintQuery1.exec(); + setDataFingerprintQuery1->exec(); - _setDataFingerprintQuery2.bindValue(1, dataFingerprint); - _setDataFingerprintQuery2.exec(); + setDataFingerprintQuery2->bindValue(1, dataFingerprint); + setDataFingerprintQuery2->exec(); } void SyncJournalDb::setConflictRecord(const ConflictRecord &record) @@ -2170,10 +2170,10 @@ void SyncJournalDb::setConflictRecord(const ConflictRecord &record) if (!checkConnect()) return; - const PreparedSqlQueryRAII query(&_setConflictRecordQuery, QByteArrayLiteral("INSERT OR REPLACE INTO conflicts " - "(path, baseFileId, baseModtime, baseEtag, basePath) " - "VALUES (?1, ?2, ?3, ?4, ?5);"), - _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::SetConflictRecordQuery, QByteArrayLiteral("INSERT OR REPLACE INTO conflicts " + "(path, baseFileId, baseModtime, baseEtag, basePath) " + "VALUES (?1, ?2, ?3, ?4, ?5);"), + _db); ASSERT(query) query->bindValue(1, record.path); query->bindValue(2, record.baseFileId); @@ -2191,7 +2191,7 @@ ConflictRecord SyncJournalDb::conflictRecord(const QByteArray &path) if (!checkConnect()) { return entry; } - const PreparedSqlQueryRAII query(&_getConflictRecordQuery, QByteArrayLiteral("SELECT baseFileId, baseModtime, baseEtag, basePath FROM conflicts WHERE path=?1;"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::GetConflictRecordQuery, QByteArrayLiteral("SELECT baseFileId, baseModtime, baseEtag, basePath FROM conflicts WHERE path=?1;"), _db); ASSERT(query) query->bindValue(1, path); ASSERT(query->exec()) @@ -2212,7 +2212,7 @@ void SyncJournalDb::deleteConflictRecord(const QByteArray &path) if (!checkConnect()) return; - const PreparedSqlQueryRAII query(&_deleteConflictRecordQuery, QByteArrayLiteral("DELETE FROM conflicts WHERE path=?1;"), _db); + const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteConflictRecordQuery, QByteArrayLiteral("DELETE FROM conflicts WHERE path=?1;"), _db); ASSERT(query) query->bindValue(1, path); ASSERT(query->exec()) @@ -2288,7 +2288,7 @@ Optional SyncJournalDb::PinStateInterface::rawForPath(const QByteArray if (!_db->checkConnect()) return {}; - const PreparedSqlQueryRAII query(&_db->_getRawPinStateQuery, QByteArrayLiteral("SELECT pinState FROM flags WHERE path == ?1;"), _db->_db); + const auto query = _db->_queryManager.get(PreparedSqlQueryManager::GetRawPinStateQuery, QByteArrayLiteral("SELECT pinState FROM flags WHERE path == ?1;"), _db->_db); ASSERT(query) query->bindValue(1, path); query->exec(); @@ -2309,14 +2309,13 @@ Optional SyncJournalDb::PinStateInterface::effectiveForPath(const QByt if (!_db->checkConnect()) return {}; - const PreparedSqlQueryRAII query(&_db->_getEffectivePinStateQuery, QByteArrayLiteral( - "SELECT pinState FROM flags WHERE" - // explicitly allow "" to represent the root path - // (it'd be great if paths started with a / and "/" could be the root) - " (" IS_PREFIX_PATH_OR_EQUAL("path", "?1") " OR path == '')" - " AND pinState is not null AND pinState != 0" - " ORDER BY length(path) DESC LIMIT 1;"), - _db->_db); + const auto query = _db->_queryManager.get(PreparedSqlQueryManager::GetEffectivePinStateQuery, QByteArrayLiteral("SELECT pinState FROM flags WHERE" + // explicitly allow "" to represent the root path + // (it'd be great if paths started with a / and "/" could be the root) + " (" IS_PREFIX_PATH_OR_EQUAL("path", "?1") " OR path == '')" + " AND pinState is not null AND pinState != 0" + " ORDER BY length(path) DESC LIMIT 1;"), + _db->_db); ASSERT(query) query->bindValue(1, path); query->exec(); @@ -2344,10 +2343,10 @@ Optional SyncJournalDb::PinStateInterface::effectiveForPathRecursive(c return {}; // Find all the non-inherited pin states below the item - const PreparedSqlQueryRAII query(&_db->_getSubPinsQuery, QByteArrayLiteral("SELECT DISTINCT pinState FROM flags WHERE" - " (" IS_PREFIX_PATH_OF("?1", "path") " OR ?1 == '')" - " AND pinState is not null and pinState != 0;"), - _db->_db); + const auto query = _db->_queryManager.get(PreparedSqlQueryManager::GetSubPinsQuery, QByteArrayLiteral("SELECT DISTINCT pinState FROM flags WHERE" + " (" IS_PREFIX_PATH_OF("?1", "path") " OR ?1 == '')" + " AND pinState is not null and pinState != 0;"), + _db->_db); ASSERT(query) query->bindValue(1, path); query->exec(); @@ -2373,13 +2372,13 @@ void SyncJournalDb::PinStateInterface::setForPath(const QByteArray &path, PinSta if (!_db->checkConnect()) return; - const PreparedSqlQueryRAII query(&_db->_setPinStateQuery, QByteArrayLiteral( - // If we had sqlite >=3.24.0 everywhere this could be an upsert, - // making further flags columns easy - //"INSERT INTO flags(path, pinState) VALUES(?1, ?2)" - //" 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);"), + const auto query = _db->_queryManager.get(PreparedSqlQueryManager::SetPinStateQuery, QByteArrayLiteral( + // If we had sqlite >=3.24.0 everywhere this could be an upsert, + // making further flags columns easy + //"INSERT INTO flags(path, pinState) VALUES(?1, ?2)" + //" 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); ASSERT(query) query->bindValue(1, path); @@ -2393,10 +2392,10 @@ void SyncJournalDb::PinStateInterface::wipeForPathAndBelow(const QByteArray &pat if (!_db->checkConnect()) return; - const PreparedSqlQueryRAII query(&_db->_wipePinStateQuery, QByteArrayLiteral("DELETE FROM flags WHERE " - // Allow "" to delete everything - " (" IS_PREFIX_PATH_OR_EQUAL("?1", "path") " OR ?1 == '');"), - _db->_db); + const auto query = _db->_queryManager.get(PreparedSqlQueryManager::WipePinStateQuery, QByteArrayLiteral("DELETE FROM flags WHERE " + // Allow "" to delete everything + " (" IS_PREFIX_PATH_OR_EQUAL("?1", "path") " OR ?1 == '');"), + _db->_db); ASSERT(query) query->bindValue(1, path); query->exec(); diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index aa949752a8..3c77a2641f 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -28,6 +28,7 @@ #include "common/utility.h" #include "common/ownsql.h" +#include "common/preparedsqlquerymanager.h" #include "common/syncjournalfilerecord.h" #include "common/result.h" #include "common/pinstate.h" @@ -397,46 +398,6 @@ private: int _transaction; bool _metadataTableIsEmpty; - SqlQuery _getFileRecordQuery; - SqlQuery _getFileRecordQueryByMangledName; - SqlQuery _getFileRecordQueryByInode; - SqlQuery _getFileRecordQueryByFileId; - SqlQuery _getFilesBelowPathQuery; - SqlQuery _getAllFilesQuery; - SqlQuery _listFilesInPathQuery; - SqlQuery _setFileRecordQuery; - SqlQuery _setFileRecordChecksumQuery; - SqlQuery _setFileRecordLocalMetadataQuery; - SqlQuery _getDownloadInfoQuery; - SqlQuery _setDownloadInfoQuery; - SqlQuery _deleteDownloadInfoQuery; - SqlQuery _getUploadInfoQuery; - SqlQuery _setUploadInfoQuery; - SqlQuery _deleteUploadInfoQuery; - SqlQuery _deleteFileRecordPhash; - SqlQuery _deleteFileRecordRecursively; - SqlQuery _getErrorBlacklistQuery; - SqlQuery _setErrorBlacklistQuery; - SqlQuery _getSelectiveSyncListQuery; - SqlQuery _getChecksumTypeIdQuery; - SqlQuery _getChecksumTypeQuery; - SqlQuery _insertChecksumTypeQuery; - SqlQuery _getDataFingerprintQuery; - SqlQuery _setDataFingerprintQuery1; - SqlQuery _setDataFingerprintQuery2; - SqlQuery _setKeyValueStoreQuery; - SqlQuery _getKeyValueStoreQuery; - SqlQuery _deleteKeyValueStoreQuery; - SqlQuery _getConflictRecordQuery; - SqlQuery _setConflictRecordQuery; - SqlQuery _deleteConflictRecordQuery; - SqlQuery _getRawPinStateQuery; - SqlQuery _getEffectivePinStateQuery; - SqlQuery _getSubPinsQuery; - SqlQuery _countDehydratedFilesQuery; - SqlQuery _setPinStateQuery; - SqlQuery _wipePinStateQuery; - /* Storing etags to these folders, or their parent folders, is filtered out. * * When schedulePathForRemoteDiscovery() is called some etags to _invalid_ in the @@ -458,6 +419,8 @@ private: * variable, for specific filesystems, or when WAL fails in a particular way. */ QByteArray _journalMode; + + PreparedSqlQueryManager _queryManager; }; bool OCSYNC_EXPORT diff --git a/test/testownsql.cpp b/test/testownsql.cpp index abfb72db01..167c53be88 100644 --- a/test/testownsql.cpp +++ b/test/testownsql.cpp @@ -136,8 +136,6 @@ private slots: q2.prepare("SELECT * FROM addresses"); SqlQuery q3("SELECT * FROM addresses", _db); SqlQuery q4; - SqlQuery q5; - PreparedSqlQueryRAII testQuery(&q5, "SELECT * FROM addresses", _db); db.reset(); } From 324d5a04c60426c95a3d4c7482f469e4c9194508 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 19 Jan 2021 09:09:27 +0100 Subject: [PATCH 02/33] Align type used for getPHash --- src/common/syncjournaldb.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 1984447ace..11dc84458b 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -892,7 +892,7 @@ QVector SyncJournalDb::tableColumns(const QByteArray &table) qint64 SyncJournalDb::getPHash(const QByteArray &file) { - int64_t h = 0; + qint64 h = 0; int len = file.length(); h = c_jhash64((uint8_t *)file.data(), len, 0); @@ -922,7 +922,7 @@ Result SyncJournalDb::setFileRecord(const SyncJournalFileRecord & << "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader << "e2eMangledName:" << record.e2eMangledName() << "isE2eEncrypted:" << record._isE2eEncrypted; - qlonglong phash = getPHash(record._path); + const qint64 phash = getPHash(record._path); if (checkConnect()) { int plen = record._path.length(); @@ -1068,7 +1068,7 @@ bool SyncJournalDb::deleteFileRecord(const QString &filename, bool recursively) return false; } - qlonglong phash = getPHash(filename.toUtf8()); + const qint64 phash = getPHash(filename.toUtf8()); query->bindValue(1, phash); if (!query->exec()) { @@ -1372,7 +1372,7 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString &filename, qCInfo(lcDb) << "Updating file checksum" << filename << contentChecksum << contentChecksumType; - qlonglong phash = getPHash(filename.toUtf8()); + const qint64 phash = getPHash(filename.toUtf8()); if (!checkConnect()) { qCWarning(lcDb) << "Failed to connect database."; return false; @@ -1401,7 +1401,7 @@ bool SyncJournalDb::updateLocalMetadata(const QString &filename, qCInfo(lcDb) << "Updating local metadata for:" << filename << modtime << size << inode; - qlonglong phash = getPHash(filename.toUtf8()); + const qint64 phash = getPHash(filename.toUtf8()); if (!checkConnect()) { qCWarning(lcDb) << "Failed to connect database."; return false; From 557b11aca701bf278aa6b222b777f12626a52ac2 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 19 Jan 2021 12:42:58 +0100 Subject: [PATCH 03/33] Include os version 'windows-10.0.19042' in about dialog --- src/libsync/theme.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libsync/theme.cpp b/src/libsync/theme.cpp index aaaf0774a5..ffe8f15e32 100644 --- a/src/libsync/theme.cpp +++ b/src/libsync/theme.cpp @@ -468,6 +468,8 @@ QString Theme::about() const devString += tr("

Using virtual files plugin: %1

") .arg(Vfs::modeToString(bestAvailableVfsMode())); + devString += tr("
%1") + .arg(QSysInfo::productType() % QLatin1Char('-') % QSysInfo::kernelVersion()); return devString; } From 3a8706734819fdb986458063a1b18b9139edb6b7 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 19 Jan 2021 14:28:04 +0100 Subject: [PATCH 04/33] Cleanup --- test/testsyncengine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index d241606595..0d3ffeb40b 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -336,7 +336,7 @@ private slots: }); // For directly editing the remote checksum - auto &remoteInfo = dynamic_cast(fakeFolder.remoteModifier()); + auto &remoteInfo = fakeFolder.remoteModifier(); // Base mtime with no ms content (filesystem is seconds only) auto mtime = QDateTime::currentDateTimeUtc().addDays(-4); From 7715583b14ba9d645a02f422ee2344ed142f5fad Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 19 Jan 2021 14:28:51 +0100 Subject: [PATCH 05/33] Correctly use indexOf --- src/common/checksums.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/common/checksums.cpp b/src/common/checksums.cpp index c6857e549b..0c016ef4da 100644 --- a/src/common/checksums.cpp +++ b/src/common/checksums.cpp @@ -144,6 +144,9 @@ QByteArray makeChecksumHeader(const QByteArray &checksumType, const QByteArray & QByteArray findBestChecksum(const QByteArray &_checksums) { + if (_checksums.isEmpty()) { + return {}; + } const auto checksums = QString::fromUtf8(_checksums); int i = 0; // The order of the searches here defines the preference ordering. @@ -162,7 +165,7 @@ QByteArray findBestChecksum(const QByteArray &_checksums) return _checksums.mid(i, end - i); } qCWarning(lcChecksums) << "Failed to parse" << _checksums; - return QByteArray(); + return {}; } bool parseChecksumHeader(const QByteArray &header, QByteArray *type, QByteArray *checksum) From 73549a052951e761142c00f8c3c9bbda68f63cfb Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Thu, 21 Jan 2021 13:10:05 +0100 Subject: [PATCH 06/33] Ignore the desktop.ini file in every directory, not only in top dir. (#8299) * Ignore the desktop.ini file in every directory, not only in top dir. See https://github.com/owncloud/client/issues/8298 for reasons. * Fix test for ignoring desktop.ini everywhere. Co-authored-by: Hannah von Reth --- src/csync/csync_exclude.cpp | 10 +++++----- test/testexcludedfiles.cpp | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/csync/csync_exclude.cpp b/src/csync/csync_exclude.cpp index f9968ae7b8..84510252bf 100644 --- a/src/csync/csync_exclude.cpp +++ b/src/csync/csync_exclude.cpp @@ -199,13 +199,13 @@ static CSYNC_EXCLUDE_TYPE _csync_excluded_common(const QString &path, bool exclu } #endif - /* We create a Desktop.ini on Windows for the sidebar icon, make sure we don't sync it. */ - if (blen == 11 && path == bname) { - if (bname.compare(QLatin1String("Desktop.ini"), Qt::CaseInsensitive) == 0) { - return CSYNC_FILE_SILENTLY_EXCLUDED; - } + /* Do not sync desktop.ini files anywhere in the tree. */ + const auto desktopIniFile = QStringLiteral("desktop.ini"); + if (blen == static_cast(desktopIniFile.length()) && bname.compare(desktopIniFile, Qt::CaseInsensitive) == 0) { + return CSYNC_FILE_SILENTLY_EXCLUDED; } + if (excludeConflictFiles && OCC::Utility::isConflictFile(path)) { return CSYNC_FILE_EXCLUDE_CONFLICT; } diff --git a/test/testexcludedfiles.cpp b/test/testexcludedfiles.cpp index ba48e5c2c8..94b23b5b45 100644 --- a/test/testexcludedfiles.cpp +++ b/test/testexcludedfiles.cpp @@ -322,7 +322,7 @@ private slots: QCOMPARE(check_file_traversal("subdir/.sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED); /* Other builtin excludes */ - QCOMPARE(check_file_traversal("foo/Desktop.ini"), CSYNC_NOT_EXCLUDED); + QCOMPARE(check_file_traversal("foo/Desktop.ini"), CSYNC_FILE_SILENTLY_EXCLUDED); QCOMPARE(check_file_traversal("Desktop.ini"), CSYNC_FILE_SILENTLY_EXCLUDED); /* pattern ]*.directory - ignore and remove */ From 90b733801e6000cfa8a729e6646e4ba77102e7a4 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Wed, 27 Jan 2021 12:39:57 +0100 Subject: [PATCH 07/33] Simplify uuid handling --- src/libsync/accessmanager.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libsync/accessmanager.cpp b/src/libsync/accessmanager.cpp index da32d912a6..fa04e6c6bf 100644 --- a/src/libsync/accessmanager.cpp +++ b/src/libsync/accessmanager.cpp @@ -51,9 +51,7 @@ AccessManager::AccessManager(QObject *parent) QByteArray AccessManager::generateRequestId() { - // Use a UUID with the starting and ending curly brace removed. - auto uuid = QUuid::createUuid().toByteArray(); - return uuid.mid(1, uuid.size() - 2); + return QUuid::createUuid().toByteArray(QUuid::WithoutBraces); } QNetworkReply *AccessManager::createRequest(QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *outgoingData) From a72ff9ea7f2e30e5fdb447f7edf7b021715e821d Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 7 Jan 2021 17:37:15 +0100 Subject: [PATCH 08/33] Set permissions for new folder --- src/libsync/propagateremotemkdir.cpp | 70 +++++++++++----------------- src/libsync/propagateremotemkdir.h | 2 - 2 files changed, 26 insertions(+), 46 deletions(-) diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index 0809617dc9..27661c6110 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -137,36 +137,34 @@ void PropagateRemoteMkdir::finalizeMkColJob(QNetworkReply::NetworkError err, con return; } - if (_item->_fileId.isEmpty()) { - // Owncloud 7.0.0 and before did not have a header with the file id. - // (https://github.com/owncloud/core/issues/9000) - // So we must get the file id using a PROPFIND - // This is required so that we can detect moves even if the folder is renamed on the server - // while files are still uploading - propagator()->_activeJobList.append(this); - auto propfindJob = new PropfindJob(propagator()->account(), jobPath, this); - propfindJob->setProperties(QList() << "http://owncloud.org/ns:id"); - QObject::connect(propfindJob, &PropfindJob::result, this, &PropagateRemoteMkdir::propfindResult); - QObject::connect(propfindJob, &PropfindJob::finishedWithError, this, &PropagateRemoteMkdir::propfindError); - propfindJob->start(); - _job = propfindJob; - return; - } + propagator()->_activeJobList.append(this); + auto propfindJob = new PropfindJob(_job->account(), _job->path(), this); + propfindJob->setProperties({"http://owncloud.org/ns:permissions"}); + connect(propfindJob, &PropfindJob::result, this, [this, jobPath](const QVariantMap &result){ + propagator()->_activeJobList.removeOne(this); + _item->_remotePerm = RemotePermissions::fromServerString(result.value(QStringLiteral("permissions")).toString()); - if (!_uploadEncryptedHelper && !_item->_isEncrypted) { - success(); - } else { - // We still need to mark that folder encrypted in case we were uploading it as encrypted one - // Another scenario, is we are creating a new folder because of move operation on an encrypted folder that works via remove + re-upload - propagator()->_activeJobList.append(this); + if (!_uploadEncryptedHelper && !_item->_isEncrypted) { + success(); + } else { + // We still need to mark that folder encrypted in case we were uploading it as encrypted one + // Another scenario, is we are creating a new folder because of move operation on an encrypted folder that works via remove + re-upload + propagator()->_activeJobList.append(this); - // We're expecting directory path in /Foo/Bar convention... - Q_ASSERT(jobPath.startsWith('/') && !jobPath.endsWith('/')); - // But encryption job expect it in Foo/Bar/ convention - auto job = new OCC::EncryptFolderJob(propagator()->account(), propagator()->_journal, jobPath.mid(1), _item->_fileId, this); - connect(job, &OCC::EncryptFolderJob::finished, this, &PropagateRemoteMkdir::slotEncryptFolderFinished); - job->start(); - } + // We're expecting directory path in /Foo/Bar convention... + Q_ASSERT(jobPath.startsWith('/') && !jobPath.endsWith('/')); + // But encryption job expect it in Foo/Bar/ convention + auto job = new OCC::EncryptFolderJob(propagator()->account(), propagator()->_journal, jobPath.mid(1), _item->_fileId, this); + connect(job, &OCC::EncryptFolderJob::finished, this, &PropagateRemoteMkdir::slotEncryptFolderFinished); + job->start(); + } + }); + connect(propfindJob, &PropfindJob::finishedWithError, this, [this]{ + // ignore the PROPFIND error + propagator()->_activeJobList.removeOne(this); + done(SyncFileItem::NormalError); + }); + propfindJob->start(); } void PropagateRemoteMkdir::slotMkdir() @@ -235,22 +233,6 @@ void PropagateRemoteMkdir::slotEncryptFolderFinished() success(); } -void PropagateRemoteMkdir::propfindResult(const QVariantMap &result) -{ - propagator()->_activeJobList.removeOne(this); - if (result.contains("id")) { - _item->_fileId = result["id"].toByteArray(); - } - success(); -} - -void PropagateRemoteMkdir::propfindError() -{ - // ignore the PROPFIND error - propagator()->_activeJobList.removeOne(this); - done(SyncFileItem::Success); -} - void PropagateRemoteMkdir::success() { // Never save the etag on first mkdir. diff --git a/src/libsync/propagateremotemkdir.h b/src/libsync/propagateremotemkdir.h index fe646b3c7c..a48fad0e98 100644 --- a/src/libsync/propagateremotemkdir.h +++ b/src/libsync/propagateremotemkdir.h @@ -54,8 +54,6 @@ private slots: void slotStartEncryptedMkcolJob(const QString &path, const QString &filename, quint64 size); void slotMkcolJobFinished(); void slotEncryptFolderFinished(); - void propfindResult(const QVariantMap &); - void propfindError(); void success(); private: From 56364f1c700fd2e63888fdd801a928771f765d91 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 7 Jan 2021 17:37:50 +0100 Subject: [PATCH 09/33] Fix unittests --- test/testsyncjournaldb.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/testsyncjournaldb.cpp b/test/testsyncjournaldb.cpp index fd404559bc..718920446c 100644 --- a/test/testsyncjournaldb.cpp +++ b/test/testsyncjournaldb.cpp @@ -117,7 +117,7 @@ private slots: { SyncJournalFileRecord record; record._path = "foo-nochecksum"; - record._remotePerm = RemotePermissions(); + record._remotePerm = RemotePermissions::fromDbValue("RW"); record._modtime = Utility::qDateTimeToTime_t(QDateTime::currentDateTimeUtc()); QVERIFY(_db.setFileRecord(record)); @@ -214,6 +214,7 @@ private slots: record._path = path; record._type = type; record._etag = initialEtag; + record._remotePerm = RemotePermissions::fromDbValue("RW"); _db.setFileRecord(record); }; auto getEtag = [&](const QByteArray &path) { @@ -275,6 +276,7 @@ private slots: auto makeEntry = [&](const QByteArray &path) { SyncJournalFileRecord record; record._path = path; + record._remotePerm = RemotePermissions::fromDbValue("RW"); _db.setFileRecord(record); }; From 789e0e45d5887e116b08cc1848b767f36a9fe228 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 12 Feb 2021 13:06:13 +0100 Subject: [PATCH 10/33] Fix Windows long path issue introduced in dd641fae997d71c8396b77def2fa25ad96fdf47f --- src/csync/vio/csync_vio_local_win.cpp | 31 ++++++++------------------- src/libsync/filesystem.cpp | 2 +- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/csync/vio/csync_vio_local_win.cpp b/src/csync/vio/csync_vio_local_win.cpp index 7aa07f4da4..4ddd4f545f 100644 --- a/src/csync/vio/csync_vio_local_win.cpp +++ b/src/csync/vio/csync_vio_local_win.cpp @@ -34,6 +34,7 @@ #include "csync.h" #include "vio/csync_vio_local.h" #include "common/filesystembase.h" +#include "common/utility.h" #include @@ -52,8 +53,6 @@ struct csync_vio_handle_t { QString path; // Always ends with '\' }; -static int _csync_vio_local_stat_mb(const QString &path, csync_file_stat_t *buf); - csync_vio_handle_t *csync_vio_local_opendir(const QString &name) { QScopedPointer handle(new csync_vio_handle_t{}); @@ -175,12 +174,9 @@ std::unique_ptr csync_vio_local_readdir(csync_vio_handle_t *h file_stat->size = (handle->ffd.nFileSizeHigh * ((int64_t)(MAXDWORD)+1)) + handle->ffd.nFileSizeLow; file_stat->modtime = FileTimeToUnixTime(&handle->ffd.ftLastWriteTime, &rem); - QString fullPath; - fullPath.reserve(handle->path.size() + std::wcslen(handle->ffd.cFileName)); - fullPath += handle->path; // path always ends with '\', by construction - fullPath += QString::fromWCharArray(handle->ffd.cFileName); + // path always ends with '\', by construction - if (_csync_vio_local_stat_mb(fullPath, file_stat.get()) < 0) { + if (csync_vio_local_stat(handle->path + QString::fromWCharArray(handle->ffd.cFileName), file_stat.get()) < 0) { // Will get excluded by _csync_detect_update. file_stat->type = ItemTypeSkip; } @@ -188,14 +184,7 @@ std::unique_ptr csync_vio_local_readdir(csync_vio_handle_t *h return file_stat; } - int csync_vio_local_stat(const QString &uri, csync_file_stat_t *buf) -{ - int rc = _csync_vio_local_stat_mb(uri, buf); - return rc; -} - -static int _csync_vio_local_stat_mb(const QString &path, csync_file_stat_t *buf) { /* Almost nothing to do since csync_vio_local_readdir already filled up most of the information But we still need to fetch the file ID. @@ -206,21 +195,19 @@ static int _csync_vio_local_stat_mb(const QString &path, csync_file_stat_t *buf) BY_HANDLE_FILE_INFORMATION fileInfo; ULARGE_INTEGER FileIndex; - const auto longPath = OCC::FileSystem::longWinPath(path); - - h = CreateFileW(longPath.toStdWString().data(), 0, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, - nullptr, OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, - nullptr ); + h = CreateFileW(reinterpret_cast(OCC::FileSystem::longWinPath(uri).utf16()), 0, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, + NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + NULL); if( h == INVALID_HANDLE_VALUE ) { - qCCritical(lcCSyncVIOLocal) << "CreateFileW failed on" << longPath; errno = GetLastError(); + qCCritical(lcCSyncVIOLocal) << "CreateFileW failed on" << uri << OCC::Utility::formatWinError(errno); return -1; } if(!GetFileInformationByHandle( h, &fileInfo ) ) { - qCCritical(lcCSyncVIOLocal) << "GetFileInformationByHandle failed on" << longPath; errno = GetLastError(); + qCCritical(lcCSyncVIOLocal) << "GetFileInformationByHandle failed on" << uri << OCC::Utility::formatWinError(errno); CloseHandle(h); return -1; } diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index af84f4d2fe..7c4f5b6261 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -115,7 +115,7 @@ static qint64 getSizeWithCsync(const QString &filename) if (csync_vio_local_stat(filename, &stat) != -1) { result = stat.size; } else { - qCWarning(lcFileSystem) << "Could not get size for" << filename << "with csync"; + qCWarning(lcFileSystem) << "Could not get size for" << filename << "with csync" << Utility::formatWinError(errno); } return result; } From 8ca5035c429ed99f322dd9679dd0f456c58523d7 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 12 Feb 2021 14:44:32 +0100 Subject: [PATCH 11/33] Add test for csync_vio_local_stat with long path --- test/CMakeLists.txt | 2 +- .../{testlongwinpath.cpp => testlongpath.cpp} | 57 ++++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) rename test/{testlongwinpath.cpp => testlongpath.cpp} (51%) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1369b241f7..33bf1ac71c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -68,12 +68,12 @@ if (WIN32) ${CMAKE_BINARY_DIR}/src/libsync/vfs/cfapi ) - nextcloud_add_test(LongWinPath) nextcloud_add_test(SyncCfApi) elseif(LINUX) # elseif(LINUX OR APPLE) nextcloud_add_test(SyncXAttr) endif() +nextcloud_add_test(LongPath) nextcloud_add_benchmark(LargeSync) nextcloud_add_test(Account) diff --git a/test/testlongwinpath.cpp b/test/testlongpath.cpp similarity index 51% rename from test/testlongwinpath.cpp rename to test/testlongpath.cpp index 16933813cb..7e912ae157 100644 --- a/test/testlongwinpath.cpp +++ b/test/testlongpath.cpp @@ -18,7 +18,10 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ #include "common/filesystembase.h" +#include "csync/csync.h" +#include "csync/vio/csync_vio_local.h" +#include #include @@ -27,6 +30,7 @@ class TestLongWindowsPath : public QObject Q_OBJECT private Q_SLOTS: +#ifdef Q_OS_WIN void check_long_win_path() { { @@ -81,7 +85,58 @@ private Q_SLOTS: // printf( "YYYYYYYYYYYY %ld\n", strlen(new_long)); QCOMPARE(new_long.length(), 286); } +#endif + + + void testLongPathStat_data() + { + QTest::addColumn("name"); + + QTest::newRow("long") << QStringLiteral("/alonglonglonglong/blonglonglonglong/clonglonglonglong/dlonglonglonglong/" + "elonglonglonglong/flonglonglonglong/glonglonglonglong/hlonglonglonglong/ilonglonglonglong/" + "jlonglonglonglong/klonglonglonglong/llonglonglonglong/mlonglonglonglong/nlonglonglonglong/" + "olonglonglonglong/file.txt"); + QTest::newRow("long emoji") << QString::fromUtf8("/alonglonglonglong/blonglonglonglong/clonglonglonglong/dlonglonglonglong/" + "elonglonglonglong/flonglonglonglong/glonglonglonglong/hlonglonglonglong/ilonglonglonglong/" + "jlonglonglonglong/klonglonglonglong/llonglonglonglong/mlonglonglonglong/nlonglonglonglong/" + "olonglonglonglong/file🐷.txt"); + QTest::newRow("long russian") << QString::fromUtf8("/alonglonglonglong/blonglonglonglong/clonglonglonglong/dlonglonglonglong/" + "elonglonglonglong/flonglonglonglong/glonglonglonglong/hlonglonglonglong/ilonglonglonglong/" + "jlonglonglonglong/klonglonglonglong/llonglonglonglong/mlonglonglonglong/nlonglonglonglong/" + "olonglonglonglong/собственное.txt"); + QTest::newRow("long arabic") << QString::fromUtf8("/alonglonglonglong/blonglonglonglong/clonglonglonglong/dlonglonglonglong/" + "elonglonglonglong/flonglonglonglong/glonglonglonglong/hlonglonglonglong/ilonglonglonglong/" + "jlonglonglonglong/klonglonglonglong/llonglonglonglong/mlonglonglonglong/nlonglonglonglong/" + "olonglonglonglong/السحاب.txt"); + QTest::newRow("long chinese") << QString::fromUtf8("/alonglonglonglong/blonglonglonglong/clonglonglonglong/dlonglonglonglong/" + "elonglonglonglong/flonglonglonglong/glonglonglonglong/hlonglonglonglong/ilonglonglonglong/" + "jlonglonglonglong/klonglonglonglong/llonglonglonglong/mlonglonglonglong/nlonglonglonglong/" + "olonglonglonglong/自己的云.txt"); + } + + void testLongPathStat() + { + QTemporaryDir tmp; + QFETCH(QString, name); + const QFileInfo longPath(tmp.path() + name); + + const auto data = QByteArrayLiteral("hello"); + qDebug() << longPath; + QVERIFY(longPath.dir().mkpath(".")); + + QFile file(longPath.filePath()); + QVERIFY(file.open(QFile::WriteOnly)); + QVERIFY(file.write(data.constData()) == data.size()); + file.close(); + + csync_file_stat_t buf; + QVERIFY(csync_vio_local_stat(longPath.filePath(), &buf) != -1); + QVERIFY(buf.size == data.size()); + QVERIFY(buf.size == longPath.size()); + + QVERIFY(tmp.remove()); + } }; QTEST_GUILESS_MAIN(TestLongWindowsPath) -#include "testlongwinpath.moc" +#include "testlongpath.moc" From baa571018eb62336c15ff2549f52409813252a3f Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 16 Feb 2021 09:56:16 +0100 Subject: [PATCH 12/33] Log invocation, useful for debugging startup issues --- src/gui/application.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 511da75586..97ecedba98 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -524,7 +524,12 @@ void Application::setupLogging() logger->enterNextLogFile(); - qCInfo(lcApplication) << QString::fromLatin1("################## %1 locale:[%2] ui_lang:[%3] version:[%4] os:[%5]").arg(_theme->appName()).arg(QLocale::system().name()).arg(property("ui_lang").toString()).arg(_theme->version()).arg(Utility::platformName()); + qCInfo(lcApplication) << "##################" << _theme->appName() + << "locale:" << QLocale::system().name() + << "ui_lang:" << property("ui_lang") + << "version:" << _theme->version() + << "os:" << Utility::platformName(); + qCInfo(lcApplication) << "Arguments:" << qApp->arguments(); } void Application::slotUseMonoIconsChanged(bool) From cb57d8e54c11258be8d6b9fd244f3e3bf019295a Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Wed, 17 Feb 2021 14:39:24 +0100 Subject: [PATCH 13/33] Add missing Q_EMIT --- src/libsync/syncengine.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 7f480913dd..e8cb1863d4 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -452,7 +452,7 @@ void SyncEngine::startSync() if (!QDir(_localPath).exists()) { _anotherSyncNeeded = DelayedFollowUp; // No _tr, it should only occur in non-mirall - syncError("Unable to find local sync folder."); + Q_EMIT syncError(QStringLiteral("Unable to find local sync folder.")); finalize(false); return; } @@ -465,11 +465,11 @@ void SyncEngine::startSync() qCWarning(lcEngine()) << "Too little space available at" << _localPath << ". Have" << freeBytes << "bytes and require at least" << minFree << "bytes"; _anotherSyncNeeded = DelayedFollowUp; - syncError(tr("Only %1 are available, need at least %2 to start", + Q_EMIT syncError(tr("Only %1 are available, need at least %2 to start", "Placeholders are postfixed with file sizes using Utility::octetsToString()") - .arg( - Utility::octetsToString(freeBytes), - Utility::octetsToString(minFree))); + .arg( + Utility::octetsToString(freeBytes), + Utility::octetsToString(minFree))); finalize(false); return; } else { @@ -498,7 +498,7 @@ void SyncEngine::startSync() // This creates the DB if it does not exist yet. if (!_journal->open()) { qCWarning(lcEngine) << "No way to create a sync journal!"; - syncError(tr("Unable to open or create the local sync database. Make sure you have write access in the sync folder.")); + Q_EMIT syncError(tr("Unable to open or create the local sync database. Make sure you have write access in the sync folder.")); finalize(false); return; // database creation error! @@ -514,7 +514,7 @@ void SyncEngine::startSync() _lastLocalDiscoveryStyle = _localDiscoveryStyle; if (_syncOptions._vfs->mode() == Vfs::WithSuffix && _syncOptions._vfs->fileSuffix().isEmpty()) { - syncError(tr("Using virtual files with suffix, but suffix is not set")); + Q_EMIT syncError(tr("Using virtual files with suffix, but suffix is not set")); finalize(false); return; } @@ -526,7 +526,7 @@ void SyncEngine::startSync() qCInfo(lcEngine) << (usingSelectiveSync ? "Using Selective Sync" : "NOT Using Selective Sync"); } else { qCWarning(lcEngine) << "Could not retrieve selective sync list from DB"; - syncError(tr("Unable to read the blacklist from the local database")); + Q_EMIT syncError(tr("Unable to read the blacklist from the local database")); finalize(false); return; } @@ -557,7 +557,7 @@ void SyncEngine::startSync() _discoveryPhase->setSelectiveSyncWhiteList(_journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncWhiteList, &ok)); if (!ok) { qCWarning(lcEngine) << "Unable to read selective sync list, aborting."; - syncError(tr("Unable to read from the sync journal.")); + Q_EMIT syncError(tr("Unable to read from the sync journal.")); finalize(false); return; } @@ -581,7 +581,7 @@ void SyncEngine::startSync() connect(_discoveryPhase.data(), &DiscoveryPhase::itemDiscovered, this, &SyncEngine::slotItemDiscovered); connect(_discoveryPhase.data(), &DiscoveryPhase::newBigFolder, this, &SyncEngine::newBigFolder); connect(_discoveryPhase.data(), &DiscoveryPhase::fatalError, this, [this](const QString &errorString) { - syncError(errorString); + Q_EMIT syncError(errorString); finalize(false); }); connect(_discoveryPhase.data(), &DiscoveryPhase::finished, this, &SyncEngine::slotDiscoveryFinished); @@ -640,7 +640,7 @@ void SyncEngine::slotDiscoveryFinished() // Sanity check if (!_journal->open()) { qCWarning(lcEngine) << "Bailing out, DB failure"; - syncError(tr("Cannot open the sync journal")); + Q_EMIT syncError(tr("Cannot open the sync journal")); finalize(false); return; } else { @@ -723,7 +723,7 @@ void SyncEngine::slotDiscoveryFinished() // Emit the started signal only after the propagator has been set up. if (_needsUpdate) - emit(started()); + Q_EMIT started(); _propagator->start(_syncItems); _syncItems.clear(); @@ -1078,7 +1078,7 @@ void SyncEngine::abort() disconnect(_discoveryPhase.data(), nullptr, this, nullptr); _discoveryPhase.take()->deleteLater(); - syncError(tr("Aborted")); + Q_EMIT syncError(tr("Aborted")); finalize(false); } } From c4f4fb48a4af5542570e3f876127606a26d84d88 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 16 Feb 2021 14:42:13 +0100 Subject: [PATCH 14/33] Minor cleanup of socket api --- src/gui/socketapi.cpp | 46 +++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp index 16b6188682..bd0dc40b7d 100644 --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi.cpp @@ -78,6 +78,18 @@ #define MIRALL_SOCKET_API_VERSION "1.1" namespace { + +const QLatin1Char RecordSeparator() +{ + return QLatin1Char('\x1e'); +} + +QStringList split(const QString &data) +{ + // TODO: string ref? + return data.split(RecordSeparator()); +} + #if GUI_TESTING using namespace OCC; @@ -356,26 +368,18 @@ void SocketApi::slotReadSocket() while (socket->canReadLine()) { // Make sure to normalize the input from the socket to // make sure that the path will match, especially on OS X. - QString line = QString::fromUtf8(socket->readLine()).normalized(QString::NormalizationForm_C); - line.chop(1); // remove the '\n' + const QString line = QString::fromUtf8(socket->readLine().trimmed()).normalized(QString::NormalizationForm_C); qCInfo(lcSocketApi) << "Received SocketAPI message <--" << line << "from" << socket; - QByteArray command = line.split(":").value(0).toLatin1(); + const QByteArray command = line.mid(0, line.indexOf(QLatin1Char(':'))).toUtf8(); + const QByteArray functionWithArguments = "command_" + command + (command.startsWith("ASYNC_") ? "(QSharedPointer)" : "(QString,SocketListener*)"); + const int indexOfMethod = staticMetaObject.indexOfMethod(functionWithArguments); - QByteArray functionWithArguments = "command_" + command; - if (command.startsWith("ASYNC_")) { - functionWithArguments += "(QSharedPointer)"; - } else { - functionWithArguments += "(QString,SocketListener*)"; - } - - int indexOfMethod = staticMetaObject.indexOfMethod(functionWithArguments); - - QString argument = line.remove(0, command.length() + 1); + const auto argument = line.midRef(command.length() + 1); if (command.startsWith("ASYNC_")) { auto arguments = argument.split('|'); if (arguments.size() != 2) { - listener->sendMessage(QLatin1String("argument count is wrong")); + listener->sendMessage(QStringLiteral("argument count is wrong")); return; } @@ -384,7 +388,7 @@ void SocketApi::slotReadSocket() auto jobId = arguments[0]; auto socketApiJob = QSharedPointer( - new SocketApiJob(jobId, listener, json), &QObject::deleteLater); + new SocketApiJob(jobId.toString(), listener, json), &QObject::deleteLater); if (indexOfMethod != -1) { staticMetaObject.method(indexOfMethod) .invoke(this, Qt::QueuedConnection, @@ -392,15 +396,15 @@ void SocketApi::slotReadSocket() } else { qCWarning(lcSocketApi) << "The command is not supported by this version of the client:" << command << "with argument:" << argument; - socketApiJob->reject("command not found"); + socketApiJob->reject(QStringLiteral("command not found")); } } else { if (indexOfMethod != -1) { // to ensure that listener is still valid we need to call it with Qt::DirectConnection ASSERT(thread() == QThread::currentThread()) staticMetaObject.method(indexOfMethod) - .invoke(this, Qt::DirectConnection, Q_ARG(QString, argument), - Q_ARG(SocketListener *, listener)); + .invoke(this, Qt::DirectConnection, Q_ARG(QString, argument.toString()), + Q_ARG(SocketListener *, listener)); } else { qCWarning(lcSocketApi) << "The command is not supported by this version of the client:" << command << "with argument:" << argument; } @@ -788,7 +792,7 @@ void SocketApi::command_OPEN_PRIVATE_LINK(const QString &localFile, SocketListen void SocketApi::command_MAKE_AVAILABLE_LOCALLY(const QString &filesArg, SocketListener *) { - QStringList files = filesArg.split(QLatin1Char('\x1e')); // Record Separator + const QStringList files = split(filesArg); for (const auto &file : files) { auto data = FileData::get(file); @@ -809,7 +813,7 @@ void SocketApi::command_MAKE_AVAILABLE_LOCALLY(const QString &filesArg, SocketLi /* Go over all the files and replace them by a virtual file */ void SocketApi::command_MAKE_ONLINE_ONLY(const QString &filesArg, SocketListener *) { - QStringList files = filesArg.split(QLatin1Char('\x1e')); // Record Separator + const QStringList files = split(filesArg); for (const auto &file : files) { auto data = FileData::get(file); @@ -1031,7 +1035,7 @@ SocketApi::FileData SocketApi::FileData::parentFolder() const void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListener *listener) { listener->sendMessage(QString("GET_MENU_ITEMS:BEGIN")); - QStringList files = argument.split(QLatin1Char('\x1e')); // Record Separator + const QStringList files = split(argument); // Find the common sync folder. // syncFolder will be null if files are in different folders. From d16befd1fd68cd19c9cce2d2a2d22cac030eb183 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Wed, 17 Feb 2021 14:30:26 +0100 Subject: [PATCH 15/33] Align MkColJob finish signal with the other jobs --- src/gui/folderwizard.cpp | 16 +++++++--------- src/gui/folderwizard.h | 2 +- src/gui/owncloudsetupwizard.cpp | 18 ++++++++++-------- src/gui/owncloudsetupwizard.h | 2 +- src/libsync/networkjobs.cpp | 6 +++++- src/libsync/networkjobs.h | 5 +++-- src/libsync/propagateremotemkdir.cpp | 10 +++++----- src/libsync/propagateupload.h | 2 +- src/libsync/propagateuploadng.cpp | 8 +++++--- 9 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index b55cb58e96..3200f4210e 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -201,21 +201,19 @@ void FolderWizardRemotePath::slotCreateRemoteFolder(const QString &folder) auto *job = new MkColJob(_account, fullPath, this); /* check the owncloud configuration file and query the ownCloud */ - connect(job, static_cast(&MkColJob::finished), + connect(job, &MkColJob::finishedWithoutError, this, &FolderWizardRemotePath::slotCreateRemoteFolderFinished); connect(job, &AbstractNetworkJob::networkError, this, &FolderWizardRemotePath::slotHandleMkdirNetworkError); job->start(); } -void FolderWizardRemotePath::slotCreateRemoteFolderFinished(QNetworkReply::NetworkError error) +void FolderWizardRemotePath::slotCreateRemoteFolderFinished() { - if (error == QNetworkReply::NoError) { - qCDebug(lcWizard) << "webdav mkdir request finished"; - showWarn(tr("Folder was successfully created on %1.").arg(Theme::instance()->appNameGUI())); - slotRefreshFolders(); - _ui.folderEntry->setText(static_cast(sender())->path()); - slotLsColFolderEntry(); - } + qCDebug(lcWizard) << "webdav mkdir request finished"; + showWarn(tr("Folder was successfully created on %1.").arg(Theme::instance()->appNameGUI())); + slotRefreshFolders(); + _ui.folderEntry->setText(static_cast(sender())->path()); + slotLsColFolderEntry(); } void FolderWizardRemotePath::slotHandleMkdirNetworkError(QNetworkReply *reply) diff --git a/src/gui/folderwizard.h b/src/gui/folderwizard.h index 58ffa69c05..7e7dd9c321 100644 --- a/src/gui/folderwizard.h +++ b/src/gui/folderwizard.h @@ -92,7 +92,7 @@ protected slots: void showWarn(const QString & = QString()) const; void slotAddRemoteFolder(); void slotCreateRemoteFolder(const QString &); - void slotCreateRemoteFolderFinished(QNetworkReply::NetworkError error); + void slotCreateRemoteFolderFinished(); void slotHandleMkdirNetworkError(QNetworkReply *); void slotHandleLsColNetworkError(QNetworkReply *); void slotUpdateDirectories(const QStringList &); diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index ce0dd773fb..6b87ca3d28 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/src/gui/owncloudsetupwizard.cpp @@ -536,26 +536,28 @@ void OwncloudSetupWizard::createRemoteFolder() _ocWizard->appendToConfigurationLog(tr("creating folder on Nextcloud: %1").arg(_remoteFolder)); auto *job = new MkColJob(_ocWizard->account(), _remoteFolder, this); - connect(job, SIGNAL(finished(QNetworkReply::NetworkError)), SLOT(slotCreateRemoteFolderFinished(QNetworkReply::NetworkError))); + connect(job, &MkColJob::finishedWithError, this, &OwncloudSetupWizard::slotCreateRemoteFolderFinished); + connect(job, &MkColJob::finishedWithoutError, this, [this] { + _ocWizard->appendToConfigurationLog(tr("Remote folder %1 created successfully.").arg(_remoteFolder)); + finalizeSetup(true); + }); job->start(); } -void OwncloudSetupWizard::slotCreateRemoteFolderFinished(QNetworkReply::NetworkError error) +void OwncloudSetupWizard::slotCreateRemoteFolderFinished(QNetworkReply *reply) { + auto error = reply->error(); qCDebug(lcWizard) << "** webdav mkdir request finished " << error; // disconnect(ownCloudInfo::instance(), SIGNAL(webdavColCreated(QNetworkReply::NetworkError)), // this, SLOT(slotCreateRemoteFolderFinished(QNetworkReply::NetworkError))); bool success = true; - - if (error == QNetworkReply::NoError) { - _ocWizard->appendToConfigurationLog(tr("Remote folder %1 created successfully.").arg(_remoteFolder)); - } else if (error == 202) { + if (error == 202) { _ocWizard->appendToConfigurationLog(tr("The remote folder %1 already exists. Connecting it for syncing.").arg(_remoteFolder)); } else if (error > 202 && error < 300) { - _ocWizard->displayError(tr("The folder creation resulted in HTTP error code %1").arg((int)error), false); + _ocWizard->displayError(tr("The folder creation resulted in HTTP error code %1").arg(static_cast(error)), false); - _ocWizard->appendToConfigurationLog(tr("The folder creation resulted in HTTP error code %1").arg((int)error)); + _ocWizard->appendToConfigurationLog(tr("The folder creation resulted in HTTP error code %1").arg(static_cast(error))); } else if (error == QNetworkReply::OperationCanceledError) { _ocWizard->displayError(tr("The remote folder creation failed because the provided credentials " "are wrong!" diff --git a/src/gui/owncloudsetupwizard.h b/src/gui/owncloudsetupwizard.h index 4762de1cd8..877c6703d5 100644 --- a/src/gui/owncloudsetupwizard.h +++ b/src/gui/owncloudsetupwizard.h @@ -65,7 +65,7 @@ private slots: void slotCreateLocalAndRemoteFolders(const QString &, const QString &); void slotRemoteFolderExists(QNetworkReply *); - void slotCreateRemoteFolderFinished(QNetworkReply::NetworkError); + void slotCreateRemoteFolderFinished(QNetworkReply *reply); void slotAssistantFinished(int); void slotSkipFolderConfiguration(); diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index 7633679881..adcc9bc4f0 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -182,7 +182,11 @@ bool MkColJob::finished() qCInfo(lcMkColJob) << "MKCOL of" << reply()->request().url() << "FINISHED WITH STATUS" << replyStatusString(); - emit finished(reply()->error()); + if (reply()->error() != QNetworkReply::NoError) { + Q_EMIT finishedWithError(reply()); + } else { + Q_EMIT finishedWithoutError(); + } return true; } diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index 8e88853bc6..c52883b078 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -273,9 +273,10 @@ public: void start() override; signals: - void finished(QNetworkReply::NetworkError); + void finishedWithError(QNetworkReply *reply); + void finishedWithoutError(); -private slots: +private: bool finished() override; }; diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index 27661c6110..4e27b42a81 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -61,8 +61,7 @@ void PropagateRemoteMkdir::start() _job = new DeleteJob(propagator()->account(), propagator()->fullRemotePath(_item->_file), this); - connect(static_cast(_job.data()), &DeleteJob::finishedSignal, - this, &PropagateRemoteMkdir::slotMkdir); + connect(qobject_cast(_job), &DeleteJob::finishedSignal, this, &PropagateRemoteMkdir::slotMkdir); _job->start(); } @@ -76,7 +75,8 @@ void PropagateRemoteMkdir::slotStartMkcolJob() _job = new MkColJob(propagator()->account(), propagator()->fullRemotePath(_item->_file), this); - connect(_job, SIGNAL(finished(QNetworkReply::NetworkError)), this, SLOT(slotMkcolJobFinished())); + connect(qobject_cast(_job), &MkColJob::finishedWithError, this, &PropagateRemoteMkdir::slotMkcolJobFinished); + connect(qobject_cast(_job), &MkColJob::finishedWithoutError, this, &PropagateRemoteMkdir::slotMkcolJobFinished); _job->start(); } @@ -95,8 +95,8 @@ void PropagateRemoteMkdir::slotStartEncryptedMkcolJob(const QString &path, const propagator()->fullRemotePath(filename), {{"e2e-token", _uploadEncryptedHelper->folderToken() }}, this); - connect(job, qOverload(&MkColJob::finished), - this, &PropagateRemoteMkdir::slotMkcolJobFinished); + connect(job, &MkColJob::finishedWithError, this, &PropagateRemoteMkdir::slotMkcolJobFinished); + connect(job, &MkColJob::finishedWithoutError, this, &PropagateRemoteMkdir::slotMkcolJobFinished); _job = job; _job->start(); } diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h index 33d1f209b3..7238a0f326 100644 --- a/src/libsync/propagateupload.h +++ b/src/libsync/propagateupload.h @@ -418,7 +418,7 @@ private slots: void slotPropfindFinishedWithError(); void slotPropfindIterate(const QString &name, const QMap &properties); void slotDeleteJobFinished(); - void slotMkColFinished(QNetworkReply::NetworkError); + void slotMkColFinished(); void slotPutFinished(); void slotMoveJobFinished(); void slotUploadProgress(qint64, qint64); diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 62d1fca038..b99fc06cce 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -249,13 +249,15 @@ void PropagateUploadFileNG::startNewUpload() headers["OC-Total-Length"] = QByteArray::number(_fileToUpload._size); auto job = new MkColJob(propagator()->account(), chunkUrl(), headers, this); - connect(job, SIGNAL(finished(QNetworkReply::NetworkError)), - this, SLOT(slotMkColFinished(QNetworkReply::NetworkError))); + connect(job, &MkColJob::finishedWithError, + this, &PropagateUploadFileNG::slotMkColFinished); + connect(job, &MkColJob::finishedWithoutError, + this, &PropagateUploadFileNG::slotMkColFinished); connect(job, &QObject::destroyed, this, &PropagateUploadFileCommon::slotJobDestroyed); job->start(); } -void PropagateUploadFileNG::slotMkColFinished(QNetworkReply::NetworkError) +void PropagateUploadFileNG::slotMkColFinished() { propagator()->_activeJobList.removeOne(this); auto job = qobject_cast(sender()); From 5a7fd3f316d0ccd974a6e5d07ff3f47e896c5277 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Wed, 17 Feb 2021 12:50:03 +0100 Subject: [PATCH 16/33] testlockedfiles use long file path --- test/testlockedfiles.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/testlockedfiles.cpp b/test/testlockedfiles.cpp index df1e8f8d04..cae7fb11c4 100644 --- a/test/testlockedfiles.cpp +++ b/test/testlockedfiles.cpp @@ -17,7 +17,8 @@ using namespace OCC; // pass combination of FILE_SHARE_READ, FILE_SHARE_WRITE, FILE_SHARE_DELETE HANDLE makeHandle(const QString &file, int shareMode) { - const wchar_t *wuri = reinterpret_cast(file.utf16()); + const auto fName = FileSystem::longWinPath(file); + const wchar_t *wuri = reinterpret_cast(fName.utf16()); auto handle = CreateFileW( wuri, GENERIC_READ | GENERIC_WRITE, @@ -39,6 +40,7 @@ class TestLockedFiles : public QObject private slots: void testBasicLockFileWatcher() { + QTemporaryDir tmp; int count = 0; QString file; @@ -46,12 +48,16 @@ private slots: watcher.setCheckInterval(std::chrono::milliseconds(50)); connect(&watcher, &LockWatcher::fileUnlocked, &watcher, [&](const QString &f) { ++count; file = f; }); - QString tmpFile; + const QString tmpFile = tmp.path() + QString::fromUtf8("/alonglonglonglong/blonglonglonglong/clonglonglonglong/dlonglonglonglong/" + "elonglonglonglong/flonglonglonglong/glonglonglonglong/hlonglonglonglong/ilonglonglonglong/" + "jlonglonglonglong/klonglonglonglong/llonglonglonglong/mlonglonglonglong/nlonglonglonglong/" + "olonglonglonglong/file🐷.txt"); { - QTemporaryFile tmp; - tmp.setAutoRemove(false); - tmp.open(); - tmpFile = tmp.fileName(); + // use a long file path to ensure we handle that correctly + QVERIFY(QFileInfo(tmpFile).dir().mkpath(".")); + QFile tmp(tmpFile); + QVERIFY(tmp.open(QFile::WriteOnly)); + QVERIFY(tmp.write("ownCLoud")); } QVERIFY(QFile::exists(tmpFile)); @@ -91,7 +97,7 @@ private slots: QCOMPARE(file, tmpFile); QVERIFY(!watcher.contains(tmpFile)); #endif - QFile::remove(tmpFile); + QVERIFY(tmp.remove()); } #ifdef Q_OS_WIN From 5b457a1663cfd6df63e6762ddc9a7d1a29200455 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 25 Feb 2021 14:42:23 +0100 Subject: [PATCH 17/33] Use byte array for etag --- src/common/result.h | 7 +++++++ src/gui/folder.cpp | 4 ++-- src/gui/folder.h | 6 +++--- src/libsync/discovery.cpp | 26 ++++++++++++-------------- src/libsync/discovery.h | 2 +- src/libsync/discoveryphase.cpp | 2 +- src/libsync/discoveryphase.h | 4 ++-- src/libsync/networkjobs.cpp | 8 ++++---- src/libsync/networkjobs.h | 8 ++++---- src/libsync/syncengine.cpp | 2 +- src/libsync/syncengine.h | 6 +++--- 11 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/common/result.h b/src/common/result.h index 25129e8751..77a9d121c1 100644 --- a/src/common/result.h +++ b/src/common/result.h @@ -104,6 +104,7 @@ public: ASSERT(!_isError); return _result; } + T operator*() && { ASSERT(!_isError); @@ -116,6 +117,12 @@ public: return &_result; } + const T &get() const + { + ASSERT(!_isError) + return _result; + } + const Error &error() const & { ASSERT(_isError); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index cda9b0cb27..4fb5a259e8 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -353,7 +353,7 @@ void Folder::slotRunEtagJob() // The _requestEtagJob is auto deleting itself on finish. Our guard pointer _requestEtagJob will then be null. } -void Folder::etagRetrieved(const QString &etag, const QDateTime &tp) +void Folder::etagRetrieved(const QByteArray &etag, const QDateTime &tp) { // re-enable sync if it was disabled because network was down FolderMan::instance()->setSyncEnabled(true); @@ -367,7 +367,7 @@ void Folder::etagRetrieved(const QString &etag, const QDateTime &tp) _accountState->tagLastSuccessfullETagRequest(tp); } -void Folder::etagRetrievedFromSyncEngine(const QString &etag, const QDateTime &time) +void Folder::etagRetrievedFromSyncEngine(const QByteArray &etag, const QDateTime &time) { qCInfo(lcFolder) << "Root etag from during sync:" << etag; accountState()->tagLastSuccessfullETagRequest(time); diff --git a/src/gui/folder.h b/src/gui/folder.h index 809ca85e79..4e0e9fa979 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -376,8 +376,8 @@ private slots: void slotItemCompleted(const SyncFileItemPtr &); void slotRunEtagJob(); - void etagRetrieved(const QString &, const QDateTime &tp); - void etagRetrievedFromSyncEngine(const QString &, const QDateTime &time); + void etagRetrieved(const QByteArray &, const QDateTime &tp); + void etagRetrievedFromSyncEngine(const QByteArray &, const QDateTime &time); void slotEmitFinishedDelayed(); @@ -447,7 +447,7 @@ private: SyncResult _syncResult; QScopedPointer _engine; QPointer _requestEtagJob; - QString _lastEtag; + QByteArray _lastEtag; QElapsedTimer _timeSinceLastSyncDone; QElapsedTimer _timeSinceLastSyncStart; QElapsedTimer _timeSinceLastFullLocalDiscovery; diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 5f0ead0663..45bcf96a16 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -528,7 +528,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( } return ParentNotChanged; }(); - + processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, serverQueryMode); return; } @@ -545,15 +545,14 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( item->_modtime = serverEntry.modtime; item->_size = serverEntry.size; - auto postProcessServerNew = [=] () { - auto tmp_path = path; + auto postProcessServerNew = [=]() mutable { if (item->isDirectory()) { _pendingAsyncJobs++; - _discoveryData->checkSelectiveSyncNewFolder(tmp_path._server, serverEntry.remotePerm, + _discoveryData->checkSelectiveSyncNewFolder(path._server, serverEntry.remotePerm, [=](bool result) { --_pendingAsyncJobs; if (!result) { - processFileAnalyzeLocalInfo(item, tmp_path, localEntry, serverEntry, dbEntry, _queryServer); + processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, _queryServer); } QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs); }); @@ -568,7 +567,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( && !FileSystem::isExcludeFile(item->_file)) { item->_type = ItemTypeVirtualFile; if (isVfsWithSuffix()) - addVirtualFileSuffix(tmp_path._original); + addVirtualFileSuffix(path._original); } if (opts._vfs->mode() != Vfs::Off && !item->_encryptedFileName.isEmpty()) { @@ -579,7 +578,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( // another scenario - we are syncing a file which is on disk but not in the database (database was removed or file was not written there yet) item->_size = serverEntry.size - Constants::e2EeTagSize; } - processFileAnalyzeLocalInfo(item, tmp_path, localEntry, serverEntry, dbEntry, _queryServer); + processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, _queryServer); }; // Potential NEW/NEW conflict is handled in AnalyzeLocal @@ -698,8 +697,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( // we need to make a request to the server to know that the original file is deleted on the server _pendingAsyncJobs++; auto job = new RequestEtagJob(_discoveryData->_account, originalPath, this); - connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult &etag) { - auto tmp_path = path; + connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult &etag) mutable { _pendingAsyncJobs--; QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs); if (etag || etag.error().code != 404 || @@ -715,8 +713,8 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( // In case the deleted item was discovered in parallel _discoveryData->findAndCancelDeletedJob(originalPath); - postProcessRename(tmp_path); - processFileFinalize(item, tmp_path, item->isDirectory(), item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _queryServer); + postProcessRename(path); + processFileFinalize(item, path, item->isDirectory(), item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _queryServer); }); job->start(); done = true; // Ideally, if the origin still exist on the server, we should continue searching... but that'd be difficult @@ -1162,8 +1160,8 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( if (base.isVirtualFile() && isVfsWithSuffix()) chopVirtualFileSuffix(serverOriginalPath); auto job = new RequestEtagJob(_discoveryData->_account, serverOriginalPath, this); - connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult &etag) mutable { - if (!etag || (*etag != base._etag && !item->isDirectory()) || _discoveryData->isRenamed(originalPath)) { + connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult &etag) mutable { + if (!etag || (etag.get() != base._etag && !item->isDirectory()) || _discoveryData->isRenamed(originalPath)) { qCInfo(lcDisco) << "Can't rename because the etag has changed or the directory is gone" << originalPath; // Can't be a rename, leave it as a new. postProcessLocalNew(); @@ -1171,7 +1169,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // In case the deleted item was discovered in parallel _discoveryData->findAndCancelDeletedJob(originalPath); processRename(path); - recurseQueryServer = *etag == base._etag ? ParentNotChanged : NormalQuery; + recurseQueryServer = etag.get() == base._etag ? ParentNotChanged : NormalQuery; } processFileFinalize(item, path, item->isDirectory(), NormalQuery, recurseQueryServer); _pendingAsyncJobs--; diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index fd70f31390..bc1737644e 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -293,6 +293,6 @@ private: signals: void finished(); // The root etag of this directory was fetched - void etag(const QString &, const QDateTime &time); + void etag(const QByteArray &, const QDateTime &time); }; } diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 6b108e4ddb..208b6e2ff5 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -486,7 +486,7 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(const QString &fi //This works in concerto with the RequestEtagJob and the Folder object to check if the remote folder changed. if (map.contains("getetag")) { if (_firstEtag.isEmpty()) { - _firstEtag = parseEtag(map.value("getetag").toUtf8()); // for directory itself + _firstEtag = parseEtag(map.value(QStringLiteral("getetag")).toUtf8()); // for directory itself } } } diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 26b06e15f1..945b74e799 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -127,7 +127,7 @@ public: // This is not actually a network job, it is just a job signals: void firstDirectoryPermissions(RemotePermissions); - void etag(const QString &, const QDateTime &time); + void etag(const QByteArray &, const QDateTime &time); void finished(const HttpResult> &result); private slots: @@ -141,7 +141,7 @@ private slots: private: QVector _results; QString _subPath; - QString _firstEtag; + QByteArray _firstEtag; QByteArray _fileId; AccountPtr _account; // The first result is for the directory itself and need to be ignored. diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index adcc9bc4f0..9224afc769 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -113,8 +113,8 @@ bool RequestEtagJob::finished() if (httpCode == 207) { // Parse DAV response QXmlStreamReader reader(reply()); - reader.addExtraNamespaceDeclaration(QXmlStreamNamespaceDeclaration("d", "DAV:")); - QString etag; + reader.addExtraNamespaceDeclaration(QXmlStreamNamespaceDeclaration(QStringLiteral("d"), QStringLiteral("DAV:"))); + QByteArray etag; while (!reader.atEnd()) { QXmlStreamReader::TokenType type = reader.readNext(); if (type == QXmlStreamReader::StartElement && reader.namespaceUri() == QLatin1String("DAV:")) { @@ -123,9 +123,9 @@ bool RequestEtagJob::finished() auto etagText = reader.readElementText(); auto parsedTag = parseEtag(etagText.toUtf8()); if (!parsedTag.isEmpty()) { - etag += QString::fromUtf8(parsedTag); + etag += parsedTag; } else { - etag += etagText; + etag += etagText.toUtf8(); } } } diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index c52883b078..45fb12a223 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -349,8 +349,8 @@ public: void start() override; signals: - void etagRetrieved(const QString &etag, const QDateTime &time); - void finishedWithResult(const HttpResult &etag); + void etagRetrieved(const QByteArray &etag, const QDateTime &time); + void finishedWithResult(const HttpResult &etag); private slots: bool finished() override; @@ -421,10 +421,10 @@ signals: * @param statusCode - the OCS status code: 100 (!) for success */ void etagResponseHeaderReceived(const QByteArray &value, int statusCode); - + /** * @brief desktopNotificationStatusReceived - signal to report if notifications are allowed - * @param status - set desktop notifications allowed status + * @param status - set desktop notifications allowed status */ void allowDesktopNotificationsChanged(bool isAllowed); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index e8cb1863d4..a3307f915b 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -614,7 +614,7 @@ void SyncEngine::slotFolderDiscovered(bool local, const QString &folder) emit transmissionProgress(*_progressInfo); } -void SyncEngine::slotRootEtagReceived(const QString &e, const QDateTime &time) +void SyncEngine::slotRootEtagReceived(const QByteArray &e, const QDateTime &time) { if (_remoteRootEtag.isEmpty()) { qCDebug(lcEngine) << "Root etag:" << e; diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index a9e50a1979..0f5e2264e5 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -138,7 +138,7 @@ public: signals: // During update, before reconcile - void rootEtag(const QString &, const QDateTime &); + void rootEtag(const QByteArray &, const QDateTime &); // after the above signals. with the items that actually need propagating void aboutToPropagate(SyncFileItemVector &); @@ -174,7 +174,7 @@ signals: private slots: void slotFolderDiscovered(bool local, const QString &folder); - void slotRootEtagReceived(const QString &, const QDateTime &time); + void slotRootEtagReceived(const QByteArray &, const QDateTime &time); /** When the discovery phase discovers an item */ void slotItemDiscovered(const SyncFileItemPtr &item); @@ -234,7 +234,7 @@ private: bool _syncRunning; QString _localPath; QString _remotePath; - QString _remoteRootEtag; + QByteArray _remoteRootEtag; SyncJournalDb *_journal; QScopedPointer _discoveryPhase; QSharedPointer _propagator; From 6c1073db92f3830c0f9b84d198ae283915c4e3cf Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 26 Feb 2021 12:02:06 +0100 Subject: [PATCH 18/33] Fix another url for etag request Fixes: #7838 --- src/libsync/discovery.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 45bcf96a16..8272ab2495 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -696,7 +696,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( } else { // we need to make a request to the server to know that the original file is deleted on the server _pendingAsyncJobs++; - auto job = new RequestEtagJob(_discoveryData->_account, originalPath, this); + auto job = new RequestEtagJob(_discoveryData->_account, _discoveryData->_remoteFolder + originalPath, this); connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult &etag) mutable { _pendingAsyncJobs--; QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs); From 1eee5c849e7a7b3ceae6de6d465c9dd1b07c5091 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 25 Feb 2021 15:07:41 +0100 Subject: [PATCH 19/33] Add assert to ensure we used _remotePath as base --- src/libsync/abstractnetworkjob.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp index be9c01c931..bbae59c5d0 100644 --- a/src/libsync/abstractnetworkjob.cpp +++ b/src/libsync/abstractnetworkjob.cpp @@ -149,11 +149,15 @@ void AbstractNetworkJob::adoptRequest(QNetworkReply *reply) QUrl AbstractNetworkJob::makeAccountUrl(const QString &relativePath) const { + // ensure we always used the remote folder + OC_ASSERT(relativePath.startsWith(QLatin1Char('/'))); return Utility::concatUrlPath(_account->url(), relativePath); } QUrl AbstractNetworkJob::makeDavUrl(const QString &relativePath) const { + // ensure we always used the remote folder + OC_ASSERT(relativePath.startsWith(QLatin1Char('/'))); return Utility::concatUrlPath(_account->davUrl(), relativePath); } From acd83a2998802ee00b3feb27668938926fe9613c Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 26 Feb 2021 12:02:52 +0100 Subject: [PATCH 20/33] Ensure unit test are using absolute paths --- src/libsync/abstractnetworkjob.cpp | 4 ++-- test/syncenginetestutils.cpp | 26 +++++++++++++++++++------- test/syncenginetestutils.h | 1 + 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp index bbae59c5d0..e026ce5451 100644 --- a/src/libsync/abstractnetworkjob.cpp +++ b/src/libsync/abstractnetworkjob.cpp @@ -150,14 +150,14 @@ void AbstractNetworkJob::adoptRequest(QNetworkReply *reply) QUrl AbstractNetworkJob::makeAccountUrl(const QString &relativePath) const { // ensure we always used the remote folder - OC_ASSERT(relativePath.startsWith(QLatin1Char('/'))); + ASSERT(relativePath.startsWith(QLatin1Char('/'))) return Utility::concatUrlPath(_account->url(), relativePath); } QUrl AbstractNetworkJob::makeDavUrl(const QString &relativePath) const { // ensure we always used the remote folder - OC_ASSERT(relativePath.startsWith(QLatin1Char('/'))); + ASSERT(relativePath.startsWith(QLatin1Char('/'))) return Utility::concatUrlPath(_account->davUrl(), relativePath); } diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp index 19a87dd35f..001791af27 100644 --- a/test/syncenginetestutils.cpp +++ b/test/syncenginetestutils.cpp @@ -243,6 +243,15 @@ QString FileInfo::path() const return (parentPath.isEmpty() ? QString() : (parentPath + QLatin1Char('/'))) + name; } +QString FileInfo::absolutePath() const +{ + if (parentPath.endsWith(QLatin1Char('/'))) { + return parentPath + name; + } else { + return parentPath + QLatin1Char('/') + name; + } +} + void FileInfo::fixupParentPathRecursively() { auto p = path(); @@ -273,7 +282,7 @@ FakePropfindReply::FakePropfindReply(FileInfo &remoteRootFileInfo, QNetworkAcces QMetaObject::invokeMethod(this, "respond404", Qt::QueuedConnection); return; } - QString prefix = request.url().path().left(request.url().path().size() - fileName.size()); + const QString prefix = request.url().path().left(request.url().path().size() - fileName.size()); // Don't care about the request and just return a full propfind const QString davUri { QStringLiteral("DAV:") }; @@ -288,11 +297,12 @@ FakePropfindReply::FakePropfindReply(FileInfo &remoteRootFileInfo, QNetworkAcces auto writeFileResponse = [&](const FileInfo &fileInfo) { xml.writeStartElement(davUri, QStringLiteral("response")); - QString url = prefix + QString::fromUtf8(QUrl::toPercentEncoding(fileInfo.path(), "/")); + auto url = QString::fromUtf8(QUrl::toPercentEncoding(fileInfo.absolutePath(), "/")); if (!url.endsWith(QChar('/'))) { url.append(QChar('/')); } - xml.writeTextElement(davUri, QStringLiteral("href"), url); + const auto href = OCC::Utility::concatUrlPath(prefix, url).path(); + xml.writeTextElement(davUri, QStringLiteral("href"), href); xml.writeStartElement(davUri, QStringLiteral("propstat")); xml.writeStartElement(davUri, QStringLiteral("prop")); @@ -488,9 +498,11 @@ FakeGetReply::FakeGetReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager:: QString fileName = getFilePathFromUrl(request.url()); Q_ASSERT(!fileName.isEmpty()); fileInfo = remoteRootFileInfo.find(fileName); - if (!fileInfo) - qWarning() << "Could not find file" << fileName << "on the remote"; - QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection); + if (!fileInfo) { + qDebug() << "meh;"; + } + Q_ASSERT_X(fileInfo, Q_FUNC_INFO, "Could not find file on the remote"); + QMetaObject::invokeMethod(this, &FakeGetReply::respond, Qt::QueuedConnection); } void FakeGetReply::respond() @@ -740,7 +752,7 @@ FakeErrorReply::FakeErrorReply(QNetworkAccessManager::Operation op, const QNetwo open(QIODevice::ReadOnly); setAttribute(QNetworkRequest::HttpStatusCodeAttribute, httpErrorCode); setError(InternalServerError, QStringLiteral("Internal Server Fake Error")); - QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection); + QMetaObject::invokeMethod(this, &FakeErrorReply::respond, Qt::QueuedConnection); } void FakeErrorReply::respond() diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index 2c2051eec5..9cca62d1c3 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -143,6 +143,7 @@ public: } QString path() const; + QString absolutePath() const; void fixupParentPathRecursively(); From 48f4c1e9e1d00b9940cfa8368ba5302fb0d10c04 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 2 Mar 2021 10:58:07 +0100 Subject: [PATCH 21/33] Log fallback result --- src/libsync/filesystem.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 7c4f5b6261..5c6cd46100 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -63,9 +63,9 @@ time_t FileSystem::getModTime(const QString &filename) && (stat.modtime != 0)) { result = stat.modtime; } else { - qCWarning(lcFileSystem) << "Could not get modification time for" << filename - << "with csync, using QFileInfo"; result = Utility::qDateTimeToTime_t(QFileInfo(filename).lastModified()); + qCWarning(lcFileSystem) << "Could not get modification time for" << filename + << "with csync, using QFileInfo:" << result; } return result; } From 990ce6ed051ce25e256f63ee67be03d83470b2c1 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Wed, 3 Mar 2021 16:47:35 +0100 Subject: [PATCH 22/33] Dump the last 20 lines of logs to a file when we crash Fixes: #8467 --- src/libsync/logger.cpp | 20 +++++++++++++++++++- src/libsync/logger.h | 4 ++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index 8eb4908ade..692a26ba89 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -32,6 +32,9 @@ #include // for stdout #endif +namespace { +constexpr int CrashLogSize = 20; +} namespace OCC { QtMessageHandler s_originalMessageHandler = nullptr; @@ -52,6 +55,7 @@ static void mirallLogCatcher(QtMsgType type, const QMessageLogContext &ctx, cons if(type == QtFatalMsg) { if (!logger->isNoop()) { + logger->dumpCrashLog(); logger->close(); } #if defined(Q_OS_WIN) @@ -71,7 +75,8 @@ Logger *Logger::instance() Logger::Logger(QObject *parent) : QObject(parent) { - qSetMessagePattern("%{time yyyy-MM-dd hh:mm:ss:zzz} [ %{type} %{category} ]%{if-debug}\t[ %{function} ]%{endif}:\t%{message}"); + qSetMessagePattern(QStringLiteral("%{time yyyy-MM-dd hh:mm:ss:zzz} [ %{type} %{category} ]%{if-debug}\t[ %{function} ]%{endif}:\t%{message}")); + _crashLog.resize(CrashLogSize); #ifndef NO_MSG_HANDLER s_originalMessageHandler = qInstallMessageHandler(mirallLogCatcher); #else @@ -135,6 +140,8 @@ void Logger::doLog(const QString &msg) { { QMutexLocker lock(&_mutex); + _crashLogIndex = (_crashLogIndex + 1) % CrashLogSize; + _crashLog[_crashLogIndex] = msg; if (_logstream) { (*_logstream) << msg << endl; if (_doFileFlush) @@ -267,6 +274,17 @@ void Logger::setLogRules(const QSet &rules) QLoggingCategory::setFilterRules(rules.toList().join(QLatin1Char('\n'))); } +void Logger::dumpCrashLog() +{ + QFile logFile(QDir::tempPath() + QStringLiteral("/" APPLICATION_NAME "-crash.log")); + if (logFile.open(QFile::WriteOnly)) { + QTextStream out(&logFile); + for (int i = 1; i <= CrashLogSize; ++i) { + out << _crashLog[(_crashLogIndex + i) % CrashLogSize] << QLatin1Char('\n'); + } + } +} + static bool compressLog(const QString &originalName, const QString &targetName) { #ifdef ZLIB_FOUND diff --git a/src/libsync/logger.h b/src/libsync/logger.h index 2796409a91..22680f1781 100644 --- a/src/libsync/logger.h +++ b/src/libsync/logger.h @@ -95,6 +95,8 @@ public: } void setLogRules(const QSet &rules); + void dumpCrashLog(); + signals: void logWindowLog(const QString &); @@ -119,6 +121,8 @@ private: QString _logDirectory; bool _temporaryFolderLogDir = false; QSet _logRules; + QVector _crashLog; + int _crashLogIndex = 0; }; } // namespace OCC From df567efd37156e50b8d4dd842a358f64448ee30b Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 4 Mar 2021 09:55:42 +0100 Subject: [PATCH 23/33] Remove dead code --- src/libsync/logger.cpp | 23 ----------------------- src/libsync/logger.h | 14 +------------- 2 files changed, 1 insertion(+), 36 deletions(-) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index 692a26ba89..5bfd27389a 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -107,20 +107,6 @@ void Logger::postGuiMessage(const QString &title, const QString &message) emit guiMessage(title, message); } -void Logger::log(Log log) -{ - QString msg; - if (_showTime) { - msg = log.timeStamp.toString(QLatin1String("MM-dd hh:mm:ss:zzz")) + QLatin1Char(' '); - } - - msg += log.message; - // _logs.append(log); - // std::cout << qPrintable(log.message) << std::endl; - - doLog(msg); -} - /** * Returns true if doLog does nothing and need not to be called */ @@ -162,15 +148,6 @@ void Logger::close() } } -void Logger::mirallLog(const QString &message) -{ - Log log_; - log_.timeStamp = QDateTime::currentDateTimeUtc(); - log_.message = message; - - Logger::instance()->log(log_); -} - QString Logger::logFile() const { return _logFile.fileName(); diff --git a/src/libsync/logger.h b/src/libsync/logger.h index 22680f1781..96697871e6 100644 --- a/src/libsync/logger.h +++ b/src/libsync/logger.h @@ -27,12 +27,6 @@ namespace OCC { -struct Log -{ - QDateTime timeStamp; - QString message; -}; - /** * @brief The Logger class * @ingroup libsync @@ -44,14 +38,9 @@ public: bool isNoop() const; bool isLoggingToFile() const; - void log(Log log); void doLog(const QString &log); void close(); - static void mirallLog(const QString &message); - - const QList &logs() const { return _logs; } - static Logger *instance(); void postGuiLog(const QString &title, const QString &message); @@ -109,8 +98,7 @@ public slots: private: Logger(QObject *parent = nullptr); - ~Logger(); - QList _logs; + ~Logger() override; bool _showTime = true; QFile _logFile; bool _doFileFlush = false; From 6324ac9cacbb4a2016d18a090ab86d544b0116c0 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 4 Mar 2021 10:00:07 +0100 Subject: [PATCH 24/33] Cleanup members --- src/libsync/logger.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libsync/logger.h b/src/libsync/logger.h index 96697871e6..32d36d617e 100644 --- a/src/libsync/logger.h +++ b/src/libsync/logger.h @@ -99,7 +99,6 @@ public slots: private: Logger(QObject *parent = nullptr); ~Logger() override; - bool _showTime = true; QFile _logFile; bool _doFileFlush = false; int _logExpire = 0; From 1ff3a0f8b695f4534298dbdd82ae9a3f2723ad30 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 4 Mar 2021 13:25:47 +0100 Subject: [PATCH 25/33] Remove anchient debug msg that mutated to a warning over the year --- src/3rdparty/libcrashreporter-qt | 2 +- src/libsync/owncloudpropagator.cpp | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/3rdparty/libcrashreporter-qt b/src/3rdparty/libcrashreporter-qt index 34b4665bd7..5423c0ef54 160000 --- a/src/3rdparty/libcrashreporter-qt +++ b/src/3rdparty/libcrashreporter-qt @@ -1 +1 @@ -Subproject commit 34b4665bd7ae5a86115f366b3ec4935d7ecb769f +Subproject commit 5423c0ef54f99cea2628c2cb3f48e39e215f9961 diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index e2f3b69509..ea082b9c99 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -535,7 +535,6 @@ bool OwncloudPropagator::localFileNameClash(const QString &relFile) #ifdef Q_OS_MAC const QFileInfo fileInfo(file); if (!fileInfo.exists()) { - qCWarning(lcPropagator) << "No valid fileinfo"; return false; } else { // Need to normalize to composited form because of QTBUG-39622/QTBUG-55896 From d16b2bd369674897dc3b7c80b6fcaa70add5c873 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 4 Mar 2021 11:27:41 +0100 Subject: [PATCH 26/33] Send crash log as comment --- src/crashreporter/main.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/crashreporter/main.cpp b/src/crashreporter/main.cpp index 7d647426f0..9d057e05a8 100644 --- a/src/crashreporter/main.cpp +++ b/src/crashreporter/main.cpp @@ -18,7 +18,9 @@ #include #include +#include #include +#include int main(int argc, char *argv[]) { @@ -52,6 +54,14 @@ int main(int argc, char *argv[]) reporter.setWindowTitle(CRASHREPORTER_PRODUCT_NAME); reporter.setText("

Sorry! " CRASHREPORTER_PRODUCT_NAME " crashed. Please tell us about it! " CRASHREPORTER_PRODUCT_NAME " has created an error report for you that can help improve the stability in the future. You can now send this report directly to the " CRASHREPORTER_PRODUCT_NAME " developers.

"); + const QFileInfo crashLog(QDir::tempPath() + QStringLiteral("/" CRASHREPORTER_PRODUCT_NAME "-crash.log")); + if (crashLog.exists()) { + QFile inFile(crashLog.filePath()); + if (inFile.open(QFile::ReadOnly)) { + reporter.setComment(inFile.readAll()); + } + } + reporter.setReportData("BuildID", CRASHREPORTER_BUILD_ID); reporter.setReportData("ProductName", CRASHREPORTER_PRODUCT_NAME); reporter.setReportData("Version", CRASHREPORTER_VERSION_STRING); From 44fa4aad888054c19ba744c2fc3948f5aa496104 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 4 Mar 2021 10:28:53 +0100 Subject: [PATCH 27/33] Always call doLog to ensure we get a crash log --- src/libsync/logger.cpp | 56 ++++++++++-------------------------------- src/libsync/logger.h | 10 ++++---- 2 files changed, 18 insertions(+), 48 deletions(-) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index 5bfd27389a..c6281252c8 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -37,35 +37,6 @@ constexpr int CrashLogSize = 20; } namespace OCC { -QtMessageHandler s_originalMessageHandler = nullptr; - -static void mirallLogCatcher(QtMsgType type, const QMessageLogContext &ctx, const QString &message) -{ - auto logger = Logger::instance(); - if (type == QtDebugMsg && !logger->logDebug()) { - if (s_originalMessageHandler) { - s_originalMessageHandler(type, ctx, message); - } - } else if (!logger->isNoop()) { - logger->doLog(qFormatLogMessage(type, ctx, message)); - } - if(type == QtCriticalMsg || type == QtFatalMsg) { - std::cerr << qPrintable(qFormatLogMessage(type, ctx, message)) << std::endl; - } - - if(type == QtFatalMsg) { - if (!logger->isNoop()) { - logger->dumpCrashLog(); - logger->close(); - } -#if defined(Q_OS_WIN) - // Make application terminate in a way that can be caught by the crash reporter - Utility::crash(); -#endif - } -} - - Logger *Logger::instance() { static Logger log; @@ -78,9 +49,9 @@ Logger::Logger(QObject *parent) qSetMessagePattern(QStringLiteral("%{time yyyy-MM-dd hh:mm:ss:zzz} [ %{type} %{category} ]%{if-debug}\t[ %{function} ]%{endif}:\t%{message}")); _crashLog.resize(CrashLogSize); #ifndef NO_MSG_HANDLER - s_originalMessageHandler = qInstallMessageHandler(mirallLogCatcher); -#else - Q_UNUSED(mirallLogCatcher) + qInstallMessageHandler([](QtMsgType type, const QMessageLogContext &ctx, const QString &message) { + Logger::instance()->doLog(type, ctx, message); + }); #endif } @@ -107,23 +78,15 @@ void Logger::postGuiMessage(const QString &title, const QString &message) emit guiMessage(title, message); } -/** - * Returns true if doLog does nothing and need not to be called - */ -bool Logger::isNoop() const -{ - QMutexLocker lock(&_mutex); - return !_logstream; -} - bool Logger::isLoggingToFile() const { QMutexLocker lock(&_mutex); return _logstream; } -void Logger::doLog(const QString &msg) +void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString &message) { + const QString msg = qFormatLogMessage(type, ctx, message); { QMutexLocker lock(&_mutex); _crashLogIndex = (_crashLogIndex + 1) % CrashLogSize; @@ -133,13 +96,20 @@ void Logger::doLog(const QString &msg) if (_doFileFlush) _logstream->flush(); } + if (type == QtFatalMsg) { + close(); +#if defined(Q_OS_WIN) + // Make application terminate in a way that can be caught by the crash reporter + Utility::crash(); +#endif + } } emit logWindowLog(msg); } void Logger::close() { - QMutexLocker lock(&_mutex); + dumpCrashLog(); if (_logstream) { _logstream->flush(); diff --git a/src/libsync/logger.h b/src/libsync/logger.h index 32d36d617e..c37da97faa 100644 --- a/src/libsync/logger.h +++ b/src/libsync/logger.h @@ -35,11 +35,9 @@ class OWNCLOUDSYNC_EXPORT Logger : public QObject { Q_OBJECT public: - bool isNoop() const; bool isLoggingToFile() const; - void doLog(const QString &log); - void close(); + void doLog(QtMsgType type, const QMessageLogContext &ctx, const QString &message); static Logger *instance(); @@ -84,8 +82,6 @@ public: } void setLogRules(const QSet &rules); - void dumpCrashLog(); - signals: void logWindowLog(const QString &); @@ -99,6 +95,10 @@ public slots: private: Logger(QObject *parent = nullptr); ~Logger() override; + + void close(); + void dumpCrashLog(); + QFile _logFile; bool _doFileFlush = false; int _logExpire = 0; From 10e02b0031f0ebe51f67267f4498574779a5ab2e Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 4 Mar 2021 10:29:17 +0100 Subject: [PATCH 28/33] Don't create QStringList copy first --- src/libsync/logger.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index c6281252c8..b43dc28cd8 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -218,7 +218,13 @@ void Logger::disableTemporaryFolderLogDir() void Logger::setLogRules(const QSet &rules) { _logRules = rules; - QLoggingCategory::setFilterRules(rules.toList().join(QLatin1Char('\n'))); + QString tmp; + QTextStream out(&tmp); + for (const auto &p : rules) { + out << p << QLatin1Char('\n'); + } + qDebug() << tmp; + QLoggingCategory::setFilterRules(tmp); } void Logger::dumpCrashLog() From ebaa98fa7a58f4ee6fca8dd07548a4b2546592d5 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 2 Apr 2019 13:35:36 +0200 Subject: [PATCH 29/33] owncloudcmd: Use env vars for chunk sizes #7078 Moves a bunch of env var reading from Folder into SyncOptions. --- src/cmd/cmd.cpp | 5 +++++ src/gui/folder.cpp | 42 ++++++++------------------------------ src/libsync/CMakeLists.txt | 1 + src/libsync/syncoptions.h | 28 +++++++++++++++++++++---- 4 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index 5b1de5e648..ac0a7051e7 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -505,6 +505,11 @@ restart_sync: selectiveSyncFixup(&db, selectiveSyncList); } + SyncOptions opt; + opt.fillFromEnvironmentVariables(); + opt.verifyChunkSizes(); + opt._deltaSyncEnabled = false; + opt._deltaSyncMinFileSize = false; SyncEngine engine(account, options.source_dir, folder, &db); engine.setIgnoreHiddenFiles(options.ignoreHiddenFiles); engine.setNetworkLimits(options.uplimit, options.downlimit); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 4fb5a259e8..814f83f821 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -874,42 +874,18 @@ void Folder::setSyncOptions() opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); opt._moveFilesToTrash = cfgFile.moveToTrash(); opt._vfs = _vfs; + opt._parallelNetworkJobs = _accountState->account()->isHttp2Supported() ? 20 : 6; - QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); - if (!chunkSizeEnv.isEmpty()) { - opt._initialChunkSize = chunkSizeEnv.toUInt(); - } else { - opt._initialChunkSize = cfgFile.chunkSize(); - } - QByteArray minChunkSizeEnv = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE"); - if (!minChunkSizeEnv.isEmpty()) { - opt._minChunkSize = minChunkSizeEnv.toUInt(); - } else { - opt._minChunkSize = cfgFile.minChunkSize(); - } - QByteArray maxChunkSizeEnv = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE"); - if (!maxChunkSizeEnv.isEmpty()) { - opt._maxChunkSize = maxChunkSizeEnv.toUInt(); - } else { - opt._maxChunkSize = cfgFile.maxChunkSize(); - } + opt._initialChunkSize = cfgFile.chunkSize(); + opt._minChunkSize = cfgFile.minChunkSize(); + opt._maxChunkSize = cfgFile.maxChunkSize(); + opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration(); - int maxParallel = qgetenv("OWNCLOUD_MAX_PARALLEL").toUInt(); - opt._parallelNetworkJobs = maxParallel ? maxParallel : _accountState->account()->isHttp2Supported() ? 20 : 6; + opt._deltaSyncEnabled = false; + opt._deltaSyncMinFileSize = false; - // Previously min/max chunk size values didn't exist, so users might - // have setups where the chunk size exceeds the new min/max default - // values. To cope with this, adjust min/max to always include the - // initial chunk size value. - opt._minChunkSize = qMin(opt._minChunkSize, opt._initialChunkSize); - opt._maxChunkSize = qMax(opt._maxChunkSize, opt._initialChunkSize); - - QByteArray targetChunkUploadDurationEnv = qgetenv("OWNCLOUD_TARGET_CHUNK_UPLOAD_DURATION"); - if (!targetChunkUploadDurationEnv.isEmpty()) { - opt._targetChunkUploadDuration = std::chrono::milliseconds(targetChunkUploadDurationEnv.toUInt()); - } else { - opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration(); - } + opt.fillFromEnvironmentVariables(); + opt.verifyChunkSizes(); _engine->setSyncOptions(opt); } diff --git a/src/libsync/CMakeLists.txt b/src/libsync/CMakeLists.txt index 7e6bee31fb..3c38c63744 100644 --- a/src/libsync/CMakeLists.txt +++ b/src/libsync/CMakeLists.txt @@ -51,6 +51,7 @@ set(libsync_SRCS syncfilestatustracker.cpp localdiscoverytracker.cpp syncresult.cpp + syncoptions.cpp theme.cpp clientsideencryption.cpp clientsideencryptionjobs.cpp diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index cc792aa9b1..57d1969baa 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -27,9 +27,8 @@ namespace OCC { */ struct OWNCLOUDSYNC_EXPORT SyncOptions { - SyncOptions() - : _vfs(new VfsOff) - {} + SyncOptions(); + ~SyncOptions(); /** Maximum size (in Bytes) a folder can have without asking for confirmation. * -1 means infinite */ @@ -67,7 +66,28 @@ struct OWNCLOUDSYNC_EXPORT SyncOptions /** The maximum number of active jobs in parallel */ int _parallelNetworkJobs = 6; + + /** Whether delta-synchronization is enabled */ + bool _deltaSyncEnabled = false; + + /** What the minimum file size (in Bytes) is for delta-synchronization */ + qint64 _deltaSyncMinFileSize = 0; + + /** Reads settings from env vars where available. + * + * Currently reads _initialChunkSize, _minChunkSize, _maxChunkSize, + * _targetChunkUploadDuration, _parallelNetworkJobs. + */ + void fillFromEnvironmentVariables(); + + /** Ensure min <= initial <= max + * + * Previously min/max chunk size values didn't exist, so users might + * have setups where the chunk size exceeds the new min/max default + * values. To cope with this, adjust min/max to always include the + * initial chunk size value. + */ + void verifyChunkSizes(); }; - } From 010fccb4fa1a951f0e6a76a41ee4fe0df1d7c143 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 14 Jul 2020 14:19:22 +0200 Subject: [PATCH 30/33] Remove dead code --- src/cmd/cmd.cpp | 2 -- src/gui/folder.cpp | 3 --- src/libsync/syncoptions.h | 6 ------ 3 files changed, 11 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index ac0a7051e7..ca54a34275 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -508,8 +508,6 @@ restart_sync: SyncOptions opt; opt.fillFromEnvironmentVariables(); opt.verifyChunkSizes(); - opt._deltaSyncEnabled = false; - opt._deltaSyncMinFileSize = false; SyncEngine engine(account, options.source_dir, folder, &db); engine.setIgnoreHiddenFiles(options.ignoreHiddenFiles); engine.setNetworkLimits(options.uplimit, options.downlimit); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 814f83f821..cbb192b01d 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -881,9 +881,6 @@ void Folder::setSyncOptions() opt._maxChunkSize = cfgFile.maxChunkSize(); opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration(); - opt._deltaSyncEnabled = false; - opt._deltaSyncMinFileSize = false; - opt.fillFromEnvironmentVariables(); opt.verifyChunkSizes(); diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 57d1969baa..7dff264f06 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -67,12 +67,6 @@ struct OWNCLOUDSYNC_EXPORT SyncOptions /** The maximum number of active jobs in parallel */ int _parallelNetworkJobs = 6; - /** Whether delta-synchronization is enabled */ - bool _deltaSyncEnabled = false; - - /** What the minimum file size (in Bytes) is for delta-synchronization */ - qint64 _deltaSyncMinFileSize = 0; - /** Reads settings from env vars where available. * * Currently reads _initialChunkSize, _minChunkSize, _maxChunkSize, From 4b0122093a97f5f1a68b0c407b79621a97b14f02 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Wed, 17 Feb 2021 15:41:32 +0100 Subject: [PATCH 31/33] Add socket command to upload a selection of files based on a regex (cherry picked from commit 0ded3a56a9f3470a951b18eaa9d3c1b5e8db1135) --- src/gui/CMakeLists.txt | 3 +- src/gui/application.cpp | 2 +- src/gui/folder.cpp | 2 +- src/gui/folderman.cpp | 2 +- src/gui/socketapi/CMakeLists.txt | 8 + src/gui/{ => socketapi}/socketapi.cpp | 140 ++++++++++++------ src/gui/{ => socketapi}/socketapi.h | 4 + src/gui/{ => socketapi}/socketapi_p.h | 3 +- src/gui/{ => socketapi}/socketapisocket_mac.h | 0 .../{ => socketapi}/socketapisocket_mac.mm | 26 ++-- src/gui/socketapi/socketuploadjob.cpp | 93 ++++++++++++ src/gui/socketapi/socketuploadjob.h | 49 ++++++ src/libsync/discovery.cpp | 2 - src/libsync/owncloudpropagator.cpp | 23 ++- src/libsync/owncloudpropagator.h | 2 +- src/libsync/syncengine.cpp | 5 +- src/libsync/syncoptions.cpp | 75 ++++++++++ src/libsync/syncoptions.h | 36 ++++- 18 files changed, 397 insertions(+), 78 deletions(-) create mode 100644 src/gui/socketapi/CMakeLists.txt rename src/gui/{ => socketapi}/socketapi.cpp (92%) rename src/gui/{ => socketapi}/socketapi.h (96%) rename src/gui/{ => socketapi}/socketapi_p.h (96%) rename src/gui/{ => socketapi}/socketapisocket_mac.h (100%) rename src/gui/{ => socketapi}/socketapisocket_mac.mm (87%) create mode 100644 src/gui/socketapi/socketuploadjob.cpp create mode 100644 src/gui/socketapi/socketuploadjob.h create mode 100644 src/libsync/syncoptions.cpp diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index 83ee277063..748526310b 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -87,7 +87,6 @@ set(client_SRCS sharemanager.cpp shareusergroupwidget.cpp sharee.cpp - socketapi.cpp sslbutton.cpp sslerrordialog.cpp syncrunfilelog.cpp @@ -338,6 +337,8 @@ target_link_libraries(nextcloudCore Qt5::QuickControls2 ) +add_subdirectory(socketapi) + if(Qt5WebEngine_FOUND AND Qt5WebEngineWidgets_FOUND) target_link_libraries(nextcloudCore PUBLIC Qt5::WebEngineWidgets) endif() diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 97ecedba98..64b2da8fa3 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -27,7 +27,7 @@ #include "folderman.h" #include "logger.h" #include "configfile.h" -#include "socketapi.h" +#include "socketapi/socketapi.h" #include "sslerrordialog.h" #include "theme.h" #include "clientproxy.h" diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index cbb192b01d..0cb4f472bf 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -28,7 +28,7 @@ #include "clientproxy.h" #include "syncengine.h" #include "syncrunfilelog.h" -#include "socketapi.h" +#include "socketapi/socketapi.h" #include "theme.h" #include "filesystem.h" #include "localdiscoverytracker.h" diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 9b0f297e95..5a38814805 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -17,7 +17,7 @@ #include "folder.h" #include "syncresult.h" #include "theme.h" -#include "socketapi.h" +#include "socketapi/socketapi.h" #include "account.h" #include "accountstate.h" #include "accountmanager.h" diff --git a/src/gui/socketapi/CMakeLists.txt b/src/gui/socketapi/CMakeLists.txt new file mode 100644 index 0000000000..9290269eed --- /dev/null +++ b/src/gui/socketapi/CMakeLists.txt @@ -0,0 +1,8 @@ +target_sources(nextcloudCore PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/socketapi.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/socketuploadjob.cpp +) + +if( APPLE ) + target_sources(nextcloudCore PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/socketapisocket_mac.mm) +endif() diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi/socketapi.cpp similarity index 92% rename from src/gui/socketapi.cpp rename to src/gui/socketapi/socketapi.cpp index bd0dc40b7d..4335c4e103 100644 --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi/socketapi.cpp @@ -16,9 +16,11 @@ #include "socketapi.h" #include "socketapi_p.h" +#include "socketapi/socketuploadjob.h" #include "conflictdialog.h" #include "conflictsolver.h" + #include "config.h" #include "configfile.h" #include "folderman.h" @@ -32,6 +34,7 @@ #include "account.h" #include "accountstate.h" #include "account.h" +#include "accountmanager.h" #include "capabilities.h" #include "common/asserts.h" #include "guiutility.h" @@ -57,6 +60,7 @@ #include +#include #include #include #include @@ -66,6 +70,8 @@ #include #include +#include +#include #ifdef Q_OS_MAC #include @@ -94,8 +100,9 @@ QStringList split(const QString &data) using namespace OCC; -QList allObjects(const QList &widgets) { - QList objects; +QList allObjects(const QList &widgets) +{ + QList objects; std::copy(widgets.constBegin(), widgets.constEnd(), std::back_inserter(objects)); objects << qApp; @@ -103,11 +110,11 @@ QList allObjects(const QList &widgets) { return objects; } -QObject *findWidget(const QString &queryString, const QList &widgets = QApplication::allWidgets()) +QObject *findWidget(const QString &queryString, const QList &widgets = QApplication::allWidgets()) { auto objects = allObjects(widgets); - QList::const_iterator foundWidget; + QList::const_iterator foundWidget; if (queryString.contains('>')) { qCDebug(lcSocketApi) << "queryString contains >"; @@ -119,33 +126,34 @@ QObject *findWidget(const QString &queryString, const QList &widgets = qCDebug(lcSocketApi) << "Find parent: " << parentQueryString; auto parent = findWidget(parentQueryString); - if(!parent) { + if (!parent) { return nullptr; } auto childQueryString = subQueries[1].trimmed(); - auto child = findWidget(childQueryString, parent->findChildren()); + auto child = findWidget(childQueryString, parent->findChildren()); qCDebug(lcSocketApi) << "found child: " << !!child; return child; - } else if(queryString.startsWith('#')) { + } else if (queryString.startsWith('#')) { auto objectName = queryString.mid(1); qCDebug(lcSocketApi) << "find objectName: " << objectName; foundWidget = std::find_if(objects.constBegin(), objects.constEnd(), [&](QObject *widget) { return widget->objectName() == objectName; }); } else { - QList matches; - std::copy_if(objects.constBegin(), objects.constEnd(), std::back_inserter(matches), [&](QObject* widget) { + QList matches; + std::copy_if(objects.constBegin(), objects.constEnd(), std::back_inserter(matches), [&](QObject *widget) { return widget->inherits(queryString.toLatin1()); }); - std::for_each(matches.constBegin(), matches.constEnd(), [](QObject* w) { - if(!w) return; + std::for_each(matches.constBegin(), matches.constEnd(), [](QObject *w) { + if (!w) + return; qCDebug(lcSocketApi) << "WIDGET: " << w->objectName() << w->metaObject()->className(); }); - if(matches.empty()) { + if (matches.empty()) { return nullptr; } return matches[0]; @@ -189,6 +197,11 @@ Q_LOGGING_CATEGORY(lcSocketApi, "nextcloud.gui.socketapi", QtInfoMsg) Q_LOGGING_CATEGORY(lcPublicLink, "nextcloud.gui.socketapi.publiclink", QtInfoMsg) +void SocketListener::sendMessage(const QString &function, const QJsonObject &obj, bool doWait) const +{ + sendMessage(function + QLatin1Char(':') + QJsonDocument(obj).toJson(QJsonDocument::Compact), doWait); +} + void SocketListener::sendMessage(const QString &message, bool doWait) const { if (!socket) { @@ -248,21 +261,20 @@ SocketApi::SocketApi(QObject *parent) CFURLRef url = (CFURLRef)CFAutorelease((CFURLRef)CFBundleCopyBundleURL(CFBundleGetMainBundle())); QString bundlePath = QUrl::fromCFURL(url).path(); - auto _system = [](const QString &cmd, const QStringList &args){ + auto _system = [](const QString &cmd, const QStringList &args) { QProcess process; process.setProcessChannelMode(QProcess::MergedChannels); process.start(cmd, args); - if (!process.waitForFinished()) - { + if (!process.waitForFinished()) { qCWarning(lcSocketApi) << "Failed to load shell extension:" << cmd << args.join(" ") << process.errorString(); } else { - qCInfo(lcSocketApi) << (process.exitCode() != 0 ? "Failed to load" : "Loaded") << "shell extension:" << cmd << args.join(" ") << process.readAll(); + qCInfo(lcSocketApi) << (process.exitCode() != 0 ? "Failed to load" : "Loaded") << "shell extension:" << cmd << args.join(" ") << process.readAll(); } }; // Add it again. This was needed for Mojave to trigger a load. - _system(QStringLiteral("pluginkit"), {QStringLiteral("-a"),QStringLiteral("%1Contents/PlugIns/FinderSyncExt.appex/").arg(bundlePath)}); + _system(QStringLiteral("pluginkit"), { QStringLiteral("-a"), QStringLiteral("%1Contents/PlugIns/FinderSyncExt.appex/").arg(bundlePath) }); // Tell Finder to use the Extension (checking it from System Preferences -> Extensions) - _system(QStringLiteral("pluginkit"), {QStringLiteral("-e"), QStringLiteral("use"), QStringLiteral("-i"), QStringLiteral(APPLICATION_REV_DOMAIN ".FinderSyncExt")}); + _system(QStringLiteral("pluginkit"), { QStringLiteral("-e"), QStringLiteral("use"), QStringLiteral("-i"), QStringLiteral(APPLICATION_REV_DOMAIN ".FinderSyncExt") }); #endif } else if (Utility::isLinux() || Utility::isBSD()) { @@ -370,13 +382,12 @@ void SocketApi::slotReadSocket() // make sure that the path will match, especially on OS X. const QString line = QString::fromUtf8(socket->readLine().trimmed()).normalized(QString::NormalizationForm_C); qCInfo(lcSocketApi) << "Received SocketAPI message <--" << line << "from" << socket; - const QByteArray command = line.mid(0, line.indexOf(QLatin1Char(':'))).toUtf8(); + const QByteArray command = line.midRef(0, line.indexOf(QLatin1Char(':'))).toUtf8().toUpper().replace("/", "_"); const QByteArray functionWithArguments = "command_" + command + (command.startsWith("ASYNC_") ? "(QSharedPointer)" : "(QString,SocketListener*)"); const int indexOfMethod = staticMetaObject.indexOfMethod(functionWithArguments); const auto argument = line.midRef(command.length() + 1); if (command.startsWith("ASYNC_")) { - auto arguments = argument.split('|'); if (arguments.size() != 2) { listener->sendMessage(QStringLiteral("argument count is wrong")); @@ -392,10 +403,10 @@ void SocketApi::slotReadSocket() if (indexOfMethod != -1) { staticMetaObject.method(indexOfMethod) .invoke(this, Qt::QueuedConnection, - Q_ARG(QSharedPointer, socketApiJob)); + Q_ARG(QSharedPointer, socketApiJob)); } else { qCWarning(lcSocketApi) << "The command is not supported by this version of the client:" << command - << "with argument:" << argument; + << "with argument:" << argument; socketApiJob->reject(QStringLiteral("command not found")); } } else { @@ -616,20 +627,20 @@ class GetOrCreatePublicLinkShare : public QObject Q_OBJECT public: GetOrCreatePublicLinkShare(const AccountPtr &account, const QString &localFile, - std::function targetFun, QObject *parent) + QObject *parent) : QObject(parent) + , _account(account) , _shareManager(account) , _localFile(localFile) - , _targetFun(targetFun) { connect(&_shareManager, &ShareManager::sharesFetched, this, &GetOrCreatePublicLinkShare::sharesFetched); connect(&_shareManager, &ShareManager::linkShareCreated, this, &GetOrCreatePublicLinkShare::linkShareCreated); + connect(&_shareManager, &ShareManager::linkShareRequiresPassword, + this, &GetOrCreatePublicLinkShare::linkShareRequiresPassword); connect(&_shareManager, &ShareManager::serverError, this, &GetOrCreatePublicLinkShare::serverError); - connect(&_shareManager, &ShareManager::linkShareRequiresPassword, - this, &GetOrCreatePublicLinkShare::passwordRequired); } void run() @@ -642,6 +653,7 @@ private slots: void sharesFetched(const QList> &shares) { auto shareName = SocketApi::tr("Context menu share"); + // If there already is a context menu share, reuse it for (const auto &share : shares) { const auto linkShare = qSharedPointerDynamicCast(share); @@ -683,6 +695,13 @@ private slots: _shareManager.createLinkShare(_localFile, QString(), password); } + void linkShareRequiresPassword(const QString &message) + { + qCInfo(lcPublicLink) << "Could not create link share:" << message; + emit error(message); + deleteLater(); + } + void serverError(int code, const QString &message) { qCWarning(lcPublicLink) << "Share fetch/create error" << code << message; @@ -692,19 +711,24 @@ private slots: tr("Could not retrieve or create the public link share. Error:\n\n%1").arg(message), QMessageBox::Ok, QMessageBox::NoButton); + emit error(message); deleteLater(); } +signals: + void done(const QString &link); + void error(const QString &message); + private: void success(const QString &link) { - _targetFun(link); + emit done(link); deleteLater(); } + AccountPtr _account; ShareManager _shareManager; QString _localFile; - std::function _targetFun; }; #else @@ -732,7 +756,11 @@ void SocketApi::command_COPY_PUBLIC_LINK(const QString &localFile, SocketListene return; AccountPtr account = fileData.folder->accountState()->account(); - auto job = new GetOrCreatePublicLinkShare(account, fileData.serverRelativePath, [](const QString &url) { copyUrlToClipboard(url); }, this); + auto job = new GetOrCreatePublicLinkShare(account, fileData.serverRelativePath, this); + connect(job, &GetOrCreatePublicLinkShare::done, this, + [](const QString &url) { copyUrlToClipboard(url); }); + connect(job, &GetOrCreatePublicLinkShare::error, this, + [=]() { emit shareCommandReceived(fileData.serverRelativePath, fileData.localPath, ShareDialogStartPage::PublicLinks); }); job->run(); } @@ -907,6 +935,22 @@ void SocketApi::command_MOVE_ITEM(const QString &localFile, SocketListener *) solver.setRemoteVersionFilename(target); } +void SocketApi::command_V2_LIST_ACCOUNTS(const QString &, SocketListener *listener) const +{ + QJsonArray out; + for (auto acc : AccountManager::instance()->accounts()) { + // TODO: Use uuid once https://github.com/owncloud/client/pull/8397 is merged + out << QJsonObject({ { "name", acc->account()->displayName() }, { "id", acc->account()->id() } }); + } + listener->sendMessage(QStringLiteral("V2/ACCOUNTS"), { { "accounts", out } }); +} + +void SocketApi::command_V2_UPLOAD_FILES_FROM(const QString &argument, SocketListener *listener) const +{ + auto job = new SocketUploadJob(listener, argument); + job->start(); +} + void SocketApi::emailPrivateLink(const QString &link) { Utility::openEmailComposer( @@ -952,8 +996,7 @@ void SocketApi::sendSharingContextMenuOptions(const FileData &fileData, SocketLi // If sharing is globally disabled, do not show any sharing entries. // If there is no permission to share for this file, add a disabled entry saying so if (isOnTheServer && !record._remotePerm.isNull() && !record._remotePerm.hasPermission(RemotePermissions::CanReshare)) { - listener->sendMessage(QLatin1String("MENU_ITEM:DISABLED:d:") + (!record.isDirectory() - ? tr("Resharing this file is not allowed") : tr("Resharing this folder is not allowed"))); + listener->sendMessage(QLatin1String("MENU_ITEM:DISABLED:d:") + (!record.isDirectory() ? tr("Resharing this file is not allowed") : tr("Resharing this folder is not allowed"))); } else { listener->sendMessage(QLatin1String("MENU_ITEM:SHARE") + flagString + tr("Share options")); @@ -1153,13 +1196,13 @@ void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListe // TODO: Should be a submenu, should use icons auto makePinContextMenu = [&](bool makeAvailableLocally, bool freeSpace) { listener->sendMessage(QLatin1String("MENU_ITEM:CURRENT_PIN:d:") - + Utility::vfsCurrentAvailabilityText(*combined)); + + Utility::vfsCurrentAvailabilityText(*combined)); listener->sendMessage(QLatin1String("MENU_ITEM:MAKE_AVAILABLE_LOCALLY:") - + (makeAvailableLocally ? QLatin1String(":") : QLatin1String("d:")) - + Utility::vfsPinActionText()); + + (makeAvailableLocally ? QLatin1String(":") : QLatin1String("d:")) + + Utility::vfsPinActionText()); listener->sendMessage(QLatin1String("MENU_ITEM:MAKE_ONLINE_ONLY:") - + (freeSpace ? QLatin1String(":") : QLatin1String("d:")) - + Utility::vfsFreeSpaceActionText()); + + (freeSpace ? QLatin1String(":") : QLatin1String("d:")) + + Utility::vfsFreeSpaceActionText()); }; if (combined) { @@ -1245,20 +1288,20 @@ void SocketApi::command_ASYNC_GET_WIDGET_PROPERTY(const QSharedPointerproperty(segment.toUtf8().constData()); - if(var.canConvert()) { + if (var.canConvert()) { var.convert(QMetaType::QString); value = var.value(); break; } - auto tmpObject = var.value(); - if(tmpObject) { + auto tmpObject = var.value(); + if (tmpObject) { currentObject = tmpObject; } else { QString message = QString(QLatin1String("Widget not found: 3: %1")).arg(widgetName); @@ -1281,7 +1324,7 @@ void SocketApi::command_ASYNC_SET_WIDGET_PROPERTY(const QSharedPointersetProperty(arguments["property"].toString().toUtf8().constData(), - arguments["value"]); + arguments["value"]); job->resolve(); } @@ -1349,20 +1392,20 @@ void SocketApi::command_ASYNC_ASSERT_ICON_IS_EQUAL(const QSharedPointerproperty(segment.toUtf8().constData()); - if(var.canConvert()) { + if (var.canConvert()) { var.convert(QMetaType::QIcon); value = var.value(); break; } - auto tmpObject = var.value(); - if(tmpObject) { + auto tmpObject = var.value(); + if (tmpObject) { currentObject = tmpObject; } else { job->reject(QString(QLatin1String("Icon not found: %1")).arg(propertyName)); @@ -1370,12 +1413,11 @@ void SocketApi::command_ASYNC_ASSERT_ICON_IS_EQUAL(const QSharedPointerarguments()[QLatin1String("iconName")].toString(); - if (value.name() == iconName) { + if (value.name() == iconName) { job->resolve(); } else { job->reject("iconName " + iconName + " does not match: " + value.name()); } - } #endif diff --git a/src/gui/socketapi.h b/src/gui/socketapi/socketapi.h similarity index 96% rename from src/gui/socketapi.h rename to src/gui/socketapi/socketapi.h index 1c2ef49f72..cd7d9ff010 100644 --- a/src/gui/socketapi.h +++ b/src/gui/socketapi/socketapi.h @@ -129,6 +129,10 @@ private: Q_INVOKABLE void command_OPEN(const QString &localFile, SocketListener *listener); #endif + // External sync + Q_INVOKABLE void command_V2_LIST_ACCOUNTS(const QString &argument, SocketListener *listener) const; + Q_INVOKABLE void command_V2_UPLOAD_FILES_FROM(const QString &argument, SocketListener *listener) const; + // Fetch the private link and call targetFun void fetchPrivateLinkUrlHelper(const QString &localFile, const std::function &targetFun); diff --git a/src/gui/socketapi_p.h b/src/gui/socketapi/socketapi_p.h similarity index 96% rename from src/gui/socketapi_p.h rename to src/gui/socketapi/socketapi_p.h index da3e90b9c1..38738c84b8 100644 --- a/src/gui/socketapi_p.h +++ b/src/gui/socketapi/socketapi_p.h @@ -68,6 +68,7 @@ public: } void sendMessage(const QString &message, bool doWait = false) const; + void sendMessage(const QString &function, const QJsonObject &obj, bool doWait = false) const; void sendMessageIfDirectoryMonitored(const QString &message, uint systemDirectoryHash) const { @@ -121,7 +122,7 @@ public: _socketListener->sendMessage(QLatin1String("RESOLVE|") + _jobId + '|' + response); } - void resolve(const QJsonObject &response) { resolve(QJsonDocument{ response }.toJson()); } + void resolve(const QJsonObject &response) { resolve(QJsonDocument { response }.toJson()); } const QJsonObject &arguments() { return _arguments; } diff --git a/src/gui/socketapisocket_mac.h b/src/gui/socketapi/socketapisocket_mac.h similarity index 100% rename from src/gui/socketapisocket_mac.h rename to src/gui/socketapi/socketapisocket_mac.h diff --git a/src/gui/socketapisocket_mac.mm b/src/gui/socketapi/socketapisocket_mac.mm similarity index 87% rename from src/gui/socketapisocket_mac.mm rename to src/gui/socketapi/socketapisocket_mac.mm index f018ac752b..926c34d8a6 100644 --- a/src/gui/socketapisocket_mac.mm +++ b/src/gui/socketapi/socketapisocket_mac.mm @@ -16,7 +16,7 @@ #import @protocol ChannelProtocol -- (void)sendMessage:(NSData*)msg; +- (void)sendMessage:(NSData *)msg; @end @protocol RemoteEndProtocol @@ -31,7 +31,7 @@ @interface Server : NSObject @property SocketApiServerPrivate *wrapper; - (instancetype)initWithWrapper:(SocketApiServerPrivate *)wrapper; -- (void)registerClient:(NSDistantObject *)remoteEnd; +- (void)registerClient:(NSDistantObject *)remoteEnd; @end class SocketApiSocketPrivate @@ -39,13 +39,13 @@ class SocketApiSocketPrivate public: SocketApiSocket *q_ptr; - SocketApiSocketPrivate(NSDistantObject *remoteEnd); + SocketApiSocketPrivate(NSDistantObject *remoteEnd); ~SocketApiSocketPrivate(); // release remoteEnd void disconnectRemote(); - NSDistantObject *remoteEnd; + NSDistantObject *remoteEnd; LocalEnd *localEnd; QByteArray inBuffer; bool isRemoteDisconnected = false; @@ -59,7 +59,7 @@ public: SocketApiServerPrivate(); ~SocketApiServerPrivate(); - QList pendingConnections; + QList pendingConnections; NSConnection *connection; Server *server; }; @@ -73,7 +73,7 @@ public: return self; } -- (void)sendMessage:(NSData*)msg +- (void)sendMessage:(NSData *)msg { if (_wrapper) { _wrapper->inBuffer += QByteArray::fromRawNSData(msg); @@ -81,7 +81,7 @@ public: } } -- (void)connectionDidDie:(NSNotification*)notification +- (void)connectionDidDie:(NSNotification *)notification { #pragma unused(notification) if (_wrapper) { @@ -99,7 +99,7 @@ public: return self; } -- (void)registerClient:(NSDistantObject *)remoteEnd +- (void)registerClient:(NSDistantObject *)remoteEnd { // This saves a few mach messages that would otherwise be needed to query the interface [remoteEnd setProtocolForProxy:@protocol(RemoteEndProtocol)]; @@ -150,7 +150,7 @@ qint64 SocketApiSocket::writeData(const char *data, qint64 len) // Since FinderSync already runs in a separate process, blocking isn't too critical. [d->remoteEnd sendMessage:[NSData dataWithBytesNoCopy:const_cast(data) length:len freeWhenDone:NO]]; return len; - } @catch(NSException* e) { + } @catch (NSException *e) { // connectionDidDie can be notified too late, also interpret any sending exception as a disconnection. d->disconnectRemote(); emit disconnected(); @@ -170,16 +170,16 @@ bool SocketApiSocket::canReadLine() const return d->inBuffer.indexOf('\n', int(pos())) != -1 || QIODevice::canReadLine(); } -SocketApiSocketPrivate::SocketApiSocketPrivate(NSDistantObject *remoteEnd) +SocketApiSocketPrivate::SocketApiSocketPrivate(NSDistantObject *remoteEnd) : remoteEnd(remoteEnd) , localEnd([[LocalEnd alloc] initWithWrapper:this]) { [remoteEnd retain]; // (Ab)use our objective-c object just to catch the notification [[NSNotificationCenter defaultCenter] addObserver:localEnd - selector:@selector(connectionDidDie:) - name:NSConnectionDidDieNotification - object:[remoteEnd connectionForProxy]]; + selector:@selector(connectionDidDie:) + name:NSConnectionDidDieNotification + object:[remoteEnd connectionForProxy]]; } SocketApiSocketPrivate::~SocketApiSocketPrivate() diff --git a/src/gui/socketapi/socketuploadjob.cpp b/src/gui/socketapi/socketuploadjob.cpp new file mode 100644 index 0000000000..8ade844b02 --- /dev/null +++ b/src/gui/socketapi/socketuploadjob.cpp @@ -0,0 +1,93 @@ +/* + * Copyright (C) by Hannah von Reth + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "socketuploadjob.h" +#include "socketapi_p.h" + +#include "accountmanager.h" +#include "common/syncjournaldb.h" +#include "syncengine.h" + +#include +#include +#include + +using namespace OCC; + +SocketUploadJob::SocketUploadJob(OCC::SocketListener *listener, const QString &argument, QObject *parent) + : QObject(parent) + , _listener(listener) +{ + const auto args = QJsonDocument::fromJson(argument.toUtf8()).object(); + _localPath = args[QLatin1String("localPath")].toString(); + _remotePath = args[QLatin1String("remotePath")].toString(); + if (!_remotePath.startsWith("/")) { + _remotePath = QLatin1Char('/') + _remotePath; + } + + _pattern = args[QLatin1String("pattern")].toString(); + // TODO: use uuid + const auto accname = args[QLatin1String("account")][QLatin1String("name")].toString(); + auto account = AccountManager::instance()->account(accname); + + ENFORCE(QFileInfo(_localPath).isAbsolute()) + ENFORCE(_tmp.open()) + + _db = new SyncJournalDb(_tmp.fileName(), this); + _engine = new SyncEngine(account->account(), _localPath.endsWith(QLatin1Char('/')) ? _localPath : _localPath + QLatin1Char('/'), _remotePath, _db); + _engine->setParent(_db); + + connect(_engine, &OCC::SyncEngine::itemCompleted, this, [this](const OCC::SyncFileItemPtr item) { + _syncedFiles.append(item->_file); + }); + + connect(_engine, &OCC::SyncEngine::finished, this, [this](bool ok) { + if (ok) { + finish({}); + } + }); + connect(_engine, &OCC::SyncEngine::syncError, this, &SocketUploadJob::finish); +} + +void SocketUploadJob::start() +{ + auto opt = _engine->syncOptions(); + opt.setFilePattern(_pattern); + if (!opt.fileRegex().isValid()) { + finish(opt.fileRegex().errorString()); + return; + } + _engine->setSyncOptions(opt); + + // create the dir, fail if it already exists + auto mkdir = new OCC::MkColJob(_engine->account(), _remotePath); + connect(mkdir, &OCC::MkColJob::finishedWithoutError, _engine, &OCC::SyncEngine::startSync); + connect(mkdir, &OCC::MkColJob::finishedWithError, this, [this](QNetworkReply *reply) { + if (reply->error() == 202) { + finish(QStringLiteral("Destination %1 already exists").arg(_remotePath)); + } else { + finish(reply->errorString()); + } + }); + mkdir->start(); +} + +void SocketUploadJob::finish(const QString &error) +{ + if (!_finished) { + _finished = true; + _listener->sendMessage(QStringLiteral("V2/UPLOAD_FILES_RESULT"), { { "localPath", _localPath }, { "error", error }, { "syncedFiles", QJsonArray::fromStringList(_syncedFiles) } }); + deleteLater(); + } +} diff --git a/src/gui/socketapi/socketuploadjob.h b/src/gui/socketapi/socketuploadjob.h new file mode 100644 index 0000000000..477b8bf0cd --- /dev/null +++ b/src/gui/socketapi/socketuploadjob.h @@ -0,0 +1,49 @@ +/* + * Copyright (C) by Hannah von Reth + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#pragma once +#include +#include + +#include "socketapi.h" +#include "account.h" + +namespace OCC { + +class SyncJournalDb; +class SyncEngine; + +class SocketUploadJob : public QObject +{ + Q_OBJECT +public: + SocketUploadJob(OCC::SocketListener *listener, const QString &argument, QObject *parent = nullptr); + + void start(); + + void finish(const QString &error); + +private: + SocketListener *_listener; + QString _localPath; + QString _remotePath; + QString _pattern; + QTemporaryFile _tmp; + SyncJournalDb *_db; + SyncEngine *_engine; + QStringList _syncedFiles; + + bool _finished = false; +}; +} diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 8272ab2495..7602a3bd47 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -67,8 +67,6 @@ void ProcessDirectoryJob::process() { ASSERT(_localQueryDone && _serverQueryDone); - QString localDir; - // Build lookup tables for local, remote and db entries. // For suffix-virtual files, the key will normally be the base file name // without the suffix. diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index ea082b9c99..352ec32c2b 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include namespace OCC { @@ -387,7 +388,7 @@ qint64 OwncloudPropagator::smallFileSize() return smallFileSize; } -void OwncloudPropagator::start(const SyncFileItemVector &items) +void OwncloudPropagator::start(SyncFileItemVector &&items) { Q_ASSERT(std::is_sorted(items.begin(), items.end())); @@ -396,6 +397,26 @@ void OwncloudPropagator::start(const SyncFileItemVector &items) * In order to do that we loop over the items. (which are sorted by destination) * When we enter a directory, we can create the directory job and push it on the stack. */ + const auto regex = syncOptions().fileRegex(); + if (regex.isValid()) { + QSet names; + for (auto &i : items) { + if (regex.match(i->_file).hasMatch()) { + int index = -1; + QStringRef ref; + do { + ref = i->_file.midRef(0, index); + names.insert(ref); + index = ref.lastIndexOf(QLatin1Char('/')); + } while (index > 0); + } + } + items.erase(std::remove_if(items.begin(), items.end(), [&names](auto i) { + return !names.contains(QStringRef { &i->_file }); + }), + items.end()); + } + _rootJob.reset(new PropagateRootDirectory(this)); QStack> directories; directories.push(qMakePair(QString(), _rootJob.data())); diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index c2df7749f9..f03ef57593 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -421,7 +421,7 @@ public: ~OwncloudPropagator(); - void start(const SyncFileItemVector &_syncedItems); + void start(SyncFileItemVector &&_syncedItems); const SyncOptions &syncOptions() const; void setSyncOptions(const SyncOptions &syncOptions); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index a3307f915b..d531fcc7d4 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -434,7 +434,7 @@ void SyncEngine::startSync() } if (s_anySyncRunning || _syncRunning) { - ASSERT(false); + ASSERT(false) return; } @@ -725,8 +725,7 @@ void SyncEngine::slotDiscoveryFinished() if (_needsUpdate) Q_EMIT started(); - _propagator->start(_syncItems); - _syncItems.clear(); + _propagator->start(std::move(_syncItems)); qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms"; }; diff --git a/src/libsync/syncoptions.cpp b/src/libsync/syncoptions.cpp new file mode 100644 index 0000000000..588a7c6343 --- /dev/null +++ b/src/libsync/syncoptions.cpp @@ -0,0 +1,75 @@ +/* + * Copyright (C) by Olivier Goffart + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "syncoptions.h" +#include "common/utility.h" + +#include + +using namespace OCC; + +SyncOptions::SyncOptions() + : _vfs(new VfsOff) +{ +} + +SyncOptions::~SyncOptions() +{ +} + +void SyncOptions::fillFromEnvironmentVariables() +{ + QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); + if (!chunkSizeEnv.isEmpty()) + _initialChunkSize = chunkSizeEnv.toUInt(); + + QByteArray minChunkSizeEnv = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE"); + if (!minChunkSizeEnv.isEmpty()) + _minChunkSize = minChunkSizeEnv.toUInt(); + + QByteArray maxChunkSizeEnv = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE"); + if (!maxChunkSizeEnv.isEmpty()) + _maxChunkSize = maxChunkSizeEnv.toUInt(); + + QByteArray targetChunkUploadDurationEnv = qgetenv("OWNCLOUD_TARGET_CHUNK_UPLOAD_DURATION"); + if (!targetChunkUploadDurationEnv.isEmpty()) + _targetChunkUploadDuration = std::chrono::milliseconds(targetChunkUploadDurationEnv.toUInt()); + + int maxParallel = qgetenv("OWNCLOUD_MAX_PARALLEL").toInt(); + if (maxParallel > 0) + _parallelNetworkJobs = maxParallel; +} + +void SyncOptions::verifyChunkSizes() +{ + _minChunkSize = qMin(_minChunkSize, _initialChunkSize); + _maxChunkSize = qMax(_maxChunkSize, _initialChunkSize); +} + +QRegularExpression SyncOptions::fileRegex() const +{ + return _fileRegex; +} + +void SyncOptions::setFilePattern(const QString &pattern) +{ + // full match or a path ending with this pattern + setPathPattern(QStringLiteral("(^|/|\\\\)") + pattern + QLatin1Char('$')); +} + +void SyncOptions::setPathPattern(const QString &pattern) +{ + _fileRegex.setPatternOptions(Utility::fsCasePreserving() ? QRegularExpression::CaseInsensitiveOption : QRegularExpression::NoPatternOption); + _fileRegex.setPattern(pattern); +} diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 7dff264f06..f109ec7fc2 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -15,18 +15,23 @@ #pragma once #include "owncloudlib.h" -#include -#include -#include #include "common/vfs.h" +#include +#include +#include + +#include + + namespace OCC { /** * Value class containing the options given to the sync engine */ -struct OWNCLOUDSYNC_EXPORT SyncOptions +class OWNCLOUDSYNC_EXPORT SyncOptions { +public: SyncOptions(); ~SyncOptions(); @@ -82,6 +87,29 @@ struct OWNCLOUDSYNC_EXPORT SyncOptions * initial chunk size value. */ void verifyChunkSizes(); + + + /** A regular expression to match file names + * If no pattern is provided the default is an invalid regular expression. + */ + QRegularExpression fileRegex() const; + + /** + * A pattern like *.txt, matching only file names + */ + void setFilePattern(const QString &pattern); + + /** + * A pattern like /own.*\/.*txt matching the full path + */ + void setPathPattern(const QString &pattern); + +private: + /** + * Only sync files that mathc the expression + * Invalid pattern by default. + */ + QRegularExpression _fileRegex = QRegularExpression(QStringLiteral("(")); }; } From c273c8f71b95f75f65c06294134ed9ed00a89361 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 19 Feb 2021 16:00:32 +0100 Subject: [PATCH 32/33] Ensure the socket listener still exists --- src/gui/socketapi/socketapi.cpp | 158 +++++++++++++++++--------- src/gui/socketapi/socketapi.h | 7 +- src/gui/socketapi/socketapi_p.h | 53 ++++++--- src/gui/socketapi/socketuploadjob.cpp | 49 ++++---- src/gui/socketapi/socketuploadjob.h | 9 +- 5 files changed, 173 insertions(+), 103 deletions(-) diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp index 4335c4e103..0669ebc64d 100644 --- a/src/gui/socketapi/socketapi.cpp +++ b/src/gui/socketapi/socketapi.cpp @@ -70,8 +70,6 @@ #include #include -#include -#include #ifdef Q_OS_MAC #include @@ -197,11 +195,6 @@ Q_LOGGING_CATEGORY(lcSocketApi, "nextcloud.gui.socketapi", QtInfoMsg) Q_LOGGING_CATEGORY(lcPublicLink, "nextcloud.gui.socketapi.publiclink", QtInfoMsg) -void SocketListener::sendMessage(const QString &function, const QJsonObject &obj, bool doWait) const -{ - sendMessage(function + QLatin1Char(':') + QJsonDocument(obj).toJson(QJsonDocument::Compact), doWait); -} - void SocketListener::sendMessage(const QString &message, bool doWait) const { if (!socket) { @@ -225,16 +218,6 @@ void SocketListener::sendMessage(const QString &message, bool doWait) const } } -struct ListenerHasSocketPred -{ - QIODevice *socket; - ListenerHasSocketPred(QIODevice *socket) - : socket(socket) - { - } - bool operator()(const SocketListener &listener) const { return listener.socket == socket; } -}; - SocketApi::SocketApi(QObject *parent) : QObject(parent) { @@ -242,6 +225,7 @@ SocketApi::SocketApi(QObject *parent) qRegisterMetaType("SocketListener*"); qRegisterMetaType>("QSharedPointer"); + qRegisterMetaType>("QSharedPointer"); if (Utility::isWindows()) { socketPath = QLatin1String(R"(\\.\pipe\)") @@ -312,7 +296,7 @@ SocketApi::~SocketApi() qCDebug(lcSocketApi) << "dtor"; _localServer.close(); // All remaining sockets will be destroyed with _localServer, their parent - ASSERT(_listeners.isEmpty() || _listeners.first().socket->parent() == &_localServer); + ASSERT(_listeners.isEmpty() || _listeners.first()->socket->parent() == &_localServer) _listeners.clear(); } @@ -331,14 +315,13 @@ void SocketApi::slotNewConnection() connect(socket, &QObject::destroyed, this, &SocketApi::slotSocketDestroyed); ASSERT(socket->readAll().isEmpty()); - _listeners.append(SocketListener(socket)); - SocketListener &listener = _listeners.last(); - - foreach (Folder *f, FolderMan::instance()->map()) { + auto listener = QSharedPointer::create(socket); + _listeners.insert(socket, listener); + for (Folder *f : FolderMan::instance()->map()) { if (f->canSync()) { QString message = buildRegisterPathMessage(removeTrailingSlash(f->path())); - qCInfo(lcSocketApi) << "Trying to send SocketAPI Register Path Message -->" << message << "to" << listener.socket; - listener.sendMessage(message); + qCInfo(lcSocketApi) << "Trying to send SocketAPI Register Path Message -->" << message << "to" << listener->socket; + listener->sendMessage(message); } } } @@ -350,13 +333,13 @@ void SocketApi::onLostConnection() auto socket = qobject_cast(sender()); ASSERT(socket); - _listeners.erase(std::remove_if(_listeners.begin(), _listeners.end(), ListenerHasSocketPred(socket)), _listeners.end()); + _listeners.remove(socket); } void SocketApi::slotSocketDestroyed(QObject *obj) { auto *socket = static_cast(obj); - _listeners.erase(std::remove_if(_listeners.begin(), _listeners.end(), ListenerHasSocketPred(socket)), _listeners.end()); + _listeners.remove(socket); } void SocketApi::slotReadSocket() @@ -370,27 +353,38 @@ void SocketApi::slotReadSocket() // the readyRead() signals are received - in that case there won't be a // valid listener. We execute the handler anyway, but it will work with // a SocketListener that doesn't send any messages. - static auto noListener = SocketListener(nullptr); - SocketListener *listener = &noListener; - auto listenerIt = std::find_if(_listeners.begin(), _listeners.end(), ListenerHasSocketPred(socket)); - if (listenerIt != _listeners.end()) { - listener = &*listenerIt; - } - + static auto invalidListener = QSharedPointer::create(nullptr); + const auto listener = _listeners.value(socket, invalidListener); while (socket->canReadLine()) { // Make sure to normalize the input from the socket to // make sure that the path will match, especially on OS X. const QString line = QString::fromUtf8(socket->readLine().trimmed()).normalized(QString::NormalizationForm_C); qCInfo(lcSocketApi) << "Received SocketAPI message <--" << line << "from" << socket; - const QByteArray command = line.midRef(0, line.indexOf(QLatin1Char(':'))).toUtf8().toUpper().replace("/", "_"); - const QByteArray functionWithArguments = "command_" + command + (command.startsWith("ASYNC_") ? "(QSharedPointer)" : "(QString,SocketListener*)"); - const int indexOfMethod = staticMetaObject.indexOfMethod(functionWithArguments); + const int argPos = line.indexOf(QLatin1Char(':')); + const QByteArray command = line.midRef(0, argPos).toUtf8().toUpper(); + const int indexOfMethod = [&] { + QByteArray functionWithArguments = QByteArrayLiteral("command_"); + if (command.startsWith("ASYNC_")) { + functionWithArguments += command + QByteArrayLiteral("(QSharedPointer)"); + } else if (command.startsWith("V2/")) { + functionWithArguments += QByteArrayLiteral("V2_") + command.mid(3) + QByteArrayLiteral("(QSharedPointer)"); + } else { + functionWithArguments += command + QByteArrayLiteral("(QString,SocketListener*)"); + } + Q_ASSERT(staticQtMetaObject.normalizedSignature(functionWithArguments) == functionWithArguments); + const auto out = staticMetaObject.indexOfMethod(functionWithArguments); + if (out == -1) { + listener->sendError(QStringLiteral("Function %1 not found").arg(QString::fromUtf8(functionWithArguments))); + } + ASSERT(out != -1) + return out; + }(); - const auto argument = line.midRef(command.length() + 1); + const auto argument = argPos != -1 ? line.midRef(argPos + 1) : QStringRef(); if (command.startsWith("ASYNC_")) { auto arguments = argument.split('|'); if (arguments.size() != 2) { - listener->sendMessage(QStringLiteral("argument count is wrong")); + listener->sendError(QStringLiteral("argument count is wrong")); return; } @@ -409,15 +403,31 @@ void SocketApi::slotReadSocket() << "with argument:" << argument; socketApiJob->reject(QStringLiteral("command not found")); } + } else if (command.startsWith("V2/")) { + QJsonParseError error; + const auto json = QJsonDocument::fromJson(argument.toUtf8(), &error).object(); + if (error.error != QJsonParseError::NoError) { + qCWarning(lcSocketApi()) << "Invalid json" << argument.toString() << error.errorString(); + listener->sendError(error.errorString()); + return; + } + auto socketApiJob = QSharedPointer::create(listener, command, json); + if (indexOfMethod != -1) { + staticMetaObject.method(indexOfMethod) + .invoke(this, Qt::QueuedConnection, + Q_ARG(QSharedPointer, socketApiJob)); + } else { + qCWarning(lcSocketApi) << "The command is not supported by this version of the client:" << command + << "with argument:" << argument; + socketApiJob->failure(QStringLiteral("command not found")); + } } else { if (indexOfMethod != -1) { // to ensure that listener is still valid we need to call it with Qt::DirectConnection ASSERT(thread() == QThread::currentThread()) staticMetaObject.method(indexOfMethod) .invoke(this, Qt::DirectConnection, Q_ARG(QString, argument.toString()), - Q_ARG(SocketListener *, listener)); - } else { - qCWarning(lcSocketApi) << "The command is not supported by this version of the client:" << command << "with argument:" << argument; + Q_ARG(SocketListener *, listener.data())); } } } @@ -431,10 +441,10 @@ void SocketApi::slotRegisterPath(const QString &alias) Folder *f = FolderMan::instance()->folder(alias); if (f) { - QString message = buildRegisterPathMessage(removeTrailingSlash(f->path())); - foreach (auto &listener, _listeners) { - qCInfo(lcSocketApi) << "Trying to send SocketAPI Register Path Message -->" << message << "to" << listener.socket; - listener.sendMessage(message); + const QString message = buildRegisterPathMessage(removeTrailingSlash(f->path())); + for (const auto &listener : qAsConst(_listeners)) { + qCInfo(lcSocketApi) << "Trying to send SocketAPI Register Path Message -->" << message << "to" << listener->socket; + listener->sendMessage(message); } } @@ -479,8 +489,8 @@ void SocketApi::slotUpdateFolderView(Folder *f) void SocketApi::broadcastMessage(const QString &msg, bool doWait) { - foreach (auto &listener, _listeners) { - listener.sendMessage(msg, doWait); + for (const auto &listener : qAsConst(_listeners)) { + listener->sendMessage(msg, doWait); } } @@ -530,8 +540,8 @@ void SocketApi::broadcastStatusPushMessage(const QString &systemPath, SyncFileSt QString msg = buildMessage(QLatin1String("STATUS"), systemPath, fileStatus.toSocketAPIString()); Q_ASSERT(!systemPath.endsWith('/')); uint directoryHash = qHash(systemPath.left(systemPath.lastIndexOf('/'))); - foreach (auto &listener, _listeners) { - listener.sendMessageIfDirectoryMonitored(msg, directoryHash); + for (const auto &listener : qAsConst(_listeners)) { + listener->sendMessageIfDirectoryMonitored(msg, directoryHash); } } @@ -935,20 +945,20 @@ void SocketApi::command_MOVE_ITEM(const QString &localFile, SocketListener *) solver.setRemoteVersionFilename(target); } -void SocketApi::command_V2_LIST_ACCOUNTS(const QString &, SocketListener *listener) const +void SocketApi::command_V2_LIST_ACCOUNTS(const QSharedPointer &job) const { QJsonArray out; for (auto acc : AccountManager::instance()->accounts()) { // TODO: Use uuid once https://github.com/owncloud/client/pull/8397 is merged out << QJsonObject({ { "name", acc->account()->displayName() }, { "id", acc->account()->id() } }); } - listener->sendMessage(QStringLiteral("V2/ACCOUNTS"), { { "accounts", out } }); + job->success({ { "accounts", out } }); } -void SocketApi::command_V2_UPLOAD_FILES_FROM(const QString &argument, SocketListener *listener) const +void SocketApi::command_V2_UPLOAD_FILES_FROM(const QSharedPointer &job) const { - auto job = new SocketUploadJob(listener, argument); - job->start(); + auto uploadJob = new SocketUploadJob(job); + uploadJob->start(); } void SocketApi::emailPrivateLink(const QString &link) @@ -1429,6 +1439,46 @@ QString SocketApi::buildRegisterPathMessage(const QString &path) return message; } +void SocketApiJob::resolve(const QString &response) +{ + _socketListener->sendMessage(QStringLiteral("RESOLVE|") + _jobId + QLatin1Char('|') + response); +} + +void SocketApiJob::resolve(const QJsonObject &response) +{ + resolve(QJsonDocument { response }.toJson()); +} + +void SocketApiJob::reject(const QString &response) +{ + _socketListener->sendMessage(QStringLiteral("REJECT|") + _jobId + QLatin1Char('|') + response); +} + +SocketApiJobV2::SocketApiJobV2(const QSharedPointer &socketListener, const QByteArray &command, const QJsonObject &arguments) + : _socketListener(socketListener) + , _command(command) + , _jobId(arguments[QStringLiteral("id")].toString()) + , _arguments(arguments[QStringLiteral("arguments")].toObject()) +{ + ASSERT(!_jobId.isEmpty()) +} + +void SocketApiJobV2::success(const QJsonObject &response) const +{ + doFinish(response); +} + +void SocketApiJobV2::failure(const QString &error) const +{ + doFinish({ { QStringLiteral("error"), error } }); +} + +void SocketApiJobV2::doFinish(const QJsonObject &obj) const +{ + _socketListener->sendMessage(_command + QStringLiteral("_RESULT:") + QJsonDocument({ { QStringLiteral("id"), _jobId }, { QStringLiteral("arguments"), obj } }).toJson(QJsonDocument::Compact)); + Q_EMIT finished(); +} + } // namespace OCC #include "socketapi.moc" diff --git a/src/gui/socketapi/socketapi.h b/src/gui/socketapi/socketapi.h index cd7d9ff010..1350607e4c 100644 --- a/src/gui/socketapi/socketapi.h +++ b/src/gui/socketapi/socketapi.h @@ -40,6 +40,7 @@ class Folder; class SocketListener; class DirectEditor; class SocketApiJob; +class SocketApiJobV2; Q_DECLARE_LOGGING_CATEGORY(lcSocketApi) @@ -130,8 +131,8 @@ private: #endif // External sync - Q_INVOKABLE void command_V2_LIST_ACCOUNTS(const QString &argument, SocketListener *listener) const; - Q_INVOKABLE void command_V2_UPLOAD_FILES_FROM(const QString &argument, SocketListener *listener) const; + Q_INVOKABLE void command_V2_LIST_ACCOUNTS(const QSharedPointer &job) const; + Q_INVOKABLE void command_V2_UPLOAD_FILES_FROM(const QSharedPointer &job) const; // Fetch the private link and call targetFun void fetchPrivateLinkUrlHelper(const QString &localFile, const std::function &targetFun); @@ -168,7 +169,7 @@ private: QString buildRegisterPathMessage(const QString &path); QSet _registeredAliases; - QList _listeners; + QMap> _listeners; SocketApiServer _localServer; }; } diff --git a/src/gui/socketapi/socketapi_p.h b/src/gui/socketapi/socketapi_p.h index 38738c84b8..91724d6da1 100644 --- a/src/gui/socketapi/socketapi_p.h +++ b/src/gui/socketapi/socketapi_p.h @@ -62,13 +62,20 @@ class SocketListener public: QPointer socket; - explicit SocketListener(QIODevice *socket) - : socket(socket) + explicit SocketListener(QIODevice *_socket) + : socket(_socket) { } void sendMessage(const QString &message, bool doWait = false) const; - void sendMessage(const QString &function, const QJsonObject &obj, bool doWait = false) const; + void sendWarning(const QString &message, bool doWait = false) const + { + sendMessage(QStringLiteral("WARNING:") + message, doWait); + } + void sendError(const QString &message, bool doWait = false) const + { + sendMessage(QStringLiteral("ERROR:") + message, doWait); + } void sendMessageIfDirectoryMonitored(const QString &message, uint systemDirectoryHash) const { @@ -110,30 +117,48 @@ class SocketApiJob : public QObject { Q_OBJECT public: - SocketApiJob(const QString &jobId, SocketListener *socketListener, const QJsonObject &arguments) + explicit SocketApiJob(const QString &jobId, const QSharedPointer &socketListener, const QJsonObject &arguments) : _jobId(jobId) , _socketListener(socketListener) , _arguments(arguments) { } - void resolve(const QString &response = QString()) - { - _socketListener->sendMessage(QLatin1String("RESOLVE|") + _jobId + '|' + response); - } + void resolve(const QString &response = QString()); - void resolve(const QJsonObject &response) { resolve(QJsonDocument { response }.toJson()); } + void resolve(const QJsonObject &response); const QJsonObject &arguments() { return _arguments; } - void reject(const QString &response) - { - _socketListener->sendMessage(QLatin1String("REJECT|") + _jobId + '|' + response); - } + void reject(const QString &response); + +protected: + QString _jobId; + QSharedPointer _socketListener; + QJsonObject _arguments; +}; + +class SocketApiJobV2 : public QObject +{ + Q_OBJECT +public: + explicit SocketApiJobV2(const QSharedPointer &socketListener, const QByteArray &command, const QJsonObject &arguments); + + void success(const QJsonObject &response) const; + void failure(const QString &error) const; + + const QJsonObject &arguments() const { return _arguments; } + QByteArray command() const { return _command; } + +Q_SIGNALS: + void finished() const; private: + void doFinish(const QJsonObject &obj) const; + + QSharedPointer _socketListener; + const QByteArray _command; QString _jobId; - SocketListener *_socketListener; QJsonObject _arguments; }; } diff --git a/src/gui/socketapi/socketuploadjob.cpp b/src/gui/socketapi/socketuploadjob.cpp index 8ade844b02..467a46973e 100644 --- a/src/gui/socketapi/socketuploadjob.cpp +++ b/src/gui/socketapi/socketuploadjob.cpp @@ -25,24 +25,30 @@ using namespace OCC; -SocketUploadJob::SocketUploadJob(OCC::SocketListener *listener, const QString &argument, QObject *parent) - : QObject(parent) - , _listener(listener) +SocketUploadJob::SocketUploadJob(const QSharedPointer &job) + : _apiJob(job) { - const auto args = QJsonDocument::fromJson(argument.toUtf8()).object(); - _localPath = args[QLatin1String("localPath")].toString(); - _remotePath = args[QLatin1String("remotePath")].toString(); - if (!_remotePath.startsWith("/")) { + connect(job.data(), &SocketApiJobV2::finished, this, &SocketUploadJob::deleteLater); + + _localPath = _apiJob->arguments()[QLatin1String("localPath")].toString(); + _remotePath = _apiJob->arguments()[QLatin1String("remotePath")].toString(); + if (!_remotePath.startsWith(QLatin1Char('/'))) { _remotePath = QLatin1Char('/') + _remotePath; } - _pattern = args[QLatin1String("pattern")].toString(); + _pattern = job->arguments()[QLatin1String("pattern")].toString(); // TODO: use uuid - const auto accname = args[QLatin1String("account")][QLatin1String("name")].toString(); + const auto accname = job->arguments()[QLatin1String("account")][QLatin1String("name")].toString(); auto account = AccountManager::instance()->account(accname); - ENFORCE(QFileInfo(_localPath).isAbsolute()) - ENFORCE(_tmp.open()) + if (!QFileInfo(_localPath).isAbsolute()) { + job->failure(QStringLiteral("Local path must be a an absolute path")); + return; + } + if (!_tmp.open()) { + job->failure(QStringLiteral("Failed to create temporary database")); + return; + } _db = new SyncJournalDb(_tmp.fileName(), this); _engine = new SyncEngine(account->account(), _localPath.endsWith(QLatin1Char('/')) ? _localPath : _localPath + QLatin1Char('/'), _remotePath, _db); @@ -54,10 +60,12 @@ SocketUploadJob::SocketUploadJob(OCC::SocketListener *listener, const QString &a connect(_engine, &OCC::SyncEngine::finished, this, [this](bool ok) { if (ok) { - finish({}); + _apiJob->success({ { "localPath", _localPath }, { "syncedFiles", QJsonArray::fromStringList(_syncedFiles) } }); } }); - connect(_engine, &OCC::SyncEngine::syncError, this, &SocketUploadJob::finish); + connect(_engine, &OCC::SyncEngine::syncError, this, [this](const QString &error, ErrorCategory) { + _apiJob->failure(error); + }); } void SocketUploadJob::start() @@ -65,7 +73,7 @@ void SocketUploadJob::start() auto opt = _engine->syncOptions(); opt.setFilePattern(_pattern); if (!opt.fileRegex().isValid()) { - finish(opt.fileRegex().errorString()); + _apiJob->failure(opt.fileRegex().errorString()); return; } _engine->setSyncOptions(opt); @@ -75,19 +83,10 @@ void SocketUploadJob::start() connect(mkdir, &OCC::MkColJob::finishedWithoutError, _engine, &OCC::SyncEngine::startSync); connect(mkdir, &OCC::MkColJob::finishedWithError, this, [this](QNetworkReply *reply) { if (reply->error() == 202) { - finish(QStringLiteral("Destination %1 already exists").arg(_remotePath)); + _apiJob->failure(QStringLiteral("Destination %1 already exists").arg(_remotePath)); } else { - finish(reply->errorString()); + _apiJob->failure(reply->errorString()); } }); mkdir->start(); } - -void SocketUploadJob::finish(const QString &error) -{ - if (!_finished) { - _finished = true; - _listener->sendMessage(QStringLiteral("V2/UPLOAD_FILES_RESULT"), { { "localPath", _localPath }, { "error", error }, { "syncedFiles", QJsonArray::fromStringList(_syncedFiles) } }); - deleteLater(); - } -} diff --git a/src/gui/socketapi/socketuploadjob.h b/src/gui/socketapi/socketuploadjob.h index 477b8bf0cd..3b04061d38 100644 --- a/src/gui/socketapi/socketuploadjob.h +++ b/src/gui/socketapi/socketuploadjob.h @@ -28,14 +28,11 @@ class SocketUploadJob : public QObject { Q_OBJECT public: - SocketUploadJob(OCC::SocketListener *listener, const QString &argument, QObject *parent = nullptr); - + SocketUploadJob(const QSharedPointer &job); void start(); - void finish(const QString &error); - private: - SocketListener *_listener; + QSharedPointer _apiJob; QString _localPath; QString _remotePath; QString _pattern; @@ -43,7 +40,5 @@ private: SyncJournalDb *_db; SyncEngine *_engine; QStringList _syncedFiles; - - bool _finished = false; }; } From 0456cacd793be4eb0186ae13b9796ee8ebf0949b Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 16 Aug 2021 18:53:36 +0200 Subject: [PATCH 33/33] fix clang-tidy check for usage of =default Signed-off-by: Matthieu Gallien --- src/libsync/syncoptions.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libsync/syncoptions.cpp b/src/libsync/syncoptions.cpp index 588a7c6343..c6312d4b41 100644 --- a/src/libsync/syncoptions.cpp +++ b/src/libsync/syncoptions.cpp @@ -24,9 +24,7 @@ SyncOptions::SyncOptions() { } -SyncOptions::~SyncOptions() -{ -} +SyncOptions::~SyncOptions() = default; void SyncOptions::fillFromEnvironmentVariables() {