From 23682ac747cf4d5248ac0c736d694f08992e36fa Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Thu, 13 Feb 2025 15:17:09 -0800 Subject: [PATCH 1/9] add `responseHead` to `FileDownloadDelegate.Progress` --- Sources/AsyncHTTPClient/FileDownloadDelegate.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift index b21499843..d335166c3 100644 --- a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift +++ b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift @@ -23,9 +23,10 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { public struct Progress: Sendable { public var totalBytes: Int? public var receivedBytes: Int + public var responseHead: HTTPResponseHead! } - private var progress = Progress(totalBytes: nil, receivedBytes: 0) + private var progress = Progress(totalBytes: nil, receivedBytes: 0, responseHead: nil) public typealias Response = Progress @@ -135,6 +136,8 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { ) -> EventLoopFuture { self.reportHead?(task, head) + self.progress.responseHead = head + if let totalBytesString = head.headers.first(name: "Content-Length"), let totalBytes = Int(totalBytesString) { From 6edf1e760eae56f1952c0243fe5873e7ed72f3b0 Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Thu, 13 Feb 2025 15:21:14 -0800 Subject: [PATCH 2/9] rename `responseHead` to `head` --- Sources/AsyncHTTPClient/FileDownloadDelegate.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift index d335166c3..7828901e8 100644 --- a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift +++ b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift @@ -23,10 +23,10 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { public struct Progress: Sendable { public var totalBytes: Int? public var receivedBytes: Int - public var responseHead: HTTPResponseHead! + public var head: HTTPResponseHead! } - private var progress = Progress(totalBytes: nil, receivedBytes: 0, responseHead: nil) + private var progress = Progress(totalBytes: nil, receivedBytes: 0, head: nil) public typealias Response = Progress @@ -136,7 +136,7 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { ) -> EventLoopFuture { self.reportHead?(task, head) - self.progress.responseHead = head + self.progress.head = head if let totalBytesString = head.headers.first(name: "Content-Length"), let totalBytes = Int(totalBytesString) From 82e74b93e7b6398e7e2d183949657c93480dd981 Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Fri, 14 Feb 2025 10:26:02 -0800 Subject: [PATCH 3/9] init `head` with bogus data it will never be seen by the library user with this data since `didReceiveHead(...)` is called before we report progress or finalize --- .../AsyncHTTPClient/FileDownloadDelegate.swift | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift index 7828901e8..0c614eadf 100644 --- a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift +++ b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift @@ -19,14 +19,22 @@ import NIOPosix /// Handles a streaming download to a given file path, allowing headers and progress to be reported. public final class FileDownloadDelegate: HTTPClientResponseDelegate { /// The response type for this delegate: the total count of bytes as reported by the response - /// "Content-Length" header (if available) and the count of bytes downloaded. + /// "Content-Length" header (if available), the count of bytes downloaded, and the + /// response head. public struct Progress: Sendable { public var totalBytes: Int? public var receivedBytes: Int - public var head: HTTPResponseHead! + public var head: HTTPResponseHead } - private var progress = Progress(totalBytes: nil, receivedBytes: 0, head: nil) + private var progress = Progress( + totalBytes: nil, + receivedBytes: 0, + head: .init( + version: .init(major: 0, minor: 0), + status: .init(statusCode: 0) + ) + ) public typealias Response = Progress @@ -134,10 +142,10 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { task: HTTPClient.Task, _ head: HTTPResponseHead ) -> EventLoopFuture { - self.reportHead?(task, head) - self.progress.head = head + self.reportHead?(task, head) + if let totalBytesString = head.headers.first(name: "Content-Length"), let totalBytes = Int(totalBytesString) { From 7433b555b6d88056c99aec6e4c558ab7ca72e710 Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Fri, 14 Feb 2025 12:24:53 -0800 Subject: [PATCH 4/9] add `FileDownloadDelegate.Response` `head` to existing tests --- .../HTTPClientTests.swift | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 546d1c3f4..626f0b7e6 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -729,11 +729,11 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { var request = try Request(url: self.defaultHTTPBinURLPrefix + "events/10/content-length") request.headers.add(name: "Accept", value: "text/event-stream") - let progress = - try TemporaryFileHelpers.withTemporaryFilePath { path -> FileDownloadDelegate.Progress in + let response = + try TemporaryFileHelpers.withTemporaryFilePath { path -> FileDownloadDelegate.Response in let delegate = try FileDownloadDelegate(path: path) - let progress = try self.defaultClient.execute( + let response = try self.defaultClient.execute( request: request, delegate: delegate ) @@ -741,19 +741,22 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { try XCTAssertEqual(50, TemporaryFileHelpers.fileSize(path: path)) - return progress + return response } - XCTAssertEqual(50, progress.totalBytes) - XCTAssertEqual(50, progress.receivedBytes) + XCTAssertEqual(response.head.status, .ok) + XCTAssertEqual("50", response.head.headers.first(name: "content-length")) + + XCTAssertEqual(50, response.totalBytes) + XCTAssertEqual(50, response.receivedBytes) } func testFileDownloadError() throws { var request = try Request(url: self.defaultHTTPBinURLPrefix + "not-found") request.headers.add(name: "Accept", value: "text/event-stream") - let progress = - try TemporaryFileHelpers.withTemporaryFilePath { path -> FileDownloadDelegate.Progress in + let response = + try TemporaryFileHelpers.withTemporaryFilePath { path -> FileDownloadDelegate.Response in let delegate = try FileDownloadDelegate( path: path, reportHead: { @@ -761,7 +764,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { } ) - let progress = try self.defaultClient.execute( + let response = try self.defaultClient.execute( request: request, delegate: delegate ) @@ -769,11 +772,14 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { XCTAssertFalse(TemporaryFileHelpers.fileExists(path: path)) - return progress + return response } - XCTAssertEqual(nil, progress.totalBytes) - XCTAssertEqual(0, progress.receivedBytes) + XCTAssertEqual(response.head.status, .notFound) + XCTAssertFalse(response.head.headers.contains(name: "content-length")) + + XCTAssertEqual(nil, response.totalBytes) + XCTAssertEqual(0, response.receivedBytes) } func testFileDownloadCustomError() throws { @@ -3910,11 +3916,11 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { var request = try Request(url: self.defaultHTTPBinURLPrefix + "chunked") request.headers.add(name: "Accept", value: "text/event-stream") - let progress = + let response = try TemporaryFileHelpers.withTemporaryFilePath { path -> FileDownloadDelegate.Progress in let delegate = try FileDownloadDelegate(path: path) - let progress = try self.defaultClient.execute( + let response = try self.defaultClient.execute( request: request, delegate: delegate ) @@ -3922,11 +3928,15 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { try XCTAssertEqual(50, TemporaryFileHelpers.fileSize(path: path)) - return progress + return response } - XCTAssertEqual(nil, progress.totalBytes) - XCTAssertEqual(50, progress.receivedBytes) + XCTAssertEqual(.ok, response.head.status) + XCTAssertEqual("chunked", response.head.headers.first(name: "transfer-encoding")) + XCTAssertFalse(response.head.headers.contains(name: "content-length")) + + XCTAssertEqual(nil, response.totalBytes) + XCTAssertEqual(50, response.receivedBytes) } func testCloseWhileBackpressureIsExertedIsFine() throws { From 601b2e4e81491523f945565f467fa598eb93b0ec Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Fri, 14 Feb 2025 12:27:44 -0800 Subject: [PATCH 5/9] fix XCTAssertEqual order for cohesion --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 626f0b7e6..0ffbcb920 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -744,7 +744,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { return response } - XCTAssertEqual(response.head.status, .ok) + XCTAssertEqual(.ok, response.head.status) XCTAssertEqual("50", response.head.headers.first(name: "content-length")) XCTAssertEqual(50, response.totalBytes) @@ -775,7 +775,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { return response } - XCTAssertEqual(response.head.status, .notFound) + XCTAssertEqual(.notFound, response.head.status) XCTAssertFalse(response.head.headers.contains(name: "content-length")) XCTAssertEqual(nil, response.totalBytes) From a73edf5802386b5c2a63278fa7015ebc571d7d25 Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Fri, 14 Feb 2025 12:41:31 -0800 Subject: [PATCH 6/9] FileDownloadDelegate.Progress -> Response --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 0ffbcb920..f6ff8bde0 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -3917,7 +3917,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { request.headers.add(name: "Accept", value: "text/event-stream") let response = - try TemporaryFileHelpers.withTemporaryFilePath { path -> FileDownloadDelegate.Progress in + try TemporaryFileHelpers.withTemporaryFilePath { path -> FileDownloadDelegate.Response in let delegate = try FileDownloadDelegate(path: path) let response = try self.defaultClient.execute( From 32751ce493ed994dc5eb59f5626faeeec425eccb Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Mon, 17 Feb 2025 09:59:16 -0800 Subject: [PATCH 7/9] project `_head` optional via `head` --- .../FileDownloadDelegate.swift | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift index 0c614eadf..0c92577ef 100644 --- a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift +++ b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift @@ -24,16 +24,30 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { public struct Progress: Sendable { public var totalBytes: Int? public var receivedBytes: Int - public var head: HTTPResponseHead + + public var head: HTTPResponseHead { + get { + #if DEBUG + _head! + #else + _head ?? .init(version: .init(major: 0, minor: 0), status: .init(statusCode: 0)) + #endif + } set { + _head = newValue + } + } + + fileprivate var _head: HTTPResponseHead? = nil + + internal init(totalBytes: Int? = nil, receivedBytes: Int) { + self.totalBytes = totalBytes + self.receivedBytes = receivedBytes + } } private var progress = Progress( totalBytes: nil, - receivedBytes: 0, - head: .init( - version: .init(major: 0, minor: 0), - status: .init(statusCode: 0) - ) + receivedBytes: 0 ) public typealias Response = Progress @@ -142,7 +156,7 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { task: HTTPClient.Task, _ head: HTTPResponseHead ) -> EventLoopFuture { - self.progress.head = head + self.progress._head = head self.reportHead?(task, head) From 16a67bad392c4318957df453196f114a8aeacc24 Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Mon, 17 Feb 2025 10:08:17 -0800 Subject: [PATCH 8/9] use assert for _head nil check, use self --- Sources/AsyncHTTPClient/FileDownloadDelegate.swift | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift index 0c92577ef..49c676512 100644 --- a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift +++ b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift @@ -27,13 +27,10 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { public var head: HTTPResponseHead { get { - #if DEBUG - _head! - #else - _head ?? .init(version: .init(major: 0, minor: 0), status: .init(statusCode: 0)) - #endif + assert(self._head != nil) + return self._head! } set { - _head = newValue + self._head = newValue } } From 484f55ecd1162d6995ef705ae61d0f8101718401 Mon Sep 17 00:00:00 2001 From: Greg Cotten Date: Mon, 17 Feb 2025 12:31:36 -0800 Subject: [PATCH 9/9] format error --- Sources/AsyncHTTPClient/FileDownloadDelegate.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift index 49c676512..16a5e5251 100644 --- a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift +++ b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift @@ -29,7 +29,8 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate { get { assert(self._head != nil) return self._head! - } set { + } + set { self._head = newValue } }