From ab2d78de6a9d33e1470ae9db2ed6ed9565c817e5 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 11 Jul 2022 10:08:44 +0200 Subject: fromJson()/toJson() refactoring fromJson() is generalised to accept any JSON-like type while passing QJsonObject to JsonConverter<>::load (instead of doLoad). This allows to (still) rely on JsonConverter<> as a customisation point while providing an opportunity to overload fromJson for custom types in a pointed way (specifically, by providing the overload for `fromJson(const QJsonObject&)`), instead of having to go with full-blown JsonConverter<> specialisation. This will be used in a further commit to simplify ReceiptEvent definition. Using if constexpr in combination with constraints (`requires()`) - the first such case in Quotient codebase - allowed to put the entire logic in a single JsonConverter<>::load() body instead of having a facility JsonExporter<> class for SFINAE. Aside from that, fromJson is entirely dropped because it's not supposed to be used that way (it's no-op after all); reflecting that, Event::unsignedPart() and Event::contentPart() no more default to QJsonValue as the expected return type, you have to explicitly provide the type instead (and as one can see from the changes in the commit, it's actually better that way since it's better to validate the value inside JSON - e.g. check QString or QJsonObject for emptiness - than the QJsonValue envelope which may still wrap an empty value). toJson() is also generalised, replacing 3 functions with one that has a constexpr if to discern between two kinds of types. --- lib/events/encryptedevent.cpp | 10 ++++++---- lib/events/event.h | 4 ++-- lib/events/roomevent.cpp | 6 +++--- lib/events/stateevent.cpp | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-) (limited to 'lib/events') diff --git a/lib/events/encryptedevent.cpp b/lib/events/encryptedevent.cpp index c97ccc16..ec00ad4c 100644 --- a/lib/events/encryptedevent.cpp +++ b/lib/events/encryptedevent.cpp @@ -49,14 +49,16 @@ RoomEventPtr EncryptedEvent::createDecrypted(const QString &decrypted) const eventObject["event_id"] = id(); eventObject["sender"] = senderId(); eventObject["origin_server_ts"] = originTimestamp().toMSecsSinceEpoch(); - if (const auto relatesToJson = contentPart("m.relates_to"_ls); !relatesToJson.isUndefined()) { + if (const auto relatesToJson = contentPart("m.relates_to"_ls); + !relatesToJson.isEmpty()) { auto content = eventObject["content"].toObject(); - content["m.relates_to"] = relatesToJson.toObject(); + content["m.relates_to"] = relatesToJson; eventObject["content"] = content; } - if (const auto redactsJson = unsignedPart("redacts"_ls); !redactsJson.isUndefined()) { + if (const auto redactsJson = unsignedPart("redacts"_ls); + !redactsJson.isEmpty()) { auto unsign = eventObject["unsigned"].toObject(); - unsign["redacts"] = redactsJson.toString(); + unsign["redacts"] = redactsJson; eventObject["unsigned"] = unsign; } return loadEvent(eventObject); diff --git a/lib/events/event.h b/lib/events/event.h index 05eb51e9..da6cf3c7 100644 --- a/lib/events/event.h +++ b/lib/events/event.h @@ -212,7 +212,7 @@ public: const QJsonObject contentJson() const; - template + template const T contentPart(KeyT&& key) const { return fromJson(contentJson()[std::forward(key)]); @@ -227,7 +227,7 @@ public: const QJsonObject unsignedJson() const; - template + template const T unsignedPart(KeyT&& key) const { return fromJson(unsignedJson()[std::forward(key)]); diff --git a/lib/events/roomevent.cpp b/lib/events/roomevent.cpp index 3ddf5ac4..e695e0ec 100644 --- a/lib/events/roomevent.cpp +++ b/lib/events/roomevent.cpp @@ -15,9 +15,9 @@ RoomEvent::RoomEvent(Type type, event_mtype_t matrixType, RoomEvent::RoomEvent(Type type, const QJsonObject& json) : Event(type, json) { - if (const auto redaction = unsignedPart(RedactedCauseKeyL); - redaction.isObject()) - _redactedBecause = makeEvent(redaction.toObject()); + if (const auto redaction = unsignedPart(RedactedCauseKeyL); + !redaction.isEmpty()) + _redactedBecause = makeEvent(redaction); } RoomEvent::~RoomEvent() = default; // Let the smart pointer do its job diff --git a/lib/events/stateevent.cpp b/lib/events/stateevent.cpp index c343e37f..43dfd6e8 100644 --- a/lib/events/stateevent.cpp +++ b/lib/events/stateevent.cpp @@ -21,7 +21,7 @@ StateEventBase::StateEventBase(Event::Type type, event_mtype_t matrixType, bool StateEventBase::repeatsState() const { - const auto prevContentJson = unsignedPart(PrevContentKeyL); + const auto prevContentJson = unsignedPart(PrevContentKeyL); return fullJson().value(ContentKeyL) == prevContentJson; } -- cgit v1.2.3 From 44aeb57e196bcd9e041c498a212f17b0dcd1244f Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Wed, 22 Jun 2022 02:52:35 +0200 Subject: Refactor things around EncryptionEvent[Content] EncryptionEvent was marked as Q_GADGET only for the sake of defining EncryptionType inside of it as Q_ENUM, with aliases also available under Quotient:: and EncryptionEventContent. This is a legacy from pre-Q_ENUM_NS times. However, event types are not really made to be proper Q_GADGETs: Q_GADGET implies access by value or reference but event types are uncopyable for the former and QML is ill-equipped for the latter. This commit moves EncryptionType definition to where other such enumerations reside - on the namespace level in quotient_common.h; and the other two places are now deprecated; and EncryptionEvent is no more Q_GADGET. With fromJson/toJson refactored in the previous commit there's no more need to specialise JsonConverter<>: specialising fromJson() is just enough. Moving EncryptionType to quotient_common.h exposed the clash of two Undefined enumerators (in RoomType and EncryptionType), warranting both enumerations to become scoped (which they ought to be, anyway). And while we're at it, the base type of enumerations is specified explicitly, as MSVC apparently uses a signed base type (int?) by default, unlike other compilers, and the upcoming enum converters will assume an unsigned base type. Finally, using fillFromJson() instead of fromJson() in the EncryptionEventContent constructor allowed to make default values explicit in the header file, rather than buried in the initialisation code. --- lib/events/encryptionevent.cpp | 50 ++++++++++++++++++++---------------------- lib/events/encryptionevent.h | 35 ++++++++++++----------------- lib/quotient_common.h | 12 +++++++--- lib/room.cpp | 4 ++-- 4 files changed, 49 insertions(+), 52 deletions(-) (limited to 'lib/events') diff --git a/lib/events/encryptionevent.cpp b/lib/events/encryptionevent.cpp index 1654d6f3..8872447b 100644 --- a/lib/events/encryptionevent.cpp +++ b/lib/events/encryptionevent.cpp @@ -6,47 +6,45 @@ #include "e2ee/e2ee.h" -namespace Quotient { +using namespace Quotient; + static constexpr std::array encryptionStrings { MegolmV1AesSha2AlgoKey }; template <> -struct JsonConverter { - static EncryptionType load(const QJsonValue& jv) - { - const auto& encryptionString = jv.toString(); - for (auto it = encryptionStrings.begin(); it != encryptionStrings.end(); - ++it) - if (encryptionString == *it) - return EncryptionType(it - encryptionStrings.begin()); - - if (!encryptionString.isEmpty()) - qCWarning(EVENTS) << "Unknown EncryptionType: " << encryptionString; - return EncryptionType::Undefined; - } -}; -} // namespace Quotient - -using namespace Quotient; +EncryptionType Quotient::fromJson(const QJsonValue& jv) +{ + const auto& encryptionString = jv.toString(); + for (auto it = encryptionStrings.begin(); it != encryptionStrings.end(); + ++it) + if (encryptionString == *it) + return EncryptionType(it - encryptionStrings.begin()); + + if (!encryptionString.isEmpty()) + qCWarning(EVENTS) << "Unknown EncryptionType: " << encryptionString; + return EncryptionType::Undefined; +} EncryptionEventContent::EncryptionEventContent(const QJsonObject& json) - : encryption(fromJson(json[AlgorithmKeyL])) + : encryption(fromJson(json[AlgorithmKeyL])) , algorithm(sanitized(json[AlgorithmKeyL].toString())) - , rotationPeriodMs(json[RotationPeriodMsKeyL].toInt(604800000)) - , rotationPeriodMsgs(json[RotationPeriodMsgsKeyL].toInt(100)) -{} +{ + // NB: fillFromJson only fills the variable if the JSON key exists + fillFromJson(json[RotationPeriodMsKeyL], rotationPeriodMs); + fillFromJson(json[RotationPeriodMsgsKeyL], rotationPeriodMsgs); +} -EncryptionEventContent::EncryptionEventContent(EncryptionType et) +EncryptionEventContent::EncryptionEventContent(Quotient::EncryptionType et) : encryption(et) { - if(encryption != Undefined) { - algorithm = encryptionStrings[encryption]; + if(encryption != Quotient::EncryptionType::Undefined) { + algorithm = encryptionStrings[static_cast(encryption)]; } } QJsonObject EncryptionEventContent::toJson() const { QJsonObject o; - if (encryption != EncryptionType::Undefined) + if (encryption != Quotient::EncryptionType::Undefined) o.insert(AlgorithmKey, algorithm); o.insert(RotationPeriodMsKey, rotationPeriodMs); o.insert(RotationPeriodMsgsKey, rotationPeriodMsgs); diff --git a/lib/events/encryptionevent.h b/lib/events/encryptionevent.h index 945b17e7..91452c3f 100644 --- a/lib/events/encryptionevent.h +++ b/lib/events/encryptionevent.h @@ -4,51 +4,44 @@ #pragma once +#include "quotient_common.h" #include "stateevent.h" namespace Quotient { class QUOTIENT_API EncryptionEventContent { public: - enum EncryptionType : size_t { MegolmV1AesSha2 = 0, Undefined }; + using EncryptionType + [[deprecated("Use Quotient::EncryptionType instead")]] = + Quotient::EncryptionType; - QUO_IMPLICIT EncryptionEventContent(EncryptionType et); - [[deprecated("This constructor will require explicit EncryptionType soon")]] // - explicit EncryptionEventContent() - : EncryptionEventContent(Undefined) - {} + // NOLINTNEXTLINE(google-explicit-constructor) + QUO_IMPLICIT EncryptionEventContent(Quotient::EncryptionType et); explicit EncryptionEventContent(const QJsonObject& json); QJsonObject toJson() const; - EncryptionType encryption; - QString algorithm; - int rotationPeriodMs; - int rotationPeriodMsgs; + Quotient::EncryptionType encryption; + QString algorithm {}; + int rotationPeriodMs = 604'800'000; + int rotationPeriodMsgs = 100; }; -using EncryptionType = EncryptionEventContent::EncryptionType; - class QUOTIENT_API EncryptionEvent : public StateEvent { - Q_GADGET public: DEFINE_EVENT_TYPEID("m.room.encryption", EncryptionEvent) - using EncryptionType = EncryptionEventContent::EncryptionType; - Q_ENUM(EncryptionType) + using EncryptionType + [[deprecated("Use Quotient::EncryptionType instead")]] = + Quotient::EncryptionType; explicit EncryptionEvent(const QJsonObject& obj) : StateEvent(typeId(), obj) {} - [[deprecated("This constructor will require an explicit parameter soon")]] // -// explicit EncryptionEvent() -// : EncryptionEvent(QJsonObject()) -// {} explicit EncryptionEvent(EncryptionEventContent&& content) : StateEvent(typeId(), matrixTypeId(), QString(), std::move(content)) {} - EncryptionType encryption() const { return content().encryption; } - + Quotient::EncryptionType encryption() const { return content().encryption; } QString algorithm() const { return content().algorithm; } int rotationPeriodMs() const { return content().rotationPeriodMs; } int rotationPeriodMsgs() const { return content().rotationPeriodMsgs; } diff --git a/lib/quotient_common.h b/lib/quotient_common.h index 233bcaa1..7fec9274 100644 --- a/lib/quotient_common.h +++ b/lib/quotient_common.h @@ -107,14 +107,20 @@ enum UriResolveResult : int8_t { }; Q_ENUM_NS(UriResolveResult) -enum RoomType { - Space, - Undefined, +enum class RoomType : uint8_t { + Space = 0, + Undefined = 0xFF, }; Q_ENUM_NS(RoomType) [[maybe_unused]] constexpr std::array RoomTypeStrings { "m.space" }; +enum class EncryptionType : uint8_t { + MegolmV1AesSha2 = 0, + Undefined = 0xFF, +}; +Q_ENUM_NS(EncryptionType) + } // namespace Quotient Q_DECLARE_OPERATORS_FOR_FLAGS(Quotient::MembershipMask) Q_DECLARE_OPERATORS_FOR_FLAGS(Quotient::JoinStates) diff --git a/lib/room.cpp b/lib/room.cpp index 2625105c..f67451ce 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -3062,7 +3062,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) return false; } if (oldEncEvt - && oldEncEvt->encryption() != EncryptionEventContent::Undefined) { + && oldEncEvt->encryption() != EncryptionType::Undefined) { qCWarning(STATE) << "The room is already encrypted but a new" " room encryption event arrived - ignoring"; return false; @@ -3508,5 +3508,5 @@ void Room::activateEncryption() qCWarning(E2EE) << "Room" << objectName() << "is already encrypted"; return; } - setState(EncryptionEventContent::MegolmV1AesSha2); + setState(EncryptionType::MegolmV1AesSha2); } -- cgit v1.2.3 From 9a65bde0fa438ee4ad101b58721b77182ae1ae70 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 9 May 2022 12:02:30 +0200 Subject: Make AliasesEventContent a simple structure JSON conversions are moved out of the class, obviating the need to define the plain data constructor and gaining default-constructibility along the way - previously the default constructor was preempted by user-defined ones. --- lib/events/roomcanonicalaliasevent.h | 43 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 24 deletions(-) (limited to 'lib/events') diff --git a/lib/events/roomcanonicalaliasevent.h b/lib/events/roomcanonicalaliasevent.h index bb8654e5..e599699d 100644 --- a/lib/events/roomcanonicalaliasevent.h +++ b/lib/events/roomcanonicalaliasevent.h @@ -7,35 +7,30 @@ #include "stateevent.h" namespace Quotient { -namespace EventContent{ - class AliasesEventContent { - - public: - - template - AliasesEventContent(T1&& canonicalAlias, T2&& altAliases) - : canonicalAlias(std::forward(canonicalAlias)) - , altAliases(std::forward(altAliases)) - { } - - AliasesEventContent(const QJsonObject& json) - : canonicalAlias(fromJson(json["alias"])) - , altAliases(fromJson(json["alt_aliases"])) - { } - - auto toJson() const - { - QJsonObject jo; - addParam(jo, QStringLiteral("alias"), canonicalAlias); - addParam(jo, QStringLiteral("alt_aliases"), altAliases); - return jo; - } - +namespace EventContent { + struct AliasesEventContent { QString canonicalAlias; QStringList altAliases; }; } // namespace EventContent +template<> +inline EventContent::AliasesEventContent fromJson(const QJsonObject& jo) +{ + return EventContent::AliasesEventContent { + fromJson(jo["alias"_ls]), + fromJson(jo["alt_aliases"_ls]) + }; +} +template<> +inline auto toJson(const EventContent::AliasesEventContent& c) +{ + QJsonObject jo; + addParam(jo, QStringLiteral("alias"), c.canonicalAlias); + addParam(jo, QStringLiteral("alt_aliases"), c.altAliases); + return jo; +} + class RoomCanonicalAliasEvent : public StateEvent { public: -- cgit v1.2.3 From e7dee15531ad357bd33ac546fb1b9332a5c1260c Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Thu, 9 Jun 2022 08:40:27 +0200 Subject: converters.*: facilities to convert enums This introduces enumTo/FromJsonString() and flagTo/FromJsonString(), four facility functions to simplify conversion between C++ enums and JSON, and refactors a couple of places where it's useful. --- lib/converters.cpp | 14 +++++++ lib/converters.h | 92 ++++++++++++++++++++++++++++++++++++++++++ lib/events/roomcreateevent.cpp | 19 +++------ lib/events/roommemberevent.cpp | 19 +++------ 4 files changed, 116 insertions(+), 28 deletions(-) (limited to 'lib/events') diff --git a/lib/converters.cpp b/lib/converters.cpp index 444ca4f6..b0e3a4b6 100644 --- a/lib/converters.cpp +++ b/lib/converters.cpp @@ -2,9 +2,23 @@ // SPDX-License-Identifier: LGPL-2.1-or-later #include "converters.h" +#include "logging.h" #include +void Quotient::_impl::warnUnknownEnumValue(const QString& stringValue, + const char* enumTypeName) +{ + qWarning(EVENTS).noquote() + << "Unknown" << enumTypeName << "value:" << stringValue; +} + +void Quotient::_impl::reportEnumOutOfBounds(uint32_t v, const char* enumTypeName) +{ + qCritical(MAIN).noquote() + << "Value" << v << "is out of bounds for enumeration" << enumTypeName; +} + QJsonValue Quotient::JsonConverter::dump(const QVariant& v) { return QJsonValue::fromVariant(v); diff --git a/lib/converters.h b/lib/converters.h index 9f42d712..bf456af9 100644 --- a/lib/converters.h +++ b/lib/converters.h @@ -131,6 +131,98 @@ inline void fillFromJson(const QJsonValue& jv, T& pod) pod = fromJson(jv); } +namespace _impl { + void warnUnknownEnumValue(const QString& stringValue, + const char* enumTypeName); + void reportEnumOutOfBounds(uint32_t v, const char* enumTypeName); +} + +//! \brief Facility string-to-enum converter +//! +//! This is to simplify enum loading from JSON - just specialise +//! Quotient::fromJson() and call this function from it, passing (aside from +//! the JSON value for the enum - that must be a string, not an int) any +//! iterable container of string'y values (const char*, QLatin1String, etc.) +//! matching respective enum values, 0-based. +//! \sa enumToJsonString +template +EnumT enumFromJsonString(const QString& s, const EnumStringValuesT& enumValues, + EnumT defaultValue) +{ + static_assert(std::is_unsigned_v>); + if (const auto it = std::find(cbegin(enumValues), cend(enumValues), s); + it != cend(enumValues)) + return EnumT(it - cbegin(enumValues)); + + if (!s.isEmpty()) + _impl::warnUnknownEnumValue(s, qt_getEnumName(EnumT())); + return defaultValue; +} + +//! \brief Facility enum-to-string converter +//! +//! This does the same as enumFromJsonString, the other way around. +//! \note The source enumeration must not have gaps in values, or \p enumValues +//! has to match those gaps (i.e., if the source enumeration is defined +//! as { Value1 = 1, Value2 = 3, Value3 = 5 } then \p enumValues +//! should be defined as { "", "Value1", "", "Value2", "", "Value3" +//! } (mind the gap at value 0, in particular). +//! \sa enumFromJsonString +template +QString enumToJsonString(EnumT v, const EnumStringValuesT& enumValues) +{ + static_assert(std::is_unsigned_v>); + if (v < size(enumValues)) + return enumValues[v]; + + _impl::reportEnumOutOfBounds(static_cast(v), + qt_getEnumName(EnumT())); + Q_ASSERT(false); + return {}; +} + +//! \brief Facility converter for flags +//! +//! This is very similar to enumFromJsonString, except that the target +//! enumeration is assumed to be of a 'flag' kind - i.e. its values must be +//! a power-of-two sequence starting from 1, without gaps, so exactly 1,2,4,8,16 +//! and so on. +//! \note Unlike enumFromJsonString, the values start from 1 and not from 0, +//! with 0 being used for an invalid value by default. +//! \note This function does not support flag combinations. +//! \sa QUO_DECLARE_FLAGS, QUO_DECLARE_FLAGS_NS +template +FlagT flagFromJsonString(const QString& s, const FlagStringValuesT& flagValues, + FlagT defaultValue = FlagT(0U)) +{ + // Enums based on signed integers don't make much sense for flag types + static_assert(std::is_unsigned_v>); + if (const auto it = std::find(cbegin(flagValues), cend(flagValues), s); + it != cend(flagValues)) + return FlagT(1U << (it - cbegin(flagValues))); + + if (!s.isEmpty()) + _impl::warnUnknownEnumValue(s, qt_getEnumName(FlagT())); + return defaultValue; +} + +template +QString flagToJsonString(FlagT v, const FlagStringValuesT& flagValues) +{ + static_assert(std::is_unsigned_v>); + if (const auto offset = + qCountTrailingZeroBits(std::underlying_type_t(v)); + offset < size(flagValues)) // + { + return flagValues[offset]; + } + + _impl::reportEnumOutOfBounds(static_cast(v), + qt_getEnumName(FlagT())); + Q_ASSERT(false); + return {}; +} + // JsonConverter<> specialisations template<> diff --git a/lib/events/roomcreateevent.cpp b/lib/events/roomcreateevent.cpp index bb6de648..3b5024d5 100644 --- a/lib/events/roomcreateevent.cpp +++ b/lib/events/roomcreateevent.cpp @@ -6,20 +6,11 @@ using namespace Quotient; template <> -struct Quotient::JsonConverter { - static RoomType load(const QJsonValue& jv) - { - const auto& roomTypeString = jv.toString(); - for (auto it = RoomTypeStrings.begin(); it != RoomTypeStrings.end(); - ++it) - if (roomTypeString == *it) - return RoomType(it - RoomTypeStrings.begin()); - - if (!roomTypeString.isEmpty()) - qCWarning(EVENTS) << "Unknown Room Type: " << roomTypeString; - return RoomType::Undefined; - } -}; +RoomType Quotient::fromJson(const QJsonValue& jv) +{ + return enumFromJsonString(jv.toString(), RoomTypeStrings, + RoomType::Undefined); +} bool RoomCreateEvent::isFederated() const { diff --git a/lib/events/roommemberevent.cpp b/lib/events/roommemberevent.cpp index c3be0e00..953ff8ae 100644 --- a/lib/events/roommemberevent.cpp +++ b/lib/events/roommemberevent.cpp @@ -11,18 +11,10 @@ template <> struct JsonConverter { static Membership load(const QJsonValue& jv) { - const auto& ms = jv.toString(); - if (ms.isEmpty()) - { - qCWarning(EVENTS) << "Empty membership state"; - return Membership::Invalid; - } - const auto it = - std::find(MembershipStrings.begin(), MembershipStrings.end(), ms); - if (it != MembershipStrings.end()) - return Membership(1U << (it - MembershipStrings.begin())); - - qCWarning(EVENTS) << "Unknown Membership value: " << ms; + if (const auto& ms = jv.toString(); !ms.isEmpty()) + return flagFromJsonString(ms, MembershipStrings); + + qCWarning(EVENTS) << "Empty membership state"; return Membership::Invalid; } }; @@ -46,8 +38,7 @@ QJsonObject MemberEventContent::toJson() const QJsonObject o; if (membership != Membership::Invalid) o.insert(QStringLiteral("membership"), - MembershipStrings[qCountTrailingZeroBits( - std::underlying_type_t(membership))]); + flagToJsonString(membership, MembershipStrings)); if (displayName) o.insert(QStringLiteral("displayname"), *displayName); if (avatarUrl && avatarUrl->isValid()) -- cgit v1.2.3