protoc-gen-openapi: path parameters are repeated in the body when using `body: "*"`

[istvan-hevele]

When a request has both path parameters and body: "*", the path parameters are being repeated in the body in the resulting OpenAPI spec. Note how the following example has name both in parameters and requestBody in the generated openapi spec.

protobuf:

service LibraryService {
  rpc MergeShelves(MergeShelvesRequest) return (Shelf) {
    option (google.api.http) = {
      post: "/v1/{name=shelves/*}:merge"
      body: "*"
    };
  }
}

// Describes the shelf being removed (other_shelf_name) and updated
// (name) in this merge.
message MergeShelvesRequest {
  // The name of the shelf we're adding books to.
  string name = 1 [
    (google.api.field_behavior) = REQUIRED,
    (google.api.resource_reference).type = "Shelf"
  ];

  // The name of the shelf we're removing books from and deleting.
  string other_shelf_name = 2 [
    (google.api.field_behavior) = REQUIRED,
    (google.api.resource_reference).type = "Shelf"
  ];
}

openapi spec:

paths:
    /v1/shelves/{shelf}:merge:
        post:
            tags:
                - LibraryService
            description: |-
                Merges two shelves by adding all books from the shelf named
                 `other_shelf_name` to shelf `name`, and deletes
                 `other_shelf_name`. Returns the updated shelf.
                 The book ids of the moved books may not be the same as the original books.
                 Returns NOT_FOUND if either shelf does not exist.
                 This call is a no-op if the specified shelves are the same.
            operationId: LibraryService_MergeShelves
            parameters:
                - name: shelf
                  in: path
                  description: The shelf id.
                  required: true
                  schema:
                    type: string
            requestBody:
                content:
                    application/json:
                        schema:
                            $ref: '#/components/schemas/MergeShelvesRequest'
                required: true
            responses:
                "200":
                    description: OK
                    content:
                        application/json:
                            schema:
                                $ref: '#/components/schemas/Shelf'
components:
    schemas:
        MergeShelvesRequest:
            required:
                - name
                - other_shelf_name
            type: object
            properties:
                name:
                    type: string
                    description: The name of the shelf we're adding books to.
                other_shelf_name:
                    type: string
                    description: The name of the shelf we're removing books from and deleting.
            description: Describes the shelf being removed (other_shelf_name) and updated (name) in this merge.

According to use_wildcard_in_body, every field not bound by the path template should be mapped to the request body. So fields that are bound by the path template shouldn't be in the request body.

Proposal: In these cases, create a new schema named {method_name}RequestBody which doesn't contain the fields bound by the path:

paths:
    /v1/shelves/{shelf}:merge:
        post:
            tags:
                - LibraryService
            description: |-
                Merges two shelves by adding all books from the shelf named
                 `other_shelf_name` to shelf `name`, and deletes
                 `other_shelf_name`. Returns the updated shelf.
                 The book ids of the moved books may not be the same as the original books.

                 Returns NOT_FOUND if either shelf does not exist.
                 This call is a no-op if the specified shelves are the same.
            operationId: LibraryService_MergeShelves
            parameters:
                - name: shelf
                  in: path
                  description: The shelf id.
                  required: true
                  schema:
                    type: string
            requestBody:
                content:
                    application/json:
                        schema:
                            $ref: '#/components/schemas/MergeShelvesRequestBody'
                required: true
            responses:
                "200":
                    description: OK
                    content:
                        application/json:
                            schema:
                                $ref: '#/components/schemas/Shelf'
components:
    schemas:
        MergeShelvesRequestBody:
            required:
                - other_shelf_name
            type: object
            properties:
                other_shelf_name:
                    type: string
                    description: The name of the shelf we're removing books from and deleting.
            description: Describes the shelf being removed (other_shelf_name) and updated (name) in this merge.

There's a PR on my fork that implements this proposal: https://github.com/istvan-hevele/gnostic/pull/1

[istvan-hevele]

cc @timburks

[lebogg]

I also find that quite confusing. Is there an opinion on this from the developers? Thanks!

[vallahaye]

Hello everyone,

I also encountered this problem when switching from protoc-gen-openapiv2 to gnostic. Fully mapped bodies are the core element used to define custom methods within Google's AIP guidelines.

Would it be possible to reprioritize this issue and evaluate istvan-hevele's PR in this regard?

Thank you

[jagobagascon]

I came across this exact problem and added my own PR to fix it: https://github.com/google/gnostic/pull/444