Merge pull request #1733 from frankbille/master

Paths defined as $ref wont add parameters to each operation

Author: gracekarina

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

@@ -60,8 +60,6 @@ public void processPaths() {
         for (String pathStr : pathMap.keySet()) {
             PathItem pathItem = pathMap.get(pathStr);
 
-            addParametersToEachOperation(pathItem);
-
             if (pathItem.get$ref() != null) {
 
                 PathItem resolvedPath = processReferencePath(pathItem);
@@ -76,6 +74,8 @@ public void processPaths() {
                 }
             }
 
+            addParametersToEachOperation(pathItem);
+
             //at this point we can process this path
             final List<Parameter> processedPathParameters = parameterProcessor.processParameters(pathItem.getParameters());
             pathItem.setParameters(processedPathParameters);

modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/PathsProcessorTest.java

@@ -0,0 +1,57 @@
+package io.swagger.v3.parser.processors;
+
+import io.swagger.v3.oas.models.OpenAPI;
+import io.swagger.v3.oas.models.Operation;
+import io.swagger.v3.oas.models.PathItem;
+import io.swagger.v3.oas.models.PathItem.HttpMethod;
+import io.swagger.v3.parser.OpenAPIV3Parser;
+import org.testng.annotations.Test;
+
+import java.util.Map.Entry;
+
+import static java.lang.String.format;
+import static org.testng.Assert.assertFalse;
+
+public class PathsProcessorTest {
+
+    @Test
+    public void testProcessPaths_parameters_internalTopLevelDefinition() {
+        OpenAPI openAPI = new OpenAPIV3Parser().read("src/test/resources/issue-1733/api.yaml");
+
+        assertOperationsHasParameters(openAPI, "/internal/test/{id}/toplevelparam");
+    }
+
+    @Test
+    public void testProcessPaths_parameters_internalOperationLevelDefinition() {
+        OpenAPI openAPI = new OpenAPIV3Parser().read("src/test/resources/issue-1733/api.yaml");
+
+        assertOperationsHasParameters(openAPI, "/internal/test/{id}/operationlevelparam");
+    }
+
+    @Test
+    public void testProcessPaths_parameters_refTopLevelDefinition() {
+        OpenAPI openAPI = new OpenAPIV3Parser().read("src/test/resources/issue-1733/api.yaml");
+
+        assertOperationsHasParameters(openAPI, "/ref/test/{id}/toplevelparam");
+    }
+
+    @Test
+    public void testProcessPaths_parameters_refOperationLevelDefinition() {
+        OpenAPI openAPI = new OpenAPIV3Parser().read("src/test/resources/issue-1733/api.yaml");
+
+        assertOperationsHasParameters(openAPI, "/ref/test/{id}/operationlevelparam");
+    }
+
+    private void assertOperationsHasParameters(OpenAPI openAPI, String path) {
+        PathItem pathItem = openAPI.getPaths().get(path);
+
+        assertFalse(pathItem.readOperations().isEmpty(), format("Expected operations for %s but found none", path));
+
+        for (Entry<HttpMethod, Operation> operationEntry : pathItem.readOperationsMap().entrySet()) {
+            HttpMethod httpMethod = operationEntry.getKey();
+            Operation operation = operationEntry.getValue();
+
+            assertFalse(operation.getParameters() == null || operation.getParameters().isEmpty(), format("Expected parameters on %s operation for %s but found none", httpMethod, path));
+        }
+    }
+}

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

@@ -2205,13 +2205,14 @@ private OpenAPI doRelativeFileTest(String location) {
         assertEquals(path.getClass(), PathItem.class); //we successfully converted the RefPath to a Path
 
         final List<Parameter> parameters = path.getParameters();
-        assertParamDetails(parameters, 0, QueryParameter.class, "param1", "query");
-        assertParamDetails(parameters, 1, HeaderParameter.class, "param2", "header");
+        assertNull(parameters);
 
         final Operation operation = path.getGet();
         final List<Parameter> operationParams = operation.getParameters();
-        assertParamDetails(operationParams, 0, PathParameter.class, "param3", "path");
-        assertParamDetails(operationParams, 1, HeaderParameter.class, "param4", "header");
+        assertParamDetails(operationParams, 0, QueryParameter.class, "param1", "query");
+        assertParamDetails(operationParams, 1, HeaderParameter.class, "param2", "header");
+        assertParamDetails(operationParams, 2, PathParameter.class, "param3", "path");
+        assertParamDetails(operationParams, 3, HeaderParameter.class, "param4", "header");
 
         final Map<String, ApiResponse> responsesMap = operation.getResponses();
 

modules/swagger-parser-v3/src/test/resources/issue-1733/api.yaml

@@ -0,0 +1,48 @@
+openapi: 3.0.0
+
+info:
+  title: Sample API
+  version: 1.0.2
+
+paths:
+  /internal/test/{id}/toplevelparam:
+    parameters:
+      - in: path
+        name: id
+        required: true
+        schema:
+          type: string
+          enum:
+            - one
+            - two
+    get:
+      summary: Test of path params defined on top level
+      responses:
+        200:
+          description: OK
+      tags:
+        - Tests
+
+  /internal/test/{id}/operationlevelparam:
+    get:
+      summary: Test of path params defined on top level
+      parameters:
+        - in: path
+          name: id
+          required: true
+          schema:
+            type: string
+            enum:
+              - one
+              - two
+      responses:
+        200:
+          description: OK
+      tags:
+        - Tests
+
+  /ref/test/{id}/toplevelparam:
+    $ref: "./test-endpoints.yaml#/paths/~1ref~1test~1{id}~1toplevelparam"
+
+  /ref/test/{id}/operationlevelparam:
+    $ref: "./test-endpoints.yaml#/paths/~1ref~1test~1{id}~1operationlevelparam"

modules/swagger-parser-v3/src/test/resources/issue-1733/test-endpoints.yaml

@@ -0,0 +1,42 @@
+openapi: 3.0.0
+
+info:
+  title: Sample API
+  version: 1.0.2
+
+paths:
+  /ref/test/{id}/toplevelparam:
+    parameters:
+      - in: path
+        name: id
+        required: true
+        schema:
+          type: string
+          enum:
+            - one
+            - two
+    get:
+      summary: Test of path params defined on top level
+      responses:
+        200:
+          description: OK
+      tags:
+        - Tests
+
+  /ref/test/{id}/operationlevelparam:
+    get:
+      summary: Test of path params defined on top level
+      parameters:
+        - in: path
+          name: id
+          required: true
+          schema:
+            type: string
+            enum:
+              - one
+              - two
+      responses:
+        200:
+          description: OK
+      tags:
+        - Tests