From 6572b4836231597de6a340296ee07fbeb13ebece Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 9 Dec 2017 22:23:21 +0900 Subject: EventType: Add more enumeration values --- events/event.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'events') diff --git a/events/event.h b/events/event.h index cc99b57b..20e7012d 100644 --- a/events/event.h +++ b/events/event.h @@ -37,11 +37,12 @@ namespace QMatrixClient Typing, Receipt, RoomEventBase = 0x1000, RoomMessage = RoomEventBase + 1, - RoomEncryptedMessage, + RoomEncryptedMessage, Redaction, RoomStateEventBase = 0x1800, RoomName = RoomStateEventBase + 1, RoomAliases, RoomCanonicalAlias, RoomMember, RoomTopic, - RoomAvatar, RoomEncryption, + RoomAvatar, RoomEncryption, RoomCreate, RoomJoinRules, + RoomPowerLevels, Reserved = 0x2000 }; -- cgit v1.2.3 From c38366210643ef0956884531910d7ece3d6a4cac Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 9 Dec 2017 22:24:16 +0900 Subject: Introduce RoomEventsView Will be used in Room to work with ranges of events. --- events/event.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'events') diff --git a/events/event.h b/events/event.h index 20e7012d..8681f4db 100644 --- a/events/event.h +++ b/events/event.h @@ -156,6 +156,27 @@ namespace QMatrixClient }; using RoomEvents = EventsBatch; + /** + * Conceptually similar to QStringView (but much more primitive), it's a + * simple abstraction over a pair of RoomEvents::const_iterator values + * referring to the beginning and the end of a range in a RoomEvents + * container. + */ + struct RoomEventsView + { + RoomEvents::const_iterator from; + RoomEvents::const_iterator to; + + RoomEvents::size_type size() const + { + Q_ASSERT(std::distance(from, to) >= 0); + return RoomEvents::size_type(std::distance(from, to)); + } + bool empty() const { return from == to; } + RoomEvents::const_iterator begin() const { return from; } + RoomEvents::const_iterator end() const { return to; } + }; + template class StateEvent: public RoomEvent { -- cgit v1.2.3 From 6ecdefd8463c0b393dc51535ddb4b4283b5891d8 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 10 Dec 2017 09:49:08 +0900 Subject: RedactionEvent and RoomEvent::redactedBecause() A RoomEvent now has an optional pointer to a RedactionEvent that is non-null if the room event is redacted. transactionId and serverTimestamp are only filled if the event is not redacted. There's no way to construct a redacted event as of yet. --- events/event.cpp | 31 ++++++++++++++++++++++++++----- events/event.h | 18 ++++++++++++++++-- events/redactionevent.cpp | 1 + events/redactionevent.h | 43 +++++++++++++++++++++++++++++++++++++++++++ events/roommessageevent.cpp | 6 +++--- libqmatrixclient.pri | 2 ++ 6 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 events/redactionevent.cpp create mode 100644 events/redactionevent.h (limited to 'events') diff --git a/events/event.cpp b/events/event.cpp index 44b742c1..d779f293 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -24,6 +24,7 @@ #include "roomavatarevent.h" #include "typingevent.h" #include "receiptevent.h" +#include "redactionevent.h" #include "logging.h" #include @@ -33,7 +34,8 @@ using namespace QMatrixClient; Event::Event(Type type, const QJsonObject& rep) : _type(type), _originalJson(rep) { - if (!rep.contains("content")) + if (!rep.contains("content") && + !rep.value("unsigned").toObject().contains("redacted_because")) { qCWarning(EVENTS) << "Event without 'content' node"; qCWarning(EVENTS) << formatJson << rep; @@ -80,13 +82,12 @@ Event* Event::fromJson(const QJsonObject& obj) TypingEvent, ReceiptEvent>(obj, obj["type"].toString()); } +RoomEvent::RoomEvent(Event::Type type) : Event(type) { } + RoomEvent::RoomEvent(Type type, const QJsonObject& rep) : Event(type, rep), _id(rep["event_id"].toString()) - , _serverTimestamp( - QMatrixClient::fromJson(rep["origin_server_ts"])) , _roomId(rep["room_id"].toString()) , _senderId(rep["sender"].toString()) - , _txnId(rep["unsigned"].toObject().value("transactionId").toString()) { // if (_id.isEmpty()) // { @@ -103,10 +104,29 @@ RoomEvent::RoomEvent(Type type, const QJsonObject& rep) // qCWarning(EVENTS) << "Can't find sender in a room event"; // qCWarning(EVENTS) << formatJson << rep; // } + auto unsignedData = rep["unsigned"].toObject(); + auto redaction = unsignedData.value("redacted_because"); + if (redaction.isObject()) + { + _redactedBecause.reset(new RedactionEvent(redaction.toObject())); + return; + } + + _serverTimestamp = + QMatrixClient::fromJson(rep["origin_server_ts"]); + _txnId = unsignedData.value("transactionId").toString(); if (!_txnId.isEmpty()) qCDebug(EVENTS) << "Event transactionId:" << _txnId; } +RoomEvent::~RoomEvent() +{ /* Let QScopedPointer do its job */ } + +QString RoomEvent::redactionReason() const +{ + return isRedacted() ? _redactedBecause->reason() : QString{}; +} + void RoomEvent::addId(const QString& id) { Q_ASSERT(_id.isEmpty()); Q_ASSERT(!id.isEmpty()); @@ -118,5 +138,6 @@ RoomEvent* RoomEvent::fromJson(const QJsonObject& obj) return makeIfMatches(obj, obj["type"].toString()); + RoomAvatarEvent, EncryptionEvent, RedactionEvent> + (obj, obj["type"].toString()); } diff --git a/events/event.h b/events/event.h index 8681f4db..62d9c23b 100644 --- a/events/event.h +++ b/events/event.h @@ -104,6 +104,8 @@ namespace QMatrixClient }; using Events = EventsBatch; + class RedactionEvent; + /** This class corresponds to m.room.* events */ class RoomEvent : public Event { @@ -112,15 +114,26 @@ namespace QMatrixClient Q_PROPERTY(QDateTime timestamp READ timestamp CONSTANT) Q_PROPERTY(QString roomId READ roomId CONSTANT) Q_PROPERTY(QString senderId READ senderId CONSTANT) - Q_PROPERTY(QString transactionId READ transactionId CONSTANT) + Q_PROPERTY(QString redactionReason READ redactionReason) + Q_PROPERTY(bool isRedacted READ isRedacted) + Q_PROPERTY(QString transactionId READ transactionId) public: - explicit RoomEvent(Type type) : Event(type) { } + // RedactionEvent is an incomplete type here so we cannot inline + // constructors and destructors + explicit RoomEvent(Type type); 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; } + bool isRedacted() const { return redactedBecause(); } + RedactionEvent* redactedBecause() const + { + return _redactedBecause.data(); + } + QString redactionReason() const; const QString& transactionId() const { return _txnId; } /** @@ -152,6 +165,7 @@ namespace QMatrixClient QDateTime _serverTimestamp; QString _roomId; QString _senderId; + QScopedPointer _redactedBecause; QString _txnId; }; using RoomEvents = EventsBatch; diff --git a/events/redactionevent.cpp b/events/redactionevent.cpp new file mode 100644 index 00000000..bf467718 --- /dev/null +++ b/events/redactionevent.cpp @@ -0,0 +1 @@ +#include "redactionevent.h" diff --git a/events/redactionevent.h b/events/redactionevent.h new file mode 100644 index 00000000..fa6902ab --- /dev/null +++ b/events/redactionevent.h @@ -0,0 +1,43 @@ +/****************************************************************************** + * Copyright (C) 2017 Kitsune Ral + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#pragma once + +#include "event.h" + +namespace QMatrixClient +{ + class RedactionEvent : public RoomEvent + { + public: + static constexpr const char* const TypeId = "m.room.redaction"; + + RedactionEvent(const QJsonObject& obj) + : RoomEvent(Type::Redaction, obj) + , _redactedEvent(obj.value("redacts").toString()) + , _reason(contentJson().value("reason").toString()) + { } + + const QString& redactedEvent() const { return _redactedEvent; } + const QString& reason() const { return _reason; } + + private: + QString _redactedEvent; + QString _reason; + }; +} // namespace QMatrixClient diff --git a/events/roommessageevent.cpp b/events/roommessageevent.cpp index f06474e9..bc41abf6 100644 --- a/events/roommessageevent.cpp +++ b/events/roommessageevent.cpp @@ -58,7 +58,6 @@ QString msgTypeToJson(MsgType enumType) if (it != msgTypes.end()) return it->jsonType; - qCCritical(EVENTS) << "Unknown msgtype:" << enumType; return {}; } @@ -69,8 +68,7 @@ MsgType jsonToMsgType(const QString& jsonType) if (it != msgTypes.end()) return it->enumType; - qCCritical(EVENTS) << "Unknown msgtype:" << jsonType; - return {}; + return MsgType::Unknown; } RoomMessageEvent::RoomMessageEvent(const QString& plainBody, @@ -81,6 +79,8 @@ RoomMessageEvent::RoomMessageEvent(const QString& plainBody, RoomMessageEvent::RoomMessageEvent(const QJsonObject& obj) : RoomEvent(Type::RoomMessage, obj), _content(nullptr) { + if (isRedacted()) + return; const QJsonObject content = contentJson(); if ( content.contains("msgtype") && content.contains("body") ) { diff --git a/libqmatrixclient.pri b/libqmatrixclient.pri index 86648860..49442197 100644 --- a/libqmatrixclient.pri +++ b/libqmatrixclient.pri @@ -18,6 +18,7 @@ HEADERS += \ $$PWD/events/roomavatarevent.h \ $$PWD/events/typingevent.h \ $$PWD/events/receiptevent.h \ + $$PWD/events/redactionevent.h \ $$PWD/jobs/basejob.h \ $$PWD/jobs/checkauthmethods.h \ $$PWD/jobs/passwordlogin.h \ @@ -44,6 +45,7 @@ SOURCES += \ $$PWD/events/roommemberevent.cpp \ $$PWD/events/typingevent.cpp \ $$PWD/events/receiptevent.cpp \ + $$PWD/events/redactionevent.cpp \ $$PWD/jobs/basejob.cpp \ $$PWD/jobs/checkauthmethods.cpp \ $$PWD/jobs/passwordlogin.cpp \ -- cgit v1.2.3 From 71185be83b646c7d5a2d6d3dc0306710e1f6fdd0 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 11 Dec 2017 19:24:38 +0900 Subject: Whitelist origin_server_ts in redaction logic The Spec doesn't mention it but both Synapse and Riot act as if origin_server_ts was whitelisted, and it was also confirmed in #matrix-dev to be reasonable behaviour. --- events/event.cpp | 4 ++-- room.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'events') diff --git a/events/event.cpp b/events/event.cpp index d779f293..354aed03 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -87,6 +87,8 @@ 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()) + , _serverTimestamp( + QMatrixClient::fromJson(rep["origin_server_ts"])) , _senderId(rep["sender"].toString()) { // if (_id.isEmpty()) @@ -112,8 +114,6 @@ RoomEvent::RoomEvent(Type type, const QJsonObject& rep) return; } - _serverTimestamp = - QMatrixClient::fromJson(rep["origin_server_ts"]); _txnId = unsignedData.value("transactionId").toString(); if (!_txnId.isEmpty()) qCDebug(EVENTS) << "Event transactionId:" << _txnId; diff --git a/room.cpp b/room.cpp index dfca6bde..7f4d7684 100644 --- a/room.cpp +++ b/room.cpp @@ -716,7 +716,7 @@ void Room::Private::processRedaction(const RedactionEvent* redaction) } static const QStringList keepKeys = { "event_id", "type", "room_id", "sender", "state_key", - "prev_content", "content" }; + "prev_content", "content", "origin_server_ts" }; static const std::vector> keepContentKeysMap { { Event::Type::RoomMember, { "membership" } } -- cgit v1.2.3 From bc1b65f872edf5f493f5fcc9792a37f03d6b7794 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 12 Dec 2017 10:16:55 +0900 Subject: Fix a compiler warning --- events/event.cpp | 2 +- events/event.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'events') diff --git a/events/event.cpp b/events/event.cpp index 354aed03..1e2d89f2 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -87,9 +87,9 @@ 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"])) - , _senderId(rep["sender"].toString()) { // if (_id.isEmpty()) // { diff --git a/events/event.h b/events/event.h index 62d9c23b..fa8a58af 100644 --- a/events/event.h +++ b/events/event.h @@ -162,9 +162,9 @@ namespace QMatrixClient private: QString _id; - QDateTime _serverTimestamp; QString _roomId; QString _senderId; + QDateTime _serverTimestamp; QScopedPointer _redactedBecause; QString _txnId; }; -- cgit v1.2.3 From 3a1f1eced05f5f7ca3244f009e111eed2e809119 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 13 Dec 2017 16:29:02 +0900 Subject: EventsBatch: document the class --- events/event.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'events') diff --git a/events/event.h b/events/event.h index fa8a58af..bd33bb50 100644 --- a/events/event.h +++ b/events/event.h @@ -83,10 +83,33 @@ namespace QMatrixClient }; using EventType = Event::Type; + /** + * \brief A vector of pointers to events with deserialisation capabilities + * + * This is a simple wrapper over a generic vector type that adds + * a convenience method to deserialise events from QJsonArray. + * Note that this type does not own pointers to events. If owning + * semantics is needed, one should use the Owning<> wrapper around + * the container (e.g. \code Owning> \endcode). + * \tparam EventT base type of all events in the vector + */ template class EventsBatch : public std::vector { public: + /** + * \brief Deserialise events from an array + * + * Given the following JSON construct, creates events from + * the array stored at key "node": + * \code + * "container": { + * "node": [ { "event_id": "!evt1:srv.org", ... }, ... ] + * } + * \endcode + * \param container - the wrapping JSON object + * \param node - the key in container that holds the array of events + */ void fromJson(const QJsonObject& container, const QString& node) { const auto objs = container.value(node).toArray(); -- cgit v1.2.3 From c5f480355c7d4c000a4ee73fd7f8107a9a9340c2 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 14 Dec 2017 18:41:55 +0900 Subject: Move all internal event pointers to std::unique_ptr<> This causes the following changes along the way: - Owning<> template is decommissioned. - event.h has been rearranged, and Event/RoomEvent::fromJson static methods have been replaced with an external makeEvent<> function template. A side effect of that is that one cannot use a factory with a type other than the one it's defined for (i.e. you cannot call makeEvent) but that feature has been out of use for long anyway. - Room::doAddNewMessageEvents() and Room::doAddHistoricalMessageEvents() have been removed, giving place to Room::onAddNewTimelineEvents() and Room::onAddHistoricalTimelineEvents(). The most important difference is that all code that must be executed now resides in addNewMessageEvents() (it moved from Room to Room::Private) and classes inheriting from Room are not obliged to call the overridden function from the overriding function (they can do it but those functions have empty bodies in Room). This was a long overdue change, and owning pointers simply mandated it. Room::onAddNewTimelineEvents/onAddHistoricalTimelineEvents should not do anything with the passed range in terms of ownership, it's just a way to allow the derived class to update his data in due course. - Room::Private::dropDuplicateEvents() and Room::Private::insertEvents(), notably, have been updated to work with owning pointers. insertEvents() move()s pointers to the timeline, while dropDuplicateEvents uses remove_if instead of stable_partition and doesn't explicitly delete event objects. Also, a bugfix: Event accidentally had not virtual destructor for quite a long time. According to the standard, deleting an object through a pointer to a base class without a virtual destructor leads to UB. So the fact that libqmatrixclient clients even worked all these months is mere coincidence and compiler authors good will :-D --- events/event.cpp | 23 +++-- events/event.h | 54 +++++++---- examples/qmc-example.cpp | 12 +-- jobs/roommessagesjob.cpp | 6 +- jobs/roommessagesjob.h | 2 +- jobs/syncjob.h | 2 +- room.cpp | 244 +++++++++++++++++++++++++---------------------- room.h | 36 +++---- util.h | 47 --------- 9 files changed, 205 insertions(+), 221 deletions(-) (limited to 'events') diff --git a/events/event.cpp b/events/event.cpp index 1e2d89f2..01f473ce 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -42,6 +42,8 @@ Event::Event(Type type, const QJsonObject& rep) } } +Event::~Event() = default; + QByteArray Event::originalJson() const { return QJsonDocument(_originalJson).toJson(); @@ -72,14 +74,15 @@ inline BaseEventT* makeIfMatches(const QJsonObject& o, const QString& selector) return makeIfMatches(o, selector); } -Event* Event::fromJson(const QJsonObject& obj) +template <> +EventPtr QMatrixClient::makeEvent(const QJsonObject& obj) { // Check more specific event types first - if (auto e = RoomEvent::fromJson(obj)) - return e; + if (auto e = makeEvent(obj)) + return EventPtr(move(e)); - return makeIfMatches(obj, obj["type"].toString()); + return EventPtr { makeIfMatches(obj, obj["type"].toString()) }; } RoomEvent::RoomEvent(Event::Type type) : Event(type) { } @@ -119,8 +122,7 @@ RoomEvent::RoomEvent(Type type, const QJsonObject& rep) qCDebug(EVENTS) << "Event transactionId:" << _txnId; } -RoomEvent::~RoomEvent() -{ /* Let QScopedPointer do its job */ } +RoomEvent::~RoomEvent() = default; // Let the smart pointer do its job QString RoomEvent::redactionReason() const { @@ -133,11 +135,12 @@ void RoomEvent::addId(const QString& id) _id = id; } -RoomEvent* RoomEvent::fromJson(const QJsonObject& obj) +template <> +RoomEventPtr QMatrixClient::makeEvent(const QJsonObject& obj) { - return makeIfMatches - (obj, obj["type"].toString()); + (obj, obj["type"].toString()) }; } diff --git a/events/event.h b/events/event.h index bd33bb50..2b18bb46 100644 --- a/events/event.h +++ b/events/event.h @@ -25,8 +25,21 @@ #include "util.h" +#include + namespace QMatrixClient { + template + using event_ptr_tt = std::unique_ptr; + + /** Create an event with proper type from a JSON object + * Use this factory template to detect the type from the JSON object + * contents (the detected event type should derive from the template + * parameter type) and create an event object of that type. + */ + template + event_ptr_tt makeEvent(const QJsonObject& obj); + class Event { Q_GADGET @@ -64,12 +77,6 @@ namespace QMatrixClient // (and in most cases it will be a combination of other fields // instead of "content" field). - /** Create an event with proper type from a JSON object - * Use this factory to detect the type from the JSON object contents - * and create an event object of that type. - */ - static Event* fromJson(const QJsonObject& obj); - protected: const QJsonObject contentJson() const; @@ -82,6 +89,10 @@ namespace QMatrixClient Q_PROPERTY(QJsonObject contentJson READ contentJson CONSTANT) }; using EventType = Event::Type; + using EventPtr = event_ptr_tt; + + template <> + EventPtr makeEvent(const QJsonObject& obj); /** * \brief A vector of pointers to events with deserialisation capabilities @@ -94,7 +105,7 @@ namespace QMatrixClient * \tparam EventT base type of all events in the vector */ template - class EventsBatch : public std::vector + class EventsBatch : public std::vector> { public: /** @@ -120,8 +131,10 @@ namespace QMatrixClient for (auto objValue: objs) { const auto o = objValue.toObject(); - auto e = EventT::fromJson(o); - this->push_back(e ? e : new EventT(EventType::Unknown, o)); + auto e { makeEvent(o) }; + if (!e) + e.reset(new EventT(EventType::Unknown, o)); + this->emplace_back(std::move(e)); } } }; @@ -151,10 +164,10 @@ namespace QMatrixClient const QDateTime& timestamp() const { return _serverTimestamp; } const QString& roomId() const { return _roomId; } const QString& senderId() const { return _senderId; } - bool isRedacted() const { return redactedBecause(); } - RedactionEvent* redactedBecause() const + bool isRedacted() const { return bool(_redactedBecause); } + const RedactionEvent* redactedBecause() const { - return _redactedBecause.data(); + return _redactedBecause.get(); } QString redactionReason() const; const QString& transactionId() const { return _txnId; } @@ -180,18 +193,19 @@ namespace QMatrixClient */ void addId(const QString& id); - // "Static override" of the one in Event - static RoomEvent* fromJson(const QJsonObject& obj); - private: QString _id; QString _roomId; QString _senderId; QDateTime _serverTimestamp; - QScopedPointer _redactedBecause; + event_ptr_tt _redactedBecause; QString _txnId; }; using RoomEvents = EventsBatch; + using RoomEventPtr = event_ptr_tt; + + template <> + RoomEventPtr makeEvent(const QJsonObject& obj); /** * Conceptually similar to QStringView (but much more primitive), it's a @@ -199,10 +213,10 @@ namespace QMatrixClient * referring to the beginning and the end of a range in a RoomEvents * container. */ - struct RoomEventsView + struct RoomEventsRange { - RoomEvents::const_iterator from; - RoomEvents::const_iterator to; + RoomEvents::iterator from; + RoomEvents::iterator to; RoomEvents::size_type size() const { @@ -212,6 +226,8 @@ namespace QMatrixClient bool empty() const { return from == to; } RoomEvents::const_iterator begin() const { return from; } RoomEvents::const_iterator end() const { return to; } + RoomEvents::iterator begin() { return from; } + RoomEvents::iterator end() { return to; } }; template diff --git a/examples/qmc-example.cpp b/examples/qmc-example.cpp index ff7944ce..dbb9912b 100644 --- a/examples/qmc-example.cpp +++ b/examples/qmc-example.cpp @@ -20,16 +20,16 @@ void onNewRoom(Room* r) << " Canonical alias: " << r->canonicalAlias().toStdString() << endl << endl; }); - QObject::connect(r, &Room::aboutToAddNewMessages, [=] (RoomEventsView events) { - cout << events.size() << " new event(s) in room " + QObject::connect(r, &Room::aboutToAddNewMessages, [=] (RoomEventsRange timeline) { + cout << timeline.size() << " new event(s) in room " << r->id().toStdString() << ":" << endl; - for (auto e: events) + for (const auto& item: timeline) { cout << "From: " - << r->roomMembername(e->senderId()).toStdString() + << r->roomMembername(item->senderId()).toStdString() << endl << "Timestamp:" - << e->timestamp().toString().toStdString() << endl - << "JSON:" << endl << string(e->originalJson()) << endl; + << item->timestamp().toString().toStdString() << endl + << "JSON:" << endl << string(item->originalJson()) << endl; } }); } diff --git a/jobs/roommessagesjob.cpp b/jobs/roommessagesjob.cpp index 9af1b3a6..e5568f17 100644 --- a/jobs/roommessagesjob.cpp +++ b/jobs/roommessagesjob.cpp @@ -23,7 +23,7 @@ using namespace QMatrixClient; class RoomMessagesJob::Private { public: - Owning events; + RoomEvents events; QString end; }; @@ -46,9 +46,9 @@ RoomMessagesJob::~RoomMessagesJob() delete d; } -RoomEvents RoomMessagesJob::releaseEvents() +RoomEvents&& RoomMessagesJob::releaseEvents() { - return d->events.release(); + return move(d->events); } QString RoomMessagesJob::end() const diff --git a/jobs/roommessagesjob.h b/jobs/roommessagesjob.h index 9680d52c..7b3fd9c9 100644 --- a/jobs/roommessagesjob.h +++ b/jobs/roommessagesjob.h @@ -34,7 +34,7 @@ namespace QMatrixClient FetchDirection dir = FetchDirection::Backward); virtual ~RoomMessagesJob(); - RoomEvents releaseEvents(); + RoomEvents&& releaseEvents(); QString end() const; protected: diff --git a/jobs/syncjob.h b/jobs/syncjob.h index 08bd773e..e9288486 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -30,7 +30,7 @@ namespace QMatrixClient { public: template - class Batch : public Owning> + class Batch : public EventsBatch { public: explicit Batch(QString k) : jsonKey(std::move(k)) { } diff --git a/room.cpp b/room.cpp index 1234c343..6c426de2 100644 --- a/room.cpp +++ b/room.cpp @@ -44,6 +44,7 @@ #include using namespace QMatrixClient; +using namespace std::placeholders; enum EventsPlacement : int { Older = -1, Newer = 1 }; @@ -101,20 +102,34 @@ class Room::Private void getPreviousContent(int limit = 10); - bool isEventNotable(const RoomEvent* e) const + bool isEventNotable(const TimelineItem& ti) const { - return !e->isRedacted() && - e->senderId() != connection->userId() && - e->type() == EventType::RoomMessage; + return !ti->isRedacted() && + ti->senderId() != connection->userId() && + ti->type() == EventType::RoomMessage; } - void insertEvents(RoomEventsView events, EventsPlacement placement); + void addNewMessageEvents(RoomEvents&& events); + void addHistoricalMessageEvents(RoomEvents&& events); + + /** + * @brief Move events into the timeline + * + * Insert events into the timeline, either new or historical. + * Pointers in the original container become empty, the ownership + * is passed to the timeline container. + * @param events - the range of events to be inserted + * @param placement - position and direction of insertion: Older for + * historical messages, Newer for new ones + */ + Timeline::size_type insertEvents(RoomEventsRange&& events, + EventsPlacement placement); /** * Removes events from the passed container that are already in the timeline */ void dropDuplicateEvents(RoomEvents* events) const; - void checkUnreadMessages(RoomEventsView events); + void checkUnreadMessages(timeline_iter_t from); void setLastReadEvent(User* u, const QString& eventId); rev_iter_pair_t promoteReadMarker(User* u, rev_iter_t newMarker, @@ -122,7 +137,13 @@ class Room::Private void markMessagesAsRead(rev_iter_t upToMarker); - void processRedaction(const RedactionEvent* redaction); + /** + * @brief Apply redaction to the timeline + * + * Tries to find an event in the timeline and redact it; deletes the + * redaction event whether the redacted event was found or not. + */ + void processRedaction(RoomEventPtr redactionEvent); QJsonObject toJson() const; @@ -250,9 +271,8 @@ Room::Private::promoteReadMarker(User* u, Room::rev_iter_t newMarker, setLastReadEvent(u, (*(eagerMarker - 1))->id()); if (isLocalUser(u) && unreadMessages) { - auto stillUnreadMessagesCount = - count_if(eagerMarker, timeline.cend(), - [=](const TimelineItem& ti) { return isEventNotable(ti.event()); }); + auto stillUnreadMessagesCount = count_if(eagerMarker, timeline.cend(), + bind(&Room::Private::isEventNotable, this, _1)); if (stillUnreadMessagesCount == 0) { @@ -437,12 +457,13 @@ void Room::Private::removeMemberFromMap(const QString& username, User* u) emit q->memberRenamed(formerNamesakes[0]); } -inline QByteArray makeErrorStr(const Event* e, QByteArray msg) +inline QByteArray makeErrorStr(const Event& e, QByteArray msg) { - return msg.append("; event dump follows:\n").append(e->originalJson()); + return msg.append("; event dump follows:\n").append(e.originalJson()); } -void Room::Private::insertEvents(RoomEventsView events, EventsPlacement placement) +Room::Timeline::size_type Room::Private::insertEvents(RoomEventsRange&& events, + EventsPlacement placement) { // Historical messages arrive in newest-to-oldest order, so the process for // them is symmetric to the one for new messages. @@ -450,22 +471,26 @@ void Room::Private::insertEvents(RoomEventsView events, EventsPlacement placemen placement == Older ? timeline.front().index() : timeline.back().index(); auto baseIndex = index; - for (const auto e: events) + for (auto&& e: events) { + const auto eId = e->id(); Q_ASSERT_X(e, __FUNCTION__, "Attempt to add nullptr to timeline"); - Q_ASSERT_X(!e->id().isEmpty(), __FUNCTION__, - makeErrorStr(e, "Event with empty id cannot be in the timeline")); - Q_ASSERT_X(!eventsIndex.contains(e->id()), __FUNCTION__, - makeErrorStr(e, "Event is already in the timeline; " + Q_ASSERT_X(!eId.isEmpty(), __FUNCTION__, + makeErrorStr(*e, + "Event with empty id cannot be in the timeline")); + Q_ASSERT_X(!eventsIndex.contains(eId), __FUNCTION__, + makeErrorStr(*e, "Event is already in the timeline; " "incoming events were not properly deduplicated")); if (placement == Older) - timeline.emplace_front(e, --index); + timeline.emplace_front(move(e), --index); else - timeline.emplace_back(e, ++index); - eventsIndex.insert(e->id(), index); - Q_ASSERT(q->findInTimeline(e->id())->event() == e); + timeline.emplace_back(move(e), ++index); + eventsIndex.insert(eId, index); + Q_ASSERT(q->findInTimeline(eId)->event()->id() == eId); } + // Pointers in "events" are empty now, but events.size() didn't change Q_ASSERT(int(events.size()) == (index - baseIndex) * int(placement)); + return events.size(); } void Room::Private::addMember(User *u) @@ -578,15 +603,15 @@ void Room::updateData(SyncRoomData&& data) << et.elapsed() << "ms," << data.timeline.size() << "events"; et.restart(); - addNewMessageEvents(data.timeline.release()); + d->addNewMessageEvents(move(data.timeline)); qCDebug(PROFILER) << "*** Room::addNewMessageEvents():" << et.elapsed() << "ms"; } if (!data.ephemeral.empty()) { et.restart(); - for( auto ephemeralEvent: data.ephemeral ) - processEphemeralEvent(ephemeralEvent); + for( auto&& ephemeralEvent: data.ephemeral ) + processEphemeralEvent(move(ephemeralEvent)); qCDebug(PROFILER) << "*** Room::processEphemeralEvents():" << et.elapsed() << "ms"; } @@ -638,7 +663,7 @@ void Room::Private::getPreviousContent(int limit) connect( roomMessagesJob, &RoomMessagesJob::result, [=]() { if( !roomMessagesJob->error() ) { - q->addHistoricalMessageEvents(roomMessagesJob->releaseEvents()); + addHistoricalMessageEvents(roomMessagesJob->releaseEvents()); prevBatch = roomMessagesJob->end(); } roomMessagesJob = nullptr; @@ -682,35 +707,35 @@ void Room::Private::dropDuplicateEvents(RoomEvents* events) const if (events->empty()) return; - // Collect all duplicate events at the end of the container - auto dupsBegin = - std::stable_partition(events->begin(), events->end(), - [&] (RoomEvent* e) { return !eventsIndex.contains(e->id()); }); - - if (dupsBegin != events->begin()) - { - // Check the batch itself for dups - auto eIt = events->begin(); - for (auto baseId = (*eIt)->id(); ++eIt < dupsBegin; baseId = (*eIt)->id()) - { - dupsBegin = - std::stable_partition(eIt, dupsBegin, - [&] (const RoomEvent* e) { return e->id() != baseId; }); - } - } + // Multiple-remove (by different criteria), single-erase + // 1. Check for duplicates against the timeline. + auto dupsBegin = remove_if(events->begin(), events->end(), + [&] (const RoomEventPtr& e) + { return eventsIndex.contains(e->id()); }); + + // 2. Check for duplicates within the batch if there are still events. + for (auto eIt = events->begin(); distance(eIt, dupsBegin) > 1; ++eIt) + dupsBegin = remove_if(eIt + 1, dupsBegin, + [&] (const RoomEventPtr& e) + { return e->id() == (*eIt)->id(); }); if (dupsBegin == events->end()) return; qCDebug(EVENTS) << "Dropping" << distance(dupsBegin, events->end()) << "duplicate event(s)"; - // Dispose of those dups - std::for_each(dupsBegin, events->end(), [] (Event* e) { delete e; }); events->erase(dupsBegin, events->end()); } -void Room::Private::processRedaction(const RedactionEvent* redaction) +inline bool isRedaction(const RoomEventPtr& e) { - Q_ASSERT(redaction && redaction->type() == EventType::Redaction); + return e->type() == EventType::Redaction; +} + +void Room::Private::processRedaction(RoomEventPtr redactionEvent) +{ + Q_ASSERT(redactionEvent && isRedaction(redactionEvent)); + const auto& redaction = + static_cast(redactionEvent.get()); const auto pIdx = eventsIndex.find(redaction->redactedEvent()); if (pIdx == eventsIndex.end()) @@ -778,11 +803,10 @@ void Room::Private::processRedaction(const RedactionEvent* redaction) // Make a new event from the redacted JSON, exchange events, // notify everyone and delete the old event - auto oldEvent = ti.replaceEvent(RoomEvent::fromJson(originalJson)); - q->onRedaction(oldEvent, ti); + auto oldEvent { ti.replaceEvent(makeEvent(originalJson)) }; + q->onRedaction(oldEvent.get(), ti.event()); qCDebug(MAIN) << "Redacted" << oldEvent->id() << "with" << redaction->id(); - emit q->replacedEvent(oldEvent, ti.event()); - delete oldEvent; + emit q->replacedEvent(ti.event(), oldEvent.get()); } Connection* Room::connection() const @@ -796,62 +820,55 @@ User* Room::localUser() const return connection()->user(); } -inline bool isRedaction(Event* e) +void Room::Private::addNewMessageEvents(RoomEvents&& events) { - return e->type() == EventType::Redaction; -} - -void Room::addNewMessageEvents(RoomEvents events) -{ - auto timelineSize = d->timeline.size(); + auto timelineSize = timeline.size(); - d->dropDuplicateEvents(&events); + dropDuplicateEvents(&events); // We want to process redactions in the order of arrival (covering the // case of one redaction superseding another one), hence stable partition. const auto normalsBegin = - std::stable_partition(events.begin(), events.end(), isRedaction); - RoomEventsView redactions { events.begin(), normalsBegin }, + stable_partition(events.begin(), events.end(), isRedaction); + RoomEventsRange redactions { events.begin(), normalsBegin }, normalEvents { normalsBegin, events.end() }; + if (!normalEvents.empty()) + emit q->aboutToAddNewMessages(normalEvents); + const auto insertedSize = insertEvents(std::move(normalEvents), Newer); + if (insertedSize > 0) { - emit aboutToAddNewMessages(normalEvents); - doAddNewMessageEvents(normalEvents); + qCDebug(MAIN) + << "Room" << displayname << "received" << insertedSize + << "new events; the last event is now" << timeline.back(); + q->onAddNewTimelineEvents(timeline.cend() - insertedSize); } - for (auto* r: redactions) - d->processRedaction(static_cast(r)); - if (!normalEvents.empty()) + for (auto&& r: redactions) + processRedaction(move(r)); + if (insertedSize > 0) { - d->checkUnreadMessages(normalEvents); - emit addedMessages(); + checkUnreadMessages(timeline.cend() - insertedSize); + emit q->addedMessages(); } - Q_ASSERT(d->timeline.size() == timelineSize + normalEvents.size()); -} - -void Room::doAddNewMessageEvents(RoomEventsView events) -{ - Q_ASSERT(!events.empty()); - d->insertEvents(events, Newer); - qCDebug(MAIN) - << "Room" << displayName() << "received" << events.size() - << "new events; the last event is now" << d->timeline.back(); + Q_ASSERT(timeline.size() == timelineSize + insertedSize); } -void Room::Private::checkUnreadMessages(RoomEventsView events) +void Room::Private::checkUnreadMessages(timeline_iter_t from) { - auto newUnreadMessages = - count_if(events.from, events.to, - [=] (const RoomEvent* e) { return isEventNotable(e); }); + Q_ASSERT(from < timeline.cend()); + const auto newUnreadMessages = count_if(from, timeline.cend(), + bind(&Room::Private::isEventNotable, this, _1)); - // The first event in the batch defines whose read marker can possibly be - // promoted any further over the same author's events newly arrived. - // Others will need explicit read receipts from the server (or, for - // the local user, markMessagesAsRead() invocation) to promote their - // read markers over the new message events. - User* firstWriter = connection->user((*events.from)->senderId()); + // The first event in the just-added batch (referred to by upTo.base()) + // defines whose read marker can possibly be promoted any further over + // the same author's events newly arrived. Others will need explicit + // 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()); if (q->readMarker(firstWriter) != timeline.crend()) { - promoteReadMarker(firstWriter, q->findInTimeline((*events.from)->id())); + promoteReadMarker(firstWriter, q->findInTimeline((*from)->id())); qCDebug(MAIN) << "Auto-promoted read marker for" << firstWriter->id() << "to" << *q->readMarker(firstWriter); } @@ -864,50 +881,45 @@ void Room::Private::checkUnreadMessages(RoomEventsView events) } } -void Room::addHistoricalMessageEvents(RoomEvents events) +void Room::Private::addHistoricalMessageEvents(RoomEvents&& events) { - auto timelineSize = d->timeline.size(); + const auto timelineSize = timeline.size(); - d->dropDuplicateEvents(&events); - auto redactionsBegin = - std::remove_if(events.begin(), events.end(), isRedaction); - RoomEventsView normalEvents { events.begin(), redactionsBegin }; + dropDuplicateEvents(&events); + const auto redactionsBegin = + remove_if(events.begin(), events.end(), isRedaction); + RoomEventsRange normalEvents { events.begin(), redactionsBegin }; if (normalEvents.empty()) return; - emit aboutToAddHistoricalMessages(normalEvents); - doAddHistoricalMessageEvents(normalEvents); - emit addedMessages(); - - Q_ASSERT(d->timeline.size() == timelineSize + normalEvents.size()); -} - -void Room::doAddHistoricalMessageEvents(RoomEventsView events) -{ - Q_ASSERT(!events.empty()); - - const bool thereWasNoReadMarker = readMarker() == timelineEdge(); - d->insertEvents(events, Older); + emit q->aboutToAddHistoricalMessages(normalEvents); + const bool thereWasNoReadMarker = q->readMarker() == timeline.crend(); + const auto insertedSize = insertEvents(std::move(normalEvents), Older); // Catch a special case when the last read event id refers to an event // that was outside the loaded timeline and has just arrived. Depending on // other messages next to the last read one, we might need to promote // the read marker and update unreadMessages flag. - const auto curReadMarker = readMarker(); - if (thereWasNoReadMarker && curReadMarker != timelineEdge()) + const auto curReadMarker = q->readMarker(); + if (thereWasNoReadMarker && curReadMarker != timeline.crend()) { qCDebug(MAIN) << "Discovered last read event in a historical batch"; - d->promoteReadMarker(localUser(), curReadMarker, true); + promoteReadMarker(q->localUser(), curReadMarker, true); } - qCDebug(MAIN) << "Room" << displayName() << "received" << events.size() - << "past events; the oldest event is now" << d->timeline.front(); + qCDebug(MAIN) << "Room" << displayname << "received" << insertedSize + << "past events; the oldest event is now" << timeline.front(); + q->onAddHistoricalTimelineEvents(timeline.crend() - insertedSize); + emit q->addedMessages(); + + Q_ASSERT(timeline.size() == timelineSize + insertedSize); } void Room::processStateEvents(const RoomEvents& events) { bool emitNamesChanged = false; - for (auto event: events) + for (const auto& e: events) { + auto* event = e.get(); switch (event->type()) { case EventType::RoomName: { @@ -973,12 +985,12 @@ void Room::processStateEvents(const RoomEvents& events) d->updateDisplayname(); } -void Room::processEphemeralEvent(Event* event) +void Room::processEphemeralEvent(EventPtr event) { switch (event->type()) { case EventType::Typing: { - auto typingEvent = static_cast(event); + auto typingEvent = static_cast(event.get()); d->usersTyping.clear(); for( const QString& userId: typingEvent->users() ) { @@ -989,7 +1001,7 @@ void Room::processEphemeralEvent(Event* event) break; } case EventType::Receipt: { - auto receiptEvent = static_cast(event); + auto receiptEvent = static_cast(event.get()); for( const auto &p: receiptEvent->eventsWithReceipts() ) { { diff --git a/room.h b/room.h index c0e041f6..08327917 100644 --- a/room.h +++ b/room.h @@ -45,25 +45,25 @@ namespace QMatrixClient { public: // For compatibility with Qt containers, even though we use - // a std:: container now + // a std:: container now for the room timeline using index_t = int; - TimelineItem(RoomEvent* e, index_t number) : evt(e), idx(number) { } + TimelineItem(RoomEventPtr&& e, index_t number) + : evt(move(e)), idx(number) { } RoomEvent* event() const { return evt.get(); } - RoomEvent* operator->() const { return event(); } //< Synonym for event()-> + RoomEvent* operator->() const { return evt.operator->(); } index_t index() const { return idx; } // Used for event redaction - RoomEvent* replaceEvent(RoomEvent* other) + RoomEventPtr replaceEvent(RoomEventPtr&& other) { - auto* old = evt.release(); - evt.reset(other); - return old; + evt.swap(other); + return move(other); } private: - std::unique_ptr evt; + RoomEventPtr evt; index_t idx; }; inline QDebug& operator<<(QDebug& d, const TimelineItem& ti) @@ -88,6 +88,7 @@ namespace QMatrixClient public: using Timeline = std::deque; using rev_iter_t = Timeline::const_reverse_iterator; + using timeline_iter_t = Timeline::const_iterator; Room(Connection* connection, QString id, JoinState initialJoinState); ~Room() override; @@ -188,8 +189,8 @@ namespace QMatrixClient void markAllMessagesAsRead(); signals: - void aboutToAddHistoricalMessages(RoomEventsView events); - void aboutToAddNewMessages(RoomEventsView events); + void aboutToAddHistoricalMessages(RoomEventsRange events); + void aboutToAddNewMessages(RoomEventsRange events); void addedMessages(); /** @@ -212,21 +213,20 @@ namespace QMatrixClient void lastReadEventChanged(User* user); void readMarkerMoved(); void unreadMessagesChanged(Room* room); - void replacedEvent(RoomEvent* before, RoomEvent* after); + void replacedEvent(const RoomEvent* newEvent, + const RoomEvent* oldEvent); protected: - virtual void doAddNewMessageEvents(RoomEventsView events); - virtual void doAddHistoricalMessageEvents(RoomEventsView events); virtual void processStateEvents(const RoomEvents& events); - virtual void processEphemeralEvent(Event* event); - virtual void onRedaction(RoomEvent*, TimelineItem&) { } + virtual void processEphemeralEvent(EventPtr event); + virtual void onAddNewTimelineEvents(timeline_iter_t from) { } + virtual void onAddHistoricalTimelineEvents(rev_iter_t from) { } + virtual void onRedaction(const RoomEvent* prevEvent, + const RoomEvent* after) { } private: class Private; Private* d; - - void addNewMessageEvents(RoomEvents events); - void addHistoricalMessageEvents(RoomEvents events); }; class MemberSorter diff --git a/util.h b/util.h index 09c58087..65de0610 100644 --- a/util.h +++ b/util.h @@ -25,53 +25,6 @@ namespace QMatrixClient { - /** - * @brief A crude wrapper around a container of pointers that owns pointers - * to contained objects - * - * Similar to vector>, upon deletion, this wrapper - * will delete all events contained in it. This wrapper can be used - * over Qt containers, which are incompatible with unique_ptr and even - * with QScopedPointer (which is the reason of its creation). - */ - template - class Owning : public ContainerT - { - public: - Owning() = default; - Owning(const Owning&) = delete; - Owning(Owning&&) = default; - Owning& operator=(Owning&& other) - { - assign(other.release()); - return *this; - } - - ~Owning() { cleanup(); } - - void assign(ContainerT&& other) - { - if (&other == this) - return; - cleanup(); - ContainerT::operator=(other); - } - - /** - * @brief returns the underlying container and releases the ownership - * - * Acts similar to unique_ptr::release. - */ - ContainerT release() - { - ContainerT c; - ContainerT::swap(c); - return c; - } - private: - void cleanup() { for (auto e: *this) delete e; } - }; - /** * @brief Lookup a value by a key in a varargs list * -- cgit v1.2.3 From 95797cf0ea78779ac98348fa1110abc09f5a344f Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 14 Dec 2017 19:05:06 +0900 Subject: That virtual ~Event() mentioned in the previous commit message --- events/event.h | 1 + 1 file changed, 1 insertion(+) (limited to 'events') diff --git a/events/event.h b/events/event.h index 2b18bb46..6a7f6ece 100644 --- a/events/event.h +++ b/events/event.h @@ -62,6 +62,7 @@ namespace QMatrixClient explicit Event(Type type) : _type(type) { } Event(Type type, const QJsonObject& rep); Event(const Event&) = delete; + virtual ~Event(); Type type() const { return _type; } bool isStateEvent() const -- cgit v1.2.3 From 468a73a804924ed6e63cd4aa58d9bcbc0ebc79d9 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 14 Dec 2017 19:18:15 +0900 Subject: One more fix for older compilers --- events/event.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'events') diff --git a/events/event.h b/events/event.h index 6a7f6ece..a5730f14 100644 --- a/events/event.h +++ b/events/event.h @@ -132,7 +132,7 @@ namespace QMatrixClient for (auto objValue: objs) { const auto o = objValue.toObject(); - auto e { makeEvent(o) }; + auto&& e = makeEvent(o); if (!e) e.reset(new EventT(EventType::Unknown, o)); this->emplace_back(std::move(e)); -- cgit v1.2.3 From 643aa92da416ab7b25c8b406a90007e4e7ebbb41 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 16 Dec 2017 19:33:49 +0900 Subject: Fix an assertion failure when redacting an unknown event Closes #135. --- events/event.cpp | 6 +++--- events/event.h | 48 ++++++++++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 21 deletions(-) (limited to 'events') diff --git a/events/event.cpp b/events/event.cpp index 01f473ce..e74bc114 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -75,10 +75,10 @@ inline BaseEventT* makeIfMatches(const QJsonObject& o, const QString& selector) } template <> -EventPtr QMatrixClient::makeEvent(const QJsonObject& obj) +EventPtr _impl::doMakeEvent(const QJsonObject& obj) { // Check more specific event types first - if (auto e = makeEvent(obj)) + if (auto e = doMakeEvent(obj)) return EventPtr(move(e)); return EventPtr { makeIfMatches -RoomEventPtr QMatrixClient::makeEvent(const QJsonObject& obj) +RoomEventPtr _impl::doMakeEvent(const QJsonObject& obj) { return RoomEventPtr { makeIfMatches using event_ptr_tt = std::unique_ptr; - /** Create an event with proper type from a JSON object - * Use this factory template to detect the type from the JSON object - * contents (the detected event type should derive from the template - * parameter type) and create an event object of that type. - */ - template - event_ptr_tt makeEvent(const QJsonObject& obj); + namespace _impl + { + template + event_ptr_tt doMakeEvent(const QJsonObject& obj); + } class Event { @@ -92,8 +90,25 @@ namespace QMatrixClient using EventType = Event::Type; using EventPtr = event_ptr_tt; - template <> - EventPtr makeEvent(const QJsonObject& obj); + /** Create an event with proper type from a JSON object + * Use this factory template to detect the type from the JSON object + * contents (the detected event type should derive from the template + * parameter type) and create an event object of that type. + */ + template + event_ptr_tt makeEvent(const QJsonObject& obj) + { + auto e = _impl::doMakeEvent(obj); + if (!e) + e.reset(new EventT(EventType::Unknown, obj)); + return e; + } + + namespace _impl + { + template <> + EventPtr doMakeEvent(const QJsonObject& obj); + } /** * \brief A vector of pointers to events with deserialisation capabilities @@ -130,13 +145,7 @@ namespace QMatrixClient // STL and Qt containers. this->reserve(static_cast(objs.size())); for (auto objValue: objs) - { - const auto o = objValue.toObject(); - auto&& e = makeEvent(o); - if (!e) - e.reset(new EventT(EventType::Unknown, o)); - this->emplace_back(std::move(e)); - } + this->emplace_back(makeEvent(objValue.toObject())); } }; using Events = EventsBatch; @@ -205,8 +214,11 @@ namespace QMatrixClient using RoomEvents = EventsBatch; using RoomEventPtr = event_ptr_tt; - template <> - RoomEventPtr makeEvent(const QJsonObject& obj); + namespace _impl + { + template <> + RoomEventPtr doMakeEvent(const QJsonObject& obj); + } /** * Conceptually similar to QStringView (but much more primitive), it's a -- cgit v1.2.3 From 00a3b28c8e561c35ad33499819d5aaced49b50da Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 25 Dec 2017 08:37:07 +0900 Subject: Code cleanup --- events/event.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'events') diff --git a/events/event.h b/events/event.h index 71f2f4b2..d4923f1f 100644 --- a/events/event.h +++ b/events/event.h @@ -115,9 +115,6 @@ namespace QMatrixClient * * This is a simple wrapper over a generic vector type that adds * a convenience method to deserialise events from QJsonArray. - * Note that this type does not own pointers to events. If owning - * semantics is needed, one should use the Owning<> wrapper around - * the container (e.g. \code Owning> \endcode). * \tparam EventT base type of all events in the vector */ template @@ -140,7 +137,7 @@ namespace QMatrixClient void fromJson(const QJsonObject& container, const QString& node) { const auto objs = container.value(node).toArray(); - using size_type = typename std::vector::size_type; + using size_type = typename std::vector>::size_type; // The below line accommodates the difference in size types of // STL and Qt containers. this->reserve(static_cast(objs.size())); -- cgit v1.2.3 From a4a1129385731c3999a6d5986a24fc069938245c Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 25 Dec 2017 09:17:39 +0900 Subject: ReceiptEvent: use QVector instead of std::vector Because we should practice what we preach in CONTRIBUTING.md. --- events/receiptevent.cpp | 6 +++--- events/receiptevent.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'events') diff --git a/events/receiptevent.cpp b/events/receiptevent.cpp index b36ddb23..e30fe4e4 100644 --- a/events/receiptevent.cpp +++ b/events/receiptevent.cpp @@ -56,15 +56,15 @@ ReceiptEvent::ReceiptEvent(const QJsonObject& obj) continue; } const QJsonObject reads = eventIt.value().toObject().value("m.read").toObject(); - std::vector receipts; - receipts.reserve(static_cast(reads.size())); + QVector receipts; + receipts.reserve(reads.size()); for( auto userIt = reads.begin(); userIt != reads.end(); ++userIt ) { const QJsonObject user = userIt.value().toObject(); receipts.push_back({userIt.key(), QMatrixClient::fromJson(user["ts"])}); } - _eventsWithReceipts.push_back({eventIt.key(), receipts}); + _eventsWithReceipts.push_back({eventIt.key(), std::move(receipts)}); } static const auto UnreadMsgsKey = QStringLiteral("x-qmatrixclient.unread_messages"); diff --git a/events/receiptevent.h b/events/receiptevent.h index 15fdf946..9494c7c6 100644 --- a/events/receiptevent.h +++ b/events/receiptevent.h @@ -30,9 +30,9 @@ namespace QMatrixClient struct ReceiptsForEvent { QString evtId; - std::vector receipts; + QVector receipts; }; - using EventsWithReceipts = std::vector; + using EventsWithReceipts = QVector; class ReceiptEvent: public Event { -- cgit v1.2.3