tomas-abrahamsson / gpb

A Google Protobuf implementation for Erlang
Other
557 stars 153 forks source link

JSON mapping support #164

Closed leozilla closed 5 years ago

leozilla commented 5 years ago

As already discussed in this issue: https://github.com/tomas-abrahamsson/gpb/issues/61

Support for the JSON mapping as described here (https://developers.google.com/protocol-buffers/docs/proto3#json) would allow to switch between human readable and the Protobuf machine optimized data formats on a per use case basis.

tomas-abrahamsson commented 5 years ago

Yep. I will look into it, but don't hold your breath.

DanielUranga commented 5 years ago

👍 How hard would it be to implement? What could I do to help it become a reality?

DanielUranga commented 5 years ago

I answer myself: https://github.com/tomas-abrahamsson/gpb/issues/61#issuecomment-471120636

leozilla commented 5 years ago

@tomas-abrahamsson any updates? just so you know, once u implemented this I will use it as a base for the same impl in elixir. So we would catch 2 birds with one stone (I hope u dont like birds.. lol)

tomas-abrahamsson commented 5 years ago

No updates yet, unfortunately. I've had little time for this for quite a while now, but I hope to be able to tend to it soon again. It is the next big thing on the agenda for gpb. Sorry, and thanks for having patience.

A while ago I took a look and realized that the translations approach, that I talked about earlier, was probably not a good idea. Better to generate specialized encoders to and decoders from json. I also took a look at some json libraries, and it seems it would not be too difficult to make it possible to have a generation-time option for supporting jsx, jiffy and mochijson2 (and maps). So in a way, I've at least set a direction.

tomas-abrahamsson commented 5 years ago

I just pushed some work-in-progress to a json branch, if you want to play with it. Note that the branch may get force-pushed in the future. If the json option is specified, the generated code will contain a to_json/1,2,3 function. So far, I've only done encoding, not decoding, so there is not yet any from_json function. That's next step.

My idea is that the generated code will output a JSON representation that can be used directly with some common JSON encoder/decoder libraries to produce/parse JSON text. Ie the code generated from gpb will not do the actual JSON text generation/parsing itself. I hope this approach is ok. There are presets to generate code with format for jsx, mochijson2, jiffy and map, but it is also possible to control details, for interwork with other JSON libraries. The list of new options are:

I have not yet double checked super-much if the output works well with all targeted JSON libraries. I also have not yet cross-checked with Google's protobuf's JSON, I've only tried to follow what the language guide says.

There is not yet support for the json_name proto option.

I've been thinking whether or not to keep the top-level json switch/option. I'm thinking now as things are a bit experimental still, it may be good to have it. Later on it may default to true, or go away entirely.

tomas-abrahamsson commented 5 years ago

I pushed again to the json branch (force push). It now generates to_json and from_json. I've interop-tested it with the json libs, and also with JSON from Google's protobuf, but there may of course be bugs. You're welcome to take it for a spin, please let me know if you think it looks useful, also from an Elixir perspective.

Except for the addition of a generated from_json function, compared to earlier versions of the branch, there are just a few additional user-visible changes:

The reason for these changes is I think it better harmonizes with the names in the JsonPrintOptions of Google's protobuf.

tomas-abrahamsson commented 5 years ago

FYI: I just force pushed again. No changes to JSON functionality. (Changes are: eunit test refactorizations for NIF tests and robustification to locating vendored proto3 well-knowns.)

tomas-abrahamsson commented 5 years ago

FYI again: Pushed again, I've been doing some probing of Google's protobuf for decoding JSON, ie the from_json function, to see what it accepts that's just outside of the specified and documented values, to try to accept the same, for better interop:

@leozilla Have you had any chance yet to take a look? If things looks generally ok, there's always also the route available to merge it and fix any more or less minor changes later on as we go along.

tomas-abrahamsson commented 5 years ago

Hi, I decided to merge this, so it is now included in 4.9.0, which I just pushed. Should you discover anything that would need reworking or fixups, please feel free to open an issue for it.