diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/processors/ExternalRefProcessor.java b/modules/swagger-parser/src/main/java/io/swagger/parser/processors/ExternalRefProcessor.java index a966bd8c65..7b349a9be1 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/processors/ExternalRefProcessor.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/processors/ExternalRefProcessor.java @@ -29,6 +29,11 @@ public ExternalRefProcessor(ResolverCache cache, Swagger swagger) { } public String processRefToExternalDefinition(String $ref, RefFormat refFormat) { + String renamedRef = cache.getRenamedRef($ref); + if(renamedRef != null) { + return renamedRef; + } + final Model model = cache.loadRef($ref, refFormat, Model.class); if(model == null) { @@ -45,7 +50,7 @@ public String processRefToExternalDefinition(String $ref, RefFormat refFormat) { definitions = new LinkedHashMap<>(); } - final String possiblyConflictingDefinitionName = computeDefinitionName($ref); + final String possiblyConflictingDefinitionName = computeDefinitionName($ref, definitions.keySet()); Model existingModel = definitions.get(possiblyConflictingDefinitionName); diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/util/RefUtils.java b/modules/swagger-parser/src/main/java/io/swagger/parser/util/RefUtils.java index 662de0d64a..e878f240fb 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/util/RefUtils.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/util/RefUtils.java @@ -9,10 +9,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.Set; public class RefUtils { - public static String computeDefinitionName(String ref) { + public static String computeDefinitionName(String ref, Set reserved) { final String[] refParts = ref.split("#/"); @@ -36,8 +37,13 @@ public static String computeDefinitionName(String ref) { final String[] split = plausibleName.split("\\."); plausibleName = split[0]; } + String tryName = plausibleName; - return plausibleName; + for (int i = 2; reserved.contains(tryName); i++) { + tryName = plausibleName + "_" + i; + } + + return tryName; } public static boolean isAnExternalRefFormat(RefFormat refFormat) { diff --git a/modules/swagger-parser/src/test/java/io/swagger/parser/JsonToYamlFileDuplicator.java b/modules/swagger-parser/src/test/java/io/swagger/parser/JsonToYamlFileDuplicator.java index 00b800c493..758f9144e6 100644 --- a/modules/swagger-parser/src/test/java/io/swagger/parser/JsonToYamlFileDuplicator.java +++ b/modules/swagger-parser/src/test/java/io/swagger/parser/JsonToYamlFileDuplicator.java @@ -42,8 +42,8 @@ private static void processFile(File next, Path inputDirectory, Path outputDirec final JsonNode jsonNode = DeserializationUtils.deserializeIntoTree(fileContents, next.toString()); - final String yamlOutput = Yaml.mapper().writerWithDefaultPrettyPrinter().writeValueAsString(jsonNode); - + final String yamlOutput = Yaml.mapper().writerWithDefaultPrettyPrinter().writeValueAsString(jsonNode) + .replaceAll("\\n", System.getProperty("line.separator")); final String relativePath = "./" + next.toString().replace(inputDirectory.toString(), "").replace(".json", ".yaml"); diff --git a/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java b/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java index 8e98e530f7..0b3509a475 100644 --- a/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java +++ b/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java @@ -220,7 +220,9 @@ public void testLoadExternalNestedDefinitions() throws Exception { assertTrue(definitions.containsKey("x")); assertTrue(definitions.containsKey("y")); assertTrue(definitions.containsKey("z")); - assertEquals(((RefModel) definitions.get("i")).get$ref(), "#/definitions/k"); + assertEquals("#/definitions/k_2", ((RefModel) definitions.get("i")).get$ref()); + assertEquals("k-definition", definitions.get("k").getTitle()); + assertEquals("k-definition", definitions.get("k_2").getTitle()); } @Test @@ -613,7 +615,7 @@ private Swagger doRelativeFileTest(String location) { assertEquals(composedCat.getInterfaces().size(), 2); assertEquals(composedCat.getInterfaces().get(0).get$ref(), "#/definitions/pet"); - assertEquals(composedCat.getInterfaces().get(1).get$ref(), "#/definitions/foo"); + assertEquals(composedCat.getInterfaces().get(1).get$ref(), "#/definitions/foo_2"); return swagger; } @@ -1013,4 +1015,15 @@ public void readingSpecNodeShouldNotOverQuotingStringExample() throws Exception Map definitions = swagger.getDefinitions(); assertEquals("NoQuotePlease", definitions.get("CustomerType").getExample()); } + + @Test + public void testRefNameConflicts() throws Exception { + Swagger swagger = new SwaggerParser().read("./refs-name-conflict/a.yaml"); + + assertEquals("#/definitions/PersonObj", ((RefProperty) swagger.getPath("/newPerson").getPost().getResponses().get("200").getSchema()).get$ref()); + assertEquals("#/definitions/PersonObj_2", ((RefProperty) swagger.getPath("/oldPerson").getPost().getResponses().get("200").getSchema()).get$ref()); + assertEquals("#/definitions/PersonObj_2", ((RefProperty) swagger.getPath("/yetAnotherPerson").getPost().getResponses().get("200").getSchema()).get$ref()); + assertEquals("local", swagger.getDefinitions().get("PersonObj").getProperties().get("location").getExample()); + assertEquals("referred", swagger.getDefinitions().get("PersonObj_2").getProperties().get("location").getExample()); + } } \ No newline at end of file diff --git a/modules/swagger-parser/src/test/java/io/swagger/parser/processors/ExternalRefProcessorTest.java b/modules/swagger-parser/src/test/java/io/swagger/parser/processors/ExternalRefProcessorTest.java index 4c081df6f7..cc0ca0ebe0 100644 --- a/modules/swagger-parser/src/test/java/io/swagger/parser/processors/ExternalRefProcessorTest.java +++ b/modules/swagger-parser/src/test/java/io/swagger/parser/processors/ExternalRefProcessorTest.java @@ -36,6 +36,10 @@ public void testProcessRefToExternalDefinition_NoNameConflict( final RefFormat refFormat = RefFormat.URL; new StrictExpectations() {{ + cache.getRenamedRef(ref); + times = 1; + result = null; + cache.loadRef(ref, refFormat, Model.class); times = 1; result = mockedModel; diff --git a/modules/swagger-parser/src/test/java/io/swagger/parser/util/RefUtilsTest.java b/modules/swagger-parser/src/test/java/io/swagger/parser/util/RefUtilsTest.java index c484ea392a..c19d186693 100644 --- a/modules/swagger-parser/src/test/java/io/swagger/parser/util/RefUtilsTest.java +++ b/modules/swagger-parser/src/test/java/io/swagger/parser/util/RefUtilsTest.java @@ -16,10 +16,14 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import static java.util.Arrays.asList; +import static java.util.Collections.singleton; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -61,10 +65,13 @@ public void testComputeDefinitionName() throws Exception { doComputeDefinitionNameTestCase("./path/to/file#/foo/bar", "bar"); doComputeDefinitionNameTestCase("./path/to/file#/foo/bar/hello", "hello"); + // Name conflicts resolved by adding _number + assertEquals("file_2", RefUtils.computeDefinitionName("http://my.company.com/path/to/file.json", singleton("file"))); + assertEquals("file_3", RefUtils.computeDefinitionName("http://my.company.com/path/to/file.json", new HashSet<>(asList("file", "file_2")))); } private void doComputeDefinitionNameTestCase(String ref, String expectedDefinitionName) { - assertEquals(expectedDefinitionName, RefUtils.computeDefinitionName(ref)); + assertEquals(expectedDefinitionName, RefUtils.computeDefinitionName(ref, Collections.emptySet())); } private Map createMap(String... keys) { diff --git a/modules/swagger-parser/src/test/resources/nested-references/b.yaml b/modules/swagger-parser/src/test/resources/nested-references/b.yaml index 184f5ae814..b978c20c21 100644 --- a/modules/swagger-parser/src/test/resources/nested-references/b.yaml +++ b/modules/swagger-parser/src/test/resources/nested-references/b.yaml @@ -11,6 +11,7 @@ definitions: $ref: "./a.yaml#/definitions/j" k: type: object + title: k-definition properties: name: type: string diff --git a/modules/swagger-parser/src/test/resources/refs-name-conflict/a.yaml b/modules/swagger-parser/src/test/resources/refs-name-conflict/a.yaml new file mode 100644 index 0000000000..e1e3711593 --- /dev/null +++ b/modules/swagger-parser/src/test/resources/refs-name-conflict/a.yaml @@ -0,0 +1,43 @@ +swagger: '2.0' +info: + version: 1.0.0 + title: Person + description: Maintain Person data +paths: + /newPerson: + post: + summary: Create new person + description: Create new person + parameters: [] + responses: + 200: + description: OK + schema: + $ref: '#/definitions/PersonObj' + /oldPerson: + post: + summary: Create old person + description: Create old person + parameters: [] + responses: + 200: + description: OK + schema: + $ref: "./refs-name-conflict/b.yaml#/definitions/PersonObj" + /yetAnotherPerson: + post: + summary: Create yet another person + description: Create yet another person + parameters: [] + responses: + 200: + description: OK + schema: + $ref: "./refs-name-conflict/b.yaml#/definitions/PersonObj" + +definitions: + PersonObj: + properties: + location: + type: string + example: local diff --git a/modules/swagger-parser/src/test/resources/refs-name-conflict/b.yaml b/modules/swagger-parser/src/test/resources/refs-name-conflict/b.yaml new file mode 100644 index 0000000000..21753b4381 --- /dev/null +++ b/modules/swagger-parser/src/test/resources/refs-name-conflict/b.yaml @@ -0,0 +1,7 @@ +--- +definitions: + PersonObj: + properties: + location: + type: string + example: referred