From 3bba0537a5f5c47b63bded62d0964d7658262750 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Fri, 19 Nov 2021 19:04:42 +0100 Subject: [PATCH 1/4] fix migrateToHTTP2() --- .../HTTPConnectionPool+HTTP1Connections.swift | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift index ce638ac29..862a27ac1 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift @@ -532,12 +532,21 @@ extension HTTPConnectionPool { mutating func migrateToHTTP2() -> HTTP1ToHTTP2MigrationContext { var migrationContext = HTTP1ToHTTP2MigrationContext() - self.connections.removeAll { connection in - switch connection.migrateToHTTP2(&migrationContext) { + let initialOverflowIndex = self.overflowIndex + + self.connections = self.connections.enumerated().compactMap { index, connectionState in + switch connectionState.migrateToHTTP2(&migrationContext) { case .removeConnection: - return true + // If the connection has an index smaller than the previous overflow index, + // we deal with a general purpose connection. + // For this reason we need to decrement the overflow index. + if index < initialOverflowIndex { + self.overflowIndex = self.connections.index(before: self.overflowIndex) + } + return nil + case .keepConnection: - return false + return connectionState } } return migrationContext @@ -654,6 +663,9 @@ extension HTTPConnectionPool { self.connections = self.connections.enumerated().compactMap { index, connectionState in switch connectionState.cleanup(&cleanupContext) { case .removeConnection: + // If the connection has an index smaller than the previous overflow index, + // we deal with a general purpose connection. + // For this reason we need to decrement the overflow index. if index < initialOverflowIndex { self.overflowIndex = self.connections.index(before: self.overflowIndex) } From 269acc108e768662d74188b81bbfe82a1e54693b Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Fri, 19 Nov 2021 21:06:10 +0100 Subject: [PATCH 2/4] add test --- ...tionPool+HTTP1ConnectionsTest+XCTest.swift | 1 + ...PConnectionPool+HTTP1ConnectionsTest.swift | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest+XCTest.swift index 78ccf2d2b..21eb3029e 100644 --- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest+XCTest.swift @@ -41,6 +41,7 @@ extension HTTPConnectionPool_HTTP1ConnectionsTests { ("testMigrationFromHTTP2WithAlreadyLeasedHTTP1Connection", testMigrationFromHTTP2WithAlreadyLeasedHTTP1Connection), ("testMigrationFromHTTP2WithMoreStartingConnectionsThanMaximumAllowedConccurentConnections", testMigrationFromHTTP2WithMoreStartingConnectionsThanMaximumAllowedConccurentConnections), ("testMigrationFromHTTP2StartsEnoghOverflowConnectionsForRequiredEventLoopRequests", testMigrationFromHTTP2StartsEnoghOverflowConnectionsForRequiredEventLoopRequests), + ("testMigrationFromHTTP1ToHTTP2AndBackToHTTP1", testMigrationFromHTTP1ToHTTP2AndBackToHTTP1), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift index 35b94c239..7fa8ba69c 100644 --- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift @@ -563,4 +563,47 @@ class HTTPConnectionPool_HTTP1ConnectionsTests: XCTestCase { XCTAssertTrue(context.eventLoop === el) } } + + func testMigrationFromHTTP1ToHTTP2AndBackToHTTP1() throws { + let elg = EmbeddedEventLoopGroup(loops: 2) + defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) } + let el1 = elg.next() + let el2 = elg.next() + + let generator = HTTPConnectionPool.Connection.ID.Generator() + var connections = HTTPConnectionPool.HTTP1Connections(maximumConcurrentConnections: 8, generator: generator) + + let connID1 = connections.createNewConnection(on: el1) + + let context = connections.migrateToHTTP2() + XCTAssertEqual(context, .init( + backingOff: [], + starting: [(connID1, el1)], + close: [] + )) + + let connID2 = generator.next() + + connections.migrateFromHTTP2( + starting: [(connID2, el2)], + backingOff: [] + ) + + let conn2: HTTPConnectionPool.Connection = .__testOnly_connection(id: connID2, eventLoop: el2) + let (_, idleContext) = connections.newHTTP1ConnectionEstablished(conn2) + XCTAssertEqual(idleContext.use, .generalPurpose) + XCTAssertEqual(idleContext.eventLoop.id, el2.id) + } +} + +extension HTTPConnectionPool.HTTP1Connections.HTTP1ToHTTP2MigrationContext: Equatable { + public static func == (lhs: Self, rhs: Self) -> Bool { + return lhs.close == rhs.close && + lhs.starting.elementsEqual(rhs.starting, by: { + $0.0 == $1.0 && $0.1 === $1.1 + }) && + lhs.backingOff.elementsEqual(rhs.backingOff, by: { + $0.0 == $1.0 && $0.1 === $1.1 + }) + } } From 7997ae2547ab62d1a409689978336469180c3d97 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 22 Nov 2021 11:00:30 +0100 Subject: [PATCH 3/4] change formatting --- .../HTTPConnectionPool+HTTP1ConnectionsTest.swift | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift index 7fa8ba69c..014aad0ac 100644 --- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift @@ -598,12 +598,10 @@ class HTTPConnectionPool_HTTP1ConnectionsTests: XCTestCase { extension HTTPConnectionPool.HTTP1Connections.HTTP1ToHTTP2MigrationContext: Equatable { public static func == (lhs: Self, rhs: Self) -> Bool { - return lhs.close == rhs.close && - lhs.starting.elementsEqual(rhs.starting, by: { - $0.0 == $1.0 && $0.1 === $1.1 - }) && - lhs.backingOff.elementsEqual(rhs.backingOff, by: { - $0.0 == $1.0 && $0.1 === $1.1 + lhs.close == rhs.close && lhs.starting.elementsEqual(rhs.starting, by: { + $0.0 == $1.0 && $0.1 === $1.1 + }) && lhs.backingOff.elsementsEqual(rhs.backingOff, by: { + $0.0 == $1.0 && $0.1 === $1.1 }) } } From 8bfa7b87e872bdfd183c328489b57728d4e3e59e Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 22 Nov 2021 11:02:59 +0100 Subject: [PATCH 4/4] improve formatting --- .../HTTPConnectionPool+HTTP1ConnectionsTest.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift index 014aad0ac..5afe755a1 100644 --- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift @@ -598,10 +598,8 @@ class HTTPConnectionPool_HTTP1ConnectionsTests: XCTestCase { extension HTTPConnectionPool.HTTP1Connections.HTTP1ToHTTP2MigrationContext: Equatable { public static func == (lhs: Self, rhs: Self) -> Bool { - lhs.close == rhs.close && lhs.starting.elementsEqual(rhs.starting, by: { - $0.0 == $1.0 && $0.1 === $1.1 - }) && lhs.backingOff.elsementsEqual(rhs.backingOff, by: { - $0.0 == $1.0 && $0.1 === $1.1 - }) + return lhs.close == rhs.close && + lhs.starting.elementsEqual(rhs.starting, by: { $0.0 == $1.0 && $0.1 === $1.1 }) && + lhs.backingOff.elementsEqual(rhs.backingOff, by: { $0.0 == $1.0 && $0.1 === $1.1 }) } }