Merge pull request #117 from pushkarnk/headers-opt

Implement a very lazy translation of headers

Sources/KituraNet/ClientResponse.swift

@@ -45,11 +45,11 @@ public class ClientResponse {
             guard let httpHeaders = httpHeaders else {
                 return HeadersContainer()
             }
-            return HeadersContainer.create(from: httpHeaders)
+            return HeadersContainer(with: httpHeaders)
         }
 
         set {
-           httpHeaders = newValue.httpHeaders()
+           httpHeaders = newValue.nioHeaders
         }
     }
     /// The HTTP Status code, as an Int, sent in the response by the remote server.

Sources/KituraNet/HTTP/HTTPServerRequest.swift

@@ -115,7 +115,7 @@ public class HTTPServerRequest: ServerRequest {
     }
 
     init(ctx: ChannelHandlerContext, requestHead: HTTPRequestHead, enableSSL: Bool) {
-        self.headers = HeadersContainer.create(from: requestHead.headers)
+        self.headers = HeadersContainer(with: requestHead.headers)
         self.method = requestHead.method.string()
         self.httpVersionMajor = requestHead.version.major
         self.httpVersionMinor = requestHead.version.minor

Sources/KituraNet/HTTP/HTTPServerResponse.swift

@@ -183,7 +183,7 @@ public class HTTPServerResponse: ServerResponse {
 
     /// Send response to the client
     private func sendResponse(channel: Channel, handler: HTTPRequestHandler, status: HTTPResponseStatus, withBody: Bool = true) throws {
-        let response = HTTPResponseHead(version: httpVersion, status: status, headers: headers.httpHeaders())
+        let response = HTTPResponseHead(version: httpVersion, status: status, headers: headers.nioHeaders)
         channel.write(handler.wrapOutboundOut(.head(response)), promise: nil)
         if withBody && buffer.readableBytes > 0 {
             channel.write(handler.wrapOutboundOut(.body(.byteBuffer(buffer))), promise: nil)

Sources/KituraNet/HTTP/HeadersContainer.swift

@@ -26,9 +26,25 @@ public class HeadersContainer {
     /// The header storage
     internal var headers: [String: (key: String, value: [String])] = [:]
 
+    /// An alternate backing store of type `NIOHTTP1.HTTPHeader` used to avoid translations between HeadersContainer and HTTPHeaders
+    var nioHeaders: HTTPHeaders = HTTPHeaders()
+
     /// Create an instance of `HeadersContainer`
     public init() {}
 
+    /// A special initializer for HTTPServerRequest, to make the latter better performant
+    init(with nioHeaders: HTTPHeaders) {
+        self.nioHeaders = nioHeaders
+    }
+
+    private enum _Mode {
+        case nio  // Headers are simply backed up by `NIOHTTP1.HTTPHeaders`
+        case dual // Headers are backed up by a dictionary as well, we switch to this mode while using HeadersContainer as Collection
+    }
+
+    // The default mode is nio
+    private var mode: _Mode = .nio
+
     /// Access the value of a HTTP header using subscript syntax.
     ///
     /// - Parameter key: The HTTP header key
@@ -42,9 +58,16 @@ public class HeadersContainer {
 
         set(newValue) {
             if let newValue = newValue {
-                set(key, value: newValue)
-            } else {
-                remove(key)
+                nioHeaders.replace(name: key, values: newValue)
+                if mode == .dual {
+                    set(key, value: newValue)
+                }
+            }
+            else {
+                nioHeaders.remove(name: key)
+                if mode == .dual {
+                    remove(key)
+                }
             }
         }
     }
@@ -54,36 +77,49 @@ public class HeadersContainer {
     /// - Parameter key: The HTTP header key
     /// - Parameter value: An array of strings to add as values of the HTTP header
     public func append(_ key: String, value: [String]) {
-
         let lowerCaseKey = key.lowercased()
-        let entry = headers[lowerCaseKey]
-
-        switch lowerCaseKey {
+        var entry = nioHeaders[key]
+        
+        switch(lowerCaseKey) {
 
         case "set-cookie":
-            if entry != nil {
-                headers[lowerCaseKey]?.value += value
+            if entry.count > 0 {
+                entry += value
+                nioHeaders.replace(name: key, values: entry)
+                if mode == .dual {
+                    headers[lowerCaseKey]?.value += value
+                }
             } else {
-                set(key, lowerCaseKey: lowerCaseKey, value: value)
+                nioHeaders.add(name: key, values: value)
+                if mode == .dual {
+                    set(key, lowerCaseKey: lowerCaseKey, value: value)
+                }
             }
 
         case "content-type", "content-length", "user-agent", "referer", "host",
              "authorization", "proxy-authorization", "if-modified-since",
              "if-unmodified-since", "from", "location", "max-forwards",
              "retry-after", "etag", "last-modified", "server", "age", "expires":
-            if entry != nil {
+            if entry.count > 0 {
                 Log.warning("Duplicate header \(key) discarded")
                 break
             }
             fallthrough
 
         default:
-            guard let oldValue = entry?.value.first else {
-                set(key, lowerCaseKey: lowerCaseKey, value: value)
-                return
+            if nioHeaders[key].count == 0 {
+                nioHeaders.add(name: key, values: value)
+                if mode == .dual {
+                    set(key, lowerCaseKey: lowerCaseKey, value: value)
+                }
+            } else {
+                let oldValue = nioHeaders[key].first!
+                let newValue = oldValue + ", " + value.joined(separator: ", ")
+                nioHeaders.replaceOrAdd(name: key, value: newValue)
+                if mode == .dual {
+                   headers[lowerCaseKey]?.value[0] = newValue
+                }
             }
-            let newValue = oldValue + ", " + value.joined(separator: ", ")
-            headers[lowerCaseKey]?.value[0] = newValue
         }
     }
 
@@ -96,12 +132,17 @@ public class HeadersContainer {
     }
 
     private func get(_ key: String) -> [String]? {
-        return headers[key.lowercased()]?.value
+        let values = nioHeaders[key]
+        // `HTTPHeaders.subscript` returns a [] if no header is found, but `HeadersContainer` is expected to return a nil
+        return values.count > 0 ? values : nil
     }
 
     /// Remove all of the headers
     public func removeAll() {
-        headers.removeAll(keepingCapacity: true)
+        nioHeaders = HTTPHeaders()
+        if mode == .dual {
+            headers.removeAll(keepingCapacity: true)
+        }
     }
 
     private func set(_ key: String, value: [String]) {
@@ -115,6 +156,14 @@ public class HeadersContainer {
     private func remove(_ key: String) {
         headers.removeValue(forKey: key.lowercased())
     }
+
+    func checkAndSwitchToDualMode() {
+        guard mode == .nio else { return }
+        mode = .dual
+        for header in nioHeaders {
+            headers[header.name.lowercased()] = (header.name, nioHeaders[header.name])
+        }
+    }
 }
 
 extension HTTPHeaders {
@@ -124,22 +173,30 @@ extension HTTPHeaders {
         }
     }
 
-    mutating func replaceOrAdd(name: String, values: [String]) {
-        values.forEach {
-            replaceOrAdd(name: name, value: $0)
+    mutating func replace(name: String, values: [String]) {
+        self.replaceOrAdd(name: name, value: values[0])
+        for value in values.suffix(from: 1) {
+           self.add(name: name, value: value)
         }
     }
 }
 /// Conformance to the `Collection` protocol
+/// As soon as either of these properties or methods are invoked, we need to switch to the `dual` mode of operation
 extension HeadersContainer: Collection {
 
     public typealias Index = DictionaryIndex<String, (key: String, value: [String])>
 
     /// The starting index of the `HeadersContainer` collection
-    public var startIndex: Index { return headers.startIndex }
+    public var startIndex:Index {
+        checkAndSwitchToDualMode()
+        return headers.startIndex
+    }
 
     /// The ending index of the `HeadersContainer` collection
-    public var endIndex: Index { return headers.endIndex }
+    public var endIndex:Index {
+        checkAndSwitchToDualMode()
+        return headers.endIndex
+    }
 
     /// Get a (key value) tuple from the `HeadersContainer` collection at the specified position.
     ///
@@ -148,37 +205,19 @@ extension HeadersContainer: Collection {
     ///
     /// - Returns: A (key, value) tuple.
     public subscript(position: Index) -> (key: String, value: [String]) {
+        get {
+            checkAndSwitchToDualMode()
             return headers[position].value
+        }
     }
 
     /// Get the next Index in the `HeadersContainer` collection after the one specified.
     ///
     /// - Parameter after: The Index whose successor is to be returned.
     ///
     /// - Returns: The Index in the `HeadersContainer` collection after the one specified.
-    public func index(after index: Index) -> Index {
-        return headers.index(after: index)
-    }
-}
-
-/// Kitura uses HeadersContainer and NIOHTTP1 expects HTTPHeader - bridging methods
-extension HeadersContainer {
-    /// HTTPHeaders to HeadersContainer
-    static func create(from httpHeaders: HTTPHeaders) -> HeadersContainer {
-        let headerContainer = HeadersContainer()
-        for header in httpHeaders {
-            headerContainer.append(header.name, value: header.value)
-        }
-        return headerContainer
-    }
-
-    func httpHeaders() -> HTTPHeaders {
-        var httpHeaders = HTTPHeaders()
-        for (_, keyValues) in headers {
-            for value in keyValues.1 {
-                httpHeaders.add(name: keyValues.0, value: value)
-            }
-        }
-        return httpHeaders
+    public func index(after i: Index) -> Index {
+        checkAndSwitchToDualMode()
+        return headers.index(after: i)
     }
 }

Tests/KituraNetTests/HTTPResponseTests.swift

@@ -64,24 +64,24 @@ class HTTPResponseTests: KituraNetTest {
     func testHeadersContainerHTTPHeaders() {
         let headers = HeadersContainer()
         headers["Content-Type"] = ["image/png, text/plain"]
-        XCTAssertEqual(headers.httpHeaders()["Content-Type"], headers["Content-Type"]!)
+        XCTAssertEqual(headers.nioHeaders["Content-Type"], headers["Content-Type"]!)
         headers["Content-Type"] = ["text/html"]
-        XCTAssertEqual(headers.httpHeaders()["Content-Type"], headers["Content-Type"]!)
+        XCTAssertEqual(headers.nioHeaders["Content-Type"], headers["Content-Type"]!)
         headers["Content-Type"] = nil
-        XCTAssertFalse(headers.httpHeaders().contains(name: "Content-Type"))
+        XCTAssertFalse(headers.nioHeaders.contains(name: "Content-Type"))
         headers["Set-Cookie"] = ["ID=123BAS; Path=/; Secure; HttpOnly"]
         headers.append("Set-Cookie", value: ["ID=KI9H12; Path=/; Secure; HttpOnly"])
-        XCTAssertEqual(headers["Set-Cookie"]!, headers.httpHeaders()["Set-Cookie"])
+        XCTAssertEqual(headers["Set-Cookie"]!, headers.nioHeaders["Set-Cookie"])
         headers["Content-Type"] = ["text/html"]
         headers.append("Content-Type", value: "text/json")
-        XCTAssertEqual(headers.httpHeaders()["Content-Type"], headers["Content-Type"]!)
+        XCTAssertEqual(headers.nioHeaders["Content-Type"], headers["Content-Type"]!)
         headers["foo"] = ["bar0"]
         headers.append("foo", value: "bar1")
-        XCTAssertEqual(headers.httpHeaders()["foo"], headers["foo"]!)
+        XCTAssertEqual(headers.nioHeaders["foo"], headers["foo"]!)
         headers.append("foo", value: ["bar2", "bar3"])
-        XCTAssertEqual(headers.httpHeaders()["foo"], headers["foo"]!)
+        XCTAssertEqual(headers.nioHeaders["foo"], headers["foo"]!)
         headers.removeAll()
-        XCTAssertFalse(headers.httpHeaders().contains(name: "foo"))
+        XCTAssertFalse(headers.nioHeaders.contains(name: "foo"))
     }
 
     func testMultipleWritesToResponse() {

Tests/KituraNetTests/MiscellaneousTests.swift

@@ -25,7 +25,8 @@ class MiscellaneousTests: KituraNetTest {
     static var allTests: [(String, (MiscellaneousTests) -> () throws -> Void)] {
         return [
             ("testEscape", testEscape),
-            ("testHeadersContainers", testHeadersContainers)
+            ("testHeadersContainers", testHeadersContainers),
+            ("testHeadersContainerDualMode", testHeadersContainerDualMode),
         ]
     }
 
@@ -63,4 +64,90 @@ class MiscellaneousTests: KituraNetTest {
         XCTAssert(foundContentType, "Didn't find the Content-Type header")
         XCTAssert(foundSetCookie, "Didn't find the Set-Cookie header")
     }
+
+    func testHeadersContainerDualMode() {
+        let headers = HeadersContainer()
+        headers.append("Foo", value: "Bar")
+        headers.append("Foo", value: "Baz")
+        let numValues = headers["Foo"]?.count ?? 0
+        XCTAssertEqual(numValues, 1, "Foo didn't have one value, as expected. It had \(numValues) values")
+
+        let numValues1 = headers[headers.startIndex].value.count
+        XCTAssertEqual(numValues1, 1, "Foo didn't have one value, as expected. It had \(numValues1) values")
+
+        // Special case: cookies
+        let cookieHeaders = HeadersContainer()
+        cookieHeaders.append("Set-Cookie", value: "os=Linux")
+        cookieHeaders.append("Set-Cookie", value: "browser=Safari")
+        let numValues2 = cookieHeaders["Set-Cookie"]?.count ?? 0
+        XCTAssertEqual(numValues2, 2, "Set-Cookie didn't have two values, as expected. It had \(numValues2) values")
+
+        let numValues3 =  cookieHeaders[cookieHeaders.startIndex].value.count
+        XCTAssertEqual(numValues3, 2, "Set-Cookie didn't have two values, as expected. It had \(numValues3) values")
+
+        // Special case: Content-Type
+        let contentHeaders = HeadersContainer()
+        contentHeaders.append("Content-Type", value: "application/json")
+        contentHeaders.append("Content-Type", value: "application/xml")
+        let numValues4 = contentHeaders["Content-Type"]?.count ?? 0
+        XCTAssertEqual(numValues4, 1, "Content-Type didn't have one value, as expected. It had \(numValues4) values")
+        XCTAssertEqual(contentHeaders["Content-Type"]?.first!, "application/json")
+
+        let numValues5 = contentHeaders[contentHeaders.startIndex].value.count
+        XCTAssertEqual(numValues5, 1, "Content-Type didn't have one value, as expected. It had \(numValues5) values")
+
+        // Append arrays
+        headers.append("Foo", value: ["Cons", "Damn"])
+        let numValues6 = headers["Foo"]?.count ?? 0
+        XCTAssertEqual(numValues6, 1, "Foo didn't have one value, as expected. It had \(numValues6) values")
+
+        cookieHeaders.append("Set-Cookie", value: ["abx=xyz", "def=fgh"])
+        let numValues7 = cookieHeaders["Set-Cookie"]?.count ?? 0
+        XCTAssertEqual(numValues7, 4, "Set-Cookie didn't have four values, as expected. It had \(numValues7) values")
+
+        // Append arrays to a new container
+        let headers1 = HeadersContainer()
+        headers1.append("Foo", value: "Bar")
+        headers1.append("Foo", value: ["Cons", "Damn"])
+        let numValues8 = headers1[headers1.startIndex].value.count
+        XCTAssertEqual(numValues8, 1, "Foo didn't have one value, as expected. It had \(numValues8) values")
+
+        let cookieHeaders1 = HeadersContainer()
+        cookieHeaders1.append("Set-Cookie", value: "os=Linux")
+        cookieHeaders1.append("Set-Cookie", value: ["browser=Safari", "temp=xyz"])
+        let numValues9 = cookieHeaders1[cookieHeaders1.startIndex].value.count
+        XCTAssertEqual(numValues9, 3, "Set-Cookie didn't have three values, as expected. It had \(numValues9) values")
+
+        // Set using subscript function before and after mode switch
+        let headers2 = HeadersContainer()
+        headers2.append("Foo", value: "Bar")
+        headers2["Foo"] = ["Baz"]
+        XCTAssertNotNil(headers2["Foo"])
+        XCTAssertEqual(headers2["Foo"]!.count, 1)
+        XCTAssertEqual(headers2["Foo"]!.first!, "Baz")
+
+        let numValues10 = headers2[headers2.startIndex].value.count
+        XCTAssertEqual(numValues10, 1)
+        headers2["Foo"] = ["Bar", "Baz"]
+        XCTAssertEqual(headers2["Foo"]!.count, 2)
+        XCTAssertEqual(headers2["Foo"]!.first!, "Bar")
+
+        let cookieHeaders2 = HeadersContainer()
+        cookieHeaders2.append("Set-Cookie", value: "os=Linux")
+        cookieHeaders2["Set-Cookie"] = ["browser=Safari", "temp=xyz"]
+        let numValues11 = cookieHeaders2[cookieHeaders2.startIndex].value.count
+        XCTAssertEqual(numValues11, 2, "Set-Cookie didn't have two values, as expected. It had \(numValues11) values")
+
+        // removeAll
+        headers2.removeAll()
+        cookieHeaders2.removeAll()
+
+        for _ in headers2 {
+            XCTFail("Headers2 must be empty!")
+        }
+
+        for _ in cookieHeaders2 {
+            XCTFail("cookieHeaders2 must be empty!")
+        }
+    }
 }