Repository: swagger-api/swagger-parser
Pull request #1733 (2022-05-24 08:13:35)

Paths defined as $ref wont add parameters to each operation
## Problem

When organizing path definitions in different files like the example below, the parameters will not be passed from the top-level `/ref/test/{id}/toplevelparam` definition to the `GET` operation.

`api.yaml`:
```yaml
openapi: 3.0.0

paths:
  /ref/test/{id}/toplevelparam:
    $ref: "./test-endpoints.yaml#/paths/~1ref~1test~1{id}~1toplevelparam"
```

`test-endpoints.yaml`:
```yaml
openapi: 3.0.0

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
```

The issue is that the passing of parameters to the operations is done before parsing of the `$ref`s.

## Solution

By calling `addParametersToEachOperation` after all the `$ref`s has been processed, the parameters can be correctly added.

I have included simple tests of `PathProcessor` for this specific case. I am not sure if I have structured the tests correctly or if they make more sense to include in `OpenAPIResolverTest`


Patch:
diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/PathsProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/PathsProcessor.java
index 370bc6a6..b2dfe866 100644
--- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/PathsProcessor.java
+++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/PathsProcessor.java
@@ -60,8 +60,6 @@ public class PathsProcessor {
         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 class PathsProcessor {
                 }
             }
 
+            addParametersToEachOperation(pathItem);
+
             //at this point we can process this path
             final List<Parameter> processedPathParameters = parameterProcessor.processParameters(pathItem.getParameters());
             pathItem.setParameters(processedPathParameters);


Test patch:
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/PathsProcessorTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/PathsProcessorTest.java
new file mode 100644
index 00000000..000ae5ad
--- /dev/null
+++ b/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));
+        }
+    }
+}
\ No newline at end of file
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 35dbf99d..82029bd9 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
@@ -2205,13 +2205,14 @@ public class OpenAPIV3ParserTest {
         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();
 
diff --git a/modules/swagger-parser-v3/src/test/resources/issue-1733/api.yaml b/modules/swagger-parser-v3/src/test/resources/issue-1733/api.yaml
new file mode 100644
index 00000000..ede55279
--- /dev/null
+++ b/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"
diff --git a/modules/swagger-parser-v3/src/test/resources/issue-1733/test-endpoints.yaml b/modules/swagger-parser-v3/src/test/resources/issue-1733/test-endpoints.yaml
new file mode 100644
index 00000000..1347703c
--- /dev/null
+++ b/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