Bugfix to solve mixed correct and error response in produces annotation (#24948)

docs/global-properties.md

@@ -36,6 +36,15 @@ one: `With<Subtype>` for the request axis, `As<Subtype>` 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

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<String, Object> 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<String, ApiResponse> 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<String, Header> 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<String> getProducesInfo(final OpenAPI openAPI, final Operation
 
         Set<String> 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) {

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<String> 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<String> 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<Operation> divideOperationsByContentType(OpenAPI openAPI, String path, String httpMethod, Operation operation) {
+        List<Operation> 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<Server> servers) {
         CodegenOperation op = super.fromOperation(path, httpMethod, operation, servers);

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

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);
 

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<Operation> 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<CodegenOperation> 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<String> mediaTypes(List<Map<String, String>> 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 -

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");
+    }
 }

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<String, Object> additionalProperties = new HashMap<>();
+            additionalProperties.put(DOCUMENTATION_PROVIDER, "none");
+            additionalProperties.put(ANNOTATION_LIBRARY, "none");
+            Map<String, File> 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");