Wrap SyncRoomData counters into Omittables

Also: introduce a merge(T1&, const Omittable<T2>&) that does pretty much
the same as Omittable<T1>::merge(const Omittable<T2>&) except it works
on non-omittables as the left/first operand.
The change removes the need for a clumsy -2 fallback in unreadCount,
and makes the logic loading those counters cleaner along the way.

4 files changed, 58 insertions, 25 deletions

diff --git a/lib/room.cpp b/lib/room.cpp
index 54c67c2f..076fd8c8 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -1570,20 +1570,18 @@ void Room::updateData(SyncRoomData&& data, bool fromCache)
         roomChanges |= processEphemeralEvent(move(ephemeralEvent));
 
     // See https://github.com/quotient-im/libQuotient/wiki/unread_count
-    if (data.unreadCount != -2 && data.unreadCount != d->unreadMessages) {
-        qCDebug(MESSAGES) << "Setting unread_count to" << data.unreadCount;
-        d->unreadMessages = data.unreadCount;
+    if (merge(d->unreadMessages, data.unreadCount)) {
+        qCDebug(MESSAGES) << "Loaded unread_count:" << *data.unreadCount //
+                          << "in" << objectName();
         emit unreadMessagesChanged(this);
     }
 
-    if (data.highlightCount != d->highlightCount) {
-        d->highlightCount = data.highlightCount;
+    if (merge(d->highlightCount, data.highlightCount))
         emit highlightCountChanged();
-    }
-    if (data.notificationCount != d->notificationCount) {
-        d->notificationCount = data.notificationCount;
+
+    if (merge(d->notificationCount, data.notificationCount))
         emit notificationCountChanged();
-    }
+
     if (roomChanges != Change::NoChange) {
         d->updateDisplayname();
         emit changed(roomChanges);
diff --git a/lib/syncdata.cpp b/lib/syncdata.cpp
index 232f3694..d3c270b5 100644
--- a/lib/syncdata.cpp
+++ b/lib/syncdata.cpp
@@ -90,13 +90,13 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_,
     }
 
     const auto unreadJson = room_.value("unread_notifications"_ls).toObject();
-    unreadCount = unreadJson.value(UnreadCountKey).toInt(-2);
-    highlightCount = unreadJson.value("highlight_count"_ls).toInt();
-    notificationCount = unreadJson.value("notification_count"_ls).toInt();
-    if (highlightCount > 0 || notificationCount > 0)
+    fromJson(unreadJson.value(UnreadCountKey), unreadCount);
+    fromJson(unreadJson.value("highlight_count"_ls), highlightCount);
+    fromJson(unreadJson.value("notification_count"_ls), notificationCount);
+    if (highlightCount.has_value() || notificationCount.has_value())
         qCDebug(SYNCJOB) << "Room" << roomId_
-                         << "has highlights:" << highlightCount
-                         << "and notifications:" << notificationCount;
+                         << "has highlights:" << *highlightCount
+                         << "and notifications:" << *notificationCount;
 }
 
 SyncData::SyncData(const QString& cacheFileName)
diff --git a/lib/syncdata.h b/lib/syncdata.h
index 0153bfd6..b0e31726 100644
--- a/lib/syncdata.h
+++ b/lib/syncdata.h
@@ -48,9 +48,9 @@ public:
 
     bool timelineLimited;
     QString timelinePrevBatch;
-    int unreadCount;
-    int highlightCount;
-    int notificationCount;
+    Omittable<int> unreadCount;
+    Omittable<int> highlightCount;
+    Omittable<int> notificationCount;
 
     SyncRoomData(const QString& roomId, JoinState joinState_,
                  const QJsonObject& room_);
diff --git a/lib/util.h b/lib/util.h
index 7f09de28..78fc9ab7 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -30,6 +30,13 @@ struct HashQ {
 template <typename KeyT, typename ValT>
 using UnorderedMap = std::unordered_map<KeyT, ValT, HashQ<KeyT>>;
 
+namespace _impl {
+    template <typename TT>
+    constexpr inline auto IsOmittableValue = false;
+    template <typename TT>
+    constexpr inline auto IsOmittable = IsOmittableValue<std::decay_t<TT>>;
+}
+
 constexpr auto none = std::nullopt;
 
 /** `std::optional` with tweaks
@@ -107,18 +114,18 @@ public:
         return !this->has_value();
     }
 
-    /// Merge the value from another Omittable
-    /// \return true if \p other is not omitted and the value of
-    ///         the current Omittable was different (or omitted);
-    ///         in other words, if the current Omittable has changed;
-    ///         false otherwise
+    //! Merge the value from another Omittable
+    //! \return true if \p other is not omitted and the value of
+    //!         the current Omittable was different (or omitted),
+    //!         in other words, if the current Omittable has changed;
+    //!         false otherwise
     template <typename T1>
     auto merge(const Omittable<T1>& other)
-        -> std::enable_if_t<std::is_convertible<T1, T>::value, bool>
+        -> std::enable_if_t<std::is_convertible_v<T1, T>, bool>
     {
         if (!other || (this->has_value() && **this == *other))
             return false;
-        *this = other;
+        emplace(*other);
         return true;
     }
 
@@ -133,6 +140,34 @@ public:
 };
 
 namespace _impl {
+    template <typename T>
+    constexpr inline auto IsOmittableValue<Omittable<T>> = true;
+}
+
+template <typename T1, typename T2>
+inline auto merge(Omittable<T1>& lhs, T2&& rhs)
+{
+    return lhs.merge(std::forward<T2>(rhs));
+}
+
+//! \brief Merge the value from an Omittable
+//! This is an adaptation of Omittable::merge() to the case when the value
+//! on the left hand side is not an Omittable.
+//! \return true if \p rhs is not omitted and the \p lhs value was different,
+//!         in other words, if \p lhs has changed;
+//!         false otherwise
+template <typename T1, typename T2>
+inline auto merge(T1& lhs, const Omittable<T2>& rhs)
+    -> std::enable_if_t<!_impl::IsOmittable<T1>
+                            && std::is_convertible_v<T2, T1>, bool>
+{
+    if (!rhs || lhs == *rhs)
+        return false;
+    lhs = *rhs;
+    return true;
+}
+
+namespace _impl {
     template <typename AlwaysVoid, typename>
     struct fn_traits {};
 }