Skip to content

Commit

Permalink
Better handling of allOf with unsupported schemas (#19964)
Browse files Browse the repository at this point in the history
* better handling of allOf with unsupported schemas

* add test spec

* better messages
  • Loading branch information
wing328 authored Oct 28, 2024
1 parent 30ff0d7 commit 62c0258
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -665,21 +665,62 @@ private void normalizeProperties(Map<String, Schema> properties, Set<Schema> vis
}
}

/*
* Remove unsupported schemas (e.g. if, then) from allOf.
*
* @param schema Schema
*/
private void removeUnsupportedSchemasFromAllOf(Schema schema) {
if (schema.getAllOf() == null) {
return;
}

Iterator<Schema> iterator = schema.getAllOf().iterator();
while (iterator.hasNext()) {
Schema item = iterator.next();

// remove unsupported schemas (e.g. if, then)
if (ModelUtils.isUnsupportedSchema(openAPI, item)) {
LOGGER.debug("Removed allOf sub-schema that's not yet supported: {}", item);
iterator.remove();
}
}

if (schema.getAllOf().size() == 0) {
// no more schema in allOf so reset to null instead
LOGGER.info("Unset/Removed allOf after cleaning up allOf sub-schemas that are not yet supported.");
schema.setAllOf(null);
}
}

private Schema normalizeAllOf(Schema schema, Set<Schema> visitedSchemas) {
removeUnsupportedSchemasFromAllOf(schema);

if (schema.getAllOf() == null) {
return schema;
}

for (Object item : schema.getAllOf()) {
if (!(item instanceof Schema)) {
throw new RuntimeException("Error! allOf schema is not of the type Schema: " + item);
}
// normalize allOf sub schemas one by one
normalizeSchema((Schema) item, visitedSchemas);
}

// process rules here
processUseAllOfRefAsParent(schema);

return schema;
}

private Schema normalizeAllOfWithProperties(Schema schema, Set<Schema> visitedSchemas) {
removeUnsupportedSchemasFromAllOf(schema);

if (schema.getAllOf() == null) {
return schema;
}

for (Object item : schema.getAllOf()) {
if (!(item instanceof Schema)) {
throw new RuntimeException("Error! allOf schema is not of the type Schema: " + item);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,16 @@ public static boolean isSet(Schema schema) {
return ModelUtils.isArraySchema(schema) && Boolean.TRUE.equals(schema.getUniqueItems());
}

/**
* Return true if the schema is a string/integer/number/boolean type in OpenAPI.
*
* @param schema the OAS schema
* @return true if the schema is a string/integer/number/boolean type in OpenAPI.
*/
public static boolean isPrimitiveType(Schema schema) {
return (isStringSchema(schema) || isIntegerSchema(schema) || isNumberSchema(schema) || isBooleanSchema(schema));
}

public static boolean isStringSchema(Schema schema) {
return schema instanceof StringSchema || SchemaTypeUtil.STRING_TYPE.equals(getType(schema));
}
Expand Down Expand Up @@ -2262,6 +2272,35 @@ public static boolean isNullTypeSchema(OpenAPI openAPI, Schema schema) {
return false;
}

/**
* Check if the schema is supported by OpenAPI Generator.
* <p>
* Return true if the schema can be handled by OpenAPI Generator
*
* @param schema Schema
* @param openAPI OpenAPIs
*
* @return true if schema is null type
*/
public static boolean isUnsupportedSchema(OpenAPI openAPI, Schema schema) {
if (schema == null) {
return true;
}

// dereference the schema
schema = ModelUtils.getReferencedSchema(openAPI, schema);

if (schema.getTypes() == null && hasValidation(schema)) {
// just validation without type
return true;
} else if (schema.getIf() != null && schema.getThen() != null) {
// if, then in 3.1 spec
return true;
}

return false;
}

@FunctionalInterface
private interface OpenAPISchemaVisitor {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,4 +774,24 @@ public void testOpenAPINormalizerSingleConstEnum31Spec() {
assertEquals(normalizedTypeSchema.getEnum().size(), 1);
assertEquals(Arrays.asList(originalConst), normalizedTypeSchema.getEnum());
}

@Test
public void testOpenAPINormalizerProcessingAllOfSchema31Spec() {
// to test array schema processing in 3.1 spec
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/unsupported_schema_test.yaml");

Schema schema = openAPI.getComponents().getSchemas().get("Dummy");
assertEquals(((Schema) schema.getProperties().get("property1")).getAllOf().size(), 2);
assertNotEquals(((Schema) ((Schema) schema.getProperties().get("property2")).getAllOf().get(0)).getIf(), null); // if is set before normalization
assertNotEquals(((Schema) ((Schema) schema.getProperties().get("property2")).getAllOf().get(1)).getThen(), null); // then is set before normalization

Map<String, String> inputRules = Map.of("NORMALIZE_31SPEC", "true");
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, inputRules);
openAPINormalizer.normalize();

Schema schema2 = openAPI.getComponents().getSchemas().get("Dummy");
assertEquals(((Schema) schema2.getProperties().get("property1")).getAllOf(), null);
assertEquals(((Schema) schema2.getProperties().get("property2")).getAllOf(), null);
assertEquals(((Schema) schema2.getProperties().get("property2")).getAllOf(), null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -501,4 +501,26 @@ public void isNullTypeSchemaTest() {
assertTrue(ModelUtils.isNullTypeSchema(openAPI, (Schema) schema.getAnyOf().get(2)));
assertTrue(ModelUtils.isNullTypeSchema(openAPI, (Schema) schema.getAnyOf().get(3)));
}

@Test
public void isUnsupportedSchemaTest() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/unsupported_schema_test.yaml");
Map<String, String> options = new HashMap<>();
Schema schema = openAPI.getComponents().getSchemas().get("Dummy");
Schema property1 = (Schema) schema.getProperties().get("property1");
Schema property2 = (Schema) schema.getProperties().get("property2");

// a test of string type with allOf (2 patterns)
assertTrue(ModelUtils.isUnsupportedSchema(openAPI, (Schema) property1.getAllOf().get(0)));
assertTrue(ModelUtils.isUnsupportedSchema(openAPI, (Schema) property1.getAllOf().get(1)));

// if, then test
assertTrue(ModelUtils.isUnsupportedSchema(openAPI, (Schema) property2.getAllOf().get(0)));
assertTrue(ModelUtils.isUnsupportedSchema(openAPI, (Schema) property2.getAllOf().get(1)));

// typical schemas, e.g. boolean, string, array of string (enum)
assertFalse(ModelUtils.isUnsupportedSchema(openAPI, (Schema) property2.getProperties().get("aBooleanCheck")));
assertFalse(ModelUtils.isUnsupportedSchema(openAPI, (Schema) property2.getProperties().get("condition")));
assertFalse(ModelUtils.isUnsupportedSchema(openAPI, (Schema) property2.getProperties().get("purpose")));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
openapi: 3.1.0
info:
version: 1.0.0
title: Example
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/Dummy"
components:
schemas:
Dummy:
type: object
properties:
property1:
type: string
allOf:
- pattern: "[abc]"
- pattern: "[a-z]"
property2:
type: object
allOf:
- if:
properties:
aBooleanCheck:
const: false
then:
required:
- condition
- if:
properties:
aBooleanCheck:
const: true
then:
required:
- purpose
properties:
aBooleanCheck:
type: boolean
condition:
type: string
purpose:
type: array
items:
type: string
enum:
- FIRST
- SECOND
- THIRD

0 comments on commit 62c0258

Please sign in to comment.