Conversation
Walkthrough프로젝트의 datastore 모듈이 api와 impl로 분리되었습니다. 관련된 Gradle 설정과 패키지 경로가 변경되었으며, TokenPreferencesDataSource 인터페이스에 suspend 함수가 추가되고 구현체에도 반영되었습니다. 기존의 AndroidManifest.xml 파일은 삭제되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TokenPreferencesDataSource(API)
participant DefaultTokenPreferencesDataSource(Impl)
Client->>TokenPreferencesDataSource(API): getAccessToken()
TokenPreferencesDataSource(API)->>DefaultTokenPreferencesDataSource(Impl): getAccessToken()
DefaultTokenPreferencesDataSource(Impl)-->>TokenPreferencesDataSource(API): String(AccessToken)
TokenPreferencesDataSource(API)-->>Client: String(AccessToken)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes해당 변경사항에는 범위를 벗어난 변경사항이 없습니다. Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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 (
|
![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
🔭 Outside diff range comments (3)
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/security/CryptoManager.kt (1)
15-18: Cipher 인스턴스 재사용으로 인한 스레드 안전·상태 문제
Cipher는 thread-safe 하지 않고, 한 번doFinal()호출 후 내부 상태가 변경됩니다.
싱글턴으로 보관하면 다중 호출 시IllegalStateException이나 경쟁 상태가 발생할 수 있습니다.
매 호출마다 새 인스턴스를 얻어 사용하도록 리팩터링이 필요합니다.-private val cipher = Cipher.getInstance(TRANSFORMATION) +private fun newCipher(): Cipher = Cipher.getInstance(TRANSFORMATION)그리고
encrypt/decrypt내부에서newCipher()를 사용하도록 교체해주세요.core/datastore/impl/build.gradle.kts (1)
16-22: Gradle 구성명 오류로 컴파일 실패 가능성
implementations(/androidTestImplementations(는 Gradle 기본 DSL에 존재하지 않는 구성입니다.
관례상 단수형(implementation,androidTestImplementation)이 맞으며, 프로젝트 전역에 커스텀 확장이 없다면 빌드가 즉시 실패합니다.-dependencies { - implementations( +dependencies { + implementation( projects.core.datastore.api, libs.androidx.datastore.preferences, libs.logger, ) - androidTestImplementations( + androidTestImplementation( libs.androidx.test.ext.junit, ... ) }위와 같이 수정하거나, 실제로 커스텀 확장이 정의돼 있는지 확인해 주세요.
app/build.gradle.kts (1)
38-45:implementations(사용 재점검여기서도
implementations(확장을 사용하고 있습니다. 앞선 모듈과 동일하게 커스텀 함수가 없으면 빌드가 깨집니다. 전역적으로 단수형으로 교체하거나, 확장 함수 정의 여부를 확인해 주세요.
🧹 Nitpick comments (3)
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/util/DataStoreUtil.kt (1)
9-16: IOException 처리 시 로그(or Crashlytics) 남기는 방안을 고려해주세요
현재는IOException발생 시emptyPreferences()만 emit 하고 아무 로그도 남기지 않습니다.
운영 중 장애 분석이 어려워질 수 있으니 최소한 warning 로그 정도는 추가하는 편이 좋습니다.catch { exception -> if (exception is IOException) { - emit(emptyPreferences()) + Timber.w(exception, "DataStore IOException 발생, emptyPreferences() 반환") + emit(emptyPreferences())(예시는 Timber 사용)
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/security/CryptoManager.kt (1)
49-55: 문자열 → 바이트 변환 시 명시적 Charset 지정 권장
플랫폼 기본 charset에 의존하면 기기별 차이가 발생할 수 있습니다.-val encryptedBytes = cipher.doFinal(plainText.toByteArray()) +val encryptedBytes = cipher.doFinal(plainText.toByteArray(Charsets.UTF_8))
decrypt쪽도 동일하게String(bytes, Charsets.UTF_8)적용을 권장합니다.core/datastore/api/build.gradle.kts (1)
9-11: 코루틴 의존성만 포함된 상태 확인현 단계에서 API 모듈이 인터페이스만 노출한다면
kotlinx-coroutines-core만으로 충분합니다.
다만 추후 Flow 확장 util 등을 추가할 경우kotlinx-coroutines-android의 필요 여부를 검토해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
app/build.gradle.kts(1 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/TokenManager.kt(1 hunks)core/data/impl/build.gradle.kts(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/RepositoryModule.kt(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultTokenManager.kt(1 hunks)core/datastore/api/build.gradle.kts(1 hunks)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/TokenPreferencesDataSource.kt(1 hunks)core/datastore/impl/.gitignore(1 hunks)core/datastore/impl/build.gradle.kts(2 hunks)core/datastore/impl/src/androidTest/java/com/ninecraft/booket/core/datastore/impl/TokenPreferenceDataSourceTest.kt(4 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultTokenPreferencesDataSource.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreModule.kt(2 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/security/CryptoManager.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/util/DataStoreUtil.kt(1 hunks)core/datastore/src/main/AndroidManifest.xml(0 hunks)settings.gradle.kts(1 hunks)
💤 Files with no reviewable changes (1)
- core/datastore/src/main/AndroidManifest.xml
🧰 Additional context used
🧠 Learnings (7)
app/build.gradle.kts (1)
Learnt from: easyhooon
PR: YAPP-Github/26th-App-Team-1-Android#16
File: feature/login/build.gradle.kts:21-28
Timestamp: 2025-06-23T05:36:08.281Z
Learning: The AndroidFeatureConventionPlugin automatically adds the core:ui module as an implementation dependency for all modules that apply the booket.android.feature plugin. Therefore, feature modules don't need to manually declare the core:ui dependency in their build.gradle.kts files.
core/datastore/api/build.gradle.kts (1)
Learnt from: easyhooon
PR: YAPP-Github/26th-App-Team-1-Android#16
File: feature/login/build.gradle.kts:21-28
Timestamp: 2025-06-23T05:36:08.281Z
Learning: The AndroidFeatureConventionPlugin automatically adds the core:ui module as an implementation dependency for all modules that apply the booket.android.feature plugin. Therefore, feature modules don't need to manually declare the core:ui dependency in their build.gradle.kts files.
core/datastore/impl/build.gradle.kts (1)
Learnt from: easyhooon
PR: YAPP-Github/26th-App-Team-1-Android#16
File: feature/login/build.gradle.kts:21-28
Timestamp: 2025-06-23T05:36:08.281Z
Learning: The AndroidFeatureConventionPlugin automatically adds the core:ui module as an implementation dependency for all modules that apply the booket.android.feature plugin. Therefore, feature modules don't need to manually declare the core:ui dependency in their build.gradle.kts files.
core/data/impl/build.gradle.kts (1)
Learnt from: easyhooon
PR: YAPP-Github/26th-App-Team-1-Android#16
File: feature/login/build.gradle.kts:21-28
Timestamp: 2025-06-23T05:36:08.281Z
Learning: The AndroidFeatureConventionPlugin automatically adds the core:ui module as an implementation dependency for all modules that apply the booket.android.feature plugin. Therefore, feature modules don't need to manually declare the core:ui dependency in their build.gradle.kts files.
settings.gradle.kts (1)
Learnt from: easyhooon
PR: YAPP-Github/26th-App-Team-1-Android#16
File: feature/login/build.gradle.kts:21-28
Timestamp: 2025-06-23T05:36:08.281Z
Learning: The AndroidFeatureConventionPlugin automatically adds the core:ui module as an implementation dependency for all modules that apply the booket.android.feature plugin. Therefore, feature modules don't need to manually declare the core:ui dependency in their build.gradle.kts files.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/RepositoryModule.kt (1)
Learnt from: easyhooon
PR: YAPP-Github/26th-App-Team-1-Android#16
File: feature/login/build.gradle.kts:21-28
Timestamp: 2025-06-23T05:36:08.281Z
Learning: The AndroidFeatureConventionPlugin automatically adds the core:ui module as an implementation dependency for all modules that apply the booket.android.feature plugin. Therefore, feature modules don't need to manually declare the core:ui dependency in their build.gradle.kts files.
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreModule.kt (1)
Learnt from: easyhooon
PR: YAPP-Github/26th-App-Team-1-Android#16
File: feature/login/build.gradle.kts:21-28
Timestamp: 2025-06-23T05:36:08.281Z
Learning: The AndroidFeatureConventionPlugin automatically adds the core:ui module as an implementation dependency for all modules that apply the booket.android.feature plugin. Therefore, feature modules don't need to manually declare the core:ui dependency in their build.gradle.kts files.
🧬 Code Graph Analysis (1)
core/datastore/api/build.gradle.kts (1)
build-logic/src/main/kotlin/com/ninecraft/booket/convention/Dependencies.kt (1)
implementation(6-8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-build
🔇 Additional comments (21)
core/datastore/impl/.gitignore (1)
1-1:.gitignore에 대한 적절한 추가
/build디렉터리만 제외하도록 설정한 것은 모듈 단위 빌드 산출물 관리에 충분합니다.core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/TokenPreferencesDataSource.kt (1)
1-11: 패키지 이동만 이루어졌음을 확인했습니다
인터페이스 자체에는 변경 사항이 없으며, 의존 모듈 분리에 맞춰 올바르게 이동되었습니다.core/data/impl/build.gradle.kts (1)
17-18: Hilt 바인딩 연결 여부 확인 필요
core.data.impl모듈이TokenPreferencesDataSource구현체를 직접 의존하지 않고 API 모듈만 참조하도록 변경되었습니다.
Hilt 멀티모듈 환경에서@Provides혹은@Binds위치(=datastore-impl)까지 앱 모듈이 의존하지 않으면 컴파일 타임에 바인딩을 찾지 못할 수 있습니다.
앱 모듈이core.datastore.impl을 이미 implementation으로 포함하고 있는지, 그리고 Hilt aggregating task가 통과하는지 한 번 빌드로 검증해 주세요.core/datastore/impl/build.gradle.kts (1)
9-10: 네임스페이스 변경 후 매니페스트 경로·R 패키지 확인 필요
namespace = "com.ninecraft.booket.core.datastore.impl"로 변경되었으므로
•src/main/AndroidManifest.xml내package값이 일치하는지,
• 코드에서R참조가 새 네임스페이스로 자동 변경되었는지 한 번 더 점검해 주세요.
맞지 않으면 런타임에서Resources$NotFoundException이 발생할 수 있습니다.core/datastore/api/build.gradle.kts (1)
5-7: 모듈 구조 분리에 맞는 네임스페이스 설정 👍API 모듈로 분리하면서 별도 네임스페이스를 지정한 점이 일관성 있게 잘 반영되었습니다.
settings.gradle.kts (1)
30-31: 기존:core:datastore모듈 중복 포함 여부 점검새로운 두 하위 모듈을 포함하면서 상위
:core:datastore항목이 제거됐는지 확인이 필요합니다.
include()블록 어딘가에 기존 모듈이 남아 있으면 Gradle 구성 충돌이 발생합니다.core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultTokenPreferencesDataSource.kt (1)
7-9: 패키지·임포트 경로 변경과 DI 바인딩 동기화 확인
TokenPreferencesDataSource인터페이스가 API 모듈로 이동했으므로, Hilt 모듈에서의@Binds/@Provides위치도 함께 업데이트되었는지 확인해 주세요.
누락 시 런타임 주입 오류가 발생할 수 있습니다.core/datastore/impl/src/androidTest/java/com/ninecraft/booket/core/datastore/impl/TokenPreferenceDataSourceTest.kt (4)
1-1: 패키지 구조 변경이 올바르게 적용되었습니다.데이터스토어 모듈의 API와 구현부 분리에 따른 패키지 선언 변경이 올바르게 적용되었습니다.
8-9: 임포트 문이 새로운 모듈 구조에 맞게 올바르게 업데이트되었습니다.
DefaultTokenPreferencesDataSource와CryptoManager의 임포트가 새로운impl패키지 위치로 정확하게 변경되었습니다.
40-40: 코드 포맷팅 개선을 위한 후행 쉼표 추가를 승인합니다.후행 쉼표 추가는 코드 일관성과 버전 관리 측면에서 좋은 변경사항입니다.
53-53: 테스트 메서드명의 한국어 변경을 승인합니다.테스트 메서드명이 한국어로 변경되어 가독성이 향상되었으며, 테스트 로직은 그대로 유지되었습니다.
Also applies to: 68-68
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/RepositoryModule.kt (2)
4-4: TokenManager 관련 임포트가 올바르게 추가되었습니다.새로운 TokenManager 인터페이스와 구현체의 임포트가 정확하게 추가되었습니다.
Also applies to: 6-6
21-23: TokenManager DI 바인딩이 올바르게 설정되었습니다.
DefaultTokenManager를TokenManager인터페이스에 싱글톤으로 바인딩하는 설정이 정확합니다.core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/TokenManager.kt (1)
3-9: 토큰 관리를 위한 깔끔한 인터페이스 설계입니다.
TokenManager인터페이스가 토큰 조회, 설정, 삭제 기능을 모두 포함하여 완전한 토큰 관리 API를 제공합니다. 모든 함수가 suspend로 선언되어 비동기 처리에 적합합니다.core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreModule.kt (3)
1-1: 패키지 선언이 새로운 모듈 구조에 맞게 올바르게 업데이트되었습니다.데이터스토어 구현 모듈의 패키지 선언이 정확하게 변경되었습니다.
7-8: 임포트 문이 API/구현 분리에 맞게 올바르게 수정되었습니다.
TokenPreferencesDataSource는 API 모듈에서,DefaultTokenPreferencesDataSource는 구현 모듈에서 임포트하도록 올바르게 변경되었습니다.
37-37: 파라미터명 변경이 일관성을 향상시켰습니다.
tokenPreferencesDataSourceImpl에서defaultTokenPreferencesDataSource로 변경되어 명명 일관성이 향상되었습니다.core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultTokenManager.kt (4)
8-10: 생성자 주입을 통한 의존성 주입이 올바르게 구현되었습니다.
TokenPreferencesDataSource를 생성자 주입으로 받아 의존성 역전 원칙을 잘 따르고 있습니다.
11-11: 토큰 조회 메서드가 올바르게 구현되었습니다.
Flow.first()를 사용하여 토큰 값을 가져오는 구현이 정확합니다.Also applies to: 13-13
19-21: setRefreshToken 메서드가 올바르게 구현되었습니다.리프레시 토큰 설정 로직이 정확합니다.
23-25: clearTokens 메서드가 올바르게 구현되었습니다.토큰 삭제 기능이 데이터소스에 적절하게 위임되었습니다.
혹시 레퍼런스 하나만 공유해줄수있나요? TokenManager라는 것을 만든 취지는 좋은데 뎁스가 너무 많아지지않나라는 생각이 드네요. 아니면 이후에 필요할진 모르겠지만 flow 형태로(변수 타입)으로 선언하는것과, suspend 형태로 one shot으로 받은 (getAccessToken, getRefreshToken)을 모두 DefaultTokenPreferencesDataStore에 추가하여 사용하는 것은 어떨까요? |
|
|
||
| @Test | ||
| fun tokenIsEncryptedWhenStored() = runTest { | ||
| fun 토큰은_암호화되어_저장된다() = runTest { |
|
아 accessToken 변수(flow 타입)는 이후 자동 로그인 구현할때 이용할 수도 있을듯 함니다~ |
DroidKnightsApp, nowinandroid 모두
|
easyhooon
left a comment
easyhooon
left a comment
There was a problem hiding this comment.
작업해주시고 merge 부탁합니다~ 믿음의 Approve
Cipher는 thread-safe 하지 않고, 한 번 doFinal() 호출 후 내부 상태가 변경되기 때문에 Singleton 클래스 내에서 필드로 보관하면 위험함(다중 호출 시 IllegalStateException 이나 경쟁 상태가 발생할 수 있음)
2 participants
🔗 관련 이슈
📙 작업 설명
📸 스크린샷 또는 시연 영상
테스트코드 메서드 한글로 변경했습니다~
💬 추가 설명 or 리뷰 포인트
datastore의datasource에서는Flow<String>타입을 유지하고.first()호출은 외부에서 처리하도록 구성하는 경우가 많더라구요. (책임 분리와 사용 편의성 등등 실무에서 주로 사용하는 방식이라고 합니다)data모듈에TokenManager인터페이스와 구현체DefaultTokenManager를 추가했습니다TokenInterceptor나 Auth관련 로직에서 쓰면 될 것 같아요TokenRepository라는 이름을 쓸까 고민했지만 지피티가TokenManager에 한표 던졌습니다 (-Repository는 datasource 위에서 데이터 가공하거나 조합해서 제공할 때 쓰는게 적합하다고 합니다)Summary by CodeRabbit
New Features
Refactor
Chores