From ef7cb8c7319dd1f94c6789a7bf5f7c55493d0aa5 Mon Sep 17 00:00:00 2001 From: William Cheng Date: Fri, 4 Oct 2024 15:53:13 +0800 Subject: [PATCH 1/3] better handling of primivitype type with oneOf --- .../org/openapitools/codegen/OpenAPINormalizer.java | 11 ++++++++--- .../openapitools/codegen/OpenAPINormalizerTest.java | 7 +++++++ .../test/resources/3_1/simplifyOneOfAnyOf_test.yaml | 13 +++++++++++++ 3 files changed, 28 insertions(+), 3 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 index aa3874d76107..37b0ff4408db 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 @@ -657,7 +657,7 @@ private Schema normalizeOneOf(Schema schema, Set visitedSchemas) { schema.getOneOf().set(i, normalizeSchema((Schema) item, visitedSchemas)); } // process rules here - schema = processSimplifyOneOf(schema); + schema = normalizeSchema(processSimplifyOneOf(schema), visitedSchemas); return schema; } @@ -683,7 +683,7 @@ private Schema normalizeAnyOf(Schema schema, Set visitedSchemas) { schema = processSimplifyAnyOf(schema); // last rule to process as the schema may become String schema (not "anyOf") after the completion - return processSimplifyAnyOfStringAndEnumString(schema); + return normalizeSchema(processSimplifyAnyOfStringAndEnumString(schema), visitedSchemas); } private Schema normalizeComplexComposedSchema(Schema schema, Set visitedSchemas) { @@ -694,7 +694,7 @@ private Schema normalizeComplexComposedSchema(Schema schema, Set visited processRemoveAnyOfOneOfAndKeepPropertiesOnly(schema); - return schema; + return normalizeSchema(schema, visitedSchemas); } // ===================== a list of rules ===================== @@ -997,6 +997,11 @@ private Schema processSimplifyOneOf(Schema schema) { return (Schema) oneOfSchemas.get(0); } } + + if (ModelUtils.isIntegerSchema(schema) || ModelUtils.isNumberSchema(schema) || ModelUtils.isStringSchema(schema)) { + // TODO convert oneOf const to enum + schema.setOneOf(null); + } } return schema; 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 641e935f2b64..2ded5069b4ad 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 @@ -705,6 +705,9 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() { Schema schema13 = openAPI.getComponents().getSchemas().get("OneOfAnyType"); assertEquals(schema13.getOneOf().size(), 6); + Schema schema15 = openAPI.getComponents().getSchemas().get("TypeIntegerWithOneOf"); + assertEquals(schema15.getOneOf().size(), 3); + Map options = new HashMap<>(); options.put("SIMPLIFY_ONEOF_ANYOF", "true"); OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options); @@ -742,5 +745,9 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() { Schema schema14 = openAPI.getComponents().getSchemas().get("OneOfAnyType"); assertEquals(schema14.getOneOf(), null); assertEquals(schema14.getType(), null); + + Schema schema16 = openAPI.getComponents().getSchemas().get("TypeIntegerWithOneOf"); + // oneOf should have been removed as the schema is essentially a primitive type + assertEquals(schema16.getOneOf(), null); } } 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 index a3463b4b12c8..2ba7e95238f7 100644 --- a/modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml +++ b/modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml @@ -112,3 +112,16 @@ components: - type: integer - type: array items: {} + TypeIntegerWithOneOf: + type: integer + oneOf: + - title: ITEM A + description: This permission is for item A. + const: 1 + - title: ITEM B + description: This permission is for item B. + const: 2 + - title: ITEM C + description: This permission is for item C. + const: 4 + format: int32 From 44b227cb5e20f7e5a8b51e17de0a9d5d51e884d7 Mon Sep 17 00:00:00 2001 From: William Cheng Date: Fri, 4 Oct 2024 19:15:07 +0800 Subject: [PATCH 2/3] fix null check, add tests --- .../codegen/OpenAPINormalizer.java | 60 +++++++++++-------- .../codegen/OpenAPINormalizerTest.java | 10 +++- .../3_1/simplifyOneOfAnyOf_test.yaml | 13 ++++ 3 files changed, 58 insertions(+), 25 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 index 37b0ff4408db..886206cb7666 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 @@ -642,22 +642,29 @@ private Schema normalizeAllOfWithProperties(Schema schema, Set visitedSc } private Schema normalizeOneOf(Schema schema, Set visitedSchemas) { - for (int i = 0; i < schema.getOneOf().size(); i++) { - // normalize oneOf sub schemas one by one - Object item = schema.getOneOf().get(i); + // simplify first as the schema may no longer be a oneOf after processing the rule below + schema = processSimplifyOneOf(schema); - if (item == null) { - continue; - } - if (!(item instanceof Schema)) { - throw new RuntimeException("Error! oneOf schema is not of the type Schema: " + item); - } + // if it's still a oneOf, loop through the sub-schemas + if (schema.getOneOf() != null) { + for (int i = 0; i < schema.getOneOf().size(); i++) { + // normalize oneOf sub schemas one by one + Object item = schema.getOneOf().get(i); - // update sub-schema with the updated schema - schema.getOneOf().set(i, normalizeSchema((Schema) item, visitedSchemas)); + if (item == null) { + continue; + } + if (!(item instanceof Schema)) { + throw new RuntimeException("Error! oneOf schema is not of the type Schema: " + item); + } + + // update sub-schema with the updated schema + schema.getOneOf().set(i, normalizeSchema((Schema) item, visitedSchemas)); + } + } else { + // normalize it as it's no longer an oneOf + schema = normalizeSchema(schema, visitedSchemas); } - // process rules here - schema = normalizeSchema(processSimplifyOneOf(schema), visitedSchemas); return schema; } @@ -898,16 +905,29 @@ private Schema processSimplifyAnyOfStringAndEnumString(Schema schema) { * 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(Schema schema) { + public boolean isNullTypeSchema(OpenAPI openAPI, Schema schema) { if (schema == null) { return true; } + // dereference the schema + schema = ModelUtils.getReferencedSchema(openAPI, schema); + if (ModelUtils.hasAllOf(schema) || ModelUtils.hasOneOf(schema) || ModelUtils.hasAnyOf(schema)) { 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 @@ -933,14 +953,6 @@ public boolean isNullTypeSchema(Schema schema) { } } - // convert referenced enum of null only to `nullable:true` - Schema referencedSchema = ModelUtils.getReferencedSchema(openAPI, schema); - if (referencedSchema.getEnum() != null && referencedSchema.getEnum().size() == 1) { - if ("null".equals(String.valueOf(referencedSchema.getEnum().get(0)))) { - return true; - } - } - return false; } @@ -986,7 +998,7 @@ private Schema processSimplifyOneOf(Schema schema) { } } - if (oneOfSchemas.removeIf(oneOf -> isNullTypeSchema(oneOf))) { + if (oneOfSchemas.removeIf(oneOf -> isNullTypeSchema(openAPI, oneOf))) { schema.setNullable(true); // if only one element left, simplify to just the element (schema) @@ -1122,7 +1134,7 @@ private Schema processSimplifyAnyOf(Schema schema) { } } - if (anyOfSchemas.removeIf(anyOf -> isNullTypeSchema(anyOf))) { + if (anyOfSchemas.removeIf(anyOf -> isNullTypeSchema(openAPI, anyOf))) { schema.setNullable(true); } 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 2ded5069b4ad..325784d8d7f4 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 @@ -113,7 +113,7 @@ public void isNullTypeSchemaTest() { Map options = new HashMap<>(); OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options); Schema schema = openAPI.getComponents().getSchemas().get("AnyOfStringArrayOfString"); - assertFalse(openAPINormalizer.isNullTypeSchema(schema)); + assertFalse(openAPINormalizer.isNullTypeSchema(openAPI, schema)); } @Test @@ -708,6 +708,9 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() { Schema schema15 = openAPI.getComponents().getSchemas().get("TypeIntegerWithOneOf"); assertEquals(schema15.getOneOf().size(), 3); + Schema schema17 = openAPI.getComponents().getSchemas().get("OneOfNullAndRef3"); + assertEquals(schema17.getOneOf().size(), 2); + Map options = new HashMap<>(); options.put("SIMPLIFY_ONEOF_ANYOF", "true"); OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options); @@ -749,5 +752,10 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() { Schema schema16 = openAPI.getComponents().getSchemas().get("TypeIntegerWithOneOf"); // oneOf should have been removed as the schema is essentially a primitive type assertEquals(schema16.getOneOf(), null); + + Schema schema18 = openAPI.getComponents().getSchemas().get("OneOfNullAndRef3"); + // original oneOf removed and simplified to just $ref (oneOf sub-schema) instead + assertEquals(schema18.getOneOf(), null); + assertEquals(schema18.get$ref(), "#/components/schemas/Parent"); } } 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 index 2ba7e95238f7..588abf3e84c2 100644 --- a/modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml +++ b/modules/openapi-generator/src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml @@ -125,3 +125,16 @@ components: description: This permission is for item C. const: 4 format: int32 + # need to repeat the issue when it only occurs with the 3rd, 4th, 5th, etc schemas with oneOf(type: null, $ref) + OneOfNullAndRef: + oneOf: + - $ref: '#/components/schemas/Parent' + - type: "null" + OneOfNullAndRef2: + oneOf: + - $ref: '#/components/schemas/Parent' + - type: "null" + OneOfNullAndRef3: + oneOf: + - $ref: '#/components/schemas/Parent' + - type: "null" \ No newline at end of file From 28c2be853110ea7cc37af4d5ba2737c56eec5a10 Mon Sep 17 00:00:00 2001 From: William Cheng Date: Fri, 4 Oct 2024 19:40:07 +0800 Subject: [PATCH 3/3] add check for properties --- .../java/org/openapitools/codegen/OpenAPINormalizer.java | 8 +++++++- 1 file changed, 7 insertions(+), 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 886206cb7666..4afcc589d120 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 @@ -900,7 +900,7 @@ private Schema processSimplifyAnyOfStringAndEnumString(Schema schema) { } /** - * Check if the schema is of type 'null' + * Check if the schema is of type 'null' or schema itself is pointing to null *

* Return true if the schema's type is 'null' or not specified * @@ -917,10 +917,16 @@ public boolean isNullTypeSchema(OpenAPI openAPI, Schema schema) { // 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)))) {