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<QJsonValue, QJsonValue> 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.

5 files changed, 50 insertions, 61 deletions

diff --git a/lib/converters.h b/lib/converters.h
index c445442c..9f42d712 100644
--- a/lib/converters.h
+++ b/lib/converters.h
@@ -28,23 +28,8 @@ struct JsonObjectConverter {
     static void fillFrom(const QJsonObject&, T&) = delete;
 };
 
-namespace _impl {
-    template <typename T, typename = void>
-    struct JsonExporter {
-        static QJsonObject dump(const T& data)
-        {
-            QJsonObject jo;
-            JsonObjectConverter<T>::dumpTo(jo, data);
-            return jo;
-        }
-    };
-
-    template <typename T>
-    struct JsonExporter<
-        T, std::enable_if_t<std::is_invocable_v<decltype(&T::toJson), T>>> {
-        static auto dump(const T& data) { return data.toJson(); }
-    };
-}
+template <typename PodT, typename JsonT>
+PodT fromJson(const JsonT&);
 
 //! \brief The switchboard for extra conversion algorithms behind from/toJson
 //!
@@ -62,13 +47,23 @@ namespace _impl {
 //! that they are not supported and it's not feasible to support those by means
 //! of overloading toJson() and specialising fromJson().
 template <typename T>
-struct JsonConverter : _impl::JsonExporter<T> {
+struct JsonConverter {
     // Unfortunately, if constexpr doesn't work with dump() and T::toJson
     // because trying to check invocability of T::toJson hits a hard
     // (non-SFINAE) compilation error if the member is not there. Hence a bit
     // more verbose SFINAE construct in _impl::JsonExporter.
+    static auto dump(const T& data)
+    {
+        if constexpr (requires() { data.toJson(); })
+            return data.toJson();
+        else {
+            QJsonObject jo;
+            JsonObjectConverter<T>::dumpTo(jo, data);
+            return jo;
+        }
+    }
 
-    static T doLoad(const QJsonObject& jo)
+    static T load(const QJsonObject& jo)
     {
         // 'else' below are required to suppress code generation for unused
         // branches - 'return' is not enough
@@ -82,65 +77,57 @@ struct JsonConverter : _impl::JsonExporter<T> {
             return pod;
         }
     }
-    static T load(const QJsonValue& jv) { return doLoad(jv.toObject()); }
-    static T load(const QJsonDocument& jd) { return doLoad(jd.object()); }
+    // By default, revert to fromJson() so that one could provide a single
+    // fromJson<T, QJsonObject> specialisation instead of specialising
+    // the entire JsonConverter; if a different type of JSON value is needed
+    // (e.g., an array), specialising JsonConverter is inevitable
+    static T load(QJsonValueRef jvr) { return fromJson<T>(QJsonValue(jvr)); }
+    static T load(const QJsonValue& jv) { return fromJson<T>(jv.toObject()); }
+    static T load(const QJsonDocument& jd) { return fromJson<T>(jd.object()); }
 };
 
 template <typename T>
-    requires (!std::is_constructible_v<QJsonValue, T>)
 inline auto toJson(const T& pod)
 // -> can return anything from which QJsonValue or, in some cases, QJsonDocument
 //    is constructible
 {
-    return JsonConverter<T>::dump(pod);
+    if constexpr (std::is_constructible_v<QJsonValue, T>)
+        return pod; // No-op if QJsonValue can be directly constructed
+    else
+        return JsonConverter<T>::dump(pod);
 }
 
-inline auto toJson(const QJsonObject& jo) { return jo; }
-inline auto toJson(const QJsonValue& jv) { return jv; }
-
 template <typename T>
 inline void fillJson(QJsonObject& json, const T& data)
 {
     JsonObjectConverter<T>::dumpTo(json, data);
 }
 
-template <typename T>
-inline T fromJson(const QJsonValue& jv)
+template <typename PodT, typename JsonT>
+inline PodT fromJson(const JsonT& json)
 {
-    return JsonConverter<T>::load(jv);
+    // JsonT here can be whatever the respective JsonConverter specialisation
+    // accepts but by default it's QJsonValue, QJsonDocument, or QJsonObject
+    return JsonConverter<PodT>::load(json);
 }
 
-template<>
-inline QJsonValue fromJson(const QJsonValue& jv) { return jv; }
-
-template <typename T>
-inline T fromJson(const QJsonDocument& jd)
-{
-    return JsonConverter<T>::load(jd);
-}
-
-// Convenience fromJson() overloads that deduce T instead of requiring
-// the coder to explicitly type it. They still enforce the
+// Convenience fromJson() overload that deduces PodT instead of requiring
+// the coder to explicitly type it. It still enforces the
 // overwrite-everything semantics of fromJson(), unlike fillFromJson()
 
-template <typename T>
-inline void fromJson(const QJsonValue& jv, T& pod)
-{
-    pod = jv.isUndefined() ? T() : fromJson<T>(jv);
-}
-
-template <typename T>
-inline void fromJson(const QJsonDocument& jd, T& pod)
+template <typename JsonT, typename PodT>
+inline void fromJson(const JsonT& json, PodT& pod)
 {
-    pod = fromJson<T>(jd);
+    pod = fromJson<PodT>(json);
 }
 
 template <typename T>
 inline void fillFromJson(const QJsonValue& jv, T& pod)
 {
-    if (jv.isObject())
+    if constexpr (requires() { JsonObjectConverter<T>::fillFrom({}, pod); }) {
         JsonObjectConverter<T>::fillFrom(jv.toObject(), pod);
-    else if (!jv.isUndefined())
+        return;
+    } else if (!jv.isUndefined())
         pod = fromJson<T>(jv);
 }
 
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<QJsonObject>("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<QString>("redacts"_ls);
+        !redactsJson.isEmpty()) {
         auto unsign = eventObject["unsigned"].toObject();
-        unsign["redacts"] = redactsJson.toString();
+        unsign["redacts"] = redactsJson;
         eventObject["unsigned"] = unsign;
     }
     return loadEvent<RoomEvent>(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 <typename T = QJsonValue, typename KeyT>
+    template <typename T, typename KeyT>
     const T contentPart(KeyT&& key) const
     {
         return fromJson<T>(contentJson()[std::forward<KeyT>(key)]);
@@ -227,7 +227,7 @@ public:
 
     const QJsonObject unsignedJson() const;
 
-    template <typename T = QJsonValue, typename KeyT>
+    template <typename T, typename KeyT>
     const T unsignedPart(KeyT&& key) const
     {
         return fromJson<T>(unsignedJson()[std::forward<KeyT>(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<RedactionEvent>(redaction.toObject());
+    if (const auto redaction = unsignedPart<QJsonObject>(RedactedCauseKeyL);
+        !redaction.isEmpty())
+        _redactedBecause = makeEvent<RedactionEvent>(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<QJsonObject>(PrevContentKeyL);
     return fullJson().value(ContentKeyL) == prevContentJson;
 }