Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes so that a oneOf schema with a single sub-schema is simplified #21043

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import static org.openapitools.codegen.utils.ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema;
import static org.openapitools.codegen.utils.StringUtils.getUniqueString;

public class OpenAPINormalizer {
Expand Down Expand Up @@ -1213,17 +1214,7 @@ private Schema processSimplifyOneOf(Schema schema) {
}
}

if (oneOfSchemas.removeIf(oneOf -> ModelUtils.isNullTypeSchema(openAPI, oneOf))) {
schema.setNullable(true);

// if only one element left, simplify to just the element (schema)
if (oneOfSchemas.size() == 1) {
if (Boolean.TRUE.equals(schema.getNullable())) { // retain nullable setting
((Schema) oneOfSchemas.get(0)).setNullable(true);
}
return (Schema) oneOfSchemas.get(0);
}
}
schema = simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, schema, oneOfSchemas);

if (ModelUtils.isIntegerSchema(schema) || ModelUtils.isNumberSchema(schema) || ModelUtils.isStringSchema(schema)) {
// TODO convert oneOf const to enum
Expand Down Expand Up @@ -1350,17 +1341,7 @@ private Schema processSimplifyAnyOf(Schema schema) {
}
}

if (anyOfSchemas.removeIf(anyOf -> ModelUtils.isNullTypeSchema(openAPI, anyOf))) {
schema.setNullable(true);
}

// if only one element left, simplify to just the element (schema)
if (anyOfSchemas.size() == 1) {
if (Boolean.TRUE.equals(schema.getNullable())) { // retain nullable setting
((Schema) anyOfSchemas.get(0)).setNullable(true);
}
return (Schema) anyOfSchemas.get(0);
}
schema = simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, schema, anyOfSchemas);
}

return schema;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2219,6 +2219,30 @@ public static Schema cloneSchema(Schema schema, boolean openapi31) {
}
}

/**
* Simplifies the schema by removing the oneOfAnyOf if the oneOfAnyOf only contains a single non-null sub-schema
*
* @param openAPI OpenAPI
* @param schema Schema
* @param subSchemas The oneOf or AnyOf schemas
* @return The simplified schema
*/
public static Schema simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(OpenAPI openAPI, Schema schema, List<Schema> subSchemas) {
if (subSchemas.removeIf(subSchema -> isNullTypeSchema(openAPI, subSchema))) {
schema.setNullable(true);
}

// if only one element left, simplify to just the element (schema)
if (subSchemas.size() == 1) {
if (Boolean.TRUE.equals(schema.getNullable())) { // retain nullable setting
subSchemas.get(0).setNullable(true);
}
return subSchemas.get(0);
}

return schema;
}

/**
* Check if the schema is of type 'null' or schema itself is pointing to null
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
import org.openapitools.codegen.utils.ModelUtils;
import org.testng.annotations.Test;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.*;

import static org.testng.Assert.*;

Expand Down Expand Up @@ -171,6 +168,12 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf() {
Schema schema15 = openAPI.getComponents().getSchemas().get("AnyOfAnyTypeWithRef");
assertEquals(schema15.getAnyOf().size(), 6);

Schema schema17 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) schema17.getProperties().get("number")).getOneOf().size(), 1);

Schema schema19 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
assertEquals(schema19.getAnyOf().size(), 1);

Map<String, String> options = new HashMap<>();
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
Expand Down Expand Up @@ -212,6 +215,30 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf() {
Schema schema16 = openAPI.getComponents().getSchemas().get("AnyOfAnyTypeWithRef");
assertEquals(schema16.getAnyOf(), null);
assertEquals(schema16.getType(), null);

Schema schema18 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) schema18.getProperties().get("number")).get$ref(), "#/components/schemas/Number");

Schema schema20 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
assertEquals(schema20.getAnyOf(), null);
assertEquals(schema20.getType(), "string");
assertEquals(schema20.getEnum().size(), 2);
}

@Test
public void testOpenAPINormalizerSimplifyOneOfWithSingleRef() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml");

Schema oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).getOneOf().size(), 1);

Map<String, String> options = new HashMap<>();
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
openAPINormalizer.normalize();

oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).get$ref(), "#/components/schemas/Number");
}

@Test
Expand Down Expand Up @@ -833,6 +860,12 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() {
Schema schema17 = openAPI.getComponents().getSchemas().get("OneOfNullAndRef3");
assertEquals(schema17.getOneOf().size(), 2);

Schema schema19 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) schema19.getProperties().get("number")).getOneOf().size(), 1);

Schema schema21 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
assertEquals(schema21.getAnyOf().size(), 1);

Map<String, String> options = new HashMap<>();
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
Expand Down Expand Up @@ -879,6 +912,30 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() {
// original oneOf removed and simplified to just $ref (oneOf sub-schema) instead
assertEquals(schema18.getOneOf(), null);
assertEquals(schema18.get$ref(), "#/components/schemas/Parent");

Schema schema20 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) schema20.getProperties().get("number")).get$ref(), "#/components/schemas/Number");

Schema schema22 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
assertEquals(schema22.getAnyOf(), null);
assertEquals(schema22.getTypes(), Set.of("string"));
assertEquals(schema22.getEnum().size(), 2);
}

@Test
public void testOpenAPINormalizerSimplifyOneOfWithSingleRef31Spec() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml");

Schema oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).getOneOf().size(), 1);

Map<String, String> options = new HashMap<>();
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
openAPINormalizer.normalize();

oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).get$ref(), "#/components/schemas/Number");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2272,7 +2272,7 @@ public static Object[][] sealedScenarios() {
"SchemaA.java", "public final class SchemaA extends RepresentationModel<SchemaA> implements PostRequest {",
"PostRequest.java", "public sealed interface PostRequest permits SchemaA {")},
{"oneOf_array.yaml", Map.of(
"MyExampleGet200Response.java", "public interface MyExampleGet200Response")},
"MyExampleGet200Response.java", "public sealed interface MyExampleGet200Response")},
{"oneOf_duplicateArray.yaml", Map.of(
"Example.java", "public interface Example {")},
{"oneOf_nonPrimitive.yaml", Map.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
import java.util.List;
import java.util.Map;

import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.*;

public class ModelUtilsTest {

Expand Down Expand Up @@ -476,6 +475,54 @@ public void testGetSchemaItemsWith31Spec() {
Assert.assertNotNull(ModelUtils.getSchemaItems((Schema) arrayWithPrefixItems.getProperties().get("without_items")));
}

@Test
public void simplyOneOfAnyOfWithOnlyOneNonNullSubSchema() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml");
Schema schema;
List<Schema> subSchemas;

Schema anyOfWithSeveralSubSchemasButSingleNonNull = ModelUtils.getSchema(openAPI, "AnyOfTest");
subSchemas = anyOfWithSeveralSubSchemasButSingleNonNull.getAnyOf();
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, anyOfWithSeveralSubSchemasButSingleNonNull, subSchemas);
assertNull(schema.getOneOf());
assertNull(schema.getAnyOf());
assertTrue(schema.getNullable());
assertEquals("string", schema.getType());

Schema anyOfWithSingleNonNullSubSchema = ModelUtils.getSchema(openAPI, "Parent");
subSchemas = ((Schema) anyOfWithSingleNonNullSubSchema.getProperties().get("number")).getAnyOf();
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, anyOfWithSingleNonNullSubSchema, subSchemas);
assertNull(schema.getOneOf());
assertNull(schema.getAnyOf());
assertNull(schema.getNullable());
assertEquals(schema.get$ref(), "#/components/schemas/Number");

Schema oneOfWithSeveralSubSchemasButSingleNonNull = ModelUtils.getSchema(openAPI, "OneOfTest");
subSchemas = oneOfWithSeveralSubSchemasButSingleNonNull.getOneOf();
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, oneOfWithSeveralSubSchemasButSingleNonNull, subSchemas);
assertNull(schema.getOneOf());
assertNull(schema.getAnyOf());
assertTrue(schema.getNullable());
assertEquals("integer", schema.getType());

Schema oneOfWithSingleNonNullSubSchema = ModelUtils.getSchema(openAPI, "ParentWithOneOfProperty");
subSchemas = ((Schema) oneOfWithSingleNonNullSubSchema.getProperties().get("number")).getOneOf();
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, oneOfWithSingleNonNullSubSchema, subSchemas);
assertNull(schema.getOneOf());
assertNull(schema.getAnyOf());
assertNull(schema.getNullable());
assertEquals(schema.get$ref(), "#/components/schemas/Number");

Schema oneOfWithSeveralSubSchemas = ModelUtils.getSchema(openAPI, "ParentWithPluralOneOfProperty");
subSchemas = ((Schema) oneOfWithSeveralSubSchemas.getProperties().get("number")).getOneOf();
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, oneOfWithSeveralSubSchemas, subSchemas);
assertNull(schema.getOneOf());
assertNotNull(oneOfWithSeveralSubSchemas.getProperties().get("number"));
assertNull(schema.getAnyOf());
assertNull(schema.getNullable());
assertEquals(((Schema) oneOfWithSeveralSubSchemas.getProperties().get("number")).getOneOf().size(), 2);
}

@Test
public void isNullTypeSchemaTest() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/null_schema_test.yaml");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ paths:
- type: array
items:
"$ref": "#/components/schemas/OneOf1"
- type: object
"$ref": "#/components/schemas/OneOf1"
components:
schemas:
OneOf1:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ components:
- items:
$ref: '#/components/schemas/OneOf1'
type: array
- $ref: '#/components/schemas/OneOf1'

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

| Name | Type | Description | Notes |
|------------ | ------------- | ------------- | -------------|
|**message1** | **String** | | [optional] |



Loading
Loading