Skip to content

Commit da621ce

Browse files
authored
Add didVisitURL delegate method (#816)
Trying to pave the way for closing #790 with some direction from @Lukasa. I have no idea where is best to insert this new delegate method. I'm currently doing it first thing in `receiveResponseHead0`, and not using an EventLoopFuture for back pressure management. The state machine seems pretty fragile and I don't want to leave too much of an imprint. Trying to be a part of an EventLoopFuture chain seems really complicated and would really leave a mark on the codebase, so I'm wondering if it's possible to just warn the user "do not block"? Anyway, just a jumping-off point and happy to take direction!
1 parent ad262cc commit da621ce

File tree

3 files changed

+51
-19
lines changed

3 files changed

+51
-19
lines changed

Sources/AsyncHTTPClient/HTTPHandler.swift

+15-1
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,16 @@ public protocol HTTPClientResponseDelegate: AnyObject {
668668
/// - task: Current request context.
669669
func didSendRequest(task: HTTPClient.Task<Response>)
670670

671-
/// Called when response head is received. Will be called once.
671+
/// Called each time a response head is received (including redirects), and always called before ``HTTPClientResponseDelegate/didReceiveHead(task:_:)-9r4xd``.
672+
/// You can use this method to keep an entire history of the request/response chain.
673+
///
674+
/// - parameters:
675+
/// - task: Current request context.
676+
/// - request: The request that was sent.
677+
/// - head: Received response head.
678+
func didVisitURL(task: HTTPClient.Task<Response>, _ request: HTTPClient.Request, _ head: HTTPResponseHead)
679+
680+
/// Called when the final response head is received (after redirects).
672681
/// You must return an `EventLoopFuture<Void>` that you complete when you have finished processing the body part.
673682
/// You can create an already succeeded future by calling `task.eventLoop.makeSucceededFuture(())`.
674683
///
@@ -734,6 +743,11 @@ extension HTTPClientResponseDelegate {
734743
/// By default, this does nothing.
735744
public func didSendRequest(task: HTTPClient.Task<Response>) {}
736745

746+
/// Default implementation of ``HTTPClientResponseDelegate/didVisitURL(task:_:_:)-2el9y``.
747+
///
748+
/// By default, this does nothing.
749+
public func didVisitURL(task: HTTPClient.Task<Response>, _: HTTPClient.Request, _: HTTPResponseHead) {}
750+
737751
/// Default implementation of ``HTTPClientResponseDelegate/didReceiveHead(task:_:)-9r4xd``.
738752
///
739753
/// By default, this does nothing.

Sources/AsyncHTTPClient/RequestBag.swift

+2
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate> {
228228
private func receiveResponseHead0(_ head: HTTPResponseHead) {
229229
self.task.eventLoop.assertInEventLoop()
230230

231+
self.delegate.didVisitURL(task: self.task, self.request, head)
232+
231233
// runs most likely on channel eventLoop
232234
switch self.state.receiveResponseHead(head) {
233235
case .none:

Tests/AsyncHTTPClientTests/RequestBagTests.swift

+34-18
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ final class RequestBagTests: XCTestCase {
127127
XCTAssertNoThrow(try executor.receiveEndOfStream())
128128
XCTAssertEqual(receivedBytes, bytesToSent, "We have sent all request bytes...")
129129

130+
XCTAssertTrue(delegate.history.isEmpty)
130131
XCTAssertNil(delegate.receivedHead, "Expected not to have a response head, before `receiveResponseHead`")
131132
let responseHead = HTTPResponseHead(
132133
version: .http1_1,
@@ -140,6 +141,10 @@ final class RequestBagTests: XCTestCase {
140141
XCTAssertEqual(responseHead, delegate.receivedHead)
141142
XCTAssertNoThrow(try XCTUnwrap(delegate.backpressurePromise).succeed(()))
142143
XCTAssertTrue(executor.signalledDemandForResponseBody)
144+
145+
XCTAssertEqual(delegate.history.map(\.request.url), [request.url])
146+
XCTAssertEqual(delegate.history.map(\.response), [responseHead])
147+
143148
executor.resetResponseStreamDemandSignal()
144149

145150
// we will receive 20 chunks with each 10 byteBuffers and 32 bytes
@@ -747,13 +752,15 @@ final class RequestBagTests: XCTestCase {
747752
let executor = MockRequestExecutor(eventLoop: embeddedEventLoop)
748753
executor.runRequest(bag)
749754
XCTAssertFalse(executor.signalledDemandForResponseBody)
750-
bag.receiveResponseHead(
751-
.init(
752-
version: .http1_1,
753-
status: .permanentRedirect,
754-
headers: ["content-length": "\(3 * 1024)", "location": "https://swift.org/sswg"]
755-
)
755+
XCTAssertTrue(delegate.history.isEmpty)
756+
let responseHead = HTTPResponseHead(
757+
version: .http1_1,
758+
status: .permanentRedirect,
759+
headers: ["content-length": "\(3 * 1024)", "location": "https://swift.org/sswg"]
756760
)
761+
bag.receiveResponseHead(responseHead)
762+
XCTAssertEqual(delegate.history.map(\.request.url), [request.url])
763+
XCTAssertEqual(delegate.history.map(\.response), [responseHead])
757764
XCTAssertNil(delegate.backpressurePromise)
758765
XCTAssertTrue(executor.signalledDemandForResponseBody)
759766
executor.resetResponseStreamDemandSignal()
@@ -833,13 +840,15 @@ final class RequestBagTests: XCTestCase {
833840
let executor = MockRequestExecutor(eventLoop: embeddedEventLoop)
834841
executor.runRequest(bag)
835842
XCTAssertFalse(executor.signalledDemandForResponseBody)
836-
bag.receiveResponseHead(
837-
.init(
838-
version: .http1_1,
839-
status: .permanentRedirect,
840-
headers: ["content-length": "\(4 * 1024)", "location": "https://swift.org/sswg"]
841-
)
843+
XCTAssertTrue(delegate.history.isEmpty)
844+
let responseHead = HTTPResponseHead(
845+
version: .http1_1,
846+
status: .permanentRedirect,
847+
headers: ["content-length": "\(4 * 1024)", "location": "https://swift.org/sswg"]
842848
)
849+
bag.receiveResponseHead(responseHead)
850+
XCTAssertEqual(delegate.history.map(\.request.url), [request.url])
851+
XCTAssertEqual(delegate.history.map(\.response), [responseHead])
843852
XCTAssertNil(delegate.backpressurePromise)
844853
XCTAssertFalse(executor.signalledDemandForResponseBody)
845854
XCTAssertTrue(executor.isCancelled)
@@ -893,13 +902,15 @@ final class RequestBagTests: XCTestCase {
893902
let executor = MockRequestExecutor(eventLoop: embeddedEventLoop)
894903
executor.runRequest(bag)
895904
XCTAssertFalse(executor.signalledDemandForResponseBody)
896-
bag.receiveResponseHead(
897-
.init(
898-
version: .http1_1,
899-
status: .permanentRedirect,
900-
headers: ["content-length": "\(3 * 1024)", "location": "https://swift.org/sswg"]
901-
)
905+
XCTAssertTrue(delegate.history.isEmpty)
906+
let responseHead = HTTPResponseHead(
907+
version: .http1_1,
908+
status: .permanentRedirect,
909+
headers: ["content-length": "\(3 * 1024)", "location": "https://swift.org/sswg"]
902910
)
911+
bag.receiveResponseHead(responseHead)
912+
XCTAssertEqual(delegate.history.map(\.request.url), [request.url])
913+
XCTAssertEqual(delegate.history.map(\.response), [responseHead])
903914
XCTAssertNil(delegate.backpressurePromise)
904915
XCTAssertTrue(executor.signalledDemandForResponseBody)
905916
executor.resetResponseStreamDemandSignal()
@@ -1001,6 +1012,7 @@ class UploadCountingDelegate: HTTPClientResponseDelegate {
10011012
private(set) var hitDidReceiveBodyPart = 0
10021013
private(set) var hitDidReceiveError = 0
10031014

1015+
private(set) var history: [(request: HTTPClient.Request, response: HTTPResponseHead)] = []
10041016
private(set) var receivedHead: HTTPResponseHead?
10051017
private(set) var lastBodyPart: ByteBuffer?
10061018
private(set) var backpressurePromise: EventLoopPromise<Void>?
@@ -1022,6 +1034,10 @@ class UploadCountingDelegate: HTTPClientResponseDelegate {
10221034
self.hitDidSendRequest += 1
10231035
}
10241036

1037+
func didVisitURL(task: HTTPClient.Task<Void>, _ request: HTTPClient.Request, _ head: HTTPResponseHead) {
1038+
self.history.append((request, head))
1039+
}
1040+
10251041
func didReceiveHead(task: HTTPClient.Task<Void>, _ head: HTTPResponseHead) -> EventLoopFuture<Void> {
10261042
self.receivedHead = head
10271043
return self.createBackpressurePromise()

0 commit comments

Comments
 (0)