Skip to content

Commit

Permalink
Merge #845(kitsune): More tweaks and cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@KitsuneRal
KitsuneRal authored Dec 20, 2024
2 parents 8d9f044 + 58e13d2 commit 757f3ce
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 66 deletions.
12 changes: 6 additions & 6 deletions Quotient/connection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions Quotient/ranges_extras.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

}
37 changes: 20 additions & 17 deletions Quotient/room.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
#include <functional>

using namespace Quotient;
using namespace std::placeholders;
using std::llround;

enum EventsPlacement : int { Older = -1, Newer = 1 };
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down
98 changes: 55 additions & 43 deletions quotest/quotest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 757f3ce

Please sign in to comment.