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.

Sign up for GitHub

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

Kotlin: Understand params with @NotNull annotations with nullable types as required #1444

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Kotlin: Understand params with @NotNull annotations with nullable types as required #1444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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())) {
Expand Down Expand Up @@ -837,7 +837,7 @@ private void processBodyParameter(VisitorContext context, OpenAPI openAPI, Javad
Optional<String> 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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
8 changes: 4 additions & 4 deletions openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 1 addition & 3 deletions openapi/src/main/java/io/micronaut/openapi/visitor/ElementUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -140,11 +139,10 @@ 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")
Expand Down
7 changes: 4 additions & 3 deletions openapi/src/main/java/io/micronaut/openapi/visitor/OpenApiEndpointVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import io.micronaut.core.annotation.AnnotationValue;
import io.micronaut.core.naming.NameUtils;
import io.micronaut.core.util.ArrayUtils;
import io.micronaut.core.util.StringUtils;
import io.micronaut.http.HttpMethod;
import io.micronaut.http.MediaType;
import io.micronaut.http.uri.UriMatchTemplate;
Expand Down Expand Up @@ -148,9 +149,9 @@ protected boolean ignore(ClassElement element, VisitorContext context) {
}
if (element.isAnnotationPresent("io.micronaut.management.endpoint.annotation.Endpoint")) {
AnnotationValue<?> ann = element.getAnnotation("io.micronaut.management.endpoint.annotation.Endpoint");
String idAnn = ann.stringValue("id").orElse(NameUtils.hyphenate(element.getSimpleName()));
if (idAnn.isEmpty()) {
idAnn = ann.stringValue("value").orElse(idAnn);
String idAnn = ann.stringValue("id").orElse(ann.stringValue("value").orElse(null));
if (StringUtils.isEmpty(idAnn)) {
idAnn = NameUtils.hyphenate(element.getSimpleName());
}
id = path + idAnn;
if (id.isEmpty() || id.charAt(0) != '/') {
Expand Down
65 changes: 64 additions & 1 deletion openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiPojoControllerKotlinSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class HelloController {

@Get
@Produces(MediaType.TEXT_PLAIN)
fun index(@Nullable @QueryValue("channels") channels: Collection<Channel?>) = ""
fun index(@Nullable @QueryValue("channels") channels: Collection<Channel?>?) = ""

@Introspected
enum class Channel {
Expand Down Expand Up @@ -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<Animal> = 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'
}
}
Loading