From 4f0d7782cd60ca6738104c45241518b589aafd80 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 11 Mar 2024 09:41:55 +1300 Subject: [PATCH 1/5] Add URLSession helpers to send request and parse response --- Package.resolved | 14 + Package.swift | 7 +- .../swift/Sources/wordpress-api/Either.swift | 15 + .../URLSession+WordPressAPI.swift | 283 ++++++++++++++++ .../Sources/wordpress-api/WordPressAPI.swift | 75 +---- .../wordpress-api/WordPressAPIError.swift | 46 +++ .../wordpress-api/URLSessionHelperTests.swift | 316 ++++++++++++++++++ .../wordpress-api/WpNetworkRequest.swift | 8 + 8 files changed, 695 insertions(+), 69 deletions(-) create mode 100644 Package.resolved create mode 100644 native/swift/Sources/wordpress-api/Either.swift create mode 100644 native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift create mode 100644 native/swift/Sources/wordpress-api/WordPressAPIError.swift create mode 100644 native/swift/Tests/wordpress-api/URLSessionHelperTests.swift create mode 100644 native/swift/Tests/wordpress-api/WpNetworkRequest.swift diff --git a/Package.resolved b/Package.resolved new file mode 100644 index 000000000..682d7ab9a --- /dev/null +++ b/Package.resolved @@ -0,0 +1,14 @@ +{ + "pins" : [ + { + "identity" : "ohhttpstubs", + "kind" : "remoteSourceControl", + "location" : "https://github.com/AliSoftware/OHHTTPStubs", + "state" : { + "revision" : "12f19662426d0434d6c330c6974d53e2eb10ecd9", + "version" : "9.1.0" + } + } + ], + "version" : 2 +} diff --git a/Package.swift b/Package.swift index 406b1c19e..fdfc301fe 100644 --- a/Package.swift +++ b/Package.swift @@ -27,7 +27,9 @@ let package = Package( targets: ["wordpress-api"] ) ], - dependencies: [], + dependencies: [ + .package(url: "https://github.com/AliSoftware/OHHTTPStubs", .upToNextMajor(from: "9.1.0")) + ], targets: [ .target( name: "wordpress-api", @@ -51,7 +53,8 @@ let package = Package( name: "wordpress-api-tests", dependencies: [ .target(name: "wordpress-api"), - .target(name: "libwordpressFFI") + .target(name: "libwordpressFFI"), + .product(name: "OHHTTPStubsSwift", package: "OHHTTPStubs") ], path: "native/swift/Tests/wordpress-api" ) diff --git a/native/swift/Sources/wordpress-api/Either.swift b/native/swift/Sources/wordpress-api/Either.swift new file mode 100644 index 000000000..4b895f7a8 --- /dev/null +++ b/native/swift/Sources/wordpress-api/Either.swift @@ -0,0 +1,15 @@ +import Foundation + +enum Either { + case left(L) + case right(R) + + func map(left: (L) -> T, right: (R) -> T) -> T { + switch self { + case let .left(value): + return left(value) + case let .right(value): + return right(value) + } + } +} diff --git a/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift b/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift new file mode 100644 index 000000000..125cecf65 --- /dev/null +++ b/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift @@ -0,0 +1,283 @@ +import Foundation +import wordpress_api_wrapper + +typealias WordPressAPIResult = Result + +extension URLSession { + + /// Create a background URLSession instance that can be used in the `perform(request:...)` async function. + /// + /// The `perform(request:...)` async function can be used in all non-background `URLSession` instances without any + /// extra work. However, there is a requirement to make the function works with with background `URLSession` instances. + /// That is the `URLSession` must have a delegate of `BackgroundURLSessionDelegate` type. + static func backgroundSession(configuration: URLSessionConfiguration) -> URLSession { + assert(configuration.identifier != nil) + // Pass `delegateQueue: nil` to get a serial queue, which is required to ensure thread safe access to + // `WordPressKitSessionDelegate` instances. + return URLSession(configuration: configuration, delegate: BackgroundURLSessionDelegate(), delegateQueue: nil) + } + + /// Send a HTTP request and return its response as a `WordPressAPIResult` instance. + /// + /// ## Progress Tracking and Cancellation + /// + /// You can track the HTTP request's overall progress by passing a `Progress` instance to the `fulfillingProgress` + /// parameter, which must satisify following requirements: + /// - `totalUnitCount` must not be zero. + /// - `completedUnitCount` must be zero. + /// - It's used exclusivity for tracking the HTTP request overal progress: No children in its progress tree. + /// - `cancellationHandler` must be nil. You can call `fulfillingProgress.cancel()` to cancel the ongoing HTTP request. + /// + /// Upon completion, the HTTP request's progress fulfills the `fulfillingProgress`. + /// + /// Please note: `parentProgress` may be updated from a background thread. + /// + /// - Parameters: + /// - request: A `WpNetworkRequest` instance that represents an HTTP request to be sent. + /// - parentProgress: A `Progress` instance that will be used as the parent progress of the HTTP request's overall + /// progress. See the function documentation regarding requirements on this argument. + func perform( + request: WpNetworkRequest, + fulfilling parentProgress: Progress? = nil + ) async -> WordPressAPIResult { + if configuration.identifier != nil { + assert(delegate is BackgroundURLSessionDelegate, "Unexpected `URLSession` delegate type. See the `backgroundSession(configuration:)`") + } + + if let parentProgress { + assert(parentProgress.completedUnitCount == 0 && parentProgress.totalUnitCount > 0, "Invalid parent progress") + assert(parentProgress.cancellationHandler == nil, "The progress instance's cancellationHandler property must be nil") + } + + return await withCheckedContinuation { continuation in + let completion: @Sendable (Data?, URLResponse?, Error?) -> Void = { data, response, error in + let result: WordPressAPIResult = Self.parseResponse( + data: data, + response: response, + error: error + ) + + continuation.resume(returning: result) + } + + let task: URLSessionTask + + do { + task = try self.task(for: request, completion: completion) + } catch { + continuation.resume(returning: .failure(.requestEncodingFailure(underlyingError: error))) + return + } + + task.resume() + + if let parentProgress, parentProgress.totalUnitCount > parentProgress.completedUnitCount { + let pending = parentProgress.totalUnitCount - parentProgress.completedUnitCount + parentProgress.addChild(task.progress, withPendingUnitCount: pending) + + parentProgress.cancellationHandler = { [weak task] in + task?.cancel() + } + } + } + } + + private func task( + for wpRequest: WpNetworkRequest, + completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, Error?) -> Void + ) throws -> URLSessionTask { + let request = try wpRequest.asURLRequest() + + // This additional `callCompletionFromDelegate` is added to unit test `BackgroundURLSessionDelegate`. + // Background `URLSession` doesn't work on unit tests, we have to create a non-background `URLSession` + // which has a `BackgroundURLSessionDelegate` delegate in order to test `BackgroundURLSessionDelegate`. + // + // In reality, `callCompletionFromDelegate` and `isBackgroundSession` have the same value. + let callCompletionFromDelegate = delegate is BackgroundURLSessionDelegate +// let isBackgroundSession = configuration.identifier != nil + let task: URLSessionTask + let body = wpRequest.body + var completion = originalCompletion + if let body { + // Use special `URLSession.uploadTask` API for multipart POST requests. + task = body.map( + left: { + if callCompletionFromDelegate { + return uploadTask(with: request, from: $0) + } else { + return uploadTask(with: request, from: $0, completionHandler: completion) + } + }, + right: { tempFileURL in + // Remove the temp file, which contains request body, once the HTTP request completes. + completion = { data, response, error in + try? FileManager.default.removeItem(at: tempFileURL) + originalCompletion(data, response, error) + } + + if callCompletionFromDelegate { + return uploadTask(with: request, fromFile: tempFileURL) + } else { + return uploadTask(with: request, fromFile: tempFileURL, completionHandler: completion) + } + } + ) + } else { + // Use `URLSession.dataTask` for all other request + if callCompletionFromDelegate { + task = dataTask(with: request) + } else { + task = dataTask(with: request, completionHandler: completion) + } + } + + if callCompletionFromDelegate { + assert(delegate is BackgroundURLSessionDelegate, "Unexpected `URLSession` delegate type. See the `backgroundSession(configuration:)`") + + set(completion: completion, forTaskWithIdentifier: task.taskIdentifier) + } + + return task + } + + private static func parseResponse( + data: Data?, + response: URLResponse?, + error: Error? + ) -> WordPressAPIResult { + let result: WordPressAPIResult + + if let error { + if let urlError = error as? URLError { + result = .failure(.connection(urlError)) + } else { + result = .failure(.unknown(underlyingError: error)) + } + } else { + if let httpResponse = response as? HTTPURLResponse { + result = .success(.init(body: data ?? Data(), statusCode: UInt16(httpResponse.statusCode), headerMap: httpResponse.httpHeaders)) + } else { + result = .failure(.unparsableResponse(response: nil, body: data, underlyingError: URLError(.badServerResponse))) + } + } + + return result + } + +} + +// MARK: - Background URL Session Support + +private final class SessionTaskData { + var responseBody = Data() + var completion: ((Data?, URLResponse?, Error?) -> Void)? +} + +class BackgroundURLSessionDelegate: NSObject, URLSessionDataDelegate { + + private var taskData = [Int: SessionTaskData]() + + func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { + session.received(data, forTaskWithIdentifier: dataTask.taskIdentifier) + } + + func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { + session.completed(with: error, response: task.response, forTaskWithIdentifier: task.taskIdentifier) + } + +} + +private extension URLSession { + + static var taskDataKey = 0 + + // A map from `URLSessionTask` identifier to in-memory data of the given task. + // + // This property is in `URLSession` not `BackgroundURLSessionDelegate` because task id (the key) is unique within + // the context of a `URLSession` instance. And in theory `BackgroundURLSessionDelegate` can be used by multiple + // `URLSession` instances. + var taskData: [Int: SessionTaskData] { + get { + objc_getAssociatedObject(self, &URLSession.taskDataKey) as? [Int: SessionTaskData] ?? [:] + } + set { + objc_setAssociatedObject(self, &URLSession.taskDataKey, newValue, .OBJC_ASSOCIATION_RETAIN) + } + } + + func updateData(forTaskWithIdentifier taskID: Int, using closure: (SessionTaskData) -> Void) { + let task = self.taskData[taskID] ?? SessionTaskData() + closure(task) + self.taskData[taskID] = task + } + + func set(completion: @escaping (Data?, URLResponse?, Error?) -> Void, forTaskWithIdentifier taskID: Int) { + updateData(forTaskWithIdentifier: taskID) { + $0.completion = completion + } + } + + func received(_ data: Data, forTaskWithIdentifier taskID: Int) { + updateData(forTaskWithIdentifier: taskID) { task in + task.responseBody.append(data) + } + } + + func completed(with error: Error?, response: URLResponse?, forTaskWithIdentifier taskID: Int) { + guard let task = taskData[taskID] else { + return + } + + if let error { + task.completion?(nil, response, error) + } else { + task.completion?(task.responseBody, response, nil) + } + + self.taskData.removeValue(forKey: taskID) + } + +} + +// MARK: - wordpress_api_wrapper helpers + +extension WpNetworkRequest { + func asURLRequest() throws -> URLRequest { + guard let url = URL(string: self.url), (url.scheme == "http" || url.scheme == "https") else { + throw URLError(.badURL) + } + var request = URLRequest(url: url) + request.httpMethod = self.method.rawValue + request.allHTTPHeaderFields = self.headerMap + return request + } + + var body: Either? { + // TODO: To be implemented + return nil + } +} + +extension HTTPURLResponse { + + var httpHeaders: [String: String] { + allHeaderFields.reduce(into: [String: String]()) { + guard + let key = $1.key as? String, + let value = $1.value as? String + else { + return + } + + $0.updateValue(value, forKey: key) + } + } +} + +// MARK: - Debug / unit test supprt + +extension URLSession { + var debugNumberOfTaskData: Int { + self.taskData.count + } +} diff --git a/native/swift/Sources/wordpress-api/WordPressAPI.swift b/native/swift/Sources/wordpress-api/WordPressAPI.swift index 8790ce1f2..621a486b4 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPI.swift @@ -6,57 +6,29 @@ import FoundationNetworking #endif public struct WordPressAPI { - - enum Errors: Error { - case unableToParseResponse - } - private let urlSession: URLSession package let helper: WpApiHelperProtocol - public init(urlSession: URLSession, baseUrl: URL, authenticationStategy: WpAuthentication) { + public init(urlSession: URLSession, baseUrl: URL, authenticationStrategy: WpAuthentication) { + // TODO: We use URLSession APIs that accept completion block, which doesn't work with background URLSession. + // See `URLSession.backgroundSession(configuration:)` in `URLSession+WordPressAPI.swift`. self.urlSession = urlSession - self.helper = WpApiHelper(siteUrl: baseUrl.absoluteString, authentication: authenticationStategy) + self.helper = WpApiHelper(siteUrl: baseUrl.absoluteString, authentication: authenticationStrategy) } package func perform(request: WpNetworkRequest) async throws -> WpNetworkResponse { - try await withCheckedThrowingContinuation { continuation in - self.perform(request: request) { result in - continuation.resume(with: result) - } - } + try await self.urlSession.perform(request: request).get() } package func perform(request: WpNetworkRequest, callback: @escaping (Result) -> Void) { - let task = self.urlSession.dataTask(with: request.asURLRequest()) { data, response, error in - if let error { - callback(.failure(error)) - return - } - - guard let data = data, let response = response else { - callback(.failure(Errors.unableToParseResponse)) - return - } - + Task { do { - let response = try WpNetworkResponse.from(data: data, response: response) - callback(.success(response)) + let result = try await self.perform(request: request) + callback(.success(result)) } catch { callback(.failure(error)) } } - task.resume() - } -} - -public extension WpNetworkRequest { - func asURLRequest() -> URLRequest { - let url = URL(string: self.url)! - var request = URLRequest(url: url) - request.httpMethod = self.method.rawValue - request.allHTTPHeaderFields = self.headerMap - return request } } @@ -77,37 +49,6 @@ extension Result { } } -extension WpNetworkResponse { - static func from(data: Data, response: URLResponse) throws -> WpNetworkResponse { - guard let response = response as? HTTPURLResponse else { - abort() - } - - return WpNetworkResponse( - body: data, - statusCode: UInt16(response.statusCode), - headerMap: response.httpHeaders - ) - - } -} - -extension HTTPURLResponse { - - var httpHeaders: [String: String] { - allHeaderFields.reduce(into: [String: String]()) { - guard - let key = $1.key as? String, - let value = $1.value as? String - else { - return - } - - $0.updateValue(value, forKey: key) - } - } -} - // Note: Everything below this line should be moved into the Rust layer public extension WpAuthentication { init(username: String, password: String) { diff --git a/native/swift/Sources/wordpress-api/WordPressAPIError.swift b/native/swift/Sources/wordpress-api/WordPressAPIError.swift new file mode 100644 index 000000000..ffe512c6a --- /dev/null +++ b/native/swift/Sources/wordpress-api/WordPressAPIError.swift @@ -0,0 +1,46 @@ +import Foundation + +public enum WordPressAPIError: Error { + static var unknownErrorMessage: String { + NSLocalizedString( + "wordpress-rs.api.error.unknown", + value: "Something went wrong, please try again later.", + comment: "Error message that describes an unknown error had occured" + ) + } + + /// Can't encode the request arguments into a valid HTTP request. This is a programming error. + case requestEncodingFailure(underlyingError: Error) + /// Error occured in the HTTP connection. + case connection(URLError) + /// The API call returned an HTTP response that WordPressKit can't parse. Receiving this error could be an indicator that there is an error response that's not handled properly by WordPressKit. + case unparsableResponse(response: HTTPURLResponse?, body: Data?, underlyingError: Error) + /// Other error occured. + case unknown(underlyingError: Error) +} + +extension WordPressAPIError: LocalizedError { + + public var errorDescription: String? { + // Considering `WordPressAPIError` is the error that's surfaced from this library to the apps, its instanes + // may be displayed on UI directly. To prevent Swift's default error message (i.e. "This operation can't be + // completed. (code=...)") from being displayed, we need to make sure this implementation + // always returns a non-nil value. + let localizedErrorMessage: String + switch self { + case .requestEncodingFailure, .unparsableResponse: + // These are usually programming errors. + localizedErrorMessage = Self.unknownErrorMessage + case let .connection(error): + localizedErrorMessage = error.localizedDescription + case let .unknown(underlyingError): + if let msg = (underlyingError as? LocalizedError)?.errorDescription { + localizedErrorMessage = msg + } else { + localizedErrorMessage = Self.unknownErrorMessage + } + } + return localizedErrorMessage + } + +} diff --git a/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift b/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift new file mode 100644 index 000000000..5e1f938e5 --- /dev/null +++ b/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift @@ -0,0 +1,316 @@ +import Foundation +import CryptoKit +import XCTest +import OHHTTPStubs +import OHHTTPStubsSwift + +@testable import wordpress_api + +class URLSessionHelperTests: XCTestCase { + + var session: URLSession! + + override func setUp() { + super.setUp() + session = .shared + } + + override func tearDown() { + super.tearDown() + HTTPStubs.removeAllStubs() + XCTAssertEqual(session.debugNumberOfTaskData, 0) + } + + func testConnectionError() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(error: URLError(.serverCertificateUntrusted)) + } + + let result = await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)) + 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 session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)) + + // The result is a successful result. This line should not throw + let response = try result.get() + + XCTAssertEqual(String(data: response.body, encoding: .utf8), "success") + } + + func test500() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(data: "Internal server error".data(using: .utf8)!, statusCode: 500, headers: nil) + } + + let result = await session + .perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)) + try XCTAssertEqual(result.get().statusCode, 500) + } + + func testHeader() async throws { + stub(condition: hasHeaderNamed("X-Request", value: "Ping")) { _ in + HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: ["X-Response": "Pong"]) + } + + let result = await session + .perform(request: .init(method: .get, url: "https://wordpress.org/hello", headerMap: ["X-Request": "Ping"])) + try XCTAssertEqual(result.get().statusCode, 200) + try XCTAssertEqual(result.get().headerMap?["X-Response"], "Pong") + } + + func testProgressTracking() async throws { + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) + } + + let progress = Progress.discreteProgress(totalUnitCount: 20) + XCTAssertEqual(progress.completedUnitCount, 0) + XCTAssertEqual(progress.fractionCompleted, 0) + + let _ = await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), fulfilling: progress) + XCTAssertEqual(progress.completedUnitCount, 20) + XCTAssertEqual(progress.fractionCompleted, 1) + } + + func testCancellation() async throws { + // Give a slow HTTP request that takes 0.5 second to complete + stub(condition: isPath("/hello")) { _ in + let response = HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) + response.responseTime = 0.5 + return response + } + + // and cancelling it (in 0.1 second) before it completes + let progress = Progress.discreteProgress(totalUnitCount: 20) + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(100)) { + progress.cancel() + } + + // The result should be an cancellation result + let result = await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), fulfilling: progress) + if case let .failure(.connection(urlError)) = result, urlError.code == .cancelled { + // Do nothing + } else { + XCTFail("Unexpected result: \(result)") + } + } + + // TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// func testEncodingError() async { +// let underlyingError = NSError(domain: "test", code: 123) +// let builder = HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) +// .method(.post) +// .body(json: { throw underlyingError }) +// let result = await session.perform(request: builder, errorType: TestError.self) +// +// if case let .failure(.requestEncodingFailure(underlyingError: error)) = result { +// XCTAssertEqual(error as NSError, underlyingError) +// } else { +// XCTFail("Unexpected result: \(result)") +// } +// } + +// TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// func testMultipartForm() async throws { +// var req: URLRequest? +// stub(condition: isPath("/hello")) { +// req = $0 +// return HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) +// } +// +// let builder = HTTPRequestBuilder(url: URL(string: "https://wordpress.org/hello")!) +// .method(.post) +// .body(form: [MultipartFormField(text: "value", name: "name", filename: nil)]) +// +// let _ = await session.perform(request: builder, errorType: TestError.self) +// +// let request = try XCTUnwrap(req) +// let boundary = try XCTUnwrap( +// request +// .value(forHTTPHeaderField: "Content-Type")?.split(separator: ";") +// .map { $0.trimmingCharacters(in: .whitespaces) } +// .reduce(into: [String: String]()) { +// let pair = $1.split(separator: "=") +// if pair.count == 2 { +// $0[String(pair[0])] = String(pair[1]) +// } +// }["boundary"] +// ) +// +// let requestBody = try XCTUnwrap(request.httpBody ?? request.httpBodyStream?.readToEnd()) +// +// let expectedBody = "--\(boundary)\r\nContent-Disposition: form-data; name=\"name\"\r\n\r\nvalue\r\n--\(boundary)--\r\n" +// XCTAssertEqual(String(data: requestBody, encoding: .utf8), expectedBody) +// } + + func testGetLargeData() async throws { + let file = try self.createLargeFile(megaBytes: 100) + defer { + try? FileManager.default.removeItem(at: file) + } + + stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(fileURL: file, statusCode: 200, headers: nil) + } + + let response = try await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)).get() + + try XCTAssertEqual( + sha256(XCTUnwrap(InputStream(url: file))), + sha256(InputStream(data: response.body)) + ) + } + +// TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// func testTempFileRemovedAfterMultipartUpload() async throws { +// stub(condition: isPath("/upload")) { _ in +// HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) +// } +// +// // Create a large file which will be uploaded. The file size needs to be larger than the hardcoded threshold of +// // creating a temporary file for upload. +// let file = try self.createLargeFile(megaBytes: 30) +// defer { +// try? FileManager.default.removeItem(at: file) +// } +// +// // Capture a list of files in temp dirs, before calling the upload function. +// let tempFilesBeforeUpload = existingTempFiles() +// +// // Perform upload HTTP request +// let builder = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org/upload")!) +// .method(.post) +// .body(form: [MultipartFormField(fileAtPath: file.path, name: "file", filename: "file.txt", mimeType: "text/plain")]) +// let _ = await session.perform(request: builder, errorType: TestError.self) +// +// // Capture a list of files in the temp dirs, after calling the upload function. +// let tempFilesAfterUpload = existingTempFiles() +// +// // There should be no new files after the HTTP request returns. This assertion relies on an implementation detail +// // where the multipart form content is put into a file in temp dirs. +// let newFiles = tempFilesAfterUpload.subtracting(tempFilesBeforeUpload) +// XCTAssertEqual(newFiles.count, 0) +// } + +// TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// func testTempFileRemovedAfterMultipartUploadError() async throws { +// stub(condition: isPath("/upload")) { _ in +// HTTPStubsResponse(error: URLError(.networkConnectionLost)) +// } +// +// // Create a large file which will be uploaded. The file size needs to be larger than the hardcoded threshold of +// // creating a temporary file for upload. +// let file = try self.createLargeFile(megaBytes: 30) +// defer { +// try? FileManager.default.removeItem(at: file) +// } +// +// // Capture a list of files in temp dirs, before calling the upload function. +// let tempFilesBeforeUpload = existingTempFiles() +// +// // Perform upload HTTP request +// let builder = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org/upload")!) +// .method(.post) +// .body(form: [MultipartFormField(fileAtPath: file.path, name: "file", filename: "file.txt", mimeType: "text/plain")]) +// let _ = await session.perform(request: builder, errorType: TestError.self) +// +// // Capture a list of files in the temp dirs, after calling the upload function. +// let tempFilesAfterUpload = existingTempFiles() +// +// // There should be no new files after the HTTP request returns. This assertion relies on an implementation detail +// // where the multipart form content is put into a file in temp dirs. +// let newFiles = tempFilesAfterUpload.subtracting(tempFilesBeforeUpload) +// XCTAssertEqual(newFiles.count, 0) +// } + + private func existingTempFiles() -> Set { + let fm = FileManager.default + let enumerators = [ + fm.enumerator(atPath: NSTemporaryDirectory()), + fm.enumerator(atPath: fm.temporaryDirectory.path) + ].compactMap { $0 } + + var result: Set = [] + for enumerator in enumerators { + while let file = enumerator.nextObject() as? String { + result.insert(file) + } + } + return result + } + + private func createLargeFile(megaBytes: Int) throws -> URL { + let file = try FileManager.default.url(for: .documentDirectory, in: .userDomainMask, appropriateFor: nil, create: true) + .appendingPathComponent("large-file-\(UUID().uuidString).txt") + + try Data(repeating: 46, count: 1024 * 1000 * megaBytes).write(to: file) + + return file + } + + private func sha256(_ stream: InputStream) -> SHA256Digest { + stream.open() + defer { stream.close() } + + var hash = SHA256() + let maxLength = 50 * 1024 + var buffer = [UInt8](repeating: 0, count: maxLength) + while stream.hasBytesAvailable { + let bytes = stream.read(&buffer, maxLength: maxLength) + let data = Data(bytesNoCopy: &buffer, count: bytes, deallocator: .none) + hash.update(data: data) + } + return hash.finalize() + } +} + +class BackgroundURLSessionHelperTests: URLSessionHelperTests { + + // swiftlint:disable weak_delegate + private var delegate: TestBackgroundURLSessionDelegate! + // swiftlint:enable weak_delegate + + override func setUp() { + super.setUp() + + delegate = TestBackgroundURLSessionDelegate() + session = URLSession(configuration: .default, delegate: delegate, delegateQueue: nil) + } + + override func tearDown() { + super.tearDown() + + if delegate.startedReceivingResponse { + XCTAssertTrue(delegate.completionCalled) + } + } + +} + +private class TestBackgroundURLSessionDelegate: BackgroundURLSessionDelegate { + var startedReceivingResponse = false + var completionCalled = false + + override func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { + startedReceivingResponse = true + super.urlSession(session, dataTask: dataTask, didReceive: data) + } + + override func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { + completionCalled = true + super.urlSession(session, task: task, didCompleteWithError: error) + } +} diff --git a/native/swift/Tests/wordpress-api/WpNetworkRequest.swift b/native/swift/Tests/wordpress-api/WpNetworkRequest.swift new file mode 100644 index 000000000..3045db438 --- /dev/null +++ b/native/swift/Tests/wordpress-api/WpNetworkRequest.swift @@ -0,0 +1,8 @@ +import Foundation +import wordpress_api_wrapper + +extension WpNetworkRequest { + init(url: URL) { + self.init(method: .get, url: url.absoluteString, headerMap: nil) + } +} From d9741a88512c7cbff4dd9751ddcf8b566863ee0f Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Mar 2024 12:04:28 +1300 Subject: [PATCH 2/5] Background URLSession is only available on Apple platforms --- Package.swift | 12 +- .../URLSession+WordPressAPI.swift | 190 ++++++++++++------ .../wordpress-api/WordPressAPIError.swift | 4 + 3 files changed, 141 insertions(+), 65 deletions(-) diff --git a/Package.swift b/Package.swift index fdfc301fe..d64637fbb 100644 --- a/Package.swift +++ b/Package.swift @@ -13,6 +13,8 @@ let libwordpressFFI: Target = .systemLibrary( let libwordpressFFI: Target = .binaryTarget(name: "libwordpressFFI", path: "target/libwordpressFFI.xcframework") #endif +let supportBackgroundURLSession: SwiftSetting = .define("WP_SUPPORT_BACKGROUND_URL_SESSION", .when(platforms: [.macOS, .iOS, .tvOS, .watchOS])) + let package = Package( name: "wordpress", platforms: [ @@ -36,7 +38,10 @@ let package = Package( dependencies: [ .target(name: "wordpress-api-wrapper") ], - path: "native/swift/Sources/wordpress-api" + path: "native/swift/Sources/wordpress-api", + swiftSettings: [ + supportBackgroundURLSession + ] ), .target( name: "wordpress-api-wrapper", @@ -56,7 +61,10 @@ let package = Package( .target(name: "libwordpressFFI"), .product(name: "OHHTTPStubsSwift", package: "OHHTTPStubs") ], - path: "native/swift/Tests/wordpress-api" + path: "native/swift/Tests/wordpress-api", + swiftSettings: [ + supportBackgroundURLSession + ] ) ] ) diff --git a/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift b/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift index 125cecf65..28557d56d 100644 --- a/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift @@ -1,22 +1,14 @@ import Foundation import wordpress_api_wrapper +#if os(Linux) +import FoundationNetworking +#endif + typealias WordPressAPIResult = Result extension URLSession { - /// Create a background URLSession instance that can be used in the `perform(request:...)` async function. - /// - /// The `perform(request:...)` async function can be used in all non-background `URLSession` instances without any - /// extra work. However, there is a requirement to make the function works with with background `URLSession` instances. - /// That is the `URLSession` must have a delegate of `BackgroundURLSessionDelegate` type. - static func backgroundSession(configuration: URLSessionConfiguration) -> URLSession { - assert(configuration.identifier != nil) - // Pass `delegateQueue: nil` to get a serial queue, which is required to ensure thread safe access to - // `WordPressKitSessionDelegate` instances. - return URLSession(configuration: configuration, delegate: BackgroundURLSessionDelegate(), delegateQueue: nil) - } - /// Send a HTTP request and return its response as a `WordPressAPIResult` instance. /// /// ## Progress Tracking and Cancellation @@ -40,9 +32,11 @@ extension URLSession { request: WpNetworkRequest, fulfilling parentProgress: Progress? = nil ) async -> WordPressAPIResult { +#if WP_SUPPORT_BACKGROUND_URL_SESSION if configuration.identifier != nil { assert(delegate is BackgroundURLSessionDelegate, "Unexpected `URLSession` delegate type. See the `backgroundSession(configuration:)`") } +#endif if let parentProgress { assert(parentProgress.completedUnitCount == 0 && parentProgress.totalUnitCount > 0, "Invalid parent progress") @@ -84,60 +78,15 @@ extension URLSession { private func task( for wpRequest: WpNetworkRequest, - completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, Error?) -> Void + completion: @escaping @Sendable (Data?, URLResponse?, Error?) -> Void ) throws -> URLSessionTask { let request = try wpRequest.asURLRequest() - // This additional `callCompletionFromDelegate` is added to unit test `BackgroundURLSessionDelegate`. - // Background `URLSession` doesn't work on unit tests, we have to create a non-background `URLSession` - // which has a `BackgroundURLSessionDelegate` delegate in order to test `BackgroundURLSessionDelegate`. - // - // In reality, `callCompletionFromDelegate` and `isBackgroundSession` have the same value. - let callCompletionFromDelegate = delegate is BackgroundURLSessionDelegate -// let isBackgroundSession = configuration.identifier != nil - let task: URLSessionTask - let body = wpRequest.body - var completion = originalCompletion - if let body { - // Use special `URLSession.uploadTask` API for multipart POST requests. - task = body.map( - left: { - if callCompletionFromDelegate { - return uploadTask(with: request, from: $0) - } else { - return uploadTask(with: request, from: $0, completionHandler: completion) - } - }, - right: { tempFileURL in - // Remove the temp file, which contains request body, once the HTTP request completes. - completion = { data, response, error in - try? FileManager.default.removeItem(at: tempFileURL) - originalCompletion(data, response, error) - } - - if callCompletionFromDelegate { - return uploadTask(with: request, fromFile: tempFileURL) - } else { - return uploadTask(with: request, fromFile: tempFileURL, completionHandler: completion) - } - } - ) + if let body = wpRequest.body { + return createUploadTask(with: request, body: body, completion: completion) } else { - // Use `URLSession.dataTask` for all other request - if callCompletionFromDelegate { - task = dataTask(with: request) - } else { - task = dataTask(with: request, completionHandler: completion) - } - } - - if callCompletionFromDelegate { - assert(delegate is BackgroundURLSessionDelegate, "Unexpected `URLSession` delegate type. See the `backgroundSession(configuration:)`") - - set(completion: completion, forTaskWithIdentifier: task.taskIdentifier) + return createDataTask(with: request, completion: completion) } - - return task } private static func parseResponse( @@ -168,6 +117,117 @@ extension URLSession { // MARK: - Background URL Session Support +#if WP_SUPPORT_BACKGROUND_URL_SESSION + +private extension URLSession { + + func createDataTask(with request: URLRequest, completion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionDataTask { + // This additional `callCompletionFromDelegate` is added to unit test `BackgroundURLSessionDelegate`. + // Background `URLSession` doesn't work on unit tests, we have to create a non-background `URLSession` + // which has a `BackgroundURLSessionDelegate` delegate in order to test `BackgroundURLSessionDelegate`. + // + // In reality, `callCompletionFromDelegate` and `isBackgroundSession` have the same value. + let callCompletionFromDelegate = delegate is BackgroundURLSessionDelegate + + let task: URLSessionDataTask + if callCompletionFromDelegate { + task = dataTask(with: request) + set(completion: completion, forTaskWithIdentifier: task.taskIdentifier) + } else { + task = dataTask(with: request, completionHandler: completion) + } + + return task + } + + func createUploadTask(with request: URLRequest, body: Either, completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionUploadTask { + // This additional `callCompletionFromDelegate` is added to unit test `BackgroundURLSessionDelegate`. + // Background `URLSession` doesn't work on unit tests, we have to create a non-background `URLSession` + // which has a `BackgroundURLSessionDelegate` delegate in order to test `BackgroundURLSessionDelegate`. + // + // In reality, `callCompletionFromDelegate` and `isBackgroundSession` have the same value. + let callCompletionFromDelegate = delegate is BackgroundURLSessionDelegate + + var completion = originalCompletion + + let task = body.map( + left: { + if callCompletionFromDelegate { + return uploadTask(with: request, from: $0) + } else { + return uploadTask(with: request, from: $0, completionHandler: completion) + } + }, + right: { tempFileURL in + // Remove the temp file, which contains request body, once the HTTP request completes. + completion = { data, response, error in + try? FileManager.default.removeItem(at: tempFileURL) + originalCompletion(data, response, error) + } + + if callCompletionFromDelegate { + return uploadTask(with: request, fromFile: tempFileURL) + } else { + return uploadTask(with: request, fromFile: tempFileURL, completionHandler: completion) + } + } + ) + + if callCompletionFromDelegate { + set(completion: completion, forTaskWithIdentifier: task.taskIdentifier) + } + + return task + } + +} + +#else + +private extension URLSession { + + func createDataTask(with request: URLRequest, completion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionDataTask { + dataTask(with: request, completionHandler: completion) + } + + func createUploadTask(with request: URLRequest, body: Either, completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionUploadTask { + body.map( + left: { + uploadTask(with: request, from: $0, completionHandler: originalCompletion) + }, + right: { tempFileURL in + // Remove the temp file, which contains request body, once the HTTP request completes. + let completion = { data, response, error in + try? FileManager.default.removeItem(at: tempFileURL) + originalCompletion(data, response, error) + } + return uploadTask(with: request, fromFile: tempFileURL, completionHandler: completion) + } + ) + } + +} + +#endif + +#if WP_SUPPORT_BACKGROUND_URL_SESSION + +extension URLSession { + + /// Create a background URLSession instance that can be used in the `perform(request:...)` async function. + /// + /// The `perform(request:...)` async function can be used in all non-background `URLSession` instances without any + /// extra work. However, there is a requirement to make the function works with with background `URLSession` instances. + /// That is the `URLSession` must have a delegate of `BackgroundURLSessionDelegate` type. + static func backgroundSession(configuration: URLSessionConfiguration) -> URLSession { + assert(configuration.identifier != nil) + // Pass `delegateQueue: nil` to get a serial queue, which is required to ensure thread safe access to + // `WordPressKitSessionDelegate` instances. + return URLSession(configuration: configuration, delegate: BackgroundURLSessionDelegate(), delegateQueue: nil) + } + +} + private final class SessionTaskData { var responseBody = Data() var completion: ((Data?, URLResponse?, Error?) -> Void)? @@ -175,8 +235,6 @@ private final class SessionTaskData { class BackgroundURLSessionDelegate: NSObject, URLSessionDataDelegate { - private var taskData = [Int: SessionTaskData]() - func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { session.received(data, forTaskWithIdentifier: dataTask.taskIdentifier) } @@ -239,6 +297,8 @@ private extension URLSession { } +#endif + // MARK: - wordpress_api_wrapper helpers extension WpNetworkRequest { @@ -278,6 +338,10 @@ extension HTTPURLResponse { extension URLSession { var debugNumberOfTaskData: Int { +#if WP_SUPPORT_BACKGROUND_URL_SESSION self.taskData.count +#else + 0 +#endif } } diff --git a/native/swift/Sources/wordpress-api/WordPressAPIError.swift b/native/swift/Sources/wordpress-api/WordPressAPIError.swift index ffe512c6a..878826604 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPIError.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPIError.swift @@ -1,5 +1,9 @@ import Foundation +#if os(Linux) +import FoundationNetworking +#endif + public enum WordPressAPIError: Error { static var unknownErrorMessage: String { NSLocalizedString( From d4499fc85dad1d6cbfcd2deaa229b83561a314a1 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Mar 2024 12:05:31 +1300 Subject: [PATCH 3/5] Replace OHHTTPStubs with in-process HTTP server OHHTTPStubs can't be compiled on Linux because it depends on the Objective-C Foundation framework, which is not available on Linux. --- Package.resolved | 17 ++- Package.swift | 6 +- .../wordpress-api/TestSupport/HTTPStub.swift | 136 ++++++++++++++++++ .../wordpress-api/URLSessionHelperTests.swift | 93 ++++++------ 4 files changed, 195 insertions(+), 57 deletions(-) create mode 100644 native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift diff --git a/Package.resolved b/Package.resolved index 682d7ab9a..5eb92fc5d 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,12 +1,21 @@ { "pins" : [ { - "identity" : "ohhttpstubs", + "identity" : "flyingfox", "kind" : "remoteSourceControl", - "location" : "https://github.com/AliSoftware/OHHTTPStubs", + "location" : "https://github.com/swhitty/FlyingFox", "state" : { - "revision" : "12f19662426d0434d6c330c6974d53e2eb10ecd9", - "version" : "9.1.0" + "revision" : "ed86fc6d68ec1467aaab3e494b581e66dd7a4512", + "version" : "0.12.2" + } + }, + { + "identity" : "swift-crypto", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-crypto.git", + "state" : { + "revision" : "f0525da24dc3c6cbb2b6b338b65042bc91cbc4bb", + "version" : "3.3.0" } } ], diff --git a/Package.swift b/Package.swift index d64637fbb..6a6dbc6de 100644 --- a/Package.swift +++ b/Package.swift @@ -30,7 +30,8 @@ let package = Package( ) ], dependencies: [ - .package(url: "https://github.com/AliSoftware/OHHTTPStubs", .upToNextMajor(from: "9.1.0")) + .package(url: "https://github.com/apple/swift-crypto", .upToNextMajor(from: "3.3.0")), + .package(url: "https://github.com/swhitty/FlyingFox", exact: "0.12.2"), ], targets: [ .target( @@ -59,7 +60,8 @@ let package = Package( dependencies: [ .target(name: "wordpress-api"), .target(name: "libwordpressFFI"), - .product(name: "OHHTTPStubsSwift", package: "OHHTTPStubs") + .product(name: "Crypto", package: "swift-crypto"), + "FlyingFox", ], path: "native/swift/Tests/wordpress-api", swiftSettings: [ diff --git a/native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift b/native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift new file mode 100644 index 000000000..f9b417a1d --- /dev/null +++ b/native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift @@ -0,0 +1,136 @@ +import Foundation +import XCTest +import FlyingFox + +#if os(Linux) +import FoundationNetworking +#endif + +final class HTTPStub { + let serverURL: URL + + private let server: HTTPServer + private var stubs: [(condition: RequestFilter, response: ResponseBlock)] + + fileprivate init() async throws { + let port: UInt16 = (8000...9999).randomElement()! + let server = HTTPServer(port: port) + Task.detached { try await server.start() } + try await server.waitUntilListening() + + self.server = server + self.serverURL = URL(string: "http://localhost:\(port)")! + self.stubs = [] + + let handler = { @Sendable [weak self] (request: HTTPRequest) async throws -> HTTPResponse in + guard let self else { return HTTPResponse(statusCode: .serviceUnavailable) } + + let urlRequest = try await request.asURLRequest(serverURL: serverURL) + guard let responseBlock = self.stubs.first(where: { condition, _ in condition(urlRequest) })?.response else { + return HTTPResponse(statusCode: .notFound) + } + + return try await responseBlock(urlRequest).response(for: urlRequest) + } + await server.appendRoute("*", handler: handler) + } + + func terminate() async { + await server.stop() + } +} + +extension HTTPRequest { + + func asURLRequest(serverURL: URL) async throws -> URLRequest { + var components = try XCTUnwrap(URLComponents(url: serverURL, resolvingAgainstBaseURL: true)) + components.path = path + components.queryItems = query.map { + URLQueryItem(name: $0.name, value: $0.value) + } + let url = try XCTUnwrap(components.url) + + var request = URLRequest(url: url) + request.httpMethod = method.rawValue + for (name, value) in headers { + request.setValue(value, forHTTPHeaderField: name.rawValue) + } + request.httpBody = try await bodyData + return request + } + +} + +extension XCTestCase { + + func launchHTTPStub() async throws -> HTTPStub { + for _ in 1...5 { + do { + let stub = try await HTTPStub() + addTeardownBlock { + await stub.terminate() + } + return stub + } catch { + print("Failed to create an HTTP server: \(error)") + } + } + + // Final attempt + return try await HTTPStub() + } + +} + +struct HTTPStubsResponse { + var fileURL: URL? + var data: Data? + var statusCode: Int? + var headers: [String: String]? + + var responseTime: TimeInterval? + + func response(for request: URLRequest) async throws -> HTTPResponse { + if let responseTime { + let milliseconds: UInt64 = UInt64(1_000 * responseTime) + try await Task.sleep(nanoseconds: milliseconds * 1_000_000) + } + + let body: HTTPBodySequence + if let data { + body = .init(data: data) + } else if let fileURL = fileURL { + body = try .init(file: fileURL) + } else { + body = .init(data: Data()) + } + + let headers: [HTTPHeader: String]? = headers?.reduce(into: [:]) { result, pair in + result[HTTPHeader(rawValue: pair.key)] = pair.value + } + let code = statusCode.flatMap { HTTPStatusCode($0, phrase: "Stubbed") } ?? .ok + + return HTTPResponse(statusCode: code, headers: headers ?? [:], body: body) + } +} + +// MARK: - Creating HTTP stubs + +typealias RequestFilter = (URLRequest) -> Bool +typealias ResponseBlock = (URLRequest) -> HTTPStubsResponse + +extension HTTPStub { + + func stub(condition: @escaping RequestFilter, response: @escaping ResponseBlock) { + stubs.append((condition, response)) + } + +} + +func isPath(_ path: String) -> RequestFilter { + { $0.url?.path == path } +} + +func hasHeaderNamed(_ name: String, value: String?) -> RequestFilter { + { $0.value(forHTTPHeaderField: name) == value } +} diff --git a/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift b/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift index 5e1f938e5..4e460246d 100644 --- a/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift +++ b/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift @@ -1,48 +1,30 @@ import Foundation -import CryptoKit +import Crypto import XCTest -import OHHTTPStubs -import OHHTTPStubsSwift + +#if os(Linux) +import FoundationNetworking +#endif @testable import wordpress_api class URLSessionHelperTests: XCTestCase { var session: URLSession! + var stub: HTTPStub! - override func setUp() { - super.setUp() - session = .shared - } - - override func tearDown() { - super.tearDown() - HTTPStubs.removeAllStubs() - XCTAssertEqual(session.debugNumberOfTaskData, 0) - } + override func setUp() async throws { + try await super.setUp() - func testConnectionError() async throws { - stub(condition: isPath("/hello")) { _ in - HTTPStubsResponse(error: URLError(.serverCertificateUntrusted)) - } - - let result = await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)) - 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)") - } + session = .shared + stub = try await launchHTTPStub() } func test200() async throws { - stub(condition: isPath("/hello")) { _ in - HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) + stub.stub(condition: isPath("/hello")) { _ in + HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200) } - - let result = await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)) + let result = await session.perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!)) // The result is a successful result. This line should not throw let response = try result.get() @@ -51,28 +33,30 @@ class URLSessionHelperTests: XCTestCase { } func test500() async throws { - stub(condition: isPath("/hello")) { _ in + stub.stub(condition: isPath("/hello")) { _ in HTTPStubsResponse(data: "Internal server error".data(using: .utf8)!, statusCode: 500, headers: nil) } let result = await session - .perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)) + .perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!)) try XCTAssertEqual(result.get().statusCode, 500) } func testHeader() async throws { - stub(condition: hasHeaderNamed("X-Request", value: "Ping")) { _ in + stub.stub(condition: hasHeaderNamed("X-Request", value: "Ping")) { _ in HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: ["X-Response": "Pong"]) } - + let result = await session - .perform(request: .init(method: .get, url: "https://wordpress.org/hello", headerMap: ["X-Request": "Ping"])) + .perform(request: .init(method: .get, url: "\(stub.serverURL.absoluteString)/hello", headerMap: ["X-Request": "Ping"])) try XCTAssertEqual(result.get().statusCode, 200) try XCTAssertEqual(result.get().headerMap?["X-Response"], "Pong") } +// URLSessionTask on Linux doesn't appear to support tracking progress. See https://github.com/apple/swift-corelibs-foundation/blob/swift-5.10-RELEASE/Sources/FoundationNetworking/URLSession/URLSessionTask.swift#L42 +#if !os(Linux) func testProgressTracking() async throws { - stub(condition: isPath("/hello")) { _ in + stub.stub(condition: isPath("/hello")) { _ in HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) } @@ -80,15 +64,16 @@ class URLSessionHelperTests: XCTestCase { XCTAssertEqual(progress.completedUnitCount, 0) XCTAssertEqual(progress.fractionCompleted, 0) - let _ = await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), fulfilling: progress) + let _ = await session.perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!), fulfilling: progress) XCTAssertEqual(progress.completedUnitCount, 20) XCTAssertEqual(progress.fractionCompleted, 1) } +#endif func testCancellation() async throws { // Give a slow HTTP request that takes 0.5 second to complete - stub(condition: isPath("/hello")) { _ in - let response = HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) + stub.stub(condition: isPath("/hello")) { _ in + var response = HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) response.responseTime = 0.5 return response } @@ -100,7 +85,7 @@ class URLSessionHelperTests: XCTestCase { } // The result should be an cancellation result - let result = await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), fulfilling: progress) + let result = await session.perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!), fulfilling: progress) if case let .failure(.connection(urlError)) = result, urlError.code == .cancelled { // Do nothing } else { @@ -111,7 +96,7 @@ class URLSessionHelperTests: XCTestCase { // TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body // func testEncodingError() async { // let underlyingError = NSError(domain: "test", code: 123) -// let builder = HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) +// let builder = HTTPRequestBuilder(url: URL(string: "\(stub.serverURL.absoluteString)")!) // .method(.post) // .body(json: { throw underlyingError }) // let result = await session.perform(request: builder, errorType: TestError.self) @@ -126,12 +111,12 @@ class URLSessionHelperTests: XCTestCase { // TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body // func testMultipartForm() async throws { // var req: URLRequest? -// stub(condition: isPath("/hello")) { +// stub.stub(condition: isPath("/hello")) { // req = $0 // return HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) // } // -// let builder = HTTPRequestBuilder(url: URL(string: "https://wordpress.org/hello")!) +// let builder = HTTPRequestBuilder(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!) // .method(.post) // .body(form: [MultipartFormField(text: "value", name: "name", filename: nil)]) // @@ -162,11 +147,11 @@ class URLSessionHelperTests: XCTestCase { try? FileManager.default.removeItem(at: file) } - stub(condition: isPath("/hello")) { _ in + stub.stub(condition: isPath("/hello")) { _ in HTTPStubsResponse(fileURL: file, statusCode: 200, headers: nil) } - let response = try await session.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!)).get() + let response = try await session.perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!)).get() try XCTAssertEqual( sha256(XCTUnwrap(InputStream(url: file))), @@ -176,7 +161,7 @@ class URLSessionHelperTests: XCTestCase { // TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body // func testTempFileRemovedAfterMultipartUpload() async throws { -// stub(condition: isPath("/upload")) { _ in +// stub.stub(condition: isPath("/upload")) { _ in // HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) // } // @@ -191,7 +176,7 @@ class URLSessionHelperTests: XCTestCase { // let tempFilesBeforeUpload = existingTempFiles() // // // Perform upload HTTP request -// let builder = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org/upload")!) +// let builder = try HTTPRequestBuilder(url: URL(string: "\(stub.serverURL.absoluteString)/upload")!) // .method(.post) // .body(form: [MultipartFormField(fileAtPath: file.path, name: "file", filename: "file.txt", mimeType: "text/plain")]) // let _ = await session.perform(request: builder, errorType: TestError.self) @@ -207,7 +192,7 @@ class URLSessionHelperTests: XCTestCase { // TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body // func testTempFileRemovedAfterMultipartUploadError() async throws { -// stub(condition: isPath("/upload")) { _ in +// stub.stub(condition: isPath("/upload")) { _ in // HTTPStubsResponse(error: URLError(.networkConnectionLost)) // } // @@ -222,7 +207,7 @@ class URLSessionHelperTests: XCTestCase { // let tempFilesBeforeUpload = existingTempFiles() // // // Perform upload HTTP request -// let builder = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org/upload")!) +// let builder = try HTTPRequestBuilder(url: URL(string: "\(stub.serverURL.absoluteString)/upload")!) // .method(.post) // .body(form: [MultipartFormField(fileAtPath: file.path, name: "file", filename: "file.txt", mimeType: "text/plain")]) // let _ = await session.perform(request: builder, errorType: TestError.self) @@ -277,6 +262,10 @@ class URLSessionHelperTests: XCTestCase { } } +// MARK: - Background URLSession Tests - Only available on Apple platforms + +#if WP_SUPPORT_BACKGROUND_URL_SESSION + class BackgroundURLSessionHelperTests: URLSessionHelperTests { // swiftlint:disable weak_delegate @@ -287,7 +276,7 @@ class BackgroundURLSessionHelperTests: URLSessionHelperTests { super.setUp() delegate = TestBackgroundURLSessionDelegate() - session = URLSession(configuration: .default, delegate: delegate, delegateQueue: nil) + session = URLSession(configuration: .ephemeral, delegate: delegate, delegateQueue: nil) } override func tearDown() { @@ -314,3 +303,5 @@ private class TestBackgroundURLSessionDelegate: BackgroundURLSessionDelegate { super.urlSession(session, task: task, didCompleteWithError: error) } } + +#endif From 4874454154aad3967c0cedf63382d3ad17dfc58a Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Mar 2024 13:32:57 +1300 Subject: [PATCH 4/5] Fix swiftlint issues --- Package.swift | 5 +- .../URLSession+WordPressAPI.swift | 64 ++++++++++---- .../wordpress-api/WordPressAPIError.swift | 3 +- .../wordpress-api/TestSupport/HTTPStub.swift | 4 +- .../wordpress-api/URLSessionHelperTests.swift | 88 +++++++++++++------ 5 files changed, 119 insertions(+), 45 deletions(-) diff --git a/Package.swift b/Package.swift index 6a6dbc6de..2e61e4e55 100644 --- a/Package.swift +++ b/Package.swift @@ -13,7 +13,10 @@ let libwordpressFFI: Target = .systemLibrary( let libwordpressFFI: Target = .binaryTarget(name: "libwordpressFFI", path: "target/libwordpressFFI.xcframework") #endif -let supportBackgroundURLSession: SwiftSetting = .define("WP_SUPPORT_BACKGROUND_URL_SESSION", .when(platforms: [.macOS, .iOS, .tvOS, .watchOS])) +let supportBackgroundURLSession: SwiftSetting = .define( + "WP_SUPPORT_BACKGROUND_URL_SESSION", + .when(platforms: [.macOS, .iOS, .tvOS, .watchOS]) +) let package = Package( name: "wordpress", diff --git a/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift b/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift index 28557d56d..c07bf2b62 100644 --- a/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/URLSession+WordPressAPI.swift @@ -18,7 +18,8 @@ extension URLSession { /// - `totalUnitCount` must not be zero. /// - `completedUnitCount` must be zero. /// - It's used exclusivity for tracking the HTTP request overal progress: No children in its progress tree. - /// - `cancellationHandler` must be nil. You can call `fulfillingProgress.cancel()` to cancel the ongoing HTTP request. + /// - `cancellationHandler` must be nil. You can call `fulfillingProgress.cancel()` to cancel the ongoing HTTP + /// equest. /// /// Upon completion, the HTTP request's progress fulfills the `fulfillingProgress`. /// @@ -26,21 +27,30 @@ extension URLSession { /// /// - Parameters: /// - request: A `WpNetworkRequest` instance that represents an HTTP request to be sent. - /// - parentProgress: A `Progress` instance that will be used as the parent progress of the HTTP request's overall - /// progress. See the function documentation regarding requirements on this argument. + /// - parentProgress: A `Progress` instance that will be used as the parent progress of the HTTP request's + /// overall progress. See the function documentation regarding requirements on this argument. func perform( request: WpNetworkRequest, fulfilling parentProgress: Progress? = nil ) async -> WordPressAPIResult { #if WP_SUPPORT_BACKGROUND_URL_SESSION if configuration.identifier != nil { - assert(delegate is BackgroundURLSessionDelegate, "Unexpected `URLSession` delegate type. See the `backgroundSession(configuration:)`") + assert( + delegate is BackgroundURLSessionDelegate, + "Unexpected `URLSession` delegate type. See the `backgroundSession(configuration:)`" + ) } #endif if let parentProgress { - assert(parentProgress.completedUnitCount == 0 && parentProgress.totalUnitCount > 0, "Invalid parent progress") - assert(parentProgress.cancellationHandler == nil, "The progress instance's cancellationHandler property must be nil") + assert( + parentProgress.completedUnitCount == 0 && parentProgress.totalUnitCount > 0, + "Invalid parent progress" + ) + assert( + parentProgress.cancellationHandler == nil, + "The progress instance's cancellationHandler property must be nil" + ) } return await withCheckedContinuation { continuation in @@ -104,9 +114,16 @@ extension URLSession { } } else { if let httpResponse = response as? HTTPURLResponse { - result = .success(.init(body: data ?? Data(), statusCode: UInt16(httpResponse.statusCode), headerMap: httpResponse.httpHeaders)) + result = .success( + .init( + body: data ?? Data(), + statusCode: UInt16(httpResponse.statusCode), + headerMap: httpResponse.httpHeaders) + ) } else { - result = .failure(.unparsableResponse(response: nil, body: data, underlyingError: URLError(.badServerResponse))) + result = .failure( + .unparsableResponse(response: nil, body: data, underlyingError: URLError(.badServerResponse)) + ) } } @@ -121,7 +138,10 @@ extension URLSession { private extension URLSession { - func createDataTask(with request: URLRequest, completion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionDataTask { + func createDataTask( + with request: URLRequest, + completion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void + ) -> URLSessionDataTask { // This additional `callCompletionFromDelegate` is added to unit test `BackgroundURLSessionDelegate`. // Background `URLSession` doesn't work on unit tests, we have to create a non-background `URLSession` // which has a `BackgroundURLSessionDelegate` delegate in order to test `BackgroundURLSessionDelegate`. @@ -140,7 +160,11 @@ private extension URLSession { return task } - func createUploadTask(with request: URLRequest, body: Either, completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionUploadTask { + func createUploadTask( + with request: URLRequest, + body: Either, + completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void + ) -> URLSessionUploadTask { // This additional `callCompletionFromDelegate` is added to unit test `BackgroundURLSessionDelegate`. // Background `URLSession` doesn't work on unit tests, we have to create a non-background `URLSession` // which has a `BackgroundURLSessionDelegate` delegate in order to test `BackgroundURLSessionDelegate`. @@ -186,11 +210,18 @@ private extension URLSession { private extension URLSession { - func createDataTask(with request: URLRequest, completion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionDataTask { + func createDataTask( + with request: URLRequest, + completion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void + ) -> URLSessionDataTask { dataTask(with: request, completionHandler: completion) } - func createUploadTask(with request: URLRequest, body: Either, completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void) -> URLSessionUploadTask { + func createUploadTask( + with request: URLRequest, + body: Either, + completion originalCompletion: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void + ) -> URLSessionUploadTask { body.map( left: { uploadTask(with: request, from: $0, completionHandler: originalCompletion) @@ -217,8 +248,8 @@ extension URLSession { /// Create a background URLSession instance that can be used in the `perform(request:...)` async function. /// /// The `perform(request:...)` async function can be used in all non-background `URLSession` instances without any - /// extra work. However, there is a requirement to make the function works with with background `URLSession` instances. - /// That is the `URLSession` must have a delegate of `BackgroundURLSessionDelegate` type. + /// extra work. However, there is a requirement to make the function works with with background `URLSession` + /// instances. That is the `URLSession` must have a delegate of `BackgroundURLSessionDelegate` type. static func backgroundSession(configuration: URLSessionConfiguration) -> URLSession { assert(configuration.identifier != nil) // Pass `delegateQueue: nil` to get a serial queue, which is required to ensure thread safe access to @@ -303,9 +334,10 @@ private extension URLSession { extension WpNetworkRequest { func asURLRequest() throws -> URLRequest { - guard let url = URL(string: self.url), (url.scheme == "http" || url.scheme == "https") else { + guard let url = URL(string: self.url), url.scheme == "http" || url.scheme == "https" else { throw URLError(.badURL) } + var request = URLRequest(url: url) request.httpMethod = self.method.rawValue request.allHTTPHeaderFields = self.headerMap @@ -313,7 +345,7 @@ extension WpNetworkRequest { } var body: Either? { - // TODO: To be implemented + // To be implemented return nil } } diff --git a/native/swift/Sources/wordpress-api/WordPressAPIError.swift b/native/swift/Sources/wordpress-api/WordPressAPIError.swift index 878826604..6e36acc1d 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPIError.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPIError.swift @@ -17,7 +17,8 @@ public enum WordPressAPIError: Error { case requestEncodingFailure(underlyingError: Error) /// Error occured in the HTTP connection. case connection(URLError) - /// The API call returned an HTTP response that WordPressKit can't parse. Receiving this error could be an indicator that there is an error response that's not handled properly by WordPressKit. + /// The API call returned an HTTP response that WordPressKit can't parse. Receiving this error could be an + /// indicator that there is an error response that's not handled properly by WordPressKit. case unparsableResponse(response: HTTPURLResponse?, body: Data?, underlyingError: Error) /// Other error occured. case unknown(underlyingError: Error) diff --git a/native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift b/native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift index f9b417a1d..a3af9b379 100644 --- a/native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift +++ b/native/swift/Tests/wordpress-api/TestSupport/HTTPStub.swift @@ -26,11 +26,11 @@ final class HTTPStub { guard let self else { return HTTPResponse(statusCode: .serviceUnavailable) } let urlRequest = try await request.asURLRequest(serverURL: serverURL) - guard let responseBlock = self.stubs.first(where: { condition, _ in condition(urlRequest) })?.response else { + guard let stub = self.stubs.first(where: { condition, _ in condition(urlRequest) })?.response else { return HTTPResponse(statusCode: .notFound) } - return try await responseBlock(urlRequest).response(for: urlRequest) + return try await stub(urlRequest).response(for: urlRequest) } await server.appendRoute("*", handler: handler) } diff --git a/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift b/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift index 4e460246d..2d2a4a56c 100644 --- a/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift +++ b/native/swift/Tests/wordpress-api/URLSessionHelperTests.swift @@ -46,14 +46,23 @@ class URLSessionHelperTests: XCTestCase { stub.stub(condition: hasHeaderNamed("X-Request", value: "Ping")) { _ in HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: ["X-Response": "Pong"]) } - + let result = await session - .perform(request: .init(method: .get, url: "\(stub.serverURL.absoluteString)/hello", headerMap: ["X-Request": "Ping"])) + .perform( + request: .init( + method: .get, + url: "\(stub.serverURL.absoluteString)/hello", + headerMap: ["X-Request": "Ping"] + ) + ) try XCTAssertEqual(result.get().statusCode, 200) try XCTAssertEqual(result.get().headerMap?["X-Response"], "Pong") } -// URLSessionTask on Linux doesn't appear to support tracking progress. See https://github.com/apple/swift-corelibs-foundation/blob/swift-5.10-RELEASE/Sources/FoundationNetworking/URLSession/URLSessionTask.swift#L42 +// swiftlint:disable line_length +// URLSessionTask on Linux doesn't appear to support tracking progress. +// See https://github.com/apple/swift-corelibs-foundation/blob/swift-5.10-RELEASE/Sources/FoundationNetworking/URLSession/URLSessionTask.swift#L42 +// swiftlint:enable line_length #if !os(Linux) func testProgressTracking() async throws { stub.stub(condition: isPath("/hello")) { _ in @@ -64,7 +73,10 @@ class URLSessionHelperTests: XCTestCase { XCTAssertEqual(progress.completedUnitCount, 0) XCTAssertEqual(progress.fractionCompleted, 0) - let _ = await session.perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!), fulfilling: progress) + _ = await session.perform( + request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!), + fulfilling: progress + ) XCTAssertEqual(progress.completedUnitCount, 20) XCTAssertEqual(progress.fractionCompleted, 1) } @@ -85,7 +97,10 @@ class URLSessionHelperTests: XCTestCase { } // The result should be an cancellation result - let result = await session.perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!), fulfilling: progress) + let result = await session.perform( + request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!), + fulfilling: progress + ) if case let .failure(.connection(urlError)) = result, urlError.code == .cancelled { // Do nothing } else { @@ -93,7 +108,7 @@ class URLSessionHelperTests: XCTestCase { } } - // TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// Re-evaluate this test once WpNetworkRequest supports HTTP body // func testEncodingError() async { // let underlyingError = NSError(domain: "test", code: 123) // let builder = HTTPRequestBuilder(url: URL(string: "\(stub.serverURL.absoluteString)")!) @@ -108,7 +123,7 @@ class URLSessionHelperTests: XCTestCase { // } // } -// TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// Re-evaluate this test once WpNetworkRequest supports HTTP body // func testMultipartForm() async throws { // var req: URLRequest? // stub.stub(condition: isPath("/hello")) { @@ -137,7 +152,8 @@ class URLSessionHelperTests: XCTestCase { // // let requestBody = try XCTUnwrap(request.httpBody ?? request.httpBodyStream?.readToEnd()) // -// let expectedBody = "--\(boundary)\r\nContent-Disposition: form-data; name=\"name\"\r\n\r\nvalue\r\n--\(boundary)--\r\n" +// let expectedBody +// = "--\(boundary)\r\nContent-Disposition: form-data; name=\"name\"\r\n\r\nvalue\r\n--\(boundary)--\r\n" // XCTAssertEqual(String(data: requestBody, encoding: .utf8), expectedBody) // } @@ -151,7 +167,11 @@ class URLSessionHelperTests: XCTestCase { HTTPStubsResponse(fileURL: file, statusCode: 200, headers: nil) } - let response = try await session.perform(request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!)).get() + let response = try await session + .perform( + request: .init(url: URL(string: "\(stub.serverURL.absoluteString)/hello")!) + ) + .get() try XCTAssertEqual( sha256(XCTUnwrap(InputStream(url: file))), @@ -159,14 +179,14 @@ class URLSessionHelperTests: XCTestCase { ) } -// TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// Re-evaluate this test once WpNetworkRequest supports HTTP body // func testTempFileRemovedAfterMultipartUpload() async throws { // stub.stub(condition: isPath("/upload")) { _ in // HTTPStubsResponse(data: "success".data(using: .utf8)!, statusCode: 200, headers: nil) // } // -// // Create a large file which will be uploaded. The file size needs to be larger than the hardcoded threshold of -// // creating a temporary file for upload. +// // Create a large file which will be uploaded. The file size needs to be larger than the hardcoded threshold +// // of creating a temporary file for upload. // let file = try self.createLargeFile(megaBytes: 30) // defer { // try? FileManager.default.removeItem(at: file) @@ -178,26 +198,35 @@ class URLSessionHelperTests: XCTestCase { // // Perform upload HTTP request // let builder = try HTTPRequestBuilder(url: URL(string: "\(stub.serverURL.absoluteString)/upload")!) // .method(.post) -// .body(form: [MultipartFormField(fileAtPath: file.path, name: "file", filename: "file.txt", mimeType: "text/plain")]) +// .body( +// form: [ +// MultipartFormField( +// fileAtPath: file.path, +// name: "file", +// filename: "file.txt", +// mimeType: "text/plain" +// ) +// ] +// ) // let _ = await session.perform(request: builder, errorType: TestError.self) // // // Capture a list of files in the temp dirs, after calling the upload function. // let tempFilesAfterUpload = existingTempFiles() // -// // There should be no new files after the HTTP request returns. This assertion relies on an implementation detail -// // where the multipart form content is put into a file in temp dirs. +// // There should be no new files after the HTTP request returns. This assertion relies on an implementation +// // detail where the multipart form content is put into a file in temp dirs. // let newFiles = tempFilesAfterUpload.subtracting(tempFilesBeforeUpload) // XCTAssertEqual(newFiles.count, 0) // } -// TODO: Re-evaluate this test once WpNetworkRequest supports HTTP body +// Re-evaluate this test once WpNetworkRequest supports HTTP body // func testTempFileRemovedAfterMultipartUploadError() async throws { // stub.stub(condition: isPath("/upload")) { _ in // HTTPStubsResponse(error: URLError(.networkConnectionLost)) // } // -// // Create a large file which will be uploaded. The file size needs to be larger than the hardcoded threshold of -// // creating a temporary file for upload. +// // Create a large file which will be uploaded. The file size needs to be larger than the hardcoded threshold +// // of creating a temporary file for upload. // let file = try self.createLargeFile(megaBytes: 30) // defer { // try? FileManager.default.removeItem(at: file) @@ -209,23 +238,31 @@ class URLSessionHelperTests: XCTestCase { // // Perform upload HTTP request // let builder = try HTTPRequestBuilder(url: URL(string: "\(stub.serverURL.absoluteString)/upload")!) // .method(.post) -// .body(form: [MultipartFormField(fileAtPath: file.path, name: "file", filename: "file.txt", mimeType: "text/plain")]) +// .body( +// form: [ +// MultipartFormField( +// fileAtPath: file.path, +// name: "file", +// filename: "file.txt", +// mimeType: "text/plain") +// ] +// ) // let _ = await session.perform(request: builder, errorType: TestError.self) // // // Capture a list of files in the temp dirs, after calling the upload function. // let tempFilesAfterUpload = existingTempFiles() // -// // There should be no new files after the HTTP request returns. This assertion relies on an implementation detail -// // where the multipart form content is put into a file in temp dirs. +// // There should be no new files after the HTTP request returns. This assertion relies on an implementation +// // detail where the multipart form content is put into a file in temp dirs. // let newFiles = tempFilesAfterUpload.subtracting(tempFilesBeforeUpload) // XCTAssertEqual(newFiles.count, 0) // } private func existingTempFiles() -> Set { - let fm = FileManager.default + let fileManager = FileManager.default let enumerators = [ - fm.enumerator(atPath: NSTemporaryDirectory()), - fm.enumerator(atPath: fm.temporaryDirectory.path) + fileManager.enumerator(atPath: NSTemporaryDirectory()), + fileManager.enumerator(atPath: fileManager.temporaryDirectory.path) ].compactMap { $0 } var result: Set = [] @@ -238,7 +275,8 @@ class URLSessionHelperTests: XCTestCase { } private func createLargeFile(megaBytes: Int) throws -> URL { - let file = try FileManager.default.url(for: .documentDirectory, in: .userDomainMask, appropriateFor: nil, create: true) + let file = try FileManager.default + .url(for: .documentDirectory, in: .userDomainMask, appropriateFor: nil, create: true) .appendingPathComponent("large-file-\(UUID().uuidString).txt") try Data(repeating: 46, count: 1024 * 1000 * megaBytes).write(to: file) From 3a909475b1e792b8e0a73cd1bfe7009a3837e525 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Mar 2024 13:55:15 +1300 Subject: [PATCH 5/5] Add assertions regarding background URLSession --- .../swift/Sources/wordpress-api/WordPressAPI.swift | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/native/swift/Sources/wordpress-api/WordPressAPI.swift b/native/swift/Sources/wordpress-api/WordPressAPI.swift index 621a486b4..3517ae274 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPI.swift @@ -10,8 +10,19 @@ public struct WordPressAPI { package let helper: WpApiHelperProtocol public init(urlSession: URLSession, baseUrl: URL, authenticationStrategy: WpAuthentication) { - // TODO: We use URLSession APIs that accept completion block, which doesn't work with background URLSession. +#if WP_SUPPORT_BACKGROUND_URL_SESSION + // We use URLSession APIs that accept completion block, which doesn't work with background URLSession. // See `URLSession.backgroundSession(configuration:)` in `URLSession+WordPressAPI.swift`. + assert( + urlSession.configuration.identifier == nil || urlSession.delegate is BackgroundURLSessionDelegate, + "Background URLSession must use BackgroundURLSessionDelegate" + ) +#else + assert( + urlSession.configuration.identifier == nil, + "Background URLSession are not supported" + ) +#endif self.urlSession = urlSession self.helper = WpApiHelper(siteUrl: baseUrl.absoluteString, authentication: authenticationStrategy) }