search

golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.8k stars 1.58k forks source link

[protojson][MarshalOptions] Issues when enum value is 0 #1649

Closed omar-scio closed 1 month ago

omar-scio commented 2 months ago

What version of protobuf and what language are you using? Using Go

What did you do? Using the protojson.Marshal function with default options, I'm seeing that enum values (when set to 0) are not showing up in the response. It looks like unpopulatedFieldRanger is used when MarshalOptions.EmitUnpopulated = false (the case I have).

In my particular usecase, I don't want every type of field to EmitUnpopulated, but I do want the enum values to be emitted (regardless of if they are the 0 value or not). In my eyes, enum should be treated as an exception. Please let me know your thoughts.

What did you expect to see? I expected the enum value to be marshalled anyways.

What did you see instead? Instead, the enum value was dropped and did not end up in the JSON. That's because the MarshalOptions.EmitUnpopulated = false option was provided. I do not want to set it true as I expect this proto to be sparsely populated.

Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs). None to include.

Anything else we should know about your project / environment? N/A

puellanivis commented 2 months ago

Enums are the same as any other field, and omitted when they are the zero value. If you have control over the protobuf definition, it is recommended that you set the first value (the 0 value) to some sort of “INVALID” or “UNSPECIFIED” name to ensure that all legitimate values are represented in the JSON, and you can distinguish “not set” from whatever state would otherwise be in the zero value.

To be clear: this is working as intended, and we do not anticipate adding a new option that would exclusively populate enum zero values, as best practice already should cover the intent of such a use-case.

puellanivis commented 1 month ago

As mentioned, this is all working as intended, and best practice should avoid the need for such an option. (Additionally, this is an option that also doesn’t exist in the other implementations. C++ JsonPrintOptions and Java JsonFormat.Printer)