to-spec json mapping via serde #558
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…prost into serde_json_serialization
danburkert
added a commit
to danburkert/prost
that referenced
this pull request
Apr 1, 2022
This PR implements to and from string methods for `prost_types::Timestamp` following the RFC-3339 timestamp format, as also specified by the [Protobuf JSON mapping spec][1]. The `Display` impl always emits the Zulu (Z) timezone, whereas the parser is slightly lenient in accepting fixed timezone offsets, as well as both the `T` and space seperated date and time portions. This leniency in parsing makes it much much more robust to parsing data of less-than-pristine formatting, even though the protobuf spec allows no leniency. Also, since it was easy with the new date time functionality, we've gone ahead and added a trio of convenience construct functions to `Timestamp` which makes it straightforward to create `Timestamp`s out of calendar date time information. This functionality will be used in follow-up PRs to add Serde [de]serialization support to the types in `prost_types`, which is itself a stepping stone towards full JSON support. This PR was extracted from tokio-rs#558, and is based on code which myself and @konradjniemiec have written, and which has been running in production at @sisudata for a few years. The to-string portion of this code has also been incorporated into [`tracing`][2], so it should be pretty well battle hardened. [1]: https://developers.google.com/protocol-buffers/docs/proto3#json [2]: https://github.com/tokio-rs/tracing/blob/master/tracing-subscriber/src/fmt/time/datetime.rs
danburkert
added a commit
to danburkert/prost
that referenced
this pull request
Apr 5, 2022
This PR implements to and from string methods for `prost_types::Timestamp` following the RFC-3339 timestamp format, as also specified by the [Protobuf JSON mapping spec][1]. The `Display` impl always emits the Zulu (Z) timezone, whereas the parser is slightly lenient in accepting fixed timezone offsets, as well as both the `T` and space seperated date and time portions. This leniency in parsing makes it much much more robust to parsing data of less-than-pristine formatting, even though the protobuf spec allows no leniency. Also, since it was easy with the new date time functionality, we've gone ahead and added a trio of convenience construct functions to `Timestamp` which makes it straightforward to create `Timestamp`s out of calendar date time information. This functionality will be used in follow-up PRs to add Serde [de]serialization support to the types in `prost_types`, which is itself a stepping stone towards full JSON support. This PR was extracted from tokio-rs#558, and is based on code which myself and @konradjniemiec have written, and which has been running in production at @sisudata for a few years. The to-string portion of this code has also been incorporated into [`tracing`][2], so it should be pretty well battle hardened. [1]: https://developers.google.com/protocol-buffers/docs/proto3#json [2]: https://github.com/tokio-rs/tracing/blob/master/tracing-subscriber/src/fmt/time/datetime.rs
danburkert
added a commit
to sisudata/prost
that referenced
this pull request
Apr 7, 2022
This PR implements to and from string methods for `prost_types::Timestamp` following the RFC-3339 timestamp format, as also specified by the [Protobuf JSON mapping spec][1]. The `Display` impl always emits the Zulu (Z) timezone, whereas the parser is slightly lenient in accepting fixed timezone offsets, as well as both the `T` and space seperated date and time portions. This leniency in parsing makes it much much more robust to parsing data of less-than-pristine formatting, even though the protobuf spec allows no leniency. Also, since it was easy with the new date time functionality, we've gone ahead and added a trio of convenience construct functions to `Timestamp` which makes it straightforward to create `Timestamp`s out of calendar date time information. This functionality will be used in follow-up PRs to add Serde [de]serialization support to the types in `prost_types`, which is itself a stepping stone towards full JSON support. This PR was extracted from tokio-rs#558, and is based on code which myself and @konradjniemiec have written, and which has been running in production at @sisudata for a few years. The to-string portion of this code has also been incorporated into [`tracing`][2], so it should be pretty well battle hardened. [1]: https://developers.google.com/protocol-buffers/docs/proto3#json [2]: https://github.com/tokio-rs/tracing/blob/master/tracing-subscriber/src/fmt/time/datetime.rs
danburkert
added a commit
to danburkert/prost
that referenced
this pull request
Apr 14, 2022
This PR implements to and from string methods for `prost_types::Timestamp` following the RFC-3339 timestamp format, as also specified by the [Protobuf JSON mapping spec][1]. The `Display` impl always emits the Zulu (Z) timezone, whereas the parser is slightly lenient in accepting fixed timezone offsets, as well as both the `T` and space seperated date and time portions. This leniency in parsing makes it much much more robust to parsing data of less-than-pristine formatting, even though the protobuf spec allows no leniency. Also, since it was easy with the new date time functionality, we've gone ahead and added a trio of convenience construct functions to `Timestamp` which makes it straightforward to create `Timestamp`s out of calendar date time information. This functionality will be used in follow-up PRs to add Serde [de]serialization support to the types in `prost_types`, which is itself a stepping stone towards full JSON support. This PR was extracted from tokio-rs#558, and is based on code which myself and @konradjniemiec have written, and which has been running in production at @sisudata for a few years. The to-string portion of this code has also been incorporated into [`tracing`][2], so it should be pretty well battle hardened. [1]: https://developers.google.com/protocol-buffers/docs/proto3#json [2]: https://github.com/tokio-rs/tracing/blob/master/tracing-subscriber/src/fmt/time/datetime.rs
danburkert
added a commit
that referenced
this pull request
Apr 14, 2022
This PR implements to and from string methods for `prost_types::Timestamp` following the RFC-3339 timestamp format, as also specified by the [Protobuf JSON mapping spec][1]. The `Display` impl always emits the Zulu (Z) timezone, whereas the parser is slightly lenient in accepting fixed timezone offsets, as well as both the `T` and space seperated date and time portions. This leniency in parsing makes it much much more robust to parsing data of less-than-pristine formatting, even though the protobuf spec allows no leniency. Also, since it was easy with the new date time functionality, we've gone ahead and added a trio of convenience construct functions to `Timestamp` which makes it straightforward to create `Timestamp`s out of calendar date time information. This functionality will be used in follow-up PRs to add Serde [de]serialization support to the types in `prost_types`, which is itself a stepping stone towards full JSON support. This PR was extracted from #558, and is based on code which myself and @konradjniemiec have written, and which has been running in production at @sisudata for a few years. The to-string portion of this code has also been incorporated into [`tracing`][2], so it should be pretty well battle hardened. [1]: https://developers.google.com/protocol-buffers/docs/proto3#json [2]: https://github.com/tokio-rs/tracing/blob/master/tracing-subscriber/src/fmt/time/datetime.rs
|
@konradjniemiec @danburkert I saw a mention that this was being used at @sisudata and I'm curious if that extends to the JSON parsing as well, and/or if this is the latest given more recent (April 2022) activity around some of this. Generally, this seems like it's a good start to moving JSON support along. Would be interested in hopping on this to keep the ball rolling if this has stalled! |
|
Hey @oldgalileo I'd love some help on this if you have some time. In the last working state, this is fully functional for most use cases (you can check the failing_tests.txt for things that don't work) I think to get these to merge ready:
I think |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #75
First round for draft review.
Adds serde annotations with the
json_mappingconfiguration option. There is also ajsonoption which will config in all the code inserde.rsVerified using the Google
conformancetest suite.All things complete for theoretical initial merging:
Not intending on addressing before merging:
google.protobuf.Anysupportgoogle.protobuf.Valuesupportgoogle.protobuf.Structsupportgoogle.protobuf.FieldMasksupportThere are a few recommended tests we are also ignoring
All currently failing tests from
conformance/failing_tests.txtshould fall into one of the above categories ^