Conversation
- 앱 전반의 세션 관리하기에 core 모듈로 패키지 구성 - SessionStore, SwitchToGuestUseCase.kt 제거 후 Manager에서 관리 및 처리
- ktor의 Auth plugin의 loadTokens 에서 헤더를 담아주고 있어서 필요없는 파라미터 제거
Walkthrough세션 관리 구현을 SessionStore에서 새로운 싱글톤 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ViewModel
participant AuthRemoteDataSource
participant AuthService
participant TokenManager
participant SessionManager
User->>ViewModel: loginWithIdToken(idToken)
ViewModel->>AuthRemoteDataSource: postIdToken(idToken)
AuthRemoteDataSource->>AuthService: POST /auth/tokens (idToken)
AuthService-->>AuthRemoteDataSource: LoginJwtTokenResponse
AuthRemoteDataSource-->>ViewModel: TuripResult.Success(tokens)
ViewModel->>TokenManager: setTokens(tokens)
TokenManager-->>ViewModel: Result.Success
ViewModel->>SessionManager: switchToMember()
SessionManager->>SessionManager: _state.value = SessionState.Member
SessionManager-->>ViewModel: done
sequenceDiagram
participant App
participant SplashViewModel
participant DetermineInitialSessionUseCase
participant AuthRepository
participant AuthRemoteDataSource
participant SessionManager
App->>SplashViewModel: determineStartDestination()
SplashViewModel->>DetermineInitialSessionUseCase: invoke()
DetermineInitialSessionUseCase->>AuthRepository: verifyToken()
AuthRepository->>AuthRemoteDataSource: verifyToken()
AuthRemoteDataSource-->>AuthRepository: TuripResult (Success|Failure)
DetermineInitialSessionUseCase-->>SplashViewModel: AuthStatus.Authenticated / UnAuthenticated
alt Authenticated
SplashViewModel->>SessionManager: switchToMember()
else UnAuthenticated
SplashViewModel->>SessionManager: switchToGuest()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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 docstrings
🧪 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.
Actionable comments posted: 6
🧹 Nitpick comments (1)
android/app/src/main/java/com/on/turip/data/result/ApiException.kt (1)
5-13: 잘 설계된 예외 계층 구조입니다.sealed class를 사용하여
SafeApiCall에서 exhaustive한 when 표현식을 보장합니다.선택적 개선 제안: 디버깅 및 로깅을 위해
message또는cause속성을 추가하는 것을 고려해 볼 수 있습니다.♻️ 선택적 개선안
-sealed class ApiException : Exception() { +sealed class ApiException( + message: String? = null, + cause: Throwable? = null, +) : Exception(message, cause) { data class Error( val errorType: ErrorType, - ) : ApiException() + override val message: String? = null, + ) : ApiException(message) - data object Auth : ApiException() + data object Auth : ApiException("Authentication failed") - data object Network : ApiException() + data object Network : ApiException("Network error") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/com/on/turip/data/result/ApiException.kt` around lines 5 - 13, Add optional message and/or cause to the ApiException hierarchy so callers and logs can include debug context: update sealed class ApiException and its subclasses (ApiException.Error, ApiException.Auth, ApiException.Network) to accept an optional message: String? and/or cause: Throwable? (preserve existing ErrorType on ApiException.Error) and propagate these parameters when constructing exceptions in SafeApiCall and other throw sites so logs can read exception.message and exception.cause.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/src/main/java/com/on/turip/core/session/SessionManager.kt`:
- Around line 21-27: switchToGuest currently swallows tokenManager.clearTokens()
failures and sets _state before knowing if clear succeeded; change its signature
to return Result<Unit> (e.g., suspend fun switchToGuest(): Result<Unit>), call
tokenManager.clearTokens(), if it fails return that Result immediately (do not
set _state), and only on success set _state.value = SessionState.Guest and
return Result.success(Unit); reference tokenManager.clearTokens(), _state, and
SessionState.Guest to locate the change.
In `@android/app/src/main/java/com/on/turip/data/result/SafeApiCall.kt`:
- Around line 17-22: The handling of ApiException.Error in SafeApiCall.kt
flattens to ErrorType.Unknown because e.toErrorType() doesn't read the embedded
errorType; update the catch branch to use the actual stored value (e.errorType)
or fix ApiException.Error.toErrorType() to return its internal errorType so the
TuripResult.Failure contains the original error type (refer to
ApiException.Error and the toErrorType/errorType symbols when making the
change).
In
`@android/app/src/main/java/com/on/turip/ui/compose/invitation/InvitationEntryViewModel.kt`:
- Around line 144-147: In confirmEnterInvitedTurip() add the same session
cleanup call as in the other branch by invoking switchToGuest() before sending
the navigation effect on authentication errors; specifically, locate the
auth-error handling that currently does
_uiEffect.send(InvitationEntryUiEffect.NavigateToLogin) and insert a call to
switchToGuest() (or ensure switchToGuest() is called) prior to returning so the
session is cleared on ErrorType.Auth.
- Around line 96-99: In InvitationEntryViewModel, when handling result.errorType
== ErrorType.Auth, call sessionManager.switchToGuest() before emitting
_uiEffect.send(InvitationEntryUiEffect.NavigateToLogin) so the token/session is
cleared and the app state becomes Guest; ensure sessionManager is provided as a
private constructor property on InvitationEntryViewModel (matching other
ViewModels) so switchToGuest() is accessible and invoked prior to the
NavigateToLogin emission.
In `@android/app/src/main/java/com/on/turip/ui/compose/mypage/MyPageViewModel.kt`:
- Around line 102-109: The error handling for session expiration is duplicated
only in the bookmark failure branch; centralize session-expiry handling so all
failures (e.g., loadProfile() and bookmark fetches) behave the same. Extract the
auth-check into a shared helper (e.g., handleError(errorType: ErrorType) or
unify into existing handleError()), have that helper call
sessionManager.switchToGuest() and send MyPageUiEffect.NavigateToLogin via
_uiEffect, and replace the inline session checks in onFailure blocks (including
the bookmark onFailure and loadProfile() failure) to call this helper instead.
In
`@android/app/src/main/java/com/on/turip/ui/compose/turipdetail/TuripDetailViewModel.kt`:
- Around line 713-715: In observeTuripStream(), handle the
TuripStreamResult.Fatal.TokenExpired branch the same way as sendErrorEffect():
call sessionManager.switchToGuest() to clear session state and then send the
navigation effect; specifically, update the TuripStreamResult.Fatal.TokenExpired
branch (in observeTuripStream()) to invoke sessionManager.switchToGuest() before
_uiEffect.send(TuripDetailUiEffect.NavigateToLogin) so session state remains
consistent with the other token-expiry paths.
---
Nitpick comments:
In `@android/app/src/main/java/com/on/turip/data/result/ApiException.kt`:
- Around line 5-13: Add optional message and/or cause to the ApiException
hierarchy so callers and logs can include debug context: update sealed class
ApiException and its subclasses (ApiException.Error, ApiException.Auth,
ApiException.Network) to accept an optional message: String? and/or cause:
Throwable? (preserve existing ErrorType on ApiException.Error) and propagate
these parameters when constructing exceptions in SafeApiCall and other throw
sites so logs can read exception.message and exception.cause.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 96544566-d8fd-409a-afd3-a237da655e1e
📒 Files selected for processing (51)
android/app/src/main/java/com/on/turip/core/session/SessionManager.ktandroid/app/src/main/java/com/on/turip/core/session/SessionState.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/AuthDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/AuthRefreshRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/AuthRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/DefaultAuthRefreshRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/DefaultAuthRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/DefaultGuestRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/DefaultMemberRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/GuestDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/GuestRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/MemberDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/datasource/MemberRemoteDataSource.ktandroid/app/src/main/java/com/on/turip/data/login/repository/DefaultAuthRepository.ktandroid/app/src/main/java/com/on/turip/data/login/repository/DefaultGuestRepository.ktandroid/app/src/main/java/com/on/turip/data/login/repository/DefaultMemberRepository.ktandroid/app/src/main/java/com/on/turip/data/login/service/AuthService.ktandroid/app/src/main/java/com/on/turip/data/result/ApiException.ktandroid/app/src/main/java/com/on/turip/data/result/SafeApiCall.ktandroid/app/src/main/java/com/on/turip/data/session/DefaultAuthTokenCacheController.ktandroid/app/src/main/java/com/on/turip/data/session/DefaultSessionStore.ktandroid/app/src/main/java/com/on/turip/data/turip/service/DefaultTuripStreamService.ktandroid/app/src/main/java/com/on/turip/di/DataSourceModule.ktandroid/app/src/main/java/com/on/turip/di/NetworkModule.ktandroid/app/src/main/java/com/on/turip/di/ServiceModule.ktandroid/app/src/main/java/com/on/turip/di/SessionModule.ktandroid/app/src/main/java/com/on/turip/di/SseHttpClient.ktandroid/app/src/main/java/com/on/turip/di/qualifier/AuthQualifiers.ktandroid/app/src/main/java/com/on/turip/di/qualifier/NetworkQualifiers.ktandroid/app/src/main/java/com/on/turip/domain/invitation/usecase/DetermineInvitationEntryRouteUseCase.ktandroid/app/src/main/java/com/on/turip/domain/login/AuthRepository.ktandroid/app/src/main/java/com/on/turip/domain/login/usecase/LoginUseCase.ktandroid/app/src/main/java/com/on/turip/domain/session/AuthStatus.ktandroid/app/src/main/java/com/on/turip/domain/session/SessionStore.ktandroid/app/src/main/java/com/on/turip/domain/session/usecase/DetermineInitialSessionUseCase.ktandroid/app/src/main/java/com/on/turip/domain/session/usecase/SwitchToGuestUseCase.ktandroid/app/src/main/java/com/on/turip/ui/common/error/UiErrorExtensions.ktandroid/app/src/main/java/com/on/turip/ui/compose/bookmark/BookmarkContentListViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/home/HomeViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/invitation/InvitationEntryViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/login/LoginViewmodel.ktandroid/app/src/main/java/com/on/turip/ui/compose/mypage/MyPageScreen.ktandroid/app/src/main/java/com/on/turip/ui/compose/mypage/MyPageViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/mypage/component/MyPageSettingsSection.ktandroid/app/src/main/java/com/on/turip/ui/compose/search/keyword/SearchViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/search/regionresult/RegionResultViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/splash/SplashViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/trip/TripDetailViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/trip/turipselection/PlaceTuripSelectionViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/turip/MyTuripViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/turipdetail/TuripDetailViewModel.kt
💤 Files with no reviewable changes (8)
- android/app/src/main/java/com/on/turip/data/login/datasource/GuestDataSource.kt
- android/app/src/main/java/com/on/turip/di/SseHttpClient.kt
- android/app/src/main/java/com/on/turip/di/SessionModule.kt
- android/app/src/main/java/com/on/turip/domain/session/usecase/SwitchToGuestUseCase.kt
- android/app/src/main/java/com/on/turip/data/login/datasource/MemberDataSource.kt
- android/app/src/main/java/com/on/turip/data/login/datasource/AuthDataSource.kt
- android/app/src/main/java/com/on/turip/domain/session/SessionStore.kt
- android/app/src/main/java/com/on/turip/data/session/DefaultSessionStore.kt
android/app/src/main/java/com/on/turip/ui/compose/invitation/InvitationEntryViewModel.kt
Show resolved
Hide resolved
android/app/src/main/java/com/on/turip/ui/compose/invitation/InvitationEntryViewModel.kt
Show resolved
Hide resolved
android/app/src/main/java/com/on/turip/ui/compose/mypage/MyPageViewModel.kt
Show resolved
Hide resolved
android/app/src/main/java/com/on/turip/ui/compose/turipdetail/TuripDetailViewModel.kt
Show resolved
Hide resolved
- SafeApiCall: ApiException에서 담아오는 ErrorType 활용하도록 수정 - InvitationEntryViewModel, TuripDetailViewModel: 토큰 만료시 세션 전환하도록 수정
There was a problem hiding this comment.
🧹 Nitpick comments (1)
android/app/src/main/java/com/on/turip/ui/compose/turipdetail/TuripDetailViewModel.kt (1)
192-193: 인증 실패 공통 처리 로직을 헬퍼로 추출해 중복을 줄이는 것을 권장합니다.동일한 “게스트 전환 + 로그인 이동” 시퀀스가 여러 군데 반복되어 있어, 다음 수정 때 일부 분기만 누락될 위험이 있습니다.
♻️ 제안 diff
class TuripDetailViewModel `@Inject` constructor( @@ ) : ViewModel() { + private suspend fun switchToGuestAndNavigateLogin() { + sessionManager.switchToGuest() + _uiEffect.send(TuripDetailUiEffect.NavigateToLogin) + } @@ - TuripStreamResult.Fatal.TokenExpired -> { - sessionManager.switchToGuest() - _uiEffect.send(TuripDetailUiEffect.NavigateToLogin) - } + TuripStreamResult.Fatal.TokenExpired -> { + switchToGuestAndNavigateLogin() + } @@ - if (errorType is ErrorType.Auth) { - sessionManager.switchToGuest() - _uiEffect.send(TuripDetailUiEffect.NavigateToLogin) + if (errorType is ErrorType.Auth) { + switchToGuestAndNavigateLogin() return@onFailure } @@ - if (errorType is ErrorType.Auth) { - sessionManager.switchToGuest() - _uiEffect.send(TuripDetailUiEffect.NavigateToLogin) + if (errorType is ErrorType.Auth) { + switchToGuestAndNavigateLogin() return@onFailure } @@ UiError.Global.TokenExpired -> { - sessionManager.switchToGuest() - _uiEffect.send(TuripDetailUiEffect.NavigateToLogin) + switchToGuestAndNavigateLogin() }Also applies to: 357-359, 541-543, 715-716
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/com/on/turip/ui/compose/turipdetail/TuripDetailViewModel.kt` around lines 192 - 193, Extract the repeated "switch to guest then navigate to login" sequence into a single helper method on TuripDetailViewModel (e.g., handleAuthFailureAndNavigateToLogin) that calls sessionManager.switchToGuest() and _uiEffect.send(TuripDetailUiEffect.NavigateToLogin), then replace the duplicated blocks (currently using sessionManager.switchToGuest() followed by _uiEffect.send(TuripDetailUiEffect.NavigateToLogin)) with a call to this new helper; ensure the helper is used in all occurrences (including the ones around the other noted locations) to avoid future omissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@android/app/src/main/java/com/on/turip/ui/compose/turipdetail/TuripDetailViewModel.kt`:
- Around line 192-193: Extract the repeated "switch to guest then navigate to
login" sequence into a single helper method on TuripDetailViewModel (e.g.,
handleAuthFailureAndNavigateToLogin) that calls sessionManager.switchToGuest()
and _uiEffect.send(TuripDetailUiEffect.NavigateToLogin), then replace the
duplicated blocks (currently using sessionManager.switchToGuest() followed by
_uiEffect.send(TuripDetailUiEffect.NavigateToLogin)) with a call to this new
helper; ensure the helper is used in all occurrences (including the ones around
the other noted locations) to avoid future omissions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f64fbb7f-e6cd-4940-b139-78832e20b13e
📒 Files selected for processing (3)
android/app/src/main/java/com/on/turip/data/result/SafeApiCall.ktandroid/app/src/main/java/com/on/turip/ui/compose/invitation/InvitationEntryViewModel.ktandroid/app/src/main/java/com/on/turip/ui/compose/turipdetail/TuripDetailViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- android/app/src/main/java/com/on/turip/data/result/SafeApiCall.kt
- android/app/src/main/java/com/on/turip/ui/compose/invitation/InvitationEntryViewModel.kt
jerry8282
left a comment
There was a problem hiding this comment.
깖끔한 코드 작성 너무 좋습니다 채넛!! 고생하셨습니다 몇가지 수정할 부분이 있어서 남겨보았습니다ㅎㅎ
| ) { | ||
| suspend operator fun invoke(): SessionState { | ||
| suspend operator fun invoke(): AuthStatus { | ||
| tokenManager.initialize() |
There was a problem hiding this comment.
| tokenManager.initialize() | |
| tokenManager.initialize() | |
| if (tokenManager.currentTokens == null) return AuthStatus.UnAuthenticated |
r: 현재 게스트 상태일 때 오프라인으로 들어가면 홈화면으로 가지는데 위 코드를 추가해서 로그인 화면으로 가도록 해야 될 거 같습니다
| headerInterceptor(tokenManager, authRepository, fidProvider) | ||
| } | ||
| @DefaultKtorfit | ||
| fun provideDefaultKtrofit( |
There was a problem hiding this comment.
a: provideDefaultKtorfit으로 네이밍 변경하면 좋겠네용ㅎㅎ
| @Singleton | ||
| fun provideKtorfit(httpClient: HttpClient): Ktorfit = | ||
| @NoAuthKtorfit | ||
| fun provideNoAuthKtrofit( |
There was a problem hiding this comment.
a: provideNoAuthKtrofit으로 네이밍 변경하면 좋겠네용ㅎㅎ
There was a problem hiding this comment.
r: 현재 SessionManager가 core인데 domain을 주입 받고 있어서 의존 방향이 이상한 거 같아요 domain으로 내려야 될 거 같아용
| class DefaultGuestRemoteDataSource @Inject constructor( | ||
| private val guestService: GuestService, | ||
| ) : GuestRemoteDataSource { | ||
| override suspend fun deleteGuest(): TuripResult<Unit> = safeApiCall { guestService.deleteGuest() } |
There was a problem hiding this comment.
c: DefaultAuthRefreshRemoteDataSource에서 withContext로 전부 해주셔서 여기도 처리해주면 좋을 거 같아요!
| class DefaultMemberRemoteDataSource @Inject constructor( | ||
| private val memberService: MemberService, | ||
| ) : MemberRemoteDataSource { | ||
| override suspend fun postMigration(): TuripResult<Unit> = safeApiCall { memberService.postMigration() } | ||
|
|
||
| override suspend fun postLogout(): TuripResult<Unit> = safeApiCall { memberService.postLogout() } | ||
|
|
||
| override suspend fun deleteMember(): TuripResult<Unit> = safeApiCall { memberService.deleteMember() } | ||
| } |
There was a problem hiding this comment.
c: DefaultAuthRefreshRemoteDataSource에서 withContext로 전부 해주셔서 여기도 처리해주면 좋을 거 같아요!
Issues
✔️ Check-list
🗒️ Work Description
1. 액세스 토큰과 리프레시 토큰이 모두 만료된 상황에서 재발급이 실패하는 문제 해결
문제 원인
Ktor의 Auth 플러그인에서
loadTokens { }를 통해 Bearer 토큰을 자동으로 Authorization 헤더에 추가하는데, 이 설정이refreshTokens { }블럭 내부에서 수행되는 토큰 재발급 요청에도 동일하게 적용되는 문제가 있었습니다.그 결과, 리프레시 토큰이 유효한 경우에도 만료된 액세스 토큰이 Authorization 헤더에 포함된 상태로 요청이 전송되었고,
서버에서 이를 인증 실패로 판단하여 401 응답을 반환하는 문제가 발생했습니다.
해결 방법
토큰 재발급 요청에는 Authorization 헤더가 포함되지 않도록 네트워크 계층을 분리했습니다.
이를 통해 토큰 재발급 요청이 인증 헤더의 영향을 받지 않도록 보장했습니다.
2. 세션 상태 및 토큰 초기화 시점 관리를 위한 SessionManager 도입
3. DataSource 네이밍 컨벤션 정리
4. 로그인 중단 시 잘못된 에러 메시지 노출 문제 수정
📷 Screenshot
세션 만료
default.webm
리프레시토큰으로 토큰 갱신
default.webm
📚 Reference
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Refactor