refactor: 로그아웃 구현 #687
refactor: 로그아웃 구현 #687
Conversation
…aredFlow -> Channel로 변경
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough로그아웃 기능 전반에 대한 리팩터링이 이루어졌습니다. 기존 AuthRepository 및 관련 API, DTO, 데이터소스의 로그아웃 구현이 AccountRepository와 AccountDataSource 계층으로 이동되었으며, UI 레이어에서는 LiveData 기반 상태관리에서 StateFlow 및 Channel 기반의 UI 이펙트 처리로 변경되었습니다. 네비게이션도 NavigatorProvider를 활용하도록 일원화되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as LogoutDialogFragment
participant VM as AccountInfoViewModel
participant Repo as AccountRepository
participant Remote as AccountRemoteDataSource
participant Local as AccountLocalDataSource
UI->>VM: signOut(signOutToPlatform)
activate VM
VM->>signOutToPlatform: (AuthPlatform.KAKAO)
VM->>Repo: deleteToken(deviceIdentifier)
activate Repo
Repo->>Remote: postLogout(refreshToken, deviceIdentifier)
Remote-->>Repo: (완료)
Repo->>Local: clearTokens()
Local-->>Repo: (완료)
deactivate Repo
VM->>UI: emit UiEffect.NavigateToLogin
deactivate VM
UI->>Navigator: navigateToLogin()
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
🪧 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: 4
🧹 Nitpick comments (2)
app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt (2)
67-72:suspend키워드가 불필요해 보입니다
shouldRefresh()내부에서 모든 작업이 즉시 반환되고 I/O 호출이 없다면suspend를 제거해 함수 호출 스택을 단순화할 수 있습니다.
36-42: 단순printStackTrace()대신 구조화된 로깅 권장
에러를 단순 출력하면 Crashlytics / Sentry 등에서 수집되지 않을 수 있습니다.Log.e또는 사내 공통 로거를 활용해 오류 원인과 파라미터(예:deviceIdentifier)를 기록해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/build.gradle.kts(1 hunks)app/src/main/java/com/into/websoso/data/remote/api/AuthApi.kt(0 hunks)app/src/main/java/com/into/websoso/data/repository/AuthRepository.kt(0 hunks)app/src/main/java/com/into/websoso/ui/accountInfo/AccountInfoViewModel.kt(3 hunks)app/src/main/java/com/into/websoso/ui/accountInfo/LogoutDialogFragment.kt(1 hunks)app/src/main/java/com/into/websoso/ui/splash/SplashActivity.kt(1 hunks)app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt(1 hunks)app/src/main/java/com/into/websoso/ui/splash/dialog/MinimumVersionDialogFragment.kt(1 hunks)core/datastore/src/main/java/com/into/websoso/core/datastore/datasource/account/DefaultAccountDataSource.kt(1 hunks)core/network/src/main/java/com/into/websoso/core/network/datasource/account/AccountApi.kt(2 hunks)core/network/src/main/java/com/into/websoso/core/network/datasource/account/DefaultAccountDataSource.kt(2 hunks)core/network/src/main/java/com/into/websoso/core/network/datasource/account/model/KakaoLogoutRequestDto.kt(1 hunks)data/account/src/main/java/com/into/websoso/data/account/AccountRepository.kt(2 hunks)data/account/src/main/java/com/into/websoso/data/account/datasource/AccountLocalDataSource.kt(1 hunks)data/account/src/main/java/com/into/websoso/data/account/datasource/AccountRemoteDataSource.kt(1 hunks)feature/signin/src/main/java/com/into/websoso/feature/signin/SignInScreen.kt(1 hunks)feature/signin/src/main/java/com/into/websoso/feature/signin/SignInViewModel.kt(1 hunks)
💤 Files with no reviewable changes (2)
- app/src/main/java/com/into/websoso/data/remote/api/AuthApi.kt
- app/src/main/java/com/into/websoso/data/repository/AuthRepository.kt
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/build.gradle.kts (1)
build-logic/src/main/kotlin/com/into/websoso/WebsosoDependenciesExtensions.kt (1)
implementation(70-76)
data/account/src/main/java/com/into/websoso/data/account/AccountRepository.kt (3)
data/account/src/main/java/com/into/websoso/data/account/datasource/AccountLocalDataSource.kt (1)
refreshToken(6-6)core/datastore/src/main/java/com/into/websoso/core/datastore/datasource/account/DefaultAccountDataSource.kt (1)
refreshToken(29-33)core/network/src/main/java/com/into/websoso/core/network/datasource/account/model/TokenReissueRequestDto.kt (1)
refreshToken(6-10)
app/src/main/java/com/into/websoso/ui/splash/SplashActivity.kt (1)
app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt (1)
updateUserDeviceIdentifier(36-44)
🔇 Additional comments (15)
app/build.gradle.kts (1)
86-86: 로그아웃 기능을 위한 의존성 추가로그아웃 기능 리팩토링의 일환으로 AccountRepository를 사용하기 위해
projects.data.account의존성이 추가되었습니다. 이는 기존의 AuthRepository에서 AccountRepository로 로그아웃 관련 기능이 이전되었음을 나타냅니다.feature/signin/src/main/java/com/into/websoso/feature/signin/SignInScreen.kt (1)
65-65: 매개변수 이름 개선:getToken에서signInToPlatform으로 변경매개변수 이름을 변경한 것은 함수의 목적을 더 명확하게 표현합니다. 단순히 토큰을 가져오는 것보다 플랫폼에 로그인하는 의미가 더 정확합니다.
app/src/main/java/com/into/websoso/ui/splash/dialog/MinimumVersionDialogFragment.kt (1)
37-38: 다이얼로그 태그 상수 추가최소 버전 업데이트 다이얼로그 표시를 위한 태그 상수를 추가했습니다. 이 태그는
SplashActivity와SplashViewModel에서 다이얼로그 표시를 위해 사용됩니다. UI 이펙트 흐름에 따라 다이얼로그를 표시하는 새로운 방식에 맞게 통합되었습니다.data/account/src/main/java/com/into/websoso/data/account/datasource/AccountRemoteDataSource.kt (1)
14-18: 로그아웃 함수 추가AccountRemoteDataSource 인터페이스에 로그아웃 기능을 위한
postLogout함수가 추가되었습니다. 이 함수는 refreshToken과 deviceIdentifier를 매개변수로 받아 서버에 로그아웃 요청을 보냅니다. 이는 기존 AuthRepository와 AuthApi에서 제거된logout기능을 대체하는 부분으로, 로그아웃 기능 리팩토링의 핵심입니다.인터페이스 설계가 명확하며 필요한 매개변수들이 잘 정의되어 있습니다.
core/network/src/main/java/com/into/websoso/core/network/datasource/account/DefaultAccountDataSource.kt (1)
34-44: 로그아웃 메서드가 잘 구현되었습니다.
postLogout메서드가 적절하게 구현되었습니다.refreshToken과deviceIdentifier를 파라미터로 받아KakaoLogoutRequestDto를 생성하고 API 호출을 수행하는 흐름이 깔끔합니다. 로그아웃 기능을 AuthRepository에서 AccountRepository 계층으로 옮겨 책임 분리가 잘 이루어졌습니다.core/network/src/main/java/com/into/websoso/core/network/datasource/account/AccountApi.kt (1)
23-26: 로그아웃 API 엔드포인트가 잘 정의되었습니다.
postLogoutWithKakao메서드가 적절하게 정의되었습니다.@POST("auth/logout")으로 올바른 엔드포인트를 지정하고, 요청 본문으로KakaoLogoutRequestDto를 사용하는 구조가 명확합니다. 기존 API 메서드들과 일관성 있게 구현되었습니다.feature/signin/src/main/java/com/into/websoso/feature/signin/SignInViewModel.kt (1)
30-30: 파라미터 이름이 더 명확하게 개선되었습니다.
getToken에서signInToPlatform으로 파라미터 이름을 변경한 것은 해당 함수의 역할을 더 정확히 표현합니다. 이 변경은 코드 가독성을 향상시키고 인증 흐름에 대한 이해를 돕습니다. 기존 로그아웃 로직을 리팩토링하는 전체적인 변경 사항과도 일관성이 있습니다.Also applies to: 34-34
data/account/src/main/java/com/into/websoso/data/account/datasource/AccountLocalDataSource.kt (1)
12-12: 토큰 초기화 기능이 적절하게 추가되었습니다.
clearTokens()메서드 추가는 로그아웃 흐름에서 필요한 기능으로 잘 구현되었습니다. 기존의 자동 로그인 상태 관리 메서드들을 대체하고 토큰 초기화에 집중하는 방식으로 개선된 점이 좋습니다. 코드가 간결해지고 책임이 명확해졌습니다.data/account/src/main/java/com/into/websoso/data/account/AccountRepository.kt (2)
10-10: TODO 잔존: 개발 완료 전에 반드시 처리 필요
// TODO:가 그대로 남아 있습니다. 실제 배포 전에 싱글톤 패턴‧네이밍‧Result 객체 적용 여부를 검토하고 주석을 제거해 주세요.
[ suggest_nitpick ]
37-43: 예외 발생 시 토큰이 삭제되지 않는 문제현재 구현은
postLogout()호출이 예외를 던지면clearTokens()가 실행되지 않아, 로컬에 남은 토큰으로 인해 재로그인 흐름이 꼬일 수 있습니다.finally블록(혹은runCatching)으로 토큰 삭제를 보장해 주세요.- suspend fun deleteToken(deviceIdentifier: String) { - accountRemoteDataSource - .postLogout( - refreshToken = refreshToken(), - deviceIdentifier = deviceIdentifier, - ).also { accountLocalDataSource.clearTokens() } - } + suspend fun deleteToken(deviceIdentifier: String) { + try { + accountRemoteDataSource.postLogout( + refreshToken = refreshToken(), + deviceIdentifier = deviceIdentifier, + ) + } catch (e: Exception) { + // TODO: 필요하다면 로그 기록 + } finally { + accountLocalDataSource.clearTokens() + } + }[ raise_critical_issue ]
core/network/src/main/java/com/into/websoso/core/network/datasource/account/model/KakaoLogoutRequestDto.kt (1)
6-12: DTO 명칭 범용성 제고 제안향후 카카오 외 플랫폼에서도 동일 DTO를 사용할 가능성이 있다면,
Kakao접두어를 제거해LogoutRequestDto등 범용 이름으로 변경하는 편이 유지보수에 유리합니다.
[ suggest_optional_refactor ]core/datastore/src/main/java/com/into/websoso/core/datastore/datasource/account/DefaultAccountDataSource.kt (1)
47-51: 로컬 토큰 삭제 구현 확인 완료
DataStore.edit { remove(...) }방식으로 토큰을 안전하게 삭제하고 있어 문제없습니다.
[ approve_code_changes ]app/src/main/java/com/into/websoso/ui/accountInfo/LogoutDialogFragment.kt (2)
38-44: 로그아웃 후 Fragment 미종료 가능성
NavigateToLogin으로 이동한 뒤LogoutDialogFragment가dismiss()되지 않으면 백스택에 남을 수 있습니다. 내비게이션 직후에dismiss()혹은findNavController().popBackStack()호출을 고려해 주세요.
[ suggest_nitpick ]
51-56: UI 스레드에서 블로킹 호출 가능성
authClient[platform]?.signOut()이 네트워크 I/O를 포함한 블로킹 연산이라면 UI 스레드에서 호출되어 ANR이 발생할 수 있습니다. ViewModel 쪽에서withContext(Dispatchers.IO)로 감싸거나, 여기서CoroutineScope.launch로 offload하는 방안을 검토해 주세요.
[ request_verification ]app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt (1)
46-51: 버전 확인 실패 시 ‘업데이트 필요 없음’으로 처리하면 잠재적 보안 리스크
네트워크 오류 등으로isUpdateRequired()호출이 실패하면, 현재 로직은false로 간주해 바로 다음 단계로 진행합니다. 실제로는 구(舊) 버전이 차단돼야 하는 상황에서 사용자가 우회 진입할 가능성이 있습니다.예외 시에는 보수적으로
true로 간주하거나, 별도의 오류 다이얼로그를 띄워 재시도하도록 하는 쪽이 안전합니다.
| private var _uiEffect = Channel<UiEffect>(Channel.BUFFERED) | ||
| val uiEffect: Flow<UiEffect> get() = _uiEffect.receiveAsFlow() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Channel → SharedFlow로 전환 및 var 삭제를 고려해 주세요
Channel을 노출할 때마다 receiveAsFlow() 로 래핑해야 하고, var 로 선언되어 재할당 가능성이 열려 있습니다. 단방향 이벤트 스트림이면 MutableSharedFlow(+ extraBufferCapacity)가 보다 간결‧안정적이며, val 로 선언하면 재할당 우려도 사라집니다.
- private var _uiEffect = Channel<UiEffect>(Channel.BUFFERED)
- val uiEffect: Flow<UiEffect> get() = _uiEffect.receiveAsFlow()
+ private val _uiEffect = MutableSharedFlow<UiEffect>(extraBufferCapacity = 1)
+ val uiEffect: SharedFlow<UiEffect> get() = _uiEffect.asSharedFlow()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private var _uiEffect = Channel<UiEffect>(Channel.BUFFERED) | |
| val uiEffect: Flow<UiEffect> get() = _uiEffect.receiveAsFlow() | |
| private val _uiEffect = MutableSharedFlow<UiEffect>(extraBufferCapacity = 1) | |
| val uiEffect: SharedFlow<UiEffect> get() = _uiEffect.asSharedFlow() |
🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt around lines
26 to 28, replace the private Channel<UiEffect> _uiEffect and its public
Flow<UiEffect> uiEffect with a private val MutableSharedFlow<UiEffect>
initialized with an appropriate extraBufferCapacity, and expose it as a public
val SharedFlow<UiEffect>. Change the declaration from var to val to prevent
reassignment and simplify usage by removing the need to call receiveAsFlow()
each time.
| private fun updateUserDeviceIdentifier() { | ||
| val ssaid = Settings.Secure.getString( | ||
| this.contentResolver, | ||
| Settings.Secure.ANDROID_ID, | ||
| ) | ||
| splashViewModel.updateUserDeviceIdentifier(deviceIdentifier = ssaid) | ||
| val deviceId = getString(contentResolver, ANDROID_ID) | ||
| splashViewModel.updateUserDeviceIdentifier(deviceIdentifier = deviceId) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ANDROID_ID 는 nullable → NPE 예방 필요
Settings.Secure.getString() 은 String? 을 반환합니다. 바로 전달하면 뷰모델에서 String non-null 로 기대하기 때문에 기기 일부에서 NPE 가 발생할 수 있습니다.
- val deviceId = getString(contentResolver, ANDROID_ID)
+ val deviceId = getString(contentResolver, ANDROID_ID).orEmpty()🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/splash/SplashActivity.kt around lines
34 to 37, the ANDROID_ID retrieved using getString(contentResolver, ANDROID_ID)
is nullable and passing it directly to the view model expecting a non-null
String can cause a NullPointerException. To fix this, add a null check or
provide a default non-null value before passing deviceId to
splashViewModel.updateUserDeviceIdentifier, ensuring the argument is always a
non-null String.
| private var _uiEffect = Channel<UiEffect>(Channel.BUFFERED) | ||
| val uiEffect: Flow<UiEffect> get() = _uiEffect.receiveAsFlow() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Channel → SharedFlow + val 로 일관성 확보
SplashViewModel 과 동일한 패턴입니다. 이벤트 스트림이면 MutableSharedFlow 사용이 더 직관적이며 var 제거로 재할당 위험을 없앨 수 있습니다.
🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/accountInfo/AccountInfoViewModel.kt
around lines 32 to 34, replace the Channel-based event stream with a
MutableSharedFlow to align with the SplashViewModel pattern. Change the private
mutable property to a val MutableSharedFlow and expose it as a SharedFlow to
prevent reassignment risks and improve clarity for event streams.
| fun signOut(signOutToPlatform: suspend (platform: AuthPlatform) -> Unit) { | ||
| viewModelScope.launch { | ||
| runCatching { | ||
| signOutToPlatform(AuthPlatform.KAKAO) | ||
| val userDeviceIdentifier = userRepository.fetchUserDeviceIdentifier() | ||
| authRepository.logout(userDeviceIdentifier) | ||
| accountRepository.deleteToken(userDeviceIdentifier) | ||
| }.onSuccess { | ||
| _isLogoutSuccess.value = true | ||
| authRepository.updateIsAutoLogin(false) | ||
| pushMessageRepository.clearFCMToken() | ||
| _uiEffect.send(NavigateToLogin) | ||
| }.onFailure { | ||
| _isLogoutSuccess.value = false | ||
| _uiEffect.send(NavigateToLogin) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
FCM 토큰 제거가 실패 경로에서 누락될 수 있습니다
pushMessageRepository.clearFCMToken() 이 onSuccess 에만 위치해 있어, 서버 로그아웃 실패 시 FCM 토큰이 남아버립니다. 성공‧실패와 무관하게 실행하도록 리팩터링해 주세요.
- }.onSuccess {
- pushMessageRepository.clearFCMToken()
- _uiEffect.send(NavigateToLogin)
- }.onFailure {
- _uiEffect.send(NavigateToLogin)
- }
+ }.onSuccess {
+ /* no-op */
+ }.onFailure {
+ /* 로그 등 추가 */
+ }.also {
+ // 무조건 실행
+ pushMessageRepository.clearFCMToken()
+ _uiEffect.send(NavigateToLogin)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun signOut(signOutToPlatform: suspend (platform: AuthPlatform) -> Unit) { | |
| viewModelScope.launch { | |
| runCatching { | |
| signOutToPlatform(AuthPlatform.KAKAO) | |
| val userDeviceIdentifier = userRepository.fetchUserDeviceIdentifier() | |
| authRepository.logout(userDeviceIdentifier) | |
| accountRepository.deleteToken(userDeviceIdentifier) | |
| }.onSuccess { | |
| _isLogoutSuccess.value = true | |
| authRepository.updateIsAutoLogin(false) | |
| pushMessageRepository.clearFCMToken() | |
| _uiEffect.send(NavigateToLogin) | |
| }.onFailure { | |
| _isLogoutSuccess.value = false | |
| _uiEffect.send(NavigateToLogin) | |
| } | |
| fun signOut(signOutToPlatform: suspend (platform: AuthPlatform) -> Unit) { | |
| viewModelScope.launch { | |
| runCatching { | |
| signOutToPlatform(AuthPlatform.KAKAO) | |
| val userDeviceIdentifier = userRepository.fetchUserDeviceIdentifier() | |
| accountRepository.deleteToken(userDeviceIdentifier) | |
| }.onSuccess { | |
| /* no-op */ | |
| }.onFailure { | |
| /* 로그 등 추가 */ | |
| }.also { | |
| // 무조건 실행 | |
| pushMessageRepository.clearFCMToken() | |
| _uiEffect.send(NavigateToLogin) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/accountInfo/AccountInfoViewModel.kt
around lines 49 to 60, the call to pushMessageRepository.clearFCMToken() is only
inside the onSuccess block, so it is not executed if signOutToPlatform or token
deletion fails. Move pushMessageRepository.clearFCMToken() outside of the
onSuccess and onFailure blocks so it runs regardless of success or failure,
ensuring the FCM token is always cleared.
There was a problem hiding this comment.
@s9hn 말씀하신 내용이 맞습니다. 로그아웃이 실패한 경우에는 사용자가 여전히 로그인 상태로 남아있어야 하므로 FCM 토큰을 지우는 것이 적절하지 않습니다. 원래 구현대로 성공 경로에서만 FCM 토큰을 지우는 것이 맞습니다. 제안 내용을 철회합니다.
✏️ Learnings added
Learnt from: s9hn
PR: Team-WSS/WSS-Android#687
File: app/src/main/java/com/into/websoso/ui/accountInfo/AccountInfoViewModel.kt:0-0
Timestamp: 2025-05-15T17:11:19.809Z
Learning: 로그아웃 성공 케이스에서만 FCM 토큰을 제거해야 합니다. 로그아웃 실패 시에는 사용자가 여전히 로그인 상태이므로 FCM 토큰을 유지해야 합니다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴