[swift-server-dev] HTTP API Review

[Adrian Zubarev]

Hi Chris,

Well I do not participate in the server workgroup, but I do track its evolution from time to time. I Also haven’t used the new API as a library to play around with, but I had a quick glance over the API.

There are a few questions and suggestions to simplify the evolution of the server side APIs in general:

- (I haven’t tracked that particular decision) Why not calling the module `HTTP`?
- Even it the module is currently called `SwiftServerHTTP`, isn’t the HTTP type prefix redundant?
    - If the Module was called `HTTP` then you could use `HTTP.Method` if you wanted or simply omit the HTTP because it would be optional.

+ I would highly suggest to include a swift lint file so that everyone who’s working on the code follows the same code style rules. (https://github.com/realm/SwiftLint)
    + case/default is part of switch statement itself - indent violation: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPHeaders.swift#L21 (there are a lot more of these)
    + use optional unwrapping instead - avoid force cast: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPHeaders.swift#L75
    + whitespace violation: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPResponse.swift#L15
    + missing whitespace for inheritance / conformance before the colon: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPResponse.swift#L85
    + use whitespaces around operators: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPResponse.swift#L321
    + use whitespaces around assigning operator: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPStreamingParser.swift#L186
    + either omit `self` as much as Swift allows or write it everywhere possible
    + probably longer than the 120 character width - choose one, either 80 or 120: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPStreamingParser.swift#L118
    + unnecessary new line: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPStreamingParser.swift#L465
    + missing back ticks around `get`: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPMethod.swift#L25
    + missing whitespace after the operator in declaration: https://github.com/swift-server/http/blob/develop/Sources/HTTP/HTTPResponse.swift#L352

These are a couple of things that my eyes could catch at a quick glance.

Here are the rules I’m using in a project:

```
opt_in_rules:
  - closure_end_indentation
  - closure_spacing
  - fatal_error_message
  - force_unwrapping
  - sorted_imports
  - operator_usage_whitespace
  - redundant_nil_coalescing
  - switch_case_on_newline
  - attributes
  - no_extension_access_modifier
  - implicit_return

# rule identifiers to exclude from running
disabled_rules: 
  - colon
  - closure_parameter_position
  - opening_brace
  - file_length
  - implicit_return

identifier_name:
  # excluded via string array
  excluded: 
    - px

large_tuple: 4

cyclomatic_complexity: 15

nesting:
  type_level: 2

trailing_whitespace:
  ignores_empty_lines: true
  ignores_comments: true

attributes:
  always_on_same_line: ["@IBAction", "@IBOutlet", "@IBInspectable"]
  always_on_line_above: ["@IBDesignable", "@UIApplicationMain", "@discardableResult", "@objc"]
```

Am 21. August 2017 um 21:54:38, Chris Bailey via swift-server-dev (swift-server-dev at swift.org) schrieb:

The HTTP part of the Swift Server API project has undergone a number of iterations and updates, and I believe its approaching the point that we have sufficient function for use to raise a Swift Evolution "Pitch" and give the wider user community and opportunity to try out the APIs and provide some early feedback.

The main documentation for the HTTP API is now available via GitHub Pages here:
        https://swift-server.github.io/http/
which describes the use of a "WebApp" to handle an incoming HTTPRequest and build a response.

The main item that's missing is a minimal set of APIs to create the HTTP server itself, and to rename the "WebApp" to something better. The following PR from Ian Partridge proposes to do that:
        https://github.com/swift-server/http/pull/26

This renames "WebApp" to "HTTPRequestHandler" provides a "SimpleHTTPServer" implementation with start() and stop() functions. This means you can create and run a simple server with the following:

class SimpleHandler: HTTPRequestHandling {
    func handle(request: HTTPRequest, response: HTTPResponseWriter ) -> HTTPBodyProcessing {
        response.writeHeader(status: .ok, headers: ["Transfer-Encoding": "chunked", "X-foo": "bar"])
        return .processBody { (chunk, stop) in
            switch chunk {
            case .chunk(let data, let finishedProcessing):
                response.writeBody(data) { _ in
                    finishedProcessing()
                }
            case .end:
                response.done()
            default:
                stop = true
                response.abort()
            }
        }
    }
}

let server = SimpleHTTPServer()

try! server.start(port: 0, handler: SimpleHandler().handle)

Please take a look and provide feedback, for example, suggesting that SimpleHTTPServer should just be HTTPServer ;-)

Thanks,

Chris
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
_______________________________________________
swift-server-dev mailing list
swift-server-dev at swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-server-dev/attachments/20170822/6783f1ee/attachment.html>