protoc-gen-openapiv2 not generating objects with repeated defined objects

[aethanol]

🐛 Bug Report

(not sure if this is actually a bug)

Updating from v2.0.1 -> 2.7.0 doesn't generate definitions for openapi objects that have a repeated other object. In this case the diff here: https://github.com/grafeas/grafeas/pull/525/files#diff-fc63bf57f2448a75b87e33e47f77ff96be847f6ce48546e973277dee7480d862L455

Is there an option to disable this behavior?

To Reproduce

(Write your steps here:)

1. repeated proto: https://github.com/grafeas/grafeas/blob/master/proto/v1beta1/grafeas.proto#L561
2. similar proto: https://github.com/grafeas/grafeas/blob/master/proto/v1beta1/grafeas.proto#L490
3. gen command: https://github.com/grafeas/grafeas/blob/master/proto/v1beta1/generate.go#L4
4. (easiest way to get this to work is to clone grafeas and make test, or go generate in the proto dir)

Expected behavior

Two openapi objects: #/definitions/v1beta1BatchCreateNotesRequest, and #/definitions/v1beta1Note.

Actual Behavior

The generated openapi simplifies the repeated object in line.

https://github.com/grafeas/grafeas/pull/525/files#diff-fc63bf57f2448a75b87e33e47f77ff96be847f6ce48546e973277dee7480d862L455

Your Environment

golang, macos

[johanbrandhorst]

Hi, thanks for your issue! I'm confused, what is the exact issue here? Is the new, inline representation not faithful to your API definitions? It looks like it just removed one level of indirection to me, is that the undesirable change?

[aethanol]

Hey @johanbrandhorst! yeah exactly, it just ends up generating some less than desirable spec. Not exactly a bug, and it still is functionally correct.

The main impact of this is in the generated api clients like: https://github.com/grafeas/client-go which end up with generated structs like Body and Body1 since they are no longer explicitly defined.

[johanbrandhorst]

Ah I see. Yes, this was a bug fix - the affected messages changed because they have path params, and where previously the OpenAPI would suggest that you can provide the parameters in either the body or the path, this is not true, as it is read from the path only. The change is here: #2078, and here is the issue: #201.

It's currently not possible to turn off, other than by removing the path parameters from your URIs, but I think it makes sense to do it this way, what do you think?

[johanbrandhorst]

This went out in v2.4.0, I've gone back and updated the release notes: https://github.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.4.0.

[aethanol]

ah I see what's going on. What is the constraint of not just using the $ref name from the defined proto message?

Defining a named schema for the body with the needed field subset and using $ref seems like it would risk schema-name collisions.

[johanbrandhorst]

The issue is that the OpenAPI spec would imply that you can specify the path parameters in both the body and the path, but it's only settable in the path.

[aethanol]

oh I see another rpc method could reference the same message and not specify the parameter in the path like:

  // Creates new notes in batch.
  rpc BatchCreateNotes(BatchCreateNotesRequest)
      returns (BatchCreateNotesResponse) {
    option (google.api.http) = {
      post: "/v1beta1/{parent=projects/*}/notes:batchCreate"
      body: "*"
    };
    option (google.api.method_signature) = "parent,notes";
  };

    // Creates new notes in batch.
    rpc TestBatchCreateNotes(BatchCreateNotesRequest)
        returns (BatchCreateNotesResponse) {
      option (google.api.http) = {
        post: "/v1beta1/test/notes:batchCreate"
        body: "*"
      };
      option (google.api.method_signature) = "notes";
    };

This would generate BatchCreateNotesRequest for the second rpc which would not work for the first.

Thanks for your help @johanbrandhorst !