From ccff4820abc329bf0ae31e845738231b040e1388 Mon Sep 17 00:00:00 2001 From: Babis Kaidos Date: Thu, 3 Dec 2020 00:21:26 +0200 Subject: [PATCH] Fix issue #1501: External schema name is correctly constructed based on all existing models --- .../processors/ExternalRefProcessor.java | 50 +++++++++---------- .../v3/parser/test/OpenAPIResolverTest.java | 3 +- .../v3/parser/test/OpenAPIV3ParserTest.java | 4 +- .../test/resources/refs-name-conflict/a.yaml | 11 ++++ .../test/resources/refs-name-conflict/c.yaml | 13 +++++ 5 files changed, 51 insertions(+), 30 deletions(-) create mode 100644 modules/swagger-parser-v3/src/test/resources/refs-name-conflict/c.yaml diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java index 520abaa515..f0e426cfbd 100644 --- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java +++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java @@ -49,6 +49,23 @@ public ExternalRefProcessor(ResolverCache cache, OpenAPI openAPI) { this.openAPI = openAPI; } + private String finalNameRec(Map schemas, String possiblyConflictingDefinitionName, Schema newScema, + int iteration) { + String tryName = + iteration == 0 ? possiblyConflictingDefinitionName : possiblyConflictingDefinitionName + "_" + iteration; + Schema existingModel = schemas.get(tryName); + if (existingModel != null) { + if (existingModel.get$ref() != null) { + // use the new model + existingModel = null; + } else if (!newScema.equals(existingModel)) { + LOGGER.debug("A model for " + existingModel + " already exists"); + return finalNameRec(schemas, possiblyConflictingDefinitionName, newScema, ++iteration); + } + } + return tryName; + } + public String processRefToExternalSchema(String $ref, RefFormat refFormat) { String renamedRef = cache.getRenamedRef($ref); if(renamedRef != null) { @@ -75,34 +92,13 @@ public String processRefToExternalSchema(String $ref, RefFormat refFormat) { } final String possiblyConflictingDefinitionName = computeDefinitionName($ref); - String tryName = null; - Schema existingModel = schemas.get(possiblyConflictingDefinitionName); - - if (existingModel != null) { - LOGGER.warn("A model for " + existingModel + " already exists"); - if(existingModel.get$ref() != null) { - // use the new model - existingModel = null; - }else{ - if (!schema.equals(existingModel)){ - //We add a number at the end of the definition name - int i = 2; - for (String name : schemas.keySet()) { - if (name.equals(possiblyConflictingDefinitionName)) { - tryName = possiblyConflictingDefinitionName + "_" + i; - existingModel = schemas.get(tryName); - i++; - } - } - } - } - } - if (StringUtils.isNotBlank(tryName)){ - newRef = tryName; - }else{ - newRef = possiblyConflictingDefinitionName; - } + newRef = finalNameRec(schemas, possiblyConflictingDefinitionName, schema, 0); cache.putRenamedRef($ref, newRef); + Schema existingModel = schemas.get(newRef); + if(existingModel != null && existingModel.get$ref() != null) { + // use the new model + existingModel = null; + } if(existingModel == null) { // don't overwrite existing model reference diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java index 5987fcdb14..59fc4da0d9 100644 --- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java +++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java @@ -777,7 +777,8 @@ public void testRefNameConflicts() throws Exception { assertEquals("referred", ((Schema)openAPI.getPaths().get("/oldPerson").getPost().getResponses().get("200").getContent().get("*/*").getSchema().getProperties().get("location")).getExample()); assertEquals("referred", ((Schema)openAPI.getPaths().get("/yetAnotherPerson").getPost().getResponses().get("200").getContent().get("*/*").getSchema().getProperties().get("location")).getExample()); assertEquals("local", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj").getProperties().get("location")).getExample()); - assertEquals("referred", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj_2").getProperties().get("location")).getExample()); + assertEquals("referred", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj_1").getProperties().get("location")).getExample()); + assertEquals("referred-again", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj_2").getProperties().get("location")).getExample()); } diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java index a9ee2ad15e..4568b04b38 100644 --- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java +++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java @@ -1082,7 +1082,7 @@ public void testOneOfExternalRefConflictName() throws Exception { Schema pet = openAPI.getComponents().getSchemas().get("Pet"); Assert.assertNotNull(pet); Assert.assertTrue(pet.getDiscriminator().getMapping().containsKey("Cat")); - Assert.assertTrue(pet.getDiscriminator().getMapping().get("Cat").equals("#/components/schemas/Cat_2")); + Assert.assertTrue(pet.getDiscriminator().getMapping().get("Cat").equals("#/components/schemas/Cat_1")); } @Test @@ -1687,7 +1687,7 @@ private OpenAPI doRelativeFileTest(String location) { assertEquals(composedCat.getAllOf().size(), 3); assertEquals(composedCat.getAllOf().get(0).get$ref(), "#/components/schemas/pet"); - assertEquals(composedCat.getAllOf().get(1).get$ref(), "#/components/schemas/foo_2"); + assertEquals(composedCat.getAllOf().get(1).get$ref(), "#/components/schemas/foo_1"); return openAPI; } diff --git a/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml index dccf1c19bb..20ebd5f125 100644 --- a/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml +++ b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml @@ -38,6 +38,17 @@ paths: '*/*': schema: $ref: './refs-name-conflict/b.yaml#/components/schemas/PersonObj' + /thisAintAnotherPerson: + post: + summary: Create no more persons + description: Create no more persons + responses: + '200': + description: OK + content: + '*/*': + schema: + $ref: './refs-name-conflict/c.yaml#/components/schemas/PersonObj' components: schemas: PersonObj: diff --git a/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/c.yaml b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/c.yaml new file mode 100644 index 0000000000..4ce0c221df --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/c.yaml @@ -0,0 +1,13 @@ +openapi: 3.0.1 +servers: [] +info: + version: '' + title: '' +paths: {} +components: + schemas: + PersonObj: + properties: + location: + type: string + example: referred-again \ No newline at end of file