-
Notifications
You must be signed in to change notification settings - Fork 1
[FEAT] 알람 스키마 변경/마이그레이션 및 database 모듈 분리 #230
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
[FEAT] 알람 스키마 변경/마이그레이션 및 database 모듈 분리 #230
Conversation
|
""" Walkthrough알람 DB 스키마에 missionType(enum-int)과 missionCount 컬럼이 추가되고, 이에 맞는 마이그레이션 정책이 적용되었습니다. AlarmDatabase 관련 코드가 core:database 모듈로 분리되었으며, TypeConverter와 enum 기반 MissionType 정의, 테스트 및 마이그레이션 코드, build 설정 등도 함께 반영되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant DataModule
participant DatabaseModule
App->>DataModule: 알람 저장/조회 요청
DataModule->>DatabaseModule: AlarmDao/AlarmDatabase 접근 (missionType, missionCount 포함)
DatabaseModule-->>DataModule: 결과 반환 (enum 변환 포함)
DataModule-->>App: 결과 반환
sequenceDiagram
participant OldDB(v1)
participant Migration
participant NewDB(v2)
OldDB(v1)->>Migration: v1 DB 데이터
Migration->>NewDB(v2): missionType=TAP, missionCount=10 기본값 추가 후 마이그레이션
NewDB(v2)-->>Migration: 마이그레이션 완료
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
🔇 Additional comments (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🔭 Outside diff range comments (2)
core/database/src/main/java/com/yapp/database/AlarmDao.kt (1)
19-35: TABLE 명 대신 DATABASE 명을 사용하고 있습니다Room @query 에서는 테이블명을 지정해야 합니다. 현재
${AlarmDatabase.DATABASE_NAME}(데이터베이스 파일명으로 추정)을 사용하고 있어 런타임에서 “no such table” 오류가 발생할 수 있습니다.-@Query("UPDATE ${AlarmDatabase.DATABASE_NAME} SET isAlarmActive = :active WHERE id = :id") +@Query("UPDATE ${AlarmEntity.TABLE_NAME} SET isAlarmActive = :active WHERE id = :id") -@Query("SELECT * FROM ${AlarmDatabase.DATABASE_NAME} WHERE id = :id") +@Query("SELECT * FROM ${AlarmEntity.TABLE_NAME} WHERE id = :id") -@Query("SELECT * FROM ${AlarmDatabase.DATABASE_NAME} ORDER BY isAm DESC, hour ASC, minute ASC LIMIT :limit OFFSET :offset") +@Query("SELECT * FROM ${AlarmEntity.TABLE_NAME} ORDER BY isAm DESC, hour ASC, minute ASC LIMIT :limit OFFSET :offset")동일한 실수가 다른 쿼리에도 반복되어 있으니 전반적으로 교체가 필요합니다.
core/database/src/main/java/com/yapp/database/AlarmEntity.kt (1)
39-73: 매핑 함수에서 새로운 필드들이 누락되었습니다.
toDomain()과toEntity()매핑 함수에서 새로 추가된missionType과missionCount필드들이 포함되지 않았습니다. 이는 데이터 무결성 문제를 야기할 수 있습니다.다음과 같이 수정하세요:
fun AlarmEntity.toDomain() = Alarm( id = id, isAm = isAm, hour = hour, minute = minute, second = second, repeatDays = repeatDays, isHolidayAlarmOff = isHolidayAlarmOff, isSnoozeEnabled = isSnoozeEnabled, snoozeInterval = snoozeInterval, snoozeCount = snoozeCount, isVibrationEnabled = isVibrationEnabled, isSoundEnabled = isSoundEnabled, soundUri = soundUri, soundVolume = soundVolume, isAlarmActive = isAlarmActive, + missionType = missionType, + missionCount = missionCount, ) fun Alarm.toEntity() = AlarmEntity( id = id, isAm = isAm, hour = hour, minute = minute, second = second, repeatDays = repeatDays, isHolidayAlarmOff = isHolidayAlarmOff, isSnoozeEnabled = isSnoozeEnabled, snoozeInterval = snoozeInterval, snoozeCount = snoozeCount, isVibrationEnabled = isVibrationEnabled, isSoundEnabled = isSoundEnabled, soundUri = soundUri, soundVolume = soundVolume, isAlarmActive = isAlarmActive, + missionType = missionType, + missionCount = missionCount, )
🧹 Nitpick comments (11)
core/media/src/main/java/com/yapp/media/storage/ImageSaver.kt (1)
16-16: 메서드 시그니처 개선이 적절합니다.
fileName매개변수 추가로 파일명 제어가 가능해졌고, suspend 제거는 동기적 특성을 명확하게 보여줍니다. 하지만 인터페이스를 구현하지 않게 되어 테스트 가능성이 저하될 수 있습니다.테스트 가능성을 위해
ImageSaver인터페이스 도입을 고려해보세요:interface ImageSaver { fun saveImage(byteArray: ByteArray, fileName: String): Boolean } class ImageSaverImpl @Inject constructor( private val contentResolver: ContentResolver, ) : ImageSaver { // 기존 구현 }data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSource.kt (1)
3-3: 엔티티 레이어 노출 여부 재검토 권장
AlarmLocalDataSource인터페이스가AlarmEntity(DB 엔티티)를 직접 파라미터·리턴 타입으로 사용하고 있습니다. 데이터 계층 내부용이라면 괜찮지만, 외부(도메인·프레젠테이션) 모듈까지 노출된다면 아키텍처 계층 간 의존성이 뒤틀릴 수 있습니다. 필요하다면 DTO 또는 매퍼를 통해 분리하는 방안을 고려해 주세요.core/network/src/main/java/com/yapp/network/model/ApiError.kt (1)
1-5:data class+Exception조합은 직렬화 이슈 가능성Kotlin
data class가 자동 생성하는copy,componentN등이 예외 객체로서 의미가 크지 않고, 불필요하게 스택트레이스가 복제될 위험이 있습니다. 일반class ApiError(message: String) : Exception(message)로 단순화하는 편이 안전합니다.data/src/main/java/com/yapp/data/remote/datasource/SignUpDataSourceImpl.kt (1)
20-25: 로깅 레벨 및 빌드 타입 분기 추가 필요
Log.d는 릴리스 빌드에서도 남아 있을 수 있으므로,if (BuildConfig.DEBUG)조건 또는 Timber 등을 사용해 릴리스 로그 노출을 방지해 주세요.core/database/src/main/AndroidManifest.xml (1)
1-4: 불필요한 빈 매니페스트는 제거를 고려해 주세요
android { namespace = "com.yapp.database" }를build.gradle.kts에 선언해 두셨다면, 라이브러리 모듈은 매니페스트가 전혀 없어도 빌드-타임에 자동 생성됩니다. 파일을 삭제하면 유지보수 대상이 줄어듭니다.core/database/src/test/java/com/yapp/database/ExampleUnitTest.kt (1)
11-15: 보일러플레이트 테스트는 실제 DB 테스트로 교체 권장
2 + 2 = 4테스트는 품질 보장에 기여하지 않습니다. DAO, TypeConverter, Migration 등의 단위 테스트로 대체해 주세요. 필요하면 샘플 코드 제공 가능합니다.core/database/proguard-rules.pro (1)
1-21: Room 관련 keep 룰 추가 검토 필요
현재 템플릿만 존재합니다. 릴리즈 빌드 시 Room 엔티티/DAO가 난독화되어 런타임 오류가 날 수 있으므로-keep class androidx.room.** { *; }와 같은 규칙 추가를 고려해 주세요.
core/database/src/main/java/com/yapp/database/MissionTypeConverter.kt (1)
13-16: 메서드 네이밍 일관성 개선타입 컨버터 메서드명을 더 명확하게 하는 것이 좋겠습니다.
@TypeConverter -fun toMissionType(missionType: MissionType): String { +fun toString(missionType: MissionType): String { return missionType.name }feature/mission/src/main/java/com/yapp/mission/MissionScreen.kt (1)
230-231: 미션 라벨 텍스트 로직 개선동일한 enum 비교가 두 번 수행되고 있습니다. 성능을 위해 한 번만 비교하는 것이 좋겠습니다.
@Composable fun MissionLabel(state: MissionContract.State) { + val isShakeMission = state.missionType == MissionType.SHAKE - val instruction = - if (state.missionType == MissionType.SHAKE) "10회를 흔들어 부적을 뒤집어줘" else "10회를 눌러 편지를 열어줘" - val count = if (state.missionType == MissionType.SHAKE) state.shakeCount else state.clickCount + val instruction = if (isShakeMission) "10회를 흔들어 부적을 뒤집어줘" else "10회를 눌러 편지를 열어줘" + val count = if (isShakeMission) state.shakeCount else state.clickCountdomain/src/main/java/com/yapp/domain/model/MissionType.kt (1)
4-5: enum 순서 일관성을 고려해보세요.원격 값 매핑에서 "tap_mission"이 먼저 나오므로, enum 순서도 TAP을 먼저 배치하는 것이 일관성 있어 보입니다.
core/network/src/main/java/com/yapp/network/utils/ApiCallUtils.kt (1)
16-16: 네트워크 예외 메시지 개선을 고려해보세요.IOException에 대한 메시지를 더 구체적으로 만들어보는 것은 어떨까요? 예: "네트워크 연결을 확인해주세요"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
app/build.gradle.kts(1 hunks)build-logic/src/main/java/com/yapp/convention/TestAndroid.kt(2 hunks)build-logic/src/main/java/orbit.android.feature.gradle.kts(0 hunks)build.gradle.kts(1 hunks)core/database/.gitignore(1 hunks)core/database/build.gradle.kts(1 hunks)core/database/proguard-rules.pro(1 hunks)core/database/schemas/com.yapp.database.AlarmDatabase/1.json(1 hunks)core/database/schemas/com.yapp.database.AlarmDatabase/2.json(1 hunks)core/database/src/androidTest/java/com/yapp/database/MigrationTest.kt(1 hunks)core/database/src/main/AndroidManifest.xml(1 hunks)core/database/src/main/java/com/yapp/database/AlarmDao.kt(1 hunks)core/database/src/main/java/com/yapp/database/AlarmDatabase.kt(1 hunks)core/database/src/main/java/com/yapp/database/AlarmEntity.kt(2 hunks)core/database/src/main/java/com/yapp/database/DatabaseMigrations.kt(1 hunks)core/database/src/main/java/com/yapp/database/MissionTypeConverter.kt(1 hunks)core/database/src/main/java/com/yapp/database/di/DatabaseModule.kt(2 hunks)core/database/src/test/java/com/yapp/database/ExampleUnitTest.kt(1 hunks)core/media/src/main/java/com/yapp/media/di/MediaModule.kt(2 hunks)core/media/src/main/java/com/yapp/media/storage/ImageSaver.kt(3 hunks)core/network/src/main/java/com/yapp/network/model/ApiError.kt(1 hunks)core/network/src/main/java/com/yapp/network/utils/ApiCallUtils.kt(1 hunks)data/build.gradle.kts(1 hunks)data/src/main/java/com/yapp/data/di/RepositoryModule.kt(0 hunks)data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSource.kt(1 hunks)data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSourceImpl.kt(1 hunks)data/src/main/java/com/yapp/data/local/datasource/ImageLocalDataSource.kt(0 hunks)data/src/main/java/com/yapp/data/remote/datasource/FortuneDataSourceImpl.kt(1 hunks)data/src/main/java/com/yapp/data/remote/datasource/SignUpDataSourceImpl.kt(1 hunks)data/src/main/java/com/yapp/data/remote/datasource/UserInfoDataSourceImpl.kt(1 hunks)data/src/main/java/com/yapp/data/remote/utils/ApiCallUtils.kt(0 hunks)data/src/main/java/com/yapp/data/repositoryimpl/AlarmRepositoryImpl.kt(1 hunks)data/src/main/java/com/yapp/data/repositoryimpl/ImageRepositoryImpl.kt(0 hunks)domain/src/main/java/com/yapp/domain/model/MissionType.kt(1 hunks)domain/src/main/java/com/yapp/domain/repository/ImageRepository.kt(0 hunks)feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt(3 hunks)feature/mission/src/main/java/com/yapp/mission/MissionContract.kt(1 hunks)feature/mission/src/main/java/com/yapp/mission/MissionScreen.kt(5 hunks)feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt(2 hunks)gradle.properties(1 hunks)gradle/libs.versions.toml(3 hunks)settings.gradle.kts(1 hunks)
💤 Files with no reviewable changes (6)
- build-logic/src/main/java/orbit.android.feature.gradle.kts
- data/src/main/java/com/yapp/data/di/RepositoryModule.kt
- data/src/main/java/com/yapp/data/repositoryimpl/ImageRepositoryImpl.kt
- data/src/main/java/com/yapp/data/local/datasource/ImageLocalDataSource.kt
- data/src/main/java/com/yapp/data/remote/utils/ApiCallUtils.kt
- domain/src/main/java/com/yapp/domain/repository/ImageRepository.kt
🧰 Additional context used
🧬 Code Graph Analysis (2)
core/database/build.gradle.kts (1)
build-logic/src/main/java/com/yapp/convention/AppNameExtension.kt (1)
setNamespace(7-8)
feature/mission/src/main/java/com/yapp/mission/MissionScreen.kt (1)
feature/mission/src/main/java/com/yapp/mission/component/FlipCard.kt (1)
FlipCard(25-97)
⏰ 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 (47)
feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (1)
102-102: 파일명 생성 로직이 개선되었습니다.타임스탬프를 사용한 파일명 생성은 파일 이름 충돌을 방지하는 좋은 방법입니다. 또한
saveImage메서드가 suspend가 아니게 되었지만 여전히 coroutine scope에서 호출되는 것은 문제없습니다.core/media/src/main/java/com/yapp/media/di/MediaModule.kt (2)
1-1: 모듈 패키지 이동이 적절합니다.
MediaModule을com.yapp.media.di패키지로 이동한 것은 미디어 관련 기능을 분리하는 좋은 구조적 결정입니다.Also applies to: 5-5
25-28: 의존성 제공 방식이 일관되게 변경되었습니다.
ImageSaver를 직접 제공하도록 변경된 것은 아키텍처 리팩토링과 일치합니다. DI 설정이 올바르게 구성되어 있습니다.core/media/src/main/java/com/yapp/media/storage/ImageSaver.kt (2)
1-1: 클래스 이름과 패키지 위치가 개선되었습니다.
ImageSaver라는 이름이 더 명확하고,com.yapp.media.storage패키지가 더 적절한 위치입니다. 기능과 위치가 잘 맞습니다.Also applies to: 12-14
35-35: 로깅 태그가 일관되게 업데이트되었습니다.클래스 이름 변경에 따라 로그 태그도 올바르게 "ImageSaver"로 변경되었습니다.
Also applies to: 38-38
gradle.properties (1)
24-24: 테스트 플랫폼 설정 추가 승인통합 테스트 플랫폼을 비활성화하는 설정이 추가되었습니다. 이는
TestAndroid.kt에서 분리된 테스트 구성과 잘 연계되어 있습니다.app/build.gradle.kts (1)
50-50: Firebase BOM 추가 승인Firebase BOM을 추가하여 Firebase 라이브러리 버전 관리를 일관되게 하는 좋은 접근 방식입니다. 다른 Firebase 의존성보다 먼저 위치하는 것이 올바릅니다.
gradle/libs.versions.toml (3)
104-104: Room 테스팅 라이브러리 추가 승인Room 테스팅 라이브러리가 추가되어 데이터베이스 마이그레이션 테스트를 지원합니다. 새로운 database 모듈의 요구사항과 잘 일치합니다.
188-188: Room 플러그인 추가 승인Room 플러그인이 추가되어 서브프로젝트에서 Room 지원을 활성화합니다. 새로운
core:database모듈과 잘 연계됩니다.
32-32: Room 최신 버전 수동 검증 필요자동화된 스크립트가 네트워크 제약으로 인해 Maven Central 메타데이터를 가져오지 못했습니다. 다음 항목을 수동으로 확인해 주세요:
- Maven Central 아티팩트 페이지(https://search.maven.org/artifact/androidx.room/room-runtime)에서 최신 릴리스 버전이 2.7.2인지 검증
- GitHub Advisory(https://github.com/advisories)에서
androidx.room:room-runtime관련 보안 권고사항 유무 확인build.gradle.kts (1)
10-10: Room 플러그인 별칭 추가 승인Room 플러그인 별칭이 추가되어 서브프로젝트에서 Room 지원을 활성화합니다.
apply false로 설정하여 필요한 모듈에서만 적용할 수 있도록 하는 것이 올바른 접근 방식입니다.build-logic/src/main/java/com/yapp/convention/TestAndroid.kt (3)
6-12: 테스트 설정 함수 분리 승인테스트 설정을 JUnit과 Compose UI 테스트로 분리하는 좋은 리팩토링입니다. 관심사 분리를 통해 코드 유지보수성이 향상되었습니다.
14-20: Compose UI 테스트 설정 단순화 승인Compose UI 테스트 관련 의존성만 처리하도록 함수가 단순화되었습니다. 이제 명확한 단일 책임을 가지게 되었습니다.
22-34: JUnit 안드로이드 설정 함수 추가 승인새로운
configureJUnitAndroid()함수가 일반적인 안드로이드 JUnit 테스트 설정을 담당합니다. 테스트 인스트루멘테이션 러너 설정과 필요한 의존성 추가가 적절하게 구성되었습니다.core/database/src/main/java/com/yapp/database/AlarmDao.kt (1)
1-1: 패키지 변경 확인 완료새로운
core:database모듈 구조와 일관성을 맞추기 위해 패키지를com.yapp.database로 옮긴 부분은 문제 없어 보입니다.data/src/main/java/com/yapp/data/remote/datasource/SignUpDataSourceImpl.kt (1)
6-7: 네트워크 모듈로 의존성 이동 👍
ApiError,safeApiCall을core/network로 분리한 방향성에 동의합니다. 네트워크 공통 로직이 데이터 모듈과 분리되어 재사용성이 올라갔습니다.data/src/main/java/com/yapp/data/remote/datasource/UserInfoDataSourceImpl.kt (1)
6-6: import 경로 변경 확인 완료
safeApiCall의 경로가com.yapp.network.utils로 정상 교체되었습니다.core/database/.gitignore (1)
1-1: 모듈 수준 /build 제외 처리 👍
/build디렉터리만 정확히 걸러 실수로 산출물이 커밋될 위험을 없앴습니다.data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSourceImpl.kt (1)
3-5: 패키지 경로 변경 확인 필요
com.yapp.database.toDomain확장 함수가 public 으로 export 되어 있는지, 그리고data모듈이 새core:database모듈에api의존성으로 연결돼 있는지 다시 한 번 점검해 주세요. 빌드 스크립트에서implementation으로 연결하면 이 파일을 컴파일할 때 접근이 불가능합니다.data/src/main/java/com/yapp/data/remote/datasource/FortuneDataSourceImpl.kt (1)
5-5: 네트워크 모듈 의존성 누락 여부 점검
safeApiCall위치가com.yapp.network로 이동했습니다.data모듈의build.gradle.kts에core:network가implementation(또는api) 로 추가됐는지 확인이 필요합니다. 빠지면 컴파일 단계에서 unresolved reference 가 발생합니다.data/src/main/java/com/yapp/data/repositoryimpl/AlarmRepositoryImpl.kt (1)
5-5: 새 데이터베이스 모듈과의 의존성 확인
com.yapp.database.toEntity를 사용하고 있으므로data모듈이core:database를 종속하도록 설정되어야 합니다.api범위가 아니면 외부 모듈에서 같은 확장 함수를 사용할 때 접근 불가 문제가 생길 수 있으니 스코프도 함께 검토해 주세요.settings.gradle.kts (1)
49-49: 새 모듈 포함 건 확인 완료
:core:database모듈이 정상적으로include되었음을 확인했습니다. CI 빌드만 통과하면 추가 조치는 없어 보입니다.core/database/build.gradle.kts (3)
8-12:schemas자산 추가 위치 재확인
androidTest소스셋에만 스키마를 포함하여 릴리즈 APK 용량을 줄이는 설정입니다. 의도된 동작이면 그대로 두셔도 됩니다.
14-16: Room 스키마 디렉터리 설정 적절
core/database/schemas경로로 스키마가 자동 갱신됩니다. VCS에 커밋되어 있는지 확인해 주세요.
18-26: androidTest 의존성 보강 여부 확인
androidx.room.testing외에androidx.test:core,runner,rules등이 필요할 수 있습니다. 상위build-logic에서 이미 제공하지 않는다면 추가를 검토해 주세요.feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (2)
75-75: 미션 타입 검증 로직이 올바르게 수정되었습니다.sealed class에서 enum으로 변경된
MissionType에 맞게 타입 검사가 올바르게 수정되었습니다.
95-95: 미션 타입 검증 로직이 올바르게 수정되었습니다.sealed class에서 enum으로 변경된
MissionType에 맞게 타입 검사가 올바르게 수정되었습니다.core/database/src/main/java/com/yapp/database/AlarmEntity.kt (1)
35-36: 새로운 미션 관련 필드가 적절히 추가되었습니다.
missionType과missionCount필드가 합리적인 기본값과 함께 추가되었습니다.core/database/src/main/java/com/yapp/database/di/DatabaseModule.kt (1)
29-29: 데이터베이스 마이그레이션이 올바르게 통합되었습니다.
DatabaseMigrations.MIGRATION_1_2를 통해 스키마 v1에서 v2로의 마이그레이션이 적절히 설정되었습니다.core/database/src/main/java/com/yapp/database/AlarmDatabase.kt (1)
7-8: 데이터베이스 설정이 올바르게 업데이트되었습니다.버전 2로 업그레이드하고 스키마 내보내기를 활성화했으며,
MissionTypeConverter를 통한 타입 변환이 적절히 설정되었습니다.core/database/schemas/com.yapp.database.AlarmDatabase/1.json (1)
1-118: 데이터베이스 스키마 v1이 올바르게 정의되었습니다.Room에서 생성된 스키마 파일이 적절한 필드 타입과 제약조건으로 잘 구성되어 있습니다. 이는 v2로의 마이그레이션을 위한 베이스라인 역할을 합니다.
feature/mission/src/main/java/com/yapp/mission/MissionScreen.kt (3)
151-151: Enum 패턴 변경이 올바르게 적용됨Sealed class에서 enum class로의 변경이 올바르게 적용되었습니다.
is MissionType.Shake에서MissionType.SHAKE로의 변경이 적절합니다.
159-159: TAP 타입 변경 확인
MissionType.Click에서MissionType.TAP로의 변경이 도메인 모델 변경사항과 일치합니다.
212-213: Progress bar 로직 일관성 유지Enum 패턴 변경 후에도 진행 상태 계산 로직이 일관되게 유지되었습니다.
core/database/src/main/java/com/yapp/database/DatabaseMigrations.kt (2)
8-13: 데이터베이스 마이그레이션 로직이 올바르게 구현됨마이그레이션 로직이 적절합니다:
- 새로운
missionType컬럼이 기본값 'TAP'로 추가- 새로운
missionCount컬럼이 기본값 10으로 추가- NOT NULL 제약조건과 기본값이 올바르게 설정됨
10-11: AlarmDatabase.DATABASE_NAME 상수 일관성 확인 완료
core/database/src/main/java/com/yapp/database/AlarmDatabase.kt에서
const val DATABASE_NAME = "alarm_database"로 정의되어 있습니다.AlarmDao,AlarmEntity,DatabaseMigrations,MigrationTest등에서도 모두 동일 상수를 참조 중입니다.하드코딩 없이 상수로 테이블명을 관리하고 있어 적절하며, 변경할 필요가 없습니다.
data/build.gradle.kts (1)
13-15: data 모듈의 API 전환 의도 및 app 모듈 영향 확인 요청
- data/build.gradle.kts에서 core 모듈들(
core.network,core.database,core.datastore)을implementation에서api로 변경함에 따라- app/build.gradle.kts (40행)의
implementation(projects.data)를 통해 app 모듈이 core 모듈들에 직접 접근할 수 있게 되었습니다.이 변경이 의도된 아키텍처 결정인지, 그리고 app 모듈의 종속성 노출이 적절한지 검토해주세요.
core/database/src/androidTest/java/com/yapp/database/MigrationTest.kt (3)
19-24: MigrationTestHelper 설정이 올바름테스트 헬퍼 설정이 적절합니다.
emptyList()를 사용하여 자동 마이그레이션을 비활성화하고 수동 마이그레이션을 테스트하는 것이 올바른 접근입니다.
29-68: 포괄적인 테스트 데이터 설정버전 1 데이터베이스에 완전한 알람 레코드를 삽입하는 것이 좋습니다. 모든 기존 컬럼을 포함하여 마이그레이션이 기존 데이터를 보존하는지 확인할 수 있습니다.
70-78: 마이그레이션 검증 로직이 정확함마이그레이션 실행 후 새로운 컬럼들의 존재와 기본값을 올바르게 검증합니다. 커서 사용 후 proper cleanup을 위해
use블록을 사용한 것도 좋습니다.domain/src/main/java/com/yapp/domain/model/MissionType.kt (2)
3-6: enum class로의 리팩토링이 적절합니다.sealed class에서 enum class로의 변경은 데이터베이스 저장 및 직렬화에 더 적합합니다. 정수 값을 통한 매핑도 잘 구현되어 있습니다.
9-11: fromInt 메서드 구현이 안전합니다.entries.find를 사용하여 안전하게 검색하고 기본값으로 TAP을 반환하는 것이 좋습니다.
core/network/src/main/java/com/yapp/network/utils/ApiCallUtils.kt (3)
8-22: safeApiCall 함수 구현이 우수합니다.이전 runCatching/recoverCatching 체인 방식에서 try-catch 방식으로 변경한 것이 좋습니다. CancellationException을 올바르게 재던지는 것으로 코루틴 취소가 정상적으로 작동할 것입니다.
12-12: CancellationException 처리가 정확합니다.코루틴 취소를 위해 CancellationException을 재던지는 것이 올바른 구현입니다.
24-33: HTTP 예외 매핑이 포괄적입니다.다양한 HTTP 상태 코드에 대한 한국어 메시지 매핑이 잘 구현되어 있습니다.
core/database/schemas/com.yapp.database.AlarmDatabase/2.json (2)
1-127: 데이터베이스 스키마 버전 2가 올바르게 정의되었습니다.새로운
missionType과missionCount컬럼이 적절히 추가되었고, Room 데이터베이스 스키마 형식을 정확히 따르고 있습니다. 모든 필드가 non-nullable로 설정된 것도 알람 엔티티의 요구사항에 맞습니다.
102-112: 새로운 미션 관련 컬럼이 적절히 추가되었습니다.
missionType을 TEXT로,missionCount를 INTEGER로 설정한 것이 MissionType enum 변경사항과 일치합니다.
core/database/src/main/java/com/yapp/database/MissionTypeConverter.kt
Outdated
Show resolved
Hide resolved
|
🚀 테스트 완료 및 커버리지 리포트가 생성되었습니다. |
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
project.dot.pngis excluded by!**/*.png
📒 Files selected for processing (2)
core/database/src/main/java/com/yapp/database/MissionTypeConverter.kt(1 hunks)data/build.gradle.kts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- data/build.gradle.kts
🧰 Additional context used
🪛 detekt (1.23.8)
core/database/src/main/java/com/yapp/database/MissionTypeConverter.kt
[warning] 12-12: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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
core/database/src/main/java/com/yapp/database/MissionTypeConverter.kt
Outdated
Show resolved
Hide resolved
|
🚀 테스트 완료 및 커버리지 리포트가 생성되었습니다. |
|
🚀 테스트 완료 및 커버리지 리포트가 생성되었습니다. |
MoonsuKang
left a 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.
수고하셨습니다~ 갓동현 ㄷㄷ
| class MissionTypeConverter { | ||
|
|
||
| @TypeConverter | ||
| fun fromMissionType(missionType: String): MissionType { |
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.
P3
enum class MissionType(val value: Int) {}
엇 혹시 여기서 Int로 정의되어 있는데 여기서도 value: Int로 저장포멧을 통일하는게 좋지 않을까염?
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.
수정햇습니당
|
|
||
| @Database(entities = [AlarmEntity::class], version = 1, exportSchema = false) | ||
| @Database(entities = [AlarmEntity::class], version = 2, exportSchema = false) | ||
| @TypeConverters(MissionTypeConverter::class) |
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.
룸에서 커스텀 타입을 저장하려면 요걸 지정해줘야되는군요..ㄷㄷ
| return try { | ||
| Result.success(action()) | ||
| } catch (exception: Throwable) { | ||
| if (exception is CancellationException) throw exception |
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.
👍
|
|
||
| @Test | ||
| @Throws(IOException::class) | ||
| fun migrate1To2() { |
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.
P5
Junit 공식문서에서 딱히 정해둔 건 없지만 Android Codelab에서는 snake case로 짓는 거 같아유.
migration_1_to_2 요런식으로? 간결하게 해도 될 것 같고
찾아보니까 BDD나 method_state_result 형식을 쫌 많이 쓰는 것 같슴다잉~
Google Android Codelab
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.
테스트 이름은 그냥 한글로 할까 고민되네요
|
👍👍👍👍 |
|
🚀 테스트 완료 및 커버리지 리포트가 생성되었습니다. |
Related issue 🛠
closed #227
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
missionType) 및 횟수(missionCount) 컬럼 추가safeApiCall로직 수정:runCatching+recoverCatching체이닝을 사용했으나,recoverCatching은 실패를 성공으로 복구하되 복구 과정에서 예외가 발생하면 해당 예외를 새로운 실패로 덮어쓰는 시맨틱을 가지므로, 예외 매핑 후 실패 반환 목적과 의미가 맞지 않아 try-catch 기반으로 대체runCatching은CancellationException을 전파하지 않아,CancellationException 발생 시 즉시 throw하여 코루틴 취소가 정상적으로 propagate 되도록 수정
configureJUnitAndroid)configureComposeUiTest)Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
신규 기능
버그 수정
리팩터
테스트
기타