From eed5a6f127ec3bb1553ac629457f196d8893665a Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Mon, 26 Jul 2021 08:19:29 +0200
Subject: Room::usersAtEventId(): switch from QMultiHash to QHash of QSets
While slightly more complex for updating, this allows COW to kick in in
the read accessor; using QSet instead of QList also provides better
consistency guarantees. For QML both are converted to an Array-like
collection since Qt 5.15; Qt 5.12 turns QSet<> in a QVariantList,
according to the documentation, which is quite reasonable too.
---
lib/room.cpp | 13 ++++++++-----
lib/room.h | 2 +-
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index 10e827d7..13589ee9 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -121,7 +121,7 @@ public:
int notificationCount = 0;
members_map_t membersMap;
QList<User*> usersTyping;
- QMultiHash<QString, User*> eventIdReadUsers;
+ QHash<QString, QSet<User*>> eventIdReadUsers;
QList<User*> usersInvited;
QList<User*> membersLeft;
int unreadMessages = 0;
@@ -627,8 +627,11 @@ Room::Changes Room::Private::setLastReadEvent(User* u, QString eventId)
auto& storedId = lastReadEventIds[u];
if (storedId == eventId)
return Change::NoChange;
- eventIdReadUsers.remove(storedId, u);
- eventIdReadUsers.insert(eventId, u);
+ auto& oldEventReadUsers = eventIdReadUsers[storedId];
+ oldEventReadUsers.remove(u);
+ if (oldEventReadUsers.isEmpty())
+ eventIdReadUsers.remove(storedId);
+ eventIdReadUsers[eventId].insert(u);
swap(storedId, eventId);
emit q->lastReadEventChanged(u);
emit q->readMarkerForUserMoved(u, eventId, storedId);
@@ -965,9 +968,9 @@ QString Room::readMarkerEventId() const
return d->lastReadEventIds.value(localUser());
}
-QList<User*> Room::usersAtEventId(const QString& eventId)
+QSet<User*> Room::usersAtEventId(const QString& eventId)
{
- return d->eventIdReadUsers.values(eventId);
+ return d->eventIdReadUsers.value(eventId);
}
int Room::notificationCount() const { return d->notificationCount; }
diff --git a/lib/room.h b/lib/room.h
index cdbfe58f..fa7b6e6d 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -364,7 +364,7 @@ public:
rev_iter_t readMarker(const User* user) const;
rev_iter_t readMarker() const;
QString readMarkerEventId() const;
- QList<User*> usersAtEventId(const QString& eventId);
+ QSet<User*> usersAtEventId(const QString& eventId);
/**
* \brief Mark the event with uptoEventId as read
*
--
cgit v1.2.3
From ed1b034089dfc1948acaa80a5200cb0df28d0c1c Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Mon, 26 Jul 2021 22:36:44 +0200
Subject: SyncData: minor update to the cache version
The minor component is now updated in .cpp, not in .h.
---
lib/syncdata.cpp | 7 ++++++-
lib/syncdata.h | 3 ++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/syncdata.cpp b/lib/syncdata.cpp
index d3c270b5..3e0eff17 100644
--- a/lib/syncdata.cpp
+++ b/lib/syncdata.cpp
@@ -103,7 +103,7 @@ SyncData::SyncData(const QString& cacheFileName)
{
QFileInfo cacheFileInfo { cacheFileName };
auto json = loadJson(cacheFileName);
- auto requiredVersion = std::get<0>(cacheVersion());
+ auto requiredVersion = MajorCacheVersion;
auto actualVersion =
json.value("cache_version"_ls).toObject().value("major"_ls).toInt();
if (actualVersion == requiredVersion)
@@ -128,6 +128,11 @@ Events&& SyncData::takeAccountData() { return std::move(accountData); }
Events&& SyncData::takeToDeviceEvents() { return std::move(toDeviceEvents); }
+std::pair<int, int> SyncData::cacheVersion()
+{
+ return { MajorCacheVersion, 1 };
+}
+
QJsonObject SyncData::loadJson(const QString& fileName)
{
QFile roomFile { fileName };
diff --git a/lib/syncdata.h b/lib/syncdata.h
index b0e31726..b869a541 100644
--- a/lib/syncdata.h
+++ b/lib/syncdata.h
@@ -87,7 +87,8 @@ public:
QStringList unresolvedRooms() const { return unresolvedRoomIds; }
- static std::pair<int, int> cacheVersion() { return { 11, 0 }; }
+ static constexpr int MajorCacheVersion = 11;
+ static std::pair<int, int> cacheVersion();
static QString fileNameForRoom(QString roomId);
private:
--
cgit v1.2.3
From 277f43defe3fa55ff32fe53952c6331a81d65a20 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Wed, 28 Jul 2021 03:32:02 +0200
Subject: Room: fix the read markers/receipts confusion
This turns the design changes laid out in #464 comments to code, as of
0.6.x branch (0.7 API will be introduced separately):
- readMarker() now returns the fully read marker, unlike
readMarker(User*) that returns a read receipt, even when called for
the local user.
- Private::setLastReadEvent() -> setLastReadReceipt(), incorporating
the "promotion" logic from promoteReadReceipt().
- The above makes promoteReadReceipt() unneeded; the remaining piece
of logic that recalculates the number of unread messages is put to
its own method - Private::recalculateUnreadCount().
- Private::updateUnreadCount() is only slightly refreshed, continues
to use the fully read marker position (as it used to).
- Now that read receipts and fully read markers are managed separately,
Private::setLastReadReceipt() has got its counterpart,
Private::setFullyReadMarker(); both only update their respective
markers locally (emitting signals as needed), without interaction
with the homeserver.
- Private::markMessagesAsRead() now delegates updating the fully read
marker to setFullyReadMarker() and on top of that sends the new
fully read marker to the homeserver.
- Private::serverReadMarker -> fullyReadUntilEventId (to be really clear
what it stores).
- The hand-written PostReadMarkersJob is replaced with the generated
SetReadMarkerJob that does the same thing (and can update the read
receipt on top).
---
lib/room.cpp | 341 +++++++++++++++++++++++++++++++----------------------------
1 file changed, 180 insertions(+), 161 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index 13589ee9..ec3a9532 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -129,7 +129,7 @@ public:
QString firstDisplayedEventId;
QString lastDisplayedEventId;
QHash<const User*, QString> lastReadEventIds;
- QString serverReadMarker;
+ QString fullyReadUntilEventId;
TagsMap tags;
UnorderedMap<QString, EventPtr> accountData;
QString prevBatch;
@@ -290,11 +290,12 @@ public:
*/
void dropDuplicateEvents(RoomEvents& events) const;
- Changes setLastReadEvent(User* u, QString eventId);
- void updateUnreadCount(const rev_iter_t& from, const rev_iter_t& to);
- Changes promoteReadMarker(User* u, const rev_iter_t& newMarker, bool force = false);
-
- Changes markMessagesAsRead(rev_iter_t upToMarker);
+ void setLastReadReceipt(User* u, rev_iter_t newMarker,
+ QString newEvtId = {});
+ Changes setFullyReadMarker(const QString &eventId);
+ Changes updateUnreadCount(const rev_iter_t& from, const rev_iter_t& to);
+ Changes recalculateUnreadCount(bool force = false);
+ void markMessagesAsRead(const rev_iter_t &upToMarker);
void getAllMembers();
@@ -622,142 +623,172 @@ void Room::setJoinState(JoinState state)
emit joinStateChanged(oldState, state);
}
-Room::Changes Room::Private::setLastReadEvent(User* u, QString eventId)
+void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
+ QString newEvtId)
{
+ if (!u) {
+ Q_ASSERT(u != nullptr); // For Debug builds
+ qCCritical(MAIN) << "Empty user, skipping read receipt registration";
+ return; // For Release builds
+ }
+ if (q->memberJoinState(u) != JoinState::Join) {
+ qCWarning(MAIN) << "Won't record read receipt for non-member" << u->id();
+ return;
+ }
+
+ if (newMarker == timeline.crend() && !newEvtId.isEmpty())
+ newMarker = q->findInTimeline(newEvtId);
+ if (newMarker != timeline.crend()) {
+ // NB: with reverse iterators, timeline history >= sync edge
+ if (newMarker >= q->readMarker(u))
+ return;
+
+ // Try to auto-promote the read marker over the user's own messages
+ // (switch to direct iterators for that).
+ const auto eagerMarker = find_if(newMarker.base(), timeline.cend(),
+ [=](const TimelineItem& ti) {
+ return ti->senderId() != u->id();
+ })
+ - 1;
+ newEvtId = (*eagerMarker)->id();
+ if (eagerMarker != newMarker.base() - 1) // &*(rIt.base() - 1) === &*rIt
+ qCDebug(EPHEMERAL) << "Auto-promoted read receipt for" << u->id()
+ << "to" << newEvtId;
+ }
+
auto& storedId = lastReadEventIds[u];
- if (storedId == eventId)
- return Change::NoChange;
+ if (storedId == newEvtId)
+ return;
+ // Finally make the change
auto& oldEventReadUsers = eventIdReadUsers[storedId];
oldEventReadUsers.remove(u);
if (oldEventReadUsers.isEmpty())
eventIdReadUsers.remove(storedId);
- eventIdReadUsers[eventId].insert(u);
- swap(storedId, eventId);
+ eventIdReadUsers[newEvtId].insert(u);
+ swap(storedId, newEvtId); // Now newEvtId actually stores the old eventId
emit q->lastReadEventChanged(u);
- emit q->readMarkerForUserMoved(u, eventId, storedId);
- if (isLocalUser(u)) {
- if (storedId != serverReadMarker)
- connection->callApi<SetReadMarkerJob>(BackgroundRequest, id,
- storedId);
- emit q->readMarkerMoved(eventId, storedId);
- return Change::ReadMarkerChange;
- }
- return Change::NoChange;
+ if (!isLocalUser(u))
+ emit q->readMarkerForUserMoved(u, newEvtId, storedId);
}
-void Room::Private::updateUnreadCount(const rev_iter_t& from,
- const rev_iter_t& to)
+Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
+ const rev_iter_t& to)
{
Q_ASSERT(from >= timeline.crbegin() && from <= timeline.crend());
Q_ASSERT(to >= from && to <= timeline.crend());
- // Catch a special case when the last read event id refers to an event
- // that has just arrived. In this case we should recalculate
- // unreadMessages and might need to promote the read marker further
- // over local-origin messages.
- auto readMarker = q->readMarker();
- if (readMarker == timeline.crend() && q->allHistoryLoaded())
- --readMarker; // Read marker not found in the timeline, initialise it
- if (readMarker >= from && readMarker < to) {
- promoteReadMarker(q->localUser(), readMarker, true);
- return;
- }
-
- Q_ASSERT(to <= readMarker);
+ auto fullyReadMarker = q->readMarker();
+ if (fullyReadMarker < from)
+ return NoChange; // What's arrived is already fully read
+
+ if (fullyReadMarker == timeline.crend() && q->allHistoryLoaded())
+ --fullyReadMarker; // No read marker in the whole room, initialise it
+ if (fullyReadMarker < to) {
+ // Catch a special case when the last fully read event id refers to an
+ // event that has just arrived. In this case we should recalculate
+ // unreadMessages to get an exact number instead of an estimation
+ // (see https://github.com/quotient-im/libQuotient/wiki/unread_count).
+ // For the same reason (switching from the estimation to the exact
+ // number) this branch always emits unreadMessagesChanged() and returns
+ // UnreadNotifsChange, even if the estimation luckily matched the exact
+ // result.
+ return recalculateUnreadCount(true);
+ }
+
+ // Fully read marker is somewhere beyond the most historical message from
+ // the arrived batch - add up newly arrived messages to the current counter,
+ // instead of a complete recalculation.
+ Q_ASSERT(to <= fullyReadMarker);
QElapsedTimer et;
et.start();
const auto newUnreadMessages =
- count_if(from, to, std::bind(&Room::Private::isEventNotable, this, _1));
+ count_if(from, to,
+ std::bind(&Room::Private::isEventNotable, this, _1));
if (et.nsecsElapsed() > profilerMinNsecs() / 10)
qCDebug(PROFILER) << "Counting gained unread messages took" << et;
- if (newUnreadMessages > 0) {
- // See https://github.com/quotient-im/libQuotient/wiki/unread_count
- if (unreadMessages < 0)
- unreadMessages = 0;
-
- unreadMessages += newUnreadMessages;
- qCDebug(MESSAGES) << "Room" << q->objectName() << "has gained"
- << newUnreadMessages << "unread message(s),"
- << (q->readMarker() == timeline.crend()
- ? "in total at least"
- : "in total")
- << unreadMessages << "unread message(s)";
- emit q->unreadMessagesChanged(q);
- }
-}
+ if (newUnreadMessages == 0)
+ return NoChange;
-Room::Changes Room::Private::promoteReadMarker(User* u,
- const rev_iter_t& newMarker,
- bool force)
-{
- Q_ASSERT_X(u, __FUNCTION__, "User* should not be nullptr");
- Q_ASSERT(newMarker >= timeline.crbegin() && newMarker <= timeline.crend());
+ // See https://github.com/quotient-im/libQuotient/wiki/unread_count
+ if (unreadMessages < 0)
+ unreadMessages = 0;
+
+ unreadMessages += newUnreadMessages;
+ qCDebug(MESSAGES) << "Room" << q->objectName() << "has gained"
+ << newUnreadMessages << "unread message(s),"
+ << (q->readMarker() == timeline.crend()
+ ? "in total at least"
+ : "in total")
+ << unreadMessages << "unread message(s)";
+ emit q->unreadMessagesChanged(q);
+ return UnreadNotifsChange;
+}
+
+Room::Changes Room::Private::recalculateUnreadCount(bool force)
+{
+ // The recalculation logic assumes that the fully read marker points at
+ // a specific position in the timeline
+ Q_ASSERT(q->readMarker() != timeline.crend());
+ const auto oldUnreadCount = unreadMessages;
+ QElapsedTimer et;
+ et.start();
+ unreadMessages =
+ int(count_if(timeline.crbegin(), q->readMarker(),
+ [this](const auto& ti) { return isEventNotable(ti); }));
+ if (et.nsecsElapsed() > profilerMinNsecs() / 10)
+ qCDebug(PROFILER) << "Recounting unread messages took" << et;
- const auto prevMarker = q->readMarker(u);
- if (!force && prevMarker <= newMarker) // Remember, we deal with reverse
- // iterators
- return Change::NoChange;
+ // See https://github.com/quotient-im/libQuotient/wiki/unread_count
+ if (unreadMessages == 0)
+ unreadMessages = -1;
- Q_ASSERT(newMarker < timeline.crend());
+ if (!force && unreadMessages == oldUnreadCount)
+ return NoChange;
- // Try to auto-promote the read marker over the user's own messages
- // (switch to direct iterators for that).
- auto eagerMarker =
- find_if(newMarker.base(), timeline.cend(), [=](const TimelineItem& ti) {
- return ti->senderId() != u->id();
- });
+ if (unreadMessages == -1)
+ qCDebug(MESSAGES)
+ << "Room" << displayname << "has no more unread messages";
+ else
+ qCDebug(MESSAGES) << "Room" << displayname << "still has"
+ << unreadMessages << "unread message(s)";
+ emit q->unreadMessagesChanged(q);
+ return UnreadNotifsChange;
+}
- auto changes = setLastReadEvent(u, (*(eagerMarker - 1))->id());
- if (isLocalUser(u)) {
- const auto oldUnreadCount = unreadMessages;
- QElapsedTimer et;
- et.start();
- unreadMessages =
- int(count_if(eagerMarker, timeline.cend(),
- [this](const auto& ti) { return isEventNotable(ti); }));
- if (et.nsecsElapsed() > profilerMinNsecs() / 10)
- qCDebug(PROFILER) << "Recounting unread messages took" << et;
+Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
+{
+ if (fullyReadUntilEventId == eventId)
+ return NoChange;
- // See https://github.com/quotient-im/libQuotient/wiki/unread_count
- if (unreadMessages == 0)
- unreadMessages = -1;
+ const auto prevFullyReadId = std::exchange(fullyReadUntilEventId, eventId);
+ qCDebug(MESSAGES) << "Fully read marker in" << q->objectName() //
+ << "moved to" << fullyReadUntilEventId;
+ emit q->readMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
- if (force || unreadMessages != oldUnreadCount) {
- if (unreadMessages == -1) {
- qCDebug(MESSAGES)
- << "Room" << displayname << "has no more unread messages";
- } else
- qCDebug(MESSAGES) << "Room" << displayname << "still has"
- << unreadMessages << "unread message(s)";
- emit q->unreadMessagesChanged(q);
- changes |= Change::UnreadNotifsChange;
- }
+ Changes changes = ReadMarkerChange;
+ if (const auto rm = q->readMarker(); rm != timeline.crend()) {
+ // Pull read receipt if it's behind
+ if (auto rr = q->readMarker(q->localUser()); rr > rm)
+ setLastReadReceipt(q->localUser(), rm);
+
+ changes |= recalculateUnreadCount();
}
return changes;
}
-Room::Changes Room::Private::markMessagesAsRead(rev_iter_t upToMarker)
+void Room::Private::markMessagesAsRead(const rev_iter_t &upToMarker)
{
- const auto prevMarker = q->readMarker();
- auto changes = promoteReadMarker(q->localUser(), upToMarker);
- if (prevMarker != upToMarker)
- qCDebug(MESSAGES) << "Marked messages as read until" << *q->readMarker();
-
- // We shouldn't send read receipts for the local user's own messages - so
- // search earlier messages for the latest message not from the local user
- // until the previous last-read message, whichever comes first.
- for (; upToMarker < prevMarker; ++upToMarker) {
- if ((*upToMarker)->senderId() != q->localUser()->id()) {
- connection->callApi<PostReceiptJob>(BackgroundRequest,
- id, QStringLiteral("m.read"),
- QUrl::toPercentEncoding(
- (*upToMarker)->id()));
- break;
- }
+ if (upToMarker < q->readMarker()) {
+ setFullyReadMarker((*upToMarker)->id());
+ // Assuming that if a read receipt was sent on a newer event, it will
+ // stay there instead of "un-reading" notifications/mentions from
+ // m.fully_read to m.read
+ connection->callApi<SetReadMarkerJob>(BackgroundRequest, id,
+ fullyReadUntilEventId,
+ fullyReadUntilEventId);
}
- return changes;
}
void Room::markMessagesAsRead(QString uptoEventId)
@@ -961,11 +992,14 @@ Room::rev_iter_t Room::readMarker(const User* user) const
return findInTimeline(d->lastReadEventIds.value(user));
}
-Room::rev_iter_t Room::readMarker() const { return readMarker(localUser()); }
+Room::rev_iter_t Room::readMarker() const
+{
+ return findInTimeline(d->fullyReadUntilEventId);
+}
QString Room::readMarkerEventId() const
{
- return d->lastReadEventIds.value(localUser());
+ return d->fullyReadUntilEventId;
}
QSet<User*> Room::usersAtEventId(const QString& eventId)
@@ -2436,24 +2470,21 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events)
<< timeline.back();
// The first event in the just-added batch (referred to by `from`)
- // defines whose read marker can possibly be promoted any further over
+ // defines whose read receipt can possibly be promoted any further over
// the same author's events newly arrived. Others will need explicit
- // read receipts from the server (or, for the local user,
- // markMessagesAsRead() invocation) to promote their read markers over
+ // read receipts from the server - or, for the local user, calling
+ // setLastDisplayedEventId() - to promote their read receipts over
// the new message events.
- if (const auto senderId = (*from)->senderId(); !senderId.isEmpty()) {
- auto* const firstWriter = q->user(senderId);
- if (q->readMarker(firstWriter) != timeline.crend()) {
- roomChanges |=
- promoteReadMarker(firstWriter, rev_iter_t(from) - 1);
- qCDebug(MESSAGES)
- << "Auto-promoted read marker for" << senderId
- << "to" << *q->readMarker(firstWriter);
- }
+ auto* const firstWriter = q->user((*from)->senderId());
+ setLastReadReceipt(firstWriter, rev_iter_t(from + 1));
+ if (firstWriter == q->localUser() && q->readMarker().base() == from) {
+ // If the local user's message(s) is/are first in the batch
+ // and the fully read marker was right before it, promote
+ // the fully read marker to the same event as the read receipt.
+ roomChanges |=
+ setFullyReadMarker(lastReadEventIds.value(firstWriter));
}
-
- updateUnreadCount(timeline.crbegin(), rev_iter_t(from));
- roomChanges |= Change::UnreadNotifsChange;
+ roomChanges |= updateUnreadCount(timeline.crbegin(), rev_iter_t(from));
}
Q_ASSERT(timeline.size() == timelineSize + totalInserted);
@@ -2498,8 +2529,11 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events)
emit q->updatedEvent(relation.eventId);
}
}
- if (from <= q->readMarker())
- updateUnreadCount(from, timeline.crend());
+ updateUnreadCount(from, timeline.crend());
+ // When there are no unread messages and the read marker is within the
+ // known timeline, unreadMessages == -1
+ // (see https://github.com/quotient-im/libQuotient/wiki/unread_count).
+ Q_ASSERT(unreadMessages != 0 || q->readMarker() == timeline.crend());
Q_ASSERT(timeline.size() == timelineSize + insertedSize);
if (insertedSize > 9 || et.nsecsElapsed() >= profilerMinNsecs())
@@ -2723,7 +2757,7 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
if (auto* evt = eventCast<TypingEvent>(event)) {
d->usersTyping.clear();
for (const auto& userId : evt->users()) {
- auto u = user(userId);
+ auto* const u = user(userId);
if (memberJoinState(u) == JoinState::Join)
d->usersTyping.append(u);
}
@@ -2746,30 +2780,21 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
<< p.receipts.size() << "users";
}
const auto newMarker = findInTimeline(p.evtId);
- if (newMarker != timelineEdge()) {
- for (const Receipt& r : p.receipts) {
- if (r.userId == connection()->userId())
- continue; // FIXME, #185
- auto u = user(r.userId);
- if (memberJoinState(u) == JoinState::Join)
- changes |= d->promoteReadMarker(u, newMarker);
- }
- } else {
- qCDebug(EPHEMERAL) << "Event" << p.evtId
- << "not found; saving read receipts anyway";
- // If the event is not found (most likely, because it's too old
- // and hasn't been fetched from the server yet), but there is
- // a previous marker for a user, keep the previous marker.
- // Otherwise, blindly store the event id for this user.
- for (const Receipt& r : p.receipts) {
- if (r.userId == connection()->userId())
- continue; // FIXME, #185
- auto u = user(r.userId);
- if (memberJoinState(u) == JoinState::Join
- && readMarker(u) == timelineEdge())
- changes |= d->setLastReadEvent(u, p.evtId);
+ if (newMarker == historyEdge())
+ qCDebug(EPHEMERAL) << "Event of the read receipt(s) is not "
+ "found; saving them anyway";
+ for (const Receipt& r : p.receipts)
+ if (auto* const u = user(r.userId);
+ memberJoinState(u) == JoinState::Join) {
+ // If the event is not found (most likely, because it's
+ // too old and hasn't been fetched from the server yet)
+ // but there is a previous marker for a user, keep
+ // the previous marker because read receipts are not
+ // supposed to move backwards. Otherwise, blindly
+ // store the event id for this user and update the read
+ // marker when/if the event is fetched later on.
+ d->setLastReadReceipt(u, newMarker, p.evtId);
}
- }
}
if (eventsWithReceipts.size() > 3 || totalReceipts > 10
|| et.nsecsElapsed() >= profilerMinNsecs())
@@ -2789,15 +2814,9 @@ Room::Changes Room::processAccountDataEvent(EventPtr&& event)
changes |= Change::TagsChange;
}
- if (auto* evt = eventCast<ReadMarkerEvent>(event)) {
- auto readEventId = evt->event_id();
- qCDebug(STATE) << "Server-side read marker at" << readEventId;
- d->serverReadMarker = readEventId;
- const auto newMarker = findInTimeline(readEventId);
- changes |= newMarker != timelineEdge()
- ? d->markMessagesAsRead(newMarker)
- : d->setLastReadEvent(localUser(), readEventId);
- }
+ if (auto* evt = eventCast<const ReadMarkerEvent>(event))
+ changes |= d->setFullyReadMarker(evt->event_id());
+
// For all account data events
auto& currentData = d->accountData[event->matrixType()];
// A polymorphic event-specific comparison might be a bit more
--
cgit v1.2.3
From 62c829cc8de8a870c08926c41331f2766e766f37 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Thu, 29 Jul 2021 21:58:56 +0200
Subject: Quotient::ReadReceipt; deprecated readMarker()
It's now possible to get receipts along with their timestamps by calling
Room::lastReadReceipt(). Together this new method, fullyReadMarker(),
and lastFullyReadEventId() deprecate readMarker() overloads and
readMarkerEventId() respectively.
lastFullyReadEventId is also a Q_PROPERTY (deprecating
readMarkerEventId); readMarkerMoved() signal is deprecated by
fullyReadMarkerMoved(), while readMarkerForUserMoved() is deprecated
in favour of existing lastReadEventChanged().
---
lib/room.cpp | 79 ++++++++++++++++++++++++++++++++++--------------------------
lib/room.h | 39 +++++++++++++++++++++++++++++-
2 files changed, 83 insertions(+), 35 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index ec3a9532..9a571127 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -128,7 +128,7 @@ public:
bool displayed = false;
QString firstDisplayedEventId;
QString lastDisplayedEventId;
- QHash<const User*, QString> lastReadEventIds;
+ QHash<const User*, ReadReceipt> lastReadReceipts;
QString fullyReadUntilEventId;
TagsMap tags;
UnorderedMap<QString, EventPtr> accountData;
@@ -291,7 +291,7 @@ public:
void dropDuplicateEvents(RoomEvents& events) const;
void setLastReadReceipt(User* u, rev_iter_t newMarker,
- QString newEvtId = {});
+ ReadReceipt newReceipt = {});
Changes setFullyReadMarker(const QString &eventId);
Changes updateUnreadCount(const rev_iter_t& from, const rev_iter_t& to);
Changes recalculateUnreadCount(bool force = false);
@@ -624,7 +624,7 @@ void Room::setJoinState(JoinState state)
}
void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
- QString newEvtId)
+ ReadReceipt newReceipt)
{
if (!u) {
Q_ASSERT(u != nullptr); // For Debug builds
@@ -636,11 +636,12 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
return;
}
- if (newMarker == timeline.crend() && !newEvtId.isEmpty())
- newMarker = q->findInTimeline(newEvtId);
+ auto& storedReceipt = lastReadReceipts[u];
+ if (newMarker == timeline.crend() && !newReceipt.eventId.isEmpty())
+ newMarker = q->findInTimeline(newReceipt.eventId);
if (newMarker != timeline.crend()) {
// NB: with reverse iterators, timeline history >= sync edge
- if (newMarker >= q->readMarker(u))
+ if (newMarker >= q->findInTimeline(storedReceipt.eventId))
return;
// Try to auto-promote the read marker over the user's own messages
@@ -650,25 +651,26 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
return ti->senderId() != u->id();
})
- 1;
- newEvtId = (*eagerMarker)->id();
+ newReceipt.eventId = (*eagerMarker)->id();
if (eagerMarker != newMarker.base() - 1) // &*(rIt.base() - 1) === &*rIt
qCDebug(EPHEMERAL) << "Auto-promoted read receipt for" << u->id()
- << "to" << newEvtId;
+ << "to" << newReceipt.eventId;
}
- auto& storedId = lastReadEventIds[u];
- if (storedId == newEvtId)
+ if (storedReceipt == newReceipt)
return;
// Finally make the change
- auto& oldEventReadUsers = eventIdReadUsers[storedId];
+ auto& oldEventReadUsers = eventIdReadUsers[storedReceipt.eventId];
oldEventReadUsers.remove(u);
if (oldEventReadUsers.isEmpty())
- eventIdReadUsers.remove(storedId);
- eventIdReadUsers[newEvtId].insert(u);
- swap(storedId, newEvtId); // Now newEvtId actually stores the old eventId
+ eventIdReadUsers.remove(storedReceipt.eventId);
+ eventIdReadUsers[newReceipt.eventId].insert(u);
+ swap(storedReceipt, newReceipt); // Now newReceipt actually stores the old receipt
emit q->lastReadEventChanged(u);
+ // TODO: remove in 0.8
if (!isLocalUser(u))
- emit q->readMarkerForUserMoved(u, newEvtId, storedId);
+ emit q->readMarkerForUserMoved(u, newReceipt.eventId,
+ storedReceipt.eventId);
}
Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
@@ -677,7 +679,7 @@ Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
Q_ASSERT(from >= timeline.crbegin() && from <= timeline.crend());
Q_ASSERT(to >= from && to <= timeline.crend());
- auto fullyReadMarker = q->readMarker();
+ auto fullyReadMarker = q->fullyReadMarker();
if (fullyReadMarker < from)
return NoChange; // What's arrived is already fully read
@@ -718,7 +720,7 @@ Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
unreadMessages += newUnreadMessages;
qCDebug(MESSAGES) << "Room" << q->objectName() << "has gained"
<< newUnreadMessages << "unread message(s),"
- << (q->readMarker() == timeline.crend()
+ << (q->fullyReadMarker() == timeline.crend()
? "in total at least"
: "in total")
<< unreadMessages << "unread message(s)";
@@ -730,12 +732,12 @@ Room::Changes Room::Private::recalculateUnreadCount(bool force)
{
// The recalculation logic assumes that the fully read marker points at
// a specific position in the timeline
- Q_ASSERT(q->readMarker() != timeline.crend());
+ Q_ASSERT(q->fullyReadMarker() != timeline.crend());
const auto oldUnreadCount = unreadMessages;
QElapsedTimer et;
et.start();
unreadMessages =
- int(count_if(timeline.crbegin(), q->readMarker(),
+ int(count_if(timeline.crbegin(), q->fullyReadMarker(),
[this](const auto& ti) { return isEventNotable(ti); }));
if (et.nsecsElapsed() > profilerMinNsecs() / 10)
qCDebug(PROFILER) << "Recounting unread messages took" << et;
@@ -765,22 +767,22 @@ Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
const auto prevFullyReadId = std::exchange(fullyReadUntilEventId, eventId);
qCDebug(MESSAGES) << "Fully read marker in" << q->objectName() //
<< "moved to" << fullyReadUntilEventId;
+ emit q->fullyReadMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
+ // TODO: Remove in 0.8
emit q->readMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
Changes changes = ReadMarkerChange;
- if (const auto rm = q->readMarker(); rm != timeline.crend()) {
+ if (const auto rm = q->fullyReadMarker(); rm != timeline.crend()) {
// Pull read receipt if it's behind
- if (auto rr = q->readMarker(q->localUser()); rr > rm)
- setLastReadReceipt(q->localUser(), rm);
-
- changes |= recalculateUnreadCount();
+ setLastReadReceipt(q->localUser(), rm);
+ changes |= recalculateUnreadCount(); // TODO: updateUnreadCount()?
}
return changes;
}
void Room::Private::markMessagesAsRead(const rev_iter_t &upToMarker)
{
- if (upToMarker < q->readMarker()) {
+ if (upToMarker < q->fullyReadMarker()) {
setFullyReadMarker((*upToMarker)->id());
// Assuming that if a read receipt was sent on a newer event, it will
// stay there instead of "un-reading" notifications/mentions from
@@ -989,17 +991,23 @@ void Room::setLastDisplayedEvent(TimelineItem::index_t index)
Room::rev_iter_t Room::readMarker(const User* user) const
{
Q_ASSERT(user);
- return findInTimeline(d->lastReadEventIds.value(user));
+ return findInTimeline(lastReadReceipt(user->id()).eventId);
}
-Room::rev_iter_t Room::readMarker() const
+Room::rev_iter_t Room::readMarker() const { return fullyReadMarker(); }
+
+QString Room::readMarkerEventId() const { return lastFullyReadEventId(); }
+
+ReadReceipt Room::lastReadReceipt(const QString& userId) const
{
- return findInTimeline(d->fullyReadUntilEventId);
+ return d->lastReadReceipts.value(user(userId));
}
-QString Room::readMarkerEventId() const
+QString Room::lastFullyReadEventId() const { return d->fullyReadUntilEventId; }
+
+Room::rev_iter_t Room::fullyReadMarker() const
{
- return d->fullyReadUntilEventId;
+ return findInTimeline(d->fullyReadUntilEventId);
}
QSet<User*> Room::usersAtEventId(const QString& eventId)
@@ -2477,12 +2485,14 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events)
// the new message events.
auto* const firstWriter = q->user((*from)->senderId());
setLastReadReceipt(firstWriter, rev_iter_t(from + 1));
- if (firstWriter == q->localUser() && q->readMarker().base() == from) {
+ if (firstWriter == q->localUser()
+ && q->fullyReadMarker().base() == from) //
+ {
// If the local user's message(s) is/are first in the batch
// and the fully read marker was right before it, promote
// the fully read marker to the same event as the read receipt.
roomChanges |=
- setFullyReadMarker(lastReadEventIds.value(firstWriter));
+ setFullyReadMarker(lastReadReceipts.value(firstWriter).eventId);
}
roomChanges |= updateUnreadCount(timeline.crbegin(), rev_iter_t(from));
}
@@ -2533,7 +2543,7 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events)
// When there are no unread messages and the read marker is within the
// known timeline, unreadMessages == -1
// (see https://github.com/quotient-im/libQuotient/wiki/unread_count).
- Q_ASSERT(unreadMessages != 0 || q->readMarker() == timeline.crend());
+ Q_ASSERT(unreadMessages != 0 || q->fullyReadMarker() == timeline.crend());
Q_ASSERT(timeline.size() == timelineSize + insertedSize);
if (insertedSize > 9 || et.nsecsElapsed() >= profilerMinNsecs())
@@ -2793,7 +2803,8 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
// supposed to move backwards. Otherwise, blindly
// store the event id for this user and update the read
// marker when/if the event is fetched later on.
- d->setLastReadReceipt(u, newMarker, p.evtId);
+ d->setLastReadReceipt(u, newMarker,
+ { p.evtId, r.timestamp });
}
}
if (eventsWithReceipts.size() > 3 || totalReceipts > 10
diff --git a/lib/room.h b/lib/room.h
index fa7b6e6d..4833bd27 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -71,6 +71,28 @@ public:
bool failed() const { return status == Failed; }
};
+//! \brief Data structure for a room member's read receipt
+//! \sa Room::lastReadReceipt
+class ReadReceipt {
+ Q_GADGET
+ Q_PROPERTY(QString eventId MEMBER eventId CONSTANT)
+ Q_PROPERTY(QDateTime timestamp MEMBER timestamp CONSTANT)
+public:
+ QString eventId;
+ QDateTime timestamp;
+
+ bool operator==(const ReadReceipt& other)
+ {
+ return eventId == other.eventId && timestamp == other.timestamp;
+ }
+ bool operator!=(const ReadReceipt& other) { return !operator==(other); }
+};
+inline void swap(ReadReceipt& lhs, ReadReceipt& rhs)
+{
+ swap(lhs.eventId, rhs.eventId);
+ swap(lhs.timestamp, rhs.timestamp);
+}
+
class Room : public QObject {
Q_OBJECT
Q_PROPERTY(Connection* connection READ connection CONSTANT)
@@ -104,9 +126,11 @@ class Room : public QObject {
setFirstDisplayedEventId NOTIFY firstDisplayedEventChanged)
Q_PROPERTY(QString lastDisplayedEventId READ lastDisplayedEventId WRITE
setLastDisplayedEventId NOTIFY lastDisplayedEventChanged)
-
+ //! \deprecated since 0.7
Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE
markMessagesAsRead NOTIFY readMarkerMoved)
+ Q_PROPERTY(QString lastFullyReadEventId READ lastFullyReadEventId WRITE
+ markMessagesAsRead NOTIFY fullyReadMarkerMoved)
Q_PROPERTY(bool hasUnreadMessages READ hasUnreadMessages NOTIFY
unreadMessagesChanged)
Q_PROPERTY(int unreadCount READ unreadCount NOTIFY unreadMessagesChanged)
@@ -361,9 +385,18 @@ public:
void setLastDisplayedEventId(const QString& eventId);
void setLastDisplayedEvent(TimelineItem::index_t index);
+ [[deprecated("Use lastReadReceipt() to get m.read receipt or"
+ " fullyReadMarker() to get m.fully_read marker")]] //
rev_iter_t readMarker(const User* user) const;
+ [[deprecated("Use lastReadReceipt() to get m.read receipt or"
+ " fullyReadMarker() to get m.fully_read marker")]] //
rev_iter_t readMarker() const;
+ [[deprecated("Use lastReadReceipt() to get m.read receipt or"
+ " fullyReadMarker() to get m.fully_read marker")]] //
QString readMarkerEventId() const;
+ ReadReceipt lastReadReceipt(const QString& userId) const;
+ QString lastFullyReadEventId() const;
+ rev_iter_t fullyReadMarker() const;
QSet<User*> usersAtEventId(const QString& eventId);
/**
* \brief Mark the event with uptoEventId as read
@@ -704,7 +737,10 @@ Q_SIGNALS:
void firstDisplayedEventChanged();
void lastDisplayedEventChanged();
void lastReadEventChanged(Quotient::User* user);
+ void fullyReadMarkerMoved(QString fromEventId, QString toEventId);
+ //! \deprecated since 0.7 - use fullyReadMarkerMoved
void readMarkerMoved(QString fromEventId, QString toEventId);
+ //! \deprecated since 0.7 - use lastReadEventChanged
void readMarkerForUserMoved(Quotient::User* user, QString fromEventId,
QString toEventId);
void unreadMessagesChanged(Quotient::Room* room);
@@ -779,4 +815,5 @@ private:
};
} // namespace Quotient
Q_DECLARE_METATYPE(Quotient::FileTransferInfo)
+Q_DECLARE_METATYPE(Quotient::ReadReceipt)
Q_DECLARE_OPERATORS_FOR_FLAGS(Quotient::Room::Changes)
--
cgit v1.2.3
From 2975b435f1ae198187183b883b5e666c7659e8fc Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Fri, 30 Jul 2021 23:25:01 +0200
Subject: Room::setReadReceipt()
---
lib/room.cpp | 9 +++++++++
lib/room.h | 7 ++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index c7f29b6d..649b1ed2 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -765,6 +765,15 @@ Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
return changes;
}
+void Room::setReadReceipt(const QString& atEventId)
+{
+ d->setLastReadReceipt(localUser(), historyEdge(),
+ { atEventId, QDateTime::currentDateTime() });
+ connection()->callApi<PostReceiptJob>(BackgroundRequest, id(),
+ QStringLiteral("m.read"),
+ QUrl::toPercentEncoding(atEventId));
+}
+
void Room::Private::markMessagesAsRead(const rev_iter_t &upToMarker)
{
if (upToMarker < q->fullyReadMarker()) {
diff --git a/lib/room.h b/lib/room.h
index 9ef553df..afe223c5 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -626,7 +626,12 @@ public Q_SLOTS:
void downloadFile(const QString& eventId, const QUrl& localFilename = {});
void cancelFileTransfer(const QString& id);
- /// Mark all messages in the room as read
+ //! \brief Set a given event as last read and post a read receipt on it
+ //!
+ //! Does nothing if the event is behind the current read receipt.
+ //! \sa lastReadReceipt, markMessagesAsRead, markAllMessagesAsRead
+ void setReadReceipt(const QString& atEventId);
+ //! Put the fully-read marker at the latest message in the room
void markAllMessagesAsRead();
/// Switch the room's version (aka upgrade)
--
cgit v1.2.3
From 6d558fa9c12b52fe3cc47cc143d0a758c4ea929a Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Sat, 31 Jul 2021 08:30:44 +0200
Subject: Add/update doc-comments on the new/updated methods
[skip ci]
---
lib/room.h | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 65 insertions(+), 10 deletions(-)
diff --git a/lib/room.h b/lib/room.h
index afe223c5..a8c3cac2 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -378,30 +378,85 @@ public:
void setLastDisplayedEventId(const QString& eventId);
void setLastDisplayedEvent(TimelineItem::index_t index);
+ //! \brief Obtain a read receipt of any user
+ //!
+ //! \deprecated Use lastReadReceipt or fullyReadMarker instead
+ //! Historically, readMarker was returning a "converged" read marker
+ //! representing both the read receipt and the fully read marker, as
+ //! Quotient managed them together. Since 0.6.8, a single-argument call of
+ //! readMarker returns the last read receipt position (for any room member)
+ //! and a call without arguments returns the last _fully read_ position,
+ //! to provide access to both positions separately while maintaining API
+ //! stability guarantees. 0.7 has separate methods to return read receipts
+ //! and the fully read marker - use them instead.
+ //! \sa lastReadReceipt
[[deprecated("Use lastReadReceipt() to get m.read receipt or"
" fullyReadMarker() to get m.fully_read marker")]] //
rev_iter_t readMarker(const User* user) const;
+ //! \brief Obtain the local user's fully-read marker
+ //! \deprecated Use fullyReadMarker instead
+ //! See the documentation for the single-argument overload
+ //! \sa fullyReadMarker
[[deprecated("Use lastReadReceipt() to get m.read receipt or"
" fullyReadMarker() to get m.fully_read marker")]] //
rev_iter_t readMarker() const;
+ //! \brief Get the event id for the local user's fully-read marker
+ //! \deprecated Use lastFullyReadEventId instead
+ //! See the readMarker documentation
[[deprecated("Use lastReadReceipt() to get m.read receipt or"
" fullyReadMarker() to get m.fully_read marker")]] //
QString readMarkerEventId() const;
+
+ //! \brief Get the latest read receipt from a user
+ //!
+ //! The user id must be valid. A read receipt with an empty event id
+ //! is returned if the user id is valid but there was no read receipt
+ //! from them.
+ //! \sa usersAtEventId
ReadReceipt lastReadReceipt(const QString& userId) const;
+
+ //! \brief Get the latest event id marked as fully read
+ //!
+ //! This can be either the event id pointed to by the actual latest
+ //! m.fully_read event, or the latest event id marked locally as fully read
+ //! if markMessagesAsRead or markAllMessagesAsRead has been called and
+ //! the homeserver didn't return an updated m.fully_read event yet.
+ //! \sa markMessagesAsRead, markAllMessagesAsRead, fullyReadMarker
QString lastFullyReadEventId() const;
+
+ //! \brief Get the iterator to the latest timeline item marked as fully read
+ //!
+ //! This method calls findInTimeline on the result of lastFullyReadEventId.
+ //! If the fully read marker turns out to be outside the timeline (because
+ //! the event marked as fully read is too far back in the history) the
+ //! returned value will be equal to historyEdge.
+ //!
+ //! Be sure to read the caveats on iterators returned by findInTimeline.
+ //! \sa lastFullyReadEventId, findInTimeline
rev_iter_t fullyReadMarker() const;
+
+ //! \brief Get users whose latest read receipts point to the event
+ //!
+ //! This method is for cases when you need to show users who have read
+ //! an event. Calling it on inexistent or empty event id will return
+ //! an empty set.
+ //! \sa lastReadReceipt
QSet<User*> usersAtEventId(const QString& eventId);
- /**
- * \brief Mark the event with uptoEventId as read
- *
- * Finds in the timeline and marks as read the event with
- * the specified id; also posts a read receipt to the server either
- * for this message or, if it's from the local user, for
- * the nearest non-local message before. uptoEventId must be non-empty.
- */
- void markMessagesAsRead(QString uptoEventId);
- /// Check whether there are unread messages in the room
+ //!
+ //! \brief Mark the event with uptoEventId as fully read
+ //!
+ //! Marks the event with the specified id as fully read locally and also
+ //! sends an update to m.fully_read account data to the server either
+ //! for this message or, if it's from the local user, for
+ //! the nearest non-local message before. uptoEventId must point to a known
+ //! event in the timeline; the method will do nothing if the event is behind
+ //! the current m.fully_read marker or is not loaded, to prevent
+ //! accidentally trying to move the marker back in the timeline.
+ //! \sa markAllMessagesAsRead, fullyReadMarker
+ Q_INVOKABLE void markMessagesAsRead(QString uptoEventId);
+
+ //! Check whether there are unread messages in the room
bool hasUnreadMessages() const;
/** Get the number of unread messages in the room
--
cgit v1.2.3
From 8093ea85347588431c0ddbf6e178a4db5f51b909 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Sun, 1 Aug 2021 17:19:23 +0200
Subject: Enhanced logging for read receipts
---
lib/room.cpp | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index 649b1ed2..e7d4e137 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -617,7 +617,8 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
return; // For Release builds
}
if (q->memberJoinState(u) != JoinState::Join) {
- qCWarning(MAIN) << "Won't record read receipt for non-member" << u->id();
+ qCWarning(EPHEMERAL)
+ << "Won't record read receipt for non-member" << u->id();
return;
}
@@ -626,8 +627,11 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
newMarker = q->findInTimeline(newReceipt.eventId);
if (newMarker != timeline.crend()) {
// NB: with reverse iterators, timeline history >= sync edge
- if (newMarker >= q->findInTimeline(storedReceipt.eventId))
+ if (newMarker >= q->findInTimeline(storedReceipt.eventId)) {
+ qCDebug(EPHEMERAL) << "The new read receipt for" << u->id()
+ << "is at or behind the old one, skipping";
return;
+ }
// Try to auto-promote the read marker over the user's own messages
// (switch to direct iterators for that).
@@ -651,6 +655,8 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
eventIdReadUsers.remove(storedReceipt.eventId);
eventIdReadUsers[newReceipt.eventId].insert(u);
swap(storedReceipt, newReceipt); // Now newReceipt actually stores the old receipt
+ qCDebug(EPHEMERAL) << "The new read receipt for" << u->id() << "is at"
+ << storedReceipt.eventId;
emit q->lastReadEventChanged(u);
// TODO: remove in 0.8
if (!isLocalUser(u))
--
cgit v1.2.3
From c79dcb673b6f2888faa4b62cd53689fde9f7ea10 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Sun, 1 Aug 2021 17:22:42 +0200
Subject: Room::toJson(): save the last local user's read receipt
Read receipts are entangled with counting unread messages, and saving
them also helps in not sending receipts for too old events. Other users'
read receipts are still treated as truly ephemeral.
---
lib/room.cpp | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/lib/room.cpp b/lib/room.cpp
index e7d4e137..1c84ece0 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -3003,6 +3003,32 @@ QJsonObject Room::Private::toJson() const
{ QStringLiteral("events"), accountDataEvents } });
}
+ if (const auto& readReceiptEventId =
+ lastReadReceipts.value(q->localUser()).eventId;
+ !readReceiptEventId.isEmpty()) //
+ {
+ // Okay, that's a mouthful; but basically, it's simply placing an m.read
+ // event in the 'ephemeral' section of the cached sync payload.
+ // See also receiptevent.* and m.read example in the spec.
+ // Only the local user's read receipt is saved - others' are really
+ // considered ephemeral but this one is useful in understanding where
+ // the user is in the timeline before any history is loaded.
+ result.insert(
+ QStringLiteral("ephemeral"),
+ QJsonObject {
+ { QStringLiteral("events"),
+ QJsonArray { QJsonObject {
+ { TypeKey, ReceiptEvent::matrixTypeId() },
+ { ContentKey,
+ QJsonObject {
+ { readReceiptEventId,
+ QJsonObject {
+ { QStringLiteral("m.read"),
+ QJsonObject {
+ { connection->userId(),
+ QJsonObject {} } } } } } } } } } } });
+ }
+
QJsonObject unreadNotifObj { { SyncRoomData::UnreadCountKey,
unreadMessages } };
--
cgit v1.2.3
From 4f08c88d234119c2a76874ebd2b1433b81992427 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Sun, 1 Aug 2021 17:47:56 +0200
Subject: Room::setDisplayed(): Don't reset unread counters
Closes #489.
---
lib/room.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index 1c84ece0..8462902f 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -924,11 +924,8 @@ void Room::setDisplayed(bool displayed)
d->displayed = displayed;
emit displayedChanged(displayed);
- if (displayed) {
- resetHighlightCount();
- resetNotificationCount();
+ if (displayed)
d->getAllMembers();
- }
}
QString Room::firstDisplayedEventId() const { return d->firstDisplayedEventId; }
--
cgit v1.2.3
From 762be1e73d6f5021e63fc9a4fea273951b3fb113 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Sun, 10 Oct 2021 22:26:08 +0200
Subject: ReadReceipt::operator==/!=: Add const where it's due
---
lib/room.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/room.h b/lib/room.h
index ca9f36cb..73f28a6e 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -81,11 +81,14 @@ public:
QString eventId;
QDateTime timestamp;
- bool operator==(const ReadReceipt& other)
+ bool operator==(const ReadReceipt& other) const
{
return eventId == other.eventId && timestamp == other.timestamp;
}
- bool operator!=(const ReadReceipt& other) { return !operator==(other); }
+ bool operator!=(const ReadReceipt& other) const
+ {
+ return !operator==(other);
+ }
};
inline void swap(ReadReceipt& lhs, ReadReceipt& rhs)
{
--
cgit v1.2.3
From 5c0346f3a700e6af31463490b7af3382b86e09d5 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Wed, 8 Sep 2021 05:47:10 +0200
Subject: Room: actually initialise read marker when needed
This fixes the `q->readMarker() != historyEdge()` assertion failure
occuring in recalculateUnreadCount() when new events from sync arrive
to a room with no read marker and all history loaded.
---
lib/room.cpp | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index a2b99039..ea8df286 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -686,23 +686,24 @@ Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
if (fullyReadMarker < from)
return NoChange; // What's arrived is already fully read
+ // If there's no read marker in the whole room, initialise it
if (fullyReadMarker == historyEdge() && q->allHistoryLoaded())
- --fullyReadMarker; // No read marker in the whole room, initialise it
- if (fullyReadMarker < to) {
- // Catch a special case when the last fully read event id refers to an
- // event that has just arrived. In this case we should recalculate
- // unreadMessages to get an exact number instead of an estimation
- // (see https://github.com/quotient-im/libQuotient/wiki/unread_count).
- // For the same reason (switching from the estimation to the exact
- // number) this branch always emits unreadMessagesChanged() and returns
- // UnreadNotifsChange, even if the estimation luckily matched the exact
- // result.
+ return setFullyReadMarker(timeline.front()->id());
+
+ // Catch a special case when the last fully read event id refers to an
+ // event that has just arrived. In this case we should recalculate
+ // unreadMessages to get an exact number instead of an estimation
+ // (see https://github.com/quotient-im/libQuotient/wiki/unread_count).
+ // For the same reason (switching from the estimation to the exact
+ // number) this branch always emits unreadMessagesChanged() and returns
+ // UnreadNotifsChange, even if the estimation luckily matched the exact
+ // result.
+ if (fullyReadMarker < to)
return recalculateUnreadCount(true);
- }
- // Fully read marker is somewhere beyond the most historical message from
- // the arrived batch - add up newly arrived messages to the current counter,
- // instead of a complete recalculation.
+ // At this point the fully read marker is somewhere beyond the "oldest"
+ // message from the arrived batch - add up newly arrived messages to
+ // the current counter, instead of a complete recalculation.
Q_ASSERT(to <= fullyReadMarker);
QElapsedTimer et;
@@ -769,7 +770,7 @@ Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
const auto prevFullyReadId = std::exchange(fullyReadUntilEventId, eventId);
qCDebug(MESSAGES) << "Fully read marker in" << q->objectName() //
- << "moved to" << fullyReadUntilEventId;
+ << "set to" << fullyReadUntilEventId;
emit q->fullyReadMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
// TODO: Remove in 0.8
emit q->readMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
--
cgit v1.2.3
From 29195491c25bcba50b78a30c779db07913f87d3a Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Thu, 11 Nov 2021 13:01:58 +0100
Subject: Update comments around read receipts code
---
lib/room.cpp | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index ceb8d111..4d11878f 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -2498,12 +2498,6 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events)
<< totalInserted << "new events; the last event is now"
<< timeline.back();
- // The first event in the just-added batch (referred to by `from`)
- // defines whose read receipt can possibly be promoted any further over
- // the same author's events newly arrived. Others will need explicit
- // read receipts from the server - or, for the local user, calling
- // setLastDisplayedEventId() - to promote their read receipts over
- // the new message events.
auto* const firstWriter = q->user((*from)->senderId());
setLastReadReceipt(firstWriter, rev_iter_t(from + 1));
if (firstWriter == q->localUser()
@@ -2813,15 +2807,14 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
if (newMarker == historyEdge())
qCDebug(EPHEMERAL) << "Event of the read receipt(s) is not "
"found; saving them anyway";
+ // If the event is not found (most likely, because it's too old and
+ // hasn't been fetched from the server yet) but there is a previous
+ // marker for a user, keep the previous marker because read receipts
+ // are not supposed to move backwards. Otherwise, blindly store
+ // the event id for this user and update the read marker when/if
+ // the event is fetched later on.
for (const Receipt& r : p.receipts)
if (isMember(r.userId)) {
- // If the event is not found (most likely, because it's
- // too old and hasn't been fetched from the server yet)
- // but there is a previous marker for a user, keep
- // the previous marker because read receipts are not
- // supposed to move backwards. Otherwise, blindly
- // store the event id for this user and update the read
- // marker when/if the event is fetched later on.
d->setLastReadReceipt(user(r.userId), newMarker,
{ p.evtId, r.timestamp });
}
--
cgit v1.2.3
From 05e71a72fdc6da3fb319edc67b723999285c9657 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Thu, 11 Nov 2021 14:35:54 +0100
Subject: Fix a few quirks in converters.h
---
lib/converters.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/converters.h b/lib/converters.h
index cc6378e4..2df18b03 100644
--- a/lib/converters.h
+++ b/lib/converters.h
@@ -242,12 +242,11 @@ struct JsonObjectConverter<QSet<QString>> {
for (const auto& e : s)
json.insert(toJson(e), QJsonObject {});
}
- static auto fillFrom(const QJsonObject& json, QSet<QString>& s)
+ static void fillFrom(const QJsonObject& json, QSet<QString>& s)
{
s.reserve(s.size() + json.size());
for (auto it = json.begin(); it != json.end(); ++it)
s.insert(it.key());
- return s;
}
};
@@ -260,7 +259,7 @@ struct HashMapFromJson {
}
static void fillFrom(const QJsonObject& jo, HashMapT& h)
{
- h.reserve(jo.size());
+ h.reserve(h.size() + jo.size());
for (auto it = jo.begin(); it != jo.end(); ++it)
h[it.key()] = fromJson<typename HashMapT::mapped_type>(it.value());
}
--
cgit v1.2.3
From f2bf3f203965c51824e8681427798f7a09784ce3 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Thu, 11 Nov 2021 22:18:18 +0100
Subject: Make ReceiptEvent constructible from content
Makes the Room::P::toJson() code more readable.
---
lib/converters.h | 7 +++++--
lib/events/receiptevent.cpp | 29 +++++++++++++++++++++++++----
lib/events/receiptevent.h | 5 +++--
lib/room.cpp | 27 +++++++--------------------
4 files changed, 40 insertions(+), 28 deletions(-)
diff --git a/lib/converters.h b/lib/converters.h
index 2df18b03..d9a68bfb 100644
--- a/lib/converters.h
+++ b/lib/converters.h
@@ -134,7 +134,10 @@ struct JsonConverter<QString> : public TrivialJsonDumper<QString> {
template <>
struct JsonConverter<QDateTime> {
- static auto dump(const QDateTime& val) = delete; // not provided yet
+ static auto dump(const QDateTime& val)
+ {
+ return val.isValid() ? val.toMSecsSinceEpoch() : QJsonValue();
+ }
static auto load(const QJsonValue& jv)
{
return QDateTime::fromMSecsSinceEpoch(fromJson<qint64>(jv), Qt::UTC);
@@ -143,7 +146,7 @@ struct JsonConverter<QDateTime> {
template <>
struct JsonConverter<QDate> {
- static auto dump(const QDate& val) = delete; // not provided yet
+ static auto dump(const QDate& val) { return toJson(QDateTime(val)); }
static auto load(const QJsonValue& jv)
{
return fromJson<QDateTime>(jv).date();
diff --git a/lib/events/receiptevent.cpp b/lib/events/receiptevent.cpp
index 4185d92d..72dbf2e3 100644
--- a/lib/events/receiptevent.cpp
+++ b/lib/events/receiptevent.cpp
@@ -25,6 +25,27 @@ Example of a Receipt Event:
using namespace Quotient;
+// The library loads the event-ids-to-receipts JSON map into a vector because
+// map lookups are not used and vectors are massively faster. Same goes for
+// de-/serialization of ReceiptsForEvent::receipts.
+// (XXX: would this be generally preferred across CS API JSON maps?..)
+QJsonObject toJson(const EventsWithReceipts& ewrs)
+{
+ QJsonObject json;
+ for (const auto& e : ewrs) {
+ QJsonObject receiptsJson;
+ for (const auto& r : e.receipts)
+ receiptsJson.insert(r.userId,
+ QJsonObject { { "ts"_ls, toJson(r.timestamp) } });
+ json.insert(e.evtId, QJsonObject { { "m.read"_ls, receiptsJson } });
+ }
+ return json;
+}
+
+ReceiptEvent::ReceiptEvent(const EventsWithReceipts &ewrs)
+ : Event(typeId(), matrixTypeId(), toJson(ewrs))
+{}
+
EventsWithReceipts ReceiptEvent::eventsWithReceipts() const
{
EventsWithReceipts result;
@@ -39,14 +60,14 @@ EventsWithReceipts ReceiptEvent::eventsWithReceipts() const
}
const auto reads =
eventIt.value().toObject().value("m.read"_ls).toObject();
- QVector<Receipt> receipts;
- receipts.reserve(reads.size());
+ QVector<UserTimestamp> usersAtEvent;
+ usersAtEvent.reserve(reads.size());
for (auto userIt = reads.begin(); userIt != reads.end(); ++userIt) {
const auto user = userIt.value().toObject();
- receipts.push_back(
+ usersAtEvent.push_back(
{ userIt.key(), fromJson<QDateTime>(user["ts"_ls]) });
}
- result.push_back({ eventIt.key(), std::move(receipts) });
+ result.push_back({ eventIt.key(), std::move(usersAtEvent) });
}
return result;
}
diff --git a/lib/events/receiptevent.h b/lib/events/receiptevent.h
index 4feec9ea..9683deef 100644
--- a/lib/events/receiptevent.h
+++ b/lib/events/receiptevent.h
@@ -9,19 +9,20 @@
#include <QtCore/QVector>
namespace Quotient {
-struct Receipt {
+struct UserTimestamp {
QString userId;
QDateTime timestamp;
};
struct ReceiptsForEvent {
QString evtId;
- QVector<Receipt> receipts;
+ QVector<UserTimestamp> receipts;
};
using EventsWithReceipts = QVector<ReceiptsForEvent>;
class ReceiptEvent : public Event {
public:
DEFINE_EVENT_TYPEID("m.receipt", ReceiptEvent)
+ explicit ReceiptEvent(const EventsWithReceipts& ewrs);
explicit ReceiptEvent(const QJsonObject& obj) : Event(typeId(), obj) {}
EventsWithReceipts eventsWithReceipts() const;
diff --git a/lib/room.cpp b/lib/room.cpp
index 4d11878f..58950aac 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -2813,7 +2813,7 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
// are not supposed to move backwards. Otherwise, blindly store
// the event id for this user and update the read marker when/if
// the event is fetched later on.
- for (const Receipt& r : p.receipts)
+ for (const auto& r : p.receipts)
if (isMember(r.userId)) {
d->setLastReadReceipt(user(r.userId), newMarker,
{ p.evtId, r.timestamp });
@@ -3021,30 +3021,17 @@ QJsonObject Room::Private::toJson() const
{ QStringLiteral("events"), accountDataEvents } });
}
- if (const auto& readReceiptEventId =
- lastReadReceipts.value(q->localUser()).eventId;
- !readReceiptEventId.isEmpty()) //
+ if (const auto& readReceipt = q->lastReadReceipt(connection->userId());
+ !readReceipt.eventId.isEmpty()) //
{
- // Okay, that's a mouthful; but basically, it's simply placing an m.read
- // event in the 'ephemeral' section of the cached sync payload.
- // See also receiptevent.* and m.read example in the spec.
- // Only the local user's read receipt is saved - others' are really
- // considered ephemeral but this one is useful in understanding where
- // the user is in the timeline before any history is loaded.
result.insert(
QStringLiteral("ephemeral"),
QJsonObject {
{ QStringLiteral("events"),
- QJsonArray { QJsonObject {
- { TypeKey, ReceiptEvent::matrixTypeId() },
- { ContentKey,
- QJsonObject {
- { readReceiptEventId,
- QJsonObject {
- { QStringLiteral("m.read"),
- QJsonObject {
- { connection->userId(),
- QJsonObject {} } } } } } } } } } } });
+ QJsonArray { ReceiptEvent({ { readReceipt.eventId,
+ { { connection->userId(),
+ readReceipt.timestamp } } } })
+ .fullJson() } } });
}
QJsonObject unreadNotifObj { { SyncRoomData::UnreadCountKey,
--
cgit v1.2.3
From edb63528e6f3048045f70eb6a48412917bdbea0b Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Mon, 15 Nov 2021 21:37:04 +0100
Subject: Bind read receipts to userIds, not to User* values
This reduces the surface interacting with the User class that eventually
will be split into LocalUser (most part) and RoomMember (a tiny wrapper
around the member data in a given room, used almost everywhere in Room
where User currently is).
Also: dropped a log message when the new receipt is at or behind
the old one as it causes a lot of noise in the logs.
---
lib/room.cpp | 163 +++++++++++++++++++++++++++++++++--------------------------
lib/room.h | 12 ++++-
2 files changed, 100 insertions(+), 75 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index 58950aac..ee76a85c 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -120,14 +120,14 @@ public:
int notificationCount = 0;
members_map_t membersMap;
QList<User*> usersTyping;
- QHash<QString, QSet<User*>> eventIdReadUsers;
+ QHash<QString, QSet<QString>> eventIdReadUsers;
QList<User*> usersInvited;
QList<User*> membersLeft;
int unreadMessages = 0;
bool displayed = false;
QString firstDisplayedEventId;
QString lastDisplayedEventId;
- QHash<const User*, ReadReceipt> lastReadReceipts;
+ QHash<QString, ReadReceipt> lastReadReceipts;
QString fullyReadUntilEventId;
TagsMap tags;
UnorderedMap<QString, EventPtr> accountData;
@@ -291,7 +291,7 @@ public:
*/
void dropDuplicateEvents(RoomEvents& events) const;
- void setLastReadReceipt(User* u, rev_iter_t newMarker,
+ bool setLastReadReceipt(const QString& userId, rev_iter_t newMarker,
ReadReceipt newReceipt = {});
Changes setFullyReadMarker(const QString &eventId);
Changes updateUnreadCount(const rev_iter_t& from, const rev_iter_t& to);
@@ -619,60 +619,62 @@ void Room::setJoinState(JoinState state)
emit joinStateChanged(oldState, state);
}
-void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
+bool Room::Private::setLastReadReceipt(const QString& userId,
+ rev_iter_t newMarker,
ReadReceipt newReceipt)
{
- if (!u) {
- Q_ASSERT(u != nullptr); // For Debug builds
- qCCritical(MAIN) << "Empty user, skipping read receipt registration";
- return; // For Release builds
- }
- if (!q->isMember(u->id())) {
- qCWarning(EPHEMERAL)
- << "Won't record read receipt for non-member" << u->id();
- return;
- }
-
- auto& storedReceipt = lastReadReceipts[u];
- if (newMarker == timeline.crend() && !newReceipt.eventId.isEmpty())
+ if (newMarker == historyEdge() && !newReceipt.eventId.isEmpty())
newMarker = q->findInTimeline(newReceipt.eventId);
- if (newMarker != timeline.crend()) {
- // NB: with reverse iterators, timeline history >= sync edge
- if (newMarker >= q->findInTimeline(storedReceipt.eventId)) {
- qCDebug(EPHEMERAL) << "The new read receipt for" << u->id()
- << "is at or behind the old one, skipping";
- return;
- }
-
+ if (newMarker != historyEdge()) {
// Try to auto-promote the read marker over the user's own messages
// (switch to direct iterators for that).
- const auto eagerMarker = find_if(newMarker.base(), timeline.cend(),
+ const auto eagerMarker = find_if(newMarker.base(), syncEdge(),
[=](const TimelineItem& ti) {
- return ti->senderId() != u->id();
- })
- - 1;
- newReceipt.eventId = (*eagerMarker)->id();
- if (eagerMarker != newMarker.base() - 1) // &*(rIt.base() - 1) === &*rIt
- qCDebug(EPHEMERAL) << "Auto-promoted read receipt for" << u->id()
+ return ti->senderId() != userId;
+ });
+ // eagerMarker is now just after the desired event for newMarker
+ if (eagerMarker != newMarker.base()) {
+ qCDebug(EPHEMERAL) << "Auto-promoted read receipt for" << userId
<< "to" << newReceipt.eventId;
- }
+ newMarker = rev_iter_t(eagerMarker);
+ }
+ // Fill newReceipt with the event (and, if needed, timestamp) from
+ // eagerMarker
+ newReceipt.eventId = (eagerMarker - 1)->event()->id();
+ if (newReceipt.timestamp.isNull())
+ newReceipt.timestamp = QDateTime::currentDateTime();
+ }
+ auto& storedReceipt =
+ lastReadReceipts[userId]; // clazy:exclude=detaching-member
+ const auto prevEventId = storedReceipt.eventId;
+ // NB: with reverse iterators, timeline history >= sync edge
+ if (newMarker >= q->findInTimeline(prevEventId))
+ return false;
- if (storedReceipt == newReceipt)
- return;
// Finally make the change
- auto& oldEventReadUsers = eventIdReadUsers[storedReceipt.eventId];
- oldEventReadUsers.remove(u);
- if (oldEventReadUsers.isEmpty())
- eventIdReadUsers.remove(storedReceipt.eventId);
- eventIdReadUsers[newReceipt.eventId].insert(u);
- swap(storedReceipt, newReceipt); // Now newReceipt actually stores the old receipt
- qCDebug(EPHEMERAL) << "The new read receipt for" << u->id() << "is at"
+
+ auto oldEventReadUsersIt =
+ eventIdReadUsers.find(prevEventId); // clazy:exclude=detaching-member
+ if (oldEventReadUsersIt != eventIdReadUsers.end()) {
+ oldEventReadUsersIt->remove(userId);
+ if (oldEventReadUsersIt->isEmpty())
+ eventIdReadUsers.erase(oldEventReadUsersIt);
+ }
+ eventIdReadUsers[newReceipt.eventId].insert(userId);
+ storedReceipt = move(newReceipt);
+ qCDebug(EPHEMERAL) << "The new read receipt for" << userId << "is now at"
<< storedReceipt.eventId;
- emit q->lastReadEventChanged(u);
+
+ // TODO: use Room::member() when it becomes a thing and only emit signals
+ // for actual members, not just any user
+ const auto member = q->user(userId);
+ Q_ASSERT(member != nullptr);
+ emit q->lastReadEventChanged(member);
// TODO: remove in 0.8
- if (!isLocalUser(u))
- emit q->readMarkerForUserMoved(u, newReceipt.eventId,
+ if (!isLocalUser(member))
+ emit q->readMarkerForUserMoved(member, prevEventId,
storedReceipt.eventId);
+ return true;
}
Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
@@ -777,7 +779,7 @@ Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
Changes changes = ReadMarkerChange;
if (const auto rm = q->fullyReadMarker(); rm != historyEdge()) {
// Pull read receipt if it's behind
- setLastReadReceipt(q->localUser(), rm);
+ setLastReadReceipt(connection->userId(), rm);
changes |= recalculateUnreadCount(); // TODO: updateUnreadCount()?
}
return changes;
@@ -785,8 +787,13 @@ Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
void Room::setReadReceipt(const QString& atEventId)
{
- d->setLastReadReceipt(localUser(), historyEdge(),
- { atEventId, QDateTime::currentDateTime() });
+ if (!d->setLastReadReceipt(localUser()->id(), historyEdge(),
+ { atEventId, QDateTime::currentDateTime() })) {
+ qCDebug(EPHEMERAL) << "The new read receipt for" << localUser()->id()
+ << "in" << objectName()
+ << "is at or behind the old one, skipping";
+ return;
+ }
connection()->callApi<PostReceiptJob>(BackgroundRequest, id(),
QStringLiteral("m.read"),
QUrl::toPercentEncoding(atEventId));
@@ -1004,7 +1011,7 @@ QString Room::readMarkerEventId() const { return lastFullyReadEventId(); }
ReadReceipt Room::lastReadReceipt(const QString& userId) const
{
- return d->lastReadReceipts.value(user(userId));
+ return d->lastReadReceipts.value(userId);
}
QString Room::lastFullyReadEventId() const { return d->fullyReadUntilEventId; }
@@ -1014,11 +1021,21 @@ Room::rev_iter_t Room::fullyReadMarker() const
return findInTimeline(d->fullyReadUntilEventId);
}
-QSet<User*> Room::usersAtEventId(const QString& eventId)
+QSet<QString> Room::userIdsAtEvent(const QString& eventId)
{
return d->eventIdReadUsers.value(eventId);
}
+QSet<User*> Room::usersAtEventId(const QString& eventId)
+{
+ const auto& userIds = d->eventIdReadUsers.value(eventId);
+ QSet<User*> users;
+ users.reserve(userIds.size());
+ for (const auto& uId : userIds)
+ users.insert(user(uId));
+ return users;
+}
+
int Room::notificationCount() const { return d->notificationCount; }
void Room::resetNotificationCount()
@@ -2498,17 +2515,16 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events)
<< totalInserted << "new events; the last event is now"
<< timeline.back();
- auto* const firstWriter = q->user((*from)->senderId());
- setLastReadReceipt(firstWriter, rev_iter_t(from + 1));
- if (firstWriter == q->localUser()
+ const auto& firstWriterId = (*from)->senderId();
+ setLastReadReceipt(firstWriterId, rev_iter_t(from + 1));
+ // If the local user's message(s) is/are first in the batch
+ // and the fully read marker was right before it, promote
+ // the fully read marker to the same event as the read receipt.
+ if (firstWriterId == connection->userId()
&& q->fullyReadMarker().base() == from) //
- {
- // If the local user's message(s) is/are first in the batch
- // and the fully read marker was right before it, promote
- // the fully read marker to the same event as the read receipt.
roomChanges |=
- setFullyReadMarker(lastReadReceipts.value(firstWriter).eventId);
- }
+ setFullyReadMarker(q->lastReadReceipt(firstWriterId).eventId);
+
roomChanges |= updateUnreadCount(timeline.crbegin(), rev_iter_t(from));
}
@@ -2781,6 +2797,7 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
et.start();
if (auto* evt = eventCast<TypingEvent>(event)) {
d->usersTyping.clear();
+ d->usersTyping.reserve(evt->users().size()); // Assume all are members
for (const auto& userId : evt->users())
if (isMember(userId))
d->usersTyping.append(user(userId));
@@ -2795,29 +2812,29 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
const auto& eventsWithReceipts = evt->eventsWithReceipts();
for (const auto& p : eventsWithReceipts) {
totalReceipts += p.receipts.size();
- {
- if (p.receipts.size() == 1)
- qCDebug(EPHEMERAL) << "Marking" << p.evtId << "as read for"
- << p.receipts[0].userId;
- else
- qCDebug(EPHEMERAL) << "Marking" << p.evtId << "as read for"
- << p.receipts.size() << "users";
- }
const auto newMarker = findInTimeline(p.evtId);
if (newMarker == historyEdge())
qCDebug(EPHEMERAL) << "Event of the read receipt(s) is not "
- "found; saving them anyway";
+ "found; saving anyway";
// If the event is not found (most likely, because it's too old and
// hasn't been fetched from the server yet) but there is a previous
// marker for a user, keep the previous marker because read receipts
// are not supposed to move backwards. Otherwise, blindly store
// the event id for this user and update the read marker when/if
// the event is fetched later on.
- for (const auto& r : p.receipts)
- if (isMember(r.userId)) {
- d->setLastReadReceipt(user(r.userId), newMarker,
- { p.evtId, r.timestamp });
- }
+ const auto updatedCount = std::count_if(
+ p.receipts.cbegin(), p.receipts.cend(),
+ [this, &newMarker, &evtId = p.evtId](const auto& r) -> bool {
+ return d->setLastReadReceipt(r.userId, newMarker,
+ { evtId, r.timestamp });
+ });
+
+ if (p.receipts.size() > 1)
+ qCDebug(EPHEMERAL) << p.evtId << "marked as read for"
+ << updatedCount << "user(s)";
+ if (updatedCount < p.receipts.size())
+ qCDebug(EPHEMERAL) << p.receipts.size() - updatedCount
+ << "receipts were skipped";
}
if (eventsWithReceipts.size() > 3 || totalReceipts > 10
|| et.nsecsElapsed() >= profilerMinNsecs())
diff --git a/lib/room.h b/lib/room.h
index 260510e6..e4d33c4e 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -442,10 +442,18 @@ public:
//! This method is for cases when you need to show users who have read
//! an event. Calling it on inexistent or empty event id will return
//! an empty set.
- //! \sa lastReadReceipt
+ //! \note The returned list may contain ids resolving to users that are
+ //! not loaded as room members yet (in particular, if members are not
+ //! yet lazy-loaded). For now this merely means that the user's
+ //! room-specific name and avatar will not be there; but generally
+ //! it's recommended to ensure that all room members are loaded
+ //! before operating on the result of this function.
+ //! \sa lastReadReceipt, allMembersLoaded
+ QSet<QString> userIdsAtEvent(const QString& eventId);
+
+ [[deprecated("Use userIdsAtEvent instead")]]
QSet<User*> usersAtEventId(const QString& eventId);
- //!
//! \brief Mark the event with uptoEventId as fully read
//!
//! Marks the event with the specified id as fully read locally and also
--
cgit v1.2.3
From 7b633ba257fc8643ef8cc2ef724f3b6ac9e186ba Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Mon, 18 Oct 2021 05:42:12 +0200
Subject: Room: doc-comments cleanup
[skip ci]
---
lib/room.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/room.h b/lib/room.h
index e4d33c4e..24025a88 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -381,8 +381,8 @@ public:
void setLastDisplayedEvent(TimelineItem::index_t index);
//! \brief Obtain a read receipt of any user
+ //! \deprecated Use lastReadReceipt or fullyReadMarker instead.
//!
- //! \deprecated Use lastReadReceipt or fullyReadMarker instead
//! Historically, readMarker was returning a "converged" read marker
//! representing both the read receipt and the fully read marker, as
//! Quotient managed them together. Since 0.6.8, a single-argument call of
@@ -397,16 +397,19 @@ public:
rev_iter_t readMarker(const User* user) const;
//! \brief Obtain the local user's fully-read marker
//! \deprecated Use fullyReadMarker instead
- //! See the documentation for the single-argument overload
+ //!
+ //! See the documentation for the single-argument overload.
//! \sa fullyReadMarker
[[deprecated("Use lastReadReceipt() to get m.read receipt or"
" fullyReadMarker() to get m.fully_read marker")]] //
rev_iter_t readMarker() const;
//! \brief Get the event id for the local user's fully-read marker
//! \deprecated Use lastFullyReadEventId instead
+ //!
//! See the readMarker documentation
[[deprecated("Use lastReadReceipt() to get m.read receipt or"
- " fullyReadMarker() to get m.fully_read marker")]] //
+ " lastFullyReadEventId() to get an event id that"
+ " m.fully_read marker points to")]] //
QString readMarkerEventId() const;
//! \brief Get the latest read receipt from a user
--
cgit v1.2.3
From b472fc355dc5ce70391ca2b9bc8da35b973ae3a3 Mon Sep 17 00:00:00 2001
From: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Date: Mon, 8 Nov 2021 21:03:37 +0100
Subject: Room: lastLocalReadReceipt(), localReadReceiptMarker()
To simplify retrieval of the local m.read receipt and the marker for it.
---
lib/room.cpp | 10 ++++++++++
lib/room.h | 16 ++++++++++++++--
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/lib/room.cpp b/lib/room.cpp
index ee76a85c..273a5753 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -1014,6 +1014,16 @@ ReadReceipt Room::lastReadReceipt(const QString& userId) const
return d->lastReadReceipts.value(userId);
}
+ReadReceipt Room::lastLocalReadReceipt() const
+{
+ return d->lastReadReceipts.value(localUser()->id());
+}
+
+Room::rev_iter_t Room::localReadReceiptMarker() const
+{
+ return findInTimeline(lastLocalReadReceipt().eventId);
+}
+
QString Room::lastFullyReadEventId() const { return d->fullyReadUntilEventId; }
Room::rev_iter_t Room::fullyReadMarker() const
diff --git a/lib/room.h b/lib/room.h
index 24025a88..d94de51c 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -400,8 +400,7 @@ public:
//!
//! See the documentation for the single-argument overload.
//! \sa fullyReadMarker
- [[deprecated("Use lastReadReceipt() to get m.read receipt or"
- " fullyReadMarker() to get m.fully_read marker")]] //
+ [[deprecated("Use localReadReceiptMarker() or fullyReadMarker()")]] //
rev_iter_t readMarker() const;
//! \brief Get the event id for the local user's fully-read marker
//! \deprecated Use lastFullyReadEventId instead
@@ -420,6 +419,19 @@ public:
//! \sa usersAtEventId
ReadReceipt lastReadReceipt(const QString& userId) const;
+ //! \brief Get the latest read receipt from the local user
+ //!
+ //! This is a shortcut for <tt>lastReadReceipt(localUserId)</tt>.
+ //! \sa lastReadReceipt
+ ReadReceipt lastLocalReadReceipt() const;
+
+ //! \brief Find the timeline item the local read receipt is at
+ //!
+ //! This is a shortcut for \code
+ //! room->findInTimeline(room->lastLocalReadReceipt().eventId);
+ //! \endcode
+ rev_iter_t localReadReceiptMarker() const;
+
//! \brief Get the latest event id marked as fully read
//!
//! This can be either the event id pointed to by the actual latest
--
cgit v1.2.3