ptypes: optimize Is to avoid prefix scan (#601)
diff --git a/ptypes/any.go b/ptypes/any.go
index b2af97f..70276e8 100644
--- a/ptypes/any.go
+++ b/ptypes/any.go
@@ -130,10 +130,12 @@
// Is returns true if any value contains a given message type.
func Is(any *any.Any, pb proto.Message) bool {
- aname, err := AnyMessageName(any)
- if err != nil {
+ // The following is equivalent to AnyMessageName(any) == proto.MessageName(pb),
+ // but it avoids scanning TypeUrl for the slash.
+ if any == nil {
return false
}
-
- return aname == proto.MessageName(pb)
+ name := proto.MessageName(pb)
+ prefix := len(any.TypeUrl) - len(name)
+ return prefix >= 1 && any.TypeUrl[prefix-1] == '/' && any.TypeUrl[prefix:] == name
}
diff --git a/ptypes/any_test.go b/ptypes/any_test.go
index ed675b4..163ca31 100644
--- a/ptypes/any_test.go
+++ b/ptypes/any_test.go
@@ -60,8 +60,13 @@
t.Fatal(err)
}
if Is(a, &pb.DescriptorProto{}) {
+ // No spurious match for message names of different length.
t.Error("FileDescriptorProto is not a DescriptorProto, but Is says it is")
}
+ if Is(a, &pb.EnumDescriptorProto{}) {
+ // No spurious match for message names of equal length.
+ t.Error("FileDescriptorProto is not an EnumDescriptorProto, but Is says it is")
+ }
if !Is(a, &pb.FileDescriptorProto{}) {
t.Error("FileDescriptorProto is indeed a FileDescriptorProto, but Is says it is not")
}
@@ -75,6 +80,22 @@
}
}
+
+func TestIsCornerCases(t *testing.T) {
+ m := &pb.FileDescriptorProto{}
+ if Is(nil, m) {
+ t.Errorf("message with nil type url incorrectly claimed to be %q", proto.MessageName(m))
+ }
+ noPrefix := &any.Any{TypeUrl: proto.MessageName(m)}
+ if Is(noPrefix, m) {
+ t.Errorf("message with type url %q incorrectly claimed to be %q", noPrefix.TypeUrl, proto.MessageName(m))
+ }
+ shortPrefix := &any.Any{TypeUrl: "/" + proto.MessageName(m)}
+ if !Is(shortPrefix, m) {
+ t.Errorf("message with type url %q didn't satisfy Is for type %q", shortPrefix.TypeUrl, proto.MessageName(m))
+ }
+}
+
func TestUnmarshalDynamic(t *testing.T) {
want := &pb.FileDescriptorProto{Name: proto.String("foo")}
a, err := MarshalAny(want)
@@ -111,3 +132,24 @@
t.Errorf("got no error for an attempt to create a message of type %q, which shouldn't be linked in", a.TypeUrl)
}
}
+
+func TestEmptyCornerCases(t *testing.T) {
+ _, err := Empty(nil)
+ if err == nil {
+ t.Error("expected Empty for nil to fail")
+ }
+ want := &pb.FileDescriptorProto{}
+ noPrefix := &any.Any{TypeUrl: proto.MessageName(want)}
+ _, err = Empty(noPrefix)
+ if err == nil {
+ t.Errorf("expected Empty for any type %q to fail", noPrefix.TypeUrl)
+ }
+ shortPrefix := &any.Any{TypeUrl: "/" + proto.MessageName(want)}
+ got, err := Empty(shortPrefix)
+ if err != nil {
+ t.Errorf("Empty for any type %q failed: %s", shortPrefix.TypeUrl, err)
+ }
+ if !proto.Equal(got, want) {
+ t.Errorf("Empty for any type %q differs, got %q, want %q", shortPrefix.TypeUrl, got, want)
+ }
+}
