From d5ac718df271699c3cd9184215f34ca66a4cd605 Mon Sep 17 00:00:00 2001 From: Okhan Okbay Date: Thu, 18 Jan 2024 11:39:04 +0000 Subject: [PATCH] Add new error codes to handle HTTP 422 Also wrap the runRequest(_:_:) function in an async wrapper. --- .github/scripts/lintEditedFiles.sh | 2 +- .github/workflows/verify-pr.yml | 6 +- .swiftlint.yml | 55 ++++++++++++++ Package.swift | 8 +- .../CheckoutClientInterface.swift | 5 +- .../CheckoutNetworkClient.swift | 40 ++++++++-- .../CheckoutNetworkError.swift | 7 ++ .../CheckoutNetwork/Models/ErrorReason.swift | 20 +++++ .../CheckoutNetworkFakeClient.swift | 14 +++- .../AsyncWrapperTests.swift | 73 +++++++++++++++++++ .../CheckoutNetworkClientTests.swift | 4 +- 11 files changed, 214 insertions(+), 20 deletions(-) create mode 100644 .swiftlint.yml create mode 100644 Sources/CheckoutNetwork/Models/ErrorReason.swift create mode 100644 Tests/CheckoutNetworkTests/AsyncWrapperTests.swift diff --git a/.github/scripts/lintEditedFiles.sh b/.github/scripts/lintEditedFiles.sh index 9d5505d..f799125 100644 --- a/.github/scripts/lintEditedFiles.sh +++ b/.github/scripts/lintEditedFiles.sh @@ -5,7 +5,7 @@ git fetch origin main echo "Fetched" # Use a conditional to check if there are edited files -if EDITED_FILES=$(git diff HEAD origin/main --name-only --diff-filter=d | grep "\.swift" | grep -v "\.swiftlint\.yml" | xargs echo | tr ' ' ','); then +if EDITED_FILES=$(git diff HEAD origin/main --name-only --diff-filter=d | grep "\.swift" | grep -v "\.swiftlint\.yml" | xargs echo | tr ' ' ',' | sed 's/,/, /g'); then echo "Got edited files" echo $EDITED_FILES diff --git a/.github/workflows/verify-pr.yml b/.github/workflows/verify-pr.yml index 7118ef2..5499c36 100644 --- a/.github/workflows/verify-pr.yml +++ b/.github/workflows/verify-pr.yml @@ -32,6 +32,10 @@ jobs: - name: Checkout repository uses: actions/checkout@v3 - - name: Build + - name: Build the Package run: | set -o pipefail && xcodebuild -scheme CheckoutNetwork -destination "platform=iOS Simulator,name=iPhone 14 Pro,OS=latest" + + - name: Run Tests + run: | + set -o pipefail && xcodebuild -scheme CheckoutNetwork -destination "platform=iOS Simulator,name=iPhone 14 Pro,OS=latest" test diff --git a/.swiftlint.yml b/.swiftlint.yml new file mode 100644 index 0000000..f84241c --- /dev/null +++ b/.swiftlint.yml @@ -0,0 +1,55 @@ +included: + - Source + +excluded: + - Tests + +opt_in_rules: + - array_init + - closure_end_indentation + - closure_spacing + - collection_alignment + - colon # promote to error + - convenience_type + - discouraged_object_literal + - empty_collection_literal + - empty_count + - empty_string + - enum_case_associated_values_count + - fatal_error_message + - first_where + - force_unwrapping + - implicitly_unwrapped_optional + - last_where + - legacy_random + - literal_expression_end_indentation + - multiline_arguments + - multiline_function_chains + - multiline_literal_brackets + - multiline_parameters + - multiline_parameters_brackets + - operator_usage_whitespace + - overridden_super_call + - pattern_matching_keywords + - prefer_self_type_over_type_of_self + - redundant_nil_coalescing + - redundant_type_annotation + - strict_fileprivate + - toggle_bool + - trailing_closure + - unneeded_parentheses_in_closure_argument + - yoda_condition + +disabled_rules: + - multiline_parameters_brackets + +analyzer_rules: + - unused_import + +line_length: + warning: 160 + ignores_urls: true + +identifier_name: + excluded: + - id diff --git a/Package.swift b/Package.swift index 9c85791..ff564ea 100644 --- a/Package.swift +++ b/Package.swift @@ -6,7 +6,7 @@ import PackageDescription let package = Package( name: "CheckoutNetwork", platforms: [ - .iOS(.v11), + .iOS(.v13), ], products: [ .library( @@ -19,12 +19,14 @@ let package = Package( ], targets: [ .target( - name: "CheckoutNetwork"), + name: "CheckoutNetwork", + path: "Sources/CheckoutNetwork"), .target( name: "CheckoutNetworkFakeClient", dependencies: ["CheckoutNetwork"]), .testTarget( name: "CheckoutNetworkTests", - dependencies: ["CheckoutNetwork"]), + dependencies: ["CheckoutNetwork"], + path: "Tests"), ] ) diff --git a/Sources/CheckoutNetwork/CheckoutClientInterface.swift b/Sources/CheckoutNetwork/CheckoutClientInterface.swift index e40adb4..9227fd3 100644 --- a/Sources/CheckoutNetwork/CheckoutClientInterface.swift +++ b/Sources/CheckoutNetwork/CheckoutClientInterface.swift @@ -19,7 +19,10 @@ public protocol CheckoutClientInterface { /// Create, customise and run a request with the given configuration, calling the completion handler once completed func runRequest(with configuration: RequestConfiguration, completionHandler: @escaping CompletionHandler) - + + /// Async wrapper of func runRequest(_:_:) with CompletionHandler + func runRequest(with configuration: RequestConfiguration) async throws -> T + /// Create, customise and run a request with the given configuration, calling the completion handler once completed func runRequest(with configuration: RequestConfiguration, completionHandler: @escaping NoDataResponseCompletionHandler) diff --git a/Sources/CheckoutNetwork/CheckoutNetworkClient.swift b/Sources/CheckoutNetwork/CheckoutNetworkClient.swift index cfd3208..efda55f 100644 --- a/Sources/CheckoutNetwork/CheckoutNetworkClient.swift +++ b/Sources/CheckoutNetwork/CheckoutNetworkClient.swift @@ -35,15 +35,17 @@ public class CheckoutNetworkClient: CheckoutClientInterface { completionHandler(.failure(error)) return } - if let responseError = self.getErrorFromResponse(response) { - completionHandler(.failure(responseError)) - return - } guard let data = data else { completionHandler(.failure(CheckoutNetworkError.noDataResponseReceived)) return } + + if let responseError = self.getErrorFromResponse(response, data: data) { + completionHandler(.failure(responseError)) + return + } + do { let dataResponse = try JSONDecoder().decode(T.self, from: data) completionHandler(.success(dataResponse)) @@ -55,7 +57,20 @@ public class CheckoutNetworkClient: CheckoutClientInterface { task.resume() } } - + + public func runRequest(with configuration: RequestConfiguration) async throws -> T { + return try await withCheckedThrowingContinuation { continuation in + runRequest(with: configuration) { (result: Result) in + switch result { + case .success(let response): + continuation.resume(returning: response) + case .failure(let error): + continuation.resume(throwing: error) + } + } + } + } + public func runRequest(with configuration: RequestConfiguration, completionHandler: @escaping NoDataResponseCompletionHandler) { taskQueue.sync { @@ -68,7 +83,7 @@ public class CheckoutNetworkClient: CheckoutClientInterface { completionHandler(error) return } - if let responseError = self.getErrorFromResponse(response) { + if let responseError = self.getErrorFromResponse(response, data: nil) { completionHandler(responseError) return } @@ -87,11 +102,20 @@ public class CheckoutNetworkClient: CheckoutClientInterface { return taskID } - private func getErrorFromResponse(_ response: URLResponse?) -> Error? { - guard let response = response as? HTTPURLResponse else { + private func getErrorFromResponse(_ response: URLResponse?, data: Data?) -> Error? { + guard let response = response as? HTTPURLResponse else { return CheckoutNetworkError.unexpectedResponseCode(code: 0) } + guard response.statusCode != 422 else { + do { + let errorReason = try JSONDecoder().decode(ErrorReason.self, from: data ?? Data()) + return CheckoutNetworkError.invalidData(reason: errorReason) + } catch { + return CheckoutNetworkError.invalidDataResponseReceivedWithNoData + } + } + guard response.statusCode >= 200, response.statusCode < 300 else { return CheckoutNetworkError.unexpectedResponseCode(code: response.statusCode) diff --git a/Sources/CheckoutNetwork/CheckoutNetworkError.swift b/Sources/CheckoutNetwork/CheckoutNetworkError.swift index 9e05be9..c85b79d 100644 --- a/Sources/CheckoutNetwork/CheckoutNetworkError.swift +++ b/Sources/CheckoutNetwork/CheckoutNetworkError.swift @@ -17,4 +17,11 @@ public enum CheckoutNetworkError: Error, Equatable { /// Network call and completion appear valid but no data was returned making the parsing impossible. Use different call if no data is expected case noDataResponseReceived + + /// Network response returned with HTTP Code 422 + case invalidData(reason: ErrorReason) + + + /// HTTP code 422 received with no meaningful data alongside + case invalidDataResponseReceivedWithNoData } diff --git a/Sources/CheckoutNetwork/Models/ErrorReason.swift b/Sources/CheckoutNetwork/Models/ErrorReason.swift new file mode 100644 index 0000000..8026257 --- /dev/null +++ b/Sources/CheckoutNetwork/Models/ErrorReason.swift @@ -0,0 +1,20 @@ +// +// InvalidData.swift +// +// +// Created by Okhan Okbay on 18/01/2024. +// + +import Foundation + +public struct ErrorReason: Decodable, Equatable { + public let requestID: String + public let errorType: String + public let errorCodes: [String] + + enum CodingKeys: String, CodingKey { + case requestID = "request_id" + case errorType = "error_type" + case errorCodes = "error_codes" + } +} diff --git a/Sources/CheckoutNetworkFakeClient/CheckoutNetworkFakeClient.swift b/Sources/CheckoutNetworkFakeClient/CheckoutNetworkFakeClient.swift index 8e9ae52..80d7cad 100644 --- a/Sources/CheckoutNetworkFakeClient/CheckoutNetworkFakeClient.swift +++ b/Sources/CheckoutNetworkFakeClient/CheckoutNetworkFakeClient.swift @@ -9,17 +9,23 @@ import Foundation import CheckoutNetwork final public class CheckoutNetworkFakeClient: CheckoutClientInterface { - public var calledRequests: [(config: RequestConfiguration, completion: Any)] = [] - + + public var calledAsyncRequests: [RequestConfiguration] = [] + public var dataToBeReturned: Decodable! + public func runRequest(with configuration: RequestConfiguration, completionHandler: @escaping CompletionHandler) { calledRequests.append((config: configuration, completion: completionHandler)) } - + + public func runRequest(with configuration: CheckoutNetwork.RequestConfiguration) async throws -> T { + calledAsyncRequests.append(configuration) + return dataToBeReturned as! T + } + public func runRequest(with configuration: RequestConfiguration, completionHandler: @escaping NoDataResponseCompletionHandler) { calledRequests.append((configuration, completionHandler)) } - } diff --git a/Tests/CheckoutNetworkTests/AsyncWrapperTests.swift b/Tests/CheckoutNetworkTests/AsyncWrapperTests.swift new file mode 100644 index 0000000..3141d75 --- /dev/null +++ b/Tests/CheckoutNetworkTests/AsyncWrapperTests.swift @@ -0,0 +1,73 @@ +// +// AsyncWrapperTests.swift +// +// +// Created by Okhan Okbay on 18/01/2024. +// + +@testable import CheckoutNetwork +import XCTest + +final class AsyncWrapperTests: XCTestCase { + + func test_whenRunRequestReturnsData_ThenAsyncRunRequestPropagatesIt() async throws { + let fakeSession = FakeSession() + let fakeDataTask = FakeDataTask() + fakeSession.calledDataTasksReturn = fakeDataTask + let client = CheckoutNetworkClientSpy(session: fakeSession) + let testConfig = try! RequestConfiguration(path: FakePath.testServices) + + let expectedResult = FakeObject(id: "some response") + client.expectedResult = expectedResult + client.expectedError = nil + let result: FakeObject = try await client.runRequest(with: testConfig) + XCTAssertEqual(client.configuration.request, testConfig.request) + XCTAssertEqual(client.runRequestCallCount, 1) + XCTAssertEqual(result, expectedResult) + } + + func test_whenRunRequestReturnsError_ThenAsyncRunRequestPropagatesIt() async throws { + let fakeSession = FakeSession() + let fakeDataTask = FakeDataTask() + fakeSession.calledDataTasksReturn = fakeDataTask + let client = CheckoutNetworkClientSpy(session: fakeSession) + let testConfig = try! RequestConfiguration(path: FakePath.testServices) + + let expectedError = FakeError.someError + client.expectedResult = nil + client.expectedError = expectedError + + do { + let _: FakeObject = try await client.runRequest(with: testConfig) + XCTFail("An error was expected to be thrown") + } catch let error as FakeError { + XCTAssertEqual(client.configuration.request, testConfig.request) + XCTAssertEqual(client.runRequestCallCount, 1) + XCTAssertEqual(error, expectedError) + } + } +} + +enum FakeError: Error { + case someError +} + +class CheckoutNetworkClientSpy: CheckoutNetworkClient { + private(set) var runRequestCallCount: Int = 0 + private(set) var configuration: RequestConfiguration! + + var expectedResult: FakeObject? + var expectedError: Error? + + override func runRequest(with configuration: RequestConfiguration, completionHandler: @escaping CheckoutNetworkClient.CompletionHandler) where T : Decodable { + runRequestCallCount += 1 + self.configuration = configuration + + if let result = expectedResult { + completionHandler(.success(result as! T)) + } else if let error = expectedError { + completionHandler(.failure(error)) + } + } +} + diff --git a/Tests/CheckoutNetworkTests/CheckoutNetworkClientTests.swift b/Tests/CheckoutNetworkTests/CheckoutNetworkClientTests.swift index 920dcb6..3e1cc3c 100644 --- a/Tests/CheckoutNetworkTests/CheckoutNetworkClientTests.swift +++ b/Tests/CheckoutNetworkTests/CheckoutNetworkClientTests.swift @@ -32,7 +32,7 @@ final class CheckoutNetworkClientTests: XCTestCase { let testConfig = try! RequestConfiguration(path: FakePath.testServices) client.runRequest(with: testConfig) { (result: Result) in } - + XCTAssertEqual(client.tasks.count, 1) XCTAssertTrue(client.tasks.values.first === fakeDataTask) XCTAssertTrue(fakeDataTask.wasStarted) @@ -73,7 +73,7 @@ final class CheckoutNetworkClientTests: XCTestCase { XCTAssertEqual(failure as NSError, expectedError) } } - + XCTAssertFalse(client.tasks.isEmpty) let requestCompletion = fakeSession.calledDataTasks.first!.completion requestCompletion(expectedData, expectedResponse, expectedError)