From e95a1e26d5eca3297eacb33b13a588e9fc416121 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Wed, 1 May 2019 14:17:56 +0100 Subject: [PATCH] Allow setting multiple options of the same type. (#40) Motivation: While we fixed a bug in SwiftNIO about setting multiple options of the same type in apple/swift-nio#597, we never brought a fix for that issue forward. We did aim to resolve this in SwiftNIO 2.0 by providing ChannelOptions.Storage, but unfortunately we forgot to make the initializer for that type public (see apple/swift-nio#988). While SwiftNIO core has to get its ducks in a row, users of this library should still be able to set more than one socket option, in my humble opinion. Modifications: - Ported over ChannelOptions.Storage until apple/swift-nio#988 is fixed. - Transitioned our code to use it. - Tested it works. Result: Users can set multiple channel options. --- .../NIOTSConnectionBootstrap.swift | 50 ++++++++++++------- .../NIOTSListenerBootstrap.swift | 16 +++--- .../NIOTSSocketOptionsOnChannelTests.swift | 22 ++++++++ 3 files changed, 62 insertions(+), 26 deletions(-) diff --git a/Sources/NIOTransportServices/NIOTSConnectionBootstrap.swift b/Sources/NIOTransportServices/NIOTSConnectionBootstrap.swift index d438be8..52a4e16 100644 --- a/Sources/NIOTransportServices/NIOTSConnectionBootstrap.swift +++ b/Sources/NIOTransportServices/NIOTSConnectionBootstrap.swift @@ -23,7 +23,7 @@ public final class NIOTSConnectionBootstrap { private let group: NIOTSEventLoopGroup private var channelInitializer: ((Channel) -> EventLoopFuture)? private var connectTimeout: TimeAmount = TimeAmount.seconds(10) - private var channelOptions = ChannelOptionStorage() + private var channelOptions = ChannelOptionsStorage() private var qos: DispatchQoS? private var tcpOptions: NWProtocolTCP.Options = .init() private var tlsOptions: NWProtocolTLS.Options? @@ -54,7 +54,7 @@ public final class NIOTSConnectionBootstrap { /// - option: The option to be applied. /// - value: The value for the option. public func channelOption(_ option: Option, value: Option.Value) -> Self { - channelOptions.put(key: option, value: value) + channelOptions.append(key: option, value: value) return self } @@ -151,7 +151,7 @@ public final class NIOTSConnectionBootstrap { let channelOptions = self.channelOptions return conn.eventLoop.submit { - return channelOptions.applyAll(channel: conn).flatMap { + return channelOptions.applyAllChannelOptions(to: conn).flatMap { initializer(conn) }.flatMap { conn.register() @@ -175,34 +175,48 @@ public final class NIOTSConnectionBootstrap { } } -internal struct ChannelOptionStorage { - private var storage: [(Any, (Any, (Channel) -> (Any, Any) -> EventLoopFuture))] = [] +// This is a backport of ChannelOptions.Storage from SwiftNIO because the initializer wasn't public, so we couldn't actually build it. +// When https://github.com/apple/swift-nio/pull/988 is in a shipped release, we can remove this and simply bump our lowest supported version of SwiftNIO. +internal struct ChannelOptionsStorage { + internal var _storage: [(Any, (Any, (Channel) -> (Any, Any) -> EventLoopFuture))] = [] - mutating func put(key: K, - value newValue: K.Value) { + internal init() { } + + /// Add `Options`, a `ChannelOption` to the `ChannelOptionsStorage`. + /// + /// - parameters: + /// - key: the key for the option + /// - value: the value for the option + internal mutating func append(key newKey: Option, value newValue: Option.Value) { func applier(_ t: Channel) -> (Any, Any) -> EventLoopFuture { - return { (x, y) in - return t.setOption(x as! K, value: y as! K.Value) + return { (option, value) in + return t.setOption(option as! Option, value: value as! Option.Value) } } var hasSet = false - self.storage = self.storage.map { typeAndValue in - let (type, value) = typeAndValue - if type is K { + self._storage = self._storage.map { currentKeyAndValue in + let (currentKey, _) = currentKeyAndValue + if let currentKey = currentKey as? Option, currentKey == newKey { hasSet = true - return (key, (newValue, applier)) + return (currentKey, (newValue, applier)) } else { - return (type, value) + return currentKeyAndValue } } if !hasSet { - self.storage.append((key, (newValue, applier))) + self._storage.append((newKey, (newValue, applier))) } } - func applyAll(channel: Channel) -> EventLoopFuture { - let applyPromise: EventLoopPromise = channel.eventLoop.makePromise() - var it = self.storage.makeIterator() + /// Apply all stored `ChannelOption`s to `Channel`. + /// + /// - parameters: + /// - channel: The `Channel` to apply the `ChannelOption`s to + /// - returns: + /// - An `EventLoopFuture` that is fulfilled when all `ChannelOption`s have been applied to the `Channel`. + public func applyAllChannelOptions(to channel: Channel) -> EventLoopFuture { + let applyPromise = channel.eventLoop.makePromise(of: Void.self) + var it = self._storage.makeIterator() func applyNext() { guard let (key, (value, applier)) = it.next() else { diff --git a/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift b/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift index 14c15ea..36e3215 100644 --- a/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift +++ b/Sources/NIOTransportServices/NIOTSListenerBootstrap.swift @@ -24,8 +24,8 @@ public final class NIOTSListenerBootstrap { private let childGroup: EventLoopGroup private var serverChannelInit: ((Channel) -> EventLoopFuture)? private var childChannelInit: ((Channel) -> EventLoopFuture)? - private var serverChannelOptions = ChannelOptionStorage() - private var childChannelOptions = ChannelOptionStorage() + private var serverChannelOptions = ChannelOptionsStorage() + private var childChannelOptions = ChannelOptionsStorage() private var serverQoS: DispatchQoS? private var childQoS: DispatchQoS? private var tcpOptions: NWProtocolTCP.Options = .init() @@ -85,7 +85,7 @@ public final class NIOTSListenerBootstrap { /// - option: The option to be applied. /// - value: The value for the option. public func serverChannelOption(_ option: Option, value: Option.Value) -> Self { - self.serverChannelOptions.put(key: option, value: value) + self.serverChannelOptions.append(key: option, value: value) return self } @@ -95,7 +95,7 @@ public final class NIOTSListenerBootstrap { /// - option: The option to be applied. /// - value: The value for the option. public func childChannelOption(_ option: Option, value: Option.Value) -> Self { - self.childChannelOptions.put(key: option, value: value) + self.childChannelOptions.append(key: option, value: value) return self } @@ -211,7 +211,7 @@ public final class NIOTSListenerBootstrap { tlsOptions: self.tlsOptions) return eventLoop.submit { - return serverChannelOptions.applyAll(channel: serverChannel).flatMap { + return serverChannelOptions.applyAllChannelOptions(to: serverChannel).flatMap { serverChannelInit(serverChannel) }.flatMap { serverChannel.pipeline.addHandler(AcceptHandler(childChannelInitializer: childChannelInit, @@ -243,14 +243,14 @@ private class AcceptHandler: ChannelInboundHandler { private let childChannelInitializer: ((Channel) -> EventLoopFuture)? private let childGroup: NIOTSEventLoopGroup - private let childChannelOptions: ChannelOptionStorage + 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: ChannelOptionStorage, + childChannelOptions: ChannelOptionsStorage, childChannelQoS: DispatchQoS?, tcpOptions: NWProtocolTCP.Options, tlsOptions: NWProtocolTLS.Options?) { @@ -276,7 +276,7 @@ private class AcceptHandler: ChannelInboundHandler { @inline(__always) func setupChildChannel() -> EventLoopFuture { - return self.childChannelOptions.applyAll(channel: newChannel).flatMap { () -> EventLoopFuture in + return self.childChannelOptions.applyAllChannelOptions(to: newChannel).flatMap { () -> EventLoopFuture in childLoop.assertInEventLoop() return childInitializer(newChannel) } diff --git a/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift b/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift index def1500..dc336af 100644 --- a/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift +++ b/Tests/NIOTransportServicesTests/NIOTSSocketOptionsOnChannelTests.swift @@ -122,5 +122,27 @@ class NIOTSSocketOptionsOnChannelTests: XCTestCase { func testSO_KEEPALIVE() throws { try self.assertChannelOptionAfterCreation(option: SocketOption(level: SOL_SOCKET, name: SO_KEEPALIVE), initialValue: 0, testAlternativeValue: 1) } + + func testMultipleSocketOptions() throws { + let listener = try NIOTSListenerBootstrap(group: group) + .serverChannelOption(ChannelOptions.socket(SOL_SOCKET, SO_REUSEADDR), value: 1) + .serverChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1) + .bind(host: "127.0.0.1", port: 0).wait() + defer { + XCTAssertNoThrow(try listener.close().wait()) + } + let connector = try NIOTSConnectionBootstrap(group: group) + .channelOption(ChannelOptions.socket(SOL_SOCKET, SO_REUSEADDR), value: 1) + .channelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1) + .connect(to: listener.localAddress!).wait() + defer { + XCTAssertNoThrow(try connector.close().wait()) + } + + XCTAssertNoThrow(XCTAssertEqual(1, try listener.getOption(ChannelOptions.socket(SOL_SOCKET, SO_REUSEADDR)).wait())) + XCTAssertNoThrow(XCTAssertEqual(1, try listener.getOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY)).wait())) + XCTAssertNoThrow(XCTAssertEqual(1, try connector.getOption(ChannelOptions.socket(SOL_SOCKET, SO_REUSEADDR)).wait())) + XCTAssertNoThrow(XCTAssertEqual(1, try connector.getOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY)).wait())) + } } #endif