Repository: swagger-api/swagger-parser (java)

Issue:
Relative refs inside of a ref'd section have the wrong context.
Observed when running `swagger-codegen generate -l typescript-node -o SDK -i index.yaml` against [this source](/communityelf/UpContent-API-Spec).

I've created two branches with minimal examples. The only relevant files for these branches are `index.yaml`, `path/infomous.yaml` and `definitions/error.yaml`.

The [testforron branch](/communityelf/UpContent-API-Spec/tree/testforron) demonstrates the breakage on what I believe to be the correct syntax/path for the ref in `path/infomous.yaml`. 

The [testforron2 branch](/communityelf/UpContent-API-Spec/tree/testforron2) demonstrates that the ref is working in `path/infomous.yaml`. In this case, the path is being resolved as if it were from the root, not from the path subdirectory.

This may be related to an [issue reported on swagger-codegen](/swagger-api/swagger-codegen/issues/1551).

Pull request description:
Added support for relative refs
Fixes #213 

Patch:
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 79e560b1..c65d2aa5 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
@@ -35,6 +35,7 @@ public class ResolverCache {
     private final Swagger swagger;
     private final List<AuthorizationValue> auths;
     private final Path parentDirectory;
+    private final String rootPath;
     private Map<String, Object> resolutionCache = new HashMap<>();
     private Map<String, String> externalFileCache = new HashMap<>();
 
@@ -46,6 +47,7 @@ public class ResolverCache {
     public ResolverCache(Swagger swagger, List<AuthorizationValue> auths, String parentFileLocation) {
         this.swagger = swagger;
         this.auths = auths;
+        this.rootPath = parentFileLocation;
 
         if(parentFileLocation != null) {
             if(parentFileLocation.startsWith("http")) {
@@ -87,7 +89,12 @@ public class ResolverCache {
         String contents = externalFileCache.get(file);
 
         if (contents == null) {
-            contents = RefUtils.readExternalRef(file, refFormat, auths, parentDirectory);
+            if(parentDirectory != null) {
+                contents = RefUtils.readExternalRef(file, refFormat, auths, parentDirectory);
+            }
+            else if(rootPath != null) {
+                contents = RefUtils.readExternalUrlRef(file, refFormat, auths, rootPath);
+            }
             externalFileCache.put(file, contents);
         }
 
diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/SwaggerResolver.java b/modules/swagger-parser/src/main/java/io/swagger/parser/SwaggerResolver.java
index 936552de..46da268d 100644
--- a/modules/swagger-parser/src/main/java/io/swagger/parser/SwaggerResolver.java
+++ b/modules/swagger-parser/src/main/java/io/swagger/parser/SwaggerResolver.java
@@ -14,7 +14,6 @@ import java.util.List;
  *
  */
 public class SwaggerResolver {
-
     private final Swagger swagger;
     private final ResolverCache cache;
     private final PathsProcessor pathProcessor;
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 c3483d5a..7bd04627 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
@@ -4,6 +4,7 @@ import io.swagger.models.Model;
 import io.swagger.models.auth.AuthorizationValue;
 import io.swagger.models.refs.RefFormat;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
 
 import java.io.FileInputStream;
 import java.nio.file.Files;
@@ -58,6 +59,75 @@ public class RefUtils {
         return refFormat == RefFormat.URL || refFormat == RefFormat.RELATIVE;
     }
 
+
+    public static String readExternalUrlRef(String file, RefFormat refFormat, List<AuthorizationValue> auths,
+                                         String rootPath) {
+
+        if (!RefUtils.isAnExternalRefFormat(refFormat)) {
+            throw new RuntimeException("Ref is not external");
+        }
+
+        String result;
+
+        try {
+            if (refFormat == RefFormat.URL) {
+                result = RemoteUrl.urlToString(file, auths);
+            } else {
+                //its assumed to be a relative ref
+                String url = buildUrl(rootPath, file);
+
+                return readExternalRef(url, RefFormat.URL, auths, null);
+            }
+        } catch (Exception e) {
+            throw new RuntimeException("Unable to load " + refFormat + " ref: " + file, e);
+        }
+
+        return result;
+
+    }
+
+    public static String buildUrl(String rootPath, String relativePath) {
+        String[] rootPathParts = rootPath.split("/");
+        String [] relPathParts = relativePath.split("/");
+
+        if(rootPath == null || relativePath == null) {
+            return null;
+        }
+
+        int trimRoot = 0;
+        int trimRel = 0;
+
+        if(!"".equals(rootPathParts[rootPathParts.length - 1])) {
+            trimRoot = 1;
+        }
+        for(int i = 0; i < rootPathParts.length; i++) {
+            if("".equals(rootPathParts[i])) {
+                trimRel += 1;
+            }
+            else {
+                break;
+            }
+        }
+        for(int i = 0; i < relPathParts.length; i ++) {
+            if(".".equals(relPathParts[i])) {
+                trimRel += 1;
+            }
+            else if ("..".equals(relPathParts[i])) {
+                trimRel += 1;
+            }
+        }
+
+        String [] outputParts = new String[rootPathParts.length + relPathParts.length - trimRoot - trimRel];
+        System.arraycopy(rootPathParts, 0, outputParts, 0, rootPathParts.length - trimRoot);
+        System.arraycopy(relPathParts,
+                trimRel,
+                outputParts,
+                rootPathParts.length - trimRoot + trimRel - 1,
+                relPathParts.length - trimRel);
+
+        return StringUtils.join(outputParts, "/");
+    }
+
     public static String readExternalRef(String file, RefFormat refFormat, List<AuthorizationValue> auths,
                                          Path parentDirectory) {
 


Test patch:
diff --git a/modules/swagger-parser/src/test/java/io/swagger/parser/RelativeReferenceTest.java b/modules/swagger-parser/src/test/java/io/swagger/parser/RelativeReferenceTest.java
new file mode 100644
index 00000000..6b00f917
--- /dev/null
+++ b/modules/swagger-parser/src/test/java/io/swagger/parser/RelativeReferenceTest.java
@@ -0,0 +1,66 @@
+package io.swagger.parser;
+
+import io.swagger.models.Swagger;
+import io.swagger.models.parameters.BodyParameter;
+import io.swagger.models.parameters.Parameter;
+import io.swagger.parser.util.RemoteUrl;
+import mockit.Expectations;
+import mockit.Mocked;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+public class RelativeReferenceTest {
+    @Mocked
+    RemoteUrl remoteUrl;
+
+    static final String spec =
+            "swagger: '2.0'\n" +
+            "schemes:\n" +
+            "  - https\n" +
+            "basePath: /\n" +
+            "produces:\n" +
+            "  - application/json\n" +
+            "info:\n" +
+            "  title: My API\n" +
+            "  description: It works.\n" +
+            "  version: '1.0.0'\n" +
+            "paths:\n" +
+            "  /samplePath:\n" +
+            "    $ref: './path/samplePath.yaml'";
+    static final String samplePath =
+            "get:\n" +
+            "  parameters:\n" +
+            "    - name: data\n" +
+            "      in: body\n" +
+            "      required: true\n" +
+            "      schema:\n" +
+            "        type: object\n" +
+            "  responses:\n" +
+            "    200:\n" +
+            "      description: It works";
+
+    @Test
+    public void testIssue213() throws Exception {
+        new Expectations() {{
+            RemoteUrl.urlToString("http://foo.bar.com/swagger.json", null);
+            times = 1;
+            result = spec;
+
+            RemoteUrl.urlToString("http://foo.bar.com/path/samplePath.yaml", null);
+            times = 1;
+            result = samplePath;
+        }};
+
+        Swagger swagger = new SwaggerParser().read("http://foo.bar.com/swagger.json");
+
+        assertNotNull(swagger.getPath("/samplePath").getGet());
+        assertNotNull(swagger.getPath("/samplePath").getGet().getParameters().get(0));
+        Parameter param = swagger.getPath("/samplePath").getGet().getParameters().get(0);
+        assertTrue(param instanceof BodyParameter);
+
+        BodyParameter bp = (BodyParameter) param;
+        assertNotNull(bp.getSchema());
+    }
+}
diff --git a/modules/swagger-parser/src/test/java/io/swagger/parser/ResolverCacheTest.java b/modules/swagger-parser/src/test/java/io/swagger/parser/ResolverCacheTest.java
index e3fc1cdd..75364de3 100644
--- a/modules/swagger-parser/src/test/java/io/swagger/parser/ResolverCacheTest.java
+++ b/modules/swagger-parser/src/test/java/io/swagger/parser/ResolverCacheTest.java
@@ -3,7 +3,6 @@ package io.swagger.parser;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
-
 import io.swagger.models.Model;
 import io.swagger.models.Response;
 import io.swagger.models.Swagger;
@@ -12,10 +11,9 @@ import io.swagger.models.parameters.Parameter;
 import io.swagger.models.refs.RefFormat;
 import io.swagger.parser.util.DeserializationUtils;
 import io.swagger.parser.util.RefUtils;
+import mockit.Expectations;
 import mockit.Injectable;
 import mockit.Mocked;
-import mockit.StrictExpectations;
-
 import org.apache.commons.lang3.tuple.Pair;
 import org.testng.annotations.Test;
 
@@ -25,7 +23,6 @@ import java.util.Map;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNull;
-import static org.testng.Assert.fail;
 
 public class ResolverCacheTest {
 
@@ -40,16 +37,14 @@ public class ResolverCacheTest {
     @Injectable
     List<AuthorizationValue> auths;
 
-
     @Test
-    public void testLoadExternalRef_NoDefinitionPath(@Injectable final Model expectedResult) throws Exception {
-
+    public void testMock(@Injectable final Model expectedResult) throws Exception {
         final RefFormat format = RefFormat.URL;
         final String ref = "http://my.company.com/path/to/file.json";
         final String contentsOfExternalFile = "really good json";
 
-        new StrictExpectations() {{
-            RefUtils.readExternalRef(ref, format, auths, null);
+        new Expectations() {{
+            RefUtils.readExternalUrlRef(ref, format, auths, "http://my.company.com/path/parent.json");
             times = 1;
             result = contentsOfExternalFile;
 
@@ -62,134 +57,39 @@ public class ResolverCacheTest {
 
         Model firstActualResult = cache.loadRef(ref, RefFormat.URL, Model.class);
 
-        assertEquals(contentsOfExternalFile, cache.getExternalFileCache().get(ref));
-        assertEquals(expectedResult, cache.getResolutionCache().get(ref));
-        assertEquals(expectedResult, firstActualResult);
-
-        //requesting the same ref a second time should not result in reading the external file again
-        Model secondActualResult = cache.loadRef(ref, format, Model.class);
-        assertEquals(expectedResult, secondActualResult);
 
+        assertEquals(firstActualResult, expectedResult);
     }
 
     @Test
-    public void testLoadExternalRef_TwoRefsFromSameFile(@Injectable final Model expectedResult1,
-                                                        @Injectable final Model expectedResult2) throws Exception {
-
-        final RefFormat format = RefFormat.URL;
-        final String file = "http://my.company.com/path/to/file.json";
-        final String path1 = "#/hello/world/foo/bar";
-        final String path2 = "#/hello/world/this/that";
-        final String fullRef1 = file + path1;
-        final String fullRef2 = file + path2;
-        final String contentsOfExternalFile = "really good json";
-        final Pair<JsonNode, JsonNode> nodePair = constructJsonTree("hello", "world", "foo", "bar");
-
-        ObjectNode parentNode = (ObjectNode) nodePair.getLeft();
-        ObjectNode intermediateNode = (ObjectNode) parentNode.get("hello").get("world");
-        final ObjectNode secondLeaf = intermediateNode.putObject("this").putObject("that");
-
-
-        new StrictExpectations() {{
-            RefUtils.readExternalRef(file, format, auths, null);
-            times = 1;
-            result = contentsOfExternalFile;
-
-            DeserializationUtils.deserializeIntoTree(contentsOfExternalFile, file);
-            times = 1;
-            result = nodePair.getLeft();
-
-            DeserializationUtils.deserialize(nodePair.getRight(), file, Model.class);
-            times = 1;
-            result = expectedResult1;
-
-            DeserializationUtils.deserializeIntoTree(contentsOfExternalFile, file);
-            times = 1;
-            result = nodePair.getLeft();
-
-            DeserializationUtils.deserialize(secondLeaf, file, Model.class);
-            times = 1;
-            result = expectedResult2;
-        }};
-
-        ResolverCache cache = new ResolverCache(swagger, auths, "http://my.company.com/path/parent.json");
-
-        Model firstRefResult = cache.loadRef(fullRef1, format, Model.class);
-        assertEquals(expectedResult1, cache.getResolutionCache().get(fullRef1));
-        assertEquals(expectedResult1, firstRefResult);
-
-        Model secondRefResult = cache.loadRef(fullRef2, format, Model.class);
-        assertEquals(expectedResult2, cache.getResolutionCache().get(fullRef2));
-        assertEquals(expectedResult2, secondRefResult);
-
-        assertEquals(contentsOfExternalFile, cache.getExternalFileCache().get(file));
-    }
-
-    @Test
-    public void testLoadExternalRef_WithDefinitionPath(@Injectable final Model expectedResult) throws Exception {
+    public void testLoadExternalRef_NoDefinitionPath(@Injectable final Model expectedResult) throws Exception {
 
         final RefFormat format = RefFormat.URL;
-        final String file = "http://my.company.com/path/to/file.json";
-        final String path = "#/hello/world/foo/bar";
-        final String fullRef = file + path;
+        final String ref = "http://my.company.com/path/to/file.json";
         final String contentsOfExternalFile = "really good json";
-        final Pair<JsonNode, JsonNode> nodePair = constructJsonTree("hello", "world", "foo", "bar");
 
-        new StrictExpectations() {{
-            RefUtils.readExternalRef(file, format, auths, null);
+        new Expectations() {{
+            RefUtils.readExternalUrlRef(ref, format, auths, "http://my.company.com/path/parent.json");
             times = 1;
             result = contentsOfExternalFile;
 
-            DeserializationUtils.deserializeIntoTree(contentsOfExternalFile, file);
-            times = 1;
-            result = nodePair.getLeft();
-
-            DeserializationUtils.deserialize(nodePair.getRight(), file, Model.class);
+            DeserializationUtils.deserialize(contentsOfExternalFile, ref, Model.class);
             times = 1;
             result = expectedResult;
         }};
 
         ResolverCache cache = new ResolverCache(swagger, auths, "http://my.company.com/path/parent.json");
-        Model firstActualResult = cache.loadRef(fullRef, format, Model.class);
 
-        assertEquals(contentsOfExternalFile, cache.getExternalFileCache().get(file));
-        assertEquals(expectedResult, cache.getResolutionCache().get(fullRef));
+        Model firstActualResult = cache.loadRef(ref, RefFormat.URL, Model.class);
+
+        assertEquals(contentsOfExternalFile, cache.getExternalFileCache().get(ref));
+        assertEquals(expectedResult, cache.getResolutionCache().get(ref));
         assertEquals(expectedResult, firstActualResult);
 
         //requesting the same ref a second time should not result in reading the external file again
-        Model secondActualResult = cache.loadRef(fullRef, format, Model.class);
+        Model secondActualResult = cache.loadRef(ref, format, Model.class);
         assertEquals(expectedResult, secondActualResult);
-    }
 
-    @Test
-    public void testLoadExternalRef_WithInvalidDefinitionPath() throws Exception {
-
-        final RefFormat format = RefFormat.URL;
-        final String file = "http://my.company.com/path/to/file.json";
-        final String path = "hello/world/bar/foo";
-        final String fullRef = file + "#/" + path;
-        final String contentsOfExternalFile = "really good json";
-        final Pair<JsonNode, JsonNode> nodePair = constructJsonTree("hello", "world", "foo", "bar");
-
-        new StrictExpectations() {{
-            RefUtils.readExternalRef(file, format, auths, null);
-            times = 1;
-            result = contentsOfExternalFile;
-
-            DeserializationUtils.deserializeIntoTree(contentsOfExternalFile, file);
-            times = 1;
-            result = nodePair.getLeft();
-
-        }};
-
-        ResolverCache cache = new ResolverCache(swagger, auths, "http://my.company.com/path/parent.json");
-
-        try {
-            cache.loadRef(fullRef, format, Model.class);
-            fail("Should have thrown an exception");
-        } catch(RuntimeException e) {
-            assertEquals("Could not find " + path + " in contents of " + file, e.getMessage());
-        }
     }
 
     @Test