diff options
| -rw-r--r-- | CMakeLists.txt | 1 | ||||
| -rw-r--r-- | lib/eventstats.cpp | 98 | ||||
| -rw-r--r-- | lib/eventstats.h | 107 | ||||
| -rw-r--r-- | lib/room.cpp | 386 | ||||
| -rw-r--r-- | lib/room.h | 116 |
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" @@ -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); |