Implement operation to clear local data created by the SDK#6090
Conversation
SDK Size Comparison 📏
|
VelikovPetar
changed the title
WalkthroughAdds StreamFileManager and StreamShareFileManager replacing StreamFileUtil, exposes ChatClient.clearCacheAndTemporaryFiles(context: Context) to remove SDK caches and external media, updates callers/DI to use the new managers, and adds comprehensive unit/instrumented tests while deleting legacy StreamFileUtil and its tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as ChatClient
participant FileMgr as StreamFileManager
participant FS as AndroidFilesystem
rect rgba(200,200,255,0.5)
Client->>FileMgr: clearAllCache(context)
FileMgr->>FS: delete stream cache dir
FS-->>FileMgr: success/failure
FileMgr->>FS: delete image cache dir
FS-->>FileMgr: success/failure
FileMgr->>FS: delete timestamped cache dirs
FS-->>FileMgr: success/failure
FileMgr->>FS: delete external photos/videos
FS-->>FileMgr: success/failure
FileMgr-->>Client: Result (first failure or success)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
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
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/internal/StorageHelper.kt (1)
45-82: Don’t silently swallow cache-write failures.
getCachedFileFromUrinow returnsnullon write failure, which breaks the documented contract (only null when both file/URI are null) and hides IO errors. Also ensure theInputStreamis always closed even if the write fails.✅ Proposed fix (preserve contract + close stream)
import android.webkit.MimeTypeMap import io.getstream.chat.android.client.internal.file.StreamFileManager import io.getstream.chat.android.models.AttachmentType import io.getstream.chat.android.ui.common.state.messages.composer.AttachmentMetaData +import io.getstream.result.Result import java.io.File +import java.io.IOException @@ - val fileName = attachmentMetaData.getTitleWithExtension() - val inputStream = context.contentResolver.openInputStream(attachmentMetaData.uri!!) - ?: return null - return fileManager.writeFileInTimestampedCache( - context = context, - fileName = fileName, - source = inputStream, - ).getOrNull() + val fileName = attachmentMetaData.getTitleWithExtension() + val inputStream = context.contentResolver.openInputStream(attachmentMetaData.uri!!) + ?: return null + return inputStream.use { stream -> + when (val result = fileManager.writeFileInTimestampedCache( + context = context, + fileName = fileName, + source = stream, + )) { + is Result.Success -> result.value + is Result.Failure -> throw IOException( + result.value.message ?: "Failed to cache attachment", + result.value.cause, + ) + } + }stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/gallery/AttachmentGalleryActivity.kt (1)
204-216: Handle shareImage failures instead of silently finishing
writeBitmapToShareableFilecan fail (and bitmap load can be null), but the UI just resets without user feedback. Align with the video flow and surface a toast on failure.🛠️ Suggested fix
- )?.let { bitmap -> - StreamShareFileManager().writeBitmapToShareableFile(applicationContext, bitmap) - }?.onSuccess { uri -> - shareLocalFile( - uri = uri, - mimeType = attachment.mimeType, - ) - } + )?.let { bitmap -> + StreamShareFileManager().writeBitmapToShareableFile(applicationContext, bitmap) + }?.let { result -> + when (result) { + is Result.Success -> { + shareLocalFile( + uri = result.value, + mimeType = attachment.mimeType, + ) + } + is Result.Failure -> toastFailedShare() + } + } ?: toastFailedShare()
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 19: Update the changelog entry to show the full method signature
including parentheses and the Context parameter so it's unambiguous; change the
line referencing ChatClient.clearCacheAndTemporaryFiles to read
ChatClient.clearCacheAndTemporaryFiles(context: Context) to match other API
entries and clarify usage.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt`:
- Around line 292-299: The code currently ignores the boolean result of
File.mkdirs() in getOrCreateCacheDir and createTimestampedCacheDir; update both
functions (getOrCreateCacheDir, createTimestampedCacheDir) to check the mkdirs()
return value and treat a false as an error path: if mkdirs() returns true then
return Result.Success(theDir), otherwise return
Result.Failure(Error.ThrowableError("Could not create cache directory:
<dirPath>")) (or include the caught exception if mkdirs() failed due to an
exception), ensuring you still catch exceptions and wrap them in ThrowableError
as before; reference getStreamCacheDir when creating the base dir and the
created timestamped dir when composing the error message.
- Around line 111-113: TIMESTAMPED_DIR_TIMESTAMP_FORMAT currently uses only time
(HHmmssSSS) and EXTERNAL_DIR_TIMESTAMP_FORMAT lacks milliseconds
(yyyyMMdd_HHmmss), causing daily and same-second collisions; update both formats
to include full date and milliseconds (use yyyyMMdd_HHmmssSSS) in their
definitions (symbols TIMESTAMPED_DIR_TIMESTAMP_FORMAT and
EXTERNAL_DIR_TIMESTAMP_FORMAT) and update the KDoc example that describes the
timestamped folder/file format in StreamFileManager (the doc block above the
method creating the timestamped folder) so the example matches the new
yyyyMMdd_HHmmssSSS pattern.
- Around line 173-179: The clear* methods currently ignore the Boolean results
of File.delete() / deleteRecursively(), so clearCache, clearExternalStorage,
clearImageCache, and clearTimestampedCacheFolders may return Success even when
deletions failed; update each method (clearCache, clearExternalStorage,
clearImageCache, clearTimestampedCacheFolders) to check deletion return values:
for deleteRecursively() capture its Boolean and return Result.Failure if false;
for clearExternalStorage check each delete() call and if any returns false
return Failure (or aggregate a failure flag), and for
clearTimestampedCacheFolders track per-folder deletion success and return
Failure when any folder fails to delete; preserve existing exception handling
but prefer returning Result.Failure with an appropriate exception when deletions
fail.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientCacheAndTemporaryFilesTest.kt`:
- Around line 44-48: The test class ChatClientCacheAndTemporaryFilesTest is
using a JUnit5 TestCoroutineExtension via the `@RegisterExtension` field
testCoroutines while still running under JUnit4
(`@RunWith`(AndroidJUnit4::class)), so the extension's
beforeAll/Dispatchers.setMain() never runs; fix by either migrating the test to
JUnit5 and replacing `@RunWith` with `@ExtendWith`(TestCoroutineExtension::class)
(removing `@RegisterExtension` and letting the extension manage Dispatchers) or
keep JUnit4 and replace TestCoroutineExtension usage with a JUnit4-compatible
setup (e.g., implement a TestRule or use a JUnit4-compatible TestCoroutineRule
that sets/cleans Dispatchers in apply()), updating the testCoroutines reference
and any setup/teardown calls accordingly.
In `@stream-chat-android-ui-common/api/stream-chat-android-ui-common.api`:
- Around line 3099-3103: The public constructor for DefaultStreamMediaRecorder
uses Kotlin default parameters but lacks the `@JvmOverloads` annotation, breaking
Java binary compatibility; modify the primary constructor of class
DefaultStreamMediaRecorder (the <init>(Context, Int, Int, Int, Int, Int,
StreamFileManager) synthetic/full constructor) to be annotated with
`@JvmOverloads` so the compiler emits Java-friendly overloaded constructors for
each default parameter combination (or, if you prefer not to change code,
document the breaking change clearly for Java consumers).
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt`:
- Around line 122-130: In getCachedFileForAttachment, the validation rejects
cached files when attachment.fileSize is 0/unset; update the check in the
flatMap (after fileManager.getFileFromCache) so that you accept the cached file
if it exists and either attachment.fileSize is non-positive (<= 0) or
file.length() equals attachment.fileSize.toLong(); i.e., treat
unknown/non-positive expected size as "accept any size" and only enforce size
equality when attachment.fileSize > 0.
🧹 Nitpick comments (7)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/state/messages/composer/AttachmentMetaData.kt (1)
52-52: Consider reusingShareableUriProviderinstance instead of creating a new one each time.
ShareableUriProvideris stateless and only delegates toFileProvider.getUriForFile(). Creating a new instance on every constructor call is unnecessary—the codebase already demonstrates a better pattern inStreamShareFileManager, which stores a single reusable instance. SinceAttachmentMetaDatais a data class whose constructor may be called frequently, consider storing a shared instance at the module level.Suggested approach
private val shareableUriProvider = ShareableUriProvider() public data class AttachmentMetaData( // ... ) { public constructor( context: Context, file: File, ) : this(file = file, uri = shareableUriProvider.getUriForFile(context, file)) { // ... } }stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt (1)
1485-1517: Add a threading note to the new public API KDoc.
The docs are detailed, but they don’t state where the work runs or whether it’s safe on the main thread.As per coding guidelines, please include thread expectations in public API KDoc.💡 Suggested KDoc tweak
/** * Clears all cache and temporary files created by the Stream Chat SDK. * * This method removes: * - All cached files from the default cache directory * - All cached images from the image cache directory * - All cached files during the upload/download process * - All temporary files stored in external storage by the SDK (Photos and videos captured using the SDK) * + * **Threading**: Performs disk I/O on the clientScope dispatcher; safe to call from the main thread. + * * **Note**: This method does NOT clear database persistence. Use [clearPersistence] to clear * database data, or call both methods if you need to clear all SDK data. *stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/file/StreamFileManagerTest.kt (1)
121-135: AvoidThread.sleepin timestamp uniqueness testsSleeping to force distinct timestamps is flaky and slows the suite. Consider injecting a clock (or a test hook) into
StreamFileManagerand using deterministic time instead of real delays. Based on learnings, prefer deterministic tests withrunTest/virtual time.Also applies to: 281-291
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt (1)
51-66: Document or narrow the generic exception suppressionThe suppression is currently undocumented. Either narrow the caught exceptions or add a short rationale to justify the lint override.
As per coding guidelines, suppressions should be documented.📝 Suggested update
- `@Suppress`("TooGenericExceptionCaught") + `@Suppress`("TooGenericExceptionCaught") // Bitmap compression/cache IO can throw runtime exceptions; wrap as Result.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt (3)
33-43: Add thread/side‑effect notes to public KDocs.Public methods perform blocking I/O and destructive cleanup; please document thread expectations (e.g., off main thread) and side effects in their KDocs. As per coding guidelines, please include explicit thread and state notes.
87-88: Avoid broad Exception suppression without rationale.
@Suppress("TooGenericExceptionCaught")appears repeatedly; please catch narrower exceptions or document why the broad suppression is required. As per coding guidelines, suppressions should be justified.
120-139: Consider moving timestamped cache writes off the caller thread.This method performs blocking I/O but isn’t suspend/IO-dispatched like
writeFileInCache, which risks main-thread stalls. If API changes are acceptable, align it with the suspend variant; otherwise, clearly document the required background-thread usage.♻️ Suggested adjustment (if API change is acceptable)
- public fun writeFileInTimestampedCache( + public suspend fun writeFileInTimestampedCache( context: Context, fileName: String, source: InputStream, - ): Result<File> { - return try { + ): Result<File> = withContext(DispatcherProvider.IO) { + try { createTimestampedCacheDir(context).flatMap { cacheDir -> val file = File(cacheDir, fileName) source.use { input -> file.outputStream().use { output -> input.copyTo(output) } } Result.Success(file) } } catch (e: Exception) { Result.Failure(Error.ThrowableError("Failed to write file to timestamped cache.", e)) } - } + }
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Show resolved
Hide resolved
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Show resolved
Hide resolved
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Show resolved
Hide resolved
...lient/src/test/java/io/getstream/chat/android/client/ChatClientCacheAndTemporaryFilesTest.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/io/getstream/chat/android/ui/common/internal/file/StreamShareFileManager.kt
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt`:
- Around line 120-138: Make writeFileInTimestampedCache a suspend function and
run its blocking I/O on the IO dispatcher: change the signature of
writeFileInTimestampedCache to suspend and wrap the body in
withContext(DispatcherProvider.IO) (same pattern as writeFileInCache), ensuring
the createTimestampedCacheDir call and the InputStream/OutputStream copy run
inside that withContext block; update the KDoc for writeFileInTimestampedCache
to document it is safe to call from the main thread because it performs IO on
DispatcherProvider.IO (or, if you prefer not to make it suspend, add KDoc
explicitly stating it must only be called from a background thread). Include
references to writeFileInTimestampedCache, createTimestampedCacheDir,
DispatcherProvider.IO, and writeFileInCache when making the change so the
pattern matches the existing implementation.
- Around line 173-185: clearCache (and similarly clearImageCache) treats
deleteRecursively() == false as failure even when the cache dir is missing;
update clearCache to check whether the directory exists first (using
getStreamCacheDir(context).exists()) and return Result.Success(Unit) if it
doesn't, otherwise attempt deleteRecursively() and return Success on true or
Failure on false, preserving the existing exception catch path (the same pattern
should be applied to clearImageCache using its cache dir helper).
🧹 Nitpick comments (2)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt (2)
33-43: Add threading/state expectations to the public API KDoc.Right now the KDoc explains behavior but not thread expectations or state notes. Consider adding a short note at the class-level so callers know which methods are blocking and which dispatch to IO. As per coding guidelines, document public APIs with thread expectations and state notes.
💡 Suggested KDoc addition
/** * Manages file operations for the Stream Chat SDK, providing utilities for caching, * retrieving, and managing files in various cache directories and external storage. * * This class handles: * - Image caching in a dedicated directory * - General file caching in the Stream cache directory * - Timestamped cache folders for isolated file storage * - Cache cleanup operations * - External storage management for photos and videos captured using the SDK + * + * Threading: Methods that touch disk are blocking unless explicitly `suspend`; + * suspend functions dispatch to `DispatcherProvider.IO`. + * State: Methods create/delete files under the app cache and external app dirs. */
87-105: Document or narrow the broad exception suppression.
@Suppress("TooGenericExceptionCaught")should be documented or replaced with narrower exception handling per the Kotlin and project guidelines. Consider either adding an inline justification comment or catchingIOExceptionspecifically (more appropriate for file/stream operations) so the suppression is intentional and reviewable.💡 Example clarification
- `@Suppress`("TooGenericExceptionCaught") + `@Suppress`("TooGenericExceptionCaught") // Wrapped to surface all I/O failures as Result.Failure.
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Show resolved
Hide resolved
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt`:
- Around line 217-223: The fallback when context.getExternalFilesDir(...)
returns null currently uses getStreamCacheDir(context) which may not exist;
update createPhotoInExternalDir (and likewise createVideoInExternalDir) to use a
method that ensures the directory exists (e.g., getOrCreateCacheDir or call
mkdirs() on the returned File) so the parent directory is created before
returning Result.Success(File(dir, filename)); reference the functions
createPhotoInExternalDir, createVideoInExternalDir and replace/getOrCreate the
fallback dir so the capture path is guaranteed to exist.
♻️ Duplicate comments (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt (1)
108-125: Document threading expectations for blocking I/O.
writeFileInTimestampedCacheperforms blocking disk I/O on the caller thread but its KDoc doesn’t mention thread expectations. Please document that it must be called off the main thread (or explicitly state the threading guarantee). As per coding guidelines, public APIs should include thread expectations.📝 Suggested KDoc tweak
/** * Writes data from an InputStream to a file in a timestamped cache folder. * * This method creates a unique timestamped folder (format: `STREAM_HHmmssSSS`) and writes * the file there. This is useful for caching files from URIs where unique folder isolation * is needed to prevent naming conflicts. + * + * Threading: performs blocking I/O; call from a background thread. * * `@param` context Android context for cache directory access * `@param` fileName Name of the file to create in the timestamped folder * `@param` source InputStream containing the data to cache * `@return` [Result.Success] with the cached File, or [Result.Failure] with an error */
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Outdated
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
gpunto
left a comment
gpunto
left a comment
There was a problem hiding this comment.
Looks good, I just left a couple of non-blocking comments for something we might want to do in v7
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Show resolved
Hide resolved
...oid-client/src/main/java/io/getstream/chat/android/client/internal/file/StreamFileManager.kt
Show resolved
Hide resolved
|
VelikovPetar
deleted the
feature/AND-1022_implement_operation_to_clear_local_data
branch
2 participants
🎯 Goal
Add a new public API method to
ChatClientthat allows developers to clear all cache and temporary files created by the Stream Chat SDK.Additionally, it re-works the current cache and file management logic to a more streamlined one, routing all operations via a new
StreamFileManager.🛠 Implementation details
ChatClient.clearCacheAndTemporaryFiles(context: Context): Call<Unit>for clearing ALL cache and files stored in the external storage.StreamFileUtiland replace it with:StreamFileManager- low-level cache and external storage managerStreamShareFileManager- high-level manager which handles the logic for sharing files from the SDKShareableFileProvider- utility class for generating sharable URIs via aFileProviderStreamFileUtilwith the new managers🎨 UI Changes
No UI changes - this is an API addition
🧪 Testing
There should be no changes in the behaviour of the SDK.
Only calling
ChatClient.clearCacheAndTemporaryFilesmanually should call clear the default cache, image cache and external storage.🎉 GIF
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.