nicer JSON for standard wrappers about rejoiner HOT 11 OPEN

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.

from rejoiner.

@siderakis No thoughts?

from rejoiner.

What is Wrappers.proto in this context?

from rejoiner.

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.

from rejoiner.

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?

from rejoiner.

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.

from rejoiner.

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

from rejoiner.

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

from rejoiner.

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.

from rejoiner.

@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?

from rejoiner.

@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.

from rejoiner.