Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paths defined as $ref wont add parameters to each operation #1733

Merged
merged 11 commits into from
Nov 3, 2022

Conversation

frankbille
Copy link
Contributor

@frankbille frankbille commented May 24, 2022

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:

openapi: 3.0.0

paths:
  /ref/test/{id}/toplevelparam:
    $ref: "./test-endpoints.yaml#/paths/~1ref~1test~1{id}~1toplevelparam"

test-endpoints.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 $refs.

Solution

By calling addParametersToEachOperation after all the $refs 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

@frankbille
Copy link
Contributor Author

There are two tests that fail as a consequence of the change to PathProcessor, because they expect there to be parameters on top level. When PathProcessor.addParametersToEachOperation is called it ends by setting the top level parameters to null.

I think the test needs to be changed because it would never have passed if the schema was all internal.

@gracekarina
Copy link
Contributor

thanks for this PR!

@gracekarina gracekarina merged commit f9d8633 into swagger-api:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants