protoc-gen-openapiv2: add example for AIP-133

[jonathaningram]

Originally opened against v1: #1514

This example illustrates that the book_id field is not converted into a query string parameter in the generated swagger.json file.

References to other Issues or PRs

This PR contains an example required for a fix for #559.

Generally speaking, the main issue is that for requests with a body, non-body fields need to be added as query parameters. I think this is the same conclusion that @wotzhs came to in their comment at #559 (comment).

Have you read the Contributing Guidelines?

Yes.

Brief description of what is fixed or changed

I added an example to examples/internal/proto/examplepb/a_bit_of_everything.proto that is very similar to the example in https://google.aip.dev/133#user-specified-ids that allows users to provide their own ID. The user-specified ID needs to become a query string parameter per aip-dev/google.aip.dev#542 (comment).

The gateway generator is fine, but the Swagger generator does not generate query string parameters when there is a body on the binding.

The generated parameters is currently:

{
    "parameters": [
          {
            "name": "parent",
            "description": "The publisher in which to create the book.\n\nFormat: `publishers/{publisher}`\n\nExample: `publishers/1257894000000000000`",
            "in": "path",
            "required": true,
            "type": "string"
          },
          {
            "name": "body",
            "description": "The book to create.",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/examplepbBook"
            }
          }
    ],
}

Notice that there is no mention of the book_id. For clients generated off the swagger.json there is literally no way for them to know they need to provide a book_id.

Thus parameters should include this:

{
  "name": "book_id",
  "description": "The ID to use for the book.\n\nThis must start with an alphanumeric character",
  "in": "query",
  "required": true,
  "type": "string"
}

Note: this is correlated by the reply on my AIP issue here: aip-dev/google.aip.dev#542 (comment)

Other comments

I started to fix this locally and I have a rough idea of what needs doing, but I wanted to start the PR with a broken example first and I will attempt to fix the issue using this as a base.

TODO

• fix https://app.circleci.com/pipelines/github/grpc-ecosystem/grpc-gateway/993/workflows/f7328257-19fe-436c-8cd4-8f2274bcb535/jobs/13566/steps (flaky or me?)
• add a Go test for this behavior—it does appear to be tested implicitly, but a Go test is probably required. Haven't looked at it properly yet. At a glance none of the tests used a Binding which I need to test the changed behavior.

Thanks for reading :)

[jonathaningram]

@johanbrandhorst hey! I added two TODO tasks to the original comment. Before I press on with those do you mind taking a peek at my changes? Some things:

1. Is the failing timeout test flakiness or me?
2. You'll see lots of changes that don't appear related to my AIP issue. If you inspect some of the things in examples/internal/proto/examplepb/a_bit_of_everything.proto you'll see that the additional fields were never mapped to query parameters (as they should have been, I believe). An example is CheckPostQueryParams. It maps the body to single_nested. All of the sibling fields were never mapped to query parameters—in my change you see they are now mapped.
3. Does this look like it's on the right path?

[johanbrandhorst]

1. Is the failing timeout test flakiness or me?

I reran the test and it looks like it passed, so that test is clearly flaky. I've raised #1527 to fix it.

2. You'll see lots of changes that don't appear related to my AIP issue. If you inspect some of the things in examples/internal/proto/examplepb/a_bit_of_everything.proto you'll see that the additional fields were never mapped to query parameters (as they should have been, I believe). An example is `CheckPostQueryParams`. It maps the body to `single_nested`. All of the sibling fields were never mapped to query parameters—in my change you see they are now mapped.

This is great! Clearly this was never working correctly.

3. Does this look like it's on the right path?

I think it looks great so far.

[jonathaningram]

Cool thanks @johanbrandhorst. I'll add that Go test next and then see how it looks, that might be all that's needed before this is ready for review.

[jonathaningram]

@johanbrandhorst and friends this is ready for review.

[johanbrandhorst]

This looks great! Thank you so much.

[johanbrandhorst]

Thanks for your contribution! How much work would it be to backport this to v1?

[jonathaningram]

@johanbrandhorst you're welcome! It should be easy, I'll get on to that.