From 439cadd808a9b5da659f515f8504dde7feb84644 Mon Sep 17 00:00:00 2001 From: VelikovPetar Date: Tue, 14 Jul 2026 15:32:12 +0200 Subject: [PATCH] fix(llc): Don't upsert out-of-window messages on `message.updated`/`message.deleted` events Co-Authored-By: Claude Opus 4.8 --- packages/stream_chat/CHANGELOG.md | 5 + .../stream_chat/lib/src/client/channel.dart | 136 +++++--- .../test/src/client/channel_test.dart | 320 +++++++++++++++++- 3 files changed, 403 insertions(+), 58 deletions(-) diff --git a/packages/stream_chat/CHANGELOG.md b/packages/stream_chat/CHANGELOG.md index eeb3d3c47..66a01cbe9 100644 --- a/packages/stream_chat/CHANGELOG.md +++ b/packages/stream_chat/CHANGELOG.md @@ -1,5 +1,9 @@ ## Upcoming +βœ… Added + +- Added an `upsert` flag to `ChannelClientState.updateMessage` and `ChannelClientState.updateThreadInfo` (defaults to `true`). Pass `false` to update a message only if it's already loaded in the state, skipping unknown messages instead of adding them. + πŸ”„ Changed - `StreamChatClient.updateSystemEnvironment` now sanitizes the passed `SystemEnvironment`: `sdkName`, `sdkVersion`, and `osName` are locked to internal defaults, and `sdkIdentifier` only accepts the `dart` β†’ `flutter` promotion (other values, including a `flutter` β†’ `dart` demotion, are ignored). `appName`, `appVersion`, `osVersion`, and `deviceModel` continue to pass through as-is. @@ -8,6 +12,7 @@ - Fixed a deprecation warning from `equatable` causing CI analysis to fail. - `ComparableField` now folds diacritics/ligatures and ignores case when comparing strings, so `SortOption` on fields like `name` no longer pushes lowercase or non-ASCII names (`jhon`, `Łukasz`, `Øystein`) to the end of client-sorted lists ([#2601](https://github.com/GetStream/stream-chat-flutter/issues/2601)). +- Fixed `message.updated` and soft `message.deleted` events being incorrectly upserted into `ChannelState.messages` (and thread reply lists) when they targeted a message outside the currently loaded window. ## 9.26.0 diff --git a/packages/stream_chat/lib/src/client/channel.dart b/packages/stream_chat/lib/src/client/channel.dart index fe4e7f89d..dd228c26c 100644 --- a/packages/stream_chat/lib/src/client/channel.dart +++ b/packages/stream_chat/lib/src/client/channel.dart @@ -2819,7 +2819,7 @@ class ChannelClientState { pollId: oldMessage?.pollId, ownReactions: oldMessage?.ownReactions, ); - updateMessage(message); + updateMessage(message, upsert: false); })); } @@ -2935,74 +2935,82 @@ class ChannelClientState { ); } - /// Updates the [message] in the state if it exists. Adds it otherwise. - void updateMessage(Message message) { + /// Updates the [message] in the state if it exists. Adds it otherwise, + /// unless [upsert] is `false`, in which case an unknown [message] is skipped. + void updateMessage(Message message, {bool upsert = true}) { // Determine if the message should be displayed in the channel view. if (message.parentId == null || message.showInChannel == true) { // Scan from the tail: server echoes, edits, and reactions almost // always target a recent message, so `lastIndexWhere` exits in a // handful of comparisons instead of walking the full list. final oldIndex = messages.lastIndexWhere((m) => m.id == message.id); - final oldMessage = oldIndex == -1 ? null : messages[oldIndex]; - - // Carry over local-only timestamps; no-op when there's no prior. - var updatedMessage = message.syncWith(oldMessage); - - // Restore `quotedMessage` stripped by a partial-update payload β€” - // the server omits it when only updating other fields. - if (oldMessage != null && - updatedMessage.quotedMessageId != null && - updatedMessage.quotedMessage == null && - oldMessage.quotedMessage != null) { - updatedMessage = updatedMessage.copyWith( - quotedMessage: oldMessage.quotedMessage, + + // Handle updates to pinned messages. + final newPinnedMessages = _updatePinnedMessages(message); + + if (oldIndex == -1 && !upsert) { + _channelState = _channelState.copyWith( + pinnedMessages: newPinnedMessages, ); - } + } else { + final oldMessage = oldIndex == -1 ? null : messages[oldIndex]; + + // Carry over local-only timestamps; no-op when there's no prior. + var updatedMessage = message.syncWith(oldMessage); + + // Restore `quotedMessage` stripped by a partial-update payload β€” + // the server omits it when only updating other fields. + if (oldMessage != null && + updatedMessage.quotedMessageId != null && + updatedMessage.quotedMessage == null && + oldMessage.quotedMessage != null) { + updatedMessage = updatedMessage.copyWith( + quotedMessage: oldMessage.quotedMessage, + ); + } - var newMessages = messages.sortedUpsertAt( - oldIndex, - updatedMessage, - compare: _sortByCreatedAt, - ); + var newMessages = messages.sortedUpsertAt( + oldIndex, + updatedMessage, + compare: _sortByCreatedAt, + ); - // When the target of a quote is deleted, rewrite the embedded - // `quotedMessage` in every message quoting it. `updateIf` returns - // the same list reference when nothing matches. - if (oldMessage != null && message.isDeleted) { - newMessages = newMessages.updateIf( - (it) => it.quotedMessageId == message.id, - (it) => it.copyWith( - quotedMessage: updatedMessage.copyWith( - type: message.type, - deletedAt: message.deletedAt, + // When the target of a quote is deleted, rewrite the embedded + // `quotedMessage` in every message quoting it. `updateIf` returns + // the same list reference when nothing matches. + if (oldMessage != null && message.isDeleted) { + newMessages = newMessages.updateIf( + (it) => it.quotedMessageId == message.id, + (it) => it.copyWith( + quotedMessage: updatedMessage.copyWith( + type: message.type, + deletedAt: message.deletedAt, + ), ), - ), - ); - } + ); + } - // Handle updates to pinned messages. - final newPinnedMessages = _updatePinnedMessages(message); + // Calculate the new last message at time. + var lastMessageAt = _channelState.channel?.lastMessageAt; + lastMessageAt ??= message.createdAt; + if (MessageRules.canUpdateChannelLastMessageAt(message, _channel)) { + lastMessageAt = [lastMessageAt, message.createdAt].max; + } - // Calculate the new last message at time. - var lastMessageAt = _channelState.channel?.lastMessageAt; - lastMessageAt ??= message.createdAt; - if (MessageRules.canUpdateChannelLastMessageAt(message, _channel)) { - lastMessageAt = [lastMessageAt, message.createdAt].max; + // Apply the updated lists to the channel state. + _channelState = _channelState.copyWith( + messages: newMessages, + pinnedMessages: newPinnedMessages, + channel: _channelState.channel?.copyWith( + lastMessageAt: lastMessageAt, + ), + ); } - - // Apply the updated lists to the channel state. - _channelState = _channelState.copyWith( - messages: newMessages, - pinnedMessages: newPinnedMessages, - channel: _channelState.channel?.copyWith( - lastMessageAt: lastMessageAt, - ), - ); } // If the message is part of a thread, update thread information. if (message.parentId case final parentId?) { - updateThreadInfo(parentId, [message]); + updateThreadInfo(parentId, [message], upsert: upsert); } } @@ -3090,7 +3098,7 @@ class ChannelClientState { /// Removes/Updates the [message] based on the [hardDelete] value. void deleteMessage(Message message, {bool hardDelete = false}) { if (hardDelete) return removeMessage(message); - return updateMessage(message); + return updateMessage(message, upsert: false); } void _listenReadEvents() { @@ -3453,16 +3461,32 @@ class ChannelClientState { } /// Update threads with updated information about messages. - void updateThreadInfo(String parentId, List messages) { + void updateThreadInfo( + String parentId, + List messages, { + bool upsert = true, + }) { + var messagesToMerge = messages; + if (!upsert) { + final existingThread = threads[parentId]; + // Don't create a phantom entry for a thread that was never paged in, + // and only update replies already loaded in it. + if (existingThread == null) return; + final existingIds = {for (final m in existingThread) m.id}; + messagesToMerge = + messages.where((m) => existingIds.contains(m.id)).toList(); + if (messagesToMerge.isEmpty) return; + } + final newThreads = {...threads}..update( parentId, (original) => original.merge( - messages, + messagesToMerge, key: (message) => message.id, update: (original, updated) => updated.syncWith(original), compare: _sortByCreatedAt, ), - ifAbsent: () => messages.sorted(_sortByCreatedAt), + ifAbsent: () => messagesToMerge.sorted(_sortByCreatedAt), ); _threads = newThreads; diff --git a/packages/stream_chat/test/src/client/channel_test.dart b/packages/stream_chat/test/src/client/channel_test.dart index 6aa947a2a..1a75b4448 100644 --- a/packages/stream_chat/test/src/client/channel_test.dart +++ b/packages/stream_chat/test/src/client/channel_test.dart @@ -1183,6 +1183,60 @@ void main() { }); }); + group('`ChannelClientState.updateMessage`', () { + test('upsert: true (default) adds an unknown message', () async { + final message = Message( + id: 'unknown-message', + user: client.state.currentUser, + text: 'hello', + createdAt: DateTime.utc(2026), + ); + + expect(channel.state!.messages, isEmpty); + + channel.state!.updateMessage(message); + + expect(channel.state!.messages.map((m) => m.id), ['unknown-message']); + }); + + test('upsert: false does NOT add an unknown message', () async { + final message = Message( + id: 'unknown-message', + user: client.state.currentUser, + text: 'hello', + createdAt: DateTime.utc(2026), + ); + + expect(channel.state!.messages, isEmpty); + + channel.state!.updateMessage(message, upsert: false); + + expect(channel.state!.messages, isEmpty); + }); + + test('upsert: false updates a message already in the window', () async { + const messageId = 'known-message'; + final seeded = Message( + id: messageId, + user: client.state.currentUser, + text: 'old', + createdAt: DateTime.utc(2026), + ); + channel.state!.updateChannelState( + channel.state!.channelState.copyWith(messages: [seeded]), + ); + + channel.state!.updateMessage( + seeded.copyWith(text: 'new'), + upsert: false, + ); + + final stored = channel.state!.messages.single; + expect(stored.id, equals(messageId)); + expect(stored.text, equals('new')); + }); + }); + test('`.partialUpdateMessage`', () async { final message = Message( id: 'test-message-id', @@ -1253,8 +1307,12 @@ void main() { when(() => client.deleteMessage(messageId)) .thenAnswer((_) async => EmptyResponse()); + // A soft delete only updates a message already in the loaded window, + // so seed it first β€” a delete must never insert a phantom record. + channel.state?.updateMessage(message); + expectLater( - // skipping first seed message list -> [] messages + // skip the seeded message -> [message] channel.state?.messagesStream.skip(1), emitsInOrder([ [ @@ -1328,8 +1386,12 @@ void main() { id: messageId, ); + // A soft delete only updates a message already in the loaded window, + // so seed it first β€” a delete must never insert a phantom record. + channel.state?.updateMessage(message); + expectLater( - // skipping first seed message list -> [] messages + // skip the seeded message -> [message] channel.state?.messagesStream.skip(1), emitsInOrder([ [ @@ -4450,6 +4512,134 @@ void main() { expect(channel.state?.pinnedMessages, isEmpty); }, ); + + // A `message.updated` event for a message outside the loaded window + // would otherwise upsert into the sorted list β€” creating a phantom + // entry with a gap. The guard is "id not in the loaded list", and is + // independent of `isUpToDate` β€” even at the latest page we may have + // paginated past older history and receive an event for a message no + // longer in memory. + group('when message is outside the loaded window', () { + test( + 'should NOT insert unknown message into `messages` list', + () async { + // Simulate "we have the latest page but not older history": + // seed the tail messages. + final tail = List.generate( + 3, + (i) => Message( + id: 'tail-$i', + user: client.state.currentUser, + text: 'tail $i', + createdAt: DateTime.utc(2026, 6, 1).add(Duration(seconds: i)), + ), + ); + channel.state!.updateChannelState( + channel.state!.channelState.copyWith(messages: tail), + ); + expect(channel.state!.messages, hasLength(3)); + + // Event for a message on an older page we don't have loaded. + final olderPageEdit = Message( + id: 'older-page-msg', + user: client.state.currentUser, + text: 'edited on older page', + createdAt: DateTime.utc(2025, 1, 1), + ); + client.addEvent(createUpdateMessageEvent(olderPageEdit)); + await Future.delayed(Duration.zero); + + // Tail is unchanged, no phantom entry inserted at position 0. + expect( + channel.state!.messages.map((m) => m.id), + ['tail-0', 'tail-1', 'tail-2'], + ); + expect(channel.state!.pinnedMessages, isEmpty); + }, + ); + + test( + 'should still add to pinnedMessages when pinned:true even if not ' + 'in loaded window', + () async { + channel.state!.isUpToDate = false; + expect(channel.state!.messages, isEmpty); + expect(channel.state!.pinnedMessages, isEmpty); + + const messageId = 'pin-me'; + final pinned = Message( + id: messageId, + user: client.state.currentUser, + pinned: true, + ); + client.addEvent(createUpdateMessageEvent(pinned)); + await Future.delayed(Duration.zero); + + expect(channel.state!.messages, isEmpty); + expect(channel.state!.pinnedMessages.length, equals(1)); + expect(channel.state!.pinnedMessages.first.id, equals(messageId)); + }, + ); + + test( + 'should NOT insert unknown reply into threads[parentId]', + () async { + const parentId = 'parent-1'; + final knownReply = Message( + id: 'known-reply', + parentId: parentId, + user: client.state.currentUser, + createdAt: DateTime.utc(2026), + ); + // Populate threads[parentId] via the thread-only update path. + channel.state!.updateMessage(knownReply); + await Future.delayed(Duration.zero); + expect(channel.state!.threads[parentId], hasLength(1)); + + channel.state!.isUpToDate = false; + + final phantomReply = Message( + id: 'other-reply', + parentId: parentId, + user: client.state.currentUser, + text: 'edited', + createdAt: DateTime.utc(2026, 1, 2), + ); + client.addEvent(createUpdateMessageEvent(phantomReply)); + await Future.delayed(Duration.zero); + + expect( + channel.state!.threads[parentId]!.map((m) => m.id), + ['known-reply'], + ); + }, + ); + + test( + 'should NOT create phantom threads[parentId] entry for unloaded ' + 'thread', + () async { + const parentId = 'unloaded-parent'; + // The thread was never paged in, so there's no entry for it. + expect(channel.state!.threads.containsKey(parentId), isFalse); + + channel.state!.isUpToDate = false; + + final phantomReply = Message( + id: 'phantom-reply', + parentId: parentId, + user: client.state.currentUser, + text: 'edited', + createdAt: DateTime.utc(2026, 1, 2), + ); + client.addEvent(createUpdateMessageEvent(phantomReply)); + await Future.delayed(Duration.zero); + + // The dropped reply must not leave behind an empty thread entry. + expect(channel.state!.threads.containsKey(parentId), isFalse); + }, + ); + }); }, ); @@ -4624,6 +4814,132 @@ void main() { }, ); + // A `message.deleted` event for a message outside the loaded window must + // not upsert a "deleted" record into the sorted list β€” that would create a + // phantom entry with a gap. Pinned side-effects must still fire. + group( + EventType.messageDeleted, + () { + const channelId = 'test-channel-id'; + const channelType = 'test-channel-type'; + late Channel channel; + + setUp(() { + final channelState = _generateChannelState( + channelId, + channelType, + mockChannelConfig: true, + ownCapabilities: const [ChannelCapability.readEvents], + ); + channel = Channel.fromState(client, channelState); + }); + + tearDown(() => channel.dispose()); + + Event createDeleteMessageEvent( + Message message, { + bool hardDelete = false, + }) { + return Event( + cid: channel.cid, + type: EventType.messageDeleted, + message: message.copyWith( + type: MessageType.deleted, + deletedAt: DateTime.timestamp(), + ), + hardDelete: hardDelete, + ); + } + + // Same design as the `messageUpdated` guards: the check is + // "message-in-loaded-window" and is independent of `isUpToDate` β€” an + // event for a message on an older, unloaded page must not be turned + // into a phantom "deleted" record inserted into the sorted list. + group('when message is outside the loaded window', () { + test( + 'soft delete does NOT insert phantom "deleted" record into messages', + () async { + final tail = List.generate( + 3, + (i) => Message( + id: 'tail-$i', + user: client.state.currentUser, + text: 'tail $i', + createdAt: DateTime.utc(2026, 6, 1).add(Duration(seconds: i)), + ), + ); + channel.state!.updateChannelState( + channel.state!.channelState.copyWith(messages: tail), + ); + expect(channel.state!.messages, hasLength(3)); + + final olderPage = Message( + id: 'older-page-msg', + user: client.state.currentUser, + text: 'gone', + createdAt: DateTime.utc(2025, 1, 1), + ); + client.addEvent(createDeleteMessageEvent(olderPage)); + await Future.delayed(Duration.zero); + + expect( + channel.state!.messages.map((m) => m.id), + ['tail-0', 'tail-1', 'tail-2'], + ); + }, + ); + + test( + 'soft delete marks message as deleted when it IS in the loaded ' + 'window', + () async { + const messageId = 'known'; + final seeded = Message( + id: messageId, + user: client.state.currentUser, + text: 'hi', + createdAt: DateTime.utc(2026), + ); + channel.state!.updateChannelState( + channel.state!.channelState.copyWith(messages: [seeded]), + ); + channel.state!.isUpToDate = false; + + client.addEvent(createDeleteMessageEvent(seeded)); + await Future.delayed(Duration.zero); + + final stored = + channel.state!.messages.singleWhere((m) => m.id == messageId); + expect(stored.type, equals(MessageType.deleted)); + expect(stored.deletedAt, isNotNull); + }, + ); + + test( + 'hard delete is a no-op when message is not in the loaded window', + () async { + channel.state!.isUpToDate = false; + expect(channel.state!.messages, isEmpty); + + final phantom = Message( + id: 'phantom', + user: client.state.currentUser, + text: 'gone', + createdAt: DateTime.utc(2026), + ); + client.addEvent( + createDeleteMessageEvent(phantom, hardDelete: true), + ); + await Future.delayed(Duration.zero); + + expect(channel.state!.messages, isEmpty); + expect(channel.state!.pinnedMessages, isEmpty); + }, + ); + }); + }, + ); + group('Member Events', () { const channelId = 'test-channel-id'; const channelType = 'test-channel-type';