feat(core): add splitOperationsByContentType option to divide operati… - #23935
Conversation
…ons by content-type (OpenAPITools#6708) OpenAPI lets a single operation expose several request/response content-types with different schemas, but generators keep only the first one ("Multiple schemas found in the OAS 'content' section, returning only the first"). This yields a single, mistyped method and makes the other content-types unusable. Add an opt-in option `splitOperationsByContentType` (default false) that divides such an operation into one operation per content-type — the cartesian product of the request-body and success-response content-types (deduplicated by schema). The division happens at the spec level in DefaultCodegen#preprocessOpenAPI. Each variant is a narrowed swagger Operation (single content-type on each axis) with a typed, collision-free operationId (request -> "With<Subtype>", response -> "As<Subtype>", e.g. createReportWithXmlAsPdf). Variants are stored on the original operation under the "x-content-type-variants" extension and expanded by DefaultGenerator#processOperation, so every variant re-enters fromOperation and is typed natively by the target generator. The feature is therefore language-neutral: no per-language type re-derivation, no template change. Tested in DefaultCodegenTest for both the request×response cartesian (4 variants) and the response-only case (2 variants), and that unambiguous operations are left untouched.
Move the clone methods to ModelUtils to lightweight DefaultCodegen
Add new split operations option in every providers
Build project and update samples
Other solution without X variant and using divide directly while processing operations
Simplification of code, removing useless "findMultiSchemaSuccessResponseCode"
There was a problem hiding this comment.
No issues found across 138 files
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
Remove default implemenation in Interface. No sense because the default implementation is in CodegenDefault
|
please PM me via Slack to further discuss this to move this PR forward when you've time. https://join.slack.com/t/openapi-generator/shared_invite/zt-36ucx4ybl-jYrN6euoYn6zxXNZdldoZA |
…ty, merged back per generator Following the review on OpenAPITools#23935: the option was a CLI option repeated in every generator that wanted it. It is now a global property, read once from GlobalSettings in DefaultCodegen.processOpts, like skipFormModel already is. DefaultGenerator.processOperation asks the config to divide an operation before processing it, and DefaultCodegen implements the division once, language neutrally: an operation whose request body and/or method response expose several content-types with different schemas becomes one operation per (request, response) content-type pair, each narrowed to a single media-type and given a typed, collision-free operationId. Every variant re-enters fromOperation, so its body and return types are resolved natively by the target generator - the shared code re-derives no types of its own. Each variant also carries x-content-type-variant-* extensions recording where it sits in the matrix, so a generator able to express the whole matrix in a single construct can merge the variants back instead of emitting one method per combination. typescript-fetch does: the variants collapse into one method whose request type is a union discriminated by `contentType`, and whose return type is picked by overloads on `accept`, each branch keeping the types the split resolved for it. A generator that does not merge simply gets the separate methods, which is what a statically-typed language needs anyway. With the option off, divideOperationsByContentType returns the operation as a singleton and the merge returns immediately: regenerating the 19 typescript-fetch sample configs produces a byte-identical tree.
No textual conflict. Verified against the merged tree: the full openapi-generator test suite passes (4335 tests), regenerating the 19 typescript-fetch sample configs leaves them byte-identical to master, and the issue6708-* specs generated with splitOperationsByContentType=true still compile under tsc --strict. Also folded in here: TypeScriptFetchClientCodegen's LOGGER was declared static, which ArchUnitRulesTest forbids for codegen classes. It is now a plain instance field, like the other TypeScript generators declare theirs.
Three conflicts, all on files this branch owns: - the two static imports simply coexist; - addMultipartFileArrayApiExampleValues, new upstream, runs before mergeContentTypeVariants. The merge has to stay last: it drops the non-default variants from the operation list while the merged operation keeps referencing their parameters and return types, so every other pass must have seen them first; - apis.mustache's form-params block now lives in apisFormParams.mustache, shared by the legacy path and the content-type switch. Upstream's change to that block (isFreeFormObject -> runtime.anyToJSON) was carried into the partial, which stays a verbatim copy of upstream's text at the method's indentation - that is what keeps the legacy path byte-identical. Verified on the merged tree: the full openapi-generator test suite passes (4676 tests), regenerating the 19 typescript-fetch sample configs leaves them byte-identical to upstream/master, and the issue6708-* specs generated with splitOperationsByContentType=true compile under tsc --strict.
…option on Nothing committed showed what splitOperationsByContentType emits, and nothing in CI compiled it: the option was exercised only by unit tests asserting on strings in a temp directory. Reviewers had to build the branch to see the feature, and a regression that produced uncompilable TypeScript would have gone unnoticed - the review of this branch found four of those. The sample's spec gathers the shapes the option has to handle: a response-only split, a split on both axes, a multipart body whose operation is split on the response axis only, a request split mixing JSON and multipart, and an enum parameter carried by a split operation. bin/ts-typecheck-all.sh discovers samples on its own - any generated directory holding both a tsconfig.json and a package.json is typechecked - so setting npmName is all it takes for CI to compile this one. No workflow change needed. Writing the sample immediately paid for itself: the form body assembled inside the content-type switch was under-indented by eight columns. IndentedLambda leaves the first line alone, and moving the partial to column zero in the previous commit removed the literal spaces that used to indent it.
There was a problem hiding this comment.
All reported issues were addressed across 30 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…mple Three of the four points raised were about the sample, which is what the sample is for - the output is readable now. A media type is case-insensitive (RFC 9110), so a server answering `Application/PDF` fell through the dispatch chain and had its body decoded as the default content-type. Both sides of the comparison are lower-cased now. The merged operation only advertised one media type per axis: the split narrows each variant to a single one and the merge never put them back, so the generated documentation hid that createReport also accepts a patch body and can answer with a PDF. The union is restored on the merged operation. apis.mustache reads consumes only where the request axis was not split - a case where the union is the single value anyway - and never reads produces, so nothing but the documentation changes. The sample demonstrated the request axis with an `application/xml` body backed by an object schema. typescript-fetch has no XML serialiser and JSON-encodes that body under an XML Content-Type - with or without this option, as generating the same spec with the option off shows. The behaviour is not this option's doing, but advertising it in the sample promised something the generator does not deliver, so the sample now splits on `application/merge-patch+json`, which it does. The limitation is stated in docs/global-properties.md instead: the option decides which content-types get an operation, not how a body is encoded. The remaining point, body serialisation reading the pre-override header map in runtime.ts, is upstream code this branch does not touch; it applies to every typescript-fetch client and belongs in its own change.
There was a problem hiding this comment.
1 issue found across 20 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java:1093">
P3: Generated API documentation can list response media types in a different order from the OpenAPI response declaration when another response status has content: each narrowed variant carries all response-status media types, so `mediaTypesOf` interleaves them as it walks variants. Preserving the pre-split response media-type order would avoid this documentation regression.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // request axis was not split - a case where this union is the single value anyway - and never | ||
| // reads produces, so this is documentation only. | ||
| base.consumes = mediaTypesOf(requestVariants, v -> v.consumes); | ||
| base.produces = mediaTypesOf(responseVariants, v -> v.produces); |
There was a problem hiding this comment.
P3: Generated API documentation can list response media types in a different order from the OpenAPI response declaration when another response status has content: each narrowed variant carries all response-status media types, so mediaTypesOf interleaves them as it walks variants. Preserving the pre-split response media-type order would avoid this documentation regression.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java, line 1093:
<comment>Generated API documentation can list response media types in a different order from the OpenAPI response declaration when another response status has content: each narrowed variant carries all response-status media types, so `mediaTypesOf` interleaves them as it walks variants. Preserving the pre-split response media-type order would avoid this documentation regression.</comment>
<file context>
@@ -1084,13 +1085,39 @@ private void mergeContentTypeVariants(OperationsMap operations) {
+ // request axis was not split - a case where this union is the single value anyway - and never
+ // reads produces, so this is documentation only.
+ base.consumes = mediaTypesOf(requestVariants, v -> v.consumes);
+ base.produces = mediaTypesOf(responseVariants, v -> v.produces);
+
variants.stream().filter(op -> op != base).forEach(superseded::add);
</file context>
There was a problem hiding this comment.
It's only mixing the order of generation, only esthetic, i propose not to fix this.
|
ran some tests locally and the results are good let's give it a try thanks for your contribution. |
… guards into the date helpers Upstream OpenAPITools#24509 null-guarded required date properties; keep that guard but route the conversion through parseDate/serializeDate. Port the form param date handling into the apisFormParams partial extracted by OpenAPITools#23935, and regenerate the affected samples. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This PR adds an opt-in
splitOperationsByContentTypeoption that fixes #6708: an operation may expose several request and/or response content-types with different schemas, but generators currently keep only the first one ("Multiple schemas found in the OAS 'content' section, returning only the first").The result is a single, mistyped method, and the other content-types are unreachable.
The same underlying problem has been reported and worked around in a language-specific way in several places — e.g. #22095 (rust-axum), #8431 (Ruby), #13426 (TypeScript/Angular) and #10973 (Python request bodies). This PR addresses it generically in the core instead of per-generator.
The implementation is inspired by #19473, where operations were divided by content-type. The key difference here: the division is gated behind an opt-in option (
splitOperationsByContentType, defaultfalse), so all existing generator outputs are unchanged and no current codegen is broken.Why it is off by default
Some dynamically- or structurally-typed languages can already represent several input/return types within a single operationId, so splitting would be unnecessary (or even undesirable) for them:
Content-Typeat runtime and returns a dynamically-typed object (see Adds request body content data to allow multiple content types to be sent to servers #10973 / Adds python-experimental which uses dynamic base classes #8325).TypeA | TypeB, in a single method signature ([TS][Angular] better handling of multiple responses #13426).Statically-typed / compiled languages (Java, C#, Go, Kotlin, Swift, ...) cannot overload a method by return type, so they benefit from one typed method per content-type — which is exactly what this option produces when enabled. Keeping it opt-in lets each consumer decide, without changing any default
output.
How it works
When the option is enabled,
DefaultCodegen#preprocessOpenAPIdivides an eligible operation into the cartesian product of its request-body and success-response content-types (deduplicated by schema). Each variant is a narrowed OpenAPIOperation(a single content-type on each axis) with a typed,collision-free operationId — request appends
With<Subtype>and response appendsAs<Subtype>(e.g.createReportWithXmlAsPdf). The variants are stored on the original operation under thex-content-type-variantsextension and expanded byDefaultGenerator#processOperation, so each one re-entersfromOperationand is typed natively by the target generator. The feature is therefore language-neutral: no per-language type re-derivation and no template changes.Tests
Added unit tests in
DefaultCodegenTest: the request×response cartesian (4 variants), the response-only case (2 variants), and a check that unambiguous operations are left untouched.PR checklist
./mvnw clean package || exit
./bin/generate-samples.sh ./bin/configs/*.yaml || exit
./bin/utils/export_docs_generators.sh || exit
(For Windows users, please run the script in WSL)
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.
Technical committee mention
This feature is opt-in and primarily benefits statically-typed / compiled generators, which cannot overload a method by return type and therefore gain one typed method per content-type. cc the relevant technical committee members:
Summary by cubic
Adds an opt-in global
splitOperationsByContentTypeto split operations by request/response content-type when schemas differ. Defaults to off;typescript-fetchmerges variants into one precisely typed method, and the sample/docs were refined to show all media types and case-insensitive matching.New Features
splitOperationsByContentType(defaultfalse), documented inglobal-properties.md; enable with--global-property splitOperationsByContentType=true.DefaultGenerator#processOperationdelegates to the config to split;DefaultCodegenemits deduped request×response variants with collision-free ids and variant-index extensions.typescript-fetch: merges variants into one method (request is a union discriminated bycontentType; response typed viaacceptoverloads). AddsExclusiveUnioninruntime.mustache; extracts form params toapisFormParams.mustacheand adds small partials to support the merge while keeping legacy output byte-identical when the option is off.bin/configs/typescript-fetch-split-by-content-type.yamland atypescript-fetchpetstore sample with the option on; included in CI typecheck to catch regressions. Tests cover enums, forms, optional/no-body responses, and name collisions.application/merge-patch+json; docs clarify this option chooses which media types get operations and does not add new encoders.Bug Fixes
typescript-fetch: compare response Content-Type case-insensitively to pick the right deserializer.resttemplate-springBoot4-jackson3sample: setspring-webto7.0.5to fix build.Written for commit eec586e. Summary will update on new commits.