From 6da29dff57a4ec5a844e28d41b7c17dfcf39bc53 Mon Sep 17 00:00:00 2001 From: Christopher Viel Date: Fri, 29 Oct 2021 12:51:27 -0400 Subject: [PATCH] Set nullable on ComposedSchema instead of allOf Fixes https://github.com/micronaut-projects/micronaut-openapi/issues/605 --- .../visitor/AbstractOpenApiVisitor.java | 40 +++++++--- .../visitor/OpenApiNullableTypesSpec.groovy | 79 +++++++++++++++++++ .../visitor/OpenApiRecursionSpec.groovy | 8 +- .../OpenApiSchemaInheritanceSpec.groovy | 8 +- 4 files changed, 117 insertions(+), 18 deletions(-) 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 b25770f864..f6fc6abe85 100644 --- a/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java +++ b/openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java @@ -145,6 +145,7 @@ abstract class AbstractOpenApiVisitor { private static final Lock VISITED_ELEMENTS_LOCK = new ReentrantLock(); private static final String ATTR_VISITED_ELEMENTS = "io.micronaut.OPENAPI.visited.elements"; private static final Schema EMPTY_SCHEMA = new Schema<>(); + private static final ComposedSchema EMPTY_COMPOSED_SCHEMA = new ComposedSchema(); /** * The JSON mapper. @@ -949,24 +950,43 @@ protected Schema bindSchemaForElement(VisitorContext context, Element element, C element.findAnnotation(Pattern.class).flatMap(p -> p.get("regexp", String.class)).ifPresent(finalSchemaToBind::setPattern); } - setSchemaDocumentation(element, schemaToBind); + final ComposedSchema composedSchema; + final Schema topLevelSchema; + if (originalSchema.get$ref() != null) { + composedSchema = new ComposedSchema(); + topLevelSchema = composedSchema; + } else { + composedSchema = null; + topLevelSchema = schemaToBind; + } + + setSchemaDocumentation(element, topLevelSchema); if (element.isAnnotationPresent(Deprecated.class)) { - schemaToBind.setDeprecated(true); + topLevelSchema.setDeprecated(true); + } + if (element.isNullable()) { + topLevelSchema.setNullable(true); } final String defaultValue = element.getValue(Bindable.class, "defaultValue", String.class).orElse(null); if (defaultValue != null && schemaToBind.getDefault() == null) { - schemaToBind.setDefault(defaultValue); - } - if (element.isNullable()) { - schemaToBind.setNullable(true); + topLevelSchema.setDefault(defaultValue); } final String defaultJacksonValue = element.stringValue(JsonProperty.class, "defaultValue").orElse(null); if (defaultJacksonValue != null && schemaToBind.getDefault() == null) { - schemaToBind.setDefault(defaultJacksonValue); + topLevelSchema.setDefault(defaultJacksonValue); + } + + if (composedSchema != null && !schemaToBind.equals(EMPTY_SCHEMA)) { + composedSchema.addAllOfItem(schemaToBind); } - return originalSchema.get$ref() != null && !schemaToBind.equals(EMPTY_SCHEMA) - ? new ComposedSchema().addAllOfItem(originalSchema).addAllOfItem(schemaToBind) - : originalSchema; + + if (composedSchema != null && !composedSchema.equals(EMPTY_COMPOSED_SCHEMA)) { + return element.isNullable() && (composedSchema.getAllOf() == null || composedSchema.getAllOf().isEmpty()) + ? composedSchema.addOneOfItem(originalSchema) + : composedSchema.addAllOfItem(originalSchema); + } + + return originalSchema; } private void setSchemaDocumentation(Element element, Schema schemaToBind) { diff --git a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiNullableTypesSpec.groovy b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiNullableTypesSpec.groovy index 1d910d59f9..df367716d1 100644 --- a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiNullableTypesSpec.groovy +++ b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiNullableTypesSpec.groovy @@ -82,4 +82,83 @@ class PetController { petSchema.properties["name"].type == "string" } + + void "test build OpenAPI for nullable fields"() { + when: + buildBeanDefinition('test.PetController',''' +package test; + +import io.micronaut.core.annotation.Nullable; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Get; +import io.micronaut.http.annotation.Put; +import io.micronaut.http.annotation.Post; +import io.micronaut.http.annotation.Status; + +import java.util.List; +import java.util.Optional; + +class Pet { + private String name; + private Pet mother; + + public void setName(String n) { + name = n; + } + + /** + * The Pet Name + * + * @return The Pet Name + */ + @Nullable + public String getName() { + return name; + } + + public void setMother(Pet mother) { + this.mother = mother; + } + + /** + * The Pet Mother + * + * @return The Pet Mother + */ + @Nullable + public Pet getMother() { + return mother; + } +} + +@Controller +class PetController { + + @Get("/pet/{name}") + HttpResponse get(String name) { + return HttpResponse.ok(); + } + + @Post("/pet/") + HttpResponse post(Pet p) { + return HttpResponse.ok(); + } +} + +''') + then:"the state is correct" + AbstractOpenApiVisitor.testReference != null + + when:"The OpenAPI is retrieved" + OpenAPI openAPI = AbstractOpenApiVisitor.testReference + Schema petSchema = openAPI.components.schemas['Pet'] + + then:"the components are valid" + petSchema.type == 'object' + petSchema.properties.size() == 2 + + petSchema.properties["name"].nullable + petSchema.properties["mother"].nullable + } } diff --git a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiRecursionSpec.groovy b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiRecursionSpec.groovy index 54e33ef6ce..231178675e 100644 --- a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiRecursionSpec.groovy +++ b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiRecursionSpec.groovy @@ -247,10 +247,10 @@ class MyBean {} Schema testImpl1 = schemas.get("TestImpl1") Schema woopsieProperty = testImpl1.getProperties()["woopsie"] woopsieProperty instanceof ComposedSchema - ((ComposedSchema) woopsieProperty).allOf[0].$ref == "#/components/schemas/TestInterface" - ((ComposedSchema) woopsieProperty).allOf[1].deprecated - ((ComposedSchema) woopsieProperty).allOf[1].description == "Some docs" - ((ComposedSchema) woopsieProperty).allOf[1].nullable + ((ComposedSchema) woopsieProperty).deprecated + ((ComposedSchema) woopsieProperty).description == "Some docs" + ((ComposedSchema) woopsieProperty).nullable + ((ComposedSchema) woopsieProperty).oneOf[0].$ref == "#/components/schemas/TestInterface" } } diff --git a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiSchemaInheritanceSpec.groovy b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiSchemaInheritanceSpec.groovy index 96c2720d1a..bc1e1755e2 100644 --- a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiSchemaInheritanceSpec.groovy +++ b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiSchemaInheritanceSpec.groovy @@ -422,10 +422,10 @@ class MyBean {} Schema owner = schemas["Owner"] Schema vehicleProperty = owner.getProperties()["vehicle"] vehicleProperty instanceof ComposedSchema - ((ComposedSchema) vehicleProperty).allOf[0].$ref == "#/components/schemas/Vehicle" - ((ComposedSchema) vehicleProperty).allOf[1].deprecated - ((ComposedSchema) vehicleProperty).allOf[1].description == "Some docs" - ((ComposedSchema) vehicleProperty).allOf[1].nullable + ((ComposedSchema) vehicleProperty).deprecated + ((ComposedSchema) vehicleProperty).description == "Some docs" + ((ComposedSchema) vehicleProperty).nullable + ((ComposedSchema) vehicleProperty).oneOf[0].$ref == "#/components/schemas/Vehicle" Schema vehicle = schemas["Vehicle"] vehicle instanceof ComposedSchema ((ComposedSchema) vehicle).oneOf[0].$ref == '#/components/schemas/Car'