Skip to content

Commit

Permalink
OpenAPI Generator Return type Improvements (#1098)
Browse files Browse the repository at this point in the history 
* Improve return type wrapping:
- Add an option to generate HttpResponse only where it is required
- Use Flux for list return types when reactive = true
  • Loading branch information
@andriy-dmytruk
andriy-dmytruk authored Jul 5, 2023
1 parent 8ce752b commit 2dc67e7
Show file tree
Hide file tree
Showing 14 changed files with 138 additions and 73 deletions.
98 changes: 70 additions & 28 deletions ...-generator/src/main/java/io/micronaut/openapi/generator/AbstractMicronautJavaCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ public abstract class AbstractMicronautJavaCodegen<T extends GeneratorOptionsBui
public static final String OPT_DATE_FORMAT = "dateFormat";
public static final String OPT_DATETIME_FORMAT = "datetimeFormat";
public static final String OPT_REACTIVE = "reactive";
public static final String OPT_WRAP_IN_HTTP_RESPONSE = "wrapInHttpResponse";
public static final String OPT_GENERATE_HTTP_RESPONSE_ALWAYS = "generateHttpResponseAlways";
public static final String OPT_GENERATE_HTTP_RESPONSE_WHERE_REQUIRED = "generateHttpResponseWhereRequired";
public static final String OPT_APPLICATION_NAME = "applicationName";
public static final String OPT_GENERATE_SWAGGER_ANNOTATIONS = "generateSwaggerAnnotations";
public static final String OPT_GENERATE_SWAGGER_ANNOTATIONS_SWAGGER_1 = "swagger1";
Expand All @@ -99,7 +100,8 @@ public abstract class AbstractMicronautJavaCodegen<T extends GeneratorOptionsBui
protected String testTool;
protected boolean requiredPropertiesInConstructor = true;
protected boolean reactive;
protected boolean wrapInHttpResponse;
protected boolean generateHttpResponseAlways;
protected boolean generateHttpResponseWhereRequired = true;
protected String appName;
protected String generateSwaggerAnnotations;
protected boolean generateOperationOnlyForFirstTag;
Expand Down Expand Up @@ -128,7 +130,6 @@ protected AbstractMicronautJavaCodegen() {
modelDocPath = "docs/models";
dateLibrary = OPT_DATE_LIBRARY_ZONED_DATETIME;
reactive = true;
wrapInHttpResponse = false;
appName = artifactId;
generateSwaggerAnnotations = this instanceof JavaMicronautClientCodegen ?
OPT_GENERATE_SWAGGER_ANNOTATIONS_FALSE : OPT_GENERATE_SWAGGER_ANNOTATIONS_SWAGGER_2;
Expand Down Expand Up @@ -168,7 +169,8 @@ protected AbstractMicronautJavaCodegen() {
cliOptions.add(CliOption.newBoolean(OPT_VISITABLE, "Generate visitor for subtypes with a discriminator", visitable));
cliOptions.add(CliOption.newBoolean(OPT_REQUIRED_PROPERTIES_IN_CONSTRUCTOR, "Allow only to create models with all the required properties provided in constructor", requiredPropertiesInConstructor));
cliOptions.add(CliOption.newBoolean(OPT_REACTIVE, "Make the responses use Reactor Mono as wrapper", reactive));
cliOptions.add(CliOption.newBoolean(OPT_WRAP_IN_HTTP_RESPONSE, "Wrap the response in HttpResponse object", wrapInHttpResponse));
cliOptions.add(CliOption.newBoolean(OPT_GENERATE_HTTP_RESPONSE_ALWAYS, "Always wrap the operations response in HttpResponse object", generateHttpResponseAlways));
cliOptions.add(CliOption.newBoolean(OPT_GENERATE_HTTP_RESPONSE_WHERE_REQUIRED, "Wrap the operations response in HttpResponse object where non-200 HTTP status codes or additional headers are defined", generateHttpResponseWhereRequired));
cliOptions.add(CliOption.newBoolean(OPT_GENERATE_OPERATION_ONLY_FOR_FIRST_TAG, "When false, the operation method will be duplicated in each of the tags if multiple tags are assigned to this operation. " +
"If true, each operation will be generated only once in the first assigned tag.", generateOperationOnlyForFirstTag));
CliOption testToolOption = new CliOption(OPT_TEST, "Specify which test tool to generate files for").defaultValue(testTool);
Expand Down Expand Up @@ -219,8 +221,12 @@ protected AbstractMicronautJavaCodegen() {
importMapping.put("LocalDate", "java.time.LocalDate");
}

public void setWrapInHttpResponse(boolean wrapInHttpResponse) {
this.wrapInHttpResponse = wrapInHttpResponse;
public void setGenerateHttpResponseAlways(boolean generateHttpResponseAlways) {
this.generateHttpResponseAlways = generateHttpResponseAlways;
}

public void setGenerateHttpResponseWhereRequired(boolean generateHttpResponseWhereRequired) {
this.generateHttpResponseWhereRequired = generateHttpResponseWhereRequired;
}

public void setReactive(boolean reactive) {
Expand Down Expand Up @@ -302,10 +308,14 @@ public void processOpts() {
}
writePropertyBack(OPT_REACTIVE, reactive);

if (additionalProperties.containsKey(OPT_WRAP_IN_HTTP_RESPONSE)) {
wrapInHttpResponse = convertPropertyToBoolean(OPT_WRAP_IN_HTTP_RESPONSE);
if (additionalProperties.containsKey(OPT_GENERATE_HTTP_RESPONSE_ALWAYS)) {
generateHttpResponseAlways = convertPropertyToBoolean(OPT_GENERATE_HTTP_RESPONSE_ALWAYS);
}
writePropertyBack(OPT_GENERATE_HTTP_RESPONSE_ALWAYS, generateHttpResponseAlways);
if (additionalProperties.containsKey(OPT_GENERATE_HTTP_RESPONSE_WHERE_REQUIRED)) {
generateHttpResponseWhereRequired = convertPropertyToBoolean(OPT_GENERATE_HTTP_RESPONSE_WHERE_REQUIRED);
}
writePropertyBack(OPT_WRAP_IN_HTTP_RESPONSE, wrapInHttpResponse);
writePropertyBack(OPT_GENERATE_HTTP_RESPONSE_WHERE_REQUIRED, generateHttpResponseWhereRequired);

if (additionalProperties.containsKey(OPT_GENERATE_OPERATION_ONLY_FOR_FIRST_TAG)) {
generateOperationOnlyForFirstTag = convertPropertyToBoolean(OPT_GENERATE_OPERATION_ONLY_FOR_FIRST_TAG);
Expand Down Expand Up @@ -605,12 +615,6 @@ public CodegenParameter fromParameter(Parameter param, Set<String> imports) {
return parameter;
}

@Override
public boolean getUseInlineModelResolver() {
// This will allow TODO
return false;
}

@Override
public CodegenOperation fromOperation(
String path, String httpMethod, Operation operation, List<Server> servers
Expand All @@ -626,6 +630,7 @@ public CodegenOperation fromOperation(
op.vendorExtensions.put("originReturnProperty", op.returnProperty);
processParametersWithAdditionalMappings(op.allParams, op.imports);
processWithResponseBodyMapping(op);
processOperationWithResponseWrappers(op);

return op;
}
Expand Down Expand Up @@ -699,23 +704,60 @@ private void processWithResponseBodyMapping(CodegenOperation op) {
}

if (bodyMapping != null) {
CodegenProperty newProperty = new CodegenProperty();
newProperty.required = true;
newProperty.isModel = bodyMapping.isValidated;
wrapOperationReturnType(op, bodyMapping.mappedBodyType,
bodyMapping.isValidated, bodyMapping.isListWrapper);
}
}

/**
* Wrap the return type of operation in the provided type.
*
* @param op The operation to modify.
* @param wrapperType The wrapper type.
* @param isValidated Whether the wrapper requires validation.
* @param isListWrapper Whether the wrapper should be around list items.
*/
private void wrapOperationReturnType(
CodegenOperation op, String wrapperType, boolean isValidated, boolean isListWrapper
) {
CodegenProperty newReturnType = new CodegenProperty();
newReturnType.required = true;
newReturnType.isModel = isValidated;

String typeName = makeSureImported(bodyMapping.mappedBodyType(), op.imports);
String typeName = makeSureImported(wrapperType, op.imports);

if (bodyMapping.isListWrapper) {
newProperty.dataType = typeName + '<' + op.returnBaseType + '>';
newProperty.items = op.returnProperty.items;
} else {
newProperty.dataType = typeName + '<' + op.returnType + '>';
newProperty.items = op.returnProperty;
if (isListWrapper && op.isArray && op.returnProperty.items != null) {
newReturnType.dataType = typeName + '<' + op.returnBaseType + '>';
newReturnType.items = op.returnProperty.items;
} else {
String originalReturnType = op.returnType;
if (originalReturnType == null) {
originalReturnType = "Void";
op.returnProperty = new CodegenProperty();
op.returnProperty.dataType = "Void";
}
newReturnType.dataType = typeName + '<' + originalReturnType + '>';
newReturnType.items = op.returnProperty;
}

op.returnType = newReturnType.dataType;
op.returnContainer = null;
op.returnProperty = newReturnType;
op.isArray = op.returnProperty.isArray;
}

private void processOperationWithResponseWrappers(CodegenOperation op) {
boolean hasNon200StatusCodes = op.responses.stream().anyMatch(
response -> !"200".equals(response.code) && response.code.startsWith("2")
);
boolean hasNonMappedHeaders = !op.responseHeaders.isEmpty();
boolean requiresHttpResponse = hasNon200StatusCodes || hasNonMappedHeaders;
if (generateHttpResponseAlways || (generateHttpResponseWhereRequired && requiresHttpResponse)) {
wrapOperationReturnType(op, "io.micronaut.http.HttpResponse", false, false);
}

op.returnType = newProperty.dataType;
op.returnContainer = null;
op.returnProperty = newProperty;
if (reactive) {
wrapOperationReturnType(op, "reactor.core.publisher.Mono", false, false);
}
}

Expand Down
20 changes: 14 additions & 6 deletions ...erator/src/main/java/io/micronaut/openapi/generator/MicronautCodeGeneratorEntryPoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private void configureOptions() {
codeGenerator.addResponseBodyMappings(options.responseBodyMappings);
}
codeGenerator.setReactive(options.reactive);
codeGenerator.setWrapInHttpResponse(options.wrapInHttpResponse);
codeGenerator.setGenerateHttpResponseAlways(options.generateHttpResponseAlways);
codeGenerator.setUseOptional(options.optional);
codeGenerator.setUseBeanValidation(options.beanValidation);
codeGenerator.setTestTool(options.testFramework.value);
Expand Down Expand Up @@ -301,7 +301,8 @@ private static class DefaultOptionsBuilder implements MicronautCodeGeneratorOpti
private List<AbstractMicronautJavaCodegen.ResponseBodyMapping> responseBodyMappings;
private boolean optional = false;
private boolean reactive = true;
private boolean wrapInHttpResponse;
private boolean generateHttpResponseAlways;
private boolean generateHttpResponseWhereRequired;
private TestFramework testFramework = TestFramework.JUNIT5;
private SerializationLibraryKind serializationLibraryKind = SerializationLibraryKind.MICRONAUT_SERDE_JACKSON;
private DateTimeFormat dateTimeFormat = DateTimeFormat.ZONED_DATETIME;
Expand Down Expand Up @@ -349,8 +350,14 @@ public MicronautCodeGeneratorOptionsBuilder withReactive(boolean reactive) {
}

@Override
public MicronautCodeGeneratorOptionsBuilder withWrapInHttpResponse(boolean wrapInHttpResponse) {
this.wrapInHttpResponse = wrapInHttpResponse;
public MicronautCodeGeneratorOptionsBuilder withGenerateHttpResponseAlways(boolean generateHttpResponseAlways) {
this.generateHttpResponseAlways = generateHttpResponseAlways;
return this;
}

@Override
public MicronautCodeGeneratorOptionsBuilder withGenerateHttpResponseWhereRequired(boolean generateHttpResponseWhereRequired) {
this.generateHttpResponseWhereRequired = generateHttpResponseWhereRequired;
return this;
}

Expand Down Expand Up @@ -385,7 +392,7 @@ public MicronautCodeGeneratorOptionsBuilder withDateTimeFormat(DateTimeFormat fo
}

private Options build() {
return new Options(apiPackage, modelPackage, invokerPackage, artifactId, parameterMappings, responseBodyMappings, beanValidation, optional, reactive, wrapInHttpResponse, testFramework, serializationLibraryKind, dateTimeFormat);
return new Options(apiPackage, modelPackage, invokerPackage, artifactId, parameterMappings, responseBodyMappings, beanValidation, optional, reactive, generateHttpResponseAlways, generateHttpResponseWhereRequired, testFramework, serializationLibraryKind, dateTimeFormat);
}
}
}
Expand Down Expand Up @@ -415,7 +422,8 @@ private record Options(
boolean beanValidation,
boolean optional,
boolean reactive,
boolean wrapInHttpResponse,
boolean generateHttpResponseAlways,
boolean generateHttpResponseWhereRequired,
TestFramework testFramework,
SerializationLibraryKind serializationLibraryKind,
MicronautCodeGeneratorOptionsBuilder.DateTimeFormat dateTimeFormat
Expand Down
16 changes: 12 additions & 4 deletions ...or/src/main/java/io/micronaut/openapi/generator/MicronautCodeGeneratorOptionsBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,20 @@ public interface MicronautCodeGeneratorOptionsBuilder {
MicronautCodeGeneratorOptionsBuilder withReactive(boolean reactive);

/**
* If true, the generated client will use responses wrapped in HttpResponse.
* If true, the generated operation return types will be wrapped in HttpResponse.
*
* @param wrapInHttpResponse the wrapping flag
* @param generateHttpResponseAlways the wrapping flag
* @return this builder
*/
MicronautCodeGeneratorOptionsBuilder withWrapInHttpResponse(boolean wrapInHttpResponse);
MicronautCodeGeneratorOptionsBuilder withGenerateHttpResponseAlways(boolean generateHttpResponseAlways);

/**
* Wrap the operations response in HttpResponse object where non-200 HTTP status codes or additional headers are defined.
*
* @param generateHttpResponseWhereRequired the wrapping flag
* @return this builder
*/
MicronautCodeGeneratorOptionsBuilder withGenerateHttpResponseWhereRequired(boolean generateHttpResponseWhereRequired);

/**
* If set to true, the generated code will use bean validation.
Expand Down Expand Up @@ -134,7 +142,7 @@ public interface MicronautCodeGeneratorOptionsBuilder {
enum DateTimeFormat {
OFFSET_DATETIME,
ZONED_DATETIME,
LOCAL_DATETIME;
LOCAL_DATETIME
}

}
2 changes: 1 addition & 1 deletion openapi-generator/src/main/resources/templates/java-micronaut/client/api.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public interface {{classname}} {
{{/authMethods}}
{{/configureAuth}}
{{!the method definition}}
{{>common/operationReturnType}} {{nickname}}({{#allParams}}
{{^returnType}}void{{/returnType}}{{#returnType}}{{{returnType}}}{{/returnType}} {{nickname}}({{#allParams}}
{{#formatSingleLine}}{{>client/params/queryParams}}{{>client/params/pathParams}}{{>client/params/headerParams}}{{>client/params/bodyParams}}{{>client/params/formParams}}{{>client/params/cookieParams}}{{^-last}},{{/-last}}{{/formatSingleLine}}
{{/allParams}});
{{/formatNoEmptyLines}}
Expand Down
12 changes: 0 additions & 12 deletions ...generator/src/main/resources/templates/java-micronaut/common/operationReturnType.mustache
View file

This file was deleted.

2 changes: 1 addition & 1 deletion ...tor/src/main/resources/templates/java-micronaut/server/controller-implementation.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class {{controllerClassname}} implements {{classname}} {
{{#operation}}
{{!the method definition}}
@Override
public {{>common/operationReturnType}} {{nickname}}({{#allParams}}{{{dataType}}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}) {
public {{^returnType}}void{{/returnType}}{{#returnType}}{{{returnType}}}{{/returnType}} {{nickname}}({{#allParams}}{{{dataType}}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}) {
{{>server/controllerOperationBody}} }
{{^-last}}

Expand Down
2 changes: 1 addition & 1 deletion ...enerator/src/main/resources/templates/java-micronaut/server/controller-interface.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public interface {{classname}} {
@Secured({{openbrace}}{{#vendorExtensions.x-roles}}{{{.}}}{{^-last}}, {{/-last}}{{/vendorExtensions.x-roles}}{{closebrace}})
{{/useAuth}}
{{!the method definition}}
{{>common/operationReturnType}} {{nickname}}({{#allParams}}
{{^returnType}}void{{/returnType}}{{#returnType}}{{{returnType}}}{{/returnType}} {{nickname}}({{#allParams}}
{{#indent}}
{{>common/params/validation}}
{{/indent}}
Expand Down
8 changes: 4 additions & 4 deletions ...pi-generator/src/test/java/io/micronaut/openapi/generator/MicronautServerCodegenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ void generateAuthRolesWithExtension() {
void doGenerateMonoWrapHttpResponse() {
JavaMicronautServerCodegen codegen = new JavaMicronautServerCodegen();
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_REACTIVE, "true");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_WRAP_IN_HTTP_RESPONSE, "true");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_GENERATE_HTTP_RESPONSE_ALWAYS, "true");
String outputPath = generateFiles(codegen, PETSTORE_PATH, CodegenConstants.MODELS, CodegenConstants.APIS);

String apiPath = outputPath + "src/main/java/org/openapitools/api/";
Expand All @@ -222,7 +222,7 @@ void doGenerateMonoWrapHttpResponse() {
void doGenerateMono() {
JavaMicronautServerCodegen codegen = new JavaMicronautServerCodegen();
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_REACTIVE, "true");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_WRAP_IN_HTTP_RESPONSE, "false");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_GENERATE_HTTP_RESPONSE_ALWAYS, "false");
String outputPath = generateFiles(codegen, PETSTORE_PATH, CodegenConstants.MODELS, CodegenConstants.APIS);

String apiPath = outputPath + "src/main/java/org/openapitools/api/";
Expand All @@ -234,7 +234,7 @@ void doGenerateMono() {
void doGenerateWrapHttpResponse() {
JavaMicronautServerCodegen codegen = new JavaMicronautServerCodegen();
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_REACTIVE, "false");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_WRAP_IN_HTTP_RESPONSE, "true");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_GENERATE_HTTP_RESPONSE_ALWAYS, "true");
String outputPath = generateFiles(codegen, PETSTORE_PATH, CodegenConstants.MODELS, CodegenConstants.APIS);

String apiPath = outputPath + "src/main/java/org/openapitools/api/";
Expand All @@ -246,7 +246,7 @@ void doGenerateWrapHttpResponse() {
void doGenerateNoMonoNoWrapHttpResponse() {
JavaMicronautServerCodegen codegen = new JavaMicronautServerCodegen();
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_REACTIVE, "false");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_WRAP_IN_HTTP_RESPONSE, "false");
codegen.additionalProperties().put(JavaMicronautServerCodegen.OPT_GENERATE_HTTP_RESPONSE_ALWAYS, "false");
String outputPath = generateFiles(codegen, PETSTORE_PATH, CodegenConstants.MODELS, CodegenConstants.APIS);

String apiPath = outputPath + "src/main/java/org/openapitools/api/";
Expand Down
7 changes: 6 additions & 1 deletion test-suite-server-generator/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ tasks.named("generateOpenApi") {
// Response with Last-Modified header mapping
[headerName: "Last-Modified", mappedBodyType: "io.micronaut.openapi.test.dated.DatedResponse"],
// Response with Page body
[headerName: "X-Page-Number", mappedBodyType: "io.micronaut.data.model.Page", isListWrapper: true]
[headerName: "X-Page-Number", mappedBodyType: "io.micronaut.data.model.Page", isListWrapper: true],
[headerName: "X-Page-Count", mappedBodyType: "io.micronaut.data.model.Page", isListWrapper: true],
[headerName: "X-Total-Count", mappedBodyType: "io.micronaut.data.model.Page", isListWrapper: true],
[headerName: "X-Page-Size", mappedBodyType: "io.micronaut.data.model.Page", isListWrapper: true],
// Ignored header - Does not wrap the response in HttpResponse
[headerName: "ignored-header"]
]
}
2 changes: 1 addition & 1 deletion test-suite-server-generator/spec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ paths:
tags: [ responseBody ]
description: A method to get a simple model as a response
responses:
202:
201:
description: Success
content:
application/json:
Expand Down
Loading

0 comments on commit 2dc67e7

Please sign in to comment.