Set nullable on ComposedSchema instead of allOf

Fixes micronaut-projects#605
Chris-V · Dec 15, 2021 · 6da29df · 6da29df
1 parent ecba1b6
commit 6da29df
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 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
@@ -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
@@ -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<Pet> get(String name) {
+        return HttpResponse.ok();
+    }
+    
+    @Post("/pet/")
+    HttpResponse<Pet> 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
@@ -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
@@ -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'