diff --git a/docs/global-properties.md b/docs/global-properties.md index 9ba39898d33c..fc672db05ef4 100644 --- a/docs/global-properties.md +++ b/docs/global-properties.md @@ -36,6 +36,15 @@ one: `With` for the request axis, `As` for the response axis, The content-type declared first on each axis is the default one, consistently with the rest of the generator. The option is opt-in and off by default, because it changes the shape of the generated API. +On each axis the split narrowed, a variant speaks only the media-type it was narrowed to: `consumes` on the +request axis, `produces` on the response axis, are that single media-type, and so are the `Content-Type` and +`Accept` of the generators that derive them from those lists (most do; `kotlin-client`, for one, still +filters `produces` down to the types it can deserialise). An axis the split left alone — a single media-type, +or several sharing one schema — keeps the operation's original list, error responses included, as any +operation that was not split. The other responses of the operation, error ones typically, are left as they +are and keep typing their own body — a server that negotiates strictly on `Accept` may then refuse to send a +JSON error body to a variant that only accepts, say, PDF. + Each generated operation carries `x-content-type-variant-*` extensions recording the group it was split from, the content-type it was narrowed to on each axis and the rank of that content-type in its axis. A generator whose language can express the whole matrix in a single construct uses them to merge the variants diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 280beee8d458..227fbd633f20 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -1250,6 +1250,20 @@ private static void tagContentTypeVariant(Operation variant, String group, Axis extensions.put(CodegenConstants.X_CONTENT_TYPE_VARIANT_RESPONSE_INDEX, response.rank); } + /** + * The media-type a content-type variant was narrowed to on one axis — {@code axisExtension} being + * {@link CodegenConstants#X_CONTENT_TYPE_VARIANT_REQUEST} or + * {@link CodegenConstants#X_CONTENT_TYPE_VARIANT_RESPONSE} — or {@code null} when the operation is not + * one of the variants {@link #divideOperationsByContentType} split an operation into (every variant + * carries the group extension) or that axis was not split. + */ + protected static String contentTypeVariantMediaType(Operation operation, String axisExtension) { + Map extensions = operation.getExtensions(); + Object mediaType = extensions != null && extensions.containsKey(CodegenConstants.X_CONTENT_TYPE_VARIANT_GROUP) + ? extensions.get(axisExtension) : null; + return mediaType instanceof String ? (String) mediaType : null; + } + /** * Builds one operation variant narrowed to a single request and/or response media-type (a {@code null} * media-type leaves that axis untouched), with a typed, collision-free operationId. @@ -4941,10 +4955,18 @@ public CodegenOperation fromOperation(String path, if (operation.getResponses() != null && !operation.getResponses().isEmpty()) { ApiResponse methodResponse = findMethodResponse(operation.getResponses()); + // a content-type variant produces only what its method response, the one the split narrowed, + // declares (see getProducesInfo) + boolean producesNarrowed = contentTypeVariantMediaType(operation, CodegenConstants.X_CONTENT_TYPE_VARIANT_RESPONSE) != null; + if (producesNarrowed) { + addProducesInfo(methodResponse, op); + } for (Map.Entry operationGetResponsesEntry : operation.getResponses().entrySet()) { String key = operationGetResponsesEntry.getKey(); ApiResponse response = ModelUtils.getReferencedApiResponse(openAPI, operationGetResponsesEntry.getValue()); - addProducesInfo(response, op); + if (!producesNarrowed) { + addProducesInfo(response, op); + } CodegenResponse r = fromResponse(key, response); Map headers = response.getHeaders(); if (headers != null) { @@ -7703,7 +7725,9 @@ private void addProducesInfo(ApiResponse inputResponse, CodegenOperation codegen } /** - * returns the list of MIME types the APIs can produce + * returns the list of MIME types the APIs can produce. A content-type variant (see + * {@link #divideOperationsByContentType}) produces the single media-type it was narrowed to, whatever + * its other responses declare. * * @param openAPI current specification instance * @param operation Operation @@ -7716,6 +7740,12 @@ public static Set getProducesInfo(final OpenAPI openAPI, final Operation Set produces = new ConcurrentSkipListSet<>(); + String variantMediaType = contentTypeVariantMediaType(operation, CodegenConstants.X_CONTENT_TYPE_VARIANT_RESPONSE); + if (variantMediaType != null) { + produces.add(variantMediaType); + return produces; + } + for (ApiResponse r : operation.getResponses().values()) { ApiResponse response = ModelUtils.getReferencedApiResponse(openAPI, r); if (response.getContent() != null) { diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java index 5b517df02b8b..4adba3b5064a 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java @@ -2349,14 +2349,7 @@ public void preprocessOpenAPI(OpenAPI openAPI) { } for (Operation operation : path.readOperations()) { LOGGER.info("Processing operation {}", operation.getOperationId()); - if (hasBodyParameter(operation) || hasFormParameter(operation)) { - String defaultContentType = hasFormParameter(operation) ? "application/x-www-form-urlencoded" : "application/json"; - List consumes = new ArrayList<>(getConsumesInfo(openAPI, operation)); - String contentType = consumes.isEmpty() ? defaultContentType : consumes.get(0); - operation.addExtension("x-content-type", contentType); - } - String[] accepts = getAccepts(openAPI, operation); - operation.addExtension("x-accepts", accepts); + addContentTypeExtensions(openAPI, operation); } } } @@ -2506,6 +2499,35 @@ public String toEnumValue(String value, String datatype) { } } + /** + * Records on the operation the Content-Type ({@code x-content-type}) and Accept ({@code x-accepts}) the + * generated client sends for it, which the templates read. + */ + private void addContentTypeExtensions(OpenAPI openAPI, Operation operation) { + if (hasBodyParameter(operation) || hasFormParameter(operation)) { + String defaultContentType = hasFormParameter(operation) ? "application/x-www-form-urlencoded" : "application/json"; + List consumes = new ArrayList<>(getConsumesInfo(openAPI, operation)); + String contentType = consumes.isEmpty() ? defaultContentType : consumes.get(0); + operation.addExtension(VendorExtension.X_CONTENT_TYPE.getName(), contentType); + } + String[] accepts = getAccepts(openAPI, operation); + operation.addExtension(VendorExtension.X_ACCEPTS.getName(), accepts); + } + + /** + * A content-type variant is split off after {@link #preprocessOpenAPI} stamped the operation it comes + * from, so it carries that operation's Content-Type and Accept, for every media-type it declares: the + * variants are stamped again here, each with the single media-type it was narrowed to on each axis. + */ + @Override + public List divideOperationsByContentType(OpenAPI openAPI, String path, String httpMethod, Operation operation) { + List variants = super.divideOperationsByContentType(openAPI, path, httpMethod, operation); + if (variants.size() > 1) { + variants.forEach(variant -> addContentTypeExtensions(openAPI, variant)); + } + return variants; + } + @Override public CodegenOperation fromOperation(String path, String httpMethod, Operation operation, List servers) { CodegenOperation op = super.fromOperation(path, httpMethod, operation, servers); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustServerCodegenDeprecated.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustServerCodegenDeprecated.java index f53a91cf6520..0aee86e93f11 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustServerCodegenDeprecated.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustServerCodegenDeprecated.java @@ -749,6 +749,9 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation // Write out the type of data we actually expect this response // to make. if (producesXml) { + // an XML response needs the XML dependency even when the operation's produces, narrowed + // to a content-type variant's own media-type, no longer lists it + additionalProperties.put("usesXml", true); rsp.vendorExtensions.put("x-produces-xml", true); } else if (producesPlainText) { // Plain text means that there is not structured data in diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java index c326ae929ab4..a00505c88605 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java @@ -1138,9 +1138,10 @@ private void mergeContentTypeVariants(OperationsMap operations) { } // the split narrowed each variant to a single media type per axis; the merged operation speaks - // them all again, so its documentation says so. apis.mustache reads consumes only where the - // request axis was not split - a case where this union is the single value anyway - and never - // reads produces, so this is documentation only. + // them all again, so its documentation lists the media types of its variants - not the ones only + // its error responses declare, which a caller never asks for. apis.mustache reads consumes only + // where the 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); diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java index 4b55bf7926c8..c1af92aff19a 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java @@ -5426,6 +5426,58 @@ public void splitOperationsByContentTypeTagsEveryVariant() { tuple("application/xml", 1, "application/pdf", 1)); } + @Test + public void splitOperationsByContentTypeNarrowsProducesToTheVariantMediaType() { + DefaultCodegen codegen = new DefaultCodegen(); + codegen.setSplitOperationsByContentType(true); + OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/issue6708-split-by-content-type-error-responses.yaml"); + codegen.setOpenAPI(openAPI); + + // GET /reports/{id}: 200 is json | csv, 400 and 404 are json. produces is the Accept a client + // sends, so each variant carries the single media-type it was narrowed to: widened back to json by + // the error responses, the csv variant would ask the server for json. + Operation get = openAPI.getPaths().get("/reports/{id}").getGet(); + List variants = codegen.divideOperationsByContentType(openAPI, "/reports/{id}", "get", get); + assertThat(variants).extracting(Operation::getOperationId, v -> DefaultCodegen.getProducesInfo(openAPI, v)) + .containsExactlyInAnyOrder( + tuple("getReportAsJson", Set.of("application/json")), + tuple("getReportAsCsv", Set.of("text/csv"))); + List ops = variants.stream() + .map(v -> codegen.fromOperation("/reports/{id}", "get", v, null)) + .collect(Collectors.toList()); + assertThat(ops).extracting(op -> op.operationId, op -> mediaTypes(op.produces)) + .containsExactlyInAnyOrder( + tuple("getReportAsJson", List.of("application/json")), + tuple("getReportAsCsv", List.of("text/csv"))); + // the error responses are left as they are: they still type their json body + assertThat(ops).allSatisfy(op -> assertThat(op.responses).filteredOn(r -> "400".equals(r.code)) + .extracting(r -> r.getContent().keySet()).containsExactly(Set.of("application/json"))); + + // POST /reports: split on both axes. consumes follows the narrowed request body, produces the + // narrowed success response, whatever the json 400 declares. + Operation post = openAPI.getPaths().get("/reports").getPost(); + assertThat(codegen.divideOperationsByContentType(openAPI, "/reports", "post", post)) + .extracting(v -> codegen.fromOperation("/reports", "post", v, null)) + .extracting(op -> op.operationId, op -> mediaTypes(op.consumes), op -> mediaTypes(op.produces)) + .containsExactlyInAnyOrder( + tuple("createReportWithJsonAsJson", List.of("application/json"), List.of("application/json")), + tuple("createReportWithJsonAsPdf", List.of("application/json"), List.of("application/pdf")), + tuple("createReportWithXmlAsJson", List.of("application/xml"), List.of("application/json")), + tuple("createReportWithXmlAsPdf", List.of("application/xml"), List.of("application/pdf"))); + + // an operation the split leaves alone keeps the union of every response, as it always has - and a + // spec-authored axis extension, with no variant group, does not make it a variant + Operation voucher = openAPI.getPaths().get("/reports/{id}/voucher").getGet(); + voucher.addExtension(CodegenConstants.X_CONTENT_TYPE_VARIANT_RESPONSE, "text/csv"); + assertThat(DefaultCodegen.getProducesInfo(openAPI, voucher)).containsExactlyInAnyOrder("application/pdf", "application/json"); + assertThat(mediaTypes(codegen.fromOperation("/reports/{id}/voucher", "get", voucher, null).produces)) + .containsExactlyInAnyOrder("application/pdf", "application/json"); + } + + private static List mediaTypes(List> media) { + return media.stream().map(m -> m.get(MEDIA_TYPE)).collect(Collectors.toList()); + } + @Test public void splitOperationsByContentTypeIsAGlobalOption() { // the behaviour is language-neutral, so the option is global rather than declared - and documented - diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java index fa55b6297490..90b5823d8d50 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java @@ -20,6 +20,7 @@ import io.swagger.parser.OpenAPIParser; import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.Operation; import io.swagger.v3.oas.models.media.*; import io.swagger.v3.oas.models.parameters.Parameter; import io.swagger.v3.parser.core.models.ParseOptions; @@ -44,6 +45,7 @@ import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; import static org.openapitools.codegen.languages.AbstractJavaCodegen.DISABLE_DISCRIMINATOR_JSON_IGNORE_PROPERTIES; public class AbstractJavaCodegenTest { @@ -1114,4 +1116,41 @@ public void removeAnnotationsTest() { public void testSanitizedDataType() { assertThat(codegen.sanitizeDataType("org.somepkg.DataType")).isEqualTo("orgsomepkgDataType"); } + + @Test + public void contentTypeVariantsCarryTheirOwnAcceptAndContentType() { + // x-accepts and x-content-type are computed in preprocessOpenAPI, before the operations are split by + // content-type; the variants are stamped again as they are split, so none inherits the media-types + // of the operation it was split from + codegen.setSplitOperationsByContentType(true); + OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/issue6708-split-by-content-type-error-responses.yaml"); + codegen.setOpenAPI(openAPI); + codegen.preprocessOpenAPI(openAPI); + + // GET /reports/{id}: 200 is json | csv, 400 and 404 are json + Operation get = openAPI.getPaths().get("/reports/{id}").getGet(); + assertThat(codegen.divideOperationsByContentType(openAPI, "/reports/{id}", "get", get)) + .extracting(v -> codegen.fromOperation("/reports/{id}", "get", v, null)) + .extracting(op -> op.operationId, op -> List.of((String[]) op.vendorExtensions.get("x-accepts"))) + .containsExactlyInAnyOrder( + tuple("getReportAsJson", List.of("application/json")), + tuple("getReportAsCsv", List.of("text/csv"))); + + // POST /reports: request json | xml, 200 json | pdf, 400 json + Operation post = openAPI.getPaths().get("/reports").getPost(); + assertThat(codegen.divideOperationsByContentType(openAPI, "/reports", "post", post)) + .extracting(v -> codegen.fromOperation("/reports", "post", v, null)) + .extracting(op -> op.operationId, op -> op.vendorExtensions.get("x-content-type"), + op -> List.of((String[]) op.vendorExtensions.get("x-accepts"))) + .containsExactlyInAnyOrder( + tuple("createReportWithJsonAsJson", "application/json", List.of("application/json")), + tuple("createReportWithJsonAsPdf", "application/json", List.of("application/pdf")), + tuple("createReportWithXmlAsJson", "application/xml", List.of("application/json")), + tuple("createReportWithXmlAsPdf", "application/xml", List.of("application/pdf"))); + + // not split: the Accept computed from every response, as before + Operation voucher = openAPI.getPaths().get("/reports/{id}/voucher").getGet(); + assertThat((String[]) codegen.fromOperation("/reports/{id}/voucher", "get", voucher, null).vendorExtensions.get("x-accepts")) + .containsExactly("application/json", "application/pdf"); + } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java index 0bd582e30076..b4e7b68565ac 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java @@ -2629,6 +2629,34 @@ public void shouldGenerateMethodsWithoutUsingResponseEntityAndDelegation_issue11 ); } + @Test + public void splitOperationsByContentTypeVariantsSendTheirOwnAccept() throws IOException { + // spring-cloud renders produces from x-accepts (singleContentTypes) and SpringMvcContract sends + // produces[0] as Accept: a variant must carry the media-type it was narrowed to, not the json of the + // error responses the operation also declares, or it would ask the server for another media-type + // than the one it is typed on + GlobalSettings.setProperty(CodegenConstants.SPLIT_OPERATIONS_BY_CONTENT_TYPE, "true"); + try { + Map additionalProperties = new HashMap<>(); + additionalProperties.put(DOCUMENTATION_PROVIDER, "none"); + additionalProperties.put(ANNOTATION_LIBRARY, "none"); + Map files = generateFromContract("src/test/resources/3_0/issue6708-split-by-content-type-error-responses.yaml", SPRING_CLOUD_LIBRARY, additionalProperties); + + JavaFileAssert.assertThat(files.get("ReportsApi.java")) + .assertMethod("getReportAsCsv") + .assertMethodAnnotations() + .containsWithNameAndAttributes("RequestMapping", ImmutableMap.of("produces", "{ \"text/csv\" }")) + .toMethod().toFileAssert() + .assertMethod("createReportWithXmlAsPdf") + .assertMethodAnnotations() + .containsWithNameAndAttributes("RequestMapping", ImmutableMap.of( + "consumes", "\"application/xml\"", + "produces", "{ \"application/pdf\" }")); + } finally { + GlobalSettings.reset(); + } + } + @Test public void testResponseWithArray_issue12524() throws Exception { GlobalSettings.setProperty("skipFormModel", "true"); diff --git a/modules/openapi-generator/src/test/resources/3_0/issue6708-split-by-content-type-error-responses.yaml b/modules/openapi-generator/src/test/resources/3_0/issue6708-split-by-content-type-error-responses.yaml new file mode 100644 index 000000000000..073c542b2658 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/issue6708-split-by-content-type-error-responses.yaml @@ -0,0 +1,118 @@ +openapi: 3.0.1 +info: + title: split operations by content-type (issue 6708) - json error responses + version: 1.0.0 +paths: + /reports/{id}: + get: + operationId: getReport + tags: + - report + parameters: + - name: id + in: path + required: true + schema: + type: string + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/Report' + text/csv: + schema: + type: string + '400': + description: invalid id + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: no such report + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + /reports: + post: + operationId: createReport + tags: + - report + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/Report' + application/xml: + schema: + $ref: '#/components/schemas/ReportXml' + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/Receipt' + application/pdf: + schema: + type: string + format: binary + '400': + description: invalid report + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + /reports/{id}/voucher: + get: + description: a single success content-type, so not split + operationId: getReportVoucher + tags: + - report + parameters: + - name: id + in: path + required: true + schema: + type: string + responses: + '200': + description: ok + content: + application/pdf: + schema: + type: string + format: binary + '400': + description: invalid id + content: + application/json: + schema: + $ref: '#/components/schemas/Error' +components: + schemas: + Report: + type: object + properties: + id: + type: string + name: + type: string + ReportXml: + type: object + properties: + ref: + type: string + Receipt: + type: object + properties: + number: + type: string + Error: + type: object + properties: + message: + type: string