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

Add an option to conform (fully) to the Protobuf 3 canonical JSON representation #76

Open
mindloaf opened this issue Aug 21, 2020 · 9 comments

Comments

@mindloaf
Copy link

int64, fixed64, uint64: JSON value will be a decimal string. Either numbers or strings are accepted.

If the Any contains a value that has a special JSON mapping, it will be converted as follows: {"@type": xxx, "value": yyy}. Otherwise, the value will be converted into a JSON object, and the "@type" field will be inserted to indicate the actual data type.

@jhaber
Copy link
Member

jhaber commented Aug 24, 2020

Thanks for opening this issue, I think it would be a great improvement 👍

@jhaber
Copy link
Member

jhaber commented Jan 20, 2022

btw I started implementing this on the compatible-serialization branch, but the changes kept growing in scope and I haven't found the time to finish them up. As part of that work I realized that we don't actually serialize/deserialize uint32/uint64/fixed32/fixed64 correctly (depending on size, we serialize them as negative numbers, rather than inspecting the sign bit and taking the correct action)

@jhaber
Copy link
Member

jhaber commented Jan 20, 2022

And I think support for serializing/deserializing DynamicMessage is going to be pretty unlikely (might as well just use com.google.protobuf.util.JsonFormat for that)

@mindloaf
Copy link
Author

Yeah DynamicMessages are definitely more "nice to have". Consistent handling of the more basic types is definitely more important.

@mindloaf
Copy link
Author

mindloaf commented Jan 20, 2022

This recently bit me as well:

Proto3 JSON parsers are required to accept both the converted lowerCamelCase name and the proto field name.

I ran into some JSON that was using both camelCased and snake_cased field names. I ended up having to hack in JsonFormat (essentially double parsing) to handle both.

@jhaber
Copy link
Member

jhaber commented Jan 20, 2022

That should be handled with this option:

public Builder acceptLiteralFieldnames(boolean acceptLiteralFieldnames) {

Did you have a case where that wasn't sufficient?

@mindloaf
Copy link
Author

mindloaf commented Jan 20, 2022

To be clear, this was a case were the field names were both literal (snake_cased, in this instance) and camelCased (not literal).

"someOne": {
  "did_this": "for some reason"
}

@mindloaf
Copy link
Author

I'll try out acceptLiteralFieldnames, but I think we also had an issue related to #81

@jhaber
Copy link
Member

jhaber commented Dec 30, 2023

ProtobufJacksonConfig.Builder now has a useCanonicalSerialization method. There are still some gaps (I believe these are mostly related to the handling of Any), but using that method will automatically pick up any fixes/features in the future

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

No branches or pull requests

2 participants