[Refactor] #162 HttpClient의 토큰 캐싱 옵션 설정 추가 및 일부 API 버전 상향#163
Merged
[Refactor] #162 HttpClient의 토큰 캐싱 옵션 설정 추가 및 일부 API 버전 상향#163
Conversation
WalkthroughKtor 3.4.0로 업그레이드하면서 수동 토큰 캐시(AuthCacheManager)를 제거하고, NetworkModule에서 Ktor의 내장 토큰 캐싱을 활성화하며(TokenRepository 및 호출부의 메서드명 변경 포함), 빌드 로직의 Kotlin JVM 타겟 설정 방식을 Kotlin DSL로 전환했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Ktor as Ktor(HttpClient)
participant Network as NetworkModule
participant Repo as TokenRepositoryImpl
participant Store as Datastore
participant AuthEvt as AuthEventManager
Note over Ktor: cacheTokens = !BuildConfig.DEBUG (enabled)
Client->>Ktor: API 요청 (Bearer 토큰 포함)
Ktor-->>Client: 401 Unauthorized
Network->>Repo: refreshToken() 호출
Repo->>Ktor: 요청 (토큰 갱신)
alt refresh 성공
Repo->>Store: saveTokens(...)
Repo-->>Ktor: 새 토큰으로 응답 흐름 재시도 (자동 캐시 사용)
else refresh 실패
Repo->>Ktor: authProvider.clearToken() rgba(255,0,0,0.5)
Repo->>Store: clear token 데이터 (clearTokensWithAuthCache)
Repo->>AuthEvt: emit(TokenExpired)
end
AuthEvt->>Client: 토큰 만료 이벤트 전파
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build-logic/src/main/java/com/neki/android/buildlogic/extensions/Android.kt`:
- Around line 30-34: The direct call to
extensions.configure<KotlinAndroidProjectExtension>("kotlin") can throw if the
Kotlin Android plugin isn't applied; guard this configuration with a plugin
check (e.g., use pluginManager.withPlugin("org.jetbrains.kotlin.android") or
plugins.withId) before calling configure so the block that sets compilerOptions
and jvmTarget (referencing KotlinAndroidProjectExtension, "kotlin",
compilerOptions, jvmTarget, BuildConst.JDK_VERSION) only runs when the Kotlin
Android plugin is present.
In
`@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt`:
- Line 75: The Ktor bearer token caching is enabled by cacheTokens =
!BuildConfig.DEBUG, which allows a cached in-memory token to persist after
TokenRepository.clearTokens() is called; to fix, either set cacheTokens = false
in the NetworkModule configuration (disable caching), or add explicit
invalidation on logout by recreating the HttpClient or clearing Ktor's bearer
auth cache when TokenRepository.clearTokens() is invoked; reference the
cacheTokens configuration in NetworkModule, TokenRepository.clearTokens(), and
the HttpClient/bearer auth setup to implement one of these approaches so logout
truly prevents reuse of the old token.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
build-logic/src/main/java/com/neki/android/buildlogic/extensions/Android.ktcore/data-api/src/main/java/com/neki/android/core/dataapi/auth/AuthCacheManager.ktcore/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (2)
- core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt
- core/data-api/src/main/java/com/neki/android/core/dataapi/auth/AuthCacheManager.kt
build-logic/src/main/java/com/neki/android/buildlogic/extensions/Android.kt
Show resolved
Hide resolved
core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/TokenRepository.kt (1)
13-13: 퍼블릭 API 네이밍에서 구현 디테일 노출을 줄이는 것을 권장합니다.
clearTokensWithAuthCache()는 데이터 API 계약에 인프라 세부사항이 드러납니다. 인터페이스는clearTokens()처럼 의도 중심으로 유지하고, Auth 캐시 정리는 구현체 내부 책임으로 두는 편이 변경 내성이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data-api/src/main/java/com/neki/android/core/dataapi/repository/TokenRepository.kt` at line 13, Rename the public interface method clearTokensWithAuthCache() to a more intention-revealing name like clearTokens() on TokenRepository and move any Auth cache-specific logic into the concrete implementation (e.g., the class implementing TokenRepository) so the interface no longer exposes infrastructure details; update all callers to use TokenRepository.clearTokens() and keep cache clearing as an internal step inside the implementation's clearTokens() method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt`:
- Around line 59-64: clearTokensWithAuthCache currently performs
authProvider.clearToken() then dataStore.edit sequentially so a failure in the
first prevents the second from running; change it to run each cleanup in its own
runCatching block so both are attempted: wrap the
httpClient.get().authProvider<BearerAuthProvider>()?.clearToken() call in
runCatching and wrap the dataStore.edit { preferences.remove(ACCESS_TOKEN);
preferences.remove(REFRESH_TOKEN) } call in a separate runCatching, then if
either runCatching failed, rethrow a combined exception (e.g., aggregate the
non-null exceptions or throw the first with the second as suppressed) so callers
still receive an error but both cleanup attempts were made.
---
Nitpick comments:
In
`@core/data-api/src/main/java/com/neki/android/core/dataapi/repository/TokenRepository.kt`:
- Line 13: Rename the public interface method clearTokensWithAuthCache() to a
more intention-revealing name like clearTokens() on TokenRepository and move any
Auth cache-specific logic into the concrete implementation (e.g., the class
implementing TokenRepository) so the interface no longer exposes infrastructure
details; update all callers to use TokenRepository.clearTokens() and keep cache
clearing as an internal step inside the implementation's clearTokens() method.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/TokenRepository.ktcore/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.ktfeature/mypage/impl/src/main/java/com/neki/android/feature/mypage/impl/main/MyPageViewModel.kt
core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt
Show resolved
Hide resolved
ikseong00
approved these changes
Mar 2, 2026
Contributor
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
로그아웃/탈퇴는 비교적 적게 발생하는 API 인데, 인터셉터를 붙이면은 필요할 때보다,
불필요한 상황(ex) 사진 조회, 지도 핀 조회 등등)에서 조건이 많이 걸릴 것 같아요.
그래서 필요시 해당 clearToken() 함수를 직접 호출하는 형식으로 하고, 그렇게 하기 위해서 Lazy 를 넣는 방향이 조금 더 나은 것 같습니다
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.
🔗 관련 이슈
📙 작업 설명
Ktor버전3.4.0으로 상향Kotlin, ksp, hilt버전 ktor-3.4.0을 지원하는 최소 버전으로 상향cacheTokens옵션 활성화되도록cacheTokens = !BuildConfig.DEBUG구문 추가AuthCacheManager제거🧪 테스트 내역 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
TokenRepository조회 횟수를 줄이기 위해cacheTokens옵션을 활성화 할 수 있도록 했습니다.Q. 토끼 리뷰에 따라 로그아웃/탈퇴 시
HttpClient에 저장된 토큰을 명시적으로 제거해 주어야 해서 반영했습니다. 0a2c091TokenRepositoryImpl에HttpClient를 주입해서 기존clearTokens()에httpClient.get().authProvider<BearerAuthProvider>()?.clearToken()구문을 추가했습니다.위 커밋처럼 Impl에
HttpClient를 Lazy 주입하는 방법이 좋을까요? (Lazy 주입을 하는 이유는HttpClient->TokenRepository->HttpClient로 참조하기 때문입니다.)혹은
HttpClient를 Impl에 주입하지 않고,NetworkModule의HttpClient인스턴스를 정의하는provideHttpClient()내부에서 인터셉터를 추가해 로그아웃/탈퇴 요청이라면httpClient.get().authProvider<BearerAuthProvider>()?.clearToken()해당 구문을 추가하는 방식이 좋을까요?익성님 의견은 어떠신지 궁금합니다.