-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: 자동로그인 로직 구현 #686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1. 라이브데이터 제거 2. UiEffect로 통합 관리 3. 함수 네이밍 변경 4. 자동 로그인 로직 새롭게 구현
1. 불필요한 바인딩 제거 2. 옵저빙 함수 -> collect 함수료 고체 3. 이벤트 통합관리 4. navigator를 통한 뷰 전환, 기존 함수 삭제
|
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자동 로그인 로직이 전면적으로 리팩터링되었습니다. LiveData 기반의 분산된 상태 및 네비게이션 처리가 단일 SharedFlow 기반의 UI 효과 처리로 통합되었고, 관련 데이터 소스 및 저장소에서 auto-login 관련 코드가 제거되었습니다. Splash 화면의 네비게이션 흐름과 ViewModel의 토큰 검증 및 갱신 로직이 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant SplashActivity
participant SplashViewModel
participant AccountRepository
participant VersionRepository
participant UserRepository
SplashActivity->>SplashViewModel: onCreate()
SplashViewModel->>VersionRepository: checkMinimumVersion()
alt 업데이트 필요
SplashViewModel-->>SplashActivity: uiEffect(ShowDialog)
else 업데이트 불필요
SplashViewModel->>AccountRepository: shouldRefresh()
alt 토큰 없음/유효하지 않음
SplashViewModel-->>SplashActivity: uiEffect(NavigateToLogin)
else 토큰 유효
SplashViewModel->>AccountRepository: renewToken()
alt 성공
SplashViewModel-->>SplashActivity: uiEffect(NavigateToMain)
else 실패
SplashViewModel-->>SplashActivity: uiEffect(NavigateToLogin)
end
end
end
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt (2)
26-27: 불변 프로퍼티로 의도 명확화 및 재할당 방지
_uiEffect는 객체 생성 이후 재할당되지 않으므로var대신val로 선언하여 의도를 명확히 하고, 실수로 재할당되는 것을 방지하는 편이 좋습니다.- private var _uiEffect: MutableSharedFlow<UiEffect> = MutableSharedFlow(replay = 1) + private val _uiEffect = MutableSharedFlow<UiEffect>(replay = 1)
54-68: 토큰 갱신 실패 시 재시도·백오프 전략 부재
accountRepository.renewToken()실패 시 곧바로 로그인 화면으로 이동합니다.
일시적인 네트워크 오류나 서버 오류일 경우 사용자가 불필요하게 로그인 절차를 거치게 될 수 있으므로,
재시도(백오프 포함) 또는 사용자에게 “다시 시도” 옵션을 제공하는 UX 개선을 고려해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/build.gradle.kts(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(0 hunks)data/account/src/main/java/com/into/websoso/data/account/AccountRepository.kt(0 hunks)data/account/src/main/java/com/into/websoso/data/account/datasource/AccountLocalDataSource.kt(0 hunks)
💤 Files with no reviewable changes (3)
- data/account/src/main/java/com/into/websoso/data/account/AccountRepository.kt
- data/account/src/main/java/com/into/websoso/data/account/datasource/AccountLocalDataSource.kt
- core/datastore/src/main/java/com/into/websoso/core/datastore/datasource/account/DefaultAccountDataSource.kt
🧰 Additional context used
🧬 Code Graph Analysis (2)
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(34-42)
app/build.gradle.kts (1)
build-logic/src/main/kotlin/com/into/websoso/WebsosoDependenciesExtensions.kt (1)
implementation(70-76)
🔇 Additional comments (3)
app/build.gradle.kts (1)
85-87:Details
❓ Verification inconclusive
data.account모듈 추가 시 의존성 그래프 확인 필요앱 모듈에서 data-layer 모듈을 직접 참조하면,
• feature → data → core 로 이어지는 기존 아키텍처 계층 구조에 순환 의존성이 생길 수 있습니다.
• DI 그래프가 중복 주입되면서 빌드 시간이 늘어나거나Duplicate class컴파일 오류가 발생할 위험도 있습니다.Gradle
dependencies태스크로 의존성 그래프를 한번 점검해 주세요.문제가 없다면 그대로 두셔도 무방합니다.
🏁 Script executed:
./gradlew :app:dependencies --configuration releaseRuntimeClasspath | grep -i "data.account"Length of output: 480
의존성 그래프 확인 필요 (build 오류 발생)
앱 모듈에서
data.account를 직접 참조할 경우 순환 의존성이나 DI 중복 주입으로 인한 빌드 오류 위험이 있습니다.
하지만 제안된 아래 Gradle 의존성 검사 명령을 실행하는 중app/build.gradle.kts27번째 줄에서value must not be null오류가 발생해 의존성 트리를 확인할 수 없는 상태입니다.조치 사항:
- 먼저
app/build.gradle.kts27번째 줄의 null 관련 설정 값을 확인·수정하세요.- 수정 후, 다음 명령어로 다시 의존성 그래프를 검토해 주세요:
./gradlew :app:dependencies --configuration releaseRuntimeClasspath | grep -i "data.account"문제가 없다면 그대로 두셔도 무방합니다.
app/src/main/java/com/into/websoso/ui/splash/dialog/MinimumVersionDialogFragment.kt (1)
37-38: TAG 상수 분리 👍
TAG상수를 Dialog 내부로 이동하여 책임 범위가 명확해졌습니다. 추가 변경 없이 그대로 사용해도 됩니다.app/src/main/java/com/into/websoso/ui/splash/SplashActivity.kt (1)
40-46: UI 효과 수집 로직의 lifecycle 안전성 확인
collectWithLifecycle(this)확장 함수가Lifecycle.State.STARTED이상에서 Flow 를 수집하도록 구현돼 있다면 문제없습니다.
확실치 않다면repeatOnLifecycle(Lifecycle.State.STARTED)사용 여부를 검토해 주세요.
| @SuppressLint("HardwareIds") | ||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ANDROID_ID 널 가능성 미처리로 컴파일 불가
Settings.Secure.getString()의 반환형은 String? 입니다.
아래처럼 Nullable → Non-Null 변환 없이 그대로 넘기면 컴파일 에러가 발생합니다.
val deviceId = getString(contentResolver, ANDROID_ID) // String?
splashViewModel.updateUserDeviceIdentifier(deviceIdentifier = deviceId) // String다음 중 한 가지로 수정해 주세요.
- val deviceId = getString(contentResolver, ANDROID_ID)
- splashViewModel.updateUserDeviceIdentifier(deviceIdentifier = deviceId)
+ val deviceId = getString(contentResolver, ANDROID_ID) ?: return
+ splashViewModel.updateUserDeviceIdentifier(deviceIdentifier = deviceId)또는 ViewModel 메서드의 파라미터를 String? 로 변경 후, 내부에서 null 체크를 수행해도 됩니다.
🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/splash/SplashActivity.kt around lines
33 to 37, the variable deviceId is assigned a nullable String from
getString(contentResolver, ANDROID_ID) but passed to a method expecting a
non-null String, causing a compile error. Fix this by either safely unwrapping
deviceId with a null check or the Elvis operator to provide a default non-null
value before passing it to updateUserDeviceIdentifier, or alternatively, modify
the ViewModel method parameter to accept a nullable String and handle null
inside that method.
| checkMinimumVersion() | ||
| handleAutoLogin() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
동시 UI 이펙트 발행으로 인한 충돌 가능성
checkMinimumVersion()과 handleAutoLogin()이 각각 별도의 코루틴으로 동시에 실행되기 때문에
- 업데이트가 필요한 경우
ShowDialog발행 후 곧바로NavigateToLogin/Main이펙트가 뒤따라 - 다이얼로그가 표시되기 전에 화면이 이동하거나, 다이얼로그 뒤에서 화면이 변경되는 현상
이 발생할 수 있습니다. 두 로직 간 선후 관계가 명확하다면 순차 실행(예: 최소 버전 검사 → 통과 시 자동 로그인)을 고려해주세요.
🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt around lines
30 to 32, the functions checkMinimumVersion() and handleAutoLogin() are
currently launched concurrently, causing UI effect conflicts such as dialogs
showing while navigation occurs. To fix this, refactor the code to execute these
functions sequentially: first complete checkMinimumVersion() and only if it
passes, then call handleAutoLogin(). This ensures the dialog is shown and
handled before any navigation effects are triggered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai 아주 좋은 의견 b
하나의 스코프로 관리해 순서를 보장하도록 수정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s9hn 네, 하나의 코루틴 스코프로 두 함수의 실행 순서를 관리하는 것이 좋은 접근 방식입니다. 이렇게 하면 UI 효과 간의 충돌을 방지하고 사용자 경험을 향상시킬 수 있습니다. 수정 감사합니다! 👍
| private fun checkMinimumVersion() { | ||
| viewModelScope.launch { | ||
| runCatching { | ||
| userRepository.fetchMyProfile() | ||
| }.onSuccess { | ||
| autoLogin() | ||
| }.onFailure { | ||
| authRepository.clearTokens() | ||
| _error.value = true | ||
| versionRepository.isUpdateRequired() | ||
| }.onSuccess { isRequired -> | ||
| if (isRequired) _uiEffect.emit(ShowDialog) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
실패 케이스 로깅 및 사용자 피드백 누락
runCatching { versionRepository.isUpdateRequired() } 호출에서 onFailure 처리가 없습니다.
네트워크 오류 등이 발생해도 아무 이펙트도 발행하지 않아 사용자는 무응답 상태를 겪을 수 있습니다.
예시처럼 로깅과 함께 적절한 fallback 네비게이션 또는 재시도 지점을 마련해 주세요.
}.onSuccess { isRequired ->
if (isRequired) _uiEffect.emit(ShowDialog)
- }
+ }.onFailure { throwable ->
+ // TODO: Crashlytics 등의 로깅
+ throwable.printStackTrace()
+ // 예: 안전하게 로그인 화면으로 이동
+ _uiEffect.emit(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.
| private fun checkMinimumVersion() { | |
| viewModelScope.launch { | |
| runCatching { | |
| userRepository.fetchMyProfile() | |
| }.onSuccess { | |
| autoLogin() | |
| }.onFailure { | |
| authRepository.clearTokens() | |
| _error.value = true | |
| versionRepository.isUpdateRequired() | |
| }.onSuccess { isRequired -> | |
| if (isRequired) _uiEffect.emit(ShowDialog) | |
| } | |
| } | |
| } | |
| private fun checkMinimumVersion() { | |
| viewModelScope.launch { | |
| runCatching { | |
| versionRepository.isUpdateRequired() | |
| }.onSuccess { isRequired -> | |
| if (isRequired) _uiEffect.emit(ShowDialog) | |
| }.onFailure { throwable -> | |
| // TODO: Crashlytics 등의 로깅 | |
| throwable.printStackTrace() | |
| // 예: 안전하게 로그인 화면으로 이동 | |
| _uiEffect.emit(NavigateToLogin) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/splash/SplashViewModel.kt around lines
44 to 52, the runCatching block calling versionRepository.isUpdateRequired()
lacks onFailure handling, so network errors or other failures do not trigger any
user feedback or logging. Add an onFailure block to log the error and emit a
suitable UI effect to inform the user or provide a fallback navigation or retry
option, ensuring the app responds appropriately to failures.
…aredFlow -> Channel로 변경
…aredFlow -> Channel로 변경
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
스플래시 뷰에서 무조건 1회 `토큰재갱신 API(/reissue)'를 호출합니다.
앱 내에서의 토큰 만료 가능성을 줄여 관련 사이드 이펙트를 최소화합니다.
자동로그인 시나리오