-
Notifications
You must be signed in to change notification settings - Fork 0
[QA] 연락처에 등록된 이미지로 프로필 이미지 수정 #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- 연락처의 프로필 이미지를 가져와 Presigned URL을 통해 업로드하는 로직을 구현했습니다. - `ContactImageReader`를 추가하여 연락처 이미지 URI로부터 이미지 데이터를 읽어옵니다. - `ImageUploader`를 추가하여 OkHttp를 사용해 이미지를 업로드합니다. - 친구 목록 및 홈 화면에서 `ImageLoader`를 사용하여 프로필 이미지를 표시하도록 수정했습니다.
- `ImageLoader`를 사용하여 친구의 프로필 이미지를 동적으로 로드하도록 수정했습니다. - 이미지 로딩 실패 시 표시할 플레이스홀더와 에러 이미지를 설정했습니다. - 프로필 이미지를 원형으로 표시하기 위해 `clip(CircleShape)`를 적용했습니다.
Summary of ChangesHello @stopstone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 연락처에 등록된 친구의 프로필 이미지를 앱 내에서 관리하고 표시할 수 있도록 기능을 확장합니다. 기존에는 기본 플레이스홀더 이미지만 사용했지만, 이제 사용자가 연락처에서 가져온 실제 프로필 이미지를 서버에 업로드하고 앱 전반에 걸쳐 해당 이미지를 표시할 수 있게 됩니다. 이는 사용자 경험을 향상시키고 친구 목록 및 상세 화면의 시각적 풍부함을 더합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
이 PR은 연락처 이미지를 프로필 사진으로 업로드하는 기능을 도입합니다. ContactImageReader를 추가하여 연락처에서 이미지 데이터를 가져오고, ImageUploader를 통해 presigned URL로 업로드합니다. 관련 로직은 주로 DefaultFriendRepository에 구현되었으며, UI는 새로운 ImageLoader 컴포저블을 사용하여 이미지를 표시하도록 업데이트되었습니다. 전반적으로 구조가 잘 잡혀있습니다. 성능, 안정성 및 리소스 관리를 개선하기 위한 몇 가지 제안 사항이 있습니다.
| class ImageUploader | ||
| @Inject | ||
| constructor() { | ||
| private val okHttpClient = OkHttpClient() |
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.
ImageUploader 내에서 OkHttpClient를 직접 생성하고 있습니다. 앱 전체에서 OkHttpClient 인스턴스를 공유하면 커넥션 풀링, 캐시, 타임아웃 설정 등 리소스를 효율적으로 관리할 수 있습니다. Hilt를 통해 이미 설정된 OkHttpClient를 주입받아 사용하는 것이 좋습니다. 이렇게 하면 네트워크 관련 설정을 한 곳에서 관리할 수 있고, ImageUploader가 앱의 다른 네트워크 요청과 동일한 설정을 공유하게 됩니다.
| class ImageUploader | |
| @Inject | |
| constructor() { | |
| private val okHttpClient = OkHttpClient() | |
| class ImageUploader | |
| @Inject | |
| constructor(private val okHttpClient: OkHttpClient) { |
| response.friendList.forEachIndexed { index, entity -> | ||
| val uploadUrl = entity.preSignedImageUrl | ||
| val imageData = payloads.getOrNull(index)?.imageData | ||
| if (uploadUrl != null && imageData != null) { | ||
| imageUploader.upload( | ||
| url = uploadUrl, | ||
| contentType = imageData.contentType, | ||
| data = imageData.data, | ||
| ) | ||
| } | ||
| } |
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,
)
}
}
}
}
Near/app/src/main/java/com/alarmy/near/local/contact/ContactImageReader.kt
Outdated
Show resolved
Hide resolved
| .build() | ||
| okHttpClient.newCall(request).execute().use { response -> | ||
| if (!response.isSuccessful) { | ||
| throw IllegalStateException("이미지 업로드에 실패했습니다.") |
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.
- `initFriends` 함수 내에서 친구 프로필 이미지를 업로드할 때, `coroutineScope`와 `launch`를 사용하여 각 이미지를 병렬로 업로드하도록 수정했습니다. - 이를 통해 여러 친구를 한 번에 추가할 때 이미지 업로드 시간을 단축하고 성능을 개선했습니다.
rhkrwngud445
left a comment
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.
번거로운 작업이었을 텐데 고생하셨습니다!
IDE 경고 문구 위주로 코멘트 남겼는데 확인 부탁드려요!
| private val contentResolver: ContentResolver, | ||
| ) { | ||
| fun read(uriString: String): ContactImageData? { | ||
| val uri = runCatching { Uri.parse(uriString) }.getOrNull() ?: return null |
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.
요거 제 환경에서는 아래처럼 바꿔라고 경고가 뜨는데, 지석님은 혹시 해당 현상 없으셨을까요?
uriString.toUri() | val fileName: String, | ||
| val contentType: String, | ||
| val fileSize: Int, | ||
| val data: ByteArray, |
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.
byteArray가 값이 다르면 다른 객체로 인식해서 equals 및 hashCode를 재정의하라는 경고가 뜨네요!
해결 방법으로 option+ enter로 해당 코드를 자동으로 추가할 수 있습니다! 코드가 복잡해진다면 필요한 곳에서 byteArray를 맵핑해주는 방법이 있을 것 같아요!
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.
감사합니다! equals, hashCode를 재정의 하는 코드가 크게 복잡하지 않아 우선 재정의 하는 방법으로 개선해보았습니다!
| .put(requestBody) | ||
| .build() | ||
| okHttpClient.newCall(request).execute().use { response -> | ||
| if (!response.isSuccessful) { |
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.
사소한 것인데
response.isSuccessful.not()으로 가독성을 올릴 수 있다고 생각해요!
반영은 안하셔도 됩니다!
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.
디테일한 코멘트 감사합니다!
궁금한 것이 .not() 함수를 사용하는 것이 가독성이 더 좋아보이는 편인가요?
저는 오히려 코드가 길어질 수 있다고 생각하여 ! 연산자를 활용하는 편인데 어떻게 생각하시나요?
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.
개인 취향인데, 저는 자연어처럼 코드가 읽히는 것을 선호해서 not을 꽤 쓰는 편인 것 같아요!
함수명이 길어지는 것, 복잡한 필드 선언시에 타입을 명시하는 것처럼 가독성 이점의 맥락으로 쓰고 있습니다!
`ContactImageData` 데이터 클래스에 `equals`와 `hashCode` 메서드를 오버라이드하여 객체 비교가 올바르게 동작하도록 수정했습니다.
작업 내용
확인 방법
연락처에서 사진이 있는 친구를 선택해 주기 설정 완료 →
서버 초기화 API 호출이 성공하고, 로그에서 presigned 업로드가 200으로 떨어지는지 확인
홈 화면, 친구 상세 화면에서 방금 등록한 친구의 실제 프로필 이미지가 표시되는지 확인
이미지가 없는 친구는 예전처럼 회색 기본 이미지가 나오는지 확인