From d59370d89af5e647fa14779b693cb5378602aa6d Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Wed, 22 May 2019 16:40:48 +0100 Subject: [PATCH] Pass Channels down pipeline not NWConnection (#45) Motivation: The expectation is that server channels use Channel as their data type, but the initial data type in NIOTSListenerChannel was actually NWConnection. This is unnecessary and it makes it hard to interop between NIOTS and NIO. Modifications: - Initialize the NIOTSConnectionChannel in the NIOTSListenerChannel instead of in AcceptHandler. - Added some missing @available annotations in the tests. Result: More consistency --- .../NIOTSListenerBootstrap.swift | 42 ++++++------------- .../NIOTSListenerChannel.swift | 32 +++++++++++++- .../NIOTSConnectionChannelTests.swift | 3 ++ .../NIOTSEndToEndTests.swift | 2 +- .../NIOTSEventLoopTests.swift | 2 + .../NIOTSListenerChannelTests.swift | 39 +++++++++++++++++ .../NIOTSSocketOptionTests.swift | 1 + .../NIOTSSocketOptionsOnChannelTests.swift | 2 +- 8 files changed, 89 insertions(+), 34 deletions(-) diff --git a/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift b/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift index 5b406dd..9102051 100644 --- a/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift +++ b/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift @@ -22,7 +22,7 @@ import Network @available(OSX 10.14, iOS 12.0, tvOS 12.0, *) public final class NIOTSListenerBootstrap { private let group: EventLoopGroup - private let childGroup: EventLoopGroup + private let childGroup: NIOTSEventLoopGroup private var serverChannelInit: ((Channel) -> EventLoopFuture)? private var childChannelInit: ((Channel) -> EventLoopFuture)? private var serverChannelOptions = ChannelOptionsStorage() @@ -200,7 +200,6 @@ public final class NIOTSListenerBootstrap { private func bind0(_ binder: @escaping (Channel) -> EventLoopFuture) -> EventLoopFuture { let eventLoop = self.group.next() as! NIOTSEventLoop - let childEventLoopGroup = self.childGroup as! NIOTSEventLoopGroup let serverChannelInit = self.serverChannelInit ?? { _ in eventLoop.makeSucceededFuture(()) } let childChannelInit = self.childChannelInit let serverChannelOptions = self.serverChannelOptions @@ -209,18 +208,18 @@ public final class NIOTSListenerBootstrap { let serverChannel = NIOTSListenerChannel(eventLoop: eventLoop, qos: self.serverQoS, tcpOptions: self.tcpOptions, - tlsOptions: self.tlsOptions) + tlsOptions: self.tlsOptions, + childLoopGroup: self.childGroup, + childChannelQoS: self.childQoS, + childTCPOptions: self.tcpOptions, + childTLSOptions: self.tlsOptions) return eventLoop.submit { return serverChannelOptions.applyAllChannelOptions(to: serverChannel).flatMap { serverChannelInit(serverChannel) }.flatMap { serverChannel.pipeline.addHandler(AcceptHandler(childChannelInitializer: childChannelInit, - childGroup: childEventLoopGroup, - childChannelOptions: childChannelOptions, - childChannelQoS: self.childQoS, - tcpOptions: self.tcpOptions, - tlsOptions: self.tlsOptions)) + childChannelOptions: childChannelOptions)) }.flatMap { serverChannel.register() }.flatMap { @@ -240,41 +239,24 @@ public final class NIOTSListenerBootstrap { @available(OSX 10.14, iOS 12.0, tvOS 12.0, *) private class AcceptHandler: ChannelInboundHandler { - typealias InboundIn = NWConnection + typealias InboundIn = NIOTSConnectionChannel typealias InboundOut = NIOTSConnectionChannel private let childChannelInitializer: ((Channel) -> EventLoopFuture)? - private let childGroup: NIOTSEventLoopGroup private let childChannelOptions: ChannelOptionsStorage - private let childChannelQoS: DispatchQoS? - private let originalTCPOptions: NWProtocolTCP.Options - private let originalTLSOptions: NWProtocolTLS.Options? init(childChannelInitializer: ((Channel) -> EventLoopFuture)?, - childGroup: NIOTSEventLoopGroup, - childChannelOptions: ChannelOptionsStorage, - childChannelQoS: DispatchQoS?, - tcpOptions: NWProtocolTCP.Options, - tlsOptions: NWProtocolTLS.Options?) { + childChannelOptions: ChannelOptionsStorage) { self.childChannelInitializer = childChannelInitializer - self.childGroup = childGroup self.childChannelOptions = childChannelOptions - self.childChannelQoS = childChannelQoS - self.originalTCPOptions = tcpOptions - self.originalTLSOptions = tlsOptions } func channelRead(context: ChannelHandlerContext, data: NIOAny) { - let conn = self.unwrapInboundIn(data) - let childLoop = self.childGroup.next() as! NIOTSEventLoop + let newChannel = self.unwrapInboundIn(data) + let childLoop = newChannel.eventLoop let ctxEventLoop = context.eventLoop let childInitializer = self.childChannelInitializer ?? { _ in childLoop.makeSucceededFuture(()) } - let newChannel = NIOTSConnectionChannel(wrapping: conn, - on: childLoop, - parent: context.channel, - qos: self.childChannelQoS, - tcpOptions: self.originalTCPOptions, - tlsOptions: self.originalTLSOptions) + @inline(__always) func setupChildChannel() -> EventLoopFuture { diff --git a/Sources/NIOTransportServices/NIOTSListenerChannel.swift b/Sources/NIOTransportServices/NIOTSListenerChannel.swift index 6a75948..19a84ec 100644 --- a/Sources/NIOTransportServices/NIOTSListenerChannel.swift +++ b/Sources/NIOTransportServices/NIOTSListenerChannel.swift @@ -83,18 +83,39 @@ internal final class NIOTSListenerChannel { /// Whether to enable peer-to-peer connectivity when using Bonjour services. private var enablePeerToPeer = false + /// The event loop group to use for child channels. + private let childLoopGroup: NIOTSEventLoopGroup + + /// The QoS to use for child channels. + private let childChannelQoS: DispatchQoS? + + /// The TCP options to use for child channels. + private let childTCPOptions: NWProtocolTCP.Options + + /// The TLS options to use for child channels. + private let childTLSOptions: NWProtocolTLS.Options? + + /// Create a `NIOTSListenerChannel` on a given `NIOTSEventLoop`. /// /// Note that `NIOTSListenerChannel` objects cannot be created on arbitrary loops types. internal init(eventLoop: NIOTSEventLoop, qos: DispatchQoS? = nil, tcpOptions: NWProtocolTCP.Options, - tlsOptions: NWProtocolTLS.Options?) { + tlsOptions: NWProtocolTLS.Options?, + childLoopGroup: NIOTSEventLoopGroup, + childChannelQoS: DispatchQoS?, + childTCPOptions: NWProtocolTCP.Options, + childTLSOptions: NWProtocolTLS.Options?) { self.tsEventLoop = eventLoop self.closePromise = eventLoop.makePromise() self.connectionQueue = eventLoop.channelQueue(label: "nio.transportservices.listenerchannel", qos: qos) self.tcpOptions = tcpOptions self.tlsOptions = tlsOptions + self.childLoopGroup = childLoopGroup + self.childChannelQoS = childChannelQoS + self.childTCPOptions = childTCPOptions + self.childTLSOptions = childTLSOptions // Must come last, as it requires self to be completely initialized. self._pipeline = ChannelPipeline(channel: self) @@ -412,7 +433,14 @@ extension NIOTSListenerChannel { return } - self.pipeline.fireChannelRead(NIOAny(connection)) + let newChannel = NIOTSConnectionChannel(wrapping: connection, + on: self.childLoopGroup.next() as! NIOTSEventLoop, + parent: self, + qos: self.childChannelQoS, + tcpOptions: self.childTCPOptions, + tlsOptions: self.childTLSOptions) + + self.pipeline.fireChannelRead(NIOAny(newChannel)) self.pipeline.fireChannelReadComplete() } } diff --git a/Tests/NIOTransportServicesTests/NIOTSConnectionChannelTests.swift b/Tests/NIOTransportServicesTests/NIOTSConnectionChannelTests.swift index e7a9d43..1e1f8e2 100644 --- a/Tests/NIOTransportServicesTests/NIOTSConnectionChannelTests.swift +++ b/Tests/NIOTransportServicesTests/NIOTSConnectionChannelTests.swift @@ -22,6 +22,7 @@ import NIOTransportServices import Foundation +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) final class ConnectRecordingHandler: ChannelOutboundHandler { typealias OutboundIn = Any typealias OutboundOut = Any @@ -71,6 +72,7 @@ final class WritabilityChangedHandler: ChannelInboundHandler { } +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) final class DisableWaitingAfterConnect: ChannelOutboundHandler { typealias OutboundIn = Any typealias OutboundOut = Any @@ -103,6 +105,7 @@ final class PromiseOnActiveHandler: ChannelInboundHandler { } +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) class NIOTSConnectionChannelTests: XCTestCase { private var group: NIOTSEventLoopGroup! diff --git a/Tests/NIOTransportServicesTests/NIOTSEndToEndTests.swift b/Tests/NIOTransportServicesTests/NIOTSEndToEndTests.swift index f857293..357b50c 100644 --- a/Tests/NIOTransportServicesTests/NIOTSEndToEndTests.swift +++ b/Tests/NIOTransportServicesTests/NIOTSEndToEndTests.swift @@ -164,7 +164,7 @@ extension ByteBufferAllocator { } } - +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) class NIOTSEndToEndTests: XCTestCase { private var group: NIOTSEventLoopGroup! diff --git a/Tests/NIOTransportServicesTests/NIOTSEventLoopTests.swift b/Tests/NIOTransportServicesTests/NIOTSEventLoopTests.swift index c490873..e415621 100644 --- a/Tests/NIOTransportServicesTests/NIOTSEventLoopTests.swift +++ b/Tests/NIOTransportServicesTests/NIOTSEventLoopTests.swift @@ -20,6 +20,8 @@ import NIO import NIOConcurrencyHelpers import NIOTransportServices + +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) class NIOTSEventLoopTest: XCTestCase { func testIsInEventLoopWorks() throws { let group = NIOTSEventLoopGroup() diff --git a/Tests/NIOTransportServicesTests/NIOTSListenerChannelTests.swift b/Tests/NIOTransportServicesTests/NIOTSListenerChannelTests.swift index e86d70f..a2ab1ee 100644 --- a/Tests/NIOTransportServicesTests/NIOTSListenerChannelTests.swift +++ b/Tests/NIOTransportServicesTests/NIOTSListenerChannelTests.swift @@ -21,6 +21,7 @@ import NIO import NIOTransportServices +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) final class BindRecordingHandler: ChannelOutboundHandler { typealias OutboundIn = Any typealias OutboundOut = Any @@ -45,6 +46,7 @@ final class BindRecordingHandler: ChannelOutboundHandler { } +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) class NIOTSListenerChannelTests: XCTestCase { private var group: NIOTSEventLoopGroup! @@ -220,5 +222,42 @@ class NIOTSListenerChannelTests: XCTestCase { XCTAssertNoThrow(try listener.close().wait()) } } + + func testChannelEmitsChannels() throws { + class ChannelReceiver: ChannelInboundHandler { + typealias InboundIn = Channel + typealias InboundOut = Channel + + private let promise: EventLoopPromise + + init(_ promise: EventLoopPromise) { + self.promise = promise + } + + func channelRead(context: ChannelHandlerContext, data: NIOAny) { + let channel = self.unwrapInboundIn(data) + self.promise.succeed(channel) + } + } + + let channelPromise = self.group.next().makePromise(of: Channel.self) + let listener = try NIOTSListenerBootstrap(group: self.group) + .serverChannelInitializer { channel in + channel.pipeline.addHandler(ChannelReceiver(channelPromise)) + } + .bind(host: "localhost", port: 0).wait() + defer { + XCTAssertNoThrow(try listener.close().wait()) + } + + let connection = try NIOTSConnectionBootstrap(group: self.group).connect(to: listener.localAddress!).wait() + defer { + XCTAssertNoThrow(try connection.close().wait()) + } + + let promisedChannel = try channelPromise.futureResult.wait() + XCTAssertEqual(promisedChannel.remoteAddress, connection.localAddress) + XCTAssertEqual(promisedChannel.localAddress, connection.remoteAddress) + } } #endif diff --git a/Tests/NIOTransportServicesTests/NIOTSSocketOptionTests.swift b/Tests/NIOTransportServicesTests/NIOTSSocketOptionTests.swift index f93fc87..9f3f81b 100644 --- a/Tests/NIOTransportServicesTests/NIOTSSocketOptionTests.swift +++ b/Tests/NIOTransportServicesTests/NIOTSSocketOptionTests.swift @@ -21,6 +21,7 @@ import Network @testable import NIOTransportServices +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) class NIOTSSocketOptionTests: XCTestCase { private var options: NWProtocolTCP.Options! diff --git a/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift b/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift index dc336af..4f11217 100644 --- a/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift +++ b/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift @@ -49,7 +49,7 @@ private extension Channel { } } - +@available(OSX 10.14, iOS 12.0, tvOS 12.0, *) class NIOTSSocketOptionsOnChannelTests: XCTestCase { private var group: NIOTSEventLoopGroup!