reflect/protoregistry: add conflict override
The ignoreConflict function provides the ability to ignore certain conflicts.
By default, all conflicts are ignored with a log message produced instead.
Change-Id: I67fe56eef492e12421e5c8cb8d618dc2a46c82ed
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186658
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/internal/filedesc/build.go b/internal/filedesc/build.go
index 52cbdf1..427e26e 100644
--- a/internal/filedesc/build.go
+++ b/internal/filedesc/build.go
@@ -6,8 +6,6 @@
package filedesc
import (
- "log"
-
"google.golang.org/protobuf/internal/encoding/wire"
"google.golang.org/protobuf/internal/fieldnum"
"google.golang.org/protobuf/reflect/protoreflect"
@@ -107,7 +105,7 @@
out.Services = fd.allServices
if err := db.FileRegistry.Register(fd); err != nil {
- CheckRegistryError(err)
+ panic(err)
}
return out
}
@@ -152,13 +150,3 @@
}
}
}
-
-// CheckRegistryError handles registration errors.
-// It is a variable so that its behavior can be replaced in another source file.
-var CheckRegistryError = func(err error) {
- log.Printf(""+
- "WARNING: %v\n"+
- "A future release will panic on registration conflicts.\n"+
- // TODO: Add a URL pointing to documentation on how to resolve conflicts.
- "\n", err)
-}
diff --git a/internal/filetype/build.go b/internal/filetype/build.go
index 3428040..fa1bb9a 100644
--- a/internal/filetype/build.go
+++ b/internal/filetype/build.go
@@ -155,7 +155,7 @@
// Register enum types.
if err := tb.TypeRegistry.Register(&out.Enums[i]); err != nil {
- fdesc.CheckRegistryError(err)
+ panic(err)
}
}
}
@@ -183,7 +183,7 @@
// Register message types.
if err := tb.TypeRegistry.Register(&out.Messages[i]); err != nil {
- fdesc.CheckRegistryError(err)
+ panic(err)
}
}
@@ -251,7 +251,7 @@
// Register extension types.
if err := tb.TypeRegistry.Register(&out.Extensions[i]); err != nil {
- fdesc.CheckRegistryError(err)
+ panic(err)
}
}
}
diff --git a/reflect/protoregistry/registry.go b/reflect/protoregistry/registry.go
index 33552f1..4abfc10 100644
--- a/reflect/protoregistry/registry.go
+++ b/reflect/protoregistry/registry.go
@@ -17,6 +17,7 @@
import (
"fmt"
+ "log"
"reflect"
"strings"
@@ -24,6 +25,18 @@
"google.golang.org/protobuf/reflect/protoreflect"
)
+// ignoreConflict reports whether to ignore a registration conflict
+// given the descriptor being registered and the error.
+// It is a variable so that the behavior is easily overridden in another file.
+var ignoreConflict = func(d protoreflect.Descriptor, err error) bool {
+ log.Printf(""+
+ "WARNING: %v\n"+
+ "A future release will panic on registration conflicts.\n"+
+ // TODO: Add a URL pointing to documentation on how to resolve conflicts.
+ "\n", err)
+ return true
+}
+
// GlobalFiles is a global registry of file descriptors.
var GlobalFiles *Files = new(Files)
@@ -92,25 +105,38 @@
path := fd.Path()
if prev := r.filesByPath[path]; prev != nil {
err := errors.New("file %q is already registered", fd.Path())
- return amendErrorWithCaller(err, prev, fd)
+ err = amendErrorWithCaller(err, prev, fd)
+ if r == GlobalFiles && ignoreConflict(fd, err) {
+ err = nil
+ }
+ return err
}
for name := fd.Package(); name != ""; name = name.Parent() {
switch prev := r.descsByName[name]; prev.(type) {
case nil, *packageDescriptor:
default:
- err := errors.New("file %q has a name conflict over %v", fd.Path(), name)
- return amendErrorWithCaller(err, prev, fd)
+ err := errors.New("file %q has a package name conflict over %v", fd.Path(), name)
+ err = amendErrorWithCaller(err, prev, fd)
+ if r == GlobalFiles && ignoreConflict(fd, err) {
+ err = nil
+ }
+ return err
}
}
var err error
+ var hasConflict bool
rangeTopLevelDescriptors(fd, func(d protoreflect.Descriptor) {
if prev := r.descsByName[d.FullName()]; prev != nil {
+ hasConflict = true
err = errors.New("file %q has a name conflict over %v", fd.Path(), d.FullName())
err = amendErrorWithCaller(err, prev, fd)
+ if r == GlobalFiles && ignoreConflict(d, err) {
+ err = nil
+ }
}
})
- if err != nil {
+ if hasConflict {
return err
}
@@ -291,6 +317,7 @@
// Type is an interface satisfied by protoreflect.EnumType,
// protoreflect.MessageType, or protoreflect.ExtensionType.
type Type interface {
+ protoreflect.Descriptor
GoType() reflect.Type
}
@@ -413,9 +440,13 @@
panic(fmt.Sprintf("invalid type: %T", t))
}
if prev := r.typesByName[name]; prev != nil {
+ err := errors.New("%v %v is already registered", typeName(typ), name)
+ err = amendErrorWithCaller(err, prev, typ)
+ if r == GlobalTypes && ignoreConflict(typ, err) {
+ err = nil
+ }
if firstErr == nil {
- err := errors.New("%v %v is already registered", typeName(typ), name)
- firstErr = amendErrorWithCaller(err, prev, typ)
+ firstErr = err
}
continue typeLoop
}
@@ -425,9 +456,13 @@
field := xt.Number()
message := xt.ContainingMessage().FullName()
if prev := r.extensionsByMessage[message][field]; prev != nil {
+ err := errors.New("extension number %d is already registered on message %v", field, message)
+ err = amendErrorWithCaller(err, prev, typ)
+ if r == GlobalTypes && ignoreConflict(typ, err) {
+ err = nil
+ }
if firstErr == nil {
- err := errors.New("extension number %d is already registered on message %v", field, message)
- firstErr = amendErrorWithCaller(err, prev, typ)
+ firstErr = err
}
continue typeLoop
}
diff --git a/reflect/protoregistry/registry_test.go b/reflect/protoregistry/registry_test.go
index 3d87651..63dd1ba 100644
--- a/reflect/protoregistry/registry_test.go
+++ b/reflect/protoregistry/registry_test.go
@@ -129,7 +129,7 @@
inFile: mustMakeFile(`syntax:"proto2" name:"test2a.proto" enum_type:[{name:"foo" value:[{name:"VALUE" number:0}]}]`),
}, {
inFile: mustMakeFile(`syntax:"proto2" name:"test2b.proto" package:"foo.bar.baz"`),
- wantErr: `file "test2b.proto" has a name conflict over foo`,
+ wantErr: `file "test2b.proto" has a package name conflict over foo`,
}},
}, {
// Test when new enum conflicts with existing enum in same package.
