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.

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

OpenAPI Generator Return type Improvements #1098

Merged
merged 2 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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
}

}
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

This file was deleted.

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