Open
Conversation
…ver time (#6199) * Fix createdLocallyAt using NTP-style server clock offset estimation Co-Authored-By: Claude <noreply@anthropic.com> * Pr remarks * Adjust thread message createdLocallyAt. * Ensure exceedsSyncThreshold is compared against estimated server time (where applicable). * Add max allowed offset. --------- Co-authored-by: Claude <noreply@anthropic.com>
…ers.IO)` (#6284) Co-authored-by: Claude <noreply@anthropic.com>
* Update `DependencyResolverTest` to verify error handling when dependency resolution races with disconnection. * Prevent race conditions during disconnects in `ChatClient`.
- Update `StorageHelper` and `AttachmentMetaDataMapper` to safely handle cases where content URIs (e.g. cloud-backed files) cannot be opened. - Introduce `hasUnresolvedAttachments` state in `AttachmentsPickerViewModel` to track failed attachment resolutions. - Show a toast message in both View-based and Compose attachment pickers when files are unavailable and need to be downloaded to the device. - Add `clearUnresolvedAttachments` to reset the error state after it has been consumed by the UI. - Add unit tests for unresolved attachment scenarios in `AttachmentsPickerViewModelTest`.
…l` (#6280) * Deprecate imagePreviewUrl and use type-specific attachment URL fields Co-Authored-By: Claude <noreply@anthropic.com> * Extract common extensions. --------- Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: adasiewiczr <17440581+adasiewiczr@users.noreply.github.com>
* Add new CDN contract. * Add CDN for document files. * Add CDN support for downloading attachments. * Deprecate current CDN methods. * Add progress indicator snackbar. * Add useDocumentGView config flag. * Add file sharing cache handling. * Add file sharing cache handling. * Remove CDNResponse.kt * Add tests * PR remarks
# Conflicts: # README.md # gradle.properties # metrics/size.json # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/ChatClientStateExtensions.kt # stream-chat-android-compose/api/stream-chat-android-compose.api # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/ImageAttachmentPreviewContent.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/LinkAttachmentContent.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentPreviewContent.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentQuotedContent.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/handler/DocumentAttachmentPreviewHandler.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/channel/attachments/ChannelMediaAttachmentsScreen.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/composer/ComposerLinkPreview.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/QuotedMessage.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentsPicker.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatTheme.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/StorageHelperWrapper.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/extensions/internal/AttachmentExtensions.kt # stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.kt # stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.kt # stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/utils/extensions/Attachment.kt # stream-chat-android-ui-components/api/stream-chat-android-ui-components.api # stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/AttachmentsPickerDialogFragment.kt # stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.kt # stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/MessageListView.kt # stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/navigation/destinations/AttachmentDestination.kt # stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModel.kt
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
WalkthroughA comprehensive CDN abstraction is introduced alongside server clock offset synchronization. The changes integrate CDN-based URL rewriting and header injection across media playback, image loading, HTTP requests, and file downloads. Multiple legacy customization interfaces are deprecated in favor of the new CDN configuration model. Attachment URL handling is refactored to use specific fields instead of a generic Changes
Sequence DiagramsequenceDiagram
actor User
participant App as Application
participant Client as ChatClient
participant CDN as CDN Interface
participant Network as Network Layer<br/>(OkHttp/Coil)
participant Media as Media System<br/>(ExoPlayer)
participant Server as Stream Server
User->>App: Request image/file/media
App->>Client: downloadAttachment() or loadMedia()
Client->>CDN: imageRequest(url) or fileRequest(url)
alt CDN Transformation
CDN-->>Client: CDNRequest{newUrl, headers}
Client->>Network: Request with transformed URL + headers
else No CDN
CDN-->>Client: CDNRequest{originalUrl, null}
Client->>Network: Request with original URL
end
Network->>Server: HTTP GET (CDN URL + headers)
Server-->>Network: Media/File content
Network-->>Media: Stream data
Media-->>App: Render/Playback
App-->>User: Display content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/internal/StorageHelper.kt (1)
271-275:⚠️ Potential issue | 🟡 MinorBug: Wrong column index used in null check for duration.
The condition checks
cursor.isNull(fileSizeIndex)but should checkcursor.isNull(durationIndex).🐛 Proposed fix
- val duration = if (durationIndex != -1 && !cursor.isNull(fileSizeIndex)) { + val duration = if (durationIndex != -1 && !cursor.isNull(durationIndex)) { cursor.getLong(durationIndex) } else { 0L }🤖 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/helper/internal/StorageHelper.kt` around lines 271 - 275, The null-check for the duration column uses the wrong index: change the condition in StorageHelper (where durationIndex and fileSizeIndex are used) to call cursor.isNull(durationIndex) instead of cursor.isNull(fileSizeIndex) so the code verifies the duration column before calling cursor.getLong(durationIndex); update the conditional that sets duration to use the correct durationIndex null check.
🧹 Nitpick comments (10)
stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModel.kt (1)
208-209: Consider adding KDoc for the public API.The
completeRecordingfunction is part of the public API and now accepts a completion callback. Per coding guidelines, public APIs should be documented with KDoc. While other recording functions in this file also lack KDoc, this new parameter would benefit from documentation explaining whenonCompleteis invoked and whatResult<Attachment>contains.Suggested KDoc
+ /** + * Completes the current audio recording. + * + * `@param` onComplete Optional callback invoked with the resulting [Attachment] on success + * or an error on failure. + */ public fun completeRecording(onComplete: ((Result<Attachment>) -> Unit)? = null): Unit = messageComposerController.completeRecording(onComplete)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModel.kt` around lines 208 - 209, Add KDoc to the public function MessageComposerViewModel.completeRecording to document the new onComplete parameter: explain when the callback is invoked (e.g., after recording successfully completes or fails), describe the Result<Attachment> contents for success (the created Attachment) and failure (the error/exception), and note threading/dispatch expectations and any nullability/optional behavior; reference the delegation to messageComposerController.completeRecording in the docs so callers understand behavior is forwarded to that controller.stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/overview/internal/MediaAttachmentAdapter.kt (1)
191-193: Consider aligning DiffUtil comparison with URL selection logic.The
areItemsTheSameuses(imageUrl ?: thumbUrl)uniformly, whileloadImageselects the URL based on attachment type (imageUrlfor images,thumbUrlotherwise). This inconsistency is unlikely to cause issues in practice since attachment types don't change, but for consistency the comparison could mirror the type-aware selection.Optional: Type-aware comparison
override fun areItemsTheSame(oldItem: AttachmentGalleryItem, newItem: AttachmentGalleryItem): Boolean { - return (oldItem.attachment.imageUrl ?: oldItem.attachment.thumbUrl) == - (newItem.attachment.imageUrl ?: newItem.attachment.thumbUrl) && + val oldUrl = if (oldItem.attachment.isImage()) oldItem.attachment.imageUrl else oldItem.attachment.thumbUrl + val newUrl = if (newItem.attachment.isImage()) newItem.attachment.imageUrl else newItem.attachment.thumbUrl + return oldUrl == newUrl && oldItem.createdAt == newItem.createdAt }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/overview/internal/MediaAttachmentAdapter.kt` around lines 191 - 193, The DiffUtil comparison in MediaAttachmentAdapter::areItemsTheSame currently compares (imageUrl ?: thumbUrl) but loadImage selects the URL based on attachment type; update areItemsTheSame to mirror loadImage's type-aware URL selection (use attachment.type or the same predicate loadImage uses to decide image vs thumbnail) when comparing URLs and keep the createdAt check; modify the comparison logic in areItemsTheSame to call the same helper or replicate the same conditional used by loadImage so both routines pick the same URL for comparison.stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/AttachmentsPickerDialogFragment.kt (1)
94-101: Consider: User feedback for unresolved attachments in XML UI.The Compose
AttachmentPickershows a toast when attachments cannot be resolved, but the XMLAttachmentsPickerDialogFragmentsilently drops them. This could confuse users when some selected files don't appear in the composer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/AttachmentsPickerDialogFragment.kt` around lines 94 - 101, The XML picker silently drops unresolved attachments in AttachmentsPickerDialogFragment.onDismiss when style.saveAttachmentsOnDismiss is true; change the logic to detect which selectedAttachments failed to convert (e.g., compare selectedAttachments to the result of selectedAttachments.mapNotNull { it.toAttachment(requireContext()) } or collect failed items via selectedAttachments.filter { it.toAttachment(requireContext()) == null }), call attachmentsSelectionListener?.onAttachmentsSelected(...) with the successfully resolved attachments as before, and then show a user-facing Toast (using requireContext()) that indicates how many or which attachments could not be resolved so users get feedback about dropped files.stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/ImageAssetTransformer.kt (1)
37-54: Add thread-expectation note totransformKDoc.Please document the expected calling thread/context for
transform(asset: Any)(and any state assumptions), so custom implementations are safe.As per coding guidelines
**/*.kt: “Document public APIs with KDoc, including thread expectations and state notes”.🤖 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/helper/ImageAssetTransformer.kt` around lines 37 - 54, The KDoc for ImageAssetTransformer.transform(asset: Any) is missing the expected calling thread and state assumptions; update the KDoc for the public function transform in ImageAssetTransformer to explicitly state which thread/context callers must use (e.g., "Called on main/UI thread" or "May be called from background threads; implementations must be thread-safe"), and document any state assumptions (e.g., no mutation of shared state, no long-running work, must be fast or offloaded). Mention whether implementations may access UI-only resources or must avoid blocking I/O so custom implementations implement correct synchronization or offloading.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/factory/StreamStatePluginFactory.kt (1)
130-143: Add a focused non-zero-offset regression test for this wiring.This is the handoff that makes sync staleness depend on the shared
ServerClockOffset, so a small test proving behavior with a non-zero offset would make this much harder to regress.If you add it, I’d keep it under
runTestwith virtual time because this path is timing-sensitive. Based on learnings: Applies to /src/test//*.kt : Use deterministic tests withrunTest+ virtual time for concurrency-sensitive logic (uploads, sync, message state).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/factory/StreamStatePluginFactory.kt` around lines 130 - 143, Add a deterministic regression test that verifies SyncManager's staleness logic respects a non-zero ServerClockOffset: create a test under src/test/**/*.kt that uses runTest with virtual time, construct the same wiring as in StreamStatePluginFactory (instantiate SyncManager with chatClient.serverClockOffset set to a non-zero value and the now = { System.currentTimeMillis() } replacement if needed), drive the timing-sensitive path (trigger sync/reconnect) and assert that sync staleness/behavior changes according to the provided offset; keep the test focused, use virtual time to avoid flakiness, and target the SyncManager/ServerClockOffset wiring to prevent regressions.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/handler/DocumentAttachmentPreviewHandler.kt (1)
25-37: Document the Activity-context/UI-call expectation for this handler.The new behavior switch is clear, but this public KDoc still doesn't spell out the required call context even though
handleAttachmentPreviewstarts activities. Please add the expected thread/call context and state notes so consumers know what kind ofContextis valid here.As per coding guidelines,
**/*.kt: Document public APIs with KDoc, including thread expectations and state notes.Also applies to: 57-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/handler/DocumentAttachmentPreviewHandler.kt` around lines 25 - 37, Update the KDoc for DocumentAttachmentPreviewHandler to state that the provided Context must be an Activity (or an unwrapped Context capable of starting activities) and that handleAttachmentPreview will start activities (via Intents/CustomTabs/Google Docs Viewer) so it must be invoked on the main/UI thread when the Activity is in a valid, not-destroyed state; reference the constructor parameter useDocumentGView and the public method handleAttachmentPreview in the note so callers know which behavior depends on that flag and when to supply an Activity-context.stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/DownloadAttachmentUriGenerator.kt (1)
22-30: Add thread/state notes to the deprecation KDoc.These types are still public API until removal, and the new deprecation docs don't yet capture the expected thread/state contract for
generateDownloadUri.As per coding guidelines,
**/*.kt: Document public APIs with KDoc, including thread expectations and state notes.Also applies to: 42-50
🤖 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/helper/DownloadAttachmentUriGenerator.kt` around lines 22 - 30, Update the deprecation KDoc for the public interface DownloadAttachmentUriGenerator (and its generateDownloadUri contract) to include thread and state expectations: state whether implementations may be called on any thread or must be called on the main/UI thread, whether it must be non-blocking, and whether it may be invoked after related message/attachment objects are recycled or mutated; add a brief "State/Threading" note describing these expectations so callers and implementers know concurrency and lifecycle guarantees. Reference the DownloadAttachmentUriGenerator interface and its generateDownloadUri method when adding this KDoc note, and mirror the same change for the analogous KDoc block at the other occurrence mentioned.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/QuotedMessageBodyBuilder.kt (1)
201-212: Resize video thumbnail URLs here as well.Images already go through
applyStreamCdnImageResizingIfEnabled, andAttachment.imagePreviewDatanow does the same for video thumbnails. Quoted video replies still use the rawthumbUrl, so they skip the CDN downsizing path.💡 Suggested change
type == AttachmentType.VIDEO -> { videoCount++ fileCount++ - mediaPreviewData = attachment.upload ?: attachment.thumbUrl + mediaPreviewData = attachment.upload ?: attachment.thumbUrl + ?.applyStreamCdnImageResizingIfEnabled(streamCdnImageResizing) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/QuotedMessageBodyBuilder.kt` around lines 201 - 212, The video branch in QuotedMessageBodyBuilder sets mediaPreviewData to attachment.upload ?: attachment.thumbUrl but does not apply CDN image resizing; update the AttachmentType.VIDEO branch so that when attachment.upload is null you pass attachment.thumbUrl through applyStreamCdnImageResizingIfEnabled(streamCdnImageResizing) (same helper used for images) so mediaPreviewData receives the resized thumb URL; adjust the assignment in the video case where mediaPreviewData is set (and ensure you reference the same helper used for images and imagePreviewData).stream-chat-android-client/src/test/java/io/getstream/chat/android/client/cdn/internal/CDNOkHttpInterceptorTest.kt (1)
65-82: Add a same-key header override regression test.This suite proves additive merging, but it never locks in what should happen when the original request already has
Authorization. Without that case, a regression that keeps both values would still pass here.♻️ Suggested test addition
+ `@Test` + fun `intercept CDN headers override existing headers for same key`() { + val cdn = object : CDN { + override suspend fun fileRequest(url: String) = + CDNRequest(url, headers = mapOf("Authorization" to "new-token")) + } + val interceptor = CDNOkHttpInterceptor(cdn) + val originalRequest = Request.Builder() + .url("https://original.com/file.mp4") + .addHeader("Authorization", "old-token") + .build() + val chain = FakeChain(FakeResponse(200), request = originalRequest) + + val response = interceptor.intercept(chain) + + assertEquals("new-token", response.request.header("Authorization")) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/cdn/internal/CDNOkHttpInterceptorTest.kt` around lines 65 - 82, Add a regression test that verifies same-key header overriding for Authorization: create a CDN stub (implementing CDN.fileRequest) that returns a CDNRequest with an "Authorization" header, build an original Request with an existing "Authorization" header value, run CDNOkHttpInterceptor.intercept using a FakeChain/FakeResponse, and assert that response.request.header("Authorization") equals the CDN-provided value (and not the original), ensuring the interceptor replaces the original Authorization header rather than keeping both.stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt (1)
95-95: Use a share-specific cache prefix before relying on prefix-scoped eviction.Line 95 now evicts by
cacheFilePrefix, while Lines 150 and 169 still default to"TMP". That prefix is generic enough to let this manager clean up unrelated cache entries if anything else in the app uses the same convention. A feature-specific prefix keeps the TTL/size policy scoped to share files only.♻️ Suggested diff
public data class ShareCacheConfig( - val cacheFilePrefix: String = "TMP", + val cacheFilePrefix: String = "stream_share_", val bitmapShareFilename: String = "shared_image.png", val bitmapQuality: Int = 90, val cacheTtlMs: Long = 5 * 60 * 1000L, val maxCacheSizeBytes: Long = 25L * 1024 * 1024, )Also applies to: 150-150, 168-170
🤖 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/internal/file/StreamShareFileManager.kt` at line 95, StreamShareFileManager is evicting and creating cache files using a generic prefix ("TMP" / config.cacheFilePrefix) which can collide with other cache users; change the calls in StreamShareFileManager (the evictCacheFiles call and places that default to "TMP" around file creation) to use a share-specific prefix (e.g., derive a constant like SHARE_CACHE_PREFIX or append a suffix to config.cacheFilePrefix such as "${config.cacheFilePrefix}_SHARE") so eviction and TTL/size policies only target share files; update every usage in this class (the evictCacheFiles invocation and the code paths that currently use "TMP") to use that new share-specific prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 5: The hero image tag <img
src="/docs/stream-chat-android-github-cover.png"/> is missing an alt attribute;
update that element in README.md to include a concise descriptive alt text (for
example alt="Stream Chat Android GitHub cover") so screen readers can interpret
the image and satisfy markdownlint MD045.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/internal/CDNDataSourceFactory.kt`:
- Around line 71-78: The catch block in CDNDataSourceFactory around runBlocking
{ cdn.fileRequest(url) } must not swallow thread interruptions or coroutine
cancellations; change the exception handling so that InterruptedException and
CancellationException (or any Throwable that represents cancellation) are
re-thrown immediately and only non-cancellation exceptions are logged and cause
fallback to CDNRequest(url). Update the catch-site for the generic exception
suppression (`@Suppress`("TooGenericExceptionCaught")) to include a brief comment
explaining why it remains (per coding guidelines) or narrow the caught type to
Exception after rethrowing cancellations, and keep the logger.e(...) and
fallback to CDNRequest only for non-cancellation failures.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt`:
- Around line 411-420: The early snapshot of plugins in resolvePluginDependency
breaks when initialization completes after the snapshot; instead of using the
pre-wait currentPlugins, re-read the plugins list after
awaitInitializationState(RESOLVE_DEPENDENCY_TIMEOUT) (or hold the same
synchronization used by connect/disconnect) so that the resolver lookup uses the
up-to-date plugins; specifically update the code around
currentPlugins/awaitInitializationState/InitializationState.COMPLETE and the
subsequent resolver lookup to fetch plugins again (or perform the lookup under
the same lock) to avoid stale-plugin "was not found" errors.
In `@stream-chat-android-compose/api/stream-chat-android-compose.api`:
- Around line 7053-7054: Add a migration note documenting the breaking change:
the JVM no-arg entry point for MessageComposerViewModel.completeRecording was
removed and now requires a callback parameter (see completeRecording and
completeRecording$default signatures); update CHANGELOG.md and the v7 migration
guide to explain that Kotlin callers will keep no-arg calls after recompilation
(due to default parameters) but Java callers must now supply a Function1
callback or use the synthetic default helper, include an example of the new Java
call shape and a short snippet explaining how to migrate existing Java usages.
- Around line 720-721: Add an entry to the v7 CHANGELOG under
stream-chat-android-compose that documents the breaking JVM signature changes:
list the new signatures for
AttachmentPreviewHandler.defaultAttachmentHandlers(Context, boolean), the
DocumentAttachmentPreviewHandler constructor (now taking useDocumentGView
boolean), and ChatTheme (with useDocumentGView), explain the upgrade path for
Java callers and existing binaries — the new useDocumentGView flag defaults to
true, existing behavior is preserved, and Java callers can pass false to opt
into CDN-aware document handling — and include a short code snippet example
showing how Java callers should call the updated
AttachmentPreviewHandler.defaultAttachmentHandlers(...) and construct
DocumentAttachmentPreviewHandler with useDocumentGView=false to opt out.
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.kt`:
- Around line 1021-1037: The current completeRecording implementation adds the
finished Attachment into selected attachments via
addAttachments(listOf(result.value)), which duplicates/stages the recording
because recordingState already sets _recordingAttachment and syncAttachments()
appends that field; remove the explicit staging to _selectedAttachments: in
MessageComposerController.completeRecording (and the branch that calls
audioRecordingController.completeRecordingSync()), stop calling
addAttachments(...) on successful Result.Success and instead rely on the
existing _recordingAttachment flow (recordingState, _recordingAttachment,
syncAttachments()) so the attachment is only appended once.
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/ImageAssetTransformer.kt`:
- Around line 58-65: Update the KDoc for DefaultImageAssetTransformer to reflect
that it is a pass-through transformer (it returns the asset unchanged) instead
of saying it "doesn't provide any headers"; locate the KDoc block above the
object declaration for DefaultImageAssetTransformer (implementing
ImageAssetTransformer) and replace the outdated phrase with wording like
"returns the asset unchanged" or "pass-through asset transformer" while
preserving the existing deprecation note.
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/internal/CDNImageInterceptor.kt`:
- Around line 52-59: The catch block around cdn.imageRequest(...) in
CDNImageInterceptor currently catches all Exceptions and swallows
CancellationException, preventing coroutine cancellation from propagating;
modify the handler so it rethrows CancellationException (e.g., if (e is
CancellationException) throw e) before logging and falling back to
chain.proceed(), ensuring cancellation is propagated for cdn.imageRequest, and
only non-cancellation exceptions are logged and trigger the fallback behavior.
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt`:
- Line 63: The code currently always compresses to PNG in
StreamShareFileManager.bitmap.compress(...), so the exposed config.bitmapQuality
has no effect; update the config and compress call so the format is configurable
and the quality is honored for lossy formats: add a bitmapFormat (e.g.,
Bitmap.CompressFormat) option to the existing configuration (or replace
bitmapQuality with a format+quality pair), use that bitmapFormat in the
bitmap.compress call instead of hardcoding PNG, and preserve the existing
bitmapQuality for formats that support it (or ignore it when bitmapFormat == PNG
with a short comment); adjust any KDoc to reflect the new configurable format
and behavior.
In
`@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManagerTest.kt`:
- Around line 361-414: The tests use a hard-coded 6*60*1000L TTL which may not
exercise the expiration branch; update both tests to compute the stale timestamp
using ShareCacheConfig().cacheTtlMs (e.g. lastModified =
System.currentTimeMillis() - (ShareCacheConfig().cacheTtlMs + 1)) so the cached
file is definitely expired when shareFileManager calls
fileManager.getFileFromCache; also strengthen assertions by verifying the
miss-specific interactions: for writeAttachmentToShareableFile assert that
chatClient.downloadFile() and fileManager.writeFileInCache(...) are invoked (or
that writeFileInCache is called with the downloaded file) and for
getShareableUriForAttachment assert that no valid URI is returned (isFailure)
and optionally verify download/write are not called or are called according to
expected behavior.
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/viewholder/impl/GiphyViewHolder.kt`:
- Around line 76-77: The bindData in GiphyViewHolder currently uses a non-local
return via "?: return" when resolving the GIF URL, which skips the rest of bind
logic (including setting giphyQueryTextView) and can leave recycled row state
stale; change the logic to avoid returning early by assigning the URL to a
nullable local (e.g., val url = it.giphyInfo(... ) ?:
it.giphyFallbackPreviewUrl) and then handle the null case inline (clear or hide
the image view, set a placeholder, but still proceed to set
giphyQueryTextView.text and other UI state). Ensure any image loading calls (the
code that used the URL) are guarded with an if (url != null) block and that
giphyQueryTextView and other fields on GiphyViewHolder are always updated
regardless of URL presence.
---
Outside diff comments:
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/internal/StorageHelper.kt`:
- Around line 271-275: The null-check for the duration column uses the wrong
index: change the condition in StorageHelper (where durationIndex and
fileSizeIndex are used) to call cursor.isNull(durationIndex) instead of
cursor.isNull(fileSizeIndex) so the code verifies the duration column before
calling cursor.getLong(durationIndex); update the conditional that sets duration
to use the correct durationIndex null check.
---
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/factory/StreamStatePluginFactory.kt`:
- Around line 130-143: Add a deterministic regression test that verifies
SyncManager's staleness logic respects a non-zero ServerClockOffset: create a
test under src/test/**/*.kt that uses runTest with virtual time, construct the
same wiring as in StreamStatePluginFactory (instantiate SyncManager with
chatClient.serverClockOffset set to a non-zero value and the now = {
System.currentTimeMillis() } replacement if needed), drive the timing-sensitive
path (trigger sync/reconnect) and assert that sync staleness/behavior changes
according to the provided offset; keep the test focused, use virtual time to
avoid flakiness, and target the SyncManager/ServerClockOffset wiring to prevent
regressions.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/cdn/internal/CDNOkHttpInterceptorTest.kt`:
- Around line 65-82: Add a regression test that verifies same-key header
overriding for Authorization: create a CDN stub (implementing CDN.fileRequest)
that returns a CDNRequest with an "Authorization" header, build an original
Request with an existing "Authorization" header value, run
CDNOkHttpInterceptor.intercept using a FakeChain/FakeResponse, and assert that
response.request.header("Authorization") equals the CDN-provided value (and not
the original), ensuring the interceptor replaces the original Authorization
header rather than keeping both.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/handler/DocumentAttachmentPreviewHandler.kt`:
- Around line 25-37: Update the KDoc for DocumentAttachmentPreviewHandler to
state that the provided Context must be an Activity (or an unwrapped Context
capable of starting activities) and that handleAttachmentPreview will start
activities (via Intents/CustomTabs/Google Docs Viewer) so it must be invoked on
the main/UI thread when the Activity is in a valid, not-destroyed state;
reference the constructor parameter useDocumentGView and the public method
handleAttachmentPreview in the note so callers know which behavior depends on
that flag and when to supply an Activity-context.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/QuotedMessageBodyBuilder.kt`:
- Around line 201-212: The video branch in QuotedMessageBodyBuilder sets
mediaPreviewData to attachment.upload ?: attachment.thumbUrl but does not apply
CDN image resizing; update the AttachmentType.VIDEO branch so that when
attachment.upload is null you pass attachment.thumbUrl through
applyStreamCdnImageResizingIfEnabled(streamCdnImageResizing) (same helper used
for images) so mediaPreviewData receives the resized thumb URL; adjust the
assignment in the video case where mediaPreviewData is set (and ensure you
reference the same helper used for images and imagePreviewData).
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/DownloadAttachmentUriGenerator.kt`:
- Around line 22-30: Update the deprecation KDoc for the public interface
DownloadAttachmentUriGenerator (and its generateDownloadUri contract) to include
thread and state expectations: state whether implementations may be called on
any thread or must be called on the main/UI thread, whether it must be
non-blocking, and whether it may be invoked after related message/attachment
objects are recycled or mutated; add a brief "State/Threading" note describing
these expectations so callers and implementers know concurrency and lifecycle
guarantees. Reference the DownloadAttachmentUriGenerator interface and its
generateDownloadUri method when adding this KDoc note, and mirror the same
change for the analogous KDoc block at the other occurrence mentioned.
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/ImageAssetTransformer.kt`:
- Around line 37-54: The KDoc for ImageAssetTransformer.transform(asset: Any) is
missing the expected calling thread and state assumptions; update the KDoc for
the public function transform in ImageAssetTransformer to explicitly state which
thread/context callers must use (e.g., "Called on main/UI thread" or "May be
called from background threads; implementations must be thread-safe"), and
document any state assumptions (e.g., no mutation of shared state, no
long-running work, must be fast or offloaded). Mention whether implementations
may access UI-only resources or must avoid blocking I/O so custom
implementations implement correct synchronization or offloading.
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt`:
- Line 95: StreamShareFileManager is evicting and creating cache files using a
generic prefix ("TMP" / config.cacheFilePrefix) which can collide with other
cache users; change the calls in StreamShareFileManager (the evictCacheFiles
call and places that default to "TMP" around file creation) to use a
share-specific prefix (e.g., derive a constant like SHARE_CACHE_PREFIX or append
a suffix to config.cacheFilePrefix such as "${config.cacheFilePrefix}_SHARE") so
eviction and TTL/size policies only target share files; update every usage in
this class (the evictCacheFiles invocation and the code paths that currently use
"TMP") to use that new share-specific prefix.
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/overview/internal/MediaAttachmentAdapter.kt`:
- Around line 191-193: The DiffUtil comparison in
MediaAttachmentAdapter::areItemsTheSame currently compares (imageUrl ?:
thumbUrl) but loadImage selects the URL based on attachment type; update
areItemsTheSame to mirror loadImage's type-aware URL selection (use
attachment.type or the same predicate loadImage uses to decide image vs
thumbnail) when comparing URLs and keep the createdAt check; modify the
comparison logic in areItemsTheSame to call the same helper or replicate the
same conditional used by loadImage so both routines pick the same URL for
comparison.
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/AttachmentsPickerDialogFragment.kt`:
- Around line 94-101: The XML picker silently drops unresolved attachments in
AttachmentsPickerDialogFragment.onDismiss when style.saveAttachmentsOnDismiss is
true; change the logic to detect which selectedAttachments failed to convert
(e.g., compare selectedAttachments to the result of
selectedAttachments.mapNotNull { it.toAttachment(requireContext()) } or collect
failed items via selectedAttachments.filter { it.toAttachment(requireContext())
== null }), call attachmentsSelectionListener?.onAttachmentsSelected(...) with
the successfully resolved attachments as before, and then show a user-facing
Toast (using requireContext()) that indicates how many or which attachments
could not be resolved so users get feedback about dropped files.
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModel.kt`:
- Around line 208-209: Add KDoc to the public function
MessageComposerViewModel.completeRecording to document the new onComplete
parameter: explain when the callback is invoked (e.g., after recording
successfully completes or fails), describe the Result<Attachment> contents for
success (the created Attachment) and failure (the error/exception), and note
threading/dispatch expectations and any nullability/optional behavior; reference
the delegation to messageComposerController.completeRecording in the docs so
callers understand behavior is forwarded to that controller.
🪄 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: c0c45f42-3bdb-4741-8651-252eba521771
⛔ Files ignored due to path filters (2)
docs/sdk-hero-android.pngis excluded by!**/*.pngdocs/stream-chat-android-github-cover.pngis excluded by!**/*.png
📒 Files selected for processing (90)
README.mdstream-chat-android-client/api/stream-chat-android-client.apistream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/ChatClientStateExtensions.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/audio/NativeMediaPlayer.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/CDN.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/CDNRequest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/internal/CDNDataSourceFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/internal/CDNOkHttpInterceptor.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/internal/StreamMediaDataSource.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/factory/StreamStatePluginFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/sync/internal/SyncManager.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/internal/ServerClockOffset.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientConnectionTests.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/DependencyResolverTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/MockClientBuilder.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/cdn/internal/CDNDataSourceTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/cdn/internal/CDNOkHttpInterceptorTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/chatclient/BaseChatClientTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/debugger/ChatClientDebuggerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/file/StreamFileManagerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/internal/SyncManagerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/socket/FakeChatSocket.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/utils/internal/ServerClockOffsetTest.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/channel/attachments/ChannelMediaAttachmentsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/LinkAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/MediaGalleryPreviewScreen.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/handler/AttachmentPreviewHandler.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/handler/DocumentAttachmentPreviewHandler.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/internal/MediaGalleryPage.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/internal/StreamMediaPlayerContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/channel/attachments/ChannelMediaAttachmentsScreen.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/composer/ComposerLinkPreview.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/QuotedMessageBodyBuilder.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPicker.ktstream-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/extensions/internal/AttachmentExtensions.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channel/ChannelMediaAttachmentsPreviewViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/MessageComposerViewModel.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/components/messages/QuotedMessageBodyBuilderTest.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.ktstream-chat-android-ui-common/src/main/AndroidManifest.xmlstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/documents/AttachmentDocumentActivity.javastream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/documents/DocumentAttachmentHandler.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.ktstream-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/helper/DownloadAttachmentUriGenerator.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/DownloadRequestInterceptor.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/ImageAssetTransformer.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/ImageHeadersProvider.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/VideoHeadersProvider.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/internal/StorageHelper.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/internal/CDNImageInterceptor.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/internal/StreamCoil.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/utils/extensions/Attachment.ktstream-chat-android-ui-common/src/main/res/values/strings.xmlstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/images/internal/CDNImageInterceptorTest.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManagerTest.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/utils/extensions/AttachmentExtensionsTest.ktstream-chat-android-ui-components-sample/src/main/kotlin/io/getstream/chat/ui/sample/feature/chat/info/shared/media/ChatInfoSharedMediaFragment.ktstream-chat-android-ui-components/api/stream-chat-android-ui-components.apistream-chat-android-ui-components/detekt-baseline.xmlstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/ChatUI.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/AttachmentMediaActivity.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/internal/AttachmentGalleryImagePageFragment.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/internal/AttachmentGalleryPagerAdapter.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/internal/AttachmentGalleryVideoPageFragment.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/overview/internal/MediaAttachmentAdapter.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/AttachmentsPickerDialogFragment.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/MessageListView.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/view/internal/DefaultQuotedAttachmentView.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/view/internal/FileAttachmentsView.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/view/internal/GiphyMediaAttachmentView.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/view/internal/LinkAttachmentView.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/view/internal/MediaAttachmentView.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/viewholder/impl/GiphyViewHolder.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/navigation/destinations/AttachmentDestination.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModel.kt
💤 Files with no reviewable changes (3)
- stream-chat-android-ui-common/src/main/AndroidManifest.xml
- stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt
- stream-chat-android-ui-components/detekt-baseline.xml
| <p align="center"> | ||
| <a href="https://getstream.io/tutorials/android-chat/"> | ||
| <img src="/docs/sdk-hero-android.png"/> | ||
| <img src="/docs/stream-chat-android-github-cover.png"/> |
There was a problem hiding this comment.
Add alt text to the hero image for accessibility.
The image lacks an alt attribute, which impacts accessibility for screen reader users. Static analysis (markdownlint MD045) flagged this.
Proposed fix
- <img src="/docs/stream-chat-android-github-cover.png"/>
+ <img src="/docs/stream-chat-android-github-cover.png" alt="Stream Chat Android SDK"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <img src="/docs/stream-chat-android-github-cover.png"/> | |
| <img src="/docs/stream-chat-android-github-cover.png" alt="Stream Chat Android SDK"/> |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 5-5: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 5, The hero image tag <img
src="/docs/stream-chat-android-github-cover.png"/> is missing an alt attribute;
update that element in README.md to include a concise descriptive alt text (for
example alt="Stream Chat Android GitHub cover") so screen readers can interpret
the image and satisfy markdownlint MD045.
Comment on lines
+71
to
+78
| val cdnRequest = try { | ||
| runBlocking { | ||
| cdn.fileRequest(url) | ||
| } | ||
| } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { | ||
| logger.e(e) { "[open] CDN.fileRequest() failed for url: $url. Falling back to original request." } | ||
| CDNRequest(url) | ||
| } |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# Locate and read the specific file
fd -t f "CDNDataSourceFactory.kt" --exec head -n 100 {} \;Repository: GetStream/stream-chat-android
Repository: GetStream/stream-chat-android
Exit code: 0
stdout:
/*
* Copyright (c) 2014-2026 Stream.io Inc. All rights reserved.
*
* Licensed under the Stream License;
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://github.com/GetStream/stream-chat-android/blob/main/LICENSE
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.getstream.chat.android.client.cdn.internal
import android.net.Uri
import androidx.core.net.toUri
import androidx.media3.common.util.UnstableApi
import androidx.media3.datasource.DataSource
import androidx.media3.datasource.DataSpec
import androidx.media3.datasource.DefaultHttpDataSource
import androidx.media3.datasource.TransferListener
import io.getstream.chat.android.client.cdn.CDN
import io.getstream.chat.android.client.cdn.CDNRequest
import io.getstream.log.taggedLogger
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking
/**
* A [DataSource.Factory] that creates [CDNDataSource] instances which transform
* media requests through the [CDN.fileRequest] method before delegating to an upstream data source.
*
* `@param` cdn The CDN used to transform file request URLs and headers.
* `@param` upstreamFactory The factory for creating the upstream data source that performs the actual HTTP requests.
*/
`@UnstableApi`
internal class CDNDataSourceFactory(
private val cdn: CDN,
private val upstreamFactory: DataSource.Factory = DefaultHttpDataSource.Factory(),
) : DataSource.Factory {
override fun createDataSource(): DataSource {
return CDNDataSource(cdn, upstreamFactory.createDataSource())
}
}
/**
* A [DataSource] that transforms media requests through [CDN.fileRequest] before
* delegating to an upstream data source. This allows custom CDN implementations
* to rewrite URLs and inject headers for video/audio/voice recording playback via ExoPlayer.
*
* [CDN.fileRequest] is a suspend function and is called via [runBlocking] on [Dispatchers.IO].
* This is safe because ExoPlayer always calls [open] from its loader thread, never the main thread.
*/
`@UnstableApi`
private class CDNDataSource(
private val cdn: CDN,
private val upstream: DataSource,
) : DataSource {
private val logger by taggedLogger("Chat:CDNDataSource")
override fun open(dataSpec: DataSpec): Long {
val scheme = dataSpec.uri.scheme
if (scheme != "http" && scheme != "https") {
return upstream.open(dataSpec)
}
val url = dataSpec.uri.toString()
val cdnRequest = try {
runBlocking {
cdn.fileRequest(url)
}
} catch (`@Suppress`("TooGenericExceptionCaught") e: Exception) {
logger.e(e) { "[open] CDN.fileRequest() failed for url: $url. Falling back to original request." }
CDNRequest(url)
}
val mergedHeaders = buildMap {
putAll(dataSpec.httpRequestHeaders)
cdnRequest.headers?.let { putAll(it) }
}
val transformedSpec = dataSpec.buildUpon()
.setUri(cdnRequest.url.toUri())
.setHttpRequestHeaders(mergedHeaders)
.build()
return upstream.open(transformedSpec)
}
override fun read(buffer: ByteArray, offset: Int, length: Int): Int =
upstream.read(buffer, offset, length)
override fun close() {
upstream.close()
}
override fun getUri(): Uri? = upstream.uri
override fun getResponseHeaders(): Map<String, List<String>> = upstream.responseHeaders
Script executed:
# Get the full context around lines 71-78
fd -t f "CDNDataSourceFactory.kt" --exec wc -l {} \;Repository: GetStream/stream-chat-android
Repository: GetStream/stream-chat-android
Exit code: 0
stdout:
104 ./stream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/internal/CDNDataSourceFactory.kt
Script executed:
# Search for similar exception handling patterns in the codebase
rg "runBlocking" -A 5 -B 2 --type ktRepository: GetStream/stream-chat-android
Repository: GetStream/stream-chat-android
Exit code: 2
stderr:
unrecognized file type: kt
Script executed:
# Check for any existing cancellation/interruption handling
rg "InterruptedException|CancellationException" --type ktRepository: GetStream/stream-chat-android
Repository: GetStream/stream-chat-android
Exit code: 2
stderr:
unrecognized file type: kt
Script executed:
# Look at the DataSpec and how open() is used in Media3 context
rg "class CDNDataSource|fun open" -A 10 --type kt | head -80Repository: GetStream/stream-chat-android
Repository: GetStream/stream-chat-android
Exit code: 0
stderr:
unrecognized file type: kt
Propagate thread interruption and coroutine cancellation instead of catching them.
When runBlocking is interrupted (by ExoPlayer's cancellation mechanism), the generic catch (Exception) catches the InterruptedException or CancellationException and falls back to the original URL, allowing a cancelled load to continue. These cancellation signals must be re-thrown.
Additionally, the @Suppress("TooGenericExceptionCaught") annotation should be documented per coding guidelines.
♻️ Proposed fix
+import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking val cdnRequest = try {
runBlocking {
cdn.fileRequest(url)
}
+ } catch (e: InterruptedException) {
+ Thread.currentThread().interrupt()
+ throw e
+ } catch (e: CancellationException) {
+ throw e
} catch (`@Suppress`("TooGenericExceptionCaught") e: Exception) {
logger.e(e) { "[open] CDN.fileRequest() failed for url: $url. Falling back to original request." }
CDNRequest(url)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/cdn/internal/CDNDataSourceFactory.kt`
around lines 71 - 78, The catch block in CDNDataSourceFactory around runBlocking
{ cdn.fileRequest(url) } must not swallow thread interruptions or coroutine
cancellations; change the exception handling so that InterruptedException and
CancellationException (or any Throwable that represents cancellation) are
re-thrown immediately and only non-cancellation exceptions are logged and cause
fallback to CDNRequest(url). Update the catch-site for the generic exception
suppression (`@Suppress`("TooGenericExceptionCaught")) to include a brief comment
explaining why it remains (per coding guidelines) or narrow the caught type to
Exception after rethrowing cancellations, and keep the logger.e(...) and
fallback to CDNRequest only for non-cancellation failures.
Comment on lines
+411
to
+420
| // Snapshot plugins BEFORE checking initializationState to avoid a race with disconnect(). | ||
| // disconnect() sets initializationState to NOT_INITIALIZED before clearing plugins, | ||
| // so if we snapshot plugins first and then see COMPLETE, the snapshot is guaranteed valid. | ||
| val currentPlugins = plugins | ||
| val initState = awaitInitializationState(RESOLVE_DEPENDENCY_TIMEOUT) | ||
| if (initState != InitializationState.COMPLETE) { | ||
| StreamLog.e(TAG) { "[resolvePluginDependency] failed (initializationState is not COMPLETE): $initState " } | ||
| throw IllegalStateException("ChatClient::connectUser() must be called before resolving any dependency") | ||
| } | ||
| val resolver = plugins.find { plugin -> | ||
| val resolver = currentPlugins.find { plugin -> |
There was a problem hiding this comment.
The early plugins snapshot breaks resolvePluginDependency() during initialization.
If Line 414 runs while the client is still INITIALIZING, it can capture the pre-init or empty plugins list. awaitInitializationState() may then return COMPLETE, but Lines 420-424 still search that stale snapshot and throw "Plugin ... was not found" even though initializeClientWithUser() has already installed the new plugins. Please re-read plugins after the wait, or guard the state/plugin pair with the same synchronization so both connect and disconnect races are covered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt`
around lines 411 - 420, The early snapshot of plugins in resolvePluginDependency
breaks when initialization completes after the snapshot; instead of using the
pre-wait currentPlugins, re-read the plugins list after
awaitInitializationState(RESOLVE_DEPENDENCY_TIMEOUT) (or hold the same
synchronization used by connect/disconnect) so that the resolver lookup uses the
up-to-date plugins; specifically update the code around
currentPlugins/awaitInitializationState/InitializationState.COMPLETE and the
subsequent resolver lookup to fetch plugins again (or perform the lookup under
the same lock) to avoid stale-plugin "was not found" errors.
Comment on lines
+720
to
+721
| public final fun defaultAttachmentHandlers (Landroid/content/Context;Z)Ljava/util/List; | ||
| public static synthetic fun defaultAttachmentHandlers$default (Lio/getstream/chat/android/compose/ui/attachments/preview/handler/AttachmentPreviewHandler$Companion;Landroid/content/Context;ZILjava/lang/Object;)Ljava/util/List; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 \
--iglob 'MIGRATION_TO_V7.md' \
--iglob 'CHANGELOG*' \
'useDocumentGView|defaultAttachmentHandlers|DocumentAttachmentPreviewHandler' || trueRepository: GetStream/stream-chat-android
Length of output: 55
🏁 Script executed:
# Find migration and changelog files
fd -t f -i 'migration.*v7|changelog' -x ls -lh {}Repository: GetStream/stream-chat-android
Length of output: 205
🏁 Script executed:
# Check if migration/changelog files exist in common locations
find . -maxdepth 2 -type f \( -iname 'MIGRATION*' -o -iname 'CHANGELOG*' -o -iname 'CHANGES*' \) | head -20Repository: GetStream/stream-chat-android
Length of output: 108
🏁 Script executed:
# Check the beginning and structure of CHANGELOG.md
head -100 CHANGELOG.mdRepository: GetStream/stream-chat-android
Length of output: 1786
🏁 Script executed:
# Search for document/attachment/preview mentions in CHANGELOG.md
rg -n -C3 'useDocumentGView|defaultAttachmentHandlers|DocumentAttachmentPreviewHandler|document.*preview|attachment.*handler' CHANGELOG.md | head -100Repository: GetStream/stream-chat-android
Length of output: 924
Add document preview API changes to the v7 CHANGELOG.
The public JVM signatures for AttachmentPreviewHandler.defaultAttachmentHandlers(), DocumentAttachmentPreviewHandler constructor, and ChatTheme have been updated with the new useDocumentGView parameter. These are breaking changes for v7, but they are not documented in CHANGELOG.md under stream-chat-android-compose.
Please update the CHANGELOG with the new signature and explain the upgrade path for Java callers and existing binaries—specifically that the new useDocumentGView flag defaults to true and can be set to false to opt into CDN-aware document handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stream-chat-android-compose/api/stream-chat-android-compose.api` around lines
720 - 721, Add an entry to the v7 CHANGELOG under stream-chat-android-compose
that documents the breaking JVM signature changes: list the new signatures for
AttachmentPreviewHandler.defaultAttachmentHandlers(Context, boolean), the
DocumentAttachmentPreviewHandler constructor (now taking useDocumentGView
boolean), and ChatTheme (with useDocumentGView), explain the upgrade path for
Java callers and existing binaries — the new useDocumentGView flag defaults to
true, existing behavior is preserved, and Java callers can pass false to opt
into CDN-aware document handling — and include a short code snippet example
showing how Java callers should call the updated
AttachmentPreviewHandler.defaultAttachmentHandlers(...) and construct
DocumentAttachmentPreviewHandler with useDocumentGView=false to opt out.
Comment on lines
+7053
to
+7054
| public final fun completeRecording (Lkotlin/jvm/functions/Function1;)V | ||
| public static synthetic fun completeRecording$default (Lio/getstream/chat/android/compose/viewmodel/messages/MessageComposerViewModel;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)V |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 \
--iglob 'MIGRATION_TO_V7.md' \
--iglob 'CHANGELOG*' \
'completeRecording|MessageComposerViewModel' || trueRepository: GetStream/stream-chat-android
Length of output: 4368
Add a migration note for completeRecording's new callback parameter.
The no-arg JVM entry point for MessageComposerViewModel.completeRecording was removed in favor of a callback-based signature. Kotlin callers can omit the callback via defaults after recompiling, but Java callers now require a different call shape. This breaking change is not documented in CHANGELOG.md or migration guides and should be called out explicitly in v7 upgrade notes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stream-chat-android-compose/api/stream-chat-android-compose.api` around lines
7053 - 7054, Add a migration note documenting the breaking change: the JVM
no-arg entry point for MessageComposerViewModel.completeRecording was removed
and now requires a callback parameter (see completeRecording and
completeRecording$default signatures); update CHANGELOG.md and the v7 migration
guide to explain that Kotlin callers will keep no-arg calls after recompilation
(due to default parameters) but Java callers must now supply a Function1
callback or use the synthetic default helper, include an example of the new Java
call shape and a short snippet explaining how to migrate existing Java usages.
Comment on lines
58
to
65
| * Default implementation of [ImageAssetTransformer] that doesn't provide any headers. | ||
| * | ||
| * @deprecated Use [io.getstream.chat.android.client.cdn.CDN] instead. Configure a custom CDN via | ||
| * [io.getstream.chat.android.client.ChatClient.Builder.cdn] to transform URLs for all image, file, | ||
| * and download requests. | ||
| */ | ||
| @Deprecated("Use CDN instead. Configure via ChatClient.Builder.cdn().") | ||
| public object DefaultImageAssetTransformer : ImageAssetTransformer { |
There was a problem hiding this comment.
Update stale KDoc wording in default transformer.
“doesn't provide any headers” is outdated here; this type now represents pass-through asset transformation. Consider wording like “returns the asset unchanged.”
Proposed KDoc tweak
- * Default implementation of [ImageAssetTransformer] that doesn't provide any headers.
+ * Default implementation of [ImageAssetTransformer] that returns the input asset unchanged.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * Default implementation of [ImageAssetTransformer] that doesn't provide any headers. | |
| * | |
| * @deprecated Use [io.getstream.chat.android.client.cdn.CDN] instead. Configure a custom CDN via | |
| * [io.getstream.chat.android.client.ChatClient.Builder.cdn] to transform URLs for all image, file, | |
| * and download requests. | |
| */ | |
| @Deprecated("Use CDN instead. Configure via ChatClient.Builder.cdn().") | |
| public object DefaultImageAssetTransformer : ImageAssetTransformer { | |
| * Default implementation of [ImageAssetTransformer] that returns the input asset unchanged. | |
| * | |
| * `@deprecated` Use [io.getstream.chat.android.client.cdn.CDN] instead. Configure a custom CDN via | |
| * [io.getstream.chat.android.client.ChatClient.Builder.cdn] to transform URLs for all image, file, | |
| * and download requests. | |
| */ | |
| `@Deprecated`("Use CDN instead. Configure via ChatClient.Builder.cdn().") | |
| public object DefaultImageAssetTransformer : ImageAssetTransformer { |
🤖 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/helper/ImageAssetTransformer.kt`
around lines 58 - 65, Update the KDoc for DefaultImageAssetTransformer to
reflect that it is a pass-through transformer (it returns the asset unchanged)
instead of saying it "doesn't provide any headers"; locate the KDoc block above
the object declaration for DefaultImageAssetTransformer (implementing
ImageAssetTransformer) and replace the outdated phrase with wording like
"returns the asset unchanged" or "pass-through asset transformer" while
preserving the existing deprecation note.
Comment on lines
+52
to
+59
| val cdnRequest = try { | ||
| withContext(Dispatchers.IO) { | ||
| cdn.imageRequest(url) | ||
| } | ||
| } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { | ||
| logger.e(e) { "[intercept] CDN.imageRequest() failed for url: $url. Falling back to original request." } | ||
| return chain.proceed() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CDNImageInterceptor.kt" -type fRepository: GetStream/stream-chat-android
Length of output: 195
🏁 Script executed:
fd -t f "CDNImageInterceptor.kt"Repository: GetStream/stream-chat-android
Length of output: 193
🏁 Script executed:
# Search for CDNImageInterceptor in the repository
rg "class CDNImageInterceptor" -A 100 -B 2Repository: GetStream/stream-chat-android
Length of output: 23229
🏁 Script executed:
head -30 stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/internal/CDNImageInterceptor.ktRepository: GetStream/stream-chat-android
Length of output: 1183
🏁 Script executed:
rg "CancellationException" stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/internal/Repository: GetStream/stream-chat-android
Length of output: 55
Propagate coroutine cancellation in the interceptor.
catch (Exception) at line 55 also catches CancellationException, which suppresses cancellation and converts cancelled image loads into fallback requests. This causes unnecessary work on disposed/off-screen requests. Rethrow cancellation explicitly.
Suggested fix
+import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
@@
- } catch (`@Suppress`("TooGenericExceptionCaught") e: Exception) {
+ } catch (e: CancellationException) {
+ throw e
+ } catch (`@Suppress`("TooGenericExceptionCaught") e: Exception) {
logger.e(e) { "[intercept] CDN.imageRequest() failed for url: $url. Falling back to original request." }
return chain.proceed()
}🤖 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/internal/CDNImageInterceptor.kt`
around lines 52 - 59, The catch block around cdn.imageRequest(...) in
CDNImageInterceptor currently catches all Exceptions and swallows
CancellationException, preventing coroutine cancellation from propagating;
modify the handler so it rethrows CancellationException (e.g., if (e is
CancellationException) throw e) before logging and falling back to
chain.proceed(), ensuring cancellation is propagated for cdn.imageRequest, and
only non-cancellation exceptions are logged and trigger the fallback behavior.
| try { | ||
| val byteArrayOutputStream = ByteArrayOutputStream() | ||
| bitmap.compress(Bitmap.CompressFormat.PNG, BITMAP_QUALITY, byteArrayOutputStream) | ||
| bitmap.compress(Bitmap.CompressFormat.PNG, config.bitmapQuality, byteArrayOutputStream) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
According to the official Android documentation for android.graphics.Bitmap.compress, is the qualityparameter ignored when the format isBitmap.CompressFormat.PNG?
💡 Result:
Yes, according to the official Android documentation, the quality parameter is ignored when the format is Bitmap.CompressFormat.PNG because PNG compression is lossless.
Citations:
🏁 Script executed:
cd stream-chat-android-ui-common && find . -name "StreamShareFileManager.kt" -type fRepository: GetStream/stream-chat-android
Length of output: 166
🏁 Script executed:
fd -t f "StreamShareFileManager.kt"Repository: GetStream/stream-chat-android
Length of output: 194
🏁 Script executed:
cat -n stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt | sed -n '40,80p'Repository: GetStream/stream-chat-android
Length of output: 2211
🏁 Script executed:
cat -n stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt | sed -n '150,180p'Repository: GetStream/stream-chat-android
Length of output: 1441
The bitmapQuality parameter has no effect on PNG compression and should be removed or the format should be made configurable.
Line 63 always writes PNG bytes. The bitmapQuality config parameter (line 171) is exposed as a configurable option but PNG compression ignores the quality hint on Android since PNG uses lossless compression. While the KDoc explicitly documents this as PNG-specific, exposing a parameter that has no practical effect is confusing. Either remove the quality parameter or make the bitmap format configurable to honor the quality setting for formats that support it.
🤖 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/internal/file/StreamShareFileManager.kt`
at line 63, The code currently always compresses to PNG in
StreamShareFileManager.bitmap.compress(...), so the exposed config.bitmapQuality
has no effect; update the config and compress call so the format is configurable
and the quality is honored for lossy formats: add a bitmapFormat (e.g.,
Bitmap.CompressFormat) option to the existing configuration (or replace
bitmapQuality with a format+quality pair), use that bitmapFormat in the
bitmap.compress call instead of hardcoding PNG, and preserve the existing
bitmapQuality for formats that support it (or ignore it when bitmapFormat == PNG
with a short comment); adjust any KDoc to reflect the new configurable format
and behavior.
Comment on lines
+361
to
+414
| @Test | ||
| fun `writeAttachmentToShareableFile treats expired cached file as cache miss`() = runTest { | ||
| // given | ||
| val attachment = randomAttachment( | ||
| assetUrl = "https://example.com/file.pdf", | ||
| fileSize = 1024, | ||
| name = "document.pdf", | ||
| ) | ||
| val cachedFile = mock<File>() | ||
| whenever(fileManager.getFileFromCache(any(), any())) | ||
| .thenReturn(Result.Success(cachedFile)) | ||
| whenever(cachedFile.exists()).thenReturn(true) | ||
| whenever(cachedFile.length()).thenReturn(1024L) | ||
| whenever(cachedFile.lastModified()).thenReturn(System.currentTimeMillis() - 6 * 60 * 1000L) | ||
|
|
||
| val chatClient = mock<ChatClient>() | ||
| val downloadedFile = File("path/to/downloaded/file.pdf") | ||
| val responseBody = TestResponseBody("test content") | ||
| whenever(chatClient.downloadFile(any())) doReturn TestCall(Result.Success(responseBody)) | ||
| whenever(fileManager.writeFileInCache(any(), any(), any())) | ||
| .thenReturn(Result.Success(downloadedFile)) | ||
|
|
||
| // when | ||
| val result = shareFileManager.writeAttachmentToShareableFile( | ||
| context = context, | ||
| attachment = attachment, | ||
| chatClient = { chatClient }, | ||
| ) | ||
|
|
||
| // then | ||
| Assert.assertTrue(result.isSuccess) | ||
| } | ||
|
|
||
| @Test | ||
| fun `getShareableUriForAttachment returns Error when cached file is expired`() = runTest { | ||
| // given | ||
| val attachment = randomAttachment( | ||
| assetUrl = "https://example.com/file.pdf", | ||
| fileSize = 1024, | ||
| name = "document.pdf", | ||
| ) | ||
| val cachedFile = mock<File>() | ||
| whenever(fileManager.getFileFromCache(any(), any())) | ||
| .thenReturn(Result.Success(cachedFile)) | ||
| whenever(cachedFile.exists()).thenReturn(true) | ||
| whenever(cachedFile.length()).thenReturn(1024L) | ||
| whenever(cachedFile.lastModified()).thenReturn(System.currentTimeMillis() - 6 * 60 * 1000L) | ||
|
|
||
| // when | ||
| val result = shareFileManager.getShareableUriForAttachment(context, attachment) | ||
|
|
||
| // then | ||
| Assert.assertTrue(result.isFailure) | ||
| } |
There was a problem hiding this comment.
Make the expiry tests prove the expiration branch.
The hard-coded 6 * 60 * 1000L assumes today's default TTL, and the first test only checks isSuccess, so it still passes if the stale file is incorrectly returned as a cache hit. Derive the age from ShareCacheConfig().cacheTtlMs and verify a miss-specific interaction like downloadFile() or writeFileInCache().
♻️ Proposed fix
- whenever(cachedFile.lastModified()).thenReturn(System.currentTimeMillis() - 6 * 60 * 1000L)
+ whenever(cachedFile.lastModified())
+ .thenReturn(System.currentTimeMillis() - ShareCacheConfig().cacheTtlMs - 1)
@@
// then
Assert.assertTrue(result.isSuccess)
+ org.mockito.kotlin.verify(chatClient).downloadFile(any())
+ org.mockito.kotlin.verify(fileManager).writeFileInCache(any(), any(), any())
@@
- whenever(cachedFile.lastModified()).thenReturn(System.currentTimeMillis() - 6 * 60 * 1000L)
+ whenever(cachedFile.lastModified())
+ .thenReturn(System.currentTimeMillis() - ShareCacheConfig().cacheTtlMs - 1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManagerTest.kt`
around lines 361 - 414, The tests use a hard-coded 6*60*1000L TTL which may not
exercise the expiration branch; update both tests to compute the stale timestamp
using ShareCacheConfig().cacheTtlMs (e.g. lastModified =
System.currentTimeMillis() - (ShareCacheConfig().cacheTtlMs + 1)) so the cached
file is definitely expired when shareFileManager calls
fileManager.getFileFromCache; also strengthen assertions by verifying the
miss-specific interactions: for writeAttachmentToShareableFile assert that
chatClient.downloadFile() and fileManager.writeFileInCache(...) are invoked (or
that writeFileInCache is called with the downloaded file) and for
getShareableUriForAttachment assert that no valid URI is returned (isFailure)
and optionally verify download/write are not called or are called according to
expected behavior.
Comment on lines
+76
to
+77
| val url = it.giphyInfo(GiphyInfoType.FIXED_HEIGHT)?.url | ||
| ?: it.giphyFallbackPreviewUrl ?: return |
There was a problem hiding this comment.
Avoid non-local return here; it can leave recycled row state stale.
?: return exits bindData, so giphyQueryTextView.text (Line 90+) is skipped when URL is missing. With the tighter fallback chain, this can now happen more often and show previous-item data.
Suggested fix
- val url = it.giphyInfo(GiphyInfoType.FIXED_HEIGHT)?.url
- ?: it.giphyFallbackPreviewUrl ?: return
-
- binding.giphyPreview.load(
- data = url,
- onStart = {
- binding.loadingProgressBar.isVisible = true
- },
- onComplete = {
- binding.loadingProgressBar.isVisible = false
- },
- )
+ val url = it.giphyInfo(GiphyInfoType.FIXED_HEIGHT)?.url
+ ?: it.giphyFallbackPreviewUrl
+
+ if (url != null) {
+ binding.giphyPreview.load(
+ data = url,
+ onStart = {
+ binding.loadingProgressBar.isVisible = true
+ },
+ onComplete = {
+ binding.loadingProgressBar.isVisible = false
+ },
+ )
+ } else {
+ binding.loadingProgressBar.isVisible = false
+ binding.giphyPreview.setImageDrawable(null)
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/viewholder/impl/GiphyViewHolder.kt`
around lines 76 - 77, The bindData in GiphyViewHolder currently uses a non-local
return via "?: return" when resolving the GIF URL, which skips the rest of bind
logic (including setting giphyQueryTextView) and can leave recycled row state
stale; change the logic to avoid returning early by assigning the URL to a
nullable local (e.g., val url = it.giphyInfo(... ) ?:
it.giphyFallbackPreviewUrl) and then handle the null case inline (clear or hide
the image view, set a placeholder, but still proceed to set
giphyQueryTextView.text and other UI state). Ensure any image loading calls (the
code that used the URL) are guarded with an if (url != null) block and that
giphyQueryTextView and other fields on GiphyViewHolder are always updated
regardless of URL presence.
|
gpunto
approved these changes
Mar 30, 2026
Contributor
gpunto
left a comment
gpunto
left a comment
There was a problem hiding this comment.
Looks good on my side, but I'm not familiar with some of the changes that went into develop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Merge develop to V7
Implementation
Merge develop to V7
Testing
There should be no regressions, and the latest additions from develop should be present on V7
Summary by CodeRabbit
New Features
Improvements
Deprecations
AsyncImageHeadersProvider,DownloadAttachmentUriGenerator, and related legacy interfaces deprecated; use CDN configuration instead.