aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlexey Rusakov <Kitsune-Ral@users.sf.net>2021-11-21 06:55:16 +0100
committerAlexey Rusakov <Kitsune-Ral@users.sf.net>2021-11-21 07:07:00 +0100
commitb2f9b212c78bc9dd7c69f6a2d1f94376adb488e3 (patch)
tree4a282de17e2b6157b1ce3dc97b2b245853adc304
parente7babe6715672a358f7cc8b90d5df27e21a1b3e8 (diff)
downloadlibquotient-b2f9b212c78bc9dd7c69f6a2d1f94376adb488e3.tar.gz
libquotient-b2f9b212c78bc9dd7c69f6a2d1f94376adb488e3.zip
EventStats and Room::partiallyRead/unreadStats()
This introduces a new API to count unread events that would allow to obtain those unread and highlight counts since either the fully read marker (Room::partiallyReadStats) or the last read receipt (Room::unreadStats). Element uses the read receipt as the anchor to count unread numbers, while Quaternion historically used the fully read marker for that (with the pre-0.7 library sticking the two markers to each other). From now on the meaning of "unread" in Quotient is aligned with that of the spec and Element, and "partially read" means events between the fully read marker and the local read receipt; the design allows client authors to use either or both counting strategies as they see fit. Respectively, Room::P::setFullyReadMarker() updates partially-read statistics, while Room::P::setLastReadReceipt(), when called on a local user, updates unread statistics. Room::notificationCount() and Room::highlightCount() maintain their previous meaning as the counters since the last read receipt; Room::notificationCount() counts unread events locally, falling back to the value from the above-mentioned key defined by MSC2654, and if that is not there, further to `unread_notifications/notification_count` defined in the current spec. Room::highlightCount(), however, is still taken from the homeserver, not from Room::unreadStats().highlightCount.
-rw-r--r--CMakeLists.txt1
-rw-r--r--lib/eventstats.cpp98
-rw-r--r--lib/eventstats.h107
-rw-r--r--lib/room.cpp386
-rw-r--r--lib/room.h116
5 files changed, 544 insertions, 164 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3814bc7e..eaf662cc 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -135,6 +135,7 @@ list(APPEND lib_SRCS
lib/avatar.cpp
lib/uri.cpp
lib/uriresolver.cpp
+ lib/eventstats.cpp
lib/syncdata.cpp
lib/settings.cpp
lib/networksettings.cpp
diff --git a/lib/eventstats.cpp b/lib/eventstats.cpp
new file mode 100644
index 00000000..9fa7f5ff
--- /dev/null
+++ b/lib/eventstats.cpp
@@ -0,0 +1,98 @@
+// SPDX-FileCopyrightText: 2021 Quotient contributors
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+#include "eventstats.h"
+
+using namespace Quotient;
+
+EventStats EventStats::fromRange(const Room* room, const Room::rev_iter_t& from,
+ const Room::rev_iter_t& to,
+ const EventStats& init)
+{
+ Q_ASSERT(to <= room->historyEdge());
+ Q_ASSERT(from >= Room::rev_iter_t(room->syncEdge()));
+ Q_ASSERT(from <= to);
+ QElapsedTimer et;
+ et.start();
+ const auto result =
+ accumulate(from, to, init,
+ [room](EventStats acc, const TimelineItem& ti) {
+ acc.notableCount += room->isEventNotable(ti);
+ acc.highlightCount += room->notificationFor(ti).type
+ == Notification::Highlight;
+ return acc;
+ });
+ if (et.nsecsElapsed() > profilerMinNsecs() / 10)
+ qCDebug(PROFILER).nospace()
+ << "Event statistics collection over index range [" << from->index()
+ << "," << (to - 1)->index() << "] took " << et;
+ return result;
+}
+
+EventStats EventStats::fromMarker(const Room* room,
+ const EventStats::marker_t& marker)
+{
+ const auto s = fromRange(room, marker_t(room->syncEdge()), marker,
+ { 0, 0, marker == room->historyEdge() });
+ Q_ASSERT(s.isValidFor(room, marker));
+ return s;
+}
+
+EventStats EventStats::fromCachedCounters(Omittable<int> notableCount,
+ Omittable<int> highlightCount)
+{
+ const auto hCount = std::max(0, highlightCount.value_or(0));
+ if (!notableCount.has_value())
+ return { 0, hCount, true };
+ auto nCount = notableCount.value_or(0);
+ return { std::max(0, nCount), hCount, nCount != -1 };
+}
+
+bool EventStats::updateOnMarkerMove(const Room* room, const marker_t& oldMarker,
+ const marker_t& newMarker)
+{
+ if (newMarker == oldMarker)
+ return false;
+
+ // Double-check consistency between the old marker and the old stats
+ Q_ASSERT(isValidFor(room, oldMarker));
+ Q_ASSERT(oldMarker > newMarker);
+
+ // A bit of optimisation: only calculate the difference if the marker moved
+ // less than half the remaining timeline ahead; otherwise, recalculation
+ // over the remaining timeline will very likely be faster.
+ if (oldMarker != room->historyEdge()
+ && oldMarker - newMarker < newMarker - marker_t(room->syncEdge())) {
+ const auto removedStats = fromRange(room, newMarker, oldMarker);
+ Q_ASSERT(notableCount >= removedStats.notableCount
+ && highlightCount >= removedStats.highlightCount);
+ notableCount -= removedStats.notableCount;
+ highlightCount -= removedStats.highlightCount;
+ return removedStats.notableCount > 0 || removedStats.highlightCount > 0;
+ }
+
+ const auto newStats = EventStats::fromMarker(room, newMarker);
+ if (!isEstimate && newStats == *this)
+ return false;
+ *this = newStats;
+ return true;
+}
+
+bool EventStats::isValidFor(const Room* room, const marker_t& marker) const
+{
+ const auto markerAtHistoryEdge = marker == room->historyEdge();
+ // Either markerAtHistoryEdge and isEstimate are in the same state, or it's
+ // a special case of no notable events and the marker at history edge
+ // (then isEstimate can assume any value).
+ return markerAtHistoryEdge == isEstimate
+ || (markerAtHistoryEdge && notableCount == 0);
+}
+
+QDebug Quotient::operator<<(QDebug dbg, const EventStats& es)
+{
+ QDebugStateSaver _(dbg);
+ dbg.nospace() << es.notableCount << '/' << es.highlightCount;
+ if (es.isEstimate)
+ dbg << " (estimated)";
+ return dbg;
+}
diff --git a/lib/eventstats.h b/lib/eventstats.h
new file mode 100644
index 00000000..9be83377
--- /dev/null
+++ b/lib/eventstats.h
@@ -0,0 +1,107 @@
+// SPDX-FileCopyrightText: 2021 Quotient contributors
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+#pragma once
+
+#include "room.h"
+
+namespace Quotient {
+
+//! \brief Counters of unread events and highlights with a precision flag
+//!
+//! This structure contains a static snapshot with values of unread counters
+//! returned by Room::partiallyReadStats and Room::unreadStats (properties
+//! or methods).
+//!
+//! \note It's just a simple grouping of counters and is not automatically
+//! updated from the room as subsequent syncs arrive.
+//! \sa Room::unreadStats, Room::partiallyReadStats, Room::isEventNotable
+struct EventStats {
+ Q_GADGET
+ Q_PROPERTY(qsizetype notableCount MEMBER notableCount CONSTANT)
+ Q_PROPERTY(qsizetype highlightCount MEMBER highlightCount CONSTANT)
+ Q_PROPERTY(bool isEstimate MEMBER isEstimate CONSTANT)
+public:
+ //! The number of "notable" events in an events range
+ //! \sa Room::isEventNotable
+ qsizetype notableCount = 0;
+ qsizetype highlightCount = 0;
+ //! \brief Whether the counter values above are exact
+ //!
+ //! This is false when the end marker (m.read receipt or m.fully_read) used
+ //! to collect the stats points to an event loaded locally and the counters
+ //! can therefore be calculated exactly using the locally available segment
+ //! of the timeline; true when the marker points to an event outside of
+ //! the local timeline (in which case the estimation is made basing on
+ //! the data supplied by the homeserver as well as counters saved from
+ //! the previous run of the client).
+ bool isEstimate = true;
+
+ bool operator==(const EventStats& rhs) const& = default;
+
+ //! \brief Check whether the event statistics are empty
+ //!
+ //! Empty statistics have notable and highlight counters of zero and
+ //! isEstimate set to false.
+ Q_INVOKABLE bool empty() const
+ {
+ return notableCount == 0 && !isEstimate && highlightCount == 0;
+ }
+
+ using marker_t = Room::rev_iter_t;
+
+ //! \brief Build event statistics on a range of events
+ //!
+ //! This is a factory that returns an EventStats instance with counts of
+ //! notable and highlighted events between \p from and \p to reverse
+ //! timeline iterators; the \p init parameter allows to override
+ //! the initial statistics object and start from other values.
+ static EventStats fromRange(const Room* room, const marker_t& from,
+ const marker_t& to,
+ const EventStats& init = { 0, 0, false });
+
+ //! \brief Build event statistics on a range from sync edge to marker
+ //!
+ //! This is mainly a shortcut for \code
+ //! <tt>fromRange(room, marker_t(room->syncEdge()), marker)</tt>
+ //! \endcode except that it also sets isEstimate to true if (and only if)
+ //! <tt>to == room->historyEdge()</tt>.
+ static EventStats fromMarker(const Room* room, const marker_t& marker);
+
+ //! \brief Loads a statistics object from the cached counters
+ //!
+ //! Sets isEstimate to `true` unless both notableCount and highlightCount
+ //! are equal to -1.
+ static EventStats fromCachedCounters(Omittable<int> notableCount,
+ Omittable<int> highlightCount = none);
+
+ //! \brief Update statistics when a read marker moves down the timeline
+ //!
+ //! Removes events between oldMarker and newMarker from statistics
+ //! calculation if \p oldMarker points to an existing event in the timeline,
+ //! or recalculates the statistics entirely if \p oldMarker points
+ //! to <tt>room->historyEdge()</tt>. Always results in exact statistics
+ //! (<tt>isEstimate == false</tt>.
+ //! \param oldMarker Must point correspond to the _current_ statistics
+ //! isEstimate state, i.e. it should point to
+ //! <tt>room->historyEdge()</tt> if <tt>isEstimate == true</tt>, or
+ //! to a valid position within the timeline otherwise
+ //! \param newMarker Must point to a valid position in the timeline (not to
+ //! <tt>room->historyEdge()</tt> that is equal to or closer to
+ //! the sync edge than \p oldMarker
+ //! \return true if either notableCount or highlightCount changed, or if
+ //! the statistics was completely recalculated; false otherwise
+ bool updateOnMarkerMove(const Room* room, const marker_t& oldMarker,
+ const marker_t& newMarker);
+
+ //! \brief Validate the statistics object against the given marker
+ //!
+ //! Checks whether the statistics object data are valid for a given marker.
+ //! No stats recalculation takes place, only isEstimate and zero-ness
+ //! of notableCount are checked.
+ bool isValidFor(const Room* room, const marker_t& marker) const;
+};
+
+QDebug operator<<(QDebug dbg, const EventStats& es);
+
+}
diff --git a/lib/room.cpp b/lib/room.cpp
index 1df5dc71..8bad9084 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -15,6 +15,7 @@
#include "e2ee.h"
#include "syncdata.h"
#include "user.h"
+#include "eventstats.h"
// NB: since Qt 6, moc_room.cpp needs User fully defined
#include "moc_room.cpp"
@@ -118,13 +119,14 @@ public:
Avatar avatar;
QHash<QString, Notification> notifications;
qsizetype serverHighlightCount = 0;
- int notificationCount = 0;
+ // Starting up with estimate event statistics as there's zero knowledge
+ // about the timeline.
+ EventStats partiallyReadStats {}, unreadStats {};
members_map_t membersMap;
QList<User*> usersTyping;
QHash<QString, QSet<QString>> eventIdReadUsers;
QList<User*> usersInvited;
QList<User*> membersLeft;
- int unreadMessages = 0;
bool displayed = false;
QString firstDisplayedEventId;
QString lastDisplayedEventId;
@@ -267,6 +269,7 @@ public:
Changes addNewMessageEvents(RoomEvents&& events);
void addHistoricalMessageEvents(RoomEvents&& events);
+ Changes updateStatsFromSyncData(const SyncRoomData &data, bool fromCache);
void postprocessChanges(Changes changes, bool saveState = true);
/** Move events into the timeline
@@ -286,12 +289,12 @@ public:
*/
void dropDuplicateEvents(RoomEvents& events) const;
- bool setLastReadReceipt(const QString& userId, rev_iter_t newMarker,
- ReadReceipt newReceipt = {});
+ Changes setLastReadReceipt(const QString& userId, rev_iter_t newMarker,
+ ReadReceipt newReceipt = {},
+ bool deferStatsUpdate = false);
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);
+ Changes updateStats(const rev_iter_t& from, const rev_iter_t& to);
+ bool markMessagesAsRead(const rev_iter_t& upToMarker);
void getAllMembers();
@@ -613,9 +616,10 @@ void Room::setJoinState(JoinState state)
emit joinStateChanged(oldState, state);
}
-bool Room::Private::setLastReadReceipt(const QString& userId,
- rev_iter_t newMarker,
- ReadReceipt newReceipt)
+Room::Changes Room::Private::setLastReadReceipt(const QString& userId,
+ rev_iter_t newMarker,
+ ReadReceipt newReceipt,
+ bool deferStatsUpdate)
{
if (newMarker == historyEdge() && !newReceipt.eventId.isEmpty())
newMarker = q->findInTimeline(newReceipt.eventId);
@@ -643,10 +647,11 @@ bool Room::Private::setLastReadReceipt(const QString& userId,
const auto prevEventId = storedReceipt.eventId;
// NB: with reverse iterators, timeline history >= sync edge
if (newMarker >= q->findInTimeline(prevEventId))
- return false;
+ return Change::None;
// Finally make the change
+ Changes changes = Change::Other;
auto oldEventReadUsersIt =
eventIdReadUsers.find(prevEventId); // clazy:exclude=detaching-member
if (oldEventReadUsersIt != eventIdReadUsers.end()) {
@@ -663,21 +668,43 @@ bool Room::Private::setLastReadReceipt(const QString& userId,
// for actual members, not just any user
const auto member = q->user(userId);
Q_ASSERT(member != nullptr);
+ if (isLocalUser(member) && !deferStatsUpdate) {
+ if (unreadStats.updateOnMarkerMove(q, q->findInTimeline(prevEventId),
+ newMarker)) {
+ qCDebug(MESSAGES)
+ << "Updated unread event statistics in" << q->objectName()
+ << "after moving the local read receipt:" << unreadStats;
+ changes |= Change::UnreadStats;
+ }
+ Q_ASSERT(unreadStats.isValidFor(q, newMarker)); // post-check
+ }
emit q->lastReadEventChanged(member);
// TODO: remove in 0.8
if (!isLocalUser(member))
emit q->readMarkerForUserMoved(member, prevEventId,
storedReceipt.eventId);
- return true;
+ return changes;
}
-Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
- const rev_iter_t& to)
+Room::Changes Room::Private::updateStats(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());
- auto fullyReadMarker = q->fullyReadMarker();
+ const auto fullyReadMarker = q->fullyReadMarker();
+ auto readReceiptMarker = q->localReadReceiptMarker();
+ Changes changes = Change::None;
+ // Correct the read receipt to never be behind the fully read marker
+ if (readReceiptMarker > fullyReadMarker
+ && setLastReadReceipt(connection->userId(), fullyReadMarker, {}, true)) {
+ changes |= Change::Other;
+ readReceiptMarker = q->localReadReceiptMarker();
+ qCInfo(MESSAGES) << "The local m.read receipt was behind m.fully_read "
+ "marker - it's now corrected to be at index"
+ << readReceiptMarker->index();
+ }
+
if (fullyReadMarker < from)
return Change::None; // What's arrived is already fully read
@@ -685,79 +712,70 @@ Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
if (fullyReadMarker == historyEdge() && q->allHistoryLoaded())
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);
-
- // 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);
+ // Catch a case when the id in the last fully read marker or the local read
+ // receipt refers to an event that has just arrived. In this case either
+ // one (unreadStats) or both statistics should be recalculated to get
+ // an exact number instead of an estimation (see documentation on
+ // EventStats::isEstimate). For the same reason (switching from the
+ // estimate to the exact number) this branch forces returning
+ // Change::UnreadStats and also possibly Change::PartiallyReadStats, even if
+ // the estimation luckily matched the exact result.
+ if (readReceiptMarker < to || changes /*i.e. read receipt was corrected*/) {
+ unreadStats = EventStats::fromMarker(q, readReceiptMarker);
+ Q_ASSERT(!unreadStats.isEstimate);
+ qCDebug(MESSAGES).nospace() << "Recalculated unread event statistics in"
+ << q->objectName() << ": " << unreadStats;
+ changes |= Change::UnreadStats;
+ if (fullyReadMarker < to) {
+ // Add up to unreadStats instead of counting same events again
+ partiallyReadStats = EventStats::fromRange(q, readReceiptMarker,
+ q->fullyReadMarker(),
+ unreadStats);
+ Q_ASSERT(!partiallyReadStats.isEstimate);
+
+ qCDebug(MESSAGES).nospace()
+ << "Recalculated partially read event statistics in "
+ << q->objectName() << ": " << partiallyReadStats;
+ return Change::PartiallyReadStats | Change::UnreadStats;
+ }
+ }
- QElapsedTimer et;
- et.start();
- const auto newUnreadMessages =
- count_if(from, to,
- std::bind(&Room::isEventNotable, q, _1));
- if (et.nsecsElapsed() > profilerMinNsecs() / 10)
- qCDebug(PROFILER) << "Counting gained unread messages in"
- << q->objectName() << "took" << et;
-
- if (newUnreadMessages == 0)
- return Change::None;
+ // As of here, at least the fully read marker (but maybe also read receipt)
+ // points to somewhere beyond the "oldest" message from the arrived batch -
+ // add up newly arrived messages to the current stats, instead of a complete
+ // recalculation.
+ Q_ASSERT(fullyReadMarker >= to);
- // See https://github.com/quotient-im/libQuotient/wiki/unread_count
- if (unreadMessages < 0)
- unreadMessages = 0;
+ const auto newStats = EventStats::fromRange(q, from, to);
+ Q_ASSERT(!newStats.isEstimate);
+ if (newStats.notableCount == 0 || newStats.highlightCount == 0)
+ return changes;
- unreadMessages += newUnreadMessages;
- qCDebug(MESSAGES) << "Room" << q->objectName() << "has gained"
- << newUnreadMessages << "unread message(s),"
- << (q->fullyReadMarker() == timeline.crend()
- ? "in total at least"
- : "in total")
- << unreadMessages << "unread message(s)";
- emit q->unreadMessagesChanged(q);
- return Change::UnreadNotifs;
-}
+ const auto doAddStats = [this, &changes, newStats](EventStats& s,
+ const rev_iter_t& marker,
+ Change c) {
+ s.notableCount += newStats.notableCount;
+ s.highlightCount += newStats.highlightCount;
+ if (!s.isEstimate)
+ s.isEstimate = marker == historyEdge();
+ changes |= c;
+ };
-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->fullyReadMarker() != timeline.crend());
- const auto oldUnreadCount = unreadMessages;
- QElapsedTimer et;
- et.start();
- unreadMessages =
- int(count_if(timeline.crbegin(), q->fullyReadMarker(),
- [this](const auto& ti) { return q->isEventNotable(ti); }));
- if (et.nsecsElapsed() > profilerMinNsecs() / 10)
- qCDebug(PROFILER) << "Recounting unread messages in" << q->objectName()
- << "took" << et;
-
- // See https://github.com/quotient-im/libQuotient/wiki/unread_count
- if (unreadMessages == 0)
- unreadMessages = -1;
-
- if (!force && unreadMessages == oldUnreadCount)
- return Change::None;
+ doAddStats(partiallyReadStats, fullyReadMarker, Change::PartiallyReadStats);
+ if (readReceiptMarker >= to) {
+ // readReceiptMarker < to branch shouldn't have been entered
+ Q_ASSERT(!changes.testFlag(Change::UnreadStats));
+ doAddStats(unreadStats, readReceiptMarker, Change::UnreadStats);
+ }
+ qCDebug(MESSAGES) << "Room" << q->objectName() << "has gained" << newStats
+ << "notable/highlighted event(s); total statistics:"
+ << partiallyReadStats << "since the fully read marker,"
+ << unreadStats << "since read receipt";
- 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 Change::UnreadNotifs;
+ // Check invariants
+ Q_ASSERT(partiallyReadStats.isValidFor(q, fullyReadMarker));
+ Q_ASSERT(unreadStats.isValidFor(q, readReceiptMarker));
+ return changes;
}
Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
@@ -765,45 +783,59 @@ Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
if (fullyReadUntilEventId == eventId)
return Change::None;
+ const auto prevReadMarker = q->fullyReadMarker();
+ const auto newReadMarker = q->findInTimeline(eventId);
+ if (newReadMarker > prevReadMarker)
+ return Change::None;
+
const auto prevFullyReadId = std::exchange(fullyReadUntilEventId, eventId);
qCDebug(MESSAGES) << "Fully read marker in" << q->objectName() //
<< "set to" << fullyReadUntilEventId;
- emit q->fullyReadMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
- // TODO: Remove in 0.8
- emit q->readMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
- QT_IGNORE_DEPRECATIONS(Changes changes = Change::ReadMarker;)
+ QT_IGNORE_DEPRECATIONS(Changes changes = Change::ReadMarker|Change::Other;)
if (const auto rm = q->fullyReadMarker(); rm != historyEdge()) {
- // Pull read receipt if it's behind
- setLastReadReceipt(connection->userId(), rm);
- changes |= recalculateUnreadCount(); // TODO: updateUnreadCount()?
+ // Pull read receipt if it's behind, and update statistics
+ changes |= setLastReadReceipt(connection->userId(), rm);
+ if (partiallyReadStats.updateOnMarkerMove(q, prevReadMarker, rm)) {
+ changes |= Change::PartiallyReadStats;
+ qCDebug(MESSAGES)
+ << "Updated partially read event statistics in"
+ << q->objectName()
+ << "after moving m.fully_read marker: " << partiallyReadStats;
+ }
+ Q_ASSERT(partiallyReadStats.isValidFor(q, rm)); // post-check
}
+ emit q->fullyReadMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
+ // TODO: Remove in 0.8
+ emit q->readMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
return changes;
}
void Room::setReadReceipt(const QString& atEventId)
{
- if (!d->setLastReadReceipt(localUser()->id(), historyEdge(),
- { atEventId, QDateTime::currentDateTime() })) {
+ if (const auto changes = d->setLastReadReceipt(localUser()->id(),
+ historyEdge(),
+ { atEventId })) {
+ connection()->callApi<PostReceiptJob>(BackgroundRequest, id(),
+ QStringLiteral("m.read"),
+ QUrl::toPercentEncoding(atEventId));
+ d->postprocessChanges(changes);
+ } else
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));
}
bool Room::Private::markMessagesAsRead(const rev_iter_t &upToMarker)
{
- if (setFullyReadMarker(upToMarker->event()->id())) {
+ if (const auto changes = setFullyReadMarker(upToMarker->event()->id())) {
// The assumption below is that if a read receipt was sent on a newer
// event, the homeserver will keep it there instead of reverting to
// m.fully_read
connection->callApi<SetReadMarkerJob>(BackgroundRequest, id,
fullyReadUntilEventId,
fullyReadUntilEventId);
+ postprocessChanges(changes);
return true;
}
if (upToMarker != q->historyEdge())
@@ -863,9 +895,18 @@ Notification Room::checkForNotifications(const TimelineItem &ti)
return { Notification::None };
}
-bool Room::hasUnreadMessages() const { return unreadCount() >= 0; }
+bool Room::hasUnreadMessages() const { return !d->partiallyReadStats.empty(); }
-int Room::unreadCount() const { return d->unreadMessages; }
+int countFromStats(const EventStats& s)
+{
+ return s.empty() ? -1 : int(s.notableCount);
+}
+
+int Room::unreadCount() const { return countFromStats(partiallyReadStats()); }
+
+EventStats Room::partiallyReadStats() const { return d->partiallyReadStats; }
+
+EventStats Room::unreadStats() const { return d->unreadStats; }
Room::rev_iter_t Room::historyEdge() const { return d->historyEdge(); }
@@ -1071,13 +1112,16 @@ QSet<User*> Room::usersAtEventId(const QString& eventId)
return users;
}
-int Room::notificationCount() const { return d->notificationCount; }
+qsizetype Room::notificationCount() const
+{
+ return d->unreadStats.notableCount;
+}
void Room::resetNotificationCount()
{
- if (d->notificationCount == 0)
+ if (d->unreadStats.notableCount == 0)
return;
- d->notificationCount = 0;
+ d->unreadStats.notableCount = 0;
emit notificationCountChanged();
}
@@ -1652,6 +1696,72 @@ QUrl Room::memberAvatarUrl(const QString &mxId) const
: QUrl();
}
+Room::Changes Room::Private::updateStatsFromSyncData(const SyncRoomData& data,
+ bool fromCache)
+{
+ Changes changes {};
+ if (fromCache) {
+ // Initial load of cached statistics
+ partiallyReadStats =
+ EventStats::fromCachedCounters(data.partiallyReadCount);
+ unreadStats = EventStats::fromCachedCounters(data.unreadCount,
+ data.highlightCount);
+ // Migrate from lib 0.6: -1 in the old unread counter overrides 0
+ // (which loads to an estimate) in notification_count. Next caching will
+ // save -1 in both places, completing the migration.
+ if (data.unreadCount == 0 && data.partiallyReadCount == -1)
+ unreadStats.isEstimate = false;
+ changes |= Change::PartiallyReadStats | Change::UnreadStats;
+ qCDebug(MESSAGES) << "Loaded" << q->objectName()
+ << "event statistics from cache:" << partiallyReadStats
+ << "since m.fully_read," << unreadStats
+ << "since m.read";
+ } else if (timeline.empty()) {
+ // In absence of actual events use statistics from the homeserver
+ if (merge(unreadStats.notableCount, data.unreadCount))
+ changes |= Change::PartiallyReadStats;
+ if (merge(unreadStats.highlightCount, data.highlightCount))
+ changes |= Change::UnreadStats;
+ unreadStats.isEstimate = !data.unreadCount.has_value()
+ || *data.unreadCount > 0;
+ qCDebug(MESSAGES)
+ << "Using server-side unread event statistics while the"
+ << q->objectName() << "timeline is empty:" << unreadStats;
+ }
+ bool correctedStats = false;
+ if (unreadStats.highlightCount > partiallyReadStats.highlightCount) {
+ correctedStats = true;
+ partiallyReadStats.highlightCount = unreadStats.highlightCount;
+ partiallyReadStats.isEstimate |= unreadStats.isEstimate;
+ }
+ if (unreadStats.notableCount > partiallyReadStats.notableCount) {
+ correctedStats = true;
+ partiallyReadStats.notableCount = unreadStats.notableCount;
+ partiallyReadStats.isEstimate |= unreadStats.isEstimate;
+ }
+ if (!unreadStats.isEstimate && partiallyReadStats.isEstimate) {
+ correctedStats = true;
+ partiallyReadStats.isEstimate = true;
+ }
+ if (correctedStats)
+ qCDebug(MESSAGES) << "Partially read event statistics in"
+ << q->objectName() << "were adjusted to"
+ << partiallyReadStats
+ << "to be consistent with the m.read receipt";
+ Q_ASSERT(partiallyReadStats.isValidFor(q, q->fullyReadMarker()));
+ Q_ASSERT(unreadStats.isValidFor(q, q->localReadReceiptMarker()));
+
+ // TODO: Once the library learns to count highlights, drop
+ // serverHighlightCount and only use the server-side counter when
+ // the timeline is empty (see the code above).
+ if (merge(serverHighlightCount, data.highlightCount)) {
+ qCDebug(MESSAGES) << "Updated highlights number in" << q->objectName()
+ << "to" << serverHighlightCount;
+ changes |= Change::Highlights;
+ }
+ return changes;
+}
+
void Room::updateData(SyncRoomData&& data, bool fromCache)
{
if (d->prevBatch.isEmpty())
@@ -1670,25 +1780,14 @@ void Room::updateData(SyncRoomData&& data, bool fromCache)
for (auto&& event : data.accountData)
roomChanges |= processAccountDataEvent(move(event));
+ roomChanges |= d->updateStatsFromSyncData(data, fromCache);
+
if (roomChanges & Change::Topic)
emit topicChanged();
if (roomChanges & (Change::Name | Change::Aliases))
emit namesChanged(this);
- // See https://github.com/quotient-im/libQuotient/wiki/unread_count
- if (merge(d->unreadMessages, data.partiallyReadCount)) {
- qCDebug(MESSAGES) << "Loaded partially read count:"
- << *data.partiallyReadCount << "in" << objectName();
- emit unreadMessagesChanged(this);
- }
-
- if (merge(d->serverHighlightCount, data.highlightCount))
- emit highlightCountChanged();
-
- if (merge(d->notificationCount, data.unreadCount))
- emit notificationCountChanged();
-
d->postprocessChanges(roomChanges, !fromCache);
}
@@ -1704,6 +1803,17 @@ void Room::Private::postprocessChanges(Changes changes, bool saveState)
& (Change::Name | Change::Aliases | Change::Members | Change::Summary))
updateDisplayname();
+ if (changes & Change::PartiallyReadStats) {
+ emit q->unreadMessagesChanged(q); // TODO: remove in 0.8
+ emit q->partiallyReadStatsChanged();
+ }
+
+ if (changes & Change::UnreadStats)
+ emit q->unreadStatsChanged();
+
+ if (changes & Change::Highlights)
+ emit q->highlightCountChanged();
+
qCDebug(MAIN) << terse << changes << "in" << q->objectName();
emit q->changed(changes);
if (saveState)
@@ -2553,17 +2663,16 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events)
<< totalInserted << "new events; the last event is now"
<< timeline.back();
- const auto& firstWriterId = (*from)->senderId();
- setLastReadReceipt(firstWriterId, rev_iter_t(from + 1));
+ roomChanges |= updateStats(timeline.crbegin(), rev_iter_t(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.
+ const auto& firstWriterId = (*from)->senderId();
if (firstWriterId == connection->userId()
- && q->fullyReadMarker().base() == from) //
+ && q->fullyReadMarker().base() == from)
roomChanges |=
setFullyReadMarker(q->lastReadReceipt(firstWriterId).eventId);
-
- roomChanges |= updateUnreadCount(timeline.crbegin(), rev_iter_t(from));
}
Q_ASSERT(timeline.size() == timelineSize + totalInserted);
@@ -2612,17 +2721,12 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events)
emit q->updatedEvent(relation.eventId);
}
}
- changes |= updateUnreadCount(from, historyEdge());
- // 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/Developer-notes#2-saving-unread-event-counts).
- Q_ASSERT(unreadMessages != 0 || q->fullyReadMarker() == historyEdge());
-
Q_ASSERT(timeline.size() == timelineSize + insertedSize);
if (insertedSize > 9 || et.nsecsElapsed() >= profilerMinNsecs())
qCDebug(PROFILER) << "Added" << insertedSize << "historical event(s) to"
<< q->objectName() << "in" << et;
+ changes |= updateStats(from, historyEdge());
if (changes)
postprocessChanges(changes);
}
@@ -2860,8 +2964,9 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
totalReceipts += p.receipts.size();
const auto newMarker = findInTimeline(p.evtId);
if (newMarker == historyEdge())
- qCDebug(EPHEMERAL) << "Event of the read receipt(s) is not "
- "found; saving anyway";
+ qCDebug(EPHEMERAL)
+ << "Event" << p.evtId
+ << "is not found; saving read receipt(s) 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
@@ -2870,9 +2975,12 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
// the event is fetched later on.
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 });
+ [this, &changes, &newMarker, &evtId = p.evtId](const auto& r) {
+ const auto change =
+ d->setLastReadReceipt(r.userId, newMarker,
+ { evtId, r.timestamp });
+ changes |= change;
+ return change.testFlag(Change::Any);
});
if (p.receipts.size() > 1)
@@ -3098,15 +3206,11 @@ QJsonObject Room::Private::toJson() const
.fullJson() } } });
}
- QJsonObject unreadNotifObj { { PartiallyReadCountKey, unreadMessages } };
-
- if (serverHighlightCount > 0)
- unreadNotifObj.insert(HighlightCountKey, serverHighlightCount);
-
- result.insert(UnreadNotificationsKey, unreadNotifObj);
-
- if (notificationCount > 0)
- unreadNotifObj.insert(NewUnreadCountKey, notificationCount);
+ result.insert(UnreadNotificationsKey,
+ QJsonObject { { PartiallyReadCountKey,
+ countFromStats(partiallyReadStats) },
+ { HighlightCountKey, serverHighlightCount } });
+ result.insert(NewUnreadCountKey, countFromStats(unreadStats));
if (et.elapsed() > 30)
qCDebug(PROFILER) << "Room::toJson() for" << q->objectName() << "took"
diff --git a/lib/room.h b/lib/room.h
index c228f6c9..2f46e3a8 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -79,7 +79,7 @@ class ReadReceipt {
Q_PROPERTY(QDateTime timestamp MEMBER timestamp CONSTANT)
public:
QString eventId;
- QDateTime timestamp;
+ QDateTime timestamp = {};
bool operator==(const ReadReceipt& other) const
{
@@ -96,6 +96,8 @@ inline void swap(ReadReceipt& lhs, ReadReceipt& rhs)
swap(lhs.timestamp, rhs.timestamp);
}
+struct EventStats;
+
struct Notification
{
enum Type { None = 0, Basic, Highlight };
@@ -146,13 +148,18 @@ class Room : public QObject {
markMessagesAsRead NOTIFY readMarkerMoved)
Q_PROPERTY(QString lastFullyReadEventId READ lastFullyReadEventId WRITE
markMessagesAsRead NOTIFY fullyReadMarkerMoved)
+ //! \deprecated since 0.7
Q_PROPERTY(bool hasUnreadMessages READ hasUnreadMessages NOTIFY
- unreadMessagesChanged STORED false)
- Q_PROPERTY(int unreadCount READ unreadCount NOTIFY unreadMessagesChanged)
+ partiallyReadStatsChanged STORED false)
+ //! \deprecated since 0.7
+ Q_PROPERTY(int unreadCount READ unreadCount NOTIFY partiallyReadStatsChanged
+ STORED false)
Q_PROPERTY(qsizetype highlightCount READ highlightCount
NOTIFY highlightCountChanged)
Q_PROPERTY(qsizetype notificationCount READ notificationCount
NOTIFY notificationCountChanged)
+ Q_PROPERTY(EventStats partiallyReadStats READ partiallyReadStats NOTIFY partiallyReadStatsChanged)
+ Q_PROPERTY(EventStats unreadStats READ unreadStats NOTIFY unreadStatsChanged)
Q_PROPERTY(bool allHistoryLoaded READ allHistoryLoaded NOTIFY addedMessages
STORED false)
Q_PROPERTY(QStringList tagNames READ tagNames NOTIFY tagsChanged)
@@ -175,12 +182,13 @@ public:
Aliases = 0x2,
CanonicalAlias = Aliases,
Topic = 0x4,
- UnreadNotifs = 0x8,
+ PartiallyReadStats = 0x8,
+ DECL_DEPRECATED_ENUMERATOR(UnreadNotifs, PartiallyReadStats),
Avatar = 0x10,
JoinState = 0x20,
Tags = 0x40,
Members = 0x80,
- /* = 0x100, */
+ UnreadStats = 0x100,
AccountData Q_DECL_ENUMERATOR_DEPRECATED_X(
"Change::AccountData will be merged into Change::Other in 0.8") =
0x200,
@@ -188,6 +196,7 @@ public:
ReadMarker Q_DECL_ENUMERATOR_DEPRECATED_X(
"Change::ReadMarker will be merged into Change::Other in 0.8") =
0x800,
+ Highlights = 0x1000,
Other = 0x8000,
Any = 0xFFFF
};
@@ -509,6 +518,7 @@ public:
//! the original event usually is);
//! - from a non-local user (events from other devices of the local
//! user are not notable).
+ //! \sa partiallyReadStats, unreadStats
virtual bool isEventNotable(const TimelineItem& ti) const;
//! \brief Get notification details for an event
@@ -517,30 +527,84 @@ public:
//! generated for \p evt.
Notification notificationFor(const TimelineItem& ti) const;
- //! Check whether there are unread messages in the room
+ //! \brief Get event statistics since the fully read marker
+ //!
+ //! This call returns a structure containing:
+ //! - the number of notable unread events since the fully read marker;
+ //! depending on the fully read marker state with respect to the local
+ //! timeline, this number may be either exact or estimated
+ //! (see EventStats::isEstimate);
+ //! - the number of highlights (TODO).
+ //!
+ //! Note that this is different from the unread count defined by MSC2654
+ //! and from the notification/highlight numbers defined by the spec in that
+ //! it counts events since the fully read marker, not since the last
+ //! read receipt position.
+ //!
+ //! As E2EE is not supported in the library, the returned result will always
+ //! be an estimate (<tt>isEstimate == true</tt>) for encrypted rooms;
+ //! moreover, since the library doesn't know how to tackle push rules yet
+ //! the number of highlights returned here will always be zero (there's no
+ //! good substitute for that now).
+ //!
+ //! \sa isEventNotable, fullyReadMarker, unreadStats, EventStats
+ EventStats partiallyReadStats() const;
+
+ //! \brief Get event statistics since the last read receipt
+ //!
+ //! This call returns a structure that contains the following three numbers,
+ //! all counted on the timeline segment between the event pointed to by
+ //! the m.fully_read marker and the sync edge:
+ //! - the number of unread events - depending on the read receipt state
+ //! with respect to the local timeline, this number may be either precise
+ //! or estimated (see EventStats::isEstimate);
+ //! - the number of highlights (TODO).
+ //!
+ //! As E2EE is not supported in the library, the returned result will always
+ //! be an estimate (<tt>isEstimate == true</tt>) for encrypted rooms;
+ //! moreover, since the library doesn't know how to tackle push rules yet
+ //! the number of highlights returned here will always be zero - use
+ //! highlightCount() for now.
+ //!
+ //! \sa isEventNotable, lastLocalReadReceipt, partiallyReadStats,
+ //! highlightCount
+ EventStats unreadStats() const;
+
+ [[deprecated(
+ "Use partiallyReadStats/unreadStats() and EventStats::empty()")]]
bool hasUnreadMessages() const;
- /** Get the number of unread messages in the room
- * Depending on the read marker state, this call may return either
- * a precise or an estimate number of unread events. Only "notable"
- * events (non-redacted message events from users other than local)
- * are counted.
- *
- * In a case when readMarker() == historyEdge() (the local read
- * marker is beyond the local timeline) only the bottom limit of
- * the unread messages number can be estimated (and even that may
- * be slightly off due to, e.g., redactions of events not loaded
- * to the local timeline).
- *
- * If all messages are read, this function will return -1 (_not_ 0,
- * as zero may mean "zero or more unread messages" in a situation
- * when the read marker is outside the local timeline.
- */
+ //! \brief Get the number of notable events since the fully read marker
+ //!
+ //! \deprecated Since 0.7 there are two ways to count unread events: since
+ //! the fully read marker (used by libQuotient pre-0.7) and since the last
+ //! read receipt (as used by most of Matrix ecosystem, including the spec
+ //! and MSCs). This function currently returns a value derived from
+ //! partiallyReadStats() for compatibility with libQuotient 0.6; it will be
+ //! removed due to ambiguity. Use unreadStats() to obtain the spec-compliant
+ //! count of unread events and the highlight count; partiallyReadStats() to
+ //! obtain the unread events count since the fully read marker.
+ //!
+ //! \return -1 (_not 0_) when all messages are known to have been fully read,
+ //! i.e. the fully read marker points to _the latest notable_ event
+ //! loaded in the local timeline (which may be different from
+ //! the latest event in the local timeline as that might not be
+ //! notable);
+ //! 0 when there may be unread messages but the current local
+ //! timeline doesn't have any notable ones (often but not always
+ //! because it's entirely empty yet);
+ //! a positive integer when there is (or estimated to be) a number
+ //! of unread notable events as described above.
+ //!
+ //! \sa partiallyReadStats, unreadStats
+ [[deprecated("Use partiallyReadStats() or unreadStats() instead")]] //
int unreadCount() const;
//! \brief Get the number of notifications since the last read receipt
//!
- //! \sa lastLocalReadReceipt
+ //! This is the same as <tt>unreadStats().notableCount</tt>.
+ //!
+ //! \sa unreadStats, lastLocalReadReceipt
qsizetype notificationCount() const;
//! \deprecated Use setReadReceipt() to drive changes in notification count
@@ -550,6 +614,8 @@ public:
//!
//! As of 0.7, this is defined by the homeserver as Quotient doesn't process
//! push rules.
+ //!
+ //! \sa unreadStats, lastLocalReadReceipt
qsizetype highlightCount() const;
//! \deprecated Use setReadReceipt() to drive changes in highlightCount
@@ -878,7 +944,11 @@ Q_SIGNALS:
//! \deprecated since 0.7 - use lastReadEventChanged
void readMarkerForUserMoved(Quotient::User* user, QString fromEventId,
QString toEventId);
+ //! \deprecated since 0.7 - use either partiallyReadStatsChanged
+ //! or unreadStatsChanged
void unreadMessagesChanged(Quotient::Room* room);
+ void partiallyReadStatsChanged();
+ void unreadStatsChanged();
void accountDataAboutToChange(QString type);
void accountDataChanged(QString type);