Improve message.new event parsing memory footprint#6344
Open
VelikovPetar wants to merge 8 commits intodevelopfrom
Open
Improve message.new event parsing memory footprint#6344VelikovPetar wants to merge 8 commits intodevelopfrom
message.new event parsing memory footprint#6344VelikovPetar wants to merge 8 commits intodevelopfrom
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
VelikovPetar
changed the title
message.new event parsing memory footprint
|
WalkthroughThis pull request introduces a new direct-to-domain JSON parsing path that bypasses DTO deserialization. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MoshiChatParser
participant DirectEventParser
participant MoshiChatParser as MoshiChatParser (DTO Path)
participant DomainMapping
Client->>MoshiChatParser: parseAndProcessEvent(raw)
MoshiChatParser->>DirectEventParser: parse(raw)
alt Direct Parse Success
DirectEventParser->>DirectEventParser: extractType(raw)
DirectEventParser->>DirectEventParser: Select adapter by type
DirectEventParser->>DirectEventParser: Parse JSON to ChatEvent
DirectEventParser-->>MoshiChatParser: ChatEvent (non-null)
MoshiChatParser-->>Client: Return ChatEvent
else Direct Parse Fails
DirectEventParser-->>MoshiChatParser: null
MoshiChatParser->>MoshiChatParser (DTO Path): Deserialize to ChatEventDto
MoshiChatParser->>DomainMapping: toDomain()
MoshiChatParser->>MoshiChatParser (DTO Path): enrichIfNeeded()
MoshiChatParser-->>Client: Return ChatEvent
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Actionable comments posted: 7
🧹 Nitpick comments (11)
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ChannelTestData.kt (1)
252-427: Avoid shared mutable expected objects in singleton test data.These
valfixtures are global and include mutable state (mutableMapOf, nested mutable model fields). A mutation in one test can leak into others and create order-dependent failures. Prefer returning fresh instances per access (factory function or custom getter).♻️ Suggested refactor
- val expectedAllFields = Channel( + val expectedAllFields: Channel + get() = Channel( // ... - ) + ) - val expectedWithNestedCollections = Channel( + val expectedWithNestedCollections: Channel + get() = Channel( // ... - ) + ) - val expectedOptionalFieldsMissing = Channel( + val expectedOptionalFieldsMissing: Channel + get() = Channel( // ... - ) + )🤖 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/parser2/testdata/ChannelTestData.kt` around lines 252 - 427, The three global vals expectedAllFields, expectedWithNestedCollections, and expectedOptionalFieldsMissing expose shared mutable state (mutableMapOf and nested mutable collections like Message.reactionCounts/reactionScores) which can leak between tests; convert each fixture into a factory function or a property with a custom getter that constructs and returns a new Channel instance (e.g., fun expectedAllFields(): Channel { ... }) and ensure nested mutable collections are created anew (use mutableMapOf() inside the factory or use immutable mapOf()/emptyMap() if mutability is not required) so every test gets a fresh, independent object; update any references to these symbols accordingly.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/PushPreferenceTestData.kt (1)
26-42: Consider adding partial-field test cases.For more comprehensive coverage, you could add fixtures where only one optional field is present (e.g., only
chat_levelor onlydisabled_until). This would verify that the parser correctly handles mixed presence of optional fields.📝 Example additional test cases
`@Language`("JSON") val jsonOnlyChatLevel = """{"chat_level":"mentions"}""" val expectedOnlyChatLevel = PushPreference( level = PushPreferenceLevel.mentions, disabledUntil = null, ) `@Language`("JSON") val jsonOnlyDisabledUntil = """{"disabled_until":"2020-06-29T06:14:28.000Z"}""" val expectedOnlyDisabledUntil = PushPreference( level = null, disabledUntil = Date(1593411268000), )🤖 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/parser2/testdata/PushPreferenceTestData.kt` around lines 26 - 42, Add two partial-field fixtures to the PushPreference test data: one with only chat_level and one with only disabled_until. Create new JSON constants (e.g., jsonOnlyChatLevel and jsonOnlyDisabledUntil) and corresponding expected PushPreference instances (e.g., expectedOnlyChatLevel and expectedOnlyDisabledUntil) where the absent field is null; reuse PushPreferenceLevel.mentions and Date(1593411268000) for values to match existing expectedAllFields; place these alongside jsonAllFields/jsonOptionalFieldsMissing and expectedAllFields/expectedOptionalFieldsMissing so the parser tests cover mixed presence of optional fields.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ChannelUserReadTestData.kt (1)
46-77: Avoid sharing a mutableDateinstance across expected fixtures.Line 46 is reused in both expected objects (Lines 58 and 76). Because
Dateis mutable, this can introduce cross-test coupling.♻️ Suggested refactor
- val lastReceivedEventDate = Date(1744200000000L) + private const val LAST_RECEIVED_EVENT_DATE_MS = 1744200000000L @@ - lastReceivedEventDate = lastReceivedEventDate, + lastReceivedEventDate = Date(LAST_RECEIVED_EVENT_DATE_MS), @@ - lastReceivedEventDate = lastReceivedEventDate, + lastReceivedEventDate = Date(LAST_RECEIVED_EVENT_DATE_MS),🤖 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/parser2/testdata/ChannelUserReadTestData.kt` around lines 46 - 77, The tests share a single mutable Date instance lastReceivedEventDate between expectedAllFields and expectedOptionalFieldsMissing, which can cause cross-test coupling; fix by creating a separate Date for each fixture (e.g., inline new Date(1744200000000L) when constructing expectedAllFields and expectedOptionalFieldsMissing or use distinct vals like lastReceivedEventDateA and lastReceivedEventDateB) so ChannelUserRead's lastReceivedEventDate isn't a shared mutable object.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/AttachmentAdapter.kt (1)
26-27: Document or remove theLongMethodsuppression.Please either split
fromJsoninto smaller helpers or add a short rationale explaining why the suppression is intentionally kept for this hot path.♻️ Minimal documentation fix
- `@Suppress`("LongMethod") + // Intentional: kept as a single parse loop for hot-path allocation/perf characteristics. + `@Suppress`("LongMethod")As per coding guidelines:
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented.🤖 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/parser2/direct/AttachmentAdapter.kt` around lines 26 - 27, The `@Suppress`("LongMethod") on AttachmentAdapter.fromJson must be either removed by refactoring or explicitly documented; choose one: either split fromJson into smaller private helpers (e.g., parseAttachment(), parseAuthor(), parseExtraData()) inside class AttachmentAdapter and replace the long method with a short orchestrator, or keep the suppression but add a brief KDoc or inline comment above fromJson explaining why the long method is intentional for this hot path (mention performance/avoiding allocations) and reference the suppression rationale and a TODO to revisit — update only AttachmentAdapter.fromJson and its related private parsing helpers accordingly.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/ReactionAdapter.kt (1)
31-32: Please document theLongMethodsuppression here too.Same as
AttachmentAdapter: either refactor into helpers or add a short rationale for keeping this as one large hot-path parser method.♻️ Minimal documentation fix
- `@Suppress`("LongMethod") + // Intentional: kept as a single parse loop for hot-path allocation/perf characteristics. + `@Suppress`("LongMethod")As per coding guidelines:
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented.🤖 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/parser2/direct/ReactionAdapter.kt` around lines 31 - 32, The LongMethod suppression on ReactionAdapter.fromJson needs a short rationale or be refactored: either split the parsing logic in fromJson into small helper methods (e.g., parseReactionBase(), parseUser(), parseExtraFields()) and remove `@Suppress`("LongMethod"), or keep the suppression but add an explanatory comment above override fun fromJson(reader: JsonReader): Reaction? that states this is a performance-sensitive hot-path JSON parser and why consolidation is intentional; also ensure coding-guideline compliance by replacing undocumented suppressions with a documented rationale (or using explicit `@OptIn` where applicable) and reference the ReactionAdapter.fromJson symbol in your change.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/OptionParsingTest.kt (1)
80-106: Strengthen parity checks by asserting exception messages too.This block verifies only
JsonDataExceptiontype. To fully enforce parity, compare DTO/direct exception messages per missing-field case.Suggested test tightening
`@Test` -fun `DTO path - throws on missing id`() { - assertThrows<JsonDataException> { - parser.fromJson(OptionTestData.jsonMissingId, DownstreamPollOptionDto::class.java) - } -} - -@Test -fun `Direct path - throws on missing id`() { - assertThrows<JsonDataException> { - adapter.fromJson(OptionTestData.jsonMissingId) - } +fun `missing id - DTO and Direct paths have same error message`() { + val dtoException = assertThrows<JsonDataException> { + parser.fromJson(OptionTestData.jsonMissingId, DownstreamPollOptionDto::class.java) + } + val directException = assertThrows<JsonDataException> { + adapter.fromJson(OptionTestData.jsonMissingId) + } + assertEquals(dtoException.message, directException.message) }🤖 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/parser2/OptionParsingTest.kt` around lines 80 - 106, Update the tests to not only assert that JsonDataException is thrown but also verify the exception messages match between the DTO path and direct path: for the missing-id case capture the exception from parser.fromJson(OptionTestData.jsonMissingId, DownstreamPollOptionDto::class.java) and from adapter.fromJson(OptionTestData.jsonMissingId) and assert their message strings are equal (and non-empty), and do the same for the missing-text case using OptionTestData.jsonMissingText; keep the existing assertThrows style but store the caught exceptions and compare their message contents to ensure parity between parser.fromJson and adapter.fromJson.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/PollParsingTest.kt (1)
118-190: Consider assertingJsonDataExceptionmessages for true parity.These tests currently prove both paths throw, but not that they fail the same way. Comparing exception messages would better protect DTO/direct parity guarantees.
🤖 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/parser2/PollParsingTest.kt` around lines 118 - 190, The tests only assert that parser.fromJson and adapter.fromJson throw JsonDataException for missing fields but don't verify the exception messages are the same; update each pair of tests (e.g., `DTO path - throws on missing id` vs `Direct path - throws on missing id`, same for name, description, options, enforce_unique_vote) to capture the thrown JsonDataException from both parser.fromJson(PollTestData.jsonMissingX, DownstreamPollDto::class.java) and adapter.fromJson(PollTestData.jsonMissingX) and assert their messages are equal (and non-null), so that exception message parity between the DTO route and direct adapter route is enforced.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/NewMessageEventParsingTest.kt (1)
151-249: Strengthen error-parity checks by asserting exception messages too.Current tests verify only
JsonDataExceptiontype. Message-level mismatches between DTO and direct paths could slip through unnoticed.♻️ Suggested test helper refactor
+ private fun assertBothPathsThrowSameJsonDataException(json: String) { + val dtoEx = assertThrows<JsonDataException> { + parser.fromJson(json, NewMessageEventDto::class.java) + } + val directEx = assertThrows<JsonDataException> { + adapter.fromJson(json) + } + assertEquals(dtoEx.message, directEx.message) + }- `@Test` - fun `DTO path - throws on missing type`() { - assertThrows<JsonDataException> { - parser.fromJson(NewMessageEventTestData.jsonMissingType, NewMessageEventDto::class.java) - } - } - - `@Test` - fun `Direct path - throws on missing type`() { - assertThrows<JsonDataException> { - adapter.fromJson(NewMessageEventTestData.jsonMissingType) - } - } + `@Test` + fun `Both paths - same error on missing type`() { + assertBothPathsThrowSameJsonDataException(NewMessageEventTestData.jsonMissingType) + }🤖 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/parser2/NewMessageEventParsingTest.kt` around lines 151 - 249, Tests for NewMessageEvent parsing only assert that JsonDataException is thrown (e.g., in tests calling parser.fromJson(..., NewMessageEventDto::class.java) and adapter.fromJson(...)), but they don't assert the exception message, so DTO and direct-path error messages could diverge unnoticed; update each failing-case test (those using NewMessageEventTestData.jsonMissingType, jsonMissingCreatedAt, jsonMissingUser, jsonMissingCid, jsonMissingChannelType, jsonMissingChannelId, jsonMissingMessage) to capture the thrown JsonDataException and assert its message equals the expected message string (or assert contains a canonical substring) for both parser.fromJson and adapter.fromJson paths to ensure message parity between DTO and direct parsing.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/DirectEventParser.kt (1)
102-104: Avoid eager direct-adapter graph initialization throughadapterMap.At first
adapterMapaccess,newMessageEventAdapteris resolved immediately, which builds the whole adapter tree even when the current event type is unsupported. Awhen(type)dispatch (or map of providers) keeps initialization truly on-demand.🤖 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/parser2/DirectEventParser.kt` around lines 102 - 104, The current adapterMap in DirectEventParser eagerly resolves newMessageEventAdapter (via adapterMap: Map<String, JsonAdapter<out ChatEvent>>), causing the entire adapter graph to be built on first access; change this to provide adapters lazily—either replace adapterMap with a dispatch that uses when(type) inside the parsing path (switch on EventType values and return the corresponding adapter only when needed) or change adapterMap to Map<String, () -> JsonAdapter<out ChatEvent>> (a provider/lambda) and call the provider to obtain newMessageEventAdapter only for EventType.MESSAGE_NEW; update any code that reads adapterMap to invoke the provider or use the when branch so adapter creation is deferred.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/MessageParsingTest.kt (1)
165-375: Assert exception-message parity, not just exception type.These cases only prove that both paths throw
JsonDataException. They still pass if the direct path fails on a different field/path, so they don't fully protect the DTO-parity contract this parser is trying to preserve.💡 Proposed refactor
+ private fun assertMissingFieldParity(json: String) { + val dtoError = assertThrows<JsonDataException> { + parser.fromJson(json, DownstreamMessageDto::class.java) + } + val directError = assertThrows<JsonDataException> { + messageAdapter.fromJson(json) + } + + assertEquals(dtoError.message, directError.message) + } + `@Test` - fun `DTO path - throws on missing cid`() { - assertThrows<JsonDataException> { - parser.fromJson(MessageTestData.jsonMissingCid, DownstreamMessageDto::class.java) - } - } - - `@Test` - fun `Direct path - throws on missing cid`() { - assertThrows<JsonDataException> { - messageAdapter.fromJson(MessageTestData.jsonMissingCid) - } + fun `missing cid has parity in both paths`() { + assertMissingFieldParity(MessageTestData.jsonMissingCid) }As per coding guidelines, "Use backtick test names (for example:
funmessage list filters muted channels()) for readability."🤖 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/parser2/MessageParsingTest.kt` around lines 165 - 375, The tests only assert JsonDataException type but must also assert that the DTO path (parser.fromJson(..., DownstreamMessageDto::class.java)) and the direct path (messageAdapter.fromJson(...)) fail for the same reason; update each paired test to capture both exceptions (use assertThrows to get the exception instances) and assert their messages are equal (e.g., assertEquals(parserEx.message, directEx.message)) so the error path/parity is verified; apply this change to all test pairs that currently only check type and ensure the test function names remain backtick-style (e.g., `Direct path - throws on missing cid`) for readability.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/EventChatJsonProvider.kt (1)
634-790: Factor the repeated event builders through a small helper.These poll/reminder/AI-indicator factories are mostly copy-paste with only the event type and one nested payload changing. Centralizing the shared payload shape would make future schema updates much less error-prone.
🤖 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/EventChatJsonProvider.kt` around lines 634 - 790, Several event factory functions (createPollUpdatedEventStringJson, createPollDeletedEventStringJson, createPollClosedEventStringJson, createVoteCastedEventStringJson, createAnswerCastedEventStringJson, createVoteChangedEventStringJson, createVoteRemovedEventStringJson, createReminderCreatedEventStringJson, createReminderUpdatedEventStringJson, createReminderDeletedEventStringJson, createNotificationReminderDueEventStringJson, createAIIndicatorUpdatedEventStringJson, createAIIndicatorClearEventStringJson, createAIIndicatorStopEventStringJson, createUserMessagesDeletedEventStringJson) repeat the same wrapper shape; introduce a small helper (e.g., createSimpleEventStringJson(eventType: String, vararg bodyParts: String) or createChatEventWithPayload(eventType, payloadJson)) that calls createChatEventStringJson with a joined payload, then refactor each of the listed functions to call that helper passing only the event type and the differing nested payload snippets (like createPollJsonString(), createPollVoteJsonString(), createReminderJsonString(), createUserJsonString(), etc.), reducing duplication and centralizing the common "cid"/"message_id" shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/MessageAdapter.kt`:
- Around line 125-129: The quoted_message parsing is order-dependent because it
uses channel ?: fallbackChannelInfo before the containing object's channel may
be read; change MessageAdapter.kt so quoted_message is parsed into a temporary
raw holder (or defer handling) and only resolved with fromJson(reader,
resolvedChannelInfo) after the enclosing message's channel variable is known
(i.e., after the "channel" field is processed), ensuring quotedMessage
resolution uses the correct resolvedChannelInfo; apply the same fix pattern to
the other quoted_message handling block referenced around lines 173-209.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/NewMessageEventAdapter.kt`:
- Around line 98-118: The adapter currently only replaces channelInfo when it is
null, which can leave stale/partial data; update the enrichedMessage logic in
NewMessageEventAdapter to merge event-level metadata into message.channelInfo
instead of only substituting when null: build a mergedChannelInfo by taking
message.channelInfo if present and overriding its fields (cid, id, type,
memberCount, name, image) with the event-level values (cid, channelId,
channelType, channelMemberCount, channelCustomName, channelCustomImage) when
those event values are non-null or different, then use that mergedChannelInfo in
message.copy(...); keep the existing fixes for message.cid and replyTo.cid
(replyTo.copy(cid = cid)) as before.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/PollAdapter.kt`:
- Around line 193-237: The parser currently swallows malformed vote objects by
returning null from parseParsedVote, causing callers to silently drop
required-data votes; change parseParsedVote to throw a JsonDataException (or
equivalent parsing exception) when any required field (id, pollId, optionId,
createdAt, updatedAt) is missing instead of returning null so deserialization
fails fast; update the error message to include which required fields are
missing and the context (e.g., "Malformed ParsedVoteDto: missing [id, poll_id]")
and ensure the exception type is one used by the surrounding JSON plumbing so
callers of parseParsedVote/ParsedVoteDto propagate the error.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/MessageReminderInfoParsingTest.kt`:
- Around line 83-111: The tests in the "Error message parity" section only
assert that both parser.fromJson(…, DownstreamReminderInfoDto::class.java) and
adapter.fromJson(…) throw JsonDataException for
MessageReminderInfoTestData.jsonMissingCreatedAt and jsonMissingUpdatedAt;
update each pair to capture both exceptions (e.g., dtoEx =
assertThrows<JsonDataException>{ parser.fromJson(...) } and directEx =
assertThrows<JsonDataException>{ adapter.fromJson(...) }) and add an assertion
that dtoEx.message == directEx.message (or assertEquals with the two messages)
to enforce identical error messages between the DTO path and Direct path.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/ReactionGroupParsingTest.kt`:
- Around line 89-165: Replace the two-step throw-only tests with parity
assertions that capture exceptions from both parsing paths (parser.fromJson
calling DownstreamReactionGroupDto and reactionGroupAdapter.parseWithType) and
assert their messages are equal; for each missing-field case (e.g.,
ReactionGroupTestData.jsonMissingCount, jsonMissingSumScores,
jsonMissingFirstReactionAt, jsonMissingLastReactionAt) call parser.fromJson and
reactionGroupAdapter.parseWithType, store the thrown JsonDataException
instances, and assert dtoException.message == directException.message to enforce
identical error text across both code paths.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/MessageTestData.kt`:
- Line 136: The JSON test fixtures in MessageTestData.kt currently use the same
timestamp for message.updated_at and poll.updated_at, so update them so
poll.updated_at and message.updated_at differ (e.g., message.updated_at =
"2020-01-01T03:00:00.000Z" and poll.updated_at = "2020-01-01T04:00:00.000Z") and
ensure the expected updatedAt in the parsed Message/fixture equals the later
timestamp (poll.updated_at); apply the same change to the other fixtures in this
file that cover the updatedAt = max(message.updated_at, poll.updated_at)
behavior.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/UserParsingTest.kt`:
- Around line 125-183: Update the parity tests to capture and compare exception
messages instead of only types: for each pair (e.g., tests `DTO path - throws on
missing id` and `Direct path - throws on missing id`) call
assertThrows<JsonDataException> for parser.fromJson(UserTestData.jsonMissingId,
DownstreamUserDto::class.java) and for
userAdapter.fromJson(UserTestData.jsonMissingId), store both exceptions (e.g.,
dtoEx and directEx) and add an assertion that dtoEx.message == directEx.message
(and optionally also assert the message contains the expected field name). Do
this for each missing-field pair (jsonMissingId, jsonMissingRole,
jsonMissingBanned, jsonMissingOnline) so the tests validate parity of
JsonDataException.message for parser.fromJson and userAdapter.fromJson.
---
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/AttachmentAdapter.kt`:
- Around line 26-27: The `@Suppress`("LongMethod") on AttachmentAdapter.fromJson
must be either removed by refactoring or explicitly documented; choose one:
either split fromJson into smaller private helpers (e.g., parseAttachment(),
parseAuthor(), parseExtraData()) inside class AttachmentAdapter and replace the
long method with a short orchestrator, or keep the suppression but add a brief
KDoc or inline comment above fromJson explaining why the long method is
intentional for this hot path (mention performance/avoiding allocations) and
reference the suppression rationale and a TODO to revisit — update only
AttachmentAdapter.fromJson and its related private parsing helpers accordingly.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/ReactionAdapter.kt`:
- Around line 31-32: The LongMethod suppression on ReactionAdapter.fromJson
needs a short rationale or be refactored: either split the parsing logic in
fromJson into small helper methods (e.g., parseReactionBase(), parseUser(),
parseExtraFields()) and remove `@Suppress`("LongMethod"), or keep the suppression
but add an explanatory comment above override fun fromJson(reader: JsonReader):
Reaction? that states this is a performance-sensitive hot-path JSON parser and
why consolidation is intentional; also ensure coding-guideline compliance by
replacing undocumented suppressions with a documented rationale (or using
explicit `@OptIn` where applicable) and reference the ReactionAdapter.fromJson
symbol in your change.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/DirectEventParser.kt`:
- Around line 102-104: The current adapterMap in DirectEventParser eagerly
resolves newMessageEventAdapter (via adapterMap: Map<String, JsonAdapter<out
ChatEvent>>), causing the entire adapter graph to be built on first access;
change this to provide adapters lazily—either replace adapterMap with a dispatch
that uses when(type) inside the parsing path (switch on EventType values and
return the corresponding adapter only when needed) or change adapterMap to
Map<String, () -> JsonAdapter<out ChatEvent>> (a provider/lambda) and call the
provider to obtain newMessageEventAdapter only for EventType.MESSAGE_NEW; update
any code that reads adapterMap to invoke the provider or use the when branch so
adapter creation is deferred.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/EventChatJsonProvider.kt`:
- Around line 634-790: Several event factory functions
(createPollUpdatedEventStringJson, createPollDeletedEventStringJson,
createPollClosedEventStringJson, createVoteCastedEventStringJson,
createAnswerCastedEventStringJson, createVoteChangedEventStringJson,
createVoteRemovedEventStringJson, createReminderCreatedEventStringJson,
createReminderUpdatedEventStringJson, createReminderDeletedEventStringJson,
createNotificationReminderDueEventStringJson,
createAIIndicatorUpdatedEventStringJson, createAIIndicatorClearEventStringJson,
createAIIndicatorStopEventStringJson, createUserMessagesDeletedEventStringJson)
repeat the same wrapper shape; introduce a small helper (e.g.,
createSimpleEventStringJson(eventType: String, vararg bodyParts: String) or
createChatEventWithPayload(eventType, payloadJson)) that calls
createChatEventStringJson with a joined payload, then refactor each of the
listed functions to call that helper passing only the event type and the
differing nested payload snippets (like createPollJsonString(),
createPollVoteJsonString(), createReminderJsonString(), createUserJsonString(),
etc.), reducing duplication and centralizing the common "cid"/"message_id"
shape.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/MessageParsingTest.kt`:
- Around line 165-375: The tests only assert JsonDataException type but must
also assert that the DTO path (parser.fromJson(...,
DownstreamMessageDto::class.java)) and the direct path
(messageAdapter.fromJson(...)) fail for the same reason; update each paired test
to capture both exceptions (use assertThrows to get the exception instances) and
assert their messages are equal (e.g., assertEquals(parserEx.message,
directEx.message)) so the error path/parity is verified; apply this change to
all test pairs that currently only check type and ensure the test function names
remain backtick-style (e.g., `Direct path - throws on missing cid`) for
readability.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/NewMessageEventParsingTest.kt`:
- Around line 151-249: Tests for NewMessageEvent parsing only assert that
JsonDataException is thrown (e.g., in tests calling parser.fromJson(...,
NewMessageEventDto::class.java) and adapter.fromJson(...)), but they don't
assert the exception message, so DTO and direct-path error messages could
diverge unnoticed; update each failing-case test (those using
NewMessageEventTestData.jsonMissingType, jsonMissingCreatedAt, jsonMissingUser,
jsonMissingCid, jsonMissingChannelType, jsonMissingChannelId,
jsonMissingMessage) to capture the thrown JsonDataException and assert its
message equals the expected message string (or assert contains a canonical
substring) for both parser.fromJson and adapter.fromJson paths to ensure message
parity between DTO and direct parsing.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/OptionParsingTest.kt`:
- Around line 80-106: Update the tests to not only assert that JsonDataException
is thrown but also verify the exception messages match between the DTO path and
direct path: for the missing-id case capture the exception from
parser.fromJson(OptionTestData.jsonMissingId,
DownstreamPollOptionDto::class.java) and from
adapter.fromJson(OptionTestData.jsonMissingId) and assert their message strings
are equal (and non-empty), and do the same for the missing-text case using
OptionTestData.jsonMissingText; keep the existing assertThrows style but store
the caught exceptions and compare their message contents to ensure parity
between parser.fromJson and adapter.fromJson.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/PollParsingTest.kt`:
- Around line 118-190: The tests only assert that parser.fromJson and
adapter.fromJson throw JsonDataException for missing fields but don't verify the
exception messages are the same; update each pair of tests (e.g., `DTO path -
throws on missing id` vs `Direct path - throws on missing id`, same for name,
description, options, enforce_unique_vote) to capture the thrown
JsonDataException from both parser.fromJson(PollTestData.jsonMissingX,
DownstreamPollDto::class.java) and adapter.fromJson(PollTestData.jsonMissingX)
and assert their messages are equal (and non-null), so that exception message
parity between the DTO route and direct adapter route is enforced.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ChannelTestData.kt`:
- Around line 252-427: The three global vals expectedAllFields,
expectedWithNestedCollections, and expectedOptionalFieldsMissing expose shared
mutable state (mutableMapOf and nested mutable collections like
Message.reactionCounts/reactionScores) which can leak between tests; convert
each fixture into a factory function or a property with a custom getter that
constructs and returns a new Channel instance (e.g., fun expectedAllFields():
Channel { ... }) and ensure nested mutable collections are created anew (use
mutableMapOf() inside the factory or use immutable mapOf()/emptyMap() if
mutability is not required) so every test gets a fresh, independent object;
update any references to these symbols accordingly.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ChannelUserReadTestData.kt`:
- Around line 46-77: The tests share a single mutable Date instance
lastReceivedEventDate between expectedAllFields and
expectedOptionalFieldsMissing, which can cause cross-test coupling; fix by
creating a separate Date for each fixture (e.g., inline new Date(1744200000000L)
when constructing expectedAllFields and expectedOptionalFieldsMissing or use
distinct vals like lastReceivedEventDateA and lastReceivedEventDateB) so
ChannelUserRead's lastReceivedEventDate isn't a shared mutable object.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/PushPreferenceTestData.kt`:
- Around line 26-42: Add two partial-field fixtures to the PushPreference test
data: one with only chat_level and one with only disabled_until. Create new JSON
constants (e.g., jsonOnlyChatLevel and jsonOnlyDisabledUntil) and corresponding
expected PushPreference instances (e.g., expectedOnlyChatLevel and
expectedOnlyDisabledUntil) where the absent field is null; reuse
PushPreferenceLevel.mentions and Date(1593411268000) for values to match
existing expectedAllFields; place these alongside
jsonAllFields/jsonOptionalFieldsMissing and
expectedAllFields/expectedOptionalFieldsMissing so the parser tests cover mixed
presence of optional fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0bf5c0c0-ac56-4b95-9b39-b960b3250b8b
📒 Files selected for processing (64)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/extensions/ChatEvent.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/DirectEventParser.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/MoshiChatParser.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/AttachmentAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/ChannelInfoAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/DeviceAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/JsonParsingUtils.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/LocationAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/MessageAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/MessageModerationDetailsAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/MessageReminderInfoAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/ModerationAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/NewMessageEventAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/OptionAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/PollAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/PrivacySettingsAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/ReactionAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/ReactionGroupAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/direct/UserAdapter.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/EventChatJsonProvider.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api/RetrofitCallAdapterFactoryTests.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser/EventArguments.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/AttachmentParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/ChannelInfoParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/DeviceParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/DirectEventParserTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/JsonParsingUtilsTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/LocationParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/MessageModerationDetailsParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/MessageParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/MessageReminderInfoParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/ModerationParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/NewMessageEventParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/OptionParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/ParserFactory.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/PollParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/PrivacySettingsParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/ReactionGroupParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/ReactionParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/UserParsingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/AnswerTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/AttachmentTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ChannelInfoTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ChannelTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ChannelUserReadTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/CommandTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ConfigTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/DeviceTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/LocationTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/MemberTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/MessageModerationDetailsTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/MessageReminderInfoTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/MessageTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ModerationTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/NewMessageEventTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/OptionTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/PollTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/PrivacySettingsTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/PushPreferenceTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ReactionGroupTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/ReactionTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/UserTestData.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/VoteTestData.kt
Comment on lines
+125
to
+129
| "quoted_message" -> { | ||
| // Recursive parsing: pass the fallback channel info along | ||
| val resolvedChannelInfo = channel ?: fallbackChannelInfo | ||
| quotedMessage = fromJson(reader, resolvedChannelInfo) | ||
| } |
There was a problem hiding this comment.
Quoted-message enrichment is field-order dependent.
quoted_message is parsed before the current object's channel is guaranteed to be known. If the payload puts "quoted_message" before "channel", the nested message never gets the parent channel fallback, so the direct path can produce different replyTo data for the same JSON object content.
💡 Proposed fix
"quoted_message" -> {
- // Recursive parsing: pass the fallback channel info along
- val resolvedChannelInfo = channel ?: fallbackChannelInfo
- quotedMessage = fromJson(reader, resolvedChannelInfo)
+ quotedMessage = fromJson(reader, fallbackChannelInfo)
}
@@
val resolvedChannelInfo = channel ?: fallbackChannelInfo
+ val enrichedQuotedMessage = quotedMessage?.let { reply ->
+ if (resolvedChannelInfo == null) {
+ reply
+ } else {
+ reply.copy(
+ channelInfo = reply.channelInfo ?: resolvedChannelInfo,
+ cid = reply.cid ?: resolvedChannelInfo.cid,
+ )
+ }
+ }
// Filter reactions by messageId (matching DomainMapping behavior)
@@
- replyTo = quotedMessage,
+ replyTo = enrichedQuotedMessage,A regression test with "quoted_message" ordered before "channel" would lock this down nicely.
Also applies to: 173-209
🤖 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/parser2/direct/MessageAdapter.kt`
around lines 125 - 129, The quoted_message parsing is order-dependent because it
uses channel ?: fallbackChannelInfo before the containing object's channel may
be read; change MessageAdapter.kt so quoted_message is parsed into a temporary
raw holder (or defer handling) and only resolved with fromJson(reader,
resolvedChannelInfo) after the enclosing message's channel variable is known
(i.e., after the "channel" field is processed), ensuring quotedMessage
resolution uses the correct resolvedChannelInfo; apply the same fix pattern to
the other quoted_message handling block referenced around lines 173-209.
Comment on lines
+98
to
+118
| // Enrich inline: set channelInfo + cid so parseAndProcessEvent can skip enrichIfNeeded(). | ||
| // Only copy if something actually needs to change. | ||
| val needsChannelInfo = message.channelInfo == null | ||
| val needsCid = message.cid != cid | ||
| val replyTo = message.replyTo | ||
| val needsReplyToCid = replyTo != null && replyTo.cid != cid | ||
|
|
||
| val enrichedMessage = if (needsChannelInfo || needsCid || needsReplyToCid) { | ||
| val fallbackChannelInfo = ChannelInfo( | ||
| cid = cid, | ||
| id = channelId, | ||
| type = channelType, | ||
| memberCount = channelMemberCount ?: 0, | ||
| name = channelCustomName, | ||
| image = channelCustomImage, | ||
| ) | ||
| message.copy( | ||
| channelInfo = message.channelInfo ?: fallbackChannelInfo, | ||
| cid = if (needsCid) cid else message.cid, | ||
| replyTo = if (needsReplyToCid) replyTo.copy(cid = cid) else replyTo, | ||
| ) |
There was a problem hiding this comment.
Merge event-level channel metadata into existing message.channelInfo.
This branch only rebuilds channelInfo when it is null. If the message payload already contains a partial or stale channelInfo, the adapter fixes message.cid but keeps the old channelInfo, so the direct path can return inconsistent channel data.
💡 Proposed fix
- val needsChannelInfo = message.channelInfo == null
+ val existingChannelInfo = message.channelInfo
+ val needsChannelInfo =
+ existingChannelInfo == null ||
+ existingChannelInfo.cid != cid ||
+ existingChannelInfo.id != channelId ||
+ existingChannelInfo.type != channelType ||
+ (channelMemberCount != null && existingChannelInfo.memberCount != channelMemberCount) ||
+ (channelCustomName != null && existingChannelInfo.name != channelCustomName) ||
+ (channelCustomImage != null && existingChannelInfo.image != channelCustomImage)
val needsCid = message.cid != cid
val replyTo = message.replyTo
val needsReplyToCid = replyTo != null && replyTo.cid != cid
val enrichedMessage = if (needsChannelInfo || needsCid || needsReplyToCid) {
- val fallbackChannelInfo = ChannelInfo(
+ val enrichedChannelInfo = (existingChannelInfo ?: ChannelInfo()).copy(
cid = cid,
id = channelId,
type = channelType,
- memberCount = channelMemberCount ?: 0,
- name = channelCustomName,
- image = channelCustomImage,
+ memberCount = channelMemberCount ?: existingChannelInfo?.memberCount ?: 0,
+ name = channelCustomName ?: existingChannelInfo?.name,
+ image = channelCustomImage ?: existingChannelInfo?.image,
)
message.copy(
- channelInfo = message.channelInfo ?: fallbackChannelInfo,
+ channelInfo = enrichedChannelInfo,
cid = if (needsCid) cid else message.cid,
replyTo = if (needsReplyToCid) replyTo.copy(cid = cid) else replyTo,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Enrich inline: set channelInfo + cid so parseAndProcessEvent can skip enrichIfNeeded(). | |
| // Only copy if something actually needs to change. | |
| val needsChannelInfo = message.channelInfo == null | |
| val needsCid = message.cid != cid | |
| val replyTo = message.replyTo | |
| val needsReplyToCid = replyTo != null && replyTo.cid != cid | |
| val enrichedMessage = if (needsChannelInfo || needsCid || needsReplyToCid) { | |
| val fallbackChannelInfo = ChannelInfo( | |
| cid = cid, | |
| id = channelId, | |
| type = channelType, | |
| memberCount = channelMemberCount ?: 0, | |
| name = channelCustomName, | |
| image = channelCustomImage, | |
| ) | |
| message.copy( | |
| channelInfo = message.channelInfo ?: fallbackChannelInfo, | |
| cid = if (needsCid) cid else message.cid, | |
| replyTo = if (needsReplyToCid) replyTo.copy(cid = cid) else replyTo, | |
| ) | |
| // Enrich inline: set channelInfo + cid so parseAndProcessEvent can skip enrichIfNeeded(). | |
| // Only copy if something actually needs to change. | |
| val existingChannelInfo = message.channelInfo | |
| val needsChannelInfo = | |
| existingChannelInfo == null || | |
| existingChannelInfo.cid != cid || | |
| existingChannelInfo.id != channelId || | |
| existingChannelInfo.type != channelType || | |
| (channelMemberCount != null && existingChannelInfo.memberCount != channelMemberCount) || | |
| (channelCustomName != null && existingChannelInfo.name != channelCustomName) || | |
| (channelCustomImage != null && existingChannelInfo.image != channelCustomImage) | |
| val needsCid = message.cid != cid | |
| val replyTo = message.replyTo | |
| val needsReplyToCid = replyTo != null && replyTo.cid != cid | |
| val enrichedMessage = if (needsChannelInfo || needsCid || needsReplyToCid) { | |
| val enrichedChannelInfo = (existingChannelInfo ?: ChannelInfo()).copy( | |
| cid = cid, | |
| id = channelId, | |
| type = channelType, | |
| memberCount = channelMemberCount ?: existingChannelInfo?.memberCount ?: 0, | |
| name = channelCustomName ?: existingChannelInfo?.name, | |
| image = channelCustomImage ?: existingChannelInfo?.image, | |
| ) | |
| message.copy( | |
| channelInfo = enrichedChannelInfo, | |
| cid = if (needsCid) cid else message.cid, | |
| replyTo = if (needsReplyToCid) replyTo.copy(cid = cid) else replyTo, | |
| ) |
🤖 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/parser2/direct/NewMessageEventAdapter.kt`
around lines 98 - 118, The adapter currently only replaces channelInfo when it
is null, which can leave stale/partial data; update the enrichedMessage logic in
NewMessageEventAdapter to merge event-level metadata into message.channelInfo
instead of only substituting when null: build a mergedChannelInfo by taking
message.channelInfo if present and overriding its fields (cid, id, type,
memberCount, name, image) with the event-level values (cid, channelId,
channelType, channelMemberCount, channelCustomName, channelCustomImage) when
those event values are non-null or different, then use that mergedChannelInfo in
message.copy(...); keep the existing fixes for message.cid and replyTo.cid
(replyTo.copy(cid = cid)) as before.
Comment on lines
+193
to
+237
| private fun parseParsedVote(reader: JsonReader): ParsedVoteDto? { | ||
| if (reader.peek() == JsonReader.Token.NULL) return reader.nextNull() | ||
|
|
||
| reader.beginObject() | ||
| var id: String? = null | ||
| var pollId: String? = null | ||
| var optionId: String? = null | ||
| var createdAt: Date? = null | ||
| var updatedAt: Date? = null | ||
| var user: User? = null | ||
| var isAnswer: Boolean = false | ||
| var answerText: String? = null | ||
|
|
||
| while (reader.hasNext()) { | ||
| when (reader.nextName()) { | ||
| "id" -> id = reader.nextString() | ||
| "poll_id" -> pollId = reader.nextString() | ||
| "option_id" -> optionId = reader.nextString() | ||
| "created_at" -> createdAt = dateAdapter.fromJson(reader) | ||
| "updated_at" -> updatedAt = dateAdapter.fromJson(reader) | ||
| "user" -> user = userAdapter.fromJson(reader) | ||
| "is_answer" -> isAnswer = JsonParsingUtils.readNullableBoolean(reader) ?: false | ||
| "answer_text" -> answerText = JsonParsingUtils.readNullableString(reader) | ||
| "user_id" -> reader.skipValue() | ||
| else -> reader.skipValue() | ||
| } | ||
| } | ||
| reader.endObject() | ||
|
|
||
| if (id == null) return null | ||
| if (pollId == null) return null | ||
| if (optionId == null) return null | ||
| if (createdAt == null) return null | ||
| if (updatedAt == null) return null | ||
|
|
||
| return ParsedVoteDto( | ||
| id = id, | ||
| pollId = pollId, | ||
| optionId = optionId, | ||
| createdAt = createdAt, | ||
| updatedAt = updatedAt, | ||
| user = user, | ||
| isAnswer = isAnswer, | ||
| answerText = answerText, | ||
| ) |
There was a problem hiding this comment.
Don't silently drop malformed vote/answer objects.
parseParsedVote returns null when required fields are missing, and the callers just skip that element. That means a corrupted payload can deserialize into a partial Poll instead of failing fast, which breaks the parity goal for required-field handling.
💡 Proposed fix
reader.endObject()
- if (id == null) return null
- if (pollId == null) return null
- if (optionId == null) return null
- if (createdAt == null) return null
- if (updatedAt == null) return null
+ JsonParsingUtils.requireField(id, "id", reader)
+ JsonParsingUtils.requireField(pollId, "poll_id", reader)
+ JsonParsingUtils.requireField(optionId, "option_id", reader)
+ JsonParsingUtils.requireField(createdAt, "created_at", reader)
+ JsonParsingUtils.requireField(updatedAt, "updated_at", reader)
return ParsedVoteDto(
id = id,
pollId = pollId,
optionId = optionId,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private fun parseParsedVote(reader: JsonReader): ParsedVoteDto? { | |
| if (reader.peek() == JsonReader.Token.NULL) return reader.nextNull() | |
| reader.beginObject() | |
| var id: String? = null | |
| var pollId: String? = null | |
| var optionId: String? = null | |
| var createdAt: Date? = null | |
| var updatedAt: Date? = null | |
| var user: User? = null | |
| var isAnswer: Boolean = false | |
| var answerText: String? = null | |
| while (reader.hasNext()) { | |
| when (reader.nextName()) { | |
| "id" -> id = reader.nextString() | |
| "poll_id" -> pollId = reader.nextString() | |
| "option_id" -> optionId = reader.nextString() | |
| "created_at" -> createdAt = dateAdapter.fromJson(reader) | |
| "updated_at" -> updatedAt = dateAdapter.fromJson(reader) | |
| "user" -> user = userAdapter.fromJson(reader) | |
| "is_answer" -> isAnswer = JsonParsingUtils.readNullableBoolean(reader) ?: false | |
| "answer_text" -> answerText = JsonParsingUtils.readNullableString(reader) | |
| "user_id" -> reader.skipValue() | |
| else -> reader.skipValue() | |
| } | |
| } | |
| reader.endObject() | |
| if (id == null) return null | |
| if (pollId == null) return null | |
| if (optionId == null) return null | |
| if (createdAt == null) return null | |
| if (updatedAt == null) return null | |
| return ParsedVoteDto( | |
| id = id, | |
| pollId = pollId, | |
| optionId = optionId, | |
| createdAt = createdAt, | |
| updatedAt = updatedAt, | |
| user = user, | |
| isAnswer = isAnswer, | |
| answerText = answerText, | |
| ) | |
| private fun parseParsedVote(reader: JsonReader): ParsedVoteDto? { | |
| if (reader.peek() == JsonReader.Token.NULL) return reader.nextNull() | |
| reader.beginObject() | |
| var id: String? = null | |
| var pollId: String? = null | |
| var optionId: String? = null | |
| var createdAt: Date? = null | |
| var updatedAt: Date? = null | |
| var user: User? = null | |
| var isAnswer: Boolean = false | |
| var answerText: String? = null | |
| while (reader.hasNext()) { | |
| when (reader.nextName()) { | |
| "id" -> id = reader.nextString() | |
| "poll_id" -> pollId = reader.nextString() | |
| "option_id" -> optionId = reader.nextString() | |
| "created_at" -> createdAt = dateAdapter.fromJson(reader) | |
| "updated_at" -> updatedAt = dateAdapter.fromJson(reader) | |
| "user" -> user = userAdapter.fromJson(reader) | |
| "is_answer" -> isAnswer = JsonParsingUtils.readNullableBoolean(reader) ?: false | |
| "answer_text" -> answerText = JsonParsingUtils.readNullableString(reader) | |
| "user_id" -> reader.skipValue() | |
| else -> reader.skipValue() | |
| } | |
| } | |
| reader.endObject() | |
| JsonParsingUtils.requireField(id, "id", reader) | |
| JsonParsingUtils.requireField(pollId, "poll_id", reader) | |
| JsonParsingUtils.requireField(optionId, "option_id", reader) | |
| JsonParsingUtils.requireField(createdAt, "created_at", reader) | |
| JsonParsingUtils.requireField(updatedAt, "updated_at", reader) | |
| return ParsedVoteDto( | |
| id = id, | |
| pollId = pollId, | |
| optionId = optionId, | |
| createdAt = createdAt, | |
| updatedAt = updatedAt, | |
| user = user, | |
| isAnswer = isAnswer, | |
| answerText = answerText, | |
| ) | |
| } |
🤖 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/parser2/direct/PollAdapter.kt`
around lines 193 - 237, The parser currently swallows malformed vote objects by
returning null from parseParsedVote, causing callers to silently drop
required-data votes; change parseParsedVote to throw a JsonDataException (or
equivalent parsing exception) when any required field (id, pollId, optionId,
createdAt, updatedAt) is missing instead of returning null so deserialization
fails fast; update the error message to include which required fields are
missing and the context (e.g., "Malformed ParsedVoteDto: missing [id, poll_id]")
and ensure the exception type is one used by the surrounding JSON plumbing so
callers of parseParsedVote/ParsedVoteDto propagate the error.
Comment on lines
+83
to
+111
| // region Error message parity | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing created_at`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt, DownstreamReminderInfoDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing created_at`() { | ||
| assertThrows<JsonDataException> { | ||
| adapter.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing updated_at`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt, DownstreamReminderInfoDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing updated_at`() { | ||
| assertThrows<JsonDataException> { | ||
| adapter.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt) | ||
| } | ||
| } |
There was a problem hiding this comment.
Error message parity section does not assert message parity.
These tests currently validate only that both paths throw, not that they fail identically. Add message comparison to lock in true parity.
💡 Suggested update
`@Test`
-fun `DTO path - throws on missing created_at`() {
- assertThrows<JsonDataException> {
- parser.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt, DownstreamReminderInfoDto::class.java)
- }
-}
-
-@Test
-fun `Direct path - throws on missing created_at`() {
- assertThrows<JsonDataException> {
- adapter.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt)
- }
+fun `Both paths - same error on missing created_at`() {
+ val dtoException = assertThrows<JsonDataException> {
+ parser.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt, DownstreamReminderInfoDto::class.java)
+ }
+ val directException = assertThrows<JsonDataException> {
+ adapter.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt)
+ }
+ assertEquals(dtoException.message, directException.message)
}
`@Test`
-fun `DTO path - throws on missing updated_at`() {
- assertThrows<JsonDataException> {
- parser.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt, DownstreamReminderInfoDto::class.java)
- }
-}
-
-@Test
-fun `Direct path - throws on missing updated_at`() {
- assertThrows<JsonDataException> {
- adapter.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt)
- }
+fun `Both paths - same error on missing updated_at`() {
+ val dtoException = assertThrows<JsonDataException> {
+ parser.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt, DownstreamReminderInfoDto::class.java)
+ }
+ val directException = assertThrows<JsonDataException> {
+ adapter.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt)
+ }
+ assertEquals(dtoException.message, directException.message)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // region Error message parity | |
| @Test | |
| fun `DTO path - throws on missing created_at`() { | |
| assertThrows<JsonDataException> { | |
| parser.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt, DownstreamReminderInfoDto::class.java) | |
| } | |
| } | |
| @Test | |
| fun `Direct path - throws on missing created_at`() { | |
| assertThrows<JsonDataException> { | |
| adapter.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt) | |
| } | |
| } | |
| @Test | |
| fun `DTO path - throws on missing updated_at`() { | |
| assertThrows<JsonDataException> { | |
| parser.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt, DownstreamReminderInfoDto::class.java) | |
| } | |
| } | |
| @Test | |
| fun `Direct path - throws on missing updated_at`() { | |
| assertThrows<JsonDataException> { | |
| adapter.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt) | |
| } | |
| } | |
| // region Error message parity | |
| `@Test` | |
| fun `Both paths - same error on missing created_at`() { | |
| val dtoException = assertThrows<JsonDataException> { | |
| parser.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt, DownstreamReminderInfoDto::class.java) | |
| } | |
| val directException = assertThrows<JsonDataException> { | |
| adapter.fromJson(MessageReminderInfoTestData.jsonMissingCreatedAt) | |
| } | |
| assertEquals(dtoException.message, directException.message) | |
| } | |
| `@Test` | |
| fun `Both paths - same error on missing updated_at`() { | |
| val dtoException = assertThrows<JsonDataException> { | |
| parser.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt, DownstreamReminderInfoDto::class.java) | |
| } | |
| val directException = assertThrows<JsonDataException> { | |
| adapter.fromJson(MessageReminderInfoTestData.jsonMissingUpdatedAt) | |
| } | |
| assertEquals(dtoException.message, directException.message) | |
| } |
🤖 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/parser2/MessageReminderInfoParsingTest.kt`
around lines 83 - 111, The tests in the "Error message parity" section only
assert that both parser.fromJson(…, DownstreamReminderInfoDto::class.java) and
adapter.fromJson(…) throw JsonDataException for
MessageReminderInfoTestData.jsonMissingCreatedAt and jsonMissingUpdatedAt;
update each pair to capture both exceptions (e.g., dtoEx =
assertThrows<JsonDataException>{ parser.fromJson(...) } and directEx =
assertThrows<JsonDataException>{ adapter.fromJson(...) }) and add an assertion
that dtoEx.message == directEx.message (or assertEquals with the two messages)
to enforce identical error messages between the DTO path and Direct path.
Comment on lines
+89
to
+165
| // region Error message parity | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing count`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(ReactionGroupTestData.jsonMissingCount, DownstreamReactionGroupDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing count`() { | ||
| assertThrows<JsonDataException> { | ||
| reactionGroupAdapter.parseWithType( | ||
| com.squareup.moshi.JsonReader.of( | ||
| okio.Buffer().writeUtf8(ReactionGroupTestData.jsonMissingCount), | ||
| ), | ||
| testType, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing sum_scores`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(ReactionGroupTestData.jsonMissingSumScores, DownstreamReactionGroupDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing sum_scores`() { | ||
| assertThrows<JsonDataException> { | ||
| reactionGroupAdapter.parseWithType( | ||
| com.squareup.moshi.JsonReader.of( | ||
| okio.Buffer().writeUtf8(ReactionGroupTestData.jsonMissingSumScores), | ||
| ), | ||
| testType, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing first_reaction_at`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(ReactionGroupTestData.jsonMissingFirstReactionAt, DownstreamReactionGroupDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing first_reaction_at`() { | ||
| assertThrows<JsonDataException> { | ||
| reactionGroupAdapter.parseWithType( | ||
| com.squareup.moshi.JsonReader.of( | ||
| okio.Buffer().writeUtf8(ReactionGroupTestData.jsonMissingFirstReactionAt), | ||
| ), | ||
| testType, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing last_reaction_at`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(ReactionGroupTestData.jsonMissingLastReactionAt, DownstreamReactionGroupDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing last_reaction_at`() { | ||
| assertThrows<JsonDataException> { | ||
| reactionGroupAdapter.parseWithType( | ||
| com.squareup.moshi.JsonReader.of( | ||
| okio.Buffer().writeUtf8(ReactionGroupTestData.jsonMissingLastReactionAt), | ||
| ), | ||
| testType, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Exception parity checks are incomplete in the Error message parity block.
The tests verify throw/no-throw, but they don’t verify both paths emit the same JsonDataException message. Consider consolidating each case into a single parity assertion (dtoException.message == directException.message) to enforce true parity.
🤖 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/parser2/ReactionGroupParsingTest.kt`
around lines 89 - 165, Replace the two-step throw-only tests with parity
assertions that capture exceptions from both parsing paths (parser.fromJson
calling DownstreamReactionGroupDto and reactionGroupAdapter.parseWithType) and
assert their messages are equal; for each missing-field case (e.g.,
ReactionGroupTestData.jsonMissingCount, jsonMissingSumScores,
jsonMissingFirstReactionAt, jsonMissingLastReactionAt) call parser.fromJson and
reactionGroupAdapter.parseWithType, store the thrown JsonDataException
instances, and assert dtoException.message == directException.message to enforce
identical error text across both code paths.
| } | ||
| ], | ||
| "created_at": "2020-01-01T00:00:00.000Z", | ||
| "updated_at": "2020-01-01T03:00:00.000Z", |
There was a problem hiding this comment.
Use different message and poll timestamps here.
This fixture claims to cover the updatedAt = max(message.updated_at, poll.updated_at) behavior, but both values are currently 2020-01-01T03:00:00.000Z. That means a parser regression that ignores poll.updated_at would still pass. Make the two timestamps differ and keep the expected updatedAt on the later one.
Also applies to: 155-155, 565-566
🤖 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/parser2/testdata/MessageTestData.kt`
at line 136, The JSON test fixtures in MessageTestData.kt currently use the same
timestamp for message.updated_at and poll.updated_at, so update them so
poll.updated_at and message.updated_at differ (e.g., message.updated_at =
"2020-01-01T03:00:00.000Z" and poll.updated_at = "2020-01-01T04:00:00.000Z") and
ensure the expected updatedAt in the parsed Message/fixture equals the later
timestamp (poll.updated_at); apply the same change to the other fixtures in this
file that cover the updatedAt = max(message.updated_at, poll.updated_at)
behavior.
Comment on lines
+125
to
+183
| // region Error message parity | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing id`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(UserTestData.jsonMissingId, DownstreamUserDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing id`() { | ||
| assertThrows<JsonDataException> { | ||
| userAdapter.fromJson(UserTestData.jsonMissingId) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing role`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(UserTestData.jsonMissingRole, DownstreamUserDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing role`() { | ||
| assertThrows<JsonDataException> { | ||
| userAdapter.fromJson(UserTestData.jsonMissingRole) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing banned`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(UserTestData.jsonMissingBanned, DownstreamUserDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing banned`() { | ||
| assertThrows<JsonDataException> { | ||
| userAdapter.fromJson(UserTestData.jsonMissingBanned) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `DTO path - throws on missing online`() { | ||
| assertThrows<JsonDataException> { | ||
| parser.fromJson(UserTestData.jsonMissingOnline, DownstreamUserDto::class.java) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `Direct path - throws on missing online`() { | ||
| assertThrows<JsonDataException> { | ||
| userAdapter.fromJson(UserTestData.jsonMissingOnline) | ||
| } | ||
| } | ||
|
|
||
| // endregion |
There was a problem hiding this comment.
Error message parity tests should compare exception messages, not only exception type.
Right now, DTO and direct paths are validated independently with assertThrows. Add paired assertions comparing JsonDataException.message to verify real parity for missing required fields.
🤖 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/parser2/UserParsingTest.kt`
around lines 125 - 183, Update the parity tests to capture and compare exception
messages instead of only types: for each pair (e.g., tests `DTO path - throws on
missing id` and `Direct path - throws on missing id`) call
assertThrows<JsonDataException> for parser.fromJson(UserTestData.jsonMissingId,
DownstreamUserDto::class.java) and for
userAdapter.fromJson(UserTestData.jsonMissingId), store both exceptions (e.g.,
dtoEx and directEx) and add an assertion that dtoEx.message == directEx.message
(and optionally also assert the message contains the expected field name). Do
this for each missing-field pair (jsonMissingId, jsonMissingRole,
jsonMissingBanned, jsonMissingOnline) so the tests validate parity of
JsonDataException.message for parser.fromJson and userAdapter.fromJson.
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
1 participant
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
Introduce a direct JSON-to-Domain parsing path for WebSocket events, bypassing the intermediate DTO layer. This reduces object allocations and speeds up event deserialization on the hot path (starting with
message.new). The new path produces domain objects identical to the existing DTO-based path.The idea behind this change is that during livestreams, massive bursts of
message.newevents can cause massive amounts of object allocations (enhanced by the intermediateDTOobjects), causing throttles and unnecessary garbage collections. Skipping the DTO layer, results in much smaller memory footprint (and fewer temporary object allocations).Resolves: https://linear.app/stream/issue/AND-1143/improve-messagenew-event-parsing-memory-footprint
Implementation
New direct JSON adapters — Hand-written
JsonAdapterimplementations that read JSON viaJsonReaderand construct domain objects directly, skipping DTO allocation:UserAdapter,MessageAdapter,NewMessageEventAdapterfor the core event chainAttachmentAdapter,ReactionAdapter,ReactionGroupAdapter,PollAdapter,OptionAdapter,DeviceAdapter,PrivacySettingsAdapter,LocationAdapter,ModerationAdapter,MessageModerationDetailsAdapter,MessageReminderInfoAdapter,ChannelInfoAdapterJsonParsingUtils— shared utilities for nullable fields, extra data accumulation, list/map parsing, and required-field validation with Moshi-parity error messagesDirectEventParser— Router that extracts the eventtypefrom raw JSON and dispatches to the appropriate direct adapter. Currently supportsmessage.new; unsupported types fall back to the existing DTO path. Wired intoMoshiChatParser.parseAndProcessEvent().UserTransformer/MessageTransformerparity — Both transformers are applied identically in the direct path (at the end of object construction, via.let(transformer::transform)), matching the DTO path.ApiModelTransformersfromChatClient.Builderare passed throughChatModule→DirectEventParser→ individual adapters.Enrichment inline — The
NewMessageEventAdapterperforms channel info enrichment (channel name/image fromchannel_custom) during parsing, soenrichIfNeeded()is not needed for directly-parsed events.Testing
UserParsingTest,MessageParsingTest,NewMessageEventParsingTest, plusAttachment,Reaction,Poll,Device,Location, etc.): each test parses the same JSON through both DTO and direct paths, asserts identical domain output, and verifies identicalJsonDataExceptionfor missing required fields.UserTransformerandMessageTransformerapplied to both paths, verifying identical output. Includes nestedUserTransformerverification (message.user, mentionedUsers, threadParticipants, reaction users, poll vote/answer/createdBy users).DirectEventParserTest: verifies type extraction, event routing, fallback for unsupported types, and end-to-end transformer wiring.JsonDataExceptionon"total_unread_count": null(Moshi default only applies when field is absent, not null).To run the benchmark comparing DTO vs Direct parsing performance (speed + allocations), apply the patch below and run:
NewMessageEventBenchmarkTest.kt — DTO vs Direct parsing benchmark
Summary by CodeRabbit
Release Notes
Performance Improvements
Bug Fixes