[Refactor] LoginViewModel에서 세션/인증 상태 분리 (SessionStore 도입)#39
Conversation
- SessionStore 신규: isLoggedIn/userInfo/name + restore/login/logout/forceLogout 인증 UseCase 호출 담당, 실패는 throw. refresh-실패 콜백 배선 이관 - LoginViewModel: 폼 표현(loginId/password/isLoading/errorMessage)만 남기고 로그인은 session.login()에 위임 - ContentView: SessionStore를 소유하고 isLoggedIn으로 루트 분기, .task로 자동 로그인 복원(restore)을 루트에서 수행 - LoginView: session 주입받아 LoginViewModel을 @State로 생성, onAppear 제거
Walkthrough세션/인증 상태를 관리하는 Changes세션 분리 리팩토링
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- MockUserUseCaseProvider: UseCase 결과 stub + 호출 기록 spy - SessionStoreTests: restore/login/logout/리프레시 실패 콜백 8케이스 - LoginViewModelTests: 입력 검증 및 isLoading/errorMessage 상태 전이 4케이스
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Rephoto_iOS/Features/User/Presentation/ViewModels/LoginViewModel.swift (1)
13-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value가이드라인상 "ViewModel은 UseCase에만 의존" 규칙과의 정합성 확인.
LoginViewModel이 이제 UseCase가 아닌SessionStore(Presentation 레이어 객체)에 의존합니다. PR 목표상 폼 상태/세션 상태 분리를 위한 의도된 설계이지만, 코딩 가이드라인은 "ViewModel depends only on UseCase"를 명시하고 있어 문자 그대로는 위반입니다.SessionStore를 Domain 레이어의 공유 유스케이스/서비스로 재정의하거나, 가이드라인에 이 패턴(세션 스토어 위임)에 대한 예외를 명시하는 것을 검토해주세요.As per coding guidelines,
**/*.swift: "Maintain Clean Architecture + MVVM pattern with clear separation: Presentation → Domain ← Data layers, where ViewModel depends only on UseCase, and Data implements Domain Protocol".Also applies to: 24-26, 38-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Rephoto_iOS/Features/User/Presentation/ViewModels/LoginViewModel.swift` around lines 13 - 14, LoginViewModel currently depends on SessionStore, which conflicts with the “ViewModel depends only on UseCase” guideline. Update LoginViewModel to depend on a Domain-layer abstraction instead of the Presentation-layer SessionStore, or move SessionStore behind a UseCase/service that the ViewModel can inject; if this dependency is intentional, document the exception in the MVVM/Clean Architecture guideline and align the initializer and session-handling members in LoginViewModel accordingly.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Rephoto_iOS/Features/User/Presentation/ViewModels/LoginViewModel.swift`:
- Around line 13-14: LoginViewModel currently depends on SessionStore, which
conflicts with the “ViewModel depends only on UseCase” guideline. Update
LoginViewModel to depend on a Domain-layer abstraction instead of the
Presentation-layer SessionStore, or move SessionStore behind a UseCase/service
that the ViewModel can inject; if this dependency is intentional, document the
exception in the MVVM/Clean Architecture guideline and align the initializer and
session-handling members in LoginViewModel accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 49159f28-fc7a-4924-82ff-690947c0644e
📒 Files selected for processing (7)
Rephoto_iOS/App/ContentView.swiftRephoto_iOS/Features/User/Presentation/Session/SessionStore.swiftRephoto_iOS/Features/User/Presentation/ViewModels/LoginViewModel.swiftRephoto_iOS/Features/User/Presentation/Views/LoginView.swiftRephoto_iOSTests/LoginViewModelTests.swiftRephoto_iOSTests/MockUserUseCaseProvider.swiftRephoto_iOSTests/SessionStoreTests.swift
- AppContainer: sessionStore singleton 등록 (@mainactor 타입이라 assumeIsolated로 해석) - ContentView: 직접 생성 대신 @injected 주입으로 전환 - Rephoto_iOSApp: userProvider 전달 배선 제거 - RephotoTabView: 세션 주입 + .environment로 하위 탭에 공급
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Rephoto_iOS/App/ContentView.swift (1)
14-23: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win초기 렌더링 시 로그인 화면이 잠깐 노출될 수 있음.
isLoggedIn의 기본값이false이므로, 저장된 토큰이 있어 자동 로그인이 될 사용자라도restore()가 완료되기 전까지는LoginView가 먼저 그려집니다. 세션 복원 중임을 나타내는 로딩 상태(예: 스플래시/로딩 뷰)를 두면 깜빡임을 피할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Rephoto_iOS/App/ContentView.swift` around lines 14 - 23, Initial render can briefly show LoginView because ContentView switches on session.isLoggedIn before session.restore() finishes. Update ContentView to account for a loading/restoring state in the Session flow, and have body render a splash/loading view while restore() is in progress instead of immediately choosing between RephotoTabView and LoginView. Use the existing session.restore() task and the session/isLoggedIn logic to add a dedicated restoring flag or equivalent state so the authenticated UI only appears after restoration completes.
🧹 Nitpick comments (2)
Rephoto_iOS/Core/DIContainer/AppContainer.swift (1)
83-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
assumeIsolated대신 Factory를@MainActor로 선언하는 것을 권장.
SessionStore가@MainActor타입이라는 전제를 런타임에MainActor.assumeIsolated로 강제하고 있습니다. Factory 라이브러리는 이런 경우를 위해 팩토리 프로퍼티 자체를@MainActor로 선언하는 방식을 공식적으로 지원하며, Swift 공식 문서도 "assume"보다 액터 어노테이션을 우선하도록 권장합니다. 현재 코드 경로(View의body)에서는 크래시 가능성이 낮지만, 추후 백그라운드 컨텍스트나 예정된SessionStoreTests.swift에서 이 팩토리를 해석(resolve)할 경우assumeIsolated는 컴파일 타임이 아닌 런타임 크래시로 이어집니다.♻️ 제안하는 리팩토링
- var sessionStore: Factory<SessionStore> { - self { - MainActor.assumeIsolated { - SessionStore(provider: self.userUseCaseProvider.resolve()) - } - }.singleton - } + `@MainActor` + var sessionStore: Factory<SessionStore> { + self { + SessionStore(provider: self.userUseCaseProvider.resolve()) + }.singleton + }Web search 결과, Factory 저장소 문서는 Factory also works with Observation,
@MainActorand actor isolation in Swift concurrency, and you just annotate the Factory as needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Rephoto_iOS/Core/DIContainer/AppContainer.swift` around lines 83 - 93, The sessionStore Factory currently relies on MainActor.assumeIsolated to enforce main-actor access at runtime, which can crash when resolved off the main actor. Update the sessionStore property in AppContainer to be annotated with `@MainActor` instead of wrapping SessionStore creation in assumeIsolated, and keep the Factory singleton setup intact. This lets Factory and the SessionStore type express actor isolation at compile time and avoids runtime isolation assumptions.Rephoto_iOS/Features/RephotoTabView.swift (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
SessionStore가Features/User에 위치해 교차 Feature 의존을 유발할 수 있음.
RephotoTabView가Features/User/Presentation/Session/SessionStore를 직접 주입받습니다. 코딩 가이드라인상 Feature 모듈 간 직접 의존은 금지되고 Core 계층을 통해야 합니다.RephotoTabView가 이미Features/Home,Features/Search의 UseCaseProvider를 주입받는 기존 패턴을 따르고 있어 이 변경 자체가 새로운 문제는 아니지만, 이제 앱 전역 세션 상태로 격상된SessionStore는 장기적으로Core계층으로 옮기는 편이 가이드라인 의도에 더 부합합니다.As per coding guidelines, "Feature modules must not depend on other Feature modules; all inter-feature communication must go through Core layer."
Also applies to: 28-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Rephoto_iOS/Features/RephotoTabView.swift` at line 13, RephotoTabView is directly injecting SessionStore from Features/User, creating a cross-Feature dependency that violates the module boundary rule. Move SessionStore into the Core layer (or expose it through a Core abstraction) and update RephotoTabView to depend on that Core-owned interface instead of Features/User. Keep the existing injection pattern used in RephotoTabView for other providers, but replace the session dependency with a Core-based symbol so inter-feature communication stays out of Feature modules.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Rephoto_iOS/App/ContentView.swift`:
- Around line 14-23: Initial render can briefly show LoginView because
ContentView switches on session.isLoggedIn before session.restore() finishes.
Update ContentView to account for a loading/restoring state in the Session flow,
and have body render a splash/loading view while restore() is in progress
instead of immediately choosing between RephotoTabView and LoginView. Use the
existing session.restore() task and the session/isLoggedIn logic to add a
dedicated restoring flag or equivalent state so the authenticated UI only
appears after restoration completes.
---
Nitpick comments:
In `@Rephoto_iOS/Core/DIContainer/AppContainer.swift`:
- Around line 83-93: The sessionStore Factory currently relies on
MainActor.assumeIsolated to enforce main-actor access at runtime, which can
crash when resolved off the main actor. Update the sessionStore property in
AppContainer to be annotated with `@MainActor` instead of wrapping SessionStore
creation in assumeIsolated, and keep the Factory singleton setup intact. This
lets Factory and the SessionStore type express actor isolation at compile time
and avoids runtime isolation assumptions.
In `@Rephoto_iOS/Features/RephotoTabView.swift`:
- Line 13: RephotoTabView is directly injecting SessionStore from Features/User,
creating a cross-Feature dependency that violates the module boundary rule. Move
SessionStore into the Core layer (or expose it through a Core abstraction) and
update RephotoTabView to depend on that Core-owned interface instead of
Features/User. Keep the existing injection pattern used in RephotoTabView for
other providers, but replace the session dependency with a Core-based symbol so
inter-feature communication stays out of Feature modules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1772ef4f-9edb-47b5-8932-22cb3b4b21fd
📒 Files selected for processing (4)
Rephoto_iOS/App/ContentView.swiftRephoto_iOS/App/Rephoto_iOSApp.swiftRephoto_iOS/Core/DIContainer/AppContainer.swiftRephoto_iOS/Features/RephotoTabView.swift
1 participant
✨ PR 유형
🛠️ 작업내용
LoginViewModel에 섞여 있던 폼 상태와 앱 전역 세션/인증 상태의 관심사를 분리했습니다.SessionStore신규 — 인증/세션 소유isLoggedIn,userInfo,namerestore()(자동 로그인 복원) /login()/logout()/forceLogout()throw로 알림 (로딩/에러 등 화면 표현 상태는 갖지 않음)LoginViewModel슬림화 — 폼 표현만 (loginId/password/isLoading/errorMessage), 로그인은session.login()에 위임ContentView—SessionStore를@State로 소유,isLoggedIn으로 루트 분기,.task로 자동 로그인 복원을 루트에서 수행LoginView—session주입받아LoginViewModel을@State로 생성, 기존onAppear제거흐름
📋 추후 진행 상황
SessionStore에 연결SessionStore단위 테스트 (로그인/복원/강제 로그아웃 시나리오)📌 리뷰 포인트
fetchUser()실패를 조용히 무시하도록 변경함 (이미 로그인은 성공, 프로필만 미로딩 → 비치명적 처리). 에러 노출은 추후 프로필 화면 관심사로 분리 예정restore)을 LoginView가 아닌 앱 루트(ContentView)에서 수행하도록 이동✅ Checklist
Closes #37
Summary by CodeRabbit