Fix passing outdated Channel data for CidEvents to ChatEventHandler.handleChatEvent#6385
Open
VelikovPetar wants to merge 1 commit intodevelopfrom
Open
Conversation
…he `ChatEventHandler.handleChatEvent`. Co-Authored-By: Claude <noreply@anthropic.com>
VelikovPetar
changed the title
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
VelikovPetar
changed the title
Channel data for CidEvents to ChatEventHandler.handleChatEvent
Contributor
SDK Size Comparison 📏
|
|
WalkthroughThe changes optimize channel event handling by prioritizing in-memory active channel state lookup over database queries. A new helper method Changes
Sequence Diagram(s)sequenceDiagram
participant Event as Chat Event
participant Parser as parseChatEventResults
participant Registry as State Registry<br/>(In-Memory)
participant DB as Database
participant Handler as Event Handler
Event->>Parser: CidEvent(s)
par For each CID
Parser->>Registry: getActiveChannelState(cid)<br/>check if active
alt Channel is Active
Registry-->>Parser: Channel (from memory)
Parser->>Handler: handleChatEvent(Channel)
else Channel Not Active
Registry-->>Parser: null
Parser->>DB: selectChannels([cid])
DB-->>Parser: Channel (from DB)
Parser->>Handler: handleChatEvent(Channel)
end
end
Handler-->>Parser: EventHandlingResult
Parser-->>Event: Combined Results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogic.kt (1)
247-255: Minor: method name implies aChannelStatebut returnsChannel.
getActiveChannelStatereturns aChannelsnapshot, not aChannelState. ConsidergetActiveChannel(orgetActiveChannelSnapshot) for clarity—Statein the name suggests the liveChannelStateflow. Not a blocker; the implementation is correct and aligns with the existingrefreshChannelspattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogic.kt` around lines 247 - 255, The method getActiveChannelState is misnamed because it returns a Channel snapshot rather than a ChannelState; rename it to getActiveChannel or getActiveChannelSnapshot for clarity, update its declaration and all references/usages accordingly (e.g., callers expecting getActiveChannelState), and ensure documentation/comments reflect that it returns a Channel snapshot from stateRegistry.channel(type, id).toChannel() and not a live ChannelState flow.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogicTest.kt (1)
154-161: Optional: also assert short-circuit when channel is inactive.The test verifies the
nullreturn value but doesn't confirm thatstateRegistry.channel(...)is not invoked whenisActiveChannelreturnsfalse. Adding averify(stateRegistry, never()).channel(any(), any())would lock in the short-circuit guarantee and protect against future regressions where someone might accidentally invert the order of operations.♻️ Proposed refinement
`@Test` fun `getActiveChannelState should return null when channel is not active in state registry`() { whenever(stateRegistry.isActiveChannel(type, id)) doReturn false val result = queryChannelsStateLogic.getActiveChannelState(testCid) assertNull(result) + verify(stateRegistry, never()).channel(any(), any()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogicTest.kt` around lines 154 - 161, The test getActiveChannelState should also assert the short-circuit behavior: after setting whenever(stateRegistry.isActiveChannel(type, id)) doReturn false and calling queryChannelsStateLogic.getActiveChannelState(testCid), add a verification that stateRegistry.channel(...) was never invoked (e.g., verify(stateRegistry, never()).channel(any(), any())) to ensure getActiveChannelState short-circuits; reference the methods isActiveChannel, channel, and getActiveChannelState and the testCid input when adding this assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogic.kt`:
- Around line 247-255: The method getActiveChannelState is misnamed because it
returns a Channel snapshot rather than a ChannelState; rename it to
getActiveChannel or getActiveChannelSnapshot for clarity, update its declaration
and all references/usages accordingly (e.g., callers expecting
getActiveChannelState), and ensure documentation/comments reflect that it
returns a Channel snapshot from stateRegistry.channel(type, id).toChannel() and
not a live ChannelState flow.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogicTest.kt`:
- Around line 154-161: The test getActiveChannelState should also assert the
short-circuit behavior: after setting
whenever(stateRegistry.isActiveChannel(type, id)) doReturn false and calling
queryChannelsStateLogic.getActiveChannelState(testCid), add a verification that
stateRegistry.channel(...) was never invoked (e.g., verify(stateRegistry,
never()).channel(any(), any())) to ensure getActiveChannelState short-circuits;
reference the methods isActiveChannel, channel, and getActiveChannelState and
the testCid input when adding this assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b3417c0e-d596-406b-aaa9-529ca6d2b277
📒 Files selected for processing (4)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogic.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogicTest.kt
gpunto
approved these changes
Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Ports V6 commit
1b7caac2a0("Fix passing outdatedChanneldata forCidEvents to theChatEventHandler.handleChatEvent. (#6381)") todevelop.This was a manual port because the files moved from
stream-chat-android-statetostream-chat-android-clientondevelop.The fix makes
QueryChannelsLogic.parseChatEventResultsprefer in-memory per-channel state (already updated by channel event handlers) over stale DB data when resolving channels forCidEvents passed toChatEventHandler.handleChatEvent.Implementation
QueryChannelsLogic.parseChatEventResults— Instead of always fetching channel data from DB, the method now first checks in-memory per-channel state viaStateRegistry. Only channels not currently active in memory fall back to DB lookup.QueryChannelsStateLogic.getActiveChannelState— New method that returns the currentChannelsnapshot from the in-memory state registry if the channel is active, ornullotherwise.parseChatEventResults(in-memory only, DB fallback, mixed resolution).getActiveChannelState(active → returns channel, inactive → returns null).UI Changes
No UI changes.
Testing
getActiveChannelStatetested for both active and inactive channels.Summary by CodeRabbit
Release Notes
Bug Fixes
Tests