Repository: swagger-api/swagger-parser
Pull request #996 (2019-01-23 18:17:18)

Fix for issue 975
As stated in [Issue 975](https://github.com/swagger-api/swagger-parser/issues/975), the reference resolver for properties does not fully recurse. This can lead to unresolved references when parsing with `ParseOptions.setResolveFully(true)` and there are properties with nested arrays/maps/composed schemas. The included test case has 10 variants of properties, of which 7 were not resolved fully before the change.

Prior to the fix, these are the failed test cases:

```
[ERROR] Failures: 
[ERROR]   OpenAPIV3ParserTest.testIssue975_allOf:1744 expected [#/components/schemas/Image] but found [./types.yaml#/components/schemas/Image]
[ERROR]   OpenAPIV3ParserTest.testIssue975_anyOf:1737 expected [#/components/schemas/Image] but found [./types.yaml#/components/schemas/Image]
[ERROR]   OpenAPIV3ParserTest.testIssue975_array_array:1715 expected [#/components/schemas/Image] but found [./types.yaml#/components/schemas/Image]
[ERROR]   OpenAPIV3ParserTest.testIssue975_array_map:1700 expected [#/components/schemas/Image] but found [./types.yaml#/components/schemas/Image]
[ERROR]   OpenAPIV3ParserTest.testIssue975_map_array:1707 expected [#/components/schemas/Image] but found [./types.yaml#/components/schemas/Image]
[ERROR]   OpenAPIV3ParserTest.testIssue975_map_map:1723 expected [#/components/schemas/Image] but found [./types.yaml#/components/schemas/Image]
[ERROR]   OpenAPIV3ParserTest.testIssue975_oneOf:1730 expected [#/components/schemas/Image] but found [./types.yaml#/components/schemas/Image]
```

One question - the tests are structured so that each Assert is in a separate test, to make it easy to see which properties were not parsed properly.  If preferred, all tests can be merged into one test method, which is more compact. If so, I can close this PR and reissue with a PR in which the tests are replaced with this code:

```
    @Test(description = "Test that relative references are resolvable when property is a reference to a relative file.")
    @SuppressWarnings("unchecked")
    public void testIssue975() {
        ParseOptions options = new ParseOptions();
        options.setResolve(true);
        OpenAPI openAPI = new OpenAPIV3Parser().readLocation("issue-975/contract/openapi.yaml", null, options).getOpenAPI();

        assertEquals(2, openAPI.getComponents().getSchemas().size());
        assertTrue(openAPI.getComponents().getSchemas().containsKey("Image"));
        assertTrue(openAPI.getComponents().getSchemas().containsKey("MyResponse"));

        Schema myResponseSchema = openAPI.getComponents().getSchemas().get("MyResponse");
        Map<String, Schema> properties = myResponseSchema.getProperties();
        assertEquals(10, properties.size());

        assertEquals( properties.get("images").get$ref(), "#/components/schemas/Image");
        assertEquals( ((ArraySchema) properties.get("imagesArray")).getItems().get$ref(), "#/components/schemas/Image");
        assertEquals( ((Schema) properties.get("imagesMap").getAdditionalProperties()).get$ref(), "#/components/schemas/Image");
        assertEquals( ((Schema) ((ArraySchema) properties.get("imagesArrayMap")).getItems().getAdditionalProperties()).get$ref(), "#/components/schemas/Image");
        assertEquals( ((ArraySchema) properties.get("imagesMapArray").getAdditionalProperties()).getItems().get$ref(), "#/components/schemas/Image");
        assertEquals( ((ArraySchema) ((ArraySchema) properties.get("imagesArrayArray")).getItems()).getItems().get$ref(), "#/components/schemas/Image");
        assertEquals( ((Schema) ((Schema) properties.get("imagesMapMap").getAdditionalProperties()).getAdditionalProperties()).get$ref(), "#/components/schemas/Image");
        assertEquals( ((ComposedSchema) properties.get("oneOfExample")).getOneOf().get(0).get$ref(), "#/components/schemas/Image");
        assertEquals( ((ComposedSchema) properties.get("anyOfExample")).getAnyOf().get(0).get$ref(), "#/components/schemas/Image");
        assertEquals( ((ComposedSchema) properties.get("allOfExample")).getAllOf().get(0).get$ref(), "#/components/schemas/Image");
    }
```


Patch:
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
index 1481e081..685242c6 100644
--- 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
@@ -24,6 +24,7 @@ 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;
 
@@ -177,37 +178,40 @@ public final class ExternalRefProcessor {
         return newRef;
     }
 
-    private void processProperties(Map<String,Schema> subProps, String file) {
-        if (subProps != null) {
-            for (Map.Entry<String, Schema> prop : subProps.entrySet()) {
-                if (prop.getValue().get$ref() != null) {
-                    processRefSchema(prop.getValue(), file);
-                } else if (prop.getValue() instanceof ArraySchema) {
-                    ArraySchema arrayProp = (ArraySchema) prop.getValue();
-                    if (arrayProp.getItems() != null && arrayProp.getItems().get$ref() != null &&
-                            StringUtils.isNotBlank(arrayProp.getItems().get$ref())) {
-                        processRefSchema(arrayProp.getItems(), file);
-                    }
-                    if (arrayProp.getItems() != null && arrayProp.getItems().getProperties() != null ) {
-                        processProperties(arrayProp.getItems().getProperties(), file);
-                    }
-                } else if (prop.getValue().getAdditionalProperties() != null && prop.getValue().getAdditionalProperties() instanceof Schema) {
-                    Schema mapProp =  (Schema) prop.getValue().getAdditionalProperties();
-                    if (mapProp.get$ref() != null) {
-                        processRefSchema(mapProp, file);
-                    } else if (mapProp.getAdditionalProperties() instanceof ArraySchema &&
-                            ((ArraySchema) mapProp.getAdditionalProperties()).getItems()!= null &&
-                            ((ArraySchema) mapProp.getAdditionalProperties()).getItems().get$ref() != null
-                            && StringUtils.isNotBlank(((ArraySchema) mapProp.getAdditionalProperties()).getItems().get$ref())) {
-                        processRefSchema(((ArraySchema) mapProp.getAdditionalProperties()).getItems(), file);
-                    }
-                }else if (prop.getValue() instanceof ObjectSchema){
-                    ObjectSchema objProp = (ObjectSchema) prop.getValue();
-                    if(objProp.getProperties() != null ){
-                        processProperties(objProp.getProperties(),file);
-                    }
-                }
+    private void processProperty(Schema property, String file) {
+        if (property != null) {
+            if (StringUtils.isNotBlank(property.get$ref())) {
+                processRefSchema(property, file);
+            }
+            if (property.getProperties() != null) {
+                processProperties(property.getProperties(), file);
             }
+            if (property instanceof ArraySchema) {
+                processProperty(((ArraySchema) property).getItems(), file);
+            }
+            if (property.getAdditionalProperties() instanceof Schema) {
+                processProperty(((Schema) property.getAdditionalProperties()), file);
+            }
+            if (property instanceof ComposedSchema) {
+                ComposedSchema composed = (ComposedSchema) property;
+                processProperties(composed.getAllOf(), file);
+                processProperties(composed.getAnyOf(), file);
+                processProperties(composed.getOneOf(), file);
+            }
+        }
+    }
+
+    private void processProperties(Collection<Schema> properties, String file) {
+        if (properties != null) {
+            for (Schema property : properties) {
+                processProperty(property, file);
+            }
+        }
+    }
+
+    private void processProperties(Map<String, Schema> properties, String file) {
+        if (properties != null) {
+            processProperties(properties.values(), file);
         }
     }
 


Test patch:
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
index e6dfdf62..0076a536 100644
--- 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
@@ -1663,6 +1663,87 @@ public class OpenAPIV3ParserTest {
         assertEquals(requestBody, "$response.body#/slug");
     }
 
+    @SuppressWarnings("unchecked")
+    private Map<String, Schema> issue975ExtractPropertiesFromTestResource() {
+        ParseOptions options = new ParseOptions();
+        options.setResolve(true);
+        OpenAPI openAPI = new OpenAPIV3Parser().readLocation("issue-975/contract/openapi.yaml", null, options).getOpenAPI();
+        Schema myResponseSchema = openAPI.getComponents().getSchemas().get("MyResponse");
+        return myResponseSchema.getProperties();
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is a reference to a relative file.")
+    public void testIssue975_property() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        assertEquals(properties.get("images").get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is an array with a reference to a relative file.")
+    public void testIssue975_array() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        ArraySchema imagesArray = (ArraySchema) properties.get("imagesArray");
+        assertEquals(imagesArray.getItems().get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is a map with a reference to a relative file.")
+    public void testIssue975_map() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        Schema imagesMap = (Schema) properties.get("imagesMap").getAdditionalProperties();
+        assertEquals(imagesMap.get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is an array with a map with a reference to a relative file.")
+    public void testIssue975_array_map() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        ArraySchema imagesArray = (ArraySchema) properties.get("imagesArrayMap");
+        Schema imagesdMap = (Schema) imagesArray.getItems().getAdditionalProperties();
+        assertEquals(imagesdMap.get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is a map with an array with a reference to a relative file.")
+    public void testIssue975_map_array() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        ArraySchema imagesArray = (ArraySchema) properties.get("imagesMapArray").getAdditionalProperties();
+        assertEquals(imagesArray.getItems().get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is an array with an array with a reference to a relative file.")
+    public void testIssue975_array_array() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        ArraySchema imagesArray = (ArraySchema) properties.get("imagesArrayArray");
+        imagesArray = (ArraySchema) imagesArray.getItems();
+        assertEquals(imagesArray.getItems().get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is a map with a map with a reference to a relative file.")
+    public void testIssue975_map_map() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        Schema imagesMap = (Schema) properties.get("imagesMapMap").getAdditionalProperties();
+        imagesMap = (Schema) imagesMap.getAdditionalProperties();
+        assertEquals(imagesMap.get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is an oneOf a reference to a relative file.")
+    public void testIssue975_oneOf() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        ComposedSchema composed = (ComposedSchema) properties.get("oneOfExample");
+        assertEquals(composed.getOneOf().get(0).get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is an anyOf a reference to a relative file.")
+    public void testIssue975_anyOf() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        ComposedSchema composed = (ComposedSchema) properties.get("anyOfExample");
+        assertEquals(composed.getAnyOf().get(0).get$ref(), "#/components/schemas/Image");
+    }
+
+    @Test(description = "Test that relative references are resolvable when property is an allOf a reference to a relative file.")
+    public void testIssue975_allOf() {
+        Map<String, Schema> properties = issue975ExtractPropertiesFromTestResource();
+        ComposedSchema composed = (ComposedSchema) properties.get("allOfExample");
+        assertEquals(composed.getAllOf().get(0).get$ref(), "#/components/schemas/Image");
+    }
+
     private static int getDynamicPort() {
         return new Random().ints(10000, 20000).findFirst().getAsInt();
     }
diff --git a/modules/swagger-parser-v3/src/test/resources/issue-975/contract/openapi.yaml b/modules/swagger-parser-v3/src/test/resources/issue-975/contract/openapi.yaml
new file mode 100644
index 00000000..c337c4f8
--- /dev/null
+++ b/modules/swagger-parser-v3/src/test/resources/issue-975/contract/openapi.yaml
@@ -0,0 +1,15 @@
+openapi: 3.0.0
+info:
+  title: Common Image Type
+  version: 5.0.0
+paths:
+  /foo:
+    get:
+      summary: Common JSON objects
+      responses:
+        '200':
+          description: Success
+          content:
+            application/json:
+              schema:
+                $ref: '../types/responses.yaml#/components/schemas/MyResponse'
diff --git a/modules/swagger-parser-v3/src/test/resources/issue-975/types/responses.yaml b/modules/swagger-parser-v3/src/test/resources/issue-975/types/responses.yaml
new file mode 100644
index 00000000..096ddd78
--- /dev/null
+++ b/modules/swagger-parser-v3/src/test/resources/issue-975/types/responses.yaml
@@ -0,0 +1,95 @@
+openapi: 3.0.0
+info:
+  title: Common Image Type
+  version: 5.0.0
+paths:
+  /foo:
+    get:
+      summary: Common JSON objects
+      responses:
+        '204':
+          description: Success
+
+components:
+  schemas:
+    MyResponse:
+      type: object
+      properties:
+
+        # Expected property type = Image
+        images:
+          $ref: './types.yaml#/components/schemas/Image'
+
+        # Expected property type = List<Image>
+        imagesArray:
+          type: array
+          items:
+            $ref: './types.yaml#/components/schemas/Image'
+
+        # Expected property type = Map<String, Image>
+        imagesMap:
+          type: object
+          additionalProperties:
+            $ref: './types.yaml#/components/schemas/Image'
+
+        # Expected property type = List<Map<String, Image>>
+        imagesArrayMap:
+          type: array
+          items:
+            type: object
+            additionalProperties:
+              $ref: './types.yaml#/components/schemas/Image'
+
+        # Expected property type = Map<String, List<Image>>
+        imagesMapArray:
+          type: object
+          additionalProperties:
+            type: array
+            items:
+              $ref: './types.yaml#/components/schemas/Image'
+
+        # Expected property type = List<List<Image>>
+        imagesArrayArray:
+          type: array
+          items:
+            type: array
+            items:
+              $ref: './types.yaml#/components/schemas/Image'
+
+        # Expected property type = Map<String, Map<String, Image>>
+        imagesMapMap:
+          type: object
+          additionalProperties:
+            type: object
+            additionalProperties:
+              $ref: './types.yaml#/components/schemas/Image'
+
+        # Expected property type = Object
+        oneOfExample:
+          type: object
+          oneOf:
+            - $ref: './types.yaml#/components/schemas/Image'
+            - type: object
+              properties:
+                localField:
+                  type: string
+
+        # Expected property type = Object
+        anyOfExample:
+          type: object
+          anyOf:
+            - $ref: './types.yaml#/components/schemas/Image'
+            - type: object
+              properties:
+                localField:
+                  type: string
+
+        # Expected property type = Object
+        allOfExample:
+          type: object
+          allOf:
+            - $ref: './types.yaml#/components/schemas/Image'
+            - type: object
+              properties:
+                localField:
+                  type: string
diff --git a/modules/swagger-parser-v3/src/test/resources/issue-975/types/types.yaml b/modules/swagger-parser-v3/src/test/resources/issue-975/types/types.yaml
new file mode 100644
index 00000000..06689f54
--- /dev/null
+++ b/modules/swagger-parser-v3/src/test/resources/issue-975/types/types.yaml
@@ -0,0 +1,21 @@
+openapi: 3.0.0
+info:
+  title: Common Image Type
+  version: 5.0.0
+paths:
+  /foo:
+    get:
+      summary: Common JSON objects
+      responses:
+        '204':
+          description: Success
+
+components:
+  schemas:
+    Image:
+      type: object
+      properties:
+        title:
+          type: string
+        URL:
+          type: string