Merge pull request #826 from swagger-api/fix-issue-811-OAS3

gracekarina · web-flow · commit 4c8aa7bb448e · 2018-09-04T17:41:08.000-05:00
Fix issue #811 OAS3 - RefResponse -> RefSchema external Resolving
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
@@ -9,6 +9,7 @@
 import io.swagger.v3.oas.models.links.Link;
 import io.swagger.v3.oas.models.media.ArraySchema;
 import io.swagger.v3.oas.models.media.ComposedSchema;
+import io.swagger.v3.oas.models.media.MediaType;
 import io.swagger.v3.oas.models.media.Schema;
 import io.swagger.v3.oas.models.parameters.Parameter;
 import io.swagger.v3.oas.models.parameters.RequestBody;
@@ -199,62 +200,33 @@ private void processProperties(Map<String,Schema> subProps, String file) {
         }
     }
 
-    public String processRefToExternalResponse(String $ref, RefFormat refFormat) {
-        String renamedRef = cache.getRenamedRef($ref);
-        if(renamedRef != null) {
-            return renamedRef;
-        }
+    public void processRefToExternalResponse(String $ref, RefFormat refFormat) {
 
         final ApiResponse response = cache.loadRef($ref, refFormat, ApiResponse.class);
 
-        if(response == null) {
-            // stop!  There's a problem.  retain the original ref
-            LOGGER.warn("unable to load model reference from `" + $ref + "`.  It may not be available " +
-                    "or the reference isn't a valid model schema");
-            return $ref;
-        }
-        String newRef;
-
-        if (openAPI.getComponents() == null) {
-            openAPI.setComponents(new Components());
-        }
-        Map<String, ApiResponse> responses = openAPI.getComponents().getResponses();
-
-        if (responses == null) {
-            responses = new LinkedHashMap<>();
-        }
-
-        final String possiblyConflictingDefinitionName = computeDefinitionName($ref);
-
-        ApiResponse existingResponse = responses.get(possiblyConflictingDefinitionName);
-
-        if (existingResponse != null) {
-            LOGGER.debug("A model for " + existingResponse + " already exists");
-            if(existingResponse.get$ref() != null) {
-                // use the new model
-                existingResponse = null;
-            }
-        }
-        newRef = possiblyConflictingDefinitionName;
-        cache.putRenamedRef($ref, newRef);
-
-        if(existingResponse == null) {
-            // don't overwrite existing model reference
-            openAPI.getComponents().addResponses(newRef, response);
-            cache.addReferencedKey(newRef);
-
-            String file = $ref.split("#/")[0];
-            if (response.get$ref() != null) {
-                RefFormat format = computeRefFormat(response.get$ref());
-                if (isAnExternalRefFormat(format)) {
-                    response.set$ref(processRefToExternalResponse(response.get$ref(), format));
-                } else {
-                    processRefToExternalResponse(file + response.get$ref(), RefFormat.RELATIVE);
+        if(response != null) {
+
+        String file = $ref.split("#/")[0];
+
+            Schema schema = null;
+            if(response.getContent() != null){
+                Map<String, MediaType> content = response.getContent();
+                for( String mediaName : content.keySet()) {
+                    MediaType mediaType = content.get(mediaName);
+                    if(mediaType.getSchema()!= null) {
+                        schema = mediaType.getSchema();
+                        if (schema.get$ref() != null) {
+                            RefFormat ref = computeRefFormat(schema.get$ref());
+                            if (isAnExternalRefFormat(ref)) {
+                               processRefSchema(schema, $ref);
+                            } else {
+                                processRefToExternalSchema(file + schema.get$ref(), RefFormat.RELATIVE);
+                            }
+                        }
+                    }
                 }
             }
         }
-
-        return newRef;
     }
 
     public String processRefToExternalRequestBody(String $ref, RefFormat refFormat) {
diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/OperationProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/OperationProcessor.java
@@ -51,7 +51,10 @@ public void processOperation(Operation operation) {
             for (String responseCode : responses.keySet()) {
                 ApiResponse response = responses.get(responseCode);
                 if(response != null) {
-                    if (response.get$ref() != null){
+                    if (response.get$ref() != null) {
+
+                        responseProcessor.processResponse(response);
+
                         RefFormat refFormat = computeRefFormat(response.get$ref());
                         ApiResponse resolvedResponse = cache.loadRef(response.get$ref(), refFormat, ApiResponse.class);
 
@@ -60,7 +63,9 @@ public void processOperation(Operation operation) {
                             responses.put(responseCode, resolvedResponse);
                         }
                     }
+
                     responseProcessor.processResponse(response);
+
                 }
             }
         }
diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ResponseProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ResponseProcessor.java
@@ -37,6 +37,7 @@ public void processResponse(ApiResponse response) {
         if (response.get$ref() != null){
             processReferenceResponse(response);
         }
+
         Schema schema = null;
         if(response.getContent() != null){
             Map<String,MediaType> content = response.getContent();
@@ -72,11 +73,7 @@ public void processReferenceResponse(ApiResponse response){
         RefFormat refFormat = computeRefFormat(response.get$ref());
         String $ref = response.get$ref();
         if (isAnExternalRefFormat(refFormat)){
-            final String newRef = externalRefProcessor.processRefToExternalResponse($ref, refFormat);
-
-            if (newRef != null) {
-                response.set$ref(newRef);
-            }
+            externalRefProcessor.processRefToExternalResponse($ref, refFormat);
         }
     }
 }
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/OperationProcessorTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/OperationProcessorTest.java
@@ -49,10 +49,12 @@ public void testProcessOperation(@Injectable final List<Parameter> inputParamete
         operation.responses(new ApiResponses().addApiResponse("200", refResponse));
         operation.getResponses().addApiResponse("400", incomingResponse);
 
+
         new Expectations() {{
             new ParameterProcessor(cache, openAPI);
             times = 1;
             result = parameterProcessor;
+
             new ResponseProcessor(cache, openAPI);
             times = 1;
             result = responseProcessor;
@@ -61,6 +63,9 @@ public void testProcessOperation(@Injectable final List<Parameter> inputParamete
             times = 1;
             result = outputParameterList;
 
+            responseProcessor.processResponse(refResponse);
+            times = 1;
+
             RefUtils.computeRefFormat(ref);
             times = 1;
 
@@ -78,10 +83,14 @@ public void testProcessOperation(@Injectable final List<Parameter> inputParamete
             times = 1;
             responseProcessor.processResponse(resolvedResponse);
             times = 1;
+
         }};
 
+
+
         new OperationProcessor(cache, openAPI).processOperation(operation);
 
+
         new FullVerifications() {{}};
 
         assertEquals(operation.getResponses().get("200"), resolvedResponse);
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java
@@ -252,11 +252,6 @@ public void componentsResolver() throws Exception {
 
         Map<String, ApiResponse> responses = openAPI.getComponents().getResponses();
 
-        //remote url response
-        ApiResponse found = responses.get("Found");
-        assertEquals(openAPI.getComponents().getResponses().get("RemoteResponse").getDescription(),"Remote Description");
-        assertEquals(found.get$ref(), "#/components/responses/RemoteResponse");
-
         //internal response headers
         ApiResponse illegalInput = responses.get("IllegalInput");
         assertEquals(illegalInput.getHeaders().get("X-Ref-Limit-Limit").get$ref(),"#/components/headers/X-Rate-Limit-Reset");
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
@@ -56,6 +56,30 @@ public class OpenAPIV3ParserTest {
     protected int serverPort = getDynamicPort();
     protected WireMockServer wireMockServer;
 
+    @Test
+    public void testIssue811_RefSchema_ToRefSchema() {
+        ParseOptions options = new ParseOptions();
+        options.setResolve(true);
+        final OpenAPI openAPI = new OpenAPIV3Parser().readLocation("oapi-reference-test2/index.yaml", null, options).getOpenAPI();
+
+        Assert.assertNotNull(openAPI);
+        Assert.assertEquals(openAPI.getPaths().get("/").getGet().getResponses().get("200").getContent().get("application/json").getSchema().get$ref(),"#/components/schemas/schema-with-reference");
+
+
+    }
+
+    @Test
+    public void testIssue811() {
+        ParseOptions options = new ParseOptions();
+        options.setResolve(true);
+        final OpenAPI openAPI = new OpenAPIV3Parser().readLocation("oapi-reference-test/index.yaml", null, options).getOpenAPI();
+
+        Assert.assertNotNull(openAPI);
+
+        Assert.assertEquals(openAPI.getPaths().get("/").getGet().getResponses().get("200").getContent().get("application/json").getSchema().get$ref(),"#/components/schemas/schema-with-reference");
+
+    }
+
     @Test
     public void testIssue719() {
         final OpenAPI openAPI = new OpenAPIV3Parser().readLocation("extensions-responses.yaml", null, new ParseOptions()).getOpenAPI();
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/util/OpenAPIDeserializerTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/util/OpenAPIDeserializerTest.java
@@ -57,6 +57,7 @@
 
 public class OpenAPIDeserializerTest {
 
+
     @Test
     public void testEmptyDefinitions() throws Exception {
         String yaml = "openapi: 3.0.0\n" +
diff --git a/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/components/referent.yaml b/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/components/referent.yaml
@@ -0,0 +1,2 @@
+---
+type: object
diff --git a/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/components/response-with-reference.yaml b/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/components/response-with-reference.yaml
@@ -0,0 +1,6 @@
+---
+description: Response with schema containing reference
+content:
+  application/json:
+    schema:
+      $ref: "./schema-with-reference.yaml"
diff --git a/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/components/schema-with-reference.yaml b/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/components/schema-with-reference.yaml
@@ -0,0 +1,7 @@
+---
+type: object
+properties:
+  reference_array:
+    type: array
+    items:
+      $ref: "./referent.yaml"
diff --git a/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/index.yaml b/modules/swagger-parser-v3/src/test/resources/oapi-reference-test/index.yaml
@@ -0,0 +1,22 @@
+---
+openapi: "3.0.0"
+externalDocs:
+  description: Test
+  url: http://test.com/test
+info:
+  version: 0.0.1
+  title: Test
+  license:
+    name: Test
+    url: http://test.com/test
+servers:
+  - url: http://test.com/test
+paths:
+  /:
+    get:
+      description: Test
+      summary: Test
+      operationId: getTest
+      responses:
+        200:
+          $ref: "./components/response-with-reference.yaml"
diff --git a/modules/swagger-parser-v3/src/test/resources/oapi-reference-test2/components/referent.yaml b/modules/swagger-parser-v3/src/test/resources/oapi-reference-test2/components/referent.yaml
@@ -0,0 +1,2 @@
+---
+type: object
diff --git a/modules/swagger-parser-v3/src/test/resources/oapi-reference-test2/components/schema-with-reference.yaml b/modules/swagger-parser-v3/src/test/resources/oapi-reference-test2/components/schema-with-reference.yaml
@@ -0,0 +1,7 @@
+---
+type: object
+properties:
+  reference_array:
+    type: array
+    items:
+      $ref: "./referent.yaml"
diff --git a/modules/swagger-parser-v3/src/test/resources/oapi-reference-test2/index.yaml b/modules/swagger-parser-v3/src/test/resources/oapi-reference-test2/index.yaml
@@ -0,0 +1,26 @@
+---
+openapi: "3.0.0"
+externalDocs:
+  description: Test
+  url: http://test.com/test
+info:
+  version: 0.0.1
+  title: Test
+  license:
+    name: Test
+    url: http://test.com/test
+servers:
+  - url: http://test.com/test
+paths:
+  /:
+    get:
+      description: Test
+      summary: Test
+      operationId: getTest
+      responses:
+        200:
+          description: Response with schema containing reference
+          content:
+            application/json:
+              schema:
+                $ref: "./components/schema-with-reference.yaml"
diff --git a/modules/swagger-parser-v3/src/test/resources/relative-file-references/json/paths/healthPath.json b/modules/swagger-parser-v3/src/test/resources/relative-file-references/json/paths/healthPath.json
@@ -1,29 +1,29 @@
 {
 	"parameters": [
 		{
-			"$ref": "./parameters/params.json#/param1"
+			"$ref": "../parameters/params.json#/param1"
 		},
 		{
-			"$ref": "./parameters/params.json#/param2"
+			"$ref": "../parameters/params.json#/param2"
 		}
 	],
 	"get": {
 		"summary": "Returns server health information",
 		"operationId": "getHealth",
 		"parameters": [
 			{
-				"$ref": "./parameters/params.json#/param3"
+				"$ref": "../parameters/params.json#/param3"
 			},
 			{
-				"$ref": "./parameters/params.json#/param4"
+				"$ref": "../parameters/params.json#/param4"
 			}
 		],
 		"requestBody": {
 			"required": true,
 			"content": {
 				"application/json": {
 					"schema": {
-						"$ref": "./models/health.json"
+						"$ref": "../models/health.json"
 					}
 				}
 			}
@@ -34,16 +34,16 @@
 				"content": {
 					"application/json": {
 						"schema": {
-							"$ref": "./models/health.json"
+							"$ref": "../models/health.json"
 						}
 					}
 				}
 			},
 			"400": {
-				"$ref": "./responses/errorResponses.json#/bad-request"
+				"$ref": "../responses/errorResponses.json#/bad-request"
 			},
 			"500": {
-				"$ref": "./responses/errorResponses.json#/internal-server-error"
+				"$ref": "../responses/errorResponses.json#/internal-server-error"
 			}
 		}
 	}
diff --git a/modules/swagger-parser-v3/src/test/resources/relative-file-references/json/responses/errorResponses.json b/modules/swagger-parser-v3/src/test/resources/relative-file-references/json/responses/errorResponses.json
@@ -4,7 +4,7 @@
 		"content": {
 			"*/*": {
 				"schema": {
-					"$ref": "./models/error.json"
+					"$ref": "../models/error.json"
 				}
 			}
 		}
@@ -14,7 +14,7 @@
 		"content": {
 			"*/*": {
 				"schema": {
-					"$ref": "./models/error.json"
+					"$ref": "../models/error.json"
 				}
 			}
 		}
diff --git a/modules/swagger-parser-v3/src/test/resources/relative-file-references/yaml/paths/healthPath.yaml b/modules/swagger-parser-v3/src/test/resources/relative-file-references/yaml/paths/healthPath.yaml
diff --git a/modules/swagger-parser-v3/src/test/resources/relative-file-references/yaml/responses/errorResponses.yaml b/modules/swagger-parser-v3/src/test/resources/relative-file-references/yaml/responses/errorResponses.yaml

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,10 @@ public void processOperation(Operation operation) {`
`51`	`51`	`for (String responseCode : responses.keySet()) {`
`52`	`52`	`ApiResponse response = responses.get(responseCode);`
`53`	`53`	`if(response != null) {`
`54`		`- if (response.get$ref() != null){`
	`54`	`+ if (response.get$ref() != null) {`
	`55`	`+`
	`56`	`+ responseProcessor.processResponse(response);`
	`57`	`+`
`55`	`58`	`RefFormat refFormat = computeRefFormat(response.get$ref());`
`56`	`59`	`ApiResponse resolvedResponse = cache.loadRef(response.get$ref(), refFormat, ApiResponse.class);`
`57`	`60`
`@@ -60,7 +63,9 @@ public void processOperation(Operation operation) {`
`60`	`63`	`responses.put(responseCode, resolvedResponse);`
`61`	`64`	`}`
`62`	`65`	`}`
	`66`	`+`
`63`	`67`	`responseProcessor.processResponse(response);`
	`68`	`+`
`64`	`69`	`}`
`65`	`70`	`}`
`66`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ public void processResponse(ApiResponse response) {`
`37`	`37`	`if (response.get$ref() != null){`
`38`	`38`	`processReferenceResponse(response);`
`39`	`39`	`}`
	`40`	`+`
`40`	`41`	`Schema schema = null;`
`41`	`42`	`if(response.getContent() != null){`
`42`	`43`	`Map<String,MediaType> content = response.getContent();`
`@@ -72,11 +73,7 @@ public void processReferenceResponse(ApiResponse response){`
`72`	`73`	`RefFormat refFormat = computeRefFormat(response.get$ref());`
`73`	`74`	`String $ref = response.get$ref();`
`74`	`75`	`if (isAnExternalRefFormat(refFormat)){`
`75`		`- final String newRef = externalRefProcessor.processRefToExternalResponse($ref, refFormat);`
`76`		`-`
`77`		`- if (newRef != null) {`
`78`		`- response.set$ref(newRef);`
`79`		`- }`
	`76`	`+ externalRefProcessor.processRefToExternalResponse($ref, refFormat);`
`80`	`77`	`}`
`81`	`78`	`}`
`82`	`79`	`}`