From 4997596b9a383d34bb72c20ce1c29cffab003afa Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 20 Dec 2023 13:04:34 +1300 Subject: [PATCH 1/3] Support tracking HTTP request overall progress --- WordPressKit/HTTPClient.swift | 81 +++++++++++++++---- .../Utilities/URLSessionHelperTests.swift | 14 ++++ 2 files changed, 79 insertions(+), 16 deletions(-) diff --git a/WordPressKit/HTTPClient.swift b/WordPressKit/HTTPClient.swift index 8ba73fb3..aa24f569 100644 --- a/WordPressKit/HTTPClient.swift +++ b/WordPressKit/HTTPClient.swift @@ -9,37 +9,86 @@ struct HTTPAPIResponse { extension URLSession { + /// Send a HTTP request and return its response as a `WordPressAPIResult` instance. + /// + /// ## Progress Tracking + /// + /// 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. + /// + /// Upon completion, the HTTP request's progress fulfills the `fulfillingProgress`. + /// + /// - Parameters: + /// - builder: A `HTTPRequestBuilder` instance that represents an HTTP request to be sent. + /// - acceptableStatusCodes: HTTP status code ranges that are considered a successful response. Responses with + /// a status code outside of these ranges are returned as a `WordPressAPIResult.unacceptableStatusCode` instance. + /// - 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. + /// - errorType: The concret endpoint error type. func perform( request builder: HTTPRequestBuilder, acceptableStatusCodes: [ClosedRange] = [200...299], + fulfillingProgress parentProgress: Progress? = nil, errorType: E.Type = E.self ) async -> WordPressAPIResult, E> { + if let parentProgress { + assert(parentProgress.completedUnitCount == 0 && parentProgress.totalUnitCount > 0, "Invalid parent progress") + } + 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)) - } - } + return await withCheckedContinuation { continuation in + let task = dataTask(with: request) { data, response, error in + let result: WordPressAPIResult, E> = Self.parseResponse( + data: data, + response: response, + error: error, + acceptableStatusCodes: acceptableStatusCodes + ) - let (body, response) = result + continuation.resume(returning: result) + } + task.resume() - guard let response = response as? HTTPURLResponse else { - return .failure(.unparsableResponse(response: nil, body: body)) + if let parentProgress, parentProgress.totalUnitCount > parentProgress.completedUnitCount { + let pending = parentProgress.totalUnitCount - parentProgress.completedUnitCount + parentProgress.addChild(task.progress, withPendingUnitCount: pending) + } } + } - guard acceptableStatusCodes.contains(where: { $0 ~= response.statusCode }) else { - return .failure(.unacceptableStatusCode(response: response, body: body)) + private static func parseResponse( + data: Data?, + response: URLResponse?, + error: Error?, + acceptableStatusCodes: [ClosedRange] + ) -> WordPressAPIResult, E> { + let result: WordPressAPIResult, E> + + 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 { + if acceptableStatusCodes.contains(where: { $0 ~= httpResponse.statusCode }) { + result = .success(HTTPAPIResponse(response: httpResponse, body: data ?? Data())) + } else { + result = .failure(.unacceptableStatusCode(response: httpResponse, body: data ?? Data())) + } + } else { + result = .failure(.unparsableResponse(response: nil, body: data)) + } } - return .success(.init(response: response, body: body)) + return result } } diff --git a/WordPressKitTests/Utilities/URLSessionHelperTests.swift b/WordPressKitTests/Utilities/URLSessionHelperTests.swift index b8dd7e8f..5839cbb7 100644 --- a/WordPressKitTests/Utilities/URLSessionHelperTests.swift +++ b/WordPressKitTests/Utilities/URLSessionHelperTests.swift @@ -106,6 +106,20 @@ class URLSessionHelperTests: XCTestCase { try XCTAssertEqual(result.get().title, "Hello Post") } + + 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 URLSession.shared.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), fulfillingProgress: progress, errorType: TestError.self) + XCTAssertEqual(progress.completedUnitCount, 20) + XCTAssertEqual(progress.fractionCompleted, 1) + } } private enum TestError: LocalizedError, Equatable { From 21d9c08111fb6355c94f3d03d82a49d6d050d558 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 20 Dec 2023 13:38:48 +1300 Subject: [PATCH 2/3] Add cancellation support --- WordPressKit/HTTPClient.swift | 7 +++++- .../Utilities/URLSessionHelperTests.swift | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/WordPressKit/HTTPClient.swift b/WordPressKit/HTTPClient.swift index aa24f569..2a871815 100644 --- a/WordPressKit/HTTPClient.swift +++ b/WordPressKit/HTTPClient.swift @@ -11,13 +11,14 @@ extension URLSession { /// Send a HTTP request and return its response as a `WordPressAPIResult` instance. /// - /// ## Progress Tracking + /// ## 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`. /// @@ -58,6 +59,10 @@ extension URLSession { 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() + } } } } diff --git a/WordPressKitTests/Utilities/URLSessionHelperTests.swift b/WordPressKitTests/Utilities/URLSessionHelperTests.swift index 5839cbb7..18ebd800 100644 --- a/WordPressKitTests/Utilities/URLSessionHelperTests.swift +++ b/WordPressKitTests/Utilities/URLSessionHelperTests.swift @@ -120,6 +120,29 @@ class URLSessionHelperTests: XCTestCase { 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 URLSession.shared.perform(request: .init(url: URL(string: "https://wordpress.org/hello")!), fulfillingProgress: progress, errorType: TestError.self) + if case let .failure(.connection(urlError)) = result, urlError.code == .cancelled { + // Do nothing + } else { + XCTFail("Unexpected result: \(result)") + } + } } private enum TestError: LocalizedError, Equatable { From 46de399c62fa550e5889ba547d35b5279af8cfcd Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 20 Dec 2023 13:56:05 +1300 Subject: [PATCH 3/3] Add an assertion --- WordPressKit/HTTPClient.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPressKit/HTTPClient.swift b/WordPressKit/HTTPClient.swift index 2a871815..799eebb2 100644 --- a/WordPressKit/HTTPClient.swift +++ b/WordPressKit/HTTPClient.swift @@ -35,8 +35,9 @@ extension URLSession { fulfillingProgress parentProgress: Progress? = nil, errorType: E.Type = E.self ) async -> WordPressAPIResult, E> { - if let parentProgress { + 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") } guard let request = try? builder.build() else {