-
Notifications
You must be signed in to change notification settings - Fork 2
[Refactor] MVI 패턴, Repository 추상화, 코루틴 전환 및 DataStore 마이그레이션 #121
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
- ViewModel을 MVI(Model-View-Intent) 패턴으로 전환
- MviViewModel 베이스 클래스 도입 (core/common/mvi)
- 각 ViewModel에 대한 Contract 파일 생성 (UiState, Event, Action)
- Repository 계층 추상화
- 7개 Repository에 대한 Interface 분리 (I*Repository)
- RepositoryModule에서 @BINDS를 통한 DI 구성
- Retrofit Callback을 suspend 함수로 전환
- 모든 API 서비스 메서드를 suspend로 변경
- try-catch 기반 에러 처리로 통일
- SharedPreferences를 DataStore로 마이그레이션
- TokenStorage, TutorialStorage를 core/datastore 모듈로 이동
- NotificationPreferences, UserPreferences 추가
- Flow 기반 반응형 API + blocking 버전 제공
WalkthroughRetrofit Call 기반 API를 suspend 함수와 Response 래퍼로 통합하고, 전역적으로 MVI 아키텍처(UiState/UiEvent/UiAction)를 도입하며, SharedPreferences를 DataStore로 마이그레이션합니다. 또한 저장소 계층을 인터페이스 기반(IRepository 패턴)으로 개선하고 DI 모듈을 추상 클래스 기반 바인딩으로 전환합니다. Changes
Sequence DiagramsequenceDiagram
participant Fragment as Fragment
participant ViewModel as MviViewModel
participant Repository as IRepository
participant Service as Service
participant DataStore as DataStore
participant TokenMgr as TokenManager
Fragment->>ViewModel: onAction(UserAction.LoadData)
activate ViewModel
ViewModel->>ViewModel: handleAction dispatch
ViewModel->>ViewModel: performLoadData()
ViewModel->>ViewModel: updateState { copy(isLoading: true) }
ViewModel->>Repository: repository.getData()
activate Repository
Repository->>Service: suspend call (Response<T>)
activate Service
Service-->>Repository: Response.success(data) or failure
deactivate Service
alt Success (200-299)
Repository-->>ViewModel: Result with data
ViewModel->>ViewModel: updateState { copy(data: ..., isLoading: false) }
ViewModel->>ViewModel: sendEvent(DataLoaded)
else API Error (401)
Repository-->>ViewModel: 401 Unauthorized
ViewModel->>TokenMgr: tokenManager.refreshToken()
activate TokenMgr
TokenMgr->>Service: suspend fun refreshTokenSuspend()
Service-->>TokenMgr: new access token
TokenMgr->>DataStore: saveTokens()
DataStore-->>TokenMgr: saved
deactivate TokenMgr
ViewModel->>ViewModel: performLoadData() [retry]
ViewModel->>Repository: repository.getData() [with new token]
Repository->>Service: retry call
Service-->>Repository: Response.success
Repository-->>ViewModel: Data
ViewModel->>ViewModel: updateState { copy(data: ..., isLoading: false) }
else Network Error
Repository-->>ViewModel: Exception
ViewModel->>ViewModel: updateState { copy(isLoading: false) }
ViewModel->>ViewModel: sendEvent(ShowError(message))
end
deactivate Repository
ViewModel->>Fragment: state.collect { newState }
deactivate ViewModel
Fragment->>Fragment: UI render(newState)
주요 변경 플로우:
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 근거:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
|
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: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
app/src/main/java/com/toyou/toyouandroid/fcm/domain/FCMRepository.kt (1)
31-106: 주석 처리된 코드 삭제 필요주석 처리된 코드 블록(76줄)을 삭제해 주세요. 버전 관리 시스템에서 히스토리를 추적할 수 있으므로 유지할 필요가 없습니다.
기존의
tokenManager.refreshToken재시도 로직은 이미 presentation layer(ViewModel)로 올바르게 리팩토링되어 있습니다. UserViewModel, CardViewModel, SocialViewModel, ProfileViewModel 등 여러 ViewModel에서 API 실패 시 토큰 갱신을 수행하고 있으므로, Repository에서는 제거된 것이 아키텍처 관점에서도 올바른 변경입니다.app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/my/MyCardViewModel.kt (1)
103-103:question.options!!강제 언래핑은 NPE 위험이 있습니다.
"OPTIONAL"타입일 때options가 항상 non-null이라고 가정하고 있으나, 서버 응답이 예상과 다를 경우 크래시가 발생할 수 있습니다.🔎 수정 제안
"OPTIONAL" -> { PreviewCardModel( question = question.content, fromWho = question.questioner, options = question.options, - type = question.options!!.size, + type = question.options?.size ?: 0, answer = question.answer, id = question.id ) }app/src/main/java/com/toyou/toyouandroid/domain/create/repository/CreateRepository.kt (2)
42-46: catch 블록에서 API 재호출 시 예외 처리 누락catch 블록 내에서
createService.patchCard()를 다시 호출하고 있지만, 이 호출이 실패하면 예외가 전파됩니다. 무한 재시도 가능성이 있고, 적절한 에러 핸들링이 없습니다.🔎 수정 제안
} catch (e: Exception) { e.printStackTrace() Timber.tag("카드 수정 실패!").d("Exception: ${e.message}") - return createService.patchCard(cardId, answerDto) + throw e }또는 실패 응답을 반환하려면:
} catch (e: Exception) { e.printStackTrace() Timber.tag("카드 수정 실패!").d("Exception: ${e.message}") throw e // 또는 적절한 에러 BaseResponse 반환 }
64-68: 동일한 문제: catch 블록에서 API 재호출
postCardData()에서도 catch 블록 내에서createService.postCard()를 재호출하고 있어 동일한 문제가 발생합니다.🔎 수정 제안
} catch (e: Exception) { e.printStackTrace() Timber.tag("post 실패").d("Exception: ${e.message}") - return createService.postCard(answerDto) + throw e }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/friend/FriendCardViewModel.kt (1)
153-155: 로그 태그 오류catch 블록의 로그 태그도 "FriendCardViewModel"로 수정하세요.
🔎 수정 제안
} catch (e: Exception) { - Timber.tag("CardViewModel").d("detail 예외 발생: ${e.message}") + Timber.tag("FriendCardViewModel").e(e, "detail 예외 발생") }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/CardInfoViewModel.kt (1)
86-86:question.options!!는 NPE 위험이 있습니다.
"OPTIONAL"타입일 때options가 null일 수 있습니다. 안전한 호출 연산자(?.)와 기본값을 사용해주세요.🔎 안전한 null 처리
"OPTIONAL" -> { PreviewCardModel( question = question.content, fromWho = question.questioner, options = question.options, - type = question.options!!.size, + type = question.options?.size ?: 0, answer = question.answer, id = question.id ) }
🟡 Minor comments (9)
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/CardInfoContract.kt-14-14 (1)
14-14:cardId타입 불일치 확인 필요
CardInfoUiState.cardId는Int타입이지만,CardInfoAction.GetCardDetail.id는Long타입입니다. API에서 반환되는 ID 타입과 일치시켜 타입 변환 오류를 방지해 주세요.🔎 타입 통일 예시
data class CardInfoUiState( val cards: List<CardModel> = emptyList(), val previewCards: List<PreviewCardModel> = emptyList(), val exposure: Boolean = false, val answer: String = "", - val cardId: Int = 0, + val cardId: Long = 0L, val date: String = "", val emotion: String = "", val receiver: String = "", val isLoading: Boolean = false ) : UiStateAlso applies to: 28-28
app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/ViewModelManager.kt-11-14 (1)
11-14: ViewModelManager의 리셋 호출 방식 통일 필요
signupNicknameViewModel.resetState()와homeViewModel.onAction(HomeAction.ResetState)호출 방식이 일관성 없이 섞여 있습니다. 두 ViewModel 모두 이미 MVI 패턴을 따르고 있으므로, 다음 중 하나로 통일해야 합니다:
- 모두
onAction()직접 호출로 통일, 또는HomeViewModel에 공개 래퍼 메서드(resetState())를 추가하고 모두 래퍼 메서드로 호출app/src/main/java/com/toyou/toyouandroid/domain/social/repostitory/ISocialRepository.kt-1-1 (1)
1-1: 패키지명 오타:repostitory→repository패키지명에 오타가 있습니다. 이는 파일 경로와 import 문에도 영향을 미치므로 수정이 필요합니다.
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/my/MyRecordViewModel.kt-54-63 (1)
54-63: API 오류 시 에러 이벤트 전송과 토큰 갱신이 동시에 발생합니다.에러 메시지를 UI에 표시한 후 토큰 갱신을 시도하면, 사용자에게 혼란을 줄 수 있습니다. 토큰 갱신이 성공하면 재시도하므로, 갱신 실패 시에만 에러를 표시하는 것이 자연스럽습니다.
🔎 수정 제안
} else { val errorMessage = response.message() updateState { copy(isLoading = false) } - sendEvent(MyRecordEvent.ShowError(errorMessage)) Timber.tag("MyRecordViewModel").d("API Error: $errorMessage") tokenManager.refreshToken( onSuccess = { performLoadDiaryCards(year, month) }, - onFailure = { Timber.e("loadDiaryCards API call failed") } + onFailure = { + sendEvent(MyRecordEvent.ShowError(errorMessage)) + Timber.e("loadDiaryCards API call failed") + } ) }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/friend/FriendCardViewModel.kt-147-151 (1)
147-151: 로그 태그 오류 및 재시도 제한 누락로그 태그가 "CardViewModel"로 되어 있지만 이 클래스는
FriendCardViewModel입니다. 또한 토큰 갱신 재시도 횟수 제한이 없습니다.🔎 수정 제안
- Timber.tag("CardViewModel").d("detail API 호출 실패: ${response.message}") + Timber.tag("FriendCardViewModel").d("detail API 호출 실패: ${response.message}")app/src/main/java/com/toyou/toyouandroid/presentation/fragment/mypage/MypageViewModel.kt-106-116 (1)
106-116: 401 응답 코드 확인 누락
performKakaoLogout()에서는response.code() == 401일 때만 토큰 갱신을 시도하지만,performKakaoSignOut()에서는 모든 실패 응답에 대해 토큰 갱신을 시도합니다. 일관성을 위해 401 체크를 추가하세요.🔎 수정 제안
} else { val errorMessage = response.errorBody()?.string() ?: "Unknown error" Timber.e("API Error: $errorMessage") + + if (response.code() == 401) { tokenManager.refreshToken( onSuccess = { performKakaoSignOut() }, onFailure = { Timber.e("Failed to refresh token and kakao signout") sendEvent(MypageEvent.SignOutResult(false)) } ) + } else { + sendEvent(MypageEvent.SignOutResult(false)) + } }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/LoginViewModel.kt-88-97 (1)
88-97: 빈 문자열 체크에isNullOrEmpty()사용을 권장합니다.
accessToken == ""는 null을 처리하지 못합니다.getAccessToken()이 null을 반환할 수 있으므로isNullOrEmpty()또는isNullOrBlank()를 사용하는 것이 안전합니다.🔎 수정 제안
private fun performCheckIfTokenExists() { tokenStorage.let { storage -> val accessToken = storage.getAccessToken() - if (accessToken == "") { + if (accessToken.isNullOrEmpty()) { Timber.d("User Info Not Existed") updateState { copy(checkIfTokenExists = false) } } else {app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/CardViewModel.kt-201-203 (1)
201-203: 빈 catch 블록에서 예외 무시예외가 발생해도 아무런 처리 없이 무시됩니다. 최소한 로깅을 추가하여 디버깅이 가능하도록 해야 합니다.
🔎 수정 제안
- } catch (_: Exception) { + } catch (e: Exception) { + Timber.tag("CardViewModel").e(e, "getAllData 예외 발생") }app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/SocialViewModel.kt-459-465 (1)
459-465: API 실패 시에도 FCM 전송 시도
response.isSuccess가 false인 경우에도retrieveTokenFromServer와postFCM이 호출됩니다. 친구 승인 API가 실패했는데 FCM 알림을 보내는 것은 의도하지 않은 동작으로 보입니다.🔎 수정 제안
if (response.isSuccess) { sendEvent(SocialEvent.FriendRequestCanceled) + retrieveTokenFromServer(friendId) + retrieveTokens?.let { tokens -> + tokens.forEach { token -> + postFCM(myName, token, 2) + } + } } else { // ... 토큰 갱신 로직 } - - retrieveTokenFromServer(friendId) - retrieveTokens?.let { tokens -> - tokens.forEach { token -> - postFCM(myName, token, 2) - } - }Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (54)
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/friend/FriendRecordContract.kt (2)
9-13: UiState 구조가 명확하고 기본값 처리가 잘 되어 있습니다.각 필드에 적절한 기본값이 설정되어 있고,
isLoading플래그로 로딩 상태를 관리하는 것이 좋습니다.선택적으로
errorMessage: String?같은 영구적인 에러 상태 필드를 추가하는 것을 고려해볼 수 있지만, 현재처럼 이벤트로 에러를 처리하는 방식도 유효한 접근입니다.선택적 개선: 영구적인 에러 상태 추가
일회성 에러 알림 외에 UI에 지속적으로 표시할 에러가 필요한 경우:
data class FriendRecordUiState( val diaryCardsNum: List<DiaryCardNum> = emptyList(), val diaryCardPerDay: List<DiaryCardPerDay> = emptyList(), - val isLoading: Boolean = false + val isLoading: Boolean = false, + val errorMessage: String? = null ) : UiState
20-23: 액션 정의가 명확하고 의도가 잘 드러나 있습니다.각 액션이 필요한 매개변수를 적절히 정의하고 있으며, sealed interface로 타입 안전성을 보장합니다.
선택적으로 날짜 매개변수(year, month, day)에 대한 유효성 검증을 추가하는 것을 고려해볼 수 있지만, ViewModel 레벨에서 처리해도 충분합니다.
선택적 개선: 날짜 매개변수 검증 추가
더 방어적인 코드를 원하는 경우, value class나 init 블록을 통한 검증을 고려할 수 있습니다:
sealed interface FriendRecordAction : UiAction { data class LoadDiaryCardsNum(val year: Int, val month: Int) : FriendRecordAction { init { require(month in 1..12) { "Month must be between 1 and 12" } } } data class LoadDiaryCardPerDay(val year: Int, val month: Int, val day: Int) : FriendRecordAction { init { require(month in 1..12) { "Month must be between 1 and 12" } require(day in 1..31) { "Day must be between 1 and 31" } } } }다만 이는 선택사항이며, ViewModel에서 검증하는 것도 충분히 유효한 접근입니다.
core/common/build.gradle.kts (1)
17-19: Lifecycle 의존성 구성 확인 - 버전 업데이트 권장androidx.lifecycle 의존성 추가는 적절하며, 버전 카탈로그 사용도 일관적입니다. 다만 현재 사용 중인 버전 2.8.3은 2025년 11월에 릴리스된 최신 버전 2.10.0에서 상당히 뒤떨어져 있습니다. 성능 개선사항과 버그 수정을 포함한 최신 버전으로 업데이트하는 것을 권장합니다.
gradle/libs.versions.toml에서 lifecycle 버전을 2.8.3 → 2.10.0으로 업데이트해 주세요.gradle/libs.versions.toml (1)
129-131: 라이브러리 선언이 올바릅니다.코루틴 라이브러리 선언이 문법적으로 정확하고 다른 라이브러리 선언과 일관성 있게 구성되어 있습니다.
선택 사항: 번들로 그룹화
여러 모듈에서 코루틴을 함께 사용할 경우, 번들로 그룹화하면 관리가 편리합니다:
[bundles] lifecycle = [ "androidx-lifecycle-runtime", "androidx-lifecycle-viewmodel", "androidx-lifecycle-livedata" ] navigation = [ "androidx-navigation-fragment", "androidx-navigation-ui" ] +coroutines = [ + "kotlinx-coroutines-android", + "kotlinx-coroutines-core" +] room = [ "androidx-room-runtime", "androidx-room-ktx" ]app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/SignupStatusContract.kt (1)
8-14: 하드코딩된 색상 값을 리소스로 분리 권장
nextButtonTextColor에 하드코딩된 색상 값(0xFFA6A6A6.toInt())이 사용되었습니다.nextButtonBackground처럼 리소스 참조(R.color.xxx)를 사용하면 테마 지원과 유지보수성이 향상됩니다. 동일한 패턴이SignupAgreeContract.kt에서도 보이므로 일괄 수정을 고려해 주세요.🔎 리소스 사용 예시
data class SignupStatusUiState( val selectedButtonId: Int? = null, val isNextButtonEnabled: Boolean = false, - val nextButtonTextColor: Int = 0xFFA6A6A6.toInt(), + val nextButtonTextColor: Int = R.color.button_disabled_text, val nextButtonBackground: Int = R.drawable.next_button, val status: String = "" ) : UiStateapp/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/SignupAgreeContract.kt (1)
17-19:ImageClicked액션의newImageResId파라미터 검토
ImageClicked액션에newImageResId를 포함하면 View 레이어가 이미지 리소스 결정 로직을 갖게 됩니다. MVI 패턴에서는 ViewModel이 상태 변경에 따라 적절한 이미지를 결정하는 것이 더 적합합니다.index만 전달하고 ViewModel에서 토글 상태에 따른 이미지를 결정하도록 하면 로직이 더 명확해집니다.🔎 간소화된 액션 예시
sealed interface SignupAgreeAction : UiAction { - data class ImageClicked(val index: Int, val newImageResId: Int) : SignupAgreeAction + data class ImageClicked(val index: Int) : SignupAgreeAction }그리고 ViewModel에서 상태에 따라 이미지 리소스를 결정:
private fun handleImageClicked(index: Int) { val newState = !currentState.imageStates[index] val newImageResId = if (newState) R.drawable.checked else R.drawable.unchecked // ... }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/my/MyCardContract.kt (2)
35-37:ClearAllData와ClearAll액션의 차이점 명확화 필요두 액션의 이름이 유사하여 역할 구분이 불명확합니다. 각 액션의 의도를 명확히 하거나, 하나로 통합하는 것을 고려해 주세요. 예:
ClearCardData,ResetState등으로 구분된 네이밍을 사용하면 가독성이 향상됩니다.
12-25:CardInfoUiState와의 중복 검토
MyCardUiState가CardInfoUiState와 많은 필드를 공유합니다(cards,previewCards,exposure,answer,cardId,date,emotion,receiver,isLoading). 공통 상태를 별도 data class로 추출하거나 상속을 고려하면 유지보수성이 향상될 수 있습니다.또한,
cardId가Int타입인 반면LoadCardDetail.id가Long타입인 불일치도CardInfoContract와 동일하게 존재합니다.app/src/main/java/com/toyou/toyouandroid/data/record/service/RecordService.kt (1)
47-50: 경로 및 반환 타입 일관성 확인 필요다른 엔드포인트들(
diarycards/mine,diarycards/{cardId}등)은 상대 경로를 사용하지만,getCardDetail은 절대 경로(/diarycards/{cardId})를 사용합니다. 또한 다른 메서드들은Response<T>를 반환하지만, 이 메서드만BaseResponse<CardDetail>을 반환합니다.Base URL 설정에 따라 절대 경로는 예상치 못한 동작을 유발할 수 있습니다. 일관성을 위해 경로와 반환 타입을 통일하는 것을 권장합니다.
🔎 제안된 수정
- @GET("/diarycards/{cardId}") + @GET("diarycards/{cardId}") suspend fun getCardDetail( @Path("cardId") card : Long - ): BaseResponse<CardDetail> + ): Response<BaseResponse<CardDetail>>app/src/main/java/com/toyou/toyouandroid/domain/record/IRecordRepository.kt (1)
12-19: 반환 타입 및 파라미터 타입 일관성 검토
getCardDetails만BaseResponse<CardDetail>을 반환하고, 나머지 메서드들은Response<T>를 반환합니다. 또한cardId파라미터 타입이Int와Long으로 혼용되어 있습니다.Repository 계층의 일관성을 위해 다음을 고려해 주세요:
- 반환 타입을
Response<BaseResponse<T>>또는BaseResponse<T>로 통일cardId타입을Long또는Int로 통일app/src/main/java/com/toyou/toyouandroid/presentation/fragment/home/HomeFragment.kt (1)
132-142: 주석 처리된 코드 정리 필요파일 전체에 많은 양의 주석 처리된 코드가 있습니다 (Lines 132-142, 169-179, 213-274). 이 코드들이 MVI 마이그레이션으로 인해 더 이상 필요하지 않다면 삭제하고, 나중에 참조가 필요하면 Git 히스토리를 활용하는 것이 좋습니다.
코드가 아직 마이그레이션 중이라면 TODO 주석을 추가하여 의도를 명확히 해주세요.
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/mypage/MypageContract.kt (2)
7-13: MVI UiState 구조가 적절합니다.nullable 기본값과
isLoading플래그를 포함한 상태 설계가 잘 되어 있습니다. 다만, API 호출 실패 시를 위한errorMessage: String?필드 추가를 고려해 보세요.
15-18: 이벤트에 에러 케이스 추가를 고려하세요.
LogoutResult와SignOutResult가success: Boolean만 포함하고 있어, 실패 시 사용자에게 보여줄 에러 메시지를 전달하기 어렵습니다.🔎 제안하는 개선안
sealed interface MypageEvent : UiEvent { - data class LogoutResult(val success: Boolean) : MypageEvent - data class SignOutResult(val success: Boolean) : MypageEvent + data class LogoutResult(val success: Boolean, val errorMessage: String? = null) : MypageEvent + data class SignOutResult(val success: Boolean, val errorMessage: String? = null) : MypageEvent }app/src/main/java/com/toyou/toyouandroid/domain/notice/INoticeRepository.kt (1)
8-12: 도메인 계층에서 RetrofitResponse<T>직접 반환은 계층 분리 원칙에 어긋납니다.
INoticeRepository가 도메인 패키지에 위치하지만retrofit2.Response를 반환하고 있습니다. 클린 아키텍처 원칙상, 도메인 계층은 데이터 계층(Retrofit)에 의존하지 않아야 합니다.Result<T>또는 커스텀 래퍼 타입을 사용하는 것이 더 적절합니다.다른 Repository 인터페이스(
IFCMRepository,ISocialRepository)는BaseResponse<T>를 사용하고 있어 일관성도 부족합니다.app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/SignupStatusFragment.kt (1)
101-102: 네비게이션 후 상태 저장 순서 확인 필요.
navigate()호출 후에setTutorialShownSync()가 실행됩니다. 만약 저장이 실패하거나 앱이 중간에 종료되면 튜토리얼이 다시 표시될 수 있습니다. 순서를 바꾸거나, 저장 완료 후 네비게이션하는 것이 더 안전합니다.🔎 제안하는 수정
- navController.navigate(R.id.action_navigation_signup_status_to_tutorial_fragment) - tutorialStorage.setTutorialShownSync() + tutorialStorage.setTutorialShownSync() + navController.navigate(R.id.action_navigation_signup_status_to_tutorial_fragment)core/common/src/main/kotlin/com/toyou/core/common/mvi/MviExtensions.kt (1)
20-55:collectState와collectEvent가 동일한 구현을 가지고 있습니다.두 함수 모두 동일한 구현을 가지고 있어 의미론적 구분을 위한 의도적인 설계로 보입니다. 코드 중복을 줄이려면 내부 헬퍼 함수를 추출할 수 있습니다.
🔎 중복 제거를 위한 리팩토링 제안
+private fun <T> LifecycleOwner.collectFlow( + flow: Flow<T>, + state: Lifecycle.State, + collector: suspend (T) -> Unit +) { + lifecycleScope.launch { + repeatOnLifecycle(state) { + flow.collect(collector) + } + } +} + fun <T> LifecycleOwner.collectState( flow: Flow<T>, state: Lifecycle.State = Lifecycle.State.STARTED, collector: suspend (T) -> Unit -) { - lifecycleScope.launch { - repeatOnLifecycle(state) { - flow.collect(collector) - } - } -} +) = collectFlow(flow, state, collector) fun <T> LifecycleOwner.collectEvent( flow: Flow<T>, state: Lifecycle.State = Lifecycle.State.STARTED, collector: suspend (T) -> Unit -) { - lifecycleScope.launch { - repeatOnLifecycle(state) { - flow.collect(collector) - } - } -} +) = collectFlow(flow, state, collector)app/src/main/java/com/toyou/toyouandroid/presentation/fragment/mypage/NoticeSettingFragment.kt (2)
54-55: 메인 스레드에서 DataStore 동기 읽기가 발생할 수 있습니다.
isSubscribed()가 blocking 호출이라면 메인 스레드에서 ANR이나 UI 지연을 유발할 수 있습니다. Flow 기반의 반응형 패턴 사용을 권장합니다.🔎 Flow 기반 구현 제안
// onViewCreated에서 Flow로 초기 상태 수집 viewLifecycleOwner.lifecycleScope.launch { notificationPreferences.isSubscribedFlow.first().let { isSubscribed -> binding.noticeToggle.isChecked = isSubscribed Timber.d("현재 구독 상태: %b", isSubscribed) } }
73-73: 메인 스레드에서 DataStore 동기 쓰기가 발생합니다.
setSubscribedSync()는 blocking 호출로 메인 스레드를 차단할 수 있습니다. UI 이벤트 핸들러에서는 비동기 저장을 사용하는 것이 좋습니다.🔎 비동기 저장 제안
binding.noticeToggle.setOnCheckedChangeListener { _, isChecked -> if (isChecked) { myFirebaseMessagingService.subscribeToTopic() Timber.d("구독됨") Toast.makeText(context, "알림 수신을 동의하였습니다", Toast.LENGTH_SHORT).show() } else { myFirebaseMessagingService.unsubscribeFromTopic() Timber.d("구독 취소됨") Toast.makeText(context, "알림 수신을 거부하였습니다", Toast.LENGTH_SHORT).show() } - notificationPreferences.setSubscribedSync(isChecked) + viewLifecycleOwner.lifecycleScope.launch { + notificationPreferences.setSubscribed(isChecked) + } }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/emotionstamp/HomeOptionFragment.kt (1)
173-177: FQN 대신 import 사용을 권장합니다.
com.toyou.toyouandroid.presentation.fragment.home.HomeAction을 import하면 코드가 더 간결해집니다.🔎 개선 제안
파일 상단에 import 추가:
import com.toyou.toyouandroid.presentation.fragment.home.HomeAction그 후 사용부 수정:
-homeViewModel.onAction( - com.toyou.toyouandroid.presentation.fragment.home.HomeAction.UpdateEmotion( - emotionData.homeEmotionDrawable.toString() - ) -) +homeViewModel.onAction(HomeAction.UpdateEmotion(emotionData.homeEmotionDrawable.toString()))app/src/main/java/com/toyou/toyouandroid/domain/profile/repository/IProfileRepository.kt (1)
7-11: Repository 인터페이스가 잘 정의되어 있습니다.suspend 함수와
Response<T>래퍼 사용으로 코루틴 기반 에러 처리가 가능합니다.다만, 도메인 레이어에서 Retrofit의
Response타입에 직접 의존하는 것은 Clean Architecture 관점에서 고려해볼 수 있습니다. 장기적으로 도메인 전용 Result 타입(예:sealed class Result<T>)으로 래핑하면 도메인-데이터 레이어 분리가 더 명확해집니다.app/src/main/java/com/toyou/toyouandroid/presentation/fragment/mypage/MypageDialogContract.kt (2)
7-16: UiState에 람다를 저장하는 것은 MVI 패턴에서 권장되지 않습니다.
leftButtonClickAction과rightButtonClickAction람다를 State에 저장하면 다음 문제가 발생할 수 있습니다:
- 람다는 equals 비교가 되지 않아 StateFlow의 distinctUntilChanged가 제대로 동작하지 않음
- 람다가 외부 참조를 캡처하면 메모리 누수 가능성
이미
MypageDialogAction.LeftButtonClick/RightButtonClick액션이 정의되어 있으므로, 클릭 핸들링은 ViewModel에서 Event로 처리하는 것이 더 적합합니다.🔎 람다 제거 제안
data class MypageDialogUiState( val title: String = "", val subTitle: String? = null, val leftButtonText: String = "", val rightButtonText: String = "", val leftButtonTextColor: Int = 0, - val rightButtonTextColor: Int = 0, - val leftButtonClickAction: (() -> Unit)? = null, - val rightButtonClickAction: (() -> Unit)? = null + val rightButtonTextColor: Int = 0 ) : UiState클릭 이벤트는 Fragment에서
MypageDialogEvent.LeftButtonClicked/RightButtonClicked를 수집하여 처리합니다.
23-35:SetDialogData액션도 람다 파라미터를 포함하고 있습니다.State에서 람다를 제거한다면 Action에서도 제거하고, 각 다이얼로그 타입별 로직은 ViewModel 내부에서 처리하는 것이 좋습니다.
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/emotionstamp/HomeOptionContract.kt (1)
18-19: Import 스타일 불일치
EmotionResponse는 상단에서 import 되었지만,EmotionRequest는 fully qualified name으로 사용되고 있습니다. 일관성을 위해 상단에 import 추가를 권장합니다.🔎 제안된 수정
파일 상단 import에 추가:
import com.toyou.toyouandroid.data.emotion.dto.EmotionRequest그 후 아래와 같이 수정:
sealed interface HomeOptionAction : UiAction { - data class UpdateEmotion(val emotionRequest: com.toyou.toyouandroid.data.emotion.dto.EmotionRequest) : HomeOptionAction + data class UpdateEmotion(val emotionRequest: EmotionRequest) : HomeOptionAction }core/common/src/main/kotlin/com/toyou/core/common/mvi/MviViewModel.kt (1)
51-53: 상태 업데이트 스레드 안전성 고려
updateState는 현재 스레드 안전하지 않습니다. 대부분의 경우viewModelScope에서 호출되어 문제가 없지만, 여러 코루틴에서 동시에 호출될 경우 race condition이 발생할 수 있습니다.필요시
MutableStateFlow.update { }사용을 고려해 보세요:🔎 스레드 안전한 대안
protected fun updateState(reduce: S.() -> S) { - _state.value = currentState.reduce() + _state.update { it.reduce() } }app/src/main/java/com/toyou/toyouandroid/utils/TokenManager.kt (1)
45-54: 언스코프드CoroutineScope사용 개선 고려
CoroutineScope(Dispatchers.IO).launch는 취소 메커니즘이 없는 "fire-and-forget" 코루틴을 생성합니다.@Singleton이므로 실질적 문제는 적지만, 구조화된 동시성을 위해 클래스 레벨CoroutineScope를 주입받거나 정의하는 것을 고려해보세요.🔎 제안된 수정
@Singleton class TokenManager @Inject constructor( private val authService: AuthService, - private val tokenStorage: TokenStorage + private val tokenStorage: TokenStorage, + @IoDispatcher private val ioDispatcher: CoroutineDispatcher ) { + private val scope = CoroutineScope(SupervisorJob() + ioDispatcher) + // ... fun refreshToken(onSuccess: (String) -> Unit, onFailure: () -> Unit) { - CoroutineScope(Dispatchers.IO).launch { + scope.launch { val result = refreshTokenSuspend()app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/my/MyCardViewModel.kt (1)
40-43: MutableLiveData가 public으로 노출되어 캡슐화가 깨집니다.
_exposure와answer가val이 아닌 공개 접근 가능한MutableLiveData로 선언되어 있어, 외부에서 직접 값을 변경할 수 있습니다. 다른 LiveData 필드들처럼private으로 선언하는 것이 좋습니다.🔎 수정 제안
- val _exposure = MutableLiveData<Boolean>() + private val _exposure = MutableLiveData<Boolean>() val exposure: LiveData<Boolean> get() = _exposure - val answer = MutableLiveData<String>() + private val _answer = MutableLiveData<String>() + val answer: LiveData<String> get() = _answer
init블록 내 할당도 변경 필요:- answer.value = newState.answer + _answer.value = newState.answerapp/src/main/java/com/toyou/toyouandroid/presentation/fragment/notice/NoticeDialogViewModel.kt (1)
40-52: UiState에 람다를 저장하는 것은 MVI 패턴에서 권장되지 않습니다.
leftButtonClickAction람다를UiState에 저장하면 상태 직렬화/복원이 불가능하고, 상태 비교 시 문제가 발생할 수 있습니다. 람다는 ViewModel 내부 프로퍼티로 관리하고, state에는 직렬화 가능한 데이터만 포함하는 것이 좋습니다.🔎 수정 제안
// ViewModel 내부 프로퍼티로 람다 관리 private var leftButtonClickAction: (() -> Unit)? = null private fun performSetDialogData( title: String, leftButtonText: String, action: () -> Unit ) { leftButtonClickAction = action updateState { copy( title = title, leftButtonText = leftButtonText ) } } private fun performLeftButtonClick() { leftButtonClickAction?.invoke() sendEvent(NoticeDialogEvent.LeftButtonClicked) }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/friend/FriendCardContract.kt (1)
12-25:isLoading상태가 누락되었습니다.
MyCardViewModel,CardInfoViewModel등 다른 ViewModel의 UiState에는isLoading필드가 있어 로딩 상태를 관리합니다.FriendCardUiState에도 일관성을 위해 추가하는 것이 좋습니다.🔎 수정 제안
data class FriendCardUiState( val cards: List<CardModel> = emptyList(), val shortCards: List<CardShortModel> = emptyList(), val previewCards: List<PreviewCardModel> = emptyList(), val chooseCards: List<ChooseModel> = emptyList(), val previewChoose: List<PreviewChooseModel> = emptyList(), val exposure: Boolean = false, val answer: String = "", val cardId: Int = 0, val isAllAnswersFilled: Boolean = false, + val isLoading: Boolean = false, val date: String = "", val emotion: String = "", val receiver: String = "" ) : UiStateapp/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/LoginContract.kt (1)
8-14:navigationEvent의Boolean?타입이 의도가 불명확합니다.
null,true,false세 가지 상태의 의미가 명확하지 않습니다. 네비게이션은 일반적으로UiEvent로 처리하거나, 명시적인 sealed class/enum을 사용하는 것이 더 명확합니다.🔎 수정 제안
네비게이션 이벤트를
LoginEvent로 이동하거나, 명시적인 타입 사용:// Option 1: Event로 이동 sealed interface LoginEvent : UiEvent { data object NavigateToHome : LoginEvent data object NavigateToSignup : LoginEvent // ... 기존 이벤트들 } // Option 2: 명시적 sealed class sealed interface NavigationState { data object Idle : NavigationState data object ToHome : NavigationState data object ToSignup : NavigationState }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/SignupNicknameContract.kt (1)
17-21: UiState에 하드코딩된 색상 값과 drawable 리소스가 포함되어 있습니다.
0xFFA6A6A6.toInt()와 같은 하드코딩된 색상 값은 테마/다크모드 대응이 어렵고, 상태와 표현의 관심사 분리를 위반합니다. 상태에는 비즈니스 로직 관련 값(예:ButtonState.ENABLED,ButtonState.DISABLED)을 두고, 실제 색상/drawable 매핑은 UI 레이어에서 처리하는 것이 좋습니다.🔎 수정 제안
enum class ButtonState { ENABLED, DISABLED } data class SignupNicknameUiState( val title: String = "회원가입", val textCount: String = "0/15", val nickname: String = "", val isDuplicateCheckEnabled: Boolean = false, val isNextButtonEnabled: Boolean = false, val duplicateCheckMessage: String = "중복된 닉네임인지 확인해주세요", val isNicknameValid: Boolean = false, val duplicateCheckMessageType: DuplicateCheckMessageType = DuplicateCheckMessageType.CHECK_REQUIRED, val duplicateButtonState: ButtonState = ButtonState.DISABLED, val nextButtonState: ButtonState = ButtonState.DISABLED ) : UiState // UI Layer에서 매핑 fun ButtonState.toTextColor() = when (this) { ButtonState.ENABLED -> R.color.enabled_text ButtonState.DISABLED -> R.color.disabled_text }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/mypage/MypageViewModel.kt (2)
24-26: MVI state와 LiveData 이중 노출
MviViewModel에서 이미stateFlow를 제공하고 있으므로,_uiStateLiveData는 중복입니다. 점진적 마이그레이션을 위한 것이라면 이해할 수 있지만, 장기적으로는 Fragment에서stateFlow를 직접 collect하도록 전환하는 것이 좋습니다.
143-151: isLoading 상태 설정 중복 및 401 체크 누락토큰 갱신 전에
isLoading = false를 설정하고,onFailure에서 다시 설정하므로 중복입니다. 또한performKakaoLogout()과 달리 401 응답 코드 확인이 없습니다.🔎 수정 제안
} else { Timber.tag("API Error").e("Failed to update Mypage. Code: ${response.code()}, Message: ${response.message()}") - updateState { copy(isLoading = false) } - tokenManager.refreshToken( - onSuccess = { performUpdateMypage() }, - onFailure = { - Timber.e("Failed to refresh token and get mypage") - updateState { copy(isLoading = false) } - } - ) + if (response.code() == 401) { + tokenManager.refreshToken( + onSuccess = { performUpdateMypage() }, + onFailure = { + Timber.e("Failed to refresh token and get mypage") + updateState { copy(isLoading = false) } + } + ) + } else { + updateState { copy(isLoading = false) } + } }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/home/HomeViewModel.kt (1)
43-86: 토큰 갱신 재시도 횟수 제한 없음
loadYesterdayCards()에서 토큰 갱신 실패 시 재시도 횟수 제한이 없습니다.NoticeViewModel의performDeleteNotice()처럼retryCount파라미터를 추가하여 무한 재시도를 방지하세요.🔎 수정 제안
- private fun loadYesterdayCards() { + private fun loadYesterdayCards(retryCount: Int = 0) { + val maxRetries = 3 viewModelScope.launch { updateState { copy(isLoading = true) } try { // ... existing code ... } else { updateState { copy(isLoading = false) } + if (retryCount < maxRetries) { tokenManager.refreshToken( - onSuccess = { loadYesterdayCards() }, + onSuccess = { loadYesterdayCards(retryCount + 1) }, onFailure = { // ... existing code ... } ) + } else { + sendEvent(HomeEvent.TokenExpired) + } } } } }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/notice/NoticeViewModel.kt (1)
86-92: performFetchNotices에 재시도 횟수 제한 필요
performDeleteNotice()와 달리performFetchNotices()에는 재시도 횟수 제한이 없어 무한 루프 가능성이 있습니다.performDeleteNotice()와 동일한 패턴을 적용하세요.app/src/main/java/com/toyou/toyouandroid/presentation/fragment/mypage/ProfileContract.kt (2)
47-48:ALREADY_IN_USE와ALREADY_IN_USE_SAME의 메시지가 동일합니다.두 enum 값이 동일한 메시지를 가지고 있어 구분의 목적이 불명확합니다.
ALREADY_IN_USE_SAME이 현재 사용자의 닉네임과 동일한 경우를 의미한다면, 다른 메시지(예: "현재 사용 중인 닉네임입니다.")를 사용하거나, 하나의 enum으로 통합하는 것을 고려해주세요.
36-41: StatusType enum의 value 속성이 enum 이름과 중복됩니다.각 enum 상수의
value가 상수 이름과 동일합니다.name속성을 직접 사용하거나, 서버 API 응답과 매핑이 필요한 경우가 아니라면value속성을 제거하는 것을 고려해주세요.🔎 간소화 제안
-enum class StatusType(val value: String) { - SCHOOL("SCHOOL"), - COLLEGE("COLLEGE"), - OFFICE("OFFICE"), - ETC("ETC") -} +enum class StatusType { + SCHOOL, COLLEGE, OFFICE, ETC +}core/datastore/src/main/kotlin/com/toyou/core/datastore/TokenStorage.kt (1)
70-101:runBlocking사용 시 메인 스레드 블로킹 위험Blocking 버전들이
runBlocking을 사용하고 있습니다. 메인 스레드에서 호출될 경우 ANR이 발생할 수 있습니다. 마이그레이션 기간 동안 사용한다면,@Deprecated어노테이션을 추가하여 점진적 마이그레이션을 유도하는 것을 권장합니다.🔎 Deprecation 어노테이션 추가 제안
// Blocking versions for backward compatibility during migration + @Deprecated("Use accessTokenFlow instead", ReplaceWith("accessTokenFlow.first()", "kotlinx.coroutines.flow.first")) fun getAccessToken(): String? = runBlocking { dataStore.data.map { it[KEY_ACCESS_TOKEN] }.first() } + @Deprecated("Use refreshTokenFlow instead", ReplaceWith("refreshTokenFlow.first()", "kotlinx.coroutines.flow.first")) fun getRefreshToken(): String? = runBlocking { dataStore.data.map { it[KEY_REFRESH_TOKEN] }.first() }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/SignupNicknameViewModel.kt (1)
51-59:LiveData를MutableLiveData로 캐스팅하는 것은 권장되지 않습니다.public으로 노출된
LiveData를MutableLiveData로 캐스팅하여 값을 변경하는 것은 캡슐화를 위반합니다. 이미_uiState가 있으므로, 개별 LiveData 필드들을 제거하거나 privateMutableLiveData를 별도로 선언하는 것을 고려해주세요.🔎 개선 제안
- val nickname: LiveData<String> = MutableLiveData() - val textCount: LiveData<String> = MutableLiveData("0/15") + private val _nickname = MutableLiveData<String>() + val nickname: LiveData<String> get() = _nickname + + private val _textCount = MutableLiveData("0/15") + val textCount: LiveData<String> get() = _textCount // ... 나머지 필드들도 동일하게 적용 init { state.onEach { mviState -> // ... - (nickname as MutableLiveData).value = mviState.nickname - (textCount as MutableLiveData).value = mviState.textCount + _nickname.value = mviState.nickname + _textCount.value = mviState.textCount // ... }.launchIn(viewModelScope) }app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/friend/FriendRecordViewModel.kt (2)
57-65: 토큰 갱신 전에 에러 이벤트를 발생시키면 UX가 저하될 수 있습니다.
performHandleError가 토큰 갱신 시도 전에 호출됩니다. 갱신이 성공하면 재시도가 이루어지지만, 사용자는 일시적으로 에러 메시지를 볼 수 있습니다. 토큰 갱신 실패 후에만 에러 이벤트를 발생시키는 것을 권장합니다.🔎 에러 처리 순서 개선 제안
} else { val errorMessage = response.message() - performHandleError(errorMessage) Timber.tag("FriendRecordViewModel").d("API Error: $errorMessage") tokenManager.refreshToken( onSuccess = { performLoadDiaryCardsNum(year, month) }, - onFailure = { Timber.e("loadDiaryCards API call failed") } + onFailure = { + Timber.e("loadDiaryCards API call failed") + performHandleError(errorMessage) + } ) }
55-56: 중복 로깅을 제거해주세요.카드 개수와 카드 목록을 별도의 로그로 출력하고 있습니다. 하나로 통합하거나 디버그 목적이 끝났다면 제거를 고려해주세요.
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/LoginViewModel.kt (1)
230-231:isSendingToken플래그가finally블록과 콜백에서 중복 리셋됩니다.
finally블록(line 247)에서 항상isSendingToken = false가 실행되므로, 콜백 내부(lines 230, 243)의 리셋은 불필요합니다.🔎 중복 제거 제안
tokenManager.refreshToken( onSuccess = { Timber.d("Token refreshed successfully. Retrying sendTokenToServer.") sendTokenToServer(token, retryCount + 1) }, onFailure = { Timber.e("sendTokenToServer API Call Failed - Refresh token failed.") - isSendingToken = false } )Also applies to: 243-244, 247-247
core/datastore/src/main/kotlin/com/toyou/core/datastore/UserPreferences.kt (1)
70-83: TokenStorage와 동일하게 blocking 버전에@Deprecated어노테이션 추가를 권장합니다.마이그레이션 완료 후 제거를 유도하기 위해 deprecation 표시를 추가해주세요.
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/record/CardInfoViewModel.kt (1)
80-114:PreviewCardModel생성 로직이 여러 ViewModel에서 중복됩니다.이 로직은
FriendCardViewModel,MyCardViewModel에서도 동일하게 존재합니다 (relevant_code_snippets 참조). 별도의 mapper 함수나 확장 함수로 추출하는 것을 권장합니다.🔎 Mapper 함수 추출 제안
// 예: domain/record/mapper/QuestionMapper.kt fun Question.toPreviewCardModel(): PreviewCardModel { val cardType = when (type) { "OPTIONAL" -> options?.size ?: 1 "SHORT_ANSWER" -> 0 else -> 1 } return PreviewCardModel( question = content, fromWho = questioner, options = options, type = cardType, answer = answer, id = id ) } // ViewModel에서 사용: val previewCardList = detailCard.questions.map { it.toPreviewCardModel() }app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/CardContract.kt (2)
40-40:SetCardCount액션의 파라미터명이 불명확합니다.
count,count2,count3가 무엇을 의미하는지 알기 어렵습니다. 의미 있는 이름(예:longCardCount,shortCardCount,chooseCardCount)을 사용해주세요.🔎 명확한 파라미터명 제안
- data class SetCardCount(val count: Int, val count2: Int, val count3: Int) : CardAction + data class SetCardCount( + val longCardCount: Int, + val shortCardCount: Int, + val chooseCardCount: Int + ) : CardAction
12-27: UiState 필드가 많아 관리 복잡도가 높을 수 있습니다.15개의 필드를 가진 상태 객체입니다. 기능적으로 관련된 필드들을 sub-state로 그룹화하는 것을 고려해보세요 (예:
CardDisplayState,SelectionState등).app/src/main/java/com/toyou/toyouandroid/presentation/fragment/mypage/ProfileViewModel.kt (3)
114-118: State와 LiveData 업데이트 불일치 가능성
isDuplicateCheckEnabled는updateState로 관리되지만,duplicateCheckButtonTextColor와duplicateCheckButtonBackground는 LiveData에 직접 업데이트됩니다.performSyncLegacyLiveData에서 이 필드들을 동기화하지 않으므로, MVI state에 해당 필드를 추가하거나 동기화 로직을 확장하는 것이 일관성 있습니다.🔎 제안: UiState에 버튼 스타일 필드 추가
// ProfileUiState에 추가 data class ProfileUiState( // ... 기존 필드 val duplicateCheckButtonTextColor: Int = 0xFFA6A6A6.toInt(), val duplicateCheckButtonBackground: Int = R.drawable.next_button, // ... ) // performSyncLegacyLiveData에서 동기화 _duplicateCheckButtonTextColor.value = uiState.duplicateCheckButtonTextColor _duplicateCheckButtonBackground.value = uiState.duplicateCheckButtonBackground
139-148: 에러 처리와 토큰 갱신 재시도 순서 검토 필요
response.isSuccessful이 false일 때performHandleNicknameCheckError()를 먼저 호출하여 에러 상태를 UI에 반영한 후, 바로 토큰 갱신을 시도합니다. 토큰 갱신 성공 시 API를 재호출하면 에러 메시지가 잠시 표시되었다가 사라지는 플리커 현상이 발생할 수 있습니다.토큰 갱신 실패 시에만 에러 상태를 표시하거나, loading 상태를 활용하는 것을 고려해 주세요.
183-190: 색상 상수 추출 권장하드코딩된 색상값(
0xFFEA9797,0xFFFF0000,0xFF000000)을 상수나 리소스로 추출하면 유지보수성이 향상됩니다. 관련 코드 스니펫의SignupNicknameViewModel에서도 동일한 패턴이 사용되고 있어, 공통 상수로 정의하는 것이 좋습니다.companion object { private const val COLOR_AVAILABLE = 0xFFEA9797.toInt() private const val COLOR_ERROR = 0xFFFF0000.toInt() private const val COLOR_ENABLED = 0xFF000000.toInt() }app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/UserViewModel.kt (1)
73-93: API 실패 시 무조건 토큰 갱신 시도
response.isSuccess가 false인 모든 경우에 토큰 갱신을 시도하지만, 모든 API 실패가 인증 문제(401)는 아닙니다. HTTP 상태 코드를 확인하여 401/403인 경우에만 토큰 갱신을 시도하고, 다른 오류는 적절히 처리하는 것이 효율적입니다.app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/SocialContract.kt (1)
83-89: Reset 액션의 일관성 검토
ResetFriendRequest,ResetFriendRequestCanceled,ResetFriendRequestRemove,ResetApproveSuccess액션들이 정의되어 있지만,SocialViewModel.handleAction에서 no-op으로 처리됩니다. 대신 별도의 legacy reset 함수들이 LiveData를 직접 수정합니다.마이그레이션 완료 후 이러한 액션들을 state 기반으로 통합하거나, 사용하지 않는 액션을 제거하는 것이 좋습니다.
app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/CardViewModel.kt (3)
29-32: State 외부의 mutable 상태
inputStatus,inputLongStatus,inputChooseStatus,toastShow가 state 외부에서MutableList와var로 관리됩니다. MVI 패턴에서는 모든 상태를UiState에서 관리하는 것이 권장됩니다. 현재 구조에서는 이 필드들이 state 변경과 독립적으로 수정될 수 있어 예측 불가능한 동작이 발생할 수 있습니다.🔎 제안: CardUiState에 포함
data class CardUiState( // ... 기존 필드 val inputStatus: List<Boolean> = emptyList(), val inputLongStatus: List<Boolean> = emptyList(), val inputChooseStatus: List<Boolean> = emptyList(), // ... )
146-152: 로깅 일관성 개선
Timber.tag("선택9${currentState.isAllAnswersFilled}")에서 동적 값이 태그에 포함되어 있어 로그 필터링이 어렵습니다. 태그는 고정값으로, 동적 값은 메시지에 포함하는 것이 좋습니다.-Timber.tag("선택9${currentState.isAllAnswersFilled}") +Timber.tag("CardViewModel").d("isAllAnswersFilled: ${currentState.isAllAnswersFilled}")
476-494: Public wrapper 함수들의 @deprecated 어노테이션 누락
ProfileViewModel에서는 legacy public 함수들에@Deprecated어노테이션이 추가되어 있지만, 이 파일에서는 없습니다. 일관성을 위해 동일한 패턴을 적용하거나, 이 함수들을 유지할 의도라면 주석으로 명시하는 것이 좋습니다.app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/SocialViewModel.kt (1)
476-499: FCM 재시도 로직이 적절합니다.재귀적 재시도가
viewModelScope.launch내에서 수행되어 스택 오버플로우 위험이 낮습니다.maxRetries = 5는 합리적인 값입니다.단, delay 없이 즉시 재시도하므로, 네트워크 문제 시 빠르게 5회 소진될 수 있습니다. 필요시 exponential backoff를 고려하세요.
| if (response.code() == 401) { | ||
| tokenManager.refreshToken( | ||
| onSuccess = { updateEmotion(emotionRequest) }, | ||
| onFailure = { | ||
| onSuccess = { performUpdateEmotion(emotionRequest) }, | ||
| onFailure = { | ||
| Timber.e("Failed to refresh token and update emotion") | ||
| updateState { copy(isLoading = false) } | ||
| _errorMessage.value = "인증 실패. 다시 로그인해주세요." | ||
| sendEvent(HomeOptionEvent.TokenExpired) | ||
| } | ||
| ) |
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.
401 재시도 시 무한 루프 위험
토큰 갱신 성공 후 performUpdateEmotion을 재호출하는데, 갱신된 토큰으로도 401이 반환되면 무한 재시도가 발생할 수 있습니다. 재시도 횟수 제한을 추가하세요.
🔎 제안된 수정
- private fun performUpdateEmotion(emotionRequest: EmotionRequest) {
+ private fun performUpdateEmotion(emotionRequest: EmotionRequest, retryCount: Int = 0) {
viewModelScope.launch {
// ...
if (response.code() == 401) {
+ if (retryCount >= 1) {
+ updateState { copy(isLoading = false) }
+ _errorMessage.value = "인증 실패. 다시 로그인해주세요."
+ sendEvent(HomeOptionEvent.TokenExpired)
+ return@launch
+ }
tokenManager.refreshToken(
- onSuccess = { performUpdateEmotion(emotionRequest) },
+ onSuccess = { performUpdateEmotion(emotionRequest, retryCount + 1) },
// ...
)
}
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/emotionstamp/HomeOptionViewModel.kt
around lines 62 to 71, the current logic retries performUpdateEmotion unbounded
after a 401 by calling tokenManager.refreshToken → performUpdateEmotion again,
which can cause an infinite retry loop if the refreshed token still yields 401;
add a retry limit by extending performUpdateEmotion to accept a retryCount (or
use a dedicated maxRetry constant) and increment it on each refresh attempt,
check against maxRetry before calling tokenManager.refreshToken again, and if
max is exceeded log and surface an error (set isLoading=false and send
TokenExpired) instead of retrying further.
| // MVI: State 수집 | ||
| viewLifecycleOwner.collectState(viewModel.state) { state -> | ||
| // UI 상태 업데이트는 여기서 처리 | ||
| Timber.tag("HomeFragment").d("State updated: $state") | ||
| } |
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.
State 수집 로직이 불완전합니다
현재 State 수집은 Timber 로깅만 수행하고 실제 UI 업데이트가 없습니다. HomeUiState의 yesterdayCards, isLoading, isEmpty 등의 상태 변화가 UI에 반영되지 않습니다.
🔎 제안된 구현
viewLifecycleOwner.collectState(viewModel.state) { state ->
- // UI 상태 업데이트는 여기서 처리
- Timber.tag("HomeFragment").d("State updated: $state")
+ binding.homeDateTv.text = state.currentDate
+ binding.homeEmotionTv.text = state.emotionText
+
+ if (state.yesterdayCards.isNotEmpty()) {
+ binding.homeBottomsheetPseudo.visibility = View.GONE
+ binding.homeBottomSheetRv.visibility = View.VISIBLE
+ setupRecyclerView(state.yesterdayCards)
+ } else {
+ binding.homeBottomsheetPseudo.visibility = View.VISIBLE
+ binding.homeBottomSheetRv.visibility = View.GONE
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| is HomeEvent.TokenExpired -> { | ||
| // Token 만료 처리 (로그인 화면으로 이동 등) | ||
| Timber.tag("HomeFragment").d("Token expired") | ||
| } |
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.
TokenExpired 이벤트 처리가 불완전합니다
주석에 "로그인 화면으로 이동 등"이라고 명시되어 있지만, 실제로는 로깅만 수행합니다. 토큰 만료 시 사용자를 로그인 화면으로 리다이렉트하는 로직이 필요합니다.
🔎 제안된 구현
is HomeEvent.TokenExpired -> {
- // Token 만료 처리 (로그인 화면으로 이동 등)
- Timber.tag("HomeFragment").d("Token expired")
+ Timber.tag("HomeFragment").d("Token expired - navigating to login")
+ navController.navigate(R.id.action_navigation_home_to_login)
}📝 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.
| is HomeEvent.TokenExpired -> { | |
| // Token 만료 처리 (로그인 화면으로 이동 등) | |
| Timber.tag("HomeFragment").d("Token expired") | |
| } | |
| is HomeEvent.TokenExpired -> { | |
| Timber.tag("HomeFragment").d("Token expired - navigating to login") | |
| navController.navigate(R.id.action_navigation_home_to_login) | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/home/HomeFragment.kt
around lines 103-106, the TokenExpired event handler only logs the expiration
but must redirect the user to the login flow and clear auth state; update the
handler to (1) clear stored credentials/session (via shared prefs or auth
repository/ViewModel), (2) navigate to the login screen using the fragment's
NavController or start the LoginActivity (ensuring back stack is cleared so user
cannot return), and (3) optionally show a short user-facing message
(Toast/Snackbar) that the session expired; invoke any ViewModel method for
logout/cleanup rather than performing persistence directly in the fragment, and
ensure navigation/cleanup runs on the main thread.
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) | ||
| Timber.i("Tokens saved successfully") | ||
| AuthNetworkModule.setAccessToken(newAccessToken) | ||
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) |
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.
saveTokens가 중복 호출됩니다.
Line 175와 178에서 saveTokens가 두 번 호출됩니다. 이는 불필요한 I/O 작업이며 잠재적인 버그입니다.
🔎 중복 호출 제거
Timber.d("Tokens received from server - Access: $newAccessToken, Refresh: $newRefreshToken")
tokenStorage.saveTokens(newAccessToken, newRefreshToken)
Timber.i("Tokens saved successfully")
AuthNetworkModule.setAccessToken(newAccessToken)
- tokenStorage.saveTokens(newAccessToken, newRefreshToken)
updateState { copy(navigationEvent = true) }
sendEvent(LoginEvent.ReissueSucceeded)📝 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.
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) | |
| Timber.i("Tokens saved successfully") | |
| AuthNetworkModule.setAccessToken(newAccessToken) | |
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) | |
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) | |
| Timber.i("Tokens saved successfully") | |
| AuthNetworkModule.setAccessToken(newAccessToken) |
🤖 Prompt for AI Agents
In
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/LoginViewModel.kt
around lines 175 to 178, remove the duplicated tokenStorage.saveTokens call (the
second occurrence on line 178) so tokens are saved only once; keep a single
saveTokens call, keep the Timber.i log and AuthNetworkModule.setAccessToken call
in the same logical order (save tokens, log, set access token) to avoid
redundant I/O and potential inconsistencies.
| } else { | ||
| handleNicknameCheckError() | ||
| performHandleNicknameCheckError() | ||
| } |
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.
API 실패 시 토큰 갱신 로직이 누락되었습니다.
ProfileViewModel의 performCheckDuplicate (relevant_code_snippets 참조)에서는 API 실패 시 tokenManager.refreshToken을 호출하여 재시도합니다. 이 ViewModel에서도 동일한 패턴을 적용해야 일관성이 유지됩니다.
🔎 토큰 갱신 로직 추가 제안
} else {
performHandleNicknameCheckError()
+ tokenManager.refreshToken(
+ onSuccess = { performCheckDuplicate(userId) },
+ onFailure = { Timber.e("Failed to refresh token and check nickname") }
+ )
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/toyou/toyouandroid/presentation/fragment/onboarding/SignupNicknameViewModel.kt
around lines 129 to 131, the API-failure branch only calls
performHandleNicknameCheckError() but is missing the token refresh-and-retry
flow used in ProfileViewModel.performCheckDuplicate; update the failure branch
to call tokenManager.refreshToken (within the same coroutine/scope), and on
successful refresh retry the duplicate-check API (or call the same
nickname-check method), otherwise fall back to
performHandleNicknameCheckError(); ensure you propagate success/failure
correctly and avoid duplicate concurrent retries.
| fun updateCardIdFromOtherViewModel(otherViewModel: CardViewModel) { | ||
| otherViewModel.cardId.observeForever { newCardId -> | ||
| _cardId.value = newCardId | ||
| onAction(UserAction.UpdateCardId(newCardId)) | ||
| } | ||
| } |
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.
observeForever 사용으로 인한 메모리 누수 위험
observeForever는 수동으로 removeObserver를 호출해야 하지만, 현재 코드에서는 제거 로직이 없습니다. ViewModel 간 직접 참조도 아키텍처적으로 권장되지 않습니다.
SharedFlow, EventBus, 또는 상위 레벨에서의 조정을 통해 ViewModel 간 통신을 처리하는 것이 좋습니다.
🔎 대안: onCleared에서 observer 제거 또는 다른 패턴 사용
+private var cardIdObserver: Observer<Int>? = null
fun updateCardIdFromOtherViewModel(otherViewModel: CardViewModel) {
- otherViewModel.cardId.observeForever { newCardId ->
+ cardIdObserver = Observer { newCardId ->
onAction(UserAction.UpdateCardId(newCardId))
}
+ otherViewModel.cardId.observeForever(cardIdObserver!!)
}
+override fun onCleared() {
+ super.onCleared()
+ cardIdObserver?.let { otherViewModel.cardId.removeObserver(it) }
+}더 나은 방법은 Fragment/Activity 레벨에서 두 ViewModel을 조정하거나 SharedFlow를 사용하는 것입니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/toyou/toyouandroid/presentation/viewmodel/UserViewModel.kt
around lines 104-108, the method uses otherViewModel.cardId.observeForever
without removing the observer which risks memory leaks and couples ViewModels;
replace this by either (A) moving the observation to a UI controller
(Fragment/Activity) that observes both ViewModels and calls
userViewModel.onAction when cardId changes, or (B) switch to a shared event
mechanism (e.g., a SharedFlow/StateFlow exposed by CardViewModel that
UserViewModel collects in a lifecycle-aware scope), or if you must keep LiveData
here, use a MediatorLiveData in UserViewModel that observes
otherViewModel.cardId and ensure you remove the observer in
UserViewModel.onCleared; implement only one of these fixes and remove
observeForever usage.
| suspend fun refreshTokenSuspend(): Result<String> { | ||
| return try { | ||
| val response = authService.reissue(tokenStorage.getRefreshToken().toString()) | ||
| if (response.isSuccessful) { | ||
| val newAccessToken = response.headers()["access_token"] | ||
| val newRefreshToken = response.headers()["refresh_token"] | ||
| if (newAccessToken != null && newRefreshToken != null) { | ||
| Timber.d("Tokens received from server - Access: $newAccessToken, Refresh: $newRefreshToken") | ||
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) | ||
| AuthNetworkModule.setAccessToken(newAccessToken) | ||
| Timber.i("Tokens saved successfully") | ||
| Result.success(newAccessToken) | ||
| } else { | ||
| val errorMessage = response.errorBody()?.string() ?: "Unknown error" | ||
| Timber.e("API Error: $errorMessage") | ||
| Timber.e("Token missing in response headers") | ||
| Result.failure(Exception("Token missing in response headers")) |
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.
getRefreshToken()의 null 처리 개선 필요
Line 20에서 getRefreshToken().toString()을 호출하면, 반환값이 null일 경우 문자열 "null"이 API에 전달됩니다. 토큰이 없는 경우 명시적으로 처리해야 합니다.
🔎 제안된 수정
suspend fun refreshTokenSuspend(): Result<String> {
return try {
+ val refreshToken = tokenStorage.getRefreshToken()
+ ?: return Result.failure(Exception("Refresh token not found"))
- val response = authService.reissue(tokenStorage.getRefreshToken().toString())
+ val response = authService.reissue(refreshToken)
if (response.isSuccessful) {📝 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.
| suspend fun refreshTokenSuspend(): Result<String> { | |
| return try { | |
| val response = authService.reissue(tokenStorage.getRefreshToken().toString()) | |
| if (response.isSuccessful) { | |
| val newAccessToken = response.headers()["access_token"] | |
| val newRefreshToken = response.headers()["refresh_token"] | |
| if (newAccessToken != null && newRefreshToken != null) { | |
| Timber.d("Tokens received from server - Access: $newAccessToken, Refresh: $newRefreshToken") | |
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) | |
| AuthNetworkModule.setAccessToken(newAccessToken) | |
| Timber.i("Tokens saved successfully") | |
| Result.success(newAccessToken) | |
| } else { | |
| val errorMessage = response.errorBody()?.string() ?: "Unknown error" | |
| Timber.e("API Error: $errorMessage") | |
| Timber.e("Token missing in response headers") | |
| Result.failure(Exception("Token missing in response headers")) | |
| suspend fun refreshTokenSuspend(): Result<String> { | |
| return try { | |
| val refreshToken = tokenStorage.getRefreshToken() | |
| ?: return Result.failure(Exception("Refresh token not found")) | |
| val response = authService.reissue(refreshToken) | |
| if (response.isSuccessful) { | |
| val newAccessToken = response.headers()["access_token"] | |
| val newRefreshToken = response.headers()["refresh_token"] | |
| if (newAccessToken != null && newRefreshToken != null) { | |
| Timber.d("Tokens received from server - Access: $newAccessToken, Refresh: $newRefreshToken") | |
| tokenStorage.saveTokens(newAccessToken, newRefreshToken) | |
| AuthNetworkModule.setAccessToken(newAccessToken) | |
| Timber.i("Tokens saved successfully") | |
| Result.success(newAccessToken) | |
| } else { | |
| Timber.e("Token missing in response headers") | |
| Result.failure(Exception("Token missing in response headers")) |
🤖 Prompt for AI Agents
In app/src/main/java/com/toyou/toyouandroid/utils/TokenManager.kt around lines
18 to 32, calling tokenStorage.getRefreshToken().toString() will convert a null
refresh token into the literal "null" and send that to the API; instead,
explicitly fetch the refresh token into a local val, check for null or blank,
and if missing return Result.failure (and log the condition) before calling
authService.reissue; only call reissue with a non-null, non-blank token and
proceed with the existing header handling on success.
| // Blocking versions for backward compatibility | ||
| fun isSubscribed(): Boolean = runBlocking { | ||
| dataStore.data.map { it[KEY_IS_SUBSCRIBED] ?: true }.first() | ||
| } | ||
|
|
||
| fun setSubscribedSync(value: Boolean) = runBlocking { | ||
| setSubscribed(value) | ||
| } |
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.
runBlocking 사용 시 메인 스레드 블로킹 위험
isSubscribed()와 setSubscribedSync() 함수에서 runBlocking을 사용하면 메인 스레드에서 호출 시 ANR이 발생할 수 있습니다. 호환성을 위해 필요하다면 Dispatchers.IO로 컨텍스트를 지정하거나, 호출 위치가 항상 백그라운드 스레드임을 문서화해 주세요.
🔎 Dispatchers.IO 적용 예시
- fun isSubscribed(): Boolean = runBlocking {
+ fun isSubscribed(): Boolean = runBlocking(Dispatchers.IO) {
dataStore.data.map { it[KEY_IS_SUBSCRIBED] ?: true }.first()
}
- fun setSubscribedSync(value: Boolean) = runBlocking {
+ fun setSubscribedSync(value: Boolean) = runBlocking(Dispatchers.IO) {
setSubscribed(value)
}필요 시 kotlinx.coroutines.Dispatchers import 추가:
import kotlinx.coroutines.Dispatchers📝 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.
| // Blocking versions for backward compatibility | |
| fun isSubscribed(): Boolean = runBlocking { | |
| dataStore.data.map { it[KEY_IS_SUBSCRIBED] ?: true }.first() | |
| } | |
| fun setSubscribedSync(value: Boolean) = runBlocking { | |
| setSubscribed(value) | |
| } | |
| // Blocking versions for backward compatibility | |
| fun isSubscribed(): Boolean = runBlocking(Dispatchers.IO) { | |
| dataStore.data.map { it[KEY_IS_SUBSCRIBED] ?: true }.first() | |
| } | |
| fun setSubscribedSync(value: Boolean) = runBlocking(Dispatchers.IO) { | |
| setSubscribed(value) | |
| } |
🤖 Prompt for AI Agents
In
core/datastore/src/main/kotlin/com/toyou/core/datastore/NotificationPreferences.kt
around lines 35 to 42, the two blocking helpers use runBlocking which can block
the main thread and cause ANR; change them to avoid main-thread blocking by
either (A) making the functions suspend and propagating suspension to callers,
or (B) if synchronous wrappers are required for backward compatibility, call
runBlocking with a background dispatcher (runBlocking(Dispatchers.IO) or
Dispatchers.Default) and import kotlinx.coroutines.Dispatchers; also update KDoc
to state the threading behavior if you keep synchronous wrappers.
| // Blocking versions for backward compatibility | ||
| fun isTutorialShown(): Boolean = runBlocking { | ||
| dataStore.data.map { it[KEY_TUTORIAL_SHOWN] ?: false }.first() | ||
| } | ||
|
|
||
| fun setTutorialShown() { | ||
| preferences.edit().putBoolean(KEY_TUTORIAL_SHOWN, true).apply() | ||
| fun setTutorialShownSync() = runBlocking { | ||
| setTutorialShown() | ||
| } | ||
|
|
||
| fun setTutorialNotShown() { | ||
| preferences.edit().putBoolean(KEY_TUTORIAL_SHOWN, false).apply() | ||
| fun setTutorialNotShownSync() = runBlocking { | ||
| setTutorialNotShown() | ||
| } |
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.
runBlocking은 메인 스레드에서 호출 시 ANR을 유발할 수 있습니다.
하위 호환성을 위한 동기 버전은 이해되지만, 메인 스레드에서 호출되면 UI가 블로킹됩니다. 사용처가 백그라운드 스레드임을 보장하거나, Dispatchers.IO로 컨텍스트를 전환하는 것이 안전합니다.
🔎 수정 제안
+ @WorkerThread
fun isTutorialShown(): Boolean = runBlocking {
dataStore.data.map { it[KEY_TUTORIAL_SHOWN] ?: false }.first()
}
+ @WorkerThread
fun setTutorialShownSync() = runBlocking {
setTutorialShown()
}
+ @WorkerThread
fun setTutorialNotShownSync() = runBlocking {
setTutorialNotShown()
}또는 Dispatchers.IO 사용:
fun isTutorialShown(): Boolean = runBlocking(Dispatchers.IO) {
dataStore.data.map { it[KEY_TUTORIAL_SHOWN] ?: false }.first()
}📝 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.
| // Blocking versions for backward compatibility | |
| fun isTutorialShown(): Boolean = runBlocking { | |
| dataStore.data.map { it[KEY_TUTORIAL_SHOWN] ?: false }.first() | |
| } | |
| fun setTutorialShown() { | |
| preferences.edit().putBoolean(KEY_TUTORIAL_SHOWN, true).apply() | |
| fun setTutorialShownSync() = runBlocking { | |
| setTutorialShown() | |
| } | |
| fun setTutorialNotShown() { | |
| preferences.edit().putBoolean(KEY_TUTORIAL_SHOWN, false).apply() | |
| fun setTutorialNotShownSync() = runBlocking { | |
| setTutorialNotShown() | |
| } | |
| @WorkerThread | |
| fun isTutorialShown(): Boolean = runBlocking { | |
| dataStore.data.map { it[KEY_TUTORIAL_SHOWN] ?: false }.first() | |
| } | |
| @WorkerThread | |
| fun setTutorialShownSync() = runBlocking { | |
| setTutorialShown() | |
| } | |
| @WorkerThread | |
| fun setTutorialNotShownSync() = runBlocking { | |
| setTutorialNotShown() | |
| } |
🤖 Prompt for AI Agents
In core/datastore/src/main/kotlin/com/toyou/core/datastore/TutorialStorage.kt
around lines 41–52, the blocking variants use runBlocking without a dispatcher
which can ANR if invoked on the main thread; change these to
runBlocking(Dispatchers.IO) (or otherwise ensure callers run on a background
thread) for all three functions (isTutorialShown, setTutorialShownSync,
setTutorialNotShownSync) and add the necessary kotlinx.coroutines.Dispatchers
import so the blocking work runs on IO rather than the main/UI thread.
| # Coroutines | ||
| coroutines = "1.7.3" |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest kotlinx-coroutines version from Maven Central
curl -s "https://search.maven.org/solrsearch/select?q=g:org.jetbrains.kotlinx+AND+a:kotlinx-coroutines-core&core=gav&rows=20&wt=json" | jq -r '.response.docs[0:10] | .[] | "\(.v)"'Repository: Team-ToYou/ToYou-Android
Length of output: 147
🌐 Web query:
kotlinx-coroutines latest version 2025
💡 Result:
The latest stable kotlinx-coroutines release is 1.10.2 (published April 8, 2025). [1][2]
Sources:
- kotlinx.coroutines GitHub README showing version 1.10.2. [1]
- Maven Central listing for kotlinx-coroutines artifacts (1.10.2 published 2025-04-08). [2]
🌐 Web query:
kotlinx-coroutines 1.7.3 security vulnerabilities
💡 Result:
Short answer: No known direct security vulnerabilities in kotlinx-coroutines 1.7.3 itself, but some artifacts (eg. kotlinx-coroutines-guava) pull vulnerable dependency versions (notably older Guava releases with CVE-2023-2976 / CVE-2020-8908). Update dependencies or use a newer coroutines release to avoid those dependency CVEs. [1][2][3][4]
Sources
- Snyk: no known direct security issues for kotlinx-coroutines 1.7.3. [1]
- Maven / artifact metadata (kotlinx-coroutines-guava 1.7.3) lists dependency CVEs (CVE-2023-2976, CVE-2020-8908). [2]
- NVD entry for CVE-2023-2976 (Guava temporary-file issue). [3]
- kotlinx.coroutines GitHub (project/home + newer releases available). Recommend upgrading to a later coroutines release that uses updated dependencies. [4]
코루틴 버전을 1.10.2로 업그레이드하세요.
1.7.3은 더 이상 최신 버전이 아닙니다. 최신 안정 버전은 1.10.2 (2025년 4월 8일 릴리스)이며, 1.7.3은 간접 의존성(예: kotlinx-coroutines-guava의 Guava 의존성)을 통해 CVE-2023-2976, CVE-2020-8908 등의 보안 취약점을 가져올 수 있습니다. 최신 버전으로 업그레이드하여 의존성 보안을 개선하세요.
🤖 Prompt for AI Agents
In gradle/libs.versions.toml around lines 51-52, the coroutines version is
pinned to 1.7.3; update it to 1.10.2 to address known security issues brought in
via transitive dependencies. Edit the coroutines entry to use "1.10.2", then run
a Gradle sync/refresh and a full build (including dependency resolution) to
ensure no version conflicts; if any modules or constraints pin an older
kotlinx-coroutines version, update those constraints or add a forced resolution
to 1.10.2 so the newer version is used project-wide.



ViewModel을 MVI(Model-View-Intent) 패턴으로 전환
Repository 계층 추상화
Retrofit Callback을 suspend 함수로 전환
SharedPreferences를 DataStore로 마이그레이션
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.