Bugfix to solve mixed correct and error response in produces annotation (#24948) - #24949
Open
AntoineDuComptoirDesPharmacies wants to merge 3 commits into
Open
AntoineDuComptoirDesPharmacies wants to merge 3 commits into
AntoineDuComptoirDesPharmacies wants to merge 3 commits into
Conversation
…pt to its own media-type splitOperationsByContentType narrows a variant to one media-type on its success response, but the other responses - errors, typically JSON - are left as they are, so produces, the union of every response's content, still listed them all. Every generator derives the Accept it sends from that list with a rule that knows nothing about variants: the Java client and Python prefer JSON, Spring Cloud's Feign contract takes the first one. On a spec with JSON errors the CSV variant of a json|csv operation therefore asked for JSON, or the JSON one for CSV - the very defect the split exists to remove, back through the Accept header. produces of a variant is now read from its method response alone, the one the split narrowed, and getProducesInfo returns that single media-type; the error responses are untouched and keep typing their body. Operations the split leaves alone are unchanged: their produces is still the union of every response. consumes needed nothing, the split already narrows the request body it is read from. The Java generators compute x-accepts and x-content-type themselves, in preprocessOpenAPI, before the split: a variant inherited them from the operation it came from. AbstractJavaCodegen now overrides divideOperationsByContentType and stamps the variants it returns, each with its own media-types; every other operation keeps the value preprocessOpenAPI computed, which the Java generators also export in the openapi.yaml they serialize. Computing the extensions in fromOperation alone was tried and dropped: the export order changes and x-internal operations never reach it, which touched 48 samples. rust-server-deprecated flagged usesXml from the operation's produces, which no longer lists an XML error response of a variant: the flag is set where the response is tagged x-produces-xml, as the maintained rust-server does. typescript-fetch needs nothing: it reads the media-types from the variant extensions, and its split sample regenerates byte for byte. The docs state the consequence for a server that negotiates strictly on Accept, and no longer claim every generator sends the narrowed Accept (kotlin-client filters produces down to what it can deserialise).
…e the extension names contentTypeVariantMediaType checks the variant group itself: isContentTypeVariant had no other caller. addContentTypeExtensions names x-accepts and x-content-type through VendorExtension, as getSupportedVendorExtensions already does, and the test helper reads the media-type key through the constant the class declares.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
A variant split on the request axis alone keeps the operation's produces, error responses included, as any operation that was not split; the docs promised a single media-type on both axes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #24948. Follow-up to #23935.
With splitOperationsByContentType=true, a variant generated for a non-JSON media-type (getReportAsCsv, typed on text/csv) still sent Accept: application/json whenever the operation also declared a JSON error response: produces is the union of every response's content, and every generator picks the Accept from that list with a rule that prefers JSON. Server generators had the mirror problem: two variants of the same route both declared application/json in produces, an ambiguous mapping.
A variant's produces is now the single media-type it was narrowed to, nothing else:
Design note for reviewers: a Spring server variant declares only its own media-type in produces; that is what lets Spring route on Accept without an ambiguous mapping, and JSON error bodies still go through @ExceptionHandler, negotiated on the client's Accept. Opinions welcome.
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
@wing328
@cachescrubber (2022/02) @welshm (2022/02) @MelleD (2022/02) @atextor (2022/02) @manedev79 (2022/02) @javisst (2022/02) @borsch (2022/02) @banlevente (2022/02) @Zomzog (2022/09) @martin-mfg (2023/08) @KannaKim (2026/07)
Summary by cubic
Fixes #24948, a follow-up to #23935. With
splitOperationsByContentType=true, a variant narrowed on the response axis still sentAccept: application/jsonwhen the operation also declared a JSON error response, becauseproduceswas the union of all response media types. Such variants now produce only the media type they were narrowed to; error responses keep their own content and typing. Variants split only on the request axis keep the operation's originalproduces, error responses included.Java generators restamp
x-acceptsandx-content-typeafter the split, so each variant carries its own media types instead of inheriting the original operation's.rust-server-deprecatedsetsusesXmlper XML response. Unsplit operations generate byte-identical output, since the fix is limited to the opt-in split path.Written for commit d81a4f9. Summary will update on new commits.