Conversation
…인터페이스 추가 및 로직 간소화
…into refactor/#87-today-journey-refactor
sonms
left a comment
There was a problem hiding this comment.
어떻게 나눠서 쓸까하다가 한 번에 흐름을 보여주는게 더 보기 편할 것 같아 여기에 쓸게요!
현재 구조상 많은 고민을 했다는게 눈에 보여서 정말 좋습니다잉 대신 몇가지 수정하면 좋을 것 같아서 이렇게 남겨요~!
-
ImageUriManager
빈 임시 파일과 URI를 생성하는 로직이라고 생각되요!(맞나요..?)
우선 System.currentTimeMillis() 대신 File.createTempFile()을 사용하는게 좋을 것 같아요
물론! 임시 파일 생성이 아닌 메모리에 파일 객체를 만들때에는 전자 방식이 좋습니다잉 -
(왕김) 현재 구조는 ViewModel 비대해지고 Repository와 DataSource 간의 경계가 무너진 상황입니다
-
ImageUploadRepositoryImpl 이 context 와 contentResolver를 주입받아 uri를 열고 byte array까지 사용 중인데요 repository 의 역할은 여러 datasource를 조합하여 비즈니스 로직에 맞는 데이터로 가공해서 전달하는 역할이죠
현재는 구현 세부 사항을 datasource가 아닌 repositoryimpl이 알고 있는 상황이죠 가공만해서 던져주면 되는데?
file을 열고 byteArray를 통해 만들고 등등 너무 많은 것을 알고있죠? -
또한 ViewModel의 함수 하나가 너무 크죠?
현재 함수 updateTempUri에서 repository 2개를 호출하고 있죠? 이번에 usecase를 도입한 김에 한 번 구현해봅시다
현재 apijob은 1) 이미지를 업로드하고 -> 2) 그 결과(URL)를 받아서 -> 3) 일정을 완료 처리(Patch) 하는 2개의 서로 다른 Repository를 조립하는 중이에요
물론 이렇게 해서 큰 문제가 없다면 상관은 없으나 domain 계층을 도입하기로 했으니 usecase에서 처리하는거고 만약 domain 레이어가 없을 경우 repositoryimpl에서 사용하는 편이 더 좋아요
가공의 역할을 하니까요!
domain 레이어가 없을 때
// Image와 관련된 모든 복잡한 처리는 얘가 전담함
class ImageRepository @Inject constructor(
private val localDataSource: ImageLocalDataSource,
private val remoteDataSource: ImageRemoteDataSource
) {
suspend fun processAndUploadImage(uri: String): String {
val compressedFile = localDataSource.getOptimizedFile(uri)
val url = remoteDataSource.upload(compressedFile)
localDataSource.clearCache()
return url
}
}
class ScheduleRepositoryImpl @Inject constructor(
private val scheduleRemoteDataSource: ScheduleRemoteDataSource,
private val imageRepository: ImageRepository
): ScheduleRepository {
....
}대신 클래스간 의존도가 높아져요!
domain 레이어가 있을 때 (usecase)
이곳에선 고화질 이미지 사용 시 file 크래시 위험을 방지 및 서버 저장소도 절약가능하게 이미지 압축을 사용할 거에요
ImageLocalDataSource
class ImageLocalDataSource @Inject constructor(
@ApplicationContext private val context: Context
) {
fun getOptimizedFile(uriString: String): File {
val uri = uriString.toUri()
val dir = getDirectory()
return compressToWebP(uri, dir)
}
fun clearCache() {
getDirectory().listFiles()?.forEach { it.delete() }
}
private fun getDirectory(): File {
return File(context.cacheDir, DIRECTORY).apply {
if (!exists()) mkdirs()
}
}
private fun compressToWebP(uri: Uri, dir: File): File {
val source = ImageDecoder.createSource(context.contentResolver, uri)
val bitmap = ImageDecoder.decodeBitmap(source) { decoder, info, _ ->
decoder.allocator = ImageDecoder.ALLOCATOR_SOFTWARE
decoder.isMutableRequired = true
val size = info.size
val targetSize = calculateTargetSize(size.width, size.height)
decoder.setTargetSize(targetSize.first, targetSize.second)
}
val format = if (Build.VERSION.SDK_INT >= 30) {
Bitmap.CompressFormat.WEBP_LOSSY
} else {
Bitmap.CompressFormat.WEBP
}
val byteArray = ByteArrayOutputStream().use { stream ->
bitmap.compress(format, WEBP_QUALITY, stream)
bitmap.recycle()
stream.toByteArray()
}
val tempFile = File(dir, "${UUID.randomUUID()}.webp")
FileOutputStream(tempFile).use { it.write(byteArray) }
return tempFile
}
private fun calculateTargetSize(width: Int, height: Int): Pair<Int, Int> {
if (width <= MAX_SIZE && height <= MAX_SIZE) return width to height
val ratio = max(width.toFloat() / MAX_SIZE, height.toFloat() / MAX_SIZE)
return (width / ratio).toInt() to (height / ratio).toInt()
}
fun getImageSize(file: File): Pair<Int, Int> {
val options = android.graphics.BitmapFactory.Options().apply {
inJustDecodeBounds = true
}
android.graphics.BitmapFactory.decodeFile(file.absolutePath, options)
return options.outWidth to options.outHeight
}
// 개별 삭제 함수
fun deleteOriginalUri(uriString: String) {
try {
context.contentResolver.delete(uriString.toUri(), null, null)
} catch (e: Exception) {
Timber.e(e, "원본 파일 삭제 실패")
}
}
companion object {
private const val DIRECTORY = "image_cache"
private const val MAX_SIZE = 1024
private const val WEBP_QUALITY = 80
}
}context는 여기서 주입 받아 줍니다잉 - 자세한 설명은 아티클로 낋여놓을게용
ImageUploadDataSourceImpl 은 그대로 사용하셔도 좋아요
ImageUploadRepositoryImpl
class ImageUploadRepositoryImpl @Inject constructor(
private val remoteDataSource: ImageUploadDataSource,
private val localDataSource: ImageLocalDataSource
) : ImageUploadRepository {
override suspend fun uploadImage(
uriString: String,
fileName: String,
contentType: String
): Result<String> = suspendRunCatching {
val optimizedFile = localDataSource.getOptimizedFile(uriString)
val requestBody = optimizedFile.asRequestBody(contentType.toMediaTypeOrNull())
val presignedResponse = remoteDataSource.postPresignedUrl(fileName, contentType).data
?: error("Presigned URL 발급 실패")
val response = remoteDataSource.uploadImageToS3(presignedResponse.presignedUrl, requestBody)
if (!response.isSuccessful) {
error("S3 업로드 실패: ${response.code()} - ${response.errorBody()?.string()}")
}
localDataSource.deleteOriginalUri(uriString)
optimizedFile.delete()
Timber.d("S3 업로드 성공: ${presignedResponse.fileName}")
presignedResponse.presignedUrl.substringBefore("?")
}
}usecase
class CompleteScheduleWithImageUseCase @Inject constructor(
private val imageUploadRepository: ImageUploadRepository,
private val scheduleRepository: ScheduleRepository
) {
suspend operator fun invoke(
uriString: String,
fileName: String,
contentType: String,
scheduleDetailId: Int
): Result<Unit> = suspendRunCatching {
val imageUrl = imageUploadRepository.uploadImage(
uriString = uriString,
fileName = fileName,
contentType = contentType
).getOrThrow()
scheduleRepository.patchScheduleComplete(
scheduleDetailId = scheduleDetailId,
imageUrl = imageUrl
).getOrThrow()
}
}이제 viewmodel에서는 usecase 하나만 사용하면 되겠죵?
참고해보시고 사용해보실 부분만 사용해주세요~ 고생많으셨습니다!!
app/src/main/java/com/kiero/core/common/util/ImageUriManager.kt
Outdated
Show resolved
Hide resolved
vvan2
left a comment
There was a problem hiding this comment.
대민성의 계층 구조 강의 이거 아주 와우띵킹이네요..
하나의 ViewModel에서 여러 역할을 책임지며 비대해진 현재 구조에서, useCase를 도입해 여러 repository의 조립과 책임을 명확히 구분하는 방향이 아주 좋아 보입니다!
#86
프로젝트 구조상 추가된 domain 계층에 적절하게 사용하면 될 것 같습니다.(당신을 위해 domain 계층을 만들었어)
서버에서 s3 업로드에 관한 변경사항은 서버 쌤들께 확정된 내용 물어보고 변동사항 있으면 수정하면 될 것 같습니다(노션 확인해봤는데 변경 내용은 없어 보여서 키하하),
대승재 고생했어요
dmp100
left a comment
There was a problem hiding this comment.
p3) 여기에 navigateToKid()가 빠져있는데, 이부분에대해선 스프린트에서 변경사항이있어서 빠진걸까요 ??
dmp100
left a comment
There was a problem hiding this comment.
수고 많으셨어용~~ 도메인 계층으로 리펙토링하는 단계까지 왔군요
저도 Repository나 ViewModel에서의 과도한 로직을 UseCase로 분리하는 방향으로 레어 책임을 확실하게 나눠볼게요 키하하
…into refactor/#87-today-journey-refactor
…into refactor/#87-today-journey-refactor # Conflicts: # app/src/main/java/com/kiero/presentation/kid/journey/KidJourneyScreen.kt
|
혹시 여유 되신다면 KieroButtonLarge <- 이거 kid journey에서만 사용된다면 이름 변경해주시고 아니라면 core/designsystem으로 옮겨주세요~ |
sonms
left a comment
There was a problem hiding this comment.
고생하셨습니다! 리뷰 남긴거만 한 번 봐주시고 나머지는 완전 굿굿입니다 ~! 역시 대승재
| val uriString = withContext(Dispatchers.IO) { | ||
| imageUriManager.createTempImageUri() | ||
| } | ||
|
|
||
| if (uriString != null) { | ||
| updateTempUri(uriString) | ||
| } |
There was a problem hiding this comment.
p2: viewmodel scope의 메인스레드에서 하지않고 io로 하는 거 아주 좋습니다!!
조오금만 리팩토링해서
suspend fun createTempImageUri(): String? = withContext(Dispatchers.IO)
으로 변경하는것은 어떨까요?
이렇게 하면 withcontext없이 모든 viewmodel에서 편히 사용할 수 있을 것 같아요~!
There was a problem hiding this comment.
viewModel 코드가 훨씬 깔끔해졌네요. 바로 반영했습니다! 😊
…내부 CoroutineContext 처리 로직 수정
|
해당 버튼은 KidFIreScreen에서만 사용되어서 이름 변경했습니다 |
ISSUE
❗ WORK DESCRIPTION
📢 TO REVIEWERS
ImageUploadRepositoryImpl내부에서 'Presigned URL 발급 -> 로직 처리(InputStream 등) -> S3 업로드 하는 로직으로 진행되고 있는디 네트워크 요청이 2번(발급 -> 업로드) 일어나는 비즈니스 흐름이라 Domain Layer로 추가되어야 할지 고민이에요(근데 서버에서 이 과정(s3 업로드)을 처리하도록 바뀔수도 있어서 모르겄음 그냥 냅둘까여)