Accept null file_size in AttachmentDto#6462
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
WalkthroughThis PR makes ChangesAttachment file_size nullable handling
🎯 2 (Simple) | ⏱️ ~8 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.kt (1)
550-577: ⚡ Quick winConsider adding an explicit test case for null file_size mapping.
While the current test correctly handles the nullable field with
?: 0, adding a dedicated test case that explicitly creates anAttachmentDtowithfile_size = nulland verifies it maps tofileSize = 0would improve coverage and documentation of this behavior.Suggested test case
`@Test` fun `AttachmentDto with null file_size is mapped to Attachment with fileSize 0`() { val attachmentDto = randomAttachmentDto().copy(file_size = null) val sut = Fixture().get() val attachment = with(sut) { attachmentDto.toDomain() } attachment.fileSize shouldBeEqualTo 0 }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.kt` around lines 550 - 577, Add a focused unit test that verifies null file_size on AttachmentDto maps to 0 on Attachment: create an AttachmentDto via randomAttachmentDto().copy(file_size = null), call toDomain() using the same Fixture().get() setup, and assert the resulting Attachment.fileSize equals 0; reference AttachmentDto, file_size, Attachment, fileSize, toDomain, randomAttachmentDto and Fixture().get() when locating where to add the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.kt`:
- Around line 550-577: Add a focused unit test that verifies null file_size on
AttachmentDto maps to 0 on Attachment: create an AttachmentDto via
randomAttachmentDto().copy(file_size = null), call toDomain() using the same
Fixture().get() setup, and assert the resulting Attachment.fileSize equals 0;
reference AttachmentDto, file_size, Attachment, fileSize, toDomain,
randomAttachmentDto and Fixture().get() when locating where to add the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 73788c53-f8bf-4bea-bb96-fca808185220
📒 Files selected for processing (5)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/DomainMapping.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/AttachmentDto.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/AttachmentDtoAdapterTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/AttachmentDtoTestData.kt
|
|
🚀 Available in v7.2.0 |



Goal
It seems that backend can return
nullforfile_sizeon attachments, butAttachmentDto.file_sizewas non-nullable so explicit nulls in the JSON caused deserialization to fail.This also aligns with iOS, where
AttachmentFiledecodes a missing or non-decodablefile_sizeas0.Closes #6428
Implementation
AttachmentDto.file_sizeis nowInt?, so an explicitnullfrom the API deserializes cleanly.DomainMappingmapsnullto0when mapping to the domainAttachment.fileSize.Testing
Deserialize JSON attachment with null file_sizecovers the parser path.Summary by CodeRabbit
Bug Fixes
Tests