Introduce thread safe AsyncImageHeadersProvider for custom image headers#6203
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
...ommon/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt
Show resolved
Hide resolved
SDK Size Comparison 📏
|
AsyncImageHeadersProviderAsyncImageHeadersProvider for custom image headers
WalkthroughThe pull request introduces async HTTP header injection for image requests in Stream Chat Android. A new Changes
Sequence DiagramsequenceDiagram
participant User as ChatTheme User
participant Theme as ChatTheme
participant Loader as ImageLoader
participant Interceptor as ImageHeadersInterceptor
participant Provider as AsyncImageHeadersProvider
participant IO as IO Dispatcher
User->>Theme: Provide asyncImageHeadersProvider
Theme->>Loader: Build ImageLoader with interceptor
User->>Loader: Request image (URL)
activate Loader
Loader->>Interceptor: Intercept request
activate Interceptor
Interceptor->>IO: Switch to IO dispatcher
activate IO
IO->>Provider: getImageRequestHeaders(url)
activate Provider
Provider-->>IO: Return headers Map
deactivate Provider
IO-->>Interceptor: Headers ready
deactivate IO
Interceptor->>Interceptor: Inject headers into request
Interceptor->>Loader: Proceed with chain
deactivate Interceptor
Loader-->>User: Return image
deactivate Loader
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt (1)
65-69: Defensively copy the interceptor list before storing it.At Line 65, the factory keeps a reference to the caller-owned list. If that list is mutated later, loader behavior changes implicitly.
♻️ Suggested fix
) : this(builder) { - this.interceptors = interceptors + this.interceptors = interceptors.toList() } private var interceptors: List<Interceptor> = emptyList()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt` around lines 65 - 69, The factory currently stores a direct reference to the caller-owned list in StreamImageLoaderFactory (the constructor assignment to this.interceptors), which can be mutated externally; change the constructor to defensively copy the provided interceptors list (e.g., assign a new List/ArrayList created from the parameter) before storing it in the private var interceptors to ensure internal immutability of StreamImageLoaderFactory's interceptor state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt`:
- Around line 65-69: The factory currently stores a direct reference to the
caller-owned list in StreamImageLoaderFactory (the constructor assignment to
this.interceptors), which can be mutated externally; change the constructor to
defensively copy the provided interceptors list (e.g., assign a new
List/ArrayList created from the parameter) before storing it in the private var
interceptors to ensure internal immutability of StreamImageLoaderFactory's
interceptor state.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
stream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatTheme.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/ImageHeadersInterceptor.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/StreamCoilImageLoaderFactory.ktstream-chat-android-ui-common/api/stream-chat-android-ui-common.apistream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/AsyncImageHeadersProvider.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt
gpunto
left a comment
There was a problem hiding this comment.
Looks good. Left just a thought and a suggestion
...ommon/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt
Show resolved
Hide resolved
...mmon/src/main/kotlin/io/getstream/chat/android/ui/common/helper/AsyncImageHeadersProvider.kt
Show resolved
Hide resolved
|
|
🚀 Available in v6.34.0 |


Goal
The existing
ImageHeadersProvideris synchronous and called on the main thread, blocking the UI for any non-trivial work (e.g. reading an encrypted auth token). This PR introducesAsyncImageHeadersProvider— a suspending, thread-safe alternative that is invoked inside Coil's interceptor pipeline on the IO dispatcher, making blocking or suspending operations safe.Implementation
AsyncImageHeadersProvider(stream-chat-android-ui-common): Newsuspendinterface with a singlegetImageRequestHeaders(url: String): Map<String, String>method. Always invoked onDispatchers.IO.ImageHeadersInterceptor(stream-chat-android-compose): A CoilInterceptorthat wraps anAsyncImageHeadersProviderand injects its headers into every image request via the background interceptor chain.StreamCoilImageLoaderFactory: Extended with a newimageLoader(context, interceptors)overload. The default factory (DefaultStreamCoilImageLoaderFactory) supports this out of the box; custom SAM-lambda factories fall back to the existing single-argimageLoader(context)and will not receive the interceptors.StreamImageLoaderFactory: Added a new constructor accepting aList<Interceptor>to prepend Coil interceptors to the component registry. Also removed the now-unnecessary custom OkHttp network fetcher setup (cache-control header + dispatcher config), simplifying the factory.ChatTheme: AddedasyncImageHeadersProvider: AsyncImageHeadersProvider? = nullparameter. When non-null,ChatThemebuilds the image loader with theImageHeadersInterceptorinjected. The oldimageHeadersProviderparameter andChatTheme.streamImageHeadersProvideraccessor are now@Deprecated.Testing
AsyncImageHeadersProviderimplementation toChatTheme(asyncImageHeadersProvider = ...)that injects anAuthorizationheader.Thread.currentThread().name).imageHeadersProviderstill works but emits a deprecation warning.Summary by CodeRabbit
New Features
asyncImageHeadersProviderparameter to supply custom headers during image loading.Deprecations
ImageHeadersProviderdeprecated in favor ofasyncImageHeadersProviderfor async header injection.