[core] consider polymorphism when computing unused schemas (#4335)

Fixed #4193
This commit is contained in:
Jérémie Bresson 2019-11-03 11:05:40 +01:00 committed by William Cheng
parent 68dbf20d7e
commit 10627ed5cc
3 changed files with 102 additions and 7 deletions

View File

@ -90,6 +90,7 @@ public class ModelUtils {
* @return schemas a list of used schemas * @return schemas a list of used schemas
*/ */
public static List<String> getAllUsedSchemas(OpenAPI openAPI) { public static List<String> getAllUsedSchemas(OpenAPI openAPI) {
Map<String, List<String>> childrenMap = getChildrenMap(openAPI);
List<String> allUsedSchemas = new ArrayList<String>(); List<String> allUsedSchemas = new ArrayList<String>();
visitOpenAPI(openAPI, (s, t) -> { visitOpenAPI(openAPI, (s, t) -> {
if (s.get$ref() != null) { if (s.get$ref() != null) {
@ -97,6 +98,13 @@ public class ModelUtils {
if (!allUsedSchemas.contains(ref)) { if (!allUsedSchemas.contains(ref)) {
allUsedSchemas.add(ref); allUsedSchemas.add(ref);
} }
if (childrenMap.containsKey(ref)) {
for (String child : childrenMap.get(ref)) {
if (!allUsedSchemas.contains(child)) {
allUsedSchemas.add(child);
}
}
}
} }
}); });
return allUsedSchemas; return allUsedSchemas;
@ -109,6 +117,7 @@ public class ModelUtils {
* @return schemas a list of unused schemas * @return schemas a list of unused schemas
*/ */
public static List<String> getUnusedSchemas(OpenAPI openAPI) { public static List<String> getUnusedSchemas(OpenAPI openAPI) {
Map<String, List<String>> childrenMap = getChildrenMap(openAPI);
List<String> unusedSchemas = new ArrayList<String>(); List<String> unusedSchemas = new ArrayList<String>();
Map<String, Schema> schemas = getSchemas(openAPI); Map<String, Schema> schemas = getSchemas(openAPI);
@ -116,7 +125,11 @@ public class ModelUtils {
visitOpenAPI(openAPI, (s, t) -> { visitOpenAPI(openAPI, (s, t) -> {
if (s.get$ref() != null) { if (s.get$ref() != null) {
unusedSchemas.remove(getSimpleRef(s.get$ref())); String ref = getSimpleRef(s.get$ref());
unusedSchemas.remove(ref);
if (childrenMap.containsKey(ref)) {
unusedSchemas.removeAll(childrenMap.get(ref));
}
} }
}); });
return unusedSchemas; return unusedSchemas;
@ -859,6 +872,18 @@ public class ModelUtils {
return null; return null;
} }
public static Map<String, List<String>> getChildrenMap(OpenAPI openAPI) {
Map<String, Schema> allSchemas = getSchemas(openAPI);
Map<String, List<Entry<String, Schema>>> groupedByParent = allSchemas.entrySet().stream()
.filter(entry -> isComposedSchema(entry.getValue()))
.collect(Collectors.groupingBy(entry -> getParentName((ComposedSchema) entry.getValue(), allSchemas)));
return groupedByParent.entrySet().stream()
.collect(Collectors.toMap(entry -> entry.getKey(), entry -> entry.getValue().stream().map(e -> e.getKey()).collect(Collectors.toList())));
}
/** /**
* Get the interfaces from the schema (composed) * Get the interfaces from the schema (composed)
* *

View File

@ -37,7 +37,7 @@ public class ModelUtilsTest {
public void testGetAllUsedSchemas() { public void testGetAllUsedSchemas() {
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/unusedSchemas.yaml"); final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/unusedSchemas.yaml");
List<String> allUsedSchemas = ModelUtils.getAllUsedSchemas(openAPI); List<String> allUsedSchemas = ModelUtils.getAllUsedSchemas(openAPI);
Assert.assertEquals(allUsedSchemas.size(), 38); Assert.assertEquals(allUsedSchemas.size(), 41);
Assert.assertTrue(allUsedSchemas.contains("SomeObjShared"), "contains 'SomeObjShared'"); Assert.assertTrue(allUsedSchemas.contains("SomeObjShared"), "contains 'SomeObjShared'");
Assert.assertTrue(allUsedSchemas.contains("SomeObj1"), "contains 'UnusedObj1'"); Assert.assertTrue(allUsedSchemas.contains("SomeObj1"), "contains 'UnusedObj1'");
@ -77,21 +77,30 @@ public class ModelUtilsTest {
Assert.assertTrue(allUsedSchemas.contains("SomeObj26"), "contains 'SomeObj26'"); Assert.assertTrue(allUsedSchemas.contains("SomeObj26"), "contains 'SomeObj26'");
Assert.assertTrue(allUsedSchemas.contains("Param27"), "contains 'Param27'"); Assert.assertTrue(allUsedSchemas.contains("Param27"), "contains 'Param27'");
Assert.assertTrue(allUsedSchemas.contains("Param28"), "contains 'Param28'"); Assert.assertTrue(allUsedSchemas.contains("Param28"), "contains 'Param28'");
Assert.assertTrue(allUsedSchemas.contains("Parent30"), "contains 'Parent30'");
Assert.assertTrue(allUsedSchemas.contains("AChild30"), "contains 'AChild30'");
Assert.assertTrue(allUsedSchemas.contains("BChild30"), "contains 'BChild30'");
} }
@Test @Test
public void testGetUnusedSchemas() { public void testGetUnusedSchemas() {
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/unusedSchemas.yaml"); final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/unusedSchemas.yaml");
List<String> unusedSchemas = ModelUtils.getUnusedSchemas(openAPI); List<String> unusedSchemas = ModelUtils.getUnusedSchemas(openAPI);
Assert.assertEquals(unusedSchemas.size(), 4); Assert.assertEquals(unusedSchemas.size(), 7);
//UnusedObj is not used at all: //UnusedObj1 is not used at all:
Assert.assertTrue(unusedSchemas.contains("UnusedObj1"), "contains 'UnusedObj1'"); Assert.assertTrue(unusedSchemas.contains("UnusedObj1"), "contains 'UnusedObj1'");
//SomeObjUnused is used in a request body that is not used. //UnusedObj2 is used in a request body that is not used.
Assert.assertTrue(unusedSchemas.contains("UnusedObj2"), "contains 'UnusedObj2'"); Assert.assertTrue(unusedSchemas.contains("UnusedObj2"), "contains 'UnusedObj2'");
//SomeObjUnused is used in a response that is not used. //UnusedObj3 is used in a response that is not used.
Assert.assertTrue(unusedSchemas.contains("UnusedObj3"), "contains 'UnusedObj3'"); Assert.assertTrue(unusedSchemas.contains("UnusedObj3"), "contains 'UnusedObj3'");
//SomeObjUnused is used in a parameter that is not used. //UnusedObj4 is used in a parameter that is not used.
Assert.assertTrue(unusedSchemas.contains("UnusedObj4"), "contains 'UnusedObj4'"); Assert.assertTrue(unusedSchemas.contains("UnusedObj4"), "contains 'UnusedObj4'");
//Parent29 is not used at all (only unused children AChild29 and BChild29 are referencing him):
Assert.assertTrue(unusedSchemas.contains("Parent29"), "contains 'Parent29'");
//AChild29 is not used at all:
Assert.assertTrue(unusedSchemas.contains("AChild29"), "contains 'AChild29'");
//BChild29 is not used at all:
Assert.assertTrue(unusedSchemas.contains("BChild29"), "contains 'BChild29'");
} }
@Test @Test

View File

@ -352,6 +352,17 @@ paths:
responses: responses:
'200': '200':
description: OK description: OK
/some/p30:
post:
operationId: op27
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/Parent30"
responses:
'200':
description: OK
components: components:
schemas: schemas:
UnusedObj1: UnusedObj1:
@ -641,6 +652,56 @@ components:
type: string type: string
r2: r2:
type: string type: string
Parent29:
type: object
required:
- childType
properties:
childType:
type: string
discriminator:
propertyName: childType
AChild29:
allOf:
- $ref: '#/components/schemas/Parent29'
- type: object
properties:
name:
type: string
BChild29:
allOf:
- $ref: '#/components/schemas/Parent29'
- type: object
properties:
firstName:
type: string
lastName:
type: string
Parent30:
type: object
required:
- childType
properties:
childType:
type: string
discriminator:
propertyName: childType
AChild30:
allOf:
- $ref: '#/components/schemas/Parent30'
- type: object
properties:
name:
type: string
BChild30:
allOf:
- $ref: '#/components/schemas/Parent30'
- type: object
properties:
firstName:
type: string
lastName:
type: string
SomeObjShared: SomeObjShared:
type: object type: object
properties: properties: