Open
Conversation
WalkthroughUserProfile 관리를 로컬 인메모리 캐시와 원격 데이터소스로 분리하고, 동기 호출을 Flow 기반 관찰형 API로 전환했습니다. Repository에 Mutex 기반 중복 요청 방지와 캐시 갱신/무효화 로직이 추가되었고, 관련 유닛테스트와 DI 바인딩이 갱신되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as ViewModel
participant UC as ObserveUserProfileUseCase
participant Repo as UserRepository
participant Local as UserLocalDataSource
participant Remote as UserRemoteDataSource
VM->>UC: observeUserProfile()
UC->>Repo: observeUserProfile()
Repo->>Repo: acquire Mutex
Repo->>Local: check userProfile StateFlow
alt cache empty
Repo->>Remote: fetchUserProfile()
Remote-->>Repo: Result<UserProfileResponse>
Repo->>Local: saveUserProfile(domainModel)
else cache present
Repo->>Local: read cached UserProfile
end
Repo->>Repo: release Mutex
Local-->>Repo: emit userProfile StateFlow
Repo-->>UC: emit Result<UserProfile>
UC-->>VM: emit Result<UserProfile>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
data/src/main/java/com/threegap/bitnagil/data/user/datasource/UserLocalDataSource.kt (1)
7-9: 인메모리 캐시 setter라면suspend는 과한 계약입니다.구현을 확인한 결과,
saveUserProfile()은 단순히_userProfile.update { userProfile }만 수행하고 있습니다. 실제 suspension point가 없으므로suspend키워드는 불필요하며, 일반 함수로 변경하는 것이 API 설계상 더 적절합니다. 현재 시그니처는 호출자를 불필요하게 코루틴 컨텍스트에 묶고 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/user/datasource/UserLocalDataSource.kt` around lines 7 - 9, The interface exposes saveUserProfile as suspend despite the implementation only doing an in-memory update (_userProfile.update { userProfile }) with no suspension points; change the signature of saveUserProfile from suspend fun saveUserProfile(userProfile: UserProfile) to a regular fun saveUserProfile(userProfile: UserProfile) in UserLocalDataSource (and update any implementing class methods accordingly) and update any callers to remove unnecessary coroutine context so the API no longer forces callers into suspend contexts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt`:
- Around line 29-31: The fetchUserProfile flow can write stale data after
clearCache(); add a generation token to UserRepositoryImpl (e.g., a private
AtomicInteger or volatile Int) that you increment in clearCache(), then capture
the current generation before calling userRemoteDataSource.fetchUserProfile()
and, inside the onSuccess handler (the lambda that calls
userLocalDataSource.saveUserProfile(response.toDomain())), compare the captured
generation with the repository's current generation and only call
saveUserProfile if they match; update references to fetchUserProfile and
clearCache accordingly so any in-flight results are ignored when the generation
advanced.
- Around line 25-37: The current double-check in observeUserProfile
(userLocalDataSource.userProfile + fetchMutex) only prevents duplicate remote
calls on success; if the first fetch fails the cache stays null and each waiting
subscriber re-enters the mutex and triggers another fetch. Fix by introducing a
shared in-flight holder (e.g., a CompletableDeferred or Deferred<Result<...>>
named inFlightFetch) that you set before calling
userRemoteDataSource.fetchUserProfile(), have all subscribers await that single
deferred, and clear it after completion; on success call
userLocalDataSource.saveUserProfile(response.toDomain()) and emit the result,
and on failure emit the failure to all awaiters as well. Update
observeUserProfile to check/await inFlightFetch when cache is null and add a
concurrency test covering failed-first-request to ensure only one remote call is
made and the failure is shared.
In
`@domain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.kt`:
- Around line 7-8: The current contract doesn't allow propagating cache
invalidation to existing collectors; change observeUserProfile() and the
implementation to model "no profile" (either make observeUserProfile():
Flow<Result<UserProfile?>> or introduce a sealed state like UserProfileState to
represent Unauthenticated/Empty), then update UserRepositoryImpl (remove
filterNotNull() on the local cache flow) so clearCache() actively emits the
empty/unauthenticated state into the shared flow/stream; ensure clearCache()
triggers emission of that empty state so live collectors receive the
invalidation.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt`:
- Around line 251-260: The subscription in HomeViewModel uses
repeatOnSubscription + observeUserProfileUseCase() while incrementing
loadingCount once then decrementing on every emission, which can drive
loadingCount negative; change the logic so the initial increment is matched by a
single decrement (e.g., only decrement once after the first emission) or
introduce a dedicated subscription loading flag (e.g., userProfileLoading)
instead of reusing loadingCount; update the reduce calls inside the
observeUserProfileUseCase().collect block (and the initial repeatOnSubscription
increment) to use the chosen approach so each subscription start has a balanced
decrement or uses an isolated boolean for this subscription.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt`:
- Around line 24-27: In MyPageViewModel's observeUserProfileUseCase collector
the code sets profileUrl to the literal string "profileUrl"; change reduce {
state.copy(name = it.nickname, profileUrl = "profileUrl") } to assign the real
value from the DTO/Domain object (e.g., it.profileUrl or it.profileImageUrl) or,
if that field can be null/empty, preserve the existing value by using
state.profileUrl when the incoming value is absent; update the mapping in the
observeUserProfileUseCase collection block and ensure reduce/state.copy uses the
actual property name instead of the hardcoded string.
---
Nitpick comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/user/datasource/UserLocalDataSource.kt`:
- Around line 7-9: The interface exposes saveUserProfile as suspend despite the
implementation only doing an in-memory update (_userProfile.update { userProfile
}) with no suspension points; change the signature of saveUserProfile from
suspend fun saveUserProfile(userProfile: UserProfile) to a regular fun
saveUserProfile(userProfile: UserProfile) in UserLocalDataSource (and update any
implementing class methods accordingly) and update any callers to remove
unnecessary coroutine context so the API no longer forces callers into suspend
contexts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07cbba97-2081-4aeb-a50e-2a9871479390
📒 Files selected for processing (15)
app/src/main/java/com/threegap/bitnagil/di/data/DataSourceModule.ktdata/build.gradle.ktsdata/src/main/java/com/threegap/bitnagil/data/user/datasource/UserLocalDataSource.ktdata/src/main/java/com/threegap/bitnagil/data/user/datasource/UserRemoteDataSource.ktdata/src/main/java/com/threegap/bitnagil/data/user/datasourceImpl/UserLocalDataSourceImpl.ktdata/src/main/java/com/threegap/bitnagil/data/user/datasourceImpl/UserRemoteDataSourceImpl.ktdata/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.ktdata/src/test/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImplTest.ktdomain/src/main/java/com/threegap/bitnagil/domain/auth/usecase/LogoutUseCase.ktdomain/src/main/java/com/threegap/bitnagil/domain/auth/usecase/WithdrawalUseCase.ktdomain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.ktdomain/src/main/java/com/threegap/bitnagil/domain/user/usecase/ObserveUserProfileUseCase.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/onboarding/OnBoardingViewModel.kt
data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
domain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.kt
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt
Show resolved
Hide resolved
l5x5l
reviewed
Mar 11, 2026
Contributor
There was a problem hiding this comment.
[1] OnBoarindgViewModel에서는 flow를 수집하는 방식보다는 기존 일회성으로 정보를 조회하여 사용하기에, 기존 방식처럼 일회성으로 UserProfile을 조회하는 함수도 1개 필요할 것 같습니다!
Member
Author
There was a problem hiding this comment.
요 부분은 first()를 통해 수집 후 cancel 하면 큰 문제는 없겠다 판단했었는데, 역할을 분리해서 각각 별도로 구성해보겠습니다~
There was a problem hiding this comment.
♻️ Duplicate comments (4)
data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt (3)
52-55:⚠️ Potential issue | 🟠 Major
clearCache()호출 후에도 진행 중인 fetch가 stale 데이터를 저장할 수 있습니다.
clearCache()가 호출되어도 이미 시작된 fetch 요청은 완료 후 이전 계정의 프로필을 캐시에 다시 저장합니다. 로그아웃/회원탈퇴 직후 교차 계정 데이터 노출 위험이 있습니다.generation token(버전 카운터)을 도입하여 fetch 시작 시점의 generation과 저장 시점의 generation이 일치하는 경우에만
saveUserProfile()을 호출하도록 개선이 필요합니다.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt` around lines 52 - 55, Introduce a generation/version counter in UserRepositoryImpl (e.g., an AtomicInteger field) and increment it inside clearCache(); when calling userRemoteDataSource.fetchUserProfile() capture the current generation into a local variable, and in the onSuccess lambda compare that captured generation to the current counter—only call userLocalDataSource.saveUserProfile(response.toDomain()) if they match; this prevents in-flight fetches started before clearCache() from writing stale profiles. Ensure the counter is thread-safe (AtomicInteger) so increments and comparisons are safe across async callbacks.
31-35:⚠️ Potential issue | 🟠 Major
clearCache()후 기존 collector가 무효화 이벤트를 수신하지 못합니다.Line 33의
filterNotNull()로 인해 캐시가 null로 초기화되어도 기존 collector는 아무 이벤트도 받지 못하고 이전 프로필을 계속 유지합니다.
Flow<Result<UserProfile?>>또는 sealed state 패턴으로 "프로필 없음" 상태를 명시적으로 모델링하면 캐시 무효화가 기존 구독자에게 전파됩니다.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt` around lines 31 - 35, The current emitAll in UserRepositoryImpl filters out nulls via userLocalDataSource.userProfile.filterNotNull(), so after clearCache() existing collectors never see the "no profile" event; remove filterNotNull() and map the nullable profile directly to a Result (e.g., userLocalDataSource.userProfile.map { Result.success(it) }) so the Flow is Result<UserProfile?> and null (cache invalidation) is propagated to subscribers, or alternatively replace with an explicit sealed state mapping that models "NoProfile" vs "Profile" to ensure clearCache() events reach existing collectors.
46-57:⚠️ Potential issue | 🟠 Major실패 시 중복 네트워크 요청 가능성이 여전히 존재합니다.
현재 double-check locking 패턴은 성공 케이스에서만 중복 요청을 방지합니다. 첫 번째 remote fetch가 실패하면 캐시가 null 상태로 유지되어, mutex를 대기 중이던 구독자들이 순차적으로 다시 fetch를 시도합니다.
Deferred<Result<UserProfile>>를 사용하여 in-flight 요청 결과를 성공/실패 모두 공유하도록 개선하면 장애 상황에서의 불필요한 backend 호출을 방지할 수 있습니다.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt` around lines 46 - 57, fetchAndCacheIfNeeded currently only prevents duplicate requests on success; to share in-flight results (success or failure) create a shared Deferred<Result<UserProfile>> field (e.g., inFlightFetch: Deferred<Result<UserProfile>>?) and use fetchMutex to set it once when starting a remote fetch, so subsequent callers return/await that same Deferred instead of re-triggering fetchUserProfile; inside fetchAndCacheIfNeeded, check userLocalDataSource.userProfile first, then inside fetchMutex if inFlightFetch exists return/await it, otherwise assign inFlightFetch = coroutineScope.async { userRemoteDataSource.fetchUserProfile().also { if (it.isSuccess) userLocalDataSource.saveUserProfile(it.getOrThrow().toDomain()) } .map { it.toDomain() } } and finally ensure you clear inFlightFetch on completion (use invokeOnCompletion or try/finally) and handle exceptions/cancellation so callers receive the same Result for both success and failure.domain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.kt (1)
7-9:⚠️ Potential issue | 🟠 Major인터페이스 계약이 캐시 무효화 상태를 표현하지 못합니다.
observeUserProfile()이Flow<Result<UserProfile>>를 반환하므로 "프로필 없음" 상태를 나타낼 수 없습니다.clearCache()호출 시 기존 collector에게 무효화를 전파하려면Flow<Result<UserProfile?>>또는 sealed class로 상태를 모델링해야 합니다.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@domain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.kt` around lines 7 - 9, The interface cannot represent a "no profile / cache invalidated" state because observeUserProfile() returns Flow<Result<UserProfile>>; change the contract to convey absence by updating observeUserProfile() to return Flow<Result<UserProfile?>> (or replace Result<UserProfile> with a sealed union like UserProfileState { Success(UserProfile), Empty, Error(...) }) and similarly adjust getUserProfile() to suspend fun getUserProfile(): Result<UserProfile?> or Result<UserProfileState>; update implementations of observeUserProfile(), getUserProfile(), and clearCache() so clearCache() emits the new "empty/invalidated" state to existing collectors (e.g., emit Result.Success(null) or the sealed Empty state) and update callers to handle the nullable/sealed state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt`:
- Around line 52-55: Introduce a generation/version counter in
UserRepositoryImpl (e.g., an AtomicInteger field) and increment it inside
clearCache(); when calling userRemoteDataSource.fetchUserProfile() capture the
current generation into a local variable, and in the onSuccess lambda compare
that captured generation to the current counter—only call
userLocalDataSource.saveUserProfile(response.toDomain()) if they match; this
prevents in-flight fetches started before clearCache() from writing stale
profiles. Ensure the counter is thread-safe (AtomicInteger) so increments and
comparisons are safe across async callbacks.
- Around line 31-35: The current emitAll in UserRepositoryImpl filters out nulls
via userLocalDataSource.userProfile.filterNotNull(), so after clearCache()
existing collectors never see the "no profile" event; remove filterNotNull() and
map the nullable profile directly to a Result (e.g.,
userLocalDataSource.userProfile.map { Result.success(it) }) so the Flow is
Result<UserProfile?> and null (cache invalidation) is propagated to subscribers,
or alternatively replace with an explicit sealed state mapping that models
"NoProfile" vs "Profile" to ensure clearCache() events reach existing
collectors.
- Around line 46-57: fetchAndCacheIfNeeded currently only prevents duplicate
requests on success; to share in-flight results (success or failure) create a
shared Deferred<Result<UserProfile>> field (e.g., inFlightFetch:
Deferred<Result<UserProfile>>?) and use fetchMutex to set it once when starting
a remote fetch, so subsequent callers return/await that same Deferred instead of
re-triggering fetchUserProfile; inside fetchAndCacheIfNeeded, check
userLocalDataSource.userProfile first, then inside fetchMutex if inFlightFetch
exists return/await it, otherwise assign inFlightFetch = coroutineScope.async {
userRemoteDataSource.fetchUserProfile().also { if (it.isSuccess)
userLocalDataSource.saveUserProfile(it.getOrThrow().toDomain()) } .map {
it.toDomain() } } and finally ensure you clear inFlightFetch on completion (use
invokeOnCompletion or try/finally) and handle exceptions/cancellation so callers
receive the same Result for both success and failure.
In
`@domain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.kt`:
- Around line 7-9: The interface cannot represent a "no profile / cache
invalidated" state because observeUserProfile() returns
Flow<Result<UserProfile>>; change the contract to convey absence by updating
observeUserProfile() to return Flow<Result<UserProfile?>> (or replace
Result<UserProfile> with a sealed union like UserProfileState {
Success(UserProfile), Empty, Error(...) }) and similarly adjust getUserProfile()
to suspend fun getUserProfile(): Result<UserProfile?> or
Result<UserProfileState>; update implementations of observeUserProfile(),
getUserProfile(), and clearCache() so clearCache() emits the new
"empty/invalidated" state to existing collectors (e.g., emit
Result.Success(null) or the sealed Empty state) and update callers to handle the
nullable/sealed state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89739430-f266-4f30-b584-2899f7f01abf
📒 Files selected for processing (4)
data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.ktdomain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.ktdomain/src/main/java/com/threegap/bitnagil/domain/user/usecase/GetUserProfileUseCase.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/onboarding/OnBoardingViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- presentation/src/main/java/com/threegap/bitnagil/presentation/screen/onboarding/OnBoardingViewModel.kt
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
[ PR Content ]
UserProfile정보를 관리하는 방식을 기존의 일회성 Fetch 방식에서 SSOT 기반의 실시간 구독 방식으로 개선했습니다.Related issue
Screenshot 📸
주요 변경 사항 및 근거
데이터 소스 분리 (Remote & Local)
UserDataSource를UserRemoteDataSource와UserLocalDataSource로 분리.UserLocalDataSource는MutableStateFlow를 통해 앱 생명주기 동안 유지되는 In-memory 캐시 역할을 수행합니다.반응형 데이터 흐름 (Observe-to-Fetch)
fetchUserProfile()(명령형) 방식을observeUserProfile()(선언형/반응형) 방식으로 전환.레이스 컨디션 방지 (Mutex & Double-Check Locking)
UserRepositoryImpl에Mutex를 도입하여 중복 네트워크 요청을 차단.UseCase 오케스트레이션 (Logout/Withdrawal)
UserRepository.clearCache()를 함께 호출.Work Description
RemoteDataSource와LocalDataSource를 분리하여 데이터 흐름의 명확성을 확보했습니다.ObserveUserProfileUseCase를 구독하는 것만으로 데이터 로드와 갱신이 자동으로 이루어집니다.UserRepositoryImplTest를 추가하여 다중 코루틴 환경에서의 중복 요청 방지 및 캐시 정합성을 검증했습니다.To Reviewers 📢
Summary by CodeRabbit
새로운 기능
행동 변경
테스트
설정