fix review comments

Author: dnadoba

Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift

@@ -195,7 +195,7 @@ extension HTTPConnectionPool {
             case keepConnection
         }
 
-        func migrateToHTTP2(_ context: inout HTTP1Connections.HTTP2ToHTTP1MigrationContext) -> MigrateAction {
+        func migrateToHTTP2(_ context: inout HTTP1Connections.HTTP1ToHTTP2MigrationContext) -> MigrateAction {
             switch self.state {
             case .starting:
                 context.starting.append((self.connectionID, self.eventLoop))
@@ -322,7 +322,7 @@ extension HTTPConnectionPool {
             var connectionsStartingForUseCase: Int
         }
 
-        struct HTTP2ToHTTP1MigrationContext {
+        struct HTTP1ToHTTP2MigrationContext {
             var backingOff: [(Connection.ID, EventLoop)] = []
             var starting: [(Connection.ID, EventLoop)] = []
             var close: [Connection] = []
@@ -517,8 +517,8 @@ extension HTTPConnectionPool {
 
         // MARK: Migration
 
-        mutating func migrateToHTTP2() -> HTTP2ToHTTP1MigrationContext {
-            var migrationContext = HTTP2ToHTTP1MigrationContext()
+        mutating func migrateToHTTP2() -> HTTP1ToHTTP2MigrationContext {
+            var migrationContext = HTTP1ToHTTP2MigrationContext()
             self.connections.removeAll { connection in
                 switch connection.migrateToHTTP2(&migrationContext) {
                 case .removeConnection:

Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP2StateMachine.swift

@@ -16,7 +16,7 @@ import NIOCore
 import NIOHTTP2
 
 extension HTTPConnectionPool {
-    struct HTTP2StateMaschine {
+    struct HTTP2StateMachine {
         typealias Action = HTTPConnectionPool.StateMachine.Action
         typealias RequestAction = HTTPConnectionPool.StateMachine.RequestAction
         typealias ConnectionAction = HTTPConnectionPool.StateMachine.ConnectionAction
@@ -70,6 +70,7 @@ extension HTTPConnectionPool {
             self.requests = requests
 
             // TODO: Close all idle connections from context.close
+            // TODO: Start new http2 connections for
             // TODO: Potentially cancel unneeded bootstraps (Needs cancellable ClientBootstrap)
 
             return .none
@@ -401,14 +402,19 @@ extension HTTPConnectionPool {
             // Any http1 connection that becomes idle should be closed right away after the transition
             // to http2.
             let connection = self.http1Connections!.closeConnection(at: index)
-            if self.http1Connections!.isEmpty {
-                self.http1Connections = nil
+            guard self.http1Connections!.isEmpty else {
+                return .init(request: .none, connection: .closeConnection(connection, isShutdown: .no))
             }
+            // if there are no more http1Connections, we can remove the struct.
+            self.http1Connections = nil
+            
+            // we must also check, if we are shutting down. Was this maybe out last connection?
             switch state {
             case .running:
                 return .init(request: .none, connection: .closeConnection(connection, isShutdown: .no))
             case .shuttingDown(let unclean):
-                if self.http1Connections == nil && self.connections.isEmpty {
+                if self.connections.isEmpty {
+                    // if the http2connections are empty as well, there are no more connections. Shutdown completed.
                     return .init(request: .none, connection: .closeConnection(connection, isShutdown: .yes(unclean: unclean)))
                 } else {
                     return .init(request: .none, connection: .closeConnection(connection, isShutdown: .no))

Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+RequestQueue.swift

@@ -147,7 +147,7 @@ extension CircularBuffer {
     ///
     /// - Complexity: O(*k*), where *k* is the number of elements removed.
     fileprivate mutating func popFirst(max: Int) -> [Element] {
-        precondition(max >= 0, "")
+        precondition(max >= 0)
         let elementCountToRemove = Swift.min(max, self.count)
         let array = Array(self[self.startIndex..<self.index(self.startIndex, offsetBy: elementCountToRemove)])
         self.removeFirst(elementCountToRemove)

Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP2StateMachineTests.swift

@@ -29,7 +29,7 @@ class HTTPConnectionPool_HTTP2StateMachineTests: XCTestCase {
         let el1 = elg.next()
         var connections = MockConnectionPool()
         var queuer = MockRequestQueuer()
-        var state = HTTPConnectionPool.HTTP2StateMaschine(idGenerator: .init())
+        var state = HTTPConnectionPool.HTTP2StateMachine(idGenerator: .init())
 
         /// first request should create a new connection
         let mockRequest = MockHTTPRequest(eventLoop: el1)
@@ -119,7 +119,7 @@ class HTTPConnectionPool_HTTP2StateMachineTests: XCTestCase {
         let elg = EmbeddedEventLoopGroup(loops: 4)
         defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) }
 
-        var state = HTTPConnectionPool.HTTP2StateMaschine(
+        var state = HTTPConnectionPool.HTTP2StateMachine(
             idGenerator: .init()
         )
 
@@ -175,7 +175,7 @@ class HTTPConnectionPool_HTTP2StateMachineTests: XCTestCase {
         let elg = EmbeddedEventLoopGroup(loops: 4)
         defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) }
 
-        var state = HTTPConnectionPool.HTTP2StateMaschine(
+        var state = HTTPConnectionPool.HTTP2StateMachine(
             idGenerator: .init()
         )
 
@@ -212,7 +212,7 @@ class HTTPConnectionPool_HTTP2StateMachineTests: XCTestCase {
         let elg = EmbeddedEventLoopGroup(loops: 4)
         defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) }
 
-        var state = HTTPConnectionPool.HTTP2StateMaschine(
+        var state = HTTPConnectionPool.HTTP2StateMachine(
             idGenerator: .init()
         )
 
@@ -294,7 +294,7 @@ class HTTPConnectionPool_HTTP2StateMachineTests: XCTestCase {
 
         // second connection is a HTTP2 connection and we need to migrate
         let conn2: HTTPConnectionPool.Connection = .__testOnly_connection(id: conn2ID, eventLoop: el1)
-        var http2State = HTTPConnectionPool.HTTP2StateMaschine(idGenerator: idGenerator)
+        var http2State = HTTPConnectionPool.HTTP2StateMachine(idGenerator: idGenerator)
 
         let migrationAction = http2State.migrateConnectionsFromHTTP1(
             connections: http1State.connections,