nicer JSON for standard wrappers

[sheepdreamofandroids]

Wrappers.proto defines types for nullable scalars. When those are mapped to JSON you get a nullable object with a value field. That's a bit awkward and unnecessary. It would be nicer if the wrapper would be mapped to a simple nullable JSON field. This could be special cased or maybe there is some way of marking wrappers. Initially just the well-known ones would be OK, I guess.

I could make a PR if you like.

[sheepdreamofandroids]

@siderakis No thoughts?

[siderakis]

What is Wrappers.proto in this context?

[sheepdreamofandroids]

In protobuf-java there is a file wrappers.proto in the package google.protobuf containing wrappers for primitive types when nullability/optionality is required. It contains messages like FloatValue or UInt64Value. It is the equivalent of java wrapper classes.

[siderakis]

Oh interesting, for JSON I have been converting the data Map<String,Object> to JSON directly using GSON. Would that work for your use case? or could you tell me about your use case?

[sheepdreamofandroids]

I'll give a simple example: a gRPC service returns painting which may have an artistID. The artistID is simply an long, or null in case of an unknown artist. In proto3 you have to wrap this long in an Int64 to make it nullable. At this time that would mean the output of the query contains either:

artist: null or artist: {value: 123456789}

Nicer would be if artist were left out if unknown and the id would simply be given as a number instead of an object.

[siderakis]

Is this functionality specific to GraphQL/Rejoiner or just general proto3 JSON conversion & proto message design?

[siderakis]

With a long ID 0 could be a valid id so I see the issue (unless you never use 0 and treat it as null in the clients). A the default value for a string wouldn't have that issue and is what's used as resource identifiers in the related AIP https://aip.dev/122

[siderakis]

If you created a stand-alone package we could use it in one of the examples, but I'm not sure if it should be in the rejoiner repo.

[sheepdreamofandroids]

Is this functionality specific to GraphQL/Rejoiner or just general proto3 JSON conversion & proto message design?

If your aim is to conform to that proto/json mapping, then the current behaviour is a bug. At least that's how I read it. The problem is not just related to ID's, that was just a made up example. Even an empty string could mean something different from an absent one.

[ncioj10]

@siderakis We actually have the same problem, as we want to implement rejoiner for transmitting measurement data updates.

For example it might be valid to have:

temperature: 0 as a value and as default value for integers, this is why we are currently using the Wrappers.proto types in our messages (e.g. temperature: null or temperature: {value: 0} to differentiate between the two). In the GraphQL representation it would make sense to have this converted as a normal integer with null value as unset. Also it is problematic that rejoiner seeems the send back temperature: {value: 0} even if temperature was unset in the proto message -> there probably should be a check for the message types if they are set or not.

Is there a way to override this behaviour as an addition globally for these types?

[ncioj10]

@sheepdreamofandroids I would also contribute to a PR here My proposal would be two steps:

1. Fix the wrong return behaviour so unset messages are null and not a default message.
2. Add possibility to map messages to different types so have a conversion override option which can then be used so that wrappers can be converted to normal JSON types and vice versa.