From 23ebed25b79f4b6edf630546d7d9d571398a1640 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 4 Nov 2018 19:55:20 +0900 Subject: Profiler logging fixes and improvements --- lib/jobs/syncjob.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/syncjob.cpp b/lib/jobs/syncjob.cpp index 9cbac71b..6baf388e 100644 --- a/lib/jobs/syncjob.cpp +++ b/lib/jobs/syncjob.cpp @@ -109,11 +109,12 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data) totalEvents += r.state.size() + r.ephemeral.size() + r.accountData.size() + r.timeline.size(); } - totalRooms += roomData.size(); + totalRooms += rs.size(); } - qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with" - << totalRooms << "room(s)," - << totalEvents << "event(s) in" << et; + if (totalRooms > 9 || et.nsecsElapsed() >= profilerMinNsecs()) + qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with" + << totalRooms << "room(s)," + << totalEvents << "event(s) in" << et; return BaseJob::Success; } -- cgit v1.2.3 From af0c8135afce32d9e06cc2446d9c675693d2c5fb Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 18 Nov 2018 14:18:45 +0900 Subject: BaseJob::rawDataSample() A new recommended (and localisable) way of getting a piece of raw response to display next to error messages as "details". BaseJob::rawData() returns exactly the trimmed piece of data, no "truncated" suffix there anymore. --- lib/connection.cpp | 13 ++++--------- lib/connection.h | 6 +++--- lib/jobs/basejob.cpp | 19 +++++++++++++++---- lib/jobs/basejob.h | 14 +++++++++++++- 4 files changed, 35 insertions(+), 17 deletions(-) (limited to 'lib/jobs') diff --git a/lib/connection.cpp b/lib/connection.cpp index df9fb112..6bda932a 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -65,10 +65,6 @@ HashT erase_if(HashT& hashMap, Pred pred) return removals; } -#ifndef TRIM_RAW_DATA -#define TRIM_RAW_DATA 65535 -#endif - class Connection::Private { public: @@ -228,8 +224,7 @@ void Connection::doConnectToServer(const QString& user, const QString& password, }); connect(loginJob, &BaseJob::failure, this, [this, loginJob] { - emit loginError(loginJob->errorString(), - loginJob->rawData(TRIM_RAW_DATA)); + emit loginError(loginJob->errorString(), loginJob->rawDataSample()); }); } @@ -306,7 +301,7 @@ void Connection::sync(int timeout) connect( job, &SyncJob::retryScheduled, this, [this,job] (int retriesTaken, int nextInMilliseconds) { - emit networkError(job->errorString(), job->rawData(TRIM_RAW_DATA), + emit networkError(job->errorString(), job->rawDataSample(), retriesTaken, nextInMilliseconds); }); connect( job, &SyncJob::failure, this, [this, job] { @@ -315,10 +310,10 @@ void Connection::sync(int timeout) { qCWarning(SYNCJOB) << "Sync job failed with ContentAccessError - login expired?"; - emit loginError(job->errorString(), job->rawData(TRIM_RAW_DATA)); + emit loginError(job->errorString(), job->rawDataSample()); } else - emit syncError(job->errorString(), job->rawData(TRIM_RAW_DATA)); + emit syncError(job->errorString(), job->rawDataSample()); }); } diff --git a/lib/connection.h b/lib/connection.h index b06fb143..20dade76 100644 --- a/lib/connection.h +++ b/lib/connection.h @@ -519,7 +519,7 @@ namespace QMatrixClient * a successful login and logout and are constant at other times. */ void stateChanged(); - void loginError(QString message, QByteArray details); + void loginError(QString message, QString details); /** A network request (job) failed * @@ -537,11 +537,11 @@ namespace QMatrixClient * @param retriesTaken - how many retries have already been taken * @param nextRetryInMilliseconds - when the job will retry again */ - void networkError(QString message, QByteArray details, + void networkError(QString message, QString details, int retriesTaken, int nextRetryInMilliseconds); void syncDone(); - void syncError(QString message, QByteArray details); + void syncError(QString message, QString details); void newUser(User* user); diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index b21173ae..4a7780b1 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -426,8 +426,8 @@ BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) const auto& json = QJsonDocument::fromJson(d->rawResponse, &error); if( error.error == QJsonParseError::NoError ) return parseJson(json); - else - return { IncorrectResponseError, error.errorString() }; + + return { IncorrectResponseError, error.errorString() }; } BaseJob::Status BaseJob::parseJson(const QJsonDocument&) @@ -519,8 +519,19 @@ BaseJob::Status BaseJob::status() const QByteArray BaseJob::rawData(int bytesAtMost) const { - return bytesAtMost > 0 && d->rawResponse.size() > bytesAtMost ? - d->rawResponse.left(bytesAtMost) + "...(truncated)" : d->rawResponse; + return bytesAtMost > 0 && d->rawResponse.size() > bytesAtMost + ? d->rawResponse.left(bytesAtMost) : d->rawResponse; +} + +QString BaseJob::rawDataSample(int bytesAtMost) const +{ + auto data = rawData(bytesAtMost); + Q_ASSERT(data.size() <= d->rawResponse.size()); + return data.size() == d->rawResponse.size() + ? data : data + tr("...(truncated, %Ln bytes in total)", + "Comes after trimmed raw network response", + d->rawResponse.size()); + } QString BaseJob::statusCaption() const diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 4ef25ab8..3d50344d 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -138,8 +138,20 @@ namespace QMatrixClient Status status() const; /** Short human-friendly message on the job status */ QString statusCaption() const; - /** Raw response body as received from the server */ + /** Get raw response body as received from the server + * \param bytesAtMost return this number of leftmost bytes, or -1 + * to return the entire response + */ QByteArray rawData(int bytesAtMost = -1) const; + /** Get UI-friendly sample of raw data + * + * This is almost the same as rawData but appends the "truncated" + * suffix if not all data fit in bytesAtMost. This call is + * recommended to present a sample of raw data as "details" next to + * error messages. Note that the default \p bytesAtMost value is + * also tailored to UI cases. + */ + QString rawDataSample(int bytesAtMost = 65535) const; /** Error (more generally, status) code * Equivalent to status().code -- cgit v1.2.3 From dc3d6bd3b46ae7a9e8d9b9f62e50db982ef2b004 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 20 Nov 2018 13:24:40 +0900 Subject: Make SyncData more self-contained and prepare for cache splitting SyncData now resides in its own pair of files and is capable to load either from file or from JSON. There is also (yet untested) capability to load rooms from files if a file name stands is the value for a given room id. This allows to store the master cache file separately from cache files for each room, massively easing the problem of bulky accounts that can overflow the poor capacity of Qt's JSON engine. --- CMakeLists.txt | 1 + lib/connection.cpp | 50 ++++----------- lib/jobs/syncjob.cpp | 110 ++------------------------------- lib/jobs/syncjob.h | 48 +-------------- lib/room.cpp | 2 +- lib/room.h | 2 +- lib/syncdata.cpp | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++ lib/syncdata.h | 85 +++++++++++++++++++++++++ 8 files changed, 278 insertions(+), 191 deletions(-) create mode 100644 lib/syncdata.cpp create mode 100644 lib/syncdata.h (limited to 'lib/jobs') diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e3eb600..49c5d8b2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -76,6 +76,7 @@ set(libqmatrixclient_SRCS lib/room.cpp lib/user.cpp lib/avatar.cpp + lib/syncdata.cpp lib/settings.cpp lib/networksettings.cpp lib/converters.cpp diff --git a/lib/connection.cpp b/lib/connection.cpp index 6bda932a..8a451a79 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -1059,9 +1058,6 @@ void Connection::setHomeserver(const QUrl& url) emit homeserverChanged(homeserver()); } -static constexpr int CACHE_VERSION_MAJOR = 8; -static constexpr int CACHE_VERSION_MINOR = 0; - void Connection::saveState(const QUrl &toFile) const { if (!d->cacheState) @@ -1091,6 +1087,8 @@ void Connection::saveState(const QUrl &toFile) const QJsonObject inviteRooms; for (const auto* i : roomMap()) // Pass on rooms in Leave state { + // TODO: instead of adding the room JSON add a file name and save + // the JSON to that file. if (i->joinState() == JoinState::Invite) inviteRooms.insert(i->id(), i->toJson()); else @@ -1123,8 +1121,8 @@ void Connection::saveState(const QUrl &toFile) const } QJsonObject versionObj; - versionObj.insert("major", CACHE_VERSION_MAJOR); - versionObj.insert("minor", CACHE_VERSION_MINOR); + versionObj.insert("major", SyncData::cacheVersion().first); + versionObj.insert("minor", SyncData::cacheVersion().second); rootObj.insert("cache_version", versionObj); QJsonDocument json { rootObj }; @@ -1142,42 +1140,20 @@ void Connection::loadState(const QUrl &fromFile) return; QElapsedTimer et; et.start(); - QFile file { - fromFile.isEmpty() ? stateCachePath() : fromFile.toLocalFile() - }; - if (!file.exists()) - { - qCDebug(MAIN) << "No state cache file found"; - return; - } - if(!file.open(QFile::ReadOnly)) - { - qCWarning(MAIN) << "file " << file.fileName() << "failed to open for read"; - return; - } - QByteArray data = file.readAll(); - auto jsonDoc = d->cacheToBinary ? QJsonDocument::fromBinaryData(data) : - QJsonDocument::fromJson(data); - if (jsonDoc.isNull()) - { - qCWarning(MAIN) << "Cache file broken, discarding"; + SyncData sync { + fromFile.isEmpty() ? stateCachePath() : fromFile.toLocalFile() }; + if (sync.nextBatch().isEmpty()) // No token means no cache by definition return; - } - auto actualCacheVersionMajor = - jsonDoc.object() - .value("cache_version").toObject() - .value("major").toInt(); - if (actualCacheVersionMajor < CACHE_VERSION_MAJOR) + + if (!sync.unresolvedRooms().isEmpty()) { - qCWarning(MAIN) - << "Major version of the cache file is" << actualCacheVersionMajor - << "but" << CACHE_VERSION_MAJOR << "required; discarding the cache"; + qCWarning(MAIN) << "State cache incomplete, discarding"; return; } - - SyncData sync; - sync.parseJson(jsonDoc); + // TODO: to handle load failures, instead of the above block: + // 1. Do initial sync on failed rooms without saving the nextBatch token + // 2. Do the sync across all rooms as normal onSyncSuccess(std::move(sync)); qCDebug(PROFILER) << "*** Cached state for" << userId() << "loaded in" << et; } diff --git a/lib/jobs/syncjob.cpp b/lib/jobs/syncjob.cpp index 6baf388e..ef9b45dd 100644 --- a/lib/jobs/syncjob.cpp +++ b/lib/jobs/syncjob.cpp @@ -18,10 +18,6 @@ #include "syncjob.h" -#include "events/eventloader.h" - -#include - using namespace QMatrixClient; static size_t jobId = 0; @@ -46,111 +42,15 @@ SyncJob::SyncJob(const QString& since, const QString& filter, int timeout, setMaxRetries(std::numeric_limits::max()); } -QString SyncData::nextBatch() const -{ - return nextBatch_; -} - -SyncDataList&& SyncData::takeRoomData() -{ - return std::move(roomData); -} - -Events&& SyncData::takePresenceData() -{ - return std::move(presenceData); -} - -Events&& SyncData::takeAccountData() -{ - return std::move(accountData); -} - -Events&&SyncData::takeToDeviceEvents() -{ - return std::move(toDeviceEvents); -} - -template -inline EventsArrayT load(const QJsonObject& batches, StrT keyName) -{ - return fromJson(batches[keyName].toObject().value("events"_ls)); -} - BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { - return d.parseJson(data); -} - -BaseJob::Status SyncData::parseJson(const QJsonDocument &data) -{ - QElapsedTimer et; et.start(); - - auto json = data.object(); - nextBatch_ = json.value("next_batch"_ls).toString(); - presenceData = load(json, "presence"_ls); - accountData = load(json, "account_data"_ls); - toDeviceEvents = load(json, "to_device"_ls); - - auto rooms = json.value("rooms"_ls).toObject(); - JoinStates::Int ii = 1; // ii is used to make a JoinState value - auto totalRooms = 0; - auto totalEvents = 0; - for (size_t i = 0; i < JoinStateStrings.size(); ++i, ii <<= 1) + d.parseJson(data.object()); + if (d.unresolvedRooms().isEmpty()) { - const auto rs = rooms.value(JoinStateStrings[i]).toObject(); - // We have a Qt container on the right and an STL one on the left - roomData.reserve(static_cast(rs.size())); - for(auto roomIt = rs.begin(); roomIt != rs.end(); ++roomIt) - { - roomData.emplace_back(roomIt.key(), JoinState(ii), - roomIt.value().toObject()); - const auto& r = roomData.back(); - totalEvents += r.state.size() + r.ephemeral.size() + - r.accountData.size() + r.timeline.size(); - } - totalRooms += rs.size(); + qCCritical(MAIN) << "Incomplete sync response, missing rooms:" + << d.unresolvedRooms().join(','); + return BaseJob::IncorrectResponseError; } - if (totalRooms > 9 || et.nsecsElapsed() >= profilerMinNsecs()) - qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with" - << totalRooms << "room(s)," - << totalEvents << "event(s) in" << et; return BaseJob::Success; } -const QString SyncRoomData::UnreadCountKey = - QStringLiteral("x-qmatrixclient.unread_count"); - -SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, - const QJsonObject& room_) - : roomId(roomId_) - , joinState(joinState_) - , state(load(room_, - joinState == JoinState::Invite ? "invite_state"_ls : "state"_ls)) -{ - switch (joinState) { - case JoinState::Join: - ephemeral = load(room_, "ephemeral"_ls); - FALLTHROUGH; - case JoinState::Leave: - { - accountData = load(room_, "account_data"_ls); - timeline = load(room_, "timeline"_ls); - const auto timelineJson = room_.value("timeline"_ls).toObject(); - timelineLimited = timelineJson.value("limited"_ls).toBool(); - timelinePrevBatch = timelineJson.value("prev_batch"_ls).toString(); - - break; - } - default: /* nothing on top of state */; - } - - const auto unreadJson = room_.value("unread_notifications"_ls).toObject(); - unreadCount = unreadJson.value(UnreadCountKey).toInt(-2); - highlightCount = unreadJson.value("highlight_count"_ls).toInt(); - notificationCount = unreadJson.value("notification_count"_ls).toInt(); - if (highlightCount > 0 || notificationCount > 0) - qCDebug(SYNCJOB) << "Room" << roomId_ - << "has highlights:" << highlightCount - << "and notifications:" << notificationCount; -} diff --git a/lib/jobs/syncjob.h b/lib/jobs/syncjob.h index 6b9bedfa..a0a3c026 100644 --- a/lib/jobs/syncjob.h +++ b/lib/jobs/syncjob.h @@ -20,56 +20,10 @@ #include "basejob.h" -#include "joinstate.h" -#include "events/stateevent.h" -#include "util.h" +#include "../syncdata.h" namespace QMatrixClient { - class SyncRoomData - { - public: - QString roomId; - JoinState joinState; - StateEvents state; - RoomEvents timeline; - Events ephemeral; - Events accountData; - - bool timelineLimited; - QString timelinePrevBatch; - int unreadCount; - int highlightCount; - int notificationCount; - - SyncRoomData(const QString& roomId, JoinState joinState_, - const QJsonObject& room_); - SyncRoomData(SyncRoomData&&) = default; - SyncRoomData& operator=(SyncRoomData&&) = default; - - static const QString UnreadCountKey; - }; - // QVector cannot work with non-copiable objects, std::vector can. - using SyncDataList = std::vector; - - class SyncData - { - public: - BaseJob::Status parseJson(const QJsonDocument &data); - Events&& takePresenceData(); - Events&& takeAccountData(); - Events&& takeToDeviceEvents(); - SyncDataList&& takeRoomData(); - QString nextBatch() const; - - private: - QString nextBatch_; - Events presenceData; - Events accountData; - Events toDeviceEvents; - SyncDataList roomData; - }; - class SyncJob: public BaseJob { public: diff --git a/lib/room.cpp b/lib/room.cpp index 656788cb..e5653258 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -45,10 +45,10 @@ #include "connection.h" #include "user.h" #include "converters.h" +#include "syncdata.h" #include #include // for efficient string concats (operator%) -#include #include #include #include diff --git a/lib/room.h b/lib/room.h index 633d19dd..b741e229 100644 --- a/lib/room.h +++ b/lib/room.h @@ -18,7 +18,6 @@ #pragma once -#include "jobs/syncjob.h" #include "csapi/message_pagination.h" #include "events/roommessageevent.h" #include "events/accountdataevents.h" @@ -34,6 +33,7 @@ namespace QMatrixClient { class Event; + class SyncRoomData; class RoomMemberEvent; class Connection; class User; diff --git a/lib/syncdata.cpp b/lib/syncdata.cpp new file mode 100644 index 00000000..f0d55fd6 --- /dev/null +++ b/lib/syncdata.cpp @@ -0,0 +1,171 @@ +/****************************************************************************** + * Copyright (C) 2018 Kitsune Ral + * + * 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 "syncdata.h" + +#include "events/eventloader.h" + +#include + +using namespace QMatrixClient; + +const QString SyncRoomData::UnreadCountKey = + QStringLiteral("x-qmatrixclient.unread_count"); + +template +inline EventsArrayT load(const QJsonObject& batches, StrT keyName) +{ + return fromJson(batches[keyName].toObject().value("events"_ls)); +} + +SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, + const QJsonObject& room_) + : roomId(roomId_) + , joinState(joinState_) + , state(load(room_, joinState == JoinState::Invite + ? "invite_state"_ls : "state"_ls)) +{ + switch (joinState) { + case JoinState::Join: + ephemeral = load(room_, "ephemeral"_ls); + FALLTHROUGH; + case JoinState::Leave: + { + accountData = load(room_, "account_data"_ls); + timeline = load(room_, "timeline"_ls); + const auto timelineJson = room_.value("timeline"_ls).toObject(); + timelineLimited = timelineJson.value("limited"_ls).toBool(); + timelinePrevBatch = timelineJson.value("prev_batch"_ls).toString(); + + break; + } + default: /* nothing on top of state */; + } + + const auto unreadJson = room_.value("unread_notifications"_ls).toObject(); + unreadCount = unreadJson.value(UnreadCountKey).toInt(-2); + highlightCount = unreadJson.value("highlight_count"_ls).toInt(); + notificationCount = unreadJson.value("notification_count"_ls).toInt(); + if (highlightCount > 0 || notificationCount > 0) + qCDebug(SYNCJOB) << "Room" << roomId_ + << "has highlights:" << highlightCount + << "and notifications:" << notificationCount; +} + +SyncData::SyncData(const QString& cacheFileName) +{ + parseJson(loadJson(cacheFileName)); +} + +SyncDataList&& SyncData::takeRoomData() +{ + return move(roomData); +} + +Events&& SyncData::takePresenceData() +{ + return std::move(presenceData); +} + +Events&& SyncData::takeAccountData() +{ + return std::move(accountData); +} + +Events&& SyncData::takeToDeviceEvents() +{ + return std::move(toDeviceEvents); +} + +QJsonObject SyncData::loadJson(const QString& fileName) +{ + QFile roomFile { fileName }; + if (!roomFile.exists()) + { + qCWarning(MAIN) << "No state cache file" << fileName; + return {}; + } + if(!roomFile.open(QIODevice::ReadOnly)) + { + qCWarning(MAIN) << "Failed to open state cache file" + << roomFile.fileName(); + return {}; + } + auto data = roomFile.readAll(); + + const auto json = + (data.startsWith('{') ? QJsonDocument::fromJson(data) + : QJsonDocument::fromBinaryData(data)).object(); + if (json.isEmpty()) + { + qCWarning(MAIN) << "State cache in" << fileName + << "is broken or empty, discarding"; + return {}; + } + auto requiredVersion = std::get<0>(cacheVersion()); + auto actualVersion = json.value("cache_version").toObject() + .value("major").toInt(); + if (actualVersion < requiredVersion) + { + qCWarning(MAIN) + << "Major version of the cache file is" << actualVersion << "but" + << requiredVersion << "is required; discarding the cache"; + return {}; + } + return json; +} + +void SyncData::parseJson(const QJsonObject& json) +{ + QElapsedTimer et; et.start(); + + nextBatch_ = json.value("next_batch"_ls).toString(); + presenceData = load(json, "presence"_ls); + accountData = load(json, "account_data"_ls); + toDeviceEvents = load(json, "to_device"_ls); + + auto rooms = json.value("rooms"_ls).toObject(); + JoinStates::Int ii = 1; // ii is used to make a JoinState value + auto totalRooms = 0; + auto totalEvents = 0; + for (size_t i = 0; i < JoinStateStrings.size(); ++i, ii <<= 1) + { + const auto rs = rooms.value(JoinStateStrings[i]).toObject(); + // We have a Qt container on the right and an STL one on the left + roomData.reserve(static_cast(rs.size())); + for(auto roomIt = rs.begin(); roomIt != rs.end(); ++roomIt) + { + auto roomJson = roomIt->isString() ? loadJson(roomIt->toString()) + : roomIt->toObject(); + if (roomJson.isEmpty()) + { + unresolvedRoomIds.push_back(roomIt.key()); + continue; + } + roomData.emplace_back(roomIt.key(), JoinState(ii), roomJson); + const auto& r = roomData.back(); + totalEvents += r.state.size() + r.ephemeral.size() + + r.accountData.size() + r.timeline.size(); + } + totalRooms += rs.size(); + } + if (totalRooms > 9 || et.nsecsElapsed() >= profilerMinNsecs()) + qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with" + << totalRooms << "room(s)," + << totalEvents << "event(s) in" << et; +} diff --git a/lib/syncdata.h b/lib/syncdata.h new file mode 100644 index 00000000..d8007db9 --- /dev/null +++ b/lib/syncdata.h @@ -0,0 +1,85 @@ +/****************************************************************************** + * Copyright (C) 2018 Kitsune Ral + * + * 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 "joinstate.h" +#include "events/stateevent.h" + +namespace QMatrixClient { + class SyncRoomData + { + public: + QString roomId; + JoinState joinState; + StateEvents state; + RoomEvents timeline; + Events ephemeral; + Events accountData; + + bool timelineLimited; + QString timelinePrevBatch; + int unreadCount; + int highlightCount; + int notificationCount; + + SyncRoomData(const QString& roomId, JoinState joinState_, + const QJsonObject& room_); + SyncRoomData(SyncRoomData&&) = default; + SyncRoomData& operator=(SyncRoomData&&) = default; + + static const QString UnreadCountKey; + }; + + // QVector cannot work with non-copiable objects, std::vector can. + using SyncDataList = std::vector; + + class SyncData + { + public: + SyncData() = default; + explicit SyncData(const QString& cacheFileName); + /** Parse sync response into room events + * \param json response from /sync or a room state cache + * \return the list of rooms with missing cache files; always + * empty when parsing response from /sync + */ + void parseJson(const QJsonObject& json); + + Events&& takePresenceData(); + Events&& takeAccountData(); + Events&& takeToDeviceEvents(); + SyncDataList&& takeRoomData(); + + QString nextBatch() const { return nextBatch_; } + + QStringList unresolvedRooms() const { return unresolvedRoomIds; } + + static std::pair cacheVersion() { return { 8, 0 }; } + + private: + QString nextBatch_; + Events presenceData; + Events accountData; + Events toDeviceEvents; + SyncDataList roomData; + QStringList unresolvedRoomIds; + + static QJsonObject loadJson(const QString& fileName); + }; +} // namespace QMatrixClient -- cgit v1.2.3 From 92a504cd7255fd23c2ce4b8ffb4880e80dc1d839 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 20 Nov 2018 15:30:59 +0900 Subject: SyncJob::parseJson: fix a validation mistake --- lib/jobs/syncjob.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/syncjob.cpp b/lib/jobs/syncjob.cpp index ef9b45dd..ac0f6685 100644 --- a/lib/jobs/syncjob.cpp +++ b/lib/jobs/syncjob.cpp @@ -46,11 +46,10 @@ BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { d.parseJson(data.object()); if (d.unresolvedRooms().isEmpty()) - { - qCCritical(MAIN) << "Incomplete sync response, missing rooms:" - << d.unresolvedRooms().join(','); - return BaseJob::IncorrectResponseError; - } - return BaseJob::Success; + return BaseJob::Success; + + qCCritical(MAIN).noquote() << "Incomplete sync response, missing rooms:" + << d.unresolvedRooms().join(','); + return BaseJob::IncorrectResponseError; } -- cgit v1.2.3 From a0053484024a85ae47dcd2b464cb15c0f85109e5 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Nov 2018 19:20:45 +0900 Subject: SyncJob: accept Filter instead of QString for the filter --- lib/jobs/syncjob.cpp | 7 +++++++ lib/jobs/syncjob.h | 3 +++ 2 files changed, 10 insertions(+) (limited to 'lib/jobs') diff --git a/lib/jobs/syncjob.cpp b/lib/jobs/syncjob.cpp index ac0f6685..84385b55 100644 --- a/lib/jobs/syncjob.cpp +++ b/lib/jobs/syncjob.cpp @@ -42,6 +42,13 @@ SyncJob::SyncJob(const QString& since, const QString& filter, int timeout, setMaxRetries(std::numeric_limits::max()); } +SyncJob::SyncJob(const QString& since, const Filter& filter, + int timeout, const QString& presence) + : SyncJob(since, + QJsonDocument(toJson(filter)).toJson(QJsonDocument::Compact), + timeout, presence) +{ } + BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { d.parseJson(data.object()); diff --git a/lib/jobs/syncjob.h b/lib/jobs/syncjob.h index a0a3c026..036b25d0 100644 --- a/lib/jobs/syncjob.h +++ b/lib/jobs/syncjob.h @@ -21,6 +21,7 @@ #include "basejob.h" #include "../syncdata.h" +#include "../csapi/definitions/sync_filter.h" namespace QMatrixClient { @@ -30,6 +31,8 @@ namespace QMatrixClient explicit SyncJob(const QString& since = {}, const QString& filter = {}, int timeout = -1, const QString& presence = {}); + explicit SyncJob(const QString& since, const Filter& filter, + int timeout = -1, const QString& presence = {}); SyncData &&takeData() { return std::move(d); } -- cgit v1.2.3 From 40979a2c81f28ccd165982bd137c25a94f34c4cc Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 24 Dec 2018 13:50:02 +0900 Subject: Clarify doc-comment for BaseJob::finished a bit [skip ci] --- lib/jobs/basejob.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 3d50344d..dd6f9fc8 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -215,9 +215,9 @@ namespace QMatrixClient * * In general, to be notified of a job's completion, client code * should connect to result(), success(), or failure() - * rather than finished(). However if you store a list of jobs - * and need to track their lifecycle, then you should connect to this - * instead of result(), to avoid dangling pointers in your list. + * rather than finished(). However if you need to track the job's + * lifecycle you should connect to this instead of result(); + * in particular, only this signal will be emitted on abandoning. * * @param job the job that emitted this signal * -- cgit v1.2.3 From 20e4f76280a8ac36ca1cdfe7d0d7bdeb2eef444d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 20 Feb 2019 17:48:39 +0900 Subject: BaseJob: M_UNSUPPORTED_ROOM_VERSION & M_INCOMPATIBLE_ROOM_VERSION --- lib/jobs/basejob.cpp | 12 +++++++++++- lib/jobs/basejob.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 4a7780b1..628d10ec 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -325,7 +325,15 @@ void BaseJob::gotReply() d->status.code = UserConsentRequiredError; d->errorUrl = json.value("consent_uri"_ls).toString(); } - else if (!json.isEmpty()) // Not localisable on the client side + else if (errCode == "M_UNSUPPORTED_ROOM_VERSION" || + errCode == "M_INCOMPATIBLE_ROOM_VERSION") + { + d->status.code = UnsupportedRoomVersionError; + if (json.contains("room_version")) + d->status.message = + tr("Requested room version: %1") + .arg(json.value("room_version").toString()); + } else if (!json.isEmpty()) // Not localisable on the client side setStatus(IncorrectRequestError, json.value("error"_ls).toString()); } @@ -568,6 +576,8 @@ QString BaseJob::statusCaption() const return tr("Network authentication required"); case UserConsentRequiredError: return tr("User consent required"); + case UnsupportedRoomVersionError: + return tr("The server does not support the needed room version"); default: return tr("Request failed"); } diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index dd6f9fc8..4c1c7706 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -64,6 +64,7 @@ namespace QMatrixClient , IncorrectResponseError , TooManyRequestsError , RequestNotImplementedError + , UnsupportedRoomVersionError , NetworkAuthRequiredError , UserConsentRequiredError , UserDefinedError = 200 -- cgit v1.2.3 From 39d44789d0f378b29d2c15994e8fa630edcdb408 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 27 Feb 2019 11:21:08 +0900 Subject: BaseJob::abandon() fixes 1. It should work with non-started jobs now (Closes #289). 2. It should allow clients to still handle `finished()` instead of cutting them off right before emitting the signal. --- lib/jobs/basejob.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 628d10ec..8c3381ae 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -603,7 +603,7 @@ void BaseJob::setStatus(Status s) if (d->status == s) return; - if (!d->connection->accessToken().isEmpty()) + if (d->connection && !d->connection->accessToken().isEmpty()) s.message.replace(d->connection->accessToken(), "(REDACTED)"); if (!s.good()) qCWarning(d->logCat) << this << "status" << s; @@ -618,9 +618,8 @@ void BaseJob::setStatus(int code, QString message) void BaseJob::abandon() { - beforeAbandon(d->reply.data()); + beforeAbandon(d->reply ? d->reply.data() : nullptr); setStatus(Abandoned); - this->disconnect(); if (d->reply) d->reply->disconnect(this); emit finished(this); -- cgit v1.2.3 From f13d54bd9931a340af862cc0a03af2ac68fe5e06 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 14 Mar 2019 07:42:24 +0900 Subject: Fix read receipts and redactions on v3 rooms Previously slashes in eventIds (that come plenty in v3 due to base64 encoding) were not properly encoded - they are now. --- lib/jobs/basejob.cpp | 2 +- lib/room.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 8c3381ae..f738ce7a 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -186,7 +186,7 @@ QUrl BaseJob::makeRequestUrl(QUrl baseUrl, if (!pathBase.endsWith('/') && !path.startsWith('/')) pathBase.push_back('/'); - baseUrl.setPath( pathBase + path ); + baseUrl.setPath(pathBase + path, QUrl::TolerantMode); baseUrl.setQuery(query); return baseUrl; } diff --git a/lib/room.cpp b/lib/room.cpp index f2e03e94..dbddad05 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -579,8 +579,8 @@ Room::Changes Room::Private::markMessagesAsRead(rev_iter_t upToMarker) { if ((*upToMarker)->senderId() != q->localUser()->id()) { - connection->callApi(id, "m.read", - (*upToMarker)->id()); + connection->callApi(id, QStringLiteral("m.read"), + QUrl::toPercentEncoding((*upToMarker)->id())); break; } } @@ -1734,8 +1734,8 @@ void Room::unban(const QString& userId) void Room::redactEvent(const QString& eventId, const QString& reason) { - connection()->callApi( - id(), eventId, connection()->generateTxnId(), reason); + connection()->callApi(id(), + QUrl::toPercentEncoding(eventId), connection()->generateTxnId(), reason); } void Room::uploadFile(const QString& id, const QUrl& localFilename, -- cgit v1.2.3 From 1c5696e8f5a9ef87a065e2496eecf178e41e75a7 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 4 Apr 2019 21:27:38 +0900 Subject: Clean up on clang-tidy/clazy analysis --- lib/avatar.cpp | 2 +- lib/connection.cpp | 25 +++++++++++++++---------- lib/connectiondata.cpp | 6 +++--- lib/events/event.cpp | 3 ++- lib/events/stateevent.cpp | 2 +- lib/jobs/basejob.cpp | 2 +- lib/jobs/downloadfilejob.cpp | 5 +++-- lib/jobs/mediathumbnailjob.cpp | 2 +- lib/networkaccessmanager.cpp | 3 ++- lib/networksettings.cpp | 2 +- lib/room.cpp | 32 ++++++++++++++++---------------- lib/settings.cpp | 21 ++++++++++++--------- lib/settings.h | 2 +- lib/syncdata.cpp | 8 ++++---- lib/user.cpp | 14 ++++++++------ lib/util.cpp | 2 +- 16 files changed, 72 insertions(+), 59 deletions(-) (limited to 'lib/jobs') diff --git a/lib/avatar.cpp b/lib/avatar.cpp index c0ef3cba..9279ef9d 100644 --- a/lib/avatar.cpp +++ b/lib/avatar.cpp @@ -191,7 +191,7 @@ bool Avatar::Private::checkUrl(const QUrl& url) const } QString Avatar::Private::localFile() const { - static const auto cachePath = cacheLocation("avatars"); + static const auto cachePath = cacheLocation(QStringLiteral("avatars")); return cachePath % _url.authority() % '_' % _url.fileName() % ".png"; } diff --git a/lib/connection.cpp b/lib/connection.cpp index c09de979..5ed72616 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -717,8 +717,8 @@ void Connection::doInDirectChat(User* u, CreateRoomJob* Connection::createDirectChat(const QString& userId, const QString& topic, const QString& name) { - return createRoom(UnpublishRoom, "", name, topic, {userId}, - "trusted_private_chat", {}, true); + return createRoom(UnpublishRoom, {}, name, topic, {userId}, + QStringLiteral("trusted_private_chat"), {}, true); } ForgetRoomJob* Connection::forgetRoom(const QString& id) @@ -964,7 +964,8 @@ QHash> Connection::tagsToRooms() const QHash> result; for (auto* r: qAsConst(d->roomMap)) { - for (const auto& tagName: r->tagNames()) + const auto& tagNames = r->tagNames(); + for (const auto& tagName: tagNames) result[tagName].push_back(r); } for (auto it = result.begin(); it != result.end(); ++it) @@ -979,9 +980,12 @@ QStringList Connection::tagNames() const { QStringList tags ({FavouriteTag}); for (auto* r: qAsConst(d->roomMap)) - for (const auto& tag: r->tagNames()) + { + const auto& tagNames = r->tagNames(); + for (const auto& tag: tagNames) if (tag != LowPriorityTag && !tags.contains(tag)) tags.push_back(tag); + } tags.push_back(LowPriorityTag); return tags; } @@ -1264,18 +1268,19 @@ void Connection::saveState() const { QJsonObject rooms; QJsonObject inviteRooms; - for (const auto* i : roomMap()) // Pass on rooms in Leave state + const auto& rs = roomMap(); // Pass on rooms in Leave state + for (const auto* i : rs) (i->joinState() == JoinState::Invite ? inviteRooms : rooms) .insert(i->id(), QJsonValue::Null); QJsonObject roomObj; if (!rooms.isEmpty()) - roomObj.insert("join", rooms); + roomObj.insert(QStringLiteral("join"), rooms); if (!inviteRooms.isEmpty()) - roomObj.insert("invite", inviteRooms); + roomObj.insert(QStringLiteral("invite"), inviteRooms); - rootObj.insert("next_batch", d->data->lastEvent()); - rootObj.insert("rooms", roomObj); + rootObj.insert(QStringLiteral("next_batch"), d->data->lastEvent()); + rootObj.insert(QStringLiteral("rooms"), roomObj); } { QJsonArray accountDataEvents { @@ -1285,7 +1290,7 @@ void Connection::saveState() const accountDataEvents.append( basicEventJson(e.first, e.second->contentJson())); - rootObj.insert("account_data", + rootObj.insert(QStringLiteral("account_data"), QJsonObject {{ QStringLiteral("events"), accountDataEvents }}); } diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp index eb516ef7..91cda09f 100644 --- a/lib/connectiondata.cpp +++ b/lib/connectiondata.cpp @@ -25,7 +25,7 @@ using namespace QMatrixClient; struct ConnectionData::Private { - explicit Private(const QUrl& url) : baseUrl(url) { } + explicit Private(QUrl url) : baseUrl(std::move(url)) { } QUrl baseUrl; QByteArray accessToken; @@ -37,7 +37,7 @@ struct ConnectionData::Private }; ConnectionData::ConnectionData(QUrl baseUrl) - : d(std::make_unique(baseUrl)) + : d(std::make_unique(std::move(baseUrl))) { } ConnectionData::~ConnectionData() = default; @@ -98,7 +98,7 @@ QString ConnectionData::lastEvent() const void ConnectionData::setLastEvent(QString identifier) { - d->lastEvent = identifier; + d->lastEvent = std::move(identifier); } QByteArray ConnectionData::generateTxnId() const diff --git a/lib/events/event.cpp b/lib/events/event.cpp index c98dfbb6..6505d89a 100644 --- a/lib/events/event.cpp +++ b/lib/events/event.cpp @@ -38,7 +38,8 @@ event_type_t EventTypeRegistry::initializeTypeId(event_mtype_t matrixTypeId) QString EventTypeRegistry::getMatrixType(event_type_t typeId) { - return typeId < get().eventTypes.size() ? get().eventTypes[typeId] : ""; + return typeId < get().eventTypes.size() + ? get().eventTypes[typeId] : QString(); } Event::Event(Type type, const QJsonObject& json) diff --git a/lib/events/stateevent.cpp b/lib/events/stateevent.cpp index e96614d2..a84f302b 100644 --- a/lib/events/stateevent.cpp +++ b/lib/events/stateevent.cpp @@ -27,7 +27,7 @@ using namespace QMatrixClient; RoomEvent::factory_t::addMethod( [] (const QJsonObject& json, const QString& matrixType) -> StateEventPtr { - if (!json.contains("state_key")) + if (!json.contains("state_key"_ls)) return nullptr; if (auto e = StateEventBase::factory_t::make(json, matrixType)) diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index f738ce7a..f521cc4b 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -430,7 +430,7 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) { d->rawResponse = reply->readAll(); - QJsonParseError error; + QJsonParseError error { 0, QJsonParseError::MissingObject }; const auto& json = QJsonDocument::fromJson(d->rawResponse, &error); if( error.error == QJsonParseError::NoError ) return parseJson(json); diff --git a/lib/jobs/downloadfilejob.cpp b/lib/jobs/downloadfilejob.cpp index 2bf9dd8f..672a7b2d 100644 --- a/lib/jobs/downloadfilejob.cpp +++ b/lib/jobs/downloadfilejob.cpp @@ -22,7 +22,8 @@ class DownloadFileJob::Private QUrl DownloadFileJob::makeRequestUrl(QUrl baseUrl, const QUrl& mxcUri) { - return makeRequestUrl(baseUrl, mxcUri.authority(), mxcUri.path().mid(1)); + return makeRequestUrl( + std::move(baseUrl), mxcUri.authority(), mxcUri.path().mid(1)); } DownloadFileJob::DownloadFileJob(const QString& serverName, @@ -31,7 +32,7 @@ DownloadFileJob::DownloadFileJob(const QString& serverName, : GetContentJob(serverName, mediaId) , d(localFilename.isEmpty() ? new Private : new Private(localFilename)) { - setObjectName("DownloadFileJob"); + setObjectName(QStringLiteral("DownloadFileJob")); } QString DownloadFileJob::targetFileName() const diff --git a/lib/jobs/mediathumbnailjob.cpp b/lib/jobs/mediathumbnailjob.cpp index aeb49839..edb9b156 100644 --- a/lib/jobs/mediathumbnailjob.cpp +++ b/lib/jobs/mediathumbnailjob.cpp @@ -59,5 +59,5 @@ BaseJob::Status MediaThumbnailJob::parseReply(QNetworkReply* reply) if( _thumbnail.loadFromData(data()->readAll()) ) return Success; - return { IncorrectResponseError, "Could not read image data" }; + return { IncorrectResponseError, QStringLiteral("Could not read image data") }; } diff --git a/lib/networkaccessmanager.cpp b/lib/networkaccessmanager.cpp index 89967a8a..7d9cb360 100644 --- a/lib/networkaccessmanager.cpp +++ b/lib/networkaccessmanager.cpp @@ -29,7 +29,8 @@ class NetworkAccessManager::Private QList ignoredSslErrors; }; -NetworkAccessManager::NetworkAccessManager(QObject* parent) : d(std::make_unique()) +NetworkAccessManager::NetworkAccessManager(QObject* parent) + : QNetworkAccessManager(parent), d(std::make_unique()) { } QList NetworkAccessManager::ignoredSslErrors() const diff --git a/lib/networksettings.cpp b/lib/networksettings.cpp index 48bd09f3..6ff2bc1f 100644 --- a/lib/networksettings.cpp +++ b/lib/networksettings.cpp @@ -27,5 +27,5 @@ void NetworkSettings::setupApplicationProxy() const } QMC_DEFINE_SETTING(NetworkSettings, QNetworkProxy::ProxyType, proxyType, "proxy_type", QNetworkProxy::DefaultProxy, setProxyType) -QMC_DEFINE_SETTING(NetworkSettings, QString, proxyHostName, "proxy_hostname", "", setProxyHostName) +QMC_DEFINE_SETTING(NetworkSettings, QString, proxyHostName, "proxy_hostname", {}, setProxyHostName) QMC_DEFINE_SETTING(NetworkSettings, quint16, proxyPort, "proxy_port", -1, setProxyPort) diff --git a/lib/room.cpp b/lib/room.cpp index 1a63866f..caeeb499 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -168,7 +168,7 @@ class Room::Private //void inviteUser(User* u); // We might get it at some point in time. void insertMemberIntoMap(User* u); - void renameMember(User* u, QString oldName); + void renameMember(User* u, const QString& oldName); void removeMemberFromMap(const QString& username, User* u); // This updates the room displayname field (which is the way a room @@ -185,7 +185,7 @@ class Room::Private void getPreviousContent(int limit = 10); template - const EventT* getCurrentState(QString stateKey = {}) const + const EventT* getCurrentState(const QString& stateKey = {}) const { static const EventT empty; const auto* evt = @@ -236,8 +236,8 @@ class Room::Private * @param placement - position and direction of insertion: Older for * historical messages, Newer for new ones */ - Timeline::difference_type moveEventsToTimeline(RoomEventsRange events, - EventsPlacement placement); + Timeline::size_type moveEventsToTimeline(RoomEventsRange events, + EventsPlacement placement); /** * Remove events from the passed container that are already in the timeline @@ -341,7 +341,7 @@ const QString& Room::id() const QString Room::version() const { const auto v = d->getCurrentState()->version(); - return v.isEmpty() ? "1" : v; + return v.isEmpty() ? QStringLiteral("1") : v; } bool Room::isUnstable() const @@ -546,8 +546,8 @@ Room::Changes Room::Private::promoteReadMarker(User* u, rev_iter_t newMarker, { const auto oldUnreadCount = unreadMessages; QElapsedTimer et; et.start(); - unreadMessages = count_if(eagerMarker, timeline.cend(), - std::bind(&Room::Private::isEventNotable, this, _1)); + unreadMessages = int(count_if(eagerMarker, timeline.cend(), + std::bind(&Room::Private::isEventNotable, this, _1))); if (et.nsecsElapsed() > profilerMinNsecs() / 10) qCDebug(PROFILER) << "Recounting unread messages took" << et; @@ -611,7 +611,7 @@ bool Room::canSwitchVersions() const // TODO, #276: m.room.power_levels const auto* plEvt = - d->currentState.value({"m.room.power_levels", ""}); + d->currentState.value({QStringLiteral("m.room.power_levels"), {}}); if (!plEvt) return true; @@ -621,7 +621,7 @@ bool Room::canSwitchVersions() const .value(localUser()->id()).toInt( plJson.value("users_default"_ls).toInt()); const auto tombstonePowerLevel = - plJson.value("events").toObject() + plJson.value("events"_ls).toObject() .value("m.room.tombstone"_ls).toInt( plJson.value("state_default"_ls).toInt()); return currentUserLevel >= tombstonePowerLevel; @@ -947,7 +947,7 @@ void Room::Private::setTags(TagsMap newTags) } tags = move(newTags); qCDebug(MAIN) << "Room" << q->objectName() << "is tagged with" - << q->tagNames().join(", "); + << q->tagNames().join(QStringLiteral(", ")); emit q->tagsChanged(); } @@ -1196,7 +1196,7 @@ void Room::Private::insertMemberIntoMap(User *u) emit q->memberRenamed(namesakes.front()); } -void Room::Private::renameMember(User* u, QString oldName) +void Room::Private::renameMember(User* u, const QString& oldName) { if (u->name(q) == oldName) { @@ -1234,7 +1234,7 @@ inline auto makeErrorStr(const Event& e, QByteArray msg) return msg.append("; event dump follows:\n").append(e.originalJson()); } -Room::Timeline::difference_type Room::Private::moveEventsToTimeline( +Room::Timeline::size_type Room::Private::moveEventsToTimeline( RoomEventsRange events, EventsPlacement placement) { Q_ASSERT(!events.empty()); @@ -1407,7 +1407,7 @@ QString Room::Private::doSendEvent(const RoomEvent* pEvent) return; } it->setDeparted(); - emit q->pendingEventChanged(it - unsyncedEvents.begin()); + emit q->pendingEventChanged(int(it - unsyncedEvents.begin())); }); Room::connect(call, &BaseJob::failure, q, std::bind(&Room::Private::onEventSendingFailure, this, txnId, call)); @@ -1423,7 +1423,7 @@ QString Room::Private::doSendEvent(const RoomEvent* pEvent) } it->setReachedServer(call->eventId()); - emit q->pendingEventChanged(it - unsyncedEvents.begin()); + emit q->pendingEventChanged(int(it - unsyncedEvents.begin())); }); } else onEventSendingFailure(txnId); @@ -1442,7 +1442,7 @@ void Room::Private::onEventSendingFailure(const QString& txnId, BaseJob* call) it->setSendingFailed(call ? call->statusCaption() % ": " % call->errorString() : tr("The call could not be started")); - emit q->pendingEventChanged(it - unsyncedEvents.begin()); + emit q->pendingEventChanged(int(it - unsyncedEvents.begin())); } QString Room::retryMessage(const QString& txnId) @@ -2045,7 +2045,7 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events) roomChanges |= q->processStateEvent(*eptr); auto timelineSize = timeline.size(); - auto totalInserted = 0; + size_t totalInserted = 0; for (auto it = events.begin(); it != events.end();) { auto nextPendingPair = findFirstOf(it, events.end(), diff --git a/lib/settings.cpp b/lib/settings.cpp index 852e19cb..124d7042 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -84,18 +84,21 @@ void SettingsGroup::remove(const QString& key) Settings::remove(fullKey); } -QMC_DEFINE_SETTING(AccountSettings, QString, deviceId, "device_id", "", setDeviceId) -QMC_DEFINE_SETTING(AccountSettings, QString, deviceName, "device_name", "", setDeviceName) +QMC_DEFINE_SETTING(AccountSettings, QString, deviceId, "device_id", {}, setDeviceId) +QMC_DEFINE_SETTING(AccountSettings, QString, deviceName, "device_name", {}, setDeviceName) QMC_DEFINE_SETTING(AccountSettings, bool, keepLoggedIn, "keep_logged_in", false, setKeepLoggedIn) +static const auto HomeserverKey = QStringLiteral("homeserver"); +static const auto AccessTokenKey = QStringLiteral("access_token"); + QUrl AccountSettings::homeserver() const { - return QUrl::fromUserInput(value("homeserver").toString()); + return QUrl::fromUserInput(value(HomeserverKey).toString()); } void AccountSettings::setHomeserver(const QUrl& url) { - setValue("homeserver", url.toString()); + setValue(HomeserverKey, url.toString()); } QString AccountSettings::userId() const @@ -105,19 +108,19 @@ QString AccountSettings::userId() const QString AccountSettings::accessToken() const { - return value("access_token").toString(); + return value(AccessTokenKey).toString(); } void AccountSettings::setAccessToken(const QString& accessToken) { qCWarning(MAIN) << "Saving access_token to QSettings is insecure." " Developers, please save access_token separately."; - setValue("access_token", accessToken); + setValue(AccessTokenKey, accessToken); } void AccountSettings::clearAccessToken() { - legacySettings.remove("access_token"); - legacySettings.remove("device_id"); // Force the server to re-issue it - remove("access_token"); + legacySettings.remove(AccessTokenKey); + legacySettings.remove(QStringLiteral("device_id")); // Force the server to re-issue it + remove(AccessTokenKey); } diff --git a/lib/settings.h b/lib/settings.h index 0b3ecaff..759bda35 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -119,7 +119,7 @@ type classname::propname() const \ \ void classname::setter(type newValue) \ { \ - setValue(QStringLiteral(qsettingname), newValue); \ + setValue(QStringLiteral(qsettingname), std::move(newValue)); \ } \ class AccountSettings: public SettingsGroup diff --git a/lib/syncdata.cpp b/lib/syncdata.cpp index f55d4396..21517884 100644 --- a/lib/syncdata.cpp +++ b/lib/syncdata.cpp @@ -72,7 +72,7 @@ void JsonObjectConverter::fillFrom(const QJsonObject& jo, { fromJson(jo["m.joined_member_count"_ls], rs.joinedMemberCount); fromJson(jo["m.invited_member_count"_ls], rs.invitedMemberCount); - fromJson(jo["m.heroes"], rs.heroes); + fromJson(jo["m.heroes"_ls], rs.heroes); } template @@ -85,7 +85,7 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, const QJsonObject& room_) : roomId(roomId_) , joinState(joinState_) - , summary(fromJson(room_["summary"])) + , summary(fromJson(room_["summary"_ls])) , state(load(room_, joinState == JoinState::Invite ? "invite_state"_ls : "state"_ls)) { @@ -121,8 +121,8 @@ SyncData::SyncData(const QString& cacheFileName) QFileInfo cacheFileInfo { cacheFileName }; auto json = loadJson(cacheFileName); auto requiredVersion = std::get<0>(cacheVersion()); - auto actualVersion = json.value("cache_version").toObject() - .value("major").toInt(); + auto actualVersion = json.value("cache_version"_ls).toObject() + .value("major"_ls).toInt(); if (actualVersion == requiredVersion) parseJson(json, cacheFileInfo.absolutePath() + '/'); else diff --git a/lib/user.cpp b/lib/user.cpp index 951ad87d..17db5760 100644 --- a/lib/user.cpp +++ b/lib/user.cpp @@ -59,7 +59,7 @@ class User::Private QMultiHash otherNames; Avatar mostUsedAvatar { makeAvatar({}) }; std::vector otherAvatars; - auto otherAvatar(QUrl url) + auto otherAvatar(const QUrl& url) { return std::find_if(otherAvatars.begin(), otherAvatars.end(), [&url] (const auto& av) { return av.url() == url; }); @@ -69,7 +69,7 @@ class User::Private mutable int totalRooms = 0; QString nameForRoom(const Room* r, const QString& hint = {}) const; - void setNameForRoom(const Room* r, QString newName, QString oldName); + void setNameForRoom(const Room* r, QString newName, const QString& oldName); QUrl avatarUrlForRoom(const Room* r, const QUrl& hint = {}) const; void setAvatarForRoom(const Room* r, const QUrl& newUrl, const QUrl& oldUrl); @@ -91,7 +91,7 @@ QString User::Private::nameForRoom(const Room* r, const QString& hint) const static constexpr int MIN_JOINED_ROOMS_TO_LOG = 20; void User::Private::setNameForRoom(const Room* r, QString newName, - QString oldName) + const QString& oldName) { Q_ASSERT(oldName != newName); Q_ASSERT(oldName == mostUsedName || otherNames.contains(oldName, r)); @@ -118,7 +118,8 @@ void User::Private::setNameForRoom(const Room* r, QString newName, et.start(); } - for (auto* r1: connection->roomMap()) + const auto& roomMap = connection->roomMap(); + for (auto* r1: roomMap) if (nameForRoom(r1) == mostUsedName) otherNames.insert(mostUsedName, r1); @@ -178,7 +179,8 @@ void User::Private::setAvatarForRoom(const Room* r, const QUrl& newUrl, auto nextMostUsedIt = otherAvatar(newUrl); Q_ASSERT(nextMostUsedIt != otherAvatars.end()); std::swap(mostUsedAvatar, *nextMostUsedIt); - for (const auto* r1: connection->roomMap()) + const auto& roomMap = connection->roomMap(); + for (const auto* r1: roomMap) if (avatarUrlForRoom(r1) == nextMostUsedIt->url()) avatarsToRooms.insert(nextMostUsedIt->url(), r1); @@ -399,7 +401,7 @@ void User::processEvent(const RoomMemberEvent& event, const Room* room, // exceptionally rare (the only reasonable case being that the bridge // changes the naming convention). For the same reason room-specific // bridge tags are not supported at all. - QRegularExpression reSuffix(" \\((IRC|Gitter|Telegram)\\)$"); + QRegularExpression reSuffix(QStringLiteral(" \\((IRC|Gitter|Telegram)\\)$")); auto match = reSuffix.match(newName); if (match.hasMatch()) { diff --git a/lib/util.cpp b/lib/util.cpp index 8d16cfc8..17674b84 100644 --- a/lib/util.cpp +++ b/lib/util.cpp @@ -156,7 +156,7 @@ static_assert(!is_callable_v>, "Test non-function object"); // "Test returns<> with static member function"); template -QString ft(T&&); +QString ft(T&&) { return {}; } static_assert(std::is_same)>, QString&&>(), "Test function templates"); -- cgit v1.2.3 From 533f837086d3522c54f0622d3f27e0a5d1347b24 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 15 Apr 2019 20:29:00 +0900 Subject: BaseJob: fix a possible crash upon logout See https://github.com/QMatrixClient/Quaternion/issues/566 for details. --- lib/jobs/basejob.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index f521cc4b..a79d0e03 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -600,10 +600,25 @@ QUrl BaseJob::errorUrl() const void BaseJob::setStatus(Status s) { + // The crash that led to this code has been reported in + // https://github.com/QMatrixClient/Quaternion/issues/566 - basically, + // when cleaning up childrent of a deleted Connection, there's a chance + // of pending jobs being abandoned, calling setStatus(Abandoned). + // There's nothing wrong with this; however, the safety check for + // cleartext access tokens below uses d->connection - which is a dangling + // pointer. + // To alleviate that, a stricter condition is applied, that for Abandoned + // and to-be-Abandoned jobs the status message will be disregarded entirely. + // For 0.6 we might rectify the situation by making d->connection + // a QPointer<> (and derive ConnectionData from QObject, respectively). + if (d->status.code == Abandoned || s.code == Abandoned) + s.message.clear(); + if (d->status == s) return; - if (d->connection && !d->connection->accessToken().isEmpty()) + if (!s.message.isEmpty() + && d->connection && !d->connection->accessToken().isEmpty()) s.message.replace(d->connection->accessToken(), "(REDACTED)"); if (!s.good()) qCWarning(d->logCat) << this << "status" << s; -- cgit v1.2.3 From 4c9ed0c64ec45c084af657e1fa188242a1d26d1a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 16 Apr 2019 16:21:53 +0900 Subject: BaseJob: preserve the calculated error code if JSON error code is unknown Resetting the code to IncorrectRequestError has been a part of the cause for the incorrect Quaternion behaviour on expired tokens. --- lib/jobs/basejob.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index a79d0e03..0d9b9f10 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -334,8 +334,7 @@ void BaseJob::gotReply() tr("Requested room version: %1") .arg(json.value("room_version").toString()); } else if (!json.isEmpty()) // Not localisable on the client side - setStatus(IncorrectRequestError, - json.value("error"_ls).toString()); + setStatus(d->status.code, json.value("error"_ls).toString()); } } -- cgit v1.2.3