Normalize relative ref paths to avoid duplicating schemas

modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/ResolverCache.java

@@ -28,6 +28,8 @@
 
 import java.io.File;
 import java.io.UnsupportedEncodingException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URLDecoder;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -68,6 +70,8 @@ public class ResolverCache {
     private final String rootPath;
     private Map<String, Object> resolutionCache = new HashMap<>();
     private Map<String, String> externalFileCache = new HashMap<>();
+    private Map<String, Object> canonicalResolutionCache = new HashMap<>();
+    private Map<String, String> canonicalExternalFileCache = new HashMap<>();
     private List<String> referencedModelKeys = new ArrayList<>();
     private Set<String> resolveValidationMessages;
     private final ParseOptions parseOptions;
@@ -79,6 +83,7 @@ public class ResolverCache {
      * references
      */
     private Map<String, String> renameCache = new HashMap<>();
+    private Map<String, String> canonicalRenameCache = new HashMap<>();
 
     public ResolverCache(OpenAPI openApi, List<AuthorizationValue> auths, String parentFileLocation) {
         this(openApi, auths, parentFileLocation, new HashSet<>());
@@ -131,11 +136,15 @@ public <T> T loadRef(String ref, RefFormat refFormat, Class<T> expectedType) {
 
         final String file = refParts[0];
         final String definitionPath = refParts.length == 2 ? refParts[1] : null;
+        final String canonicalRef = canonicalize(ref);
+        final String canonicalFile = canonicalize(file);
 
-        //we might have already resolved this ref, so check the resolutionCache
-        Object previouslyResolvedEntity = resolutionCache.get(ref);
+        //we might have already resolved an equivalent ref, so check the canonical cache
+        Object previouslyResolvedEntity = canonicalResolutionCache.get(canonicalRef);
 
         if (previouslyResolvedEntity != null) {
+            resolutionCache.putIfAbsent(ref, previouslyResolvedEntity);
+            externalFileCache.putIfAbsent(file, canonicalExternalFileCache.get(canonicalFile));
             if(expectedType.equals(Header.class)){
                 if (expectedType.getClass().equals(previouslyResolvedEntity.getClass())) {
                     return expectedType.cast(previouslyResolvedEntity);
@@ -147,7 +156,7 @@ public <T> T loadRef(String ref, RefFormat refFormat, Class<T> expectedType) {
 
         //we have not resolved this particular ref
         //but we may have already loaded the file or url in question
-        String contents = externalFileCache.get(file);
+        String contents = canonicalExternalFileCache.get(canonicalFile);
 
         if (contents == null) {
             if(parseOptions.isSafelyResolveURL()){
@@ -164,8 +173,9 @@ else if (rootPath != null) {
                 contents = RefUtils.readExternalClasspathRef(file, refFormat, auths, rootPath, permittedUrlsChecker);
 
             }
-            externalFileCache.put(file, contents);
+            canonicalExternalFileCache.put(canonicalFile, contents);
         }
+        externalFileCache.putIfAbsent(file, contents);
         SwaggerParseResult deserializationUtilResult = new SwaggerParseResult();
         JsonNode tree = DeserializationUtils.deserializeIntoTree(contents, file, parseOptions, deserializationUtilResult);
 
@@ -177,6 +187,7 @@ else if (rootPath != null) {
                 result = DeserializationUtils.deserialize(contents, file, expectedType, openapi31);
             }
             resolutionCache.put(ref, result);
+            canonicalResolutionCache.put(canonicalRef, result);
             if (deserializationUtilResult.getMessages() != null) {
                 if (this.resolveValidationMessages != null) {
                     this.resolveValidationMessages.addAll(deserializationUtilResult.getMessages());
@@ -216,6 +227,7 @@ else if (rootPath != null) {
         }
         updateLocalRefs(file, result);
         resolutionCache.put(ref, result);
+        canonicalResolutionCache.put(canonicalRef, result);
         if (deserializationUtilResult.getMessages() != null) {
             if (this.resolveValidationMessages != null) {
                 this.resolveValidationMessages.addAll(deserializationUtilResult.getMessages());
@@ -402,11 +414,23 @@ public void addReferencedKey(String modelKey) {
     }
 
     public String getRenamedRef(String originalRef) {
-        return renameCache.get(originalRef);
+        return canonicalRenameCache.get(canonicalize(originalRef));
     }
 
     public void putRenamedRef(String originalRef, String newRef) {
         renameCache.put(originalRef, newRef);
+        canonicalRenameCache.put(canonicalize(originalRef), newRef);
+    }
+
+    private static String canonicalize(String ref) {
+        if (ref == null || ref.isEmpty()) {
+            return ref;
+        }
+        try {
+            return new URI(ref).normalize().toString();
+        } catch (URISyntaxException ignored) {
+            return ref;
+        }
     }
 
     public Map<String, Object> getResolutionCache() {

modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java

@@ -34,7 +34,6 @@
 import io.swagger.v3.parser.ResolverCache;
 import io.swagger.v3.parser.models.RefFormat;
 import io.swagger.v3.parser.models.RefType;
-
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.LoggerFactory;
@@ -65,7 +64,7 @@ private String finalNameRec(Map<String, Schema> schemas, String possiblyConflict
                 // use the new model
                 existingModel = null;
             } else if (!newSchema.equals(existingModel)) {
-                if(cache.getResolutionCache().get(newSchema.get$ref())!= null){
+                if (cache.getRenamedRef(newSchema.get$ref()) != null) {
                     return tryName;
                 }
                 LOGGER.debug("A model for " + existingModel + " already exists");

modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java

@@ -600,6 +600,9 @@ public void testIssue1518() {
         OpenAPI openAPI = result.getOpenAPI();
         assertEquals(((Schema) openAPI.getComponents().getSchemas().get("Analemmata").getProperties().get("tashotSipe")).get$ref(), "#/components/schemas/TashotSipe");
         assertNull(openAPI.getComponents().getSchemas().get("analemmata"));
+        assertNull(openAPI.getComponents().getSchemas().get("TashotSipe_1"));
+        assertNotNull(openAPI.getComponents().getSchemas().get("Stunts"));
+        assertNull(openAPI.getComponents().getSchemas().get("Stunts_1"));
     }
 
     @Test
@@ -2014,6 +2017,20 @@ public void testRelativePath2() {
         Assert.assertEquals(readResult.getOpenAPI().getPaths().get("/pet/findByTags").getGet().getResponses().get("default").getContent().get("application/json").getSchema().get$ref(), "#/components/schemas/ErrorModel");
     }
 
+    @Test
+    public void testIssue2105EquivalentExternalRefsUseSingleComponent() {
+        ParseOptions options = new ParseOptions();
+        options.setResolve(true);
+        SwaggerParseResult result = new OpenAPIV3Parser()
+                .readLocation("src/test/resources/oas3.fetched/openapi3.yaml", null, options);
+
+        OpenAPI openAPI = result.getOpenAPI();
+        assertNotNull(openAPI.getComponents().getSchemas().get("Event"));
+        assertNotNull(openAPI.getComponents().getSchemas().get("EventList"));
+        assertNull(openAPI.getComponents().getSchemas().get("Event_1"));
+        assertNull(openAPI.getComponents().getSchemas().get("EventList_1"));
+    }
+
     private OpenAPI doRelativeFileTest(String location) {
         OpenAPIV3Parser parser = new OpenAPIV3Parser();
         ParseOptions options = new ParseOptions();
@@ -3342,9 +3359,11 @@ public void testIssue1886() {
         OpenAPIV3Parser openApiParser = new OpenAPIV3Parser();
         SwaggerParseResult parseResult = openApiParser.readLocation("issue-1886/openapi.yaml", null, options);
         OpenAPI openAPI = parseResult.getOpenAPI();
+        assertNotNull(openAPI.getComponents().getSchemas().get("Enum1"));
+        assertNull(openAPI.getComponents().getSchemas().get("Enum1_1"));
         assertEqualsNoOrder(
             openAPI.getComponents().getSchemas().keySet(),
-            Arrays.asList("ArrayPojo", "Enum1", "Enum1_1", "Enum2", "Enum3", "MapPojo", "SetPojo", "SimplePojo",
+            Arrays.asList("ArrayPojo", "Enum1", "Enum2", "Enum3", "MapPojo", "SetPojo", "SimplePojo",
                 "TransactionsPatchRequestBody", "additional-properties", "array-pojo", "locale-translation-item",
                 "map-pojo", "set-pojo", "simple-pojo", "translation-item")
         );

modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/ResolverCacheTest.java

@@ -29,6 +29,7 @@
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertSame;
 
 public class ResolverCacheTest {
 
@@ -258,4 +259,136 @@ public void testRenameCache() {
         cache.putRenamedRef("foo", "bar");
         assertEquals(cache.getRenamedRef("foo"), "bar");
     }
+
+    @Test
+    public void testEquivalentUriRefsShareCacheIdentity() {
+        final String refWithDotSegments =
+                "http://my.company.com/schemas/../schemas/file.yaml#/components/schemas/Foo";
+        final String canonicalRef =
+                "http://my.company.com/schemas/file.yaml#/components/schemas/Foo";
+        final String contents = "components:\n  schemas:\n    Foo:\n      type: string\n";
+
+        new Expectations() {{
+            RefUtils.readExternalUrlRef(
+                    "http://my.company.com/schemas/../schemas/file.yaml",
+                    RefFormat.URL,
+                    auths,
+                    "http://my.company.com/root.yaml",
+                    (PermittedUrlsChecker) any);
+            times = 1;
+            result = contents;
+        }};
+
+        ResolverCache cache = new ResolverCache(openAPI, auths, "http://my.company.com/root.yaml");
+        Schema first = cache.loadRef(refWithDotSegments, RefFormat.URL, Schema.class);
+        Schema second = cache.loadRef(canonicalRef, RefFormat.URL, Schema.class);
+
+        assertSame(first, second);
+        assertEquals(cache.getExternalFileCache().size(), 2);
+        assertEquals(
+                cache.getExternalFileCache().get(
+                        "http://my.company.com/schemas/../schemas/file.yaml"),
+                contents);
+        assertEquals(
+                cache.getExternalFileCache().get("http://my.company.com/schemas/file.yaml"),
+                contents);
+        assertEquals(cache.getResolutionCache().size(), 2);
+        assertSame(cache.getResolutionCache().get(refWithDotSegments), first);
+        assertSame(cache.getResolutionCache().get(canonicalRef), first);
+        cache.putRenamedRef(refWithDotSegments, "Foo");
+        assertEquals(cache.getRenamedRef(canonicalRef), "Foo");
+        assertEquals(cache.getRenameCache().get(refWithDotSegments), "Foo");
+        assertNull(cache.getRenameCache().get(canonicalRef));
+    }
+
+    @Test
+    public void testEquivalentApiResponseRefsShareCacheIdentity() {
+        final String refWithDotSegments =
+                "http://my.company.com/responses/../responses/common.yaml#/components/responses/Error";
+        final String canonicalRef =
+                "http://my.company.com/responses/common.yaml#/components/responses/Error";
+        final String contents =
+                "components:\n  responses:\n    Error:\n      description: Error response\n";
+
+        new Expectations() {{
+            RefUtils.readExternalUrlRef(
+                    "http://my.company.com/responses/../responses/common.yaml",
+                    RefFormat.URL,
+                    auths,
+                    "http://my.company.com/root.yaml",
+                    (PermittedUrlsChecker) any);
+            times = 1;
+            result = contents;
+        }};
+
+        ResolverCache cache = new ResolverCache(openAPI, auths, "http://my.company.com/root.yaml");
+        ApiResponse first = cache.loadRef(refWithDotSegments, RefFormat.URL, ApiResponse.class);
+        ApiResponse second = cache.loadRef(canonicalRef, RefFormat.URL, ApiResponse.class);
+
+        assertSame(first, second);
+        assertEquals(first.getDescription(), "Error response");
+        assertEquals(cache.getExternalFileCache().size(), 2);
+        assertEquals(cache.getResolutionCache().size(), 2);
+        assertSame(cache.getResolutionCache().get(refWithDotSegments), first);
+        assertSame(cache.getResolutionCache().get(canonicalRef), first);
+    }
+
+    @Test
+    public void testIssue2016EquivalentRelativeRefsShareRenameCacheIdentity() {
+        ResolverCache cache = new ResolverCache(openAPI, auths, null);
+        String refWithRedundantDotSegment = "./../A.yaml#/components/schemas/A";
+        String equivalentRef = "../A.yaml#/components/schemas/A";
+
+        cache.putRenamedRef(refWithRedundantDotSegment, "A");
+
+        assertEquals(cache.getRenamedRef(equivalentRef), "A");
+        assertEquals(cache.getRenameCache().size(), 1);
+        assertEquals(cache.getRenameCache().get(refWithRedundantDotSegment), "A");
+        assertNull(cache.getRenameCache().get(equivalentRef));
+    }
+
+    @Test
+    public void testCanonicalRenameCacheKeysPreserveUriParts() {
+        ResolverCache cache = new ResolverCache(openAPI, auths, null);
+
+        cache.putRenamedRef(
+                "file:///tmp/schemas/../Foo.yaml#/components/schemas/Foo", "FileFoo");
+        cache.putRenamedRef(
+                "/tmp/schemas/../Foo.yaml#/components/schemas/Foo", "AbsoluteFoo");
+        cache.putRenamedRef(
+                "https://example.com/a/../Foo.yaml?version=1#/components/schemas/Foo", "HttpFoo");
+        cache.putRenamedRef("#/components/schemas/Foo", "InternalFoo");
+        cache.putRenamedRef("C:\\schemas\\Foo.yaml", "WindowsPath");
+        cache.putRenamedRef(
+                "my schemas/../Foo.yaml#/components/schemas/Foo", "UnencodedSpace");
+
+        assertEquals(
+                cache.getRenameCache().get(
+                        "file:///tmp/schemas/../Foo.yaml#/components/schemas/Foo"),
+                "FileFoo");
+        assertEquals(
+                cache.getRenameCache().get(
+                        "/tmp/schemas/../Foo.yaml#/components/schemas/Foo"),
+                "AbsoluteFoo");
+        assertEquals(
+                cache.getRenameCache().get(
+                        "https://example.com/a/../Foo.yaml?version=1#/components/schemas/Foo"),
+                "HttpFoo");
+        assertEquals(cache.getRenameCache().get("#/components/schemas/Foo"), "InternalFoo");
+        assertEquals(cache.getRenameCache().get("C:\\schemas\\Foo.yaml"), "WindowsPath");
+        assertEquals(
+                cache.getRenameCache().get(
+                        "my schemas/../Foo.yaml#/components/schemas/Foo"),
+                "UnencodedSpace");
+        assertEquals(
+                cache.getRenamedRef("file:/tmp/Foo.yaml#/components/schemas/Foo"),
+                "FileFoo");
+        assertEquals(
+                cache.getRenamedRef("/tmp/Foo.yaml#/components/schemas/Foo"),
+                "AbsoluteFoo");
+        assertEquals(
+                cache.getRenamedRef(
+                        "https://example.com/Foo.yaml?version=1#/components/schemas/Foo"),
+                "HttpFoo");
+    }
 }

modules/swagger-parser-v3/src/test/resources/oas3.fetched/components/schemas/Error.yaml

@@ -0,0 +1,13 @@
+description: Error responses are included with 4xx and 5xx HTTP responses from the
+  API service. Either "error" or "errors" will be set.
+properties:
+  error:
+    description: A description of the error that caused the request to fail.
+    type: string
+  errors:
+    description: A list of errors that contributed to the request failing.
+    items:
+      description: An error message that contributed to the request failing.
+      type: string
+    type: array
+type: object

modules/swagger-parser-v3/src/test/resources/oas3.fetched/components/schemas/Event.yaml

@@ -0,0 +1,26 @@
+properties:
+  body:
+    type: string
+  created_at:
+    format: date-time
+    type: string
+  href:
+    type: string
+  id:
+    format: uuid
+    type: string
+  interpolated:
+    type: string
+  relationships:
+    items:
+      $ref: './Href.yaml'
+    type: array
+  state:
+    type: string
+  type:
+    type: string
+  modified_by:
+    type: object
+  ip:
+    type: string
+type: object

modules/swagger-parser-v3/src/test/resources/oas3.fetched/components/schemas/EventList.yaml

@@ -0,0 +1,8 @@
+properties:
+  events:
+    items:
+      $ref: './Event.yaml'
+    type: array
+  meta:
+    $ref: './Meta.yaml'
+type: object

modules/swagger-parser-v3/src/test/resources/oas3.fetched/components/schemas/Href.yaml

@@ -0,0 +1,6 @@
+properties:
+  href:
+    type: string
+required:
+- href
+type: object

modules/swagger-parser-v3/src/test/resources/oas3.fetched/components/schemas/Meta.yaml

@@ -0,0 +1,18 @@
+properties:
+  first:
+    $ref: './Href.yaml'
+  last:
+    $ref: './Href.yaml'
+  next:
+    $ref: './Href.yaml'
+  previous:
+    $ref: './Href.yaml'
+  self:
+    $ref: './Href.yaml'
+  total:
+    type: integer
+  current_page:
+    type: integer
+  last_page:
+    type: integer
+type: object