From d271a171dbec4068e43e8f711d5d2e966a2072ac Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Tue, 4 Jan 2022 15:25:49 +0100 Subject: Simplify EventContent a bit Main changes: 1. Base::fillJson() gets a QJsonObject& instead of QJsonObject* - c'mon, there's nothing inherently wrong with using an lvalue reference for a read-write parameter. 2. UrlWithThumbnailContent merged into UrlBasedContent. The original UrlBasedContent was only used to produce a single class, AudioContent, and even that can logically have a thumbnail even if the spec doesn't provision that. And there's no guarantee even for visual content (ImageContent, e.g.) to have thumbnail data; the fallback is already tested. 3. toInfoJson is converted from a template to a couple of overloads that supersede fillInfoJson() member functions in FileInfo/ImageInfo. These overloads are easier on the eye; and clang-tidy no more warns about ImageInfo::fillInfoJson() shadowing FileInfo::fillInfoJson(). 4. Now that UrlWithThumbnail is gone, PlayableContent can directly derive from UrlBasedContent since both its specialisations use it. 5. Instead of FileInfo/ImageInfo, fillInfoJson() has been reinvented within UrlBasedContent so that, in particular, PlayableContent wouldn't need to extract 'info' subobject and then roll it back inside the content JSON object. --- lib/events/encryptionevent.cpp | 9 ++-- lib/events/encryptionevent.h | 2 +- lib/events/eventcontent.cpp | 36 ++++++++-------- lib/events/eventcontent.h | 84 ++++++++++++++----------------------- lib/events/roommemberevent.cpp | 11 +++-- lib/events/roommemberevent.h | 2 +- lib/events/roommessageevent.cpp | 18 ++++---- lib/events/roommessageevent.h | 29 +++++++------ lib/events/roompowerlevelsevent.cpp | 22 +++++----- lib/events/roompowerlevelsevent.h | 2 +- 10 files changed, 98 insertions(+), 117 deletions(-) (limited to 'lib/events') diff --git a/lib/events/encryptionevent.cpp b/lib/events/encryptionevent.cpp index 6272c668..47b0a032 100644 --- a/lib/events/encryptionevent.cpp +++ b/lib/events/encryptionevent.cpp @@ -47,11 +47,10 @@ EncryptionEventContent::EncryptionEventContent(EncryptionType et) } } -void EncryptionEventContent::fillJson(QJsonObject* o) const +void EncryptionEventContent::fillJson(QJsonObject& o) const { - Q_ASSERT(o); if (encryption != EncryptionType::Undefined) - o->insert(AlgorithmKey, algorithm); - o->insert(RotationPeriodMsKey, rotationPeriodMs); - o->insert(RotationPeriodMsgsKey, rotationPeriodMsgs); + 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 124ced33..fe76b1af 100644 --- a/lib/events/encryptionevent.h +++ b/lib/events/encryptionevent.h @@ -26,7 +26,7 @@ public: int rotationPeriodMsgs; protected: - void fillJson(QJsonObject* o) const override; + void fillJson(QJsonObject& o) const override; }; using EncryptionType = EncryptionEventContent::EncryptionType; diff --git a/lib/events/eventcontent.cpp b/lib/events/eventcontent.cpp index 479d5da0..6218e3b8 100644 --- a/lib/events/eventcontent.cpp +++ b/lib/events/eventcontent.cpp @@ -15,7 +15,7 @@ using std::move; QJsonObject Base::toJson() const { QJsonObject o; - fillJson(&o); + fillJson(o); return o; } @@ -68,14 +68,15 @@ bool FileInfo::isValid() const && (url.authority() + url.path()).count('/') == 1; } -void FileInfo::fillInfoJson(QJsonObject* infoJson) const +QJsonObject Quotient::EventContent::toInfoJson(const FileInfo& info) { - Q_ASSERT(infoJson); - if (payloadSize != -1) - infoJson->insert(QStringLiteral("size"), payloadSize); - if (mimeType.isValid()) - infoJson->insert(QStringLiteral("mimetype"), mimeType.name()); + QJsonObject infoJson; + if (info.payloadSize != -1) + infoJson.insert(QStringLiteral("size"), info.payloadSize); + if (info.mimeType.isValid()) + infoJson.insert(QStringLiteral("mimetype"), info.mimeType.name()); //TODO add encryptedfile + return infoJson; } ImageInfo::ImageInfo(const QFileInfo& fi, QSize imageSize) @@ -96,13 +97,14 @@ ImageInfo::ImageInfo(const QUrl& mxcUrl, const QJsonObject& infoJson, , imageSize(infoJson["w"_ls].toInt(), infoJson["h"_ls].toInt()) {} -void ImageInfo::fillInfoJson(QJsonObject* infoJson) const +QJsonObject Quotient::EventContent::toInfoJson(const ImageInfo& info) { - FileInfo::fillInfoJson(infoJson); - if (imageSize.width() != -1) - infoJson->insert(QStringLiteral("w"), imageSize.width()); - if (imageSize.height() != -1) - infoJson->insert(QStringLiteral("h"), imageSize.height()); + auto infoJson = toInfoJson(static_cast(info)); + if (info.imageSize.width() != -1) + infoJson.insert(QStringLiteral("w"), info.imageSize.width()); + if (info.imageSize.height() != -1) + infoJson.insert(QStringLiteral("h"), info.imageSize.height()); + return infoJson; } Thumbnail::Thumbnail(const QJsonObject& infoJson, @@ -111,11 +113,11 @@ Thumbnail::Thumbnail(const QJsonObject& infoJson, infoJson["thumbnail_info"_ls].toObject(), move(encryptedFile)) {} -void Thumbnail::fillInfoJson(QJsonObject* infoJson) const +void Thumbnail::dumpTo(QJsonObject& infoJson) const { if (url.isValid()) - infoJson->insert(QStringLiteral("thumbnail_url"), url.toString()); + infoJson.insert(QStringLiteral("thumbnail_url"), url.toString()); if (!imageSize.isEmpty()) - infoJson->insert(QStringLiteral("thumbnail_info"), - toInfoJson(*this)); + infoJson.insert(QStringLiteral("thumbnail_info"), + toInfoJson(*this)); } diff --git a/lib/events/eventcontent.h b/lib/events/eventcontent.h index 4134202a..6aee333d 100644 --- a/lib/events/eventcontent.h +++ b/lib/events/eventcontent.h @@ -40,7 +40,7 @@ namespace EventContent { Base(const Base&) = default; Base(Base&&) = default; - virtual void fillJson(QJsonObject* o) const = 0; + virtual void fillJson(QJsonObject& o) const = 0; }; // The below structures fairly follow CS spec 11.2.1.6. The overall @@ -50,12 +50,14 @@ namespace EventContent { // A quick classes inheritance structure follows (the definitions are // spread across eventcontent.h and roommessageevent.h): + // UrlBasedContent : InfoT + url and thumbnail data + // PlayableContent : + duration attribute // FileInfo - // FileContent : UrlWithThumbnailContent - // AudioContent : PlayableContent> + // FileContent = UrlBasedContent + // AudioContent = PlayableContent // ImageInfo : FileInfo + imageSize attribute - // ImageContent : UrlWithThumbnailContent - // VideoContent : PlayableContent> + // ImageContent = UrlBasedContent + // VideoContent = PlayableContent //! \brief Mix-in class representing `info` subobject in content JSON //! @@ -117,13 +119,7 @@ namespace EventContent { Omittable file = none; }; - template - QJsonObject toInfoJson(const InfoT& info) - { - QJsonObject infoJson; - info.fillInfoJson(&infoJson); - return infoJson; - } + QUOTIENT_API QJsonObject toInfoJson(const FileInfo& info); //! \brief A content info class for image/video content types and thumbnails class QUOTIENT_API ImageInfo : public FileInfo { @@ -138,12 +134,12 @@ namespace EventContent { Omittable encryptedFile, const QString& originalFilename = {}); - void fillInfoJson(QJsonObject* infoJson) const; - public: QSize imageSize; }; + QUOTIENT_API QJsonObject toInfoJson(const ImageInfo& info); + //! \brief An auxiliary class for an info type that carries a thumbnail //! //! This class saves/loads a thumbnail to/from `info` subobject of @@ -156,7 +152,7 @@ namespace EventContent { Omittable encryptedFile = none); //! \brief Add thumbnail information to the passed `info` JSON object - void fillInfoJson(QJsonObject* infoJson) const; + void dumpTo(QJsonObject& infoJson) const; }; class QUOTIENT_API TypedBase : public Base { @@ -185,57 +181,41 @@ namespace EventContent { explicit UrlBasedContent(const QJsonObject& json) : TypedBase(json) , InfoT(QUrl(json["url"].toString()), json["info"].toObject(), - fromJson>(json["file"]), json["filename"].toString()) + fromJson>(json["file"]), + json["filename"].toString()) + , thumbnail(FileInfo::originalInfoJson) { - // A small hack to facilitate links creation in QML. + // Two small hacks on originalJson to expose mediaIds to QML originalJson.insert("mediaId", InfoT::mediaId()); + originalJson.insert("thumbnailMediaId", thumbnail.mediaId()); } QMimeType type() const override { return InfoT::mimeType; } const FileInfo* fileInfo() const override { return this; } FileInfo* fileInfo() override { return this; } - - protected: - void fillJson(QJsonObject* json) const override - { - Q_ASSERT(json); - if (!InfoT::file.has_value()) { - json->insert("url", InfoT::url.toString()); - } else { - json->insert("file", Quotient::toJson(*InfoT::file)); - } - if (!InfoT::originalName.isEmpty()) - json->insert("filename", InfoT::originalName); - json->insert("info", toInfoJson(*this)); - } - }; - - template - class QUOTIENT_API UrlWithThumbnailContent : public UrlBasedContent { - public: - // NB: when using inherited constructors, thumbnail has to be - // initialised separately - using UrlBasedContent::UrlBasedContent; - explicit UrlWithThumbnailContent(const QJsonObject& json) - : UrlBasedContent(json), thumbnail(InfoT::originalInfoJson) - { - // Another small hack, to simplify making a thumbnail link - UrlBasedContent::originalJson.insert("thumbnailMediaId", - thumbnail.mediaId()); - } - const Thumbnail* thumbnailInfo() const override { return &thumbnail; } public: Thumbnail thumbnail; protected: - void fillJson(QJsonObject* json) const override + virtual void fillInfoJson(QJsonObject& infoJson [[maybe_unused]]) const + {} + + void fillJson(QJsonObject& json) const override { - UrlBasedContent::fillJson(json); - auto infoJson = json->take("info").toObject(); - thumbnail.fillInfoJson(&infoJson); - json->insert("info", infoJson); + if (!InfoT::file.has_value()) { + json.insert("url", InfoT::url.toString()); + } else { + json.insert("file", Quotient::toJson(*InfoT::file)); + } + if (!InfoT::originalName.isEmpty()) + json.insert("filename", InfoT::originalName); + auto infoJson = toInfoJson(*this); + if (thumbnail.isValid()) + thumbnail.dumpTo(infoJson); + fillInfoJson(infoJson); + json.insert("info", infoJson); } }; diff --git a/lib/events/roommemberevent.cpp b/lib/events/roommemberevent.cpp index b4770224..55da5809 100644 --- a/lib/events/roommemberevent.cpp +++ b/lib/events/roommemberevent.cpp @@ -43,19 +43,18 @@ MemberEventContent::MemberEventContent(const QJsonObject& json) displayName = sanitized(*displayName); } -void MemberEventContent::fillJson(QJsonObject* o) const +void MemberEventContent::fillJson(QJsonObject& o) const { - Q_ASSERT(o); if (membership != Membership::Invalid) - o->insert(QStringLiteral("membership"), + o.insert(QStringLiteral("membership"), MembershipStrings[qCountTrailingZeroBits( std::underlying_type_t(membership))]); if (displayName) - o->insert(QStringLiteral("displayname"), *displayName); + o.insert(QStringLiteral("displayname"), *displayName); if (avatarUrl && avatarUrl->isValid()) - o->insert(QStringLiteral("avatar_url"), avatarUrl->toString()); + o.insert(QStringLiteral("avatar_url"), avatarUrl->toString()); if (!reason.isEmpty()) - o->insert(QStringLiteral("reason"), reason); + o.insert(QStringLiteral("reason"), reason); } bool RoomMemberEvent::changesMembership() const diff --git a/lib/events/roommemberevent.h b/lib/events/roommemberevent.h index ceb7826b..afbaf825 100644 --- a/lib/events/roommemberevent.h +++ b/lib/events/roommemberevent.h @@ -26,7 +26,7 @@ public: QString reason; protected: - void fillJson(QJsonObject* o) const override; + void fillJson(QJsonObject& o) const override; }; using MembershipType [[deprecated("Use Membership instead")]] = Membership; diff --git a/lib/events/roommessageevent.cpp b/lib/events/roommessageevent.cpp index d63352cb..d9d3fbe0 100644 --- a/lib/events/roommessageevent.cpp +++ b/lib/events/roommessageevent.cpp @@ -302,17 +302,16 @@ TextContent::TextContent(const QJsonObject& json) } } -void TextContent::fillJson(QJsonObject* json) const +void TextContent::fillJson(QJsonObject &json) const { static const auto FormatKey = QStringLiteral("format"); - Q_ASSERT(json); if (mimeType.inherits("text/html")) { - json->insert(FormatKey, HtmlContentTypeId); - json->insert(FormattedBodyKey, body); + json.insert(FormatKey, HtmlContentTypeId); + json.insert(FormattedBodyKey, body); } if (relatesTo) { - json->insert( + json.insert( QStringLiteral("m.relates_to"), relatesTo->type == EventRelation::ReplyType ? QJsonObject { { relatesTo->type, @@ -326,7 +325,7 @@ void TextContent::fillJson(QJsonObject* json) const newContentJson.insert(FormatKey, HtmlContentTypeId); newContentJson.insert(FormattedBodyKey, body); } - json->insert(QStringLiteral("m.new_content"), newContentJson); + json.insert(QStringLiteral("m.new_content"), newContentJson); } } } @@ -347,9 +346,8 @@ QMimeType LocationContent::type() const return QMimeDatabase().mimeTypeForData(geoUri.toLatin1()); } -void LocationContent::fillJson(QJsonObject* o) const +void LocationContent::fillJson(QJsonObject& o) const { - Q_ASSERT(o); - o->insert(QStringLiteral("geo_uri"), geoUri); - o->insert(QStringLiteral("info"), toInfoJson(thumbnail)); + o.insert(QStringLiteral("geo_uri"), geoUri); + o.insert(QStringLiteral("info"), toInfoJson(thumbnail)); } diff --git a/lib/events/roommessageevent.h b/lib/events/roommessageevent.h index 03a51328..6968ad70 100644 --- a/lib/events/roommessageevent.h +++ b/lib/events/roommessageevent.h @@ -136,7 +136,7 @@ namespace EventContent { Omittable relatesTo; protected: - void fillJson(QJsonObject* json) const override; + void fillJson(QJsonObject& json) const override; }; /** @@ -164,28 +164,25 @@ namespace EventContent { Thumbnail thumbnail; protected: - void fillJson(QJsonObject* o) const override; + void fillJson(QJsonObject& o) const override; }; /** * A base class for info types that include duration: audio and video */ - template - class QUOTIENT_API PlayableContent : public ContentT { + template + class PlayableContent : public UrlBasedContent { public: - using ContentT::ContentT; + using UrlBasedContent::UrlBasedContent; PlayableContent(const QJsonObject& json) - : ContentT(json) - , duration(ContentT::originalInfoJson["duration"_ls].toInt()) + : UrlBasedContent(json) + , duration(FileInfo::originalInfoJson["duration"_ls].toInt()) {} protected: - void fillJson(QJsonObject* json) const override + void fillInfoJson(QJsonObject& infoJson) const override { - ContentT::fillJson(json); - auto infoJson = json->take("info"_ls).toObject(); infoJson.insert(QStringLiteral("duration"), duration); - json->insert(QStringLiteral("info"), infoJson); } public: @@ -211,7 +208,7 @@ namespace EventContent { * - mimeType * - imageSize */ - using VideoContent = PlayableContent>; + using VideoContent = PlayableContent; /** * Content class for m.audio @@ -224,7 +221,13 @@ namespace EventContent { * - payloadSize ("size" in JSON) * - mimeType ("mimetype" in JSON) * - duration + * - thumbnail.url ("thumbnail_url" in JSON - extension to the spec) + * - corresponding to the "info/thumbnail_info" subobject: contents of + * thumbnail field (extension to the spec): + * - payloadSize + * - mimeType + * - imageSize */ - using AudioContent = PlayableContent>; + using AudioContent = PlayableContent; } // namespace EventContent } // namespace Quotient diff --git a/lib/events/roompowerlevelsevent.cpp b/lib/events/roompowerlevelsevent.cpp index 8d262ddf..1c87fd47 100644 --- a/lib/events/roompowerlevelsevent.cpp +++ b/lib/events/roompowerlevelsevent.cpp @@ -21,17 +21,17 @@ PowerLevelsEventContent::PowerLevelsEventContent(const QJsonObject& json) : { } -void PowerLevelsEventContent::fillJson(QJsonObject* o) const { - o->insert(QStringLiteral("invite"), invite); - o->insert(QStringLiteral("kick"), kick); - o->insert(QStringLiteral("ban"), ban); - o->insert(QStringLiteral("redact"), redact); - o->insert(QStringLiteral("events"), Quotient::toJson(events)); - o->insert(QStringLiteral("events_default"), eventsDefault); - o->insert(QStringLiteral("state_default"), stateDefault); - o->insert(QStringLiteral("users"), Quotient::toJson(users)); - o->insert(QStringLiteral("users_default"), usersDefault); - o->insert(QStringLiteral("notifications"), QJsonObject{{"room", notifications.room}}); +void PowerLevelsEventContent::fillJson(QJsonObject& o) const { + o.insert(QStringLiteral("invite"), invite); + o.insert(QStringLiteral("kick"), kick); + o.insert(QStringLiteral("ban"), ban); + o.insert(QStringLiteral("redact"), redact); + o.insert(QStringLiteral("events"), Quotient::toJson(events)); + o.insert(QStringLiteral("events_default"), eventsDefault); + o.insert(QStringLiteral("state_default"), stateDefault); + o.insert(QStringLiteral("users"), Quotient::toJson(users)); + o.insert(QStringLiteral("users_default"), usersDefault); + o.insert(QStringLiteral("notifications"), QJsonObject{{"room", notifications.room}}); } int RoomPowerLevelsEvent::powerLevelForEvent(const QString &eventId) const { diff --git a/lib/events/roompowerlevelsevent.h b/lib/events/roompowerlevelsevent.h index 415cc814..3ce83763 100644 --- a/lib/events/roompowerlevelsevent.h +++ b/lib/events/roompowerlevelsevent.h @@ -31,7 +31,7 @@ public: Notifications notifications; protected: - void fillJson(QJsonObject* o) const override; + void fillJson(QJsonObject& o) const override; }; class QUOTIENT_API RoomPowerLevelsEvent -- cgit v1.2.3