From 236dc9677d90621e5e3033852db85eafb72f7a34 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 14 Sep 2025 05:50:41 +0200 Subject: [PATCH 1/6] Support event classes with two Matrix event types So that we could use unstable event types with a bit more comfort. --- Quotient/events/event.cpp | 43 +++++++--- Quotient/events/event.h | 122 ++++++++++++++++------------ Quotient/events/roomevent.cpp | 7 +- Quotient/keyverificationsession.cpp | 2 +- quotest/quotest.cpp | 16 +++- 5 files changed, 118 insertions(+), 72 deletions(-) diff --git a/Quotient/events/event.cpp b/Quotient/events/event.cpp index de97a05b9..078dbf0be 100644 --- a/Quotient/events/event.cpp +++ b/Quotient/events/event.cpp @@ -10,35 +10,52 @@ using namespace Quotient; +namespace { +std::pair findFirstOverlap( + std::span metaTypes, + std::span matrixTypeIds) +{ + using namespace std::ranges; + for (const auto *metaType : metaTypes) + if (const auto overlappingIdIt = find_first_of(matrixTypeIds, metaType->matrixIds); + overlappingIdIt != end(matrixTypeIds)) + return {metaType, *overlappingIdIt}; + return {}; +} +} + AbstractEventMetaType::AbstractEventMetaType(const std::type_info &typeInfo, const char *className, AbstractEventMetaType *nearestBase, - event_type_t matrixId) - : typeInfo(typeInfo), className(className), baseType(nearestBase), matrixId(matrixId) + std::vector matrixTypeIds) + : typeInfo(typeInfo) + , className(className) + , baseType(nearestBase) + , matrixIds(std::move(matrixTypeIds)) { if (nearestBase) { - if (const auto existing = std::ranges::find(nearestBase->derivedTypes(), matrixId, - &AbstractEventMetaType::matrixId); - existing != nearestBase->derivedTypes().end()) // macOS still has no std::span::cend() + if (const auto overlap = findFirstOverlap(nearestBase->derivedTypes(), matrixIds); + overlap.first) { - if (QUO_ALARM_X(*existing == this, "Attempt to re-register the same event class")) + if (QUO_ALARM_X(overlap.first == this, "Attempt to re-register the same event class")) return; // This is kinda fine but extremely fishy // Two different metatype objects claim the same Matrix type id; this // is not normal, so give as much information as possible to diagnose - if (QUO_ALARM_X((*existing)->typeInfo == typeInfo, - QLatin1StringView(className) % " claims '"_L1 % matrixId + if (QUO_ALARM_X(overlap.first->typeInfo == typeInfo, + QLatin1StringView(className) % " claims '"_L1 % overlap.second % "' repeatedly; check that the C++ symbol is properly exported"_L1)) return; // That situation is very wrong (see #413) so maybe std::terminate() even? - qWarning(EVENTS).nospace() << matrixId << " is already mapped to " - << (*existing)->className << " before " << className + qWarning(EVENTS).nospace() << overlap.second << " is already mapped to " + << overlap.first->className << " before " << className << "; unless the two have different isValid() conditions, " "the latter class will never be used"; } nearestBase->_derivedTypes.emplace_back(this); - qDebug(EVENTS).nospace() << matrixId << " -> " << className << "; " - << nearestBase->_derivedTypes.size() - << " derived type(s) registered for " << nearestBase->className; + qDebug(EVENTS).nospace().noquote() + << std::format("{:n:s}", matrixIds) << " -> " << className << "; " + << nearestBase->_derivedTypes.size() << " derived type(s) registered for " + << nearestBase->className; } } diff --git a/Quotient/events/event.h b/Quotient/events/event.h index fb5b3c23b..14e31fb5b 100644 --- a/Quotient/events/event.h +++ b/Quotient/events/event.h @@ -46,7 +46,7 @@ class QUOTIENT_API AbstractEventMetaType { const std::type_info &typeInfo; const char *const className; const AbstractEventMetaType *const baseType; - const event_type_t matrixId; + const std::vector matrixIds; // NOLINTEND(misc-non-private-member-variables-in-classes) auto derivedTypes() const { return std::span(_derivedTypes); } @@ -64,8 +64,8 @@ class QUOTIENT_API AbstractEventMetaType { friend class EventMetaType; explicit AbstractEventMetaType(const std::type_info &typeInfo, const char *className, - AbstractEventMetaType *nearestBase = nullptr, - event_type_t matrixId = nullptr); + AbstractEventMetaType *nearestBase, + std::vector matrixTypeIds); // The returned value indicates whether a generic object has to be created // on the top level when `event` is empty, instead of returning nullptr @@ -92,14 +92,15 @@ class QUOTIENT_API EventMetaType : public AbstractEventMetaType { // Above: can't constrain EventT to be EventClass because it's incomplete // at the point of EventMetaType instantiation (see QUO_BASE_EVENT and QUO_EVENT) public: - explicit EventMetaType(AbstractEventMetaType *nearestBase = nullptr, - const char* matrixTypeId = {}) + template ... TypeIdTs> + explicit EventMetaType(AbstractEventMetaType *nearestBase = nullptr, TypeIdTs... matrixTypeIds) + requires (sizeof...(TypeIdTs) <= 2) // NB: typeid(T&) == typeid(T) but typeid(T&) can be used with an incomplete type // NB2: it would be lovely to "just" use QMetaType::fromType<> instead of QtPrivate API // but QMetaType tries to instantiate constructor wrappers and it's not possible while // the type is incomplete : AbstractEventMetaType(typeid(EventT &), QtPrivate::QMetaTypeForType().getName(), - nearestBase, event_type_t(matrixTypeId)) + nearestBase, {event_type_t(matrixTypeIds)...}) {} //! \brief Try to load an event from JSON, with dynamic type resolution @@ -107,9 +108,9 @@ class QUOTIENT_API EventMetaType : public AbstractEventMetaType { //! The generic logic defined in this class template and invoked applies to //! all event types defined in the library and boils down to the following: //! 1. - //! a. If EventT has TypeId defined (which normally is a case of - //! all leaf - specific - event types, via QUO_EVENT macro) and - //! \p type doesn't exactly match it, nullptr is immediately returned. + //! a. If EventT has TypeId defined (which normally is a case of all leaf - specific - + //! event types, via QUO_EVENT macro) and \p type doesn't exactly match any of matrixIds, + //! nullptr is immediately returned. //! b. In absence of TypeId, an event type is assumed to be a base; //! its derivedTypes are examined, and this algorithm is applied //! recursively on each. @@ -120,8 +121,8 @@ class QUOTIENT_API EventMetaType : public AbstractEventMetaType { //! of `state_key` is checked in any type derived from StateEvent. //! 3. If step 1b above returned non-nullptr, immediately return it. //! 4. - //! a. If EventT::isValid() or EventT::TypeId (either, or both) exist and - //! are satisfied (see steps 1a and 2 above), an object of this type + //! a. If EventT::isValid() or EventT::TypeId (either, or both) exist and validations in + //! steps 1a and 2 have been either skipped or satisfied, an object of this type //! is created from the passed JSON and returned. In case of a base //! event type, this will be a generic (aka "unknown") event. //! b. If neither exists, a generic event is only created and returned @@ -147,7 +148,7 @@ class QUOTIENT_API EventMetaType : public AbstractEventMetaType { Event*& event) const override { if constexpr (requires { EventT::TypeId; }) { - if (EventT::TypeId != type) + if (std::ranges::find(matrixIds, type) == std::ranges::end(matrixIds)) return false; } else { for (const auto& p : _derivedTypes) { @@ -392,6 +393,14 @@ class EventTemplate : public BaseEventT { ContentT content() const { return fromJson(this->contentJson()); } }; +#define QUO_EVENT_IMPL(StaticVarName_, CppType_, ...) \ + friend class EventMetaType; \ + static inline auto StaticVarName_ = EventMetaType(__VA_ARGS__); \ + static_assert(&CppType_::StaticVarName_ == &StaticVarName_, \ + #CppType_ " is wrong here - check for copy-pasta"); \ + const AbstractEventMetaType &metaType() const override { return StaticVarName_; } \ + // End of macro + //! \brief Supply event metatype information in base event types //! //! Use this macro in a public section of your base event class to provide @@ -401,14 +410,11 @@ class EventTemplate : public BaseEventT { //! initialised by parameters passed to the macro, and a metaType() override //! pointing to that BaseMetaType. //! \sa EventMetaType -#define QUO_BASE_EVENT(CppType_, BaseCppType_, ...) \ - friend class EventMetaType; \ - static inline auto BaseMetaType = \ - EventMetaType(&BaseCppType_::BaseMetaType __VA_OPT__(, ) __VA_ARGS__); \ - static_assert(&CppType_::BaseMetaType == &BaseMetaType, \ - #CppType_ " is wrong here - check for copy-pasta"); \ - const AbstractEventMetaType &metaType() const override { return BaseMetaType; } \ - // End of macro +#define QUO_BASE_EVENT(CppType_, BaseCppType_, ...) \ + QUO_EVENT_IMPL(BaseMetaType, CppType_, &BaseCppType_::BaseMetaType __VA_OPT__(, ) __VA_ARGS__) + +//! A helper macro to pass two event type identifiers to QUO_EVENT and QUO_DEFINE_SIMPLE_EVENT +#define QUO_LIST(...) __VA_ARGS__ //! \brief Supply event metatype information in (specific) event types //! @@ -417,21 +423,26 @@ class EventTemplate : public BaseEventT { //! Do _not_ use this macro if your class is an intermediate wrapper and is not //! supposed to be instantiated on its own. Provides MetaType static field //! initialised as described below; a metaType() override pointing to it; and -//! the TypeId static field that is equal to MetaType.matrixId. +//! the TypeId static field that is equal to MetaType.matrixIds[0]. //! -//! The first two macro parameters are used as the first two EventMetaType -//! constructor parameters; the third EventMetaType parameter is always -//! BaseMetaType; and additional base types can be passed in extra macro -//! parameters if you need to include the same event type in more than one -//! event factory hierarchy (e.g., EncryptedEvent). -//! \sa EventMetaType -#define QUO_EVENT(CppType_, MatrixType_) \ - friend class EventMetaType; \ - static inline const auto MetaType = EventMetaType(&BaseMetaType, MatrixType_); \ - static_assert(&CppType_::MetaType == &MetaType, \ - #CppType_ " is wrong here - check for copy-pasta"); \ - static inline const auto &TypeId = MetaType.matrixId; \ - const AbstractEventMetaType &metaType() const override { return MetaType; } \ +//! There are cases when the underlying Matrix event type has two type ids, namely when the type +//! is being proposed for the specification (i.e. there's an MSC for it) or has just been merged +//! into the spec. In such situations it is often desirable to recognise both the canonical +//! ("m.example_event") and the unstable (i.e., "org.matrix.mscXXXX.example_event") types but only +//! send events with the unstable (while the MSC is in flight) or, later, stable (when it is +//! accepted) types. If you use QUO_LIST to pass two event type ids instead of one, e.g. +//! `QUO_EVENT(ExampleEvent, QUO_LIST("org.matrix.mscXXXX.example_event", "m.example_event"))`, then +//! only the first type will be used both to load the event from JSON and create a new instance of +//! the event, the second type will only be used to detect the event in JSON but you cannot create +//! new class instances using this type. Once the MSC is accepted, simply switch the two type ids +//! around and rebuild the code that creates or loads events of this type. +//! \note QUO_LIST is used here instead of making QUO_EVENT a variadic macro to provide an extra +//! safeguard, but also to use the same syntax in cases when the type id is not the last +//! parameter, e.g. for DEFINE_SIMPLE_EVENT +//! \sa Quotient::EventMetaType, QUO_LIST +#define QUO_EVENT(CppType_, MatrixTypeOrTypeList_) \ + QUO_EVENT_IMPL(MetaType, CppType_, &BaseMetaType, MatrixTypeOrTypeList_) \ + static inline const auto &TypeId = MetaType.matrixIds[0]; \ // End of macro #define QUO_CONTENT_GETTER_X(PartType_, PartName_, JsonKey_) \ @@ -452,27 +463,32 @@ class EventTemplate : public BaseEventT { #define QUO_CONTENT_GETTER(PartType_, PartName_) \ QUO_CONTENT_GETTER_X(PartType_, PartName_, toSnakeCase(#PartName_##_L1)) -/// \brief Define a new event class with a single key-value pair in the content -/// -/// This macro defines a new event class \p Name_ derived from \p Base_, -/// with Matrix event type \p TypeId_, providing a getter named \p GetterName_ -/// for a single value of type \p ValueType_ inside the event content. -/// To retrieve the value the getter uses a JSON key name that corresponds to -/// its own (getter's) name but written in snake_case. \p GetterName_ must be -/// in camelCase, no quotes (an identifier, not a literal). -#define DEFINE_SIMPLE_EVENT(Name_, Base_, TypeId_, ValueType_, GetterName_, JsonKey_) \ - constexpr inline auto Name_##ContentKey = JsonKey_##_L1; \ - class QUOTIENT_API Name_ \ - : public ::Quotient::EventTemplate< \ - Name_, Base_, EventContent::SingleKeyValue> { \ - public: \ - QUO_EVENT(Name_, TypeId_) \ - using value_type = ValueType_; \ - using EventTemplate::EventTemplate; \ - QUO_CONTENT_GETTER_X(ValueType_, GetterName_, Name_##ContentKey) \ - }; \ +//! \brief Define a new event class with a single key-value pair in the content +//! +//! This macro defines a new event class \p Name_ derived from \p Base_, with Matrix event type +//! \p TypeId_, providing a getter named \p GetterName_ for a single value of type \p ValueType_ +//! stored under \p JsonKey_ inside the event content. +//! +//! \p TypeId_ can be a QUO_LIST() with two event types, the same way it works with QUO_EVENT. +//! \sa QUO_LIST, QUO_EVENT +#define QUO_DEFINE_SIMPLE_EVENT(Name_, Base_, TypeId_, ValueType_, GetterName_, JsonKey_) \ + constexpr inline auto Name_##ContentKey = QLatin1String(JsonKey_); \ + class QUOTIENT_API Name_ \ + : public ::Quotient::EventTemplate< \ + Name_, Base_, EventContent::SingleKeyValue> \ + { \ + public: \ + QUO_EVENT(Name_, QUO_LIST(TypeId_)) \ + using value_type = ValueType_; \ + using EventTemplate::EventTemplate; \ + QUO_CONTENT_GETTER_X(ValueType_, GetterName_, Name_##ContentKey) \ + }; \ // End of macro +//! \deprecated Use QUO_DEFINE_SIMPLE_EVENT instead +#define DEFINE_SIMPLE_EVENT(Name_, Base_, TypeId_, ValueType_, GetterName_, JsonKey_) \ + QUO_DEFINE_SIMPLE_EVENT(Name_, Base_, QUO_LIST(TypeId_), ValueType_, GetterName_, JsonKey_) + // === is<>(), eventCast<>() and switchOnType<>() === template diff --git a/Quotient/events/roomevent.cpp b/Quotient/events/roomevent.cpp index d1688bf95..b21334171 100644 --- a/Quotient/events/roomevent.cpp +++ b/Quotient/events/roomevent.cpp @@ -103,10 +103,11 @@ const QJsonObject RoomEvent::encryptedJson() const } namespace { -bool containsEventType(const auto& haystack, const auto& needle) +bool containsEventType(std::span haystack, const QString &needle) { - return std::ranges::any_of(haystack, [needle](const AbstractEventMetaType* candidate) { - return candidate->matrixId == needle || containsEventType(candidate->derivedTypes(), needle); + return std::ranges::any_of(haystack, [needle](const AbstractEventMetaType *candidate) { + return std::ranges::contains(candidate->matrixIds, needle) + || containsEventType(candidate->derivedTypes(), needle); }); } } diff --git a/Quotient/keyverificationsession.cpp b/Quotient/keyverificationsession.cpp index 3193aff4a..b2fa16ada 100644 --- a/Quotient/keyverificationsession.cpp +++ b/Quotient/keyverificationsession.cpp @@ -719,7 +719,7 @@ void KeyVerificationSession::sendEvent(const QString &userId, const QString &dev if (m_room) { auto json = event.contentJson(); json.remove("transaction_id"_L1); - if (event.metaType().matrixId == KeyVerificationRequestEvent::TypeId) { + if (event.is()) { json["msgtype"_L1] = event.matrixType(); json["body"_L1] = m_connection->userId() + " sent a verification request"_L1; json["to"_L1] = m_remoteUserId; diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index f5e8f8982..c000c4eef 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -100,6 +100,7 @@ private slots: TEST_DECL(sendMessage) TEST_DECL(sendReaction) TEST_DECL(sendFile) + TEST_DECL(loadCustomEvent) TEST_DECL(sendCustomEvent) TEST_DECL(setTopic) TEST_DECL(redactEvent) @@ -564,8 +565,19 @@ bool TestSuite::checkFileSendingOutcome(const TestToken& thisTest, return true; } -DEFINE_SIMPLE_EVENT(CustomEvent, RoomEvent, "quotest.custom", int, testValue, - "test_value") +QUO_DEFINE_SIMPLE_EVENT(CustomEvent, RoomEvent, + QUO_LIST("quotest.custom.unstable", "quotest.custom"), int, testValue, + "test_value") + +TEST_IMPL(loadCustomEvent) +{ + // TODO: Make a unit test for event.* + FAIL_TEST_IF(CustomEvent::MetaType.matrixIds.size() != 2); + auto testEvent = loadEvent(CustomEvent::MetaType.matrixIds[1], + QJsonObject{{"test_value"_L1, 17}}); + FINISH_TEST(testEvent && testEvent->is() + && eventCast(testEvent)->testValue() == 17); +} TEST_IMPL(sendCustomEvent) { From 8b221666d06f35bd4fb3837ae6fa638b3278f00a Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 15 Sep 2025 11:33:54 +0200 Subject: [PATCH 2/6] Fix building on macOS They still don't have std::ranges::contains(). --- Quotient/events/roomevent.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Quotient/events/roomevent.cpp b/Quotient/events/roomevent.cpp index b21334171..74deafcfe 100644 --- a/Quotient/events/roomevent.cpp +++ b/Quotient/events/roomevent.cpp @@ -6,8 +6,8 @@ #include "encryptedevent.h" #include "redactionevent.h" #include "stateevent.h" - #include "../logging_categories_p.h" +#include "../ranges_extras.h" using namespace Quotient; @@ -106,7 +106,7 @@ namespace { bool containsEventType(std::span haystack, const QString &needle) { return std::ranges::any_of(haystack, [needle](const AbstractEventMetaType *candidate) { - return std::ranges::contains(candidate->matrixIds, needle) + return rangeContains(candidate->matrixIds, needle) || containsEventType(candidate->derivedTypes(), needle); }); } From 9ad152ac06ab9630b8ee01c18d3fdec03df4bdab Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sat, 20 Sep 2025 17:24:20 +0200 Subject: [PATCH 3/6] Make rangeContains() definition more flexible --- Quotient/ranges_extras.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Quotient/ranges_extras.h b/Quotient/ranges_extras.h index 8692294bf..c7e5ca85f 100644 --- a/Quotient/ranges_extras.h +++ b/Quotient/ranges_extras.h @@ -85,7 +85,8 @@ template