From 992afd51eb8531fb2a9263c658500271c616879d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Bresson?= Date: Sat, 9 Jun 2018 11:56:08 +0200 Subject: [PATCH] Fix ModelUtils.getUnusedSchema() (#253) Fix #252 `ModelUtils.getUnusedSchema()` consider Schemas referenced in other Schemas. Implemented for: * array * object * maps * ComposedSchema - oneOf - anyOf - allOf * not --- .../codegen/utils/ModelUtils.java | 60 ++++- .../codegen/utils/ModelUtilsTest.java | 18 +- .../src/test/resources/3_0/unusedSchemas.yaml | 209 ++++++++++++++++++ 3 files changed, 283 insertions(+), 4 deletions(-) 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 3dcd8736914..ee8a1a8dfa2 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 @@ -159,6 +159,7 @@ public class ModelUtils { */ private static void visitOpenAPI(OpenAPI openAPI, OpenAPISchemaVisitor visitor) { Map paths = openAPI.getPaths(); + List visitedSchemas = new ArrayList<>(); if (paths != null) { for (PathItem path : paths.values()) { @@ -170,7 +171,7 @@ public class ModelUtils { for (Parameter p : operation.getParameters()) { Parameter parameter = getReferencedParameter(openAPI, p); if (parameter.getSchema() != null) { - visitor.visit(parameter.getSchema(), null); + visitSchema(openAPI, parameter.getSchema(), null, visitedSchemas, visitor); } } } @@ -180,7 +181,7 @@ public class ModelUtils { if (requestBody != null && requestBody.getContent() != null) { for (Entry e : requestBody.getContent().entrySet()) { if (e.getValue().getSchema() != null) { - visitor.visit(e.getValue().getSchema(), e.getKey()); + visitSchema(openAPI, e.getValue().getSchema(), e.getKey(), visitedSchemas, visitor); } } } @@ -192,7 +193,7 @@ public class ModelUtils { if (apiResponse != null && apiResponse.getContent() != null) { for (Entry e : apiResponse.getContent().entrySet()) { if (e.getValue().getSchema() != null) { - visitor.visit(e.getValue().getSchema(), e.getKey()); + visitSchema(openAPI, e.getValue().getSchema(), e.getKey(), visitedSchemas, visitor); } } } @@ -204,6 +205,59 @@ public class ModelUtils { } } + private static void visitSchema(OpenAPI openAPI, Schema schema, String mimeType, List visitedSchemas, OpenAPISchemaVisitor visitor) { + visitor.visit(schema, mimeType); + if(schema.get$ref() != null) { + String ref = getSimpleRef(schema.get$ref()); + if(!visitedSchemas.contains(ref)) { + visitedSchemas.add(ref); + Schema referencedSchema = getSchemas(openAPI).get(ref); + if(referencedSchema != null) { + visitSchema(openAPI, referencedSchema, mimeType, visitedSchemas, visitor); + } + } + } + if(schema instanceof ComposedSchema) { + List oneOf = ((ComposedSchema) schema).getOneOf(); + if(oneOf != null) { + for (Schema s : oneOf) { + visitSchema(openAPI, s, mimeType, visitedSchemas, visitor); + } + } + List allOf = ((ComposedSchema) schema).getAllOf(); + if(allOf != null) { + for (Schema s : allOf) { + visitSchema(openAPI, s, mimeType, visitedSchemas, visitor); + } + } + List anyOf = ((ComposedSchema) schema).getAnyOf(); + if(anyOf != null) { + for (Schema s : anyOf) { + visitSchema(openAPI, s, mimeType, visitedSchemas, visitor); + } + } + } else if(schema instanceof ArraySchema) { + Schema itemsSchema = ((ArraySchema) schema).getItems(); + if(itemsSchema != null) { + visitSchema(openAPI, itemsSchema, mimeType, visitedSchemas, visitor); + } + } else if(isMapSchema(schema)) { + Object additionalProperties = schema.getAdditionalProperties(); + if(additionalProperties instanceof Schema) { + visitSchema(openAPI, (Schema) additionalProperties, mimeType, visitedSchemas, visitor); + } + } + if(schema.getNot() != null) { + visitSchema(openAPI, schema.getNot(), mimeType, visitedSchemas, visitor); + } + Map properties = schema.getProperties(); + if(properties != null) { + for (Schema property : properties.values()) { + visitSchema(openAPI, property, mimeType, visitedSchemas, visitor); + } + } + } + @FunctionalInterface private static interface OpenAPISchemaVisitor { 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 index a7f62543e23..9927008ffd9 100644 --- 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 @@ -40,7 +40,7 @@ public class ModelUtilsTest { public void testGetAllUsedSchemas() { final OpenAPI openAPI = new OpenAPIParser().readLocation("src/test/resources/3_0/unusedSchemas.yaml", null, new ParseOptions()).getOpenAPI(); List allUsedSchemas = ModelUtils.getAllUsedSchemas(openAPI); - Assert.assertEquals(allUsedSchemas.size(), 12); + Assert.assertEquals(allUsedSchemas.size(), 28); Assert.assertTrue(allUsedSchemas.contains("SomeObjShared"), "contains 'SomeObjShared'"); Assert.assertTrue(allUsedSchemas.contains("SomeObj1"), "contains 'UnusedObj1'"); @@ -54,6 +54,22 @@ public class ModelUtilsTest { Assert.assertTrue(allUsedSchemas.contains("SomeObj10A"), "contains 'SomeObj10A'"); Assert.assertTrue(allUsedSchemas.contains("SomeObj10B"), "contains 'SomeObj10B'"); Assert.assertTrue(allUsedSchemas.contains("SomeObj11"), "contains 'SomeObj11'"); + Assert.assertTrue(allUsedSchemas.contains("SomeArrayObj12"), "contains 'SomeArrayObj12'"); + Assert.assertTrue(allUsedSchemas.contains("ArrayItem12"), "contains 'ArrayItem12'"); + Assert.assertTrue(allUsedSchemas.contains("SomeArrayObj13"), "contains 'SomeArrayObj13'"); + Assert.assertTrue(allUsedSchemas.contains("ArrayItem13"), "contains 'ArrayItem13'"); + Assert.assertTrue(allUsedSchemas.contains("SomeObj14"), "contains 'SomeObj14'"); + Assert.assertTrue(allUsedSchemas.contains("PropertyObj14"), "contains 'PropertyObj14'"); + Assert.assertTrue(allUsedSchemas.contains("SomeObj15"), "contains 'SomeObj15'"); + Assert.assertTrue(allUsedSchemas.contains("SomeMapObj16"), "contains 'SomeMapObj16'"); + Assert.assertTrue(allUsedSchemas.contains("MapItem16"), "contains 'MapItem16'"); + Assert.assertTrue(allUsedSchemas.contains("SomeObj17"), "contains 'SomeObj17'"); + Assert.assertTrue(allUsedSchemas.contains("SomeObj18"), "contains 'SomeObj18'"); + Assert.assertTrue(allUsedSchemas.contains("Common18"), "contains 'Common18'"); + Assert.assertTrue(allUsedSchemas.contains("Obj19ByAge"), "contains 'Obj19ByAge'"); + Assert.assertTrue(allUsedSchemas.contains("Obj19ByType"), "contains 'Obj19ByType'"); + Assert.assertTrue(allUsedSchemas.contains("SomeObj20"), "contains 'SomeObj20'"); + Assert.assertTrue(allUsedSchemas.contains("OtherObj20"), "contains 'OtherObj20'"); } @Test diff --git a/modules/openapi-generator/src/test/resources/3_0/unusedSchemas.yaml b/modules/openapi-generator/src/test/resources/3_0/unusedSchemas.yaml index 1027aec67f2..a01797944af 100644 --- a/modules/openapi-generator/src/test/resources/3_0/unusedSchemas.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/unusedSchemas.yaml @@ -135,6 +135,104 @@ paths: 200: description: Successful Operation content: {} + /some/p12: + post: + operationId: p12 + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/SomeArrayObj12' + responses: + 200: + description: Successful Operation + content: {} + /some/p13: + get: + operationId: p13 + responses: + 200: + description: Successful Operation + content: + application/json: + schema: + $ref: '#/components/schemas/SomeArrayObj13' + /some/p14: + get: + operationId: p14 + responses: + 200: + description: Successful Operation + content: + application/json: + schema: + $ref: '#/components/schemas/SomeObj14' + /some/p15: + get: + operationId: p15 + responses: + '200': + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/SomeObj15' + /some/p16: + get: + operationId: p16 + responses: + '200': + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/SomeMapObj16' + /some/p17: + get: + operationId: p17 + responses: + '200': + description: OK + content: + text/plain: + schema: + oneOf: + - type: string + - $ref: '#/components/schemas/SomeObj17' + /some/p18: + get: + operationId: p18 + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/SomeObj18' + responses: + 200: + description: Successful Operation + content: {} + /some/p19: + patch: + requestBody: + content: + application/json: + schema: + anyOf: + - $ref: '#/components/schemas/Obj19ByAge' + - $ref: '#/components/schemas/Obj19ByType' + responses: + '200': + description: Updated + /some/p20: + get: + operationId: op20 + responses: + '200': + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/SomeObj20' components: schemas: UnusedObj1: @@ -254,6 +352,117 @@ components: - v1 - v2 default: v1 + SomeArrayObj12: + type: array + items: + $ref: "#/components/schemas/ArrayItem12" + ArrayItem12: + type: object + properties: + prop1: + type: string + prop2: + type: integer + format: int32 + SomeArrayObj13: + type: array + items: + type: array + items: + $ref: "#/components/schemas/ArrayItem13" + ArrayItem13: + type: object + properties: + prop1: + type: string + prop2: + type: integer + format: int64 + SomeObj14: + type: object + properties: + obj14: + $ref: "#/components/schemas/PropertyObj14" + PropertyObj14: + type: object + properties: + prop1: + type: string + SomeObj15: + type: object + properties: + message: + type: string + details: + type: array + items: + $ref: '#/components/schemas/SomeObj15' + SomeMapObj16: + type: array + items: + $ref: "#/components/schemas/MapItem16" + MapItem16: + type: object + properties: + prop1: + type: integer + format: int32 + prop2: + type: integer + format: int64 + SomeObj17: + type: object + properties: + id: + type: String + message: + type: String + SomeObj18: + allOf: + - $ref: '#/components/schemas/Common18' + - type: object + properties: + firstName: + type: String + lastName: + type: String + Common18: + type: object + properties: + id: + type: String + active: + type: boolean + Obj19ByAge: + type: object + properties: + age: + type: integer + firstName: + type: String + lastName: + type: String + required: + - age + Obj19ByType: + type: object + properties: + sometype: + type: string + enum: [A, B, C] + enabled: + type: boolean + required: + - sometype + SomeObj20: + type: object + properties: + other: + not: + $ref: '#/components/schemas/OtherObj20' + OtherObj20: + type: string + enum: [A, B, C] SomeObjShared: type: object properties: