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

protoc-gen-swagger: support JSON Schema Validation properties and add openapiv2_field option #687

Merged

Conversation

co3k
Copy link
Contributor

@co3k co3k commented Jun 28, 2018

This change supports JSON Schema Validation properties.
It allows you to specify some validation rules in .proto file like this:

message GeoCoordinate {
  option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = {
    json_schema: {
      required: ["latitude", "longitude"]
    }
  };

  string latitude = 1;
  string longitude = 2;
}

And also this supports openapiv2_field field option.
It allows you to specifiy additional JSON schema setting like this:

string latitude = 1 [(grpc.gateway.protoc_gen_swagger.options.openapiv2_field) = {pattern: "^[\-]?[0-9]{0,2}\.[0-9]+$"}];
string longitude = 2 [(grpc.gateway.protoc_gen_swagger.options.openapiv2_field) = {pattern: "^[\-]?[0-9]{0,3}\.[0-9]+$"}];

The both of grpc.gateway.protoc_gen_swagger.options.openapiv2_schema and grpc.gateway.protoc_gen_swagger.options.openapiv2_field now supports the following validation properties.

  • multiple_of
  • maximum
  • exclusive_maximum
  • minimum
  • exclusive_minimum
  • max_length
  • min_length
  • pattern
  • max_items
  • min_items
  • unique_items
  • max_properties
  • min_properties
  • required

Please see also JSON Schema Validation spec to understand about these properties.

@co3k co3k changed the title Add support to configure JSON Schema in openapiv2_field_json_schema field option and JSONSchema message type supports JSON Schema Validation properties protoc-gen-swagger: Add ability to JSON Schema Validation properties and openapiv2_field_json_schema field option Jul 24, 2018
@co3k co3k force-pushed the support_json_schema_field_tag branch 2 times, most recently from 5c5945d to 95a51b4 Compare July 24, 2018 08:48
@co3k co3k changed the title protoc-gen-swagger: Add ability to JSON Schema Validation properties and openapiv2_field_json_schema field option protoc-gen-swagger: support JSON Schema Validation properties and add openapiv2_field_json_schema field option Jul 24, 2018
@co3k co3k force-pushed the support_json_schema_field_tag branch from 95a51b4 to f1640f4 Compare July 24, 2018 09:02
@codecov-io
Copy link

codecov-io commented Jul 24, 2018

Codecov Report

Merging #687 into master will decrease coverage by 0.13%.
The diff coverage is 39.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #687      +/-   ##
==========================================
- Coverage    56.3%   56.16%   -0.14%     
==========================================
  Files          30       30              
  Lines        3062     3112      +50     
==========================================
+ Hits         1724     1748      +24     
- Misses       1167     1193      +26     
  Partials      171      171
Impacted Files Coverage Δ
protoc-gen-swagger/genswagger/types.go 19.04% <ø> (ø) ⬆️
protoc-gen-swagger/genswagger/template.go 38.75% <39.06%> (+0.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7951e5b...757aaa0. Read the comment docs.

@co3k
Copy link
Contributor Author

co3k commented Jul 29, 2018

Hi, I really need these changes for my works but there aren't any reactions for a month.
What should I do to get any feedbacks about this pull request?

@johanbrandhorst
Copy link
Collaborator

This is really interesting, but to make things easier for us maintainers, could you please include a description of the changes? Include any external links as well.

@co3k
Copy link
Contributor Author

co3k commented Jul 29, 2018

@johanbrandhorst Thanks for your advice! I've added descriptions to this pull request. If it's OK, I will also change the commit message to follow it.

@johanbrandhorst
Copy link
Collaborator

@co3k that's great thanks! I think this looks good but I would like the input of @ivucica as well before we merge.

Copy link
Collaborator

@ivucica ivucica left a comment

Choose a reason for hiding this comment

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

Can't think of a better way to go about it. I would just rename the new option, but otherwise this seems to be it.

//
// All IDs are the same, as assigned. It is okay that they are the same, as they extend
// different descriptor messages.
JSONSchema openapiv2_field_json_schema = 1042;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we name this just openapiv2_field similar to what was done with other options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed it in 48e32c9

@co3k co3k changed the title protoc-gen-swagger: support JSON Schema Validation properties and add openapiv2_field_json_schema field option protoc-gen-swagger: support JSON Schema Validation properties and add openapiv2_field option Jul 31, 2018
@co3k co3k force-pushed the support_json_schema_field_tag branch 2 times, most recently from 390572b to 48e32c9 Compare July 31, 2018 04:42
@co3k
Copy link
Contributor Author

co3k commented Jul 31, 2018

I wonder why the Go: 1.10.x GATEWAY_PLUGIN_FLAGS= Travis CI build gets failed after my rebasing. I've tried make realclean examples but this problem isn't solved.

@johanbrandhorst
Copy link
Collaborator

@co3k There's definitely something weird up with our CI pipeline at the moment, I'm trying to work out with the other maintainers what we need to do, sorry for the inconvenience. We'll get this change in as soon as possible!

@co3k co3k force-pushed the support_json_schema_field_tag branch from 48e32c9 to 2f1899b Compare August 6, 2018 06:27
… openapiv2_field option

This change supports [JSON Schema Validation](https://tools.ietf.org/html/draft-fge-json-schema-validation-00) properties.
It allows you to specify some validation rules in `openapiv2_schema` option and `openapiv2_field` option of your .proto files.
@co3k co3k force-pushed the support_json_schema_field_tag branch from 2f1899b to 757aaa0 Compare August 6, 2018 15:18
@co3k
Copy link
Contributor Author

co3k commented Aug 6, 2018

Oh, I'm sorry...I've forgotten to do go get -u github.com/golang/protobuf/protoc-gen-go before my local building.

Finally, Travis CI builds have been passed.

@johanbrandhorst
Copy link
Collaborator

go get -u github.com/golang/protobuf/protoc-gen-go

You absolutely should not be expected to do this. Install it from the vendor folder: go install ./vendor/github.com/golang/protobuf/protoc-gen-go

@johanbrandhorst johanbrandhorst merged commit 221ef34 into grpc-ecosystem:master Aug 6, 2018
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

@achew22
Copy link
Collaborator

achew22 commented Aug 6, 2018

@johanbrandhorst would it be possible to have the Makefile build protoc-gen-go from the vendor directory and use it instead?

@johanbrandhorst
Copy link
Collaborator

@achew22 This was part of the change I made already:

go install ./vendor/$(GO_PLUGIN_PKG)

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

Successfully merging this pull request may close these issues.

6 participants