From 81bbf68a24ea9bfe5aefdcdce8c43eecc5d39fd5 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Thu, 11 Jan 2024 11:31:09 +0100 Subject: [PATCH 1/4] chore: Make CI a bit more quiet Signed-off-by: Fabrizio Demaria --- scripts/run_tests.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 9d1a9077..72fc485b 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -15,6 +15,7 @@ fi (cd $root_dir && TEST_RUNNER_CLIENT_TOKEN=$test_runner_client_token TEST_RUNNER_TEST_FLAG_NAME=$2 xcodebuild \ + -quiet \ -scheme ConfidenceProvider \ -sdk "iphonesimulator" \ -destination 'platform=iOS Simulator,name=iPhone 14 Pro,OS=16.2' \ From 64026c7c5201389a95f4bb3b497de0b39e10e992 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Thu, 11 Jan 2024 16:09:08 +0100 Subject: [PATCH 2/4] fix: apply returns after full completion of the inner tasks Signed-off-by: Fabrizio Demaria --- .../Apply/FlagApplierWithRetries.swift | 74 ++++++++++--------- .../ConfidenceProvider/Http/HttpClient.swift | 4 +- .../Http/NetworkClient.swift | 74 +++++++++---------- .../Helpers/HttpClientMock.swift | 8 +- 4 files changed, 81 insertions(+), 79 deletions(-) diff --git a/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift b/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift index e37c7fe9..b6dbb173 100644 --- a/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift +++ b/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift @@ -37,19 +37,19 @@ final class FlagApplierWithRetries: FlagApplier { public func apply(flagName: String, resolveToken: String) async { let applyTime = Date.backport.now - let (data, added) = await cacheDataInteractor.add( + async let (data, added) = await cacheDataInteractor.add( resolveToken: resolveToken, flagName: flagName, applyTime: applyTime ) - guard added == true else { + guard await added == true else { // If record is found in the cache, early return (de-duplication). // Triggerring batch apply in case if there are any unsent events stored await triggerBatch() return } - self.writeToFile(data: data) + await self.writeToFile(data: data) await triggerBatch() } @@ -57,7 +57,7 @@ final class FlagApplierWithRetries: FlagApplier { private func triggerBatch() async { async let cacheData = await cacheDataInteractor.cache - await cacheData.resolveEvents.forEach { resolveEvent in + await cacheData.resolveEvents.asyncForEach { resolveEvent in let appliesToSend = resolveEvent.events.filter { $0.status == .created } .chunk(size: 20) @@ -65,40 +65,38 @@ final class FlagApplierWithRetries: FlagApplier { return } - appliesToSend.forEach { chunk in - self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .sending) - executeApply( + await appliesToSend.asyncForEach { chunk in + await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .sending) + await executeApply( resolveToken: resolveEvent.resolveToken, items: chunk ) { success in guard success else { - self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .created) + await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .created) return } // Set 'sent' property of apply events to true - self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .sent) + await self.writeStatus(resolveToken: resolveEvent.resolveToken, events: chunk, status: .sent) } } } } - private func writeStatus(resolveToken: String, events: [FlagApply], status: ApplyEventStatus) { + private func writeStatus(resolveToken: String, events: [FlagApply], status: ApplyEventStatus) async { let lastIndex = events.count - 1 - events.enumerated().forEach { index, event in - Task(priority: .medium) { - var data = await self.cacheDataInteractor.setEventStatus( - resolveToken: resolveToken, - name: event.name, - status: status - ) - - if index == lastIndex { - let unsentFlagApplies = data.resolveEvents.filter { - $0.isSent == false - } - data.resolveEvents = unsentFlagApplies - try? self.storage.save(data: data) + await events.enumerated().asyncForEach { index, event in + var data = await self.cacheDataInteractor.setEventStatus( + resolveToken: resolveToken, + name: event.name, + status: status + ) + + if index == lastIndex { + let unsentFlagApplies = data.resolveEvents.filter { + $0.isSent == false } + data.resolveEvents = unsentFlagApplies + try? self.storage.save(data: data) } } } @@ -110,8 +108,8 @@ final class FlagApplierWithRetries: FlagApplier { private func executeApply( resolveToken: String, items: [FlagApply], - completion: @escaping (Bool) -> Void - ) { + completion: @escaping (Bool) async -> Void + ) async { let applyFlagRequestItems = items.map { applyEvent in AppliedFlagRequestItem( flag: applyEvent.name, @@ -126,25 +124,25 @@ final class FlagApplierWithRetries: FlagApplier { sdk: Sdk(id: metadata.name, version: metadata.version) ) - performRequest(request: request) { result in + await performRequest(request: request) { result in switch result { case .success: - completion(true) + await completion(true) case .failure(let error): self.logApplyError(error: error) - completion(false) + await completion(false) } } } private func performRequest( request: ApplyFlagsRequest, - completion: @escaping (ApplyFlagResult) -> Void - ) { + completion: @escaping (ApplyFlagResult) async -> Void + ) async { do { - try httpClient.post(path: ":apply", data: request, completion: completion) + try await httpClient.post(path: ":apply", data: request, completion: completion) } catch { - completion(.failure(handleError(error: error))) + await completion(.failure(handleError(error: error))) } } @@ -168,3 +166,13 @@ final class FlagApplierWithRetries: FlagApplier { } } } + +extension Sequence { + func asyncForEach( + _ transform: (Element) async throws -> Void + ) async rethrows { + for element in self { + try await transform(element) + } + } +} diff --git a/Sources/ConfidenceProvider/Http/HttpClient.swift b/Sources/ConfidenceProvider/Http/HttpClient.swift index 2f747604..8f0d1721 100644 --- a/Sources/ConfidenceProvider/Http/HttpClient.swift +++ b/Sources/ConfidenceProvider/Http/HttpClient.swift @@ -6,8 +6,8 @@ protocol HttpClient { func post( path: String, data: Codable, - completion: @escaping (HttpClientResult) -> Void - ) throws + completion: @escaping (HttpClientResult) async -> Void + ) async throws func post(path: String, data: Codable) async throws -> HttpClientResponse } diff --git a/Sources/ConfidenceProvider/Http/NetworkClient.swift b/Sources/ConfidenceProvider/Http/NetworkClient.swift index 9928ac4a..585cb5c3 100644 --- a/Sources/ConfidenceProvider/Http/NetworkClient.swift +++ b/Sources/ConfidenceProvider/Http/NetworkClient.swift @@ -26,14 +26,14 @@ final class NetworkClient: HttpClient { retry: Retry = .none ) { self.session = - session - ?? { - let configuration = URLSessionConfiguration.default - configuration.timeoutIntervalForRequest = timeout - configuration.httpAdditionalHeaders = defaultHeaders + session + ?? { + let configuration = URLSessionConfiguration.default + configuration.timeoutIntervalForRequest = timeout + configuration.httpAdditionalHeaders = defaultHeaders - return URLSession(configuration: configuration) - }() + return URLSession(configuration: configuration) + }() self.headers = defaultHeaders self.retry = retry @@ -44,26 +44,26 @@ final class NetworkClient: HttpClient { func post( path: String, data: Codable, - completion: @escaping (HttpClientResult) -> Void - ) throws { + completion: @escaping (HttpClientResult) async -> Void + ) async throws { let request = try buildRequest(path: path, data: data) - perform(request: request, retry: self.retry) { response, data, error in + await perform(request: request, retry: self.retry) { response, data, error in if let error { - completion(.failure(error)) + await completion(.failure(error)) return } guard let response, let data else { - completion(.failure(ConfidenceError.internalError(message: "Bad response"))) + await completion(.failure(ConfidenceError.internalError(message: "Bad response"))) return } do { let httpClientResult: HttpClientResponse = - try self.buildResponse(response: response, data: data) - completion(.success(httpClientResult)) + try self.buildResponse(response: response, data: data) + await completion(.success(httpClientResult)) } catch { - completion(.failure(error)) + await completion(.failure(error)) } } } @@ -71,40 +71,34 @@ final class NetworkClient: HttpClient { private func perform( request: URLRequest, retry: Retry, - completion: @escaping (HTTPURLResponse?, Data?, Error?) -> Void - ) { + completion: @escaping (HTTPURLResponse?, Data?, Error?) async -> Void + ) async { let retryHandler = retry.handler() let retryWait: TimeInterval? = retryHandler.retryIn() - self.session.dataTask(with: request) { data, response, error in - if let error { - if self.shouldRetry(error: error), let retryWait { - DispatchQueue.main.asyncAfter(deadline: .now() + retryWait) { - self.perform(request: request, retry: retry, completion: completion) - } - return - } else { - completion(nil, nil, error) - } - } - - guard let httpResponse = response as? HTTPURLResponse else { - completion(nil, nil, HttpClientError.invalidResponse) + do { + async let (data, response) = await self.session.data(for: request) + guard let httpResponse = try? await response as? HTTPURLResponse else { + await completion(nil, nil, HttpClientError.invalidResponse) return } - if self.shouldRetry(httpResponse: httpResponse), let retryWait { - DispatchQueue.main.asyncAfter(deadline: .now() + retryWait) { - self.perform(request: request, retry: retry, completion: completion) - } + try? await Task.sleep(nanoseconds: UInt64(retryWait * 1_000_000_000)) + await self.perform(request: request, retry: retry, completion: completion) return } - - if let data { - completion(httpResponse, data, nil) - } else { + guard let data = try? await data else { let error = ConfidenceError.internalError(message: "Unable to complete request") - completion(httpResponse, nil, error) + await completion(httpResponse, nil, error) + return + } + await completion(httpResponse, data, 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) + } else { + await completion(nil, nil, error) } } } diff --git a/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift b/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift index 6d325e6c..3a46a0d8 100644 --- a/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift +++ b/Tests/ConfidenceProviderTests/Helpers/HttpClientMock.swift @@ -22,13 +22,13 @@ final class HttpClientMock: HttpClient { func post( path: String, data: Codable, - completion: @escaping (ConfidenceProvider.HttpClientResult) -> Void - ) throws where T: Decodable { + completion: @escaping (ConfidenceProvider.HttpClientResult) async -> Void + ) async throws where T: Decodable { do { let result: HttpClientResponse = try handlePost(path: path, data: data) - completion(.success(result)) + await completion(.success(result)) } catch { - completion(.failure(error)) + await completion(.failure(error)) } } From 223418c65fb313f10df0c88ebfca773b66b3f54d Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Fri, 12 Jan 2024 15:07:32 +0100 Subject: [PATCH 3/4] fix: Setup async tests without Task Signed-off-by: Fabrizio Demaria --- .../CacheDataInteractorTests.swift | 47 ++++++++----------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/Tests/ConfidenceProviderTests/CacheDataInteractorTests.swift b/Tests/ConfidenceProviderTests/CacheDataInteractorTests.swift index 30ba8ace..e9425a69 100644 --- a/Tests/ConfidenceProviderTests/CacheDataInteractorTests.swift +++ b/Tests/ConfidenceProviderTests/CacheDataInteractorTests.swift @@ -5,7 +5,7 @@ import XCTest @testable import ConfidenceProvider final class CacheDataInteractorTests: XCTestCase { - func testCacheDataInteractor_loadsEventsFromStorage() throws { + func testCacheDataInteractor_loadsEventsFromStorage() async throws { // Given prefilled storage with 10 resolve events (20 apply events in each) let prefilledCache = try CacheDataUtility.prefilledCacheData( resolveEventCount: 10, @@ -16,30 +16,25 @@ final class CacheDataInteractorTests: XCTestCase { let cacheDataInteractor = CacheDataInteractor(cacheData: prefilledCache) // Then cache data is loaded from storage - Task { - // Wrapped it in the Task in order to ensure that async code is completed before assertions - let cache = await cacheDataInteractor.cache - XCTAssertEqual(cache.resolveEvents.count, 10) - XCTAssertEqual(cache.resolveEvents.last?.events.count, 20) - } + let cache = await cacheDataInteractor.cache + XCTAssertEqual(cache.resolveEvents.count, 10) + XCTAssertEqual(cache.resolveEvents.last?.events.count, 20) } func testCacheDataInteractor_addEventToEmptyCache() async throws { // Given cache data interactor with no previously stored data let cacheDataInteractor = CacheDataInteractor(cacheData: .empty()) - Task { - let cache = await cacheDataInteractor.cache - XCTAssertEqual(cache.resolveEvents.count, 0) - } - - Task { - // When cache data add method is called - _ = await cacheDataInteractor.add(resolveToken: "token", flagName: "name", applyTime: Date()) - - // Then event is added with - let cache = await cacheDataInteractor.cache - XCTAssertEqual(cache.resolveEvents.count, 1) - } + + let cache = await cacheDataInteractor.cache + XCTAssertEqual(cache.resolveEvents.count, 0) + + + // When cache data add method is called + _ = await cacheDataInteractor.add(resolveToken: "token", flagName: "name", applyTime: Date()) + + // Then event is added with + let cache2 = await cacheDataInteractor.cache + XCTAssertEqual(cache2.resolveEvents.count, 1) } func testCacheDataInteractor_addEventToPreFilledCache() async throws { @@ -47,13 +42,11 @@ final class CacheDataInteractorTests: XCTestCase { let prefilledCacheData = try CacheDataUtility.prefilledCacheData(applyEventCount: 2) let cacheDataInteractor = CacheDataInteractor(cacheData: prefilledCacheData) - Task { - // When cache data add method is called - _ = await cacheDataInteractor.add(resolveToken: "token", flagName: "name", applyTime: Date()) + // When cache data add method is called + _ = await cacheDataInteractor.add(resolveToken: "token", flagName: "name", applyTime: Date()) - // Then event is added with - let cache = await cacheDataInteractor.cache - XCTAssertEqual(cache.resolveEvents.count, 2) - } + // Then event is added with + let cache = await cacheDataInteractor.cache + XCTAssertEqual(cache.resolveEvents.count, 2) } } From 0536a6357902dae04f3ea0ad6477707cb867ec3d Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Mon, 15 Jan 2024 13:28:08 +0100 Subject: [PATCH 4/4] fix: Remove async lets Signed-off-by: Fabrizio Demaria --- .../Apply/FlagApplierWithRetries.swift | 8 ++++---- Sources/ConfidenceProvider/Http/NetworkClient.swift | 9 ++------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift b/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift index b6dbb173..f5813677 100644 --- a/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift +++ b/Sources/ConfidenceProvider/Apply/FlagApplierWithRetries.swift @@ -37,26 +37,26 @@ final class FlagApplierWithRetries: FlagApplier { public func apply(flagName: String, resolveToken: String) async { let applyTime = Date.backport.now - async let (data, added) = await cacheDataInteractor.add( + let (data, added) = await cacheDataInteractor.add( resolveToken: resolveToken, flagName: flagName, applyTime: applyTime ) - guard await added == true else { + guard added == true else { // If record is found in the cache, early return (de-duplication). // Triggerring batch apply in case if there are any unsent events stored await triggerBatch() return } - await self.writeToFile(data: data) + self.writeToFile(data: data) await triggerBatch() } // MARK: private private func triggerBatch() async { - async let cacheData = await cacheDataInteractor.cache + let cacheData = await cacheDataInteractor.cache await cacheData.resolveEvents.asyncForEach { resolveEvent in let appliesToSend = resolveEvent.events.filter { $0.status == .created } .chunk(size: 20) diff --git a/Sources/ConfidenceProvider/Http/NetworkClient.swift b/Sources/ConfidenceProvider/Http/NetworkClient.swift index 585cb5c3..0ba073b4 100644 --- a/Sources/ConfidenceProvider/Http/NetworkClient.swift +++ b/Sources/ConfidenceProvider/Http/NetworkClient.swift @@ -77,8 +77,8 @@ final class NetworkClient: HttpClient { let retryWait: TimeInterval? = retryHandler.retryIn() do { - async let (data, response) = await self.session.data(for: request) - guard let httpResponse = try? await response as? HTTPURLResponse else { + let (data, response) = try await self.session.data(for: request) + guard let httpResponse = response as? HTTPURLResponse else { await completion(nil, nil, HttpClientError.invalidResponse) return } @@ -87,11 +87,6 @@ final class NetworkClient: HttpClient { await self.perform(request: request, retry: retry, completion: completion) return } - guard let data = try? await data else { - let error = ConfidenceError.internalError(message: "Unable to complete request") - await completion(httpResponse, nil, error) - return - } await completion(httpResponse, data, nil) } catch { if self.shouldRetry(error: error), let retryWait {