SurfGen icon indicating copy to clipboard operation
SurfGen copied to clipboard

Handling path parameters declaration place

Open dmitryd20 opened this issue 4 years ago • 3 comments

Closes #64

Done:

  • taking path parameters from Path object instead of Operation
  • giving warning if path parameters are declared in Operation

Details:

First, let's compare two specs.

1:

paths:
      /messages/{id}:
        parameters:
          - name: id
            required: true
            in: path
            schema:
              type: string
        get:
          responses:
            default:
                description: "Все ок"
                content:
                    application/json:
                        schema:
                            type: integer

2:

paths:
      /messages/{id}:
        get:
          parameters:
            - name: id
              required: true
              in: path
              schema:
                type: string
          responses:
            default:
                description: "Все ок"
                content:
                    application/json:
                        schema:
                            type: integer

In spec 1 path parameter id is declared inside path object and this it how it should be.

In spec 2 path the parameter is declared inside operation. Previously we didn't take attention on this, but this declaration is a potential problem. If there are more then one operations for one path, the parameter should be duplicated for each operation. So now we give warning if path parameters are declared in Operation, like in spec 2.

How to check:

Specs from example above are used in ParameterTests.

Spec 2 is used in SwaggerCorrectorTests to check if SwaggerCorrector moves path parameters declared in Operation to Path

dmitryd20 avatar Nov 08 '21 10:11 dmitryd20

I'm not sure that this is correct behavior. For instance:

paths:
  /user:
    get:
      summary: Return specific user buy id
      parameters:
        - name:
           required: true
           in: path
           schema:
             type: string
    ...........
    post:
       summary: Create new user
    ...........
    delete:
       summary: Delete specific user
      parameters:
        - name:
           required: true
           in: path
           schema:
             type: string
     ...........

So I declared CRUD controller in the way when only a part of operations contains path parameter which is declared in operation.

Tbh I don't know is it useful, but I guess that we have to mange situation with global parameters as it is - I means we have to add global parameter to each operation, don't we?

LastSprint avatar Nov 09 '21 04:11 LastSprint

So I declared CRUD controller in the way when only a part of operations contains path parameter which is declared in operation.

Situation where only a part of operations in service use path parameter is complicated for generation. In your example path string is /user. It doesn't include parameter template, so it is impossible to guess, where to paste parameter value. And if path string is /user/{id}, the template for parameter id will be unused with POST operation, which leads to an error too. As I see, the only way to solve this is to declare /user and /user/{id} as different paths.

dmitryd20 avatar Nov 10 '21 08:11 dmitryd20

So I declared CRUD controller in the way when only a part of operations contains path parameter which is declared in operation.

Situation where only a part of operations in service use path parameter is complicated for generation. In your example path string is /user. It doesn't include parameter template, so it is impossible to guess, where to paste parameter value. And if path string is /user/{id}, the template for parameter id will be unused with POST operation, which leads to an error too. As I see, the only way to solve this is to declare /user and /user/{id} as different paths.

And if I will rewrite it in that way

paths:
  /user/{id}:
    get:
      summary: Return specific user buy id
      parameters:
        - name: id
           required: true
           in: path
           schema:
             type: string
    ...........
    post:
       summary: Create new user
    ...........
    delete:
       summary: Delete specific user
      parameters:
        - name: id
           required: true
           in: path
           schema:
             type: string
     ...........

Now it's possible to guess and POST doesn't have any parameter

LastSprint avatar Nov 16 '21 08:11 LastSprint