Adopt the generated Attachment for downstream message attachments - #6671
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe client now parses generated ChangesAttachment handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to 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: 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
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation 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.
✨ Finishing Touches 💡 1📝 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 |
|
🚀 Available in v7.10.0 |


Goal
Adopt the generated
Attachmentfor downstream message attachments.Part of AND-1291
Implementation
AttachmentRequestAdapterbecomesNetworkAttachmentAdapter. The same generated model is both therequest and the response body, and its
fromJsonused to throw, so it now parses as well as serializes.Renamed rather than kept as
AttachmentAdapterbecauseparser2.direct.AttachmentAdapteralready existsfor the WebSocket event path.
DownstreamMessageDtoandDownstreamDraftMessageDtoto the generated model.UpstreamMessageDtoandAttachment.toDto()keep the hand-written DTO, so it stays for now.Attachment.toDomain()readsfile_size,image,mime_typeandnameback out of the collectedcustom 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
extraDatainstead. An undeclared number arrives untyped, sofile_sizeis aDoubleand needs thenumeric conversion rather than a cast to
Int.author_icon,color,footer,footer_icon,pretext,actions,fieldsandgiphyare declaredby the generated model but were not by the hand-written DTO, so they would stop reaching
Attachment.extraData. They go toalsoKeepInExtraData, and away with AND-1398.ChatClient.warmUpReflection()also warms the generated model. It lists the classes whoseKClass.memberscosts hundreds of milliseconds on first use, so leaving it stale means the first realparse pays that cost.
Notes
giphyis kept as the raw object rather than rebuilt from the typed field.Attachment.giphyInfo()inui-common reads gif urls out of
extraData["giphy"], and keeping the wire object preserves the exact mapit already reads. The reference implementation re-emitted it through
Images.toLegacyMap(), which can onlyreproduce the seven variants and five inner keys the model declares;
alsoKeepInExtraDatadid not existwhen that was written.
A partial
giphycannot arrive:giphy.Imagesis seven value-type structs of five value-type strings withno
omitempty, so an unfilled variant serializes as empty strings rather than being omitted.Testing
AttachmentParsingTestgains a generated-path region covering the undeclared root fields landing incustom, the declared keys the keep set holds, and the giphy object keeping the shapegiphyInfo()reads. The existing DTO-path and direct-path tests are untouched.
DomainMappingTestcovers the recovery of the four undeclared fields, that they do not also linger inextraData, and the giphy map surviving the mapping./giphymessage returned all seven variants with a usableoriginal.urland
actionsretained, and an uploaded file kept itsname,mimeTypeandfileSizethrough both thesend response and a fresh channel query with no wire names left in
extraData. Giphy rendering checkedby hand in the sample app, since the risk here is a display regression rather than a parse failure.
Summary by CodeRabbit