From 429b3adaf921dd7534a04fccae1bc3c1801d2e40 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Fri, 26 Nov 2021 17:27:23 +0100 Subject: Room::processEphemeralEvents: Fix updatedCount always being zero Trying to test bits with Changes::testFlag(Change::Any) was a bad idea. Along the way: made logging in setLastReadReceipt() refer to the actual timeline item when possible. --- lib/room.cpp | 24 +++++++++++++++++------- lib/room.h | 7 ++++--- 2 files changed, 21 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index aa00025c..68095412 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -632,9 +632,9 @@ Room::Changes Room::Private::setLastReadReceipt(const QString& userId, }); // eagerMarker is now just after the desired event for newMarker if (eagerMarker != newMarker.base()) { - qCDebug(EPHEMERAL) << "Auto-promoted read receipt for" << userId - << "to" << newReceipt.eventId; newMarker = rev_iter_t(eagerMarker); + qCDebug(EPHEMERAL) << "Auto-promoted read receipt for" << userId + << "to" << *newMarker; } // Fill newReceipt with the event (and, if needed, timestamp) from // eagerMarker @@ -645,8 +645,11 @@ Room::Changes Room::Private::setLastReadReceipt(const QString& userId, auto& storedReceipt = lastReadReceipts[userId]; // clazy:exclude=detaching-member const auto prevEventId = storedReceipt.eventId; - // NB: with reverse iterators, timeline history >= sync edge - if (newMarker >= q->findInTimeline(prevEventId)) + // Check that either the new marker is actually "newer" than the current one + // or, if both markers are at historyEdge(), event ids are different. + // NB: with reverse iterators, timeline history edge >= sync edge + if (prevEventId == newReceipt.eventId + || newMarker > q->findInTimeline(prevEventId)) return Change::None; // Finally make the change @@ -661,8 +664,15 @@ Room::Changes Room::Private::setLastReadReceipt(const QString& userId, } eventIdReadUsers[newReceipt.eventId].insert(userId); storedReceipt = move(newReceipt); - qCDebug(EPHEMERAL) << "The new read receipt for" << userId << "is now at" - << storedReceipt.eventId; + + { + auto dbg = qDebug(EPHEMERAL); // This trick needs qDebug, not qCDebug + dbg << "The new read receipt for" << userId << "is now at"; + if (newMarker == historyEdge()) + dbg << storedReceipt.eventId; + else + dbg << *newMarker; + } // TODO: use Room::member() when it becomes a thing and only emit signals // for actual members, not just any user @@ -2984,7 +2994,7 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event) d->setLastReadReceipt(r.userId, newMarker, { evtId, r.timestamp }); changes |= change; - return change.testFlag(Change::Any); + return change & Change::Any; }); if (p.receipts.size() > 1) diff --git a/lib/room.h b/lib/room.h index 706fb17f..65217290 100644 --- a/lib/room.h +++ b/lib/room.h @@ -212,9 +212,10 @@ public: //! \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. + //! adding & Change::Any has the same meaning as + //! !testFlag(Change::None) or adding != Change::None + //! \note testFlag(Change::Any) tests that _all_ bits are on and + //! will always return false. Any = 0xFFFF }; QUO_DECLARE_FLAGS(Changes, Change) -- cgit v1.2.3