From f869516815b97ac13c4e9a9fc3e0329f1e3ad4ad Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Sat, 20 Jun 2026 18:16:00 -0700 Subject: [PATCH] Unify emitter/consumer mediatype filter Closes: #2389 A response component exposing the same schema under multiple content types (e.g. application/json + application/xml) generated client wrapper fields typed as undefined per-content-type names (*ErrorResponseApplicationJSON / *ErrorResponseApplicationXML), breaking compilation. The type declarer (GenerateTypesForResponses) and the wrapper-field consumer (GetResponseTypeDefinitions) independently decided when to append a media-type suffix to a response type name, and the two had drifted apart: the consumer suffixed for every supported content type while the declarer only declared a suffixed name when a response carried more than one JSON content type. Both now route that decision through a single responseMediaTypeSuffix helper, so they cannot disagree. Non-JSON content types reuse the base type name, and JSON entries are suffixed only when a response has more than one JSON content type. The now-unused isMediaTypeSupported helper is removed, and a regression test is added under internal/test/issues/issue-2389. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/test/issues/issue-2389/api.gen.go | 602 ++++++++++++++++++ internal/test/issues/issue-2389/cfg.yaml | 6 + internal/test/issues/issue-2389/generate.go | 3 + .../test/issues/issue-2389/issue_2389_test.go | 57 ++ internal/test/issues/issue-2389/spec.yaml | 98 +++ pkg/codegen/codegen.go | 18 +- pkg/codegen/operations.go | 50 +- pkg/codegen/template_helpers.go | 22 - 8 files changed, 811 insertions(+), 45 deletions(-) create mode 100644 internal/test/issues/issue-2389/api.gen.go create mode 100644 internal/test/issues/issue-2389/cfg.yaml create mode 100644 internal/test/issues/issue-2389/generate.go create mode 100644 internal/test/issues/issue-2389/issue_2389_test.go create mode 100644 internal/test/issues/issue-2389/spec.yaml diff --git a/internal/test/issues/issue-2389/api.gen.go b/internal/test/issues/issue-2389/api.gen.go new file mode 100644 index 000000000..2f3678f3e --- /dev/null +++ b/internal/test/issues/issue-2389/api.gen.go @@ -0,0 +1,602 @@ +// Package issue2389 provides primitives to interact with the openapi HTTP API. +// +// Code generated by github.com/oapi-codegen/oapi-codegen/v2 version v2.0.0-00010101000000-000000000000 DO NOT EDIT. +package issue2389 + +import ( + "context" + "encoding/json" + "encoding/xml" + "fmt" + "io" + "net/http" + "net/url" + "strings" +) + +// ErrorResponse defines model for errorResponse. +type ErrorResponse struct { + Code *int32 `json:"code,omitempty"` + Message *string `json:"message,omitempty"` +} + +// JsonError defines model for jsonError. +type JsonError struct { + Code *int `json:"code,omitempty"` +} + +// XmlError defines model for xmlError. +type XmlError struct { + Reason *string `json:"reason,omitempty"` +} + +// MixedError defines model for mixedError. +type MixedError = JsonError + +// RequestEditorFn is the function signature for the RequestEditor callback function +type RequestEditorFn func(ctx context.Context, req *http.Request) error + +// Doer performs HTTP requests. +// +// The standard http.Client implements this interface. +type HttpRequestDoer interface { + Do(req *http.Request) (*http.Response, error) +} + +// Client which conforms to the OpenAPI3 specification for this service. +type Client struct { + // The endpoint of the server conforming to this interface, with scheme, + // https://api.deepmap.com for example. This can contain a path relative + // to the server, such as https://api.deepmap.com/dev-test, and all the + // paths in the swagger spec will be appended to the server. + Server string + + // Doer for performing requests, typically a *http.Client with any + // customized settings, such as certificate chains. + Client HttpRequestDoer + + // A list of callbacks for modifying requests which are generated before sending over + // the network. + RequestEditors []RequestEditorFn +} + +// ClientOption allows setting custom parameters during construction +type ClientOption func(*Client) error + +// Creates a new Client, with reasonable defaults +func NewClient(server string, opts ...ClientOption) (*Client, error) { + // create a client with sane default values + client := Client{ + Server: server, + } + // mutate client and add all optional params + for _, o := range opts { + if err := o(&client); err != nil { + return nil, err + } + } + // ensure the server URL always has a trailing slash + if !strings.HasSuffix(client.Server, "/") { + client.Server += "/" + } + // create httpClient, if not already present + if client.Client == nil { + client.Client = &http.Client{} + } + return &client, nil +} + +// WithHTTPClient allows overriding the default Doer, which is +// automatically created using http.Client. This is useful for tests. +func WithHTTPClient(doer HttpRequestDoer) ClientOption { + return func(c *Client) error { + c.Client = doer + return nil + } +} + +// WithRequestEditorFn allows setting up a callback function, which will be +// called right before sending the request. This can be used to mutate the request. +func WithRequestEditorFn(fn RequestEditorFn) ClientOption { + return func(c *Client) error { + c.RequestEditors = append(c.RequestEditors, fn) + return nil + } +} + +// The interface specification for the client above. +type ClientInterface interface { + + // GetMixed performs a GET /mixed (the `GetMixed` operationId) request. + GetMixed(ctx context.Context, reqEditors ...RequestEditorFn) (*http.Response, error) + + // GetThing performs a GET /thing (the `GetThing` operationId) request. + GetThing(ctx context.Context, reqEditors ...RequestEditorFn) (*http.Response, error) + + // GetXMLOnly performs a GET /xml-only (the `GetXMLOnly` operationId) request. + GetXMLOnly(ctx context.Context, reqEditors ...RequestEditorFn) (*http.Response, error) +} + +// GetMixed performs a GET /mixed (the `GetMixed` operationId) request. +func (c *Client) GetMixed(ctx context.Context, reqEditors ...RequestEditorFn) (*http.Response, error) { + req, err := NewGetMixedRequest(c.Server) + if err != nil { + return nil, err + } + req = req.WithContext(ctx) + if err := c.applyEditors(ctx, req, reqEditors); err != nil { + return nil, err + } + return c.Client.Do(req) +} + +// GetThing performs a GET /thing (the `GetThing` operationId) request. +func (c *Client) GetThing(ctx context.Context, reqEditors ...RequestEditorFn) (*http.Response, error) { + req, err := NewGetThingRequest(c.Server) + if err != nil { + return nil, err + } + req = req.WithContext(ctx) + if err := c.applyEditors(ctx, req, reqEditors); err != nil { + return nil, err + } + return c.Client.Do(req) +} + +// GetXMLOnly performs a GET /xml-only (the `GetXMLOnly` operationId) request. +func (c *Client) GetXMLOnly(ctx context.Context, reqEditors ...RequestEditorFn) (*http.Response, error) { + req, err := NewGetXMLOnlyRequest(c.Server) + if err != nil { + return nil, err + } + req = req.WithContext(ctx) + if err := c.applyEditors(ctx, req, reqEditors); err != nil { + return nil, err + } + return c.Client.Do(req) +} + +// NewGetMixedRequest constructs an http.Request for the GetMixed method +func NewGetMixedRequest(server string) (*http.Request, error) { + var err error + + serverURL, err := url.Parse(server) + if err != nil { + return nil, err + } + + operationPath := fmt.Sprintf("/mixed") + if operationPath[0] == '/' { + operationPath = "." + operationPath + } + + queryURL, err := serverURL.Parse(operationPath) + if err != nil { + return nil, err + } + + req, err := http.NewRequest(http.MethodGet, queryURL.String(), nil) + if err != nil { + return nil, err + } + + return req, nil +} + +// NewGetThingRequest constructs an http.Request for the GetThing method +func NewGetThingRequest(server string) (*http.Request, error) { + var err error + + serverURL, err := url.Parse(server) + if err != nil { + return nil, err + } + + operationPath := fmt.Sprintf("/thing") + if operationPath[0] == '/' { + operationPath = "." + operationPath + } + + queryURL, err := serverURL.Parse(operationPath) + if err != nil { + return nil, err + } + + req, err := http.NewRequest(http.MethodGet, queryURL.String(), nil) + if err != nil { + return nil, err + } + + return req, nil +} + +// NewGetXMLOnlyRequest constructs an http.Request for the GetXMLOnly method +func NewGetXMLOnlyRequest(server string) (*http.Request, error) { + var err error + + serverURL, err := url.Parse(server) + if err != nil { + return nil, err + } + + operationPath := fmt.Sprintf("/xml-only") + if operationPath[0] == '/' { + operationPath = "." + operationPath + } + + queryURL, err := serverURL.Parse(operationPath) + if err != nil { + return nil, err + } + + req, err := http.NewRequest(http.MethodGet, queryURL.String(), nil) + if err != nil { + return nil, err + } + + return req, nil +} + +func (c *Client) applyEditors(ctx context.Context, req *http.Request, additionalEditors []RequestEditorFn) error { + for _, r := range c.RequestEditors { + if err := r(ctx, req); err != nil { + return err + } + } + for _, r := range additionalEditors { + if err := r(ctx, req); err != nil { + return err + } + } + return nil +} + +// ClientWithResponses builds on ClientInterface to offer response payloads +type ClientWithResponses struct { + ClientInterface +} + +// NewClientWithResponses creates a new ClientWithResponses, which wraps +// Client with return type handling +func NewClientWithResponses(server string, opts ...ClientOption) (*ClientWithResponses, error) { + client, err := NewClient(server, opts...) + if err != nil { + return nil, err + } + return &ClientWithResponses{client}, nil +} + +// WithBaseURL overrides the baseURL. +func WithBaseURL(baseURL string) ClientOption { + return func(c *Client) error { + newBaseURL, err := url.Parse(baseURL) + if err != nil { + return err + } + c.Server = newBaseURL.String() + return nil + } +} + +// ClientWithResponsesInterface is the interface specification for the client with responses above. +type ClientWithResponsesInterface interface { + + // GetMixedWithResponse performs a GET /mixed (the `GetMixed` operationId) request. + // + // Returns a wrapper object for the known response body format(s). + GetMixedWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetMixedResponse, error) + + // GetThingWithResponse performs a GET /thing (the `GetThing` operationId) request. + // + // Returns a wrapper object for the known response body format(s). + GetThingWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetThingResponse, error) + + // GetXMLOnlyWithResponse performs a GET /xml-only (the `GetXMLOnly` operationId) request. + // + // Returns a wrapper object for the known response body format(s). + GetXMLOnlyWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetXMLOnlyResponse, error) +} + +type GetMixedResponse struct { + Body []byte + HTTPResponse *http.Response + // JSON200 the response for an HTTP 200 `application/json` response + JSON200 *string + // JSON500 the response for an HTTP 500 `application/json` response + JSON500 *MixedError + // XML500 the response for an HTTP 500 `application/xml` response + XML500 *XmlError +} + +// GetJSON200 returns the response for an HTTP 200 `application/json` response +func (r GetMixedResponse) GetJSON200() *string { + return r.JSON200 +} + +// GetJSON500 returns the response for an HTTP 500 `application/json` response +func (r GetMixedResponse) GetJSON500() *MixedError { + return r.JSON500 +} + +// GetXML500 returns the response for an HTTP 500 `application/xml` response +func (r GetMixedResponse) GetXML500() *XmlError { + return r.XML500 +} + +// GetBody returns the raw response body bytes +func (r GetMixedResponse) GetBody() []byte { + return r.Body +} + +// Status returns HTTPResponse.Status +func (r GetMixedResponse) Status() string { + if r.HTTPResponse != nil { + return r.HTTPResponse.Status + } + return http.StatusText(0) +} + +// StatusCode returns HTTPResponse.StatusCode +func (r GetMixedResponse) StatusCode() int { + if r.HTTPResponse != nil { + return r.HTTPResponse.StatusCode + } + return 0 +} + +// ContentType is a convenience method to retrieve the Content-Type value from the HTTP response headers +func (r GetMixedResponse) ContentType() string { + if r.HTTPResponse != nil { + return r.HTTPResponse.Header.Get("Content-Type") + } + return "" +} + +type GetThingResponse struct { + Body []byte + HTTPResponse *http.Response + // JSON200 the response for an HTTP 200 `application/json` response + JSON200 *string + // JSON500 the response for an HTTP 500 `application/json` response + JSON500 *ErrorResponse + // XML500 the response for an HTTP 500 `application/xml` response + XML500 *ErrorResponse +} + +// GetJSON200 returns the response for an HTTP 200 `application/json` response +func (r GetThingResponse) GetJSON200() *string { + return r.JSON200 +} + +// GetJSON500 returns the response for an HTTP 500 `application/json` response +func (r GetThingResponse) GetJSON500() *ErrorResponse { + return r.JSON500 +} + +// GetXML500 returns the response for an HTTP 500 `application/xml` response +func (r GetThingResponse) GetXML500() *ErrorResponse { + return r.XML500 +} + +// GetBody returns the raw response body bytes +func (r GetThingResponse) GetBody() []byte { + return r.Body +} + +// Status returns HTTPResponse.Status +func (r GetThingResponse) Status() string { + if r.HTTPResponse != nil { + return r.HTTPResponse.Status + } + return http.StatusText(0) +} + +// StatusCode returns HTTPResponse.StatusCode +func (r GetThingResponse) StatusCode() int { + if r.HTTPResponse != nil { + return r.HTTPResponse.StatusCode + } + return 0 +} + +// ContentType is a convenience method to retrieve the Content-Type value from the HTTP response headers +func (r GetThingResponse) ContentType() string { + if r.HTTPResponse != nil { + return r.HTTPResponse.Header.Get("Content-Type") + } + return "" +} + +type GetXMLOnlyResponse struct { + Body []byte + HTTPResponse *http.Response + // JSON200 the response for an HTTP 200 `application/json` response + JSON200 *string + // XML500 the response for an HTTP 500 `application/xml` response + XML500 *XmlError +} + +// GetJSON200 returns the response for an HTTP 200 `application/json` response +func (r GetXMLOnlyResponse) GetJSON200() *string { + return r.JSON200 +} + +// GetXML500 returns the response for an HTTP 500 `application/xml` response +func (r GetXMLOnlyResponse) GetXML500() *XmlError { + return r.XML500 +} + +// GetBody returns the raw response body bytes +func (r GetXMLOnlyResponse) GetBody() []byte { + return r.Body +} + +// Status returns HTTPResponse.Status +func (r GetXMLOnlyResponse) Status() string { + if r.HTTPResponse != nil { + return r.HTTPResponse.Status + } + return http.StatusText(0) +} + +// StatusCode returns HTTPResponse.StatusCode +func (r GetXMLOnlyResponse) StatusCode() int { + if r.HTTPResponse != nil { + return r.HTTPResponse.StatusCode + } + return 0 +} + +// ContentType is a convenience method to retrieve the Content-Type value from the HTTP response headers +func (r GetXMLOnlyResponse) ContentType() string { + if r.HTTPResponse != nil { + return r.HTTPResponse.Header.Get("Content-Type") + } + return "" +} + +// GetMixedWithResponse performs a GET /mixed (the `GetMixed` operationId) request. +// +// Returns a wrapper object for the known response body format(s). +func (c *ClientWithResponses) GetMixedWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetMixedResponse, error) { + rsp, err := c.GetMixed(ctx, reqEditors...) + if err != nil { + return nil, err + } + return ParseGetMixedResponse(rsp) +} + +// GetThingWithResponse performs a GET /thing (the `GetThing` operationId) request. +// +// Returns a wrapper object for the known response body format(s). +func (c *ClientWithResponses) GetThingWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetThingResponse, error) { + rsp, err := c.GetThing(ctx, reqEditors...) + if err != nil { + return nil, err + } + return ParseGetThingResponse(rsp) +} + +// GetXMLOnlyWithResponse performs a GET /xml-only (the `GetXMLOnly` operationId) request. +// +// Returns a wrapper object for the known response body format(s). +func (c *ClientWithResponses) GetXMLOnlyWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetXMLOnlyResponse, error) { + rsp, err := c.GetXMLOnly(ctx, reqEditors...) + if err != nil { + return nil, err + } + return ParseGetXMLOnlyResponse(rsp) +} + +// ParseGetMixedResponse parses an HTTP response from a GetMixedWithResponse call +func ParseGetMixedResponse(rsp *http.Response) (*GetMixedResponse, error) { + bodyBytes, err := io.ReadAll(rsp.Body) + defer func() { _ = rsp.Body.Close() }() + if err != nil { + return nil, err + } + + response := &GetMixedResponse{ + Body: bodyBytes, + HTTPResponse: rsp, + } + + switch { + case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 200: + var dest string + if err := json.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.JSON200 = &dest + + case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 500: + var dest MixedError + if err := json.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.JSON500 = &dest + + case strings.Contains(rsp.Header.Get("Content-Type"), "xml") && rsp.StatusCode == 500: + var dest XmlError + if err := xml.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.XML500 = &dest + + } + + return response, nil +} + +// ParseGetThingResponse parses an HTTP response from a GetThingWithResponse call +func ParseGetThingResponse(rsp *http.Response) (*GetThingResponse, error) { + bodyBytes, err := io.ReadAll(rsp.Body) + defer func() { _ = rsp.Body.Close() }() + if err != nil { + return nil, err + } + + response := &GetThingResponse{ + Body: bodyBytes, + HTTPResponse: rsp, + } + + switch { + case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 200: + var dest string + if err := json.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.JSON200 = &dest + + case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 500: + var dest ErrorResponse + if err := json.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.JSON500 = &dest + + case strings.Contains(rsp.Header.Get("Content-Type"), "xml") && rsp.StatusCode == 500: + var dest ErrorResponse + if err := xml.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.XML500 = &dest + + } + + return response, nil +} + +// ParseGetXMLOnlyResponse parses an HTTP response from a GetXMLOnlyWithResponse call +func ParseGetXMLOnlyResponse(rsp *http.Response) (*GetXMLOnlyResponse, error) { + bodyBytes, err := io.ReadAll(rsp.Body) + defer func() { _ = rsp.Body.Close() }() + if err != nil { + return nil, err + } + + response := &GetXMLOnlyResponse{ + Body: bodyBytes, + HTTPResponse: rsp, + } + + switch { + case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 200: + var dest string + if err := json.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.JSON200 = &dest + + case strings.Contains(rsp.Header.Get("Content-Type"), "xml") && rsp.StatusCode == 500: + var dest XmlError + if err := xml.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.XML500 = &dest + + } + + return response, nil +} diff --git a/internal/test/issues/issue-2389/cfg.yaml b/internal/test/issues/issue-2389/cfg.yaml new file mode 100644 index 000000000..451598c8f --- /dev/null +++ b/internal/test/issues/issue-2389/cfg.yaml @@ -0,0 +1,6 @@ +# yaml-language-server: $schema=../../../../configuration-schema.json +package: issue2389 +output: api.gen.go +generate: + models: true + client: true diff --git a/internal/test/issues/issue-2389/generate.go b/internal/test/issues/issue-2389/generate.go new file mode 100644 index 000000000..7fcecf8a2 --- /dev/null +++ b/internal/test/issues/issue-2389/generate.go @@ -0,0 +1,3 @@ +package issue2389 + +//go:generate go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen --config=cfg.yaml spec.yaml diff --git a/internal/test/issues/issue-2389/issue_2389_test.go b/internal/test/issues/issue-2389/issue_2389_test.go new file mode 100644 index 000000000..19a85954b --- /dev/null +++ b/internal/test/issues/issue-2389/issue_2389_test.go @@ -0,0 +1,57 @@ +package issue2389 + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// When a components/responses entry exposes the same schema under more than one +// content type (here application/json + application/xml), the client response +// wrapper grows one field per content type (JSON500, XML500). The regression in +// issue #2389 typed those fields as undefined per-content-type names +// (*ErrorResponseApplicationJSON / *ErrorResponseApplicationXML), so the +// generated package failed to compile. Both fields must instead point at the +// single declared component type, ErrorResponse. +// +// This test exists primarily to prove the generated package compiles; the +// assignments below would not type-check if either field had a different (or +// undefined) type. +func TestResponseComponentMultipleContentTypesShareDeclaredType(t *testing.T) { + body := &ErrorResponse{} + + resp := GetThingResponse{ + JSON500: body, + XML500: body, + } + + assert.Same(t, resp.JSON500, resp.XML500) +} + +// A component response only declares a Go type for its JSON content, so an +// XML-only component response has no declared base type. The wrapper field must +// point at the XML content's own schema type (XmlError); pointing at the +// component base type would reference an undeclared name and fail to compile. +func TestResponseComponentXMLOnlyUsesContentSchemaType(t *testing.T) { + resp := GetXMLOnlyResponse{ + XML500: &XmlError{}, + } + + assert.NotNil(t, resp.XML500) +} + +// When the JSON and XML content of a component response resolve to different +// schemas, the two wrapper fields must keep distinct types: JSON uses the +// declared component type (MixedError) and XML keeps its own schema type +// (XmlError). The regression shared the JSON type for both, which would +// silently decode XML into the JSON-shaped type. These statements would not +// type-check if the fields shared a type. +func TestResponseComponentMixedDifferingSchemasKeepDistinctTypes(t *testing.T) { + resp := GetMixedResponse{ + JSON500: &MixedError{}, + XML500: &XmlError{}, + } + + assert.NotNil(t, resp.JSON500) + assert.NotNil(t, resp.XML500) +} diff --git a/internal/test/issues/issue-2389/spec.yaml b/internal/test/issues/issue-2389/spec.yaml new file mode 100644 index 000000000..f486043ca --- /dev/null +++ b/internal/test/issues/issue-2389/spec.yaml @@ -0,0 +1,98 @@ +openapi: "3.0.0" +info: + title: issue-2389 + version: 1.0.0 +paths: + /thing: + get: + operationId: getThing + responses: + "200": + description: ok + content: + application/json: + schema: + type: string + "500": + $ref: '#/components/responses/error' + /xml-only: + get: + operationId: getXMLOnly + responses: + "200": + description: ok + content: + application/json: + schema: + type: string + "500": + $ref: '#/components/responses/xmlOnlyError' + /mixed: + get: + operationId: getMixed + responses: + "200": + description: ok + content: + application/json: + schema: + type: string + "500": + $ref: '#/components/responses/mixedError' +components: + schemas: + errorResponse: + type: object + properties: + code: + type: integer + format: int32 + message: + type: string + jsonError: + type: object + properties: + code: + type: integer + xmlError: + type: object + properties: + reason: + type: string + responses: + error: + description: Error response + x-go-name: ErrorResponse + # A single response component exposing the same schema under more than + # one content type. Both content types must reference the one declared + # Go type (ErrorResponse); the regression generated undefined + # per-content-type names (ErrorResponseApplicationJSON / ...XML). + content: + application/json: + schema: + $ref: '#/components/schemas/errorResponse' + application/xml: + schema: + $ref: '#/components/schemas/errorResponse' + xmlOnlyError: + description: XML-only response component + # No JSON content. GenerateTypesForResponses only declares a Go type for + # JSON media, so the wrapper field must point at the XML content's own + # schema type (XmlError), not the (never-declared) component base type. + content: + application/xml: + schema: + $ref: '#/components/schemas/xmlError' + mixedError: + description: JSON and XML content with different schemas + # The JSON and XML content types resolve to different schemas. The JSON + # field uses the declared component type; the XML field must keep its own + # schema type (XmlError) rather than silently decoding XML into the JSON + # type. + content: + application/json: + schema: + $ref: '#/components/schemas/jsonError' + application/xml: + schema: + $ref: '#/components/schemas/xmlError' diff --git a/pkg/codegen/codegen.go b/pkg/codegen/codegen.go index 7b890d3a0..51b009d9d 100644 --- a/pkg/codegen/codegen.go +++ b/pkg/codegen/codegen.go @@ -1094,15 +1094,9 @@ func GenerateTypesForResponses(t *template.Template, responses openapi3.Response // handle media types that conform to JSON. Other responses should // simply be specified as strings or byte arrays. response := responseOrRef.Value + content := response.Content - jsonCount := 0 - for mediaType := range response.Content { - if util.IsMediaTypeJson(mediaType) { - jsonCount++ - } - } - - SortedMapKeys := SortedMapKeys(response.Content) + SortedMapKeys := SortedMapKeys(content) for _, mediaType := range SortedMapKeys { response := response.Content[mediaType] if !util.IsMediaTypeJson(mediaType) { @@ -1121,8 +1115,8 @@ func GenerateTypesForResponses(t *template.Template, responses openapi3.Response // TODO: revisit this at the next major version change — // always include the media type in the schema path. schemaPath := []string{responseName} - if jsonCount > 1 && globalState.options.OutputOptions.ResolveTypeNameCollisions { - schemaPath = append(schemaPath, mediaTypeToCamelCase(mediaType)) + if suffix := responseMediaTypeSuffix(content, mediaType); suffix != "" && globalState.options.OutputOptions.ResolveTypeNameCollisions { + schemaPath = append(schemaPath, suffix) } goType, err := GenerateGoSchema(response.Schema, schemaPath) if err != nil { @@ -1153,8 +1147,8 @@ func GenerateTypesForResponses(t *template.Template, responses openapi3.Response typeDef.TypeName = SchemaNameToTypeName(refType) } - if jsonCount > 1 { - typeDef.TypeName = typeDef.TypeName + mediaTypeToCamelCase(mediaType) + if suffix := responseMediaTypeSuffix(content, mediaType); suffix != "" { + typeDef.TypeName = typeDef.TypeName + suffix } types = append(types, typeDef) diff --git a/pkg/codegen/operations.go b/pkg/codegen/operations.go index 3a150b7e4..b1de0f94f 100644 --- a/pkg/codegen/operations.go +++ b/pkg/codegen/operations.go @@ -565,6 +565,33 @@ func (o *OperationDefinition) DeprecationComment() string { return DeprecationComment(reason) } +// responseMediaTypeSuffix returns the media-type discriminator suffix (e.g. +// "ApplicationJSON") that must be appended to the Go type name generated for a +// single content entry of a response, or "" when the base name is used as-is. +// +// A suffix is required only when one response carries more than one +// JSON-compatible content type, and only the JSON entries receive one: non-JSON +// media types (XML, YAML, ...) never get a dedicated generated type, so they +// reuse the base name. Both the type declaration (GenerateTypesForResponses) and +// the client response-wrapper field types (GetResponseTypeDefinitions) must make +// this identical decision — otherwise the wrapper references per-content-type +// type names that were never declared (see issue #2389). +func responseMediaTypeSuffix(content openapi3.Content, mediaType string) string { + if !util.IsMediaTypeJson(mediaType) { + return "" + } + jsonCount := 0 + for mt := range content { + if util.IsMediaTypeJson(mt) { + jsonCount++ + } + } + if jsonCount <= 1 { + return "" + } + return mediaTypeToCamelCase(mediaType) +} + // GetResponseTypeDefinitions produces a list of type definitions for a given Operation for the response // types which we know how to parse. These will be turned into fields on a // response object for automatic deserialization of responses in the generated @@ -582,13 +609,6 @@ func (o *OperationDefinition) GetResponseTypeDefinitions() ([]ResponseTypeDefini // We can only generate a type if we have a value: if responseRef.Value != nil { - supportedCount := 0 - for mediaType := range responseRef.Value.Content { - if isMediaTypeSupported(mediaType) { - supportedCount++ - } - } - sortedContentKeys := SortedMapKeys(responseRef.Value.Content) for _, contentTypeName := range sortedContentKeys { contentType := responseRef.Value.Content[contentTypeName] @@ -662,16 +682,24 @@ func (o *OperationDefinition) GetResponseTypeDefinitions() ([]ResponseTypeDefini ContentTypeName: contentTypeName, AdditionalTypeDefinitions: responseSchema.GetAdditionalTypeDefs(), } - if IsGoTypeReference(responseRef.Ref) { + // A component response only declares a Go type for its JSON + // content (GenerateTypesForResponses skips non-JSON media). + // Point the wrapper field at that declared component type for + // JSON content only; non-JSON content keeps the type derived + // from its own schema above, so it neither references an + // undeclared base type (e.g. an XML-only component response) + // nor silently decodes into the JSON type when the JSON and + // non-JSON schemas differ. + if IsGoTypeReference(responseRef.Ref) && util.IsMediaTypeJson(contentTypeName) { refType, err := RefPathToGoType(responseRef.Ref) if err != nil { return nil, fmt.Errorf("error dereferencing response Ref: %w", err) } - if supportedCount > 1 { + if suffix := responseMediaTypeSuffix(responseRef.Value.Content, contentTypeName); suffix != "" { if resolved := resolvedNameForRefPath(responseRef.Ref, contentTypeName); resolved != "" { - refType = resolved + mediaTypeToCamelCase(contentTypeName) + refType = resolved + suffix } else { - refType += mediaTypeToCamelCase(contentTypeName) + refType += suffix } } td.Schema.RefType = refType diff --git a/pkg/codegen/template_helpers.go b/pkg/codegen/template_helpers.go index a72d2911c..f19a79dd3 100644 --- a/pkg/codegen/template_helpers.go +++ b/pkg/codegen/template_helpers.go @@ -47,28 +47,6 @@ var ( titleCaser = cases.Title(language.English) ) -// isMediaTypeSupported reports whether code generation produces a typed -// body for this media type. Today this is the closed set of JSON / YAML / -// XML variants the response and request templates know how to handle — -// see the typeName switch in GetResponseTypeDefinitions and the body -// definition switch in GenerateBodyDefinitions. A future configuration -// option is intended to let users extend this list. -func isMediaTypeSupported(mediaType string) bool { - switch { - case slices.Contains(contentTypesHalJSON, mediaType): - return true - case slices.Contains(contentTypesJSON, mediaType): - return true - case util.IsMediaTypeJson(mediaType): - return true - case slices.Contains(contentTypesYAML, mediaType): - return true - case slices.Contains(contentTypesXML, mediaType): - return true - } - return false -} - // genParamArgs takes an array of Parameter definition, and generates a valid // Go parameter declaration from them, eg: // ", foo int, bar string, baz float32". The preceding comma is there to save