From f1bb82ac7a01ce39eb7cac3dd08d7ba7e0853b47 Mon Sep 17 00:00:00 2001 From: Andriy Slobodyanyk Date: Wed, 18 Dec 2024 12:25:39 +0400 Subject: [PATCH] Adds @Nullability annotation to Spring Boot generator * 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 --- .../codegen/languages/SpringCodegen.java | 3 + .../JavaSpring/nullableAnnotation.mustache | 1 + .../main/resources/JavaSpring/pojo.mustache | 10 +-- .../java/assertions/PropertyAssert.java | 14 +++ .../java/spring/SpringCodegenTest.java | 89 +++++++++++++++---- .../resources/3_0/nullable-annotation.yaml | 15 ++++ 6 files changed, 112 insertions(+), 20 deletions(-) create mode 100644 modules/openapi-generator/src/main/resources/JavaSpring/nullableAnnotation.mustache create mode 100644 modules/openapi-generator/src/test/resources/3_0/nullable-annotation.yaml diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java index cb3c52a99c750..4479ded2856fc 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java @@ -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"); @@ -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 diff --git a/modules/openapi-generator/src/main/resources/JavaSpring/nullableAnnotation.mustache b/modules/openapi-generator/src/main/resources/JavaSpring/nullableAnnotation.mustache new file mode 100644 index 0000000000000..352250582e44c --- /dev/null +++ b/modules/openapi-generator/src/main/resources/JavaSpring/nullableAnnotation.mustache @@ -0,0 +1 @@ +{{^required}}{{^useOptional}}{{#openApiNullable}}{{^isNullable}}@Nullable {{/isNullable}}{{/openApiNullable}}{{^openApiNullable}}@Nullable {{/openApiNullable}}{{/useOptional}}{{/required}} \ No newline at end of file diff --git a/modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache b/modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache index e94b47c326fe7..60f469641ce0d 100644 --- a/modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache +++ b/modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache @@ -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}} @@ -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}} @@ -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}} diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/assertions/PropertyAssert.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/assertions/PropertyAssert.java index d75727c8bc89e..2ae08ad55064b 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/assertions/PropertyAssert.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/assertions/PropertyAssert.java @@ -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); + } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java index f2d7bd6274589..cd1215179c839 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java @@ -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 photoUrls = new ArrayList<>();"); } @@ -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 stringList") - .fileContains("private List stringDefaultList = new ArrayList<>(Arrays.asList(\"A\", \"B\"));") - .fileContains("private List stringEmptyDefaultList = new ArrayList<>();") - .fileContains("Set stringSet") - .fileContains("private Set stringDefaultSet = new LinkedHashSet<>(Arrays.asList(\"A\", \"B\"));") - .fileContains("private Set stringEmptyDefaultSet = new LinkedHashSet<>();") - .fileDoesNotContain("private List<@Valid TagDto> tags = new ArrayList<>()") - .fileDoesNotContain("private Set<@Valid TagDto> tagsUnique = new LinkedHashSet<>()") - .fileDoesNotContain("private List stringList = new ArrayList<>()") - .fileDoesNotContain("private Set 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 stringList") + .fileContains("private @Nullable List stringDefaultList = new ArrayList<>(Arrays.asList(\"A\", \"B\"));") + .fileContains("private @Nullable List stringEmptyDefaultList = new ArrayList<>();") + .fileContains("@Nullable Set stringSet") + .fileContains("private @Nullable Set stringDefaultSet = new LinkedHashSet<>(Arrays.asList(\"A\", \"B\"));") + .fileContains("private @Nullable Set stringEmptyDefaultSet = new LinkedHashSet<>();") + .fileDoesNotContain("List<@Valid TagDto> tags = new ArrayList<>()") + .fileDoesNotContain("Set<@Valid TagDto> tagsUnique = new LinkedHashSet<>()") + .fileDoesNotContain("List stringList = new ArrayList<>()") + .fileDoesNotContain("Set stringSet = new LinkedHashSet<>()"); } @Test @@ -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 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 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 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"); + } } diff --git a/modules/openapi-generator/src/test/resources/3_0/nullable-annotation.yaml b/modules/openapi-generator/src/test/resources/3_0/nullable-annotation.yaml new file mode 100644 index 0000000000000..8af42a7b072c5 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/nullable-annotation.yaml @@ -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