swift-aws-lambda-events
swift-aws-lambda-events copied to clipboard
swift-server
Use Swift HTTP types package for status and method
Use Swift HTTP types package for status and method
Motivation:
As mentioned in #46 there is some overlap between the definitions of http types in this library and in the recent Apple Swift HTTP Types library.
This means when working with Swift HTTP Types, you have to convert from the AWS Lambda Events representation of http types into Swift HTTP Types.
Modifications:
I have replaced the request method and response status Swift types in this library with those from Swift HTTP Types.
Result:
Now you must use Swift HTTP Types for request methods and response types.
Further discussion:
No Headers?
I initially indented to also replace the representation of headers to Swift HTTP Types' HTTPField, but this turned out to be... non-trivial. The way AWS encodes the different forms of headers in different situations and the way the Swift HTTP Types library handles encoding and decoding are both (of course) very different, so you can't just replace public let headers: [String: String] with public let header: HTTPFields.
I think I could have added manual encode and decode functions to the various types that have headers to support this, rather than relying on the synthized codable functions. But that seemed like overkill and a future maintenance burden that I am not in a place to decide about.
Extending Status and Method to add Codable support
In Swift HTTP Types, the higher level structs support Codable, but Method and Status themselves don't directly support Codable. Instead the structs that include Method and Status handle the encoding / decoding of those types. This means I've had to add a Codable implementation for Method and Status in this library. This seems a little odd to me. I wonder if that would be better placed in Swift HTTP Types?
Anything holding this up? I can see a soundness check failed but I can't see the details of that
yes, lets address the soundness checks violations. re-kicked it so we have a fresh copy
I think this is a good idea to align on the HTTP types package.
I would suggest two actions
- Open an issue against HTTP types project to check why / if they can add Codable to Status and Method (or even send them the PR) , Then, follow up depending on them. We might delete the
Utils/HTTP.swiftfile if they implementCodableon their side. - Open an issue here that will serve as TODO to track the later addition of the Headers
This is a breaking change, we will probably need to announce it on the forum and slack (that will allow us to see how many are using these)
I will update https://github.com/swift-server/swift-openapi-lambda when merged.
I've opened a PR, but there might be problems with this approach regarding encoding / decoding http status as the swift http struct for status also incldudes a reason phrase that would get lost in encoding, see https://github.com/apple/swift-http-types/pull/53
Why would it be lost in encoding ? The Swift Lambda Events for APIGateway HTTP.Status also has a reason phrase
https://github.com/swift-server/swift-aws-lambda-events/blob/main/Sources/AWSLambdaEvents/Utils/HTTP.swift#L123
It would get lost as my Codable implementation doesn't encode it, it only encodes the status code number. I've added some more detail on the other PR.
As per discussion https://github.com/apple/swift-http-types/pull/53, it is not a good idea to implement Codable on swift-http-type. Some coding (HTTP Status reason) is specific to the API Gateway.