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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Items support on OpenAPI V2 Responses #3026

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

krak3n
Copy link
Contributor

@krak3n krak3n commented Nov 23, 2022

Feature 馃殌

Adds support for array items on Open API V2 response objects. This is currently not supported when disable_default_responses is true.

This allows users to define array response objects, for example:

message Foo {
  string bar = 1;
}

message ListFoosResponse {
  repeated Foo foos = 1;
}

service FooService {
  rpc ListFoos(google.protobuf.Empty) returns (ListFoosResponse) {
    option (google.api.http) = {
      get: "/foo";
      response_body: "foos"; // This will not be handled when disable_default_responses=true
    };

    option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = {
      responses: {
        key: "200",
        value: {
          description: "List Foos"
          schema: {
            json_schema: {
              type: ARRAY;
              items: {ref: ".some.package.v1.Foo"};
            }
          }
        };
      };
    };
  };
};

This will generate the following yaml when disable_default_responses=true

  /foo:
    get:
      summary: List Foos.
      operationId: ListFoos
      responses:
        "200":
          description: List Foos
          schema:
            type: array
            items:
              $ref: '#/definitions/some.package.v1.Foo'

See examples here on the OpenAPI V2 spec page.

TODO

  • Documentation
  • Test Case

@krak3n
Copy link
Contributor Author

krak3n commented Nov 23, 2022

Before I went any further adding tests / docs I wanted to get your thoughts on this one @johanbrandhorst 馃槃

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Chris, thanks for the PR! I understand we already allow users to specify custom responses, but this now adds the ability to specify a custom response where the outermost type is an array, is that correct? If so, this isn't so much about how this feature interacts with the disable_default_responses feature, but rather just about adding this functionality, correct? If so, should we rewrite the proto file comment to focus more on that?

// };
// };
// ```
JSONSchema items = 50;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we shouldn't just make this 19 and hoist it up to the comment above?

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.

None yet

3 participants