Skip to content

Commit 3eff1de

Browse files
fabianfettfabianfett
committed
Remove verifyRequest step
1 parent eba6df3 commit 3eff1de

File tree

3 files changed

+66
-66
lines changed

3 files changed

+66
-66
lines changed

‎Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift

Lines changed: 34 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,19 @@ import NIOHTTP1
1818
struct HTTPRequestStateMachine {
1919
fileprivate enum State {
2020
/// The initial state machine state. The only valid mutation is `start()`. The state will
21-
/// transition to `.verifyingRequest`
21+
/// transitions to:
22+
/// - `.waitForChannelToBecomeWritable`
23+
/// - `.running(.streaming, .initialized)` (if the Channel is writable and if a request body is expected)
24+
/// - `.running(.endSent, .initialized)` (if the Channel is writable and no request body is expected)
2225
case initialized
23-
/// During this state the request's soundness is checked, before sending it out on the wire.
24-
/// Valid transitions are:
25-
/// - .waitForChannelToBecomeWritable (if the Channel is not writable)
26-
/// - .running(.streaming, .initialized) (if the Channel is writable and a request body is expected)
27-
/// - .running(.endSent, .initialized) (if the Channel is writable and no request body is expected)
28-
/// - .failed (if an error was found in the request soundness check)
29-
case verifyingRequest
3026
/// Waiting for the channel to be writable. Valid transitions are:
31-
/// - .running(.streaming, .initialized) (once the Channel is writable again and if a request body is expected)
32-
/// - .running(.endSent, .initialized) (once the Channel is writable again and no request body is expected)
33-
/// - .failed (if a connection error occurred)
34-
case waitForChannelToBecomeWritable(HTTPRequestHead)
27+
/// - `.running(.streaming, .initialized)` (once the Channel is writable again and if a request body is expected)
28+
/// - `.running(.endSent, .initialized)` (once the Channel is writable again and no request body is expected)
29+
/// - `.failed` (if a connection error occurred)
30+
case waitForChannelToBecomeWritable(HTTPRequestHead, RequestFramingMetadata)
3531
/// A request is on the wire. Valid transitions are:
36-
/// - .finished
37-
/// - .failed
32+
/// - `.finished`
33+
/// - `.failed`
3834
case running(RequestState, ResponseState)
3935
/// The request has completed successfully
4036
case finished
@@ -89,8 +85,6 @@ struct HTTPRequestStateMachine {
8985
case none
9086
}
9187

92-
case verifyRequest
93-
9488
case sendRequestHead(HTTPRequestHead, startBody: Bool)
9589
case sendBodyPart(IOData)
9690
case sendRequestEnd
@@ -118,12 +112,17 @@ struct HTTPRequestStateMachine {
118112
self.idleReadTimeout = idleReadTimeout
119113
}
120114

121-
mutating func start() -> Action {
115+
mutating func startRequest(head: HTTPRequestHead, metadata: RequestFramingMetadata) -> Action {
122116
guard case .initialized = self.state else {
123117
preconditionFailure("`start()` must be called first, and exactly once. Invalid state: \(self.state)")
124118
}
125-
self.state = .verifyingRequest
126-
return .verifyRequest
119+
120+
guard self.isChannelWritable else {
121+
self.state = .waitForChannelToBecomeWritable(head, metadata)
122+
return .wait
123+
}
124+
125+
return self.startSendingRequest(head: head, metadata: metadata)
127126
}
128127

129128
mutating func writabilityChanged(writable: Bool) -> Action {
@@ -139,15 +138,14 @@ struct HTTPRequestStateMachine {
139138

140139
switch self.state {
141140
case .initialized,
142-
.verifyingRequest,
143141
.running(.streaming(_, _, producer: .producing), _),
144142
.running(.endSent, _),
145143
.finished,
146144
.failed:
147145
return .wait
148146

149-
case .waitForChannelToBecomeWritable(let head):
150-
return self.startSendingRequestHead(head)
147+
case .waitForChannelToBecomeWritable(let head, let metadata):
148+
return self.startSendingRequest(head: head, metadata: metadata)
151149

152150
case .running(.streaming(_, _, producer: .paused), .receivingBody(let head, _)) where head.status.code >= 300:
153151
// If we are receiving a response with a status of >= 300, we should not send out
@@ -172,7 +170,6 @@ struct HTTPRequestStateMachine {
172170

173171
switch self.state {
174172
case .initialized,
175-
.verifyingRequest,
176173
.waitForChannelToBecomeWritable,
177174
.running(.streaming(_, _, producer: .paused), _),
178175
.running(.endSent, _),
@@ -194,7 +191,6 @@ struct HTTPRequestStateMachine {
194191
mutating func readEventCaught() -> Action {
195192
switch self.state {
196193
case .initialized,
197-
.verifyingRequest,
198194
.waitForChannelToBecomeWritable,
199195
.running(_, .waitingForHead),
200196
.running(_, .endReceived),
@@ -220,7 +216,7 @@ struct HTTPRequestStateMachine {
220216
switch self.state {
221217
case .initialized:
222218
preconditionFailure("After the state machine has been initialized, start must be called immediately. Thus this state is unreachable")
223-
case .verifyingRequest, .waitForChannelToBecomeWritable:
219+
case .waitForChannelToBecomeWritable:
224220
// the request failed, before it was sent onto the wire.
225221
self.state = .failed(error)
226222
return .failRequest(error, .none)
@@ -232,24 +228,10 @@ struct HTTPRequestStateMachine {
232228
}
233229
}
234230

235-
mutating func requestVerified(_ head: HTTPRequestHead) -> Action {
236-
guard case .verifyingRequest = self.state else {
237-
preconditionFailure("Invalid state: \(self.state)")
238-
}
239-
240-
guard self.isChannelWritable else {
241-
self.state = .waitForChannelToBecomeWritable(head)
242-
return .wait
243-
}
244-
245-
return self.startSendingRequestHead(head)
246-
}
247-
248231
mutating func requestStreamPartReceived(_ part: IOData) -> Action {
249232
switch self.state {
250233
case .initialized,
251234
.waitForChannelToBecomeWritable,
252-
.verifyingRequest,
253235
.running(.endSent, _):
254236
preconditionFailure("We must be in the request streaming phase, if we receive further body parts. Invalid state: \(self.state)")
255237

@@ -310,7 +292,6 @@ struct HTTPRequestStateMachine {
310292
mutating func requestStreamFinished() -> Action {
311293
switch self.state {
312294
case .initialized,
313-
.verifyingRequest,
314295
.waitForChannelToBecomeWritable,
315296
.running(.endSent, _),
316297
.finished:
@@ -355,7 +336,7 @@ struct HTTPRequestStateMachine {
355336

356337
mutating func requestCancelled() -> Action {
357338
switch self.state {
358-
case .initialized, .verifyingRequest, .waitForChannelToBecomeWritable:
339+
case .initialized, .waitForChannelToBecomeWritable:
359340
let error = HTTPClientError.cancelled
360341
self.state = .failed(error)
361342
return .failRequest(error, .none)
@@ -372,7 +353,7 @@ struct HTTPRequestStateMachine {
372353

373354
mutating func channelInactive() -> Action {
374355
switch self.state {
375-
case .initialized, .verifyingRequest, .waitForChannelToBecomeWritable, .running:
356+
case .initialized, .waitForChannelToBecomeWritable, .running:
376357
let error = HTTPClientError.remoteConnectionClosed
377358
self.state = .failed(error)
378359
return .failRequest(error, .none)
@@ -388,7 +369,7 @@ struct HTTPRequestStateMachine {
388369

389370
mutating func receivedHTTPResponseHead(_ head: HTTPResponseHead) -> Action {
390371
switch self.state {
391-
case .initialized, .verifyingRequest, .waitForChannelToBecomeWritable:
372+
case .initialized, .waitForChannelToBecomeWritable:
392373
preconditionFailure("How can we receive a response head before sending a request head ourselves")
393374

394375
case .running(.streaming(let expectedBodyLength, let sentBodyBytes, producer: .paused), .waitingForHead):
@@ -431,7 +412,7 @@ struct HTTPRequestStateMachine {
431412

432413
mutating func receivedHTTPResponseBodyPart(_ body: ByteBuffer) -> Action {
433414
switch self.state {
434-
case .initialized, .verifyingRequest, .waitForChannelToBecomeWritable:
415+
case .initialized, .waitForChannelToBecomeWritable:
435416
preconditionFailure("How can we receive a response head before sending a request head ourselves. Invalid state: \(self.state)")
436417

437418
case .running(_, .waitingForHead):
@@ -455,7 +436,7 @@ struct HTTPRequestStateMachine {
455436

456437
mutating func receivedHTTPResponseEnd() -> Action {
457438
switch self.state {
458-
case .initialized, .verifyingRequest, .waitForChannelToBecomeWritable:
439+
case .initialized, .waitForChannelToBecomeWritable:
459440
preconditionFailure("How can we receive a response head before sending a request head ourselves. Invalid state: \(self.state)")
460441

461442
case .running(_, .waitingForHead):
@@ -496,7 +477,6 @@ struct HTTPRequestStateMachine {
496477
mutating func forwardMoreBodyParts() -> Action {
497478
switch self.state {
498479
case .initialized,
499-
.verifyingRequest,
500480
.running(_, .waitingForHead),
501481
.waitForChannelToBecomeWritable:
502482
preconditionFailure("The response is expected to only ask for more data after the response head was forwarded")
@@ -520,7 +500,6 @@ struct HTTPRequestStateMachine {
520500
mutating func idleReadTimeoutTriggered() -> Action {
521501
switch self.state {
522502
case .initialized,
523-
.verifyingRequest,
524503
.waitForChannelToBecomeWritable,
525504
.running(.streaming, _):
526505
preconditionFailure("We only schedule idle read timeouts after we have sent the complete request. Invalid state: \(self.state)")
@@ -538,14 +517,16 @@ struct HTTPRequestStateMachine {
538517
}
539518
}
540519

541-
private mutating func startSendingRequestHead(_ head: HTTPRequestHead) -> Action {
542-
if let value = head.headers.first(name: "content-length"), let length = Int(value), length > 0 {
543-
self.state = .running(.streaming(expectedBodyLength: length, sentBodyBytes: 0, producer: .producing), .waitingForHead)
544-
return .sendRequestHead(head, startBody: true)
545-
} else if head.headers.contains(name: "transfer-encoding") {
520+
private mutating func startSendingRequest(head: HTTPRequestHead, metadata: RequestFramingMetadata) -> Action {
521+
switch metadata.body {
522+
case .stream:
546523
self.state = .running(.streaming(expectedBodyLength: nil, sentBodyBytes: 0, producer: .producing), .waitingForHead)
547524
return .sendRequestHead(head, startBody: true)
548-
} else {
525+
case .fixedSize(let length) where length > 0:
526+
self.state = .running(.streaming(expectedBodyLength: length, sentBodyBytes: 0, producer: .producing), .waitingForHead)
527+
return .sendRequestHead(head, startBody: true)
528+
case .none, .fixedSize:
529+
// fallback if fixed size is 0
549530
self.state = .running(.endSent, .waitingForHead)
550531
return .sendRequestHead(head, startBody: false)
551532
}
@@ -557,8 +538,6 @@ extension HTTPRequestStateMachine: CustomStringConvertible {
557538
switch self.state {
558539
case .initialized:
559540
return "HTTPRequestStateMachine(.initialized, isWritable: \(self.isChannelWritable))"
560-
case .verifyingRequest:
561-
return "HTTPRequestStateMachine(.verifyingRequest, isWritable: \(self.isChannelWritable))"
562541
case .waitForChannelToBecomeWritable:
563542
return "HTTPRequestStateMachine(.waitForChannelToBecomeWritable, isWritable: \(self.isChannelWritable))"
564543
case .running(let requestState, let responseState):

‎Sources/AsyncHTTPClient/ConnectionPool/RequestFramingMetadata.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the AsyncHTTPClient open source project
4+
//
5+
// Copyright (c) 2021 Apple Inc. and the AsyncHTTPClient project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
15+
struct RequestFramingMetadata {
16+
enum Body {
17+
case none
18+
case stream
19+
case fixedSize(Int)
20+
}
21+
22+
var connectionClose: Bool
23+
var body: Body
24+
}

‎Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import XCTest
2020
class HTTPRequestStateMachineTests: XCTestCase {
2121
func testSimpleGETRequest() {
2222
var state = HTTPRequestStateMachine(isChannelWritable: true, idleReadTimeout: nil)
23-
XCTAssertEqual(state.start(), .verifyRequest)
2423
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
25-
XCTAssertEqual(state.requestVerified(requestHead), .sendRequestHead(requestHead, startBody: false))
24+
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
25+
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
2626

2727
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok)
2828
XCTAssertEqual(state.receivedHTTPResponseHead(responseHead), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
@@ -33,9 +33,9 @@ class HTTPRequestStateMachineTests: XCTestCase {
3333

3434
func testPOSTRequestWithWriterBackpressure() {
3535
var state = HTTPRequestStateMachine(isChannelWritable: true, idleReadTimeout: nil)
36-
XCTAssertEqual(state.start(), .verifyRequest)
3736
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "4")]))
38-
XCTAssertEqual(state.requestVerified(requestHead), .sendRequestHead(requestHead, startBody: true))
37+
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
38+
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true))
3939
let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0]))
4040
let part1 = IOData.byteBuffer(ByteBuffer(bytes: [1]))
4141
let part2 = IOData.byteBuffer(ByteBuffer(bytes: [2]))
@@ -66,9 +66,9 @@ class HTTPRequestStateMachineTests: XCTestCase {
6666

6767
func testPOSTContentLengthIsTooLong() {
6868
var state = HTTPRequestStateMachine(isChannelWritable: true, idleReadTimeout: nil)
69-
XCTAssertEqual(state.start(), .verifyRequest)
7069
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "4")]))
71-
XCTAssertEqual(state.requestVerified(requestHead), .sendRequestHead(requestHead, startBody: true))
70+
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
71+
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true))
7272
let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0, 1, 2, 3]))
7373
let part1 = IOData.byteBuffer(ByteBuffer(bytes: [0, 1, 2, 3]))
7474
XCTAssertEqual(state.requestStreamPartReceived(part0), .sendBodyPart(part0))
@@ -83,9 +83,9 @@ class HTTPRequestStateMachineTests: XCTestCase {
8383

8484
func testPOSTContentLengthIsTooShort() {
8585
var state = HTTPRequestStateMachine(isChannelWritable: true, idleReadTimeout: nil)
86-
XCTAssertEqual(state.start(), .verifyRequest)
8786
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "8")]))
88-
XCTAssertEqual(state.requestVerified(requestHead), .sendRequestHead(requestHead, startBody: true))
87+
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(8))
88+
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true))
8989
let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0, 1, 2, 3]))
9090
XCTAssertEqual(state.requestStreamPartReceived(part0), .sendBodyPart(part0))
9191

@@ -101,9 +101,6 @@ class HTTPRequestStateMachineTests: XCTestCase {
101101
extension HTTPRequestStateMachine.Action: Equatable {
102102
public static func == (lhs: HTTPRequestStateMachine.Action, rhs: HTTPRequestStateMachine.Action) -> Bool {
103103
switch (lhs, rhs) {
104-
case (.verifyRequest, .verifyRequest):
105-
return true
106-
107104
case (.sendRequestHead(let lhsHead, let lhsStartBody), .sendRequestHead(let rhsHead, let rhsStartBody)):
108105
return lhsHead == rhsHead && lhsStartBody == rhsStartBody
109106
case (.sendBodyPart(let lhsData), .sendBodyPart(let rhsData)):

0 commit comments

Comments
 (0)