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

[Java] Feature/wrapped types and default values #11666

Open
wants to merge 39 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
d35c175
POC - generic wrapper data type
cachescrubber Feb 19, 2022
c18252c
Generate Samples
cachescrubber Feb 19, 2022
939e9cb
POC - generic wrapper data type (skip arrys and maps for now to reduc…
cachescrubber Feb 19, 2022
3493e69
Honor useOptional.
cachescrubber Feb 19, 2022
b173d8a
Generate Samples
cachescrubber Feb 19, 2022
49367c9
use Optional as setter method argument
cachescrubber Feb 19, 2022
cea8c75
Generate Samples
cachescrubber Feb 19, 2022
17f636a
Generate Samples
cachescrubber Feb 19, 2022
a574d46
Add Javadoc to new Properties, add equals, hasCode, and toString.
cachescrubber Feb 20, 2022
60bf823
Generate Samples
cachescrubber Feb 20, 2022
fafec77
Add JsonIgnore to imports...
cachescrubber Feb 20, 2022
7e7380f
Generate Samples
cachescrubber Feb 20, 2022
43c41c8
Fix Javadoc
cachescrubber Feb 20, 2022
1a696b9
vendorExtensions.x-expose-wrapper-in-setter
cachescrubber Feb 20, 2022
328a906
Generate Samples
cachescrubber Feb 20, 2022
1c26ef4
Use wrappedDefault (do not require to proactively adapt existing temp…
cachescrubber Feb 20, 2022
8d96802
Generate Samples
cachescrubber Feb 20, 2022
dab08d2
JsonNullable: Use lazy initialization for container types (restore pr…
cachescrubber Feb 21, 2022
d76b733
Add in samples and test data
welshm Feb 22, 2022
98b14d1
Update tests
welshm Feb 22, 2022
49d29f2
Merge pull request #1 from cachescrubber/add_tests
cachescrubber Feb 22, 2022
c23eeb1
Adjust nullability and optional tests
cachescrubber Feb 22, 2022
9bbc6eb
Add new samples
cachescrubber Feb 22, 2022
76392d5
Add new samples...
cachescrubber Feb 22, 2022
720827f
trigger ci
cachescrubber Feb 22, 2022
f2a7ea0
Merge branch 'master' into feature/wrapped_tyoes_and_default_values
cachescrubber Feb 22, 2022
a33cb8e
Generate Samples
cachescrubber Feb 22, 2022
fbd9726
Adjust JsonNullable activation
cachescrubber Feb 23, 2022
14038a3
Introduce useOptionalInModel and exposeOptionalInSetter cli options, …
cachescrubber Feb 23, 2022
126a825
Add options to sample projects
cachescrubber Feb 23, 2022
6c00762
Generate Samples
cachescrubber Feb 23, 2022
1ab0920
Generate Samples
cachescrubber Feb 23, 2022
87f6900
Merge branch 'feature/wrapped_tyoes_and_default_values' of github.com…
welshm Mar 23, 2022
166a60c
Update documentation and generated examples
welshm Mar 23, 2022
eee53f5
Merge branch 'master' into feature/wrapped_tyoes_and_default_values
welshm Jun 1, 2022
976dad2
Update generated output
welshm Jun 1, 2022
90d8fec
Convert to from `http` to `https` for maven repo for SpringBoot 3
welshm Jun 1, 2022
e20c617
Update to https for maven repo
welshm Jun 1, 2022
7fe6942
Get springboot-3 to pass with Java 17
welshm Jun 2, 2022
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
3 changes: 2 additions & 1 deletion bin/configs/spring-boot-oas3.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
generatorName: spring
outputDir: samples/openapi3/server/petstore/springboot
inputSpec: modules/openapi-generator/src/test/resources/3_0/petstore.yaml
inputSpec: modules/openapi-generator/src/test/resources/3_0/petstore-nullable.yaml
templateDir: modules/openapi-generator/src/main/resources/JavaSpring
additionalProperties:
groupId: org.openapitools.openapi3
documentationProvider: springdoc
useOptional: true
artifactId: springboot
snapshotVersion: "true"
hideGenerationTimestamp: "true"
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,27 @@ public class CodegenProperty implements Cloneable, IJsonSchemaValidationProperti
public String dataType;
public String datatypeWithEnum;
public String dataFormat;

/**
* Detect support for Wrapper Types.
* When isWrapped is true, the dataType should be wrapped using wrapperType.
*/
public boolean isWrapped;
/**
* The Wrapper Type itself. Wrapper Types are for Example {@link Optional} or
* org.openapitools.jackson.nullable.JsonNullable.
*/
public String wrapperType;
/**
* The concrete Wrapped Type declaration, ie {@code JsonNullable<String>} or {@code Optional<String>}.
*/
public String wrappedType;
/**
* A Function to wrap a dataType.
* For example JsonNullable.of or Optional.ofNullable.
*/
public String wrapperFunc;

/**
* The name of this property in the OpenAPI schema.
*/
Expand Down Expand Up @@ -955,6 +976,10 @@ public String toString() {
sb.append(", getHasDiscriminatorWithNonEmptyMapping=").append(hasDiscriminatorWithNonEmptyMapping);
sb.append(", composedSchemas=").append(composedSchemas);
sb.append(", hasMultipleTypes=").append(hasMultipleTypes);
sb.append(", isWrapped=").append(isWrapped);
sb.append(", wrapperType=").append(wrapperType);
sb.append(", wrappedType=").append(wrappedType);
sb.append(", wrapperFunc=").append(wrapperFunc);
sb.append('}');
return sb.toString();
}
Expand Down Expand Up @@ -1054,7 +1079,11 @@ public boolean equals(Object o) {
Objects.equals(xmlPrefix, that.xmlPrefix) &&
Objects.equals(xmlName, that.xmlName) &&
Objects.equals(xmlNamespace, that.xmlNamespace) &&
Objects.equals(multipleOf, that.multipleOf);
Objects.equals(multipleOf, that.multipleOf) &&
isWrapped == that.isWrapped &&
Objects.equals(wrapperType, that.wrapperType) &&
Objects.equals(wrappedType, that.wrappedType) &&
Objects.equals(wrapperFunc, that.wrapperFunc);
}

@Override
Expand All @@ -1074,6 +1103,7 @@ public int hashCode() {
vendorExtensions, hasValidation, isInherited, discriminatorValue, nameInCamelCase,
nameInSnakeCase, enumName, maxItems, minItems, isXmlAttribute, xmlPrefix, xmlName,
xmlNamespace, isXmlWrapped, isNull, additionalPropertiesIsAnyType, hasVars, hasRequired,
hasDiscriminatorWithNonEmptyMapping, composedSchemas, hasMultipleTypes);
hasDiscriminatorWithNonEmptyMapping, composedSchemas, hasMultipleTypes, isWrapped, wrapperType,
wrappedType, wrapperFunc);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,72 @@

package org.openapitools.codegen.languages;

import static org.openapitools.codegen.utils.StringUtils.camelize;
import static org.openapitools.codegen.utils.StringUtils.escape;
import static org.openapitools.codegen.utils.StringUtils.underscore;

import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.Operation;
import io.swagger.v3.oas.models.PathItem;
import io.swagger.v3.oas.models.examples.Example;
import io.swagger.v3.oas.models.media.*;
import io.swagger.v3.oas.models.media.ArraySchema;
import io.swagger.v3.oas.models.media.ComposedSchema;
import io.swagger.v3.oas.models.media.Content;
import io.swagger.v3.oas.models.media.MediaType;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.media.StringSchema;
import io.swagger.v3.oas.models.parameters.Parameter;
import io.swagger.v3.oas.models.parameters.RequestBody;
import io.swagger.v3.oas.models.servers.Server;
import io.swagger.v3.parser.util.SchemaTypeUtil;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.*;
import org.openapitools.codegen.languages.features.DocumentationProviderFeatures;
import org.openapitools.codegen.meta.features.*;
import org.openapitools.codegen.utils.ModelUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;
import java.io.IOException;
import java.time.LocalDate;
import java.time.ZoneId;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentSkipListSet;
import java.util.regex.Pattern;
import java.util.stream.Stream;

import static org.openapitools.codegen.utils.StringUtils.*;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.CliOption;
import org.openapitools.codegen.CodegenConfig;
import org.openapitools.codegen.CodegenConstants;
import org.openapitools.codegen.CodegenModel;
import org.openapitools.codegen.CodegenOperation;
import org.openapitools.codegen.CodegenParameter;
import org.openapitools.codegen.CodegenProperty;
import org.openapitools.codegen.DefaultCodegen;
import org.openapitools.codegen.languages.features.DocumentationProviderFeatures;
import org.openapitools.codegen.languages.features.OptionalFeatures;
import org.openapitools.codegen.meta.features.ClientModificationFeature;
import org.openapitools.codegen.meta.features.DocumentationFeature;
import org.openapitools.codegen.meta.features.GlobalFeature;
import org.openapitools.codegen.meta.features.SchemaSupportFeature;
import org.openapitools.codegen.meta.features.SecurityFeature;
import org.openapitools.codegen.meta.features.WireFormatFeature;
import org.openapitools.codegen.utils.ModelUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public abstract class AbstractJavaCodegen extends DefaultCodegen implements CodegenConfig,
DocumentationProviderFeatures {
DocumentationProviderFeatures, OptionalFeatures {

private final Logger LOGGER = LoggerFactory.getLogger(AbstractJavaCodegen.class);
private static final String ARTIFACT_VERSION_DEFAULT_VALUE = "1.0.0";
Expand Down Expand Up @@ -117,6 +149,7 @@ public abstract class AbstractJavaCodegen extends DefaultCodegen implements Code
protected String outputTestFolder = "";
protected DocumentationProvider documentationProvider;
protected AnnotationLibrary annotationLibrary;
protected boolean useOptional = false;

public AbstractJavaCodegen() {
super();
Expand Down Expand Up @@ -585,6 +618,7 @@ public void processOpts() {
importMapping.put("IOException", "java.io.IOException");
importMapping.put("Arrays", "java.util.Arrays");
importMapping.put("Objects", "java.util.Objects");
importMapping.put("Optional", "java.util.Optional");
importMapping.put("StringUtil", invokerPackage + ".StringUtil");
// import JsonCreator if JsonProperty is imported
// used later in recursive import in postProcessingModels
Expand Down Expand Up @@ -1287,9 +1321,43 @@ public void postProcessModelProperty(CodegenModel model, CodegenProperty propert

if (openApiNullable) {
if (Boolean.FALSE.equals(property.required) && Boolean.TRUE.equals(property.isNullable)) {
// Only add import when needed
model.imports.add("JsonNullable");
model.getVendorExtensions().put("x-jackson-optional-nullable-helpers", true);
}
// TODO: adjust other java based mustache templates...
if (SpringCodegen.class.equals(this.getClass()) && Boolean.FALSE.equals(property.required) && Boolean.TRUE.equals(property.isNullable)) {
// Only add import when needed
model.imports.add("JsonIgnore");
// Wrap dataType and defaults
property.isWrapped = true;
property.wrapperType = "JsonNullable";
property.wrappedType = "JsonNullable<" + property.datatypeWithEnum + ">";
property.wrapperFunc = "JsonNullable.of";
if (property.defaultValue != null) {
property.defaultValue = "JsonNullable.of(" + property.defaultValue + ")";
} else {
property.defaultValue = "JsonNullable.undefined()";
}
}
}
if (useOptional) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if optionals are enabled, it would be nice for non-nullable non-required maps/arrays to default to empty map/array instead of null since an empty list/map has the same meanining as a null one in that case.

if (Boolean.FALSE.equals(property.required) && Boolean.FALSE.equals(property.isNullable)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this misses a case:

useOptional: true
openApiNullable: false

private Optional<String> message;
private Optional<String> nullableMessage
private String requiredMessage;
private String requiredNullableMessage;

I think this would skip nullableMessage because it's marked nullable (but the overall nullability feature is disabled)

Copy link
Contributor

Choose a reason for hiding this comment

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

So I think this might still be a problem but I'm happy to iterate on this

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean it should be:

a) Boolean.FALSE.equals(property.required) && Boolean.FALSE.equals(property.isNullable)
or
b) Boolean.FALSE.equals(property.required) && Boolean.FALSE.equals(openApiNullable)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes - but happy to follow up with that or merge into my PR dealing with Optionals in POJOs

&& !property.isArray && !property.isMap) {
// Only add import when needed
model.imports.add("JsonIgnore");
model.imports.add("Optional");
// Wrap dataType and defaults
property.isWrapped = true;
property.wrapperType = "Optional";
property.wrappedType = "Optional<" + property.datatypeWithEnum + ">";
property.wrapperFunc = "Optional.ofNullable";
if (property.defaultValue != null) {
property.defaultValue = "Optional.of(" + property.defaultValue + ")";
} else {
property.defaultValue = "Optional.empty()";
}
}
}

if (property.isReadOnly) {
Expand Down Expand Up @@ -1808,6 +1876,11 @@ public void setAnnotationLibrary(AnnotationLibrary annotationLibrary) {
this.annotationLibrary = annotationLibrary;
}

@Override
public void setUseOptional(boolean useOptional) {
this.useOptional = useOptional;
}

@Override
public String escapeQuotationMark(String input) {
// remove " to avoid code injection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
import org.slf4j.LoggerFactory;

public class SpringCodegen extends AbstractJavaCodegen
implements BeanValidationFeatures, PerformBeanValidationFeatures, OptionalFeatures, SwaggerUIFeatures {
implements BeanValidationFeatures, PerformBeanValidationFeatures, SwaggerUIFeatures {
private final Logger LOGGER = LoggerFactory.getLogger(SpringCodegen.class);

public static final String TITLE = "title";
Expand Down Expand Up @@ -111,7 +111,6 @@ public class SpringCodegen extends AbstractJavaCodegen
protected boolean performBeanValidation = false;
protected boolean implicitHeaders = false;
protected boolean apiFirst = false;
protected boolean useOptional = false;
protected boolean virtualService = false;
protected boolean hateoas = false;
protected boolean returnSuccessCode = false;
Expand Down Expand Up @@ -974,11 +973,6 @@ public void setPerformBeanValidation(boolean performBeanValidation) {
this.performBeanValidation = performBeanValidation;
}

@Override
public void setUseOptional(boolean useOptional) {
this.useOptional = useOptional;
}

@Override
public void setUseSwaggerUI(boolean useSwaggerUI) {
this.useSwaggerUI = useSwaggerUI;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{#isWrapped}}{{{wrapperType}}}<{{{datatypeWithEnum}}}>{{/isWrapped}}{{^isWrapped}}{{{datatypeWithEnum}}}{{/isWrapped}}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{#isWrapped}}{{{wrapperType}}}<{{#useBeanValidation}}{{>beanValidationCore}}{{/useBeanValidation}}{{{datatypeWithEnum}}}>{{/isWrapped}}{{^isWrapped}}{{{datatypeWithEnum}}}{{/isWrapped}}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -49,39 +49,28 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}}{{^parent}}{{#ha
@SerializedName("{{baseName}}")
{{/gson}}
{{#isContainer}}
{{#useBeanValidation}}@Valid{{/useBeanValidation}}
{{#openApiNullable}}
private {{>nullableDataType}} {{name}} = {{#isNullable}}JsonNullable.undefined(){{/isNullable}}{{^isNullable}}{{#required}}{{{defaultValue}}}{{/required}}{{^required}}null{{/required}}{{/isNullable}};
{{/openApiNullable}}
{{^openApiNullable}}
private {{>nullableDataType}} {{name}} = {{#required}}{{{defaultValue}}}{{/required}}{{^required}}null{{/required}};
{{/openApiNullable}}
{{#useBeanValidation}}
@Valid
{{/useBeanValidation}}
{{/isContainer}}
{{^isContainer}}
{{#isDate}}
@DateTimeFormat(iso = DateTimeFormat.ISO.DATE)
{{/isDate}}
{{#isDateTime}}
@DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME)
{{/isDateTime}}
{{#openApiNullable}}
private {{>nullableDataType}} {{name}}{{#isNullable}} = JsonNullable.undefined(){{/isNullable}}{{^isNullable}}{{#defaultValue}} = {{{.}}}{{/defaultValue}}{{/isNullable}};
{{/openApiNullable}}
{{^openApiNullable}}
private {{>nullableDataType}} {{name}}{{#isNullable}} = null{{/isNullable}}{{^isNullable}}{{#defaultValue}} = {{{.}}}{{/defaultValue}}{{/isNullable}};
{{/openApiNullable}}
{{/isContainer}}
private {{>dataType}} {{name}}{{#defaultValue}} = {{{defaultValue}}}{{/defaultValue}};
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth noting: This changes the default way that containers were working before (defaulted to null if not required)

{{/vars}}
{{#vars}}

{{! begin feature: fluent setter methods }}
public {{classname}} {{name}}({{{datatypeWithEnum}}} {{name}}) {
{{#openApiNullable}}
this.{{name}} = {{#isNullable}}JsonNullable.of({{name}}){{/isNullable}}{{^isNullable}}{{name}}{{/isNullable}};
{{/openApiNullable}}
{{^openApiNullable}}
{{#isWrapped}}
this.{{name}} = {{wrapperFunc}}({{name}});
{{/isWrapped}}
{{^isWrapped}}
this.{{name}} = {{name}};
{{/openApiNullable}}
{{/isWrapped}}
return this;
}
{{#isArray}}
Expand All @@ -90,7 +79,7 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}}{{^parent}}{{#ha
{{#openApiNullable}}
{{^required}}
if (this.{{name}} == null{{#isNullable}} || !this.{{name}}.isPresent(){{/isNullable}}) {
this.{{name}} = {{#isNullable}}JsonNullable.of({{{defaultValue}}}){{/isNullable}}{{^isNullable}}{{{defaultValue}}}{{/isNullable}};
this.{{name}} = {{{defaultValue}}};
}
{{/required}}
this.{{name}}{{#isNullable}}.get(){{/isNullable}}.add({{name}}Item);
Expand Down Expand Up @@ -137,25 +126,42 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}}{{^parent}}{{#ha
{{#vendorExtensions.x-extra-annotation}}
{{{vendorExtensions.x-extra-annotation}}}
{{/vendorExtensions.x-extra-annotation}}
{{^isWrapped}}
{{#useBeanValidation}}
{{>beanValidation}}
{{/useBeanValidation}}
{{/isWrapped}}
{{#swagger2AnnotationLibrary}}
@Schema(name = "{{{baseName}}}", {{#isReadOnly}}accessMode = Schema.AccessMode.READ_ONLY, {{/isReadOnly}}{{#example}}example = "{{{.}}}", {{/example}}{{#description}}description = "{{{.}}}", {{/description}}required = {{{required}}})
{{/swagger2AnnotationLibrary}}
{{#swagger1AnnotationLibrary}}
@ApiModelProperty({{#example}}example = "{{{.}}}", {{/example}}{{#required}}required = {{required}}, {{/required}}{{#isReadOnly}}readOnly = {{{isReadOnly}}}, {{/isReadOnly}}value = "{{{description}}}")
{{/swagger1AnnotationLibrary}}
public {{>nullableDataType}} {{getter}}() {
public {{>dataType_beanValidation}} {{getter}}() {
return {{name}};
}

{{#vendorExtensions.x-setter-extra-annotation}}
{{{vendorExtensions.x-setter-extra-annotation}}}
{{/vendorExtensions.x-setter-extra-annotation}}
public void {{setter}}({{>nullableDataType}} {{name}}) {
{{^x-expose-wrapper-in-setter}}
{{#isWrapped}}
@JsonIgnore
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth documenting why JsonIgnore is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, what do we need @JsonIgnore for? I added because you have advised to do so.

Copy link
Contributor

Choose a reason for hiding this comment

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

Jackson will automatically look at all public getter, all setter and all public member variables (see https://github.com/FasterXML/jackson-annotations).

In order to make sure that this doesn't expect two different fields (one for the Optional<T> and one for T) for Jackson serialization, I think it's safe to include @JsonIgnore here.

{{/isWrapped}}
public void {{setter}}({{{datatypeWithEnum}}} {{name}}) {
cachescrubber marked this conversation as resolved.
Show resolved Hide resolved
{{#isWrapped}}
this.{{name}} = {{wrapperFunc}}({{name}});
{{/isWrapped}}
{{^isWrapped}}
this.{{name}} = {{name}};
{{/isWrapped}}
}
{{/x-expose-wrapper-in-setter}}
{{#x-expose-wrapper-in-setter}}
public void {{setter}}({{>dataType}} {{name}}) {
this.{{name}} = {{name}};
}
{{/x-expose-wrapper-in-setter}}
{{! end feature: getter and setter }}
{{/vars}}

Expand Down
Loading