[swift-server-dev] Thoughts on the current HTTP API

[Johannes Weiß]

Hi,

I just had a brief look at the HTTP APIs [1]. Some thoughts (I realise some are different to what I initially proposed 😉):

- why do we store `original` here? [2]

    public struct HTTPHeaders {
        var original: [(name: Name, value: String)]?
        var storage: [Name: [String]] {
            didSet { original = nil }
        }

- I don't think we should name the method to write the HTTP response head `writeHeader`, that's too confusing [3], it should be `writeResponseHead` maybe?

- why do we have a `struct HTTPResponse` [4] and then we don't use it to write the response? I think the `writeResponseHead` (currently named `writeHeader`) should take a `HTTPResponseHead` at its sole argument

- I think we should rename `HTTPRequest` and `HTTPResponse` to `HTTPRequestHead` and `HTTPResponseHead` as they're not full requests/responses but just the head (body is not included)

- `func writeBody(_ data: UnsafeHTTPResponseBody, ...) ` [5]: where did this come from?
  `UnsafeHTTPResponseBody` is an existential (a protocol-typed variable). That means with this type signature, you'll do an allocation for every call, that's pretty bad. A better option would be
   `func writeBody<Body: UnsafeHTTPResponseBody>(_ data: Body, ...)` which would get rid of the existential and thus make it faster. But I think this should really

   `func writeBody<Bytes: Collection>(_ data: Bytes, ...) where Collection.Element == UInt8`

   that way you
     * don't have the existential
     * it works directly (no conversion necessary) with `[UInt8]`, `Data`, `DispatchData`, `"someString".utf8`, `UnsafeRawBufferPointer`, `UnsafeBufferPointer<UInt8>` and all sorts of things

- we should decide and document the flushing behaviour. Probably it's good enough if we flush after each `write{ReponseHead, Trailer, Body}(...)` call. Some applications would get more performance if there'd be an explicit `flush()` call but then we need to make that a requirement which would make the API even less appealing to use directly. What do people think?

- regarding the PoC implementation: we really need an asynchronous PoC implementation and see if our APIs make sense. I don't quite see the value of the synchronous PoC. I'd recommend to do that on top of `DispatchIO` (without backpressure quite straightforward) or `DispatchSources` (bit more work). Obviously libevent, libuv, ... are also options but then there's always a dependency on quite a bit of C code that doesn't come with Swift

- given Swift 4's new String APIs, we should not depend on `Foundation` I believe. There's no need of going through `Data` anymore as `String` now has a new constructor: `String(decoding: bytes, as: UTF8.self)`:

    let bytes = "some string".utf8                             // utf8 encoding (previously `"some string".data(using: .utf8)` from Foundation)
    let asStringAgain = String(decoding: bytes, as: UTF8.self) // utf8 decoding (previously `String(bytes: ..., encoding: .utf8)` from Foundation)

  or
    
     let bytes = [240, 159, 145, 138]
     let firstEmoji = String(decoding: bytes, as: UTF8.self)

  which works on all Collections with UInt8 elements, ie. Data, DispatchData, [UInt8], UTF8View, ...

- I think it'd make sense to clearly split (two different top-level directories) what's the API we propose and what's the PoC implementation. Maybe even two different repositories?


-- Johannes
PS: Please let me know if you want me to expand on the overhead of existential types in APIs. It seems like that's often missed in Swift and often it probably doesn't matter all that much. But in APIs that are meant for performant stuff it does.

[1]: https://github.com/swift-server/http/tree/develop/Sources/HTTP
[2]: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPHeaders.swift#L11
[3]: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPResponse.swift#L28
[4]: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPResponse.swift#L13
[5]: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPResponse.swift#L39