Skip to content

Conversation

@brandonbloom
Copy link

Motivation

Follow-up to #860.

responses.content['*/*'] is often used to represent a dynamic/unconstrained payload where the server chooses the actual response Content-Type at runtime.

PR #860 fixes the immediate incorrect behavior by skipping Accept validation and not emitting Content-Type: */*.

This PR takes the next step by allowing the handler to provide dynamic response headers (especially Content-Type) for the */* case by modeling the any body as OpenAPIRuntime.UndocumentedPayload (which carries headerFields + body). This is source-breaking for generated APIs that used any(HTTPBody).

Related: #859 (kept open; this improves dynamic header plumbing but there may still be follow-up design discussion).

Modifications

  • Generate the */* response body case as case any(OpenAPIRuntime.UndocumentedPayload).
  • Update generated server serializer for the */* case to:
    • skip validateAcceptIfPresent,
    • append value.headerFields onto the response headers, and
    • require value.body (throws RuntimeError.missingRequiredResponseBody if absent).
  • Update generated client deserializer for the */* case to wrap the received HTTPBody and response.headerFields into UndocumentedPayload.
  • Update the snippet-based regression test accordingly.

Result

Generated servers can return */* responses with a real, runtime-chosen Content-Type (and other headers) without middleware hacks, while still avoiding Accept validation and avoiding Content-Type: */*.

Test Plan

  • swift test

@czechboy0
Copy link
Contributor

I like the shape of this solution. Some more thoughts:

  1. We'll need to put this new behavior behind a config, allowing adopters to opt into the source break. We already have that ability in openapi-generator-config.yaml, and we'd make this a feature flag, since presumably we'd default to this behavior in a future major.
  2. I'm not loving reusing UndocumentedPayload, because we do have more static information about this payload - it's a byte stream with a known concrete content type. So I wonder if adding a new type (or reusing OpenAPIMIMEType, but that might not be appropriate since it allows wildcards, whereas here we statically know we want a concrete type), e.g. called OpenAPIConcreteMIMEType (naming suggestions welcome) that has type and subtype string properties would be the right spelling here. That'll force, at compile time, adopters to pass in both the HTTPBody and a concrete MIME type.

What do you think @brandonbloom (and also @simonjbeaumont)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants