Open
Conversation
Walkthrough이벤트 기반 감정 갱신을 제거하고 StateFlow 기반의 반응형 dailyEmotionFlow를 도입했습니다. DailyEmotion에 fetchedDate/isStale가 추가되고, 저장소에서 onSubscription 자동 로드 및 AtomicBoolean 기반 중복 호출 방지가 구현되었습니다. ViewModel들은 ObserveDailyEmotionUseCase로 전환되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as ViewModel
participant UC as ObserveDailyEmotionUseCase
participant Repo as EmotionRepositoryImpl
participant Remote as RemoteDataSource
participant DB as LocalCache
rect rgba(135,206,250,0.5)
VM->>UC: 구독 시작 (ObserveDailyEmotionUseCase())
end
UC->>Repo: dailyEmotionFlow 구독
Note right of Repo: MutableStateFlow onSubscription
Repo->>Repo: isFetching 검사 (AtomicBoolean)
alt stale or INIT
Repo->>Remote: API 호출 (fetch)
Remote-->>Repo: 응답
Repo->>DB: 캐시 갱신 (선택적)
Repo->>Repo: _dailyEmotionFlow 업데이트 (DailyEmotion with fetchedDate)
Repo-->>UC: Flow 업데이트 전파
UC-->>VM: DailyEmotion 방출
else fresh
Repo-->>UC: 현재 값 반환 (no network)
UC-->>VM: DailyEmotion 방출
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
시
🚥 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.kt (1)
21-28:⚠️ Potential issue | 🟠 Major
fetchedDate는 변환 시각이 아니라 요청 기준 일자를 사용해야 합니다.27번 줄에서
LocalDate.now()를 호출하면,EmotionRepositoryImpl.kt46번 줄에서 고정한 요청 일자와 어긋날 수 있습니다. 자정 전후에 응답이 돌아오면 전날 데이터를 받아놓고도fetchedDate가 오늘 날짜로 기록되어isStale이false가 되고, 다음 구독에서 재조회가 차단됩니다. 요청에 사용한LocalDate를toDomain(requestedDate: LocalDate)매개변수로 전달하는 방식이 안전합니다.수정 예시
-fun DailyEmotionResponse.toDomain(): DailyEmotion = +fun DailyEmotionResponse.toDomain(fetchedDate: LocalDate): DailyEmotion = DailyEmotion( type = emotionMarbleType, name = emotionMarbleName, imageUrl = imageUrl, homeMessage = emotionMarbleHomeMessage, - fetchedDate = LocalDate.now(), + fetchedDate = fetchedDate, )- val currentDate = LocalDate.now().toString() - emotionDataSource.fetchDailyEmotion(currentDate).map { - _dailyEmotionFlow.value = it.toDomain() + val requestedDate = LocalDate.now() + emotionDataSource.fetchDailyEmotion(requestedDate.toString()).map { + _dailyEmotionFlow.value = it.toDomain(requestedDate) }🤖 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/emotion/model/response/DailyEmotionResponse.kt` around lines 21 - 28, DailyEmotionResponse.toDomain currently sets fetchedDate = LocalDate.now(), which can diverge from the request date used in EmotionRepositoryImpl and cause stale-check bugs; change the conversion to accept a requestedDate parameter (e.g., fun DailyEmotionResponse.toDomain(requestedDate: LocalDate)): construct DailyEmotion with fetchedDate = requestedDate, update all callers (notably where EmotionRepositoryImpl maps responses at the request site) to pass the same requestedDate used for the request, and ensure the DailyEmotion data class usage remains consistent.
🤖 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/emotion/repositoryImpl/EmotionRepositoryImpl.kt`:
- Around line 20-25: The current compareAndSet in isFetching causes a concurrent
fetch to short-circuit and return Result.success(Unit), which can drop a
subsequent refresh triggered by registerEmotion() or a subscription's initial
fetch; change the fetch control in fetchDailyEmotion() (the routine guarded by
isFetching.compareAndSet) so that when a fetch is already in progress it either
queues a single follow-up fetch or serializes requests: e.g., replace the
immediate success return with a flag/queue (e.g., pendingRefresh boolean) that
is set when a fetch is in progress and a new fetch is requested, and after the
in-flight fetch completes have the code check pendingRefresh and perform one
additional fetch if needed; update references to isFetching, compareAndSet,
fetchDailyEmotion(), and registerEmotion() to use this handshake so a trailing
refresh is not lost.
---
Outside diff comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.kt`:
- Around line 21-28: DailyEmotionResponse.toDomain currently sets fetchedDate =
LocalDate.now(), which can diverge from the request date used in
EmotionRepositoryImpl and cause stale-check bugs; change the conversion to
accept a requestedDate parameter (e.g., fun
DailyEmotionResponse.toDomain(requestedDate: LocalDate)): construct DailyEmotion
with fetchedDate = requestedDate, update all callers (notably where
EmotionRepositoryImpl maps responses at the request site) to pass the same
requestedDate used for the request, and ensure the DailyEmotion data class usage
remains consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52f22984-cc82-4ed7-bda4-56d2b9b2db63
📒 Files selected for processing (9)
data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/model/DailyEmotion.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/model/EmotionChangeEvent.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/repository/EmotionRepository.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/GetEmotionChangeEventFlowUseCase.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/ObserveDailyEmotionUseCase.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/recommendroutine/RecommendRoutineViewModel.kt
💤 Files with no reviewable changes (2)
- domain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/GetEmotionChangeEventFlowUseCase.kt
- domain/src/main/java/com/threegap/bitnagil/domain/emotion/model/EmotionChangeEvent.kt
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt
Show resolved
Hide resolved
l5x5l
reviewed
Mar 9, 2026
|
|
||
| init { | ||
| initialize() | ||
| observeDailyEmotion() |
Contributor
There was a problem hiding this comment.
[2] RecommendRoutineViewModel에서는 container의 onCreate 블록에 observeDailyEmotion을 배치했는데, 여기에서는 init 블록에 배치한 이유가 있으신가요?
Member
Author
There was a problem hiding this comment.
기술적인 이유보단 homeViewmodel에서는 아직 변경하지 않은 로직이 남아있어서 onCreate와 init 각각 정의하면 혼동할 수 가능성 때문에 기존 init에 정의하고 추후 다른 변경사항과 함께 onCreate로 변경하고자 했숩니다~!
l5x5l
reviewed
Mar 9, 2026
Contributor
l5x5l
left a comment
l5x5l
left a comment
There was a problem hiding this comment.
Event를 각 ViewModel에서 감지해 감정 데이터 조회를 호출하는 공통 로직을 제거한 것은 좋아 보입니다! 👍
다만 수정해주신 코드에서 DailyEmotionResponse의 toDomain이나 DailyEmotion의 isStale 같이 LocalDate를 사용하는 부분을 보면 해당 정보를 인자로 전달받아 사용하기 보다는 함수 내부에서 매번 LocalData.now()를 호출하고 있어 호출 시점마다 결과가 달라질 수 있는 비결정적 코드라는 느낌을 받았습니다...!
이 부분만 외부에서 LocalDate를 주입받아 사용할 수 있도록 변경해주시면 추후 테스트시 또는 기능 유지보수시에 더 좋을 것 같아요!
Member
Author
수정완료! -> 3edbfd1 |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt (1)
20-25:⚠️ Potential issue | 🟠 Major진행 중 fetch를 성공으로 단락하면 최신 감정 갱신이 유실됩니다.
Line 44에서 이미 fetch 중이면 바로
Result.success(Unit)을 반환해서, Line 39의 후행 refresh가 Lines 23-25의 초기 구독 fetch와 겹칠 때 그대로 드롭됩니다. 첫 요청이 등록 이전 상태를 읽었다면_dailyEmotionFlow는 같은 날짜 동안 stale로 간주되지 않아 오래된 값을 유지할 수 있습니다. 중복 호출만 막지 말고, 진행 중 요청 뒤에 refresh 1회를 보장하도록 직렬화하거나 pending 플래그를 두는 쪽이 안전합니다.예시 수정안
-import java.util.concurrent.atomic.AtomicBoolean +import kotlinx.coroutines.sync.Mutex - private val isFetching = AtomicBoolean(false) + private val fetchMutex = Mutex() + `@Volatile` + private var pendingRefresh = false override suspend fun fetchDailyEmotion(): Result<Unit> { - if (!isFetching.compareAndSet(false, true)) return Result.success(Unit) + if (!fetchMutex.tryLock()) { + pendingRefresh = true + return Result.success(Unit) + } return try { - val today = LocalDate.now() - emotionDataSource.fetchDailyEmotion(today.toString()).map { - _dailyEmotionFlow.value = it.toDomain(today) - } + var result: Result<Unit> + do { + pendingRefresh = false + val today = LocalDate.now() + result = emotionDataSource.fetchDailyEmotion(today.toString()).map { + _dailyEmotionFlow.value = it.toDomain(today) + } + } while (pendingRefresh) + result } finally { - isFetching.set(false) + fetchMutex.unlock() } }Also applies to: 39-52
🤖 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/emotion/repositoryImpl/EmotionRepositoryImpl.kt` around lines 20 - 25, The current concurrency handling in EmotionRepositoryImpl uses isFetching to short-circuit fetchDailyEmotion and returns early, which can drop a later refresh requested from onSubscription and leave _dailyEmotionFlow stale; update fetchDailyEmotion (and related callers) so that when isFetching is true you record a pending-refresh flag (or enqueue one) instead of returning success immediately, and when the in-flight fetch completes the code checks that flag to run exactly one additional fetch (or emit a refresh) and clear it; reference isFetching, fetchDailyEmotion, _dailyEmotionFlow and dailyEmotionFlow to locate where to add the pending flag/serialization and ensure onSubscription-triggered fetches are not lost.
🤖 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/emotion/repositoryImpl/EmotionRepositoryImpl.kt`:
- Around line 20-25: The current concurrency handling in EmotionRepositoryImpl
uses isFetching to short-circuit fetchDailyEmotion and returns early, which can
drop a later refresh requested from onSubscription and leave _dailyEmotionFlow
stale; update fetchDailyEmotion (and related callers) so that when isFetching is
true you record a pending-refresh flag (or enqueue one) instead of returning
success immediately, and when the in-flight fetch completes the code checks that
flag to run exactly one additional fetch (or emit a refresh) and clear it;
reference isFetching, fetchDailyEmotion, _dailyEmotionFlow and dailyEmotionFlow
to locate where to add the pending flag/serialization and ensure
onSubscription-triggered fetches are not lost.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5f7366c-1d2e-4fa6-aeff-8c044aa64d10
📒 Files selected for processing (3)
data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/model/DailyEmotion.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 ]
스크럼 시간에 언급했던 flow 기반의 데이터 스트림 방식을 DailyEmotion(오늘의 감정) 부분에 먼저 적용을 해봤습니다.
작업 문서는 노션에 띠로 정리했습니다.! -> 🔗 노션 링크
Related issue
Screenshot 📸
Work Description
To Reviewers 📢
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
제거