From 818fbda8c43fa76ede72db2e941ec81fe093cc59 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 2 Feb 2018 19:37:53 +0900 Subject: Add CII Best Practices badge to the flair section --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 370380c5..fc9ad5e0 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ [![license](https://img.shields.io/github/license/QMatrixClient/libqmatrixclient.svg)](https://github.com/QMatrixClient/libqmatrixclient/blob/master/COPYING) ![status](https://img.shields.io/badge/status-beta-yellow.svg) [![release](https://img.shields.io/github/release/QMatrixClient/libqmatrixclient/all.svg)](https://github.com/QMatrixClient/libqmatrixclient/releases/latest) +[![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/1023/badge)](https://bestpractices.coreinfrastructure.org/projects/1023) [![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg?style=flat-square)](http://makeapullrequest.com) libqmatrixclient is a Qt5-based library to make IM clients for the [Matrix](https://matrix.org) protocol. It is the backbone of [Quaternion](https://github.com/QMatrixClient/Quaternion), [Tensor](https://matrix.org/docs/projects/client/tensor.html) and some other projects. -- cgit v1.2.3 From 6a9b1876bf8ebeca398c9c57ef90e01c25a7ada6 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 4 Feb 2018 23:12:42 +0900 Subject: MediaThumbnailJob::parseReply: Do not ignore underlying errors --- jobs/mediathumbnailjob.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp index 261978ec..dda1cdb4 100644 --- a/jobs/mediathumbnailjob.cpp +++ b/jobs/mediathumbnailjob.cpp @@ -52,10 +52,12 @@ QImage MediaThumbnailJob::scaledThumbnail(QSize toSize) const BaseJob::Status MediaThumbnailJob::parseReply(QNetworkReply* reply) { - GetContentThumbnailJob::parseReply(reply); - if( !_thumbnail.loadFromData(content()->readAll()) ) - { - qCDebug(JOBS) << "MediaThumbnailJob: could not read image data"; - } - return Success; + auto result = GetContentThumbnailJob::parseReply(reply); + if (!result.good()) + return result; + + if( _thumbnail.loadFromData(content()->readAll()) ) + return Success; + + return { IncorrectResponseError, "Could not read image data" }; } -- cgit v1.2.3 From b7c1ff183384738f170d53128c684681cb34f3b7 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 7 Feb 2018 15:10:19 +0900 Subject: RoomEvent/RoomMemberEvent: do not store what can be calculated on the fly Basically, segments of originalJsonObject() are used as-you-go instead of parsing them upon event creation. --- events/event.cpp | 27 ++++++++++++++++++++++----- events/event.h | 14 +++++++------- events/roommemberevent.h | 7 ++++--- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/events/event.cpp b/events/event.cpp index c7345a13..7bc25a07 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -88,11 +88,12 @@ EventPtr _impl::doMakeEvent(const QJsonObject& obj) RoomEvent::RoomEvent(Event::Type type) : Event(type) { } RoomEvent::RoomEvent(Type type, const QJsonObject& rep) - : Event(type, rep), _id(rep["event_id"].toString()) - , _roomId(rep["room_id"].toString()) - , _senderId(rep["sender"].toString()) - , _serverTimestamp( - QMatrixClient::fromJson(rep["origin_server_ts"])) + : Event(type, rep) + , _id(rep["event_id"].toString()) +// , _roomId(rep["room_id"].toString()) +// , _senderId(rep["sender"].toString()) +// , _serverTimestamp( +// QMatrixClient::fromJson(rep["origin_server_ts"])) { // if (_id.isEmpty()) // { @@ -124,6 +125,22 @@ RoomEvent::RoomEvent(Type type, const QJsonObject& rep) RoomEvent::~RoomEvent() = default; // Let the smart pointer do its job +QDateTime RoomEvent::timestamp() const +{ + return QMatrixClient::fromJson( + originalJsonObject().value("origin_server_ts")); +} + +QString RoomEvent::roomId() const +{ + return originalJsonObject().value("room_id").toString(); +} + +QString RoomEvent::senderId() const +{ + return originalJsonObject().value("sender_id").toString(); +} + QString RoomEvent::redactionReason() const { return isRedacted() ? _redactedBecause->reason() : QString{}; diff --git a/events/event.h b/events/event.h index b5a4d94e..968bc1ad 100644 --- a/events/event.h +++ b/events/event.h @@ -167,10 +167,10 @@ namespace QMatrixClient RoomEvent(Type type, const QJsonObject& rep); ~RoomEvent(); - const QString& id() const { return _id; } - const QDateTime& timestamp() const { return _serverTimestamp; } - const QString& roomId() const { return _roomId; } - const QString& senderId() const { return _senderId; } + QString id() const { return _id; } + QDateTime timestamp() const; + QString roomId() const; + QString senderId() const; bool isRedacted() const { return bool(_redactedBecause); } const RedactionEvent* redactedBecause() const { @@ -202,9 +202,9 @@ namespace QMatrixClient private: QString _id; - QString _roomId; - QString _senderId; - QDateTime _serverTimestamp; +// QString _roomId; +// QString _senderId; +// QDateTime _serverTimestamp; event_ptr_tt _redactedBecause; QString _txnId; }; diff --git a/events/roommemberevent.h b/events/roommemberevent.h index d0c63f15..224f29c2 100644 --- a/events/roommemberevent.h +++ b/events/roommemberevent.h @@ -53,16 +53,17 @@ namespace QMatrixClient explicit RoomMemberEvent(const QJsonObject& obj) : StateEvent(Type::RoomMember, obj) - , _userId(obj["state_key"].toString()) +// , _userId(obj["state_key"].toString()) { } MembershipType membership() const { return content().membership; } - QString userId() const { return _userId; } + QString userId() const + { return originalJsonObject().value("state_key").toString(); } QString displayName() const { return content().displayName; } QUrl avatarUrl() const { return content().avatarUrl; } private: - QString _userId; +// QString _userId; REGISTER_ENUM(MembershipType) }; } // namespace QMatrixClient -- cgit v1.2.3 From 3bf51eea5e6152cd39daa971ac6f88d0571ce198 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 19 Feb 2018 18:51:08 +0900 Subject: Fix a typo in the previous commit A symptom that you suffered from it is you don't see any usernames whatsoever, and after restarting room names are also gone (to fix that, delete cache files). --- events/event.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/events/event.cpp b/events/event.cpp index 7bc25a07..366aa858 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -138,7 +138,7 @@ QString RoomEvent::roomId() const QString RoomEvent::senderId() const { - return originalJsonObject().value("sender_id").toString(); + return originalJsonObject().value("sender").toString(); } QString RoomEvent::redactionReason() const -- cgit v1.2.3 From d54bdd710163afe9e09b2c4c65bbf21454ed2ceb Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 20 Feb 2018 10:38:37 +0900 Subject: BaseJob: added Abandoned status For a very brief period between calling BaseJob::abandon() and deletion of the job object. --- jobs/basejob.cpp | 1 + jobs/basejob.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index c35a7711..5f5aa410 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -448,6 +448,7 @@ void BaseJob::setStatus(int code, QString message) void BaseJob::abandon() { beforeAbandon(d->reply.data()); + setStatus(Abandoned); this->disconnect(); if (d->reply) d->reply->disconnect(this); diff --git a/jobs/basejob.h b/jobs/basejob.h index c03c914f..a5b457c5 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -53,7 +53,8 @@ namespace QMatrixClient enum StatusCode { NoError = 0 // To be compatible with Qt conventions , Success = 0 , Pending = 1 - , ErrorLevel = 100 // Errors have codes starting from this + , Abandoned = 50 //< A very brief period between abandoning and object deletion + , ErrorLevel = 100 //< Errors have codes starting from this , NetworkError = 100 , JsonParseError , TimeoutError -- cgit v1.2.3 From 8fbbd6063a2d313c630d14842bbce4de8c1e7851 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 10:42:44 +0900 Subject: BaseJob: In case of 4xx errors, fill the status with the message from the response --- jobs/basejob.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 5f5aa410..c3b110f0 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -267,6 +267,12 @@ void BaseJob::gotReply() setStatus(checkReply(d->reply.data())); if (status().good()) setStatus(parseReply(d->reply.data())); + else + { + auto json = QJsonDocument::fromJson(d->reply->readAll()).object(); + if (!json.isEmpty()) + setStatus(IncorrectRequestError, json.value("error").toString()); + } finishJob(); } -- cgit v1.2.3 From 76b1d775edae36dd2f36fdd4886c6c956bf6b49b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 10:50:03 +0900 Subject: RoomMemberEvent: make it sendable To do that, both RoomMemberEvent and MemberEventContent got respective constructors. Also: the fallback value for unknown _received_ membership types is now Undefined; it's not allowed in member events for sending (will fail on assertion now) because the server requires membership to be explicitly set. --- events/roommemberevent.cpp | 10 +++++++--- events/roommemberevent.h | 11 +++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/events/roommemberevent.cpp b/events/roommemberevent.cpp index 76df5f2e..a9e301a4 100644 --- a/events/roommemberevent.cpp +++ b/events/roommemberevent.cpp @@ -44,7 +44,7 @@ namespace QMatrixClient return MembershipType(it - membershipStrings.begin()); qCWarning(EVENTS) << "Unknown MembershipType: " << membershipString; - return MembershipType::Join; + return MembershipType::Undefined; } }; } @@ -58,7 +58,11 @@ MemberEventContent::MemberEventContent(const QJsonObject& json) void MemberEventContent::fillJson(QJsonObject* o) const { Q_ASSERT(o); - o->insert("membership", membershipStrings[membership]); + Q_ASSERT_X(membership != MembershipType::Undefined, __FUNCTION__, + "The key 'membership' must be explicit in MemberEventContent"); + if (membership != MembershipType::Undefined) + o->insert("membership", membershipStrings[membership]); o->insert("displayname", displayName); - o->insert("avatar_url", avatarUrl.toString()); + if (avatarUrl.isValid()) + o->insert("avatar_url", avatarUrl.toString()); } diff --git a/events/roommemberevent.h b/events/roommemberevent.h index 224f29c2..b9ff0d70 100644 --- a/events/roommemberevent.h +++ b/events/roommemberevent.h @@ -29,9 +29,13 @@ namespace QMatrixClient class MemberEventContent: public EventContent::Base { public: - enum MembershipType : size_t {Invite = 0, Join, Knock, Leave, Ban}; + enum MembershipType : size_t { Invite = 0, Join, Knock, Leave, Ban, + Undefined }; - MemberEventContent(const QJsonObject& json); + explicit MemberEventContent(MembershipType mt = MembershipType::Join) + : membership(mt) + { } + explicit MemberEventContent(const QJsonObject& json); MembershipType membership; QString displayName; @@ -51,6 +55,9 @@ namespace QMatrixClient using MembershipType = MemberEventContent::MembershipType; + RoomMemberEvent(MemberEventContent&& c) + : StateEvent(Type::RoomMember, c) + { } explicit RoomMemberEvent(const QJsonObject& obj) : StateEvent(Type::RoomMember, obj) // , _userId(obj["state_key"].toString()) -- cgit v1.2.3 From 9474a9afeb7ff63538ee85b4b59e172e5d32db32 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 11:16:10 +0900 Subject: Avatar: provide common logic for uploading; don't store Connection Although the setting part of the work is class(User or Room)-specific, the uploading part is common, so Avatar provides it now. Also: there's no need to store Connection, as it's only used in get() and upload() - just pass it as the parameter to the methods. --- avatar.cpp | 84 +++++++++++++++++++++++++++++++++++++++++++++++--------------- avatar.h | 22 ++++++++++++---- room.cpp | 8 +++--- user.cpp | 40 ++++++++++++++---------------- user.h | 3 ++- 5 files changed, 105 insertions(+), 52 deletions(-) diff --git a/avatar.cpp b/avatar.cpp index 040bf9bb..9664199c 100644 --- a/avatar.cpp +++ b/avatar.cpp @@ -30,10 +30,13 @@ using namespace QMatrixClient; class Avatar::Private { public: - Private(Connection* c, QIcon di) : _connection(c), _defaultIcon(di) { } - QImage get(QSize size, Avatar::notifier_t notifier) const; + explicit Private(QIcon di, QUrl url = {}) + : _defaultIcon(di), _url(url) + { } + QImage get(Connection* connection, QSize size, + get_callback_t callback) const; + bool upload(UploadContentJob* job, upload_callback_t callback); - Connection* _connection; const QIcon _defaultIcon; QUrl _url; @@ -42,24 +45,52 @@ class Avatar::Private mutable std::vector> _scaledImages; mutable QSize _requestedSize; mutable bool _valid = false; - mutable QPointer _ongoingRequest = nullptr; - mutable std::vector notifiers; + mutable QPointer _thumbnailRequest = nullptr; + mutable QPointer _uploadRequest = nullptr; + mutable std::vector callbacks; + mutable get_callback_t uploadCallback; }; -Avatar::Avatar(Connection* connection, QIcon defaultIcon) - : d(new Private { connection, std::move(defaultIcon) }) +Avatar::Avatar(QIcon defaultIcon) + : d(std::make_unique(std::move(defaultIcon))) { } +Avatar::Avatar(QUrl url, QIcon defaultIcon) + : d(std::make_unique(std::move(defaultIcon), std::move(url))) +{ } + +Avatar::Avatar(Avatar&&) = default; + Avatar::~Avatar() = default; -QImage Avatar::get(int dimension, notifier_t notifier) const +Avatar& Avatar::operator=(Avatar&&) = default; + +QImage Avatar::get(Connection* connection, int dimension, + get_callback_t callback) const { - return d->get({dimension, dimension}, notifier); + return d->get(connection, {dimension, dimension}, callback); } -QImage Avatar::get(int width, int height, notifier_t notifier) const +QImage Avatar::get(Connection* connection, int width, int height, + get_callback_t callback) const { - return d->get({width, height}, notifier); + return d->get(connection, {width, height}, callback); +} + +bool Avatar::upload(Connection* connection, const QString& fileName, + upload_callback_t callback) const +{ + if (isJobRunning(d->_uploadRequest)) + return false; + return d->upload(connection->uploadFile(fileName), callback); +} + +bool Avatar::upload(Connection* connection, QIODevice* source, + upload_callback_t callback) const +{ + if (isJobRunning(d->_uploadRequest) || !source->isReadable()) + return false; + return d->upload(connection->uploadContent(source), callback); } QString Avatar::mediaId() const @@ -67,28 +98,29 @@ QString Avatar::mediaId() const return d->_url.authority() + d->_url.path(); } -QImage Avatar::Private::get(QSize size, Avatar::notifier_t notifier) const +QImage Avatar::Private::get(Connection* connection, QSize size, + get_callback_t callback) const { // FIXME: Alternating between longer-width and longer-height requests // is a sure way to trick the below code into constantly getting another // image from the server because the existing one is alleged unsatisfactory. // This is plain abuse by the client, though; so not critical for now. - if( ( !(_valid || _ongoingRequest) + if( ( !(_valid || _thumbnailRequest) || size.width() > _requestedSize.width() || size.height() > _requestedSize.height() ) && _url.isValid() ) { qCDebug(MAIN) << "Getting avatar from" << _url.toString(); _requestedSize = size; - if (isJobRunning(_ongoingRequest)) - _ongoingRequest->abandon(); - notifiers.emplace_back(std::move(notifier)); - _ongoingRequest = _connection->getThumbnail(_url, size); - QObject::connect( _ongoingRequest, &MediaThumbnailJob::success, [this] + if (isJobRunning(_thumbnailRequest)) + _thumbnailRequest->abandon(); + callbacks.emplace_back(std::move(callback)); + _thumbnailRequest = connection->getThumbnail(_url, size); + QObject::connect( _thumbnailRequest, &MediaThumbnailJob::success, [this] { _valid = true; - _originalImage = _ongoingRequest->scaledThumbnail(_requestedSize); + _originalImage = _thumbnailRequest->scaledThumbnail(_requestedSize); _scaledImages.clear(); - for (auto n: notifiers) + for (auto n: callbacks) n(); }); } @@ -111,6 +143,16 @@ QImage Avatar::Private::get(QSize size, Avatar::notifier_t notifier) const return result; } +bool Avatar::Private::upload(UploadContentJob* job, upload_callback_t callback) +{ + _uploadRequest = job; + if (!isJobRunning(_uploadRequest)) + return false; + _uploadRequest->connect(_uploadRequest, &BaseJob::success, + [job,callback] { callback(job->contentUri()); }); + return true; +} + QUrl Avatar::url() const { return d->_url; } bool Avatar::updateUrl(const QUrl& newUrl) @@ -127,6 +169,8 @@ bool Avatar::updateUrl(const QUrl& newUrl) } d->_url = newUrl; d->_valid = false; + if (isJobRunning(d->_thumbnailRequest)) + d->_thumbnailRequest->abandon(); return true; } diff --git a/avatar.h b/avatar.h index 4d476ea5..ecd5bc52 100644 --- a/avatar.h +++ b/avatar.h @@ -22,6 +22,7 @@ #include #include +#include namespace QMatrixClient { @@ -30,13 +31,24 @@ namespace QMatrixClient class Avatar { public: - explicit Avatar(Connection* connection, QIcon defaultIcon = {}); + explicit Avatar(QIcon defaultIcon = {}); + Avatar(QUrl url, QIcon defaultIcon = {}); + Avatar(Avatar&&); ~Avatar(); + Avatar& operator=(Avatar&&); - using notifier_t = std::function; + using get_callback_t = std::function; + using upload_callback_t = std::function; - QImage get(int dimension, notifier_t notifier) const; - QImage get(int w, int h, notifier_t notifier) const; + QImage get(Connection* connection, int dimension, + get_callback_t callback) const; + QImage get(Connection* connection, int w, int h, + get_callback_t callback) const; + + bool upload(Connection* connection, const QString& fileName, + upload_callback_t callback) const; + bool upload(Connection* connection, QIODevice* source, + upload_callback_t callback) const; QString mediaId() const; QUrl url() const; @@ -44,6 +56,6 @@ namespace QMatrixClient private: class Private; - QScopedPointer d; + std::unique_ptr d; }; } // namespace QMatrixClient diff --git a/room.cpp b/room.cpp index 73591c98..51c5f5cc 100644 --- a/room.cpp +++ b/room.cpp @@ -64,7 +64,7 @@ class Room::Private Private(Connection* c, QString id_, JoinState initialJoinState) : q(nullptr), connection(c), id(std::move(id_)) - , avatar(c), joinState(initialJoinState) + , joinState(initialJoinState) { } Room* q; @@ -287,7 +287,7 @@ QImage Room::avatar(int dimension) QImage Room::avatar(int width, int height) { if (!d->avatar.url().isEmpty()) - return d->avatar.get(width, height, [=] { emit avatarChanged(); }); + return d->avatar.get(connection(), width, height, [=] { emit avatarChanged(); }); // Use the other side's avatar for 1:1's if (d->membersMap.size() == 2) @@ -295,8 +295,8 @@ QImage Room::avatar(int width, int height) auto theOtherOneIt = d->membersMap.begin(); if (theOtherOneIt.value() == localUser()) ++theOtherOneIt; - return (*theOtherOneIt)->avatarObject() - .get(width, height, [=] { emit avatarChanged(); }); + return (*theOtherOneIt)->avatar(width, height, + [=] { emit avatarChanged(); }); } return {}; } diff --git a/user.cpp b/user.cpp index c80ec883..289f0bac 100644 --- a/user.cpp +++ b/user.cpp @@ -39,17 +39,15 @@ class User::Private public: Private(QString userId, Connection* connection) : userId(std::move(userId)), connection(connection) - , avatar(connection, QIcon::fromTheme(QStringLiteral("user-available"))) { } QString userId; QString name; QString bridged; Connection* connection; - Avatar avatar; - QPointer avatarUploadJob = nullptr; + Avatar avatar { QIcon::fromTheme(QStringLiteral("user-available")) }; - void setAvatar(UploadContentJob* job, User* q); + void setAvatar(QString contentUri, User* q); }; User::User(QString userId, Connection* connection) @@ -107,28 +105,20 @@ void User::rename(const QString& newName) bool User::setAvatar(const QString& fileName) { - if (isJobRunning(d->avatarUploadJob)) - return false; - d->setAvatar(d->connection->uploadFile(fileName), this); - return true; + return avatarObject().upload(d->connection, fileName, + [this] (QString contentUri) { d->setAvatar(contentUri, this); }); } bool User::setAvatar(QIODevice* source) { - if (isJobRunning(d->avatarUploadJob) || !source->isReadable()) - return false; - d->setAvatar(d->connection->uploadContent(source), this); - return true; + return avatarObject().upload(d->connection, source, + [this] (QString contentUri) { d->setAvatar(contentUri, this); }); } -void User::Private::setAvatar(UploadContentJob* job, User* q) +void User::Private::setAvatar(QString contentUri, User* q) { - avatarUploadJob = job; - connect(job, &BaseJob::success, q, [this,q] { - auto* j = connection->callApi( - userId, avatarUploadJob->contentUri()); - connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); - }); + auto* j = connection->callApi(userId, contentUri); + connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); } QString User::displayname() const @@ -159,17 +149,23 @@ QImage User::avatar(int dimension) QImage User::avatar(int width, int height) { - return d->avatar.get(width, height, [=] { emit avatarChanged(this); }); + return avatar(width, height, []{}); +} + +QImage User::avatar(int width, int height, Avatar::get_callback_t callback) +{ + return avatarObject().get(d->connection, width, height, + [this,callback] { emit avatarChanged(this); callback(); }); } QString User::avatarMediaId() const { - return d->avatar.mediaId(); + return avatarObject().mediaId(); } QUrl User::avatarUrl() const { - return d->avatar.url(); + return avatarObject().url(); } void User::processEvent(Event* event) diff --git a/user.h b/user.h index 37977e08..fa85d778 100644 --- a/user.h +++ b/user.h @@ -84,7 +84,8 @@ namespace QMatrixClient const Avatar& avatarObject() const; Q_INVOKABLE QImage avatar(int dimension); - Q_INVOKABLE QImage avatar(int requestedWidth, int requestedHeight); + Q_INVOKABLE QImage avatar(int width, int height); + QImage avatar(int width, int height, Avatar::get_callback_t callback); QString avatarMediaId() const; QUrl avatarUrl() const; -- cgit v1.2.3 From b243fc6495e06fa4d41562aa20028dfdb3efd28e Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 11:16:10 +0900 Subject: Room: user() and memberJoinState(); code cleanup Room::user() came instead of Room::Private::member() and memberJoinState() came instead of Private::hasMember. --- room.cpp | 169 +++++++++++++++++++++++++++++---------------------------------- room.h | 22 +++++++++ 2 files changed, 100 insertions(+), 91 deletions(-) diff --git a/room.cpp b/room.cpp index 51c5f5cc..3c6fb223 100644 --- a/room.cpp +++ b/room.cpp @@ -140,16 +140,10 @@ class Room::Private const RoomMessageEvent* getEventWithFile(const QString& eventId) const; - // Convenience methods to work with the membersMap and usersLeft. - // addMember() and removeMember() emit respective Room:: signals - // after a succesful operation. //void inviteUser(User* u); // We might get it at some point in time. - void addMember(User* u); - bool hasMember(User* u) const; - // You can't identify a single user by displayname, only by id - User* member(const QString& id) const; + void insertMemberIntoMap(User* u); void renameMember(User* u, QString oldName); - void removeMember(User* u); + void removeMemberFromMap(const QString& username, User* u); void getPreviousContent(int limit = 10); @@ -202,12 +196,9 @@ class Room::Private QString calculateDisplayname() const; QString roomNameFromMemberNames(const QList& userlist) const; - void insertMemberIntoMap(User* u); - void removeMemberFromMap(const QString& username, User* u); - bool isLocalUser(const User* u) const { - return u == connection->user(); + return u == q->localUser(); } }; @@ -301,6 +292,18 @@ QImage Room::avatar(int width, int height) return {}; } +User* Room::user(const QString& userId) const +{ + return connection()->user(userId); +} + +JoinState Room::memberJoinState(User* user) const +{ + return + d->membersMap.contains(user->name(), user) ? JoinState::Join : + JoinState::Leave; +} + JoinState Room::joinState() const { return d->joinState; @@ -700,7 +703,7 @@ QStringList Room::memberNames() const { QStringList res; for (auto u : d->membersMap) - res.append( this->roomMembername(u) ); + res.append( roomMembername(u) ); return res; } @@ -717,12 +720,31 @@ int Room::timelineSize() const void Room::Private::insertMemberIntoMap(User *u) { - auto namesakes = membersMap.values(u->name()); - membersMap.insert(u->name(), u); + const auto userName = u->name(); + auto namesakes = membersMap.values(userName); + membersMap.insert(userName, 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]); + emit q->memberRenamed(namesakes.front()); +} + +void Room::Private::renameMember(User* u, QString oldName) +{ + if (q->memberJoinState(u) == JoinState::Join) + { + qCWarning(MAIN) << "Room::Private::renameMember(): the user " + << u->fullName() + << "is already known in the room under a new name."; + return; + } + + if (membersMap.contains(oldName, u)) + { + removeMemberFromMap(oldName, u); + insertMemberIntoMap(u); + emit q->memberRenamed(u); + } } void Room::Private::removeMemberFromMap(const QString& username, User* u) @@ -731,9 +753,8 @@ void Room::Private::removeMemberFromMap(const QString& username, User* 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. - auto formerNamesakes = membersMap.values(username); - if (formerNamesakes.size() == 1) - emit q->memberRenamed(formerNamesakes[0]); + if (membersMap.count(username) == 1) + emit q->memberRenamed(membersMap.value(username)); } inline auto makeErrorStr(const Event& e, QByteArray msg) @@ -772,69 +793,17 @@ Room::Timeline::size_type Room::Private::insertEvents(RoomEventsRange&& events, return events.size(); } -void Room::Private::addMember(User *u) -{ - if (!hasMember(u)) - { - insertMemberIntoMap(u); - connect(u, &User::nameChanged, q, - std::bind(&Private::renameMember, this, u, _2)); - emit q->userAdded(u); - } -} - -bool Room::Private::hasMember(User* u) const -{ - return membersMap.values(u->name()).contains(u); -} - -User* Room::Private::member(const QString& id) const -{ - auto u = connection->user(id); - return hasMember(u) ? u : nullptr; -} - -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."; - return; - } - - if (membersMap.values(oldName).contains(u)) - { - removeMemberFromMap(oldName, u); - insertMemberIntoMap(u); - emit q->memberRenamed(u); - } -} - -void Room::Private::removeMember(User* u) -{ - if (hasMember(u)) - { - if ( !membersLeft.contains(u) ) - membersLeft.append(u); - removeMemberFromMap(u->name(), u); - emit q->userRemoved(u); - } -} - QString Room::roomMembername(const User* u) const { // See the CS spec, section 11.2.2.3 - QString username = u->name(); + const auto username = u->name(); if (username.isEmpty()) return u->id(); // Get the list of users with the same display name. Most likely, // there'll be one, but there's a chance there are more. - auto namesakes = d->membersMap.values(username); - if (namesakes.size() == 1) + if (d->membersMap.count(username) == 1) return username; // We expect a user to be a member of the room - but technically it is @@ -856,7 +825,7 @@ QString Room::roomMembername(const User* u) const QString Room::roomMembername(const QString& userId) const { - return roomMembername(connection()->user(userId)); + return roomMembername(user(userId)); } void Room::updateData(SyncRoomData&& data) @@ -1250,7 +1219,7 @@ void Room::Private::checkUnreadMessages(timeline_iter_t from) // read receipts from the server (or, for the local user, // markMessagesAsRead() invocation) to promote their read markers over // the new message events. - auto firstWriter = connection->user((*from)->senderId()); + auto firstWriter = q->user((*from)->senderId()); if (q->readMarker(firstWriter) != timeline.crend()) { promoteReadMarker(firstWriter, q->findInTimeline((*from)->id())); @@ -1349,16 +1318,27 @@ void Room::processStateEvents(const RoomEvents& events) } case EventType::RoomMember: { auto memberEvent = static_cast(event); - // Can't use d->member() below because the user may be not a member (yet) - auto u = d->connection->user(memberEvent->userId()); - u->processEvent(event); + auto u = user(memberEvent->userId()); + u->processEvent(memberEvent); if( memberEvent->membership() == MembershipType::Join ) { - d->addMember(u); + if (memberJoinState(u) != JoinState::Join) + { + d->insertMemberIntoMap(u); + connect(u, &User::nameChanged, this, + std::bind(&Private::renameMember, d, u, _2)); + emit userAdded(u); + } } else if( memberEvent->membership() == MembershipType::Leave ) { - d->removeMember(u); + if (memberJoinState(u) == JoinState::Join) + { + if (!d->membersLeft.contains(u)) + d->membersLeft.append(u); + d->removeMemberFromMap(u->name(), u); + emit userRemoved(u); + } } break; } @@ -1380,8 +1360,9 @@ void Room::processEphemeralEvent(EventPtr event) d->usersTyping.clear(); for( const QString& userId: typingEvent->users() ) { - if (auto m = d->member(userId)) - d->usersTyping.append(m); + auto u = user(userId); + if (memberJoinState(u) == JoinState::Join) + d->usersTyping.append(u); } emit typingChanged(); break; @@ -1403,8 +1384,11 @@ void Room::processEphemeralEvent(EventPtr event) { const auto newMarker = findInTimeline(p.evtId); for( const Receipt& r: p.receipts ) - if (auto m = d->member(r.userId)) - d->promoteReadMarker(m, newMarker); + { + auto u = user(r.userId); + if (memberJoinState(u) == JoinState::Join) + d->promoteReadMarker(u, newMarker); + } } else { qCDebug(EPHEMERAL) << "Event" << p.evtId @@ -1414,9 +1398,12 @@ void Room::processEphemeralEvent(EventPtr event) // a previous marker for a user, keep the previous marker. // Otherwise, blindly store the event id for this user. for( const Receipt& r: p.receipts ) - if (auto m = d->member(r.userId)) - if (readMarker(m) == timelineEdge()) - d->setLastReadEvent(m, p.evtId); + { + auto u = user(r.userId); + if (memberJoinState(u) == JoinState::Join && + readMarker(u) == timelineEdge()) + d->setLastReadEvent(u, p.evtId); + } } } if (receiptEvent->unreadMessages()) @@ -1541,16 +1528,16 @@ QJsonObject Room::Private::toJson() const appendStateEvent(stateEvents, "m.room.canonical_alias", "alias", canonicalAlias); - for (const auto &i : membersMap) + for (const auto *m : membersMap) { QJsonObject content; content.insert("membership", QStringLiteral("join")); - content.insert("displayname", i->name()); - content.insert("avatar_url", i->avatarUrl().toString()); + content.insert("displayname", m->name()); + content.insert("avatar_url", m->avatarUrl().toString()); QJsonObject memberEvent; memberEvent.insert("type", QStringLiteral("m.room.member")); - memberEvent.insert("state_key", i->id()); + memberEvent.insert("state_key", m->id()); memberEvent.insert("content", content); stateEvents.append(memberEvent); } diff --git a/room.h b/room.h index a0dad8b3..0ef17abb 100644 --- a/room.h +++ b/room.h @@ -160,6 +160,28 @@ namespace QMatrixClient */ Q_INVOKABLE QImage avatar(int width, int height); + /** + * @brief Get a user object for a given user id + * This is the recommended way to get a user object in a room + * context. The actual object type may be changed in further + * versions to provide room-specific user information (display name, + * avatar etc.). + * \note The method will return a valid user regardless of + * the membership. + */ + Q_INVOKABLE User* user(const QString& userId) const; + + /** + * \brief Check the join state of a given user in this room + * + * \note Banned and invited users are not tracked for now (Leave + * will be returned for them). + * + * \return either of Join, Leave, depending on the given + * user's state in the room + */ + Q_INVOKABLE JoinState memberJoinState(User* user) const; + /** * @brief Produces a disambiguated name for a given user in * the context of the room -- cgit v1.2.3 From 2c095d29b96393dcfa3121c8cb9f4c4fd4f88d6a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 24 Feb 2018 19:18:22 +0900 Subject: Don't copy event content in accessors --- events/event.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/events/event.h b/events/event.h index 968bc1ad..1a1b994d 100644 --- a/events/event.h +++ b/events/event.h @@ -296,10 +296,10 @@ namespace QMatrixClient QJsonObject toJson() const { return _content.toJson(); } - ContentT content() const { return _content; } + const ContentT& content() const { return _content; } /** @deprecated Use prevContent instead */ - ContentT* prev_content() const { return prevContent(); } - ContentT* prevContent() const + const ContentT* prev_content() const { return prevContent(); } + const ContentT* prevContent() const { return _prev ? &_prev->content : nullptr; } QString prevSenderId() const { return _prev ? _prev->senderId : ""; } -- cgit v1.2.3 From c12f5cc213ecbb40c506e304e6b41c1437ca0d33 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 11:16:10 +0900 Subject: Support per-room user traits User::Private stores two new maps (for display names and avatars respectively) and is able to retrieve and store names/avatars on a per-room basis. The "most used" trait is stored separately and room lists are not kept for it (because most people have a single name and a single avatar across all or most rooms). TODO: The avatar container should be replaced with something less clumsy; the current code doesn't even compile with MSVC. A "handle" copyable structure that would hold the Avatar is one of options. Closes #141. --- room.cpp | 28 +++--- user.cpp | 307 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------- user.h | 36 ++++---- 3 files changed, 289 insertions(+), 82 deletions(-) diff --git a/room.cpp b/room.cpp index 3c6fb223..97f046d3 100644 --- a/room.cpp +++ b/room.cpp @@ -142,7 +142,7 @@ class Room::Private //void inviteUser(User* u); // We might get it at some point in time. void insertMemberIntoMap(User* u); - void renameMember(User* u, QString oldName); + void renameMember(User* u, QString oldName, const Room* context); void removeMemberFromMap(const QString& username, User* u); void getPreviousContent(int limit = 10); @@ -286,7 +286,7 @@ QImage Room::avatar(int width, int height) auto theOtherOneIt = d->membersMap.begin(); if (theOtherOneIt.value() == localUser()) ++theOtherOneIt; - return (*theOtherOneIt)->avatar(width, height, + return (*theOtherOneIt)->avatar(width, height, this, [=] { emit avatarChanged(); }); } return {}; @@ -300,7 +300,7 @@ User* Room::user(const QString& userId) const JoinState Room::memberJoinState(User* user) const { return - d->membersMap.contains(user->name(), user) ? JoinState::Join : + d->membersMap.contains(user->name(this), user) ? JoinState::Join : JoinState::Leave; } @@ -720,7 +720,7 @@ int Room::timelineSize() const void Room::Private::insertMemberIntoMap(User *u) { - const auto userName = u->name(); + const auto userName = u->name(q); auto namesakes = membersMap.values(userName); membersMap.insert(userName, u); // If there is exactly one namesake of the added user, signal member renaming @@ -729,12 +729,14 @@ void Room::Private::insertMemberIntoMap(User *u) emit q->memberRenamed(namesakes.front()); } -void Room::Private::renameMember(User* u, QString oldName) +void Room::Private::renameMember(User* u, QString oldName, const Room* context) { + if (context != q) + return; // It's not a rename for this room if (q->memberJoinState(u) == JoinState::Join) { qCWarning(MAIN) << "Room::Private::renameMember(): the user " - << u->fullName() + << u->fullName(q) << "is already known in the room under a new name."; return; } @@ -797,7 +799,7 @@ QString Room::roomMembername(const User* u) const { // See the CS spec, section 11.2.2.3 - const auto username = u->name(); + const auto username = u->name(this); if (username.isEmpty()) return u->id(); @@ -820,7 +822,7 @@ QString Room::roomMembername(const User* u) const // } // In case of more than one namesake, use the full name to disambiguate - return u->fullName(); + return u->fullName(this); } QString Room::roomMembername(const QString& userId) const @@ -1319,14 +1321,14 @@ void Room::processStateEvents(const RoomEvents& events) case EventType::RoomMember: { auto memberEvent = static_cast(event); auto u = user(memberEvent->userId()); - u->processEvent(memberEvent); + u->processEvent(memberEvent, this); if( memberEvent->membership() == MembershipType::Join ) { if (memberJoinState(u) != JoinState::Join) { d->insertMemberIntoMap(u); connect(u, &User::nameChanged, this, - std::bind(&Private::renameMember, d, u, _2)); + std::bind(&Private::renameMember, d, u, _2, _3)); emit userAdded(u); } } @@ -1336,7 +1338,7 @@ void Room::processStateEvents(const RoomEvents& events) { if (!d->membersLeft.contains(u)) d->membersLeft.append(u); - d->removeMemberFromMap(u->name(), u); + d->removeMemberFromMap(u->name(this), u); emit userRemoved(u); } } @@ -1532,8 +1534,8 @@ QJsonObject Room::Private::toJson() const { QJsonObject content; content.insert("membership", QStringLiteral("join")); - content.insert("displayname", m->name()); - content.insert("avatar_url", m->avatarUrl().toString()); + content.insert("displayname", m->name(q)); + content.insert("avatar_url", m->avatarUrl(q).toString()); QJsonObject memberEvent; memberEvent.insert("type", QStringLiteral("m.room.member")); diff --git a/user.cpp b/user.cpp index 289f0bac..2a3071af 100644 --- a/user.cpp +++ b/user.cpp @@ -19,9 +19,11 @@ #include "user.h" #include "connection.h" +#include "room.h" #include "avatar.h" #include "events/event.h" #include "events/roommemberevent.h" +#include "jobs/setroomstatejob.h" #include "jobs/generated/profile.h" #include "jobs/generated/content-repo.h" @@ -29,37 +31,208 @@ #include #include #include +#include #include +#include using namespace QMatrixClient; +using std::move; +using std::exchange; class User::Private { public: + static QIcon defaultIcon(); + static Avatar makeAvatar(QUrl url) { return { url, defaultIcon() }; } + Private(QString userId, Connection* connection) - : userId(std::move(userId)), connection(connection) + : userId(move(userId)), connection(connection) { } QString userId; - QString name; - QString bridged; Connection* connection; - Avatar avatar { QIcon::fromTheme(QStringLiteral("user-available")) }; + + QString mostUsedName; + QString bridged; + Avatar mostUsedAvatar { defaultIcon() }; + QMultiHash otherNames; + std::vector>> otherAvatars; + + mutable int totalRooms = 0; + + QString nameForRoom(const Room* r) const; + std::pair setNameForRoom(const Room* r, QString newName); + const Avatar& avatarForRoom(const Room* r) const; + bool setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl); void setAvatar(QString contentUri, User* q); + }; +QIcon User::Private::defaultIcon() +{ + static const QIcon icon + { QIcon::fromTheme(QStringLiteral("user-available")) }; + return icon; +} + +QString User::Private::nameForRoom(const Room* r) const +{ + return otherNames.key(r, mostUsedName); +} + +static constexpr int MIN_JOINED_ROOMS_TO_LOG = 100; + +std::pair User::Private::setNameForRoom(const Room* r, + QString newName) +{ + if (totalRooms < 2) + { + Q_ASSERT_X(totalRooms > 0 && otherNames.empty(), __FUNCTION__, + "Internal structures inconsistency"); + // The below uses that initialization list evaluation is ordered + return { mostUsedName != newName, + exchange(mostUsedName, move(newName)) }; + } + QString oldName; + // The below works because QMultiHash iterators dereference to stored values + auto it = std::find(otherNames.begin(), otherNames.end(), r); + if (it != otherNames.end()) + { + oldName = it.key(); + if (oldName == newName) + return { false, oldName }; // old name and new name coincide + otherNames.erase(it); + } + if (newName != mostUsedName) + { + // Check if the newName is about to become most used. + if (otherNames.count(newName) >= totalRooms - otherNames.size()) + { + Q_ASSERT(totalRooms > 1); + QElapsedTimer et; + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + { + qCDebug(MAIN) << "Switching the most used name of user" << userId + << "from" << mostUsedName << "to" << newName; + qCDebug(MAIN) << "The user is in" << totalRooms << "rooms"; + et.start(); + } + + for (auto* r1: connection->roomMap()) + if (nameForRoom(r1) == mostUsedName) + otherNames.insert(mostUsedName, r1); + + mostUsedName = newName; + otherNames.remove(newName); + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + qCDebug(PROFILER) << et.elapsed() + << "ms to switch the most used name"; + } + else + otherNames.insert(newName, r); + } + return { true, oldName }; +} + +const Avatar& User::Private::avatarForRoom(const Room* r) const +{ + for (const auto& p: otherAvatars) + { + auto roomIt = p.second.find(r); + if (roomIt != p.second.end()) + return p.first; + } + return mostUsedAvatar; +} + +bool User::Private::setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl) +{ + if (totalRooms < 2) + { + Q_ASSERT_X(totalRooms > 0 && otherAvatars.empty(), __FUNCTION__, + "Internal structures inconsistency"); + return + exchange(mostUsedAvatar, makeAvatar(avatarUrl)).url() != avatarUrl; + } + for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) + { + auto roomIt = it->second.find(r); + if (roomIt != it->second.end()) + { + if (it->first.url() == avatarUrl) + return false; // old url and new url coincide + it->second.erase(r); + if (avatarUrl == mostUsedAvatar.url()) + { + if (it->second.empty()) + otherAvatars.erase(it); + // The most used avatar will be used for this room + return true; + } + } + } + if (avatarUrl != mostUsedAvatar.url()) + { + size_t othersCount = 0; + auto entryToReplace = otherAvatars.end(); + for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) + { + othersCount += it->second.size(); + if (it->first.url() == avatarUrl) + entryToReplace = it; + } + // Check if the new avatar is about to become most used. + if (entryToReplace != otherAvatars.end() && + entryToReplace->second.size() >= size_t(totalRooms) - othersCount) + { + QElapsedTimer et; + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + { + qCDebug(MAIN) << "Switching the most used avatar of user" << userId + << "from" << mostUsedAvatar.url().toDisplayString() + << "to" << avatarUrl.toDisplayString(); + et.start(); + } + entryToReplace->first = + exchange(mostUsedAvatar, makeAvatar(avatarUrl)); + entryToReplace->second.clear(); + for (const auto* r1: connection->roomMap()) + { + if (avatarForRoom(r1).url() == mostUsedAvatar.url()) + entryToReplace->second.insert(r1); + } + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + qCDebug(PROFILER) << et.elapsed() + << "ms to switch the most used avatar"; + } + } + if (avatarUrl != mostUsedAvatar.url()) // It could have changed above + { + // Create a new entry if necessary and add the room to it. + auto it = find_if(otherAvatars.begin(), otherAvatars.end(), + [&avatarUrl] (const auto& p) { + return p.first.url() == avatarUrl; + }); + if (it == otherAvatars.end()) + { + otherAvatars.push_back({ Avatar(avatarUrl, defaultIcon()), {} }); + it = otherAvatars.end() - 1; + } + it->second.insert(r); + } + return true; +} + User::User(QString userId, Connection* connection) : QObject(connection), d(new Private(std::move(userId), connection)) { setObjectName(userId); } -User::~User() -{ - delete d; -} +User::~User() = default; QString User::id() const { @@ -75,34 +248,44 @@ bool User::isGuest() const return *it == ':'; } -QString User::name() const +QString User::name(const Room* room) const { - return d->name; + return d->nameForRoom(room); } -void User::updateName(const QString& newName) +void User::updateName(const QString& newName, const Room* room) { - const auto oldName = name(); - if (oldName != newName) + const auto setNameResult = d->setNameForRoom(room, newName); + if (setNameResult.first) { - d->name = newName; setObjectName(displayname()); - emit nameChanged(newName, oldName); + emit nameChanged(newName, setNameResult.second, room); } } -void User::updateAvatarUrl(const QUrl& newUrl) -{ - if (d->avatar.updateUrl(newUrl)) - emit avatarChanged(this); -} - void User::rename(const QString& newName) { auto job = d->connection->callApi(id(), newName); connect(job, &BaseJob::success, this, [=] { updateName(newName); }); } +void User::rename(const QString& newName, const Room* r) +{ + if (!r) + { + qCWarning(MAIN) << "Passing a null room to two-argument User::rename()" + "is incorrect; client developer, please fix it"; + rename(newName); + } + Q_ASSERT_X(r->memberJoinState(this) == JoinState::Join, __FUNCTION__, + "Attempt to rename a user that's not a room member"); + MemberEventContent evtC; + evtC.displayName = newName; + auto job = d->connection->callApi( + r->id(), id(), RoomMemberEvent(move(evtC))); + connect(job, &BaseJob::success, this, [=] { updateName(newName, r); }); +} + bool User::setAvatar(const QString& fileName) { return avatarObject().upload(d->connection, fileName, @@ -118,18 +301,20 @@ bool User::setAvatar(QIODevice* source) void User::Private::setAvatar(QString contentUri, User* q) { auto* j = connection->callApi(userId, contentUri); - connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); + connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q, nullptr); }); } -QString User::displayname() const +QString User::displayname(const Room* room) const { - return d->name.isEmpty() ? d->userId : d->name; + auto name = d->nameForRoom(room); + return name.isEmpty() ? d->userId : + room ? room->roomMembername(name) : name; } -QString User::fullName() const +QString User::fullName(const Room* room) const { - return d->name.isEmpty() ? d->userId : - d->name % " (" % d->userId % ')'; + auto name = d->nameForRoom(room); + return name.isEmpty() ? d->userId : name % " (" % d->userId % ')'; } QString User::bridged() const @@ -137,54 +322,70 @@ QString User::bridged() const return d->bridged; } -const Avatar& User::avatarObject() const +const Avatar& User::avatarObject(const Room* room) const { - return d->avatar; + return d->avatarForRoom(room); } -QImage User::avatar(int dimension) +QImage User::avatar(int dimension, const Room* room) { - return avatar(dimension, dimension); + return avatar(dimension, dimension, room); } -QImage User::avatar(int width, int height) +QImage User::avatar(int width, int height, const Room* room) { - return avatar(width, height, []{}); + return avatar(width, height, room, []{}); } -QImage User::avatar(int width, int height, Avatar::get_callback_t callback) +QImage User::avatar(int width, int height, const Room* room, Avatar::get_callback_t callback) { - return avatarObject().get(d->connection, width, height, - [this,callback] { emit avatarChanged(this); callback(); }); + return avatarObject(room).get(d->connection, width, height, + [=] { emit avatarChanged(this, room); callback(); }); } -QString User::avatarMediaId() const +QString User::avatarMediaId(const Room* room) const { - return avatarObject().mediaId(); + return avatarObject(room).mediaId(); } -QUrl User::avatarUrl() const +QUrl User::avatarUrl(const Room* room) const { - return avatarObject().url(); + return avatarObject(room).url(); } -void User::processEvent(Event* event) +void User::processEvent(RoomMemberEvent* event, const Room* room) { - if( event->type() == EventType::RoomMember ) + if (event->membership() != MembershipType::Invite && + event->membership() != MembershipType::Join) + return; + + auto aboutToEnter = room->memberJoinState(this) == JoinState::Leave && + (event->membership() == MembershipType::Join || + event->membership() == MembershipType::Invite); + if (aboutToEnter) + ++d->totalRooms; + + auto newName = event->displayName(); + // `bridged` value uses the same notification signal as the name; + // it is assumed that first setting of the bridge occurs together with + // the first setting of the name, and further bridge updates are + // exceptionally rare (the only reasonable case being that the bridge + // changes the naming convention). For the same reason room-specific + // bridge tags are not supported at all. + QRegularExpression reSuffix(" \\((IRC|Gitter|Telegram)\\)$"); + auto match = reSuffix.match(newName); + if (match.hasMatch()) { - auto e = static_cast(event); - if (e->membership() == MembershipType::Leave) - return; - - auto newName = e->displayName(); - QRegularExpression reSuffix(" \\((IRC|Gitter|Telegram)\\)$"); - auto match = reSuffix.match(newName); - if (match.hasMatch()) + if (d->bridged != match.captured(1)) { + if (!d->bridged.isEmpty()) + qCWarning(MAIN) << "Bridge for user" << id() << "changed:" + << d->bridged << "->" << match.captured(1); d->bridged = match.captured(1); - newName.truncate(match.capturedStart(0)); } - updateName(newName); - updateAvatarUrl(e->avatarUrl()); + newName.truncate(match.capturedStart(0)); } + updateName(event->displayName(), room); + if (d->setAvatarUrlForRoom(room, event->avatarUrl())) + emit avatarChanged(this, room); } diff --git a/user.h b/user.h index fa85d778..0a9d3da1 100644 --- a/user.h +++ b/user.h @@ -24,8 +24,10 @@ namespace QMatrixClient { - class Event; class Connection; + class Room; + class RoomMemberEvent; + class User: public QObject { Q_OBJECT @@ -51,7 +53,7 @@ namespace QMatrixClient * it. * \sa displayName */ - QString name() const; + QString name(const Room* room = nullptr) const; /** Get the displayed user name * This method returns the result of name() if its non-empty; @@ -60,7 +62,7 @@ namespace QMatrixClient * should be disambiguated. * \sa name, id, fullName Room::roomMembername */ - QString displayname() const; + QString displayname(const Room* room = nullptr) const; /** Get user name and id in one string * The constructed string follows the format 'name (id)' @@ -68,7 +70,7 @@ namespace QMatrixClient * places. * \sa displayName, Room::roomMembername */ - QString fullName() const; + QString fullName(const Room* room = nullptr) const; /** * Returns the name of bridge the user is connected from or empty. @@ -82,32 +84,34 @@ namespace QMatrixClient */ bool isGuest() const; - const Avatar& avatarObject() const; - Q_INVOKABLE QImage avatar(int dimension); - Q_INVOKABLE QImage avatar(int width, int height); - QImage avatar(int width, int height, Avatar::get_callback_t callback); + const Avatar& avatarObject(const Room* room = nullptr) const; + Q_INVOKABLE QImage avatar(int dimension, const Room* room = nullptr); + Q_INVOKABLE QImage avatar(int requestedWidth, int requestedHeight, + const Room* room = nullptr); + QImage avatar(int width, int height, const Room* room, Avatar::get_callback_t callback); - QString avatarMediaId() const; - QUrl avatarUrl() const; + QString avatarMediaId(const Room* room = nullptr) const; + QUrl avatarUrl(const Room* room = nullptr) const; - void processEvent(Event* event); + void processEvent(RoomMemberEvent* event, const Room* r = nullptr); public slots: void rename(const QString& newName); + void rename(const QString& newName, const Room* r); bool setAvatar(const QString& fileName); bool setAvatar(QIODevice* source); signals: - void nameChanged(QString newName, QString oldName); - void avatarChanged(User* user); + void nameChanged(QString newName, QString oldName, + const Room* roomContext); + void avatarChanged(User* user, const Room* roomContext); private slots: - void updateName(const QString& newName); - void updateAvatarUrl(const QUrl& newUrl); + void updateName(const QString& newName, const Room* room = nullptr); private: class Private; - Private* d; + QScopedPointer d; }; } Q_DECLARE_METATYPE(QMatrixClient::User*) -- cgit v1.2.3 From e09c740c85e2e9a861e84f706680ce1ec662a4bc Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 24 Feb 2018 14:43:03 +0900 Subject: User/Room: signal that a user/member is about to change the name Enables to address QMatrixClient/Quaternion#284. Also fixes a gibberish condition in Room::Private::renameMember() that led to improper warnings and a too early return. --- room.cpp | 53 ++++++++++++++++++++++++++++++++++++----------------- room.h | 8 +++++--- user.cpp | 58 +++++++++++++++++++++++++++++++++------------------------- user.h | 7 ++++++- 4 files changed, 80 insertions(+), 46 deletions(-) diff --git a/room.cpp b/room.cpp index 97f046d3..06041090 100644 --- a/room.cpp +++ b/room.cpp @@ -142,7 +142,7 @@ class Room::Private //void inviteUser(User* u); // We might get it at some point in time. void insertMemberIntoMap(User* u); - void renameMember(User* u, QString oldName, const Room* context); + void renameMember(User* u, QString oldName); void removeMemberFromMap(const QString& username, User* u); void getPreviousContent(int limit = 10); @@ -721,42 +721,49 @@ int Room::timelineSize() const void Room::Private::insertMemberIntoMap(User *u) { const auto userName = u->name(q); - auto namesakes = membersMap.values(userName); - membersMap.insert(userName, 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. + auto namesakes = membersMap.values(userName); + if (namesakes.size() == 1) + emit q->memberAboutToRename(namesakes.front(), + namesakes.front()->fullName(q)); + membersMap.insert(userName, u); if (namesakes.size() == 1) emit q->memberRenamed(namesakes.front()); } -void Room::Private::renameMember(User* u, QString oldName, const Room* context) +void Room::Private::renameMember(User* u, QString oldName) { - if (context != q) - return; // It's not a rename for this room - if (q->memberJoinState(u) == JoinState::Join) + if (u->name(q) == oldName) { qCWarning(MAIN) << "Room::Private::renameMember(): the user " << u->fullName(q) << "is already known in the room under a new name."; - return; } - - if (membersMap.contains(oldName, u)) + else if (membersMap.contains(oldName, u)) { removeMemberFromMap(oldName, u); insertMemberIntoMap(u); - emit q->memberRenamed(u); } + emit q->memberRenamed(u); } void Room::Private::removeMemberFromMap(const QString& username, User* u) { + User* namesake = nullptr; + auto namesakes = membersMap.values(username); + if (namesakes.size() == 2) + { + namesake = namesakes.front() == u ? namesakes.back() : namesakes.front(); + Q_ASSERT_X(namesake != u, __FUNCTION__, "Room members list is broken"); + emit q->memberAboutToRename(namesake, username); + } 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. - if (membersMap.count(username) == 1) - emit q->memberRenamed(membersMap.value(username)); + if (namesake) + emit q->memberRenamed(namesake); } inline auto makeErrorStr(const Event& e, QByteArray msg) @@ -1327,8 +1334,16 @@ void Room::processStateEvents(const RoomEvents& events) if (memberJoinState(u) != JoinState::Join) { d->insertMemberIntoMap(u); + connect(u, &User::nameAboutToChange, this, + [=] (QString newName, QString, const Room* context) { + if (context == this) + emit memberAboutToRename(u, newName); + }); connect(u, &User::nameChanged, this, - std::bind(&Private::renameMember, d, u, _2, _3)); + [=] (QString, QString oldName, const Room* context) { + if (context == this) + d->renameMember(u, oldName); + }); emit userAdded(u); } } @@ -1602,12 +1617,16 @@ MemberSorter Room::memberSorter() const } bool MemberSorter::operator()(User *u1, User *u2) const +{ + return operator()(u1, room->roomMembername(u2)); +} + +bool MemberSorter::operator ()(User* u1, const QString& u2name) const { auto n1 = room->roomMembername(u1); - auto n2 = room->roomMembername(u2); if (n1.startsWith('@')) n1.remove(0, 1); - if (n2.startsWith('@')) - n2.remove(0, 1); + auto n2 = u2name.midRef(u2name.startsWith('@') ? 1 : 0); + return n1.localeAwareCompare(n2) < 0; } diff --git a/room.h b/room.h index 0ef17abb..5253a7c6 100644 --- a/room.h +++ b/room.h @@ -301,6 +301,7 @@ namespace QMatrixClient void avatarChanged(); void userAdded(User* user); void userRemoved(User* user); + void memberAboutToRename(User* user, QString newName); void memberRenamed(User* user); void memberListChanged(); @@ -345,12 +346,13 @@ namespace QMatrixClient explicit MemberSorter(const Room* r) : room(r) { } bool operator()(User* u1, User* u2) const; + bool operator()(User* u1, const QString& u2name) const; - template + template typename ContT::size_type lowerBoundIndex(const ContT& c, - typename ContT::value_type v) const + const ValT& v) const { - return std::lower_bound(c.begin(), c.end(), v, *this) - c.begin(); + return std::lower_bound(c.begin(), c.end(), v, *this) - c.begin(); } private: diff --git a/user.cpp b/user.cpp index 2a3071af..8a9fa515 100644 --- a/user.cpp +++ b/user.cpp @@ -62,8 +62,8 @@ class User::Private mutable int totalRooms = 0; - QString nameForRoom(const Room* r) const; - std::pair setNameForRoom(const Room* r, QString newName); + QString nameForRoom(const Room* r, const QString& hint = {}) const; + void setNameForRoom(const Room* r, QString newName, QString oldName); const Avatar& avatarForRoom(const Room* r) const; bool setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl); @@ -78,34 +78,29 @@ QIcon User::Private::defaultIcon() return icon; } -QString User::Private::nameForRoom(const Room* r) const +QString User::Private::nameForRoom(const Room* r, const QString& hint) const { + // If the hint is accurate, this function is O(1) instead of O(n) + if (hint == mostUsedName || otherNames.contains(hint, r)) + return hint; return otherNames.key(r, mostUsedName); } -static constexpr int MIN_JOINED_ROOMS_TO_LOG = 100; +static constexpr int MIN_JOINED_ROOMS_TO_LOG = 20; -std::pair User::Private::setNameForRoom(const Room* r, - QString newName) +void User::Private::setNameForRoom(const Room* r, QString newName, + QString oldName) { + Q_ASSERT(oldName != newName); + Q_ASSERT(oldName == mostUsedName || otherNames.contains(oldName, r)); if (totalRooms < 2) { Q_ASSERT_X(totalRooms > 0 && otherNames.empty(), __FUNCTION__, "Internal structures inconsistency"); - // The below uses that initialization list evaluation is ordered - return { mostUsedName != newName, - exchange(mostUsedName, move(newName)) }; - } - QString oldName; - // The below works because QMultiHash iterators dereference to stored values - auto it = std::find(otherNames.begin(), otherNames.end(), r); - if (it != otherNames.end()) - { - oldName = it.key(); - if (oldName == newName) - return { false, oldName }; // old name and new name coincide - otherNames.erase(it); + mostUsedName = move(newName); + return; } + otherNames.remove(oldName, r); if (newName != mostUsedName) { // Check if the newName is about to become most used. @@ -134,7 +129,6 @@ std::pair User::Private::setNameForRoom(const Room* r, else otherNames.insert(newName, r); } - return { true, oldName }; } const Avatar& User::Private::avatarForRoom(const Room* r) const @@ -255,11 +249,19 @@ QString User::name(const Room* room) const void User::updateName(const QString& newName, const Room* room) { - const auto setNameResult = d->setNameForRoom(room, newName); - if (setNameResult.first) + updateName(newName, d->nameForRoom(room), room); +} + +void User::updateName(const QString& newName, const QString& oldName, + const Room* room) +{ + Q_ASSERT(oldName == d->mostUsedName || d->otherNames.contains(oldName, room)); + if (newName != oldName) { + emit nameAboutToChange(newName, oldName, room); + d->setNameForRoom(room, newName, oldName); setObjectName(displayname()); - emit nameChanged(newName, setNameResult.second, room); + emit nameChanged(newName, oldName, room); } } @@ -337,7 +339,8 @@ QImage User::avatar(int width, int height, const Room* room) return avatar(width, height, room, []{}); } -QImage User::avatar(int width, int height, const Room* room, Avatar::get_callback_t callback) +QImage User::avatar(int width, int height, const Room* room, + Avatar::get_callback_t callback) { return avatarObject(room).get(d->connection, width, height, [=] { emit avatarChanged(this, room); callback(); }); @@ -385,7 +388,12 @@ void User::processEvent(RoomMemberEvent* event, const Room* room) } newName.truncate(match.capturedStart(0)); } - updateName(event->displayName(), room); + if (event->prevContent()) + updateName(event->displayName(), + d->nameForRoom(room, event->prevContent()->displayName), + room); + else + updateName(event->displayName(), room); if (d->setAvatarUrlForRoom(room, event->avatarUrl())) emit avatarChanged(this, room); } diff --git a/user.h b/user.h index 0a9d3da1..d1d28312 100644 --- a/user.h +++ b/user.h @@ -88,7 +88,8 @@ namespace QMatrixClient Q_INVOKABLE QImage avatar(int dimension, const Room* room = nullptr); Q_INVOKABLE QImage avatar(int requestedWidth, int requestedHeight, const Room* room = nullptr); - QImage avatar(int width, int height, const Room* room, Avatar::get_callback_t callback); + QImage avatar(int width, int height, const Room* room, + Avatar::get_callback_t callback); QString avatarMediaId(const Room* room = nullptr) const; QUrl avatarUrl(const Room* room = nullptr) const; @@ -102,12 +103,16 @@ namespace QMatrixClient bool setAvatar(QIODevice* source); signals: + void nameAboutToChange(QString newName, QString oldName, + const Room* roomContext); void nameChanged(QString newName, QString oldName, const Room* roomContext); void avatarChanged(User* user, const Room* roomContext); private slots: void updateName(const QString& newName, const Room* room = nullptr); + void updateName(const QString& newName, const QString& oldName, + const Room* room = nullptr); private: class Private; -- cgit v1.2.3 From e10927767faaf7a03a772ab97fe6292907cc4b4b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 24 Feb 2018 17:42:05 +0900 Subject: User: store avatars on the heap; use two containers to store avatars-to-rooms mapping Because they are uncopiable, unlike pointers to them; and a combination of QHash of avatars and a QMultiHash of rooms is much more convenient than a std::vector>. --- avatar.h | 2 +- user.cpp | 159 ++++++++++++++++++++++++++++++--------------------------------- user.h | 2 + 3 files changed, 78 insertions(+), 85 deletions(-) diff --git a/avatar.h b/avatar.h index ecd5bc52..0166ae9e 100644 --- a/avatar.h +++ b/avatar.h @@ -32,7 +32,7 @@ namespace QMatrixClient { public: explicit Avatar(QIcon defaultIcon = {}); - Avatar(QUrl url, QIcon defaultIcon = {}); + explicit Avatar(QUrl url, QIcon defaultIcon = {}); Avatar(Avatar&&); ~Avatar(); Avatar& operator=(Avatar&&); diff --git a/user.cpp b/user.cpp index 8a9fa515..9cdbb420 100644 --- a/user.cpp +++ b/user.cpp @@ -37,14 +37,13 @@ #include using namespace QMatrixClient; +using namespace std::placeholders; using std::move; -using std::exchange; class User::Private { public: - static QIcon defaultIcon(); - static Avatar makeAvatar(QUrl url) { return { url, defaultIcon() }; } + static Avatar* makeAvatar(QUrl url); Private(QString userId, Connection* connection) : userId(move(userId)), connection(connection) @@ -55,27 +54,28 @@ class User::Private QString mostUsedName; QString bridged; - Avatar mostUsedAvatar { defaultIcon() }; + const QScopedPointer mostUsedAvatar { makeAvatar({}) }; QMultiHash otherNames; - std::vector>> otherAvatars; + QHash otherAvatars; + QMultiHash avatarsToRooms; mutable int totalRooms = 0; QString nameForRoom(const Room* r, const QString& hint = {}) const; void setNameForRoom(const Room* r, QString newName, QString oldName); - const Avatar& avatarForRoom(const Room* r) const; - bool setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl); + QUrl avatarUrlForRoom(const Room* r, const QUrl& hint = {}) const; + void setAvatarForRoom(const Room* r, const QUrl& newUrl, + const QUrl& oldUrl); - void setAvatar(QString contentUri, User* q); + void setAvatarOnServer(QString contentUri, User* q); }; -QIcon User::Private::defaultIcon() +Avatar* User::Private::makeAvatar(QUrl url) { static const QIcon icon { QIcon::fromTheme(QStringLiteral("user-available")) }; - return icon; + return new Avatar(url, icon); } QString User::Private::nameForRoom(const Room* r, const QString& hint) const @@ -131,97 +131,67 @@ void User::Private::setNameForRoom(const Room* r, QString newName, } } -const Avatar& User::Private::avatarForRoom(const Room* r) const +QUrl User::Private::avatarUrlForRoom(const Room* r, const QUrl& hint) const { - for (const auto& p: otherAvatars) - { - auto roomIt = p.second.find(r); - if (roomIt != p.second.end()) - return p.first; - } - return mostUsedAvatar; + // If the hint is accurate, this function is O(1) instead of O(n) + if (hint == mostUsedAvatar->url() || avatarsToRooms.contains(hint, r)) + return hint; + auto it = std::find(avatarsToRooms.begin(), avatarsToRooms.end(), r); + return it == avatarsToRooms.end() ? mostUsedAvatar->url() : it.key(); } -bool User::Private::setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl) +void User::Private::setAvatarForRoom(const Room* r, const QUrl& newUrl, + const QUrl& oldUrl) { + Q_ASSERT(oldUrl != newUrl); + Q_ASSERT(oldUrl == mostUsedAvatar->url() || + avatarsToRooms.contains(oldUrl, r)); if (totalRooms < 2) { Q_ASSERT_X(totalRooms > 0 && otherAvatars.empty(), __FUNCTION__, "Internal structures inconsistency"); - return - exchange(mostUsedAvatar, makeAvatar(avatarUrl)).url() != avatarUrl; + mostUsedAvatar->updateUrl(newUrl); + return; } - for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) + avatarsToRooms.remove(oldUrl, r); + if (!avatarsToRooms.contains(oldUrl)) { - auto roomIt = it->second.find(r); - if (roomIt != it->second.end()) - { - if (it->first.url() == avatarUrl) - return false; // old url and new url coincide - it->second.erase(r); - if (avatarUrl == mostUsedAvatar.url()) - { - if (it->second.empty()) - otherAvatars.erase(it); - // The most used avatar will be used for this room - return true; - } - } + delete otherAvatars.value(oldUrl); + otherAvatars.remove(oldUrl); } - if (avatarUrl != mostUsedAvatar.url()) + if (newUrl != mostUsedAvatar->url()) { - size_t othersCount = 0; - auto entryToReplace = otherAvatars.end(); - for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) - { - othersCount += it->second.size(); - if (it->first.url() == avatarUrl) - entryToReplace = it; - } // Check if the new avatar is about to become most used. - if (entryToReplace != otherAvatars.end() && - entryToReplace->second.size() >= size_t(totalRooms) - othersCount) + if (avatarsToRooms.count(newUrl) >= totalRooms - avatarsToRooms.size()) { QElapsedTimer et; if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) { qCDebug(MAIN) << "Switching the most used avatar of user" << userId - << "from" << mostUsedAvatar.url().toDisplayString() - << "to" << avatarUrl.toDisplayString(); + << "from" << mostUsedAvatar->url().toDisplayString() + << "to" << newUrl.toDisplayString(); et.start(); } - entryToReplace->first = - exchange(mostUsedAvatar, makeAvatar(avatarUrl)); - entryToReplace->second.clear(); + avatarsToRooms.remove(newUrl); + auto* nextMostUsed = otherAvatars.take(newUrl); + std::swap(*mostUsedAvatar, *nextMostUsed); + otherAvatars.insert(nextMostUsed->url(), nextMostUsed); for (const auto* r1: connection->roomMap()) - { - if (avatarForRoom(r1).url() == mostUsedAvatar.url()) - entryToReplace->second.insert(r1); - } + if (avatarUrlForRoom(r1) == nextMostUsed->url()) + avatarsToRooms.insert(nextMostUsed->url(), r1); + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) qCDebug(PROFILER) << et.elapsed() << "ms to switch the most used avatar"; + } else { + otherAvatars.insert(newUrl, makeAvatar(newUrl)); + avatarsToRooms.insert(newUrl, r); } } - if (avatarUrl != mostUsedAvatar.url()) // It could have changed above - { - // Create a new entry if necessary and add the room to it. - auto it = find_if(otherAvatars.begin(), otherAvatars.end(), - [&avatarUrl] (const auto& p) { - return p.first.url() == avatarUrl; - }); - if (it == otherAvatars.end()) - { - otherAvatars.push_back({ Avatar(avatarUrl, defaultIcon()), {} }); - it = otherAvatars.end() - 1; - } - it->second.insert(r); - } - return true; } User::User(QString userId, Connection* connection) - : QObject(connection), d(new Private(std::move(userId), connection)) + : QObject(connection), d(new Private(move(userId), connection)) { setObjectName(userId); } @@ -265,6 +235,20 @@ void User::updateName(const QString& newName, const QString& oldName, } } +void User::updateAvatarUrl(const QUrl& newUrl, const QUrl& oldUrl, + const Room* room) +{ + Q_ASSERT(oldUrl == d->mostUsedAvatar->url() || + d->avatarsToRooms.contains(oldUrl, room)); + if (newUrl != oldUrl) + { + d->setAvatarForRoom(room, newUrl, oldUrl); + setObjectName(displayname()); + emit avatarChanged(this, room); + } + +} + void User::rename(const QString& newName) { auto job = d->connection->callApi(id(), newName); @@ -291,19 +275,20 @@ void User::rename(const QString& newName, const Room* r) bool User::setAvatar(const QString& fileName) { return avatarObject().upload(d->connection, fileName, - [this] (QString contentUri) { d->setAvatar(contentUri, this); }); + std::bind(&Private::setAvatarOnServer, d.data(), _1, this)); } bool User::setAvatar(QIODevice* source) { return avatarObject().upload(d->connection, source, - [this] (QString contentUri) { d->setAvatar(contentUri, this); }); + std::bind(&Private::setAvatarOnServer, d.data(), _1, this)); } -void User::Private::setAvatar(QString contentUri, User* q) +void User::Private::setAvatarOnServer(QString contentUri, User* q) { auto* j = connection->callApi(userId, contentUri); - connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q, nullptr); }); + connect(j, &BaseJob::success, q, + [=] { q->updateAvatarUrl(contentUri, avatarUrlForRoom(nullptr)); }); } QString User::displayname(const Room* room) const @@ -326,7 +311,8 @@ QString User::bridged() const const Avatar& User::avatarObject(const Room* room) const { - return d->avatarForRoom(room); + return *d->otherAvatars.value(d->avatarUrlForRoom(room), + d->mostUsedAvatar.data()); } QImage User::avatar(int dimension, const Room* room) @@ -389,11 +375,16 @@ void User::processEvent(RoomMemberEvent* event, const Room* room) newName.truncate(match.capturedStart(0)); } if (event->prevContent()) - updateName(event->displayName(), - d->nameForRoom(room, event->prevContent()->displayName), - room); - else + { + // FIXME: the hint doesn't work for bridged users + auto oldNameHint = + d->nameForRoom(room, event->prevContent()->displayName); + updateName(event->displayName(), oldNameHint, room); + updateAvatarUrl(event->avatarUrl(), + d->avatarUrlForRoom(room, event->prevContent()->avatarUrl), + room); + } else { updateName(event->displayName(), room); - if (d->setAvatarUrlForRoom(room, event->avatarUrl())) - emit avatarChanged(this, room); + updateAvatarUrl(event->avatarUrl(), d->avatarUrlForRoom(room), room); + } } diff --git a/user.h b/user.h index d1d28312..d19fa8f4 100644 --- a/user.h +++ b/user.h @@ -113,6 +113,8 @@ namespace QMatrixClient void updateName(const QString& newName, const Room* room = nullptr); void updateName(const QString& newName, const QString& oldName, const Room* room = nullptr); + void updateAvatarUrl(const QUrl& newUrl, const QUrl& oldUrl, + const Room* room = nullptr); private: class Private; -- cgit v1.2.3 From fea4f3b5c8d313b429777a8c73ee520af24eecfe Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Feb 2018 14:37:49 +0900 Subject: ReceiptEvent: code cleanup --- events/receiptevent.cpp | 4 ++-- events/receiptevent.h | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/events/receiptevent.cpp b/events/receiptevent.cpp index e30fe4e4..3c4d34ee 100644 --- a/events/receiptevent.cpp +++ b/events/receiptevent.cpp @@ -43,10 +43,10 @@ using namespace QMatrixClient; ReceiptEvent::ReceiptEvent(const QJsonObject& obj) : Event(Type::Receipt, obj) { - Q_ASSERT(obj["type"].toString() == jsonType); + Q_ASSERT(obj["type"].toString() == TypeId); const QJsonObject contents = contentJson(); - _eventsWithReceipts.reserve(static_cast(contents.size())); + _eventsWithReceipts.reserve(contents.size()); for( auto eventIt = contents.begin(); eventIt != contents.end(); ++eventIt ) { if (eventIt.key().isEmpty()) diff --git a/events/receiptevent.h b/events/receiptevent.h index 9494c7c6..92dace82 100644 --- a/events/receiptevent.h +++ b/events/receiptevent.h @@ -48,7 +48,5 @@ namespace QMatrixClient private: EventsWithReceipts _eventsWithReceipts; bool _unreadMessages; // Spec extension for caching purposes - - static constexpr const char * jsonType = "m.receipt"; }; } // namespace QMatrixClient -- cgit v1.2.3 From e77a53946805649be99f8c0f6ee9c00702348132 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Feb 2018 18:25:22 +0900 Subject: Room: show 1-on-1 invitations in a better way Previously it was just an "Empty room" name, now it's "Invitation from %1". --- room.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/room.cpp b/room.cpp index 06041090..762e929c 100644 --- a/room.cpp +++ b/room.cpp @@ -1452,6 +1452,12 @@ QString Room::Private::roomNameFromMemberNames(const QList &userlist) co } ); + // Spec extension. A single person in the chat but not the local user + // (the local user is apparently invited). + if (userlist.size() == 1 && !isLocalUser(first_two.front())) + return tr("Invitation from %1") + .arg(q->roomMembername(first_two.front())); + // i. One-on-one chat. first_two[1] == localUser() in this case. if (userlist.size() == 2) return q->roomMembername(first_two[0]); -- cgit v1.2.3 From a2f991555bec7b317606093e95ec2b5684b0005a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Feb 2018 15:09:17 +0900 Subject: SyncJob: parse events from global account data too Closes #123 (room account data were parsed even before). No specific event classes for account data yet, though. --- jobs/syncjob.cpp | 11 ++++++++--- jobs/syncjob.h | 36 +++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index ce5dd894..7b066f4f 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -54,6 +54,11 @@ SyncDataList&& SyncData::takeRoomData() return std::move(roomData); } +SyncBatch&& SyncData::takeAccountData() +{ + return std::move(accountData); +} + BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { return d.parseJson(data); @@ -63,12 +68,12 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data) { QElapsedTimer et; et.start(); - QJsonObject json = data.object(); + auto json { data.object() }; nextBatch_ = json.value("next_batch").toString(); // TODO: presence - // TODO: account_data - QJsonObject rooms = json.value("rooms").toObject(); + accountData.fromJson(json); + QJsonObject rooms = json.value("rooms").toObject(); for (size_t i = 0; i < JoinStateStrings.size(); ++i) { const auto rs = rooms.value(JoinStateStrings[i]).toObject(); diff --git a/jobs/syncjob.h b/jobs/syncjob.h index aed36e0b..5956e73b 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -26,30 +26,30 @@ namespace QMatrixClient { - class SyncRoomData + template + class SyncBatch : public EventsBatch { public: - template - class Batch : public EventsBatch + explicit SyncBatch(QString k) : jsonKey(std::move(k)) { } + void fromJson(const QJsonObject& roomContents) { - public: - explicit Batch(QString k) : jsonKey(std::move(k)) { } - void fromJson(const QJsonObject& roomContents) - { - EventsBatch::fromJson( - roomContents[jsonKey].toObject(), "events"); - } + EventsBatch::fromJson( + roomContents[jsonKey].toObject(), "events"); + } - private: - QString jsonKey; - }; + private: + QString jsonKey; + }; + class SyncRoomData + { + public: QString roomId; JoinState joinState; - Batch state; - Batch timeline; - Batch ephemeral; - Batch accountData; + SyncBatch state; + SyncBatch timeline; + SyncBatch ephemeral; + SyncBatch accountData; bool timelineLimited; QString timelinePrevBatch; @@ -68,11 +68,13 @@ namespace QMatrixClient { public: BaseJob::Status parseJson(const QJsonDocument &data); + SyncBatch&& takeAccountData(); SyncDataList&& takeRoomData(); QString nextBatch() const; private: QString nextBatch_; + SyncBatch accountData { "account_data" }; SyncDataList roomData; }; -- cgit v1.2.3 From 7c10807549b2a73527bd594789d0e5b9ab58c874 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Feb 2018 17:24:59 +0900 Subject: TagEvent: m.tag events parsing Using them in rooms and connection comes in the next commit. --- CMakeLists.txt | 1 + events/event.cpp | 8 +++++++- events/event.h | 4 ++-- events/tagevent.cpp | 42 +++++++++++++++++++++++++++++++++++++++++ events/tagevent.h | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++ libqmatrixclient.pri | 3 ++- 6 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 events/tagevent.cpp create mode 100644 events/tagevent.h diff --git a/CMakeLists.txt b/CMakeLists.txt index da5bac9b..709860e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -60,6 +60,7 @@ set(libqmatrixclient_SRCS events/roomavatarevent.cpp events/typingevent.cpp events/receiptevent.cpp + events/tagevent.cpp jobs/requestdata.cpp jobs/basejob.cpp jobs/checkauthmethods.cpp diff --git a/events/event.cpp b/events/event.cpp index 366aa858..b55c44c4 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -24,6 +24,7 @@ #include "roomavatarevent.h" #include "typingevent.h" #include "receiptevent.h" +#include "tagevent.h" #include "redactionevent.h" #include "logging.h" @@ -44,6 +45,11 @@ Event::Event(Type type, const QJsonObject& rep) Event::~Event() = default; +QString Event::jsonType() const +{ + return originalJsonObject().value("type").toString(); +} + QByteArray Event::originalJson() const { return QJsonDocument(_originalJson).toJson(); @@ -82,7 +88,7 @@ EventPtr _impl::doMakeEvent(const QJsonObject& obj) return EventPtr(move(e)); return EventPtr { makeIfMatches(obj, obj["type"].toString()) }; + TypingEvent, ReceiptEvent, TagEvent>(obj, obj["type"].toString()) }; } RoomEvent::RoomEvent(Event::Type type) : Event(type) { } diff --git a/events/event.h b/events/event.h index 1a1b994d..4bd08b55 100644 --- a/events/event.h +++ b/events/event.h @@ -45,7 +45,7 @@ namespace QMatrixClient enum class Type : quint16 { Unknown = 0, - Typing, Receipt, + Typing, Receipt, Tag, DirectChat, RoomEventBase = 0x1000, RoomMessage = RoomEventBase + 1, RoomEncryptedMessage, Redaction, @@ -63,6 +63,7 @@ namespace QMatrixClient virtual ~Event(); Type type() const { return _type; } + QString jsonType() const; bool isStateEvent() const { return (quint16(_type) & 0x1800) == 0x1800; @@ -76,7 +77,6 @@ namespace QMatrixClient // (and in most cases it will be a combination of other fields // instead of "content" field). - protected: const QJsonObject contentJson() const; private: diff --git a/events/tagevent.cpp b/events/tagevent.cpp new file mode 100644 index 00000000..9f381c76 --- /dev/null +++ b/events/tagevent.cpp @@ -0,0 +1,42 @@ +#include "tagevent.h" + +using namespace QMatrixClient; + +TagRecord::TagRecord(const QJsonObject& json) + : order(json.value("order").toString()) +{ } + +TagEvent::TagEvent(const QJsonObject& obj) + : Event(Type::Tag, obj) +{ + Q_ASSERT(obj["type"].toString() == TypeId); +} + +QStringList TagEvent::tagNames() const +{ + return tagsObject().keys(); +} + +QHash TagEvent::tags() const +{ + QHash result; + auto allTags { tagsObject() }; + for (auto it = allTags.begin(); it != allTags.end(); ++ it) + result.insert(it.key(), TagRecord(it.value().toObject())); + return result; +} + +bool TagEvent::isFavourite() const +{ + return tagsObject().contains("m.favourite"); +} + +bool TagEvent::isLowPriority() const +{ + return tagsObject().contains("m.lowpriority"); +} + +QJsonObject TagEvent::tagsObject() const +{ + return contentJson().value("tags").toObject(); +} diff --git a/events/tagevent.h b/events/tagevent.h new file mode 100644 index 00000000..23b0b703 --- /dev/null +++ b/events/tagevent.h @@ -0,0 +1,53 @@ +/****************************************************************************** + * Copyright (C) 2018 Kitsune Ral + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#pragma once + +#include "event.h" + +namespace QMatrixClient +{ + struct TagRecord + { + explicit TagRecord(const QJsonObject& json = {}); + + QString order; + }; + + class TagEvent : public Event + { + public: + explicit TagEvent(const QJsonObject& obj); + + /** Get the list of tag names */ + QStringList tagNames() const; + + /** Get the list of tags along with information on each */ + QHash tags() const; + + /** Check whether the list of tags has m.favourite */ + bool isFavourite() const; + /** Check whether the list of tags has m.lowpriority */ + bool isLowPriority() const; + + static constexpr const char * TypeId = "m.tag"; + + protected: + QJsonObject tagsObject() const; + }; +} diff --git a/libqmatrixclient.pri b/libqmatrixclient.pri index 72637caf..7cfa94a1 100644 --- a/libqmatrixclient.pri +++ b/libqmatrixclient.pri @@ -24,6 +24,7 @@ HEADERS += \ $$PWD/events/roomavatarevent.h \ $$PWD/events/typingevent.h \ $$PWD/events/receiptevent.h \ + $$PWD/events/tagevent.h \ $$PWD/events/redactionevent.h \ $$PWD/jobs/requestdata.h \ $$PWD/jobs/basejob.h \ @@ -55,7 +56,7 @@ SOURCES += \ $$PWD/events/roommemberevent.cpp \ $$PWD/events/typingevent.cpp \ $$PWD/events/receiptevent.cpp \ - $$PWD/events/redactionevent.cpp \ + $$PWD/events/tagevent.cpp \ $$PWD/jobs/requestdata.cpp \ $$PWD/jobs/basejob.cpp \ $$PWD/jobs/checkauthmethods.cpp \ -- cgit v1.2.3 From ec412621d71b1a7758b15d2b3c5cd5e7b2081ab1 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Feb 2018 18:57:02 +0900 Subject: Room and Connection: support room tags Closes #134. --- connection.cpp | 24 ++++++++++++++++++++++++ connection.h | 15 +++++++++++++++ room.cpp | 34 ++++++++++++++++++++++++++++++++++ room.h | 9 +++++++++ 4 files changed, 82 insertions(+) diff --git a/connection.cpp b/connection.cpp index 4b7d4abb..52fcc40b 100644 --- a/connection.cpp +++ b/connection.cpp @@ -520,6 +520,30 @@ QHash< QPair, Room* > Connection::roomMap() const return roomMap; } +QHash> Connection::tagsToRooms() const +{ + QHash> result; + for (auto* r: d->roomMap) + { + for (const auto& tagName: r->tagNames()) + result[tagName].push_back(r); + } + for (auto it = result.begin(); it != result.end(); ++it) + std::sort(it->begin(), it->end(), + [t=it.key()] (Room* r1, Room* r2) { + return r1->tags().value(t).order < r2->tags().value(t).order; + }); + return result; +} + +QVector Connection::roomsWithTag(const QString& tagName) const +{ + QVector rooms; + std::copy_if(d->roomMap.begin(), d->roomMap.end(), std::back_inserter(rooms), + [&tagName] (Room* r) { return r->tags().contains(tagName); }); + return rooms; +} + QMap Connection::users() const { return d->userMap; diff --git a/connection.h b/connection.h index 3ec4fd9d..b45a171d 100644 --- a/connection.h +++ b/connection.h @@ -70,7 +70,22 @@ namespace QMatrixClient explicit Connection(const QUrl& server, QObject* parent = nullptr); virtual ~Connection(); + /** Get all Invited and Joined rooms + * \return a hashmap from a composite key - room name and whether + * it's an Invite rather than Join - to room pointers + */ QHash, Room*> roomMap() const; + + /** Get all Invited and Joined rooms grouped by tag + * \return a hashmap from tag name to a vector of room pointers, + * sorted by their order in the tag - details are at + * https://matrix.org/speculator/spec/drafts%2Fe2e/client_server/unstable.html#id95 + */ + QHash> tagsToRooms() const; + + /** Get the list of rooms with the specified tag */ + QVector roomsWithTag(const QString& tagName) const; + QMap users() const; // FIXME: Convert Q_INVOKABLEs to Q_PROPERTIES diff --git a/room.cpp b/room.cpp index 762e929c..29244da2 100644 --- a/room.cpp +++ b/room.cpp @@ -95,6 +95,8 @@ class Room::Private QString firstDisplayedEventId; QString lastDisplayedEventId; QHash lastReadEventIds; + QHash tags; + QHash accountData; QString prevBatch; QPointer roomMessagesJob; @@ -552,6 +554,16 @@ void Room::resetHighlightCount() emit highlightCountChanged(this); } +QStringList Room::tagNames() const +{ + return d->tags.keys(); +} + +const QHash& Room::tags() const +{ + return d->tags; +} + const RoomMessageEvent* Room::Private::getEventWithFile(const QString& eventId) const { @@ -873,6 +885,15 @@ void Room::updateData(SyncRoomData&& data) << et.elapsed() << "ms"; } + if (!data.accountData.empty()) + { + et.restart(); + for (auto&& event: data.accountData) + processAccountDataEvent(move(event)); + qCDebug(PROFILER) << "*** Room::processAccountData():" + << et.elapsed() << "ms"; + } + if( data.highlightCount != d->highlightCount ) { d->highlightCount = data.highlightCount; @@ -1433,6 +1454,19 @@ void Room::processEphemeralEvent(EventPtr event) } } +void Room::processAccountDataEvent(EventPtr event) +{ + switch (event->type()) + { + case EventType::Tag: + d->tags = static_cast(event.get())->tags(); + emit tagsChanged(); + break; + default: + d->accountData[event->jsonType()] = event->contentJson(); + } +} + QString Room::Private::roomNameFromMemberNames(const QList &userlist) const { // This is part 3(i,ii,iii) in the room displayname algorithm described diff --git a/room.h b/room.h index 5253a7c6..6dba6156 100644 --- a/room.h +++ b/room.h @@ -20,6 +20,7 @@ #include "jobs/syncjob.h" #include "events/roommessageevent.h" +#include "events/tagevent.h" #include "joinstate.h" #include @@ -116,6 +117,8 @@ namespace QMatrixClient Q_PROPERTY(QString lastDisplayedEventId READ lastDisplayedEventId WRITE setLastDisplayedEventId NOTIFY lastDisplayedEventChanged) Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE markMessagesAsRead NOTIFY readMarkerMoved) + Q_PROPERTY(QStringList tagNames READ tagNames NOTIFY tagsChanged) + public: using Timeline = std::deque; using rev_iter_t = Timeline::const_reverse_iterator; @@ -237,6 +240,9 @@ namespace QMatrixClient Q_INVOKABLE int highlightCount() const; Q_INVOKABLE void resetHighlightCount(); + QStringList tagNames() const; + const QHash& tags() const; + Q_INVOKABLE QUrl urlToThumbnail(const QString& eventId); Q_INVOKABLE QUrl urlToDownload(const QString& eventId); Q_INVOKABLE QString fileNameToDownload(const QString& eventId); @@ -318,6 +324,8 @@ namespace QMatrixClient void readMarkerMoved(); void unreadMessagesChanged(Room* room); + void tagsChanged(); + void replacedEvent(const RoomEvent* newEvent, const RoomEvent* oldEvent); @@ -330,6 +338,7 @@ namespace QMatrixClient protected: virtual void processStateEvents(const RoomEvents& events); virtual void processEphemeralEvent(EventPtr event); + virtual void processAccountDataEvent(EventPtr event); virtual void onAddNewTimelineEvents(timeline_iter_t from) { } virtual void onAddHistoricalTimelineEvents(rev_iter_t from) { } virtual void onRedaction(const RoomEvent* prevEvent, -- cgit v1.2.3 From 41171cc181c4bc48ba2bdc39b582d0f6c2fdde0d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Feb 2018 19:04:03 +0900 Subject: qmc-example: Single-shot sync instead of continuous; room tags; code cleanup Single-shot sync is now used because with that qmc-example can be used as a crude auto-testing tool. --- examples/qmc-example.cpp | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/examples/qmc-example.cpp b/examples/qmc-example.cpp index dbb9912b..857c1190 100644 --- a/examples/qmc-example.cpp +++ b/examples/qmc-example.cpp @@ -4,12 +4,10 @@ #include #include -#include using namespace QMatrixClient; using std::cout; using std::endl; -using std::string; void onNewRoom(Room* r) { @@ -17,39 +15,56 @@ void onNewRoom(Room* r) QObject::connect(r, &Room::namesChanged, [=] { cout << "Room " << r->id().toStdString() << ", name(s) changed:" << endl << " Name: " << r->name().toStdString() << endl - << " Canonical alias: " << r->canonicalAlias().toStdString() + << " Canonical alias: " << r->canonicalAlias().toStdString() << endl << endl << endl; }); + QObject::connect(r, &Room::tagsChanged, [=] { + cout << "Room " << r->id().toStdString() << ", tag(s) changed:" << endl + << " " << r->tagNames().join(", ").toStdString() << endl << endl; + }); QObject::connect(r, &Room::aboutToAddNewMessages, [=] (RoomEventsRange timeline) { cout << timeline.size() << " new event(s) in room " - << r->id().toStdString() << ":" << endl; + << r->id().toStdString() << ":" + << endl; for (const auto& item: timeline) { cout << "From: " << r->roomMembername(item->senderId()).toStdString() << endl << "Timestamp:" << item->timestamp().toString().toStdString() << endl - << "JSON:" << endl << string(item->originalJson()) << endl; + << "JSON:" << endl << item->originalJson().toStdString() << endl; } }); } +void finalize(Connection* conn) +{ + cout << "Logging out" << endl; + conn->logout(); + QObject::connect(conn, &Connection::loggedOut, QCoreApplication::instance(), + [conn] { + conn->deleteLater(); + QCoreApplication::instance()->processEvents(); + QCoreApplication::instance()->quit(); + }); +} + int main(int argc, char* argv[]) { QCoreApplication app(argc, argv); - if (argc < 2) + if (argc < 3) return -1; - auto conn = new Connection(QUrl("https://matrix.org")); + cout << "Connecting to the server as " << argv[1] << endl; + auto conn = new Connection; conn->connectToServer(argv[1], argv[2], "QMatrixClient example application"); - auto c = QObject::connect(conn, &Connection::connected, [=] { - cout << "Connected" << endl; - conn->sync(); - }); - QObject::connect(conn, &Connection::syncDone, [=] { - cout << "Sync done" << endl; - conn->sync(30000); + QObject::connect(conn, &Connection::connected, [=] { + cout << "Connected, server: " + << conn->homeserver().toDisplayString().toStdString() << endl; + cout << "Access token: " << conn->accessToken().toStdString() << endl; + conn->sync(); }); QObject::connect(conn, &Connection::newRoom, onNewRoom); + QObject::connect(conn, &Connection::syncDone, std::bind(finalize, conn)); return app.exec(); } -- cgit v1.2.3 From e18be10126ad06d217d855fe1bab9383aa508894 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Feb 2018 19:05:11 +0900 Subject: Travis CI: an attempt to introduce automatic testing Adding valgrind to the mix. --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 1b67119d..cf1b1397 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ addons: packages: - g++-5 - qt56base + - valgrind matrix: include: @@ -28,7 +29,8 @@ script: - cmake .. - cmake --build . --target all - cd .. -- qmake qmc-example.pro "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" && make all +- qmake qmc-example.pro "CONFIG += debug" "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" && make all +- valgrind --tool=memcheck --leak-check=yes --show-reachable=yes ./qmc-example "$QMC_TEST_USER" "$QMC_TEST_PWD" notifications: webhooks: -- cgit v1.2.3 From 06cc6a80a44a510db1e11ebe8795c8e13a0156e2 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 09:45:49 +0900 Subject: Fix (not working for quite a long time) CI on OSX --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index cf1b1397..14de125e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,7 +18,7 @@ matrix: - os: linux compiler: clang - os: osx - env: [ ENV_EVAL="brew update && brew install qt5 && PATH=/usr/local/opt/qt/bin" ] + env: [ ENV_EVAL="brew update && brew install qt5 && PATH=/usr/local/opt/qt/bin:$PATH" ] before_install: - eval "${ENV_EVAL}" -- cgit v1.2.3 From 4bb5f23d1b70136769a8df76907acb75aa824af9 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 09:53:46 +0900 Subject: Fix on the previous fix (sorry for the mess) --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 14de125e..ff255ac1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,7 +18,7 @@ matrix: - os: linux compiler: clang - os: osx - env: [ ENV_EVAL="brew update && brew install qt5 && PATH=/usr/local/opt/qt/bin:$PATH" ] + env: [ 'ENV_EVAL="brew update && brew install qt5 && PATH=/usr/local/opt/qt/bin:$PATH"' ] before_install: - eval "${ENV_EVAL}" -- cgit v1.2.3 From 979756e26af57e715efe64f8de8068243fa27e9f Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 11:09:34 +0900 Subject: Deal with memory more carefully Plugs some memory leaks reported by Valgrind. --- connection.cpp | 2 +- events/event.cpp | 3 ++- events/event.h | 2 +- networkaccessmanager.cpp | 7 ++++--- networkaccessmanager.h | 2 +- user.cpp | 5 +++++ 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/connection.cpp b/connection.cpp index 52fcc40b..7b72c592 100644 --- a/connection.cpp +++ b/connection.cpp @@ -599,7 +599,7 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) { qCDebug(MAIN) << "Deleting Invite state for room" << prevInvite->id(); emit aboutToDeleteRoom(prevInvite); - delete prevInvite; + prevInvite->deleteLater(); } } diff --git a/events/event.cpp b/events/event.cpp index b55c44c4..74a2c3d7 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -120,7 +120,8 @@ RoomEvent::RoomEvent(Type type, const QJsonObject& rep) auto redaction = unsignedData.value("redacted_because"); if (redaction.isObject()) { - _redactedBecause.reset(new RedactionEvent(redaction.toObject())); + _redactedBecause = + std::make_unique(redaction.toObject()); return; } diff --git a/events/event.h b/events/event.h index 4bd08b55..f0ca2d15 100644 --- a/events/event.h +++ b/events/event.h @@ -100,7 +100,7 @@ namespace QMatrixClient { auto e = _impl::doMakeEvent(obj); if (!e) - e.reset(new EventT(EventType::Unknown, obj)); + e = std::make_unique(EventType::Unknown, obj); return e; } diff --git a/networkaccessmanager.cpp b/networkaccessmanager.cpp index 7fb2f602..89967a8a 100644 --- a/networkaccessmanager.cpp +++ b/networkaccessmanager.cpp @@ -19,6 +19,7 @@ #include "networkaccessmanager.h" #include +#include using namespace QMatrixClient; @@ -28,7 +29,7 @@ class NetworkAccessManager::Private QList ignoredSslErrors; }; -NetworkAccessManager::NetworkAccessManager() : d(std::make_unique()) +NetworkAccessManager::NetworkAccessManager(QObject* parent) : d(std::make_unique()) { } QList NetworkAccessManager::ignoredSslErrors() const @@ -48,7 +49,7 @@ void NetworkAccessManager::clearIgnoredSslErrors() static NetworkAccessManager* createNam() { - auto nam = new NetworkAccessManager; + auto nam = new NetworkAccessManager(QCoreApplication::instance()); // See #109. Once Qt bearer management gets better, this workaround // should become unnecessary. nam->connect(nam, &QNetworkAccessManager::networkAccessibleChanged, @@ -56,7 +57,7 @@ static NetworkAccessManager* createNam() return nam; } -NetworkAccessManager*NetworkAccessManager::instance() +NetworkAccessManager* NetworkAccessManager::instance() { static auto* nam = createNam(); return nam; diff --git a/networkaccessmanager.h b/networkaccessmanager.h index ea08c591..ae847582 100644 --- a/networkaccessmanager.h +++ b/networkaccessmanager.h @@ -28,7 +28,7 @@ namespace QMatrixClient { Q_OBJECT public: - NetworkAccessManager(); + NetworkAccessManager(QObject* parent = nullptr); ~NetworkAccessManager() override; QList ignoredSslErrors() const; diff --git a/user.cpp b/user.cpp index 9cdbb420..308b217c 100644 --- a/user.cpp +++ b/user.cpp @@ -48,6 +48,11 @@ class User::Private Private(QString userId, Connection* connection) : userId(move(userId)), connection(connection) { } + ~Private() + { + for (auto a: otherAvatars) + delete a; + } QString userId; Connection* connection; -- cgit v1.2.3 From 431eea5f4a03f8a5622eaf30f45187ad51e50b42 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 16:50:01 +0900 Subject: Travis CI: Skip valgrind on OSX; send a message to a test room on every run --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index ff255ac1..777d956a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,7 +30,7 @@ script: - cmake --build . --target all - cd .. - qmake qmc-example.pro "CONFIG += debug" "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" && make all -- valgrind --tool=memcheck --leak-check=yes --show-reachable=yes ./qmc-example "$QMC_TEST_USER" "$QMC_TEST_PWD" +- if [ "$TRAVIS_OS_NAME" = "linux" ]; then valgrind --tool=memcheck --leak-check=yes --show-reachable=yes ./qmc-example "$QMC_TEST_USER" "$QMC_TEST_PWD" '#qmc-test:matrix.org' notifications: webhooks: -- cgit v1.2.3 From 4b5d5d20de1a3c00c77e395260f572c2b90f54f4 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 17:26:15 +0900 Subject: qmc-example: Optionally send a test message; don't dump incoming messages to cout The room alias is the third parameter (after user and password). --- examples/qmc-example.cpp | 50 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/examples/qmc-example.cpp b/examples/qmc-example.cpp index 857c1190..caea562f 100644 --- a/examples/qmc-example.cpp +++ b/examples/qmc-example.cpp @@ -1,15 +1,19 @@ #include "connection.h" #include "room.h" +#include "user.h" -#include +#include +#include #include using namespace QMatrixClient; using std::cout; using std::endl; +using std::bind; +using namespace std::placeholders; -void onNewRoom(Room* r) +void onNewRoom(Room* r, const char* targetRoomName) { cout << "New room: " << r->id().toStdString() << endl; QObject::connect(r, &Room::namesChanged, [=] { @@ -17,6 +21,20 @@ void onNewRoom(Room* r) << " Name: " << r->name().toStdString() << endl << " Canonical alias: " << r->canonicalAlias().toStdString() << endl << endl << endl; + if (targetRoomName && (r->name() == targetRoomName || + r->canonicalAlias() == targetRoomName)) + { + r->postMessage( + "This is a test message from an example application\n" + "The current user is " % r->localUser()->fullName(r) % "\n" % + QStringLiteral("This room has %1 member(s)") + .arg(r->memberCount()) % "\n" % + "The room is " % + (r->isDirectChat() ? "" : "not ") % "a direct chat\n" % + "Have a good day", + MessageEventType::Notice + ); + } }); QObject::connect(r, &Room::tagsChanged, [=] { cout << "Room " << r->id().toStdString() << ", tag(s) changed:" << endl @@ -24,16 +42,15 @@ void onNewRoom(Room* r) }); QObject::connect(r, &Room::aboutToAddNewMessages, [=] (RoomEventsRange timeline) { cout << timeline.size() << " new event(s) in room " - << r->id().toStdString() << ":" - << endl; - for (const auto& item: timeline) - { - cout << "From: " - << r->roomMembername(item->senderId()).toStdString() - << endl << "Timestamp:" - << item->timestamp().toString().toStdString() << endl - << "JSON:" << endl << item->originalJson().toStdString() << endl; - } + << r->id().toStdString() << endl; +// for (const auto& item: timeline) +// { +// cout << "From: " +// << r->roomMembername(item->senderId()).toStdString() +// << endl << "Timestamp:" +// << item->timestamp().toString().toStdString() << endl +// << "JSON:" << endl << item->originalJson().toStdString() << endl; +// } }); } @@ -64,7 +81,12 @@ int main(int argc, char* argv[]) cout << "Access token: " << conn->accessToken().toStdString() << endl; conn->sync(); }); - QObject::connect(conn, &Connection::newRoom, onNewRoom); - QObject::connect(conn, &Connection::syncDone, std::bind(finalize, conn)); + const char* targetRoomName = argc >= 4 ? argv[3] : nullptr; + if (targetRoomName) + cout << "Target room name: " << targetRoomName; + QObject::connect(conn, &Connection::newRoom, + bind(onNewRoom, _1, targetRoomName)); + QObject::connect(conn, &Connection::syncDone, + bind(finalize, conn)); return app.exec(); } -- cgit v1.2.3 From 967b18197aa262751f357c25d812e63b9bcce8b2 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 17:27:44 +0900 Subject: TagEvent: drop unneeded methods; add a license block to the .cpp file Those methods are more appropriate for Room. --- events/tagevent.cpp | 28 ++++++++++++++++++---------- events/tagevent.h | 8 +++----- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/events/tagevent.cpp b/events/tagevent.cpp index 9f381c76..c6297003 100644 --- a/events/tagevent.cpp +++ b/events/tagevent.cpp @@ -1,3 +1,21 @@ +/****************************************************************************** + * Copyright (C) 2018 Kitsune Ral + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + #include "tagevent.h" using namespace QMatrixClient; @@ -26,16 +44,6 @@ QHash TagEvent::tags() const return result; } -bool TagEvent::isFavourite() const -{ - return tagsObject().contains("m.favourite"); -} - -bool TagEvent::isLowPriority() const -{ - return tagsObject().contains("m.lowpriority"); -} - QJsonObject TagEvent::tagsObject() const { return contentJson().value("tags").toObject(); diff --git a/events/tagevent.h b/events/tagevent.h index 23b0b703..44a7e49a 100644 --- a/events/tagevent.h +++ b/events/tagevent.h @@ -22,6 +22,9 @@ namespace QMatrixClient { + static constexpr const char* FavouriteTag = "m.favourite"; + static constexpr const char* LowPriorityTag = "m.lowpriority"; + struct TagRecord { explicit TagRecord(const QJsonObject& json = {}); @@ -40,11 +43,6 @@ namespace QMatrixClient /** Get the list of tags along with information on each */ QHash tags() const; - /** Check whether the list of tags has m.favourite */ - bool isFavourite() const; - /** Check whether the list of tags has m.lowpriority */ - bool isLowPriority() const; - static constexpr const char * TypeId = "m.tag"; protected: -- cgit v1.2.3 From 37b67cbd07beca1b6f010794a7ff1920bc053e6e Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 17:50:56 +0900 Subject: qmc-example: Temporarily comment out the code that uses uncommitted features --- examples/qmc-example.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/qmc-example.cpp b/examples/qmc-example.cpp index caea562f..e0aabca9 100644 --- a/examples/qmc-example.cpp +++ b/examples/qmc-example.cpp @@ -29,8 +29,8 @@ void onNewRoom(Room* r, const char* targetRoomName) "The current user is " % r->localUser()->fullName(r) % "\n" % QStringLiteral("This room has %1 member(s)") .arg(r->memberCount()) % "\n" % - "The room is " % - (r->isDirectChat() ? "" : "not ") % "a direct chat\n" % +// "The room is " % +// (r->isDirectChat() ? "" : "not ") % "a direct chat\n" % "Have a good day", MessageEventType::Notice ); -- cgit v1.2.3 From f366cb25d1a5256341c1253fb04e36bd70373a70 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 27 Feb 2018 11:36:42 +0900 Subject: Room: Save tags in the cache; isFavourite() and isLowPriority() --- connection.cpp | 2 +- room.cpp | 29 +++++++++++++++++++++++++++++ room.h | 9 +++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/connection.cpp b/connection.cpp index 7b72c592..9a5a5a4e 100644 --- a/connection.cpp +++ b/connection.cpp @@ -627,7 +627,7 @@ void Connection::setHomeserver(const QUrl& url) emit homeserverChanged(homeserver()); } -static constexpr int CACHE_VERSION_MAJOR = 2; +static constexpr int CACHE_VERSION_MAJOR = 3; static constexpr int CACHE_VERSION_MINOR = 0; void Connection::saveState(const QUrl &toFile) const diff --git a/room.cpp b/room.cpp index 29244da2..db36a713 100644 --- a/room.cpp +++ b/room.cpp @@ -564,6 +564,21 @@ const QHash& Room::tags() const return d->tags; } +TagRecord Room::tag(const QString& name) const +{ + return d->tags.value(name); +} + +bool Room::isFavourite() const +{ + return d->tags.contains(FavouriteTag); +} + +bool Room::isLowPriority() const +{ + return d->tags.contains(LowPriorityTag); +} + const RoomMessageEvent* Room::Private::getEventWithFile(const QString& eventId) const { @@ -1635,6 +1650,20 @@ QJsonObject Room::Private::toJson() const result.insert("ephemeral", ephemeralObj); } + { + QJsonObject accountDataObj; + if (!tags.empty()) + { + QJsonObject tagsObj; + for (auto it = tags.begin(); it != tags.end(); ++it) + tagsObj.insert(it.key(), { {"order", it->order} }); + if (!tagsObj.empty()) + accountDataObj.insert("m.tag", tagsObj); + } + if (!accountDataObj.empty()) + result.insert("account_data", accountDataObj); + } + QJsonObject unreadNotificationsObj; if (highlightCount > 0) unreadNotificationsObj.insert("highlight_count", highlightCount); diff --git a/room.h b/room.h index 6dba6156..8e27a608 100644 --- a/room.h +++ b/room.h @@ -242,6 +242,15 @@ namespace QMatrixClient QStringList tagNames() const; const QHash& tags() const; + TagRecord tag(const QString& name) const; + + /** Check whether the list of tags has m.favourite */ + bool isFavourite() const; + /** Check whether the list of tags has m.lowpriority */ + bool isLowPriority() const; + + /** Check whether this room is a direct chat */ + bool isDirectChat() const; Q_INVOKABLE QUrl urlToThumbnail(const QString& eventId); Q_INVOKABLE QUrl urlToDownload(const QString& eventId); -- cgit v1.2.3 From efeeca097a3c69991683615366f07625855ba2ac Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 27 Feb 2018 11:51:06 +0900 Subject: Travis CI: Fix a typo --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 777d956a..b75418f2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,7 +30,7 @@ script: - cmake --build . --target all - cd .. - qmake qmc-example.pro "CONFIG += debug" "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" && make all -- if [ "$TRAVIS_OS_NAME" = "linux" ]; then valgrind --tool=memcheck --leak-check=yes --show-reachable=yes ./qmc-example "$QMC_TEST_USER" "$QMC_TEST_PWD" '#qmc-test:matrix.org' +- if [ "$TRAVIS_OS_NAME" = "linux" ]; then valgrind --tool=memcheck --leak-check=yes --show-reachable=yes ./qmc-example "$QMC_TEST_USER" "$QMC_TEST_PWD" '#qmc-test:matrix.org'; fi notifications: webhooks: -- cgit v1.2.3