From 403712a03c89bd3a3c6fb8aa9c2fc262c4133ddf Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Tue, 30 Jan 2024 15:48:16 +0100 Subject: [PATCH] refactor: Remove callbacks from HttpClient (#73) --- .../Apply/FlagApplierWithRetries.swift | 44 ++++--- .../RemoteConfidenceClient.swift | 32 +++--- .../ConfidenceProvider/Http/HttpClient.swift | 8 +- .../Http/NetworkClient.swift | 108 +++++------------- .../Helpers/HttpClientMock.swift | 29 +---- 5 files changed, 68 insertions(+), 153 deletions(-) diff --git a/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift b/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift index f5813677..4eb2e182 100644 --- a/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift +++ b/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift @@ -67,17 +67,16 @@ final class FlagApplierWithRetries: FlagApplier { await appliesToSend.asyncForEach { chunk in await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .sending) - await executeApply( + let success = await executeApply( resolveToken: resolveEvent.resolveToken, items: chunk - ) { success in - guard success else { - await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .created) - return - } - // Set 'sent' property of apply events to true - await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .sent) + ) + guard success else { + await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .created) + return } + // Set 'sent' property of apply events to true + await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .sent) } } } @@ -107,9 +106,8 @@ final class FlagApplierWithRetries: FlagApplier { private func executeApply( resolveToken: String, - items: [FlagApply], - completion: @escaping (Bool) async -> Void - ) async { + items: [FlagApply] + ) async -> Bool { let applyFlagRequestItems = items.map { applyEvent in AppliedFlagRequestItem( flag: applyEvent.name, @@ -124,25 +122,23 @@ final class FlagApplierWithRetries: FlagApplier { sdk: Sdk(id: metadata.name, version: metadata.version) ) - await performRequest(request: request) { result in - switch result { - case .success: - await completion(true) - case .failure(let error): - self.logApplyError(error: error) - await completion(false) - } + let result = await performRequest(request: request) + switch result { + case .success: + return true + case .failure(let error): + self.logApplyError(error: error) + return false } } private func performRequest( - request: ApplyFlagsRequest, - completion: @escaping (ApplyFlagResult) async -> Void - ) async { + request: ApplyFlagsRequest + ) async -> ApplyFlagResult { do { - try await httpClient.post(path: ":apply", data: request, completion: completion) + return try await httpClient.post(path: ":apply", data: request) } catch { - await completion(.failure(handleError(error: error))) + return .failure(handleError(error: error)) } } diff --git a/Sources/ConfidenceProvider/ConfidenceClient/RemoteConfidenceClient.swift b/Sources/ConfidenceProvider/ConfidenceClient/RemoteConfidenceClient.swift index 3ff9f361..3ceba804 100644 --- a/Sources/ConfidenceProvider/ConfidenceClient/RemoteConfidenceClient.swift +++ b/Sources/ConfidenceProvider/ConfidenceClient/RemoteConfidenceClient.swift @@ -36,23 +36,23 @@ public class RemoteConfidenceClient: ConfidenceClient { ) do { - let result: HttpClientResponse = - try await self.httpClient.post(path: ":resolve", data: request) - - guard result.response.status == .ok else { - throw result.response.mapStatusToError(error: result.decodedError) - } - - guard let response = result.decodedData else { - throw OpenFeatureError.parseError(message: "Unable to parse request response") - } - - let resolvedValues = try response.resolvedFlags.map { resolvedFlag in - try convert(resolvedFlag: resolvedFlag, ctx: ctx) + let result: HttpClientResult = + try await self.httpClient.post(path: ":resolve", data: request) + switch result { + case .success(let successData): + guard successData.response.status == .ok else { + throw successData.response.mapStatusToError(error: successData.decodedError) + } + guard let response = successData.decodedData else { + throw OpenFeatureError.parseError(message: "Unable to parse request response") + } + let resolvedValues = try response.resolvedFlags.map { resolvedFlag in + try convert(resolvedFlag: resolvedFlag, ctx: ctx) + } + return ResolvesResult(resolvedValues: resolvedValues, resolveToken: response.resolveToken) + case .failure(let errorData): + throw handleError(error: errorData) } - return ResolvesResult(resolvedValues: resolvedValues, resolveToken: response.resolveToken) - } catch let error { - throw handleError(error: error) } } diff --git a/Sources/ConfidenceProvider/Http/HttpClient.swift b/Sources/ConfidenceProvider/Http/HttpClient.swift index 8f0d1721..c24dc4e9 100644 --- a/Sources/ConfidenceProvider/Http/HttpClient.swift +++ b/Sources/ConfidenceProvider/Http/HttpClient.swift @@ -3,13 +3,7 @@ import Foundation typealias HttpClientResult = Result, Error> protocol HttpClient { - func post( - path: String, - data: Codable, - completion: @escaping (HttpClientResult) async -> Void - ) async throws - - func post(path: String, data: Codable) async throws -> HttpClientResponse + func post(path: String, data: Codable) async throws -> HttpClientResult } struct HttpClientResponse { diff --git a/Sources/ConfidenceProvider/Http/NetworkClient.swift b/Sources/ConfidenceProvider/Http/NetworkClient.swift index 0ba073b4..e422f80c 100644 --- a/Sources/ConfidenceProvider/Http/NetworkClient.swift +++ b/Sources/ConfidenceProvider/Http/NetworkClient.swift @@ -43,113 +43,59 @@ final class NetworkClient: HttpClient { func post( path: String, - data: Codable, - completion: @escaping (HttpClientResult) async -> Void - ) async throws { + data: Codable + ) async throws -> HttpClientResult { let request = try buildRequest(path: path, data: data) - await perform(request: request, retry: self.retry) { response, data, error in - if let error { - await completion(.failure(error)) - return - } + let requestResult = await perform(request: request, retry: self.retry) + if let error = requestResult.error { + return .failure(error) + } - guard let response, let data else { - await completion(.failure(ConfidenceError.internalError(message: "Bad response"))) - return - } + guard let response = requestResult.httpResponse, let data = requestResult.data else { + return .failure(ConfidenceError.internalError(message: "Bad response")) + } - do { - let httpClientResult: HttpClientResponse = - try self.buildResponse(response: response, data: data) - await completion(.success(httpClientResult)) - } catch { - await completion(.failure(error)) - } + do { + let httpClientResult: HttpClientResponse = + try self.buildResponse(response: response, data: data) + return .success(httpClientResult) + } catch { + return .failure(error) } } private func perform( request: URLRequest, - retry: Retry, - completion: @escaping (HTTPURLResponse?, Data?, Error?) async -> Void - ) async { + retry: Retry + ) async -> RequestResult { let retryHandler = retry.handler() let retryWait: TimeInterval? = retryHandler.retryIn() do { let (data, response) = try await self.session.data(for: request) guard let httpResponse = response as? HTTPURLResponse else { - await completion(nil, nil, HttpClientError.invalidResponse) - return + return RequestResult(httpResponse: nil, data: nil, error: HttpClientError.invalidResponse) } if self.shouldRetry(httpResponse: httpResponse), let retryWait { try? await Task.sleep(nanoseconds: UInt64(retryWait * 1_000_000_000)) - await self.perform(request: request, retry: retry, completion: completion) - return + return await self.perform(request: request, retry: retry) } - await completion(httpResponse, data, nil) + return RequestResult(httpResponse: httpResponse, data: data, error: nil) } catch { if self.shouldRetry(error: error), let retryWait { try? await Task.sleep(nanoseconds: UInt64(retryWait * 1_000_000_000)) - await self.perform(request: request, retry: retry, completion: completion) + return await self.perform(request: request, retry: retry) } else { - await completion(nil, nil, error) + return RequestResult(httpResponse: nil, data: nil, error: error) } } } +} - // MARK: Async - - func post(path: String, data: Codable) async throws -> HttpClientResponse { - let request = try buildRequest(path: path, data: data) - let result = try await perform(request: request, retry: self.retry) - - return try buildResponse(response: result.response, data: result.data) - } - - private func perform( - request: URLRequest, - retry: Retry - ) async throws -> (response: HTTPURLResponse, data: Data?) { - let retryHandler = retry.handler() - let retryWait: TimeInterval? = retryHandler.retryIn() - - var resultData: Data? - var resultResponse: HTTPURLResponse? - - do { - let result: (Data, URLResponse) = try await self.session.data(for: request) - - guard let httpResponse = result.1 as? HTTPURLResponse else { - throw HttpClientError.invalidResponse - } - - if shouldRetry(httpResponse: httpResponse) { - if let retryWait { - try await Task.sleep(nanoseconds: UInt64(retryWait * 1_000_000_000)) - return try await perform(request: request, retry: retry) - } - } - - resultData = result.0 - resultResponse = httpResponse - } catch { - if shouldRetry(error: error) { - if let retryWait { - try await Task.sleep(nanoseconds: UInt64(retryWait * 1_000_000_000)) - return try await perform(request: request, retry: retry) - } - } - - throw error - } - - guard let resultResponse else { - throw HttpClientError.internalError - } - - return (resultResponse, resultData) - } +struct RequestResult { + var httpResponse: HTTPURLResponse? + var data: Data? + var error: Error? } // MARK: Private diff --git a/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift b/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift index 3a46a0d8..662e10e7 100644 --- a/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift +++ b/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift @@ -19,34 +19,13 @@ final class HttpClientMock: HttpClient { self.testMode = testMode } - func post( - path: String, - data: Codable, - completion: @escaping (ConfidenceProvider.HttpClientResult) async -> Void - ) async throws where T: Decodable { - do { - let result: HttpClientResponse = try handlePost(path: path, data: data) - await completion(.success(result)) - } catch { - await completion(.failure(error)) - } - } - - func post( - path: String, data: Codable - ) async throws -> ConfidenceProvider.HttpClientResponse where T: Decodable { - try handlePost(path: path, data: data) - } - - func post( - path: String, data: Codable - ) throws -> ConfidenceProvider.HttpClientResponse where T: Decodable { + func post(path: String, data: Codable) async throws -> ConfidenceProvider.HttpClientResult where T: Decodable { try handlePost(path: path, data: data) } private func handlePost( path: String, data: Codable - ) throws -> ConfidenceProvider.HttpClientResponse where T: Decodable { + ) throws -> ConfidenceProvider.HttpClientResult where T: Decodable { defer { expectation?.fulfill() } @@ -56,12 +35,12 @@ final class HttpClientMock: HttpClient { switch testMode { case .success: - return HttpClientResponse(response: HTTPURLResponse()) + return .success(HttpClientResponse(response: HTTPURLResponse())) case .failFirstChunk: if postCallCounter == 1 { throw HttpClientError.invalidResponse } else { - return HttpClientResponse(response: HTTPURLResponse()) + return .success(HttpClientResponse(response: HTTPURLResponse())) } case .offline: throw HttpClientError.invalidResponse