Skip to content

Commit 9fc3530

Browse files
fabianfettfabianfett
committed
Code review
1 parent 0890c4b commit 9fc3530

6 files changed

+19
-45
lines changed

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ extension HTTPConnectionPool {
375375
self.connections[index].lease()
376376
}
377377

378-
mutating func parkConnection(at index: Int) -> (Connection.ID, EventLoop) {
378+
func parkConnection(at index: Int) -> (Connection.ID, EventLoop) {
379379
precondition(self.connections[index].isIdle)
380380
return (self.connections[index].connectionID, self.connections[index].eventLoop)
381381
}

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift

+6-8
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,9 @@ extension HTTPConnectionPool {
7272
}
7373

7474
// No matter what we do now, the request will need to wait!
75-
let requestID = self.queue.push(request)
75+
self.queue.push(request)
7676
let requestAction: StateMachine.RequestAction = .scheduleRequestTimeout(
77-
request.connectionDeadline,
78-
for: requestID,
77+
for: request,
7978
on: eventLoop
8079
)
8180

@@ -110,10 +109,9 @@ extension HTTPConnectionPool {
110109
}
111110

112111
// No matter what we do now, the request will need to wait!
113-
let requestID = self.queue.push(request)
112+
self.queue.push(request)
114113
let requestAction: StateMachine.RequestAction = .scheduleRequestTimeout(
115-
request.connectionDeadline,
116-
for: requestID,
114+
for: request,
117115
on: eventLoop
118116
)
119117

@@ -423,7 +421,7 @@ extension HTTPConnectionPool {
423421
return .none
424422
}
425423

426-
private mutating func calculateBackoff(for attempt: Int) -> TimeAmount {
424+
private func calculateBackoff(for attempt: Int) -> TimeAmount {
427425
// Our backoff formula is: 100ms * 1.25^attempt that is capped of at 1minute
428426
// This means for:
429427
// - 1 failed attempt : 100ms
@@ -435,7 +433,7 @@ extension HTTPConnectionPool {
435433
// - 29 failed attempts: ~60s (max out)
436434

437435
let start = Double(TimeAmount.milliseconds(100).nanoseconds)
438-
let backoffNanoseconds = Int64(start * pow(1.25, Double(self.failedConsecutiveConnectionAttempts)))
436+
let backoffNanoseconds = Int64(start * pow(1.25, Double(attempt)))
439437

440438
let backoff: TimeAmount = min(.nanoseconds(backoffNanoseconds), .seconds(60))
441439

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+StateMachine.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ extension HTTPConnectionPool {
5454
case failRequest(Request, Error, cancelTimeout: Bool)
5555
case failRequestsAndCancelTimeouts([Request], Error)
5656

57-
case scheduleRequestTimeout(NIODeadline?, for: Request.ID, on: EventLoop)
57+
case scheduleRequestTimeout(for: Request, on: EventLoop)
5858
case cancelRequestTimeout(Request.ID)
5959

6060
case none

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests.swift

+8-31
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,7 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
4040
guard case .createConnection(let connectionID, let connectionEL) = action.connection else {
4141
return XCTFail("Unexpected connection action")
4242
}
43-
guard case .scheduleRequestTimeout(_, for: request.id, on: let queueEL) = action.request else {
44-
return XCTFail("Unexpected request action")
45-
}
46-
XCTAssert(queueEL === mockRequest.eventLoop)
43+
XCTAssertEqual(.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop), action.request)
4744
XCTAssert(connectionEL === mockRequest.eventLoop)
4845

4946
XCTAssertNoThrow(try connections.createConnection(connectionID, on: connectionEL))
@@ -59,10 +56,7 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
5956
guard case .none = action.connection else {
6057
return XCTFail("Unexpected connection action")
6158
}
62-
guard case .scheduleRequestTimeout(_, for: request.id, on: let queueEL) = action.request else {
63-
return XCTFail("Unexpected request action")
64-
}
65-
XCTAssert(queueEL === mockRequest.eventLoop)
59+
XCTAssertEqual(.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop), action.request)
6660
XCTAssertNoThrow(try queuer.queue(mockRequest, id: request.id))
6761
}
6862

@@ -122,10 +116,7 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
122116
let request = HTTPConnectionPool.Request(mockRequest)
123117

124118
let action = state.executeRequest(request)
125-
guard case .scheduleRequestTimeout(_, request.id, on: let returnedEL) = action.request else {
126-
return XCTFail("Unexpected request action: \(action.request)")
127-
}
128-
XCTAssert(returnedEL === mockRequest.eventLoop) // XCTAssertIdentical not available on Linux
119+
XCTAssertEqual(.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop), action.request)
129120

130121
// 1. connection attempt
131122
guard case .createConnection(let connectionID, on: let connectionEL) = action.connection else {
@@ -183,10 +174,7 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
183174
let request = HTTPConnectionPool.Request(mockRequest)
184175

185176
let executeAction = state.executeRequest(request)
186-
guard case .scheduleRequestTimeout(_, request.id, on: let returnedEL) = executeAction.request else {
187-
return XCTFail("Unexpected request action: \(executeAction.request)")
188-
}
189-
XCTAssert(returnedEL === mockRequest.eventLoop) // XCTAssertIdentical not available on Linux
177+
XCTAssertEqual(.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop), executeAction.request)
190178

191179
// 1. connection attempt
192180
guard case .createConnection(let connectionID, on: let connectionEL) = executeAction.connection else {
@@ -223,10 +211,7 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
223211
let request = HTTPConnectionPool.Request(mockRequest)
224212

225213
let executeAction = state.executeRequest(request)
226-
guard case .scheduleRequestTimeout(_, request.id, on: let returnedEL) = executeAction.request else {
227-
return XCTFail("Unexpected request action: \(executeAction.request)")
228-
}
229-
XCTAssert(returnedEL === mockRequest.eventLoop) // XCTAssertIdentical not available on Linux
214+
XCTAssertEqual(.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop), executeAction.request)
230215

231216
// 1. connection attempt
232217
guard case .createConnection(let connectionID, on: let connectionEL) = executeAction.connection else {
@@ -314,11 +299,7 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
314299
let action = state.executeRequest(request)
315300

316301
XCTAssertEqual(action.connection, .none)
317-
guard case .scheduleRequestTimeout(_, request.id, on: let timeoutEL) = action.request else {
318-
return XCTFail("Unexpected request action: \(action.request)")
319-
}
320-
321-
XCTAssert(timeoutEL === mockRequest.eventLoop)
302+
XCTAssertEqual(.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop), action.request)
322303

323304
XCTAssertNoThrow(try queuer.queue(mockRequest, id: request.id))
324305
queuedRequestsOrder.append(request.id)
@@ -495,12 +476,8 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
495476
let request = HTTPConnectionPool.Request(mockRequest)
496477
let action = state.executeRequest(request)
497478

498-
XCTAssertEqual(action.connection, .none)
499-
guard case .scheduleRequestTimeout(_, request.id, on: let timeoutEL) = action.request else {
500-
return XCTFail("Unexpected request action: \(action.request)")
501-
}
502-
503-
XCTAssert(mockRequest.eventLoop === timeoutEL)
479+
XCTAssertEqual(.none, action.connection)
480+
XCTAssertEqual(.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop), action.request)
504481
XCTAssertNoThrow(try queuer.queue(mockRequest, id: request.id))
505482
queuedRequestsOrder.append(request.id)
506483
}

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPConnectionPool+StateTestUtils.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ extension HTTPConnectionPool.StateMachine.RequestAction: Equatable {
106106
return lhsReq == rhsReq && lhsReqID == rhsReqID
107107
case (.failRequestsAndCancelTimeouts(let lhsReqs, _), .failRequestsAndCancelTimeouts(let rhsReqs, _)):
108108
return lhsReqs.elementsEqual(rhsReqs, by: { $0 == $1 })
109-
case (.scheduleRequestTimeout(let lhsDeadline, for: let lhsReqID, on: let lhsEL), .scheduleRequestTimeout(let rhsDeadline, for: let rhsReqID, on: let rhsEL)):
110-
return lhsDeadline == rhsDeadline && lhsReqID == rhsReqID && lhsEL === rhsEL
109+
case (.scheduleRequestTimeout(for: let lhsReq, on: let lhsEL), .scheduleRequestTimeout(for: let rhsReq, on: let rhsEL)):
110+
return lhsReq == rhsReq && lhsEL === rhsEL
111111
case (.cancelRequestTimeout(let lhsReqID), .cancelRequestTimeout(let rhsReqID)):
112112
return lhsReqID == rhsReqID
113113
case (.none, .none):

Diff for: ‎Tests/AsyncHTTPClientTests/Mocks/MockConnectionPool.swift

+1-2
Original file line numberDiff line numberDiff line change
@@ -496,8 +496,7 @@ extension MockConnectionPool {
496496
let request = HTTPConnectionPool.Request(mockRequest)
497497
let action = state.executeRequest(request)
498498

499-
guard case .scheduleRequestTimeout(_, request.id, on: let waitEL) = action.request,
500-
mockRequest.eventLoop === waitEL else {
499+
guard case .scheduleRequestTimeout(request, on: let waitEL) = action.request, mockRequest.eventLoop === waitEL else {
501500
throw SetupError.expectedRequestToBeAddedToQueue
502501
}
503502

0 commit comments

Comments
 (0)