Skip to content

Accept null file_size in AttachmentDto#6462

Merged
gpunto merged 2 commits into
developfrom
null-file-size
May 22, 2026
Merged

Accept null file_size in AttachmentDto#6462
gpunto merged 2 commits into
developfrom
null-file-size

Conversation

@gpunto
Copy link
Copy Markdown
Contributor

@gpunto gpunto commented May 21, 2026

Goal

It seems that backend can return null for file_size on attachments, but AttachmentDto.file_size was non-nullable so explicit nulls in the JSON caused deserialization to fail.

This also aligns with iOS, where AttachmentFile decodes a missing or non-decodable file_size as 0.

Closes #6428

Implementation

  • AttachmentDto.file_size is now Int?, so an explicit null from the API deserializes cleanly.
  • DomainMapping maps null to 0 when mapping to the domain Attachment.fileSize.

Testing

  • New unit test Deserialize JSON attachment with null file_size covers the parser path.
  • Existing attachment parser and mapping tests continue to pass.

Summary by CodeRabbit

  • Bug Fixes

    • Improved attachment data handling by making file size processing null-safe. When file size information is unavailable, the system now defaults to zero instead of propagating null values, ensuring more robust behavior.
  • Tests

    • Expanded test coverage with new test cases validating correct deserialization and mapping of attachments when file size values are null or missing.

Review Change Stack

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

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
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 5.82 MB 5.82 MB 0.00 MB 🟢
stream-chat-android-ui-components 11.02 MB 11.02 MB 0.00 MB 🟢
stream-chat-android-compose 12.42 MB 12.42 MB 0.00 MB 🟢

@gpunto gpunto marked this pull request as ready for review May 21, 2026 15:55
@gpunto gpunto requested a review from a team as a code owner May 21, 2026 15:55
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

Walkthrough

This PR makes AttachmentDto.file_size nullable to handle API responses with explicit null values. The DTO now coerces null to 0 during domain mapping, and deserialization tests verify the behavior with new null-file-size fixtures.

Changes

Attachment file_size nullable handling

Layer / File(s) Summary
DTO contract and domain mapping
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/AttachmentDto.kt, 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
AttachmentDto.file_size is now Int? (default 0). The mapping applies null-safe coercion (file_size ?: 0) when converting to domain Attachment. Test expectation updated to validate the mapping behavior.
Test coverage for null file_size
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/AttachmentDtoAdapterTest.kt
New test fixtures (jsonWithNullFileSize, attachmentWithNullFileSize) and deserialization test verify that null file_size in JSON is correctly parsed to the expected DTO with null field.

🎯 2 (Simple) | ⏱️ ~8 minutes


🐰 A nullable file, oh what a sight!
Once broken by null, now fixed just right.
The API sends nothing, and we don't despair,
Zero by default—a null-safe affair! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. 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 and concisely summarizes the main change: making AttachmentDto.file_size accept null values instead of requiring a non-null integer.
Description check ✅ Passed The description includes Goal and Implementation sections explaining the change, test coverage, and alignment with iOS. However, it lacks UI Changes section (not applicable), Testing details (partially covered), and incomplete Contributor/Reviewer checklists.
Linked Issues check ✅ Passed All requirements from issue #6428 are met: AttachmentDto.file_size is now nullable (Int?), domain mapping coerces null to 0, and new unit tests verify deserialization of null file_size.
Out of Scope Changes check ✅ Passed All changes are directly scoped to resolving issue #6428: making file_size nullable in the DTO, updating domain mapping, and adding tests for the null case.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch null-file-size

Warning

Review ran into problems

🔥 Problems

Stopped 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 @coderabbit review after the pipeline has finished.


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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.kt (1)

550-577: ⚡ Quick win

Consider 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 an AttachmentDto with file_size = null and verifies it maps to fileSize = 0 would 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

📥 Commits

Reviewing files that changed from the base of the PR and between acf931d and e4d7725.

📒 Files selected for processing (5)
  • 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/AttachmentDto.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/AttachmentDtoAdapterTest.kt
  • stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/AttachmentDtoTestData.kt

@sonarqubecloud
Copy link
Copy Markdown

@gpunto gpunto enabled auto-merge (squash) May 22, 2026 14:30
@gpunto gpunto merged commit 5edb6a3 into develop May 22, 2026
16 checks passed
@gpunto gpunto deleted the null-file-size branch May 22, 2026 14:40
@stream-public-bot stream-public-bot added the released Included in a release label May 22, 2026
@stream-public-bot
Copy link
Copy Markdown
Contributor

🚀 Available in v7.2.0

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

Labels

pr:bug Bug fix released Included in a release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AttachmentDto / Moshi fails when API returns file_size: null on attachments (watch/query surfaces as NETWORK_FAILED / code 1000)

3 participants