Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix processing multipart form data #1055

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Fix processing multipart form data #1055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 58 additions & 10 deletions openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
import io.swagger.v3.oas.models.PathItem;
import io.swagger.v3.oas.models.media.ComposedSchema;
import io.swagger.v3.oas.models.media.Content;
import io.swagger.v3.oas.models.media.Encoding;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.parameters.CookieParameter;
import io.swagger.v3.oas.models.parameters.HeaderParameter;
Expand All @@ -106,10 +107,11 @@
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.JsonNode;

import static io.micronaut.openapi.visitor.ElementUtils.isFileUpload;
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.TYPE_OBJECT;
import static io.micronaut.openapi.visitor.ElementUtils.isNullable;
import static io.micronaut.openapi.visitor.Utils.DEFAULT_MEDIA_TYPES;

/**
Expand Down Expand Up @@ -392,6 +394,7 @@ public void visitMethod(MethodElement element, VisitorContext context) {
for (Map.Entry<String, List<PathItem>> pathItemEntry : pathItemsMap.entrySet()) {
List<PathItem> pathItems = pathItemEntry.getValue();

List<MediaType> consumesMediaTypes = consumesMediaTypes(element);
Map<PathItem, io.swagger.v3.oas.models.Operation> swaggerOperations = readOperations(pathItemEntry.getKey(), httpMethod, pathItems, element, context);

for (Map.Entry<PathItem, io.swagger.v3.oas.models.Operation> operationEntry : swaggerOperations.entrySet()) {
Expand Down Expand Up @@ -423,13 +426,13 @@ public void visitMethod(MethodElement element, VisitorContext context) {
readResponse(element, context, openAPI, swaggerOperation, javadocDescription);

if (permitsRequestBody) {
Optional<RequestBody> requestBody = readSwaggerRequestBody(element, context);
if (requestBody.isPresent()) {
RequestBody requestBody = readSwaggerRequestBody(element, consumesMediaTypes, context);
if (requestBody != null) {
RequestBody currentRequestBody = swaggerOperation.getRequestBody();
if (currentRequestBody != null) {
swaggerOperation.setRequestBody(mergeRequestBody(currentRequestBody, requestBody.get()));
swaggerOperation.setRequestBody(mergeRequestBody(currentRequestBody, requestBody));
} else {
swaggerOperation.setRequestBody(requestBody.get());
swaggerOperation.setRequestBody(requestBody);
}
}
}
Expand All @@ -447,7 +450,6 @@ public void visitMethod(MethodElement element, VisitorContext context) {
// @Parameters declared at method level take precedence over the declared as method arguments, so we process them first
processParameterAnnotationInMethod(element, openAPI, matchTemplate, httpMethod, pathVariables);
}
List<MediaType> consumesMediaTypes = consumesMediaTypes(element);
List<TypedElement> 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);
Expand Down Expand Up @@ -483,6 +485,10 @@ private void processExtraBodyParameters(VisitorContext context, HttpMethod httpM
if (requestBody != null && !extraBodyParameters.isEmpty()) {
requestBody.getContent().forEach((mediaTypeName, mediaType) -> {
Schema schema = mediaType.getSchema();
if (schema == null) {
schema = new Schema();
mediaType.setSchema(schema);
}
if (schema.get$ref() != null) {
ComposedSchema composedSchema = new ComposedSchema();
Schema extraBodyParametersSchema = new Schema();
Expand All @@ -494,6 +500,24 @@ private void processExtraBodyParameters(VisitorContext context, HttpMethod httpM
}
for (TypedElement parameter : extraBodyParameters) {
processBodyParameter(context, openAPI, javadocDescription, MediaType.of(mediaTypeName), schema, parameter);
if (mediaTypeName.equals(MediaType.MULTIPART_FORM_DATA)) {
for (String prop : (Set<String>) schema.getProperties().keySet()) {
Map<String, Encoding> 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);
}
}
}
}
});
}
Expand Down Expand Up @@ -637,7 +661,10 @@ private void processParameter(VisitorContext context, OpenAPI openAPI,
return;
}
if (permitsRequestBody && swaggerOperation.getRequestBody() == null) {
readSwaggerRequestBody(parameter, context).ifPresent(swaggerOperation::setRequestBody);
RequestBody requestBody = readSwaggerRequestBody(parameter, consumesMediaTypes, context);
if (requestBody != null) {
swaggerOperation.setRequestBody(requestBody);
}
}

consumesMediaTypes = CollectionUtils.isNotEmpty(consumesMediaTypes) ? consumesMediaTypes : DEFAULT_MEDIA_TYPES;
Expand Down Expand Up @@ -1689,9 +1716,30 @@ private void processResponses(io.swagger.v3.oas.models.Operation operation, List
}
}

private Optional<RequestBody> readSwaggerRequestBody(Element element, VisitorContext context) {
return element.findAnnotation(io.swagger.v3.oas.annotations.parameters.RequestBody.class)
.flatMap(annotation -> toValue(annotation.getValues(), context, RequestBody.class));
private RequestBody readSwaggerRequestBody(Element element, List<MediaType> consumesMediaTypes, VisitorContext context) {
AnnotationValue<io.swagger.v3.oas.annotations.parameters.RequestBody> requestBodyAnnValue =
element.findAnnotation(io.swagger.v3.oas.annotations.parameters.RequestBody.class).orElse(null);

if (requestBodyAnnValue == null) {
return null;
}

AnnotationValue<io.swagger.v3.oas.annotations.media.Content> content = requestBodyAnnValue.getAnnotation("content", io.swagger.v3.oas.annotations.media.Content.class).orElse(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
&& !content.stringValue("mediaType").isPresent()
&& requestBody != null
&& requestBody.getContent() != null
&& !consumesMediaTypes.equals(DEFAULT_MEDIA_TYPES)) {

io.swagger.v3.oas.models.media.MediaType defaultSwaggerMediaType = requestBody.getContent().remove(MediaType.APPLICATION_JSON);
for (MediaType mediaType : consumesMediaTypes) {
requestBody.getContent().put(mediaType.toString(), defaultSwaggerMediaType);
}
}

return requestBody;
}

private void readServers(MethodElement element, VisitorContext context, io.swagger.v3.oas.models.Operation swaggerOperation) {
Expand Down
6 changes: 2 additions & 4 deletions openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
import com.fasterxml.jackson.databind.node.ObjectNode;

import static io.micronaut.openapi.visitor.ConvertUtils.resolveExtensions;
import static io.micronaut.openapi.visitor.ElementUtils.isFileUpload;
import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.MICRONAUT_OPENAPI_FIELD_VISIBILITY_LEVEL;
import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.expandProperties;
import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.getConfigurationProperty;
Expand Down Expand Up @@ -880,10 +881,7 @@ protected Schema resolveSchema(OpenAPI openAPI, @Nullable Element definingElemen
}

// File upload case
if ("io.micronaut.http.multipart.StreamingFileUpload".equals(typeName) ||
"io.micronaut.http.multipart.CompletedFileUpload".equals(typeName) ||
"io.micronaut.http.multipart.CompletedPart".equals(typeName) ||
"io.micronaut.http.multipart.PartData".equals(typeName)) {
if (isFileUpload(type)) {
isPublisher = isPublisher && !"io.micronaut.http.multipart.PartData".equals(typeName);
// For file upload, we use PrimitiveType.BINARY
typeName = PrimitiveType.BINARY.name();
Expand Down
15 changes: 15 additions & 0 deletions openapi/src/main/java/io/micronaut/openapi/visitor/ElementUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,21 @@ public static boolean isNullable(TypedElement element) {
|| element.hasStereotype("org.jspecify.annotations.Nullable");
}

/**
* Checking if the type is file upload type.
*
* @param type type element
*
* @return true if this type one of known file upload types
*/
public static boolean isFileUpload(ClassElement type) {
String typeName = type.getName();
return "io.micronaut.http.multipart.StreamingFileUpload".equals(typeName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these types not accessible in the context?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Available, but that's not my code. so it was written before me. In general, I'm thinking about abandoning the direct use of the micronaut-http library and converting all places to strings, instead of explicit classes, and leaving only micronaut injection and micronaut core in dependencies. But this is just an idea for now. In any case, I don't see a problem with using the explicit class name and not the class itself at that point.

|| "io.micronaut.http.multipart.CompletedFileUpload".equals(typeName)
|| "io.micronaut.http.multipart.CompletedPart".equals(typeName)
|| "io.micronaut.http.multipart.PartData".equals(typeName);
}

/**
* Checking if the type is file.
*
Expand Down
163 changes: 163 additions & 0 deletions openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiEncodingSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,4 +644,167 @@ class MyBean {}
operation.requestBody.content."multipart/mixed2".encoding.secondOject.extensions."x-myExt22".prop1 == "prop1Val1"
operation.requestBody.content."multipart/mixed2".encoding.secondOject.extensions."x-myExt22".prop2 == "prop2Val2"
}

void "test build OpenAPI multipart form data"() {

when:
buildBeanDefinition('test.MyBean', '''
package test;

import javax.validation.constraints.NotNull;

import io.micronaut.http.HttpResponse;
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.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(encoding = {
@Encoding(name = "template", contentType = MediaType.APPLICATION_OCTET_STREAM),
@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;
}
}

@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".schema
operation.requestBody.content."multipart/form-data".schema.required.size() == 1
operation.requestBody.content."multipart/form-data".schema.required.get(0) == 'template'
operation.requestBody.content."multipart/form-data".schema.properties.size() == 2
operation.requestBody.content."multipart/form-data".schema.properties.'template'.type == 'string'
operation.requestBody.content."multipart/form-data".schema.properties.'template'.format == 'binary'
operation.requestBody.content."multipart/form-data".schema.properties.'parameters'.type == 'string'
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 auto set application/octet-stream"() {

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(encoding = {
@Encoding(name = "parameters", contentType = MediaType.APPLICATION_JSON),
}))
@Post(consumes = MediaType.MULTIPART_FORM_DATA)
public String print(
String input,
@NotNull CompletedFileUpload template,
@Schema(implementation = Parameters.class) String parameters) {
return null;
}
}

class Parameters extends HashMap<String, Object> {

}

@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".schema
operation.requestBody.content."multipart/form-data".schema.required.size() == 1
operation.requestBody.content."multipart/form-data".schema.required.get(0) == 'template'
operation.requestBody.content."multipart/form-data".schema.properties.size() == 2
operation.requestBody.content."multipart/form-data".schema.properties.'template'.type == 'string'
operation.requestBody.content."multipart/form-data".schema.properties.'template'.format == 'binary'
operation.requestBody.content."multipart/form-data".schema.properties.'parameters'.type == 'object'
operation.requestBody.content."multipart/form-data".encoding."template".contentType == "application/octet-stream"
operation.requestBody.content."multipart/form-data".encoding."parameters".contentType == "application/json"
}
}