-
Notifications
You must be signed in to change notification settings - Fork 0
NetworkKit : SessionDeinitialized 이슈 해결 #22
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // | ||
| // APIClient+Extension.swift | ||
| // Climeet-iOS | ||
| // | ||
| // Created by 송형욱 on 12/18/24. | ||
| // | ||
|
|
||
| import Foundation | ||
| import Alamofire | ||
| import NetworkKit | ||
|
|
||
| extension APIClient { | ||
| static let shared: APIClient = APIClient(session: .default, tokenRefresher: TokenRefresher()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shared 싱글톤 쓰는것보다 더 좋은 방법이 있을지 고민인데 같이 생각해보면 좋을것 같아요!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저는 네트워크 객체 특성상 싱글턴 패턴이 적절해 보입니다!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 세션 객체는 하나만 유지하는 게 좋을 것 같아서요~
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아래 추가한 내용처럼 session 설정을 달리할 필요가 있을때가 있어요!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그렇다면 싱글턴 API 클라이언트가 필요한 세션들을 가지고 있고, request 형태에 따라 적절한 세션을 통해 요청을 수행하는 방식은 어떤가요?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
요청마다 세션을 생성하는 것이 안티패턴인 이유도 함께 첨부드립니다!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 외부에서 Session 주입할 때 유의하여 사용하고 APIClient는 보통 싱글톤을 사용하되 예외케이스 시 주입하여 사용하는 방식으로 수정하였습니다! |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,33 +2,21 @@ import Foundation | |
| import Alamofire | ||
|
|
||
| public final class APIClient: APIProtocol, @unchecked Sendable { | ||
| private let session: Session | ||
|
|
||
| public static let shared = APIClient() | ||
| private var tokenRefresher: TokenRefreshable? | ||
| private var isConfigured = false | ||
|
|
||
| private init() { } | ||
|
|
||
| private lazy var session: Session = { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. session memory 가 해제될 가능성이 있는건 인지하고 있었지만 |
||
| let configuration = URLSessionConfiguration.af.default | ||
| configuration.waitsForConnectivity = true | ||
| configuration.timeoutIntervalForRequest = 60 // seconds that a task will wait for data to arrive | ||
| configuration.timeoutIntervalForResource = 300 // seconds for whole resource request to complete ,. | ||
| return Session( | ||
| configuration: configuration, | ||
| interceptor: tokenRefresher.map { APIInterceptor(tokenRefresher: $0) }, | ||
| eventMonitors: [APILogger()] | ||
| ) | ||
| }() | ||
|
|
||
| public func configure(tokenRefresher: TokenRefreshable) { | ||
| guard !isConfigured else { | ||
| print("APIClient는 이미 초기화 되었습니다.") | ||
| return | ||
| public init(session: Session, tokenRefresher: TokenRefreshable?) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. token 없이 network할 경우 retry method가 필요가 없습니다! |
||
| if let tokenRefresher { | ||
| self.session = Session( | ||
| configuration: session.sessionConfiguration, | ||
| interceptor: APIInterceptor(tokenRefresher: tokenRefresher), | ||
| eventMonitors: [APILogger()] | ||
| ) | ||
| } else { | ||
| self.session = Session( | ||
| configuration: session.sessionConfiguration, | ||
| eventMonitors: [APILogger()] | ||
| ) | ||
| } | ||
|
|
||
| self.tokenRefresher = tokenRefresher | ||
| self.isConfigured = true | ||
| } | ||
|
|
||
| public func request<T: Decodable>(_ endpoint: Endpoint, decode: T.Type) async throws -> T { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,11 @@ final class APIInterceptor: RequestInterceptor { | |
| let token = tokenRefresher.readToken() | ||
|
|
||
| guard !token.isEmpty else { | ||
| completion(.failure(APIError(errorCode: "401 Token Error", message: "Token Missing"))) | ||
| completion(.failure(APIError( | ||
| statusCode: 401, | ||
| errorCode: "401 Token Error", | ||
| message: "Token Missing" | ||
| ))) | ||
| return | ||
| } | ||
|
|
||
|
|
@@ -34,7 +38,11 @@ final class APIInterceptor: RequestInterceptor { | |
| } | ||
|
|
||
| func retry(_ request: Request, for session: Session, dueTo error: any Error, completion: @escaping @Sendable (RetryResult) -> Void) { | ||
|
|
||
| guard let response = request.task?.response as? HTTPURLResponse, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 테스트해보니 Token이 없으면 무조건 retry를 타고 무조건 retry API를 돌리는건 비효율적이라
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 고생하셨습니다~
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. request 자체에 토큰 관련 검증 로직이 무엇일까요?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그러네요 request 자체에서는 할 수 없겠군요...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 싱글톤 내부에서 세션을 여러개 사용하는 것이 아닌 싱글톤을 여러개 생성하는 방식으로 회의 때 결정하였습니다. https://www.notion.so/climbers/24-12-21-iOS-27-e5ff159050534540b6575120e13d5d19?pvs=4 |
||
| response.statusCode == 401 else { | ||
| completion(.doNotRetryWithError(error)) | ||
| return | ||
| } | ||
| let retryLimit = 3 | ||
| guard request.retryCount < retryLimit else { | ||
| completion(.doNotRetry) | ||
|
|
||

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.
login API는 token 없이 network해야하므로 기존 shared(공통)으로 빼놓은 경우랑 달리 해야합니다.