Restructuring exposing connection Id

Sources/SignalRClient/HttpConnection.swift

@@ -46,7 +46,6 @@ public class HttpConnection: Connection {
         self.transportFactory = transportFactory
         self.logger = logger
         self.state = .initial
-        self.transportDelegate = ConnectionTransportDelegate(connection: self)
     }
 
     public func start() {
@@ -136,8 +135,8 @@ public class HttpConnection: Connection {
             return
         }
 
-        self.connectionId = negotiationResponse.connectionId
         let startUrl = self.createStartUrl(connectionId: negotiationResponse.connectionId)
+        self.transportDelegate = ConnectionTransportDelegate(connection: self, connectionId: negotiationResponse.connectionId)
         self.transport!.delegate = self.transportDelegate
         self.transport!.start(url: startUrl, options: self.options)
     }
@@ -154,9 +153,7 @@ public class HttpConnection: Connection {
         if changeState {
             _ = self.changeState(from: nil, to: .stopped)
         }
-
-        self.connectionId = nil
-
+
         if leaveStartDispatchGroup {
             logger.log(logLevel: .debug, message: "Leaving startDispatchGroup (\(#function): \(#line))")
             startDispatchGroup.leave()
@@ -192,8 +189,6 @@ public class HttpConnection: Connection {
             return
         }
 
-        self.connectionId = nil
-
         self.startDispatchGroup.wait()
 
         // The transport can be nil if connection was stopped immediately after starting
@@ -210,7 +205,7 @@ public class HttpConnection: Connection {
         }
     }
 
-    fileprivate func transportDidOpen() {
+    fileprivate func transportDidOpen(connectionId: String?) {
         logger.log(logLevel: .info, message: "Transport started")
 
         let previousState = changeState(from: .connecting, to: .connected)
@@ -219,6 +214,7 @@ public class HttpConnection: Connection {
         startDispatchGroup.leave()
         if  previousState != nil {
             logger.log(logLevel: .debug, message: "Invoking connectionDidOpen")
+            self.connectionId = connectionId
             Util.dispatchToMainThread {
                 self.delegate?.connectionDidOpen(connection: self)
             }
@@ -240,8 +236,6 @@ public class HttpConnection: Connection {
         let previousState = changeState(from: nil, to: .stopped)
         logger.log(logLevel: .debug, message: "Previous state \(previousState!)")
 
-        self.connectionId = nil
-
         if previousState == .connecting {
             logger.log(logLevel: .debug, message: "Leaving startDispatchGroup (\(#function): \(#line))")
             // unblock the dispatch group if transport closed when starting (likely due to an error)
@@ -253,6 +247,9 @@ public class HttpConnection: Connection {
             }
         } else {
             logger.log(logLevel: .debug, message: "Invoking connectionDidClose (\(#function): \(#line))")
+
+            self.connectionId = nil
+
             Util.dispatchToMainThread {
                 self.delegate?.connectionDidClose(error: self.stopError ?? error)
             }
@@ -277,13 +274,15 @@ public class HttpConnection: Connection {
 
 public class ConnectionTransportDelegate: TransportDelegate {
     private weak var connection: HttpConnection?
+    private let connectionId: String?
 
-    fileprivate init(connection: HttpConnection!) {
+    fileprivate init(connection: HttpConnection!, connectionId: String?) {
         self.connection = connection
+        self.connectionId = connectionId
     }
 
     public func transportDidOpen() {
-        connection?.transportDidOpen()
+        connection?.transportDidOpen(connectionId: connectionId)
     }
 
     public func transportDidReceiveData(_ data: Data) {

Tests/SignalRClientTests/HttpConnectionTests.swift

@@ -607,7 +607,6 @@ class HttpConnectionTests: XCTestCase {
         let connectionDelegate = TestConnectionDelegate()
         connectionDelegate.connectionDidFailToOpenHandler = { error in
             XCTAssertEqual("\(SignalRError.invalidNegotiationResponse(message: "negotiate returned nil httpResponse."))", "\(error)")
-            // didFailToOpenExpectation.fulfill()
         }
         httpConnection.delegate = connectionDelegate
 
@@ -645,9 +644,13 @@ class HttpConnectionTests: XCTestCase {
         waitForExpectations(timeout: 5 /*seconds*/)
     }
 
-    func testThatConnectionIdIsAvailableAfterStart() {
+    func testThatConnectionIdIsAvailableAfterStartAndClearedAfterStop() {
+        let connectionIdSetExpectation = expectation(description: "connectionId set")
+        let connectionIdClearedExpectation = expectation(description: "connectionId cleared")
+
         let httpConnectionOptions = HttpConnectionOptions()
-        let httpConnection = HttpConnection(url: URL(string:"http://fakeuri.org/")!, options: httpConnectionOptions, logger: PrintLogger())
+        let transport = TestTransport()
+        let httpConnection = HttpConnection(url: URL(string:"http://fakeuri.org/")!, options: httpConnectionOptions, transportFactory: TestTransportFactory(transport), logger: PrintLogger())
         let httpClient = TestHttpClient(postHandler: { _ in
             return (HttpResponse(statusCode: 200, contents: self.negotiatePayload.data(using: .utf8)!), nil)
         })
@@ -656,20 +659,52 @@ class HttpConnectionTests: XCTestCase {
         let connectionDelegate = TestConnectionDelegate()
         connectionDelegate.connectionDidOpenHandler = { connection in
             XCTAssertEqual("6baUtSEmluCoKvmUIqLUJw", connection.connectionId)
+            connectionIdSetExpectation.fulfill()
             httpConnection.stop();
         }
         connectionDelegate.connectionDidCloseHandler = { error in
             XCTAssertNil(httpConnection.connectionId)
+            connectionIdClearedExpectation.fulfill()
         }
 
+        httpConnection.delegate = connectionDelegate
+        httpConnection.start()
+
+        waitForExpectations(timeout: 5 /*seconds*/)
+    }
+
+    func testThatConnectionIdNotSetIfTransportFailsToOpen() {
+        class UnopenableTransport: TestTransport {
+            override func start(url: URL, options: HttpConnectionOptions) {
+                delegate?.transportDidClose(SignalRError.invalidOperation(message: "testError"))
+            }
+        }
+
+        let connectionIdNotSetExpectation = expectation(description: "connectionId set")
+
+        let httpConnectionOptions = HttpConnectionOptions()
+        let transport = UnopenableTransport()
+        let httpConnection = HttpConnection(url: URL(string:"http://fakeuri.org/")!, options: httpConnectionOptions, transportFactory: TestTransportFactory(transport), logger: PrintLogger())
+        let httpClient = TestHttpClient(postHandler: { _ in
+            return (HttpResponse(statusCode: 200, contents: self.negotiatePayload.data(using: .utf8)!), nil)
+        })
+        httpConnectionOptions.httpClientFactory = { options in httpClient }
+
+        let connectionDelegate = TestConnectionDelegate()
+        connectionDelegate.connectionDidOpenHandler = { connection in
+            XCTAssert(false)
+        }
+
         connectionDelegate.connectionDidFailToOpenHandler = { error in
             XCTAssertNil(httpConnection.connectionId)
+            connectionIdNotSetExpectation.fulfill()
         }
-        
+
         httpConnection.delegate = connectionDelegate
-        
+
         httpConnection.start()
-        
+
         waitForExpectations(timeout: 5 /*seconds*/)
     }
+
 }

Tests/SignalRClientTests/HubConnectionTests.swift

@@ -150,6 +150,23 @@ class HubConnectionTests: XCTestCase {
     }
 
     func testTestThatInvokingVoidHubMethodRetunsErrorIfInvokedBeforeHandshakeReceived() {
+        let didOpenExpectation = expectation(description: "connection opened")
+
+        let hubConnectionDelegate = TestHubConnectionDelegate()
+        hubConnectionDelegate.connectionDidOpenHandler = { hubConnection in
+            XCTAssertNotNil(hubConnection.connectionId)
+            didOpenExpectation.fulfill()
+            hubConnection.stop()
+        }
+
+        let hubConnection = HubConnectionBuilder(url: URL(string: "\(BASE_URL)/testhub")!).build()
+        hubConnection.delegate = hubConnectionDelegate
+        hubConnection.start()
+
+        waitForExpectations(timeout: 5 /*seconds*/)
+    }
+
+    func testTestThatCanGetConnectionId() {
         let didComplete = expectation(description: "test completed")
 
         let hubConnection = HubConnectionBuilder(url: URL(string: "\(BASE_URL)/testhub")!).build()
@@ -961,10 +978,13 @@ class TestHubConnectionDelegate: HubConnectionDelegate {
 }
 
 class TestConnection: Connection {
+    var connectionId: String?
+
     var delegate: ConnectionDelegate!
     var sendDelegate: ((_ data: Data, _ sendDidComplete: (_ error: Error?) -> Void) -> Void)?
 
     func start() {
+        connectionId = "00000000-0000-0000-C000-000000000046"
         delegate?.connectionDidOpen(connection: self)
         delegate?.connectionDidReceiveData(connection: self, data: "{}\u{1e}".data(using: .utf8)!)
     }
@@ -974,6 +994,7 @@ class TestConnection: Connection {
     }
 
     func stop(stopError: Error? = nil) -> Void {
+        connectionId = nil
         delegate?.connectionDidClose(error: stopError)
     }
 }