feat: 로그아웃/리프레시 구현#66
Merged
Merged
Conversation
|
""" Walkthrough이 변경사항은 로그아웃 및 토큰 리프레시 기능의 구현을 중심으로, 인증 관련 저장소, 네트워크 계층, 프레젠테이션 계층, 도메인 계층에 걸쳐 다양한 구조체와 프로토콜, 의존성 주입 등록 및 이벤트 흐름을 확장 및 개선합니다. 또한, 기존 델리게이트 패턴을 Combine 기반 이벤트 퍼블리셔로 전환하고, 관련된 에러 처리 및 상태 관리를 통합합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant LoginView
participant LoginViewController
participant LoginViewModel
participant LogoutUseCase
participant AuthRepository
participant TokenProvider
User->>LoginView: 로그아웃 버튼 탭
LoginView-->>LoginViewController: .logoutButtonTapped 이벤트 (Combine)
LoginViewController-->>LoginViewModel: .logoutButtonTapped 액션
LoginViewModel-->>LoginViewModel: .logout 사이드이펙트 발생
LoginViewModel->>LogoutUseCase: execute()
LogoutUseCase->>AuthRepository: logout()
AuthRepository->>TokenProvider: clearCache()
AuthRepository->>AuthRepository: 토큰스토어 클리어
LogoutUseCase-->>LoginViewModel: 완료
LoginViewModel-->>LoginViewController: .logoutSuccessed 액션
sequenceDiagram
participant OAuthNetworkProvider
participant AuthRetrier
participant RefreshHandler
participant TokenProvider
OAuthNetworkProvider->>AuthRetrier: retryIfNeeded(response, data)
AuthRetrier->>TokenProvider: refreshToken 조회
AuthRetrier->>RefreshHandler: refresh(token)
RefreshHandler-->>AuthRetrier: 완료/실패
AuthRetrier-->>OAuthNetworkProvider: 완료/에러
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
""" 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
src/Projects/BKNetwork/Sources/NetworkAssembly.swift (1)
30-38: 에러 매핑 로직을 개선해주세요.클로저 내부에서
@Autowired를 사용하여 순환 참조 문제를 해결한 것은 좋은 접근입니다. 하지만 모든AuthError를NetworkError.retryFailed로 일괄 매핑하면 구체적인 에러 정보가 손실됩니다.다음과 같이 에러 타입별로 세분화된 매핑을 고려해보세요:
return handler.refresh(token: token) - .mapError { _ in NetworkError.retryFailed } + .mapError { error in + switch error { + case .serverError: + return NetworkError.retryFailed + case .missingToken: + return NetworkError.unauthorized + default: + return NetworkError.retryFailed + } + } .eraseToAnyPublisher()src/Projects/BKPresentation/Sources/AuthFlow/View/LoginViewController.swift (1)
18-33: Combine 기반 이벤트 처리로의 전환이 잘 구현되었습니다.델리게이트 패턴에서 Combine 기반으로의 전환으로 반응형 프로그래밍의 장점을 잘 활용했습니다. 하지만 프로바이더별 분기 처리에서 확장성을 개선할 수 있습니다.
다음과 같이 switch 문을 사용하여 타입 안전성을 높이는 것을 고려해보세요:
case .loginButtonTapped(let provider): - if provider == .apple { - self.viewModel.send(.appleLoginButtonTapped) - } else if provider == .kakao { - self.viewModel.send(.kakaoLoginButtonTapped) - } + switch provider { + case .apple: + self.viewModel.send(.appleLoginButtonTapped) + case .kakao: + self.viewModel.send(.kakaoLoginButtonTapped) + }이렇게 하면 새로운 프로바이더 추가 시 컴파일 타임에 누락된 케이스를 확인할 수 있습니다.
src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift (1)
55-60:clearCache()호출의 에러 처리를 추가하세요.
tokenProvider.clearCache()가 실패할 수 있는 경우를 대비해 에러 처리를 추가하는 것이 좋습니다.
clearCache()가 에러를 반환하도록 설계되어 있다면 에러 처리 코드를 생성해드릴까요?src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift (1)
9-13: 로딩 상태 제거가 사용자 경험에 미칠 영향을 고려하세요.
isLoading상태가 제거되어 인증 과정 중 로딩 표시가 불가능해졌습니다. 사용자가 버튼을 여러 번 탭하는 것을 방지하기 위해 로딩 상태 관리가 필요할 수 있습니다.로딩 상태를 다시 추가하거나 다른 방식으로 중복 탭을 방지하는 코드를 생성해드릴까요?
src/Projects/BKPresentation/Sources/AuthFlow/View/LoginView.swift (1)
26-33: 디자인 시스템의 색상을 사용하세요.로그아웃 버튼의 배경색이
.red로 하드코딩되어 있습니다. 일관된 UI를 위해 디자인 시스템의 색상을 사용하는 것이 좋습니다.private let logoutButton: UIButton = { let button = UIButton(type: .roundedRect) - button.backgroundColor = .red + button.backgroundColor = DesignSystem.Colors.danger // 또는 적절한 디자인 시스템 색상 button.setTitle("로그아웃", for: .normal) button.setTitleColor(.black, for: .normal) button.layer.cornerRadius = LayoutConstants.buttonCornerRadius return button }()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/Projects/BKData/Sources/DTO/Response/EmptyResponse.swift(1 hunks)src/Projects/BKData/Sources/DataAssembly.swift(2 hunks)src/Projects/BKData/Sources/Error/NetworkError.swift(1 hunks)src/Projects/BKData/Sources/Interface/Network/RefreshHandler.swift(1 hunks)src/Projects/BKData/Sources/Interface/Storage/TokenProvider.swift(0 hunks)src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift(4 hunks)src/Projects/BKDomain/Sources/DomainAssembly.swift(1 hunks)src/Projects/BKDomain/Sources/UseCase/DefaultLogoutUseCase.swift(1 hunks)src/Projects/BKNetwork/Sources/Extension/RequestTarget+.swift(1 hunks)src/Projects/BKNetwork/Sources/Helper/AuthInterceptor.swift(0 hunks)src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift(1 hunks)src/Projects/BKNetwork/Sources/NetworkAssembly.swift(1 hunks)src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift(2 hunks)src/Projects/BKPresentation/Sources/AuthFlow/View/LoginView.swift(6 hunks)src/Projects/BKPresentation/Sources/AuthFlow/View/LoginViewController.swift(1 hunks)src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift(5 hunks)src/Projects/BKStorage/Sources/StorageAssembly.swift(1 hunks)src/Projects/BKStorage/Sources/TokenStorage/KeychainTokenProvider.swift(0 hunks)
💤 Files with no reviewable changes (3)
- src/Projects/BKData/Sources/Interface/Storage/TokenProvider.swift
- src/Projects/BKStorage/Sources/TokenStorage/KeychainTokenProvider.swift
- src/Projects/BKNetwork/Sources/Helper/AuthInterceptor.swift
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/Constant/PresentationKeys.swift:3-5
Timestamp: 2025-07-07T13:53:13.006Z
Learning: clxxrlove prefers using enum over struct for constants namespace due to better runtime performance, memory layout, and compiler optimizations. Enum prevents instantiation at compile time without needing private init().
src/Projects/BKData/Sources/Interface/Network/RefreshHandler.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKData/Sources/DataAssembly.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKPresentation/Sources/AuthFlow/View/LoginViewController.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKDomain/Sources/UseCase/DefaultLogoutUseCase.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKPresentation/Sources/AuthFlow/View/LoginView.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
🧬 Code Graph Analysis (7)
src/Projects/BKDomain/Sources/DomainAssembly.swift (1)
src/Projects/BKCore/Sources/DiContainer/DIContainer.swift (1)
register(25-46)
src/Projects/BKData/Sources/Interface/Network/RefreshHandler.swift (1)
src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift (1)
refresh(86-104)
src/Projects/BKData/Sources/DataAssembly.swift (1)
src/Projects/BKCore/Sources/DiContainer/DIContainer.swift (1)
register(25-46)
src/Projects/BKPresentation/Sources/AuthFlow/View/LoginViewController.swift (1)
src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift (1)
send(47-51)
src/Projects/BKNetwork/Sources/NetworkAssembly.swift (1)
src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift (1)
refresh(86-104)
src/Projects/BKDomain/Sources/UseCase/DefaultLogoutUseCase.swift (1)
src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift (1)
logout(48-63)
src/Projects/BKPresentation/Sources/AuthFlow/View/LoginView.swift (1)
src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift (1)
send(47-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
src/Projects/BKNetwork/Sources/Extension/RequestTarget+.swift (1)
23-23: 구문 간소화 개선 승인명시적인
NetworkError.invalidURL에서.invalidURL단축 표현으로 변경하여 코드 가독성을 향상시켰습니다. 기능적으로는 동일하며 Swift의 타입 추론을 잘 활용한 좋은 개선입니다.src/Projects/BKStorage/Sources/StorageAssembly.swift (1)
18-19: TokenProvider 싱글톤 도입 검증 완료
TokenProvider가 NetworkAssembly, DataAssembly, StorageAssembly의 모든 @Autowired 주입 지점에서 동일 인스턴스로 사용됨을 확인했습니다. 의도한 로그아웃 버그 수정 효과에 부합하며 추가 조치가 필요 없습니다.src/Projects/BKDomain/Sources/DomainAssembly.swift (1)
48-55: DefaultLogoutUseCase 구현체 존재 확인 및 로그아웃 UseCase 등록 승인DefaultLogoutUseCase 구현체가
src/Projects/BKDomain/Sources/UseCase/DefaultLogoutUseCase.swift에 정상적으로 존재함을 확인했습니다.
의존성 주입 설정도 다른 UseCase들과 일관되게 적용되어 있어 변경 사항을 승인합니다.src/Projects/BKData/Sources/DTO/Response/EmptyResponse.swift (1)
3-6: EmptyResponse 명시적 초기화자 구현 승인 (일관성 확인 완료)EmptyResponse에 public init(from:)와 public init()를 추가한 변경사항을 승인합니다.
다음 사용처에서 HTTP 204 처리 및 Decodable 구조체 동작이 올바르게 적용됨을 확인했습니다:
- src/Projects/BKData/Sources/Repository/DefaultAuthStateRepository.swift (type: EmptyResponse.self 사용)
- src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift (type: EmptyResponse.self 사용)
- src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift (HTTP 204 → EmptyResponse 반환)
위 일관된 구현을 기반으로 코드 변경을 승인합니다.
src/Projects/BKData/Sources/Interface/Network/RefreshHandler.swift (1)
7-9: 토큰 리프레시 프로토콜 설계가 잘 구현되었습니다.단일 책임 원칙을 따르는 간결하고 명확한 프로토콜입니다. Combine 기반의 비동기 처리와 도메인 특화 에러 타입을 사용하여 타입 안전성을 확보했습니다.
src/Projects/BKDomain/Sources/UseCase/DefaultLogoutUseCase.swift (1)
6-18: Clean Architecture 유스케이스 패턴이 잘 구현되었습니다.의존성 주입을 통한 레포지토리 추상화와 단순한 위임 패턴으로 유스케이스 레이어의 역할을 명확히 정의했습니다. 향후 로그아웃 관련 비즈니스 로직 확장 시 이 레이어에서 처리할 수 있는 좋은 구조입니다.
src/Projects/BKData/Sources/DataAssembly.swift (1)
16-21: AuthRepository의 TokenProvider 의존성 추가가 잘 구현되었습니다.토큰 캐시 관리를 위한
TokenProvider의존성 추가가 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Projects/BKData/Sources/Interface/Storage/TokenProvider.swift(1 hunks)src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift(4 hunks)src/Projects/BKNetwork/Sources/Extension/HTTPURLResponse+.swift(1 hunks)src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift(1 hunks)src/Projects/BKNetwork/Sources/NetworkAssembly.swift(1 hunks)src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift(2 hunks)src/Projects/BKStorage/Sources/TokenStorage/KeychainTokenProvider.swift(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Projects/BKNetwork/Sources/NetworkAssembly.swift
- src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift
- src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#66
File: src/Projects/BKData/Sources/DataAssembly.swift:49-60
Timestamp: 2025-07-09T02:32:03.794Z
Learning: clxxrlove prefers separating protocol registrations for RefreshHandler and AuthRepository even when using the same implementation (DefaultAuthRepository) to maintain Interface Segregation Principle, where RefreshHandler focuses on token refresh and AuthRepository handles broader authentication operations.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/Constant/PresentationKeys.swift:3-5
Timestamp: 2025-07-07T13:53:13.006Z
Learning: clxxrlove prefers using enum over struct for constants namespace due to better runtime performance, memory layout, and compiler optimizations. Enum prevents instantiation at compile time without needing private init().
src/Projects/BKData/Sources/Interface/Storage/TokenProvider.swift (2)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#66
File: src/Projects/BKData/Sources/DataAssembly.swift:49-60
Timestamp: 2025-07-09T02:32:03.794Z
Learning: clxxrlove prefers separating protocol registrations for RefreshHandler and AuthRepository even when using the same implementation (DefaultAuthRepository) to maintain Interface Segregation Principle, where RefreshHandler focuses on token refresh and AuthRepository handles broader authentication operations.
src/Projects/BKStorage/Sources/TokenStorage/KeychainTokenProvider.swift (1)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift (2)
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#66
File: src/Projects/BKData/Sources/DataAssembly.swift:49-60
Timestamp: 2025-07-09T02:32:03.794Z
Learning: clxxrlove prefers separating protocol registrations for RefreshHandler and AuthRepository even when using the same implementation (DefaultAuthRepository) to maintain Interface Segregation Principle, where RefreshHandler focuses on token refresh and AuthRepository handles broader authentication operations.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/Projects/BKData/Sources/Interface/Storage/TokenProvider.swift (1)
3-7: 프로토콜 개선이 잘 되었습니다!
refreshIfNeeded()메소드를 제거하고refreshToken프로퍼티를 추가한 것이 좋은 설계 결정입니다. 이로써 TokenProvider의 책임이 더 명확해졌습니다.src/Projects/BKNetwork/Sources/Extension/HTTPURLResponse+.swift (1)
13-14: HTTP 400 에러 처리 추가 👍클라이언트 요청 오류(400)에 대한 명시적 처리를 추가한 것이 좋습니다. 더 정확한 에러 핸들링이 가능해집니다.
src/Projects/BKStorage/Sources/TokenStorage/KeychainTokenProvider.swift (1)
10-10: refreshToken 캐싱 구현이 잘 되었습니다!accessToken과 동일한 패턴으로 refreshToken 캐싱을 구현하여 일관성이 유지되었고, clearCache()도 적절히 업데이트되었습니다.
Also applies to: 29-41, 45-45
src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift (1)
39-39: 보안 개선이 잘 적용되었습니다!리프레시 토큰의 처음 10자만 로깅하도록 수정하여 보안이 향상되었습니다.
Comment on lines
+20
to
+41
| func retryIfNeeded( | ||
| _ response: URLResponse, | ||
| _ data: Data | ||
| ) -> AnyPublisher<Void, NetworkError> { | ||
| guard let httpResponse = response as? HTTPURLResponse else { | ||
| return Fail(error: .invalidResponse) | ||
| .eraseToAnyPublisher() | ||
| } | ||
|
|
||
| guard httpResponse.statusCode == 401 else { | ||
| return Fail(error: .retryTrigger) | ||
| .eraseToAnyPublisher() | ||
| } | ||
|
|
||
| guard let refreshToken = tokenProvider.refreshToken else { | ||
| return Fail(error: .retryFailed) | ||
| .eraseToAnyPublisher() | ||
| } | ||
|
|
||
| Log.debug("[Refresh] RefreshToken: \(refreshToken.prefix(10))", logger: AppLogger.auth) | ||
| return refreshHandler(refreshToken) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
관심사 분리를 위한 아키텍처 개선 제안
현재 구현은 401 감지와 토큰 리프레시를 모두 처리하고 있어 책임이 과중합니다. 더 나은 아키텍처를 위해 다음과 같이 개선하는 것을 제안합니다:
func retryIfNeeded(
_ response: URLResponse,
_ data: Data
) -> AnyPublisher<Void, NetworkError> {
guard let httpResponse = response as? HTTPURLResponse else {
return Fail(error: .invalidResponse)
.eraseToAnyPublisher()
}
guard httpResponse.statusCode == 401 else {
- return Fail(error: .retryTrigger)
+ return Just(())
+ .setFailureType(to: NetworkError.self)
.eraseToAnyPublisher()
}
- guard let refreshToken = tokenProvider.refreshToken else {
- return Fail(error: .retryFailed)
- .eraseToAnyPublisher()
- }
-
- Log.debug("[Refresh] RefreshToken: \(refreshToken.prefix(10))", logger: AppLogger.auth)
- return refreshHandler(refreshToken)
+ // 401이면 retryTrigger를 던져서 상위 레이어에서 처리하도록 함
+ return Fail(error: .retryTrigger)
+ .eraseToAnyPublisher()
}그리고 OAuthNetworkProvider에서 기존 Publisher+.swift의 retryIf 확장을 활용:
.retryIf({ $0 == .retryTrigger }, maxRetries: 1)이렇게 하면:
- ✅ 관심사 분리: 401 감지와 리프레시 로직 분리
- ✅ 재시도 횟수 제한: 무한 재시도 방지
- ✅ 기존 인프라 활용:
retryIf확장 사용
🤖 Prompt for AI Agents
In src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift around lines 20 to
41, the retryIfNeeded function currently handles both 401 response detection and
token refresh, causing responsibility overlap. Refactor by separating concerns:
remove the 401 status code check and token refresh logic from this function, and
instead use the existing retryIf extension in OAuthNetworkProvider with a
condition checking for .retryTrigger error and a maxRetries limit of 1. This
will delegate retry logic to the retryIf extension, enforce retry limits, and
keep the code modular.
clxxrlove
force-pushed
the
BOOK-110-feature/#59
branch
from
1216b17 to
8e3bc1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift (1)
64-69: 에러 처리 로직을 명확히 하세요.
.retryFailed에러를 성공으로 변환하는 로직이 있는데, 이는 재시도 실패 시에도 원본 응답으로 파이프라인을 계속 진행하겠다는 의미로 보입니다. 이 동작이 의도된 것인지 확인하고, 필요시 주석을 추가하여 의도를 명확히 하세요.다음과 같이 주석을 추가하여 의도를 명확히 할 수 있습니다:
.catch { error -> AnyPublisher<Void, NetworkError> in if case .retryFailed = error { + // 재시도 실패 시 원본 응답으로 계속 진행하여 + // 후속 validation에서 적절한 에러 처리 return Just(()).setFailureType(to: NetworkError.self).eraseToAnyPublisher() } return Fail(error: error).eraseToAnyPublisher() }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/Projects/BKData/Sources/DTO/Response/EmptyResponse.swift(1 hunks)src/Projects/BKData/Sources/DataAssembly.swift(2 hunks)src/Projects/BKData/Sources/Error/NetworkError.swift(1 hunks)src/Projects/BKData/Sources/Interface/Network/RefreshHandler.swift(1 hunks)src/Projects/BKData/Sources/Interface/Storage/TokenProvider.swift(1 hunks)src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift(4 hunks)src/Projects/BKDomain/Sources/DomainAssembly.swift(1 hunks)src/Projects/BKDomain/Sources/UseCase/DefaultLogoutUseCase.swift(1 hunks)src/Projects/BKNetwork/Sources/Extension/HTTPURLResponse+.swift(1 hunks)src/Projects/BKNetwork/Sources/Extension/RequestTarget+.swift(1 hunks)src/Projects/BKNetwork/Sources/Helper/AuthInterceptor.swift(0 hunks)src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift(1 hunks)src/Projects/BKNetwork/Sources/NetworkAssembly.swift(1 hunks)src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift(2 hunks)src/Projects/BKPresentation/Sources/AuthFlow/View/LoginView.swift(6 hunks)src/Projects/BKPresentation/Sources/AuthFlow/View/LoginViewController.swift(1 hunks)src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift(5 hunks)src/Projects/BKStorage/Sources/StorageAssembly.swift(1 hunks)src/Projects/BKStorage/Sources/TokenStorage/KeychainTokenProvider.swift(2 hunks)
💤 Files with no reviewable changes (1)
- src/Projects/BKNetwork/Sources/Helper/AuthInterceptor.swift
✅ Files skipped from review due to trivial changes (1)
- src/Projects/BKDomain/Sources/UseCase/DefaultLogoutUseCase.swift
🚧 Files skipped from review as they are similar to previous changes (16)
- src/Projects/BKNetwork/Sources/Extension/RequestTarget+.swift
- src/Projects/BKDomain/Sources/DomainAssembly.swift
- src/Projects/BKData/Sources/Interface/Storage/TokenProvider.swift
- src/Projects/BKNetwork/Sources/Extension/HTTPURLResponse+.swift
- src/Projects/BKData/Sources/DTO/Response/EmptyResponse.swift
- src/Projects/BKData/Sources/Error/NetworkError.swift
- src/Projects/BKData/Sources/Interface/Network/RefreshHandler.swift
- src/Projects/BKData/Sources/DataAssembly.swift
- src/Projects/BKStorage/Sources/TokenStorage/KeychainTokenProvider.swift
- src/Projects/BKStorage/Sources/StorageAssembly.swift
- src/Projects/BKData/Sources/Repository/DefaultAuthRepository.swift
- src/Projects/BKPresentation/Sources/AuthFlow/View/LoginView.swift
- src/Projects/BKNetwork/Sources/Helper/AuthRetrier.swift
- src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift
- src/Projects/BKNetwork/Sources/NetworkAssembly.swift
- src/Projects/BKPresentation/Sources/AuthFlow/View/LoginViewController.swift
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#66
File: src/Projects/BKData/Sources/DataAssembly.swift:49-60
Timestamp: 2025-07-09T02:32:03.794Z
Learning: clxxrlove prefers separating protocol registrations for RefreshHandler and AuthRepository even when using the same implementation (DefaultAuthRepository) to maintain Interface Segregation Principle, where RefreshHandler focuses on token refresh and AuthRepository handles broader authentication operations.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/Constant/PresentationKeys.swift:3-5
Timestamp: 2025-07-07T13:53:13.006Z
Learning: clxxrlove prefers using enum over struct for constants namespace due to better runtime performance, memory layout, and compiler optimizations. Enum prevents instantiation at compile time without needing private init().
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift (2)
11-11: 의존성 주입 구조가 올바르게 구현되었습니다.
AuthRetrier의존성이 적절히 추가되고 초기화되었습니다. 생성자 주입 패턴을 따르고 있어 테스트 가능성과 유지보수성이 좋습니다.Also applies to: 16-16, 20-20
58-73: 재시도 로직 응답값 검증 필요
handleRetryIfNeeded가 재시도 성공/실패 구분 없이 항상 원본(data, response)를 반환하고 있습니다.
그러나AuthRetrier.retryIfNeeded의 실제 구현이 확인되지 않아, 재시도 성공 시 새로운 응답을 반환해야 하는지 여부를 검토해야 합니다.확인해야 할 항목:
AuthRetrier.retryIfNeeded가 정의된 위치 및 구현 내용 확인- 재시도 성공 시 새로운
(Data, URLResponse)를 emit하도록 의도된 로직인지 검증원하는 파일을 찾으려면 다음 명령어를 실행해 보세요:
rg 'struct AuthRetrier' -R . rg 'func retryIfNeeded' -R .
Comment on lines
+40
to
+46
| if httpResponse.statusCode == 204 { | ||
| if let empty = EmptyResponse() as? T { | ||
| return empty | ||
| } else { | ||
| throw NetworkError.invalidResponse | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
HTTP 204 응답 처리 로직을 개선하세요.
현재 구현에서 EmptyResponse()를 생성한 후 T 타입으로 캐스팅하는 방식은 런타임에 실패할 수 있습니다. 타입 안전성을 위해 컴파일 타임에 검증하는 방법을 고려해보세요.
다음과 같이 개선할 수 있습니다:
-if let empty = EmptyResponse() as? T {
- return empty
-} else {
- throw NetworkError.invalidResponse
-}
+if T.self == EmptyResponse.self {
+ return EmptyResponse() as! T
+} else {
+ throw NetworkError.invalidResponse
+}📝 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.
Suggested change
| if httpResponse.statusCode == 204 { | |
| if let empty = EmptyResponse() as? T { | |
| return empty | |
| } else { | |
| throw NetworkError.invalidResponse | |
| } | |
| } | |
| if httpResponse.statusCode == 204 { | |
| if T.self == EmptyResponse.self { | |
| return EmptyResponse() as! T | |
| } else { | |
| throw NetworkError.invalidResponse | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/Projects/BKNetwork/Sources/Provider/OAuthNetworkProvider.swift around
lines 40 to 46, the current handling of HTTP 204 responses creates an
EmptyResponse instance and casts it to type T at runtime, which may fail. To
improve type safety, modify the code to check at compile time if T is
EmptyResponse or a compatible type before returning, avoiding unsafe casting.
Implement a generic constraint or use conditional logic that ensures the
returned value matches T without forced casting, and throw
NetworkError.invalidResponse if the types are incompatible.
Member
Author
There was a problem hiding this comment.
강제 캐스팅하는 것보단 나을거 같아요...
There was a problem hiding this comment.
@clxxrlove 맞습니다! 타입을 먼저 확인하고 나서 캐스팅하는 방식이 더 안전하죠. 런타임에 실패할 수 있는 옵셔널 캐스팅보다는 컴파일 타임에 타입을 검증하는 방법이 훨씬 예측 가능하고 안전합니다.
🧠 Learnings used
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/Constant/PresentationKeys.swift:3-5
Timestamp: 2025-07-07T13:53:13.006Z
Learning: clxxrlove prefers using enum over struct for constants namespace due to better runtime performance, memory layout, and compiler optimizations. Enum prevents instantiation at compile time without needing private init().
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#56
File: src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift:109-111
Timestamp: 2025-07-07T13:47:32.784Z
Learning: The LoginViewModel's authenticateWithToken case in src/Projects/BKPresentation/Sources/AuthFlow/ViewModel/LoginViewModel.swift is test code and not important for implementation according to the user.
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#66
File: src/Projects/BKData/Sources/DataAssembly.swift:49-60
Timestamp: 2025-07-09T02:32:03.794Z
Learning: clxxrlove prefers separating protocol registrations for RefreshHandler and AuthRepository even when using the same implementation (DefaultAuthRepository) to maintain Interface Segregation Principle, where RefreshHandler focuses on token refresh and AuthRepository handles broader authentication operations.
doyeonk429
reviewed
Jul 9, 2025
doyeonk429
approved these changes
Jul 9, 2025
Member
doyeonk429
left a comment
doyeonk429
left a comment
There was a problem hiding this comment.
수고하셨습니다 ! refresh 처리 깔끔 - ⭐
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
ScreenRecording_07-08-2025.21-47-12_1.mov
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링
기타