golang
types/dynamicpb: add NewTypes helper
The protodesc package provides the ability to go from a descriptorpb.FileDescriptorSet to a protoregistry.Files. However, the latter type is still not useful to use with most of the protobuf module since many APIs expect a interface that only protoregistry.Types implements (e.g., protojson.MarshalOptions.Resolver).
Should we provide helper functionality somewhere that takes a protoregistry.Files-like object and creates a protoregistry.Types-like object from it?
The signature for this helper could possibly be something like:
// NewTypes constructs a new types resolver from a file descriptor ranger.
// It constructs protoreflect.MessageTypes and protoreflect.ExtensionTypes
// from descriptors using the dynamicpb package.
//
// The input is implemented by protoregistry.Files.
// The output provides methods similar to protoregistry.Types.
func NewTypes(interface {
RangeFiles(func(protoreflect.FileDescriptor) bool)
}) interface {
FindExtensionByName(field protoreflect.FullName) (protoreflect.ExtensionType, error)
FindExtensionByNumber(message protoreflect.FullName, field protoreflect.FieldNumber) (protoreflect.ExtensionType, error)
FindMessageByName(message protoreflect.FullName) (protoreflect.MessageType, error)
FindMessageByURL(url string) (protoreflect.MessageType, error)
} {
...
}
(The actual implementation probably wouldn't return an interface but a concrete type so that we have the freedom to add new methods).
I don't know where such help functionality should live. Reasonable packages could be protodesc, protoregistry, or dynamicpb or somewhere entirely new. As a straw-man proposal, I suggested dynamicpb since this implementation must rely on dynamicpb.NewExtensionType and dynamicpb.NewMessageType.
(Note for consideration in the future) We need to decide whether this function keeps a reference to a protoregistry.Files-like object or not. The semantic difference is whether NewTypes iterates through all files up-front or if it simply holds a reference to it and dynamically queries it when each Find method is called. If the latter, I should note that FindExtensionByNumber currently can't be implemented by the lone FindDescriptorByName method on protoregistry.Files.
I do have a use for such helper. Our particular usage will have a descriptorpb.FileDescriptorSet as input and we'll need to have a resolver for prototext.UnmarshalOptions.
I'm good with the proposed API as protodesc already allows for constructing protoregistry.Files from a descriptorpb.FileDescriptorSet. Nicer if helper can just go from a descriptorpb.FileDescriptorSet to a resolver of course.
dynamicpb package sounds good. I'd suggest naming the func as NewTypeResolver though.
I could have sworn I gave a thumbs up or an verbal approval to something like this, but now I don’t see anything.
Especially with the protobuf webpage talking about the concept, we definitely might want to make this more accessible.
As an implementation note, I'm waffling whether to add a FindExtensionByNumber method to protoregistry.Files so that NewTypes can simply be a thin wrapper over the Files-like object that is provided.
In fact, I'm wondering whether we should make the APIs of protoregistry.Files and protoregistry.Types more similar in all respects.
I do have a use for such helper. Our particular usage will have a
descriptorpb.FileDescriptorSetas input and we'll need to have a resolver forprototext.UnmarshalOptions.
Same usage encountered on the Resolver in proto.UnmarshalOptions
We should add this. (Inspiration: Ran across yet another case where someone would have an easier time if we had a dynamicpb.NewTypes.)
A sticking point in the past has been resolving the question of whether we should add new methods to protoregistry.Files to allow dynamicpb.Types to be a thin wrapper around it. In particular, there's currently no way to look up an extension by containing message number in a Files.
It probably makes sense to add methods like FindMessageByName to Files, but adding an index of extensions by containing message and number would be a cost on all programs despite few needing to use that index. Perhaps we could lazily initialize the index to avoid that cost. But let's not worry about changing the Files API for the moment, and instead define dynamicpb.NewTypes in a way that lets us take advantage of any changes in the future:
package protoregistry
// A Types is a collection of dynamically constructed descriptors.
// Its methods are safe for concurrent use.
type Types
// NewTypes creates a new Types registry with the provided files.
// The Files registry must not be changed after calling NewTypes.
func NewTypes(f *protoregistry.Files) *Types
func (t *Types) FindEnumByName(name protoreflect.FullName) (protoreflect.EnumType, error)
func (t *Types) FindExtensionByName(name protoreflect.FullName) (protoreflect.ExtensionType, error)
func (t *Types) FindExtensionByNumber(message protoreflect.FullName, field protoreflect.FieldNumber) (proto reflect.ExtensionType, error)
func (t *Types) FindMessageByName(name protoreflect.FullName) (protoreflect.MessageType, error)
func (t *Types) FindMessageByURL(url string) (protoreflect.MessageType, error)
Since this requires Types to build an enum number index, we disallow changes to the input Files after construction. This restriction could be relaxed in the future if we add Files.FindExtensionByNumber.
