Testing JSON including_default_value_fields behavior#5
Draft
antongrbin wants to merge 1 commit intomasterfrom
Draft
Testing JSON including_default_value_fields behavior#5antongrbin wants to merge 1 commit intomasterfrom
antongrbin wants to merge 1 commit intomasterfrom
Conversation
garyp
pushed a commit
to streem/pbandk
that referenced
this pull request
Oct 25, 2022
…#238) # Motivation Fixes #235 # Changes * Change the behavior for JSON serialization for fields with explicit presence. * If unset -> don't emit on the wire. * If set -> emit on the wire, even if the value is default (regardless of the `jsonConfig.outputDefaultValues`) This is consistent with the following: https://github.com/protocolbuffers/protobuf/blob/main/docs/field_presence.md#semantic-differences This PR also changes behavior of `outputDefaultValues` for unset nested message fields. Before we were outputting `null` and now we don't output any value. This is consistent with c++, python and java, as explained here: noom/protobuf#5 # Tested Run all unit tests in `runtime`
garyp
pushed a commit
to streem/pbandk
that referenced
this pull request
Mar 7, 2023
…#238) Fixes #235 * Change the behavior for JSON serialization for fields with explicit presence. * If unset -> don't emit on the wire. * If set -> emit on the wire, even if the value is default (regardless of the `jsonConfig.outputDefaultValues`) This is consistent with the following: https://github.com/protocolbuffers/protobuf/blob/main/docs/field_presence.md#semantic-differences This PR also changes behavior of `outputDefaultValues` for unset nested message fields. Before we were outputting `null` and now we don't output any value. This is consistent with c++, python and java, as explained here: noom/protobuf#5 Run all unit tests in `runtime`
antongrbin
pushed a commit
that referenced
this pull request
Jan 10, 2024
PiperOrigin-RevId: 575857972
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Motivation
I don't intend to merge this.
Test behavior of
includingDefaultValueFieldswhen we have an unset nested message field.Observations
In C++ "include default fields" is actually called always_print_primitive_fields which specifies that this affects only primitive fields.
In Java, there is a unit test that shows that even if
includingDefaultValueFieldswe will not emit anullfor unset nested messageIn Python, we see in this PR that
including_default_value_fieldsdoesn't affect nested messages. Tested withbazel test //python:json_format_test