Add an EmitUnpopulatedPrimitiveFields option

[sam-utila]

The problem I am facing:

I have a gRPC-protobuf service which follows Google's resource oriented design (https://cloud.google.com/apis/design/resources).

In some resources I want to be able to specify sub-messages that are INPUT_ONLY:

message Resource {
  string id = 1;
  
  message CreateParams {
    int32 hello = 1;
  };

  CreateParams params = 2 [(google.api.field_behavior) = INPUT_ONLY];
}

I want to params to be specified on input, but when I invoke Get/List endpoints on that resource, I want to params to be omitted.

In addition I want to support GRPC-transcoding to JSON, both with envoy (cpp) and with grpc-gateway (Golang)

When I look at Google's APIs I notice the following behavior in transcoding: empty strings, zero numbers, false booleans are a part of the response, but empty sub-messages are not.
So for the given example with the following input:
id := ""
params := null

I get the following json:

{
  "id": ""
}

And I suspect its due to the following flag in the cpp json transcoding in envoy:

always_print_primitive_fields

Which always emits primitive fields, but does not emit empty messages.

When I transcode using grpc-gateway which uses protojson I get:

{
  "id": "",
  "params": null
}

And I assume this is due to the EmitUnpopulated option in protojson:

which is set to true.

If I set it to false I would get

{}

So it seems that it is not possible to have the same json transcoding in cpp and Golang in regards to "unpopulated/default" field values.

Adding EmitUnpopulatedPrimitiveFields might give the option to have the same transcoding to other tools like grpc-gateway

	// EmitUnpopulatedPrimitiveFields specifies whether to emit unpopulated fields. It does not
	// emit unpopulated oneof fields or unpopulated extension fields.
	// The JSON value emitted for unpopulated fields are as follows:
	//  ╔═══════╤════════════════════════════╗
	//  ║ JSON  │ Protobuf field             ║
	//  ╠═══════╪════════════════════════════╣
	//  ║ false │ proto3 boolean fields      ║
	//  ║ 0     │ proto3 numeric fields      ║
	//  ║ ""    │ proto3 string/bytes fields ║
	//  ║ null  │ proto2 scalar fields       ║
	//  ║ []    │ list fields                ║
	//  ║ {}    │ map fields                 ║
	//  ╚═══════╧════════════════════════════╝
	EmitUnpopulatedPrimitiveFields bool

[puellanivis]

In response to your second part, this seems to be a divergence in implementing the standard. The standard provides:

Emit fields with default values: Fields with default values are omitted by default in proto3 JSON output. An implementation may provide an option to override this behavior and output fields with their default values.

It seems that CPP has presumed that this should only apply to primitive values, while Golang has elected to have it apply to all fields. I would argue the Golang implementation is more true to the standard as phrased.

The two representations should however be semantically identical, that is, rendering into a SubMessage == nil state, which is different from the &Submessage{} “message is present but all fields are default” state. This is per the row covering the JSON mapping for message:

null is an accepted value for all field types and treated as the default value of the corresponding field type.

I’m not sure how useful it would be to offer a second marshalling option that is almost essentially the same as EmitUnpopulated, but doesn’t cover messages.

But since this is a disagreement between two implementations of the protobuf standard, it might be helpful to the main protobuf isuses board and seek discussion about the resolution there. As is, the way I see it, the Golang implementation is within standard, and the CPP implementation is out-of-spec, but also as noted: neither is incompatible, and both should be semantically identical.

[neild]

Since message, map, and repeated fields don't have default values, I think you could interpret "default values" in the proto3 JSON specification as referring to the zero values of primitive fields such as int.

A protojson marshal option that matches the behavior of the existing C++ always_print_primitive_fields seems fairly reasonable to me.

[puellanivis]

Are you sure that message and repeated fields do not have default values?

For message fields, the field is not set. Its exact value is language-dependent.

The default value for repeated fields is empty (generally an empty list in the appropriate language).

And through transitive properties, maps should as well:

Map fields are just a shorthand for a special kind of repeated field.

[sam-utila]

I'm not talking about maps and repeated fields - I actually do want to emit them.
I just don't want to emit unset submessages as "key": null in json.

Actually from this:

For message fields, the field is not set. Its exact value is language-dependent.

The exact value is language-dependent, but is not set.
There are two ways to "not set" a value in json, either no key, or "null" value, I would say that both fall into the spec.

To be honest there is a way to get what I want but I think it pollutes the .proto file and it is to set the message as optional - it is a bit funny since from the spec in proto3 submessages are optional and have an IsSet function, but I guess that by making it optional it actually makes this a oneof behind the scenes - which forces protojson to omit the key - which otherwise can mistakenly be used as a selector - but it makes the proto file worse, take a look at this:

message Resource {
  string id = 1;
  
  message CreateParams {
    int32 hello = 1;
  };

  optional CreateParams params = 2 [(google.api.field_behavior) = INPUT_ONLY; (google.api.field_behavior) = REQUIRED];
}

optional at the beginning to avoid getting this on output since this is CreateParams.
REQUIRED in field_behavior since when I'm generating documentation, I want the specs to require this to be present when creating the resource.

This is how my journey began, and to my surprise, when looking at responses from Google APIs, I noticed that optional is not required on the protobuf message.

[sam-utila]

Notice that there are 3 states:

{
  'id': 0,
  'params': null
}

{
  'id': 0
}

{
  'id': 0,
  'params': {}
}

Obviously I agree that 3 is not even an option here, it's a totally different message.
I think that both 1 and 2 are the same and both are valid and EmitUnpopulatedPrimitiveFields can give a way to choose which flavor I want. 1 is Golang style, 2 is cpp style.

[puellanivis]

While there are three versions of outputs possible, it is also true that both state 1, and state 2 that you present should compose into semantically the same data-structure in Go. That is, it should be Params == nil in both.

[sam-utila]

I agree, so I'm not sure if I convinced you that it's fine to add such option or not :-)

(Of course naming is pending, can also have the flag be an additional flag to EmitUnpopulated such as NoEmitUnpopulatedMessaged - default is to Emit)

[puellanivis]

🤷‍♀️ I suppose the only thing left is to make a Gerrit change request.

[same-id]

Ok, I'll create one

[same-id]

https://go-review.googlesource.com/c/protobuf/+/521215