-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix/#148] 토큰 401 관리 및 투데이 시간 파싱 #149
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
imtaejugkim
left a comment
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.
고생하셨습니다~
| package org.memento.core.event | ||
|
|
||
| import kotlinx.coroutines.flow.MutableSharedFlow | ||
|
|
||
| object GlobalLogoutEvent { | ||
| val trigger = MutableSharedFlow<Unit>(replay = 0) | ||
| } |
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.
p3 : eventbus 와 mutablesharedflow 이벤트 구독이 이번에 제가 구현한 todo, today event 알리는 패턴과 굉장히 유사해보이네요!
현재 구현도 잘하셨지만, 이렇게 구현되면 SettingViewModel의 logout()함수 로직의 역할이 모호하고, 이벤트 타입이 부족할 것 같다는 생각도 들어요. 추가로 제가 ToReviewer에 작성했던 구독자 관련 메모리 누수 위험성도 체킹이 필요하구요!
이벤트 타입 개선 관련해서는 sealedclass로 각 이벤트 타입을 정의하고, emit을 globallogoutevent에서 처리하는 방법으로 수정할 수 있을거같아요. 예를 들어,
sealed class LogoutEvent {
object UserLogout : LogoutEvent()
object AccountDeleted : LogoutEvent()
data class TokenExpired(val reason: String) : LogoutEvent()
}와 같이 이벤트 타입을 정의한 seald class 를 만들었다고 하면, 다음과 같이 해당 싱글톤에서 관리가 될 수 있을 것 같습니다.
| package org.memento.core.event | |
| import kotlinx.coroutines.flow.MutableSharedFlow | |
| object GlobalLogoutEvent { | |
| val trigger = MutableSharedFlow<Unit>(replay = 0) | |
| } | |
| object GlobalLogoutEvent { | |
| private val _trigger = MutableSharedFlow<LogoutEvent>(replay = 0) | |
| val trigger: SharedFlow<LogoutEvent> = _trigger.asSharedFlow() | |
| suspend fun emit(event: LogoutEvent) { | |
| try { | |
| _trigger.emit(event) | |
| } catch (e: Exception) { | |
| Timber.e(e, "로그아웃 이벤트 발생 실패") | |
| } | |
| } | |
| } |
이렇게 되면, 기존에 모호했던 logout()함수에서도 다음과 같이 관리가 될 것 같아요
fun logout() {
viewModelScope.launch {
try {
tokenDataStore.clearInfo()
GlobalLogoutEvent.emit(LogoutEvent.UserLogout)
} catch (e: Exception) {
Timber.e(e, "로그아웃 처리 실패")
}
}
}
// UI
LaunchedEffect(Unit) {
GlobalLogoutEvent.trigger.collect { event ->
when (event) {
is LogoutEvent.UserLogout -> {
viewModel.setLoggedOut()
// TODO()
}
is LogoutEvent.AccountDeleted -> {
viewModel.setLoggedOut()
// TODO()
}
is LogoutEvent.TokenExpired -> {
viewModel.setLoggedOut()
// TODO()
}
}
}
}이렇게 한번 구독자와 event bus 정리해보시면 좋을 것 같아요~!
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.
애햄님이 올린 pr 보고 다시 읽어보니 정리가 됐어요 !! 이미 애햄오빠가 만들어놓은 EventType 활용해서 제 코드를 리팩토링 할 수 있겠군요!! 정리 감사합니다!!
| fun formatTimeRangeWithDuration( | ||
| start: String, | ||
| end: String, | ||
| ): String { | ||
| return try { | ||
| val startTime = LocalDateTime.parse(start, isoFormatter) | ||
| val endTime = LocalDateTime.parse(end, isoFormatter) | ||
| val duration = Duration.between(startTime, endTime) | ||
|
|
||
| val formattedStart = | ||
| if (startTime.minute == 0) { | ||
| startTime.format(DateTimeFormatter.ofPattern("h a", Locale.ENGLISH)) | ||
| } else { | ||
| startTime.format(DateTimeFormatter.ofPattern("h:mm a", Locale.ENGLISH)) | ||
| } | ||
|
|
||
| val formattedEnd = | ||
| if (endTime.minute == 0) { | ||
| endTime.format(DateTimeFormatter.ofPattern("h a", Locale.ENGLISH)) | ||
| } else { | ||
| endTime.format(DateTimeFormatter.ofPattern("h:mm a", Locale.ENGLISH)) | ||
| } | ||
|
|
||
| val durationString = | ||
| buildString { | ||
| val hours = duration.toHours() | ||
| val minutes = duration.toMinutes() % 60 | ||
| if (hours > 0) append("${hours}h") | ||
| if (minutes > 0) { | ||
| if (isNotEmpty()) append(" ") | ||
| append("${minutes}m") | ||
| } | ||
| } | ||
| "$formattedStart - $formattedEnd${if (durationString.isNotEmpty()) " ($durationString)" else ""}" | ||
| } catch (e: Exception) { | ||
| "$start - $end" | ||
| } | ||
| } | ||
|
|
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.
p3 : 포맷팅 정리 한번 해야겠네요
hyoeunjoo
left a comment
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.
태정오빠가 너무 잘 남겨줘서.. 제가 더 남길 말이 없네요..?ㅠ
햄동이 고생 많았구 태정오빠도 좋은 코리 고생많았슈~!!
…-time # Conflicts: # app/src/main/java/org/memento/presentation/today/TodayViewModel.kt
hyoeunjoo
left a comment
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.
히동이 고생많아써요 💯 🥇 ❤️
| fun NavController.navigationOnboarding1(navOptions: NavOptions? = null) { | ||
| navigate("OnboardingScreen1", navOptions) | ||
| navigate(OnboardingRoute.OnboardingScreen1.route, navOptions) | ||
| } | ||
|
|
||
| fun NavController.navigationOnboarding2(navOptions: NavOptions? = null) { | ||
| navigate("OnboardingScreen2", navOptions) | ||
| navigate(OnboardingRoute.OnboardingScreen2.route, navOptions) | ||
| } | ||
|
|
||
| fun NavController.navigationOnboarding3(navOptions: NavOptions? = null) { | ||
| navigate("OnboardingScreen3", navOptions) | ||
| navigate(OnboardingRoute.OnboardingScreen3.route, navOptions) | ||
| } | ||
|
|
||
| fun NavController.navigationOnboarding4(navOptions: NavOptions? = null) { | ||
| navigate("OnboardingScreen4", navOptions) | ||
| navigate(OnboardingRoute.OnboardingScreen4.route, navOptions) | ||
| } | ||
|
|
||
| fun NavGraphBuilder.onboardingNavGraph( | ||
| navigateToOnboardingScreen1: () -> Unit, | ||
| navigateToOnboardingScreen2: () -> Unit, | ||
| navigateToOnboardingScreen3: () -> Unit, | ||
| navigateToOnboardingScreen4: () -> Unit, | ||
| navigateToMainScreen: () -> Unit, | ||
| popBackStack: () -> Unit, | ||
| navController: NavController, | ||
| ) { | ||
| composable("LoginScreen") { | ||
| composable(OnboardingRoute.Login.route) { | ||
| LoginScreen( | ||
| navigationToOnboardingScreen1 = navigateToOnboardingScreen1, | ||
| navigationToMainScreen = navigateToMainScreen, | ||
| navigationToOnboardingScreen1 = { navController.navigationOnboarding1() }, | ||
| ) | ||
| } | ||
| composable("OnboardingScreen1") { | ||
| OnboardingScreen1(navigateToOnboardingScreen2 = navigateToOnboardingScreen2, navigateToOnboardingScreen4 = navigateToOnboardingScreen4) | ||
| composable(OnboardingRoute.OnboardingScreen1.route) { | ||
| OnboardingScreen1( | ||
| navigateToOnboardingScreen2 = { navController.navigationOnboarding2() }, | ||
| navigateToOnboardingScreen4 = { navController.navigationOnboarding4() }, | ||
| ) | ||
| } | ||
| composable(OnboardingRoute.OnboardingScreen2.route) { | ||
| OnboardingScreen2( | ||
| navigateToOnboardingScreen3 = { navController.navigationOnboarding3() }, | ||
| navigateToOnboardingScreen4 = { navController.navigationOnboarding4() }, | ||
| popBackStack = { navController.popBackStack() }, | ||
| ) | ||
| } |
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.
아.. 속시원해 👍 👍
| when (event) { | ||
| is EventType.UserLogout, | ||
| is EventType.AccountDeleted, | ||
| is EventType.TokenExpired, | ||
| -> { | ||
| viewModel.setLoggedOut() | ||
| } |
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.
이벤트 수신 시 viewModel.setLoggedOut()만 호출하고 토큰/세션 정리를 안 하면 실제 상태와 UI가 어긋날 수 있어보여요. 토큰 삭제·세션 초기화까지 ViewModel에서 함께 실행되도록 합치는건 어떻게 생각하시나요!?!?
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.
방식이 통일 되지 못하고 분산되어 있네요!!
리팩토링 하려고 관련된 코드를 다시 보는데,
로그아웃 -> clear info 하면 안됨
계정삭제 -> clear info 해야 함
토큰만료 -> 일부만 clear info
이 분기처리를 다시 한번 확인하고 구현해야 할 것 같아요
꼼꼼하게 짚어주셔서 감사합니다아아아
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.
DITTO
| fun logout() { | ||
| viewModelScope.launch { | ||
| tokenDataStore.loginSuccess | ||
| tokenDataStore.loginSuccess = false | ||
| tokenDataStore.clearInfo() | ||
| eventBus.emit(EventType.UserLogout) | ||
| } |
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.
여기처럼요!
✅ 𝗖𝗵𝗲𝗰𝗸-𝗟𝗶𝘀𝘁
📌 𝗜𝘀𝘀𝘂𝗲𝘀
📎𝗪𝗼𝗿𝗸 𝗗𝗲𝘀𝗰𝗿𝗶𝗽𝘁𝗶𝗼𝗻
📷 𝗦𝗰𝗿𝗲𝗲𝗻𝘀𝗵𝗼𝘁
Screen_recording_20250805_133159.mp4
와우ㅋ 갑자기 토큰 만료돼서 로그아웃 로직도 찍혀따
💬 𝗧𝗼 𝗥𝗲𝘃𝗶𝗲𝘄𝗲𝗿𝘀
오랜만이야 ~~~ 😍😍😍😍
FormatDate 파일에 반복되는 객체 생성이 많아서 언젠가 수정이 필요해 보이넴!!