From 5038ae0a0099c2a5c6ffdd08734b597d92edac70 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 7 May 2017 20:02:34 +0900 Subject: Code cleanup and tweaking (partially driven by clang-tidy) Mainly it's about const-ification (in particular, passing const-refs instead of values) and deleting unneeded declarations/#includes. Since the changes alter the external interface, this is submitted as a PR for peer review. One of unneeded declarations/definitions is a virtual destructor in BaseJob descendants. Since a job object should be deleted through QObject::deleteLater() anyway (and it's the only correct way of disposing of the object), all deletions will call the stack of destructors through virtual QObject::~QObject(). Therefore even BaseJob could get on with a non-virtual destructor but for the sake of clarity BaseJob::~BaseJob() is still declared virtual. --- jobs/basejob.cpp | 27 ++++++++++++++------------- jobs/basejob.h | 12 +++++++----- jobs/checkauthmethods.cpp | 9 +-------- jobs/checkauthmethods.h | 4 +--- jobs/joinroomjob.cpp | 6 +----- jobs/joinroomjob.h | 16 +++++++--------- jobs/leaveroomjob.cpp | 5 +---- jobs/leaveroomjob.h | 4 +--- jobs/logoutjob.cpp | 6 +----- jobs/logoutjob.h | 3 +-- jobs/mediathumbnailjob.cpp | 2 +- jobs/mediathumbnailjob.h | 2 +- jobs/passwordlogin.cpp | 6 +----- jobs/passwordlogin.h | 5 ++--- jobs/postmessagejob.cpp | 18 +++++++----------- jobs/postmessagejob.h | 6 +++--- jobs/postreceiptjob.cpp | 10 ++-------- jobs/postreceiptjob.h | 4 ++-- jobs/roommessagesjob.cpp | 6 ++---- jobs/roommessagesjob.h | 6 +++--- jobs/syncjob.cpp | 14 +++++++------- jobs/syncjob.h | 17 ++++++++--------- 22 files changed, 74 insertions(+), 114 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 3e2ecd54..f5de75f4 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include "../connectiondata.h" @@ -44,16 +43,18 @@ struct NetworkReplyDeleter : public QScopedPointerDeleteLater class BaseJob::Private { public: - Private(ConnectionData* c, HttpVerb v, QString endpoint, - const QUrlQuery& q, const Data& data, bool nt) - : connection(c), verb(v), apiEndpoint(endpoint), requestQuery(q) - , requestData(data), needsToken(nt) - , reply(nullptr), status(NoError) + // Using an idiom from clang-tidy: + // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html + Private(const ConnectionData* c, HttpVerb v, + QString endpoint, QUrlQuery q, Data data, bool nt) + : connection(c), verb(v), apiEndpoint(std::move(endpoint)) + , requestQuery(std::move(q)), requestData(std::move(data)) + , needsToken(nt), reply(nullptr), status(NoError) { } inline void sendRequest(); - ConnectionData* connection; + const ConnectionData* connection; // Contents for the network request HttpVerb verb; @@ -77,9 +78,9 @@ inline QDebug operator<<(QDebug dbg, const BaseJob* j) return dbg << "Job" << j->objectName(); } -BaseJob::BaseJob(ConnectionData* connection, HttpVerb verb, QString name, - QString endpoint, BaseJob::Query query, BaseJob::Data data, - bool needsToken) +BaseJob::BaseJob(const ConnectionData* connection, HttpVerb verb, + const QString& name, const QString& endpoint, + const Query& query, const Data& data, bool needsToken) : d(new Private(connection, verb, endpoint, query, data, needsToken)) { setObjectName(name); @@ -96,12 +97,12 @@ BaseJob::~BaseJob() qCDebug(JOBS) << this << "destroyed"; } -ConnectionData* BaseJob::connection() const +const ConnectionData* BaseJob::connection() const { return d->connection; } -const QUrlQuery&BaseJob::query() const +const QUrlQuery& BaseJob::query() const { return d->requestQuery; } @@ -111,7 +112,7 @@ void BaseJob::setRequestQuery(const QUrlQuery& query) d->requestQuery = query; } -const BaseJob::Data&BaseJob::requestData() const +const BaseJob::Data& BaseJob::requestData() const { return d->requestData; } diff --git a/jobs/basejob.h b/jobs/basejob.h index ed35e3e8..460af0fc 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -100,7 +100,7 @@ namespace QMatrixClient { public: Status(StatusCode c) : code(c) { } - Status(int c, QString m) : code(c), message(m) { } + Status(int c, QString m) : code(c), message(std::move(m)) { } bool good() const { return code < ErrorLevel; } @@ -111,10 +111,10 @@ namespace QMatrixClient using duration_t = int; // milliseconds public: - BaseJob(ConnectionData* connection, HttpVerb verb, QString name, - QString endpoint, Query query = Query(), Data data = Data(), + BaseJob(const ConnectionData* connection, HttpVerb verb, + const QString& name, const QString& endpoint, + const Query& query = {}, const Data& data = {}, bool needsToken = true); - virtual ~BaseJob(); Status status() const; int error() const; @@ -198,7 +198,7 @@ namespace QMatrixClient void failure(BaseJob*); protected: - ConnectionData* connection() const; + const ConnectionData* connection() const; const QUrlQuery& query() const; void setRequestQuery(const QUrlQuery& query); @@ -241,6 +241,8 @@ namespace QMatrixClient void setStatus(Status s); void setStatus(int code, QString message); + // Job objects should only be deleted via QObject::deleteLater + virtual ~BaseJob(); protected slots: void timeout(); void sslErrors(const QList& errors); diff --git a/jobs/checkauthmethods.cpp b/jobs/checkauthmethods.cpp index 5c6a95d2..95b9a8f2 100644 --- a/jobs/checkauthmethods.cpp +++ b/jobs/checkauthmethods.cpp @@ -18,25 +18,18 @@ #include "checkauthmethods.h" -#include -#include #include #include -#include - -#include "../connectiondata.h" using namespace QMatrixClient; class CheckAuthMethods::Private { public: - Private() {} - QString session; }; -CheckAuthMethods::CheckAuthMethods(ConnectionData* connection) +CheckAuthMethods::CheckAuthMethods(const ConnectionData* connection) : BaseJob(connection, HttpVerb::Get, "CheckAuthMethods", "_matrix/client/r0/login", Query(), Data(), false) , d(new Private) diff --git a/jobs/checkauthmethods.h b/jobs/checkauthmethods.h index f6eb978a..7d7dc40f 100644 --- a/jobs/checkauthmethods.h +++ b/jobs/checkauthmethods.h @@ -22,12 +22,10 @@ namespace QMatrixClient { - class ConnectionData; - class CheckAuthMethods : public BaseJob { public: - CheckAuthMethods(ConnectionData* connection); + CheckAuthMethods(const ConnectionData* connection); virtual ~CheckAuthMethods(); QString session(); diff --git a/jobs/joinroomjob.cpp b/jobs/joinroomjob.cpp index dce1f54e..6278c18b 100644 --- a/jobs/joinroomjob.cpp +++ b/jobs/joinroomjob.cpp @@ -19,10 +19,6 @@ #include "joinroomjob.h" #include "util.h" -#include - -#include "../connectiondata.h" - using namespace QMatrixClient; class JoinRoomJob::Private @@ -31,7 +27,7 @@ class JoinRoomJob::Private QString roomId; }; -JoinRoomJob::JoinRoomJob(ConnectionData* data, QString roomAlias) +JoinRoomJob::JoinRoomJob(const ConnectionData* data, const QString& roomAlias) : BaseJob(data, HttpVerb::Post, "JoinRoomJob", QString("_matrix/client/r0/join/%1").arg(roomAlias)) , d(new Private) diff --git a/jobs/joinroomjob.h b/jobs/joinroomjob.h index a69843ed..7cf90fd5 100644 --- a/jobs/joinroomjob.h +++ b/jobs/joinroomjob.h @@ -22,21 +22,19 @@ namespace QMatrixClient { - class ConnectionData; - class JoinRoomJob: public BaseJob { public: - JoinRoomJob(ConnectionData* data, QString roomAlias); + JoinRoomJob(const ConnectionData* data, const QString& roomAlias); virtual ~JoinRoomJob(); QString roomId(); - protected: - Status parseJson(const QJsonDocument& data) override; + protected: + Status parseJson(const QJsonDocument& data) override; - private: - class Private; - Private* d; + private: + class Private; + Private* d; }; -} +} // namespace QMatrixClient diff --git a/jobs/leaveroomjob.cpp b/jobs/leaveroomjob.cpp index 22a5d34b..5557b8e4 100644 --- a/jobs/leaveroomjob.cpp +++ b/jobs/leaveroomjob.cpp @@ -22,10 +22,7 @@ using namespace QMatrixClient; -LeaveRoomJob::LeaveRoomJob(ConnectionData* data, Room* room) +LeaveRoomJob::LeaveRoomJob(const ConnectionData* data, Room* room) : BaseJob(data, HttpVerb::Post, "LeaveRoomJob", QString("_matrix/client/r0/rooms/%1/leave").arg(room->id())) { } - -LeaveRoomJob::~LeaveRoomJob() -{ } diff --git a/jobs/leaveroomjob.h b/jobs/leaveroomjob.h index 4a62810f..03851f4c 100644 --- a/jobs/leaveroomjob.h +++ b/jobs/leaveroomjob.h @@ -22,13 +22,11 @@ namespace QMatrixClient { - class ConnectionData; class Room; class LeaveRoomJob: public BaseJob { public: - LeaveRoomJob(ConnectionData* data, Room* room); - virtual ~LeaveRoomJob(); + LeaveRoomJob(const ConnectionData* data, Room* room); }; } diff --git a/jobs/logoutjob.cpp b/jobs/logoutjob.cpp index 9b9cacb6..84e88760 100644 --- a/jobs/logoutjob.cpp +++ b/jobs/logoutjob.cpp @@ -20,11 +20,7 @@ using namespace QMatrixClient; -LogoutJob::LogoutJob(ConnectionData* connection) +LogoutJob::LogoutJob(const ConnectionData* connection) : BaseJob(connection, HttpVerb::Post, "LogoutJob", "/_matrix/client/r0/logout") { } - -LogoutJob::~LogoutJob() -{ -} diff --git a/jobs/logoutjob.h b/jobs/logoutjob.h index 7d70a74c..780719e4 100644 --- a/jobs/logoutjob.h +++ b/jobs/logoutjob.h @@ -25,7 +25,6 @@ namespace QMatrixClient class LogoutJob: public BaseJob { public: - LogoutJob(ConnectionData* connection); - virtual ~LogoutJob(); + explicit LogoutJob(const ConnectionData* connection); }; } diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp index cfde902a..9bb731b9 100644 --- a/jobs/mediathumbnailjob.cpp +++ b/jobs/mediathumbnailjob.cpp @@ -29,7 +29,7 @@ class MediaThumbnailJob::Private QPixmap thumbnail; }; -MediaThumbnailJob::MediaThumbnailJob(ConnectionData* data, QUrl url, QSize requestedSize, +MediaThumbnailJob::MediaThumbnailJob(const ConnectionData* data, QUrl url, QSize requestedSize, ThumbnailType thumbnailType) : BaseJob(data, HttpVerb::Get, "MediaThumbnailJob", QString("/_matrix/media/v1/thumbnail/%1%2").arg(url.host(), url.path()), diff --git a/jobs/mediathumbnailjob.h b/jobs/mediathumbnailjob.h index cf1e9afb..307d0a99 100644 --- a/jobs/mediathumbnailjob.h +++ b/jobs/mediathumbnailjob.h @@ -29,7 +29,7 @@ namespace QMatrixClient class MediaThumbnailJob: public BaseJob { public: - MediaThumbnailJob(ConnectionData* data, QUrl url, QSize requestedSize, + MediaThumbnailJob(const ConnectionData* data, QUrl url, QSize requestedSize, ThumbnailType thumbnailType=ThumbnailType::Scale); virtual ~MediaThumbnailJob(); diff --git a/jobs/passwordlogin.cpp b/jobs/passwordlogin.cpp index 2f34e86a..081e19bc 100644 --- a/jobs/passwordlogin.cpp +++ b/jobs/passwordlogin.cpp @@ -18,10 +18,6 @@ #include "passwordlogin.h" -#include - -#include "../connectiondata.h" - using namespace QMatrixClient; class PasswordLogin::Private @@ -32,7 +28,7 @@ class PasswordLogin::Private QString returned_token; }; -PasswordLogin::PasswordLogin(ConnectionData* connection, QString user, QString password) +PasswordLogin::PasswordLogin(const ConnectionData* connection, QString user, QString password) : BaseJob(connection, HttpVerb::Post, "PasswordLogin" , "_matrix/client/r0/login" , Query() diff --git a/jobs/passwordlogin.h b/jobs/passwordlogin.h index c1291389..713a1821 100644 --- a/jobs/passwordlogin.h +++ b/jobs/passwordlogin.h @@ -22,12 +22,11 @@ namespace QMatrixClient { - class ConnectionData; - class PasswordLogin : public BaseJob { public: - PasswordLogin(ConnectionData* connection, QString user, QString password); + PasswordLogin(const ConnectionData* connection, + QString user, QString password); virtual ~PasswordLogin(); QString token(); diff --git a/jobs/postmessagejob.cpp b/jobs/postmessagejob.cpp index 9a102325..df30614c 100644 --- a/jobs/postmessagejob.cpp +++ b/jobs/postmessagejob.cpp @@ -17,33 +17,29 @@ */ #include "postmessagejob.h" -#include "../connectiondata.h" #include "util.h" -#include - using namespace QMatrixClient; class PostMessageJob::Private { public: - Private() {} - QString eventId; // unused yet }; -PostMessageJob::PostMessageJob(ConnectionData* connection, const QString& roomId, - const QString& type, const QString& plainText) +PostMessageJob::PostMessageJob(const ConnectionData* connection, + const QString& roomId, const QString& type, + const QString& plainText) : BaseJob(connection, HttpVerb::Post, "PostMessageJob", - QString("_matrix/client/r0/rooms/%1/send/m.room.message").arg(roomId), + QStringLiteral("_matrix/client/r0/rooms/%1/send/m.room.message").arg(roomId), Query(), Data({ { "msgtype", type }, { "body", plainText } }) ) , d(new Private) { } -PostMessageJob::PostMessageJob(ConnectionData* connection, const QString& roomId, - const QString& type, const QString& plainText, - const QString& richText) +PostMessageJob::PostMessageJob(const ConnectionData* connection, + const QString& roomId, const QString& type, + const QString& plainText, const QString& richText) : BaseJob(connection, HttpVerb::Post, "PostMessageJob", QStringLiteral("_matrix/client/r0/rooms/%1/send/m.room.message").arg(roomId), Query(), diff --git a/jobs/postmessagejob.h b/jobs/postmessagejob.h index 14de52f0..f4ae809b 100644 --- a/jobs/postmessagejob.h +++ b/jobs/postmessagejob.h @@ -26,10 +26,10 @@ namespace QMatrixClient { public: /** Constructs a plain text message job */ - PostMessageJob(ConnectionData* connection, const QString& roomId, + PostMessageJob(const ConnectionData* connection, const QString& roomId, const QString& type, const QString& plainText); /** Constructs a rich text message job */ - PostMessageJob(ConnectionData* connection, const QString& roomId, + PostMessageJob(const ConnectionData* connection, const QString& roomId, const QString& type, const QString& plainText, const QString& richText); virtual ~PostMessageJob(); @@ -43,4 +43,4 @@ namespace QMatrixClient class Private; Private* d; }; -} +} // namespace QMatrixClient diff --git a/jobs/postreceiptjob.cpp b/jobs/postreceiptjob.cpp index ee750dbf..00926de6 100644 --- a/jobs/postreceiptjob.cpp +++ b/jobs/postreceiptjob.cpp @@ -17,17 +17,11 @@ */ #include "postreceiptjob.h" -#include "../room.h" -#include "../connectiondata.h" - -#include using namespace QMatrixClient; -PostReceiptJob::PostReceiptJob(ConnectionData* connection, QString roomId, QString eventId) +PostReceiptJob::PostReceiptJob(const ConnectionData* connection, + const QString& roomId, const QString& eventId) : BaseJob(connection, HttpVerb::Post, "PostReceiptJob", QString("/_matrix/client/r0/rooms/%1/receipt/m.read/%2").arg(roomId, eventId)) { } - -PostReceiptJob::~PostReceiptJob() -{ } diff --git a/jobs/postreceiptjob.h b/jobs/postreceiptjob.h index c0002dc0..1c84f411 100644 --- a/jobs/postreceiptjob.h +++ b/jobs/postreceiptjob.h @@ -25,7 +25,7 @@ namespace QMatrixClient class PostReceiptJob: public BaseJob { public: - PostReceiptJob(ConnectionData* connection, QString roomId, QString eventId); - virtual ~PostReceiptJob(); + PostReceiptJob(const ConnectionData* connection, const QString& roomId, + const QString& eventId); }; } diff --git a/jobs/roommessagesjob.cpp b/jobs/roommessagesjob.cpp index 5779d695..a48403c8 100644 --- a/jobs/roommessagesjob.cpp +++ b/jobs/roommessagesjob.cpp @@ -26,14 +26,12 @@ using namespace QMatrixClient; class RoomMessagesJob::Private { public: - Private() {} - Owning events; QString end; }; -RoomMessagesJob::RoomMessagesJob(ConnectionData* data, QString roomId, - QString from, int limit, FetchDirection dir) +RoomMessagesJob::RoomMessagesJob(const ConnectionData* data, const QString& roomId, + const QString& from, int limit, FetchDirection dir) : BaseJob(data, HttpVerb::Get, "RoomMessagesJob", QString("/_matrix/client/r0/rooms/%1/messages").arg(roomId), Query( diff --git a/jobs/roommessagesjob.h b/jobs/roommessagesjob.h index af7d65df..2d15d9d4 100644 --- a/jobs/roommessagesjob.h +++ b/jobs/roommessagesjob.h @@ -29,8 +29,8 @@ namespace QMatrixClient class RoomMessagesJob: public BaseJob { public: - RoomMessagesJob(ConnectionData* data, QString roomId, - QString from, int limit = 10, + RoomMessagesJob(const ConnectionData* data, const QString& roomId, + const QString& from, int limit = 10, FetchDirection dir = FetchDirection::Backward); virtual ~RoomMessagesJob(); @@ -44,4 +44,4 @@ namespace QMatrixClient class Private; Private* d; }; -} +} // namespace QMatrixClient diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 8db1e2ca..17c637b0 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -32,8 +32,8 @@ class SyncJob::Private static size_t jobId = 0; -SyncJob::SyncJob(ConnectionData* connection, - QString since, QString filter, int timeout, QString presence) +SyncJob::SyncJob(const ConnectionData* connection, const QString& since, + const QString& filter, int timeout, const QString& presence) : BaseJob(connection, HttpVerb::Get, QString("SyncJob-%1").arg(++jobId), "_matrix/client/r0/sync") , d(new Private) @@ -84,11 +84,10 @@ BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) for (auto roomState: roomStates) { const QJsonObject rs = rooms.value(roomState.jsonKey).toObject(); - d->roomData.reserve(rs.size()); + // We have a Qt container on the right and an STL one on the left + d->roomData.reserve(static_cast(rs.size())); for( auto rkey: rs.keys() ) - { - d->roomData.push_back({rkey, roomState.enumVal, rs[rkey].toObject()}); - } + d->roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject()); } return Success; @@ -99,7 +98,8 @@ void SyncRoomData::EventList::fromJson(const QJsonObject& roomContents) assign(eventsFromJson(roomContents[jsonKey].toObject()["events"].toArray())); } -SyncRoomData::SyncRoomData(QString roomId_, JoinState joinState_, const QJsonObject& room_) +SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, + const QJsonObject& room_) : roomId(roomId_) , joinState(joinState_) , state("state") diff --git a/jobs/syncjob.h b/jobs/syncjob.h index b41c09d4..21d3cfca 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -34,7 +34,7 @@ namespace QMatrixClient private: QString jsonKey; public: - explicit EventList(QString k) : jsonKey(k) { } + explicit EventList(QString k) : jsonKey(std::move(k)) { } void fromJson(const QJsonObject& roomContents); }; @@ -51,11 +51,10 @@ namespace QMatrixClient int highlightCount; int notificationCount; - SyncRoomData(QString roomId_ = QString(), - JoinState joinState_ = JoinState::Join, - const QJsonObject& room_ = QJsonObject()); + SyncRoomData(const QString& roomId, JoinState joinState_, + const QJsonObject& room_); }; -} +} // namespace QMatrixClient Q_DECLARE_TYPEINFO(QMatrixClient::SyncRoomData, Q_MOVABLE_TYPE); namespace QMatrixClient @@ -63,12 +62,12 @@ namespace QMatrixClient // QVector cannot work with non-copiable objects, std::vector can. using SyncData = std::vector; - class ConnectionData; class SyncJob: public BaseJob { public: - SyncJob(ConnectionData* connection, QString since = {}, QString filter = {}, - int timeout = -1, QString presence = {}); + explicit SyncJob(const ConnectionData* connection, const QString& since = {}, + const QString& filter = {}, + int timeout = -1, const QString& presence = {}); virtual ~SyncJob(); SyncData& roomData(); @@ -81,4 +80,4 @@ namespace QMatrixClient class Private; Private* d; }; -} +} // namespace QMatrixClient -- cgit v1.2.3 From b903a3776aabb44863a9a9a4d83c6ee4e033cd5d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 8 May 2017 21:38:44 +0900 Subject: LeaveRoomJob now accepts a roomId, not a Room object; Room::leaveRoom() introduced; Connection and Room cleanup Helps to better encapsulate Room --- jobs/leaveroomjob.cpp | 6 ++---- jobs/leaveroomjob.h | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) (limited to 'jobs') diff --git a/jobs/leaveroomjob.cpp b/jobs/leaveroomjob.cpp index 5557b8e4..f73919ac 100644 --- a/jobs/leaveroomjob.cpp +++ b/jobs/leaveroomjob.cpp @@ -18,11 +18,9 @@ #include "leaveroomjob.h" -#include "../room.h" - using namespace QMatrixClient; -LeaveRoomJob::LeaveRoomJob(const ConnectionData* data, Room* room) +LeaveRoomJob::LeaveRoomJob(const ConnectionData* data, const QString& roomId) : BaseJob(data, HttpVerb::Post, "LeaveRoomJob", - QString("_matrix/client/r0/rooms/%1/leave").arg(room->id())) + QStringLiteral("_matrix/client/r0/rooms/%1/leave").arg(roomId)) { } diff --git a/jobs/leaveroomjob.h b/jobs/leaveroomjob.h index 03851f4c..70883b68 100644 --- a/jobs/leaveroomjob.h +++ b/jobs/leaveroomjob.h @@ -22,11 +22,9 @@ namespace QMatrixClient { - class Room; - class LeaveRoomJob: public BaseJob { public: - LeaveRoomJob(const ConnectionData* data, Room* room); + LeaveRoomJob(const ConnectionData* data, const QString& roomId); }; -} +} // namespace QMatrixClient -- cgit v1.2.3 From 63db311d5a64b9942dc69e1b13b4570a548554c0 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 13 May 2017 15:10:07 +0900 Subject: Refactored logging enhancements logging.h/logging.cpp is now a full-fledged pair for all things logging. Two more categories added, EPHEMERAL and SYNCJOB, that control logging for ephemeral events and SyncJob, respectively (in particular, switching off EPHEMERAL greatly reduces the logspam about moving read markers and how many users have read up to which event). --- jobs/basejob.cpp | 36 ++++++++++++++++++++++-------------- jobs/basejob.h | 9 ++++++++- jobs/syncjob.cpp | 9 ++++++--- 3 files changed, 36 insertions(+), 18 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index f5de75f4..77f90596 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -17,16 +17,15 @@ */ #include "basejob.h" -#include "util.h" -#include +#include "connectiondata.h" #include #include #include #include -#include "../connectiondata.h" +#include using namespace QMatrixClient; @@ -49,10 +48,10 @@ class BaseJob::Private QString endpoint, QUrlQuery q, Data data, bool nt) : connection(c), verb(v), apiEndpoint(std::move(endpoint)) , requestQuery(std::move(q)), requestData(std::move(data)) - , needsToken(nt), reply(nullptr), status(NoError) + , needsToken(nt) { } - inline void sendRequest(); + void sendRequest(); const ConnectionData* connection; @@ -64,13 +63,15 @@ class BaseJob::Private bool needsToken; QScopedPointer reply; - Status status; + Status status = NoError; QTimer timer; QTimer retryTimer; size_t maxRetries = 3; size_t retriesTaken = 0; + + LoggingCategory logCat = JOBS; }; inline QDebug operator<<(QDebug dbg, const BaseJob* j) @@ -88,13 +89,13 @@ BaseJob::BaseJob(const ConnectionData* connection, HttpVerb verb, connect (&d->timer, &QTimer::timeout, this, &BaseJob::timeout); d->retryTimer.setSingleShot(true); connect (&d->retryTimer, &QTimer::timeout, this, &BaseJob::start); - qCDebug(JOBS) << this << "created"; + qCDebug(d->logCat) << this << "created"; } BaseJob::~BaseJob() { stop(); - qCDebug(JOBS) << this << "destroyed"; + qCDebug(d->logCat) << this << "destroyed"; } const ConnectionData* BaseJob::connection() const @@ -176,7 +177,7 @@ void BaseJob::gotReply() BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const { if (reply->error() != QNetworkReply::NoError) - qCDebug(JOBS) << this << "returned" << reply->error(); + qCDebug(d->logCat) << this << "returned" << reply->error(); switch( reply->error() ) { case QNetworkReply::NoError: @@ -219,11 +220,11 @@ void BaseJob::stop() d->timer.stop(); if (!d->reply) { - qCWarning(JOBS) << this << "stopped with empty network reply"; + qCWarning(d->logCat) << this << "stopped with empty network reply"; } else if (d->reply->isRunning()) { - qCWarning(JOBS) << this << "stopped without ready network reply"; + qCWarning(d->logCat) << this << "stopped without ready network reply"; d->reply->disconnect(this); // Ignore whatever comes from the reply d->reply->abort(); } @@ -237,7 +238,7 @@ void BaseJob::finishJob() { const auto retryInterval = getNextRetryInterval(); ++d->retriesTaken; - qCWarning(JOBS) << this << "will take retry" << d->retriesTaken + qCWarning(d->logCat) << this << "will take retry" << d->retriesTaken << "in" << retryInterval/1000 << "s"; d->retryTimer.start(retryInterval); emit retryScheduled(d->retriesTaken, retryInterval); @@ -303,7 +304,8 @@ void BaseJob::setStatus(Status s) d->status = s; if (!s.good()) { - qCWarning(JOBS) << this << "status" << s.code << ":" << s.message; + qCWarning(d->logCat) << this << "status" << s.code + << ":" << s.message; } } @@ -326,7 +328,13 @@ void BaseJob::timeout() void BaseJob::sslErrors(const QList& errors) { foreach (const QSslError &error, errors) { - qCWarning(JOBS) << "SSL ERROR" << error.errorString(); + qCWarning(d->logCat) << "SSL ERROR" << error.errorString(); } d->reply->ignoreSslErrors(); // TODO: insecure! should prompt user first } + +void BaseJob::setLoggingCategory(LoggingCategory lcf) +{ + d->logCat = lcf; +} + diff --git a/jobs/basejob.h b/jobs/basejob.h index 460af0fc..2be4577f 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -18,6 +18,8 @@ #pragma once +#include "logging.h" + #include #include #include @@ -241,6 +243,11 @@ namespace QMatrixClient void setStatus(Status s); void setStatus(int code, QString message); + // Q_DECLARE_LOGGING_CATEGORY return different function types + // in different versions + using LoggingCategory = decltype(JOBS)*; + void setLoggingCategory(LoggingCategory lcf); + // Job objects should only be deleted via QObject::deleteLater virtual ~BaseJob(); protected slots: @@ -257,4 +264,4 @@ namespace QMatrixClient class Private; QScopedPointer d; }; -} +} // namespace QMatrixClient diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 17c637b0..fb6bb9b9 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -19,7 +19,7 @@ #include "syncjob.h" #include -#include +#include using namespace QMatrixClient; @@ -38,6 +38,7 @@ SyncJob::SyncJob(const ConnectionData* connection, const QString& since, "_matrix/client/r0/sync") , d(new Private) { + setLoggingCategory(SYNCJOB); QUrlQuery query; if( !filter.isEmpty() ) query.addQueryItem("filter", filter); @@ -69,6 +70,7 @@ SyncData& SyncJob::roomData() BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { + QElapsedTimer et; et.start(); QJsonObject json = data.object(); d->nextBatch = json.value("next_batch").toString(); // TODO: presence @@ -89,6 +91,7 @@ BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) for( auto rkey: rs.keys() ) d->roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject()); } + qCDebug(PROFILER) << "*** SyncJob::parseJson():" << et.elapsed() << "ms"; return Success; } @@ -123,7 +126,7 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, timeline.fromJson(room_); break; default: - qCWarning(JOBS) << "SyncRoomData: Unknown JoinState value, ignoring:" << int(joinState); + qCWarning(SYNCJOB) << "SyncRoomData: Unknown JoinState value, ignoring:" << int(joinState); } QJsonObject timeline = room_.value("timeline").toObject(); @@ -133,5 +136,5 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, QJsonObject unread = room_.value("unread_notifications").toObject(); highlightCount = unread.value("highlight_count").toInt(); notificationCount = unread.value("notification_count").toInt(); - qCDebug(JOBS) << "Highlights: " << highlightCount << " Notifications:" << notificationCount; + qCDebug(SYNCJOB) << "Highlights: " << highlightCount << " Notifications:" << notificationCount; } -- cgit v1.2.3 From c25de4e19801c7931ce857c29a7a48be7f5c4dbe Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 13 May 2017 16:00:26 +0900 Subject: More code cleanup and tweaks; fine-tuning logs; performance improvements After adding some profiling it became clear that to recalculate the room name and emit namesChanged() upon each member event is a waste, especially when there are thousands of those coming at initial sync (*cough* Matrix HQ room). So the room name is recalculated only once and unconditionally (in most cases this will boil down to checking whether name/canonicalAlias changed after processing the events batch), and namesChanged is only emitted once per batch, if any name or alias changed. --- jobs/syncjob.cpp | 4 ++-- jobs/syncjob.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'jobs') diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index fb6bb9b9..5984128f 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -63,9 +63,9 @@ QString SyncJob::nextBatch() const return d->nextBatch; } -SyncData& SyncJob::roomData() +SyncData&& SyncJob::takeRoomData() { - return d->roomData; + return std::move(d->roomData); } BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) diff --git a/jobs/syncjob.h b/jobs/syncjob.h index 21d3cfca..48be9423 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -70,7 +70,7 @@ namespace QMatrixClient int timeout = -1, const QString& presence = {}); virtual ~SyncJob(); - SyncData& roomData(); + SyncData&& takeRoomData(); QString nextBatch() const; protected: -- cgit v1.2.3