From 4a661262b8e9b5fa869b6c0ba30e87ca2d232631 Mon Sep 17 00:00:00 2001 From: James Graham Date: Sat, 25 Jan 2025 16:22:25 +0000 Subject: [PATCH 1/2] Always return pending event items when posting events rather than transaction IDs --- Quotient/room.cpp | 21 +++++++++++---------- Quotient/room.h | 12 ++++++------ quotest/quotest.cpp | 13 +++++++------ 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/Quotient/room.cpp b/Quotient/room.cpp index 89c1bd49c..27444e39d 100644 --- a/Quotient/room.cpp +++ b/Quotient/room.cpp @@ -297,7 +297,7 @@ class Q_DECL_HIDDEN Room::Private { const PendingEventItem& sendEvent(RoomEventPtr&& event); - QString doPostFile(event_ptr_tt fileEvent, const QUrl& localUrl); + const PendingEventItem& doPostFile(event_ptr_tt fileEvent, const QUrl& localUrl); PendingEvents::iterator addAsPending(RoomEventPtr&& event); @@ -1976,7 +1976,7 @@ Room::PendingEvents::iterator Room::Private::addAsPending(RoomEventPtr&& event) event->setSender(connection->userId()); emit q->pendingEventAboutToAdd(std::to_address(event)); auto it = unsyncedEvents.emplace(unsyncedEvents.end(), std::move(event)); - emit q->pendingEventAdded(it->event()); + emit q->pendingEventAdded(*it); return it; } @@ -2162,14 +2162,15 @@ void Room::discardMessage(const QString& txnId) emit pendingEventDiscarded(); } -QString Room::postReaction(const QString& eventId, const QString& key) +const PendingEventItem& Room::postReaction(const QString& eventId, const QString& key) { - return post(eventId, key)->transactionId(); + return post(eventId, key); } -QString Room::Private::doPostFile(event_ptr_tt fileEvent, const QUrl& localUrl) +const PendingEventItem& Room::Private::doPostFile(event_ptr_tt fileEvent, const QUrl& localUrl) { - const auto txnId = addAsPending(std::move(fileEvent))->event()->transactionId(); + const auto& pendingItem = *addAsPending(std::move(fileEvent)); + const auto txnId = pendingItem->transactionId(); // Remote URL will only be known after upload; fill in the local path // to enable the preview while the event is pending. q->uploadFile(txnId, localUrl); @@ -2212,10 +2213,10 @@ QString Room::Private::doPostFile(event_ptr_tt fileEvent, cons emit q->pendingEventDiscarded(); }); - return txnId; + return pendingItem; } -QString Room::postFile(const QString& plainText, +const PendingEventItem& Room::postFile(const QString& plainText, std::unique_ptr fileContent, std::optional relatesTo) { @@ -2237,9 +2238,9 @@ const PendingEventItem& Room::post(RoomEventPtr event) return d->sendEvent(std::move(event)); } -QString Room::postJson(const QString& matrixType, const QJsonObject& eventContent) +const PendingEventItem& Room::postJson(const QString& matrixType, const QJsonObject& eventContent) { - return d->sendEvent(loadEvent(matrixType, eventContent))->transactionId(); + return d->sendEvent(loadEvent(matrixType, eventContent)); } SetRoomStateWithKeyJob* Room::setState(const StateEvent& evt) diff --git a/Quotient/room.h b/Quotient/room.h index 6b7c28329..1cb2f34ca 100644 --- a/Quotient/room.h +++ b/Quotient/room.h @@ -725,7 +725,7 @@ class QUOTIENT_API Room : public QObject { //! //! This means MessageEventType Text, Emote or Notice. template - QString postText(const QString& plainText, + const PendingEventItem& postText(const QString& plainText, const std::optional& html = std::nullopt, const std::optional& relatesTo = std::nullopt) { @@ -739,19 +739,19 @@ class QUOTIENT_API Room : public QObject { if (html) { content = std::make_unique(*html, u"text/html"_s); } - return post(plainText, type, std::move(content), relatesTo)->transactionId(); + return post(plainText, type, std::move(content), relatesTo); } //! Send a file with the given content - QString postFile(const QString& plainText, + const PendingEventItem& postFile(const QString& plainText, std::unique_ptr fileContent, std::optional relatesTo = std::nullopt); //! Send the given Json as a message - QString postJson(const QString& matrixType, const QJsonObject& eventContent); + const PendingEventItem& postJson(const QString& matrixType, const QJsonObject& eventContent); //! Send a reaction on a given event with a given key - QString postReaction(const QString& eventId, const QString& key); + const PendingEventItem& postReaction(const QString& eventId, const QString& key); PendingEventItem::future_type whenMessageMerged(QString txnId) const; @@ -850,7 +850,7 @@ public Q_SLOTS: /// The event is about to be appended to the list of pending events void pendingEventAboutToAdd(Quotient::RoomEvent* event); /// An event has been appended to the list of pending events - void pendingEventAdded(const Quotient::RoomEvent* event); + void pendingEventAdded(const PendingEventItem& pendingItem); /// The remote echo has arrived with the sync and will be merged /// with its local counterpart /** NB: Requires a sync loop to be emitted */ diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 45d6c797b..3975fdc3a 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -379,12 +379,13 @@ TEST_IMPL(loadMembers) TEST_IMPL(sendMessage) { - auto txnId = targetRoom->postText("Hello, "_L1 % origin % " is here"_L1); + const auto& pendingItem = targetRoom->postText("Hello, "_L1 % origin % " is here"_L1); + const auto txnId = pendingItem->transactionId(); if (!validatePendingEvent(txnId)) { clog << "Invalid pending event right after submitting" << endl; FAIL_TEST(); } - targetRoom->whenMessageMerged(txnId).then(this, [this, thisTest, txnId](const RoomEvent& evt) { + pendingItem.whenMerged().then([this, thisTest, txnId](const RoomEvent& evt) { const auto pendingIt = targetRoom->findPendingEvent(txnId); if (pendingIt == targetRoom->pendingEvents().end()) { clog << "Pending event not found at the moment of local echo merging\n"; @@ -414,7 +415,7 @@ TEST_IMPL(sendReaction) } const auto key = u"+"_s; - const auto txnId = targetRoom->postReaction(targetEvtId, key); + const auto txnId = targetRoom->postReaction(targetEvtId, key)->transactionId(); FAIL_TEST_IF(!validatePendingEvent(txnId), "Invalid pending event right after submitting"); @@ -452,7 +453,7 @@ TEST_IMPL(sendFile) const auto tfName = tfi.fileName(); clog << "Sending file " << tfName.toStdString() << endl; const auto txnId = targetRoom->postFile( - "Test file"_L1, std::make_unique(tfi)); + "Test file"_L1, std::make_unique(tfi))->transactionId(); if (!validatePendingEvent(txnId)) { clog << "Invalid pending event right after submitting" << endl; tf->deleteLater(); @@ -905,7 +906,7 @@ TEST_IMPL(visitResources) TEST_IMPL(thread) { - auto rootTxnId = targetRoom->postText("Threadroot"_L1); + auto rootTxnId = targetRoom->postText("Threadroot"_L1)->transactionId(); connect(targetRoom, &Room::pendingEventAboutToMerge, this, [this, thisTest, rootTxnId](Quotient::RoomEvent* rootEvt) { if (rootEvt->transactionId() == rootTxnId) { const auto relation = EventRelation::replyInThread(rootEvt->id(), true, rootEvt->id()); @@ -975,7 +976,7 @@ void TestManager::conclude() htmlReport += "
Did not finish:"_L1 + QString::fromUtf8(dnfList); } - auto txnId = room->postText(plainReport, htmlReport); + auto txnId = room->postText(plainReport, htmlReport)->transactionId(); // Now just wait until all the pending events reach the server connectUntil(room, &Room::messageSent, this, [this, txnId, room, plainReport] { const auto& pendingEvents = room->pendingEvents(); From 722bc3d138592385d666c477441af05e4a51004b Mon Sep 17 00:00:00 2001 From: James Graham Date: Sat, 25 Jan 2025 18:53:07 +0000 Subject: [PATCH 2/2] Add a future for about to merge --- Quotient/eventitem.h | 35 ++++++++++++++++++++++++++--------- Quotient/room.cpp | 3 ++- Quotient/room.h | 2 +- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/Quotient/eventitem.h b/Quotient/eventitem.h index 800e4c8ad..28058b1ed 100644 --- a/Quotient/eventitem.h +++ b/Quotient/eventitem.h @@ -98,11 +98,13 @@ class QUOTIENT_API TimelineItem : public EventItemBase { class QUOTIENT_API PendingEventItem : public EventItemBase { public: - using future_type = QFuture>; + using about_to_merge_future_type = QFuture, int>>; + using merged_future_type = QFuture>; explicit PendingEventItem(RoomEventPtr&& e) : EventItemBase(std::move(e)) { - _promise.setProgressRange(0, 5); + _aboutToMergePromise.setProgressRange(0, 5); + _mergePromise.setProgressRange(0, 5); } EventStatus::Code deliveryStatus() const { return _status; } @@ -116,10 +118,15 @@ class QUOTIENT_API PendingEventItem : public EventItemBase { setStatus(EventStatus::ReachedServer); (*this)->addId(eventId); } + void setAboutToMerge(const RoomEvent& intoEvent, int pendingEventIndex) + { + _aboutToMergePromise.addResult(std::make_pair(std::ref(intoEvent), pendingEventIndex)); + _aboutToMergePromise.finish(); + } void setMerged(const RoomEvent& intoEvent) { - _promise.addResult(intoEvent); - _promise.finish(); + _mergePromise.addResult(intoEvent); + _mergePromise.finish(); } void setSendingFailed(QString errorText) { @@ -128,12 +135,19 @@ class QUOTIENT_API PendingEventItem : public EventItemBase { } void resetStatus() { setStatus(EventStatus::Submitted); } - //! \brief Get a future for the moment when the item gets merged in the timeline + //! \brief Get a future for the moment just before the item gets merged in the timeline //! //! The future will get finished just before this pending item is merged into its remote //! counterpart that comes with /sync. The pending item will always be in ReachedServer state. + //! The future result has type implicitly convertible to std::pair with a `const RoomEvent&` and an `int`. + about_to_merge_future_type whenAboutToMerged() const { return _aboutToMergePromise.future(); } + + //! \brief Get a future for the moment when the item gets merged in the timeline + //! + //! The future will get finished just after this pending item is merged into its remote + //! counterpart that comes with /sync. The pending item will always be in ReachedServer state. //! The future result has type implicitly convertible to `const RoomEvent&`. - future_type whenMerged() const { return _promise.future(); } + merged_future_type whenMerged() const { return _mergePromise.future(); } private: // Unlike TimelineItems, it's reasonable to assume PendingEventItems are not many; so we can @@ -141,15 +155,18 @@ class QUOTIENT_API PendingEventItem : public EventItemBase { EventStatus::Code _status = EventStatus::Submitted; QDateTime _lastUpdated = QDateTime::currentDateTimeUtc(); QString _annotation; - QPromise> _promise; + QPromise, int>> _aboutToMergePromise; + QPromise> _mergePromise; void setStatus(EventStatus::Code status) { _status = status; _lastUpdated = QDateTime::currentDateTimeUtc(); _annotation.clear(); - _promise.start(); - _promise.setProgressValue(_status); + _aboutToMergePromise.start(); + _mergePromise.start(); + _aboutToMergePromise.setProgressValue(_status); + _mergePromise.setProgressValue(_status); } }; diff --git a/Quotient/room.cpp b/Quotient/room.cpp index 27444e39d..d15338b3e 100644 --- a/Quotient/room.cpp +++ b/Quotient/room.cpp @@ -2093,7 +2093,7 @@ void Room::Private::onEventSendingFailure(PendingEvents::iterator eventItemIter, emit q->pendingEventChanged(int(eventItemIter - unsyncedEvents.begin())); } -PendingEventItem::future_type Room::whenMessageMerged(QString txnId) const +PendingEventItem::merged_future_type Room::whenMessageMerged(QString txnId) const { if (auto it = findPendingEvent(txnId); it != d->unsyncedEvents.cend()) return it->whenMerged(); @@ -2803,6 +2803,7 @@ Room::Timeline::size_type Room::Private::mergePendingEvent(PendingEvents::iterat auto* remoteEcho = remoteEchoIt->get(); const auto pendingEvtIdx = int(localEchoIt - unsyncedEvents.begin()); onEventReachedServer(localEchoIt, remoteEcho->id()); + localEchoIt->setAboutToMerge(*remoteEcho, pendingEvtIdx); emit q->pendingEventAboutToMerge(remoteEcho, pendingEvtIdx); qCDebug(MESSAGES) << "Merging pending event from transaction" << remoteEcho->transactionId() << "into" << remoteEcho->id(); diff --git a/Quotient/room.h b/Quotient/room.h index 1cb2f34ca..cf624ae47 100644 --- a/Quotient/room.h +++ b/Quotient/room.h @@ -753,7 +753,7 @@ class QUOTIENT_API Room : public QObject { //! Send a reaction on a given event with a given key const PendingEventItem& postReaction(const QString& eventId, const QString& key); - PendingEventItem::future_type whenMessageMerged(QString txnId) const; + PendingEventItem::merged_future_type whenMessageMerged(QString txnId) const; //! Send a request to update the room state with the given event SetRoomStateWithKeyJob* setState(const StateEvent& evt);