#1040 Fixed duplication of external reference path

jakubmalek · gracekarina · commit 9c408f058bc1 · 2019-03-15T15:58:31.000-05:00
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
@@ -20,16 +20,20 @@
 import io.swagger.v3.parser.ResolverCache;
 import io.swagger.v3.parser.models.RefFormat;
 import io.swagger.v3.parser.models.RefType;
+import io.swagger.v3.parser.util.RefUtils;
+
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.LoggerFactory;
 
 import java.net.URI;
 import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import static io.swagger.v3.parser.util.RefUtils.computeDefinitionName;
 import static io.swagger.v3.parser.util.RefUtils.computeRefFormat;
+import static io.swagger.v3.parser.util.RefUtils.getExternalPath;
 import static io.swagger.v3.parser.util.RefUtils.isAnExternalRefFormat;
 
 public final class ExternalRefProcessor {
@@ -689,8 +693,10 @@ private void processRefSchema(Schema subRef, String externalFile) {
             return;
         }
         String $ref = subRef.get$ref();
+        String subRefExternalPath = getExternalPath(subRef.get$ref())
+            .orElse(null);
 
-        if (format.equals(RefFormat.RELATIVE)) {
+        if (format.equals(RefFormat.RELATIVE) && !Objects.equals(subRefExternalPath, externalFile)) {
             $ref = constructRef(subRef, externalFile);
             subRef.set$ref($ref);
         }else {
diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RefUtils.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RefUtils.java
@@ -5,19 +5,27 @@
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 
-
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.Set;
+import java.util.Optional;
 
 public class RefUtils {
 
+    private static final String REFERENCE_SEPARATOR = "#/";
+
+    private RefUtils() {
+        // static access only
+    }
+
     public static String computeDefinitionName(String ref) {
 
-        final String[] refParts = ref.split("#/");
+        final String[] refParts = ref.split(REFERENCE_SEPARATOR);
 
         if (refParts.length > 2) {
             throw new RuntimeException("Invalid ref format: " + ref);
@@ -43,6 +51,16 @@ public static String computeDefinitionName(String ref) {
         return plausibleName;
     }
 
+    public static Optional<String> getExternalPath(String ref) {
+        if (ref == null) {
+            return Optional.empty();
+        }
+        return Optional.of(ref.split(REFERENCE_SEPARATOR))
+            .filter(it -> it.length == 2)
+            .map(it -> it[0])
+            .filter(it -> !it.isEmpty());
+    }
+
     public static boolean isAnExternalRefFormat(RefFormat refFormat) {
         return refFormat == RefFormat.URL || refFormat == RefFormat.RELATIVE;
     }
@@ -52,9 +70,9 @@ public static RefFormat computeRefFormat(String ref) {
         ref = mungedRef(ref);
         if(ref.startsWith("http")||ref.startsWith("https")) {
             result = RefFormat.URL;
-        } else if(ref.startsWith("#/")) {
+        } else if(ref.startsWith(REFERENCE_SEPARATOR)) {
             result = RefFormat.INTERNAL;
-        } else if(ref.startsWith(".") || ref.startsWith("/") || ref.indexOf("#/") > 0) {
+        } else if(ref.startsWith(".") || ref.startsWith("/") || ref.indexOf(REFERENCE_SEPARATOR) > 0) {
             result = RefFormat.RELATIVE;
         }
 
@@ -159,7 +177,7 @@ public static String readExternalRef(String file, RefFormat refFormat, List<Auth
                 final Path pathToUse = parentDirectory.resolve(file).normalize();
 
                 if(Files.exists(pathToUse)) {
-                    result = IOUtils.toString(new FileInputStream(pathToUse.toFile()), "UTF-8");
+                    result = readAll(pathToUse);
                 } else {
                     String url = file;
                     if(url.contains("..")) {
@@ -170,7 +188,7 @@ public static String readExternalRef(String file, RefFormat refFormat, List<Auth
                     final Path pathToUse2 = parentDirectory.resolve(url).normalize();
 
                     if(Files.exists(pathToUse2)) {
-                        result = IOUtils.toString(new FileInputStream(pathToUse2.toFile()), "UTF-8");
+                        result = readAll(pathToUse2);
                     }
                 }
                 if (result == null){
@@ -186,4 +204,10 @@ public static String readExternalRef(String file, RefFormat refFormat, List<Auth
         return result;
 
     }
+
+    private static String readAll(Path path) throws IOException {
+        try (InputStream inputStream = new FileInputStream(path.toFile())) {
+            return IOUtils.toString(inputStream, UTF_8);
+        }
+    }
 }
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
@@ -52,6 +52,9 @@
 import java.util.*;
 
 import static com.github.tomakehurst.wiremock.client.WireMock.*;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.junit.Assert.assertThat;
 import static org.testng.Assert.*;
 
@@ -1780,6 +1783,24 @@ public void testValidationIssue() {
         assertThat(result.getMessages().size(), CoreMatchers.is(0));
     }
 
+    @Test
+    public void shouldParseExternalSchemaModelHavingReferenceToItsLocalModel() {
+        // given
+        String location = "src/test/resources/issue-1040/api.yaml";
+        OpenAPIV3Parser tested = new OpenAPIV3Parser();
+
+        // when
+        OpenAPI result = tested.read(location);
+
+        // then
+        Components components = result.getComponents();
+        Schema modelSchema = components.getSchemas().get("Value");
+
+        assertThat(modelSchema, notNullValue());
+        assertThat(modelSchema.getProperties().get("id"), instanceOf(Schema.class));
+        assertThat(((Schema) modelSchema.getProperties().get("id")).get$ref(), equalTo("#/components/schemas/ValueId"));
+    }
+
     private static int getDynamicPort() {
         return new Random().ints(10000, 20000).findFirst().getAsInt();
     }
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/util/RefUtilsTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/util/RefUtilsTest.java
@@ -24,7 +24,12 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
@@ -155,7 +160,7 @@ public void testReadExternalRef_RelativeFileFormat(@Injectable final List<Author
         setupRelativeFileExpectations(parentDirectory, pathToUse, file, filePath);
 
         new StrictExpectations() {{
-            IOUtils.toString((FileInputStream) any, "UTF-8");
+            IOUtils.toString((FileInputStream) any, UTF_8);
             times = 1;
             result = expectedResult;
 
@@ -198,7 +203,7 @@ public void testReadExternalRef_RelativeFileFormat_ExceptionThrown(@Injectable f
         setupRelativeFileExpectations(parentDirectory, pathToUse, file, filePath);
 
         new StrictExpectations() {{
-            IOUtils.toString((FileInputStream) any, "UTF-8");
+            IOUtils.toString((FileInputStream) any, UTF_8);
             times = 1;
             result = new IOException();
         }};
@@ -293,4 +298,78 @@ public void testPathJoin1() {
         // relative locations
         assertEquals(ExternalRefProcessor.join("./foo#/definitions/Foo", "./bar#/definitions/Bar"), "./bar#/definitions/Bar");
     }
+
+    @Test
+    public void shouldReturnEmptyExternalPathForInternalReference() {
+        // given
+        String ref = "#/components/test";
+
+        // when
+        Optional<String> externalPath = RefUtils.getExternalPath(ref);
+
+        // then
+        assertThat(externalPath.isPresent(), is(false));
+    }
+
+    @Test
+    public void shouldReturnEmptyExternalPathForNullReference() {
+        // given
+        String ref = null;
+
+        // when
+        Optional<String> externalPath = RefUtils.getExternalPath(ref);
+
+        // then
+        assertThat(externalPath.isPresent(), is(false));
+    }
+
+    @Test
+    public void shouldReturnEmptyExternalPathForEmptyReference() {
+        // given
+        String ref = "";
+
+        // when
+        Optional<String> externalPath = RefUtils.getExternalPath(ref);
+
+        // then
+        assertThat(externalPath.isPresent(), is(false));
+    }
+
+    @Test
+    public void shouldReturnEmptyExternalPathForInvalidReference() {
+        // given
+        String ref = "test";
+
+        // when
+        Optional<String> externalPath = RefUtils.getExternalPath(ref);
+
+        // then
+        assertThat(externalPath.isPresent(), is(false));
+    }
+
+    @Test
+    public void shouldReturnExternalPathForFileReference() {
+        // given
+        String ref = "test.yaml#/components/test";
+
+        // when
+        Optional<String> externalPath = RefUtils.getExternalPath(ref);
+
+        // then
+        assertThat(externalPath.isPresent(), is(true));
+        assertThat(externalPath.get(), equalTo("test.yaml"));
+    }
+
+    @Test
+    public void shouldReturnExternalPathForHttpReference() {
+        // given
+        String ref = "http://localhost/schema.json#/components/test";
+
+        // when
+        Optional<String> externalPath = RefUtils.getExternalPath(ref);
+
+        // then
+        assertThat(externalPath.isPresent(), is(true));
+        assertThat(externalPath.get(), equalTo("http://localhost/schema.json"));
+    }
 }
diff --git a/modules/swagger-parser-v3/src/test/resources/issue-1040/api.yaml b/modules/swagger-parser-v3/src/test/resources/issue-1040/api.yaml
@@ -0,0 +1,16 @@
+openapi: 3.0.0
+info:
+  title: Test
+  version: 1.0.0
+
+paths:
+  /value:
+    get:
+      operationId: getValues
+      responses:
+        200:
+          description: Successful response
+          content:
+            application/json:
+              schema:
+                $ref: 'lib/lib.yaml#/components/schemas/Value'
diff --git a/modules/swagger-parser-v3/src/test/resources/issue-1040/lib/lib.yaml b/modules/swagger-parser-v3/src/test/resources/issue-1040/lib/lib.yaml
@@ -0,0 +1,13 @@
+components:
+  schemas:
+    ValueId:
+      type: object
+      properties:
+        id:
+          type: integer
+          format: int64
+    Value:
+      type: object
+      properties:
+        id:
+          $ref: '#/components/schemas/ValueId'
