[feat] #77 포즈 API 연동 및 랜덤 포즈 추천 구현#82
Conversation
…ount`를 초기화하도록 변경
포즈피드 랜덤 API 호출 시, 불필요하게 `ImageLoader`를 통해 이미지를 미리 캐싱하는 로직을 제거했습니다.
랜덤 포즈 API를 호출하는 `fetchRandomPose` 함수의 반환 타입을 `FetchPoseResult` 실드 클래스로 변경하여, API 호출 결과를 성공, 중복, 실패 세 가지 케이스로 더 명확하게 구분하도록 리팩토링했습니다. - API 호출 실패 시, 실패 토스트 메시지를 띄우고 에러를 로깅합니다. - 마지막 포즈까지 모두 탐색했을 경우, "모든 포즈를 불러왔어요" 토스트 메시지를 표시합니다. - 중복 포즈가 발생했을 때는 로깅만 하고 다음 로직을 수행하도록 변경했습니다. - 마지막 인덱스 계산 로직의 오류를 수정했습니다.
Walkthrough스크랩 전용 페이징 소스·API·리포지토리 플로우 추가, 포즈 상세에서 스크랩 변경을 이벤트 버스로 방출해 메인에 반영, 랜덤 포즈 UI를 Pager 기반으로 리팩터링하고 프리페치·초기 로드 로직을 중앙화함. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant DetailScreen as PoseDetailScreen
participant DetailVM as PoseDetailViewModel
participant EventBus as LocalResultEventBus
participant PoseMain as PoseMainEntry
participant MainVM as PoseViewModel
User->>DetailScreen: 스크랩 토글
DetailScreen->>DetailVM: ScrapCommitted 인텐트
DetailVM->>DetailVM: committedScrap 갱신
DetailVM->>DetailScreen: 사이드이펙트 NotifyScrapChanged(poseId,isScrapped)
DetailScreen->>EventBus: emit PoseResult.ScrapChanged(poseId,isScrapped)
EventBus->>PoseMain: 전달
PoseMain->>MainVM: PoseIntent.ScrapChanged(poseId,isScrapped)
MainVM->>MainVM: updatedScraps 갱신 → posePagingData 반영
MainVM->>PoseMain: UI 갱신
sequenceDiagram
participant User as 사용자
participant RandomScreen as RandomPoseScreen
participant RandomVM as RandomPoseViewModel
participant Service as PoseService
participant State as PoseListState
User->>RandomScreen: 좌/우 클릭
RandomScreen->>RandomVM: MovePrevious/MoveNext intent
RandomVM->>RandomVM: currentIndex 업데이트
RandomVM->>RandomScreen: emit SwipePoseImage(newIndex)
RandomScreen->>RandomScreen: PagerState 애니메이션 이동
alt 남은 아이템 < POSE_PREFETCH_THRESHOLD
RandomVM->>Service: getRandomPose(...) (fetchRandomPose)
Service-->>RandomVM: Pose / 중복 / 오류
RandomVM->>State: poseList 추가 또는 중복/오류 처리
RandomVM->>RandomScreen: UI 상태 갱신
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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 |
ikseong00
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailViewModel.kt (1)
35-40:⚠️ Potential issue | 🟠 Major
store자기참조로 인한 초기화 순서 위험
initialFetchData람다가store변수를 캡처하고 있으나, Kotlin에서는val초기화 중에 해당 변수를 참조할 수 없습니다.mviIntentStore함수가initialFetchData를 초기화 시점에 즉시 실행할 경우,store할당이 완료되지 않은 상태에서 접근하게 되어 초기화되지 않은 값을 참조할 수 있습니다.
store생성 이후init블록에서 인텐트를 호출하는 것이 안전합니다.수정 제안
val store: MviIntentStore<PoseDetailState, PoseDetailIntent, PoseDetailSideEffect> = mviIntentStore( initialState = PoseDetailState(), onIntent = ::onIntent, - initialFetchData = { store.onIntent(PoseDetailIntent.EnterPoseDetailScreen) }, ) init { + store.onIntent(PoseDetailIntent.EnterPoseDetailScreen) viewModelScope.launch { scrapRequests .debounce(500)
🤖 Fix all issues with AI agents
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/component/RandomPoseImagePager.kt`:
- Around line 36-46: The pager currently accesses poseList[index] directly in
the HorizontalPager lambda (index, pagerState, RandomPoseImage), which can OOB
if pageCount and poseList.size disagree; add a defensive bound check before
using index: ensure poseList is non-empty and compute a safeIndex (e.g., clamp
index into poseList.indices via coerceIn or check index in poseList.indices) and
use that when calling RandomPoseImage, and if poseList is empty render a safe
fallback (empty state or placeholder) instead of accessing poseList[...].
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt`:
- Around line 152-172: Multiple concurrent prefetches can add duplicate poses
because fetchRandomPose uses the snapshot state.poseList but the list can change
before the API returns; to fix, introduce a prefetch guard (e.g., an
atomic/volatile flag like isPrefetching) in RandomPoseViewModel and check/set it
around the viewModelScope.launch to prevent overlapping launches, and
additionally, before applying the Success branch that updates
poseList/committedScraps, re-check the latest state.poseList (by reading current
state or using the reducer's current value) to ensure result.pose.id isn't
already present and skip adding if it is; clear the flag in all result branches
(Success/Duplicated/Failure) so subsequent prefetches can proceed.
🧹 Nitpick comments (5)
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseScreen.kt (1)
72-76:derivedStateOf사용 시 키 누락 또는 불필요한 래핑 가능성
remember블록이posePagingItems를 키로 사용하지 않아, 만약posePagingItems인스턴스가 변경되면 이전 참조가 유지될 수 있습니다. Paging 3의loadState와itemCount는 이미 snapshot state이므로, 아래 두 가지 방법 중 하나를 고려해 보세요:
- 키 추가:
remember(posePagingItems) { derivedStateOf { ... } }- 간소화: snapshot state 읽기가 자동으로 recomposition을 트리거하므로
derivedStateOf없이 직접 계산♻️ 옵션 2 - 간소화된 구현 제안
- val isRefreshing by remember { - derivedStateOf { - posePagingItems.loadState.refresh is LoadState.Loading && posePagingItems.itemCount == 0 - } - } + val isRefreshing = posePagingItems.loadState.refresh is LoadState.Loading && posePagingItems.itemCount == 0core/data/src/main/java/com/neki/android/core/data/paging/ScrapPosePagingSource.kt (1)
24-28: 마지막 페이지 판정이 너무 느슨합니다.현재는
poses.isEmpty()만 검사해 부분 페이지에서도 다음 요청이 발생할 수 있습니다. 서버가 마지막 페이지를 “부분 반환”하는 경우 불필요한 호출이 생깁니다.🛠️ 개선 제안
- nextKey = if (poses.isEmpty()) null else page + 1, + nextKey = if (poses.size < params.loadSize) null else page + 1,feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseViewModel.kt (2)
49-58:posePagingData에cachedIn()누락
originalPagingData는cachedIn(viewModelScope)을 사용하지만,posePagingData는cachedIn()을 사용하지 않습니다. 이로 인해 여러 collector가 구독할 경우combine블록이 중복 실행되어 불필요한 매핑 연산이 발생할 수 있습니다.♻️ 제안하는 수정 사항
val posePagingData: Flow<PagingData<Pose>> = combine( originalPagingData, updatedScraps, ) { pagingData, scraps -> pagingData.map { pose -> scraps[pose.id]?.let { isScrapped -> pose.copy(isScrapped = isScrapped) } ?: pose } - } + }.cachedIn(viewModelScope)
130-139: 불필요한takeIf조건Line 135의
takeIf { it != state.selectedPeopleCount }는 항상 true입니다. 이 코드는else분기 내에 있으므로,intent.peopleCount != state.selectedPeopleCount조건이 이미 충족된 상태입니다. 따라서takeIf는 제거하고intent.peopleCount를 직접 사용하면 됩니다.♻️ 제안하는 수정 사항
} else { _headCountFilter.value = intent.peopleCount reduce { copy( isShowScrappedPose = false, - selectedPeopleCount = intent.peopleCount.takeIf { it != state.selectedPeopleCount }, + selectedPeopleCount = intent.peopleCount, isShowPeopleCountBottomSheet = false, ) } }feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt (1)
183-208: 초기 로드 시 fallback 카운트 누적 로직 확인
totalFallbackCount를 누적하여 전체 재시도 횟수를 제한하는 방식은 좋습니다. 다만,fetchRandomPose호출 시maxFallbackCount로 남은 횟수를 전달하고, 결과의tryCount를 다시 누적하는 로직이 약간 복잡합니다.예를 들어,
MAXIMUM_RANDOM_POSE_FALLBACK_COUNT가 10이고 첫 번째 호출에서 3번 시도 후 성공하면, 두 번째 호출의maxFallbackCount는 7이 됩니다. 이 방식은 의도대로 동작하지만, 가독성을 위해 간단한 주석을 추가하는 것을 고려해 주세요.
포즈 이미지 상단에 어두워지는 그라데이션 효과를 추가하여, 나중에 추가될 태그 UI가 더 잘 보이도록 개선했습니다.
- 포즈피드 각 아이템에 스크랩된 경우, 우측 상단에 스크랩 아이콘을 표시하도록 변경했습니다. - 기존에 사용되던 `ic_scrap_selected` 아이콘을 `icon_scrap`으로 이름을 변경하여 재사용했습니다. - 포즈 상세, 랜덤 포즈 화면에서 `ic_scrap_selected` 대신 `icon_scrap` 아이콘을 사용하도록 수정했습니다.
포즈피드, 랜덤포즈 등 포즈 상세 화면 진입 시, 초기 스크랩 상태가 항상 `false`로 지정되어 스크랩 여부와 관계 없이 빈 하트로 표시되는 문제가 있었습니다. API 응답(`PoseResponse`)을 `Pose` 도메인 모델로 변환할 때, `isScrapped` 필드의 기본값을 `true`로 설정하여 이 문제를 해결했습니다.
포즈피드 상세화면의 상단 그라데이션의 알파 값을 0.4f에서 0.2f로 조정했습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@core/designsystem/src/main/java/com/neki/android/core/designsystem/modifier/Background.kt`:
- Line 7: Remove the unused import androidx.compose.ui.draw.alpha from the file:
the symbol alpha is not referenced anywhere in Background.kt, so delete that
import line (or confirm and add intended usage if you actually meant to use
alpha in the Background composable/function) to eliminate the unused-import
warning.
🧹 Nitpick comments (4)
core/data/src/main/java/com/neki/android/core/data/remote/model/response/PoseResponse.kt (1)
32-54:PoseResponse.Item과ScrappedPoseResponse.Item간 중복 코드 고려두
Item클래스의 필드 5개와toModel()매핑 로직이isScrapped값만 다르고 완전히 동일합니다. 현재 상태에서 동작에 문제는 없지만, 향후 필드 추가/변경 시 양쪽을 동기화해야 하는 부담이 생길 수 있습니다.공통
Item을 최상위로 추출하고toModel(isScrapped: Boolean)파라미터로 분기하는 방식을 고려해 볼 수 있습니다.♻️ 리팩터링 제안
+@Serializable +data class PoseItem( + `@SerialName`("poseId") val poseId: Long, + `@SerialName`("headCount") val headCount: String, + `@SerialName`("imageUrl") val imageUrl: String, + `@SerialName`("contentType") val contentType: String, + `@SerialName`("createdAt") val createdAt: String, +) { + internal fun toModel(isScrapped: Boolean) = Pose( + id = poseId, + poseImageUrl = imageUrl, + peopleCount = PeopleCount.entries.find { it.name == headCount }?.value ?: 1, + isScrapped = isScrapped, + ) +} + `@Serializable` data class PoseResponse( `@SerialName`("hasNext") val hasNext: Boolean, - `@SerialName`("items") val items: List<Item>, + `@SerialName`("items") val items: List<PoseItem>, ) { - `@Serializable` - data class Item( ... ) { ... } - fun toModels() = items.map { it.toModel() } + fun toModels() = items.map { it.toModel(isScrapped = false) } } `@Serializable` data class ScrappedPoseResponse( `@SerialName`("hasNext") val hasNext: Boolean, - `@SerialName`("items") val items: List<Item>, + `@SerialName`("items") val items: List<PoseItem>, ) { - `@Serializable` - data class Item( ... ) { ... } - fun toModels() = items.map { it.toModel() } + fun toModels() = items.map { it.toModel(isScrapped = true) } }core/designsystem/src/main/java/com/neki/android/core/designsystem/modifier/Background.kt (1)
43-53:poseBackground구현이 기존photoBackground패턴과 일관되게 잘 작성되었습니다.한 가지 사소한 제안:
134f / 242f매직 넘버에 대해 디자인 스펙 출처를 간단한 인라인 주석으로 남겨두면 추후 유지보수 시 의도 파악이 수월합니다.💡 선택적 개선 제안
colorStops = arrayOf( 0f to Color.Black.copy(alpha = 0.2f), - 134f / 242f to Color.Black.copy(alpha = 0f), + 134f / 242f to Color.Black.copy(alpha = 0f), // 디자인 기준: 포즈 카드 높이 242dp 중 상단 134dp까지 그라데이션 ),feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailScreen.kt (1)
40-53:resultEventBus캡처 참조 안정성 확인 필요
resultEventBus는 Line 40에서CompositionLocal로부터 가져온 뒤,collectWithLifecycle람다(Line 48-53) 안에서 클로저로 캡처됩니다. 만약LocalResultEventBus가 리컴포지션 중에 다른 인스턴스를 제공하게 되면, 람다가 이전(stale) 참조를 들고 있을 수 있습니다.현재
LocalResultEventBus가 상위 컴포지션 트리에서 안정적인 싱글턴으로 제공되고 있다면 문제 없지만, 방어적으로rememberUpdatedState를 사용하는 것도 고려해 볼 수 있습니다.♻️ rememberUpdatedState 적용 예시
+ import androidx.compose.runtime.rememberUpdatedState ... val resultEventBus = LocalResultEventBus.current + val currentResultEventBus by rememberUpdatedState(resultEventBus) viewModel.store.sideEffects.collectWithLifecycle { sideEffect -> when (sideEffect) { ... is PoseDetailSideEffect.NotifyScrapChanged -> { - resultEventBus.sendResult( + currentResultEventBus.sendResult( result = PoseResult.ScrapChanged(sideEffect.poseId, sideEffect.isScrapped), allowDuplicate = false, ) } } }feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/component/RandomPoseFloatingBar.kt (1)
184-194:@Previewvs@ComponentPreview어노테이션 불일치.다른 프리뷰 함수들은 모두
@ComponentPreview를 사용하는데,RandomPoseFloatingBarPreview만 표준@Preview를 사용하고 있습니다. 의도적인 차이가 아니라면@ComponentPreview로 통일하는 것이 좋겠습니다.♻️ 수정 제안
-@Preview +@ComponentPreview `@Composable` private fun RandomPoseFloatingBarPreview() {
Ojongseok
left a comment
Ojongseok
left a comment
There was a problem hiding this comment.
현재 랜덤 포즈 조회에서 중복된 포즈가 내려올 경우 클라이언트에서 처리하고 있습니다. 중복일 경우 새로운 랜덤 포즈 조회 API를 호출해야 하는데, 과도한 API 호출을 방지하기 위해 PoseConst.MAXIMUM_RANDOM_POSE_FALLBACK_COUNT를 선언하여 임계값 이상 시도 시 중단하도록 했습니다. (랜덤 포즈 튜플 수와 poseList.size가 동일하거나 거의 유사한 경우)
Q. 현재 임의로 7로 설정했는데 이 정도 괜찮을까요?
추가로 코멘트 남겨주신 것처럼 POSE_PREFETCH_THRESHOLD를 2 → 3으로 증가시켰습니다.
코드를 보면서 문득 든 생각인데 과도한 API 호출을 방지하기 위해 중복 횟수를 체크하며 조회 성공/실패를 구분하는 로직을 ViewModel에서 while문을 돌려 요청 횟수를 카운트하는 것 보다 관련 로직을 PoseRepositoryImpl에서 작성하는게 적절하지 않을까? 라는 생각이 들었습니다.
// PoseRepositoryImpl
override suspend fun getRandomPose(
headCount: PeopleCount,
excludeIds: Set<Long>,
maxRetry: Int,
): Result<Pose> = runSuspendCatching {
repeat(maxRetry) { attempt ->
val pose = poseService.getRandomPose(headCount = headCount.name).data.toModel()
if (pose.id !in excludeIds) {
return@runSuspendCatching pose
}
}
// 저희가 커스텀으로 정의한 Exception 발생
throw PoseException.DuplicatePoseException("포즈를 불러오는데 실패했어요")
}
// RnadomPoseViewModel
fetchRandomPose(poses)
.onSuccess { pose -> poses.add(pose) }
.onFailure { error ->
PoseException.DuplicatePoseException -> {
Timber.d("초기 로드: ${result.tryCount}회 시도 후 중복 포즈")
}
}
약간 이런 느낌으로 ViewModel에서는 RepositoryImpl로 조회된 포즈 id 목록을 전달하고 Impl에서 중복포즈에 대한 확인 후 중복 시 저희가 정의한 CustomException, 성공 시 이어서 진행. 이렇게 진행하면 ViewModel이 조금 깔끔해 질 것 같고, 해당 과정을 비즈니스 로직으로 본다면 RepositoryImpl에 있는게 괜찮아 보이더라구요.
한 번 고민해봐주시면 감사드리겠습니다. 임의로 지정하신 7회는 문제 없어 보이고, db에 포즈가 점점 많아지게 되면 조금씩 줄여나가면 좋을 것 같습니다! 어느정도 쌓이고 나면 2~3회로도 괜찮지 않을까 싶네요
HorizontalPager에서 자체적으로 특정 갯수만큼 좌우로 미리 컴포지션 할 수 있는 기능으르 제공하는군요..? 그럼 굳이 캐싱할 필요가 없겠네요! 좋습니다.
ViewModel 의 코드가 깔끔해지는 것 같습니다! |
2 participants
🔗 관련 이슈
📙 작업 설명
📸 스크린샷 또는 시연 영상 (선택)
default.mp4
default.mp4
💬 추가 설명 or 리뷰 포인트 (선택)
ImageRequest를 통해 이미지를 미리 캐싱하는 방식에서,HorizontalPager의beyondViewportPageCount = PoseConst.POSE_PREFETCH_THRESHOLD를 활용하여 뷰포트 밖의 페이지를 미리 컴포지션하는 방식으로 변경했습니다.PoseConst.MAXIMUM_RANDOM_POSE_FALLBACK_COUNT를 선언하여 임계값 이상 시도 시 중단하도록 했습니다. (랜덤 포즈 튜플 수와poseList.size가 동일하거나 거의 유사한 경우)POSE_PREFETCH_THRESHOLD를 2 → 3으로 증가시켰습니다.Summary by CodeRabbit
새로운 기능
개선 사항
변경 사항