Skip to content

Commit 3c59b1f

Browse files
committed
More tests
1 parent 107366f commit 3c59b1f

File tree

6 files changed

+302
-1
lines changed

6 files changed

+302
-1
lines changed

Diff for: Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1Connection.swift

+15-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,13 @@ final class HTTP1Connection {
8787
}
8888

8989
func execute(request: HTTPExecutableRequest) {
90-
self.channel.write(request, promise: nil)
90+
if self.channel.eventLoop.inEventLoop {
91+
self.execute0(request: request)
92+
} else {
93+
self.channel.eventLoop.execute {
94+
self.execute0(request: request)
95+
}
96+
}
9197
}
9298

9399
func cancel() {
@@ -101,4 +107,12 @@ final class HTTP1Connection {
101107
func taskCompleted() {
102108
self.delegate.http1ConnectionReleased(self)
103109
}
110+
111+
private func execute0(request: HTTPExecutableRequest) {
112+
guard self.channel.isActive else {
113+
return request.fail(ChannelError.ioOnClosedChannel)
114+
}
115+
116+
self.channel.write(request, promise: nil)
117+
}
104118
}

Diff for: Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests+XCTest.swift

+4
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import XCTest
2525
extension HTTP1ClientChannelHandlerTests {
2626
static var allTests: [(String, (HTTP1ClientChannelHandlerTests) -> () throws -> Void)] {
2727
return [
28+
("testResponseBackpressure", testResponseBackpressure),
29+
("testWriteBackpressure", testWriteBackpressure),
30+
("testClientHandlerCancelsRequestIfWeWantToShutdown", testClientHandlerCancelsRequestIfWeWantToShutdown),
31+
("testIdleReadTimeout", testIdleReadTimeout),
2832
]
2933
}
3034
}

Diff for: Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift

+180
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,186 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
104104
XCTAssertNoThrow(try requestBag.task.futureResult.wait())
105105
}
106106

107+
func testWriteBackpressure() {
108+
class TestWriter {
109+
let eventLoop: EventLoop
110+
111+
let parts: Int
112+
113+
var finishFuture: EventLoopFuture<Void> { self.finishPromise.futureResult }
114+
private let finishPromise: EventLoopPromise<Void>
115+
private(set) var written: Int = 0
116+
117+
private var channelIsWritable: Bool = false
118+
119+
init(eventLoop: EventLoop, parts: Int) {
120+
self.eventLoop = eventLoop
121+
self.parts = parts
122+
123+
self.finishPromise = eventLoop.makePromise(of: Void.self)
124+
}
125+
126+
func start(writer: HTTPClient.Body.StreamWriter) -> EventLoopFuture<Void> {
127+
func recursive() {
128+
XCTAssert(self.eventLoop.inEventLoop)
129+
XCTAssert(self.channelIsWritable)
130+
if self.written == self.parts {
131+
self.finishPromise.succeed(())
132+
} else {
133+
self.eventLoop.execute {
134+
let future = writer.write(.byteBuffer(.init(bytes: [0, 1])))
135+
self.written += 1
136+
future.whenComplete { result in
137+
switch result {
138+
case .success:
139+
recursive()
140+
case .failure(let error):
141+
XCTFail("Unexpected error: \(error)")
142+
}
143+
}
144+
}
145+
}
146+
}
147+
148+
recursive()
149+
150+
return self.finishFuture
151+
}
152+
153+
func writabilityChanged(_ newValue: Bool) {
154+
self.channelIsWritable = newValue
155+
}
156+
}
157+
158+
let embedded = EmbeddedChannel()
159+
let testWriter = TestWriter(eventLoop: embedded.eventLoop, parts: 50)
160+
var maybeTestUtils: HTTP1TestTools?
161+
XCTAssertNoThrow(maybeTestUtils = try embedded.setupHTTP1Connection())
162+
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
163+
164+
var maybeRequest: HTTPClient.Request?
165+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 100) { writer in
166+
testWriter.start(writer: writer)
167+
}))
168+
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
169+
170+
let delegate = ResponseAccumulator(request: request)
171+
var maybeRequestBag: RequestBag<ResponseAccumulator>?
172+
XCTAssertNoThrow(maybeRequestBag = try RequestBag(
173+
request: request,
174+
eventLoopPreference: .delegate(on: embedded.eventLoop),
175+
task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
176+
redirectHandler: nil,
177+
connectionDeadline: .now() + .seconds(30),
178+
idleReadTimeout: .milliseconds(200),
179+
delegate: delegate
180+
))
181+
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
182+
183+
// the handler only writes once the channel is writable
184+
embedded.isWritable = false
185+
testWriter.writabilityChanged(false)
186+
embedded.pipeline.fireChannelWritabilityChanged()
187+
testUtils.connection.execute(request: requestBag)
188+
189+
XCTAssertEqual(try embedded.readOutbound(as: HTTPClientRequestPart.self), .none)
190+
191+
embedded.isWritable = true
192+
testWriter.writabilityChanged(true)
193+
embedded.pipeline.fireChannelWritabilityChanged()
194+
195+
XCTAssertNoThrow(try embedded.receiveHeadAndVerify {
196+
XCTAssertEqual($0.method, .POST)
197+
XCTAssertEqual($0.uri, "/")
198+
XCTAssertEqual($0.headers.first(name: "host"), "localhost")
199+
XCTAssertEqual($0.headers.first(name: "content-length"), "100")
200+
})
201+
202+
// the next body write will be executed once we tick the el. before we make the channel
203+
// unwritable
204+
205+
for index in 0..<50 {
206+
embedded.isWritable = false
207+
testWriter.writabilityChanged(false)
208+
embedded.pipeline.fireChannelWritabilityChanged()
209+
210+
XCTAssertEqual(testWriter.written, index)
211+
212+
embedded.embeddedEventLoop.run()
213+
214+
XCTAssertNoThrow(try embedded.receiveBodyAndVerify {
215+
XCTAssertEqual($0.readableBytes, 2)
216+
})
217+
218+
XCTAssertEqual(testWriter.written, index + 1)
219+
220+
embedded.isWritable = true
221+
testWriter.writabilityChanged(true)
222+
embedded.pipeline.fireChannelWritabilityChanged()
223+
}
224+
225+
embedded.embeddedEventLoop.run()
226+
XCTAssertNoThrow(try embedded.receiveEnd())
227+
228+
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok)
229+
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.head(responseHead)))
230+
embedded.read()
231+
232+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionClosed, 0)
233+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionReleased, 0)
234+
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
235+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionClosed, 0)
236+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionReleased, 1)
237+
238+
XCTAssertNoThrow(try requestBag.task.futureResult.wait())
239+
}
240+
241+
func testClientHandlerCancelsRequestIfWeWantToShutdown() {
242+
let embedded = EmbeddedChannel()
243+
var maybeTestUtils: HTTP1TestTools?
244+
XCTAssertNoThrow(maybeTestUtils = try embedded.setupHTTP1Connection())
245+
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
246+
247+
var maybeRequest: HTTPClient.Request?
248+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/"))
249+
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
250+
251+
let delegate = ResponseAccumulator(request: request)
252+
var maybeRequestBag: RequestBag<ResponseAccumulator>?
253+
XCTAssertNoThrow(maybeRequestBag = try RequestBag(
254+
request: request,
255+
eventLoopPreference: .delegate(on: embedded.eventLoop),
256+
task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
257+
redirectHandler: nil,
258+
connectionDeadline: .now() + .seconds(30),
259+
idleReadTimeout: .milliseconds(200),
260+
delegate: delegate
261+
))
262+
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
263+
264+
testUtils.connection.execute(request: requestBag)
265+
266+
XCTAssertNoThrow(try embedded.receiveHeadAndVerify {
267+
XCTAssertEqual($0.method, .GET)
268+
XCTAssertEqual($0.uri, "/")
269+
XCTAssertEqual($0.headers.first(name: "host"), "localhost")
270+
})
271+
XCTAssertNoThrow(try embedded.receiveEnd())
272+
273+
XCTAssertTrue(embedded.isActive)
274+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionClosed, 0)
275+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionReleased, 0)
276+
testUtils.connection.cancel()
277+
XCTAssertFalse(embedded.isActive)
278+
embedded.embeddedEventLoop.run()
279+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionClosed, 1)
280+
XCTAssertEqual(testUtils.connectionDelegate.hitConnectionReleased, 0)
281+
282+
XCTAssertThrowsError(try requestBag.task.futureResult.wait()) {
283+
XCTAssertEqual($0 as? HTTPClientError, .cancelled)
284+
}
285+
}
286+
107287
func testIdleReadTimeout() {
108288
let embedded = EmbeddedChannel()
109289
var maybeTestUtils: HTTP1TestTools?

Diff for: Tests/AsyncHTTPClientTests/HTTP1ConnectionTests+XCTest.swift

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ extension HTTP1ConnectionTests {
2929
("testCreateNewConnectionWithoutDecompression", testCreateNewConnectionWithoutDecompression),
3030
("testCreateNewConnectionFailureClosedIO", testCreateNewConnectionFailureClosedIO),
3131
("testGETRequest", testGETRequest),
32+
("testConnectionClosesOnCloseHeader", testConnectionClosesOnCloseHeader),
3233
]
3334
}
3435
}

Diff for: Tests/AsyncHTTPClientTests/HTTP1ConnectionTests.swift

+101
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,72 @@ class HTTP1ConnectionTests: XCTestCase {
185185
// connection is closed
186186
XCTAssertNoThrow(try XCTUnwrap(delegate.closePromise).futureResult.wait())
187187
}
188+
189+
func testConnectionClosesOnCloseHeader() {
190+
let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
191+
let eventLoop = eventLoopGroup.next()
192+
defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) }
193+
194+
let closeOnRequest = (30...100).randomElement()!
195+
let httpBin = HTTPBin(handlerFactory: { _ in SuddenlySendsCloseHeaderChannel(closeOnRequest: closeOnRequest) })
196+
197+
var maybeChannel: Channel?
198+
199+
XCTAssertNoThrow(maybeChannel = try ClientBootstrap(group: eventLoop).connect(host: "localhost", port: httpBin.port).wait())
200+
let connectionDelegate = MockConnectionDelegate()
201+
let logger = Logger(label: "test")
202+
var maybeConnection: HTTP1Connection?
203+
XCTAssertNoThrow(maybeConnection = try eventLoop.submit { try HTTP1Connection(
204+
channel: XCTUnwrap(maybeChannel),
205+
connectionID: 0,
206+
configuration: .init(),
207+
delegate: connectionDelegate,
208+
logger: logger
209+
) }.wait())
210+
guard let connection = maybeConnection else { return XCTFail("Expected to have a connection here") }
211+
212+
var counter = 0
213+
while true {
214+
counter += 1
215+
216+
var maybeRequest: HTTPClient.Request?
217+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/"))
218+
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
219+
220+
let delegate = ResponseAccumulator(request: request)
221+
var maybeRequestBag: RequestBag<ResponseAccumulator>?
222+
XCTAssertNoThrow(maybeRequestBag = try RequestBag(
223+
request: request,
224+
eventLoopPreference: .delegate(on: eventLoopGroup.next()),
225+
task: .init(eventLoop: eventLoopGroup.next(), logger: logger),
226+
redirectHandler: nil,
227+
connectionDeadline: .now() + .seconds(30),
228+
idleReadTimeout: nil,
229+
delegate: delegate
230+
))
231+
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
232+
233+
connection.execute(request: requestBag)
234+
235+
var response: HTTPClient.Response?
236+
if counter <= closeOnRequest {
237+
XCTAssertNoThrow(response = try requestBag.task.futureResult.wait())
238+
XCTAssertEqual(response?.status, .ok)
239+
240+
if response?.headers.first(name: "connection") == "close" {
241+
XCTAssertEqual(closeOnRequest, counter)
242+
XCTAssertEqual(maybeChannel?.isActive, false)
243+
}
244+
} else {
245+
// io on close channel leads to error
246+
XCTAssertThrowsError(try requestBag.task.futureResult.wait()) {
247+
XCTAssertEqual($0 as? ChannelError, .ioOnClosedChannel)
248+
}
249+
250+
break // the loop
251+
}
252+
}
253+
}
188254
}
189255

190256
class MockHTTP1ConnectionDelegate: HTTP1ConnectionDelegate {
@@ -199,3 +265,38 @@ class MockHTTP1ConnectionDelegate: HTTP1ConnectionDelegate {
199265
self.closePromise?.succeed(())
200266
}
201267
}
268+
269+
class SuddenlySendsCloseHeaderChannel: ChannelInboundHandler {
270+
typealias InboundIn = HTTPServerRequestPart
271+
typealias OutboundOut = HTTPServerResponsePart
272+
273+
var counter = 1
274+
let closeOnRequest: Int
275+
276+
init(closeOnRequest: Int) {
277+
self.closeOnRequest = closeOnRequest
278+
}
279+
280+
func channelRead(context: ChannelHandlerContext, data: NIOAny) {
281+
switch self.unwrapInboundIn(data) {
282+
case .head(let head):
283+
XCTAssertLessThanOrEqual(self.counter, self.closeOnRequest)
284+
XCTAssertTrue(head.headers.contains(name: "host"))
285+
XCTAssertEqual(head.method, .GET)
286+
case .body:
287+
break
288+
case .end:
289+
if self.closeOnRequest == self.counter {
290+
context.write(self.wrapOutboundOut(.head(.init(version: .http1_1, status: .ok, headers: ["connection": "close"]))), promise: nil)
291+
context.write(self.wrapOutboundOut(.end(nil)), promise: nil)
292+
context.flush()
293+
self.counter += 1
294+
} else {
295+
context.write(self.wrapOutboundOut(.head(.init(version: .http1_1, status: .ok))), promise: nil)
296+
context.write(self.wrapOutboundOut(.end(nil)), promise: nil)
297+
context.flush()
298+
self.counter += 1
299+
}
300+
}
301+
}
302+
}

Diff for: Tests/AsyncHTTPClientTests/RequestBagTests+XCTest.swift

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ extension RequestBagTests {
3030
("testCancelFailsTaskBeforeRequestIsSent", testCancelFailsTaskBeforeRequestIsSent),
3131
("testCancelFailsTaskAfterRequestIsSent", testCancelFailsTaskAfterRequestIsSent),
3232
("testCancelFailsTaskWhenTaskIsQueued", testCancelFailsTaskWhenTaskIsQueued),
33+
("testFailsTaskWhenTaskIsWaitingForMoreFromServer", testFailsTaskWhenTaskIsWaitingForMoreFromServer),
3334
("testHTTPUploadIsCancelledEvenThoughRequestSucceeds", testHTTPUploadIsCancelledEvenThoughRequestSucceeds),
3435
]
3536
}

0 commit comments

Comments
 (0)