allow two distinct ChannelOptions of one type

Motivation:

Quite embarrasingly, we previously would only store one `ChannelOption`
per `ChannelOption` type. Most channel option types are distinct and
that's probably why it took so long to find this issue. Thanks
@pushkarnk for reporting. Unfortunately though, the most important
`ChannelOption` is `.socket` which crucially also holds a level and a
name. That means if you set two `ChannelOptions.socket` options with
distinct name/level, one would still override the other.

Example:

    .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT), value: 1)
    .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)

would only actually set the latter.

Modifications:

- made all common `ChannelOption` types equatable (for 2.0 this will
  be a protocol requirement)
- deprecated non-Equatable `ChannelOption` types

Result:

you can now set two distinct `ChannelOptions` for one type

Sources/NIO/Bootstrap.swift

@@ -119,21 +119,45 @@ public final class ServerBootstrap {
     /// - parameters:
     ///     - option: The option to be applied.
     ///     - value: The value for the option.
+    @available(*, deprecated, message: "non-Equatable ChannelOptions are deprecated")
     public func serverChannelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
         serverChannelOptions.put(key: option, value: value)
         return self
     }
 
+    /// Specifies a `ChannelOption` to be applied to the `ServerSocketChannel`.
+    ///
+    /// - note: To specify options for the accepted `SocketChannel`s, look at `ServerBootstrap.childChannelOption`.
+    ///
+    /// - parameters:
+    ///     - option: The option to be applied.
+    ///     - value: The value for the option.
+    public func serverChannelOption<T: ChannelOption & Equatable>(_ option: T, value: T.OptionType) -> Self {
+        serverChannelOptions.put(key: option, value: value)
+        return self
+    }
+
     /// Specifies a `ChannelOption` to be applied to the accepted `SocketChannel`s.
     ///
     /// - parameters:
     ///     - option: The option to be applied.
     ///     - value: The value for the option.
+    @available(*, deprecated, message: "non-Equatable ChannelOptions are deprecated")
     public func childChannelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
         childChannelOptions.put(key: option, value: value)
         return self
     }
 
+    /// Specifies a `ChannelOption` to be applied to the accepted `SocketChannel`s.
+    ///
+    /// - parameters:
+    ///     - option: The option to be applied.
+    ///     - value: The value for the option.
+    public func childChannelOption<T: ChannelOption & Equatable>(_ option: T, value: T.OptionType) -> Self {
+        childChannelOptions.put(key: option, value: value)
+        return self
+    }
+
     /// Bind the `ServerSocketChannel` to `host` and `port`.
     ///
     /// - parameters:
@@ -385,11 +409,22 @@ public final class ClientBootstrap {
     /// - parameters:
     ///     - option: The option to be applied.
     ///     - value: The value for the option.
+    @available(*, deprecated, message: "non-Equatable ChannelOptions are deprecated")
     public func channelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
         channelOptions.put(key: option, value: value)
         return self
     }
 
+    /// Specifies a `ChannelOption` to be applied to the `SocketChannel`.
+    ///
+    /// - parameters:
+    ///     - option: The option to be applied.
+    ///     - value: The value for the option.
+    public func channelOption<T: ChannelOption & Equatable>(_ option: T, value: T.OptionType) -> Self {
+        channelOptions.put(key: option, value: value)
+        return self
+    }
+
     /// Specifies a timeout to apply to a connection attempt.
     //
     /// - parameters:
@@ -581,11 +616,22 @@ public final class DatagramBootstrap {
     /// - parameters:
     ///     - option: The option to be applied.
     ///     - value: The value for the option.
+    @available(*, deprecated, message: "non-Equatable ChannelOptions are deprecated")
     public func channelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
         channelOptions.put(key: option, value: value)
         return self
     }
 
+    /// Specifies a `ChannelOption` to be applied to the `DatagramChannel`.
+    ///
+    /// - parameters:
+    ///     - option: The option to be applied.
+    ///     - value: The value for the option.
+    public func channelOption<T: ChannelOption & Equatable>(_ option: T, value: T.OptionType) -> Self {
+        channelOptions.put(key: option, value: value)
+        return self
+    }
+
     /// Use the existing bound socket file descriptor.
     ///
     /// - parameters:
@@ -672,11 +718,34 @@ public final class DatagramBootstrap {
     }
 }
 
-fileprivate struct ChannelOptionStorage {
+/* for tests */ internal struct ChannelOptionStorage {
     private var storage: [(Any, (Any, (Channel) -> (Any, Any) -> EventLoopFuture<Void>))] = []
 
+    mutating func put<K: ChannelOption & Equatable>(key: K,
+                                                    value newValue: K.OptionType) {
+        func applier(_ t: Channel) -> (Any, Any) -> EventLoopFuture<Void> {
+            return { (x, y) in
+                return t.setOption(option: x as! K, value: y as! K.OptionType)
+            }
+        }
+        var hasSet = false
+        self.storage = self.storage.map { typeAndValue in
+            let (type, value) = typeAndValue
+            if type is K && (type as! K) == key {
+                hasSet = true
+                return (key, (newValue, applier))
+            } else {
+                return (type, value)
+            }
+        }
+        if !hasSet {
+            self.storage.append((key, (newValue, applier)))
+        }
+    }
+
+    @available(*, deprecated, message: "non-Equatable ChannelOptions are deprecated")
     mutating func put<K: ChannelOption>(key: K,
-                             value newValue: K.OptionType) {
+                                        value newValue: K.OptionType) {
         func applier(_ t: Channel) -> (Any, Any) -> EventLoopFuture<Void> {
             return { (x, y) in
                 return t.setOption(option: x as! K, value: y as! K.OptionType)

Sources/NIO/ChannelOption.swift

@@ -48,7 +48,7 @@ public typealias SocketOptionName = Int32
 /// `SocketOption` allows to specify configuration settings that are directly applied to the underlying socket file descriptor.
 ///
 /// Valid options are typically found in the various man pages like `man 4 tcp`.
-public enum SocketOption: ChannelOption {
+public enum SocketOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = (SocketOptionLevel, SocketOptionName)
 
     public typealias OptionType = (SocketOptionValue)
@@ -70,58 +70,107 @@ public enum SocketOption: ChannelOption {
             return (level, name)
         }
     }
+
+    public static func == (lhs: SocketOption, rhs: SocketOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(let lLevel, let lName), .const(let rLevel, let rName)):
+            return lLevel == rLevel && lName == rName
+        }
+    }
 }
 
 /// `AllocatorOption` allows to specify the `ByteBufferAllocator` to use.
-public enum AllocatorOption: ChannelOption {
+public enum AllocatorOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = ByteBufferAllocator
 
     case const(())
+
+    public static func == (lhs: AllocatorOption, rhs: AllocatorOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// `RecvAllocatorOption` allows to specify the `RecvByteBufferAllocator` to use.
-public enum RecvAllocatorOption: ChannelOption {
+public enum RecvAllocatorOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = RecvByteBufferAllocator
 
     case const(())
+
+    public static func == (lhs: RecvAllocatorOption, rhs: RecvAllocatorOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// `AutoReadOption` allows to configure if a `Channel` should automatically call `Channel.read` again once all data was read from the transport or
 /// if the user is responsible to call `Channel.read` manually.
-public enum AutoReadOption: ChannelOption {
+public enum AutoReadOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = Bool
 
     case const(())
+
+    public static func == (lhs: AutoReadOption, rhs: AutoReadOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// `WriteSpinOption` allows users to configure the number of repetitions of a only partially successful write call before considering the `Channel` not writable.
 /// Setting this option to `0` means that we only issue one write call and if that call does not write all the bytes,
 /// we consider the `Channel` not writable.
-public enum WriteSpinOption: ChannelOption {
+public enum WriteSpinOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = UInt
 
     case const(())
+
+    public static func == (lhs: WriteSpinOption, rhs: WriteSpinOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// `MaxMessagesPerReadOption` allows to configure the maximum number of read calls to the underlying transport are performed before wait again until
 /// there is more to read and be notified.
-public enum MaxMessagesPerReadOption: ChannelOption {
+public enum MaxMessagesPerReadOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = UInt
 
     case const(())
+
+    public static func == (lhs: MaxMessagesPerReadOption, rhs: MaxMessagesPerReadOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// `BacklogOption` allows to configure the `backlog` value as specified in `man 2 listen`. This is only useful for `ServerSocketChannel`s.
-public enum BacklogOption: ChannelOption {
+public enum BacklogOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = Int32
 
     case const(())
+
+    public static func == (lhs: BacklogOption, rhs: BacklogOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// The watermark used to detect when `Channel.isWritable` returns `true` or `false`.
@@ -158,32 +207,53 @@ public struct WriteBufferWaterMark {
 /// `Channel.isWritable` will return `false`. Once we were able to write some data out of the outbound buffer and the amount of bytes queued
 /// falls below `WriteBufferWaterMark.low` the `Channel` will become writable again. Once this happens `Channel.writable` will return
 /// `true` again. These writability changes are also propagated through the `ChannelPipeline` and so can be intercepted via `ChannelInboundHandler.channelWritabilityChanged`.
-public enum WriteBufferWaterMarkOption: ChannelOption {
+public enum WriteBufferWaterMarkOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = WriteBufferWaterMark
 
     case const(())
+
+    public static func == (lhs: WriteBufferWaterMarkOption, rhs: WriteBufferWaterMarkOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// `ConnectTimeoutOption` allows to configure the `TimeAmount` after which a connect will fail if it was not established in the meantime. May be
 /// `nil`, in which case the connection attempt will never time out.
-public enum ConnectTimeoutOption: ChannelOption {
+public enum ConnectTimeoutOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = TimeAmount?
 
     case const(())
+
+    public static func == (lhs: ConnectTimeoutOption, rhs: ConnectTimeoutOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// `AllowRemoteHalfClosureOption` allows users to configure whether the `Channel` will close itself when its remote
 /// peer shuts down its send stream, or whether it will remain open. If set to `false` (the default), the `Channel`
 /// will be closed automatically if the remote peer shuts down its send stream. If set to true, the `Channel` will
 /// not be closed: instead, a `ChannelEvent.inboundClosed` user event will be sent on the `ChannelPipeline`,
 /// and no more data will be received.
-public enum AllowRemoteHalfClosureOption: ChannelOption {
+public enum AllowRemoteHalfClosureOption: ChannelOption, Equatable {
     public typealias AssociatedValueType = ()
     public typealias OptionType = Bool
 
     case const(())
+
+    public static func == (lhs: AllowRemoteHalfClosureOption, rhs: AllowRemoteHalfClosureOption) -> Bool {
+        switch (lhs, rhs) {
+        case (.const(()), .const(())):
+            return true
+        }
+    }
 }
 
 /// Provides `ChannelOption`s to be used with a `Channel`, `Bootstrap` or `ServerBootstrap`.

Tests/LinuxMain.swift

@@ -41,6 +41,7 @@ import XCTest
          testCase(ByteBufferUtilsTest.allTests),
          testCase(ByteToMessageDecoderTest.allTests),
          testCase(ChannelNotificationTest.allTests),
+         testCase(ChannelOptionStorageTest.allTests),
          testCase(ChannelPipelineTest.allTests),
          testCase(ChannelTests.allTests),
          testCase(CircularBufferTests.allTests),

Tests/NIOTests/ChannelOptionStorageTest+XCTest.swift

@@ -0,0 +1,36 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the SwiftNIO open source project
+//
+// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+//
+// ChannelOptionStorageTest+XCTest.swift
+//
+import XCTest
+
+///
+/// NOTE: This file was generated by generate_linux_tests.rb
+///
+/// Do NOT edit this file directly as it will be regenerated automatically when needed.
+///
+
+extension ChannelOptionStorageTest {
+
+   static var allTests : [(String, (ChannelOptionStorageTest) -> () throws -> Void)] {
+      return [
+                ("testWeStartWithNoOptions", testWeStartWithNoOptions),
+                ("testSetTwoOptionsOfDifferentType", testSetTwoOptionsOfDifferentType),
+                ("testSetTwoOptionsOfSameType", testSetTwoOptionsOfSameType),
+                ("testSetOneOptionTwice", testSetOneOptionTwice),
+           ]
+   }
+}
+