Open
Conversation
WalkthroughUserProfile 데이터 관리를 로컬 캐시 기반 반응형 패턴으로 리팩토링했습니다. 단일 데이터소스를 로컬/원격으로 분리하고, 동기 API를 Flow 기반의 관찰 가능한 구조로 변환하며, 중복 요청 방지를 위한 Mutex 로직을 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as ViewModel
participant UC as ObserveUserProfileUseCase
participant Repo as UserRepository
participant Local as LocalDataSource
participant Remote as RemoteDataSource
VM->>UC: observeUserProfile()
UC->>Repo: observeUserProfile()
Repo->>Repo: Acquire Mutex Lock
Repo->>Local: Check userProfile
alt Cache Empty (First Time)
Repo->>Remote: fetchUserProfile()
Remote-->>Repo: Result<UserProfileResponse>
Repo->>Local: saveUserProfile()
Local-->>Repo: ✓
else Cache Exists
Repo->>Local: Get cached userProfile
end
Repo->>Repo: Release Mutex Lock
Repo->>Local: Observe userProfile StateFlow
Local-->>Repo: emit UserProfile
Repo-->>UC: emit Result<UserProfile>
UC-->>VM: emit Result<UserProfile>
VM->>VM: Update UI
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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
Comment on lines
+25
to
+37
| override fun observeUserProfile(): Flow<Result<UserProfile>> = flow { | ||
| if (userLocalDataSource.userProfile.value == null) { | ||
| fetchMutex.withLock { | ||
| if (userLocalDataSource.userProfile.value == null) { | ||
| userRemoteDataSource.fetchUserProfile() | ||
| .onSuccess { response -> | ||
| userLocalDataSource.saveUserProfile(response.toDomain()) | ||
| } | ||
| .onFailure { | ||
| emit(Result.failure(it)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
실패한 첫 요청 뒤에 대기 중 구독자들이 순차 재호출합니다.
Line 26-28의 double-check는 성공 케이스에서만 중복 억제가 됩니다. 첫 remote fetch가 실패하면 캐시는 계속 null이라, mutex를 기다리던 각 구독자가 락을 순서대로 잡으면서 Line 29를 다시 호출합니다. 장애 상황에서 구독 수만큼 backend 호출이 터질 수 있으니, in-flight 결과를 성공/실패 모두 공유하도록 바꿔야 합니다. 이 경로를 고정하는 실패-path 동시성 테스트도 같이 있으면 좋겠습니다.
🤖 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 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.
Comment on lines
+29
to
+31
| userRemoteDataSource.fetchUserProfile() | ||
| .onSuccess { response -> | ||
| userLocalDataSource.saveUserProfile(response.toDomain()) |
There was a problem hiding this comment.
clearCache()가 진행 중인 fetch 결과를 무효화하지 못합니다.
Line 29-31의 요청이 이미 시작된 상태에서 Line 47-48이 실행돼도, 나중에 fetch가 성공하면 이전 계정의 프로필을 다시 캐시에 써버립니다. 그러면 로그아웃/회원탈퇴 직후에도 stale 데이터가 남고, 다음 구독자가 이전 사용자 프로필을 읽는 교차 계정 노출까지 생길 수 있습니다. clearCache() 시점마다 세대(version/token)를 올리고 저장 직전에 같은 세대인지 확인하거나, in-flight fetch를 취소할 수 있어야 합니다.
Also applies to: 47-48
🤖 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 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.
Comment on lines
+7
to
+8
| fun observeUserProfile(): Flow<Result<UserProfile>> | ||
| fun clearCache() |
There was a problem hiding this comment.
현재 계약으로는 캐시 무효화가 기존 구독자에게 전파되지 않습니다.
clearCache()를 추가했지만 observeUserProfile(): Flow<Result<UserProfile>>는 "프로필 없음" 상태를 표현할 수 없습니다. 제공된 data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt:18-37 스니펫도 로컬 캐시를 filterNotNull() 한 뒤에만 내보내고 있어서, clearCache() 후 살아 있는 collector는 아무 이벤트도 받지 못하고 이전 프로필을 계속 들고 있을 수 있습니다. 이 계약은 UserProfile? 또는 별도 sealed state처럼 비인증/초기화 상태까지 모델링하도록 바꾸는 편이 안전합니다.
🤖 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 - 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.
Comment on lines
+251
to
+260
| repeatOnSubscription { | ||
| reduce { state.copy(loadingCount = state.loadingCount + 1) } | ||
| observeUserProfileUseCase().collect { result -> | ||
| result.fold( | ||
| onSuccess = { | ||
| reduce { state.copy(userNickname = it.nickname, loadingCount = state.loadingCount - 1) } | ||
| }, | ||
| onFailure = { | ||
| Log.e("HomeViewModel", "유저 정보 가져오기 실패: ${it.message}") | ||
| reduce { state.copy(loadingCount = state.loadingCount - 1) } |
There was a problem hiding this comment.
장기 구독에 loadingCount 증감 로직을 그대로 붙이면 상태가 깨집니다.
Line 252에서는 1회만 증가시키는데, Line 256/260에서는 구독 중 들어오는 모든 emission마다 감소합니다. 이 Flow는 SSOT 구독이라 캐시 재방출이나 후속 업데이트가 오면 loadingCount가 음수가 되고, 첫 emission 전에 취소되면 증가만 남습니다. 이후 다른 로딩이 시작돼도 스피너가 안 뜰 수 있으니, 초기 1회만 감소시키거나 이 구독용 로딩 상태를 별도로 분리하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt`
around lines 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.
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt
Show resolved
Hide resolved
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
1 participant
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
릴리스 노트
새로운 기능
테스트