[QA] 연락처에 등록된 이미지로 프로필 이미지 수정 #65
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package com.alarmy.near.local.contact | ||
|
|
||
| import android.content.ContentResolver | ||
| import android.net.Uri | ||
| import android.webkit.MimeTypeMap | ||
| import javax.inject.Inject | ||
|
|
||
| data class ContactImageData( | ||
| val fileName: String, | ||
| val contentType: String, | ||
| val fileSize: Int, | ||
| val data: ByteArray, | ||
|
Contributor
There was a problem hiding this comment. byteArray가 값이 다르면 다른 객체로 인식해서 equals 및 hashCode를 재정의하라는 경고가 뜨네요! 해결 방법으로 option+ enter로 해당 코드를 자동으로 추가할 수 있습니다! 코드가 복잡해진다면 필요한 곳에서 byteArray를 맵핑해주는 방법이 있을 것 같아요!
Contributor
Author
There was a problem hiding this comment. 감사합니다! equals, hashCode를 재정의 하는 코드가 크게 복잡하지 않아 우선 재정의 하는 방법으로 개선해보았습니다! |
||
| ) | ||
stopstone marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| class ContactImageReader | ||
| @Inject | ||
| constructor( | ||
| private val contentResolver: ContentResolver, | ||
| ) { | ||
| fun read(uriString: String): ContactImageData? { | ||
| val uri = runCatching { Uri.parse(uriString) }.getOrNull() ?: return null | ||
|
||
| val bytes = contentResolver.openInputStream(uri)?.use { inputStream -> inputStream.readBytes() } ?: return null | ||
| val resolvedMimeType = contentResolver.getType(uri) ?: guessMimeType(uriString) ?: DEFAULT_MIME_TYPE | ||
| val extension = MimeTypeMap.getSingleton().getExtensionFromMimeType(resolvedMimeType) ?: DEFAULT_EXTENSION | ||
| val fileName = "contact_${System.currentTimeMillis()}.$extension" | ||
| return ContactImageData( | ||
| fileName = fileName, | ||
| contentType = resolvedMimeType, | ||
| fileSize = bytes.size, | ||
| data = bytes, | ||
| ) | ||
| } | ||
|
|
||
| private fun guessMimeType(uriString: String): String? { | ||
| val lowerCase = uriString.lowercase() | ||
| return when { | ||
| lowerCase.endsWith(".png") -> "image/png" | ||
| lowerCase.endsWith(".webp") -> "image/webp" | ||
| lowerCase.endsWith(".jpg") || lowerCase.endsWith(".jpeg") -> "image/jpeg" | ||
| else -> null | ||
| } | ||
| } | ||
|
|
||
| companion object { | ||
| private const val DEFAULT_MIME_TYPE = "image/jpeg" | ||
| private const val DEFAULT_EXTENSION = "jpg" | ||
| } | ||
| } | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||||||||||||
| package com.alarmy.near.network.uploader | ||||||||||||||||
|
|
||||||||||||||||
| import kotlinx.coroutines.Dispatchers | ||||||||||||||||
| import kotlinx.coroutines.withContext | ||||||||||||||||
| import okhttp3.MediaType.Companion.toMediaTypeOrNull | ||||||||||||||||
| import okhttp3.OkHttpClient | ||||||||||||||||
| import okhttp3.Request | ||||||||||||||||
| import okhttp3.RequestBody.Companion.toRequestBody | ||||||||||||||||
| import javax.inject.Inject | ||||||||||||||||
| import javax.inject.Singleton | ||||||||||||||||
|
|
||||||||||||||||
| @Singleton | ||||||||||||||||
| class ImageUploader | ||||||||||||||||
| @Inject | ||||||||||||||||
| constructor() { | ||||||||||||||||
| private val okHttpClient = OkHttpClient() | ||||||||||||||||
|
||||||||||||||||
| class ImageUploader | |
| @Inject | |
| constructor() { | |
| private val okHttpClient = OkHttpClient() | |
| class ImageUploader | |
| @Inject | |
| constructor(private val okHttpClient: OkHttpClient) { |
Contributor
There was a problem hiding this comment.
사소한 것인데
response.isSuccessful.not()으로 가독성을 올릴 수 있다고 생각해요!
반영은 안하셔도 됩니다!
Contributor
Author
There was a problem hiding this comment.
디테일한 코멘트 감사합니다!
궁금한 것이 .not() 함수를 사용하는 것이 가독성이 더 좋아보이는 편인가요?
저는 오히려 코드가 길어질 수 있다고 생각하여 ! 연산자를 활용하는 편인데 어떻게 생각하시나요?
Contributor
There was a problem hiding this comment.
개인 취향인데, 저는 자연어처럼 코드가 읽히는 것을 선호해서 not을 꽤 쓰는 편인 것 같아요!
함수명이 길어지는 것, 복잡한 필드 선언시에 타입을 명시하는 것처럼 가독성 이점의 맥락으로 쓰고 있습니다!
There was a problem hiding this comment.
이미지 업로드 실패 시 던지는 IllegalStateException에 더 자세한 오류 정보를 포함하면 디버깅에 도움이 됩니다. HTTP 응답 코드와 메시지를 예외 메시지에 추가하는 것을 고려해보세요.
Suggested change
| throw IllegalStateException("이미지 업로드에 실패했습니다.") | |
| throw IllegalStateException("이미지 업로드에 실패했습니다. Code: ${response.code}, Message: ${response.message}") |
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 이미지 업로드는
forEachIndexed루프 내에서 순차적으로 실행됩니다. 연락처에 이미지가 있는 친구가 많을 경우, 모든 이미지가 하나씩 업로드될 때까지 기다려야 하므로 전체 프로세스가 느려질 수 있습니다.coroutineScope와launch를 사용하여 이미지 업로드를 병렬로 처리하면 성능을 개선할 수 있습니다. 이렇게 하면 여러 이미지를 동시에 업로드하여 전체 대기 시간을 줄일 수 있습니다. 만약 업로드 중 하나라도 실패하면coroutineScope가 나머지 업로드를 취소하고 예외를 전파하므로, 기존의 동작 방식도 유지됩니다.(참고:
kotlinx.coroutines.coroutineScope와kotlinx.coroutines.launch를 import해야 합니다.)coroutineScope { response.friendList.forEachIndexed { index, entity -> val uploadUrl = entity.preSignedImageUrl val imageData = payloads.getOrNull(index)?.imageData if (uploadUrl != null && imageData != null) { launch { imageUploader.upload( url = uploadUrl, contentType = imageData.contentType, data = imageData.data, ) } } } }