ChannelLogic refactor (part 1) and event handling bug fixes
#6038
Conversation
… events and mark deprecated properties
SDK Size Comparison 📏
|
VelikovPetar
changed the title
ChannelLogic refactor (part 1)ChannelLogic refactor (part 1) and event handling bug fixes
WalkthroughIntroduces a ChannelEventHandler and ChannelLogicImpl, converts ChannelLogic to an interface, switches stateLogic access from function to property, renames toggleHidden→setHidden, simplifies typing handling, adds ThreadInfo test factories, updates tests, and appends changelog entries. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ChannelLogicImpl
participant ChannelEventHandler
participant ChannelStateLogic
participant ChannelMutableState
Caller->>ChannelLogicImpl: handleEvent(event)
ChannelLogicImpl->>ChannelEventHandler: handle(event)
alt New message / notification
ChannelEventHandler->>ChannelStateLogic: upsertMessage(message)
ChannelStateLogic->>ChannelMutableState: upsertMessage(message)
else Message update
ChannelEventHandler->>ChannelStateLogic: updateMessage(message)
ChannelStateLogic->>ChannelMutableState: updateMessage(message)
else Message delete
ChannelEventHandler->>ChannelStateLogic: deleteMessage(message)
ChannelStateLogic->>ChannelMutableState: deleteMessage(message)
else Visibility change
ChannelEventHandler->>ChannelStateLogic: setHidden(flag)
ChannelStateLogic->>ChannelMutableState: setHidden(flag)
else Typing / presence / reads
ChannelEventHandler->>ChannelStateLogic: updateTypingEvent / updateRead / updateDelivered(...)
ChannelStateLogic->>ChannelMutableState: apply respective updates
end
ChannelMutableState-->>ChannelStateLogic: state changed
ChannelStateLogic-->>ChannelLogicImpl: acknowledge
ChannelLogicImpl-->>Caller: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (1)
75-86: Consider documenting thegetCurrentUserIdparameter.The KDoc documents
cidandstateLogicbut omitsgetCurrentUserId. As per coding guidelines, public APIs should include complete KDoc.🔎 Suggested documentation improvement:
/** * Class responsible for updating the local database based on channel-related events. * * @param cid The channel identifier. * @param stateLogic The [ChannelStateLogic] instance used to update the channel state. - * @param getCurrentUserId Returns the currently logged in user ID. + * @param getCurrentUserId A function that returns the currently logged in user ID, or null if not logged in. */CHANGELOG.md (1)
37-39: Polish wording for clarity in the new fixed itemsThe current bullets are understandable but could read more clearly in English. Consider tightening them slightly:
Suggested text tweaks
- Fix `notification.thread_message_new` event upserting the message in the channel even when not sent to channel. [#6038](...) + Fix `notification.thread_message_new` event upserting the message in the channel even when it was not sent to the channel. [#6038](...) - Fix `message.updated` and `message.deleted` events unhiding the channel. [#6038](...) + Fix `message.updated` and `message.deleted` events unintentionally unhiding the channel. [#6038](...) - Fix inserting instead of updating a message breaking pagination. [#6038](...) + Fix inserting a message instead of updating it, which was breaking pagination. [#6038](...)stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
296-296: Consider defaulting thread to null for backward compatibility.The default
thread = randomThreadInfo()means tests that don't explicitly setthreadwill now receive a randomThreadInfo. This could cause unexpected behavior in existing tests that assume a channel-level read event (no thread).For example, in
WhenHandleEvent.ktline 245, the test explicitly setsthread = nullto work around this. Consider usingthread: ThreadInfo? = nullas the default to maintain backward compatibility.🔎 Suggested change:
public fun randomNotificationMarkReadEvent( createdAt: Date = Date(), user: User = randomUser(), cid: String = randomCID(), channelType: String = randomString(), channelId: String = randomString(), totalUnreadCount: Int = randomInt(), unreadChannels: Int = randomInt(), lastReadMessageId: String? = randomString(), threadId: String? = randomString(), - thread: ThreadInfo? = randomThreadInfo(), + thread: ThreadInfo? = null, unreadThreads: Int? = randomInt(), unreadThreadMessages: Int? = randomInt(), ): NotificationMarkReadEvent = NotificationMarkReadEvent(stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (1)
226-268: Consider adding error handling for gap-filling coroutine.The
coroutineScope.launchblock lacks error handling. If the recursivefillTheGapcall orrunChannelQueryOnlinefails, the error is silently swallowed. Consider adding aCoroutineExceptionHandleror logging failures.🔎 Suggested improvement:
private fun fillTheGap( messageLimit: Int, loadedMessages: List<Message>, requestedMessages: List<Message>, ) { if (loadedMessages.isEmpty() || requestedMessages.isEmpty() || messageLimit <= 0) return coroutineScope.launch { + try { val loadedMessageIds = loadedMessages .filter { it.getCreatedAtOrNull() != null } .sortedBy { it.getCreatedAtOrDefault(NEVER) } .map { it.id } // ... rest of the logic - }?.onSuccess { fillTheGap(messageLimit, loadedMessages, it.messages) } + }?.onSuccess { fillTheGap(messageLimit, loadedMessages, it.messages) } + } catch (e: Exception) { + logger.e(e) { "[fillTheGap] Failed to fill message gap" } + } } }
📜 Review details
Configuration used: defaults
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 (26)
CHANGELOG.md(1 hunks)stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt(4 hunks)stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt(2 hunks)stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt(2 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.kt(4 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerState.kt(2 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.kt(3 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt(2 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt(3 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.kt(4 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt(3 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt(8 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.kt(5 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.kt(4 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.ktstream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented
Document public APIs with KDoc, including thread expectations and state notes
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.ktstream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt
**/src/test/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/src/test/**/*.kt: Use backtick test names (for example:funmessage list filters muted channels()) for readability
Use deterministic tests withrunTest+ virtual time for concurrency-sensitive logic (uploads, sync, message state)
Keep helper extensions private/internal in test files
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.kt
🧠 Learnings (4)
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-ui-components/**/*Test.kt : Record Shot baselines when behaviour changes in XML kit UI tests
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use deterministic tests with `runTest` + virtual time for concurrency-sensitive logic (uploads, sync, message state)
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*Test.kt : Add Paparazzi snapshots for Compose UI regressions and run `verifyPaparazziDebug`
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use backtick test names (for example: `fun `message list filters muted channels`()`) for readability
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
🧬 Code graph analysis (4)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt (1)
stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
randomNotificationMarkReadEvent(286-314)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (1)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (2)
handleEvent(171-171)updateDataForChannel(150-157)
stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (1)
randomThreadInfo(1008-1036)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (3)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (1)
upsertMessage(131-131)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (1)
upsertMessage(334-341)stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/ChannelMessagesUpdateLogic.kt (2)
upsertMessage(23-40)upsertMessage(26-26)
⏰ 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). (5)
- GitHub Check: API check
- GitHub Check: compare-sdk-sizes / Compare SDK sizes
- GitHub Check: Unit tests
- GitHub Check: Debug build
- GitHub Check: Build / compose apks
🔇 Additional comments (45)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.kt (4)
337-337: LGTM! Clean refactoring to new loader method.The change from the old loader to
loadBefore(messageId = null, limit = messageLimit)is correct. Passingnullas the messageId appropriately indicates loading from the oldest available messages.
358-358: LGTM! Correct delegation to new loader method.The change to
loadAfter(messageId = baseMessageId, limit = messageLimit)correctly delegates to the new loader implementation, maintaining the same behavior.
383-383: LGTM! Appropriate refactoring to new loader.The change to
loadAround(messageId)correctly aligns with the new loader naming convention while preserving the function's behavior.
477-477: LGTM! Internal refactoring to new loader.The change from calling
loadMessagesAroundIdto callingloadAround(messageId)directly is a clean refactoring that maintains the same behavior while eliminating an unnecessary indirection.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.kt (1)
79-79: Centralized error handling is properly implemented.The delegation to
logic.channel(...).onQueryChannelResult(request, result)successfully centralizes result handling. The implementation inChannelLogicImplhandles bothResult.SuccessandResult.Failurecases, delegating tochannelStateLogic.propagateChannelQuery()andchannelStateLogic.propagateQueryError()respectively, maintaining full behavioral consistency with the previous explicit implementation.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.kt (1)
72-79: LGTM!The added comment clarifies why
lastReadMessageIdis explicitly set tonullforMarkAllReadEvent, which is helpful for maintainability.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerState.kt (2)
65-67: LGTM!The property access pattern change is consistent with the refactored
ChannelLogicinterface.
48-50: ThestateLogicaccess patterns are correctly implemented.ChannelLogicexposesstateLogicas a property (accessed without parentheses), whileThreadLogicexposesstateLogicas a function (called with parentheses). The code correctly reflects these different interface definitions.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (3)
213-214: LGTM!The deprecation annotation clearly communicates that
oldMessageswill be removed in future versions, giving consumers time to migrate.
428-430: LGTM!The simplified
updateTypingEventmethod with a single parameter is cleaner than the previous multi-argument version.
540-552: Key fix for pagination integrity.This guarded update correctly distinguishes between upsert (insert or update) and update-only operations. By skipping updates for non-existent messages, this prevents
message.updatedandmessage.deletedevents from inadvertently inserting out-of-order messages that would break pagination.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (4)
149-172: Core bug fix: UsingupdateMessageinstead ofupsertMessagefor update/delete events.This correctly distinguishes between events that should insert messages (
message.new,notification.message_new,notification.thread_message_new) and those that should only update existing messages (message.updated,message.deleted). This prevents pagination breakage when updates introduce out-of-order messages.
174-183: Fix fornotification.thread_message_newhandling.Correctly guards upsert with
showInChannelcheck, ensuring thread-only replies don't appear in the main channel message list. The hidden state update is also correctly guarded by!shadowed.
300-321: Well-designed message update helpers.The
upsertMessagehelper preservescreatedLocallyAtfor proper message sorting and retainsownReactions. TheupdateMessagehelper correctly returns early if the message doesn't exist, preventing phantom message insertions.
243-253: Thread-scoped read events correctly filtered.Read events are properly filtered to exclude thread-scoped reads (
event.thread == null), ensuring channel-level read state isn't affected by thread reads.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt (2)
63-64: LGTM!The deprecation annotation on the public interface aligns with the implementation in
ChannelMutableState.kt.
135-136: LGTM!Good catch fixing the documentation typo from "performatic" to "performant".
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.kt (1)
56-56: LGTM: Idiomatic constructor declaration.Removing the explicit
constructorkeyword is standard Kotlin style when there are no annotations or visibility modifiers on the primary constructor itself.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.kt (1)
42-42: LGTM: Mock setup correctly reflects the API change.The mock configuration properly reflects the change from
stateLogic()function tostateLogicproperty, consistent with the broader ChannelLogic refactor.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.kt (1)
53-53: LGTM: Property access aligns with the refactored API.The change from function call to property access is correct and consistent with the ChannelLogic interface evolution throughout this PR.
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.kt (1)
32-32: LGTM! Clean refactoring to interface-based design.The changes correctly implement the ChannelLogic interface pattern:
- Import of ChannelLogicImpl (Line 32)
- Instantiation using ChannelLogicImpl while returning ChannelLogic interface (Lines 119-127)
- Property-based access to stateLogic instead of function calls (Lines 135, 207)
This aligns with the PR's goal of extracting ChannelLogic as an interface with ChannelLogicImpl as the concrete implementation.
Also applies to: 119-127, 135-136, 207-208
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.kt (1)
57-57: LGTM! API improvements applied correctly.The changes correctly apply the refactored API:
- Property access for stateLogic (Lines 57, 74)
- Renamed method from
toggleHidden()tosetHidden()(Lines 57, 85) - the new name is more explicit about the operationAlso applies to: 74-74, 85-85
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.kt (1)
50-50: LGTM! Test mocks updated for property-based access.All mock setups correctly changed from
on(it.stateLogic())toon(it.stateLogic)to match the new property-based API.Also applies to: 89-89, 129-129, 169-169, 209-209
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt (1)
210-210: LGTM! Test assertion updated for simplified typing API.The verification correctly updated from
updateTypingEvents(...)(plural, two arguments) toupdateTypingEvent(...)(singular, one argument), reflecting the API simplification in ChannelMutableState.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.kt (1)
45-45: LGTM! Mock setup updated for property-based access.The mock correctly changed from
on(it.stateLogic())toon(it.stateLogic)to align with the new property-based API.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.kt (1)
41-41: LGTM! Null-safe property access applied correctly.The null-safe chain correctly updated from
?.stateLogic()?to?.stateLogic?while maintaining proper null handling.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.kt (1)
40-40: LGTM! Test updated for both API changes.The test correctly reflects both refactoring changes:
- Mock setup uses property access:
on(it.stateLogic)(Line 40)- Verifications use renamed method:
setHidden(true/false)(Lines 51, 63, 75)Also applies to: 51-51, 63-63, 75-75
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.kt (1)
54-60: LGTM! Test correctly uses concrete implementation.The test properly instantiates
ChannelLogicImplwith all required dependencies while keeping thesutvariable typed asChannelLogicinterface. This is good practice for testing the implementation while maintaining interface-based design.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt (6)
91-94: LGTM: Mock delegation for upsertMessage is correctly configured.The stub bridges
channelStateLogic.upsertMessage()tochannelMutableState.upsertMessage(), ensuring the mock propagates messages to state for tests that verify message presence.
105-110: LGTM: Updated to use ChannelLogicImpl.The test correctly instantiates the new
ChannelLogicImplconcrete implementation, aligning with the refactor that extractsChannelLogicas an interface.
134-134: LGTM: Reflects API rename from toggleHidden to setHidden.The verification correctly uses the renamed method
setHidden(false).
153-154: Correct distinction between upsert and update operations.The test now verifies that:
- Initial message insertion calls
upsertMessage(times(1))- Subsequent update event calls
updateMessage(not upsert)This aligns with the PR's bug fix to prevent pagination breakage when updates introduced out-of-order messages.
177-181: LGTM: Poll preservation tests updated to verify updateMessage.Both poll preservation tests correctly verify that message updates (with or without polls) use
updateMessagerather thanupsertMessage, consistent with the event handling refinement.Also applies to: 205-209
245-245: LGTM: Explicit thread = null for channel-level read events.Setting
thread = nullexplicitly clarifies this is a channel-level (not thread-level) read event, making the test intent clearer.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (3)
191-198: LGTM: New updateMessage method for in-place message updates.This method correctly delegates to
mutableState.updateMessage(), providing a distinct path fromupsertMessage(). This supports the PR's fix to prevent pagination breakage by not inserting out-of-order messages during updates.
582-586: LGTM: Renamed toggleHidden to setHidden with improved documentation.The method name
setHiddenis more accurate since it sets a specific value rather than toggling. The parameter documentation is also improved from "Boolean." to "Whether the channel is hidden."
303-308: Simplification of typing state update is correct.The
updateTypingEventmethod inChannelMutableStateproperly handles internal state management by updating the_typingmutable state flow. The parameterrawTypingEventsis not needed since thetypingEventobject contains all required information for state updates. Both channel-local and global typing state are correctly updated viamutableState.updateTypingEvent()andglobalMutableState.tryEmitTypingEvent().stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
71-71: LGTM: Imports added for ThreadInfo support.The imports correctly bring in
ThreadInfoandrandomThreadInfoto support the new parameter inrandomNotificationMarkReadEvent.Also applies to: 90-90
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (2)
1008-1036: LGTM: Well-structured randomThreadInfo factory function.The factory follows the established pattern of other
random*functions in this file:
- All parameters have sensible defaults using existing random helpers
- Parameters correctly map to
ThreadInfoconstructor- Public visibility is appropriate for test fixtures
As per coding guidelines for test files, this helper correctly uses
publicvisibility for shared test utilities.
62-62: LGTM: Import added for ThreadInfo model.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (5)
43-58: LGTM: Well-documented class with clear responsibilities.The KDoc clearly explains the purpose and dependencies. The class properly implements
ChannelLogicinterface and encapsulates channel state management.
60-62: Initialization order is correct but subtle.The initialization relies on
mutableStatebeing initialized first (line 60), theneventHandlerusescid(line 61) which delegates tomutableState.cid. This works because Kotlin initializes properties in declaration order.
172-183: LGTM: Event handling delegated to ChannelEventHandler.The refactor correctly delegates event handling to the new
ChannelEventHandler, keepingChannelLogicImplfocused on channel operations while centralizing event-to-state mapping.
100-118: LGTM: Watch method with proper loading guard.The loading guard at line 103 prevents duplicate watch requests, which protects against UI bugs that could DDOS the API. Good defensive programming.
213-216: Singleton architecture is intentional per Stream Chat design; testing requires mocking tools like MockK.
ChatClient.instance()is the documented, intended pattern for this SDK—ChatClient is a singleton: you'll create it once and re-use it across your application. The coupling is architecturally justified for state consistency, avoiding duplicate connections, and simplifying resource management. Testing this requires mock tools (MockK, Mockito with extensions for Kotlin finals), which is standard practice in this codebase. The code meets all Kotlin style guidelines: proper indentation, no wildcard imports, license header present, and KDoc documentation included.
...in/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
Outdated
Show resolved
Hide resolved
...in/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
Outdated
Show resolved
Hide resolved
.../src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt
Outdated
Show resolved
Hide resolved
...ava/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (1)
318-327: Potential inconsistency betweengetMessageandmutableState.getMessageById.The
getMessagehelper on line 326 only looks up messages fromvisibleMessages, while other code paths (lines 150, 155, 331, 336) usemutableState.getMessageById(). This inconsistency could causeupdateMessageto skip updates for messages that exist in the full message state but are not currently visible (e.g., during search or pagination).Consider using
mutableState.getMessageById()for consistency:🔎 Proposed fix
private fun getMessage(id: String): Message? { - return mutableState.visibleMessages.value[id]?.copy() + return mutableState.getMessageById(id)?.copy() }
📜 Review details
Configuration used: Repository 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 (2)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented
Document public APIs with KDoc, including thread expectations and state notes
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
**/src/test/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/src/test/**/*.kt: Use backtick test names (for example:funmessage list filters muted channels()) for readability
Use deterministic tests withrunTest+ virtual time for concurrency-sensitive logic (uploads, sync, message state)
Keep helper extensions private/internal in test files
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
🧠 Learnings (5)
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-ui-components/**/*Test.kt : Record Shot baselines when behaviour changes in XML kit UI tests
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use deterministic tests with `runTest` + virtual time for concurrency-sensitive logic (uploads, sync, message state)
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*Test.kt : Add Paparazzi snapshots for Compose UI regressions and run `verifyPaparazziDebug`
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use backtick test names (for example: `fun `message list filters muted channels`()`) for readability
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*.kt : Compose components should follow noun-based naming (e.g., `MessageList`, `ChannelListHeader`)
Applied to files:
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
🧬 Code graph analysis (2)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt (2)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (9)
randomCID(102-102)randomString(95-99)randomMessage(303-400)randomUser(183-229)randomPoll(866-908)randomDate(708-708)randomChannel(424-484)randomReaction(241-267)randomMember(652-678)stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (6)
randomNewMessageEvent(516-542)randomNotificationMessageNewEvent(434-454)randomMemberAddedEvent(390-408)randomNotificationRemovedFromChannelEvent(664-682)randomMessageReadEvent(246-264)randomChannelUserBannedEvent(702-721)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (3)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (2)
upsertMessage(115-115)getMessage(108-108)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (2)
upsertMessage(334-341)updateMessage(196-198)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (2)
upsertMessage(536-538)updateMessage(545-552)
⏰ 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). (6)
- GitHub Check: Build / compose apks
- GitHub Check: compare-sdk-sizes / Compare SDK sizes
- GitHub Check: API check
- GitHub Check: Debug build
- GitHub Check: Unit tests
- GitHub Check: ktlint
🔇 Additional comments (5)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt (5)
99-120: LGTM!Test class setup is well-structured with proper mocking. The use of backtick test names follows coding guidelines, and the
@Suppress("LargeClass")annotation is appropriate given the comprehensive coverage needed.
122-228: LGTM!Comprehensive test coverage for
NewMessageEventandMessageUpdatedEventscenarios, including edge cases like preservingcreatedLocallyAtfor current user messages, poll enrichment, and handling non-existing messages.
229-324: LGTM!Good coverage of
MessageDeletedEvent(hard vs soft delete) andNotificationMessageNewEvent(inside search vs not) scenarios. The tests correctly verify the distinction betweenupsertMessageandupdateMessagecalls per PR objectives.
326-938: LGTM!Excellent comprehensive coverage across all event types: thread messages, reactions, members, watchers, channel updates, typing, read/delivery receipts, invitations, bans, and polls. The tests correctly verify edge cases like thread-specific reads not updating channel read state (lines 704-720).
940-1096: LGTM!Complete test coverage for reminder events, user presence, mute updates, and message deletion scenarios. The tests properly verify edge cases like operations on non-existing messages.
|
🎯 Goal
Part 1 of the
ChannelLogic/JumpToMessagerefactoring.Sets the baseline on which the new and optimised channel logic will be built:
ChannelLogicinterface with the common operations (to be implemented by the current and the new implementation)ChannelLogicinterface (now namedChannelLogicImpl)ChannelLogicto a separateChannelEventHandlerclassAdditionally, includes couple of bug-fixes related to event handling in the channel logic.
notification.thread_message_newhandling: Upsert the message andshowthe channel only if it was sent to channel as wellmessage.updated/message.deletedshowing the channel (this event doesn't update the channel visibility)message.new,notification.message_newandnotification.thread_message_new. Every other event is handled via an update. We are currently ALWAYS upserting the message, and this can severely break the pagination in following scenario:message.updatedevent for a message in a page which is not yet loaded🛠 Implementation details
ChannelLogicand its implementationChannelLogicImplChannelEventHandler🎨 UI Changes
NA
🧪 Testing
Test various scenarios related to channel / message list: pagination, reactions, quoting, pinning, updating, threading - everything should work as expected.
Summary by CodeRabbit
Bug Fixes
Documentation
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.