feat(llc, persistence): handle user.messages.deleted event #2423
Conversation
Adds support for the `user.messages.deleted` WebSocket event. This event is now handled at the client level and propagated to all active channels. Each channel will then proceed to either soft or hard delete all messages from the specified user, depending on the event payload. Refactors message update and removal logic to support bulk operations, improving performance and maintainability.
This commit refactors the message handling logic within the `Channel` class to improve robustness and correctness, especially for delete and update operations. - **Unified Message Handling:** Introduces internal methods `_updateMessages` and `_removeMessages` to serve as central points for modifying channel state. These methods now consistently update channel messages, thread messages, pinned messages, and active live locations. - **Improved Deletion Logic:** The `deleteMessagesFromUser` method now correctly identifies all messages from a user, including those in threads, and marks them for deletion. The logic for hard and soft deletes is now handled more robustly. - **State Update Refactoring:** Message list modifications (updates and removals) have been refactored into dedicated helper methods (`_mergeMessagesIntoExisting`, `_removeMessagesFromExisting`, etc.). This change centralizes logic and ensures that related state, such as quoted messages and pinned messages, is updated consistently. - **Pinned Message and Live Location Consistency:** Pinned messages and active live locations are now correctly removed or updated when their underlying messages are deleted or modified. A deleted message is no longer considered a valid pin. - **Generalized `merge` Extension:** A new `mergeFrom` extension method is introduced, allowing an iterable to be merged with an iterable of a different type. This provides more flexibility than the original `merge` method. - **Event Listener Organization:** Event listener registrations in both `Client` and `Channel` have been reordered to group related listeners, improving code readability.
Refactors the tests for the `user.messages.deleted` WebSocket event to improve reliability and correctness. - Connects the client and waits for a connected status in `setUp`, ensuring a realistic test environment. - Simplifies event dispatching by sending a single global event instead of one per channel. - Removes an unnecessary test for channel-specific `user.messages.deleted` events, as these are handled by the channel itself. - Adds tests to verify that messages are correctly deleted from the persistence client when `hardDelete` is true.
Introduces a new method `deleteMessagesFromUser` to the `ChatPersistenceClient` to allow for the bulk deletion (hard or soft) of all messages from a specific user. This operation can be scoped to a particular channel or applied globally. The implementation is added to `StreamChatPersistenceClient`, which now delegates this action to both `messageDao` and `pinnedMessageDao` to ensure all user messages, including pinned ones, are removed from the local database. The `Channel` class now calls this new persistence method when handling a `user.messages.deleted` event to ensure that all of a user's messages are cleared from local storage, not just those currently loaded in the channel's state. Comprehensive unit tests have been added for the new DAO methods, the persistence client implementation, and the updated channel event handling logic.
This change introduces an optional `messageLimit` parameter to the `getChannelStates` method in `StreamChatPersistenceClient`. This allows developers to control the number of messages fetched for each channel when querying offline channel states, defaulting to 25 if not specified. The new parameter is passed down through `StreamChatClient` and `ChatPersistenceClient` to the database query layer.
Increments the database schema version from 26 to 27.
Refactors the tests for the `user.messages.deleted` WebSocket event to improve reliability and correctness. - Connects the client and waits for a connected status in `setUp`, ensuring a realistic test environment. - Simplifies event dispatching by sending a single global event instead of one per channel. - Removes an unnecessary test for channel-specific `user.messages.deleted` events, as these are handled by the channel itself. - Adds tests to verify that messages are correctly deleted from the persistence client when `hardDelete` is true.
WalkthroughThis PR introduces support for the Changes
Sequence DiagramsequenceDiagram
actor WS as WebSocket
participant Client
participant Channel
participant Persistence
participant Store as DB
WS->>Client: userMessagesDeleted event
activate Client
Client->>Client: _listenUserMessagesDeleted()
alt event.cid is null
Client->>Client: iterate all cached channels
loop for each channel
Client->>Channel: forward event
end
else event.cid is set
Client->>Channel: forward event to specific channel
end
deactivate Client
activate Channel
Channel->>Channel: _listenUserMessagesDeleted()
alt hardDelete = true
Channel->>Channel: _deleteMessagesFromUser(hard)
Channel->>Channel: _removeMessages (from memory)
Channel->>Persistence: deleteMessageByIds (pinned)
Channel->>Persistence: deleteMessageByIds (messages)
else hardDelete = false
Channel->>Channel: _deleteMessagesFromUser(soft)
Channel->>Channel: mark messages deleted,<br/>set deletedAt, state flag
end
Persistence->>Store: execute deletions
Store-->>Persistence: confirm
deactivate Channel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Rationale: This PR involves substantial refactoring of core message handling logic in Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Replaces the `MessageState.deleted` factory with a `switch` expression to set the message state. This change simplifies how a message's state is updated upon deletion by using `MessageState.hardDeleted` or `MessageState.softDeleted` directly, based on the `hardDelete` boolean.
xsahil03x
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v10.0.0 #2423 +/- ##
==========================================
Coverage ? 65.40%
==========================================
Files ? 421
Lines ? 26266
Branches ? 0
==========================================
Hits ? 17180
Misses ? 9086
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| final persistence = _channel.client.chatPersistenceClient; | ||
| persistence?.deleteMessagesFromUser( | ||
| userId: userId, | ||
| cid: _channel.cid, | ||
| hardDelete: hardDelete, | ||
| deletedAt: deletedAt, | ||
| ); |
There was a problem hiding this comment.
Shouldn't we await here? Because otherwise we'll try to delete the same messages in _deleteMessages which might give a race condition?
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/stream_chat/lib/src/client/channel.dart (1)
3777-3811: Persistence-first user-wide deletion is correct (await fixed).Awaiting deleteMessagesFromUser before in-memory changes prevents races with subsequent removals. Matches earlier feedback; thanks for addressing.
🧹 Nitpick comments (9)
packages/stream_chat_persistence/lib/src/dao/pinned_message_dao.dart (1)
245-278: Implementation is correct and consistent with message_dao.The method properly mirrors the logic from
message_dao.dart, handling both hard and soft deletion for pinned messages. While there is code duplication between the two DAOs, it's acceptable given the different Drift table types (PinnedMessagesCompanion vs MessagesCompanion).If code duplication becomes a maintenance concern in the future, consider extracting the shared deletion logic into a helper function or mixin. However, this is not critical given the relatively small and stable nature of this code.
packages/stream_chat/test/src/client/channel_test.dart (4)
6663-6695: Soft-delete: assert deletedAt equals event time and verify persistence call.Strengthen the test by checking deletedAt == event.createdAt and that persistence is instructed to soft‑delete user messages (with the same timestamp).
Apply this diff:
@@ - final deletedAt = DateTime.now(); + final deletedAt = DateTime.now(); @@ - for (final message in deletedMessages!) { + for (final message in deletedMessages!) { expect(message.type, equals(MessageType.deleted)); expect(message.deletedAt, isNotNull); expect(message.state.isDeleted, isTrue); + expect(message.deletedAt!.isAtSameMomentAs(deletedAt), isTrue); } @@ - expect(user2Message?.deletedAt, isNull); + expect(user2Message?.deletedAt, isNull); + + // Persistence should receive a soft-delete for all user messages in storage + final capturedDeletedAt = verify( + () => persistenceClient.deleteMessagesFromUser( + cid: channel.cid, + userId: user1.id, + hardDelete: false, + deletedAt: captureAny(named: 'deletedAt'), + ), + ).captured.first as DateTime; + expect(capturedDeletedAt.isAtSameMomentAs(deletedAt), isTrue);
6791-6811: Thread soft-delete: set event.createdAt and assert thread messages’ deletedAt.Improves correctness and parity with non-thread case.
Apply this diff:
@@ - // Create user.messages.deleted event (soft delete) - final userMessagesDeletedEvent = Event( + // Create user.messages.deleted event (soft delete) + final deletedAt = DateTime.now(); + final userMessagesDeletedEvent = Event( cid: channel.cid, type: EventType.userMessagesDeleted, user: user1, hardDelete: false, + createdAt: deletedAt, ); @@ - for (final message in threadMessages!) { + for (final message in threadMessages!) { expect(message.type, equals(MessageType.deleted)); expect(message.state.isDeleted, isTrue); + expect(message.deletedAt!.isAtSameMomentAs(deletedAt), isTrue); }
6963-6973: Soft-delete should still touch persistence (storage), not only in-memory.Even when hardDelete is false, the storage layer should be instructed to soft-delete all user messages. Add an assertion for deleteMessagesFromUser with hardDelete: false.
Apply this diff:
@@ - // Verify persistence deletion methods were NOT called + // Verify per-ID deletion methods were NOT called verifyNever(() => persistenceClient.deleteMessageByIds(any())); verifyNever(() => persistenceClient.deletePinnedMessageByIds(any())); // Verify message is soft deleted (still in state) expect(channel.state?.messages.length, equals(1)); expect( channel.state?.messages.first.type, equals(MessageType.deleted)); + + // But storage should receive a soft-delete for all user messages + verify( + () => persistenceClient.deleteMessagesFromUser( + cid: channel.cid, + userId: user1.id, + hardDelete: false, + deletedAt: any(named: 'deletedAt'), + ), + ).called(1);
7063-7075: Also verify pinned IDs removed from persistence in the storage-wide delete test.You already capture deleteMessageByIds. Mirror that for pinned IDs to ensure both tables are cleaned consistently.
Apply this diff:
@@ final capturedIds = verify( () => persistenceClient.deleteMessageByIds(captureAny()), ).captured.first as List<String>; @@ expect( capturedIds, containsAll([ 'msg-1', // state message 'thread-msg-1', // state thread message ]), ); + + // Pinned clean-up should include pinned state message + final capturedPinnedIds = verify( + () => persistenceClient.deletePinnedMessageByIds(captureAny()), + ).captured.first as List<String>; + expect(capturedPinnedIds, unorderedEquals(['msg-1']));packages/stream_chat/lib/src/client/channel.dart (3)
3494-3498: Safe message merging into channel state._mergeMessagesIntoExisting with stable sort is clear. Watch for O(n log n) on very large merges; acceptable here.
If hot, consider skipping sort when toMerge is already >= last createdAt.
3800-3805: Minor: unify timestamp source.You use DateTime.timestamp() elsewhere; here you use DateTime.now() for deletedAt/state. Consider using one consistently.
- deletedAt: deletedAt ?? DateTime.now(), + deletedAt: deletedAt ?? DateTime.timestamp(),
4139-4154: Channel-scoped user.messages.deleted listener is correct.Ignores null user, applies hard/soft via unified path. Consider logging count of affected messages for observability.
packages/stream_chat/lib/src/client/client.dart (1)
2396-2412: Global-to-channel event fan-out is sound.Skips events already scoped to a cid, iterates cached channels, and reuses handleEvent so per-channel filters apply. Consider short-circuiting to only channels where the user is (recent) member to reduce no-op work on large caches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
packages/stream_chat/CHANGELOG.md(1 hunks)packages/stream_chat/lib/src/client/channel.dart(7 hunks)packages/stream_chat/lib/src/client/client.dart(5 hunks)packages/stream_chat/lib/src/core/util/extension.dart(2 hunks)packages/stream_chat/lib/src/db/chat_persistence_client.dart(2 hunks)packages/stream_chat/lib/src/event_type.dart(1 hunks)packages/stream_chat/test/src/client/channel_test.dart(1 hunks)packages/stream_chat/test/src/client/client_test.dart(6 hunks)packages/stream_chat/test/src/db/chat_persistence_client_test.dart(2 hunks)packages/stream_chat_persistence/CHANGELOG.md(1 hunks)packages/stream_chat_persistence/lib/src/dao/message_dao.dart(2 hunks)packages/stream_chat_persistence/lib/src/dao/pinned_message_dao.dart(2 hunks)packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart(1 hunks)packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart(2 hunks)packages/stream_chat_persistence/test/src/dao/message_dao_test.dart(2 hunks)packages/stream_chat_persistence/test/src/dao/pinned_message_dao_test.dart(2 hunks)packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message with MessageSendingStatus.failed or MessageSendingStatus.failed_update status, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.
Applied to files:
packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dartpackages/stream_chat/lib/src/db/chat_persistence_client.dartpackages/stream_chat_persistence/lib/src/dao/message_dao.dartpackages/stream_chat/lib/src/client/channel.dart
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message that hasn't been sent to the server yet (message.remoteCreatedAt == null) or is bounced with error, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.
Applied to files:
packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dartpackages/stream_chat/lib/src/db/chat_persistence_client.dartpackages/stream_chat_persistence/lib/src/dao/message_dao.dartpackages/stream_chat/lib/src/client/channel.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build (ios)
- GitHub Check: test
- GitHub Check: build (android)
- GitHub Check: format
🔇 Additional comments (26)
packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart (1)
60-60: LGTM! Schema version bumped appropriately.The schema version increment aligns with the new persistence features introduced in this PR (user message deletion and message limit support).
packages/stream_chat/lib/src/event_type.dart (1)
193-194: LGTM! Event type constant added correctly.The new
userMessagesDeletedevent type follows the established naming convention and is properly documented.packages/stream_chat/lib/src/core/util/extension.dart (2)
75-86: LGTM! Clean delegation pattern.The
mergemethod provides a convenient API for same-type merging by delegating to the more generalmergeFrommethod with an identity mapper.
142-164: LGTM! Well-implemented merge utility with excellent documentation.The
mergeFrommethod correctly handles null values, skips items when the value mapper returns null, and properly merges items by key. The comprehensive documentation with examples makes the API easy to understand.packages/stream_chat/test/src/db/chat_persistence_client_test.dart (2)
52-58: LGTM! Test stub properly defined.The
deleteMessagesFromUserstub correctly throwsUnimplementedError, which is appropriate for a test mock that's not meant to be called in this test suite.
90-95: LGTM! Signature updated to match new API.The
messageLimitparameter addition aligns with the new per-channel message limiting feature.packages/stream_chat/CHANGELOG.md (1)
1-5: LGTM! Changelog entry is clear and well-formatted.The entry correctly documents the new
user.messages.deletedevent support and follows the established changelog format.packages/stream_chat_persistence/CHANGELOG.md (1)
1-8: LGTM! Changelog entries are comprehensive and well-documented.Both new features are clearly described:
- The new
deleteMessagesFromUser()method for user-level deletion- The
messageLimitparameter for controlling per-channel message paginationpackages/stream_chat_persistence/lib/src/dao/message_dao.dart (1)
248-281: Code implementation is correct — no changes needed.The
MessageState.softDeletedis properly serializable through the@freezedpackage's auto-generatedtoJson()method. UsingjsonEncode(MessageState.softDeleted)in the soft delete operation is correct and consistent with existing codebase patterns.packages/stream_chat/test/src/client/client_test.dart (1)
4114-4249: Excellent end-to-end coverage for user.ws deletionsAppreciate how the new tests exercise both soft and hard delete flows across multiple channels, mirroring the production
ClientState._listenUserMessagesDeletedbehavior. That should give us strong regression protection for the global event fan-out.packages/stream_chat/lib/src/db/chat_persistence_client.dart (1)
133-193: API surface evolves cleanlyThe additional
messageLimitknob and the newdeleteMessagesFromUsercontract slot neatly into the abstraction—nice job tightening the persistence API without breaking cohesion.packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart (1)
214-236: Delegation stays DRY and consistentGreat to see
deleteMessagesFromUserfan out through both message and pinned-message DAOs with a shared parameter set viaFuture.wait; keeps the orchestration lean and consistent.packages/stream_chat_persistence/test/src/dao/pinned_message_dao_test.dart (1)
431-608: Pinned-message delete paths thoroughly exercisedThese specs do a solid job validating both scoped and global deletions, including soft-delete metadata—exactly what we need to guard the new DAO logic.
packages/stream_chat_persistence/test/src/dao/message_dao_test.dart (1)
440-634: Great breadth on message DAO coverageCovering per-channel, global, and threaded cases (both hard and soft) gives high confidence the DAO implementation handles every path we care about.
packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart (1)
334-945: Integration tests nail the delegation contractsThe mock-based assertions around
deleteMessagesFromUserand the updated expectations for per-channel message limits nicely validate the orchestration layer.packages/stream_chat/lib/src/client/channel.dart (9)
2339-2339: Good addition: channel-level listener bootstrapped.Wires up user.messages.deleted handling at channel state creation. Looks correct.
3210-3210: Update/remove paths centralized.Delegating update/remove/delete to unified helpers reduces drift and keeps persistence/thread/pin/location state consistent. Nice.
Also applies to: 3218-3219, 3222-3223, 3226-3227
3597-3609: Thread merge path is consistent with channel path.Maintains order and uses the same merge semantics. Good.
3813-3821: lastMessageAt behavior on deletions — confirm UX.soft delete updates messages in place and recalculates lastMessageAt only via max(existing, createdAt). If the latest message becomes deleted, lastMessageAt won’t regress. Is this the intended UX? If not, consider recomputing from last non-deleted message.
Also applies to: 3855-3888
3823-3831: Solid fan-out: threads, channel, pins, live locations.Update helpers correctly touch all affected collections and rely on a single merge function. Pin invalidation handled later. Good.
Also applies to: 3832-3853, 3890-3902, 3904-3916
3918-3944: Live-location cleanup on message changes.Removes expired or deleted-attached locations. Correct keying and filters.
3946-3954: Merge semantics preserve quotedMessage and sync fields.updated.syncWith(original) and quotedMessage preservation prevent UI flicker and dangling refs. Nice.
Also applies to: 3956-3988
3990-4003: Removals keep persistence and references in sync.Pinned, threads, channel list, and locations are all pruned; persistence cleared via DAO calls. Looks robust.
Also applies to: 4005-4037, 4039-4063, 4065-4091, 4093-4106, 4108-4116, 4118-4137
4173-4185: Pin validity guard updated for deleted messages.Treating deleted as invalid pin is the right call. 👍
packages/stream_chat/lib/src/client/client.dart (2)
657-657: messageLimit forwarded consistently (default 25).Good parity between online/offline code paths; avoids surprising page sizes.
Also applies to: 738-740, 780-795
2281-2281: Subscribed to user.messages.deleted at client level.Ensures propagation is active once websocket connects. Good placement.
| final message1 = Message( | ||
| id: 'msg-1', | ||
| text: 'Message from user 1', | ||
| user: user1, | ||
| ); | ||
| final message2 = Message( | ||
| id: 'msg-2', | ||
| text: 'Another message from user 1', | ||
| user: user1, | ||
| ); | ||
| final message3 = Message( | ||
| id: 'msg-3', | ||
| text: 'Message from user 2', | ||
| user: user2, | ||
| ); |
There was a problem hiding this comment.
Pinned deletion verification is incorrect; make a message pinned and assert only pinned IDs are sent.
The test expects deletePinnedMessageByIds to be called with all message IDs, even though none are pinned. This likely fails against an implementation that deletes pinned entries only for pinned messages. Pin one message and assert pinned IDs specifically; also capture lists to avoid order flakiness.
Apply this diff:
@@
- final message1 = Message(
+ final message1 = Message(
id: 'msg-1',
text: 'Message from user 1',
user: user1,
);
- final message2 = Message(
+ final message2 = Message(
id: 'msg-2',
text: 'Another message from user 1',
- user: user1,
+ user: user1,
+ pinned: true, // make one message pinned
);
@@
- // Verify messages are removed from persistence
- verify(
- () => persistenceClient.deleteMessageByIds(['msg-1', 'msg-2']),
- ).called(1);
- verify(
- () =>
- persistenceClient.deletePinnedMessageByIds(['msg-1', 'msg-2']),
- ).called(1);
+ // Verify messages are removed from persistence (order-independent)
+ final capturedMsgIds = verify(
+ () => persistenceClient.deleteMessageByIds(captureAny()),
+ ).captured.first as List<String>;
+ expect(capturedMsgIds, unorderedEquals(['msg-1', 'msg-2']));
+
+ // Only pinned IDs should be sent to pinned deletion
+ final capturedPinnedIds = verify(
+ () => persistenceClient.deletePinnedMessageByIds(captureAny()),
+ ).captured.first as List<String>;
+ expect(capturedPinnedIds, unorderedEquals(['msg-2']));Also applies to: 6926-6933
Submit a pull request
Fixes: FLU-266
Description of the pull request
Adds support for the
user.messages.deletedWebSocket event.This event is now handled at the client level and propagated to all active channels. Each channel will then proceed to either soft or hard delete all messages from the specified user, depending on the event payload.
Refactors message update and removal logic to support bulk operations, improving performance and maintainability.
Summary by CodeRabbit
Release Notes
New Features
Improvements