Open
Conversation
- S3 to File : S3 버킷은 서버 단에서 활용하는 도구이므로, 본 프로젝트에서 해당 이름을 사용하지 않습니다
- 원본 사이즈 읽어 비율 유지해 리사이징 - 생성된 파일 객체 제거
- file 기반 이미지 업로드 - uri 기반 이미지 생성 - 생성된 이미지 제거
- 다음 과정 순차 수행 : presigned-url 발급 > 리사이징된 파일 객체 생성 > s3 업로드 > 파일 객체 메모리에서 삭제
- FileUploadDataSource to FileUploadRemoteDataSource
Open
2 tasks
📝 WalkthroughWalkthrough이미지 변환(URI→파일), 압축, 임시 저장, 프리사인드 URL 기반 업로드 파이프라인과 DI 바인딩(한정자 리네임, Repository 바인딩)이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UC as UploadImagesUseCaseV2
participant IR as ImageRepository
participant FR as FileUploadRepository
participant FL as FileLocalDataSource
participant RU as FileUploadRemoteDataSource
UC->>IR: getPresignedUrls(uploadType, extensions)
IR-->>UC: List<String>
loop for each uri
UC->>FR: createImage(uriString)
FR->>FL: createImage(uriString)
FL->>FL: decode, resize, compress -> temp File
FL-->>FR: File
end
loop for each file & url
UC->>FR: uploadImage(uploadUrl, file)
FR->>RU: uploadImage(uploadUrl, file)
RU->>RU: build PUT request (image/jpeg)
RU-->>FR: success/failure
end
UC->>FR: clearDirectory()
FR->>FL: clearDirectory()
FL-->>FR: Result
UC-->>UC: Result<List<String>>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
|
✅ Ktlint check passed. Run: https://github.com/team-poti/POTI-ANDROID/actions/runs/21852143764 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/data/repository/FileUploadRepositoryImpl.kt`:
- Around line 10-13: The constructor parameter name fileUplaodRemoteDataSource
in class FileUploadRepositoryImpl is misspelled; rename it to
fileUploadRemoteDataSource everywhere (constructor, property declaration, usages
inside FileUploadRepositoryImpl) and update any places that inject or reference
this dependency (e.g., constructor injection sites and calls to methods on
fileUplaodRemoteDataSource) to use the corrected identifier so compilation and
searches work correctly.
In
`@app/src/main/java/com/poti/android/domain/repository/FileUplaodRepository.kt`:
- Around line 1-14: Rename the source file from "FileUplaodRepository.kt" to the
correctly spelled "FileUploadRepository.kt" and update any references or imports
that refer to the misspelled filename; ensure the interface declaration
FileUploadRepository remains unchanged and that build files, tests, or other
modules that import this interface are adjusted to reference the new filename.
In
`@app/src/main/java/com/poti/android/domain/usecase/image/UploadImagesUseCaseV2.kt`:
- Around line 19-25: If createImages(...) succeeds but an exception occurs
afterwards (e.g., during uploadImages(...)), clearDirectory() may never run;
wrap the sequence that calls getUploadUrls(...), createImages(...), and
uploadImages(...) in a try/finally so clearDirectory() is invoked in the finally
block as a best-effort cleanup, while preserving and rethrowing any exceptions
from uploadImages; specifically modify the flow around getUploadUrls,
createImages, uploadImages, and clearDirectory to ensure clearDirectory() always
runs even on error.
- Around line 18-29: The catch-all Throwable in UploadImagesUseCaseV2
(surrounding getUploadUrls, createImages, uploadImages, clearDirectory) swallows
coroutine cancellation; change the error handling to either catch Exception
instead of Throwable or explicitly rethrow CancellationException
(kotlinx.coroutines.CancellationException) when caught, then return
Result.failure for other exceptions—ensure CancellationException from coroutines
is not swallowed by rethrowing it immediately.
- Around line 46-51: The uploadImages function fails to compile because zip's
transform lambda isn't suspend; replace the zip+lambda with an explicit
suspend-safe loop: first validate urls.size == files.size and throw or return an
error if they differ, then iterate indices (for (i in urls.indices)) and call
the suspend function fileUploadRepository.uploadImage(urls[i], files[i]) to
collect results into a List to return; also ensure temporary cleanup by invoking
clearDirectory() in a finally block (or using try/catch/finally) so
clearDirectory() runs regardless of success or failure. Reference: uploadImages,
fileUploadRepository.uploadImage, clearDirectory.
🧹 Nitpick comments (4)
app/src/main/java/com/poti/android/data/remote/datasource/FileUploadRemoteDataSource.kt (1)
14-20: 테스트 용이성을 위해 Dispatcher 주입을 권장합니다.현재
Dispatchers.IO가 하드코딩되어 있어 단위 테스트 시 Dispatcher를 교체하기 어렵습니다. 코딩 가이드라인에 따라 생성자를 통해 Dispatcher를 주입받는 것이 좋습니다.♻️ Dispatcher 주입 제안
+import kotlinx.coroutines.CoroutineDispatcher + class FileUploadRemoteDataSource `@Inject` constructor( `@param`:FileUploadClient private val okHttpClient: OkHttpClient, + private val ioDispatcher: CoroutineDispatcher, ) { suspend fun uploadImage( uploadUrl: String, file: File, - ) = withContext(Dispatchers.IO) { + ) = withContext(ioDispatcher) {DI 모듈에서
@IoDispatcherqualifier와 함께Dispatchers.IO를 제공하시면 됩니다.app/src/main/java/com/poti/android/data/local/datasource/FileLocalDataSource.kt (2)
20-27: 블로킹 I/O 작업에 대한 스레드 안전성을 고려해 주세요.
createImageFile()함수는 이미지 디코딩, 압축, 파일 쓰기 등 상당한 I/O 작업을 수행하지만 suspend 함수가 아닙니다. 메인 스레드에서 호출될 경우 ANR이 발생할 수 있습니다.
suspend함수로 변경하고withContext(Dispatchers.IO)를 사용하거나, 호출부에서 백그라운드 스레드를 보장해야 합니다.♻️ suspend 함수로 변경 제안
+import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext + - fun createImageFile(uriString: String): File { + suspend fun createImageFile(uriString: String): File = withContext(Dispatchers.IO) { val uri = uriString.toUri() val directory = getDirectory() val compressedImage = compressImage(uri, directory) - return compressedImage + compressedImage }
29-32: clearDirectory()도 동일하게 suspend 함수 변환을 권장합니다.파일 삭제 작업도 I/O 작업이므로
createImageFile()과 동일한 패턴을 적용하시면 좋겠습니다.app/src/main/java/com/poti/android/data/repository/FileUploadRepositoryImpl.kt (1)
22-37: runCatching으로 보일러플레이트 축소 제안
try/catch + Result패턴이 반복됩니다.runCatching으로 간결하게 표현할 수 있습니다.♻️ 제안 수정
override fun createImage(uriString: String): Result<File> { - try { - val file = fileLocalDataSource.createImageFile(uriString) - return Result.success(file) - } catch (exception: Throwable) { - return Result.failure(exception) - } + return runCatching { + fileLocalDataSource.createImageFile(uriString) + } } override fun clearDirectory(): Result<Unit> { - try { - fileLocalDataSource.clearDirectory() - return Result.success(Unit) - } catch (exception: Throwable) { - return Result.failure(exception) - } + return runCatching { + fileLocalDataSource.clearDirectory() + } }
- as-is : finally 블록에서 에러를 throw로 전파해 위험 - to-be : throw 제거, runCatching으로 에러 시 로그만 찍음
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/domain/usecase/image/UploadImagesUseCaseV2.kt`:
- Line 7: UploadImagesUseCaseV2 currently imports and uses Android-specific
Timber; remove the direct Timber dependency from the domain layer and replace it
with a pure-Kotlin logging abstraction or move logging out of the use case.
Create or use an injected Logger interface (e.g., Logger.log/d debug/warn/error)
and update UploadImagesUseCaseV2 to accept the Logger in its constructor (or
remove all logging calls here and let the caller in the data/presentation layer
handle logs) so the domain module remains Android-free; ensure all usages of
Timber in UploadImagesUseCaseV2 are replaced by the Logger methods or removed.
🧹 Nitpick comments (2)
app/src/main/java/com/poti/android/domain/repository/FileUploadRepository.kt (1)
5-14:createImage와clearDirectory가 I/O 작업임에도suspend가 아닌 점이 아쉽습니다.
createImage는 URI로부터 파일을 생성하는 I/O 작업이고,clearDirectory역시 파일 시스템 작업입니다. 현재 non-suspend로 선언되어 있어, 호출하는 코루틴의 스레드(예:Main)를 블로킹할 수 있습니다.구현체(
Impl)에서withContext(Dispatchers.IO)를 사용하여 처리하시는 구조라면suspend로 선언하시는 것이 더 안전하고, Dispatcher 주입을 통한 테스트 용이성도 확보할 수 있습니다.🛠️ 제안 수정
interface FileUploadRepository { suspend fun uploadImage( uploadUrl: String, file: File, ): Result<Unit> - fun createImage(uriString: String): Result<File> + suspend fun createImage(uriString: String): Result<File> - fun clearDirectory(): Result<Unit> + suspend fun clearDirectory(): Result<Unit> }As per coding guidelines, "Dispatcher 주입:
Dispatchers.IO등을 하드코딩하지 말고, 테스트 용이성을 위해 생성자를 통해 주입(Injection)받도록 제안해줘."app/src/main/java/com/poti/android/domain/usecase/image/UploadImagesUseCaseV2.kt (1)
63-68:clearDirectory()에서runCatching과Result가 이중으로 감싸지고 있습니다.
fileUploadRepository.clearDirectory()가 이미Result<Unit>을 반환하므로 예외를 던지지 않습니다. 바깥의runCatching은 불필요한 이중 래핑이 됩니다. 간결하게 정리하시면 좋겠습니다.♻️ 제안 수정
- private fun clearDirectory() = runCatching { - fileUploadRepository.clearDirectory() - .onFailure { - Timber.e(it, "fail on clearDirectory") - } - } + private fun clearDirectory() { + fileUploadRepository.clearDirectory() + .onFailure { + Timber.e(it, "fail on clearDirectory") + } + }
- Timber 의존성 제거 - Repository에 기본 에러처리가 되어있어 중복처리 제거
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
2 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.
Related issue 🛠️
Work Description ✏️
파일 양이 엄청 많은 건 아니지만 image 관련 data layer에 대폭 변경사항이 있었고 쉽게 이해할 수 있는 로직은 아니어서 노션에 아티클 작성했습니다! 참고해주세욤
이미지 최적화
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
data~domain 레이어까지만 작성했습니다! presentation에는 변경된 이미지 로직을 적용하지 않았어요. 본 PR과 등록 뷰모델 리팩토링 건이 모두 병합된 후에 이슈 따로 파서 진행하겠습니다. (이 때 S3Repository, ImageUtil, UploadImagesUseCase도 삭제할게용)
Summary by CodeRabbit