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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openapiv2: Field options are not properly rendered for repeated fields #2531

Open
sumabn opened this issue Feb 9, 2022 · 11 comments
Open

openapiv2: Field options are not properly rendered for repeated fields #2531

sumabn opened this issue Feb 9, 2022 · 11 comments

Comments

@sumabn
Copy link

sumabn commented Feb 9, 2022

I need to generate swagger json using proto annotations.
generated schema should look like this

"schema": {
              "type": "object",
              "example": {
                "product_id": ["8900099100000113079", "8900099100000082373"]
              },
              "properties": {
                "product_id": {
                  "type": "array",
                  "items": {
                    "type": "string",
                    "maxLength": 19,
                   "minLength": 1,
                   "pattern": "^[0-9]+$"
                  },
                  "description": "Only digits are allowed.",
                  "title": "Provide list of ids"           
                }
              }
            }

and the annotation added was something like this

message product { 
repeated string product_id=1[(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {
                  pattern: "^[0-9]+$"
                  max_length: 19
                  min_length: 1
                  description: "Only digits are allowed."
    }];
}

But the problem is above annotation is generating schema like this

"schema": {
              "type": "object",
              "example": {
                "product_id": ["8900099100000113079", "8900099100000082373"]
              },
              "properties": {
                "product_id": {
                  "type": "array",
                  "items": {
                    "type": "string"                   
                  },
                  "description": "Only digits are allowed.",
                  "title": "Provide list of ids",
                   "maxLength": 19,
                   "minLength": 1,
                   "pattern": "^[0-9]+$"           
                }
              }
            }

which is not correct. We could not find the relevant example in a_bit_of_everything.proto.

Please help in adding the annotations.

@johanbrandhorst
Copy link
Collaborator

Hi! I went and looked at the OpenAPIv2 spec and it seems you can specify the field properties both at the top level and inside the items object: https://swagger.io/specification/v2/#items-object. I think we should probably put it inside the items if the specified field is repeated. Would you be willing to help contribute a fix for this?

The easiest way to get started would be to add a new test to https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-openapiv2/internal/genopenapi/template_test.go that would have the expected behavior. Having had a quick glance at the code, I think you'll need to make the changes here:

func renderMessageAsDefinition(msg *descriptor.Message, reg *descriptor.Registry, customRefs refMap, excludeFields []*descriptor.Field) openapiSchemaObject {
, but I could be wrong.

@johanbrandhorst johanbrandhorst changed the title how to give annotation for an repeated string openapiv2: Field options are not properly rendered for repeated fields Feb 9, 2022
@lakshkeswani
Copy link
Contributor

lakshkeswani commented Feb 21, 2022

/assign @johanbrandhorst i am willing to contribute on this kindly assign it to me

@johanbrandhorst
Copy link
Collaborator

Hi @lakshkeswani, please have at it!

@jan-sykora
Copy link

We're dealing with similar issue. Our proto file:

import "google/protobuf/field_mask.proto";

message SomeMessage {
  google.protobuf.FieldMask update_mask = 10;
}

(Here's the definition of FieldMask:)

// google/protobuf/field_mask.proto

message FieldMask {
  // The set of field mask paths.
  repeated string paths = 1;
}

is generated into openapiv2 spec as:

"parameters": [
  {
    "name": "updateMask",
    "in": "query",
    "type": "string"
  }
]

The issue is that FieldMask should be generated into OpenAPI as a "type": "array" (expected) but it is generated as a "type": "string" (actual).

@johanbrandhorst
Copy link
Collaborator

This sounds like a separate issue, could you create a separate issue and share your RPC definition too please?

@jan-sykora
Copy link

@johanbrandhorst I have created a separate issue #2580 with reproducible example.

johanbrandhorst added a commit that referenced this issue Jun 8, 2022
 (#2742)

* added product_id

* updated updateswaggerObjectFromJSONSchema to resolve issue #2531

* added test cases in schemaoffield for issue #2531

* updated openapiItemsObject type to openapiSchemaObject for issue #2531

* genrated files after adding product id

* updates

* formated file

* regenrated files

* formated the file

* Update examples/internal/proto/examplepb/a_bit_of_everything.proto

Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>

* regenrated files

Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
@wjkoh
Copy link

wjkoh commented Aug 12, 2024

This change prevents setting min_length and max_length for the repeated field itself, I believe. Can we limit the number of product IDs to 4 in OpenAPI for repeated string product_id = 1; like this?

{
    "type": "array",
    "items": {
        "type": "string"
        // not here
    }
    // here
    "maxLength": 4
}

@johanbrandhorst
Copy link
Collaborator

I'm not sure what you're asking exactly? I'm not an expert on the OpenAPI spec. Do you have an example protobuf file you're using?

@wjkoh
Copy link

wjkoh commented Aug 14, 2024

Let's use the same proto definition as an example:

message Order { 
  repeated string product_ids = 1 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {
    max_length: 10
  }];
}

Suppose we have an order containing multiple product IDs. We want to limit the maximum number of product IDs to 10. I initially thought the above annotation would achieve this, but based on the changes mentioned in the issue, it appears that this annotation actually limits the length of each product ID string (e.g., "PROD123") to 10 characters, not the total number of product IDs to 10. Is this interpretation correct?

If so, could you please advise on how to limit the number of items in a repeated field in the OpenAPI v2 specification?

@johanbrandhorst
Copy link
Collaborator

I believe your interpretation is right. To my knowledge there is no way to do what you want with OpenAPI. Perhaps you want something like https://github.com/bufbuild/protoc-gen-validate?

@wjkoh
Copy link

wjkoh commented Aug 22, 2024

I believe your interpretation is right. To my knowledge there is no way to do what you want with OpenAPI. Perhaps you want something like https://github.com/bufbuild/protoc-gen-validate?

OpenAPI supports it. It's called "minItems" and "maxItems". See https://stackoverflow.com/questions/57035988/how-to-limit-my-swagger-definition-array-to-be-either-0-or-3 for example. However, there's no way that we can generate minItems and maxItems for repeated fields using protoc_gen_openapiv2 afaik.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants