Programmatically build descriptors

[slinkydeveloper]

Hi,
I wonder whether it might be possible to programmatically build descriptors in code. I have a use case where I get user descriptors and I modify them, based on an extension value.

[andrewhickman]

What do you imagine the API would look like? Would it be an alternative constructor for something like MessageDescriptor?

[slinkydeveloper]

In my specific case, I need something like this:

let some_other_msg_desc = {}; // This comes from user descriptors

let other_message_field = some_other_msg_desc.get_field(1).unwrap();

let msg_desc = MessageDescriptor::builder("package.Name")
  .with_field(other_message_field)
  .build();

Perhaps I need to manipulate other_message_field as well, e.g.:

let manipulated_message_field = FieldDescriptor::builder_from(other_message_field)
  .set_name("my_new_field")
  .set_number(1)
  .build();

Once I have this new MessageDescriptor, I wish to use it with DynamicMessage to use the JSON mapping functionalities.

I guess to be able to work with DynamicMessage, those build need to write the new descriptor in some DescriptorPool, where you also have the dependencies to all the other messages you need (e.g. in case a field refers to another MessageDescriptor).

Does this makes somewhat sense?

[andrewhickman]

Thanks, that does make sense!

The current API is very much based around using FileDescriptorProtos from the protobuf compiler - but maybe a builder API could be implemented as a more convenient wrapper around adding a dummy FileDescriptorProto and DescriptorProto to the pool.

[flisky]

Build a dummy prost_types::FileDescriptorSet and then feed it into prost_reflect::DescriptorPool::from_file_descriptor_set is totally accepted to me.

However, prost_reflect::DynamicMessage::deserialize is a little bit annoying to ask for the unnecessary ownership of MessageDescriptor. We have to call desc.clone() in the hot path. Would you accept a PR to change desc into a reference? @andrewhickman

[andrewhickman]

However, prost_reflect::DynamicMessage::deserialize is a little bit annoying to ask for the unnecessary ownership of MessageDescriptor. We have to call desc.clone() in the hot path. Would you accept a PR to change desc into a reference? @andrewhickman

The clone should be pretty fast, since its just cloning an Arc internally. I don't think changing the parameter in deserialize would actually help, unless we also changed DynamicMessage to store a reference, which would be a breaking change.

I've considered adding a reference based set of descriptor types in #7 but its a pretty big API change and I haven't run into any performance bottlenecks yet. I'm interested to know if its an issue in your case though!

[andrewhickman]

One issue with building descriptors manually is that extra validation checks in future may cause the add_file_descriptor_proto call to start failing. For example, as part of #5 I'd like to validate that all referenced types are contained in dependency files. A builder style API could automatically ensure that this kind of detail is correct.