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

Handle google/protobuf/wrappers.proto types on protoc-gen-openapi #366

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

galihputera
Copy link
Contributor

@galihputera galihputera commented Aug 31, 2022

According to https://developers.google.com/protocol-buffers/docs/proto3#json, Wrapper Types use the same representation in JSON as the wrapped primitive type. On this MR I Modify protoc-gen-openapi code to be able to parse Wrapper Types on schema and parameters.

Also in this MR is changes to map google.protobuf.Timestamp field into string when used as URL query parameters. Previously it will be mapped into two separate fields, for example timestamp_type will mapped into timestampType.seconds and timestampType.nanos
image

Manual Testing

Unit Test

Steps

make all
go test ./...

Result

ok      github.com/google/gnostic       0.702s
?       github.com/google/gnostic/cmd/disco     [no test files]
?       github.com/google/gnostic/cmd/parse-linter-output       [no test files]
ok      github.com/google/gnostic/cmd/petstore-builder  0.468s
ok      github.com/google/gnostic/cmd/protoc-gen-jsonschema     0.745s
?       github.com/google/gnostic/cmd/protoc-gen-jsonschema/generator   [no test files]
ok      github.com/google/gnostic/cmd/protoc-gen-openapi        1.900s
?       github.com/google/gnostic/cmd/protoc-gen-openapi/generator      [no test files]
?       github.com/google/gnostic/cmd/protoc-gen-openapi/generator/wellknown    [no test files]
?       github.com/google/gnostic/cmd/report    [no test files]
?       github.com/google/gnostic/cmd/report-messages   [no test files]
?       github.com/google/gnostic/cmd/vocabulary-operations     [no test files]
ok      github.com/google/gnostic/compiler      0.110s
?       github.com/google/gnostic/conversions   [no test files]
ok      github.com/google/gnostic/discovery     0.375s
ok      github.com/google/gnostic/extensions    0.302s
?       github.com/google/gnostic/extensions/sample/generated/gnostic-x-sampleone       [no test files]
?       github.com/google/gnostic/extensions/sample/generated/gnostic-x-sampleone/proto [no test files]
?       github.com/google/gnostic/extensions/sample/generated/gnostic-x-sampletwo       [no test files]
?       github.com/google/gnostic/extensions/sample/generated/gnostic-x-sampletwo/proto [no test files]
ok      github.com/google/gnostic/generate-gnostic      1.077s
?       github.com/google/gnostic/jsonschema    [no test files]
ok      github.com/google/gnostic/jsonwriter    1.077s
?       github.com/google/gnostic/lib   [no test files]
?       github.com/google/gnostic/linters/go/gnostic-lint-descriptions  [no test files]
?       github.com/google/gnostic/linters/go/gnostic-lint-paths [no test files]
?       github.com/google/gnostic/metrics       [no test files]
?       github.com/google/gnostic/metrics/lint  [no test files]
?       github.com/google/gnostic/metrics/metrics       [no test files]
ok      github.com/google/gnostic/metrics/rules 0.779s
ok      github.com/google/gnostic/metrics/sourceinfo    0.927s
ok      github.com/google/gnostic/metrics/vocabulary    0.773s
ok      github.com/google/gnostic/openapiv2     0.596s
ok      github.com/google/gnostic/openapiv3     0.500s
?       github.com/google/gnostic/openapiv3/schema-generator    [no test files]
ok      github.com/google/gnostic/plugins       0.453s
?       github.com/google/gnostic/plugins/gnostic-analyze       [no test files]
?       github.com/google/gnostic/plugins/gnostic-analyze/statistics    [no test files]
?       github.com/google/gnostic/plugins/gnostic-analyze/summarize     [no test files]
ok      github.com/google/gnostic/plugins/gnostic-complexity    0.214s
?       github.com/google/gnostic/plugins/gnostic-linter        [no test files]
?       github.com/google/gnostic/plugins/gnostic-plugin-request        [no test files]
?       github.com/google/gnostic/plugins/gnostic-process-plugin-response       [no test files]
?       github.com/google/gnostic/plugins/gnostic-summary       [no test files]
ok      github.com/google/gnostic/plugins/gnostic-vocabulary    0.166s
?       github.com/google/gnostic/printer       [no test files]
?       github.com/google/gnostic/surface       [no test files]
?       github.com/google/gnostic/tools/format-schema   [no test files]
?       github.com/google/gnostic/tools/j2y2j   [no test files]

Check generated openapi using Swagger UI

Command

cd cmd/protoc-gen-openapi
protoc -I ../../ -I ../../third_party -I examples examples/tests/protobuftypes/message.proto --openapi_out=naming=proto:.
docker run -p 8080:8080 -e SWAGGER_JSON=/api.yaml -v $(pwd)/openapi.yaml:/api.yaml swaggerapi/swagger-ui

Result on Query String

image

Result on Schema

image

@galihputera galihputera requested a review from a team as a code owner August 31, 2022 04:11
@google-cla
Copy link

google-cla bot commented Aug 31, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@galihputera galihputera changed the title Draft: Handle google/protobuf/wrappers.proto types Handle google/protobuf/wrappers.proto types on protoc-gen-openapi Sep 1, 2022
@chaayac
Copy link

chaayac commented Sep 16, 2022

Bump on this. Can a maintainer please review?

@galihputera
Copy link
Contributor Author

up

@galihputera
Copy link
Contributor Author

I signed it!

@morphar
Copy link
Collaborator

morphar commented Dec 10, 2022

Hi @galihputera

Sorry for the slow response.
This looks good! Thank you!

Have you tested this against Envoy or something like that?
I don't have a test setup running, but I will get one up and test this with Envoy, just to verify that it converts correctly.

@same-id
Copy link

same-id commented Dec 20, 2022

Hi @galihputera

Sorry for the slow response. This looks good! Thank you!

Have you tested this against Envoy or something like that? I don't have a test setup running, but I will get one up and test this with Envoy, just to verify that it converts correctly.

Hey,

This has nothing to do with envoy.
Envoy works with the json representation of the protobuf which by design has to work that way.
(i.e. you don't need to set up envoy - just invoke "to_json" on the protobuf and you'll see that these values are translated directly into strings)

You can also see it in the official documentation of wrappers.proto:

// The JSON representation for `StringValue` is JSON string.

// The JSON representation for `StringValue` is JSON string.

@aditanase
Copy link

👍 this looks great - I came here looking for the Timestamp conversion - so glad I found this PR.

Haven't tested the wrapper types with envoy - but I can confirm that for Timestamp <> String it works as advertised, we've been using it in our GRPC APIs for years, with the official ESPv2 json-grpc transcoder.

// # JSON Mapping
//
// In JSON format, the Timestamp type is encoded as a string in the
// [RFC 3339](https://www.ietf.org/rfc/rfc3339.txt) format. That is, the
// format is "{year}-{month}-{day}T{hour}:{min}:{sec}[.{frac_sec}]Z"
// where {year} is always expressed using four digits while {month}, {day},
// {hour}, {min}, and {sec} are zero-padded to two digits each. The fractional
// seconds, which can go up to 9 digits (i.e. up to 1 nanosecond resolution),
// are optional. The "Z" suffix indicates the timezone ("UTC"); the timezone
// is required. A proto3 JSON serializer should always use UTC (as indicated by
// "Z") when printing the Timestamp type and a proto3 JSON parser should be
// able to accept both UTC and other timezones (as indicated by an offset).
//
// For example, "2017-01-15T01:30:15.01Z" encodes 15.01 seconds past
// 01:30 UTC on January 15, 2017.
//
// In JavaScript, one can convert a Date object to this format using the
// standard
// [toISOString()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString)
// method. In Python, a standard `datetime.datetime` object can be converted
// to this format using
// [`strftime`](https://docs.python.org/2/library/time.html#time.strftime) with
// the time format spec '%Y-%m-%dT%H:%M:%S.%fZ'. Likewise, in Java, one can use
// the Joda Time's [`ISODateTimeFormat.dateTime()`](
// http://www.joda.org/joda-time/apidocs/org/joda/time/format/ISODateTimeFormat.html#dateTime%2D%2D
// ) to obtain a formatter capable of generating timestamps in this format.
//
//
message Timestamp {
// Represents seconds of UTC time since Unix epoch
// 1970-01-01T00:00:00Z. Must be from 0001-01-01T00:00:00Z to
// 9999-12-31T23:59:59Z inclusive.
int64 seconds = 1;
// Non-negative fractions of a second at nanosecond resolution. Negative
// second values with fractions must still have non-negative nanos values
// that count forward in time. Must be from 0 to 999,999,999
// inclusive.
int32 nanos = 2;
}

Same is valid for Duration.

@galihputera
Copy link
Contributor Author

Thanks, @timburks for your approval! but I still couldn't merge it. Could you help me do that?

@timburks timburks merged commit 0835de8 into google:main Mar 24, 2023
@timburks
Copy link
Contributor

@galihputera done, thank you!

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

8 participants