[swift-server-dev] Prototype of the discussed HTTP API Spec

[Carl Brown]

Hi, Helge,

For purposes of building this prototype, I tried to stay with as many of Johannes' original type signatures as I could.  Therefore, I used an enum for HTTPMethod because that's what Johannes had in his email from https://lists.swift.org/pipermail/swift-server-dev/Week-of-Mon-20170403/000422.html <https://lists.swift.org/pipermail/swift-server-dev/Week-of-Mon-20170403/000422.html>

I'm happy to get into that and other specifics as we go (and yes I added the `RawRepresentable` to the enum because I'm more used to thinking in "200" and "404" than .ok and .notFound, and you are right, a `UInt16` would have been better for status (and maybe I shouldn't have done it at all) - my bad).

But the question of the moment (for you and for the group) is - do you think this is a good enough starting point for us to move it to the https://github.com/swift-server/ <https://github.com/swift-server/> organization and start iterating via GitHub issues & Pull Requests, or do you think we're still at the stage where we need to have more email discussions about it before we're ready to take that step?

-Carl

-- 
Carl Brown, Swift at IBM
Carl.Brown1 at IBM.com <mailto:Carl.Brown1 at IBM.com> (Work)
Austin, TX

> On May 26, 2017, at 4:22 PM, Helge Heß via swift-server-dev <swift-server-dev at swift.org> wrote:
> 
> On 26. May 2017, at 22:00, Chris Bailey via swift-server-dev <swift-server-dev at swift.org> wrote:
>> HTTPRequest:        https://github.com/carlbrown/HTTPSketch/blob/master/Sources/HTTPSketch/HTTPRequest.swift 
> 
> 
>  /// HTTP Methods handled by http_parser.[ch] supports
>  public enum HTTPMethod: String {
>    // case custom(method: String)
>    case UNKNOWN
> 
> Don’t restrict the API to a specific HTTP parser implementation. The number of methods supported by http_parser changed multiple times in the past (I myself added one to the official tree).
> 
> I still highly recommend using constants instead of an enum here. The methods *will* change in the future. (At least BATCH is going to be added)
> At the minimum do the custom(String) variant. UNKNOWN seems unacceptable to me.
> 
> In the same run, I don’t understand why an enum is used for methods but not for headers. Extra weird because the method is just a header in HTTP/2 …
> I’d prefer constants for both.
> 
> 
>> func writeBody(data: DispatchData, completion: @escaping (Result<POSIXError, ()>) -> Void)
> 
> Do we really want to expose POSIXError’s? I’d say have an own error type for HTTP level errors and maybe something like this:
> 
>  enum HTTPError {
>    case connectionClosed(cause: POSIXError?)
>    ...
>  }
> 
> Also: DispatchData vs Data vs Ptr.
> 
> 
>> public enum HTTPResponseStatus: UInt, RawRepresentable {
>>   /* The original spec used custom if you want to use a non-standard response code or
>>    have it available in a (UInt, String) pair from a higher-level web framework. 
>> 
>>    Swift 3 doesn't like it - will revisit in Swift 4*/
>>   //case custom(code: UInt, reasonPhrase: String)
> 
> Swift 3 does like it, you just can’t combine raw w/ extra values. That just doesn’t make sense. I think the only thing you could hope for is that this works on a raw:
> 
>  case custom(code: UInt)
> 
> Also, why a `UInt`? Do you foresee 64-bit status codes? Should be UInt16.
> 
> 
> Again: I’d prefer constants because status codes are defined as part of RFCs and
> will *definitely* change in the future. Something like
> 
>  struct HTTPStatus {
>    let code : UInt16
> 
>    static let `continue` : UInt16 = 100
>    etc.
>  }
> 
> That is future and source-compat proof.
> 
> 
>> public struct HTTPHeaders {
>>    var storage: [String:[String]]     /* lower cased keys */
>>    var original: [(String, String)]   /* original casing */
> 
> I don’t think the storage semantics should be part of the API. I’d prefer
> a protocol here providing indexed access.
> 
> 
> Finally, I still propose calling it HTTPRequestHead, HTTPResponseHead. The body belongs to the request/response. Don’t care too much, but would be better.
> 
> hh
> 
> _______________________________________________
> 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/20170526/6f3aabf2/attachment.html>