From 127a9ae358d88a6d945da851c18a6483cbe46bef Mon Sep 17 00:00:00 2001 From: altro3 Date: Wed, 28 Aug 2024 13:42:21 +0700 Subject: [PATCH] Divide operations by content type Fixed #17877 --- .../codegen/CodegenConstants.java | 2 + .../openapitools/codegen/DefaultCodegen.java | 123 +++++++++++++++++- .../codegen/DefaultGenerator.java | 33 ++++- .../languages/AbstractJavaCodegen.java | 3 + .../languages/AbstractKotlinCodegen.java | 2 + .../JavaMicronautClientCodegenTest.java | 37 +++++- .../JavaMicronautServerCodegenTest.java | 43 +++++- .../3_0/java/multiple-content-types.yaml | 57 ++++++++ 8 files changed, 287 insertions(+), 13 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_0/java/multiple-content-types.yaml diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java index 0a340ecb14e1..da431fcc4f4d 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java @@ -453,4 +453,6 @@ public static enum ENUM_PROPERTY_NAMING_TYPE {camelCase, PascalCase, snake_case, public static final String WAIT_TIME_OF_THREAD = "waitTimeMillis"; public static final String USE_DEFAULT_VALUES_FOR_REQUIRED_VARS = "useDefaultValuesForRequiredVars"; + + public static final String DIVIDE_OPERATIONS_BY_CONTENT_TYPE = "divideOperationsByContentType"; } 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 88377789e408..73c47ae0096e 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 @@ -86,6 +86,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.openapitools.codegen.CodegenConstants.DIVIDE_OPERATIONS_BY_CONTENT_TYPE; import static org.openapitools.codegen.CodegenConstants.UNSUPPORTED_V310_SPEC_MSG; import static org.openapitools.codegen.utils.CamelizeOption.LOWERCASE_FIRST_LETTER; import static org.openapitools.codegen.utils.OnceLogger.once; @@ -392,7 +393,7 @@ public void processOpts() { convertPropertyToBooleanAndWriteBack(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, this::setDisallowAdditionalPropertiesIfNotPresent); convertPropertyToBooleanAndWriteBack(CodegenConstants.ENUM_UNKNOWN_DEFAULT_CASE, this::setEnumUnknownDefaultCase); convertPropertyToBooleanAndWriteBack(CodegenConstants.AUTOSET_CONSTANTS, this::setAutosetConstants); - } + } /*** @@ -999,6 +1000,49 @@ public void postProcessParameter(CodegenParameter parameter) { @Override @SuppressWarnings("unused") public void preprocessOpenAPI(OpenAPI openAPI) { + + var divideOperationsByContentType = Boolean.parseBoolean(GlobalSettings.getProperty(DIVIDE_OPERATIONS_BY_CONTENT_TYPE, "false")); + + if (divideOperationsByContentType && openAPI.getPaths() != null && !openAPI.getPaths().isEmpty()) { + + for (Map.Entry entry : openAPI.getPaths().entrySet()) { + String pathStr = entry.getKey(); + PathItem path = entry.getValue(); + List getOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.GET, path.getGet()); + if (!getOps.isEmpty()) { + path.addExtension("x-get", getOps); + } + List putOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.PUT, path.getPut()); + if (!putOps.isEmpty()) { + path.addExtension("x-put", putOps); + } + List postOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.POST, path.getPost()); + if (!postOps.isEmpty()) { + path.addExtension("x-post", postOps); + } + List deleteOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.DELETE, path.getDelete()); + if (!deleteOps.isEmpty()) { + path.addExtension("x-delete", deleteOps); + } + List optionsOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.OPTIONS, path.getOptions()); + if (!optionsOps.isEmpty()) { + path.addExtension("x-options", optionsOps); + } + List headOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.HEAD, path.getHead()); + if (!headOps.isEmpty()) { + path.addExtension("x-head", headOps); + } + List patchOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.PATCH, path.getPatch()); + if (!patchOps.isEmpty()) { + path.addExtension("x-patch", patchOps); + } + List traceOps = divideOperationsByContentType(pathStr, PathItem.HttpMethod.TRACE, path.getTrace()); + if (!traceOps.isEmpty()) { + path.addExtension("x-trace", traceOps); + } + } + } + if (useOneOfInterfaces && openAPI.getComponents() != null) { // we process the openapi schema here to find oneOf schemas and create interface models for them Map schemas = new HashMap<>(openAPI.getComponents().getSchemas()); @@ -1080,6 +1124,77 @@ public void preprocessOpenAPI(OpenAPI openAPI) { } } + private List divideOperationsByContentType(String path, PathItem.HttpMethod httpMethod, Operation op) { + + if (op == null) { + return Collections.emptyList(); + } + + var additionalOps = new ArrayList(); + + RequestBody body = op.getRequestBody(); + if (body == null || body.getContent() == null) { + return Collections.emptyList(); + } + Content content = body.getContent(); + if (content.size() <= 1) { + return Collections.emptyList(); + } + var firstEntry = content.entrySet().iterator().next(); + var mediaTypesToRemove = new ArrayList(); + for (var entry : content.entrySet()) { + if (mediaTypesToRemove.contains(entry.getKey()) || entry.getKey().equals(firstEntry.getKey()) || entry.getValue().equals(firstEntry.getValue())) { + continue; + } + var foundSameOpSignature = false; + for (var additionalOp : additionalOps) { + RequestBody additionalBody = additionalOp.getRequestBody(); + if (additionalBody == null || additionalBody.getContent() == null) { + return Collections.emptyList(); + } + for (var addContentEntry : additionalBody.getContent().entrySet()) { + if (addContentEntry.getValue().equals(entry.getValue())) { + foundSameOpSignature = true; + break; + } + } + if (foundSameOpSignature) { + additionalBody.getContent().put(entry.getKey(), entry.getValue()); + break; + } + } + mediaTypesToRemove.add(entry.getKey()); + if (foundSameOpSignature) { + continue; + } + additionalOps.add(new Operation() + .deprecated(op.getDeprecated()) + .callbacks(op.getCallbacks()) + .description(op.getDescription()) + .extensions(op.getExtensions()) + .externalDocs(op.getExternalDocs()) + .operationId(getOrGenerateOperationId(op, path, httpMethod.name())) + .parameters(op.getParameters()) + .responses(op.getResponses()) + .security(op.getSecurity()) + .servers(op.getServers()) + .summary(op.getSummary()) + .tags(op.getTags()) + .requestBody(new RequestBody() + .description(body.getDescription()) + .extensions(body.getExtensions()) + .content(new Content() + .addMediaType(entry.getKey(), entry.getValue())) + ) + ); + } + if (!mediaTypesToRemove.isEmpty()) { + content.entrySet().removeIf(stringMediaTypeEntry -> mediaTypesToRemove.contains(stringMediaTypeEntry.getKey())); + } + + return additionalOps; + } + // override with any special handling of the entire OpenAPI spec document @Override @SuppressWarnings("unused") @@ -1164,8 +1279,7 @@ public String encodePath(String input) { */ @Override public String escapeUnsafeCharacters(String input) { - LOGGER.warn("escapeUnsafeCharacters should be overridden in the code generator with proper logic to escape " + - "unsafe characters"); + LOGGER.warn("escapeUnsafeCharacters should be overridden in the code generator with proper logic to escape unsafe characters"); // doing nothing by default and code generator should implement // the logic to prevent code injection // later we'll make this method abstract to make sure @@ -1181,8 +1295,7 @@ public String escapeUnsafeCharacters(String input) { */ @Override public String escapeQuotationMark(String input) { - LOGGER.warn("escapeQuotationMark should be overridden in the code generator with proper logic to escape " + - "single/double quote"); + LOGGER.warn("escapeQuotationMark should be overridden in the code generator with proper logic to escape single/double quote"); return input.replace("\"", "\\\""); } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java index e5366475d554..770bb86eec52 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java @@ -616,10 +616,10 @@ private void generateModelsForVariable(List files, List allModel if (!processedModels.contains(key) && allSchemas.containsKey(key)) { generateModels(files, allModels, unusedModels, aliasModels, processedModels, () -> Set.of(key)); } else { - LOGGER.info("Type " + variable.getComplexType()+" of variable " + variable.getName() + " could not be resolve because it is not declared as a model."); + LOGGER.info("Type {} of variable {} could not be resolve because it is not declared as a model.", variable.getComplexType(), variable.getName()); } } else { - LOGGER.info("Type " + variable.getOpenApiType()+" of variable " + variable.getName() + " could not be resolve because it is not declared as a model."); + LOGGER.info("Type {} of variable {} could not be resolve because it is not declared as a model.", variable.getComplexType(), variable.getName()); } } @@ -1012,7 +1012,7 @@ private void generateOpenapiGeneratorIgnoreFile() { File ignoreFile = new File(ignoreFileNameTarget); // use the entries provided by the users to pre-populate .openapi-generator-ignore try { - LOGGER.info("Writing file " + ignoreFileNameTarget + " (which is always overwritten when the option `openapiGeneratorIgnoreFile` is enabled.)"); + LOGGER.info("Writing file {} (which is always overwritten when the option `openapiGeneratorIgnoreFile` is enabled.)", ignoreFileNameTarget); new File(config.outputFolder()).mkdirs(); if (!ignoreFile.createNewFile()) { // file may already exist, do nothing @@ -1475,6 +1475,9 @@ public Map> processPaths(Paths paths) { if (paths == null) { return ops; } + + var divideOperationsByContentType = Boolean.parseBoolean(GlobalSettings.getProperty(CodegenConstants.DIVIDE_OPERATIONS_BY_CONTENT_TYPE, "false")); + for (Map.Entry pathsEntry : paths.entrySet()) { String resourcePath = pathsEntry.getKey(); PathItem path = pathsEntry.getValue(); @@ -1486,10 +1489,34 @@ public Map> processPaths(Paths paths) { processOperation(resourcePath, "patch", path.getPatch(), ops, path); processOperation(resourcePath, "options", path.getOptions(), ops, path); processOperation(resourcePath, "trace", path.getTrace(), ops, path); + + if (divideOperationsByContentType) { + processAdditionalOperations(resourcePath, "x-get", "get", ops, path); + processAdditionalOperations(resourcePath, "x-head", "head", ops, path); + processAdditionalOperations(resourcePath, "x-put", "put", ops, path); + processAdditionalOperations(resourcePath, "x-post", "post", ops, path); + processAdditionalOperations(resourcePath, "x-delete", "delete", ops, path); + processAdditionalOperations(resourcePath, "x-patch", "patch", ops, path); + processAdditionalOperations(resourcePath, "x-options", "options", ops, path); + processAdditionalOperations(resourcePath, "x-trace", "trace", ops, path); + } } return ops; } + protected void processAdditionalOperations(String resourcePath, String extName, String httpMethod, Map> ops, PathItem path) { + if (path.getExtensions() == null || !path.getExtensions().containsKey(extName)) { + return; + } + var xOps = (List) path.getExtensions().get(extName); + if (xOps == null) { + return; + } + for (Operation op : xOps) { + processOperation(resourcePath, httpMethod, op, ops, path); + } + } + public Map> processWebhooks(Map webhooks) { Map> ops = new TreeMap<>(); // when input file is not valid and doesn't contain any paths 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 e34cbee7799e..d2926a662b7a 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 @@ -38,6 +38,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.text.StringEscapeUtils; import org.openapitools.codegen.*; +import org.openapitools.codegen.config.GlobalSettings; import org.openapitools.codegen.languages.features.BeanValidationFeatures; import org.openapitools.codegen.languages.features.DocumentationProviderFeatures; import org.openapitools.codegen.meta.features.*; @@ -193,6 +194,8 @@ public abstract class AbstractJavaCodegen extends DefaultCodegen implements Code public AbstractJavaCodegen() { super(); + GlobalSettings.setProperty(CodegenConstants.DIVIDE_OPERATIONS_BY_CONTENT_TYPE, "true"); + modifyFeatureSet(features -> features .includeDocumentationFeatures(DocumentationFeature.Readme) .wireFormatFeatures(EnumSet.of(WireFormatFeature.JSON, WireFormatFeature.XML)) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractKotlinCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractKotlinCodegen.java index 456b972244a8..f0a602a2b8f1 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractKotlinCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractKotlinCodegen.java @@ -27,6 +27,7 @@ import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; import org.openapitools.codegen.*; +import org.openapitools.codegen.config.GlobalSettings; import org.openapitools.codegen.model.ModelMap; import org.openapitools.codegen.model.ModelsMap; import org.openapitools.codegen.templating.mustache.EscapeChar; @@ -88,6 +89,7 @@ public abstract class AbstractKotlinCodegen extends DefaultCodegen implements Co public AbstractKotlinCodegen() { super(); + GlobalSettings.setProperty(CodegenConstants.DIVIDE_OPERATIONS_BY_CONTENT_TYPE, "true"); supportsInheritance = true; setSortModelPropertiesByRequiredFlag(true); diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautClientCodegenTest.java index 23e808675f79..b31bcf839f42 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautClientCodegenTest.java @@ -7,10 +7,10 @@ import org.openapitools.codegen.CodegenConstants; import org.openapitools.codegen.DefaultGenerator; import org.openapitools.codegen.config.CodegenConfigurator; +import org.openapitools.codegen.config.GlobalSettings; import org.openapitools.codegen.java.assertions.JavaFileAssert; import org.openapitools.codegen.languages.JavaMicronautClientCodegen; import org.openapitools.codegen.testutils.ConfigAssert; -import org.testng.Assert; import org.testng.annotations.Test; import java.io.File; @@ -21,7 +21,6 @@ import static org.openapitools.codegen.TestUtils.newTempFolder; import static org.testng.Assert.assertEquals; - public class JavaMicronautClientCodegenTest extends AbstractMicronautCodegenTest { @Test public void clientOptsUnicity() { @@ -323,7 +322,8 @@ public void testConfigurePathSeparator() { * Includes regression tests for: * - Correct Jackson annotation when `wrapped: false` */ - @Test public void shouldGenerateCorrectXmlAnnotations() { + @Test + public void shouldGenerateCorrectXmlAnnotations() { // Arrange final CodegenConfigurator config = new CodegenConfigurator() .addAdditionalProperty(CodegenConstants.WITH_XML, true) @@ -333,7 +333,7 @@ public void testConfigurePathSeparator() { .setGeneratorName(JavaMicronautClientCodegen.NAME) .setInputSpec("src/test/resources/3_0/java/xml-annotations-test.yaml") .setOutputDir(newTempFolder().toString()); - + // Act final List files = new DefaultGenerator().opts(config.toClientOptInput()).generate(); @@ -457,4 +457,33 @@ public void testConfigurePathSeparator() { .hasAnnotation("JacksonXmlProperty", Map.of("localName", "\"item\"")) .hasAnnotation("JacksonXmlElementWrapper", Map.of("localName", "\"activities-array\"")); } + + @Test + public void testMultipleContentTypesToPath() { + + GlobalSettings.setProperty(CodegenConstants.DIVIDE_OPERATIONS_BY_CONTENT_TYPE, "true"); + + var codegen = new JavaMicronautClientCodegen(); + String outputPath = generateFiles(codegen, "src/test/resources/3_0/java/multiple-content-types.yaml", CodegenConstants.APIS, CodegenConstants.MODELS); + + // Micronaut declarative http client should use the provided path separator + assertFileContains(outputPath + "/src/main/java/org/openapitools/api/DefaultApi.java", + " @Post(uri=\"/multiplecontentpath\")\n" + + " @Produces({\"application/json\", \"application/xml\"})\n" + + " Mono myOp(\n" + + " @Body @Nullable @Valid Coordinates coordinates\n" + + " );\n", + " @Post(uri=\"/multiplecontentpath\")\n" + + " @Produces({\"multipart/form-data\"})\n" + + " Mono myOp_1(\n" + + " @Nullable @Valid Coordinates coordinates, \n" + + " @Nullable File _file\n" + + " );", + " @Post(uri=\"/multiplecontentpath\")\n" + + " @Produces({\"application/yaml\", \"text/json\"})\n" + + " Mono myOp_2(\n" + + " @Body @Nullable @Valid MySchema mySchema\n" + + " );" + ); + } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautServerCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautServerCodegenTest.java index ceb6aee4b852..cce5e9470c14 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautServerCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/micronaut/JavaMicronautServerCodegenTest.java @@ -11,7 +11,6 @@ import org.openapitools.codegen.java.assertions.JavaFileAssert; import org.openapitools.codegen.languages.JavaMicronautServerCodegen; import org.openapitools.codegen.testutils.ConfigAssert; -import org.testng.Assert; import org.testng.annotations.Test; import java.io.File; @@ -487,4 +486,46 @@ public void doRepeatOperationForAllTags() { .hasAnnotation("JacksonXmlProperty", Map.of("localName", "\"item\"")) .hasAnnotation("JacksonXmlElementWrapper", Map.of("localName", "\"activities-array\"")); } + + @Test + public void testMultipleContentTypesToPath() { + + var codegen = new JavaMicronautServerCodegen(); + String outputPath = generateFiles(codegen, "src/test/resources/3_0/java/multiple-content-types.yaml", CodegenConstants.APIS, CodegenConstants.MODELS); + + // Micronaut declarative http client should use the provided path separator + assertFileContains(outputPath + "/src/main/java/org/openapitools/controller/DefaultController.java", + " @Post(uri=\"/multiplecontentpath\")\n" + + " @Produces(value = {})\n" + + " @Consumes(value = {\"application/json\", \"application/xml\"})\n" + + " @Secured({SecurityRule.IS_ANONYMOUS})\n" + + " public Mono myOp(\n" + + " @Body @Nullable @Valid Coordinates coordinates\n" + + " ) {\n" + + " // TODO implement myOp();\n" + + " return Mono.error(new HttpStatusException(HttpStatus.NOT_IMPLEMENTED, null));\n" + + " }", + " @Post(uri=\"/multiplecontentpath\")\n" + + " @Produces(value = {})\n" + + " @Consumes(value = {\"multipart/form-data\"})\n" + + " @Secured({SecurityRule.IS_ANONYMOUS})\n" + + " public Mono myOp_1(\n" + + " @Nullable @Valid Coordinates coordinates, \n" + + " @Nullable CompletedFileUpload _file\n" + + " ) {\n" + + " // TODO implement myOp_1();\n" + + " return Mono.error(new HttpStatusException(HttpStatus.NOT_IMPLEMENTED, null));\n" + + " }", + " @Post(uri=\"/multiplecontentpath\")\n" + + " @Produces(value = {})\n" + + " @Consumes(value = {\"application/yaml\", \"text/json\"})\n" + + " @Secured({SecurityRule.IS_ANONYMOUS})\n" + + " public Mono myOp_2(\n" + + " @Body @Nullable @Valid MySchema mySchema\n" + + " ) {\n" + + " // TODO implement myOp_2();\n" + + " return Mono.error(new HttpStatusException(HttpStatus.NOT_IMPLEMENTED, null));\n" + + " }" + ); + } } diff --git a/modules/openapi-generator/src/test/resources/3_0/java/multiple-content-types.yaml b/modules/openapi-generator/src/test/resources/3_0/java/multiple-content-types.yaml new file mode 100644 index 000000000000..f2cfcaf94916 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/java/multiple-content-types.yaml @@ -0,0 +1,57 @@ +openapi: 3.0.3 +info: + version: "1" + title: Multiple Content Types for same request +paths: + /multiplecontentpath: + post: + operationId: myOp + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/coordinates' + application/xml: + schema: + $ref: '#/components/schemas/coordinates' + multipart/form-data: + schema: + type: object + properties: + coordinates: + $ref: '#/components/schemas/coordinates' + file: + type: string + format: binary + application/yaml: + schema: + $ref: '#/components/schemas/MySchema' + text/json: + schema: + $ref: '#/components/schemas/MySchema' + responses: + 201: + description: Successfully created + headers: + Location: + schema: + type: string +components: + schemas: + coordinates: + type: object + required: + - lat + - long + properties: + lat: + type: number + long: + type: number + MySchema: + type: object + required: + - lat + properties: + lat: + type: number