New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: allow transformations between descriptor.proto objects and reflection objects #757

Open
murgatroid99 opened this Issue Apr 11, 2017 · 8 comments

Comments

Projects
None yet
2 participants
@murgatroid99

murgatroid99 commented Apr 11, 2017

protobuf.js version: 6

It would be useful to be able to take a message of a type defined in google/protobuf/descriptor.proto, and convert it into the corresponding reflection object, and also be able to convert the other way. Of course, some of them don't really make sense, but I would like, for example, to be able to load a DescriptorProto message instance as a Message object, and conversely to be able to extract a DescriptorProto instance from a Message object.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 11, 2017

That's basically self-describing messages-support, right?

@dcodeIO dcodeIO added the enhancement label Apr 11, 2017

@murgatroid99

This comment has been minimized.

murgatroid99 commented Apr 11, 2017

Not really. The types in descriptor.proto are structurally equivalent to the information you can load from a .proto file. For example, and instance of the DescriptorProto message type contains the same information as a message declaration in a .proto file. What I'm asking for is a transformation between instances of these messages and the ProtoBuf.js reflection types.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 11, 2017

To my understanding, DescriptorProtos reference enums and messages of other DescriptorProtos, including only nested enums and messages. Hence, resolving any such type referencing a non-nested enum or message would throw.

Only a FileDescriptorSet, which is used in self-describing messages, would contain all relevant DescriptorProtos making it possibly to successfully work with (like encode and decode) the types imported this way.

I might just be misunderstanding your use-case, though, so feel free to point me into the right direction.

@murgatroid99

This comment has been minimized.

murgatroid99 commented Apr 12, 2017

Right, DescriptorProto may have been a bad example. It is likely that I would only need to transform between instances of FileDescriptorSet and Root objects. The goal here is to support the gRPC server reflection protocol.

dcodeIO added a commit that referenced this issue Apr 12, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 12, 2017

I quickly put something together as a starting point, see commit above. The reason why I'd prefer an extension here is that it's usually not necessary to include the entire descriptor (functionality) as part of the standard library.

Idea is that doing require("protobufjs/ext/descriptor") loads the extension along with the descriptor definitions and then adds fromDescriptor, #toDescriptor methods to all reflection types. See protobuf.Type.fromDescriptor for how these methods could look like.

@murgatroid99

This comment has been minimized.

murgatroid99 commented Apr 12, 2017

That looks like what I was asking for.

dcodeIO added a commit that referenced this issue Apr 12, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 12, 2017

Sigh, this will take a while. Added a few comments to document what's (still) necessary.

dcodeIO added a commit that referenced this issue Apr 12, 2017

dcodeIO added a commit that referenced this issue Apr 12, 2017

dcodeIO added a commit that referenced this issue Apr 12, 2017

dcodeIO added a commit that referenced this issue Apr 13, 2017

Other: Fixed obvious issues with ext/descriptor, does not throw anymo…
…re when throwing descriptor.proto itself at it, see #757
@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 13, 2017

Still pretty rough around the edges, but it's now able to convert a root with loaded descriptor.proto in it to a FileDescriptorSet, encode it, decode it back and have it return something similar (still needs a real test suite) to the descriptor we initially put in. Wheee...

dcodeIO added a commit that referenced this issue Apr 13, 2017

dcodeIO added a commit that referenced this issue Apr 13, 2017

dcodeIO added a commit that referenced this issue Apr 13, 2017

dcodeIO added a commit that referenced this issue Apr 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment