From 8d86df21fd2baec94504af7944018912dc07f5d6 Mon Sep 17 00:00:00 2001 From: altro3 Date: Sun, 18 Feb 2024 15:43:29 +0700 Subject: [PATCH] Kotlin: Understand params with `@NotNull` annotations with nullable types as required Fixed #611 --- .../AbstractOpenApiEndpointVisitor.java | 14 ++--- .../visitor/AbstractOpenApiVisitor.java | 8 +-- .../openapi/visitor/ElementUtils.java | 5 +- .../OpenApiPojoControllerKotlinSpec.groovy | 63 +++++++++++++++++++ 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java index 1d2e28d7b8..07b517c4be 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java @@ -120,9 +120,9 @@ import static io.micronaut.openapi.visitor.ConfigUtils.isOpenApiEnabled; import static io.micronaut.openapi.visitor.ConfigUtils.isSpecGenerationEnabled; import static io.micronaut.openapi.visitor.ContextUtils.warn; -import static io.micronaut.openapi.visitor.ElementUtils.isIgnoredParameter; -import static io.micronaut.openapi.visitor.ElementUtils.isElementNotNullable; import static io.micronaut.openapi.visitor.ElementUtils.isFileUpload; +import static io.micronaut.openapi.visitor.ElementUtils.isIgnoredParameter; +import static io.micronaut.openapi.visitor.ElementUtils.isNotNullable; import static io.micronaut.openapi.visitor.ElementUtils.isNullable; import static io.micronaut.openapi.visitor.SchemaUtils.COMPONENTS_CALLBACKS_PREFIX; import static io.micronaut.openapi.visitor.SchemaUtils.COMPONENTS_SCHEMAS_PREFIX; @@ -727,7 +727,7 @@ private void processParameter(VisitorContext context, OpenAPI openAPI, if (StringUtils.isNotEmpty(bodyAnnValue)) { var wrapperSchema = new Schema<>(); wrapperSchema.setType(TYPE_OBJECT); - if (isElementNotNullable(parameter, parameterType)) { + if (isNotNullable(parameter)) { wrapperSchema.addRequiredItem(bodyAnnValue); } wrapperSchema.addProperty(bodyAnnValue, propertySchema); @@ -780,7 +780,7 @@ private void processParameter(VisitorContext context, OpenAPI openAPI, newParameter.setName(parameter.getName()); } - if (newParameter.getRequired() == null && !isNullable(parameter)) { + if (newParameter.getRequired() == null && (!isNullable(parameter) || isNotNullable(parameter))) { newParameter.setRequired(true); } if (javadocDescription != null && StringUtils.isEmpty(newParameter.getDescription())) { @@ -837,7 +837,7 @@ private void processBodyParameter(VisitorContext context, OpenAPI openAPI, Javad Optional description = parameter.stringValue(io.swagger.v3.oas.annotations.Parameter.class, "description"); description.ifPresent(propertySchema::setDescription); processSchemaProperty(context, parameter, parameter.getType(), null, schema, propertySchema); - if (isNullable(parameter)) { + if (isNullable(parameter) && !isNotNullable(parameter)) { // Keep null if not propertySchema.setNullable(true); } @@ -1060,7 +1060,7 @@ private Parameter processMethodParameterAnnotation(VisitorContext context, io.sw } } - if (newParameter != null && isNullable(parameter)) { + if (newParameter != null && isNullable(parameter) && !isNotNullable(parameter)) { newParameter.setRequired(null); } @@ -1097,7 +1097,7 @@ private void processBody(VisitorContext context, OpenAPI openAPI, requestBody.setDescription(desc.toString()); } } - if (requestBody.getRequired() == null && !isNullable(parameterType)) { + if (requestBody.getRequired() == null && (!isNullable(parameterType) || isNotNullable(parameterType))) { requestBody.setRequired(true); } diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java index 4c01374452..957cdd97d7 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java @@ -138,7 +138,7 @@ import static io.micronaut.openapi.visitor.ConvertUtils.parseJsonString; import static io.micronaut.openapi.visitor.ConvertUtils.setDefaultValueObject; import static io.micronaut.openapi.visitor.ConvertUtils.toTupleSubMap; -import static io.micronaut.openapi.visitor.ElementUtils.isElementNotNullable; +import static io.micronaut.openapi.visitor.ElementUtils.isNotNullable; import static io.micronaut.openapi.visitor.ElementUtils.isFileUpload; import static io.micronaut.openapi.visitor.ElementUtils.isNullable; import static io.micronaut.openapi.visitor.OpenApiApplicationVisitor.expandProperties; @@ -1097,7 +1097,7 @@ protected void processSchemaProperty(VisitorContext context, TypedElement elemen } // check field annotations (@NonNull, @Nullable, etc.) - boolean isNotNullable = isElementNotNullable(element, classElement); + boolean isNotNullable = isNotNullable(element); // check as mandatory in constructor boolean isMandatoryInConstructor = doesParamExistsMandatoryInConstructor(element, classElement); boolean required = elementSchemaRequired.orElse(isNotNullable || isMandatoryInConstructor); @@ -1266,7 +1266,7 @@ protected Schema bindSchemaForElement(VisitorContext context, TypedElement el } // @Schema annotation takes priority over nullability annotations Boolean isSchemaNullable = element.booleanValue(io.swagger.v3.oas.annotations.media.Schema.class, "nullable").orElse(null); - boolean isNullable = (isSchemaNullable == null && isNullable(element)) || Boolean.TRUE.equals(isSchemaNullable); + boolean isNullable = (isSchemaNullable == null && isNullable(element) && !isNotNullable(element)) || Boolean.TRUE.equals(isSchemaNullable); if (isNullable) { topLevelSchema.setNullable(true); notOnlyRef = true; @@ -2598,7 +2598,7 @@ private void processArgTypeAnnotations(ClassElement type, @Nullable Schema schem if (schema == null || type == null || type.getAnnotationNames().isEmpty()) { return; } - if (isNullable(type)) { + if (isNullable(type) && !isNotNullable(type)) { schema.setNullable(true); } processJavaxValidationAnnotations(type, type, schema); diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/ElementUtils.java b/openapi/src/main/java/io/micronaut/openapi/visitor/ElementUtils.java index b67c0f6ddb..ab5a53a287 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/ElementUtils.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/ElementUtils.java @@ -27,7 +27,6 @@ import io.micronaut.core.annotation.AnnotationValue; import io.micronaut.core.annotation.Internal; -import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; import io.micronaut.http.multipart.FileUpload; import io.micronaut.inject.ast.ClassElement; @@ -140,17 +139,17 @@ public static boolean isFileUpload(ClassElement type) { * Checking if the element not nullable. * * @param element element - * @param classElement class element * * @return true if element is not nullable */ - public static boolean isElementNotNullable(Element element, @Nullable Element classElement) { + public static boolean isNotNullable(Element element) { return element.isAnnotationPresent("javax.validation.constraints.NotNull$List") || element.isAnnotationPresent("jakarta.validation.constraints.NotNull$List") || element.isAnnotationPresent("javax.validation.constraints.NotBlank$List") || element.isAnnotationPresent("jakarta.validation.constraints.NotBlank$List") || element.isAnnotationPresent("javax.validation.constraints.NotEmpty$List") || element.isAnnotationPresent("jakarta.validation.constraints.NotEmpty$List") + || element.isAnnotationPresent("jakarta.validation.constraints.NotEmpty$List") || element.isNonNull() || element.booleanValue(JsonProperty.class, "required").orElse(false); } diff --git a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiPojoControllerKotlinSpec.groovy b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiPojoControllerKotlinSpec.groovy index 2effb05112..0bb8c019b7 100644 --- a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiPojoControllerKotlinSpec.groovy +++ b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiPojoControllerKotlinSpec.groovy @@ -157,4 +157,67 @@ class MyBean schemas.Animal schemas.ColorEnum } + + void "test kotlin NotNull on nullable types"() { + + when: + buildBeanDefinition('test.MyBean', ''' +package test + +import io.micronaut.core.annotation.NonNull +import io.micronaut.http.annotation.Body +import io.micronaut.http.annotation.Controller +import io.micronaut.http.annotation.Get +import io.micronaut.http.annotation.Put +import io.micronaut.http.annotation.PathVariable +import io.micronaut.http.annotation.QueryValue +import io.micronaut.serde.annotation.Serdeable +import jakarta.validation.Valid +import jakarta.validation.constraints.* +import jakarta.validation.constraints.NotNull +import reactor.core.publisher.Mono + +@Controller +class HelloController { + + @Put("/sendModelWithDiscriminator") + fun sendModelWithDiscriminator( + @Body @NotNull @Valid animal: Animal + ): Mono = Mono.empty() + + @Get("/test{/myVar}") + fun sendModelWithDiscriminator( + @PathVariable myVar: String?, + @QueryValue param1: String? + ): String = "OK" +} + +@Serdeable +data class Animal ( + @NotNull + var color: String?, + @NonNull + var propertyClass: String?, +) + +@jakarta.inject.Singleton +class MyBean {} +''') + then: "the state is correct" + Utils.testReference != null + + when: "The OpenAPI is retrieved" + def openAPI = Utils.testReference + Schema schema = openAPI.components.schemas.Animal + + then: "the components are valid" + schema + schema.properties.size() == 2 + !schema.properties.color.nullable + !schema.properties.propertyClass.nullable + schema.required + schema.required.size() == 2 + schema.required[0] == 'color' + schema.required[1] == 'propertyClass' + } }