refacot is null schema check (#19873)

OpenAPITools · Oct 15, 2024 · 3b3f9a7 · 3b3f9a7
1 parent 2354d40
commit 3b3f9a7
Show file tree

Hide file tree

Showing 5 changed files with 193 additions and 72 deletions.
diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java
@@ -951,68 +951,7 @@ private Schema processSimplifyAnyOfStringAndEnumString(Schema schema) {
         return result;
     }
 
-    /**
-     * Check if the schema is of type 'null' or schema itself is pointing to null
-     * <p>
-     * Return true if the schema's type is 'null' or not specified
-     *
-     * @param schema Schema
-     * @param openAPI OpenAPI
-     *
-     * @return true if schema is null type
-     */
-    public boolean isNullTypeSchema(OpenAPI openAPI, Schema schema) {
-        if (schema == null) {
-            return true;
-        }
-
-        // dereference the schema
-        schema = ModelUtils.getReferencedSchema(openAPI, schema);
-
-        // allOf/anyOf/oneOf
-        if (ModelUtils.hasAllOf(schema) || ModelUtils.hasOneOf(schema) || ModelUtils.hasAnyOf(schema)) {
-            return false;
-        }
-
-        // schema with properties
-        if (schema.getProperties() != null) {
-            return false;
-        }
-
-        // convert referenced enum of null only to `nullable:true`
-        if (schema.getEnum() != null && schema.getEnum().size() == 1) {
-            if ("null".equals(String.valueOf(schema.getEnum().get(0)))) {
-                return true;
-            }
-        }
-
-        if (schema.getTypes() != null && !schema.getTypes().isEmpty()) {
-            // 3.1 spec
-            if (schema.getTypes().size() == 1) { // 1 type only
-                String type = (String) schema.getTypes().iterator().next();
-                return type == null || "null".equals(type);
-            } else { // more than 1 type so must not be just null
-                return false;
-            }
-        }
 
-        if (schema instanceof JsonSchema) { // 3.1 spec
-            if (Boolean.TRUE.equals(schema.getNullable())) {
-                return true;
-            }
-
-            // for `type: null`
-            if (schema.getTypes() == null && schema.get$ref() == null) {
-                return true;
-            }
-        } else { // 3.0.x or 2.x spec
-            if ((schema.getType() == null || schema.getType().equals("null")) && schema.get$ref() == null) {
-                return true;
-            }
-        }
-
-        return false;
-    }
 
     /**
      * If the schema is oneOf and the sub-schemas is null, set `nullable: true`
@@ -1056,7 +995,7 @@ private Schema processSimplifyOneOf(Schema schema) {
                 }
             }
 
-            if (oneOfSchemas.removeIf(oneOf -> isNullTypeSchema(openAPI, oneOf))) {
+            if (oneOfSchemas.removeIf(oneOf -> ModelUtils.isNullTypeSchema(openAPI, oneOf))) {
                 schema.setNullable(true);
 
                 // if only one element left, simplify to just the element (schema)
@@ -1192,7 +1131,7 @@ private Schema processSimplifyAnyOf(Schema schema) {
                 }
             }
 
-            if (anyOfSchemas.removeIf(anyOf -> isNullTypeSchema(openAPI, anyOf))) {
+            if (anyOfSchemas.removeIf(anyOf -> ModelUtils.isNullTypeSchema(openAPI, anyOf))) {
                 schema.setNullable(true);
             }
 

diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java
@@ -2199,6 +2199,69 @@ public static Schema cloneSchema(Schema schema, boolean openapi31) {
         }
     }
 
+    /**
+     * Check if the schema is of type 'null' or schema itself is pointing to null
+     * <p>
+     * Return true if the schema's type is 'null' or not specified
+     *
+     * @param schema Schema
+     * @param openAPI OpenAPI
+     *
+     * @return true if schema is null type
+     */
+    public static boolean isNullTypeSchema(OpenAPI openAPI, Schema schema) {
+        if (schema == null) {
+            return true;
+        }
+
+        // dereference the schema
+        schema = ModelUtils.getReferencedSchema(openAPI, schema);
+
+        // allOf/anyOf/oneOf
+        if (ModelUtils.hasAllOf(schema) || ModelUtils.hasOneOf(schema) || ModelUtils.hasAnyOf(schema)) {
+            return false;
+        }
+
+        // schema with properties
+        if (schema.getProperties() != null) {
+            return false;
+        }
+
+        // convert referenced enum of null only to `nullable:true`
+        if (schema.getEnum() != null && schema.getEnum().size() == 1) {
+            if ("null".equals(String.valueOf(schema.getEnum().get(0)))) {
+                return true;
+            }
+        }
+
+        if (schema.getTypes() != null && !schema.getTypes().isEmpty()) {
+            // 3.1 spec
+            if (schema.getTypes().size() == 1) { // 1 type only
+                String type = (String) schema.getTypes().iterator().next();
+                return type == null || "null".equals(type);
+            } else { // more than 1 type so must not be just null
+                return false;
+            }
+        }
+
+        if (schema instanceof JsonSchema) { // 3.1 spec
+            if (Boolean.TRUE.equals(schema.getNullable())) {
+                return true;
+            }
+
+            // for `type: null`
+            if (schema.getTypes() == null && schema.get$ref() == null) {
+                return true;
+            }
+        } else { // 3.0.x or 2.x spec
+            if ((schema.getType() == null || schema.getType().equals("null")) && schema.get$ref() == null) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     @FunctionalInterface
     private interface OpenAPISchemaVisitor {
 

diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java
@@ -107,15 +107,6 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOfStringAndEnumString() {
         assertTrue(schema3.getEnum().size() > 0);
     }
 
-    @Test
-    public void isNullTypeSchemaTest() {
-        OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml");
-        Map<String, String> options = new HashMap<>();
-        OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
-        Schema schema = openAPI.getComponents().getSchemas().get("AnyOfStringArrayOfString");
-        assertFalse(openAPINormalizer.isNullTypeSchema(openAPI, schema));
-    }
-
     @Test
     public void testOpenAPINormalizerSimplifyOneOfAnyOf() {
         // to test the rule SIMPLIFY_ONEOF_ANYOF

diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java
@@ -22,13 +22,17 @@
 import io.swagger.v3.oas.models.parameters.Parameter;
 import io.swagger.v3.oas.models.parameters.RequestBody;
 import io.swagger.v3.oas.models.responses.ApiResponse;
+import org.openapitools.codegen.OpenAPINormalizer;
 import org.openapitools.codegen.TestUtils;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import java.math.BigDecimal;
 import java.util.*;
 
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
 public class ModelUtilsTest {
 
     @Test
@@ -469,4 +473,32 @@ public void testGetSchemaItemsWith31Spec() {
         Assert.assertNotNull(ModelUtils.getSchemaItems((Schema) arrayWithPrefixItems.getProperties().get("with_prefixitems")));
         Assert.assertNotNull(ModelUtils.getSchemaItems((Schema) arrayWithPrefixItems.getProperties().get("without_items")));
     }
+
+    @Test
+    public void isNullTypeSchemaTest() {
+        OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/null_schema_test.yaml");
+        Map<String, String> options = new HashMap<>();
+        Schema schema = openAPI.getComponents().getSchemas().get("AnyOfStringArrayOfString");
+        assertFalse(ModelUtils.isNullTypeSchema(openAPI, schema));
+
+        schema = openAPI.getComponents().getSchemas().get("IntegerRef");
+        assertFalse(ModelUtils.isNullTypeSchema(openAPI, schema));
+
+        schema = openAPI.getComponents().getSchemas().get("OneOfAnyType");
+        assertFalse(ModelUtils.isNullTypeSchema(openAPI, schema));
+
+        schema = openAPI.getComponents().getSchemas().get("AnyOfAnyType");
+        assertFalse(ModelUtils.isNullTypeSchema(openAPI, schema));
+
+        schema = openAPI.getComponents().getSchemas().get("Parent");
+        assertFalse(ModelUtils.isNullTypeSchema(openAPI, schema));
+        // the dummy property is a ref to integer
+        assertFalse(ModelUtils.isNullTypeSchema(openAPI, (Schema) schema.getProperties().get("dummy")));
+
+        schema = openAPI.getComponents().getSchemas().get("AnyOfTest");
+        assertFalse(ModelUtils.isNullTypeSchema(openAPI, schema));
+        assertTrue(ModelUtils.isNullTypeSchema(openAPI, (Schema) schema.getAnyOf().get(1)));
+        assertTrue(ModelUtils.isNullTypeSchema(openAPI, (Schema) schema.getAnyOf().get(2)));
+        assertTrue(ModelUtils.isNullTypeSchema(openAPI, (Schema) schema.getAnyOf().get(3)));
+    }
 }
diff --git a/modules/openapi-generator/src/test/resources/3_0/null_schema_test.yaml b/modules/openapi-generator/src/test/resources/3_0/null_schema_test.yaml
@@ -0,0 +1,96 @@
+openapi: 3.0.1
+info:
+  version: 1.0.0
+  title: Example
+  license:
+    name: MIT
+servers:
+  - url: http://api.example.xyz/v1
+paths:
+  /person/display/{personId}:
+    get:
+      parameters:
+        - name: personId
+          in: path
+          required: true
+          description: The id of the person to retrieve
+          schema:
+            type: string
+      operationId: list
+      responses:
+        '200':
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/AnyOfTest"
+components:
+  schemas:
+    AnyOfTest:
+      description: to test anyOf
+      anyOf:
+        - type: string
+        - type: 'null'
+        - type: null
+        - $ref: null
+    OneOfTest:
+      description: to test oneOf
+      oneOf:
+        - type: integer
+        - type: 'null'
+        - type: null
+        - $ref: null
+    OneOfTest2:
+      description: to test oneOf
+      oneOf:
+        - type: string
+        - type: 'null'
+    OneOfNullableTest:
+      description: to test oneOf nullable
+      oneOf:
+        - type: integer
+        - type: string
+        - $ref: null
+    Parent:
+      type: object
+      properties:
+        dummy:
+          $ref: '#/components/schemas/IntegerRef'
+        number:
+          anyOf:
+            - $ref: '#/components/schemas/Number'
+    AnyOfStringArrayOfString:
+      anyOf:
+        - type: string
+        - type: array
+          items:
+            type: string
+    AnyOfAnyType:
+      anyOf:
+        - type: boolean
+        - type: array
+          items: {}
+        - type: object
+        - type: string
+        - type: number
+        - type: integer
+    AnyOfAnyTypeWithRef:
+      anyOf:
+        - type: boolean
+        - type: array
+          items: { }
+        - type: object
+        - type: string
+        - type: number
+        - $ref: '#/components/schemas/IntegerRef'
+    IntegerRef:
+      type: integer
+    OneOfAnyType:
+      oneOf:
+        - type: object
+        - type: boolean
+        - type: number
+        - type: string
+        - type: integer
+        - type: array
+          items: {}