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.

4 files changed, 49 insertions, 52 deletions

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<EncryptionType> {
-    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<EncryptionType>(json[AlgorithmKeyL]))
+    : encryption(fromJson<Quotient::EncryptionType>(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<int>(json[RotationPeriodMsKeyL], rotationPeriodMs);
+    fillFromJson<int>(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<size_t>(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<EncryptionEventContent> {
-    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<EncryptionEvent>(EncryptionEventContent::MegolmV1AesSha2);
+    setState<EncryptionEvent>(EncryptionType::MegolmV1AesSha2);
 }