search

FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
316 stars 136 forks source link

[protobuf] create ProtoSchema from descriptor file #78

Open knoguchi opened 7 years ago

knoguchi commented 7 years ago

I would like to create ProtobufSchema objects from the descriptor files instead of proto files. protoc -o output.desc can generate the descriptor file output.desc by compiling multiple proto files. It's often easier to use the descriptor file than dealing with multiple .proto files. At my work we have fairly complex proto files.

I actually have an experimental implementation. I used Google's official protobuf-java and protobuf-dynamic to read the descriptor file.

Questions

  1. would you consider adding the "ProtobufSchema by descriptor file" feature?
  2. if the answer is yes, do you think the above approach is acceptable i.e. use google's official protobuf jars, and this._source being null.
  3. If the answer is no, would you add a public constructor that takes a ProtobufMessage argument to the ProtobufSchema so that the 3rd party library can create ProtobufSchema objects without ProtoFile object.

thanks.

cowtowncoder commented 7 years ago

I'll have to think about this a bit. But let's start with easy answers....

First of all, yes, allowing use of descriptor files sounds like a good addition, so +1 for that. I like the idea a lot, since as you say, dealing with multiple complex proto files gets tricky.

As to adding new dependencies: I am bit on the fence on that one -- I prefer minimizing dependencies. But there is no strict limit on what can be added.

So perhaps the best way would be to first start with (3), as I would be happy to add extension points first, to make this possible. And then if and when things work nicely, see if some or all of functionality could be added here. Or maybe it could be an extension module as sub-project here -- that might be the best way? That is, another maven sub-module (jackson-module-protobuf-schema or something), which would be built and released along with format module, but optional to use.

Maybe it'd be good if you just do simple PR first, adding extension points you would need, and we can go from there?

knoguchi commented 7 years ago

I totally understand your thoughts. We are on the same page. There is one thing I would like to try before making invasive changes. The .desc file itself is a PB defined by this proto. It means we should be able to load the .desc files out of the box without protobuf-java. The other dependency, protobuf-dynamic, is just a "nice to have" thin wrapper. No extra jars needed if this idea works.

cowtowncoder commented 7 years ago

Excellent! I like the idea even more.