reflect/protoreflect, reflect/protoregistry: move descriptor lookup to registry
Drop the protoreflect.FileDescriptor.DescriptorByName method.
Descriptor lookup will always happen through a protoregistry.Files, which
is more generally useful (it's rare that you want to find a descriptor in a
specific file, as opposed to a package which may be composed of multiple files).
Split protoregistry.Files descriptor lookup into individual per-type functions
(enum, message, extension, service), matching the preg.Types API.
Drop the ability to look up enum values, message fields, and service methods
for now. This can be easily added later if needed, and is trivial to implement
in user code. (e.g., look up the service and then consult sd.Methods.ByName().)
Change-Id: I2b3d8ef888921a8464ba1434eddab20c7d3a458e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/172118
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/internal/fileinit/desc.go b/internal/fileinit/desc.go
index f305ee0..199e38d 100644
--- a/internal/fileinit/desc.go
+++ b/internal/fileinit/desc.go
@@ -246,7 +246,6 @@
fileLazy struct {
syntax pref.Syntax
imports fileImports
- byName map[pref.FullName]pref.Descriptor
options []byte
}
)
@@ -260,17 +259,16 @@
func (fd *fileDesc) Options() pref.ProtoMessage {
return unmarshalOptions(descopts.File, fd.lazyInit().options)
}
-func (fd *fileDesc) Path() string { return fd.path }
-func (fd *fileDesc) Package() pref.FullName { return fd.protoPackage }
-func (fd *fileDesc) Imports() pref.FileImports { return &fd.lazyInit().imports }
-func (fd *fileDesc) Enums() pref.EnumDescriptors { return &fd.enums }
-func (fd *fileDesc) Messages() pref.MessageDescriptors { return &fd.messages }
-func (fd *fileDesc) Extensions() pref.ExtensionDescriptors { return &fd.extensions }
-func (fd *fileDesc) Services() pref.ServiceDescriptors { return &fd.services }
-func (fd *fileDesc) DescriptorByName(s pref.FullName) pref.Descriptor { return fd.lazyInit().byName[s] }
-func (fd *fileDesc) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, fd) }
-func (fd *fileDesc) ProtoType(pref.FileDescriptor) {}
-func (fd *fileDesc) ProtoInternal(pragma.DoNotImplement) {}
+func (fd *fileDesc) Path() string { return fd.path }
+func (fd *fileDesc) Package() pref.FullName { return fd.protoPackage }
+func (fd *fileDesc) Imports() pref.FileImports { return &fd.lazyInit().imports }
+func (fd *fileDesc) Enums() pref.EnumDescriptors { return &fd.enums }
+func (fd *fileDesc) Messages() pref.MessageDescriptors { return &fd.messages }
+func (fd *fileDesc) Extensions() pref.ExtensionDescriptors { return &fd.extensions }
+func (fd *fileDesc) Services() pref.ServiceDescriptors { return &fd.services }
+func (fd *fileDesc) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, fd) }
+func (fd *fileDesc) ProtoType(pref.FileDescriptor) {}
+func (fd *fileDesc) ProtoInternal(pragma.DoNotImplement) {}
type (
enumDesc struct {
diff --git a/internal/fileinit/desc_lazy.go b/internal/fileinit/desc_lazy.go
index 7356b32..bbcc9d9 100644
--- a/internal/fileinit/desc_lazy.go
+++ b/internal/fileinit/desc_lazy.go
@@ -333,7 +333,7 @@
var hasSyntax bool
var enumIdx, messageIdx, extensionIdx, serviceIdx int
- fd.lazy = &fileLazy{byName: make(map[pref.FullName]pref.Descriptor)}
+ fd.lazy = &fileLazy{}
for len(b) > 0 {
num, typ, n := wire.ConsumeTag(b)
b = b[n:]
@@ -424,8 +424,6 @@
ed.lazy.values.list[i].unmarshalFull(b, nb, ed.parentFile, ed, i)
}
}
-
- ed.parentFile.lazy.byName[ed.FullName()] = ed
}
func unmarshalEnumReservedRange(b []byte) (r [2]pref.EnumNumber) {
@@ -480,8 +478,6 @@
b = b[m:]
}
}
-
- vd.parentFile.lazy.byName[vd.FullName()] = vd
}
func (md *messageDesc) unmarshalFull(b []byte, nb *nameBuilder) {
@@ -546,8 +542,6 @@
if isMapEntry != md.isMapEntry {
panic("mismatching map entry property")
}
-
- md.parentFile.lazy.byName[md.FullName()] = md.asDesc()
}
func (md *messageDesc) unmarshalOptions(b []byte, isMapEntry *bool) {
@@ -694,8 +688,6 @@
}
fd.messageType = ptype.PlaceholderMessage(pref.FullName(rawTypeName[1:]))
}
-
- fd.parentFile.lazy.byName[fd.FullName()] = fd
}
func (fd *fieldDesc) unmarshalOptions(b []byte) {
@@ -744,8 +736,6 @@
b = b[m:]
}
}
-
- od.parentFile.lazy.byName[od.FullName()] = od
}
func (xd *extensionDesc) unmarshalFull(b []byte, nb *nameBuilder) {
@@ -790,8 +780,6 @@
panic(err)
}
}
-
- xd.parentFile.lazy.byName[xd.FullName()] = xd
}
func (xd *extensionDesc) unmarshalOptions(b []byte) {
@@ -842,8 +830,6 @@
sd.lazy.methods.list[i].unmarshalFull(b, nb, sd.parentFile, sd, i)
}
}
-
- sd.parentFile.lazy.byName[sd.FullName()] = sd
}
func (md *methodDesc) unmarshalFull(b []byte, nb *nameBuilder, pf *fileDesc, pd pref.Descriptor, i int) {
@@ -878,6 +864,4 @@
b = b[m:]
}
}
-
- md.parentFile.lazy.byName[md.FullName()] = md
}
diff --git a/internal/fileinit/fileinit_test.go b/internal/fileinit/fileinit_test.go
index e900168..c116751 100644
--- a/internal/fileinit/fileinit_test.go
+++ b/internal/fileinit/fileinit_test.go
@@ -70,9 +70,9 @@
// Verify that message descriptors for map entries have no Go type info.
mapEntryName := protoreflect.FullName("goproto.proto.test.TestAllTypes.MapInt32Int32Entry")
- d := testpb.File_test_test_proto.DescriptorByName(mapEntryName)
- if _, ok := d.(protoreflect.MessageDescriptor); !ok {
- t.Errorf("message descriptor for %v not found", mapEntryName)
+ d := testpb.File_test_test_proto.Messages().ByName("TestAllTypes").Fields().ByName("map_int32_int32").MessageType()
+ if gotName, wantName := d.FullName(), mapEntryName; gotName != wantName {
+ t.Fatalf("looked up wrong descriptor: got %v, want %v", gotName, wantName)
}
if _, ok := d.(protoreflect.MessageType); ok {
t.Errorf("message descriptor for %v must not implement protoreflect.MessageType", mapEntryName)
diff --git a/internal/prototype/placeholder_type.go b/internal/prototype/placeholder_type.go
index a99bc38..be28bd3 100644
--- a/internal/prototype/placeholder_type.go
+++ b/internal/prototype/placeholder_type.go
@@ -43,17 +43,16 @@
placeholderName
}
-func (t placeholderFile) Options() pref.ProtoMessage { return descopts.File }
-func (t placeholderFile) Path() string { return t.path }
-func (t placeholderFile) Package() pref.FullName { return t.FullName() }
-func (t placeholderFile) Imports() pref.FileImports { return &emptyFiles }
-func (t placeholderFile) Enums() pref.EnumDescriptors { return &emptyEnums }
-func (t placeholderFile) Messages() pref.MessageDescriptors { return &emptyMessages }
-func (t placeholderFile) Extensions() pref.ExtensionDescriptors { return &emptyExtensions }
-func (t placeholderFile) Services() pref.ServiceDescriptors { return &emptyServices }
-func (t placeholderFile) DescriptorByName(pref.FullName) pref.Descriptor { return nil }
-func (t placeholderFile) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
-func (t placeholderFile) ProtoType(pref.FileDescriptor) {}
+func (t placeholderFile) Options() pref.ProtoMessage { return descopts.File }
+func (t placeholderFile) Path() string { return t.path }
+func (t placeholderFile) Package() pref.FullName { return t.FullName() }
+func (t placeholderFile) Imports() pref.FileImports { return &emptyFiles }
+func (t placeholderFile) Enums() pref.EnumDescriptors { return &emptyEnums }
+func (t placeholderFile) Messages() pref.MessageDescriptors { return &emptyMessages }
+func (t placeholderFile) Extensions() pref.ExtensionDescriptors { return &emptyExtensions }
+func (t placeholderFile) Services() pref.ServiceDescriptors { return &emptyServices }
+func (t placeholderFile) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
+func (t placeholderFile) ProtoType(pref.FileDescriptor) {}
type placeholderMessage struct {
placeholderName
diff --git a/internal/prototype/protofile_type.go b/internal/prototype/protofile_type.go
index 1db0b9a..9f23ea2 100644
--- a/internal/prototype/protofile_type.go
+++ b/internal/prototype/protofile_type.go
@@ -44,7 +44,6 @@
es enumsMeta
xs extensionsMeta
ss servicesMeta
- ds descriptorsMeta
}
type fileDesc struct{ f *File }
@@ -63,99 +62,23 @@
f.fileMeta = new(fileMeta)
return fileDesc{f}
}
-func (t fileDesc) Parent() (pref.Descriptor, bool) { return nil, false }
-func (t fileDesc) Index() int { return 0 }
-func (t fileDesc) Syntax() pref.Syntax { return t.f.Syntax }
-func (t fileDesc) Name() pref.Name { return t.f.Package.Name() }
-func (t fileDesc) FullName() pref.FullName { return t.f.Package }
-func (t fileDesc) IsPlaceholder() bool { return false }
-func (t fileDesc) Options() pref.ProtoMessage { return altOptions(t.f.Options, descopts.File) }
-func (t fileDesc) Path() string { return t.f.Path }
-func (t fileDesc) Package() pref.FullName { return t.f.Package }
-func (t fileDesc) Imports() pref.FileImports { return (*fileImports)(&t.f.Imports) }
-func (t fileDesc) Enums() pref.EnumDescriptors { return t.f.es.lazyInit(t, t.f.Enums) }
-func (t fileDesc) Messages() pref.MessageDescriptors { return t.f.ms.lazyInit(t, t.f.Messages) }
-func (t fileDesc) Extensions() pref.ExtensionDescriptors { return t.f.xs.lazyInit(t, t.f.Extensions) }
-func (t fileDesc) Services() pref.ServiceDescriptors { return t.f.ss.lazyInit(t, t.f.Services) }
-func (t fileDesc) DescriptorByName(s pref.FullName) pref.Descriptor { return t.f.ds.lookup(t, s) }
-func (t fileDesc) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
-func (t fileDesc) ProtoType(pref.FileDescriptor) {}
-func (t fileDesc) ProtoInternal(pragma.DoNotImplement) {}
-
-// descriptorsMeta is a lazily initialized map of all descriptors declared in
-// the file by full name.
-type descriptorsMeta struct {
- once sync.Once
- m map[pref.FullName]pref.Descriptor
-}
-
-func (m *descriptorsMeta) lookup(fd pref.FileDescriptor, s pref.FullName) pref.Descriptor {
- m.once.Do(func() {
- m.m = make(map[pref.FullName]pref.Descriptor)
- m.initMap(fd)
- delete(m.m, fd.Package()) // avoid registering the file descriptor itself
- })
- return m.m[s]
-}
-func (m *descriptorsMeta) initMap(d pref.Descriptor) {
- m.m[d.FullName()] = d
- if ds, ok := d.(interface {
- Enums() pref.EnumDescriptors
- }); ok {
- for i := 0; i < ds.Enums().Len(); i++ {
- m.initMap(ds.Enums().Get(i))
- }
- }
- if ds, ok := d.(interface {
- Values() pref.EnumValueDescriptors
- }); ok {
- for i := 0; i < ds.Values().Len(); i++ {
- m.initMap(ds.Values().Get(i))
- }
- }
- if ds, ok := d.(interface {
- Messages() pref.MessageDescriptors
- }); ok {
- for i := 0; i < ds.Messages().Len(); i++ {
- m.initMap(ds.Messages().Get(i))
- }
- }
- if ds, ok := d.(interface {
- Fields() pref.FieldDescriptors
- }); ok {
- for i := 0; i < ds.Fields().Len(); i++ {
- m.initMap(ds.Fields().Get(i))
- }
- }
- if ds, ok := d.(interface {
- Oneofs() pref.OneofDescriptors
- }); ok {
- for i := 0; i < ds.Oneofs().Len(); i++ {
- m.initMap(ds.Oneofs().Get(i))
- }
- }
- if ds, ok := d.(interface {
- Extensions() pref.ExtensionDescriptors
- }); ok {
- for i := 0; i < ds.Extensions().Len(); i++ {
- m.initMap(ds.Extensions().Get(i))
- }
- }
- if ds, ok := d.(interface {
- Services() pref.ServiceDescriptors
- }); ok {
- for i := 0; i < ds.Services().Len(); i++ {
- m.initMap(ds.Services().Get(i))
- }
- }
- if ds, ok := d.(interface {
- Methods() pref.MethodDescriptors
- }); ok {
- for i := 0; i < ds.Methods().Len(); i++ {
- m.initMap(ds.Methods().Get(i))
- }
- }
-}
+func (t fileDesc) Parent() (pref.Descriptor, bool) { return nil, false }
+func (t fileDesc) Index() int { return 0 }
+func (t fileDesc) Syntax() pref.Syntax { return t.f.Syntax }
+func (t fileDesc) Name() pref.Name { return t.f.Package.Name() }
+func (t fileDesc) FullName() pref.FullName { return t.f.Package }
+func (t fileDesc) IsPlaceholder() bool { return false }
+func (t fileDesc) Options() pref.ProtoMessage { return altOptions(t.f.Options, descopts.File) }
+func (t fileDesc) Path() string { return t.f.Path }
+func (t fileDesc) Package() pref.FullName { return t.f.Package }
+func (t fileDesc) Imports() pref.FileImports { return (*fileImports)(&t.f.Imports) }
+func (t fileDesc) Enums() pref.EnumDescriptors { return t.f.es.lazyInit(t, t.f.Enums) }
+func (t fileDesc) Messages() pref.MessageDescriptors { return t.f.ms.lazyInit(t, t.f.Messages) }
+func (t fileDesc) Extensions() pref.ExtensionDescriptors { return t.f.xs.lazyInit(t, t.f.Extensions) }
+func (t fileDesc) Services() pref.ServiceDescriptors { return t.f.ss.lazyInit(t, t.f.Services) }
+func (t fileDesc) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
+func (t fileDesc) ProtoType(pref.FileDescriptor) {}
+func (t fileDesc) ProtoInternal(pragma.DoNotImplement) {}
type messageMeta struct {
inheritedMeta
diff --git a/internal/prototype/type_test.go b/internal/prototype/type_test.go
index f2ce64b..fa4258e 100644
--- a/internal/prototype/type_test.go
+++ b/internal/prototype/type_test.go
@@ -634,33 +634,6 @@
},
},
},
- "DescriptorByName:": nil,
- "DescriptorByName:A": nil,
- "DescriptorByName:test": nil,
- "DescriptorByName:test.": nil,
- "DescriptorByName:test.A": M{"FullName": pref.FullName("test.A")},
- "DescriptorByName:test.A.key": M{"FullName": pref.FullName("test.A.key")},
- "DescriptorByName:test.A.A": nil,
- "DescriptorByName:test.A.field_one": nil,
- "DescriptorByName:test.B.field_one": M{"FullName": pref.FullName("test.B.field_one")},
- "DescriptorByName:test.B.O1": M{"FullName": pref.FullName("test.B.O1")},
- "DescriptorByName:test.B.O3": nil,
- "DescriptorByName:test.C.E1": M{"FullName": pref.FullName("test.C.E1")},
- "DescriptorByName:test.C.E1.FOO": nil,
- "DescriptorByName:test.C.FOO": M{"FullName": pref.FullName("test.C.FOO")},
- "DescriptorByName:test.C.Foo": nil,
- "DescriptorByName:test.C.BAZ": nil,
- "DescriptorByName:test.E1": M{"FullName": pref.FullName("test.E1")},
- "DescriptorByName:test.E1.FOO": nil,
- "DescriptorByName:test.FOO": M{"FullName": pref.FullName("test.FOO")},
- "DescriptorByName:test.Foo": nil,
- "DescriptorByName:test.BAZ": nil,
- "DescriptorByName:test.C.X": M{"FullName": pref.FullName("test.C.X")},
- "DescriptorByName:test.X": M{"FullName": pref.FullName("test.X")},
- "DescriptorByName:test.X.": nil,
- "DescriptorByName:test.S": M{"FullName": pref.FullName("test.S")},
- "DescriptorByName:test.S.M": M{"FullName": pref.FullName("test.S.M")},
- "DescriptorByName:test.M": nil,
}
checkAccessors(t, "", reflect.ValueOf(fd), want)
}
diff --git a/reflect/protodesc/protodesc.go b/reflect/protodesc/protodesc.go
index 41b4055..406e37f 100644
--- a/reflect/protodesc/protodesc.go
+++ b/reflect/protodesc/protodesc.go
@@ -311,18 +311,11 @@
return nil, errors.New("identifier name must be fully qualified with a leading dot: %v", s)
}
name := protoreflect.FullName(strings.TrimPrefix(s, "."))
- switch m, err := r.FindDescriptorByName(name); {
- case err == nil:
- m, ok := m.(protoreflect.MessageDescriptor)
- if !ok {
- return nil, errors.New("resolved wrong type: got %v, want message", typeName(m))
- }
- return m, nil
- case err == protoregistry.NotFound:
+ md, err := r.FindMessageByName(name)
+ if err != nil {
return prototype.PlaceholderMessage(name), nil
- default:
- return nil, err
}
+ return md, nil
}
func findEnumDescriptor(s string, r *protoregistry.Files) (protoreflect.EnumDescriptor, error) {
@@ -330,18 +323,11 @@
return nil, errors.New("identifier name must be fully qualified with a leading dot: %v", s)
}
name := protoreflect.FullName(strings.TrimPrefix(s, "."))
- switch e, err := r.FindDescriptorByName(name); {
- case err == nil:
- e, ok := e.(protoreflect.EnumDescriptor)
- if !ok {
- return nil, errors.New("resolved wrong type: got %T, want enum", typeName(e))
- }
- return e, nil
- case err == protoregistry.NotFound:
+ ed, err := r.FindEnumByName(name)
+ if err != nil {
return prototype.PlaceholderEnum(name), nil
- default:
- return nil, err
}
+ return ed, nil
}
func typeName(t protoreflect.Descriptor) string {
diff --git a/reflect/protoreflect/type.go b/reflect/protoreflect/type.go
index 66a3e5b..50324d6 100644
--- a/reflect/protoreflect/type.go
+++ b/reflect/protoreflect/type.go
@@ -129,9 +129,6 @@
Extensions() ExtensionDescriptors
// Services is a list of the top-level service declarations.
Services() ServiceDescriptors
- // DescriptorByName looks up any descriptor declared within this file
- // by full name. It returns nil if not found.
- DescriptorByName(FullName) Descriptor
isFileDescriptor
}
diff --git a/reflect/protoregistry/registry.go b/reflect/protoregistry/registry.go
index 7b0cd95..ee91406 100644
--- a/reflect/protoregistry/registry.go
+++ b/reflect/protoregistry/registry.go
@@ -18,7 +18,6 @@
import (
"fmt"
"reflect"
- "sort"
"strings"
"github.com/golang/protobuf/v2/internal/errors"
@@ -44,25 +43,22 @@
// descriptors contained within them.
// The Find and Range methods are safe for concurrent use.
type Files struct {
- filesByPackage filesByPackage
- filesByPath filesByPath
+ // The map of descs contains:
+ // EnumDescriptor
+ // MessageDescriptor
+ // ExtensionDescriptor
+ // ServiceDescriptor
+ // *packageDescriptor
+ //
+ // Note that files are stored as a slice, since a package may contain
+ // multiple files.
+ descs map[protoreflect.FullName]interface{}
+ filesByPath map[string][]protoreflect.FileDescriptor
}
-type (
- filesByPackage struct {
- // files is a list of files all in the same package.
- files []protoreflect.FileDescriptor
- // subs is a tree of files all in a sub-package scope.
- // It also maps all top-level identifiers declared in files
- // as the notProtoPackage sentinel value.
- subs map[protoreflect.Name]*filesByPackage // invariant: len(Name) > 0
- }
- filesByPath map[string][]protoreflect.FileDescriptor
-)
-
-// notProtoPackage is a sentinel value to indicate that some identifier maps
-// to an actual protobuf declaration and is not a sub-package.
-var notProtoPackage = new(filesByPackage)
+type packageDescriptor struct {
+ files []protoreflect.FileDescriptor
+}
// NewFiles returns a registry initialized with the provided set of files.
// If there are duplicates, the first one takes precedence.
@@ -88,119 +84,105 @@
//
// It is permitted for multiple files to have the same file path.
func (r *Files) Register(files ...protoreflect.FileDescriptor) error {
+ if r.descs == nil {
+ r.descs = map[protoreflect.FullName]interface{}{
+ "": &packageDescriptor{},
+ }
+ r.filesByPath = make(map[string][]protoreflect.FileDescriptor)
+ }
var firstErr error
-fileLoop:
for _, file := range files {
- if file.IsPlaceholder() {
- continue // TODO: Should this be an error instead?
+ if err := r.registerFile(file); err != nil && firstErr == nil {
+ firstErr = err
}
-
- // Register the file into the filesByPackage tree.
- //
- // The prototype package validates that a FileDescriptor is internally
- // consistent such it does not have conflicts within itself.
- // However, we need to ensure that the inserted file does not conflict
- // with other previously inserted files.
- pkg := file.Package()
- root := &r.filesByPackage
- for len(pkg) > 0 {
- var prefix protoreflect.Name
- prefix, pkg = splitPrefix(pkg)
-
- // Add a new sub-package segment.
- switch nextRoot := root.subs[prefix]; nextRoot {
- case nil:
- nextRoot = new(filesByPackage)
- if root.subs == nil {
- root.subs = make(map[protoreflect.Name]*filesByPackage)
- }
- root.subs[prefix] = nextRoot
- root = nextRoot
- case notProtoPackage:
- if firstErr == nil {
- name := strings.TrimSuffix(strings.TrimSuffix(string(file.Package()), string(pkg)), ".")
- firstErr = errors.New("file %q has a name conflict over %v", file.Path(), name)
- }
- continue fileLoop
- default:
- root = nextRoot
- }
- }
-
- // Check for top-level conflicts within the same package.
- // The current file cannot add any top-level declaration that conflict
- // with another top-level declaration or sub-package name.
- var conflicts []protoreflect.Name
- rangeTopLevelDeclarations(file, func(s protoreflect.Name) {
- if root.subs[s] == nil {
- if root.subs == nil {
- root.subs = make(map[protoreflect.Name]*filesByPackage)
- }
- root.subs[s] = notProtoPackage
- } else {
- conflicts = append(conflicts, s)
- }
- })
- if len(conflicts) > 0 {
- // Remove inserted identifiers to make registration failure atomic.
- sort.Slice(conflicts, func(i, j int) bool { return conflicts[i] < conflicts[j] })
- rangeTopLevelDeclarations(file, func(s protoreflect.Name) {
- i := sort.Search(len(conflicts), func(i int) bool { return conflicts[i] >= s })
- if has := i < len(conflicts) && conflicts[i] == s; !has {
- delete(root.subs, s) // remove everything not in conflicts
- }
- })
-
- if firstErr == nil {
- name := file.Package().Append(conflicts[0])
- firstErr = errors.New("file %q has a name conflict over %v", file.Path(), name)
- }
- continue fileLoop
- }
- root.files = append(root.files, file)
-
- // Register the file into the filesByPath map.
- //
- // There is no check for conflicts in file path since the path is
- // heavily dependent on how protoc is invoked. When protoc is being
- // invoked by different parties in a distributed manner, it is
- // unreasonable to assume nor ensure that the path is unique.
- if r.filesByPath == nil {
- r.filesByPath = make(filesByPath)
- }
- r.filesByPath[file.Path()] = append(r.filesByPath[file.Path()], file)
}
return firstErr
}
+func (r *Files) registerFile(file protoreflect.FileDescriptor) error {
+ for name := file.Package(); name != ""; name = name.Parent() {
+ switch r.descs[name].(type) {
+ case nil, *packageDescriptor:
+ default:
+ return errors.New("file %q has a name conflict over %v", file.Path(), name)
+ }
+ }
+ var err error
+ rangeRegisteredDescriptors(file, func(desc protoreflect.Descriptor) {
+ if r.descs[desc.FullName()] != nil {
+ err = errors.New("file %q has a name conflict over %v", file.Path(), desc.FullName())
+ }
+ })
+ if err != nil {
+ return err
+ }
-// FindDescriptorByName looks up any descriptor (except files) by its full name.
-// Files are not handled since multiple file descriptors may belong in
-// the same package and have the same full name (see RangeFilesByPackage).
+ path := file.Path()
+ r.filesByPath[path] = append(r.filesByPath[path], file)
+
+ for name := file.Package(); name != ""; name = name.Parent() {
+ if r.descs[name] == nil {
+ r.descs[name] = &packageDescriptor{}
+ }
+ }
+ p := r.descs[file.Package()].(*packageDescriptor)
+ p.files = append(p.files, file)
+ rangeRegisteredDescriptors(file, func(desc protoreflect.Descriptor) {
+ r.descs[desc.FullName()] = desc
+ })
+ return nil
+}
+
+// FindEnumByName looks up an enum by the enum's full name.
//
-// This return (nil, NotFound) if not found.
-func (r *Files) FindDescriptorByName(name protoreflect.FullName) (protoreflect.Descriptor, error) {
+// This returns (nil, NotFound) if not found.
+func (r *Files) FindEnumByName(name protoreflect.FullName) (protoreflect.EnumDescriptor, error) {
if r == nil {
return nil, NotFound
}
- pkg := name
- root := &r.filesByPackage
- for len(pkg) > 0 {
- var prefix protoreflect.Name
- prefix, pkg = splitPrefix(pkg)
- switch nextRoot := root.subs[prefix]; nextRoot {
- case nil:
- return nil, NotFound
- case notProtoPackage:
- // Search current root's package for the descriptor.
- for _, fd := range root.files {
- if d := fd.DescriptorByName(name); d != nil {
- return d, nil
- }
- }
- return nil, NotFound
- default:
- root = nextRoot
- }
+ if d, ok := r.descs[name].(protoreflect.EnumDescriptor); ok {
+ return d, nil
+ }
+ return nil, NotFound
+}
+
+// FindMessageByName looks up a message by the message's full name.
+//
+// This returns (nil, NotFound) if not found.
+func (r *Files) FindMessageByName(name protoreflect.FullName) (protoreflect.MessageDescriptor, error) {
+ if r == nil {
+ return nil, NotFound
+ }
+ if d, ok := r.descs[name].(protoreflect.MessageDescriptor); ok {
+ return d, nil
+ }
+ return nil, NotFound
+}
+
+// FindExtensionByName looks up an extension field by the field's full name.
+// Note that this is the full name of the field as determined by
+// where the extension is declared and is unrelated to the full name of the
+// message being extended.
+//
+// This returns (nil, NotFound) if not found.
+func (r *Files) FindExtensionByName(name protoreflect.FullName) (protoreflect.ExtensionDescriptor, error) {
+ if r == nil {
+ return nil, NotFound
+ }
+ if d, ok := r.descs[name].(protoreflect.ExtensionDescriptor); ok {
+ return d, nil
+ }
+ return nil, NotFound
+}
+
+// FindServiceByName looks up a service by the service's full name.
+//
+// This returns (nil, NotFound) if not found.
+func (r *Files) FindServiceByName(name protoreflect.FullName) (protoreflect.ServiceDescriptor, error) {
+ if r == nil {
+ return nil, NotFound
+ }
+ if d, ok := r.descs[name].(protoreflect.ServiceDescriptor); ok {
+ return d, nil
}
return nil, NotFound
}
@@ -208,45 +190,18 @@
// RangeFiles iterates over all registered files.
// The iteration order is undefined.
func (r *Files) RangeFiles(f func(protoreflect.FileDescriptor) bool) {
- r.RangeFilesByPackage("", f) // empty package is a prefix for all packages
-}
-
-// RangeFilesByPackage iterates over all registered files filtered by
-// the given proto package prefix. It iterates over files with an exact package
-// match before iterating over files with general prefix match.
-// The iteration order is undefined within exact matches or prefix matches.
-func (r *Files) RangeFilesByPackage(pkg protoreflect.FullName, f func(protoreflect.FileDescriptor) bool) {
if r == nil {
return
}
- if strings.HasSuffix(string(pkg), ".") {
- return // avoid edge case where splitPrefix allows trailing dot
- }
- root := &r.filesByPackage
- for len(pkg) > 0 && root != nil {
- var prefix protoreflect.Name
- prefix, pkg = splitPrefix(pkg)
- root = root.subs[prefix]
- }
- rangeFiles(root, f)
-}
-func rangeFiles(fs *filesByPackage, f func(protoreflect.FileDescriptor) bool) bool {
- if fs == nil {
- return true
- }
- // Iterate over exact matches.
- for _, fd := range fs.files { // TODO: iterate non-deterministically
- if !f(fd) {
- return false
+ for _, d := range r.descs {
+ if p, ok := d.(*packageDescriptor); ok {
+ for _, file := range p.files {
+ if !f(file) {
+ return
+ }
+ }
}
}
- // Iterate over prefix matches.
- for _, fs := range fs.subs {
- if !rangeFiles(fs, f) {
- return false
- }
- }
- return true
}
// RangeFilesByPath iterates over all registered files filtered by
@@ -255,46 +210,57 @@
if r == nil {
return
}
- for _, fd := range r.filesByPath[path] { // TODO: iterate non-deterministically
- if !f(fd) {
+ for _, file := range r.filesByPath[path] {
+ if !f(file) {
return
}
}
}
-func splitPrefix(name protoreflect.FullName) (protoreflect.Name, protoreflect.FullName) {
- if i := strings.IndexByte(string(name), '.'); i >= 0 {
- return protoreflect.Name(name[:i]), name[i+len("."):]
+// RangeFilesByPackage iterates over all registered files in a give proto package.
+// The iteration order is undefined.
+func (r *Files) RangeFilesByPackage(name protoreflect.FullName, f func(protoreflect.FileDescriptor) bool) {
+ if r == nil {
+ return
}
- return protoreflect.Name(name), ""
-}
-
-// rangeTopLevelDeclarations iterates over the name of all top-level
-// declarations in the proto file.
-func rangeTopLevelDeclarations(fd protoreflect.FileDescriptor, f func(protoreflect.Name)) {
- for i := 0; i < fd.Enums().Len(); i++ {
- e := fd.Enums().Get(i)
- f(e.Name())
-
- // TODO: Drop ranging over top-level enum values. The current
- // implementation of fileinit.FileBuilder does not initialize the names
- // for enum values in enums. Doing so reduces init time considerably.
- // If we drop this, it means that conflict checks in the registry
- // is not complete. However, this may be okay since the most common
- // reason for a conflict is due to vendored proto files, which are
- // most certainly going to have a name conflict on the parent enum.
- for i := 0; i < e.Values().Len(); i++ {
- f(e.Values().Get(i).Name())
+ p, ok := r.descs[name].(*packageDescriptor)
+ if !ok {
+ return
+ }
+ for _, file := range p.files {
+ if !f(file) {
+ return
}
}
- for i := 0; i < fd.Messages().Len(); i++ {
- f(fd.Messages().Get(i).Name())
+}
+
+// rangeRegisteredDescriptors iterates over all descriptors in a file which
+// will be entered into the registry: enums, messages, extensions, and services.
+func rangeRegisteredDescriptors(fd protoreflect.FileDescriptor, f func(protoreflect.Descriptor)) {
+ rangeRegisteredMessageDescriptors(fd.Messages(), f)
+ for i := 0; i < fd.Enums().Len(); i++ {
+ e := fd.Enums().Get(i)
+ f(e)
}
for i := 0; i < fd.Extensions().Len(); i++ {
- f(fd.Extensions().Get(i).Name())
+ f(fd.Extensions().Get(i))
}
for i := 0; i < fd.Services().Len(); i++ {
- f(fd.Services().Get(i).Name())
+ f(fd.Services().Get(i))
+ }
+}
+func rangeRegisteredMessageDescriptors(messages protoreflect.MessageDescriptors, f func(protoreflect.Descriptor)) {
+ for i := 0; i < messages.Len(); i++ {
+ md := messages.Get(i)
+ f(md)
+ rangeRegisteredMessageDescriptors(md.Messages(), f)
+ for i := 0; i < md.Enums().Len(); i++ {
+ e := md.Enums().Get(i)
+ f(e)
+ }
+ for i := 0; i < md.Extensions().Len(); i++ {
+ f(md.Extensions().Get(i))
+ }
}
}
diff --git a/reflect/protoregistry/registry_test.go b/reflect/protoregistry/registry_test.go
index c668dfa..8fbd0e6 100644
--- a/reflect/protoregistry/registry_test.go
+++ b/reflect/protoregistry/registry_test.go
@@ -16,6 +16,7 @@
pref "github.com/golang/protobuf/v2/reflect/protoreflect"
preg "github.com/golang/protobuf/v2/reflect/protoregistry"
+ test2pb "github.com/golang/protobuf/v2/internal/testprotos/test"
testpb "github.com/golang/protobuf/v2/reflect/protoregistry/testprotos"
)
@@ -29,10 +30,6 @@
inFile *ptype.File
wantErr string
}
- testFindDesc struct {
- inName pref.FullName
- wantOk bool
- }
testRangePkg struct {
inPkg pref.FullName
wantFiles []file
@@ -45,7 +42,6 @@
tests := []struct {
files []testFile
- findDescs []testFindDesc
rangePkgs []testRangePkg
rangePaths []testRangePath
}{{
@@ -63,28 +59,21 @@
inPkg: "nothing",
}, {
inPkg: "",
- wantFiles: []file{
- {"", "foo.bar"},
- {"", "foo.bar"},
- {"foo/bar/test.proto", "foo.bar.baz"},
- {"foo/bar/test.proto", "my.test"},
- {"", "my.test.package"},
- {"foo/bar/baz/../test.proto", "my.test"},
- },
}, {
inPkg: ".",
}, {
inPkg: "foo",
- wantFiles: []file{
- {"", "foo.bar"},
- {"", "foo.bar"},
- {"foo/bar/test.proto", "foo.bar.baz"},
- },
}, {
inPkg: "foo.",
}, {
inPkg: "foo..",
}, {
+ inPkg: "foo.bar",
+ wantFiles: []file{
+ {"", "foo.bar"},
+ {"", "foo.bar"},
+ },
+ }, {
inPkg: "foo.bar.baz",
wantFiles: []file{
{"foo/bar/test.proto", "foo.bar.baz"},
@@ -182,23 +171,6 @@
}},
},
}, {
- // Conflict over a single declaration.
- inFile: &ptype.File{
- Syntax: pref.Proto2,
- Package: "fizz.buzz",
- Enums: []ptype.Enum{{
- Name: "Enum1",
- Values: []ptype.EnumValue{{Name: "EnumValue1", Number: 0}},
- }, {
- Name: "Enum2",
- Values: []ptype.EnumValue{{Name: "EnumValue2", Number: 0}},
- }, {
- Name: "Enum3",
- Values: []ptype.EnumValue{{Name: "Enum", Number: 0}}, // conflict
- }},
- },
- wantErr: "name conflict over fizz.buzz.Enum",
- }, {
// Previously failed registration should not pollute the namespace.
inFile: &ptype.File{
Syntax: pref.Proto2,
@@ -226,28 +198,6 @@
}},
},
}},
-
- findDescs: []testFindDesc{
- {"", false},
- {"Enum", false},
- {"Message", true},
- {"Message.", false},
- {"Message.Message", true},
- {"Message.Message.Message", true},
- {"Message.Message.Message.Message", false},
- {"fizz.buzz", false},
- {"fizz.buzz.Enum", true},
- {"fizz.buzz.Enum1", true},
- {"fizz.buzz.Enum1.EnumValue", false},
- {"fizz.buzz.EnumValue", true},
- {"fizz.buzz.Message", true},
- {"fizz.buzz.Message.Field", true},
- {"fizz.buzz.Message.Oneof", true},
- {"fizz.buzz.Extension", true},
- {"fizz.buzz.Service", true},
- {"fizz.buzz.Service.Method", true},
- {"fizz.buzz.Method", false},
- },
}}
sortFiles := cmpopts.SortSlices(func(x, y file) bool {
@@ -267,29 +217,6 @@
}
}
- for _, tc := range tt.findDescs {
- got, err := files.FindDescriptorByName(tc.inName)
- if (got == nil) == (err == nil) {
- if tc.wantOk {
- t.Errorf("FindDescriptorByName(%v) = (%v, %v), want (non-nil, nil)", tc.inName, got, err)
- } else {
- t.Errorf("FindDescriptorByName(%v) = (%v, %v), want (nil, NotFound)", tc.inName, got, err)
- }
- }
-
- gotName := pref.FullName("<nil>")
- if got != nil {
- gotName = got.FullName()
- }
- wantName := pref.FullName("<nil>")
- if tc.wantOk {
- wantName = tc.inName
- }
- if gotName != wantName {
- t.Errorf("FindDescriptorByName(%v) = %v, want %v", tc.inName, gotName, wantName)
- }
- }
-
for _, tc := range tt.rangePkgs {
var gotFiles []file
files.RangeFilesByPackage(tc.inPkg, func(fd pref.FileDescriptor) bool {
@@ -315,6 +242,64 @@
}
}
+func TestFilesLookup(t *testing.T) {
+ files := []pref.FileDescriptor{
+ test2pb.File_test_test_proto,
+ test2pb.File_test_test_import_proto,
+ }
+ r := preg.NewFiles(files...)
+ checkEnums := func(enums pref.EnumDescriptors) {
+ for i := 0; i < enums.Len(); i++ {
+ want := enums.Get(i)
+ if got, err := r.FindEnumByName(want.FullName()); err != nil {
+ t.Errorf("FindEnumByName(%q): unexpected error: %v", want.FullName(), err)
+ } else if got != want {
+ t.Errorf("FindEnumByName(%q): found descriptor %v (%p), %p", want.FullName(), got.FullName(), got, want)
+ }
+ }
+ }
+ checkExtensions := func(exts pref.ExtensionDescriptors) {
+ for i := 0; i < exts.Len(); i++ {
+ want := exts.Get(i)
+ if got, err := r.FindExtensionByName(want.FullName()); err != nil {
+ t.Errorf("FindExtensionByName(%q): unexpected error: %v", want.FullName(), err)
+ } else if got != want {
+ t.Errorf("FindExtensionByName(%q): found descriptor %v (%p), %p", want.FullName(), got.FullName(), got, want)
+ }
+ }
+ }
+ var checkMessages func(pref.MessageDescriptors)
+ checkMessages = func(messages pref.MessageDescriptors) {
+ for i := 0; i < messages.Len(); i++ {
+ want := messages.Get(i)
+ if got, err := r.FindMessageByName(want.FullName()); err != nil {
+ t.Errorf("FindMessageByName(%q): unexpected error: %v", want.FullName(), err)
+ } else if got != want {
+ t.Errorf("FindMessageByName(%q): found descriptor %v (%p), %p", want.FullName(), got.FullName(), got, want)
+ }
+ checkEnums(want.Enums())
+ checkExtensions(want.Extensions())
+ checkMessages(want.Messages())
+ }
+ }
+ checkServices := func(services pref.ServiceDescriptors) {
+ for i := 0; i < services.Len(); i++ {
+ want := services.Get(i)
+ if got, err := r.FindServiceByName(want.FullName()); err != nil {
+ t.Errorf("FindServiceByName(%q): unexpected error: %v", want.FullName(), err)
+ } else if got != want {
+ t.Errorf("FindServiceByName(%q): found descriptor %v (%p), %p", want.FullName(), got.FullName(), got, want)
+ }
+ }
+ }
+ for _, fd := range files {
+ checkEnums(fd.Enums())
+ checkExtensions(fd.Extensions())
+ checkMessages(fd.Messages())
+ checkServices(fd.Services())
+ }
+}
+
func TestTypes(t *testing.T) {
// Suffix 1 in registry, 2 in parent, 3 in resolver.
mt1 := (&testpb.Message1{}).ProtoReflect().Type()