-
Notifications
You must be signed in to change notification settings - Fork 384
feat(llc): Implement local unread counts #2804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1852,9 +1852,17 @@ class Channel { | |
| /// | ||
| /// Optionally provide a [messageId] if you want to mark channel as | ||
| /// read from a particular message onwards. | ||
| /// | ||
| /// If [usesLocalUnreadCount] is `true` for this channel, this updates the | ||
| /// unread count locally, on-device, without making a network request. | ||
| Future<EmptyResponse> markRead({String? messageId}) async { | ||
| _checkInitialized(); | ||
|
|
||
| if (usesLocalUnreadCount) { | ||
| state!.markReadLocally(messageId: messageId); | ||
| return EmptyResponse(); | ||
| } | ||
|
|
||
| if (!canUseReadReceipts) { | ||
| throw const StreamChatError( | ||
| 'Cannot mark as read: Channel does not support read events. ' | ||
|
|
@@ -1867,10 +1875,43 @@ class Channel { | |
|
|
||
| /// Marks the channel as unread by a given [messageId]. | ||
| /// | ||
| /// All messages from the provided message onwards will be marked as unread. | ||
| /// All messages from the provided message onwards will be marked as unread, | ||
| /// **including** the message itself. Contrast with | ||
| /// [markUnreadByTimestamp], which is exclusive: a message created at | ||
| /// exactly the given timestamp stays read. | ||
| /// | ||
| /// If [usesLocalUnreadCount] is `true` for this channel, this updates the | ||
| /// unread count locally, on-device, without making a network request. The | ||
| /// message must be part of the locally-known messages ([Channel.messages]) | ||
| /// for the count to be recomputed. | ||
| Future<EmptyResponse> markUnread(String messageId) async { | ||
| _checkInitialized(); | ||
|
|
||
| if (usesLocalUnreadCount) { | ||
| // [ChannelClientState.messages] is sorted ascending by `createdAt`, so | ||
| // the entry before the anchor is the newest message that stays read. | ||
| final messages = state!.messages; | ||
| final anchorIndex = messages.indexWhere((it) => it.id == messageId); | ||
| if (anchorIndex < 0) { | ||
| throw StreamChatError( | ||
| 'Cannot mark as unread: Message "$messageId" was not found in the ' | ||
| 'locally-known messages for this channel.', | ||
| ); | ||
| } | ||
|
|
||
| final anchor = messages[anchorIndex]; | ||
| final previous = anchorIndex > 0 ? messages[anchorIndex - 1] : null; | ||
|
|
||
| // Subtract a microsecond so the anchor message itself is treated as | ||
| // "after" the new read boundary, matching the "from the provided | ||
| // message onwards" semantics described above. Preferred over using | ||
| // `previous.createdAt` as the boundary, which would leak the anchor | ||
| // back into the read set if the two share an identical `createdAt`. | ||
| final lastRead = anchor.createdAt.subtract(const Duration(microseconds: 1)); | ||
| state!.markUnreadLocally(lastRead: lastRead, lastReadMessageId: previous?.id); | ||
| return EmptyResponse(); | ||
| } | ||
|
|
||
| if (!canUseReadReceipts) { | ||
| throw const StreamChatError( | ||
| 'Cannot mark as unread: Channel does not support read events. ' | ||
|
|
@@ -1883,10 +1924,32 @@ class Channel { | |
|
|
||
| /// Marks the channel as unread by a given [timestamp]. | ||
| /// | ||
| /// All messages after the provided timestamp will be marked as unread. | ||
| /// All messages after the provided timestamp will be marked as unread. This | ||
| /// boundary is **exclusive**: a message created at exactly [timestamp] stays | ||
| /// read. Contrast with [markUnread], which is inclusive of the message it is | ||
| /// given — `markUnread(m.id)` is equivalent to | ||
| /// `markUnreadByTimestamp(m.createdAt - 1µs)`, not to | ||
| /// `markUnreadByTimestamp(m.createdAt)`. | ||
| /// | ||
| /// If [usesLocalUnreadCount] is `true` for this channel, this updates the | ||
| /// unread count locally, on-device, without making a network request. | ||
| Future<EmptyResponse> markUnreadByTimestamp(DateTime timestamp) async { | ||
| _checkInitialized(); | ||
|
|
||
| if (usesLocalUnreadCount) { | ||
| // The newest locally-known message at or before the boundary is the last | ||
| // one that stays read. | ||
| final lastReadMessage = state!.messages.lastWhereOrNull( | ||
| (it) => !it.createdAt.isAfter(timestamp), | ||
| ); | ||
|
|
||
| state!.markUnreadLocally( | ||
| lastRead: timestamp, | ||
| lastReadMessageId: lastReadMessage?.id, | ||
| ); | ||
| return EmptyResponse(); | ||
| } | ||
|
|
||
| if (!canUseReadReceipts) { | ||
| throw const StreamChatError( | ||
| 'Cannot mark as unread: Channel does not support read events. ' | ||
|
|
@@ -2096,7 +2159,7 @@ class Channel { | |
| }; | ||
|
|
||
| if (isQueryingAround) this.state?.truncate(); | ||
| this.state?.updateChannelState(channelState); | ||
| this.state?.updateChannelStateFromServer(channelState); | ||
| } | ||
|
|
||
| // Submit for delivery reporting only when fetching the latest messages. | ||
|
|
@@ -3209,6 +3272,15 @@ class ChannelClientState { | |
| deletedForMe: event.deletedForMe, | ||
| ); | ||
|
|
||
| // Decrement the locally-tracked unread count for hard-deleted | ||
| // messages that would have counted as unread. Soft-deleted messages | ||
| // keep their slot. Only applies to channels that track unread counts | ||
| // locally (see [Channel.usesLocalUnreadCount]) — server-driven | ||
| // channels get corrected counts from server read events instead. | ||
| if (hardDelete && _channel.usesLocalUnreadCount && MessageRules.canCountAsUnread(message, _channel)) { | ||
| unreadCount = math.max(0, unreadCount - 1); | ||
| } | ||
|
|
||
| return deleteMessage(message, hardDelete: hardDelete); | ||
| }), | ||
| ); | ||
|
|
@@ -3563,6 +3635,75 @@ class ChannelClientState { | |
| return updateRead([existingUserRead.copyWith(unreadMessages: count)]); | ||
| } | ||
|
|
||
| /// Marks the channel as read locally, without making a network request. | ||
| /// | ||
| /// Used for channels that track unread counts locally (see | ||
| /// [Channel.usesLocalUnreadCount]), since the server rejects the mark-read | ||
| /// endpoint for channels that have read events disabled. | ||
| void markReadLocally({String? messageId}) { | ||
| final currentUser = _client.state.currentUser; | ||
| if (currentUser == null) return; | ||
|
|
||
| final now = DateTime.now(); | ||
| final lastReadMessageId = messageId ?? messages.lastOrNull?.id; | ||
|
|
||
| final existingUserRead = currentUserRead; | ||
| updateRead([ | ||
| Read( | ||
| user: currentUser, | ||
| lastRead: now, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Does the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The moment of reading. DateTime.now() is correct and aligned with the server. The server treats the two as separate fields, and we already follow that for remote events (channel.dart:3393-3421):
Swift is the same: resetChannelRead(lastReadAt: event.createdAt), and its local variant uses markChannelAsRead(cid:userId:at: .init()) — i.e. Date(), now (ChannelRepository.swift:74). So lastRead = timestamp, lastReadMessageId = which message. ✅ However, this question does surface a real gap in markReadLocally. When called as markRead(messageId: X) with an older X, we set lastRead = now and reset the count to 0 (Read's unreadMessages defaults to 0). The server instead recomputes unread as the number of messages after X — so messages between X and now stay unread. Locally we clear the whole count and silently ignore the messageId for counting purposes. Swift sidesteps this by not accepting a message id in markReadLocally at all. Two options:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm so if I understood correctly - basically we always set the unread count to 0 - no matter how many messages are actually after the marked message? Concerning the option one: Can we reliable calculate this? Because the user might be in a state where the latest messages are not loaded in memory (jump-to-message flow), so we might not have all the messages after the anchor loaded in memory - resulting in a wrong count. |
||
| lastReadMessageId: lastReadMessageId, | ||
| lastDeliveredAt: existingUserRead?.lastDeliveredAt, | ||
| lastDeliveredMessageId: existingUserRead?.lastDeliveredMessageId, | ||
| ), | ||
| ]); | ||
|
|
||
| // Read supersedes delivered, so drop any pending delivery candidate the | ||
| // new read boundary just made ineligible. `delivery_events` is configured | ||
| // independently of `read_events`, so a channel tracking unread counts | ||
| // locally can still have delivery receipts enabled. Mirrors what the | ||
| // `message.read` event listener does for server-driven channels. | ||
| _client.channelDeliveryReporter.reconcileDelivery([_channel]); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /// Marks the channel as unread locally, without making a network request. | ||
| /// | ||
| /// [lastRead] and [lastReadMessageId] define the new read boundary: any | ||
| /// locally-known message that is still eligible per | ||
| /// [MessageRules.canCountAsUnread] once this boundary is applied is counted | ||
| /// as unread. | ||
| /// | ||
| /// Used for channels that track unread counts locally (see | ||
| /// [Channel.usesLocalUnreadCount]), since the server rejects the | ||
| /// mark-unread endpoint for channels that have read events disabled. | ||
| void markUnreadLocally({ | ||
| required DateTime lastRead, | ||
| String? lastReadMessageId, | ||
| }) { | ||
| final currentUser = _client.state.currentUser; | ||
| if (currentUser == null) return; | ||
|
|
||
| final existingUserRead = currentUserRead; | ||
|
|
||
| // Apply the new read boundary first so `MessageRules.canCountAsUnread` | ||
| // (which reads `channel.state?.currentUserRead`) evaluates against it. | ||
| updateRead([ | ||
| Read( | ||
| user: currentUser, | ||
| lastRead: lastRead, | ||
| lastReadMessageId: lastReadMessageId, | ||
| lastDeliveredAt: existingUserRead?.lastDeliveredAt, | ||
| lastDeliveredMessageId: existingUserRead?.lastDeliveredMessageId, | ||
| ), | ||
| ]); | ||
|
|
||
| // Recompute the unread count from the locally-known messages now that | ||
| // the boundary above is in effect. | ||
| final unread = messages.where((it) => MessageRules.canCountAsUnread(it, _channel)).length; | ||
|
|
||
| unreadCount = unread; | ||
| } | ||
|
|
||
| /// Counts the number of unread messages mentioning the current user. | ||
| /// | ||
| /// **NOTE**: The method relies on the [Channel.messages] list and doesn't do | ||
|
|
@@ -3645,6 +3786,47 @@ class ChannelClientState { | |
| ); | ||
| } | ||
|
|
||
| /// Applies a [remoteState] received from the server or offline storage | ||
| /// (e.g. a `query`/`watch` response), merging it into local state. | ||
| /// | ||
| /// Unlike [updateChannelState], this preserves the current user's | ||
| /// locally-tracked read state for channels that track unread counts | ||
| /// on-device (see [Channel.usesLocalUnreadCount]) — their `lastRead`, | ||
| /// `lastReadMessageId`, and `unreadMessages` are kept as-is instead of | ||
| /// being overwritten by the remote payload; only delivery fields are | ||
| /// still applied from it. | ||
| /// | ||
| /// Call this instead of [updateChannelState] whenever [remoteState] | ||
| /// genuinely comes from the network or offline storage. | ||
| void updateChannelStateFromServer(ChannelState remoteState) { | ||
| updateChannelState(_preserveLocalUnreadState(remoteState)); | ||
| } | ||
|
|
||
| /// Rewrites the current user's [Read] in [remoteState], if present, to | ||
| /// keep the locally-tracked `lastRead` / `lastReadMessageId` / | ||
| /// `unreadMessages` while still adopting the remote delivery fields. | ||
| /// | ||
| /// No-op unless [Channel.usesLocalUnreadCount] is enabled and a local read | ||
| /// already exists for the current user. | ||
| ChannelState _preserveLocalUnreadState(ChannelState remoteState) { | ||
| if (!_channel.usesLocalUnreadCount) return remoteState; | ||
|
|
||
| final localRead = currentUserRead; | ||
| final remoteReads = remoteState.read; | ||
| if (localRead == null || remoteReads == null) return remoteState; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Will we always get an initial
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, we might not — but we don't need one. Three layers cover it, and iOS handles it the same defensive way.
iOS is defensive in the same places: the middleware uses loadOrCreateChannelRead, markChannelAsRead creates a read for members when one is missing, saveChannelRead only preserves local values when !dto.isInserted (ChannelReadDTO.swift:105-114 — the direct analogue of _preserveLocalUnreadState, including the "first insert takes the payload" rule), and ChatChannelVC returns true // no read state, always mark as read. One genuine (minor) edge: if a message arrives while isUpToDate == false, updateMessage is skipped so lastMessageAt isn't advanced, and the synthesized lastRead sits before the counted message. The count is still right (1); only the boundary timestamp is slightly conservative, which could let a later recompute re-count that message. If we want to be strict, messages.lastOrNull?.createdAt ?? lastMessageAt ?? DateTime.now() closes it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it - thanks for explaining! Concerning the mentioned edge case: This is also a valid concern for 'regular' |
||
|
|
||
| final currentUserId = localRead.user.id; | ||
| final preservedReads = remoteReads.map((read) { | ||
| if (read.user.id != currentUserId) return read; | ||
| return localRead.copyWith( | ||
| lastDeliveredAt: read.lastDeliveredAt, | ||
| lastDeliveredMessageId: read.lastDeliveredMessageId, | ||
| ); | ||
| }); | ||
|
|
||
| return remoteState.copyWith(read: preservedReads.toList()); | ||
| } | ||
|
|
||
| int _sortByCreatedAt(Message a, Message b) => a.createdAt.compareTo(b.createdAt); | ||
|
|
||
| /// The channel state related to this client. | ||
|
|
@@ -4554,6 +4736,17 @@ extension ChannelCapabilityCheck on Channel { | |
| return ownCapabilities.contains(ChannelCapability.readEvents); | ||
| } | ||
|
|
||
| /// True, if unread counts for this channel should be tracked locally, | ||
| /// on-device, rather than relying on the server. | ||
| /// | ||
| /// This is the case when [StreamChatClient.isLocalUnreadCountEnabled] is | ||
| /// enabled and the channel doesn't support read receipts (for example, | ||
| /// livestream channel types that disable read events). Channels that | ||
| /// support read receipts always rely on server-driven unread counts. | ||
| bool get usesLocalUnreadCount { | ||
| return _client.isLocalUnreadCountEnabled && !canUseReadReceipts; | ||
| } | ||
|
|
||
| /// True, if the current user has connect events capability. | ||
| bool get canReceiveConnectEvents { | ||
| return ownCapabilities.contains(ChannelCapability.connectEvents); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious: Don't soft-deletes also decrement the unread count?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No — intentionally, and this matches iOS exactly.
Swift short-circuits soft deletes with a dedicated skip reason before any of the other eligibility checks run:
// ChannelReadUpdaterMiddleware.swift:216-247
if let skipReason = !event.hardDelete
? .messageIsSoftDeleted
: unreadCountUpdateSkippingReason(...) { return log.debug(...) }
channelRead.unreadMessageCount = max(0, channelRead.unreadMessageCount - 1)
The reasoning: a soft-deleted message keeps its slot in the message list (it renders as "This message was deleted"), so there is still an item the user hasn't seen — the count should keep pointing at it. A hard delete removes the row entirely, so the count has to shrink or it points at nothing. There's a test for both directions (does not decrement unreadCount when a message is soft-deleted / decrements unreadCount when a counted message is hard-deleted).
One divergence worth naming, though it's out of scope for this PR: Swift's decrement isn't gated on local unread counting at all — it runs for every channel, mirroring the backend. Flutter gates it on usesLocalUnreadCount, so a server-driven channel keeps a stale unread count after a hard delete until the next query/read event. That's a pre-existing gap in Flutter (nothing decremented before this PR either).
Shall I remove the
_channel.usesLocalUnreadCountcheck?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aham interesting, I wasn't aware of this.
I would leave this up to you, maybe perhaps we can do it separately in a different PR, to have a cleaner changelog.