[Refactor] Token 관리 Keychain + Actor 전환#29
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Walkthrough이 PR은 Keychain 기반 비동기 TokenStore와 NetworkClient actor, MoyaNetworkAdapter, TokenRefreshService를 도입하고 기존 Moya Provider/AuthPlugin/동기 토큰 저장소를 제거하여 네트워크·인증 스택과 DI를 재구성합니다. 변경 사항네트워크 및 토큰 관리 리팩토링
Sequence DiagramsequenceDiagram
participant App as App/UI
participant NC as NetworkClient(actor)
participant Policy as DefaultAuthenticationPolicy
participant KVS as KeychainTokenStore(actor)
participant RS as TokenRefreshServiceImpl
participant S as URLSession
App->>NC: request(URLRequest)
NC->>Policy: requireAuthentication(request)
NC->>KVS: getAccessToken()
KVS-->>NC: accessToken
NC->>S: data(for: request with Authorization)
S-->>NC: (Data, HTTPURLResponse)
NC->>Policy: isUnauthorizedResponse(response)
alt unauthorized
NC->>KVS: getRefreshToken()
KVS-->>NC: refreshToken
NC->>RS: refresh(refreshToken)
RS-->>NC: TokenPair
NC->>KVS: save(access, refresh)
NC->>S: retry original request
end
NC-->>App: (Data, HTTPURLResponse) or throw NetworkError
예상 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
Rephoto_iOS/App/ContentView.swift (1)
19-29: 💤 Low valueDEBUG 로그인 우회가 모든 디버그 빌드에서 로그인 화면을 비활성화합니다.
#if DEBUG의if true는 디버그 빌드에서 항상RephotoTabView()를 표시하므로LoginView와 이번 PR에서 추가된await provider.hasTokens()기반 로그인 흐름을 디버그에서 전혀 확인할 수 없게 됩니다. 또한 하드코딩된if true는 컴파일러 경고/코드 스멜을 유발합니다. 환경 변수나 launch argument 같은 토글로 우회 여부를 제어하면 디버그에서도 로그인 흐름 테스트가 가능합니다.런치 인자나 환경 변수 기반 토글로 전환하는 구현을 작성해 드릴까요?
🤖 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 19 - 29, The debug-only bypass uses `#if DEBUG` with `if true`, which always shows `RephotoTabView()` and prevents testing the `LoginView` and the new `await provider.hasTokens()` login flow; replace the hardcoded `if true` with a controllable toggle (read a launch argument or environment variable) so in debug builds you can opt-in/out of the bypass; modify the conditional around `RephotoTabView()`/`LoginView(loginVM:)` to consult that toggle (e.g., check ProcessInfo.processInfo.environment or CommandLine.arguments) and fall back to `loginVM.isLoggedIn` and `await provider.hasTokens()` logic when the toggle is off, removing the `if true` to eliminate the warning.
🤖 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.
Inline comments:
In `@Rephoto_iOS/Core/DIContainer/AppContainer.swift`:
- Around line 85-90: The autoRegister() method in AppContainer is registering
Mock*UseCaseProvider for every DEBUG build, which overrides real implementations
too broadly. Narrow the registration scope by using a dedicated flag or
Preview/Test-only condition inside autoRegister(), so homeUseCaseProvider,
searchUseCaseProvider, and userUseCaseProvider only resolve to mock providers in
those targeted cases and not during normal debug runs.
In `@Rephoto_iOS/Core/NetworkAdapter/NetworkClient/NetworkClient.swift`:
- Around line 99-112: 현재 refreshToken()가 네트워크/서버 일시적 오류로 실패해도 catch에서 무조건
onRefreshFailed?()를 호출하고 NetworkError.unauthorized를 던져 강제 로그아웃을 유발합니다; 수정 방법은
performRequest의 catch에서 잡은 error의 타입/상태를 판별해 인증 실패로 명확히 판단되는 경우(예:
refreshToken()이 반환하는 TokenRefreshError.serverError로 HTTP 401 응답을 포함하거나 refresh
경로에서 authPolicy가 인증 실패를 명시할 때)에만 onRefreshFailed?()를 호출하고
NetworkError.unauthorized를 던지며, 그렇지 않은 일시적 오류는 원래 에러(또는 네트워크/서버 에러로 적절히 래핑한 에러)를
다시 throw하거나 재시도 로직으로 넘기도록 변경하세요; 관련 식별자: authPolicy.isUnauthorizedResponse(_:),
refreshToken(), onRefreshFailed?(), NetworkError.unauthorized,
TokenRefreshError.serverError.
In `@Rephoto_iOS/Core/NetworkAdapter/NetworkClient/TokenPair.swift`:
- Around line 10-19: Remove the unnecessary nonisolated annotations and the
explicit memberwise initializer from the TokenPair struct: delete the
nonisolated keywords on accessToken and refreshToken and remove the custom
init(accessToken:refreshToken:) so the compiler-synthesized memberwise
initializer is used; keep TokenPair as a plain struct with the two stored
properties (accessToken, refreshToken).
In
`@Rephoto_iOS/Core/NetworkAdapter/TokenRefreshService/MoyaNetworkAdapter.swift`:
- Around line 17-21: The injected baseURL property in MoyaNetworkAdapter is
currently not used when creating request URLs, which causes the actual requests
to always use target.baseURL, ignoring the injected value. Fix this by modifying
the request construction logic (likely in methods around lines 17-21 and 90-93)
to build URLs starting from self.baseURL instead of target.baseURL, ensuring the
injected baseURL is respected and allows environment or test host switching.
In
`@Rephoto_iOS/Core/NetworkAdapter/TokenRefreshService/TokenRefreshServiceImpl.swift`:
- Line 27: The refresh request is currently sending the token in the request
body under a capitalized key (RefreshTokenRequestBody(Authorization:
refreshToken)) which likely mismatches the server contract; inspect and confirm
the server's expected field name (e.g. "authorization" vs "Authorization" or
whether the token should be in the Authorization header) and then update
TokenRefreshServiceImpl and the RefreshTokenRequestBody model so the serialized
request.httpBody uses the exact field name the server expects (or move the token
to request.setValue("Bearer <token>", forHTTPHeaderField: "Authorization") if
the API requires a header); ensure the encoder/serialization used for
request.httpBody reflects the corrected property name.
In `@Rephoto_iOS/Features/Home/Data/Target/DescriptionAPITarget.swift`:
- Line 10: The file contains the nonstandard declaration "internal import
Alamofire" which is not compatible with Swift 5/older language modes and appears
unnecessary in DescriptionAPITarget.swift (no Alamofire usage); change it to a
regular "import Alamofire" only if the target actually uses Alamofire, otherwise
remove the import entirely; also audit the same pattern in PhotosAPITarget,
UserAPITarget, TagAPITarget, SearchAPITarget, AlbumAPITarget and
MoyaNetworkAdapter and replace "internal import Alamofire" with either a plain
"import Alamofire" when needed or delete the import when unused to restore Swift
5 compatibility.
In `@Rephoto_iOS/Features/Home/Data/Target/TagAPITarget.swift`:
- Line 10: The import line using an access modifier "internal import Alamofire"
in TagAPITarget.swift should be fixed because Swift 5 does not support
AccessLevelOnImport by default; either remove the access modifier to use "import
Alamofire" in TagAPITarget.swift, or update the project's Swift language version
to 6, or add the compiler flag -enable-experimental-feature AccessLevelOnImport
to the project's Other Swift Flags; locate and update the offending "internal
import Alamofire" statement in TagAPITarget.swift accordingly.
In `@Rephoto_iOS/Features/User/Data/Repositories/UserRepository.swift`:
- Around line 50-52: The setOnRefreshFailed registration in UserRepository
currently wraps networkClient.setOnRefreshFailed in _Concurrency.Task which does
not guarantee registration completion; change UserRepository.setOnRefreshFailed
to be async and directly await networkClient.setOnRefreshFailed so the callback
is registered before returning (update the method signature in UserRepository
from func setOnRefreshFailed(_:) to async func setOnRefreshFailed(_:) and remove
the Task wrapper), then propagate that signature change through
UserRepositoryProtocol and UserUseCaseProviderProtocol and update call sites
such as LoginViewModel to await userRepository.setOnRefreshFailed(...) (or adapt
initialization flow) so registration is guaranteed before any refresh can occur.
In `@Rephoto_iOS/Features/User/Data/Targets/UserAPITarget.swift`:
- Line 10: The import line uses the Swift-6-only syntax "internal import
Alamofire" which is not stable on older toolchains; change it to a plain module
import ("import Alamofire") or, if you must keep the access-on-import form,
guard it with a Swift version conditional so the code uses "internal import
Alamofire" only when compiling with Swift 6+ and falls back to "import
Alamofire" for earlier versions—update the import statement in
UserAPITarget.swift accordingly.
In `@Rephoto_iOS/Utilities/Keychain/KeychainTokenStore.swift`:
- Around line 31-34: The save(accessToken:refreshToken:) method can leave
Keychain in an inconsistent state if saving refreshToken fails after accessToken
succeeded; modify save(accessToken:refreshToken:) so it attempts to save
accessToken via saveToKeychain(key: accessTokenKey, value: accessToken) and then
save refreshToken, and if the second save throws perform a rollback by deleting
the previously saved accessToken (call deleteFromKeychain(key: accessTokenKey)
or an existing remove method) before rethrowing the original error; ensure both
async throws are awaited and propagate the correct error (if delete also fails,
combine or prefer the refreshToken save error) so callers receive failure and
Keychain remains consistent.
---
Nitpick comments:
In `@Rephoto_iOS/App/ContentView.swift`:
- Around line 19-29: The debug-only bypass uses `#if DEBUG` with `if true`,
which always shows `RephotoTabView()` and prevents testing the `LoginView` and
the new `await provider.hasTokens()` login flow; replace the hardcoded `if true`
with a controllable toggle (read a launch argument or environment variable) so
in debug builds you can opt-in/out of the bypass; modify the conditional around
`RephotoTabView()`/`LoginView(loginVM:)` to consult that toggle (e.g., check
ProcessInfo.processInfo.environment or CommandLine.arguments) and fall back to
`loginVM.isLoggedIn` and `await provider.hasTokens()` logic when the toggle is
off, removing the `if true` to eliminate the warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 22f1b5ab-3ac7-4372-9746-6d401c3f79b9
📒 Files selected for processing (34)
Rephoto_iOS/App/ContentView.swiftRephoto_iOS/Core/DIContainer/AppContainer.swiftRephoto_iOS/Core/Error/NetworkError.swiftRephoto_iOS/Core/NetworkAdapter/AuthSystemFactory.swiftRephoto_iOS/Core/NetworkAdapter/AuthedProvider+Async.swiftRephoto_iOS/Core/NetworkAdapter/MoyaProvider+Async.swiftRephoto_iOS/Core/NetworkAdapter/NetworkClient/DefaultAuthenticationPolicy.swiftRephoto_iOS/Core/NetworkAdapter/NetworkClient/NetworkClient.swiftRephoto_iOS/Core/NetworkAdapter/NetworkClient/TokenPair.swiftRephoto_iOS/Core/NetworkAdapter/NetworkClient/TokenStoreProtocol.swiftRephoto_iOS/Core/NetworkAdapter/TokenRefreshService/MoyaNetworkAdapter.swiftRephoto_iOS/Core/NetworkAdapter/TokenRefreshService/TokenRefreshServiceImpl.swiftRephoto_iOS/Features/Home/Data/Repositories/DescriptionRepository.swiftRephoto_iOS/Features/Home/Data/Repositories/PhotoRepository.swiftRephoto_iOS/Features/Home/Data/Repositories/TagRepository.swiftRephoto_iOS/Features/Home/Data/Target/DescriptionAPITarget.swiftRephoto_iOS/Features/Home/Data/Target/PhotosAPITarget.swiftRephoto_iOS/Features/Home/Data/Target/TagAPITarget.swiftRephoto_iOS/Features/Search/Data/Repositories/AlbumRepository.swiftRephoto_iOS/Features/Search/Data/Repositories/SearchRepository.swiftRephoto_iOS/Features/Search/Data/Targets/AlbumAPITarget.swiftRephoto_iOS/Features/Search/Data/Targets/SearchAPITarget.swiftRephoto_iOS/Features/User/Auth/AuthPlugin.swiftRephoto_iOS/Features/User/Auth/AuthProvider.swiftRephoto_iOS/Features/User/Auth/TokenStore.swiftRephoto_iOS/Features/User/Data/Repositories/UserRepository.swiftRephoto_iOS/Features/User/Data/Targets/UserAPITarget.swiftRephoto_iOS/Features/User/Domain/Interfaces/UserRepositoryProtocol.swiftRephoto_iOS/Features/User/Presentation/Preview/MockUserUseCaseProvider.swiftRephoto_iOS/Features/User/Presentation/Provider/UserUseCaseProvider.swiftRephoto_iOS/Features/User/Presentation/ViewModels/LoginViewModel.swiftRephoto_iOS/Network/APITargetType.swiftRephoto_iOS/Utilities/Keychain/KeychainTokenStore.swiftRephoto_iOSTests/TokenPerformanceTests.swift
💤 Files with no reviewable changes (5)
- Rephoto_iOS/Core/NetworkAdapter/MoyaProvider+Async.swift
- Rephoto_iOS/Features/User/Auth/AuthPlugin.swift
- Rephoto_iOS/Core/NetworkAdapter/AuthedProvider+Async.swift
- Rephoto_iOS/Features/User/Auth/AuthProvider.swift
- Rephoto_iOS/Features/User/Auth/TokenStore.swift
- NetworkClient: 일시적 오류와 인증 실패를 구분하여 강제 로그아웃 방지 - TokenPair: 일반 struct에서 불필요한 nonisolated 제거 - MoyaNetworkAdapter: target.baseURL 대신 주입된 self.baseURL 사용 - KeychainTokenStore: refreshToken 저장 실패 시 accessToken 롤백
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
1 participant
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.
✨ PR 유형
어떤 변경 사항이 있나요??
🛠️ 작업내용
아키텍처 변경
신규 파일 (9개)
수정 파일
var hasTokens: Bool→func hasTokens() async -> Bool삭제 파일 (5개)
기타
📋 추후 진행 상황
📌 리뷰 포인트
refreshTask = task를await task.value이전에 저장하는 순서✅ Checklist
Closes #28
Summary by CodeRabbit
New Features
Refactoring
Behavior
Tests