[Feature/#167] 갤러리 이미지 저장 #168
Conversation
|
""" WalkthroughThis update introduces the foundational infrastructure for an image saving feature. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant UI (FeedImageDialog)
participant ViewModel (FeedImageViewModel)
participant UseCase (SaveImageUseCase)
participant Repo (FeedImageRepository)
participant Fetcher (BitmapFetcher)
participant Saver (GallerySaver)
UI->>ViewModel: saveImage(imgUrl)
ViewModel->>UseCase: invoke(imgUrl)
UseCase->>Repo: saveToGallery(imgUrl)
Repo->>Fetcher: fetchBitmapFromUrl(imgUrl)
Fetcher-->>Repo: Bitmap
Repo->>Saver: saveBitmapToGallery(Bitmap, filename)
Saver-->>Repo: (done)
Repo-->>UseCase: Result<Unit>
UseCase-->>ViewModel: Result<Unit>
ViewModel-->>UI: Emit sideEffect (ShowSnackBar)
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (10)
core/ui/src/main/res/layout/dialog_feed_image.xml (1)
40-50: Ensure save button is interactive and accessible
ATextViewcan work, but consider using aButtonor at least adding:
android:clickable="true"andandroid:focusable="true"for touch handling- a ripple or
selectableItemBackgroundBorderlessforeground for feedbackandroid:contentDescription="@string/save_image"for accessibilitycore/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt (1)
17-17: Consider more robust filename generation to prevent collisions.Using only
System.currentTimeMillis()for filename generation could potentially lead to collisions if multiple images are saved rapidly. Consider adding additional uniqueness factors.-gallerySaver.saveBitmapToGallery(bitmap, "wable_${System.currentTimeMillis()}$FILE_EXTENSION") +gallerySaver.saveBitmapToGallery(bitmap, "wable_${System.currentTimeMillis()}_${Random.nextInt(1000)}$FILE_EXTENSION")Or use UUID for guaranteed uniqueness:
+import java.util.UUID -gallerySaver.saveBitmapToGallery(bitmap, "wable_${System.currentTimeMillis()}$FILE_EXTENSION") +gallerySaver.saveBitmapToGallery(bitmap, "wable_${UUID.randomUUID()}$FILE_EXTENSION")core/data/src/main/java/com/teamwable/data/gallery/BitmapFetcher.kt (1)
24-27: Consider adding input validation for large images to prevent OOM.While the current implementation is correct, downloading very large images could potentially cause OutOfMemoryError. Consider adding size validation or bitmap sampling for production robustness.
response.body?.byteStream().use { inputStream -> + val options = BitmapFactory.Options().apply { + inJustDecodeBounds = true + } + BitmapFactory.decodeStream(inputStream, null, options) + + // Check if image is too large (example: > 4096x4096) + if (options.outWidth > 4096 || options.outHeight > 4096) { + options.inSampleSize = calculateInSampleSize(options, 2048, 2048) + options.inJustDecodeBounds = false + inputStream.reset() // May need to re-fetch if stream doesn't support reset + } + BitmapFactory.decodeStream(inputStream) ?: throw IOException("Failed to decode bitmap from stream") }core/data/src/main/java/com/teamwable/data/gallery/GallerySaver.kt (5)
13-13: Address version-specific branching for MediaStore APIs.The TODO comment highlights an important consideration. The current implementation uses MediaStore APIs that require different approaches for different Android versions (particularly API 29+ vs older versions).
Consider implementing version checks for:
- API 29+ (Android 10): Current scoped storage approach
- API 28 and below: Legacy external storage approach with WRITE_EXTERNAL_STORAGE permission
Do you want me to generate a version-specific implementation or create an issue to track this requirement?
10-12: Consider making the function suspend for better integration.Since this operation involves I/O, consider making the function a suspend function for better integration with coroutines and non-blocking behavior.
- fun saveBitmapToGallery(bitmap: Bitmap, filename: String) { + suspend fun saveBitmapToGallery(bitmap: Bitmap, filename: String) {This would align better with the async nature of the calling code in the repository layer.
13-13: Address the TODO comment for version-specific handling.The TODO comment indicates missing version-specific branching, which is crucial for MediaStore operations. Android API 29+ introduced scoped storage changes that affect how files are saved to external storage.
Do you want me to generate the version-specific implementation to handle pre-API 29 devices, or should this be tracked as a separate issue?
14-34: Consider making this function suspending for better performance.The
saveBitmapToGalleryfunction performs I/O operations on the calling thread, which could block the UI if called from the main thread. Since this is used in a coroutine context (via SaveImageUseCase), consider making it a suspend function and usingDispatchers.IO.Apply this diff to make it a suspend function:
- fun saveBitmapToGallery(bitmap: Bitmap, filename: String) { + suspend fun saveBitmapToGallery(bitmap: Bitmap, filename: String) = withContext(Dispatchers.IO) {Don't forget to add the import:
+import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext
37-37: Consider renaming FILE_EXTENSION constant for clarity.The constant name
FILE_EXTENSIONwith a dot prefix could be confusing when used in different contexts. Consider separate constants for file extension and MIME type suffix.Apply this diff to improve constant naming:
- const val FILE_EXTENSION = ".jpeg" + private const val FILE_EXTENSION = ".jpeg" + private const val MIME_TYPE = "image/jpeg"core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageViewModel.kt (2)
20-31: Consider enhancing error handling and user feedback.The implementation correctly handles success and failure cases, but could be improved for better user experience.
Consider these enhancements:
- More specific error messages: Different error types could show different messages to the user.
- Input validation: Validate the imageUrl before proceeding.
fun saveImage(imageUrl: String) { + if (imageUrl.isBlank()) { + viewModelScope.launch { + _sideEffect.send(FeedImageSideEffect.ShowSnackBar(SnackbarType.ERROR)) + } + return + } + viewModelScope.launch { _sideEffect.send(FeedImageSideEffect.ShowSnackBar(SnackbarType.VIEW_IT_ING)) saveImageUseCase(imageUrl) .onSuccess { _sideEffect.send(FeedImageSideEffect.ShowSnackBar(SnackbarType.VIEW_IT_COMPLETE)) } .onFailure { + // Log the error for debugging + android.util.Log.e("FeedImageViewModel", "Failed to save image", it) _sideEffect.send(FeedImageSideEffect.ShowSnackBar(SnackbarType.ERROR, it)) } } }
17-17: Consider using Channel.UNLIMITED for side effects.The
Channel.BUFFEREDhas a default capacity that might drop side effects if the consumer is slow. For UI side effects like snackbars, consider usingChannel.UNLIMITEDto ensure no side effects are lost.Apply this diff to prevent side effect loss:
- private val _sideEffect: Channel<FeedImageSideEffect> = Channel(Channel.BUFFERED) + private val _sideEffect: Channel<FeedImageSideEffect> = Channel(Channel.UNLIMITED)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
core/data/src/main/java/com/teamwable/data/di/RepositoryModule.kt(3 hunks)core/data/src/main/java/com/teamwable/data/gallery/BitmapFetcher.kt(1 hunks)core/data/src/main/java/com/teamwable/data/gallery/GallerySaver.kt(1 hunks)core/data/src/main/java/com/teamwable/data/repository/FeedImageRepository.kt(1 hunks)core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt(1 hunks)core/domain/src/main/java/com/teamwable/domain/usecase/SaveImageUseCase.kt(1 hunks)core/network/src/main/java/com/teamwable/network/TokenInterceptor.kt(6 hunks)core/network/src/main/java/com/teamwable/network/di/NetworkModule.kt(3 hunks)core/ui/build.gradle.kts(1 hunks)core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageDialog.kt(3 hunks)core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageViewModel.kt(1 hunks)core/ui/src/main/java/com/teamwable/ui/util/FeedActionHandler.kt(1 hunks)core/ui/src/main/res/layout/dialog_feed_image.xml(1 hunks)core/ui/src/main/res/navigation/graph_ui.xml(1 hunks)feature/news/src/main/java/com/teamwable/news/detail/NewsDetailFragment.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt (1)
core/data/src/main/java/com/teamwable/data/util/runSuspendCatching.kt (1)
runSuspendCatching(6-16)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Checker
🔇 Additional comments (26)
feature/news/src/main/java/com/teamwable/news/detail/NewsDetailFragment.kt (1)
12-12: Import updated to new package path
The import forFeedImageDialognow points to its relocatedfeedimagepackage. This aligns with your refactoring and ensures the correct class is referenced.core/ui/src/main/java/com/teamwable/ui/util/FeedActionHandler.kt (1)
18-18: Aligned import for relocated dialog component
Updating theFeedImageDialogimport to thefeedimagesubpackage is correct and keeps this handler in sync with the refactored UI module.core/ui/build.gradle.kts (1)
11-11: Added dependency on:core:domain
Including the domain module here is necessary for accessingSaveImageUseCase. Theimplementationconfiguration is appropriate unless you need to expose domain APIs transitively.core/ui/src/main/res/navigation/graph_ui.xml (1)
32-32: Navigation graph updated to relocated dialog class
The dialog node now referencescom.teamwable.ui.component.feedimage.FeedImageDialog, matching the new package structure. Navigation will resolve correctly.core/domain/src/main/java/com/teamwable/domain/usecase/SaveImageUseCase.kt (1)
6-11: Good use case implementation pattern.The operator invoke pattern and direct delegation to the repository is clean and follows use case best practices.
core/data/src/main/java/com/teamwable/data/di/RepositoryModule.kt (2)
6-6: Import additions look correct.The imports for the new repository interface and implementation follow the established patterns.
Note: If the
FeedImageRepositoryinterface is moved to the domain layer (as suggested in the architecture review), this import will need to be updated accordingly.Also applies to: 17-17
98-102: Dependency injection binding is correctly implemented.The binding follows the established pattern and correctly maps the interface to its implementation.
core/network/src/main/java/com/teamwable/network/TokenInterceptor.kt (2)
6-6: Good refactor to use interface dependency.Changing from the concrete
DefaultWablePreferenceDatasourceto theWablePreferencesDataSourceinterface improves testability and follows dependency inversion principles.Also applies to: 24-24
51-52: Consistent interface usage throughout the file.All references to the preferences data source have been consistently updated to use the interface, maintaining the same functionality while improving the dependency injection pattern.
Also applies to: 61-61, 78-78, 103-103
core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageDialog.kt (4)
1-1: Package relocation looks appropriate.Moving the dialog to a more specific package
com.teamwable.ui.component.feedimageimproves code organization and follows the principle of logical grouping.
24-27: Hilt integration and ViewModel injection implemented correctly.The
@AndroidEntryPointannotation andby viewModels()delegation properly integrate with Hilt for dependency injection. This follows Android architecture best practices.
52-54: Save button implementation with duplicate click prevention is well done.Using
setOnDuplicateBlockClickprevents rapid successive clicks that could trigger multiple save operations, which is a good UX practice for potentially long-running operations like image downloading and saving.
56-64: Side effect collection follows best practices for lifecycle-aware UI updates.The implementation properly uses
flowWithLifecycleandviewLifeCycleScopeto ensure the collection is lifecycle-aware, preventing memory leaks and crashes from UI updates when the view is not active.core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt (2)
11-14: Constructor injection and internal visibility are appropriate.The constructor properly injects dependencies via Hilt, and the
internalvisibility modifier correctly restricts access to the module scope.
15-21:⚠️ Potential issueFix the error handling flow - early return prevents proper Result wrapping.
The current implementation has a logical error in the error handling. The
onFailureblock returns early, which means the originalResultfromrunSuspendCatchingis never returned on success.Apply this diff to fix the error handling:
override suspend fun saveToGallery(imageUrl: String): Result<Unit> = runSuspendCatching { val bitmap = bitmapFetcher.fetchBitmapFromUrl(imageUrl) gallerySaver.saveBitmapToGallery(bitmap, "wable_${System.currentTimeMillis()}$FILE_EXTENSION") -}.onFailure { - return it.handleThrowable() -} +}.fold( + onSuccess = { Result.success(Unit) }, + onFailure = { it.handleThrowable() } +)Likely an incorrect or invalid review comment.
core/network/src/main/java/com/teamwable/network/di/NetworkModule.kt (3)
27-34: JSON configuration is appropriate for the application needs.The JSON configuration with
prettyPrint,isLenient, andignoreUnknownKeysprovides good flexibility for API responses while maintaining readability during development.
42-43: Timeout configuration is reasonable for image downloading.The 5-second timeouts for both connection and read operations provide a good balance between allowing sufficient time for network operations and preventing indefinite hangs. This is particularly important for the image downloading functionality.
71-76: Retrofit parameter injection follows dependency injection best practices.Injecting the
Jsoninstance as a parameter rather than creating it inline improves testability and consistency across the application. Both retrofit providers now use the same JSON configuration.Also applies to: 92-96
core/data/src/main/java/com/teamwable/data/gallery/BitmapFetcher.kt (2)
13-15: Dependency injection setup is correct for image fetching.Using
@WithoutTokenInterceptorOkHttpClient is appropriate for downloading public images, as authentication tokens are not needed and could potentially cause issues with external image URLs.
16-29: Bitmap fetching implementation follows best practices.The implementation correctly:
- Uses
Dispatchers.IOfor network operations- Properly handles resources with
.use()- Checks response success before processing
- Provides meaningful error messages
- Handles null bitmap decode gracefully
core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageViewModel.kt (6)
13-16: LGTM! Well-structured ViewModel with proper dependency injection.The ViewModel follows Android architecture best practices with proper Hilt integration and clean separation of concerns by delegating business logic to the domain layer.
17-18: Good use of Channel for side effects.The Channel-based approach for managing side effects is appropriate and provides good separation between state and one-time events.
34-39: Well-designed sealed interface for type safety.The sealed interface provides excellent type safety and includes optional error details for comprehensive error handling.
20-31: Excellent implementation of the save image functionality.The function properly handles the async operation with coroutines, provides immediate feedback to users, and handles both success and failure cases appropriately. The use of the domain layer use case maintains good separation of concerns.
34-39: Well-designed side effect interface.The sealed interface provides a clean and type-safe way to represent different side effects. The optional
throwableparameter allows for detailed error reporting when needed.
22-22:Details
❓ Verification inconclusive
Verify the SnackbarType.VIEW_IT_ING constant exists.
Ensure that the
VIEW_IT_INGconstant is properly defined in theSnackbarTypeenum and follows naming conventions.
🏁 Script executed:
#!/bin/bash # Description: Verify SnackbarType enum contains the referenced constants # Expected: Find VIEW_IT_ING, VIEW_IT_COMPLETE, and ERROR constants ast-grep --pattern 'enum class SnackbarType { $$$ }'Length of output: 56
🏁 Script executed:
#!/bin/bash # Locate the file defining SnackbarType enum_file=$(rg --files-with-matches 'enum class SnackbarType' -l) echo "Found enum in: $enum_file" # Print the enum definition to inspect its constants sed -n '/enum class SnackbarType\s*{/,/}/p' "$enum_file"Length of output: 392
#!/bin/bash # Display the SnackbarType enum definition for constant verification file="core/ui/src/main/java/com/teamwable/ui/type/SnackbarType.kt" echo "Enum definition in $file:" rg -n "enum class SnackbarType" -A 50 "$file"
#!/bin/bash # Show complete SnackbarType enum for verification file="core/ui/src/main/java/com/teamwable/ui/type/SnackbarType.kt" echo "----- $file -----" sed -n '1,200p' "$file"
#!/bin/bash # Verify that VIEW_IT_ING is defined in SnackbarType enum file="core/ui/src/main/java/com/teamwable/ui/type/SnackbarType.kt" echo "Searching for VIEW_IT_ING in $file:" rg -n "VIEW_IT_ING" "$file" || echo "VIEW_IT_ING not found" echo -e "\nFull enum definition:" sed -n '/enum class SnackbarType\s*{/,/}/p' "$file"
#!/usr/bin/env bash file="core/ui/src/main/java/com/teamwable/ui/type/SnackbarType.kt" echo "=== Searching for VIEW_IT_ING constant ===" grep -n "VIEW_IT_ING" "$file" || echo "VIEW_IT_ING not found" echo -e "\n=== Full enum definition ===" sed -n '/enum class SnackbarType\s*{/,/}/p' "$file"
#!/usr/bin/env bash # Verify presence of VIEW_IT_ING in the SnackbarType enum file="core/ui/src/main/java/com/teamwable/ui/type/SnackbarType.kt" echo "Searching for VIEW_IT_ING in $file:" rg -n "VIEW_IT_ING" "$file" || echo "VIEW_IT_ING not found"
Confirm
VIEW_IT_INGis defined inSnackbarTypePlease verify that the enum at
core/ui/src/main/java/com/teamwable/ui/type/SnackbarType.kt
includes theVIEW_IT_INGconstant (alongside other expected values likeVIEW_IT_COMPLETEandERROR) and that its naming aligns with the project’s enum conventions.
- File to check:
core/ui/src/main/java/com/teamwable/ui/type/SnackbarType.kt- Ensure
VIEW_IT_INGis present and correctly named
core/data/src/main/java/com/teamwable/data/repository/FeedImageRepository.kt
Show resolved
Hide resolved
core/domain/src/main/java/com/teamwable/domain/usecase/SaveImageUseCase.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
Adds functionality for saving feed images to the device gallery by introducing a new dialog component, ViewModel, use case, repository, and data-layer logic, along with necessary DI and network adjustments.
- Refactored
FeedImageDialoginto a dedicatedfeedimagepackage and updated navigation graph. - Added UI elements (save button),
FeedImageViewModel, and side-effect handling for image saving. - Integrated
SaveImageUseCaseand repository implementations, updated DI modules, and configured JSON converter and network timeouts.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| feature/news/src/main/java/com/teamwable/news/detail/NewsDetailFragment.kt | Updated import to new FeedImageDialog package path |
| core/ui/src/main/res/navigation/graph_ui.xml | Renamed dialog class reference in navigation graph |
| core/ui/src/main/res/layout/dialog_feed_image.xml | Added save action TextView (tv_feed_save) in dialog layout |
| core/ui/src/main/java/com/teamwable/ui/util/FeedActionHandler.kt | Updated FeedImageDialog import |
| core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageViewModel.kt | Added ViewModel for save-image use case and side effects |
| core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageDialog.kt | New dialog with save button setup and side-effect collection |
| core/ui/build.gradle.kts | Added :core:domain dependency |
| core/network/src/main/java/com/teamwable/network/di/NetworkModule.kt | Provided a Json instance and configured 5s timeouts |
| core/network/src/main/java/com/teamwable/network/TokenInterceptor.kt | Switched to WablePreferencesDataSource in interceptor |
| core/domain/src/main/java/com/teamwable/domain/usecase/SaveImageUseCase.kt | Introduced use case for saving images |
| core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt | Implemented repository to fetch and save images |
| core/data/src/main/java/com/teamwable/data/repository/FeedImageRepository.kt | Added repository interface |
| core/data/src/main/java/com/teamwable/data/gallery/GallerySaver.kt | Added logic for saving bitmaps to gallery |
| core/data/src/main/java/com/teamwable/data/gallery/BitmapFetcher.kt | Added logic for downloading image bitmaps |
| core/data/src/main/java/com/teamwable/data/di/RepositoryModule.kt | Bound new FeedImageRepository in DI |
Comments suppressed due to low confidence (2)
core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageDialog.kt:56
- [nitpick] The method name
collectis too generic. Consider renaming it tocollectSideEffectsor similar to clarify its purpose.
private fun collect() {
core/domain/src/main/java/com/teamwable/domain/usecase/SaveImageUseCase.kt:1
- The new
SaveImageUseCasedoes not have unit tests. Add tests to cover both success and failure paths ofinvoke(imageUrl).
package com.teamwable.domain.usecase
| // Todo : 버전별 분기처리 | ||
| fun saveBitmapToGallery(bitmap: Bitmap, filename: String) { | ||
| val contentValues = ContentValues().apply { | ||
| put(MediaStore.MediaColumns.DISPLAY_NAME, filename) | ||
| put(MediaStore.MediaColumns.MIME_TYPE, "image/$FILE_EXTENSION") | ||
| put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_PICTURES}/$FOLDER_NAME") | ||
| put(MediaStore.MediaColumns.IS_PENDING, 1) | ||
| } | ||
|
|
||
| val uri = contentResolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues) | ||
| if (uri != null) { | ||
| contentResolver.openOutputStream(uri).use { output -> | ||
| if (output != null) { | ||
| bitmap.compress(Bitmap.CompressFormat.JPEG, 100, output) | ||
| } | ||
| } | ||
|
|
||
| contentValues.clear() | ||
| contentValues.put(MediaStore.MediaColumns.IS_PENDING, 0) | ||
| contentResolver.update(uri, contentValues, null, null) |
There was a problem hiding this comment.
The TODO notes version-specific branching for gallery saving. Implement proper handling for pre- and post-Android Q APIs to ensure compatibility.
| // Todo : 버전별 분기처리 | |
| fun saveBitmapToGallery(bitmap: Bitmap, filename: String) { | |
| val contentValues = ContentValues().apply { | |
| put(MediaStore.MediaColumns.DISPLAY_NAME, filename) | |
| put(MediaStore.MediaColumns.MIME_TYPE, "image/$FILE_EXTENSION") | |
| put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_PICTURES}/$FOLDER_NAME") | |
| put(MediaStore.MediaColumns.IS_PENDING, 1) | |
| } | |
| val uri = contentResolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues) | |
| if (uri != null) { | |
| contentResolver.openOutputStream(uri).use { output -> | |
| if (output != null) { | |
| bitmap.compress(Bitmap.CompressFormat.JPEG, 100, output) | |
| } | |
| } | |
| contentValues.clear() | |
| contentValues.put(MediaStore.MediaColumns.IS_PENDING, 0) | |
| contentResolver.update(uri, contentValues, null, null) | |
| fun saveBitmapToGallery(bitmap: Bitmap, filename: String) { | |
| if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.Q) { | |
| val contentValues = ContentValues().apply { | |
| put(MediaStore.MediaColumns.DISPLAY_NAME, filename) | |
| put(MediaStore.MediaColumns.MIME_TYPE, "image/$FILE_EXTENSION") | |
| put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_PICTURES}/$FOLDER_NAME") | |
| put(MediaStore.MediaColumns.IS_PENDING, 1) | |
| } | |
| val uri = contentResolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues) | |
| if (uri != null) { | |
| contentResolver.openOutputStream(uri).use { output -> | |
| if (output != null) { | |
| bitmap.compress(Bitmap.CompressFormat.JPEG, 100, output) | |
| } | |
| } | |
| contentValues.clear() | |
| contentValues.put(MediaStore.MediaColumns.IS_PENDING, 0) | |
| contentResolver.update(uri, contentValues, null, null) | |
| } | |
| } else { | |
| val directory = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_PICTURES) | |
| val folder = File(directory, FOLDER_NAME) | |
| if (!folder.exists()) { | |
| folder.mkdirs() | |
| } | |
| val file = File(folder, "$filename$FILE_EXTENSION") | |
| FileOutputStream(file).use { output -> | |
| bitmap.compress(Bitmap.CompressFormat.JPEG, 100, output) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultCommunityRepository.kt (1)
18-28: 동일한 클래스 내 다른 메서드들도 일관성 있게 리팩토링하는 것을 고려해보세요.
getCommunities()와getJoinedCommunity()메서드들도 여전히 기존 에러 핸들링 패턴(.catch { throw it.toCustomError() })을 사용하고 있습니다. 일관성을 위해 이들도 새로운 패턴으로 변경하는 것을 고려해볼 수 있습니다.core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultProfileRepository.kt (1)
27-86: 클래스 전체에 걸쳐 일관된 에러 핸들링 패턴 적용을 고려해보세요.현재 일부 메서드만 새로운
runHandledCatching패턴으로 업데이트되었지만, 다른 메서드들(getProfileInfo,getMemberData,patchWithdrawal,postReport,postBan)은 여전히 기존 패턴을 사용합니다. 또한 16번째 줄의handleThrowableimport도 더 이상 사용되지 않아 제거할 수 있습니다.일관성을 위해 모든 메서드를 새로운 패턴으로 변경하거나, 단계적으로 진행 중이라면 이후 PR에서 완료하는 것을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultAuthRepository.kt(1 hunks)core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultCommunityRepository.kt(2 hunks)core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt(1 hunks)core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultProfileRepository.kt(2 hunks)core/data/src/main/java/com/teamwable/data/util/runSuspendCatching.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt
🧰 Additional context used
🧬 Code Graph Analysis (2)
core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultAuthRepository.kt (1)
core/data/src/main/java/com/teamwable/data/util/runSuspendCatching.kt (1)
runHandledCatching(19-24)
core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultCommunityRepository.kt (1)
core/data/src/main/java/com/teamwable/data/util/runSuspendCatching.kt (1)
runHandledCatching(19-24)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Checker
🔇 Additional comments (5)
core/data/src/main/java/com/teamwable/data/util/runSuspendCatching.kt (1)
19-24: 새로운 에러 핸들링 유틸리티 함수가 잘 구현되었습니다.
runHandledCatching함수는 기존의 반복적인 에러 핸들링 코드를 중앙화하여 코드 중복을 줄이고 일관성을 향상시킵니다. 인라인 확장 함수로 구현되어 성능상 오버헤드도 없고,toCustomError()를 통한 자동 에러 변환이 깔끔하게 처리됩니다.core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultCommunityRepository.kt (1)
30-33:runHandledCatching사용으로 깔끔하게 리팩토링되었습니다.새로운 유틸리티 함수를 사용하여 명시적인 에러 핸들링 코드가 제거되고 코드가 간결해졌습니다.
core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultAuthRepository.kt (1)
14-16: 에러 핸들링 리팩토링이 올바르게 적용되었습니다.
runCatching에서runHandledCatching으로 변경하여 코드가 더 간결해지고 에러 핸들링이 중앙화되었습니다. 메서드 시그니처도 그대로 유지되어 호환성에 문제가 없습니다.core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultProfileRepository.kt (2)
46-52:patchUserProfile메서드의 에러 핸들링이 깔끔하게 개선되었습니다.
runHandledCatching을 사용하여 기존의 명시적인 에러 핸들링 코드가 제거되고 코드가 간결해졌습니다.
54-57:getNickNameDoubleCheck메서드도 일관성 있게 리팩토링되었습니다.새로운 에러 핸들링 패턴이 올바르게 적용되었습니다.
Docstrings generation was requested by @chanubc. * #168 (comment) The following files were modified: * `core/data/src/main/java/com/teamwable/data/di/RepositoryModule.kt` * `core/data/src/main/java/com/teamwable/data/gallery/BitmapFetcher.kt` * `core/data/src/main/java/com/teamwable/data/gallery/GallerySaver.kt` * `core/data/src/main/java/com/teamwable/data/repository/FeedImageRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultAuthRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultCommunityRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultProfileRepository.kt` * `core/data/src/main/java/com/teamwable/data/util/runSuspendCatching.kt` * `core/domain/src/main/java/com/teamwable/domain/usecase/SaveImageUseCase.kt` * `core/network/src/main/java/com/teamwable/network/TokenInterceptor.kt` * `core/network/src/main/java/com/teamwable/network/di/NetworkModule.kt` * `core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageDialog.kt` * `core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageViewModel.kt`
|
Note Generated docstrings for this pull request at #169 |
Docstrings generation was requested by @chanubc. * #168 (comment) The following files were modified: * `core/data/src/main/java/com/teamwable/data/di/RepositoryModule.kt` * `core/data/src/main/java/com/teamwable/data/gallery/BitmapFetcher.kt` * `core/data/src/main/java/com/teamwable/data/gallery/GallerySaver.kt` * `core/data/src/main/java/com/teamwable/data/repository/FeedImageRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultAuthRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultCommunityRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultFeedImageRepository.kt` * `core/data/src/main/java/com/teamwable/data/repositoryimpl/DefaultProfileRepository.kt` * `core/data/src/main/java/com/teamwable/data/util/runSuspendCatching.kt` * `core/domain/src/main/java/com/teamwable/domain/usecase/SaveImageUseCase.kt` * `core/network/src/main/java/com/teamwable/network/TokenInterceptor.kt` * `core/network/src/main/java/com/teamwable/network/di/NetworkModule.kt` * `core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageDialog.kt` * `core/ui/src/main/java/com/teamwable/ui/component/feedimage/FeedImageViewModel.kt` * `feature/main/src/main/java/com/teamwable/main/MainActivity.kt`
|
Note Generated docstrings for this pull request at #170 |
| inline fun <T, R> T.runHandledCatching(block: T.() -> R): Result<R> { | ||
| return runSuspendCatching(block).fold( | ||
| onSuccess = { Result.success(it) }, | ||
| onFailure = { Result.failure(it.toCustomError()) }, | ||
| ) | ||
| } |
| fun providesJson(): Json = Json { | ||
| prettyPrint = true | ||
| isLenient = true | ||
| ignoreUnknownKeys = true |
There was a problem hiding this comment.
서버에서 줄 때 모델에 정의되지 않은 필드가 있어도 무시해주는 설정이 있었군요,, 유용하네요!
There was a problem hiding this comment.
앗 돈비때 있던 코드 긁어왔는디요..
이때는 Json을 새로 객체화 해서 주입해서 적용이 안됬고 이미 생성된 json을 사용하니 적용이 되더라구요!
|
아 그리고 code rabbit이 요약도 있고 더 자세해서 좋은거 같아요~~ |
✅ 𝗖𝗵𝗲𝗰𝗸-𝗟𝗶𝘀𝘁
📌 𝗜𝘀𝘀𝘂𝗲𝘀
📎𝗪𝗼𝗿𝗸 𝗗𝗲𝘀𝗰𝗿𝗶𝗽𝘁𝗶𝗼𝗻
📷 𝗦𝗰𝗿𝗲𝗲𝗻𝘀𝗵𝗼𝘁
Screen_recording_20250608_225840.webm
KakaoTalk_20250609_000359666.mp4
💬 𝗧𝗼 𝗥𝗲𝘃𝗶𝗲𝘄𝗲𝗿𝘀
ui나오면 추후 수정할께요!
feed image dialog fragemnt가 viewmodel을 가져야 하는구조라
ui에서 data 의존해야 하는데 이건 과도한 정보 노출이라
일단 domain에 의존하도록 했습니다.
더 좋은 방법 있으면 의견 남겨주세요!
기존의 runSuspendCatching과 더불어
failure시에 custom error로 매핑 시켜주는 runHandledCatching 함수를 만들었습니다.
이제 onFailure블록을 직접 명시할 필요가 없어졌어요!
추가로 code rabbit vs copilot 코드리뷰 중에 뭐가 더 나으신가요?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Other