[init] #4 DataStore, API 응답 핸들링 로직 추가#11
Conversation
…init/#4-setup-api-library # Conflicts: # core/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.kt # core/data/src/main/java/com/neki/android/core/data/repository/impl/SampleRepositoryImpl.kt # gradle/libs.versions.toml
Walkthrough샘플 기능 모듈(feature/sample/api 및 feature/sample/impl)을 제거하고, Android Keystore 기반 AES‑GCM 암호화(CryptoManager), DataStore 기반 암호화 토큰 저장소(DataStoreRepository 및 구현), runSuspendCatching 유틸과 관련 의존성(kotlinx.coroutines, androidx-security-crypto)을 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant Repo as DataStoreRepositoryImpl
participant Crypto as CryptoManager
participant DS as DataStore<Preferences>
rect rgb(240,248,255)
Note over App,Repo: 토큰 저장 흐름
App->>Repo: saveTokens(access, refresh)
Repo->>Crypto: encrypt(access)
Crypto->>Crypto: get/create Key\ninit Cipher(ENCRYPT)
Crypto-->>Repo: encryptedAccess (Base64 IV + ciphertext)
Repo->>Crypto: encrypt(refresh)
Crypto-->>Repo: encryptedRefresh
Repo->>DS: dataStore.edit { set encrypted tokens }
DS-->>Repo: 저장 완료
end
rect rgb(245,255,250)
Note over App,Repo: 토큰 조회 흐름 (Flow)
App->>Repo: getAccessToken()
Repo->>DS: dataStore.data (collect)
DS-->>Repo: encryptedAccess
Repo->>Crypto: decrypt(encryptedAccess)
Crypto->>Crypto: Base64 decode\nextract IV\ninit Cipher(DECRYPT)\ndecrypt
Crypto-->>Repo: plainAccess
Repo-->>App: Flow<plainAccess>
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
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: 4
🧹 Nitpick comments (3)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.kt (1)
5-13: 인터페이스 설계가 적절합니다.토큰 저장소의 계약을 명확하게 정의했습니다. Flow를 사용한 토큰 조회 방식은 반응형 업데이트를 지원하며, suspend 함수를 통한 저장/삭제는 적절한 설계입니다.
한 가지 고려 사항:
saveTokens가 암호화 실패 시 예외를 던지는데,Result<Unit>반환 타입으로 변경하면 호출측에서 에러 처리가 더 명시적으로 가능합니다. 현재 PR의runSuspendCatching패턴과도 일관성이 맞습니다.core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (1)
44-46:clearTokens가 모든 preferences를 삭제합니다.
it.clear()는 DataStore의 모든 데이터를 삭제합니다. 현재는 토큰만 저장하므로 문제가 없지만, 향후 다른 데이터가 추가될 경우 의도치 않게 삭제될 수 있습니다. 토큰만 명시적으로 제거하는 것을 고려해주세요.🔎 토큰만 명시적으로 제거하는 방식
override suspend fun clearTokens() { - dataStore.edit { it.clear() } + dataStore.edit { preferences -> + preferences.remove(ACCESS_TOKEN) + preferences.remove(REFRESH_TOKEN) + } }core/data/src/main/java/com/neki/android/core/data/util/RunSuspendCatching.kt (1)
8-11: 함수 시그니처와 네이밍 불일치 검토 필요함수명은
runSuspendCatching이지만block파라미터가suspend () -> T가 아닌() -> T로 정의되어 있습니다. inline 함수이므로 suspend 컨텍스트에서는 정상 작동하지만, 시그니처만으로는 suspend 함수 지원 의도가 명확히 드러나지 않습니다.또한
@OptIn(ExperimentalContracts::class)는 실험적 API를 사용하므로 향후 Kotlin 버전에서 변경될 수 있습니다.🔎 더 명확한 시그니처 제안
더 명시적인 suspend 함수 지원을 원한다면 다음과 같이 수정할 수 있습니다:
@OptIn(ExperimentalContracts::class) -internal inline fun <T> runSuspendCatching(block: () -> T): Result<T> { +internal suspend inline fun <T> runSuspendCatching(block: suspend () -> T): Result<T> { contract { callsInPlace(block, InvocationKind.EXACTLY_ONCE) }다만 현재 구현도 inline 특성상 정상 작동하므로, 프로젝트 컨벤션에 따라 선택하시면 됩니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
app/build.gradle.ktscore/common/build.gradle.ktscore/common/src/main/java/com/neki/android/core/common/crypto/CryptoManager.ktcore/data-api/build.gradle.ktscore/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.ktcore/data-api/src/main/java/com/neki/android/core/dataapi/repository/SampleRespository.ktcore/data/src/main/java/com/neki/android/core/data/local/di/DataStoreModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/SampleRepositoryImpl.ktcore/data/src/main/java/com/neki/android/core/data/util/RunSuspendCatching.ktfeature/sample/api/.gitignorefeature/sample/api/build.gradle.ktsfeature/sample/api/src/main/java/com/neki/android/feature/sample/api/SampleNavKey.ktfeature/sample/impl/.gitignorefeature/sample/impl/build.gradle.ktsfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleEntryProvider.ktfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleScreen.ktfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleViewModel.ktgradle/libs.versions.tomlsettings.gradle.kts
💤 Files with no reviewable changes (9)
- feature/sample/api/build.gradle.kts
- feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleViewModel.kt
- feature/sample/impl/build.gradle.kts
- settings.gradle.kts
- feature/sample/api/src/main/java/com/neki/android/feature/sample/api/SampleNavKey.kt
- feature/sample/impl/.gitignore
- feature/sample/api/.gitignore
- feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleEntryProvider.kt
- feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleScreen.kt
🧰 Additional context used
🧬 Code graph analysis (1)
core/data/src/main/java/com/neki/android/core/data/repository/impl/SampleRepositoryImpl.kt (1)
core/data/src/main/java/com/neki/android/core/data/util/RunSuspendCatching.kt (1)
runSuspendCatching(8-20)
⏰ 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: ci-build
🔇 Additional comments (9)
app/build.gradle.kts (1)
14-33: :feature:sample 모듈 의존성 제거 확인됨PR 목표에 따라
:feature:sample:api와:feature:sample:impl모듈의 의존성이 깔끔하게 제거되었습니다. 남아있는 의존성들(core 모듈 및 다른 feature 모듈들)은 올바르게 유지되고 있습니다.core/data-api/build.gradle.kts (1)
7-8: LGTM!DataStoreRepository 인터페이스에서 Flow 타입을 사용하기 위해 필요한 coroutines-core 의존성 추가가 적절합니다.
core/common/build.gradle.kts (1)
12-12: LGTM!CryptoManager에서 Android Keystore 기반 암호화를 사용하기 위한 security-crypto 의존성 추가가 적절합니다.
core/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.kt (1)
15-28: Hilt 모듈 구조 개선이 좋습니다.abstract class에서 interface로 변경한 것은
@Binds메서드만 포함하는 모듈에서 권장되는 패턴입니다.@Singleton스코프 추가와 DataStoreRepository 바인딩이 적절하게 구성되었습니다.core/common/src/main/java/com/neki/android/core/common/crypto/CryptoManager.kt (2)
41-50: 암호화 구현이 적절합니다.AES-GCM 암호화 구현이 올바르게 되어있습니다. AndroidKeyStore의 GCM 모드는 자동으로 보안적으로 안전한 랜덤 IV를 생성하며, IV와 암호문을 결합하여 저장하는 방식이 표준적입니다.
26-27: 문제 없음 - 프로젝트의 minSdk는 29이므로 @SuppressLint("NewApi") 적용이 적절합니다
KeyGenParameterSpec과KeyProperties는 API 23 이상에서만 사용 가능하지만, 이 프로젝트의 minSdk는 29로 설정되어 있습니다. minSdk가 API 23 요구사항을 충족하므로 현재의@SuppressLint("NewApi")적용은 적절합니다.Likely an incorrect or invalid review comment.
core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (1)
21-29: 토큰 저장 구현이 적절합니다.암호화된 토큰을 DataStore에 저장하는 구현이 올바릅니다. CryptoManager를 통한 암호화가 적절히 적용되었습니다.
core/data/src/main/java/com/neki/android/core/data/repository/impl/SampleRepositoryImpl.kt (1)
12-18: runSuspendCatching을 사용한 깔끔한 구현입니다API 호출을
runSuspendCatching으로 래핑하여Result타입을 반환하는 패턴이 일관되게 적용되었습니다. 에러 핸들링이 유틸 함수에 위임되어 코드가 간결하고 가독성이 높습니다.core/data-api/src/main/java/com/neki/android/core/dataapi/repository/SampleRespository.kt (1)
6-7: Result 패턴 도입으로 인한 호출자 확인은 불필요현재 코드베이스에서 SampleRepository의
getPosts()및getPost()메서드를 호출하는 클래스가 없습니다. 따라서 즉시적인 breaking change 문제는 없으며, 호출자 업데이트 확인 요청은 필요하지 않습니다.Result 패턴 구현은 제대로 되어 있습니다.
runSuspendCatching을 통해 예외를 안전하게 처리하고 있으므로 승인합니다. 향후 이 저장소의 다른 부분에서 SampleRepository 메서드를 호출할 때는 Result 타입을 적절히 처리하도록 주의하세요.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (1)
31-42: 이전 리뷰에서 지적된 문제가 아직 해결되지 않았습니다.
- Line 34의 주석 처리된 코드를 제거해야 합니다.
CryptoManager.decrypt()는 손상된 데이터나 키 변경 시 예외를 던질 수 있으며, Flow 내에서 예외가 발생하면 collector가 크래시할 수 있습니다.이전 리뷰 코멘트에서 제안된
runCatching을 사용한 안전한 복호화 패턴을getAccessToken과getRefreshToken모두에 적용해주세요.
🧹 Nitpick comments (2)
core/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.kt (1)
15-28: DI 바인딩이 올바르게 구성되었습니다.인터페이스 기반 모듈과
@Binds사용이 적절합니다.@Singleton어노테이션을 바인딩 메서드가 아닌 구현 클래스의 생성자에 직접 붙이는 것이 더 일반적인 패턴이지만, 현재 방식도 정상적으로 동작합니다.🔎 선택적 개선 사항: 구현 클래스에 @singleton 이동
더 관용적인 패턴을 따르려면
DataStoreRepositoryImpl과SampleRepositoryImpl생성자에@Singleton을 붙이고, 바인딩 메서드에서는 제거할 수 있습니다.@Singleton class DataStoreRepositoryImpl @Inject constructor(...)core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (1)
21-29: 암호화 실패 시 에러 처리 고려 권장
CryptoManager.encrypt()가 예외를 던질 경우 호출자에게 전파되는데, 보다 방어적인 코드를 위해 명시적인 에러 처리를 추가하는 것을 고려해보세요. 현재 구현도 동작하지만, 암호화 실패 시 적절한 로깅이나 사용자 정의 예외를 제공하면 디버깅에 도움이 됩니다.🔎 제안하는 방어적 코드 패턴
override suspend fun saveTokens( accessToken: String, refreshToken: String, ) { try { val encryptedAccess = CryptoManager.encrypt(accessToken) val encryptedRefresh = CryptoManager.encrypt(refreshToken) dataStore.edit { preferences -> preferences[ACCESS_TOKEN] = encryptedAccess preferences[REFRESH_TOKEN] = encryptedRefresh } } catch (e: Exception) { // 로깅 또는 사용자 정의 예외 처리 throw TokenEncryptionException("Failed to encrypt tokens", e) } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.ktcore/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt
⏰ 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: ci-build
🔇 Additional comments (3)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.kt (1)
5-14: 인터페이스 설계가 적절합니다.토큰 저장/조회를 위한 계약이 명확하게 정의되어 있으며, suspend 함수와 Flow를 적절히 활용하고 있습니다.
core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (2)
16-19: Preference 키 정의가 적절합니다.상수를 companion object에 private로 정의하여 캡슐화가 잘 되어 있습니다.
44-46: 토큰 초기화 로직이 올바릅니다.
dataStore.edit를 사용하여 모든 preference를 안전하게 삭제하고 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (2)
31-35: 복호화 예외 처리 누락 (이전 리뷰와 동일한 이슈)
CryptoManager.decrypt는 손상된 데이터나 키 변경 시 예외를 던질 수 있습니다. Flow 내에서 예외가 발생하면 collector가 크래시할 수 있으므로 에러 처리가 필요합니다.🔎 제안하는 수정 사항
override fun getAccessToken(): Flow<String?> { return dataStore.data.map { preferences -> - preferences[ACCESS_TOKEN]?.let { CryptoManager.decrypt(it) } + preferences[ACCESS_TOKEN]?.let { + runCatching { CryptoManager.decrypt(it) }.getOrNull() + } } }
37-41: 복호화 예외 처리 누락 (getAccessToken과 동일한 이슈)
getAccessToken과 마찬가지로 복호화 예외 처리가 필요합니다. Flow collector 크래시를 방지하기 위해runCatching으로 감싸주세요.🔎 제안하는 수정 사항
override fun getRefreshToken(): Flow<String?> { return dataStore.data.map { preferences -> - preferences[REFRESH_TOKEN]?.let { CryptoManager.decrypt(it) } + preferences[REFRESH_TOKEN]?.let { + runCatching { CryptoManager.decrypt(it) }.getOrNull() + } } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/common/src/main/java/com/neki/android/core/common/crypto/CryptoManager.ktcore/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- core/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.kt
- core/common/src/main/java/com/neki/android/core/common/crypto/CryptoManager.kt
⏰ 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: ci-build
🔇 Additional comments (2)
core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (2)
1-15: 클래스 구조가 적절합니다.DataStore와 CryptoManager를 활용한 구조가 잘 설계되었으며, DI 설정도 올바르게 되어 있습니다.
43-45: clearTokens 구현이 적절합니다.DataStore의 모든 데이터를 안전하게 삭제하는 로직이 올바르게 구현되어 있습니다.
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
고생하셨습니다!!
PR 제목 컨벤션만 맞춰주시면 좋을 것 같아요!
| interface SampleRepository { | ||
| suspend fun getPosts(): List<Post> | ||
| suspend fun getPost( | ||
| id: Int, | ||
| ): Post | ||
| suspend fun getPosts(): Result<List<Post>> | ||
| suspend fun getPost(id: Int): Result<Post> | ||
| } |
There was a problem hiding this comment.
넵! 그럼 SampleRepository/Impl 제거한 후 병합하도록 하겠습니다 !
Ojongseok
changed the title
2 participants
🔗 관련 이슈
📙 작업 설명
💬 추가 설명 or 리뷰 포인트 (선택)
DataStore 암호화/복호화 레퍼런스 - https://medium.com/@dev.chaaba/advanced-encrypt-and-decrypt-data-and-store-them-in-shared-preferences-and-datastore-in-android-24ce1ceef13a
Summary by CodeRabbit
새로운 기능
개선 사항
Chores
✏️ Tip: You can customize this high-level summary in your review settings.