From 098a3855650c16f08df1e24139cd0cbac9b112c2 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 8 May 2017 19:22:30 +0900 Subject: Make QNetworkAccessManager member a singleton As Qt documentation advises, a single QNetworkAccessManager is enough for the whole Qt application. --- connectiondata.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/connectiondata.cpp b/connectiondata.cpp index a6d80d53..483d2f0c 100644 --- a/connectiondata.cpp +++ b/connectiondata.cpp @@ -24,27 +24,27 @@ using namespace QMatrixClient; -class ConnectionData::Private +QNetworkAccessManager* getNam() { - public: - Private() : nam(nullptr) { } - - QUrl baseUrl; - QString accessToken; - QString lastEvent; - QNetworkAccessManager* nam; + static QNetworkAccessManager* _nam = new QNetworkAccessManager(); + return _nam; +} + +struct ConnectionData::Private +{ + QUrl baseUrl; + QString accessToken; + QString lastEvent; }; ConnectionData::ConnectionData(QUrl baseUrl) : d(new Private) { d->baseUrl = baseUrl; - d->nam = new QNetworkAccessManager(); } ConnectionData::~ConnectionData() { - d->nam->deleteLater(); delete d; } @@ -60,7 +60,7 @@ QUrl ConnectionData::baseUrl() const QNetworkAccessManager* ConnectionData::nam() const { - return d->nam; + return getNam(); } void ConnectionData::setToken(QString token) -- cgit v1.2.3 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. --- connection.cpp | 80 +++++++++++++++++----------------------------- connection.h | 46 ++++++++++++++------------ events/receiptevent.h | 2 -- events/roommemberevent.h | 5 ++- events/roommessageevent.h | 4 +-- events/roomnameevent.cpp | 4 +-- events/roomnameevent.h | 24 +++++++------- events/roomtopicevent.h | 2 -- events/typingevent.h | 4 +-- 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 +++++----- room.cpp | 16 +++++----- room.h | 4 +-- user.cpp | 15 +++++---- 34 files changed, 166 insertions(+), 228 deletions(-) diff --git a/connection.cpp b/connection.cpp index 0fc23f2f..8d5ff2fa 100644 --- a/connection.cpp +++ b/connection.cpp @@ -39,20 +39,20 @@ using namespace QMatrixClient; class Connection::Private { public: - explicit Private(QUrl serverUrl) + explicit Private(const QUrl& serverUrl) : q(nullptr) , data(new ConnectionData(serverUrl)) - , isConnected(false) , syncJob(nullptr) { } - Private(Private&) = delete; + Q_DISABLE_COPY(Private) + Private(Private&&) = delete; + Private operator=(Private&&) = delete; ~Private() { delete data; } Connection* q; ConnectionData* data; QHash roomMap; QHash userMap; - bool isConnected; QString username; QString password; QString userId; @@ -60,7 +60,7 @@ class Connection::Private SyncJob* syncJob; }; -Connection::Connection(QUrl server, QObject* parent) +Connection::Connection(const QUrl& server, QObject* parent) : QObject(parent) , d(new Private(server)) { @@ -75,10 +75,11 @@ Connection::Connection() Connection::~Connection() { qCDebug(MAIN) << "deconstructing connection object for" << d->userId; + stopSync(); delete d; } -void Connection::resolveServer(QString domain) +void Connection::resolveServer(const QString& domain) { // Find the Matrix server for the given domain. QScopedPointer dns { new QDnsLookup() }; @@ -102,23 +103,21 @@ void Connection::resolveServer(QString domain) }); } -void Connection::connectToServer(QString user, QString password) +void Connection::connectToServer(const QString& user, const QString& password) { - PasswordLogin* loginJob = new PasswordLogin(d->data, user, password); + auto loginJob = callApi(user, password); connect( loginJob, &PasswordLogin::success, [=] () { connectWithToken(loginJob->id(), loginJob->token()); }); connect( loginJob, &PasswordLogin::failure, [=] () { emit loginError(loginJob->errorString()); }); - loginJob->start(); d->username = user; // to be able to reconnect d->password = password; } -void Connection::connectWithToken(QString userId, QString token) +void Connection::connectWithToken(const QString& userId, const QString& token) { - d->isConnected = true; d->userId = userId; d->data->setToken(token); qCDebug(MAIN) << "Accessing" << d->data->baseUrl() @@ -130,22 +129,14 @@ void Connection::connectWithToken(QString userId, QString token) void Connection::reconnect() { - PasswordLogin* loginJob = new PasswordLogin(d->data, d->username, d->password ); + auto loginJob = callApi(d->username, d->password); connect( loginJob, &PasswordLogin::success, [=] () { d->userId = loginJob->id(); emit reconnected(); }); connect( loginJob, &PasswordLogin::failure, [=] () { emit loginError(loginJob->errorString()); - d->isConnected = false; }); - loginJob->start(); -} - -void Connection::disconnectFromServer() -{ - stopSync(); - d->isConnected = false; } void Connection::logout() @@ -162,7 +153,8 @@ void Connection::sync(int timeout) if (d->syncJob) return; - const QString filter = "{\"room\": { \"timeline\": { \"limit\": 100 } } }"; + // Raw string: http://en.cppreference.com/w/cpp/language/string_literal + const QString filter { R"({"room": { "timeline": { "limit": 100 } } })" }; auto job = d->syncJob = callApi(d->data->lastEvent(), filter, timeout); connect( job, &SyncJob::success, [=] () { @@ -194,20 +186,17 @@ void Connection::stopSync() } } -void Connection::postMessage(Room* room, QString type, QString message) +void Connection::postMessage(Room* room, const QString& type, const QString& message) const { - PostMessageJob* job = new PostMessageJob(d->data, room->id(), type, message); - job->start(); + callApi(room->id(), type, message); } -PostReceiptJob* Connection::postReceipt(Room* room, Event* event) +PostReceiptJob* Connection::postReceipt(Room* room, Event* event) const { - PostReceiptJob* job = new PostReceiptJob(d->data, room->id(), event->id()); - job->start(); - return job; + return callApi(room->id(), event->id()); } -JoinRoomJob* Connection::joinRoom(QString roomAlias) +JoinRoomJob* Connection::joinRoom(const QString& roomAlias) { auto job = callApi(roomAlias); connect( job, &BaseJob::success, [=] () { @@ -219,25 +208,21 @@ JoinRoomJob* Connection::joinRoom(QString roomAlias) void Connection::leaveRoom(Room* room) { - LeaveRoomJob* job = new LeaveRoomJob(d->data, room); - job->start(); + callApi(room); } -RoomMessagesJob* Connection::getMessages(Room* room, QString from) +RoomMessagesJob* Connection::getMessages(Room* room, const QString& from) const { - RoomMessagesJob* job = new RoomMessagesJob(d->data, room->id(), from); - job->start(); - return job; + return callApi(room->id(), from); } -MediaThumbnailJob* Connection::getThumbnail(QUrl url, QSize requestedSize) +MediaThumbnailJob* Connection::getThumbnail(const QUrl& url, QSize requestedSize) const { - MediaThumbnailJob* job = new MediaThumbnailJob(d->data, url, requestedSize); - job->start(); - return job; + return callApi(url, requestedSize); } -MediaThumbnailJob* Connection::getThumbnail(QUrl url, int requestedWidth, int requestedHeight) +MediaThumbnailJob* Connection::getThumbnail(const QUrl& url, int requestedWidth, + int requestedHeight) const { return getThumbnail(url, QSize(requestedWidth, requestedHeight)); } @@ -247,7 +232,7 @@ QUrl Connection::homeserver() const return d->data->baseUrl(); } -User* Connection::user(QString userId) +User* Connection::user(const QString& userId) { if( d->userMap.contains(userId) ) return d->userMap.value(userId); @@ -293,17 +278,12 @@ QHash< QString, Room* > Connection::roomMap() const return d->roomMap; } -bool Connection::isConnected() -{ - return d->isConnected; -} - -ConnectionData* Connection::connectionData() +const ConnectionData* Connection::connectionData() const { return d->data; } -Room* Connection::provideRoom(QString id) +Room* Connection::provideRoom(const QString& id) { if (id.isEmpty()) { @@ -327,12 +307,12 @@ Room* Connection::provideRoom(QString id) return room; } -User* Connection::createUser(QString userId) +User* Connection::createUser(const QString& userId) { return new User(userId, this); } -Room* Connection::createRoom(QString roomId) +Room* Connection::createRoom(const QString& roomId) { return new Room(this, roomId); } diff --git a/connection.h b/connection.h index 849106a1..8fc4a6b3 100644 --- a/connection.h +++ b/connection.h @@ -39,34 +39,38 @@ namespace QMatrixClient class Connection: public QObject { Q_OBJECT public: - Connection(QUrl server, QObject* parent = nullptr); + explicit Connection(const QUrl& server, QObject* parent = nullptr); Connection(); virtual ~Connection(); QHash roomMap() const; - Q_INVOKABLE virtual bool isConnected(); - Q_INVOKABLE virtual void resolveServer( QString domain ); - Q_INVOKABLE virtual void connectToServer( QString user, QString password ); - Q_INVOKABLE virtual void connectWithToken( QString userId, QString token ); + Q_INVOKABLE virtual void resolveServer(const QString& domain); + Q_INVOKABLE virtual void connectToServer(const QString& user, + const QString& password); + Q_INVOKABLE virtual void connectWithToken(const QString& userId, + const QString& token); Q_INVOKABLE virtual void reconnect(); - Q_INVOKABLE virtual void disconnectFromServer(); Q_INVOKABLE virtual void logout(); - Q_INVOKABLE virtual void sync(int timeout=-1); - Q_INVOKABLE virtual void stopSync(); + Q_INVOKABLE void sync(int timeout = -1); + Q_INVOKABLE void stopSync(); /** @deprecated Use callApi() or Room::postMessage() instead */ - Q_INVOKABLE virtual void postMessage( Room* room, QString type, QString message ); + Q_INVOKABLE virtual void postMessage(Room* room, const QString& type, + const QString& message) const; /** @deprecated Use callApi() or Room::postReceipt() instead */ - Q_INVOKABLE virtual PostReceiptJob* postReceipt( Room* room, Event* event ); - Q_INVOKABLE virtual JoinRoomJob* joinRoom( QString roomAlias ); + Q_INVOKABLE virtual PostReceiptJob* postReceipt( Room* room, Event* event ) const; + Q_INVOKABLE virtual JoinRoomJob* joinRoom(const QString& roomAlias); Q_INVOKABLE virtual void leaveRoom( Room* room ); - Q_INVOKABLE virtual RoomMessagesJob* getMessages( Room* room, QString from ); - virtual MediaThumbnailJob* getThumbnail( QUrl url, QSize requestedSize ); - MediaThumbnailJob* getThumbnail( QUrl url, int requestedWidth, int requestedHeight ); + Q_INVOKABLE virtual RoomMessagesJob* getMessages(Room* room, + const QString& from) const; + virtual MediaThumbnailJob* getThumbnail(const QUrl& url, + QSize requestedSize) const; + MediaThumbnailJob* getThumbnail(const QUrl& url, int requestedWidth, + int requestedHeight) const; Q_INVOKABLE QUrl homeserver() const; - Q_INVOKABLE User* user(QString userId); + Q_INVOKABLE User* user(const QString& userId); Q_INVOKABLE User* user(); Q_INVOKABLE QString userId() const; /** @deprecated Use accessToken() instead. */ @@ -76,7 +80,7 @@ namespace QMatrixClient Q_INVOKABLE int millisToReconnect() const; template - JobT* callApi(JobArgTs... jobArgs) + JobT* callApi(JobArgTs... jobArgs) const { auto job = new JobT(connectionData(), jobArgs...); job->start(); @@ -103,7 +107,7 @@ namespace QMatrixClient /** * @brief Access the underlying ConnectionData class */ - ConnectionData* connectionData(); + const ConnectionData* connectionData() const; /** * @brief Find a (possibly new) Room object for the specified id @@ -114,20 +118,20 @@ namespace QMatrixClient * @return a pointer to a Room object with the specified id; nullptr * if roomId is empty if createRoom() failed to create a Room object. */ - Room* provideRoom(QString roomId); + Room* provideRoom(const QString& roomId); /** * makes it possible for derived classes to have its own User class */ - virtual User* createUser(QString userId); + virtual User* createUser(const QString& userId); /** * makes it possible for derived classes to have its own Room class */ - virtual Room* createRoom(QString roomId); + virtual Room* createRoom(const QString& roomId); private: class Private; Private* d; }; -} +} // namespace QMatrixClient diff --git a/events/receiptevent.h b/events/receiptevent.h index a7e1debf..40c0384f 100644 --- a/events/receiptevent.h +++ b/events/receiptevent.h @@ -20,8 +20,6 @@ #include "event.h" -#include - namespace QMatrixClient { class Receipt diff --git a/events/roommemberevent.h b/events/roommemberevent.h index f37cdc04..a33c2982 100644 --- a/events/roommemberevent.h +++ b/events/roommemberevent.h @@ -18,11 +18,10 @@ #pragma once -#include -#include - #include "event.h" +#include + namespace QMatrixClient { enum class MembershipType {Invite, Join, Knock, Leave, Ban}; diff --git a/events/roommessageevent.h b/events/roommessageevent.h index 67789ef7..5d5336aa 100644 --- a/events/roommessageevent.h +++ b/events/roommessageevent.h @@ -18,12 +18,12 @@ #pragma once +#include "event.h" + #include #include #include -#include "event.h" - namespace QMatrixClient { enum class MessageEventType diff --git a/events/roomnameevent.cpp b/events/roomnameevent.cpp index c5bcf011..c94cb2c3 100644 --- a/events/roomnameevent.cpp +++ b/events/roomnameevent.cpp @@ -22,8 +22,8 @@ using namespace QMatrixClient; class RoomNameEvent::Private { -public: - QString name; + public: + QString name; }; RoomNameEvent::RoomNameEvent() : diff --git a/events/roomnameevent.h b/events/roomnameevent.h index 0997ad9c..8748c4be 100644 --- a/events/roomnameevent.h +++ b/events/roomnameevent.h @@ -22,20 +22,18 @@ namespace QMatrixClient { + class RoomNameEvent : public Event + { + public: + RoomNameEvent(); + virtual ~RoomNameEvent(); -class RoomNameEvent : public Event -{ -public: - RoomNameEvent(); - virtual ~RoomNameEvent(); - - QString name() const; - - static RoomNameEvent* fromJson(const QJsonObject& obj); + QString name() const; -private: - class Private; - Private *d; -}; + static RoomNameEvent* fromJson(const QJsonObject& obj); + private: + class Private; + Private *d; + }; } diff --git a/events/roomtopicevent.h b/events/roomtopicevent.h index d4347953..4b0a24b0 100644 --- a/events/roomtopicevent.h +++ b/events/roomtopicevent.h @@ -18,8 +18,6 @@ #pragma once -#include - #include "event.h" namespace QMatrixClient diff --git a/events/typingevent.h b/events/typingevent.h index 5a8b045c..da57a389 100644 --- a/events/typingevent.h +++ b/events/typingevent.h @@ -18,10 +18,10 @@ #pragma once -#include - #include "event.h" +#include + namespace QMatrixClient { class TypingEvent: public Event 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 diff --git a/room.cpp b/room.cpp index f13fae19..5159cafd 100644 --- a/room.cpp +++ b/room.cpp @@ -115,7 +115,7 @@ class Room::Private /** * Removes events from the passed container that are already in the timeline */ - void dropDuplicateEvents(Events& events) const; + void dropDuplicateEvents(Events* events) const; void setLastReadEvent(User* u, QString eventId); rev_iter_pair_t promoteReadMarker(User* u, rev_iter_t newMarker); @@ -510,7 +510,7 @@ QString Room::roomMembername(User *u) const return username % " (" % u->id() % ")"; } -QString Room::roomMembername(QString userId) const +QString Room::roomMembername(const QString& userId) const { return roomMembername(connection()->user(userId)); } @@ -577,15 +577,15 @@ void Room::Private::getPreviousContent(int limit) } } -void Room::Private::dropDuplicateEvents(Events& events) const +void Room::Private::dropDuplicateEvents(Events* events) const { // Collect all duplicate events at the end of the container auto dupsBegin = - std::stable_partition(events.begin(), events.end(), + std::stable_partition(events->begin(), events->end(), [&] (Event* e) { return !eventsIndex.contains(e->id()); }); // Dispose of those dups - std::for_each(dupsBegin, events.end(), [] (Event* e) { delete e; }); - events.erase(dupsBegin, events.end()); + std::for_each(dupsBegin, events->end(), [] (Event* e) { delete e; }); + events->erase(dupsBegin, events->end()); } Connection* Room::connection() const @@ -595,7 +595,7 @@ Connection* Room::connection() const void Room::addNewMessageEvents(Events events) { - d->dropDuplicateEvents(events); + d->dropDuplicateEvents(&events); if (events.empty()) return; emit aboutToAddNewMessages(events); @@ -640,7 +640,7 @@ void Room::doAddNewMessageEvents(const Events& events) void Room::addHistoricalMessageEvents(Events events) { - d->dropDuplicateEvents(events); + d->dropDuplicateEvents(&events); if (events.empty()) return; emit aboutToAddHistoricalMessages(events); diff --git a/room.h b/room.h index 315e4016..8b810ae4 100644 --- a/room.h +++ b/room.h @@ -63,13 +63,13 @@ namespace QMatrixClient class Room: public QObject { Q_OBJECT - Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE markMessagesAsRead NOTIFY readMarkerMoved) Q_PROPERTY(QString id READ id CONSTANT) Q_PROPERTY(QString name READ name NOTIFY namesChanged) Q_PROPERTY(QStringList aliases READ aliases NOTIFY namesChanged) Q_PROPERTY(QString canonicalAlias READ canonicalAlias NOTIFY namesChanged) Q_PROPERTY(QString displayName READ displayName NOTIFY namesChanged) Q_PROPERTY(QString topic READ topic NOTIFY topicChanged) + Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE markMessagesAsRead NOTIFY readMarkerMoved) public: using Timeline = std::deque; using rev_iter_t = Timeline::const_reverse_iterator; @@ -98,7 +98,7 @@ namespace QMatrixClient * @brief Produces a disambiguated name for a user with this id in * the context of the room */ - Q_INVOKABLE QString roomMembername(QString userId) const; + Q_INVOKABLE QString roomMembername(const QString& userId) const; Q_INVOKABLE void updateData(SyncRoomData& data ); Q_INVOKABLE void setJoinState( JoinState state ); diff --git a/user.cpp b/user.cpp index c5dac921..17714bee 100644 --- a/user.cpp +++ b/user.cpp @@ -34,6 +34,12 @@ using namespace QMatrixClient; class User::Private { public: + Private(QString userId, Connection* connection) + : q(nullptr), userId(std::move(userId)), connection(connection) + , defaultIcon(QIcon::fromTheme(QStringLiteral("user-available"))) + , avatarValid(false) , avatarOngoingRequest(false) + { } + User* q; QString userId; QString name; @@ -51,14 +57,9 @@ class User::Private }; User::User(QString userId, Connection* connection) - : QObject(connection), d(new Private) + : QObject(connection), d(new Private(userId, connection)) { - d->connection = connection; - d->userId = userId; - d->avatarValid = false; - d->avatarOngoingRequest = false; - d->q = this; - d->defaultIcon = QIcon::fromTheme(QStringLiteral("user-available")); + d->q = this; // Initialization finished } User::~User() -- cgit v1.2.3 From 6431636e6a02d4fddb848e09e79156724e00f6f6 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 9 May 2017 04:43:27 +0900 Subject: Allow to get a Connection object of the Room It's just natural, after all, Connection is a parent of Room. But seriously, this will be needed when we have rooms from different Connections living next to each other. --- room.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/room.h b/room.h index 8b810ae4..e277a3f9 100644 --- a/room.h +++ b/room.h @@ -63,6 +63,7 @@ namespace QMatrixClient class Room: public QObject { Q_OBJECT + Q_PROPERTY(const Connection* connection READ connection CONSTANT) Q_PROPERTY(QString id READ id CONSTANT) Q_PROPERTY(QString name READ name NOTIFY namesChanged) Q_PROPERTY(QStringList aliases READ aliases NOTIFY namesChanged) @@ -77,6 +78,7 @@ namespace QMatrixClient Room(Connection* connection, QString id); virtual ~Room(); + Connection* connection() const; QString id() const; QString name() const; QStringList aliases() const; @@ -173,7 +175,6 @@ namespace QMatrixClient void unreadMessagesChanged(Room* room); protected: - Connection* connection() const; virtual void doAddNewMessageEvents(const Events& events); virtual void doAddHistoricalMessageEvents(const Events& events); virtual void processStateEvents(const Events& events); -- 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 --- connection.cpp | 2 +- connection.h | 1 + jobs/leaveroomjob.cpp | 6 ++---- jobs/leaveroomjob.h | 6 ++---- room.cpp | 6 ++++++ room.h | 2 ++ 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/connection.cpp b/connection.cpp index 8d5ff2fa..0c73e22d 100644 --- a/connection.cpp +++ b/connection.cpp @@ -208,7 +208,7 @@ JoinRoomJob* Connection::joinRoom(const QString& roomAlias) void Connection::leaveRoom(Room* room) { - callApi(room); + callApi(room->id()); } RoomMessagesJob* Connection::getMessages(Room* room, const QString& from) const diff --git a/connection.h b/connection.h index 8fc4a6b3..7ca1ccd6 100644 --- a/connection.h +++ b/connection.h @@ -61,6 +61,7 @@ namespace QMatrixClient /** @deprecated Use callApi() or Room::postReceipt() instead */ Q_INVOKABLE virtual PostReceiptJob* postReceipt( Room* room, Event* event ) const; Q_INVOKABLE virtual JoinRoomJob* joinRoom(const QString& roomAlias); + /** @deprecated Use callApi() or Room::leaveRoom() instead */ Q_INVOKABLE virtual void leaveRoom( Room* room ); Q_INVOKABLE virtual RoomMessagesJob* getMessages(Room* room, const QString& from) const; 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 diff --git a/room.cpp b/room.cpp index 5159cafd..af39da52 100644 --- a/room.cpp +++ b/room.cpp @@ -39,6 +39,7 @@ #include "jobs/postmessagejob.h" #include "jobs/roommessagesjob.h" #include "jobs/postreceiptjob.h" +#include "jobs/leaveroomjob.h" using namespace QMatrixClient; @@ -577,6 +578,11 @@ void Room::Private::getPreviousContent(int limit) } } +void Room::leaveRoom() const +{ + connection()->callApi(id()); +} + void Room::Private::dropDuplicateEvents(Events* events) const { // Collect all duplicate events at the end of the container diff --git a/room.h b/room.h index e277a3f9..689a99b0 100644 --- a/room.h +++ b/room.h @@ -147,6 +147,8 @@ namespace QMatrixClient void postMessage(const QString& type, const QString& plainText, const QString& richText); void getPreviousContent(int limit = 10); + + void leaveRoom() const; void userRenamed(User* user, QString oldName); signals: -- cgit v1.2.3 From cff6aaafeee5c5f7d337c6001694bda119d3cba9 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 9 May 2017 14:15:29 +0900 Subject: Returned Connection::disconnectFromServer() for back-compatibility --- connection.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/connection.h b/connection.h index 7ca1ccd6..08184d0d 100644 --- a/connection.h +++ b/connection.h @@ -51,6 +51,8 @@ namespace QMatrixClient Q_INVOKABLE virtual void connectWithToken(const QString& userId, const QString& token); Q_INVOKABLE virtual void reconnect(); + /** @deprecated Use stopSync() instead */ + Q_INVOKABLE virtual void disconnectFromServer() { stopSync(); } Q_INVOKABLE virtual void logout(); Q_INVOKABLE void sync(int timeout = -1); -- 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). --- CMakeLists.txt | 2 +- connectiondata.cpp | 4 +-- debug.cpp | 30 ------------------ events/event.cpp | 5 ++- events/receiptevent.cpp | 8 ++--- events/roomaliasesevent.cpp | 4 +-- events/roommemberevent.cpp | 3 +- events/roommessageevent.cpp | 3 +- events/typingevent.cpp | 6 ++-- events/unknownevent.cpp | 5 ++- jobs/basejob.cpp | 36 +++++++++++++--------- jobs/basejob.h | 9 +++++- jobs/syncjob.cpp | 9 ++++-- logging.cpp | 26 ++++++++++++++++ logging.h | 74 +++++++++++++++++++++++++++++++++++++++++++++ util.h | 55 +-------------------------------- 16 files changed, 156 insertions(+), 123 deletions(-) delete mode 100644 debug.cpp create mode 100644 logging.cpp create mode 100644 logging.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 4692605c..11cf015d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,7 +47,7 @@ message( STATUS ) set(libqmatrixclient_SRCS connectiondata.cpp connection.cpp - debug.cpp + logging.cpp room.cpp user.cpp state.cpp diff --git a/connectiondata.cpp b/connectiondata.cpp index 483d2f0c..6c7eff8c 100644 --- a/connectiondata.cpp +++ b/connectiondata.cpp @@ -17,9 +17,9 @@ */ #include "connectiondata.h" -#include "util.h" -#include +#include "logging.h" + #include using namespace QMatrixClient; diff --git a/debug.cpp b/debug.cpp deleted file mode 100644 index b80438e5..00000000 --- a/debug.cpp +++ /dev/null @@ -1,30 +0,0 @@ -/****************************************************************************** - * Copyright (C) 2017 Elvis Angelaccio - * - * 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 "util.h" - -#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0) -Q_LOGGING_CATEGORY(MAIN, "libqmatrixclient.main", QtInfoMsg) -Q_LOGGING_CATEGORY(EVENTS, "libqmatrixclient.events", QtInfoMsg) -Q_LOGGING_CATEGORY(JOBS, "libqmatrixclient.jobs", QtInfoMsg) -#else -Q_LOGGING_CATEGORY(MAIN, "libqmatrixclient.main") -Q_LOGGING_CATEGORY(EVENTS, "libqmatrixclient.events") -Q_LOGGING_CATEGORY(JOBS, "libqmatrixclient.jobs") -#endif - diff --git a/events/event.cpp b/events/event.cpp index b3f75ca9..07649b02 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -20,10 +20,7 @@ #include #include -#include -#include -#include "util.h" #include "roommessageevent.h" #include "roomnameevent.h" #include "roomaliasesevent.h" @@ -33,6 +30,8 @@ #include "typingevent.h" #include "receiptevent.h" #include "unknownevent.h" +#include "logging.h" +#include "util.h" using namespace QMatrixClient; diff --git a/events/receiptevent.cpp b/events/receiptevent.cpp index 5d11a0dd..c163424f 100644 --- a/events/receiptevent.cpp +++ b/events/receiptevent.cpp @@ -34,10 +34,10 @@ Example of a Receipt Event: */ #include "receiptevent.h" -#include "util.h" + +#include "logging.h" #include -#include using namespace QMatrixClient; @@ -73,8 +73,8 @@ ReceiptEvent* ReceiptEvent::fromJson(const QJsonObject& obj) { if (eventIt.key().isEmpty()) { - qCWarning(EVENTS) << "ReceiptEvent has an empty event id, skipping"; - qCDebug(EVENTS) << "ReceiptEvent content follows:\n" << contents; + qCWarning(EPHEMERAL) << "ReceiptEvent has an empty event id, skipping"; + qCDebug(EPHEMERAL) << "ReceiptEvent content follows:\n" << contents; continue; } const QJsonObject reads = eventIt.value().toObject().value("m.read").toObject(); diff --git a/events/roomaliasesevent.cpp b/events/roomaliasesevent.cpp index e0dbdb38..ab414498 100644 --- a/events/roomaliasesevent.cpp +++ b/events/roomaliasesevent.cpp @@ -33,10 +33,10 @@ // } #include "roomaliasesevent.h" -#include "util.h" + +#include "logging.h" #include -#include using namespace QMatrixClient; diff --git a/events/roommemberevent.cpp b/events/roommemberevent.cpp index 0dafd303..51dbbbab 100644 --- a/events/roommemberevent.cpp +++ b/events/roommemberevent.cpp @@ -17,9 +17,8 @@ */ #include "roommemberevent.h" -#include "util.h" -#include +#include "logging.h" using namespace QMatrixClient; diff --git a/events/roommessageevent.cpp b/events/roommessageevent.cpp index fd6de464..677bb79f 100644 --- a/events/roommessageevent.cpp +++ b/events/roommessageevent.cpp @@ -17,10 +17,11 @@ */ #include "roommessageevent.h" + +#include "logging.h" #include "util.h" #include -#include using namespace QMatrixClient; diff --git a/events/typingevent.cpp b/events/typingevent.cpp index 11c3a565..009059af 100644 --- a/events/typingevent.cpp +++ b/events/typingevent.cpp @@ -17,10 +17,10 @@ */ #include "typingevent.h" -#include "util.h" + +#include "logging.h" #include -#include using namespace QMatrixClient; @@ -55,6 +55,6 @@ TypingEvent* TypingEvent::fromJson(const QJsonObject& obj) { e->d->users << user.toString(); } - qCDebug(EVENTS) << "Typing:" << e->d->users; + qCDebug(EPHEMERAL) << "Typing:" << e->d->users; return e; } diff --git a/events/unknownevent.cpp b/events/unknownevent.cpp index b2947bf7..1670ff1d 100644 --- a/events/unknownevent.cpp +++ b/events/unknownevent.cpp @@ -18,10 +18,9 @@ #include "unknownevent.h" -#include -#include +#include "logging.h" -#include "util.h" +#include using namespace QMatrixClient; 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; } diff --git a/logging.cpp b/logging.cpp new file mode 100644 index 00000000..97540dd4 --- /dev/null +++ b/logging.cpp @@ -0,0 +1,26 @@ +/****************************************************************************** + * Copyright (C) 2017 Elvis Angelaccio + * + * 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 "logging.h" + +LOGGING_CATEGORY(MAIN, "libqmatrixclient.main") +LOGGING_CATEGORY(PROFILER, "libqmatrixclient.profiler") +LOGGING_CATEGORY(EVENTS, "libqmatrixclient.events") +LOGGING_CATEGORY(EPHEMERAL, "libqmatrixclient.events.ephemeral") +LOGGING_CATEGORY(JOBS, "libqmatrixclient.jobs") +LOGGING_CATEGORY(SYNCJOB, "libqmatrixclient.jobs.sync") diff --git a/logging.h b/logging.h new file mode 100644 index 00000000..a769568c --- /dev/null +++ b/logging.h @@ -0,0 +1,74 @@ +/****************************************************************************** + * Copyright (C) 2017 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 + +Q_DECLARE_LOGGING_CATEGORY(MAIN) +Q_DECLARE_LOGGING_CATEGORY(PROFILER) +Q_DECLARE_LOGGING_CATEGORY(EVENTS) +Q_DECLARE_LOGGING_CATEGORY(EPHEMERAL) +Q_DECLARE_LOGGING_CATEGORY(JOBS) +Q_DECLARE_LOGGING_CATEGORY(SYNCJOB) + +// Use LOGGING_CATEGORY instead of Q_LOGGING_CATEGORY in the rest of the code +#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0) +#define LOGGING_CATEGORY(Name, Id) Q_LOGGING_CATEGORY((Name), (Id), QtInfoMsg) +#else +#define LOGGING_CATEGORY(Name, Id) Q_LOGGING_CATEGORY((Name), (Id)) +#endif + +namespace QMatrixClient +{ + // QDebug manipulators + + using QDebugManip = QDebug (*)(QDebug); + + /** + * @brief QDebug manipulator to setup the stream for JSON output + * + * Originally made to encapsulate the change in QDebug behavior in Qt 5.4 + * and the respective addition of QDebug::noquote(). + * Together with the operator<<() helper, the proposed usage is + * (similar to std:: I/O manipulators): + * + * @example qCDebug() << formatJson << json_object; // (QJsonObject, etc.) + */ + inline QDebug formatJson(QDebug debug_object) + { +#if QT_VERSION < QT_VERSION_CHECK(5, 4, 0) + return debug_object; +#else + return debug_object.noquote(); +#endif + }; + + /** + * @brief A helper operator to facilitate usage of formatJson (and possibly + * other manipulators) + * + * @param debug_object to output the json to + * @param qdm a QDebug manipulator + * @return a copy of debug_object that has its mode altered by qdm + */ + inline QDebug operator<< (QDebug debug_object, QDebugManip qdm) + { + return qdm(debug_object); + } +} diff --git a/util.h b/util.h index 29e623c9..79f76860 100644 --- a/util.h +++ b/util.h @@ -18,51 +18,8 @@ #pragma once -#include - -Q_DECLARE_LOGGING_CATEGORY(EVENTS) -Q_DECLARE_LOGGING_CATEGORY(JOBS) -Q_DECLARE_LOGGING_CATEGORY(MAIN) - namespace QMatrixClient { - - // QDebug manipulators - - using QDebugManip = QDebug (*)(QDebug); - - /** - * @brief QDebug manipulator to setup the stream for JSON output - * - * Originally made to encapsulate the change in QDebug behavior in Qt 5.4 - * and the respective addition of QDebug::noquote(). - * Together with the operator<<() helper, the proposed usage is - * (similar to std:: I/O manipulators): - * - * @example qCDebug() << formatJson << json_object; // (QJsonObject, etc.) - */ - inline QDebug formatJson(QDebug debug_object) - { -#if QT_VERSION < QT_VERSION_CHECK(5, 4, 0) - return debug_object; -#else - return debug_object.noquote(); -#endif - }; - - /** - * @brief A helper operator to facilitate usage of formatJson (and possibly - * other manipulators) - * - * @param debug_object to output the json to - * @param qdm a QDebug manipulator - * @return a copy of debug_object that has its mode altered by qdm - */ - inline QDebug operator<< (QDebug debug_object, QDebugManip qdm) - { - return qdm(debug_object); - } - /** * @brief A crude wrapper around a container of pointers that owns pointers * to contained objects @@ -77,17 +34,8 @@ namespace QMatrixClient { public: Owning() = default; -#if defined(_MSC_VER) && _MSC_VER < 1900 - // Workaround: Dangerous (auto_ptr style) copy constructor because - // in case of Owning< QVector<> > VS2013 (unnecessarily) instantiates - // QVector<>::toList() which instantiates QList< Owning<> > which - // requires the contained object to have a copy constructor. - Owning(Owning& other) : ContainerT(other.release()) { } - Owning(Owning&& other) : ContainerT(other.release()) { } -#else Owning(Owning&) = delete; Owning(Owning&& other) = default; -#endif Owning& operator=(Owning&& other) { assign(other.release()); @@ -166,6 +114,5 @@ namespace QMatrixClient { return fallback; } - -} +} // namespace QMatrixClient -- 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. --- connection.cpp | 6 +-- jobs/syncjob.cpp | 4 +- jobs/syncjob.h | 2 +- room.cpp | 110 +++++++++++++++++++++++++++++-------------------------- room.h | 4 +- 5 files changed, 67 insertions(+), 59 deletions(-) diff --git a/connection.cpp b/connection.cpp index 0c73e22d..f9e2e7ae 100644 --- a/connection.cpp +++ b/connection.cpp @@ -159,10 +159,10 @@ void Connection::sync(int timeout) callApi(d->data->lastEvent(), filter, timeout); connect( job, &SyncJob::success, [=] () { d->data->setLastEvent(job->nextBatch()); - for( auto& roomData: job->roomData() ) + for( auto&& roomData: job->takeRoomData() ) { - if ( Room* r = provideRoom(roomData.roomId) ) - r->updateData(roomData); + if ( auto* r = provideRoom(roomData.roomId) ) + r->updateData(std::move(roomData)); } d->syncJob = nullptr; emit syncDone(); 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: diff --git a/room.cpp b/room.cpp index af39da52..9f57f3fd 100644 --- a/room.cpp +++ b/room.cpp @@ -23,7 +23,7 @@ #include #include #include // for efficient string concats (operator%) -#include +#include #include "connection.h" #include "state.h" @@ -50,10 +50,10 @@ class Room::Private typedef QMultiHash members_map_t; typedef std::pair rev_iter_pair_t; - Private(Connection* c, const QString& id_) - : q(nullptr), connection(c), id(id_), joinState(JoinState::Join) - , unreadMessages(false), highlightCount(0), notificationCount(0) - , roomMessagesJob(nullptr) + Private(Connection* c, QString id_) + : q(nullptr), connection(c), id(std::move(id_)) + , joinState(JoinState::Join), unreadMessages(false) + , highlightCount(0), notificationCount(0), roomMessagesJob(nullptr) { } Room* q; @@ -90,7 +90,7 @@ class Room::Private void addMember(User* u); bool hasMember(User* u) const; // You can't identify a single user by displayname, only by id - User* member(QString id) const; + User* member(const QString& id) const; void renameMember(User* u, QString oldName); void removeMember(User* u); @@ -118,7 +118,7 @@ class Room::Private */ void dropDuplicateEvents(Events* events) const; - void setLastReadEvent(User* u, QString eventId); + void setLastReadEvent(User* u, const QString& eventId); rev_iter_pair_t promoteReadMarker(User* u, rev_iter_t newMarker); private: @@ -126,7 +126,7 @@ class Room::Private QString roomNameFromMemberNames(const QList& userlist) const; void insertMemberIntoMap(User* u); - void removeMemberFromMap(QString username, User* u); + void removeMemberFromMap(const QString& username, User* u); void insertEvent(Event* e, Timeline::iterator where, TimelineItem::index_t index); @@ -139,8 +139,6 @@ Room::Room(Connection* connection, QString id) // https://marcmutz.wordpress.com/translated-articles/pimp-my-pimpl-%E2%80%94-reloaded/ d->q = this; qCDebug(MAIN) << "New Room:" << id; - - //connection->getMembers(this); // I don't think we need this anymore in r0.0.1 } Room::~Room() @@ -197,7 +195,7 @@ void Room::setJoinState(JoinState state) emit joinStateChanged(oldState, state); } -void Room::Private::setLastReadEvent(User* u, QString eventId) +void Room::Private::setLastReadEvent(User* u, const QString& eventId) { lastReadEventIds.insert(u, eventId); emit q->lastReadEventChanged(u); @@ -236,7 +234,7 @@ Room::Private::promoteReadMarker(User* u, Room::rev_iter_t newMarker) emit q->unreadMessagesChanged(q); } else qCDebug(MAIN) << "Room" << displayname << "still has" - << stillUnreadMessagesCount << "unread message(s)"; + << stillUnreadMessagesCount << "unread message(s)"; } // Return newMarker, rather than eagerMarker, to save markMessagesAsRead() @@ -246,7 +244,7 @@ Room::Private::promoteReadMarker(User* u, Room::rev_iter_t newMarker) void Room::markMessagesAsRead(Room::rev_iter_t upToMarker) { - User* localUser = connection()->user(); + auto localUser = connection()->user(); Private::rev_iter_pair_t markers = d->promoteReadMarker(localUser, upToMarker); if (markers.first != markers.second) qCDebug(MAIN) << "Marked messages as read until" << *readMarker(); @@ -308,7 +306,7 @@ Room::rev_iter_t Room::findInTimeline(TimelineItem::index_t index) const (isValidIndex(index) ? index - minTimelineIndex() + 1 : 0); } -Room::rev_iter_t Room::findInTimeline(QString evtId) const +Room::rev_iter_t Room::findInTimeline(const QString& evtId) const { if (!d->timeline.empty() && d->eventsIndex.contains(evtId)) return findInTimeline(d->eventsIndex.value(evtId)); @@ -351,7 +349,7 @@ int Room::highlightCount() const void Room::resetHighlightCount() { -if( d->highlightCount == 0 ) + if( d->highlightCount == 0 ) return; d->highlightCount = 0; emit highlightCountChanged(this); @@ -374,27 +372,23 @@ QList< User* > Room::users() const void Room::Private::insertMemberIntoMap(User *u) { - QList namesakes = membersMap.values(u->name()); + auto namesakes = membersMap.values(u->name()); membersMap.insert(u->name(), u); // If there is exactly one namesake of the added user, signal member renaming // for that other one because the two should be disambiguated now. if (namesakes.size() == 1) emit q->memberRenamed(namesakes[0]); - - updateDisplayname(); } -void Room::Private::removeMemberFromMap(QString username, User* u) +void Room::Private::removeMemberFromMap(const QString& username, User* u) { membersMap.remove(username, u); // If there was one namesake besides the removed user, signal member renaming // for it because it doesn't need to be disambiguated anymore. // TODO: Think about left users. - QList formerNamesakes = membersMap.values(username); + auto formerNamesakes = membersMap.values(username); if (formerNamesakes.size() == 1) emit q->memberRenamed(formerNamesakes[0]); - - updateDisplayname(); } inline QByteArray makeErrorStr(const Event* e, const char* msg) @@ -415,7 +409,7 @@ void Room::Private::insertEvent(Event* e, Timeline::iterator where, { qCWarning(MAIN) << "Event" << e->id() << "is already in the timeline."; qCWarning(MAIN) << "Either dropDuplicateEvents() wasn't called or duplicate " - "events within the same batch arrived from the server."; + "events within the same batch arrived from the server."; return; } timeline.emplace(where, e, index); @@ -438,9 +432,9 @@ bool Room::Private::hasMember(User* u) const return membersMap.values(u->name()).contains(u); } -User* Room::Private::member(QString id) const +User* Room::Private::member(const QString& id) const { - User* u = connection->user(id); + auto u = connection->user(id); return hasMember(u) ? u : nullptr; } @@ -449,8 +443,8 @@ void Room::Private::renameMember(User* u, QString oldName) if (hasMember(u)) { qCWarning(MAIN) << "Room::Private::renameMember(): the user " - << u->name() - << "is already known in the room under a new name."; + << u->name() + << "is already known in the room under a new name."; return; } @@ -459,8 +453,6 @@ void Room::Private::renameMember(User* u, QString oldName) removeMemberFromMap(oldName, u); insertMemberIntoMap(u); emit q->memberRenamed(u); - - updateDisplayname(); } } @@ -477,7 +469,7 @@ void Room::Private::removeMember(User* u) void Room::userRenamed(User* user, QString oldName) { - d->renameMember(user, oldName); + d->renameMember(user, std::move(oldName)); } QString Room::roomMembername(User *u) const @@ -516,22 +508,33 @@ QString Room::roomMembername(const QString& userId) const return roomMembername(connection()->user(userId)); } -void Room::updateData(SyncRoomData& data) +void Room::updateData(SyncRoomData&& data) { if( d->prevBatch.isEmpty() ) d->prevBatch = data.timelinePrevBatch; setJoinState(data.joinState); + QElapsedTimer et; et.start(); + processStateEvents(data.state); + qCDebug(PROFILER) << "*** Room::processStateEvents(state):" + << et.elapsed() << "ms," << data.state.size() << "events"; + et.restart(); // State changes can arrive in a timeline event; so check those. processStateEvents(data.timeline); + qCDebug(PROFILER) << "*** Room::processStateEvents(timeline):" + << et.elapsed() << "ms," << data.timeline.size() << "events"; + et.restart(); addNewMessageEvents(data.timeline.release()); + qCDebug(PROFILER) << "*** Room::addNewMessageEvents():" << et.elapsed() << "ms"; + et.restart(); for( Event* ephemeralEvent: data.ephemeral ) { processEphemeralEvent(ephemeralEvent); } + qCDebug(PROFILER) << "*** Room::processEphemeralEvents():" << et.elapsed() << "ms"; if( data.highlightCount != d->highlightCount ) { @@ -620,8 +623,8 @@ void Room::doAddNewMessageEvents(const Events& events) newUnreadMessages += d->isEventNotable(e); } qCDebug(MAIN) << "Room" << displayName() << "received" << events.size() - << "(with" << newUnreadMessages << "notable)" - << "new events; the last event is now" << d->timeline.back(); + << "(with" << newUnreadMessages << "notable)" + << "new events; the last event is now" << d->timeline.back(); // The first event in the batch defines whose read marker can possibly be // promoted any further over the same author's events newly arrived. @@ -633,7 +636,7 @@ void Room::doAddNewMessageEvents(const Events& events) { d->promoteReadMarker(firstWriter, findInTimeline(events.front()->id())); qCDebug(MAIN) << "Auto-promoted read marker for" << firstWriter->id() - << "to" << *readMarker(firstWriter); + << "to" << *readMarker(firstWriter); } if( !d->unreadMessages && newUnreadMessages > 0) @@ -661,46 +664,44 @@ void Room::doAddHistoricalMessageEvents(const Events& events) for (auto e: events) d->prependEvent(e); qCDebug(MAIN) << "Room" << displayName() << "received" << events.size() - << "past events; the oldest event is now" << d->timeline.front(); + << "past events; the oldest event is now" << d->timeline.front(); } void Room::processStateEvents(const Events& events) { + bool emitNamesChanged = false; for (auto event: events) { if( event->type() == EventType::RoomName ) { - RoomNameEvent* nameEvent = static_cast(event); + auto nameEvent = static_cast(event); d->name = nameEvent->name(); qCDebug(MAIN) << "room name:" << d->name; - d->updateDisplayname(); - emit namesChanged(this); + emitNamesChanged = true; } if( event->type() == EventType::RoomAliases ) { - RoomAliasesEvent* aliasesEvent = static_cast(event); + auto aliasesEvent = static_cast(event); d->aliases = aliasesEvent->aliases(); qCDebug(MAIN) << "room aliases:" << d->aliases; - // No displayname update - aliases are not used to render a displayname - emit namesChanged(this); + emitNamesChanged = true; } if( event->type() == EventType::RoomCanonicalAlias ) { - RoomCanonicalAliasEvent* aliasEvent = static_cast(event); + auto aliasEvent = static_cast(event); d->canonicalAlias = aliasEvent->alias(); qCDebug(MAIN) << "room canonical alias:" << d->canonicalAlias; - d->updateDisplayname(); - emit namesChanged(this); + emitNamesChanged = true; } if( event->type() == EventType::RoomTopic ) { - RoomTopicEvent* topicEvent = static_cast(event); + auto topicEvent = static_cast(event); d->topic = topicEvent->topic(); emit topicChanged(); } if( event->type() == EventType::RoomMember ) { - RoomMemberEvent* memberEvent = static_cast(event); + auto memberEvent = static_cast(event); // Can't use d->member() below because the user may be not a member (yet) User* u = d->connection->user(memberEvent->userId()); u->processEvent(event); @@ -714,13 +715,17 @@ void Room::processStateEvents(const Events& events) } } } + if (emitNamesChanged) { + emit namesChanged(this); + } + d->updateDisplayname(); } void Room::processEphemeralEvent(Event* event) { if( event->type() == EventType::Typing ) { - TypingEvent* typingEvent = static_cast(event); + auto typingEvent = static_cast(event); d->usersTyping.clear(); for( const QString& userId: typingEvent->users() ) { @@ -738,9 +743,12 @@ void Room::processEphemeralEvent(Event* event) const auto& receipts = eventReceiptPair.second; { if (receipts.size() == 1) - qCDebug(MAIN) << "Marking event" << eventId << "as read for" << receipts[0].userId; + qCDebug(EPHEMERAL) << "Marking" << eventId + << "as read for" << receipts[0].userId; else - qCDebug(MAIN) << "Marking event" << eventId << "as read for" << receipts.size() << "users"; + qCDebug(EPHEMERAL) << "Marking" << eventId + << "as read for" + << receipts.size() << "users"; } if (d->eventsIndex.contains(eventId)) { @@ -750,8 +758,8 @@ void Room::processEphemeralEvent(Event* event) d->promoteReadMarker(m, newMarker); } else { - qCDebug(MAIN) << "Event" << eventId - << "not found; saving read markers anyway"; + qCDebug(EPHEMERAL) << "Event" << eventId + << "not found; saving read receipts anyway"; // If the event is not found (most likely, because it's too old // and hasn't been fetched from the server yet), but there is // a previous marker for a user, keep the previous marker. diff --git a/room.h b/room.h index 689a99b0..60be33b9 100644 --- a/room.h +++ b/room.h @@ -102,7 +102,7 @@ namespace QMatrixClient */ Q_INVOKABLE QString roomMembername(const QString& userId) const; - Q_INVOKABLE void updateData(SyncRoomData& data ); + void updateData(SyncRoomData&& data ); Q_INVOKABLE void setJoinState( JoinState state ); const Timeline& messageEvents() const; @@ -116,7 +116,7 @@ namespace QMatrixClient Q_INVOKABLE bool isValidIndex(TimelineItem::index_t timelineIndex) const; rev_iter_t findInTimeline(TimelineItem::index_t index) const; - rev_iter_t findInTimeline(QString evtId) const; + rev_iter_t findInTimeline(const QString& evtId) const; rev_iter_t readMarker(const User* user) const; rev_iter_t readMarker() const; -- cgit v1.2.3