Sitelet https://github.com/GetStream/stream-chat-android/pull/6671
Skip to content

Adopt the generated Attachment for downstream message attachments - #6671

Merged
gpunto merged 3 commits into
developfrom
migrate/attachment-downstream
Aug 31, 2026
Merged

gpunto merged 3 commits into
developfrom
migrate/attachment-downstream

Conversation

@gpunto

@gpunto gpunto commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Goal

Adopt the generated Attachment for downstream message attachments.

Part of AND-1291

Implementation

  • AttachmentRequestAdapter becomes NetworkAttachmentAdapter. The same generated model is both the
    request and the response body, and its fromJson used to throw, so it now parses as well as serializes.
    Renamed rather than kept as AttachmentAdapter because parser2.direct.AttachmentAdapter already exists
    for the WebSocket event path.
  • Swap the attachments on DownstreamMessageDto and DownstreamDraftMessageDto to the generated model.
    UpstreamMessageDto and Attachment.toDto() keep the hand-written DTO, so it stays for now.
  • Attachment.toDomain() reads file_size, image, mime_type and name back out of the collected
    custom data and removes them. The spec does not declare them while the wire sends them at the root, so
    without this an attachment loses its size, name and mime type and carries them under their wire names in
    extraData instead. An undeclared number arrives untyped, so file_size is a Double and needs the
    numeric conversion rather than a cast to Int.
  • author_icon, color, footer, footer_icon, pretext, actions, fields and giphy are declared
    by the generated model but were not by the hand-written DTO, so they would stop reaching
    Attachment.extraData. They go to alsoKeepInExtraData, and away with AND-1398.
  • ChatClient.warmUpReflection() also warms the generated model. It lists the classes whose
    KClass.members costs hundreds of milliseconds on first use, so leaving it stale means the first real
    parse pays that cost.

Notes

giphy is kept as the raw object rather than rebuilt from the typed field. Attachment.giphyInfo() in
ui-common reads gif urls out of extraData["giphy"], and keeping the wire object preserves the exact map
it already reads. The reference implementation re-emitted it through Images.toLegacyMap(), which can only
reproduce the seven variants and five inner keys the model declares; alsoKeepInExtraData did not exist
when that was written.

A partial giphy cannot arrive: giphy.Images is seven value-type structs of five value-type strings with
no omitempty, so an unfilled variant serializes as empty strings rather than being omitted.

Testing

  • AttachmentParsingTest gains a generated-path region covering the undeclared root fields landing in
    custom, the declared keys the keep set holds, and the giphy object keeping the shape giphyInfo()
    reads. The existing DTO-path and direct-path tests are untouched.
  • DomainMappingTest covers the recovery of the four undeclared fields, that they do not also linger in
    extraData, and the giphy map surviving the mapping.
  • Device-probed on the wire: a /giphy message returned all seven variants with a usable original.url
    and actions retained, and an uploaded file kept its name, mimeType and fileSize through both the
    send response and a fresh channel query with no wire names left in extraData. Giphy rendering checked
    by hand in the sample app, since the risk here is a display regression rather than a parse failure.

Summary by CodeRabbit

  • Bug Fixes
    • Improved attachment parsing and mapping for messages and drafts.
    • Attachment metadata such as file size, image URL, MIME type, and name is now correctly restored.
    • Preserved custom attachment data, including Giphy information, for reliable UI display.
    • Reduced initial delays when attachments are used for the first time.
  • Tests
    • Added coverage for generated attachment parsing, metadata conversion, and custom data preservation.

@gpunto gpunto added the pr:internal Internal changes / housekeeping label Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR checklist ✅

All required conditions are satisfied:

  • Title length is OK (or ignored by label).
  • At least one pr: label exists.
  • Sections ### Goal, ### Implementation, and ### Testing are filled, or the PR is bot-authored.
  • An issue is linked (Linear ticket or GitHub issue), or the PR is bot-authored.

🎉 Great job! This PR is ready for review.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 6.11 MB 6.11 MB 0.00 MB 🟢
stream-chat-android-ui-components 11.41 MB 11.41 MB 0.00 MB 🟢
stream-chat-android-compose 12.90 MB 12.90 MB 0.00 MB 🟢

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
9.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@gpunto
gpunto marked this pull request as ready for review August 31, 2026 11:19
@gpunto
gpunto requested a review from a team as a code owner August 31, 2026 11:19
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 416937e3-cd9a-479a-9af9-868e542fad55

📥 Commits

Reviewing files that changed from the base of the PR and between ff5eb69 and ae87a82.

📒 Files selected for processing (10)
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/DomainMapping.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/MessageDtos.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/MoshiChatParser.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/adapters/NetworkAttachmentAdapter.kt
  • stream-chat-android-client/src/test/java/io/getstream/chat/android/client/Mother.kt
  • stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.kt
  • stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/AttachmentParsingTest.kt
  • stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/AttachmentDtoTestData.kt
  • stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/MessageDtoTestData.kt

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

The client now parses generated Attachment models for downstream messages, preserves selected custom fields, maps undeclared fields to domain attachments, and warms generated attachment reflection. Tests cover parsing, mapping, Giphy data, and updated fixtures.

Changes

Attachment handling

Layer / File(s) Summary
Generated attachment parsing and wiring
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/..., stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/MessageDtos.kt, stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt, stream-chat-android-client/src/test/...
NetworkAttachmentAdapter now handles generated Attachment parsing and serialization. Downstream message DTOs use generated attachments. Reflection warm-up includes Attachment. Tests and fixtures cover custom fields and Giphy data.
Attachment domain mapping
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/DomainMapping.kt, stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.kt
Attachment mapping extracts file_size, image, mime_type, and name, converts file_size to Int, and retains remaining values in extraData. OG mapping uses the shared extraction logic.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to ae87a

Downstream attachments now use the generated model while preserving required wire fields and domain mapping behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: velikovpetar, andremion

Sequence Diagram(s)

sequenceDiagram
  participant JSON
  participant MoshiChatParser
  participant NetworkAttachmentAdapter
  participant Attachment
  participant DomainMapping
  participant DomainAttachment
  JSON->>MoshiChatParser: deserialize attachment payload
  MoshiChatParser->>NetworkAttachmentAdapter: parse attachment JSON
  NetworkAttachmentAdapter->>Attachment: populate typed fields and custom
  Attachment->>DomainMapping: map generated attachment
  DomainMapping->>DomainAttachment: set fields and extraData
Loading

Poem

A rabbit packs fields in a bright little stream
Custom maps keep Giphy close to the dream
Four names hop neatly to domains in a row
Reflection warms softly before messages flow
The parser thumps paws: “All attachments go!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 10 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: adopting the generated Attachment model for downstream message attachments.
Description check ✅ Passed The description explains the goal, implementation, scope limitations, retained data behavior, and testing. UI screenshots and checklist items are not relevant or are not completed, but the description…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the goal, implementation, scope limitations, retained data behavior, and testing. UI screenshots and checklist items are not relevant or are not completed, but the description is otherwise sufficiently complete for this non-UI change.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch migrate/attachment-downstream

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gpunto
gpunto added this pull request to the merge queue Aug 31, 2026
Merged via the queue into develop with commit 21c0925 Aug 31, 2026
19 of 20 checks passed
@gpunto
gpunto deleted the migrate/attachment-downstream branch August 31, 2026 14:10
@stream-public-bot stream-public-bot added the released Included in a release label Sep 1, 2026
@stream-public-bot

Copy link
Copy Markdown
Contributor

🚀 Available in v7.10.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:internal Internal changes / housekeeping released Included in a release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants