More tweaks and cleanup

Quotient/connection.cpp

@@ -12,6 +12,7 @@
 #include "database.h"
 #include "logging_categories_p.h"
 #include "qt_connection_util.h"
+#include "ranges_extras.h"
 #include "room.h"
 #include "settings.h"
 #include "user.h"
@@ -1665,13 +1666,12 @@ QVector<Connection::SupportedRoomVersion> Connection::availableRoomVersions() co
 
     // Can't stuff QKeyValueRange in a std:: view directly because it's not move-assignable and
     // most views require that - using std::views::all to go around this
-    // TODO: use std::ranges::to() when all toolchains support it
     const auto allVersions = d->capabilities.roomVersions->available.asKeyValueRange();
-    auto allVersionsView = std::views::all(allVersions) | std::views::transform([](const auto& p) {
-                               return SupportedRoomVersion{ p.first, p.second };
-                           });
-    QVector<SupportedRoomVersion> result(allVersionsView.begin(), allVersionsView.end());
-    // Put stable versions over unstable; for
+    auto result =
+        rangeTo<QVector>(std::views::all(allVersions) | std::views::transform([](const auto& p) {
+                             return SupportedRoomVersion{ p.first, p.second };
+                         }));
+    // Put stable versions over unstable
     std::ranges::sort(result, [](const SupportedRoomVersion& v1, const SupportedRoomVersion& v2) {
         if (const auto stable1 = v1.isStable(), stable2 = v2.isStable(); stable1 != stable2)
             return stable1 && !stable2; // Put all stable versions over unstable

Quotient/ranges_extras.h

@@ -16,4 +16,36 @@ inline auto findIndex(const RangeT& range, const ValT& value, ProjT proj = {})
     return distance(begin(range), find(range, value, std::move(proj)));
 }
 
+//! \brief A replacement of std::ranges::to() while toolchains catch up
+//!
+//! Returns a container of type \p TargetT created from \p sourceRange. Unlike std::ranges::to(),
+//! you have to pass the range to it (e.g. `rangeTo<TargetT>(someRange)`); using it in a pipeline
+//! (`someRange | rangeTo<TargetT>()`) won't compile. Internally calls std::ranges::to() if it's
+//! available; otherwise, returns the result of calling
+//! `TargetT(ranges::begin(sourceRange), ranges::end(sourceRange))`.
+template <class TargetT, typename SourceT>
+[[nodiscard]] constexpr auto rangeTo(SourceT&& sourceRange)
+{
+#if defined(__cpp_lib_ranges_to_container)
+    return std::ranges::to<TargetT>(std::forward<SourceT>(sourceRange));
+#else
+    using std::begin, std::end;
+    return TargetT(begin(sourceRange), end(sourceRange));
+#endif
+}
+
+//! An overload that accepts unspecialised container template
+template <template <typename> class TargetT, typename SourceT>
+[[nodiscard]] constexpr auto rangeTo(SourceT&& sourceRange)
+{
+    // Avoid template argument deduction because Xcode still can't do it when TargetT is an alias
+#if defined(__cpp_lib_ranges_to_container)
+    return std::ranges::to<TargetT<std::ranges::range_value_t<SourceT>>>(
+        std::forward<SourceT>(sourceRange));
+#else
+    using std::begin, std::end;
+    return TargetT<std::ranges::range_value_t<SourceT>>(begin(sourceRange), end(sourceRange));
+#endif
+}
+
 }

Quotient/room.cpp

@@ -75,7 +75,6 @@
 #include <functional>
 
 using namespace Quotient;
-using namespace std::placeholders;
 using std::llround;
 
 enum EventsPlacement : int { Older = -1, Newer = 1 };
@@ -1734,15 +1733,16 @@ Room::Private::moveEventsToTimeline(RoomEventsRange events,
                                           : timeline.back().index();
     auto baseIndex = index;
     for (auto&& e : events) {
-        Q_ASSERT_X(e, __FUNCTION__, "Attempt to add nullptr to timeline");
+        if (QUO_ALARM_X(e == nullptr, "Attempt to add nullptr to timeline"))
+            continue;
         const auto eId = e->id();
-        Q_ASSERT_X(
-            !eId.isEmpty(), __FUNCTION__,
-            makeErrorStr(*e, "Event with empty id cannot be in the timeline"));
-        Q_ASSERT_X(
-            !eventsIndex.contains(eId), __FUNCTION__,
-            makeErrorStr(*e, "Event is already in the timeline; "
-                             "incoming events were not properly deduplicated"));
+        if (QUO_ALARM_X(eId.isEmpty(),
+                        makeErrorStr(*e, "An event with empty id cannot be in the timeline")))
+            continue;
+        if (QUO_ALARM_X(eventsIndex.contains(eId),
+                        makeErrorStr(*e, "Event is already in the timeline; "
+                                         "incoming events were not properly deduplicated")))
+            continue;
         const auto& ti = placement == Older
                              ? timeline.emplace_front(std::move(e), --index)
                              : timeline.emplace_back(std::move(e), ++index);
@@ -1762,7 +1762,7 @@ Room::Private::moveEventsToTimeline(RoomEventsRange events,
         updateThread(ti.event());
     }
     const auto insertedSize = (index - baseIndex) * placement;
-    Q_ASSERT(insertedSize == int(events.size()));
+    QUO_CHECK(insertedSize == int(events.size()));
     return Timeline::size_type(insertedSize);
 }
 
@@ -2803,18 +2803,21 @@ void Room::Private::addRelation(const ReactionEvent& reactionEvt)
         return;
     }
     thisEventReactions << &reactionEvt;
-    emit q->updatedEvent(content.eventId);
+    if (q->findInTimeline(content.eventId) != historyEdge())
+        emit q->updatedEvent(content.eventId);
 }
 
+namespace {
 /// Whether the event is a redaction or a replacement
 inline bool isEditing(const RoomEventPtr& ep)
 {
-    Q_ASSERT(ep);
-    return ep->switchOnType([](const RedactionEvent&) { return true; },
-                     [](const RoomMessageEvent& rme) {
-                         return !rme.replacedEvent().isEmpty();
-                     },
-                     false);
+    return QUO_CHECK(ep != nullptr)
+           && ep->switchOnType([](const RedactionEvent&) { return true; },
+                               [](const RoomMessageEvent& rme) {
+                                   return !rme.replacedEvent().isEmpty();
+                               },
+                               false);
+}
 }
 
 Room::Timeline::size_type Room::Private::mergePendingEvent(PendingEvents::iterator localEchoIt,

quotest/quotest.cpp

@@ -129,8 +129,8 @@ private slots:
     template <EventClass<RoomEvent> EventT>
     [[nodiscard]] bool validatePendingEvent(const QString& txnId);
     [[nodiscard]] bool checkDirectChat() const;
-    void finishTest(const TestToken& token, bool condition, const char* file,
-                    int line);
+    void finishTest(const TestToken& token, bool condition,
+                    std::source_location loc = std::source_location::current());
 
 private:
     Room* targetRoom;
@@ -142,11 +142,24 @@ private slots:
 // Returning true (rather than a void) allows to reuse the convention with
 // connectUntil() to break the QMetaObject::Connection upon finishing the test
 // item.
-#define FINISH_TEST(Condition) \
-    return (finishTest(thisTest, (Condition), __FILE__, __LINE__), true)
+#define FINISH_TEST(Condition) return (finishTest(thisTest, (Condition)), true)
+
+#define FINISH_TEST_IF(Condition) \
+    do {                          \
+        if (Condition)            \
+            FINISH_TEST(true);    \
+    } while (false)
 
 #define FAIL_TEST() FINISH_TEST(false)
 
+#define FAIL_TEST_IF(Condition, ...)                           \
+    do {                                                       \
+        if (Condition) {                                       \
+            __VA_OPT__(clog << QUO_CSTR(__VA_ARGS__) << endl;) \
+            FAIL_TEST();                                       \
+        }                                                      \
+    } while (false)
+
 void TestSuite::doTest(const QByteArray& testName)
 {
     clog << "Starting: " << testName.constData() << endl;
@@ -164,8 +177,7 @@ bool TestSuite::validatePendingEvent(const QString& txnId)
            && (*it)->matrixType() == EventT::TypeId;
 }
 
-void TestSuite::finishTest(const TestToken& token, bool condition,
-                           const char* file, int line)
+void TestSuite::finishTest(const TestToken& token, bool condition, std::source_location loc)
 {
     const auto& item = testName(token);
     if (condition) {
@@ -174,10 +186,11 @@ void TestSuite::finishTest(const TestToken& token, bool condition,
             targetRoom->postMessage(origin % ": "_L1 % QString::fromUtf8(item) % " successful"_L1,
                                     MessageEventType::Notice);
     } else {
-        clog << item << " FAILED at " << file << ":" << line << endl;
+        clog << item << " FAILED at " << loc.file_name() << ":" << loc.line() << endl;
         if (targetRoom)
             targetRoom->postPlainText(origin % ": "_L1 % QString::fromUtf8(item) % " FAILED at "_L1
-                                      % QString::fromUtf8(file) % ", line "_L1 % QString::number(line));
+                                      % QString::fromUtf8(loc.file_name()) % ", line "_L1
+                                      % QString::number(loc.line()));
     }
 
     emit finishedItem(item, condition);
@@ -385,44 +398,43 @@ TEST_IMPL(sendMessage)
 
 TEST_IMPL(sendReaction)
 {
-    clog << "Reacting to the newest message in the room" << endl;
-    Q_ASSERT(targetRoom->timelineSize() > 0);
-    const auto targetEvtId = targetRoom->messageEvents().back()->id();
-
-    // TODO: a separate test unit for reactionevent.h
-    if (loadEvent<ReactionEvent>(RoomEvent::basicJson(
-            ReactionEvent::TypeId,
-            { { RelatesToKey, toJson(EventRelation::replace(targetEvtId)) } }))) {
-        clog << "ReactionEvent can be created with an invalid relation type"
-             << endl;
-        FAIL_TEST();
-    }
-
-    const auto key = u"+1"_s;
-    const auto txnId = targetRoom->postReaction(targetEvtId, key);
-    if (!validatePendingEvent<ReactionEvent>(txnId)) {
-        clog << "Invalid pending event right after submitting" << endl;
-        FAIL_TEST();
-    }
+    return targetRoom->post<RoomMessageEvent>(u"Reaction target"_s)
+        .whenMerged()
+        .then([this, thisTest](const RoomEvent& targetEvt) {
+            const auto targetEvtId = targetEvt.id();
+            clog << "Reacting to the message just sent to the room: " << targetEvtId.toStdString()
+                 << endl;
 
-    connectUntil(targetRoom, &Room::updatedEvent, this,
-        [this, thisTest, txnId, key, targetEvtId](const QString& actualTargetEvtId) {
-            if (actualTargetEvtId != targetEvtId)
-                return false;
-            const auto reactions = targetRoom->relatedEvents(
-                targetEvtId, EventRelation::AnnotationType);
-            // It's a test room, assuming no interference there should
-            // be exactly one reaction
-            if (reactions.size() != 1)
+            // TODO: a separate test unit for reactionevent.h
+            if (loadEvent<ReactionEvent>(RoomEvent::basicJson(
+                    ReactionEvent::TypeId,
+                    { { RelatesToKey, toJson(EventRelation::replace(targetEvtId)) } }))) {
+                clog << "ReactionEvent can be created with an invalid relation type" << endl;
                 FAIL_TEST();
+            }
 
-            const auto* evt =
-                eventCast<const ReactionEvent>(reactions.back());
-            FINISH_TEST(is<ReactionEvent>(*evt) && !evt->id().isEmpty()
-                        && evt->key() == key && evt->transactionId() == txnId);
-            // TODO: Test removing the reaction
-        });
-    return false;
+            const auto key = u"+"_s;
+            const auto txnId = targetRoom->postReaction(targetEvtId, key);
+            FAIL_TEST_IF(!validatePendingEvent<ReactionEvent>(txnId),
+                         "Invalid pending event right after submitting");
+
+            connectUntil(targetRoom, &Room::updatedEvent, this,
+                         [this, thisTest, txnId, key, targetEvtId](const QString& actualTargetEvtId) {
+                             if (actualTargetEvtId != targetEvtId)
+                                 return false;
+                             const auto reactions =
+                                 targetRoom->relatedEvents(targetEvtId,
+                                                           EventRelation::AnnotationType);
+                             FAIL_TEST_IF(reactions.size() != 1);
+
+                             const auto* evt = eventCast<const ReactionEvent>(reactions.back());
+                             FINISH_TEST(is<ReactionEvent>(*evt) && !evt->id().isEmpty()
+                                         && evt->key() == key && evt->transactionId() == txnId);
+                             // TODO: Test removing the reaction
+                         });
+            return false;
+        })
+        .isRunning();
 }
 
 TEST_IMPL(sendFile)