From 3a8e782ce090d7636d7050102add9cb26f62b85e Mon Sep 17 00:00:00 2001 From: Tony Tam Date: Thu, 27 Oct 2016 16:47:36 -0700 Subject: [PATCH] added check to avoid removing referenced models #278 --- .../java/io/swagger/parser/ResolverCache.java | 17 +++++++++++++---- .../parser/processors/DefinitionsProcessor.java | 10 +++++++++- .../parser/processors/ExternalRefProcessor.java | 1 + .../io/swagger/parser/SwaggerParserTest.java | 9 +++++---- .../processors/ExternalRefProcessorTest.java | 4 ++++ 5 files changed, 32 insertions(+), 9 deletions(-) diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/ResolverCache.java b/modules/swagger-parser/src/main/java/io/swagger/parser/ResolverCache.java index b2585c74f2..ba73f50a00 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/ResolverCache.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/ResolverCache.java @@ -14,10 +14,7 @@ import java.io.File; import java.nio.file.Path; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -41,6 +38,7 @@ public class ResolverCache { private final String rootPath; private Map resolutionCache = new HashMap<>(); private Map externalFileCache = new HashMap<>(); + private Set referencedModelKeys = new HashSet<>(); /* a map that stores original external references, and their associated renamed references @@ -173,6 +171,17 @@ private Object getFromMap(String ref, Map map, Pattern pattern) { return null; } + public boolean hasReferencedKey(String modelKey) { + if(referencedModelKeys == null) { + return false; + } + return referencedModelKeys.contains(modelKey); + } + + public void addReferencedKey(String modelKey) { + referencedModelKeys.add(modelKey); + } + public String getRenamedRef(String originalRef) { return renameCache.get(originalRef); } diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/processors/DefinitionsProcessor.java b/modules/swagger-parser/src/main/java/io/swagger/parser/processors/DefinitionsProcessor.java index 537c7e43c5..6588735b33 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/processors/DefinitionsProcessor.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/processors/DefinitionsProcessor.java @@ -53,7 +53,15 @@ public void processDefinitions(Set modelKeys, Map definit if (renamedRef != null) { //we definitely resolved the referenced and shoved it in the definitions map - final Model resolvedModel = definitions.remove(renamedRef); + // because the referenced model may be in the definitions map, we need to remove old instances + final Model resolvedModel = definitions.get(renamedRef); + + // ensure the reference isn't still in use + if(!cache.hasReferencedKey(renamedRef)) { + definitions.remove(renamedRef); + } + + // add the new key definitions.put(modelName, resolvedModel); } } 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 dc74e89e14..f674bcb45b 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 @@ -54,6 +54,7 @@ public String processRefToExternalDefinition(String $ref, RefFormat refFormat) { if(existingModel == null) { // don't overwrite existing model reference swagger.addDefinition(newRef, model); + cache.addReferencedKey(newRef); String file = $ref.split("#/")[0]; if (model instanceof RefModel) { 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 b7c99486c8..38ce386f57 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 @@ -185,9 +185,10 @@ public void testLoadRelativeFileTree_Json() throws Exception { public void testLoadExternalNestedDefinitions() throws Exception { SwaggerParser parser = new SwaggerParser(); final Swagger swagger = parser.read("src/test/resources/nested-references/b.yaml"); + Map definitions = swagger.getDefinitions(); assertTrue(definitions.containsKey("x")); - assertTrue(!definitions.containsKey("y")); + assertTrue(definitions.containsKey("y")); assertTrue(definitions.containsKey("z")); assertEquals(((RefModel) definitions.get("i")).get$ref(), "#/definitions/k"); } @@ -223,15 +224,15 @@ public void testLoadRelativeFileTree_Yaml() throws Exception { assertNotNull(Yaml.mapper().writeValueAsString(swagger)); } - @Test(enabled = false) + @Test public void testLoadRecursiveExternalDef() throws Exception { SwaggerParser parser = new SwaggerParser(); final Swagger swagger = parser.read("src/test/resources/file-reference-to-recursive-defs/b.yaml"); Map definitions = swagger.getDefinitions(); assertEquals(((RefProperty) ((ArrayProperty) definitions.get("v").getProperties().get("children")).getItems()).get$ref(), "#/definitions/v"); - assertTrue(!definitions.containsKey("y")); - assertEquals(((RefProperty) ((ArrayProperty) definitions.get("x").getProperties().get("children")).getItems()).get$ref(), "#/definitions/x"); + assertTrue(definitions.containsKey("y")); + assertEquals(((RefProperty) ((ArrayProperty) definitions.get("x").getProperties().get("children")).getItems()).get$ref(), "#/definitions/y"); } @Test 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 9e1e66015b..4c081df6f7 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 @@ -46,6 +46,10 @@ public void testProcessRefToExternalDefinition_NoNameConflict( cache.putRenamedRef(ref, "bar"); swagger.addDefinition("bar", mockedModel); times=1; + + cache.addReferencedKey("bar"); + times = 1; + result = null; }}; String newRef = new ExternalRefProcessor(cache, swagger).processRefToExternalDefinition(ref, refFormat);