[refactor] EntryRepository/NaviPreferenceRepository를 core:domain UseCase로 감싸기#19
Conversation
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughEntryRepository, NaviApp, NaviPreferenceRepository 인터페이스가 core:data에서 core:domain으로 이동되고, core:data에는 각각의 Impl 클래스만 남았습니다. core:domain에 GetEntryCompletedUseCase, SetEntryCompletedUseCase, GetNaviAlwaysUseCase, SetNaviAlwaysUseCase 4개 UseCase가 신규 추가되었습니다. feature:entry와 feature:home의 ViewModel이 Repository 직접 주입 대신 해당 UseCase를 주입받도록 변경되었고, 관련 gradle 의존성 및 테스트, NaviApp import 경로도 함께 갱신되었습니다. 리팩터링 과정을 정리한 핸드오프 문서가 추가되었습니다. Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
안녕하세요! 이번 변경사항을 살펴보니, Repository를 도메인 계층으로 옮기고 UseCase로 감싸는 클린 아키텍처 방향의 리팩터링이 꼼꼼하게 진행된 것 같습니다 🐰 한 가지 참고차 말씀드리면,
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
feature/entry/src/test/java/com/dororong/rodi/feature/entry/EntryViewModelTest.kt (1)
126-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
complete()예외/취소 케이스 테스트도 함께 보완하면 좋을 것 같아요.현재 성공 시나리오만 검증되고 있는데요,
setEntryCompletedUseCase()가 예외를 던지거나 코루틴이 취소되는 경우onDone콜백이 호출되지 않는 상황에 대한 테스트가 빠져 있는 것 같습니다. 도메인 계층으로 위임이 바뀐 시점이라 실패 흐름도 함께 커버해두면 이후 리팩터링 시 회귀를 잡기 좋을 것 같습니다.As per path instructions, "CancellationException 등 예외 케이스가 테스트되어 있는지 확인합니다" 및 "성공/실패/예외 케이스가 균형 있게 작성되어 있는지 확인합니다".
🧪 예외 케이스 테스트 추가 예시
`@Test` fun `complete stores entry completion and invokes callback`() = runTest(testDispatcher) { val setEntryCompletedUseCase = testSetEntryCompletedUseCase() coEvery { setEntryCompletedUseCase() } returns Unit val viewModel = EntryViewModel(setEntryCompletedUseCase) var done = false viewModel.complete { done = true } advanceUntilIdle() coVerify(exactly = 1) { setEntryCompletedUseCase() } assertTrue(done) } + + `@Test` + fun `complete does not invoke callback when use case throws`() = runTest(testDispatcher) { + val setEntryCompletedUseCase = testSetEntryCompletedUseCase() + coEvery { setEntryCompletedUseCase() } throws IllegalStateException("failed") + val viewModel = EntryViewModel(setEntryCompletedUseCase) + var done = false + + viewModel.complete { done = true } + advanceUntilIdle() + + assertFalse(done) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/entry/src/test/java/com/dororong/rodi/feature/entry/EntryViewModelTest.kt` around lines 126 - 140, `EntryViewModelTest.complete()`는 성공 경로만 검증하므로, `setEntryCompletedUseCase()`가 예외를 던지거나 `CancellationException`으로 취소될 때 `onDone` 콜백이 호출되지 않는 케이스를 추가하세요. `EntryViewModel.complete`, `testSetEntryCompletedUseCase`, `setEntryCompletedUseCase()`를 기준으로 실패/예외 시 `coVerify`와 `assertTrue/False`로 콜백 동작을 분리해 검증하고, 성공/실패/취소 흐름이 균형 있게 커버되도록 테스트를 보완하세요.Source: Path instructions
docs/handoff/archive/20260703-0345-HANDOFF.md (1)
363-369: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVerification 블록에 언어 지정 추가하면 좋을 것 같아요.
markdownlint가 364라인 fenced code block에 언어 미지정을 지적하고 있어요(MD040).
shell을 지정해두면 문서 렌더링 시 구문 강조도 되고 린트 경고도 사라집니다.📝 제안 diff
## Verification -``` +```shell ./gradlew :core:domain:compileKotlin ./gradlew test --stacktrace ./gradlew assembleDebug ./gradlew lint</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/handoff/archive/20260703-0345-HANDOFF.mdaround lines 363 - 369, The
Verification fenced code block is missing a language tag, which triggers the
markdownlint MD040 warning. Update the fenced block under the Verification
section to specify shell so the commands in the handoff doc are properly
highlighted and lint-clean. Use the Verification heading and its fenced command
block as the reference point when making the change.</details> <!-- cr-comment:v1:56a1a9fb410901489cfb59fc --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>core/domain/build.gradle.kts (1)</summary><blockquote> `8-8`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **kotlinx-coroutines-core를 `api`로 노출하는 편이 안전할 것 같아요.** 같은 모듈의 `EntryRepository.isCompleted: Flow<Boolean?>`처럼 `Flow` 타입이 core:domain의 public 인터페이스 시그니처에 그대로 노출되고 있어요. `implementation`으로 선언하면 이 의존성이 컴파일 클래스패스로 전이되지 않기 때문에, `feature:entry`처럼 core:domain을 직접 소비하는 모듈이 별도로 coroutines 의존성을 갖고 있지 않을 경우 `Flow` 타입 참조 시 컴파일 오류가 날 수 있어요. (현재는 다른 경로로 coroutines가 이미 전이되고 있어 문제가 드러나지 않을 수 있지만, Gradle의 일반적인 권장 사항은 public API에 노출되는 타입의 의존성은 `api`로 선언하는 것입니다.) <details> <summary>♻️ 제안하는 변경</summary> ```diff dependencies { implementation(project(":core:common")) implementation(libs.javax.inject) - implementation(libs.kotlinx.coroutines.core) + api(libs.kotlinx.coroutines.core)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/domain/build.gradle.kts` at line 8, The core:domain module exposes Flow in its public API through symbols like EntryRepository.isCompleted, so kotlinx.coroutines.core should not stay as implementation-only dependency. Update the dependency declaration in the build.gradle.kts for core:domain from implementation to api so any module consuming core:domain can compile against the Flow type without needing a separate coroutines dependency.core/domain/src/main/kotlin/com/dororong/rodi/core/domain/NaviPreferenceRepository.kt (1)
3-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEntryRepository와 비교했을 때 비동기 계약이 다소 일관되지 않아 보여요.
같은 도메인 계층에 새로 추가된
EntryRepository는Flow<Boolean?>/suspend fun으로 정의된 반면, 이 인터페이스는getAlways()/setAlways()가 완전히 동기 함수로 정의되어 있어요.core/data의NaviPreferenceRepositoryImpl을 보면 결국NaviPreference(SharedPreferences 기반으로 추정)를 직접 동기 호출하고,HomeViewModel에서는viewModelScope.launch { getNaviAlwaysUseCase() }형태로 호출되어 기본 디스패처(Main)에서 실행될 가능성이 있어요.SharedPreferences는 API 자체는 동기지만 내부적으로 디스크 I/O가 수행되기 때문에, 메인 스레드에서 호출 시 ANR 위험이 있다는 점이 공식 문서에서도 안내되고 있습니다. 지금 당장 문제를 일으킬 정도는 아니겠지만, 장기적으로는
suspend fun또는Flow기반으로 통일해서 디스패처 제어 여지를 남겨두면 어떨까요? (추후 DataStore 전환 시에도 계약을 그대로 유지할 수 있어요.)As per path instructions,
**/domain/**/*.kt에서 "Repository 인터페이스가 구현 세부사항을 노출하지 않는지 확인"하도록 명시되어 있어, 동기 시그니처가 결국 SharedPreferences와 같은 구현 세부사항(블로킹 I/O 특성)을 암묵적으로 노출하고 있는지 점검이 필요해 보입니다.♻️ 제안하는 시그니처 변경 예시
interface NaviPreferenceRepository { - fun getAlways(): NaviApp? - fun setAlways(app: NaviApp) + suspend fun getAlways(): NaviApp? + suspend fun setAlways(app: NaviApp) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/domain/src/main/kotlin/com/dororong/rodi/core/domain/NaviPreferenceRepository.kt` around lines 3 - 6, `NaviPreferenceRepository`의 `getAlways()`/`setAlways()`가 동기 API라서 `EntryRepository`와 비동기 계약이 불일치합니다. `NaviPreferenceRepository`와 이를 구현한 `NaviPreferenceRepositoryImpl`, 호출부인 `HomeViewModel`/`GetNaviAlwaysUseCase`가 블로킹 I/O를 메인 스레드에서 직접 실행하지 않도록 `suspend fun` 또는 `Flow` 기반으로 시그니처를 통일하고, 구현체와 사용처도 함께 비동기 방식으로 맞춰 주세요.Source: Path instructions
core/data/src/main/java/com/dororong/rodi/core/data/EntryRepository.kt (1)
4-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win도메인 타입 의존으로의 전환은 깔끔합니다. 다만 파일명 정리를 고려해보세요.
EntryRepository인터페이스가 core:domain으로 이동하면서 이 파일에는 이제EntryRepositoryImpl클래스만 남았습니다. 파일명은 여전히EntryRepository.kt라 실제 내용과 다소 어긋나 보입니다. Kotlin/Android 컨벤션상 파일명을EntryRepositoryImpl.kt로 맞추면 파일 탐색 시 혼란을 줄일 수 있을 것 같습니다.As per path instructions, "변수명, 함수명이 의도대로 드러나는지 확인하고 Clean Code 원칙에 어긋나는 부분을 교정합니다."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/data/src/main/java/com/dororong/rodi/core/data/EntryRepository.kt` around lines 4 - 11, 파일명과 실제 선언이 맞지 않는 상태입니다. `EntryRepository.kt`에는 이제 `EntryRepositoryImpl`만 남아 있으므로, 파일명을 `EntryRepositoryImpl.kt`로 변경해 내용과 이름이 일치하도록 정리하세요. `EntryRepositoryImpl` 클래스 기준으로 찾을 수 있게 유지하고, 관련 import와 패키지 선언은 그대로 두되 파일 식별만 명확하게 맞추면 됩니다.Source: Path instructions
core/domain/src/main/kotlin/com/dororong/rodi/core/domain/usecase/GetNaviAlwaysUseCase.kt (1)
7-11: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
GetNaviAlwaysUseCase도suspend경계로 맞춰두는 것을 고려해보세요.
지금은naviPreferenceRepository.getAlways()를 그대로 동기 호출해서, Entry 계열과 달리 호출부의 스레딩 규칙이 섞입니다. API를 통일해두면 저장소 구현이 바뀌어도 호출부를 같은 방식으로 다루기 쉬워집니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/domain/src/main/kotlin/com/dororong/rodi/core/domain/usecase/GetNaviAlwaysUseCase.kt` around lines 7 - 11, GetNaviAlwaysUseCase is still exposing a synchronous invoke path while its repository call is treated inconsistently with the other use cases. Update the GetNaviAlwaysUseCase.invoke method to be suspend and make it delegate to NaviPreferenceRepository.getAlways() through the same suspend boundary as the Entry-related use cases, so callers use a consistent coroutine-based API.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/data/src/main/java/com/dororong/rodi/core/data/EntryRepository.kt`:
- Around line 4-11: 파일명과 실제 선언이 맞지 않는 상태입니다. `EntryRepository.kt`에는 이제
`EntryRepositoryImpl`만 남아 있으므로, 파일명을 `EntryRepositoryImpl.kt`로 변경해 내용과 이름이 일치하도록
정리하세요. `EntryRepositoryImpl` 클래스 기준으로 찾을 수 있게 유지하고, 관련 import와 패키지 선언은 그대로 두되 파일
식별만 명확하게 맞추면 됩니다.
In `@core/domain/build.gradle.kts`:
- Line 8: The core:domain module exposes Flow in its public API through symbols
like EntryRepository.isCompleted, so kotlinx.coroutines.core should not stay as
implementation-only dependency. Update the dependency declaration in the
build.gradle.kts for core:domain from implementation to api so any module
consuming core:domain can compile against the Flow type without needing a
separate coroutines dependency.
In
`@core/domain/src/main/kotlin/com/dororong/rodi/core/domain/NaviPreferenceRepository.kt`:
- Around line 3-6: `NaviPreferenceRepository`의 `getAlways()`/`setAlways()`가 동기
API라서 `EntryRepository`와 비동기 계약이 불일치합니다. `NaviPreferenceRepository`와 이를 구현한
`NaviPreferenceRepositoryImpl`, 호출부인 `HomeViewModel`/`GetNaviAlwaysUseCase`가 블로킹
I/O를 메인 스레드에서 직접 실행하지 않도록 `suspend fun` 또는 `Flow` 기반으로 시그니처를 통일하고, 구현체와 사용처도 함께
비동기 방식으로 맞춰 주세요.
In
`@core/domain/src/main/kotlin/com/dororong/rodi/core/domain/usecase/GetNaviAlwaysUseCase.kt`:
- Around line 7-11: GetNaviAlwaysUseCase is still exposing a synchronous invoke
path while its repository call is treated inconsistently with the other use
cases. Update the GetNaviAlwaysUseCase.invoke method to be suspend and make it
delegate to NaviPreferenceRepository.getAlways() through the same suspend
boundary as the Entry-related use cases, so callers use a consistent
coroutine-based API.
In `@docs/handoff/archive/20260703-0345-HANDOFF.md`:
- Around line 363-369: The Verification fenced code block is missing a language
tag, which triggers the markdownlint MD040 warning. Update the fenced block
under the Verification section to specify shell so the commands in the handoff
doc are properly highlighted and lint-clean. Use the Verification heading and
its fenced command block as the reference point when making the change.
In
`@feature/entry/src/test/java/com/dororong/rodi/feature/entry/EntryViewModelTest.kt`:
- Around line 126-140: `EntryViewModelTest.complete()`는 성공 경로만 검증하므로,
`setEntryCompletedUseCase()`가 예외를 던지거나 `CancellationException`으로 취소될 때 `onDone`
콜백이 호출되지 않는 케이스를 추가하세요. `EntryViewModel.complete`,
`testSetEntryCompletedUseCase`, `setEntryCompletedUseCase()`를 기준으로 실패/예외 시
`coVerify`와 `assertTrue/False`로 콜백 동작을 분리해 검증하고, 성공/실패/취소 흐름이 균형 있게 커버되도록 테스트를
보완하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cced8429-bf0e-4572-92ea-8832d63e1468
📒 Files selected for processing (21)
core/data/src/main/java/com/dororong/rodi/core/data/EntryRepository.ktcore/data/src/main/java/com/dororong/rodi/core/data/di/DataModule.ktcore/data/src/main/java/com/dororong/rodi/core/data/navi/NaviPreference.ktcore/data/src/main/java/com/dororong/rodi/core/data/navi/NaviPreferenceRepository.ktcore/domain/build.gradle.ktscore/domain/src/main/kotlin/com/dororong/rodi/core/domain/EntryRepository.ktcore/domain/src/main/kotlin/com/dororong/rodi/core/domain/NaviApp.ktcore/domain/src/main/kotlin/com/dororong/rodi/core/domain/NaviPreferenceRepository.ktcore/domain/src/main/kotlin/com/dororong/rodi/core/domain/usecase/GetEntryCompletedUseCase.ktcore/domain/src/main/kotlin/com/dororong/rodi/core/domain/usecase/GetNaviAlwaysUseCase.ktcore/domain/src/main/kotlin/com/dororong/rodi/core/domain/usecase/SetEntryCompletedUseCase.ktcore/domain/src/main/kotlin/com/dororong/rodi/core/domain/usecase/SetNaviAlwaysUseCase.ktdocs/handoff/archive/20260703-0345-HANDOFF.mdfeature/entry/build.gradle.ktsfeature/entry/src/main/java/com/dororong/rodi/feature/entry/EntryViewModel.ktfeature/entry/src/test/java/com/dororong/rodi/feature/entry/EntryViewModelTest.ktfeature/home/src/main/java/com/dororong/rodi/feature/home/HomeContract.ktfeature/home/src/main/java/com/dororong/rodi/feature/home/HomeScreen.ktfeature/home/src/main/java/com/dororong/rodi/feature/home/HomeViewModel.ktfeature/home/src/main/java/com/dororong/rodi/feature/home/components/NaviPickerSheet.ktfeature/home/src/test/java/com/dororong/rodi/feature/home/HomeViewModelTest.kt
UiHyeon-Kim
changed the title
- NaviPreferenceRepository와 내비 선호 UseCase를 suspend API로 맞춤 - SharedPreferences 접근을 data 구현체에서 IO 디스패처로 이동 - Entry 완료 실패/취소 콜백 테스트와 리뷰 문서 정리 반영
1 participant
📋 작업 요약
EntryRepository/NaviPreferenceRepository를core:domain의 UseCase로 감싸 ViewModel이 Repository 대신 UseCase에 의존하도록 리팩터링합니다. (PR #15와 동일 패턴)✏️ 주요 변경사항
EntryRepository/NaviPreferenceRepository인터페이스를 core:data → core:domain으로 이동 (구현체는 core:data에 유지)NaviAppenum도 순수 Kotlin이라 core:domain으로 함께 이동GetEntryCompletedUseCase/SetEntryCompletedUseCase/GetNaviAlwaysUseCase/SetNaviAlwaysUseCase추가EntryViewModel/HomeViewModel이 Repository 대신 UseCase를 주입받도록 리팩터링👀 리뷰 포인트
docs/handoff/archive/20260703-0345-HANDOFF.md✅ 체크리스트