From f7a2e0f9885ecc622c67dd457993cb19c293f515 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 1 Aug 2021 17:17:41 +0200 Subject: SyncRoomData: distinguish between omitted and 0 unread counters This is a more conservative but less idiomatic.readable fix for entirely missing notification_count/highlight_count. In reality, Synapse seems to always send them; but that is not required by The Spec. --- lib/room.cpp | 14 ++++++++++++-- lib/syncdata.cpp | 8 ++------ 2 files changed, 14 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index b461b0a1..77abf04c 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1561,11 +1561,21 @@ void Room::updateData(SyncRoomData&& data, bool fromCache) emit unreadMessagesChanged(this); } - if (data.highlightCount != d->highlightCount) { + // Similar to unreadCount, SyncRoomData constructor assigns -1 to + // highlightCount/notificationCount when those are missing in the payload + if (data.highlightCount != -1 && data.highlightCount != d->highlightCount) { + qCDebug(MESSAGES).nospace() + << "Highlights in " << objectName() // + << ": " << d->highlightCount << " -> " << data.highlightCount; d->highlightCount = data.highlightCount; emit highlightCountChanged(); } - if (data.notificationCount != d->notificationCount) { + if (data.notificationCount != -1 + && data.notificationCount != d->notificationCount) // + { + qCDebug(MESSAGES).nospace() + << "Notifications in " << objectName() // + << ": " << d->notificationCount << " -> " << data.notificationCount; d->notificationCount = data.notificationCount; emit notificationCountChanged(); } diff --git a/lib/syncdata.cpp b/lib/syncdata.cpp index 64aa65fd..70c4a15f 100644 --- a/lib/syncdata.cpp +++ b/lib/syncdata.cpp @@ -106,12 +106,8 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, const auto unreadJson = room_.value("unread_notifications"_ls).toObject(); unreadCount = unreadJson.value(UnreadCountKey).toInt(-2); - highlightCount = unreadJson.value("highlight_count"_ls).toInt(); - notificationCount = unreadJson.value("notification_count"_ls).toInt(); - if (highlightCount > 0 || notificationCount > 0) - qCDebug(SYNCJOB) << "Room" << roomId_ - << "has highlights:" << highlightCount - << "and notifications:" << notificationCount; + highlightCount = unreadJson.value("highlight_count"_ls).toInt(-1); + notificationCount = unreadJson.value("notification_count"_ls).toInt(-1); } SyncData::SyncData(const QString& cacheFileName) -- cgit v1.2.3 From 000710d2c78b0843d920b7cf983f693a3ddf193e Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 1 Aug 2021 17:19:23 +0200 Subject: Enhanced logging for read receipts --- lib/room.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 77abf04c..a643ed68 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -649,7 +649,8 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker, return; // For Release builds } if (q->memberJoinState(u) != JoinState::Join) { - qCWarning(MAIN) << "Won't record read receipt for non-member" << u->id(); + qCWarning(EPHEMERAL) + << "Won't record read receipt for non-member" << u->id(); return; } @@ -657,8 +658,11 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker, newMarker = q->findInTimeline(newEvtId); if (newMarker != historyEdge()) { // NB: with reverse iterators, timeline history >= sync edge - if (newMarker >= q->readMarker(u)) + if (newMarker >= q->readMarker(u)) { + qCDebug(EPHEMERAL) << "The new read receipt for" << u->id() + << "is at or behind the old one, skipping"; return; + } // Try to auto-promote the read marker over the user's own messages // (switch to direct iterators for that). @@ -680,6 +684,8 @@ void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker, eventIdReadUsers.remove(storedId, u); eventIdReadUsers.insert(newEvtId, u); swap(storedId, newEvtId); // Now newEvtId actually stores the old eventId + qCDebug(EPHEMERAL) << "The new read receipt for" << u->id() << "is at" + << storedId; emit q->lastReadEventChanged(u); if (!isLocalUser(u)) emit q->readMarkerForUserMoved(u, newEvtId, storedId); -- cgit v1.2.3 From e4a8251d90c2e9f2847366085b3a2897a2d02201 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 1 Aug 2021 17:22:42 +0200 Subject: Room::toJson(): save the last local user's read receipt Read receipts are entangled with counting unread messages, and saving them also helps in not sending receipts for too old events. Other users' read receipts are still treated as truly ephemeral. --- lib/room.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index a643ed68..7242c32e 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -2923,6 +2923,31 @@ QJsonObject Room::Private::toJson() const { QStringLiteral("events"), accountDataEvents } }); } + if (const auto& readReceiptEventId = lastReadEventIds.value(q->localUser()); + !readReceiptEventId.isEmpty()) // + { + // Okay, that's a mouthful; but basically, it's simply placing an m.read + // event in the 'ephemeral' section of the cached sync payload. + // See also receiptevent.* and m.read example in the spec. + // Only the local user's read receipt is saved - others' are really + // considered ephemeral but this one is useful in understanding where + // the user is in the timeline before any history is loaded. + result.insert( + QStringLiteral("ephemeral"), + QJsonObject { + { QStringLiteral("events"), + QJsonArray { QJsonObject { + { TypeKey, ReceiptEvent::matrixTypeId() }, + { ContentKey, + QJsonObject { + { readReceiptEventId, + QJsonObject { + { QStringLiteral("m.read"), + QJsonObject { + { connection->userId(), + QJsonObject {} } } } } } } } } } } }); + } + QJsonObject unreadNotifObj { { SyncRoomData::UnreadCountKey, unreadMessages } }; -- cgit v1.2.3 From b17867c2b74916055adbc158cdf6437b27bceb14 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 1 Aug 2021 17:47:56 +0200 Subject: Room::setDisplayed(): Don't reset unread counters Closes #489. --- lib/room.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 7242c32e..029ff786 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -943,11 +943,8 @@ void Room::setDisplayed(bool displayed) d->displayed = displayed; emit displayedChanged(displayed); - if (displayed) { - resetHighlightCount(); - resetNotificationCount(); + if (displayed) d->getAllMembers(); - } } QString Room::firstDisplayedEventId() const { return d->firstDisplayedEventId; } -- cgit v1.2.3