-
Notifications
You must be signed in to change notification settings - Fork 1
[BUGFIX] 오늘의 운세 생성에 실패할 시 다음날이 되기 전까지 계속 운세 생성에 실패하는 문제 #279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
개요WorkManager 기반 백그라운드 작업 스케줄링을 제거하고 직접 운세 생성 처리로 전환하는 아키텍처 개선. 운세 생성 상태 모델을 단순화하고 새로운 in-memory 추적 시스템을 도입하며, 복잡한 상태 머신 로직을 제거합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant AlarmService
participant UserInfoRepository
participant FortuneRepository
participant FortuneCreationTracker
participant FortunePreferences
Note over AlarmService: 알람 트리거 수신
rect rgb(200, 220, 255)
Note over AlarmService,FortunePreferences: 기존: WorkManager 스케줄링 (async)
AlarmService->>AlarmService: PostFortuneTaskScheduler.enqueueOnceForToday()
Note over AlarmService: WorkManager가 PostFortuneWorker 실행<br/>(백그라운드 작업)
end
rect rgb(200, 255, 220)
Note over AlarmService,FortunePreferences: 신규: 직접 동기 처리
AlarmService->>FortunePreferences: hasTodayFortune() 확인
FortunePreferences-->>AlarmService: 오늘 운세 여부 반환
alt 오늘 운세 없음
AlarmService->>UserInfoRepository: userIdFlow.first()로 userId 조회
UserInfoRepository-->>AlarmService: userId 반환
AlarmService->>FortuneCreationTracker: start()
FortuneCreationTracker->>FortuneCreationTracker: state = Start 설정
AlarmService->>FortuneRepository: postFortune(userId)
FortuneRepository-->>AlarmService: fortune (id, score)
AlarmService->>FortuneRepository: markFortuneAsCreated(fortuneId)
FortuneRepository->>FortunePreferences: markFortuneCreated(fortuneId)
AlarmService->>FortuneRepository: saveFortuneScore(score)
AlarmService->>FortuneCreationTracker: succeed(fortuneId)
FortuneCreationTracker->>FortuneCreationTracker: state = Success(fortuneId) 설정
else 포스팅 실패
AlarmService->>FortuneCreationTracker: fail()
FortuneCreationTracker->>FortuneCreationTracker: state = Failure 설정
end
end
Note over AlarmService,FortuneCreationTracker: UI는 FortuneCreationTracker.state<br/>구독으로 상태 반영
코드 리뷰 예상 소요 시간🎯 4 (복잡) | ⏱️ ~60분 주의 깊게 검토해야 할 영역
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/yapp/orbit/OrbitApplication.kt (1)
8-12: WorkManager 제거 이후Application구성 단순화 적절해 보입니다
Configuration.Provider/HiltWorkerFactory를 제거하고 일반Application서브클래스로만 둔 선택은, PR 설명대로 WorkManager 기반 트리거를 걷어낸 방향성과 잘 맞습니다. 현재 보이는 범위에서는 추가적인 초기화 누락이나 DI 설정 문제는 없어 보입니다.추가로, 아래처럼 Kotlin 관용 스타일로 괄호를 정리해도 좋을 것 같습니다 (완전 선택 사항입니다).
-@HiltAndroidApp -class OrbitApplication() : Application() { +@HiltAndroidApp +class OrbitApplication : Application() {core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt (1)
16-66: 운세가 생성되지 않은 경우의 테스트 추가를 고려해 주세요.현재 테스트는 운세 생성 후의 시나리오를 다루고 있습니다. 운세가 한 번도 생성되지 않았을 때
hasTodayFortune()이 false를 반환하는지 확인하는 테스트를 추가하면 더 완전한 커버리지를 확보할 수 있습니다.@Test fun `운세가_생성되지_않았다면_hasTodayFortune이_거짓이다`() = runTest { // given val dataStore = createNewDataStoreWithFile("empty.preferences_pb") val preferences = createFortunePreferencesWithClock(dataStore, fixedClockAtT0) // when & then val result = preferences.hasTodayFortune() assertFalse(result) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
app/build.gradle.kts(0 hunks)app/src/main/AndroidManifest.xml(0 hunks)app/src/main/java/com/yapp/orbit/OrbitApplication.kt(1 hunks)build-logic/src/main/java/com/yapp/convention/HiltAndroid.kt(0 hunks)core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt(5 hunks)core/alarm/src/main/java/com/yapp/alarm/scheduler/PostFortuneTaskScheduler.kt(0 hunks)core/alarm/src/main/java/com/yapp/alarm/services/AlarmService.kt(4 hunks)core/common/src/main/java/com/yapp/common/di/FortuneTrackerModule.kt(1 hunks)core/common/src/main/java/com/yapp/common/tracker/InMemoryFortuneCreationTracker.kt(1 hunks)core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt(3 hunks)core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt(3 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt(1 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt(2 hunks)data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt(2 hunks)domain/src/main/java/com/yapp/domain/model/FortuneCreateStatus.kt(0 hunks)domain/src/main/java/com/yapp/domain/model/FortuneCreationState.kt(1 hunks)domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt(2 hunks)domain/src/main/java/com/yapp/domain/tracker/FortuneCreationTracker.kt(1 hunks)feature/fortune/build.gradle.kts(0 hunks)feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt(3 hunks)feature/fortune/src/main/java/com/yapp/fortune/di/SchedulerModule.kt(0 hunks)feature/fortune/src/main/java/com/yapp/fortune/scheduler/WorkManagerPostFortuneTaskScheduler.kt(0 hunks)feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt(0 hunks)feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt(1 hunks)gradle/libs.versions.toml(1 hunks)
💤 Files with no reviewable changes (9)
- app/build.gradle.kts
- core/alarm/src/main/java/com/yapp/alarm/scheduler/PostFortuneTaskScheduler.kt
- feature/fortune/src/main/java/com/yapp/fortune/scheduler/WorkManagerPostFortuneTaskScheduler.kt
- feature/fortune/build.gradle.kts
- build-logic/src/main/java/com/yapp/convention/HiltAndroid.kt
- app/src/main/AndroidManifest.xml
- domain/src/main/java/com/yapp/domain/model/FortuneCreateStatus.kt
- feature/fortune/src/main/java/com/yapp/fortune/di/SchedulerModule.kt
- feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: DongChyeon
Repo: YAPP-Github/Orbit-Android PR: 252
File: domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt:16-17
Timestamp: 2025-09-14T15:32:44.064Z
Learning: Fortune creation requests have at least a 1-minute gap between them in the Orbit Android app, making atomic guards for race condition prevention unnecessary in the fortune creation flow.
📚 Learning: 2025-09-14T15:32:44.064Z
Learnt from: DongChyeon
Repo: YAPP-Github/Orbit-Android PR: 252
File: domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt:16-17
Timestamp: 2025-09-14T15:32:44.064Z
Learning: Fortune creation requests have at least a 1-minute gap between them in the Orbit Android app, making atomic guards for race condition prevention unnecessary in the fortune creation flow.
Applied to files:
domain/src/main/java/com/yapp/domain/tracker/FortuneCreationTracker.ktcore/common/src/main/java/com/yapp/common/tracker/InMemoryFortuneCreationTracker.ktfeature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.ktcore/alarm/src/main/java/com/yapp/alarm/services/AlarmService.ktdomain/src/main/java/com/yapp/domain/repository/FortuneRepository.ktdata/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.ktcore/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.ktcore/datastore/src/main/java/com/yapp/datastore/FortunePreferences.ktdata/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.ktfeature/mission/src/main/java/com/yapp/mission/MissionViewModel.ktdata/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.ktdomain/src/main/java/com/yapp/domain/model/FortuneCreationState.kt
📚 Learning: 2025-09-18T04:28:52.855Z
Learnt from: DongChyeon
Repo: YAPP-Github/Orbit-Android PR: 257
File: core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt:70-77
Timestamp: 2025-09-18T04:28:52.855Z
Learning: hasUnseenFortuneFlow와 shouldShowFortuneToolTipFlow는 지속적인 관찰이 아닌 호출 시점의 상태 판단용 Flow이므로, 자정 롤오버 대응을 위한 ticker Flow 결합이 불필요함. Flow가 collect될 때마다 todayEpoch()가 현재 시점으로 계산되어 호출 시점 판단에 충분함.
Applied to files:
feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.ktdomain/src/main/java/com/yapp/domain/repository/FortuneRepository.ktdata/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.ktcore/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.ktcore/datastore/src/main/java/com/yapp/datastore/FortunePreferences.ktdata/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.ktfeature/mission/src/main/java/com/yapp/mission/MissionViewModel.ktdata/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt
📚 Learning: 2025-09-15T07:43:50.275Z
Learnt from: DongChyeon
Repo: YAPP-Github/Orbit-Android PR: 254
File: data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt:22-34
Timestamp: 2025-09-15T07:43:50.275Z
Learning: FortuneCreateStatusFlow에서 todayEpoch()를 combine 내부에서 직접 호출하는 것이 충분한 이유: 운세 생성할 때마다 fortuneDateEpochFlow가 변경되어 combine이 재평가되므로, 그 순간의 todayEpoch() 계산으로 충분함. 운세 요청 간격 제약으로 인해 자정 롤오버 문제는 실용적으로 발생하지 않음.
Applied to files:
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.ktdata/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt
🧬 Code graph analysis (1)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (1)
core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)
todayEpoch(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (20)
feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (1)
141-144: 로직 단순화가 잘 되었습니다.
FortuneCreateStatus열거형 대신 boolean 기반 접근 방식으로 변경하여 가독성이 향상되었습니다.!hasTodayFortune || hasUnseenFortune로직은 운세 생성이 필요하거나 미확인 운세가 있는 경우를 모두 올바르게 처리합니다.core/common/src/main/java/com/yapp/common/tracker/InMemoryFortuneCreationTracker.kt (1)
11-27: 구현이 깔끔합니다.
StateFlow를 사용한 간결한 in-memory 상태 추적 구현입니다.StateFlow의 스레드 안전성과 학습된 정보(운세 생성 요청 간 최소 1분 간격)에 따라 추가적인 동기화 메커니즘이 필요하지 않습니다.core/common/src/main/java/com/yapp/common/di/FortuneTrackerModule.kt (1)
11-19: DI 설정이 올바릅니다.싱글톤 스코프로
InMemoryFortuneCreationTracker를FortuneCreationTracker인터페이스에 바인딩하는 표준적인 Dagger/Hilt 패턴입니다.domain/src/main/java/com/yapp/domain/model/FortuneCreationState.kt (1)
3-7: 상태 모델링이 적절합니다.운세 생성 라이프사이클을 표현하는 명확한 sealed class 설계입니다.
Success가fortuneId를 포함하고, 세 가지 상태(Start, Success, Failure)가 모든 시나리오를 커버합니다.domain/src/main/java/com/yapp/domain/tracker/FortuneCreationTracker.kt (1)
6-11: 인터페이스 설계가 명확합니다.
StateFlow를 통한 반응형 관찰과 상태 전환을 위한 메서드들이 잘 정의되어 있습니다. 도메인 레이어에 적합한 간결한 API입니다.data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)
5-24: 인터페이스 단순화가 적절합니다.
FortuneCreateStatus기반의 복잡한 라이프사이클 메서드들을 제거하고 데이터 영속성에 집중하도록 변경되었습니다. 생성 상태는FortuneCreationTracker로 분리되고, 이 인터페이스는 운세 데이터(ID, 날짜, 이미지, 점수 등)만 관리합니다. 이는 PR 목표인 "실패 상태가 다음날까지 지속되는 문제"를 해결하는 핵심 변경사항입니다.domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (1)
6-28: 도메인 API 단순화가 아키텍처 목표와 일치합니다.
FortuneCreateStatus기반의 복잡한 상태 관리를 제거하고 간결한 boolean 체크(hasTodayFortune())와 단순화된 생성 메서드(markFortuneAsCreated(fortuneId))로 대체되었습니다. 이는 영속적인 생성 상태를 in-memory tracker로 분리하는 PR의 핵심 아키텍처 변경을 올바르게 반영합니다.추가된 메서드들(
saveFortuneScore,markFirstAlarmDismissedToday)은 운세 데이터 관리를 위한 정상적인 확장입니다.core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)
53-85: State-based navigation logic is appropriate.The branching logic by
FortuneCreationStateis clear:
Start: Fortune creation is needed, so navigate to fortune screen to trigger creationSuccess: Navigate only if there are unseen fortunesFailure: Error occurred, no navigation (user can retry manually)However, the referenced verification about how the fortune UI layer (
FortuneViewModelor screen) handles reactive state observation could not be completed. The fortune screen should observetracker.statereactively to handle state transitions that occur while the screen is displayed, particularly if theAlarmServicecreates fortunes concurrently. Ensure the UI layer properly manages any timing edge cases where state transitions occur between the snapshot taken with.first()and screen display.data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1)
24-34: LGTM! 간결하게 잘 단순화되었습니다.복잡한 다단계 운세 생성 라이프사이클(
markFortuneAsCreating,markFortuneAsCreated(attemptId, fortuneId),markFortuneAsFailed)이 두 개의 간단한 메서드로 깔끔하게 대체되었습니다. 이 변경은 PR 목표인 DataStore 기반 상태 관리를 단순화하는 것과 잘 일치합니다.feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (2)
29-30: 의존성 주입이 적절하게 구성되었습니다.
FortuneCreationTracker가 생성자 주입으로 추가되어 Hilt DI 패턴과 일관성을 유지합니다.
55-79: No issues identified. TheFortuneCreationTrackerproperly initializes state asStateFlow<FortuneCreationState>(FortuneCreationState.Start), which guarantees immediate emission to collectors. Indefinite waiting cannot occur.data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (2)
18-20: LGTM! 단순화된 API로 깔끔하게 변경되었습니다.기존의 복잡한 attemptId 기반 로직이 제거되고, 단순한 위임 패턴으로 대체되었습니다.
46-48: LGTM!
hasTodayFortune()메서드가 적절하게 추가되었습니다. DataStore에서 오늘 운세 존재 여부를 확인하는 간단한 쿼리입니다.core/alarm/src/main/java/com/yapp/alarm/services/AlarmService.kt (2)
91-91: LGTM! suspend 함수로의 변경이 적절합니다.
handleIntent가 이제 비동기 호출(hasTodayFortune(),userIdFlow.first(),postFortune()등)을 수행하므로 suspend 함수로 변경된 것이 올바릅니다.serviceScope.launch내에서 호출되어 적절하게 처리됩니다.
57-64: LGTM! 새로운 의존성 주입이 적절합니다.WorkManager 기반 스케줄러 대신 직접 운세 생성을 수행하기 위한
FortuneRepository,UserInfoRepository,FortuneCreationTracker의존성이 추가되었습니다. PR 목표에 부합하는 변경입니다.core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt (2)
39-51: LGTM! 핵심 케이스가 잘 테스트되었습니다.오늘 운세 생성 후
hasTodayFortune()이 true를 반환하는지 검증하는 명확한 테스트입니다.
53-66: LGTM! 날짜 경계 로직이 잘 테스트되었습니다.고정된 Clock을 사용하여 날짜가 변경되었을 때
hasTodayFortune()이 false를 반환하는지 검증합니다. 이는 PR에서 수정하려는 버그(운세 생성 실패 시 다음 날까지 계속 실패)의 핵심 로직입니다.core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (2)
85-99: 간결하고 명확한 구현입니다.
isNewForToday조건이 ID 변경 또는 날짜 변경을 모두 체크하여 운세가 새로운지 판단하는 로직이 적절합니다. DataStore의edit이 atomic하게 동작하므로 상태 일관성도 보장됩니다. 기존의 복잡한 attemptId/expiration 로직을 제거하고 단순화한 것이 버그 수정 목적에 부합합니다.
134-138: 적절한 일회성 상태 조회 구현입니다.
hasTodayFortune()이 Flow 구독 대신first()를 사용하여 호출 시점의 상태를 즉시 반환하는 방식은 ViewModel에서 네비게이션 결정에 활용하기에 적합합니다. 학습 내용에 따르면, 호출 시점의 상태 판단용이므로 이 접근법이 적절합니다.gradle/libs.versions.toml (1)
38-41: Compose BOM version exists but check library version overridesCompose BOM 2025.02.00 is a valid stable release (published Feb 12, 2025). However, the individually pinned library versions (compose-navigation 2.8.5, compose-material3 1.4.0, compose-ui 1.8.3) appear to override the BOM's pinned versions and may not be compatible with the stable 2025.02.00 BOM, which targets Compose 1.7.8. When using a BOM, library versions are typically omitted to rely on the BOM's curated compatibility set, or all overrides must be verified for compatibility.
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (5.33%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #279 +/- ##
============================================
- Coverage 6.24% 5.33% -0.92%
+ Complexity 67 58 -9
============================================
Files 53 53
Lines 4422 4352 -70
Branches 774 751 -23
============================================
- Hits 276 232 -44
+ Misses 4112 4106 -6
+ Partials 34 14 -20
🚀 New features to boost your workflow:
|
|
운세 생성은 GlobalScope 아니면 Custom Scope에서 실행하자 |
Related issue 🛠
closed #278
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
기존 알람–운세 생성 흐름은
이 구조는 DataStore와 WorkManager 사이의 비동기 경합, 상태 불일치, 워커 재실행 시의 예외 처리 문제 등 다양한 자잘한 버그를 유발했습니다.
WorkManager를 통한 재시도 로직보다는 실패했을 경우 다음 알람에서 운세를 받아볼 수 있도록 하기로 했습니다.
FortuneCreationTracker에서 인메모리 브로드캐스트hasTodayFortune/hasUnseenFortune두 값만으로 운세 화면 진입 여부 판단이번 변경으로 DataStore는 다음 정보만 관리합니다:
Why
Uncompleted Tasks 😅
N/A
To Reviewers 📢
Summary by CodeRabbit
릴리즈 노트
개선 사항
의존성 업데이트
✏️ Tip: You can customize this high-level summary in your review settings.