You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Unknown fields not being repopulated upon de-serialization from json #23
@PeteyPabPro - it seems that your observation is correct - neither JsonFormat nor JsonJacksonFormat seem to slurp the unknown fields into the Builder/Message as they could/should.
I wrote a simple unit tests (disabled in the pull request) that shows this off. In the future, that's something you can/should do too - write a simple test along with minimal protobuf so that a fix can be written against the test as a specification.
I don't have time to fix this - would you be able to give your 👍 to PR #24, if not also enable & write a fix for it?
Hmm… I was just looking at JsonFormat vs. JsonJacksonFormat and found that they are different 😦 This might not be a problem if you always used the same one on both ends, but I don't think that's fair to assume, and furthermore, JsonJackson has other issues, specifically with Groups, where it prints a JSON object, but expects a binary delimited ByteString on input/merge.
I'm not sure there's a spec for JSON, but the two JSON-related formatters should agree with one another.
For your complex example, here is the output from JsonFormat:
Good questions - I acknowledge that my observation didn't give opinion on next steps, so let me think a moment.
I think we need to understand & document if possible what the format should be for JSON - It's unfortunate - I don't see wiki enabled for this, but we can add the gh-pages branch for documenting things…
Activity
[Issue bivas#23] Write a unit test that shows that unknown fields are…
scr commentedon Mar 13, 2016
@PeteyPabPro - it seems that your observation is correct - neither JsonFormat nor JsonJacksonFormat seem to slurp the unknown fields into the Builder/Message as they could/should.
I wrote a simple unit tests (disabled in the pull request) that shows this off. In the future, that's something you can/should do too - write a simple test along with minimal protobuf so that a fix can be written against the test as a specification.
I don't have time to fix this - would you be able to give your 👍 to PR #24, if not also enable & write a fix for it?
Merge pull request #24 from scr/unknownFields
primshnick commentedon Mar 14, 2016
I'll take a look..
scr commentedon Apr 6, 2016
Hmm… I was just looking at JsonFormat vs. JsonJacksonFormat and found that they are different 😦 This might not be a problem if you always used the same one on both ends, but I don't think that's fair to assume, and furthermore, JsonJackson has other issues, specifically with Groups, where it prints a JSON object, but expects a binary delimited ByteString on input/merge.
I'm not sure there's a spec for JSON, but the two JSON-related formatters should agree with one another.
For your complex example, here is the output from JsonFormat:
And here is the output from JsonJacksonFormat:
primshnick commentedon Apr 6, 2016
Hmm.. So do you think
should all be part of my PR? Or is it ok to just stick with (1) for this PR, and then do the Jackson stuff in another one?
scr commentedon Apr 6, 2016
Good questions - I acknowledge that my observation didn't give opinion on next steps, so let me think a moment.
I think we need to understand & document if possible what the format should be for JSON - It's unfortunate - I don't see wiki enabled for this, but we can add the gh-pages branch for documenting things…