From 24827f83b4212729a0e1984fc74339c1ff62ea00 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 1 Dec 2023 09:00:16 +1300 Subject: [PATCH 1/4] Add URLSession helpers to send requests and handle responses --- WordPressKit.xcodeproj/project.pbxproj | 4 ++ WordPressKit/HTTPClient.swift | 68 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 WordPressKit/HTTPClient.swift diff --git a/WordPressKit.xcodeproj/project.pbxproj b/WordPressKit.xcodeproj/project.pbxproj index 861db606..26c8fc6c 100644 --- a/WordPressKit.xcodeproj/project.pbxproj +++ b/WordPressKit.xcodeproj/project.pbxproj @@ -141,6 +141,7 @@ 46ABD0EA262EEE0400C7FF24 /* AppTransportSecuritySettingsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ABD0E9262EEE0400C7FF24 /* AppTransportSecuritySettingsTests.swift */; }; 4A11239A2B19269A004690CF /* WordPressAPIError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A1123992B19269A004690CF /* WordPressAPIError.swift */; }; 4A11239C2B1926B7004690CF /* HTTPRequestBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A11239B2B1926B7004690CF /* HTTPRequestBuilder.swift */; }; + 4A11239E2B1926D1004690CF /* HTTPClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A11239D2B1926D1004690CF /* HTTPClient.swift */; }; 4A1DEF44293051BC00322608 /* LoggingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A1DEF43293051BC00322608 /* LoggingTests.swift */; }; 4A1DEF46293051C600322608 /* LoggingTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 4A1DEF45293051C600322608 /* LoggingTests.m */; }; 4A68E3CD29404181004AC3DC /* RemoteBlog.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A68E3CC29404181004AC3DC /* RemoteBlog.swift */; }; @@ -835,6 +836,7 @@ 46ABD0E9262EEE0400C7FF24 /* AppTransportSecuritySettingsTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AppTransportSecuritySettingsTests.swift; sourceTree = ""; }; 4A1123992B19269A004690CF /* WordPressAPIError.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = WordPressAPIError.swift; sourceTree = ""; }; 4A11239B2B1926B7004690CF /* HTTPRequestBuilder.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HTTPRequestBuilder.swift; sourceTree = ""; }; + 4A11239D2B1926D1004690CF /* HTTPClient.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HTTPClient.swift; sourceTree = ""; }; 4A1DEF43293051BC00322608 /* LoggingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoggingTests.swift; sourceTree = ""; }; 4A1DEF45293051C600322608 /* LoggingTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = LoggingTests.m; sourceTree = ""; }; 4A68E3CC29404181004AC3DC /* RemoteBlog.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoteBlog.swift; sourceTree = ""; }; @@ -2424,6 +2426,7 @@ 93BD27791EE73944002BB00B /* WordPressOrgXMLRPCApi.swift */, 93BD277A1EE73944002BB00B /* WordPressOrgXMLRPCValidator.swift */, 93BD277B1EE73944002BB00B /* WordPressRSDParser.swift */, + 4A11239D2B1926D1004690CF /* HTTPClient.swift */, 4A11239B2B1926B7004690CF /* HTTPRequestBuilder.swift */, 4A1123992B19269A004690CF /* WordPressAPIError.swift */, ); @@ -3235,6 +3238,7 @@ C76F456825B9F30E00BFEC87 /* JetpackScanHistory.swift in Sources */, E1D6B556200E46F300325669 /* WPTimeZone.swift in Sources */, 93F50A3F1F227C8900B5BEBA /* UsersServiceRemoteXMLRPC.swift in Sources */, + 4A11239E2B1926D1004690CF /* HTTPClient.swift in Sources */, 8C5734F925681A6A005E61EE /* Enum+UnknownCaseRepresentable.swift in Sources */, 9F3E0BA32087345F009CB5BA /* ReaderTopicServiceRemote+Subscription.swift in Sources */, 9F3E0B9E208733C3009CB5BA /* ReaderServiceDeliveryFrequency.swift in Sources */, diff --git a/WordPressKit/HTTPClient.swift b/WordPressKit/HTTPClient.swift new file mode 100644 index 00000000..a81b640a --- /dev/null +++ b/WordPressKit/HTTPClient.swift @@ -0,0 +1,68 @@ +import Foundation + +public typealias WordPressAPIResult = Result> + +struct HTTPAPIResponse{ + typealias Body = Body + + var response: HTTPURLResponse + var body: Body +} + +extension URLSession { + + func apiResult( + with builder: HTTPRequestBuilder, + errorType: E.Type = E.self + ) async -> WordPressAPIResult, E> { + guard let request = try? builder.build() else { + return .failure(.requestEncodingFailure) + } + + let result: (Data, URLResponse) + do { + result = try await data(for: request) + } catch { + if let urlError = error as? URLError { + return .failure(.connection(urlError)) + } else { + return .failure(.unknown(underlyingError: error)) + } + } + + let (body, response) = result + + guard let response = response as? HTTPURLResponse else { + return .failure(.unparsableResponse(response: nil, body: body)) + } + + return .success(.init(response: response, body: body)) + } + +} + +extension Result where Success == HTTPAPIResponse { + + func assessStatusCode( + acceptable: [ClosedRange] = [200...299], + success: (Success) -> S?, + failure: (Success) -> E? + ) -> WordPressAPIResult where Failure == WordPressAPIError { + flatMap { response in + if acceptable.contains(where: { $0 ~= response.response.statusCode }) { + if let result = success(response) { + return .success(result) + } else { + return .failure(.unparsableResponse(response: response.response, body: response.body)) + } + } else { + if let endpointError = failure(response) { + return .failure(.endpointError(endpointError)) + } else { + return .failure(.unparsableResponse(response: response.response, body: response.body)) + } + } + } + } + +} From 9091464710af05acf1e36828aacecf496c06f8c4 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 11 Dec 2023 14:53:04 +1300 Subject: [PATCH 2/4] Add unit tests for URLSession helpers --- WordPressKit.xcodeproj/project.pbxproj | 4 + .../Utilities/URLSessionHelperTests.swift | 129 ++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 WordPressKitTests/Utilities/URLSessionHelperTests.swift diff --git a/WordPressKit.xcodeproj/project.pbxproj b/WordPressKit.xcodeproj/project.pbxproj index 26c8fc6c..86714efd 100644 --- a/WordPressKit.xcodeproj/project.pbxproj +++ b/WordPressKit.xcodeproj/project.pbxproj @@ -153,6 +153,7 @@ 4A68E3DF29407100004AC3DC /* RemoteReaderTopic.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A68E3DE29407100004AC3DC /* RemoteReaderTopic.swift */; }; 4A68E3E1294076C1004AC3DC /* RemoteReaderSiteInfo.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A68E3E0294076C1004AC3DC /* RemoteReaderSiteInfo.swift */; }; 4A6B4A842B26974F00802316 /* HTTPRequestBuilderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A6B4A832B26974F00802316 /* HTTPRequestBuilderTests.swift */; }; + 4A6B4A862B269D0C00802316 /* URLSessionHelperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A6B4A852B269D0C00802316 /* URLSessionHelperTests.swift */; }; 4AA5A1A32AA68F6B00969464 /* MediaLibraryTestSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AA5A1A22AA68F6B00969464 /* MediaLibraryTestSupport.swift */; }; 4AA5A1A52AA695D700969464 /* LoadMediaLibraryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AA5A1A42AA695D700969464 /* LoadMediaLibraryTests.swift */; }; 57BCD3D426209D9500292CB3 /* AppTransportSecuritySettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57BCD3D326209D9500292CB3 /* AppTransportSecuritySettings.swift */; }; @@ -848,6 +849,7 @@ 4A68E3DE29407100004AC3DC /* RemoteReaderTopic.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoteReaderTopic.swift; sourceTree = ""; }; 4A68E3E0294076C1004AC3DC /* RemoteReaderSiteInfo.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoteReaderSiteInfo.swift; sourceTree = ""; }; 4A6B4A832B26974F00802316 /* HTTPRequestBuilderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPRequestBuilderTests.swift; sourceTree = ""; }; + 4A6B4A852B269D0C00802316 /* URLSessionHelperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLSessionHelperTests.swift; sourceTree = ""; }; 4AA5A1A22AA68F6B00969464 /* MediaLibraryTestSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaLibraryTestSupport.swift; sourceTree = ""; }; 4AA5A1A42AA695D700969464 /* LoadMediaLibraryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadMediaLibraryTests.swift; sourceTree = ""; }; 57BCD3D326209D9500292CB3 /* AppTransportSecuritySettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppTransportSecuritySettings.swift; sourceTree = ""; }; @@ -2635,6 +2637,7 @@ 4A1DEF43293051BC00322608 /* LoggingTests.swift */, 4A1DEF45293051C600322608 /* LoggingTests.m */, 4A6B4A832B26974F00802316 /* HTTPRequestBuilderTests.swift */, + 4A6B4A852B269D0C00802316 /* URLSessionHelperTests.swift */, ); path = Utilities; sourceTree = ""; @@ -3433,6 +3436,7 @@ buildActionMask = 2147483647; files = ( E1E89C6A1FD6BDB1006E7A33 /* PluginDirectoryTests.swift in Sources */, + 4A6B4A862B269D0C00802316 /* URLSessionHelperTests.swift in Sources */, 9F3E0BAA20873773009CB5BA /* MockServiceRequest.swift in Sources */, 8B2F4BE524ABB3C70056C08A /* RemoteReaderPostTests.m in Sources */, 4AA5A1A32AA68F6B00969464 /* MediaLibraryTestSupport.swift in Sources */, diff --git a/WordPressKitTests/Utilities/URLSessionHelperTests.swift b/WordPressKitTests/Utilities/URLSessionHelperTests.swift new file mode 100644 index 00000000..c2c6c22d --- /dev/null +++ b/WordPressKitTests/Utilities/URLSessionHelperTests.swift @@ -0,0 +1,129 @@ +import Foundation +import XCTest +import OHHTTPStubs + +@testable import WordPressKit + +class URLSessionHelperTests: XCTestCase { + + override func tearDown() { + HTTPStubs.removeAllStubs() + } + + func testConnectionError() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(error: URLError(.serverCertificateUntrusted)) + } + + let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + do { + _ = try result.get() + XCTFail("The above call should throw") + } catch let WordPressAPIError.connection(error) { + XCTAssertEqual(error.code, URLError.Code.serverCertificateUntrusted) + } catch { + XCTFail("Unknown error: \(error)") + } + } + + func test200() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) + } + + let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + + // The result is a successful result. This line should not throw + _ = try result.get() + + let expectation = expectation(description: "API call returns a successful result") + _ = result + .assessStatusCode { result in + XCTAssertEqual(String(data: result.body, encoding: .utf8), "success") + expectation.fulfill() + return result + } failure: { _ in + // Do nothing + return nil + } + await fulfillment(of: [expectation]) + } + + func testUnacceptable500() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(data: "Internal server error".data(using: .utf8)!, statusCode: 500, headers: nil) + } + + let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + + // The result is a successful result. This line should not throw + _ = try result.get() + + let expectation = expectation(description: "API call returns server error") + _ = result + .assessStatusCode { result in + return result + } failure: { result in + XCTAssertEqual(String(data: result.body, encoding: .utf8), "Internal server error") + expectation.fulfill() + return nil + } + await fulfillment(of: [expectation]) + } + + func testAcceptable404() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(data: "Not found".data(using: .utf8)!, statusCode: 404, headers: nil) + } + + let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + + // The result is a successful result. This line should not throw + _ = try result.get() + + let expectation = expectation(description: "API call returns not found") + _ = result + .assessStatusCode(acceptable: [200...299, 400...499]) { result in + XCTAssertEqual(String(data: result.body, encoding: .utf8), "Not found") + expectation.fulfill() + return result + } failure: { result in + return nil + } + await fulfillment(of: [expectation]) + } + + func testParseError() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(data: "Not found".data(using: .utf8)!, statusCode: 404, headers: nil) + } + + let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + + // The result is a successful result. This line should not throw + _ = try result.get() + + let expectation = expectation(description: "API call returns not found") + let parsedResult = result + .assessStatusCode { result in + return result + } failure: { result in + expectation.fulfill() + if result.response.statusCode == 404 { + return .postNotFound + } + return nil + } + await fulfillment(of: [expectation]) + + if case .failure(WordPressAPIError.endpointError(.postNotFound)) = parsedResult { + // DO nothing + } else { + XCTFail("Unexpected result: \(parsedResult)") + } + } +} + +private enum TestError: LocalizedError { + case postNotFound +} From 34ead3d13087072d277b02a16932eb90645d7c55 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 11 Dec 2023 15:06:59 +1300 Subject: [PATCH 3/4] Fix swiftlint issues --- WordPressKit/HTTPClient.swift | 2 +- WordPressKitTests/Utilities/URLSessionHelperTests.swift | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPressKit/HTTPClient.swift b/WordPressKit/HTTPClient.swift index a81b640a..28c7477b 100644 --- a/WordPressKit/HTTPClient.swift +++ b/WordPressKit/HTTPClient.swift @@ -2,7 +2,7 @@ import Foundation public typealias WordPressAPIResult = Result> -struct HTTPAPIResponse{ +struct HTTPAPIResponse { typealias Body = Body var response: HTTPURLResponse diff --git a/WordPressKitTests/Utilities/URLSessionHelperTests.swift b/WordPressKitTests/Utilities/URLSessionHelperTests.swift index c2c6c22d..647e42e5 100644 --- a/WordPressKitTests/Utilities/URLSessionHelperTests.swift +++ b/WordPressKitTests/Utilities/URLSessionHelperTests.swift @@ -7,6 +7,7 @@ import OHHTTPStubs class URLSessionHelperTests: XCTestCase { override func tearDown() { + super.tearDown() HTTPStubs.removeAllStubs() } From 996cabecf9a27c6b92513144251f825cd05b4385 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 12 Dec 2023 09:40:31 +1300 Subject: [PATCH 4/4] Address PR comments --- WordPressKit/HTTPClient.swift | 8 +++----- .../Utilities/URLSessionHelperTests.swift | 10 +++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/WordPressKit/HTTPClient.swift b/WordPressKit/HTTPClient.swift index 28c7477b..ac72deb2 100644 --- a/WordPressKit/HTTPClient.swift +++ b/WordPressKit/HTTPClient.swift @@ -1,18 +1,16 @@ import Foundation -public typealias WordPressAPIResult = Result> +public typealias WordPressAPIResult = Result> struct HTTPAPIResponse { - typealias Body = Body - var response: HTTPURLResponse var body: Body } extension URLSession { - func apiResult( - with builder: HTTPRequestBuilder, + func perform( + request builder: HTTPRequestBuilder, errorType: E.Type = E.self ) async -> WordPressAPIResult, E> { guard let request = try? builder.build() else { diff --git a/WordPressKitTests/Utilities/URLSessionHelperTests.swift b/WordPressKitTests/Utilities/URLSessionHelperTests.swift index 647e42e5..4fdc1be5 100644 --- a/WordPressKitTests/Utilities/URLSessionHelperTests.swift +++ b/WordPressKitTests/Utilities/URLSessionHelperTests.swift @@ -16,7 +16,7 @@ class URLSessionHelperTests: XCTestCase { HTTPStubsResponse(error: URLError(.serverCertificateUntrusted)) } - let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + let result = await URLSession.shared.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) do { _ = try result.get() XCTFail("The above call should throw") @@ -32,7 +32,7 @@ class URLSessionHelperTests: XCTestCase { HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) } - let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + let result = await URLSession.shared.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) // The result is a successful result. This line should not throw _ = try result.get() @@ -55,7 +55,7 @@ class URLSessionHelperTests: XCTestCase { HTTPStubsResponse(data: "Internal server error".data(using: .utf8)!, statusCode: 500, headers: nil) } - let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + let result = await URLSession.shared.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) // The result is a successful result. This line should not throw _ = try result.get() @@ -77,7 +77,7 @@ class URLSessionHelperTests: XCTestCase { HTTPStubsResponse(data: "Not found".data(using: .utf8)!, statusCode: 404, headers: nil) } - let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + let result = await URLSession.shared.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) // The result is a successful result. This line should not throw _ = try result.get() @@ -99,7 +99,7 @@ class URLSessionHelperTests: XCTestCase { HTTPStubsResponse(data: "Not found".data(using: .utf8)!, statusCode: 404, headers: nil) } - let result = await URLSession.shared.apiResult(with: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) + let result = await URLSession.shared.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), errorType: TestError.self) // The result is a successful result. This line should not throw _ = try result.get()