added check to avoid removing referenced models #278

swagger-api · Oct 27, 2016 · 3a8e782 · 3a8e782
1 parent 6e83b1d
commit 3a8e782
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 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
@@ -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<String, Object> resolutionCache = new HashMap<>();
     private Map<String, String> externalFileCache = new HashMap<>();
+    private Set<String> 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
@@ -53,7 +53,15 @@ public void processDefinitions(Set<String> modelKeys, Map<String, Model> 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
@@ -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
@@ -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<String, Model> 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<String, Model> 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/...s/swagger-parser/src/test/java/io/swagger/parser/processors/ExternalRefProcessorTest.java b/...s/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);