From bf5f209d2d237301c65cc0973f1707b9386f3110 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 21 Nov 2021 06:55:09 +0100 Subject: Room: isEventNotable, notificationFor, checkForNotifications Room::isEventNotable has been moved out from Room::Private and made compliant with MSC2654. The concept of Room::checkForNotifications is taken from Quaternion where a method with the same name has been in QuaternionRoom for a long time - however, actual body is a stub for now, always returning { Notification::None } (Quaternion's implementation is too crude to be taken to the library). Now we really need a pushrules processor to fill this method with something reasonably good. Internally the library now calls checkForNotifications() on every event added to the timeline, filling up the events-to-notifications map because it is anticipated that calculation of notifications can be rather resource-intensive and should only be done once for a given event. Finally, Room::notificationsFor is an accessor into the mentioned map, standing next to isEventNotable (but unlike isEventNotable, it's not virtual; checkForNotifications is). --- lib/room.h | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'lib/room.h') diff --git a/lib/room.h b/lib/room.h index 8a544e82..124c8cb4 100644 --- a/lib/room.h +++ b/lib/room.h @@ -96,6 +96,18 @@ inline void swap(ReadReceipt& lhs, ReadReceipt& rhs) swap(lhs.timestamp, rhs.timestamp); } +struct Notification +{ + enum Type { None = 0, Basic, Highlight }; + Q_ENUM(Notification) + + Type type = None; + +private: + Q_GADGET + Q_PROPERTY(Type type MEMBER type CONSTANT) +}; + class Room : public QObject { Q_OBJECT Q_PROPERTY(Connection* connection READ connection CONSTANT) @@ -482,6 +494,29 @@ public: //! \sa markAllMessagesAsRead, fullyReadMarker Q_INVOKABLE void markMessagesAsRead(QString uptoEventId); + //! \brief Determine whether an event should be counted as unread + //! + //! The criteria of including an event in unread counters are described in + //! [MSC2654](https://github.com/matrix-org/matrix-doc/pull/2654); according + //! to these, the event should be counted as unread (or, in libQuotient + //! parlance, is "notable") if it is: + //! - either + //! - a message event that is not m.notice, or + //! - a state event with type being one of: + //! `m.room.topic`, `m.room.name`, `m.room.avatar`, `m.room.tombstone`; + //! - neither redacted, nor an edit (redactions cause the redacted event + //! to stop being notable, while edits are not notable themselves while + //! the original event usually is); + //! - from a non-local user (events from other devices of the local + //! user are not notable). + virtual bool isEventNotable(const TimelineItem& ti) const; + + //! \brief Get notification details for an event + //! + //! This allows to get details on the kind of notification that should + //! generated for \p evt. + Notification notificationFor(const TimelineItem& ti) const; + //! Check whether there are unread messages in the room bool hasUnreadMessages() const; @@ -873,6 +908,7 @@ protected: {} virtual QJsonObject toJson() const; virtual void updateData(SyncRoomData&& data, bool fromCache = false); + virtual Notification checkForNotifications(const TimelineItem& ti); private: friend class Connection; -- cgit v1.2.3 From 96f3daf7a2c4ec875904c11350c93612265e2eed Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 21 Nov 2021 06:03:49 +0100 Subject: SyncData: support MSC2654; partiallyReadCount Since MSC2654's unread count is counted from the m.read receipt, and the course is to follow the spec's terminology and use "unread count" for the number of notable events since m.read, this required to move the existing number of notable events since m.fully_read to another field, henceforth called partiallyReadCount. At the same time, SyncData::notificationCount is dropped completely since MSC2654 claims to supersede it. Also: Room::resetNotificationCount() and Room::resetHighlightCount() are deprecated, as these never worked properly overwriting values that can be calculated or sourced from the server, only for these values to be set back again the next time the room is updated from /sync. --- lib/room.cpp | 33 ++++++++++++++++----------------- lib/room.h | 24 ++++++++++++++++++------ lib/syncdata.cpp | 44 ++++++++++++++++++++++---------------------- lib/syncdata.h | 15 +++++++++------ 4 files changed, 65 insertions(+), 51 deletions(-) (limited to 'lib/room.h') diff --git a/lib/room.cpp b/lib/room.cpp index 67f65472..bc686962 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -117,7 +117,7 @@ public: QString displayname; Avatar avatar; QHash notifications; - int highlightCount = 0; + qsizetype serverHighlightCount = 0; int notificationCount = 0; members_map_t membersMap; QList usersTyping; @@ -1072,13 +1072,13 @@ void Room::resetNotificationCount() emit notificationCountChanged(); } -int Room::highlightCount() const { return d->highlightCount; } +qsizetype Room::highlightCount() const { return d->serverHighlightCount; } void Room::resetHighlightCount() { - if (d->highlightCount == 0) + if (d->serverHighlightCount == 0) return; - d->highlightCount = 0; + d->serverHighlightCount = 0; emit highlightCountChanged(); } @@ -1673,16 +1673,16 @@ void Room::updateData(SyncRoomData&& data, bool fromCache) roomChanges |= processEphemeralEvent(move(ephemeralEvent)); // See https://github.com/quotient-im/libQuotient/wiki/unread_count - if (merge(d->unreadMessages, data.unreadCount)) { - qCDebug(MESSAGES) << "Loaded unread_count:" << *data.unreadCount // - << "in" << objectName(); + if (merge(d->unreadMessages, data.partiallyReadCount)) { + qCDebug(MESSAGES) << "Loaded partially read count:" + << *data.partiallyReadCount << "in" << objectName(); emit unreadMessagesChanged(this); } - if (merge(d->highlightCount, data.highlightCount)) + if (merge(d->serverHighlightCount, data.highlightCount)) emit highlightCountChanged(); - if (merge(d->notificationCount, data.notificationCount)) + if (merge(d->notificationCount, data.unreadCount)) emit notificationCountChanged(); if (roomChanges) { @@ -3077,16 +3077,15 @@ QJsonObject Room::Private::toJson() const .fullJson() } } }); } - QJsonObject unreadNotifObj { { SyncRoomData::UnreadCountKey, - unreadMessages } }; + QJsonObject unreadNotifObj { { PartiallyReadCountKey, unreadMessages } }; - if (highlightCount > 0) - unreadNotifObj.insert(QStringLiteral("highlight_count"), highlightCount); - if (notificationCount > 0) - unreadNotifObj.insert(QStringLiteral("notification_count"), - notificationCount); + if (serverHighlightCount > 0) + unreadNotifObj.insert(HighlightCountKey, serverHighlightCount); + + result.insert(UnreadNotificationsKey, unreadNotifObj); - result.insert(QStringLiteral("unread_notifications"), unreadNotifObj); + if (notificationCount > 0) + unreadNotifObj.insert(NewUnreadCountKey, notificationCount); if (et.elapsed() > 30) qCDebug(PROFILER) << "Room::toJson() for" << q->objectName() << "took" diff --git a/lib/room.h b/lib/room.h index 124c8cb4..a75311fb 100644 --- a/lib/room.h +++ b/lib/room.h @@ -149,10 +149,10 @@ class Room : public QObject { Q_PROPERTY(bool hasUnreadMessages READ hasUnreadMessages NOTIFY unreadMessagesChanged STORED false) Q_PROPERTY(int unreadCount READ unreadCount NOTIFY unreadMessagesChanged) - Q_PROPERTY(int highlightCount READ highlightCount NOTIFY - highlightCountChanged RESET resetHighlightCount) - Q_PROPERTY(int notificationCount READ notificationCount NOTIFY - notificationCountChanged RESET resetNotificationCount) + Q_PROPERTY(qsizetype highlightCount READ highlightCount + NOTIFY highlightCountChanged) + Q_PROPERTY(qsizetype notificationCount READ notificationCount + NOTIFY notificationCountChanged) Q_PROPERTY(bool allHistoryLoaded READ allHistoryLoaded NOTIFY addedMessages STORED false) Q_PROPERTY(QStringList tagNames READ tagNames NOTIFY tagsChanged) @@ -538,9 +538,21 @@ public: */ int unreadCount() const; - Q_INVOKABLE int notificationCount() const; + //! \brief Get the number of notifications since the last read receipt + //! + //! \sa lastLocalReadReceipt + qsizetype notificationCount() const; + + //! \deprecated Use setReadReceipt() to drive changes in notification count Q_INVOKABLE void resetNotificationCount(); - Q_INVOKABLE int highlightCount() const; + + //! \brief Get the number of highlights since the last read receipt + //! + //! As of 0.7, this is defined by the homeserver as Quotient doesn't process + //! push rules. + qsizetype highlightCount() const; + + //! \deprecated Use setReadReceipt() to drive changes in highlightCount Q_INVOKABLE void resetHighlightCount(); /** Check whether the room has account data of the given type diff --git a/lib/syncdata.cpp b/lib/syncdata.cpp index e86d3100..396e77eb 100644 --- a/lib/syncdata.cpp +++ b/lib/syncdata.cpp @@ -10,9 +10,6 @@ using namespace Quotient; -const QString SyncRoomData::UnreadCountKey = - QStringLiteral("x-quotient.unread_count"); - bool RoomSummary::isEmpty() const { return !joinedMemberCount && !invitedMemberCount && !heroes; @@ -64,23 +61,23 @@ inline EventsArrayT load(const QJsonObject& batches, StrT keyName) return fromJson(batches[keyName].toObject().value("events"_ls)); } -SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, - const QJsonObject& room_) - : roomId(roomId_) - , joinState(joinState_) - , summary(fromJson(room_["summary"_ls])) - , state(load(room_, joinState == JoinState::Invite +SyncRoomData::SyncRoomData(QString roomId_, JoinState joinState, + const QJsonObject& roomJson) + : roomId(std::move(roomId_)) + , joinState(joinState) + , summary(fromJson(roomJson["summary"_ls])) + , state(load(roomJson, joinState == JoinState::Invite ? "invite_state"_ls : "state"_ls)) { switch (joinState) { case JoinState::Join: - ephemeral = load(room_, "ephemeral"_ls); + ephemeral = load(roomJson, "ephemeral"_ls); [[fallthrough]]; case JoinState::Leave: { - accountData = load(room_, "account_data"_ls); - timeline = load(room_, "timeline"_ls); - const auto timelineJson = room_.value("timeline"_ls).toObject(); + accountData = load(roomJson, "account_data"_ls); + timeline = load(roomJson, "timeline"_ls); + const auto timelineJson = roomJson.value("timeline"_ls).toObject(); timelineLimited = timelineJson.value("limited"_ls).toBool(); timelinePrevBatch = timelineJson.value("prev_batch"_ls).toString(); @@ -89,14 +86,17 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, default: /* nothing on top of state */; } - const auto unreadJson = room_.value("unread_notifications"_ls).toObject(); - fromJson(unreadJson.value(UnreadCountKey), unreadCount); - fromJson(unreadJson.value("highlight_count"_ls), highlightCount); - fromJson(unreadJson.value("notification_count"_ls), notificationCount); - if (highlightCount.has_value() || notificationCount.has_value()) - qCDebug(SYNCJOB) << "Room" << roomId_ - << "has highlights:" << *highlightCount - << "and notifications:" << *notificationCount; + const auto unreadJson = roomJson.value(UnreadNotificationsKey).toObject(); + + fromJson(unreadJson.value(PartiallyReadCountKey), partiallyReadCount); + if (!partiallyReadCount.has_value()) + fromJson(unreadJson.value("x-quotient.unread_count"_ls), + partiallyReadCount); + + fromJson(roomJson.value(NewUnreadCountKey), unreadCount); + if (!unreadCount.has_value()) + fromJson(unreadJson.value("notification_count"_ls), unreadCount); + fromJson(unreadJson.value(HighlightCountKey), highlightCount); } SyncData::SyncData(const QString& cacheFileName) @@ -130,7 +130,7 @@ Events&& SyncData::takeToDeviceEvents() { return std::move(toDeviceEvents); } std::pair SyncData::cacheVersion() { - return { MajorCacheVersion, 1 }; + return { MajorCacheVersion, 2 }; } QJsonObject SyncData::loadJson(const QString& fileName) diff --git a/lib/syncdata.h b/lib/syncdata.h index b869a541..36d2e0bf 100644 --- a/lib/syncdata.h +++ b/lib/syncdata.h @@ -8,6 +8,12 @@ #include "events/stateevent.h" namespace Quotient { + +constexpr auto UnreadNotificationsKey = "unread_notifications"_ls; +constexpr auto PartiallyReadCountKey = "x-quotient.since_fully_read_count"_ls; +constexpr auto NewUnreadCountKey = "org.matrix.msc2654.unread_count"_ls; +constexpr auto HighlightCountKey = "highlight_count"_ls; + /// Room summary, as defined in MSC688 /** * Every member of this structure is an Omittable; as per the MSC, only @@ -29,7 +35,6 @@ struct RoomSummary { }; QDebug operator<<(QDebug dbg, const RoomSummary& rs); - template <> struct JsonObjectConverter { static void dumpTo(QJsonObject& jo, const RoomSummary& rs); @@ -48,16 +53,14 @@ public: bool timelineLimited; QString timelinePrevBatch; + Omittable partiallyReadCount; Omittable unreadCount; Omittable highlightCount; - Omittable notificationCount; - SyncRoomData(const QString& roomId, JoinState joinState_, - const QJsonObject& room_); + SyncRoomData(QString roomId, JoinState joinState, + const QJsonObject& roomJson); SyncRoomData(SyncRoomData&&) = default; SyncRoomData& operator=(SyncRoomData&&) = default; - - static const QString UnreadCountKey; }; // QVector cannot work with non-copyable objects, std::vector can. -- cgit v1.2.3 From e7babe6715672a358f7cc8b90d5df27e21a1b3e8 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 21 Nov 2021 07:03:48 +0100 Subject: Room::Private::postprocessChanges() This makes updating display name and emission of necessary signals including Room::changed() more systematic when it has to occur outside of updateData() flow - e.g. when loading all members. --- lib/room.cpp | 85 +++++++++++++++++++++++++++++++++++++----------------------- lib/room.h | 2 +- 2 files changed, 54 insertions(+), 33 deletions(-) (limited to 'lib/room.h') diff --git a/lib/room.cpp b/lib/room.cpp index bc686962..1df5dc71 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -267,6 +267,8 @@ public: Changes addNewMessageEvents(RoomEvents&& events); void addHistoricalMessageEvents(RoomEvents&& events); + void postprocessChanges(Changes changes, bool saveState = true); + /** Move events into the timeline * * Insert events into the timeline, either new or historical. @@ -793,28 +795,36 @@ void Room::setReadReceipt(const QString& atEventId) QUrl::toPercentEncoding(atEventId)); } -void Room::Private::markMessagesAsRead(const rev_iter_t &upToMarker) +bool Room::Private::markMessagesAsRead(const rev_iter_t &upToMarker) { - 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 - // m.fully_read to m.read + if (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(BackgroundRequest, id, fullyReadUntilEventId, fullyReadUntilEventId); + return true; } + if (upToMarker != q->historyEdge()) + qCDebug(MESSAGES) << "Event" << *upToMarker << "in" << q->objectName() + << "is behind the current fully read marker at" + << *q->fullyReadMarker() + << "- won't move fully read marker back in timeline"; + else + qCWarning(MESSAGES) << "Cannot mark an unknown event in" + << q->objectName() << "as fully read"; + return false; } -void Room::markMessagesAsRead(QString uptoEventId) +void Room::markMessagesAsRead(const QString& uptoEventId) { d->markMessagesAsRead(findInTimeline(uptoEventId)); } void Room::markAllMessagesAsRead() { - if (!d->timeline.empty()) - d->markMessagesAsRead(d->timeline.crbegin()); + d->markMessagesAsRead(d->timeline.crbegin()); } bool Room::canSwitchVersions() const @@ -941,8 +951,7 @@ void Room::Private::getAllMembers() it != syncEdge(); ++it) if (is(**it)) roomChanges |= q->processStateEvent(**it); - if (roomChanges & Change::Members) - emit q->memberListChanged(); + postprocessChanges(roomChanges); emit q->allMembersLoaded(); }); } @@ -1459,7 +1468,6 @@ Room::Changes Room::Private::setSummary(RoomSummary&& newSummary) return Change::None; qCDebug(STATE).nospace().noquote() << "Updated room summary for " << q->objectName() << ": " << summary; - emit q->memberListChanged(); return Change::Summary; } @@ -1651,27 +1659,23 @@ void Room::updateData(SyncRoomData&& data, bool fromCache) setJoinState(data.joinState); Changes roomChanges {}; - for (auto&& event : data.accountData) - roomChanges |= processAccountDataEvent(move(event)); - + // The order of calculation is important - don't merge the lines! roomChanges |= d->updateStateFrom(data.state); - // The order of calculation is important - don't merge these lines! + roomChanges |= d->setSummary(move(data.summary)); roomChanges |= d->addNewMessageEvents(move(data.timeline)); + for (auto&& ephemeralEvent : data.ephemeral) + roomChanges |= processEphemeralEvent(move(ephemeralEvent)); + + for (auto&& event : data.accountData) + roomChanges |= processAccountDataEvent(move(event)); + if (roomChanges & Change::Topic) emit topicChanged(); if (roomChanges & (Change::Name | Change::Aliases)) emit namesChanged(this); - if (roomChanges & Change::Members) - emit memberListChanged(); - - roomChanges |= d->setSummary(move(data.summary)); - - for (auto&& ephemeralEvent : data.ephemeral) - roomChanges |= processEphemeralEvent(move(ephemeralEvent)); - // See https://github.com/quotient-im/libQuotient/wiki/unread_count if (merge(d->unreadMessages, data.partiallyReadCount)) { qCDebug(MESSAGES) << "Loaded partially read count:" @@ -1685,12 +1689,25 @@ void Room::updateData(SyncRoomData&& data, bool fromCache) if (merge(d->notificationCount, data.unreadCount)) emit notificationCountChanged(); - if (roomChanges) { - d->updateDisplayname(); - emit changed(roomChanges); - if (!fromCache) - connection()->saveRoomState(this); - } + d->postprocessChanges(roomChanges, !fromCache); +} + +void Room::Private::postprocessChanges(Changes changes, bool saveState) +{ + if (!changes) + return; + + if (changes & Change::Members) + emit q->memberListChanged(); + + if (changes + & (Change::Name | Change::Aliases | Change::Members | Change::Summary)) + updateDisplayname(); + + qCDebug(MAIN) << terse << changes << "in" << q->objectName(); + emit q->changed(changes); + if (saveState) + connection->saveRoomState(q); } RoomEvent* Room::Private::addAsPending(RoomEventPtr&& event) @@ -2566,6 +2583,7 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events) if (events.empty()) return; + Changes changes {}; // In case of lazy-loading new members may be loaded with historical // messages. Also, the cache doesn't store events with empty content; // so when such events show up in the timeline they should be properly @@ -2574,7 +2592,7 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events) const auto& e = *eptr; if (e.isStateEvent() && !currentState.contains({ e.matrixType(), e.stateKey() })) { - q->processStateEvent(e); + changes |= q->processStateEvent(e); } } @@ -2594,7 +2612,7 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events) emit q->updatedEvent(relation.eventId); } } - updateUnreadCount(from, historyEdge()); + 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). @@ -2604,6 +2622,9 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events) if (insertedSize > 9 || et.nsecsElapsed() >= profilerMinNsecs()) qCDebug(PROFILER) << "Added" << insertedSize << "historical event(s) to" << q->objectName() << "in" << et; + + if (changes) + postprocessChanges(changes); } Room::Changes Room::processStateEvent(const RoomEvent& e) diff --git a/lib/room.h b/lib/room.h index a75311fb..c228f6c9 100644 --- a/lib/room.h +++ b/lib/room.h @@ -492,7 +492,7 @@ public: //! 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); + Q_INVOKABLE void markMessagesAsRead(const QString& uptoEventId); //! \brief Determine whether an event should be counted as unread //! -- cgit v1.2.3 From b2f9b212c78bc9dd7c69f6a2d1f94376adb488e3 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 21 Nov 2021 06:55:16 +0100 Subject: 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. --- CMakeLists.txt | 1 + lib/eventstats.cpp | 98 ++++++++++++++ lib/eventstats.h | 107 +++++++++++++++ lib/room.cpp | 386 ++++++++++++++++++++++++++++++++++------------------- lib/room.h | 116 ++++++++++++---- 5 files changed, 544 insertions(+), 164 deletions(-) create mode 100644 lib/eventstats.cpp create mode 100644 lib/eventstats.h (limited to 'lib/room.h') 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 notableCount, + Omittable 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 + //! fromRange(room, marker_t(room->syncEdge()), marker) + //! \endcode except that it also sets isEstimate to true if (and only if) + //! to == room->historyEdge(). + 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 notableCount, + Omittable 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 room->historyEdge(). Always results in exact statistics + //! (isEstimate == false. + //! \param oldMarker Must point correspond to the _current_ statistics + //! isEstimate state, i.e. it should point to + //! room->historyEdge() if isEstimate == true, or + //! to a valid position within the timeline otherwise + //! \param newMarker Must point to a valid position in the timeline (not to + //! room->historyEdge() 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 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 usersTyping; QHash> eventIdReadUsers; QList usersInvited; QList 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(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(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(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 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 (isEstimate == true) 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 (isEstimate == true) 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 unreadStats().notableCount. + //! + //! \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); -- cgit v1.2.3 From 9e5c9ff3ef0eeff8abd0bc3508d9152506e27e30 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sat, 20 Nov 2021 11:42:53 +0100 Subject: Document Room::Change and its enumerators --- lib/room.h | 45 ++++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) (limited to 'lib/room.h') diff --git a/lib/room.h b/lib/room.h index 2f46e3a8..706fb17f 100644 --- a/lib/room.h +++ b/lib/room.h @@ -176,28 +176,45 @@ public: using rev_iter_t = Timeline::const_reverse_iterator; using timeline_iter_t = Timeline::const_iterator; + //! \brief Room changes that can be tracked using Room::changed() signal + //! + //! This enumeration lists kinds of changes that can be tracked with + //! a "cumulative" changed() signal instead of using individual signals for + //! each change. Specific enumerators mention these individual signals. + //! \sa changed enum class Change : uint { - None = 0x0, - Name = 0x1, - Aliases = 0x2, + None = 0x0, //< No changes occurred in the room + Name = 0x1, //< \sa namesChanged, displaynameChanged + Aliases = 0x2, //< \sa namesChanged, displaynameChanged CanonicalAlias = Aliases, - Topic = 0x4, - PartiallyReadStats = 0x8, + Topic = 0x4, //< \sa topicChanged + PartiallyReadStats = 0x8, //< \sa partiallyReadStatsChanged DECL_DEPRECATED_ENUMERATOR(UnreadNotifs, PartiallyReadStats), - Avatar = 0x10, - JoinState = 0x20, - Tags = 0x40, + Avatar = 0x10, //< \sa avatarChanged + JoinState = 0x20, //< \sa joinStateChanged + Tags = 0x40, //< \sa tagsChanged + //! \sa userAdded, userRemoved, memberRenamed, memberListChanged, + //! displaynameChanged Members = 0x80, - UnreadStats = 0x100, + UnreadStats = 0x100, //< \sa unreadStatsChanged AccountData Q_DECL_ENUMERATOR_DEPRECATED_X( "Change::AccountData will be merged into Change::Other in 0.8") = 0x200, - Summary = 0x400, + Summary = 0x400, //< \sa summaryChanged, displaynameChanged ReadMarker Q_DECL_ENUMERATOR_DEPRECATED_X( "Change::ReadMarker will be merged into Change::Other in 0.8") = 0x800, - Highlights = 0x1000, + Highlights = 0x1000, //< \sa highlightCountChanged + //! A catch-all value that covers changes not listed above (such as + //! encryption turned on or the room having been upgraded), as well as + //! changes in the room state that the library is not aware of (e.g., + //! custom state events) and m.read/m.fully_read position changes. + //! \sa encryptionChanged, upgraded, accountDataChanged Other = 0x8000, + //! This is intended to test a Change/Changes value for non-emptiness; + //! testFlag(Change::Any) or adding & Change::Any has + //! the same meaning as !testFlag(Change::None) or adding + //! != Change::None. Any = 0xFFFF }; QUO_DECLARE_FLAGS(Changes, Change) @@ -931,12 +948,14 @@ Q_SIGNALS: Quotient::JoinState newState); void typingChanged(); - void highlightCountChanged(); - void notificationCountChanged(); + void highlightCountChanged(); //< \sa highlightCount + void notificationCountChanged(); //< \sa notificationCount void displayedChanged(bool displayed); void firstDisplayedEventChanged(); void lastDisplayedEventChanged(); + //! The event that m.read receipt points to has changed + //! \sa lastReadReceipt void lastReadEventChanged(Quotient::User* user); void fullyReadMarkerMoved(QString fromEventId, QString toEventId); //! \deprecated since 0.7 - use fullyReadMarkerMoved -- cgit v1.2.3