Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.neki.android.core.common.exception

class RandomPoseRetryExhaustedException(
message: String,
) : RuntimeException(message)
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,18 @@ interface PoseRepository {

suspend fun getPose(poseId: Long): Result<Pose>

suspend fun getRandomPose(headCount: PeopleCount): Result<Pose>
suspend fun getSingleRandomPose(
headCount: PeopleCount,
excludeIds: Set<Long>,
maxRetry: Int,
): Result<Pose>

suspend fun getMultipleRandomPose(
headCount: PeopleCount,
excludeIds: Set<Long>,
poseSize: Int,
maxRetry: Int,
): Result<List<Pose>>

suspend fun updateScrap(poseId: Long, scrap: Boolean): Result<Unit>
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.neki.android.core.data.repository.impl
import androidx.paging.Pager
import androidx.paging.PagingConfig
import androidx.paging.PagingData
import com.neki.android.core.common.exception.RandomPoseRetryExhaustedException
import com.neki.android.core.data.paging.PosePagingSource
import com.neki.android.core.data.paging.ScrapPosePagingSource
import com.neki.android.core.data.remote.api.PoseService
Expand Down Expand Up @@ -65,8 +66,41 @@ class PoseRepositoryImpl @Inject constructor(
poseService.getPose(poseId).data.toModel()
}

override suspend fun getRandomPose(headCount: PeopleCount): Result<Pose> = runSuspendCatching {
poseService.getRandomPose(headCount = headCount.name).data.toModel()
override suspend fun getSingleRandomPose(
headCount: PeopleCount,
excludeIds: Set<Long>,
maxRetry: Int,
): Result<Pose> = runSuspendCatching {
repeat(maxRetry) {
val pose = poseService.getRandomPose(headCount = headCount.name).data.toModel()
if (pose.id !in excludeIds) {
return@runSuspendCatching pose
}
}
throw RandomPoseRetryExhaustedException("새로운 포즈를 찾지 못했어요")
}

override suspend fun getMultipleRandomPose(
headCount: PeopleCount,
excludeIds: Set<Long>,
poseSize: Int,
maxRetry: Int,
): Result<List<Pose>> = runSuspendCatching {
val result = mutableListOf<Pose>()
val collectedIds = excludeIds.toMutableSet()
var retryCount = 0

while (result.size < poseSize && retryCount < maxRetry) {
val pose = poseService.getRandomPose(headCount = headCount.name).data.toModel()
if (pose.id !in collectedIds) {
result.add(pose)
collectedIds.add(pose.id)
} else {
retryCount++
}
}

result.ifEmpty { throw RandomPoseRetryExhaustedException("새로운 포즈를 찾지 못했어요") }
}
Comment on lines +83 to 104
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

getMultipleRandomPoseposeSize보다 적은 결과를 조용히 반환할 수 있습니다.

retryCountmaxRetry에 도달하면 while 루프가 종료되지만, result가 비어있지 않으면 (1개 이상) 예외 없이 부분 결과를 반환합니다. 예를 들어 poseSize=4, maxRetry=7일 때 2개만 수집하고 7회 중복이 발생하면 2개짜리 리스트가 반환됩니다.

호출부(fetchInitialPoses)에서는 반환된 리스트를 그대로 poseList에 설정하므로, 요청한 개수보다 적은 포즈가 표시될 수 있습니다.

의도된 동작이라면 주석으로 명시하고, 그렇지 않다면 result.size < poseSize일 때도 예외를 던지거나 호출부에서 처리하는 것을 권장합니다.

💡 부분 결과도 예외로 처리하는 경우
-        result.ifEmpty { throw RandomPoseRetryExhaustedException("새로운 포즈를 찾지 못했어요") }
+        if (result.size < poseSize) {
+            throw RandomPoseRetryExhaustedException("요청한 ${poseSize}개 중 ${result.size}개만 찾았어요")
+        }
+        result
🤖 Prompt for AI Agents
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/PoseRepositoryImpl.kt`
around lines 83 - 104, getMultipleRandomPose currently returns a partial list
when maxRetry is reached before collecting poseSize items; update the method
(getMultipleRandomPose) to treat partial results as failure by throwing
RandomPoseRetryExhaustedException when result.size < poseSize (not just when
empty), or alternatively add a clear comment and change the caller
(fetchInitialPoses) to handle partial lists—prefer throwing the same
RandomPoseRetryExhaustedException with a descriptive message so callers always
receive either a full list of size poseSize or an exception.


override suspend fun updateScrap(poseId: Long, scrap: Boolean): Result<Unit> = runSuspendCatching {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package com.neki.android.feature.pose.impl.const
internal object PoseConst {
internal const val INITIAL_POSE_LOAD_COUNT = 4
internal const val POSE_PREFETCH_THRESHOLD = 3
internal const val MAXIMUM_RANDOM_POSE_FALLBACK_COUNT = 7
internal const val MAXIMUM_RANDOM_POSE_RETRY_COUNT = 7

internal const val POSE_LAYOUT_DEFAULT_TOP_PADDING = 12
internal const val POSE_LAYOUT_BOTTOM_PADDING = 28
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,9 @@ data class RandomPoseUiState(

val hasPrevious: Boolean
get() = currentIndex > 0
}

internal sealed class FetchPoseResult(val tryCount: Int) {
class Success(tryCount: Int, val pose: Pose) : FetchPoseResult(tryCount)
class Duplicated(tryCount: Int) : FetchPoseResult(tryCount)
class Failure(tryCount: Int, val throwable: Throwable) : FetchPoseResult(tryCount)
val randomPoseIds: Set<Long>
get() = poseList.map { it.id }.toSet()
}

sealed interface RandomPoseIntent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ package com.neki.android.feature.pose.impl.random
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.neki.android.core.common.coroutine.di.ApplicationScope
import com.neki.android.core.common.exception.RandomPoseRetryExhaustedException
import com.neki.android.core.dataapi.repository.PoseRepository
import com.neki.android.core.model.PeopleCount
import com.neki.android.core.model.Pose
import com.neki.android.core.ui.MviIntentStore
import com.neki.android.core.ui.mviIntentStore
import com.neki.android.feature.pose.impl.const.PoseConst
Expand Down Expand Up @@ -152,21 +152,16 @@ internal class RandomPoseViewModel @AssistedInject constructor(
// 여분 포즈가 POSE_PREFETCH_THRESHOLD 이하이면 다음 포즈 미리 캐싱
if (state.poseList.lastIndex - nextIndex < PoseConst.POSE_PREFETCH_THRESHOLD) {
viewModelScope.launch {
when (val result = fetchRandomPose(poseList = state.poseList)) {
is FetchPoseResult.Success -> reduce {
copy(
poseList = (poseList + result.pose).toImmutableList(),
committedScraps = committedScraps + (result.pose.id to result.pose.isScrapped),
)
}

is FetchPoseResult.Duplicated ->
Timber.d("프리페치 생략: ${result.tryCount}회 시도 후 중복 포즈")

is FetchPoseResult.Failure -> {
Timber.e(result.throwable)
postSideEffect(RandomPoseEffect.ShowToast("포즈를 불러오는데 실패했어요"))
}
poseRepository.getSingleRandomPose(
headCount = peopleCount,
excludeIds = state.randomPoseIds,
maxRetry = PoseConst.MAXIMUM_RANDOM_POSE_RETRY_COUNT,
).onSuccess { pose ->
reduce { copy(poseList = (poseList + pose).toImmutableList()) }
}.onFailure { error ->
Comment on lines +155 to +161
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Prefetch로 추가된 포즈의 committedScraps가 갱신되지 않습니다.

Line 160에서 새 포즈를 poseList에만 추가하고 committedScraps에는 추가하지 않습니다. handleScrapToggle(Line 96)에서 committedScrap == null이면 조기 반환하므로, prefetch된 포즈에 대한 스크랩 토글이 서버에 반영되지 않습니다.

🐛 committedScraps 업데이트 추가
                 ).onSuccess { pose ->
-                    reduce { copy(poseList = (poseList + pose).toImmutableList()) }
+                    reduce {
+                        copy(
+                            poseList = (poseList + pose).toImmutableList(),
+                            committedScraps = committedScraps + (pose.id to pose.isScrapped),
+                        )
+                    }
                 }.onFailure { error ->
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
poseRepository.getSingleRandomPose(
headCount = peopleCount,
excludeIds = state.randomPoseIds,
maxRetry = PoseConst.MAXIMUM_RANDOM_POSE_RETRY_COUNT,
).onSuccess { pose ->
reduce { copy(poseList = (poseList + pose).toImmutableList()) }
}.onFailure { error ->
poseRepository.getSingleRandomPose(
headCount = peopleCount,
excludeIds = state.randomPoseIds,
maxRetry = PoseConst.MAXIMUM_RANDOM_POSE_RETRY_COUNT,
).onSuccess { pose ->
reduce {
copy(
poseList = (poseList + pose).toImmutableList(),
committedScraps = committedScraps + (pose.id to pose.isScrapped),
)
}
}.onFailure { error ->
🤖 Prompt for AI Agents
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt`
around lines 155 - 161, Prefetched poses added in RandomPoseViewModel (inside
the getSingleRandomPose onSuccess block) are appended only to poseList and not
to committedScraps, causing handleScrapToggle to early-return when
committedScrap == null; update the reducer to also add an entry to
committedScraps for the new pose(s) (use the pose's id) — e.g., merge
state.committedScraps with a mapping for pose.id to the pose.committedScrap
value (or false/default) when calling reduce so prefetched items have a non-null
committedScrap and handleScrapToggle will work.

if (error is RandomPoseRetryExhaustedException)
Timber.e(error, "중복 포즈")
else Timber.e(error)
}
}
}
Expand All @@ -179,73 +174,31 @@ internal class RandomPoseViewModel @AssistedInject constructor(
viewModelScope.launch {
reduce { copy(isLoading = true) }

val poses = mutableListOf<Pose>()
var totalFallbackCount = 0

// 초기에 INITIAL_POSE_LOAD_COUNT개 로드
while (
poses.size < PoseConst.INITIAL_POSE_LOAD_COUNT &&
totalFallbackCount < PoseConst.MAXIMUM_RANDOM_POSE_FALLBACK_COUNT
) {
val result = fetchRandomPose(
poseList = poses,
maxFallbackCount = PoseConst.MAXIMUM_RANDOM_POSE_FALLBACK_COUNT - totalFallbackCount,
)

totalFallbackCount += result.tryCount

when (result) {
is FetchPoseResult.Success -> poses.add(result.pose)
is FetchPoseResult.Failure -> {
Timber.e(result.throwable)
postSideEffect(RandomPoseEffect.ShowToast("포즈를 불러오는데 실패했어요"))
break
}

is FetchPoseResult.Duplicated ->
Timber.d("초기 로드: ${result.tryCount}회 시도 후 중복 포즈")
}
}

if (poses.isNotEmpty()) {
poseRepository.getMultipleRandomPose(
headCount = peopleCount,
excludeIds = emptySet(),
poseSize = PoseConst.INITIAL_POSE_LOAD_COUNT,
maxRetry = PoseConst.MAXIMUM_RANDOM_POSE_RETRY_COUNT,
).onSuccess { data ->
reduce {
copy(
isLoading = false,
poseList = poses.toImmutableList(),
committedScraps = poses.associate { it.id to it.isScrapped },
poseList = data.toImmutableList(),
committedScraps = data.associate { it.id to it.isScrapped },
currentIndex = 0,
)
}
} else {
}.onFailure { error ->
Timber.e(error)
reduce { copy(isLoading = false) }
postSideEffect(RandomPoseEffect.ShowToast("포즈를 불러오는데 실패했어요"))
if (error is RandomPoseRetryExhaustedException)
Timber.e(error, "중복 포즈")
else Timber.e(error)
}
Comment on lines +192 to 198
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

onFailure에서 에러가 이중 로깅됩니다.

Line 193에서 Timber.e(error)로 한 번 로깅한 후, Line 195-197에서 동일한 에러를 다시 로깅합니다. 모든 실패 케이스에서 에러 메시지가 두 번 출력됩니다.

🐛 중복 로깅 수정
             }.onFailure { error ->
-                Timber.e(error)
                 reduce { copy(isLoading = false) }
                 if (error is RandomPoseRetryExhaustedException)
                     Timber.e(error, "중복 포즈")
                 else Timber.e(error)
             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
}.onFailure { error ->
Timber.e(error)
reduce { copy(isLoading = false) }
postSideEffect(RandomPoseEffect.ShowToast("포즈를 불러오는데 실패했어요"))
if (error is RandomPoseRetryExhaustedException)
Timber.e(error, "중복 포즈")
else Timber.e(error)
}
}.onFailure { error ->
reduce { copy(isLoading = false) }
if (error is RandomPoseRetryExhaustedException)
Timber.e(error, "중복 포즈")
else Timber.e(error)
}
🤖 Prompt for AI Agents
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt`
around lines 192 - 198, The onFailure block in RandomPoseViewModel.kt currently
logs the same error twice; remove the first unconditional Timber.e(error) inside
the onFailure lambda and keep the conditional logging that follows (i.e., retain
reduce { copy(isLoading = false) } and then log either Timber.e(error, "중복 포즈")
when error is RandomPoseRetryExhaustedException or Timber.e(error) otherwise) so
the error is only emitted once.

}
}

private suspend fun fetchRandomPose(
poseList: List<Pose>,
maxFallbackCount: Int = PoseConst.MAXIMUM_RANDOM_POSE_FALLBACK_COUNT,
): FetchPoseResult {
var tryCount = 0

while (tryCount < maxFallbackCount) {
tryCount++
poseRepository.getRandomPose(headCount = peopleCount)
.onSuccess { pose ->
if (poseList.none { it.id == pose.id }) {
return FetchPoseResult.Success(tryCount, pose)
}
}
.onFailure { error ->
Timber.e(error)
return FetchPoseResult.Failure(tryCount, error)
}
}

return FetchPoseResult.Duplicated(tryCount)
}

override fun onCleared() {
super.onCleared()

Expand Down