From d94a114caa043ac6ca65996bda036d0daa2968dd Mon Sep 17 00:00:00 2001 From: William Cheng Date: Thu, 11 Apr 2024 16:38:48 +0800 Subject: [PATCH 1/2] fix null check in 3.1 spec --- .../codegen/OpenAPINormalizer.java | 12 +- .../codegen/utils/ModelUtils.java | 19 +++ .../codegen/OpenAPINormalizerTest.java | 86 ++++++++++++- .../3_0/simplifyOneOfAnyOf_test.yaml | 5 + .../3_1/simplifyOneOfAnyOf_test.yaml | 114 ++++++++++++++++++ 5 files changed, 230 insertions(+), 6 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml 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 index b0b4d2a50fa7..1a83e1a494fb 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java @@ -880,6 +880,7 @@ private Schema processSimplifyAnyOfStringAndEnumString(Schema schema) { * @param schema Schema */ public boolean isNullTypeSchema(Schema schema) { + LOGGER.info("null? {}", schema); if (schema == null) { return true; } @@ -890,7 +891,7 @@ public boolean isNullTypeSchema(Schema schema) { if (schema.getTypes() != null && !schema.getTypes().isEmpty()) { // 3.1 spec - if (schema.getTypes().size() ==1) { // 1 type only + 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 @@ -902,6 +903,11 @@ public boolean isNullTypeSchema(Schema schema) { 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; @@ -938,7 +944,7 @@ private Schema processSimplifyOneOf(Schema schema) { if (oneOfSchemas.size() == 6) { TreeSet ts = new TreeSet<>(); for (Schema s: oneOfSchemas) { - ts.add(s.getType()); + ts.add(ModelUtils.getType(s)); } if (ts.equals(anyTypeTreeSet)) { @@ -1063,7 +1069,7 @@ private Schema processSimplifyAnyOf(Schema schema) { if (anyOfSchemas.size() == 6) { TreeSet ts = new TreeSet<>(); for (Schema s: anyOfSchemas) { - ts.add(s.getType()); + ts.add(ModelUtils.getType(s)); } if (ts.equals(anyTypeTreeSet)) { 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 index 703d6490b15d..bbaf74bf4245 100644 --- 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 @@ -2126,6 +2126,25 @@ public static boolean hasAnyOf(Schema schema) { return false; } + /** + * Returns schema type. + * For 3.1 spec, return the first one. + * + * @param schema the schema + * @return schema type + */ + public static String getType(Schema schema) { + if (schema == null) { + return null; + } + + if (schema instanceof JsonSchema) { + return String.valueOf(schema.getTypes().iterator().next()); + } else { + return schema.getType(); + } + } + /** * Returns true if any of the common attributes of the schema (e.g. readOnly, default, maximum, etc) is defined. * 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 index a57d7dfd8814..5dc0b8f3cf51 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java @@ -158,6 +158,10 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf() { assertEquals(schema2.getOneOf().size(), 4); assertNull(schema2.getNullable()); + Schema schema2b = openAPI.getComponents().getSchemas().get("OneOfTest2"); + assertEquals(schema2b.getOneOf().size(), 2); + assertNull(schema2b.getNullable()); + Schema schema5 = openAPI.getComponents().getSchemas().get("OneOfNullableTest"); assertEquals(schema5.getOneOf().size(), 3); assertNull(schema5.getNullable()); @@ -189,6 +193,11 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf() { assertTrue(schema4 instanceof IntegerSchema); assertTrue(schema4.getNullable()); + Schema schema4b = openAPI.getComponents().getSchemas().get("OneOfTest2"); + assertNull(schema4b.getOneOf()); + assertTrue(schema4b instanceof StringSchema); + assertTrue(schema4b.getNullable()); + Schema schema6 = openAPI.getComponents().getSchemas().get("OneOfNullableTest"); assertEquals(schema6.getOneOf().size(), 2); assertTrue(schema6.getNullable()); @@ -532,7 +541,7 @@ public void testSetContainerToNullable() { @Test public void testSetPrimitiveTypesToNullable() { // test `string|integer|number|boolean` - OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0//setPrimitiveTypesToNullable_test.yaml"); + OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/setPrimitiveTypesToNullable_test.yaml"); Schema schema = openAPI.getComponents().getSchemas().get("Person"); assertEquals(((Schema) schema.getProperties().get("lastName")).getNullable(), null); @@ -552,7 +561,7 @@ public void testSetPrimitiveTypesToNullable() { assertEquals(((Schema) schema2.getProperties().get("first_boolean")).getNullable(), true); // test `number` only - OpenAPI openAPI2 = TestUtils.parseSpec("src/test/resources/3_0//setPrimitiveTypesToNullable_test.yaml"); + OpenAPI openAPI2 = TestUtils.parseSpec("src/test/resources/3_0/setPrimitiveTypesToNullable_test.yaml"); Schema schema3 = openAPI2.getComponents().getSchemas().get("Person"); assertEquals(((Schema) schema3.getProperties().get("lastName")).getNullable(), null); @@ -572,7 +581,7 @@ public void testSetPrimitiveTypesToNullable() { } @Test - public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() { + public void testOpenAPINormalizerSimplifyOneOfAnyOf31SpecForIssue18184 () { // to test the rule SIMPLIFY_ONEOF_ANYOF in 3.1 spec OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/issue_18184.yaml"); // test spec contains anyOf with a ref to enum and another scheme type is null @@ -636,6 +645,77 @@ public void testOpenAPINormalizerProcessingArraySchema31Spec() { assertEquals(((Schema) schema6.getProperties().get("arrayOfStrings")).getItems().getTypes().contains("string"), true); assertEquals(((Schema) schema6.getProperties().get("arrayOfStrings")).getItems().getType(), "string"); assertEquals(((Schema) schema6.getProperties().get("arrayOfStrings")).getType(), "array"); + } + + @Test + public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() { + // to test the rule SIMPLIFY_ONEOF_ANYOF with 3.1 spec + OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml"); + + Schema schema = openAPI.getComponents().getSchemas().get("AnyOfTest"); + assertEquals(schema.getAnyOf().size(), 4); + assertNull(schema.getNullable()); + + Schema schema2 = openAPI.getComponents().getSchemas().get("OneOfTest"); + assertEquals(schema2.getOneOf().size(), 4); + assertNull(schema2.getNullable()); + + Schema schema2b = openAPI.getComponents().getSchemas().get("OneOfTest2"); + assertEquals(schema2b.getOneOf().size(), 2); + assertNull(schema2b.getNullable()); + + Schema schema5 = openAPI.getComponents().getSchemas().get("OneOfNullableTest"); + assertEquals(schema5.getOneOf().size(), 3); + assertNull(schema5.getNullable()); + + Schema schema7 = openAPI.getComponents().getSchemas().get("Parent"); + assertEquals(((Schema) schema7.getProperties().get("number")).getAnyOf().size(), 1); + + Schema schema9 = openAPI.getComponents().getSchemas().get("AnyOfStringArrayOfString"); + assertEquals(schema9.getAnyOf().size(), 2); + + Schema schema11 = openAPI.getComponents().getSchemas().get("AnyOfAnyType"); + assertEquals(schema11.getAnyOf().size(), 6); + + Schema schema13 = openAPI.getComponents().getSchemas().get("OneOfAnyType"); + assertEquals(schema13.getOneOf().size(), 6); + Map options = new HashMap<>(); + options.put("SIMPLIFY_ONEOF_ANYOF", "true"); + OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options); + openAPINormalizer.normalize(); + + Schema schema3 = openAPI.getComponents().getSchemas().get("AnyOfTest"); + assertNull(schema3.getAnyOf()); + assertEquals(ModelUtils.getType(schema3), "string"); + assertTrue(schema3.getNullable()); + + Schema schema4 = openAPI.getComponents().getSchemas().get("OneOfTest"); + assertNull(schema4.getOneOf()); + assertEquals(ModelUtils.getType(schema4), "integer"); + assertTrue(schema4.getNullable()); + + Schema schema4b = openAPI.getComponents().getSchemas().get("OneOfTest2"); + assertNull(schema4b.getOneOf()); + assertEquals(ModelUtils.getType(schema4b), "string"); + assertTrue(schema4b.getNullable()); + + Schema schema6 = openAPI.getComponents().getSchemas().get("OneOfNullableTest"); + assertEquals(schema6.getOneOf().size(), 2); + assertTrue(schema6.getNullable()); + + Schema schema8 = openAPI.getComponents().getSchemas().get("Parent"); + assertEquals(((Schema) schema8.getProperties().get("number")).get$ref(), "#/components/schemas/Number"); + + Schema schema10 = openAPI.getComponents().getSchemas().get("AnyOfStringArrayOfString"); + assertEquals(schema10.getAnyOf().size(), 2); + + Schema schema12 = openAPI.getComponents().getSchemas().get("AnyOfAnyType"); + assertEquals(schema12.getAnyOf(), null); + assertEquals(schema12.getType(), null); + + Schema schema14 = openAPI.getComponents().getSchemas().get("OneOfAnyType"); + assertEquals(schema14.getOneOf(), null); + assertEquals(schema14.getType(), null); } } diff --git a/modules/openapi-generator/src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml b/modules/openapi-generator/src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml index 6487740be512..8f829a884d14 100644 --- a/modules/openapi-generator/src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml @@ -40,6 +40,11 @@ components: - type: 'null' - type: null - $ref: null + OneOfTest2: + description: to test oneOf + oneOf: + - type: string + - type: 'null' OneOfNullableTest: description: to test oneOf nullable oneOf: diff --git a/modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml b/modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml new file mode 100644 index 000000000000..a3463b4b12c8 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml @@ -0,0 +1,114 @@ +openapi: 3.1.0 +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 + SingleAnyOfTest: + description: to test anyOf (enum string only) + anyOf: + - type: string + enum: + - A + - B + Parent: + type: object + properties: + number: + anyOf: + - $ref: '#/components/schemas/Number' + ParentWithOneOfProperty: + type: object + properties: + number: + oneOf: + - $ref: '#/components/schemas/Number' + ParentWithPluralOneOfProperty: + type: object + properties: + number: + oneOf: + - $ref: '#/components/schemas/Number' + - $ref: '#/components/schemas/Number2' + Number: + enum: + - one + - two + - three + type: string + Number2: + enum: + - one + - two + type: string + AnyOfStringArrayOfString: + anyOf: + - type: string + - type: array + items: + type: string + AnyOfAnyType: + anyOf: + - type: boolean + - type: array + items: {} + - type: object + - type: string + - type: number + - type: integer + OneOfAnyType: + oneOf: + - type: object + - type: boolean + - type: number + - type: string + - type: integer + - type: array + items: {} From 6b6c10c5479342b25fa30d24f65e2900ea1f8857 Mon Sep 17 00:00:00 2001 From: William Cheng Date: Thu, 11 Apr 2024 16:51:03 +0800 Subject: [PATCH 2/2] clean up --- .../main/java/org/openapitools/codegen/OpenAPINormalizer.java | 1 - 1 file changed, 1 deletion(-) 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 index 1a83e1a494fb..4f129b2f4c68 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java @@ -880,7 +880,6 @@ private Schema processSimplifyAnyOfStringAndEnumString(Schema schema) { * @param schema Schema */ public boolean isNullTypeSchema(Schema schema) { - LOGGER.info("null? {}", schema); if (schema == null) { return true; }