From 3337b2098935bc0031d44363cddde0ec3d64e627 Mon Sep 17 00:00:00 2001 From: altro3 Date: Tue, 20 Jun 2023 16:56:21 +0700 Subject: [PATCH] Fixed wrong schema when schema set by RequestBody annotation (#1072) --- .../AbstractOpenApiEndpointVisitor.java | 88 +++++++++------ .../visitor/AbstractOpenApiVisitor.java | 8 +- .../visitor/OpenApiApplicationVisitor.java | 4 +- .../openapi/visitor/SchemaUtils.java | 71 ++++++++++++- .../visitor/OpenApiEncodingSpec.groovy | 100 ++++++++++++++++++ 5 files changed, 231 insertions(+), 40 deletions(-) diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java index 2940c8e9a8..c9a78f8e58 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java @@ -111,6 +111,8 @@ import static io.micronaut.openapi.visitor.ElementUtils.isNullable; import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.getSecurityProperties; import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.isOpenApiEnabled; +import static io.micronaut.openapi.visitor.SchemaUtils.COMPONENTS_CALLBACKS_PREFIX; +import static io.micronaut.openapi.visitor.SchemaUtils.COMPONENTS_SCHEMAS_PREFIX; import static io.micronaut.openapi.visitor.SchemaUtils.TYPE_OBJECT; import static io.micronaut.openapi.visitor.Utils.DEFAULT_MEDIA_TYPES; @@ -122,8 +124,6 @@ */ public abstract class AbstractOpenApiEndpointVisitor extends AbstractOpenApiVisitor { - public static final String COMPONENTS_CALLBACKS_PREFIX = "#/components/callbacks/"; - protected static final String CONTEXT_CHILD_PATH = "internal.child.path"; protected static final String CONTEXT_CHILD_OP_ID_PREFIX = "internal.opId.prefix"; protected static final String CONTEXT_CHILD_OP_ID_SUFFIX = "internal.opId.suffix"; @@ -398,6 +398,8 @@ public void visitMethod(MethodElement element, VisitorContext context) { List consumesMediaTypes = consumesMediaTypes(element); Map swaggerOperations = readOperations(pathItemEntry.getKey(), httpMethod, pathItems, element, context); + boolean isRequestBodySchemaSet = false; + for (Map.Entry operationEntry : swaggerOperations.entrySet()) { io.swagger.v3.oas.models.Operation swaggerOperation = operationEntry.getValue(); io.swagger.v3.oas.models.ExternalDocumentation externalDocs = readExternalDocs(element, context); @@ -427,7 +429,12 @@ public void visitMethod(MethodElement element, VisitorContext context) { readResponse(element, context, openAPI, swaggerOperation, javadocDescription); if (permitsRequestBody) { - RequestBody requestBody = readSwaggerRequestBody(element, consumesMediaTypes, context); + Pair requestBodyPair = readSwaggerRequestBody(element, consumesMediaTypes, context); + RequestBody requestBody = null; + if (requestBodyPair != null) { + requestBody = requestBodyPair.getFirst(); + isRequestBodySchemaSet = requestBodyPair.getSecond(); + } if (requestBody != null) { RequestBody currentRequestBody = swaggerOperation.getRequestBody(); if (currentRequestBody != null) { @@ -454,7 +461,7 @@ public void visitMethod(MethodElement element, VisitorContext context) { List extraBodyParameters = new ArrayList<>(); for (io.swagger.v3.oas.models.Operation operation : swaggerOperations.values()) { processParameters(element, context, openAPI, operation, javadocDescription, permitsRequestBody, pathVariables, consumesMediaTypes, extraBodyParameters, httpMethod, matchTemplates, pathItems); - processExtraBodyParameters(context, httpMethod, openAPI, operation, javadocDescription, consumesMediaTypes, extraBodyParameters); + processExtraBodyParameters(context, httpMethod, openAPI, operation, javadocDescription, isRequestBodySchemaSet, consumesMediaTypes, extraBodyParameters); } } } @@ -462,6 +469,7 @@ public void visitMethod(MethodElement element, VisitorContext context) { private void processExtraBodyParameters(VisitorContext context, HttpMethod httpMethod, OpenAPI openAPI, io.swagger.v3.oas.models.Operation swaggerOperation, JavadocDescription javadocDescription, + boolean isRequestBodySchemaSet, List consumesMediaTypes, List extraBodyParameters) { RequestBody requestBody = swaggerOperation.getRequestBody(); @@ -491,31 +499,39 @@ private void processExtraBodyParameters(VisitorContext context, HttpMethod httpM mediaType.setSchema(schema); } if (schema.get$ref() != null) { - ComposedSchema composedSchema = new ComposedSchema(); - Schema extraBodyParametersSchema = new Schema(); - // Composition of existing + a new schema where extra body parameters are going to be added - composedSchema.addAllOfItem(schema); - composedSchema.addAllOfItem(extraBodyParametersSchema); - schema = extraBodyParametersSchema; - mediaType.setSchema(composedSchema); + if (isRequestBodySchemaSet) { + schema = openAPI.getComponents().getSchemas().get(schema.get$ref().substring(COMPONENTS_SCHEMAS_PREFIX.length())); + } else { + ComposedSchema composedSchema = new ComposedSchema(); + Schema extraBodyParametersSchema = new Schema(); + // Composition of existing + a new schema where extra body parameters are going to be added + composedSchema.addAllOfItem(schema); + composedSchema.addAllOfItem(extraBodyParametersSchema); + schema = extraBodyParametersSchema; + mediaType.setSchema(composedSchema); + } } for (TypedElement parameter : extraBodyParameters) { - processBodyParameter(context, openAPI, javadocDescription, MediaType.of(mediaTypeName), schema, parameter); + if (!isRequestBodySchemaSet) { + processBodyParameter(context, openAPI, javadocDescription, MediaType.of(mediaTypeName), schema, parameter); + } if (mediaTypeName.equals(MediaType.MULTIPART_FORM_DATA)) { - for (String prop : (Set) schema.getProperties().keySet()) { - Map encodings = mediaType.getEncoding(); - if (encodings == null) { - encodings = new HashMap<>(); - mediaType.setEncoding(encodings); - } - // if content type doesn't set by annotation, - // we can set application/octet-stream for file upload classes - Encoding encoding = encodings.get(prop); - if (encoding == null && isFileUpload(parameter.getType())) { - encoding = new Encoding(); - encodings.put(prop, encoding); - - encoding.setContentType(MediaType.APPLICATION_OCTET_STREAM); + if (CollectionUtils.isNotEmpty(schema.getProperties())) { + for (String prop : (Set) schema.getProperties().keySet()) { + Map encodings = mediaType.getEncoding(); + if (encodings == null) { + encodings = new HashMap<>(); + mediaType.setEncoding(encodings); + } + // if content type doesn't set by annotation, + // we can set application/octet-stream for file upload classes + Encoding encoding = encodings.get(prop); + if (encoding == null && isFileUpload(parameter.getType())) { + encoding = new Encoding(); + encodings.put(prop, encoding); + + encoding.setContentType(MediaType.APPLICATION_OCTET_STREAM); + } } } } @@ -662,9 +678,9 @@ private void processParameter(VisitorContext context, OpenAPI openAPI, return; } if (permitsRequestBody && swaggerOperation.getRequestBody() == null) { - RequestBody requestBody = readSwaggerRequestBody(parameter, consumesMediaTypes, context); - if (requestBody != null) { - swaggerOperation.setRequestBody(requestBody); + Pair requestBodyPair = readSwaggerRequestBody(parameter, consumesMediaTypes, context); + if (requestBodyPair != null && requestBodyPair.getFirst() != null) { + swaggerOperation.setRequestBody(requestBodyPair.getFirst()); } } @@ -1717,7 +1733,8 @@ private void processResponses(io.swagger.v3.oas.models.Operation operation, List } } - private RequestBody readSwaggerRequestBody(Element element, List consumesMediaTypes, VisitorContext context) { + // boolean - is swagger schema has implementation + private Pair readSwaggerRequestBody(Element element, List consumesMediaTypes, VisitorContext context) { AnnotationValue requestBodyAnnValue = element.findAnnotation(io.swagger.v3.oas.annotations.parameters.RequestBody.class).orElse(null); @@ -1725,7 +1742,16 @@ private RequestBody readSwaggerRequestBody(Element element, List cons return null; } + boolean hasSchemaImplementation = false; + AnnotationValue content = requestBodyAnnValue.getAnnotation("content", io.swagger.v3.oas.annotations.media.Content.class).orElse(null); + if (content != null) { + AnnotationValue swaggerSchema = content.getAnnotation("schema", io.swagger.v3.oas.annotations.media.Schema.class).orElse(null); + if (swaggerSchema != null) { + hasSchemaImplementation = swaggerSchema.stringValue("implementation").orElse(null) != null; + } + } + RequestBody requestBody = toValue(requestBodyAnnValue.getValues(), context, RequestBody.class).orElse(null); // if media type doesn't set in swagger annotation, check micronaut annotation if (content != null @@ -1740,7 +1766,7 @@ private RequestBody readSwaggerRequestBody(Element element, List cons } } - return requestBody; + return Pair.of(requestBody, hasSchemaImplementation); } private void readServers(MethodElement element, VisitorContext context, io.swagger.v3.oas.models.Operation swaggerOperation) { diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java index 051d788999..e4cd41f599 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java @@ -131,7 +131,6 @@ import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.getConfigurationProperty; import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.getExpandableProperties; import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.resolvePlaceholders; -import static io.micronaut.openapi.visitor.SchemaUtils.EMPTY_SCHEMA; import static io.micronaut.openapi.visitor.SchemaUtils.TYPE_OBJECT; import static io.micronaut.openapi.visitor.Utils.resolveComponents; import static java.util.stream.Collectors.toMap; @@ -146,7 +145,6 @@ abstract class AbstractOpenApiVisitor { private static final Lock VISITED_ELEMENTS_LOCK = new ReentrantLock(); - private static final ComposedSchema EMPTY_COMPOSED_SCHEMA = new ComposedSchema(); /** * Stores relations between schema names and class names. @@ -1258,7 +1256,7 @@ protected Schema bindSchemaForElement(VisitorContext context, TypedElement eleme notOnlyRef = true; } - boolean addSchemaToBind = !schemaToBind.equals(EMPTY_SCHEMA); + boolean addSchemaToBind = !SchemaUtils.isEmptySchema(schemaToBind); if (addSchemaToBind) { if (TYPE_OBJECT.equals(originalSchema.getType())) { @@ -1267,7 +1265,7 @@ protected Schema bindSchemaForElement(VisitorContext context, TypedElement eleme } originalSchema.setType(null); } - if (!originalSchema.equals(EMPTY_SCHEMA)) { + if (!SchemaUtils.isEmptySchema(originalSchema)) { composedSchema.addAllOfItem(originalSchema); } } else if (isNullable && CollectionUtils.isEmpty(composedSchema.getAllOf())) { @@ -1283,7 +1281,7 @@ protected Schema bindSchemaForElement(VisitorContext context, TypedElement eleme composedSchema.addAllOfItem(schemaToBind); } - if (!composedSchema.equals(EMPTY_COMPOSED_SCHEMA) + if (!SchemaUtils.isEmptySchema(composedSchema) && ((CollectionUtils.isNotEmpty(composedSchema.getAllOf()) && composedSchema.getAllOf().size() > 1) || CollectionUtils.isNotEmpty(composedSchema.getOneOf()) || CollectionUtils.isNotEmpty(composedSchema.getAnyOf()) diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/OpenApiApplicationVisitor.java b/openapi/src/main/java/io/micronaut/openapi/visitor/OpenApiApplicationVisitor.java index d7db580054..b93cc92403 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/OpenApiApplicationVisitor.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/OpenApiApplicationVisitor.java @@ -98,8 +98,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.TextNode; -import static io.micronaut.openapi.visitor.SchemaUtils.EMPTY_COMPOSED_SCHEMA; -import static io.micronaut.openapi.visitor.SchemaUtils.EMPTY_SCHEMA; import static io.micronaut.openapi.visitor.SchemaUtils.EMPTY_SIMPLE_SCHEMA; import static io.micronaut.openapi.visitor.SchemaUtils.TYPE_OBJECT; import static io.swagger.v3.oas.models.Components.COMPONENTS_SCHEMAS_REF; @@ -1450,7 +1448,7 @@ private Schema normalizeSchema(Schema schema) { } boolean isSameType = allOfSchema.getType() == null || allOfSchema.getType().equals(type); - if (schema.equals(EMPTY_SCHEMA) || schema.equals(EMPTY_COMPOSED_SCHEMA) + if (SchemaUtils.isEmptySchema(schema) && (serializedDefaultValue == null || serializedDefaultValue.equals(serializedAllOfDefaultValue)) && (type == null || allOfSchema.getType() == null || allOfSchema.getType().equals(type))) { normalizedSchema = allOfSchema; diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/SchemaUtils.java b/openapi/src/main/java/io/micronaut/openapi/visitor/SchemaUtils.java index b151e5472c..c2af446c34 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/SchemaUtils.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/SchemaUtils.java @@ -15,15 +15,33 @@ */ package io.micronaut.openapi.visitor; +import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import io.micronaut.core.annotation.Internal; import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.OpenAPI; import io.swagger.v3.oas.models.media.ArraySchema; +import io.swagger.v3.oas.models.media.BinarySchema; +import io.swagger.v3.oas.models.media.BooleanSchema; +import io.swagger.v3.oas.models.media.ByteArraySchema; import io.swagger.v3.oas.models.media.ComposedSchema; +import io.swagger.v3.oas.models.media.DateSchema; +import io.swagger.v3.oas.models.media.DateTimeSchema; +import io.swagger.v3.oas.models.media.EmailSchema; +import io.swagger.v3.oas.models.media.FileSchema; +import io.swagger.v3.oas.models.media.IntegerSchema; +import io.swagger.v3.oas.models.media.JsonSchema; +import io.swagger.v3.oas.models.media.MapSchema; +import io.swagger.v3.oas.models.media.NumberSchema; +import io.swagger.v3.oas.models.media.ObjectSchema; +import io.swagger.v3.oas.models.media.PasswordSchema; import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.oas.models.media.StringSchema; +import io.swagger.v3.oas.models.media.UUIDSchema; import static io.micronaut.openapi.visitor.Utils.resolveComponents; import static io.swagger.v3.oas.models.Components.COMPONENTS_SCHEMAS_REF; @@ -36,14 +54,65 @@ @Internal public final class SchemaUtils { + public static final String COMPONENTS_CALLBACKS_PREFIX = "#/components/callbacks/"; + public static final String COMPONENTS_SCHEMAS_PREFIX = "#/components/schemas/"; + public static final Schema EMPTY_SCHEMA = new Schema<>(); - public static final Schema EMPTY_SIMPLE_SCHEMA = new SimpleSchema(); + public static final Schema EMPTY_ARRAY_SCHEMA = new ArraySchema(); + public static final Schema EMPTY_BINARY_SCHEMA = new BinarySchema(); + public static final Schema EMPTY_BOOLEAN_SCHEMA = new BooleanSchema(); + public static final Schema EMPTY_BYTE_ARRAY_SCHEMA = new ByteArraySchema(); public static final Schema EMPTY_COMPOSED_SCHEMA = new ComposedSchema(); + public static final Schema EMPTY_DATE_SCHEMA = new DateSchema(); + public static final Schema EMPTY_DATE_TIME_SCHEMA = new DateTimeSchema(); + public static final Schema EMPTY_EMAIL_SCHEMA = new EmailSchema(); + public static final Schema EMPTY_FILE_SCHEMA = new FileSchema(); + public static final Schema EMPTY_INTEGER_SCHEMA = new IntegerSchema(); + public static final Schema EMPTY_JSON_SCHEMA = new JsonSchema(); + public static final Schema EMPTY_MAP_SCHEMA = new MapSchema(); + public static final Schema EMPTY_NUMBER_SCHEMA = new NumberSchema(); + public static final Schema EMPTY_OBJECT_SCHEMA = new ObjectSchema(); + public static final Schema EMPTY_PASSWORD_SCHEMA = new PasswordSchema(); + public static final Schema EMPTY_STRING_SCHEMA = new StringSchema(); + public static final Schema EMPTY_UUID_SCHEMA = new UUIDSchema(); + public static final Schema EMPTY_SIMPLE_SCHEMA = new SimpleSchema(); + public static final String TYPE_OBJECT = "object"; + private static final List> ALL_EMPTY_SCHEMAS; + + static { + List> schemas = new ArrayList<>(); + schemas.add(EMPTY_SCHEMA); + schemas.add(EMPTY_ARRAY_SCHEMA); + schemas.add(EMPTY_BINARY_SCHEMA); + schemas.add(EMPTY_BOOLEAN_SCHEMA); + schemas.add(EMPTY_BYTE_ARRAY_SCHEMA); + schemas.add(EMPTY_COMPOSED_SCHEMA); + schemas.add(EMPTY_DATE_SCHEMA); + schemas.add(EMPTY_DATE_TIME_SCHEMA); + schemas.add(EMPTY_EMAIL_SCHEMA); + schemas.add(EMPTY_FILE_SCHEMA); + schemas.add(EMPTY_INTEGER_SCHEMA); + schemas.add(EMPTY_JSON_SCHEMA); + schemas.add(EMPTY_MAP_SCHEMA); + schemas.add(EMPTY_NUMBER_SCHEMA); + schemas.add(EMPTY_OBJECT_SCHEMA); + schemas.add(EMPTY_PASSWORD_SCHEMA); + schemas.add(EMPTY_STRING_SCHEMA); + schemas.add(EMPTY_UUID_SCHEMA); + schemas.add(EMPTY_SIMPLE_SCHEMA); + + ALL_EMPTY_SCHEMAS = Collections.unmodifiableList(schemas); + } + private SchemaUtils() { } + public static boolean isEmptySchema(Schema schema) { + return ALL_EMPTY_SCHEMAS.contains(schema); + } + public static Map resolveSchemas(OpenAPI openAPI) { Components components = resolveComponents(openAPI); Map schemas = components.getSchemas(); diff --git a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiEncodingSpec.groovy b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiEncodingSpec.groovy index e56ea982dd..e1143f3527 100644 --- a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiEncodingSpec.groovy +++ b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiEncodingSpec.groovy @@ -807,4 +807,104 @@ class MyBean {} operation.requestBody.content."multipart/form-data".encoding."template".contentType == "application/octet-stream" operation.requestBody.content."multipart/form-data".encoding."parameters".contentType == "application/json" } + + void "test build OpenAPI multipart form data with custom schema"() { + + when: + buildBeanDefinition('test.MyBean', ''' +package test; + +import java.util.HashMap; + +import javax.validation.constraints.NotNull; + +import io.micronaut.http.MediaType; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Post; +import io.micronaut.http.multipart.CompletedFileUpload; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Encoding; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.parameters.RequestBody; + +import jakarta.inject.Singleton; + +@Controller("/path/{input}") +class OpenApiController { + + /** + * Operation description. + * + * @param template Template description + * @param parameters Parameters description + */ + @RequestBody( + description = "Body description", + content = @Content( + schema = @Schema(implementation = UploadItem.class), + encoding = @Encoding(name = "parameters", contentType = MediaType.APPLICATION_JSON))) + @Post(consumes = MediaType.MULTIPART_FORM_DATA) + public String print(String input, + @NotNull CompletedFileUpload template, + String parameters) { + return null; + } +} + +class UploadItem { + + /** + * Upload item template description + */ + @Schema(type = "string", format = "binary") + @NotNull + public String template; + + @NotNull + public Parameters parameters; +} + + +class Parameters extends HashMap { + +} + +@Singleton +class MyBean {} +''') + then: "the state is correct" + Utils.testReference != null + + when: "The OpenAPI is retrieved" + OpenAPI openAPI = Utils.testReference + Operation operation = openAPI.paths.get("/path/{input}").post + + then: + operation + + operation.parameters.size() == 1 + operation.parameters.get(0).name == 'input' + operation.parameters.get(0).in == 'path' + operation.parameters.get(0).required + operation.parameters.get(0).schema + operation.parameters.get(0).schema.type == 'string' + + operation.requestBody + operation.requestBody.description == "Body description" + operation.requestBody.content + operation.requestBody.content.size() == 1 + operation.requestBody.content."multipart/form-data" + operation.requestBody.content."multipart/form-data".encoding."template".contentType == "application/octet-stream" + operation.requestBody.content."multipart/form-data".encoding."parameters".contentType == "application/json" + operation.requestBody.content."multipart/form-data".schema.$ref == '#/components/schemas/UploadItem' + + openAPI.components.schemas.UploadItem.required.size() == 2 + openAPI.components.schemas.UploadItem.required.contains('template') + openAPI.components.schemas.UploadItem.required.contains('parameters') + openAPI.components.schemas.UploadItem.properties.size() == 2 + openAPI.components.schemas.UploadItem.properties.'template'.type == 'string' + openAPI.components.schemas.UploadItem.properties.'template'.format == 'binary' + openAPI.components.schemas.UploadItem.properties.'template'.description == 'Upload item template description' + openAPI.components.schemas.UploadItem.properties.'parameters'.type == 'object' + } }