[Proposal] SOAR-0005: Adopting the Swift HTTP Types package

Server Development
1

Hi Swift community,

The proposal SOAR-0005: Adopting the Swift HTTP Types package for Swift OpenAPI Generator is now up and In Review.

The review period will run until 15th September - please feel free to post your feedback either as a reply to this thread, or on the pull request .

Thanks!

cc @Honza_Dvorsky

7 Likes
2

This is a move in the right direction. While these may not be used by any of the transports at the moment, they will be introduced over time.

One thing to note the HTTP types are in pre-release at the moment. Given the OpenAPI runtime is also in prerelease that shouldn’t be a concern in the immediate future, only it may impede your move to a v1.0

1 Like
3

Thanks @adam-fowler - yes the HTTP types will have to tag 1.0 before we adopt it, we're coordinating with the maintainers.

4

Really glad that we're pushing forward on shedding our custom currency types in favour of standardised ones :heart:.

public protocol ClientMiddleware {
    func intercept(
        _ request: HTTPRequest, // <<< changed
        body: HTTPBody?, // <<< added
        baseURL: URL,
        operationID: String,
        next: @Sendable (HTTPRequest, HTTPBody?, URL) async throws -> (HTTPResponse, HTTPBody) // <<< changed
    ) async throws -> (HTTPResponse, HTTPBody) // <<< changed
}

Why does the middleware take an optional body but is forced to return a non-optional body? I can see that this is reflecting the existing OpenAPI.Request and OpenAPI.Response types which have Body? and Body properties respectively, but we should take this opportunity to check that's correct. Doesn't OpenAPI support defining a response without a body? AFAICT the OAS doesn't list the content map of the Response Object as required[1].

Relatedly, I can see the ServerTransport protocol does have provision for an optional response body:

public protocol ServerTransport {
    func register(
        _ handler: @Sendable @escaping (HTTPRequest, HTTPBody?, ServerRequestMetadata) async throws -> (HTTPResponse, HTTPBody?), // <<< changed
        method: HTTPRequest.Method, // <<< changed
        path: String // <<< changed
    ) throws
}

The same is true for ServerMiddleware.

  1. OpenAPI Specification v3.0.3 | Introduction, Definitions, & More ↩︎

1 Like
5

Yeah, @guscairo asked the same question on the PR, and I replied here. Also copying it below for posterity.

Just one question though: wouldn't it make sense to also make the HTTPBodys in ClientTransport and ClientMiddleware also optional?

Great question, I actually tried to make that change, unfortunately unlike with request bodies (which OpenAPI documents as optional, and semantically, a nil body is distinct from an empty body), response bodies are required, and if there's no documented response body, it's considered empty. There's no concept of a nil response body.

The reasoning seems to come from HTTP semantics, where GET and DELETE requests are not supposed to have request bodies, unlike other methods, where a request body is allowed. In contrast, response bodies are not discouraged per method by the HTTP spec, so there's no nullability to communicate.

Consider the client transport implementing this and trying to make a decision of whether to return an empty or nil body - how do you make that? You can't, as there's no way to know whether the server sent a "nil body" or an empty body. That, again, is in contrast to a server transport, which can reliably, for GET and DELETE requests, provide a nil body (no need to even instantiate an empty HTTPBody value) without looking at the wire, as even if there are bytes, servers are supposed to ignore them for these methods.

// Edit: However, there is the issue with HEAD requests, supported by OpenAPI, and required not to have a response body. So maybe the client responses should be nil after all as well, and transports would have a way to decide whether to send an empty body or a nil body.

6

@guscairo @beaumont Okay, discussed this more and let's make things consistent and make the response body optional as well. Changed in v1.2 (diff).

1 Like
7

The review period for this proposal is now over. The main piece of feedback was around making HTTPBodys on the client-side optional, which was applied.
For a complete picture of the discussed topics, you can refer to the previous comments on this thread, as well as the conversation on the PR.

The proposal is accepted. SOAR-0005 is now Ready for Implementation.

1 Like