Skip to content

Commit

Permalink
Adds @nullability annotation to Spring Boot generator
Browse files Browse the repository at this point in the history 
* issue-14427: [REQ][spring] Null-Safety annotations
* issue-17382: [REQ] spring generator add Nullable annotations

Motivations:
* Have Spring Boot generator client properly annotated for nullability to be able to check code using them with tools like NullAway
* As it is related to Spring then the `org.springframework.lang.Nullable` annotation was chosen to avoid discussion which `@Nullable` one is true one
* `@NonNull` wasn't used as I didn't see much benefit of it. Anyhow, an empty constructor and/or setters allow to put a `null` value there

Modifications:
* Adds nullableAnnotation template to handle nullability annotation on vars
* Adjust pojo templates to use the nullability template
* Adapts tests

Modifications:
* Runs export_docs_generator.sh script to update samples
  • Loading branch information
@slobodator
slobodator committed Dec 18, 2024
1 parent 289425b commit f1bb82a
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 20 deletions.
3 changes: 3 additions & 0 deletions ...les/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ public void processOpts() {
convertPropertyToStringAndWriteBack(RESOURCE_FOLDER, this::setResourceFolder);

typeMapping.put("file", "org.springframework.core.io.Resource");
importMapping.put("Nullable", "org.springframework.lang.Nullable");
importMapping.put("org.springframework.core.io.Resource", "org.springframework.core.io.Resource");
importMapping.put("DateTimeFormat", "org.springframework.format.annotation.DateTimeFormat");
importMapping.put("ApiIgnore", "springfox.documentation.annotations.ApiIgnore");
Expand Down Expand Up @@ -952,6 +953,8 @@ public void postProcessModelProperty(CodegenModel model, CodegenProperty propert
if (model.getVendorExtensions().containsKey("x-jackson-optional-nullable-helpers")) {
model.imports.add("Arrays");
}

model.imports.add("Nullable");
}

@Override
Expand Down
1 change: 1 addition & 0 deletions modules/openapi-generator/src/main/resources/JavaSpring/nullableAnnotation.mustache
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{^required}}{{^useOptional}}{{#openApiNullable}}{{^isNullable}}@Nullable {{/isNullable}}{{/openApiNullable}}{{^openApiNullable}}@Nullable {{/openApiNullable}}{{/useOptional}}{{/required}}
10 changes: 5 additions & 5 deletions modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ public class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}{{^parent}}
{{#isContainer}}
{{#useBeanValidation}}@Valid{{/useBeanValidation}}
{{#openApiNullable}}
private {{#isNullable}}{{>nullableDataTypeBeanValidation}} {{name}} = JsonNullable.<{{{datatypeWithEnum}}}>undefined();{{/isNullable}}{{^required}}{{^isNullable}}{{>nullableDataTypeBeanValidation}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/isNullable}}{{/required}}{{#required}}{{^isNullable}}{{>nullableDataTypeBeanValidation}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/isNullable}}{{/required}}
private {{>nullableAnnotation}}{{#isNullable}}{{>nullableDataTypeBeanValidation}} {{name}} = JsonNullable.<{{{datatypeWithEnum}}}>undefined();{{/isNullable}}{{^required}}{{^isNullable}}{{>nullableDataTypeBeanValidation}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/isNullable}}{{/required}}{{#required}}{{^isNullable}}{{>nullableDataTypeBeanValidation}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/isNullable}}{{/required}}
{{/openApiNullable}}
{{^openApiNullable}}
private {{>nullableDataType}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};
private {{>nullableAnnotation}}{{>nullableDataType}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};
{{/openApiNullable}}
{{/isContainer}}
{{^isContainer}}
Expand All @@ -86,10 +86,10 @@ public class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}{{^parent}}
@DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME)
{{/isDateTime}}
{{#openApiNullable}}
private {{#isNullable}}{{>nullableDataTypeBeanValidation}} {{name}} = JsonNullable.<{{{datatypeWithEnum}}}>undefined();{{/isNullable}}{{^required}}{{^isNullable}}{{>nullableDataTypeBeanValidation}} {{name}}{{#useOptional}} = Optional.{{^defaultValue}}empty(){{/defaultValue}}{{#defaultValue}}of({{{.}}}){{/defaultValue}};{{/useOptional}}{{^useOptional}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/useOptional}}{{/isNullable}}{{/required}}{{^isNullable}}{{#required}}{{>nullableDataTypeBeanValidation}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/required}}{{/isNullable}}
private {{>nullableAnnotation}}{{#isNullable}}{{>nullableDataTypeBeanValidation}} {{name}} = JsonNullable.<{{{datatypeWithEnum}}}>undefined();{{/isNullable}}{{^required}}{{^isNullable}}{{>nullableDataTypeBeanValidation}} {{name}}{{#useOptional}} = Optional.{{^defaultValue}}empty(){{/defaultValue}}{{#defaultValue}}of({{{.}}}){{/defaultValue}};{{/useOptional}}{{^useOptional}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/useOptional}}{{/isNullable}}{{/required}}{{^isNullable}}{{#required}}{{>nullableDataTypeBeanValidation}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};{{/required}}{{/isNullable}}
{{/openApiNullable}}
{{^openApiNullable}}
private {{>nullableDataType}} {{name}}{{#isNullable}} = null{{/isNullable}}{{^isNullable}}{{#defaultValue}} = {{{.}}}{{/defaultValue}}{{/isNullable}};
private {{>nullableAnnotation}}{{>nullableDataType}} {{name}}{{#isNullable}} = null{{/isNullable}}{{^isNullable}}{{#defaultValue}} = {{{.}}}{{/defaultValue}}{{/isNullable}};
{{/openApiNullable}}
{{/isContainer}}
{{/vars}}
Expand Down Expand Up @@ -130,7 +130,7 @@ public class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}{{^parent}}
/**
* Constructor with all args parameters
*/
public {{classname}}({{#vendorExtensions.x-java-all-args-constructor-vars}}{{{datatypeWithEnum}}} {{name}}{{^-last}}, {{/-last}}{{/vendorExtensions.x-java-all-args-constructor-vars}}) {
public {{classname}}({{#vendorExtensions.x-java-all-args-constructor-vars}}{{>nullableAnnotation}}{{{datatypeWithEnum}}} {{name}}{{^-last}}, {{/-last}}{{/vendorExtensions.x-java-all-args-constructor-vars}}) {
{{#parent}}
super({{#parentVars}}{{name}}{{^-last}}, {{/-last}}{{/parentVars}});
{{/parent}}
Expand Down
14 changes: 14 additions & 0 deletions ...napi-generator/src/test/java/org/openapitools/codegen/java/assertions/PropertyAssert.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,18 @@ public PropertyAssert withType(final String expectedType) {
public PropertyAnnotationsAssert assertPropertyAnnotations() {
return new PropertyAnnotationsAssert(this, actual.getAnnotations());
}

public PropertyAnnotationsAssert doesNotHaveAnnotation(String annotationName) {
return new PropertyAnnotationsAssert(
this,
actual.getAnnotations()
).doesNotContainWithName(annotationName);
}

public PropertyAnnotationsAssert hasAnnotation(String annotationName) {
return new PropertyAnnotationsAssert(
this,
actual.getAnnotations()
).containsWithName(annotationName);
}
}
89 changes: 74 additions & 15 deletions ...enapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4859,7 +4859,7 @@ public void optionalListShouldBeEmpty() throws IOException {
.collect(Collectors.toMap(File::getName, Function.identity()));

JavaFileAssert.assertThat(files.get("PetDto.java"))
.fileContains("private List<@Valid TagDto> tags = new ArrayList<>();")
.fileContains("private @Nullable List<@Valid TagDto> tags = new ArrayList<>();")
.fileContains("private List<String> photoUrls = new ArrayList<>();");

}
Expand Down Expand Up @@ -4893,20 +4893,20 @@ public void testCollectionTypesWithDefaults_issue_18102() throws IOException {
.collect(Collectors.toMap(File::getName, Function.identity()));

JavaFileAssert.assertThat(files.get("PetDto.java"))
.fileContains("private List<@Valid TagDto> tags")
.fileContains("private List<@Valid TagDto> tagsDefaultList = new ArrayList<>()")
.fileContains("private Set<@Valid TagDto> tagsUnique")
.fileContains("private Set<@Valid TagDto> tagsDefaultSet = new LinkedHashSet<>();")
.fileContains("private List<String> stringList")
.fileContains("private List<String> stringDefaultList = new ArrayList<>(Arrays.asList(\"A\", \"B\"));")
.fileContains("private List<String> stringEmptyDefaultList = new ArrayList<>();")
.fileContains("Set<String> stringSet")
.fileContains("private Set<String> stringDefaultSet = new LinkedHashSet<>(Arrays.asList(\"A\", \"B\"));")
.fileContains("private Set<String> stringEmptyDefaultSet = new LinkedHashSet<>();")
.fileDoesNotContain("private List<@Valid TagDto> tags = new ArrayList<>()")
.fileDoesNotContain("private Set<@Valid TagDto> tagsUnique = new LinkedHashSet<>()")
.fileDoesNotContain("private List<String> stringList = new ArrayList<>()")
.fileDoesNotContain("private Set<String> stringSet = new LinkedHashSet<>()");
.fileContains("private @Nullable List<@Valid TagDto> tags")
.fileContains("private @Nullable List<@Valid TagDto> tagsDefaultList = new ArrayList<>()")
.fileContains("private @Nullable Set<@Valid TagDto> tagsUnique")
.fileContains("private @Nullable Set<@Valid TagDto> tagsDefaultSet = new LinkedHashSet<>();")
.fileContains("private @Nullable List<String> stringList")
.fileContains("private @Nullable List<String> stringDefaultList = new ArrayList<>(Arrays.asList(\"A\", \"B\"));")
.fileContains("private @Nullable List<String> stringEmptyDefaultList = new ArrayList<>();")
.fileContains("@Nullable Set<String> stringSet")
.fileContains("private @Nullable Set<String> stringDefaultSet = new LinkedHashSet<>(Arrays.asList(\"A\", \"B\"));")
.fileContains("private @Nullable Set<String> stringEmptyDefaultSet = new LinkedHashSet<>();")
.fileDoesNotContain("List<@Valid TagDto> tags = new ArrayList<>()")
.fileDoesNotContain("Set<@Valid TagDto> tagsUnique = new LinkedHashSet<>()")
.fileDoesNotContain("List<String> stringList = new ArrayList<>()")
.fileDoesNotContain("Set<String> stringSet = new LinkedHashSet<>()");
}

@Test
Expand Down Expand Up @@ -5099,4 +5099,63 @@ public void testEnumUnknownDefaultCaseDeserializationNotSet_issue13241() throws
.assertMethod("build")
.doesNotHaveAnnotation("Deprecated");
}

@Test
public void shouldAnnotateNonRequiredFieldsAsNullable() throws IOException {
SpringCodegen codegen = new SpringCodegen();
codegen.setLibrary(SPRING_BOOT);

Map<String, File> files = generateFiles(codegen, "src/test/resources/3_0/nullable-annotation.yaml");
var file = files.get("Item.java");

JavaFileAssert.assertThat(file)
.assertProperty("mandatoryName")
.doesNotHaveAnnotation("Nullable");
JavaFileAssert.assertThat(file)
.assertProperty("optionalDescription")
.hasAnnotation("Nullable");
JavaFileAssert.assertThat(file)
.assertProperty("nullableStr")
.doesNotHaveAnnotation("Nullable");
}

@Test
public void shouldNotAnnotateNonRequiredFieldsAsNullableWhileUseOptional() throws IOException {
SpringCodegen codegen = new SpringCodegen();
codegen.setLibrary(SPRING_BOOT);
codegen.setUseOptional(true);

Map<String, File> files = generateFiles(codegen, "src/test/resources/3_0/nullable-annotation.yaml");
var file = files.get("Item.java");

JavaFileAssert.assertThat(file)
.assertProperty("mandatoryName")
.doesNotHaveAnnotation("Nullable");
JavaFileAssert.assertThat(file)
.assertProperty("optionalDescription")
.doesNotHaveAnnotation("Nullable");
JavaFileAssert.assertThat(file)
.assertProperty("nullableStr")
.doesNotHaveAnnotation("Nullable");
}

@Test
public void shouldNotAnnotateNonRequiredFieldsAsNullableWhileNotUsingOpenApiNullable() throws IOException {
SpringCodegen codegen = new SpringCodegen();
codegen.setLibrary(SPRING_BOOT);
codegen.setOpenApiNullable(false);

Map<String, File> files = generateFiles(codegen, "src/test/resources/3_0/nullable-annotation.yaml");
var file = files.get("Item.java");

JavaFileAssert.assertThat(file)
.assertProperty("mandatoryName")
.doesNotHaveAnnotation("Nullable");
JavaFileAssert.assertThat(file)
.assertProperty("optionalDescription")
.hasAnnotation("Nullable");
JavaFileAssert.assertThat(file)
.assertProperty("nullableStr")
.hasAnnotation("Nullable");
}
}
15 changes: 15 additions & 0 deletions modules/openapi-generator/src/test/resources/3_0/nullable-annotation.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
openapi: 3.0.0
components:
schemas:
Item:
type: object
required:
- mandatoryName
properties:
mandatoryName:
type: String
optionalDescription:
type: string
nullableStr:
type: String
nullable: true

0 comments on commit f1bb82a

Please sign in to comment.