Skip to content

Register the OkHttp network fetcher explicitly in the default image loader#6384

Merged
VelikovPetar merged 2 commits intodevelopfrom
chore/coil-image-loader
Apr 27, 2026
Merged

Register the OkHttp network fetcher explicitly in the default image loader#6384
VelikovPetar merged 2 commits intodevelopfrom
chore/coil-image-loader

Conversation

@andremion
Copy link
Copy Markdown
Contributor

@andremion andremion commented Apr 24, 2026

Goal

Since the Coil 2 → Coil 3 migration in 6.13.0, customers have reported blank/placeholder images in release builds (support ticket). Coil 3 registers OkHttpNetworkFetcherFactory via ServiceLoader, which R8 full-mode can strip, leaving the ImageLoader without a fetcher for HTTP URLs. Registering it explicitly removes the ServiceLoader dependency.

Implementation

StreamImageLoaderFactory — add OkHttpNetworkFetcherFactory() to the components block. Integrators delegating to StreamImageLoaderFactory (Compose StreamCoilImageLoaderFactory.defaultFactory() and the XML StreamCoil singleton) pick up the fix for free. No public API changes.

Not included — consumer R8 keep rules for Coil 3 ServiceLoader descriptors. With the fetcher registered in code, those only matter for integrators building a fully custom ImageLoader, which is already covered by the docs.

Testing

  1. ./gradlew :stream-chat-android-ui-common:testDebugUnitTest --tests "StreamImageLoaderFactoryTest" — includes a new assertion that fetcherFactories contains a NetworkFetcher.Factory.
  2. Build a minified release of stream-chat-android-compose-sample on a device that previously reproduced the blank-image symptom, open a channel with image attachments, confirm images load without any integrator-side fetcher registration.
  3. Run the Compose sample in debug — images, GIFs, and video thumbnails render as before.

No UI changes.

Summary by CodeRabbit

  • Tests
    • Added test coverage to verify proper registration of image loading components.

Coil 3 auto-registers OkHttpNetworkFetcherFactory via ServiceLoader, which R8 can strip in release builds, leaving HTTP image loading without a fetcher and rendering only placeholders. Adding it directly to the component registry removes the ServiceLoader dependency for the default SDK path.
@andremion andremion requested a review from a team as a code owner April 24, 2026 10:55
@andremion andremion added the pr:improvement Improvement label Apr 24, 2026
@andremion
Copy link
Copy Markdown
Contributor Author

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 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.

🎉 Great job! This PR is ready for review.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Walkthrough

Two files modified: StreamImageLoaderFactory.kt adds OkHttpNetworkFetcherFactory to the Coil ImageLoader component registry for network image fetching. A corresponding unit test validates that NetworkFetcher.Factory is properly registered in the image loader's fetcher factories.

Changes

Cohort / File(s) Summary
ImageLoader Configuration
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt
Added OkHttpNetworkFetcherFactory to the .components {} registry, registered after custom interceptors and before decoder factories.
ImageLoader Verification
stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactoryTest.kt
New unit test added to verify NetworkFetcher.Factory is registered in the image loader's component fetcherFactories.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A fetcher so fine, from OkHttp it came,
In Coil's registry, now registered with fame,
Network images streaming, both swift and so bright,
The test validates all—everything's right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 specifically describes the main change: explicitly registering the OkHttp network fetcher in the default image loader to resolve the ServiceLoader stripping issue.
Description check ✅ Passed The description provides a comprehensive explanation of the problem, implementation, and testing approach, though some contributor checklist items are not explicitly marked as completed.
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.

✏️ 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 chore/coil-image-loader

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.

Actionable comments posted: 1

🤖 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-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactoryTest.kt`:
- Around line 107-115: The test newImageLoader registers network fetcher for
HTTP URLs is currently asserting only that any NetworkFetcher.Factory exists,
which can be satisfied by ServiceLoader-discovered implementations; update the
test to assert the explicit OkHttp implementation is present by checking for an
instance of OkHttpNetworkFetcherFactory (or its exact class name) within
imageLoader.components.fetcherFactories, or alternatively mock/override
ServiceLoader so you can verify StreamImageLoaderFactory.newImageLoader actually
calls add(OkHttpNetworkFetcherFactory()) rather than relying on a discovered
factory; reference the test method name,
StreamImageLoaderFactory.newImageLoader,
imageLoader.components.fetcherFactories, and OkHttpNetworkFetcherFactory in your
change.
🪄 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: c48c1968-99ac-4566-98f2-98b3462f3234

📥 Commits

Reviewing files that changed from the base of the PR and between 217d159 and 648b2f9.

📒 Files selected for processing (2)
  • stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt
  • stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactoryTest.kt

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 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.33 MB 12.33 MB 0.00 MB 🟢

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@VelikovPetar VelikovPetar left a comment

Choose a reason for hiding this comment

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

Should we also do the fix for V6?

@andremion
Copy link
Copy Markdown
Contributor Author

Should we also do the fix for V6?

Yes, we should. I'll cherry-pick this into a separate v6 PR.

@VelikovPetar VelikovPetar merged commit 6d5d6c1 into develop Apr 27, 2026
16 checks passed
@VelikovPetar VelikovPetar deleted the chore/coil-image-loader branch April 27, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:improvement Improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants