proto: add Clone function and MergeOptions.Clone method
We resisted adding Clone for a while since:
* It is a function that is perfectly suited for generics.
However, generics probably still won't be available in Go for some time
and it is impractical to block addition of this function when it is very
widely used and will be necessary for the v1 to v2 migration.
* In the past, there was no protoreflect.Message.IsValid, so there was
no proper API to detect invalid top-level messages and return them as such.
Since Clone relies on certain properties about proper round-tripping
of ProtoMessage.ProtoReflect <-> Message.Interface, we add a test
in testing/prototest to check for this.
Change-Id: Ic492b68f27b8b88322a6a3fa3a5e492228db79d9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213297
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/reflect/protodesc/desc.go b/reflect/protodesc/desc.go
index 5a65e9a..84f2919 100644
--- a/reflect/protodesc/desc.go
+++ b/reflect/protodesc/desc.go
@@ -11,6 +11,7 @@
"google.golang.org/protobuf/internal/filedesc"
"google.golang.org/protobuf/internal/pragma"
"google.golang.org/protobuf/internal/strs"
+ "google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
@@ -98,7 +99,7 @@
return nil, errors.New("invalid package: %q", f.L1.Package)
}
if opts := fd.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.FileOptions)
+ opts = proto.Clone(opts).(*descriptorpb.FileOptions)
f.L2.Options = func() protoreflect.ProtoMessage { return opts }
}
diff --git a/reflect/protodesc/desc_init.go b/reflect/protodesc/desc_init.go
index ef2d071..ddb6c59 100644
--- a/reflect/protodesc/desc_init.go
+++ b/reflect/protodesc/desc_init.go
@@ -8,6 +8,7 @@
"google.golang.org/protobuf/internal/errors"
"google.golang.org/protobuf/internal/filedesc"
"google.golang.org/protobuf/internal/strs"
+ "google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/descriptorpb"
@@ -24,7 +25,7 @@
return nil, err
}
if opts := ed.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.EnumOptions)
+ opts = proto.Clone(opts).(*descriptorpb.EnumOptions)
e.L2.Options = func() protoreflect.ProtoMessage { return opts }
}
for _, s := range ed.GetReservedName() {
@@ -51,7 +52,7 @@
return nil, err
}
if opts := vd.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.EnumValueOptions)
+ opts = proto.Clone(opts).(*descriptorpb.EnumValueOptions)
v.L1.Options = func() protoreflect.ProtoMessage { return opts }
}
v.L1.Number = protoreflect.EnumNumber(vd.GetNumber())
@@ -68,7 +69,7 @@
return nil, err
}
if opts := md.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.MessageOptions)
+ opts = proto.Clone(opts).(*descriptorpb.MessageOptions)
m.L2.Options = func() protoreflect.ProtoMessage { return opts }
m.L1.IsMapEntry = opts.GetMapEntry()
m.L1.IsMessageSet = opts.GetMessageSetWireFormat()
@@ -89,7 +90,7 @@
})
var optsFunc func() protoreflect.ProtoMessage
if opts := xr.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.ExtensionRangeOptions)
+ opts = proto.Clone(opts).(*descriptorpb.ExtensionRangeOptions)
optsFunc = func() protoreflect.ProtoMessage { return opts }
}
m.L2.ExtensionRangeOptions = append(m.L2.ExtensionRangeOptions, optsFunc)
@@ -121,7 +122,7 @@
return nil, err
}
if opts := fd.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.FieldOptions)
+ opts = proto.Clone(opts).(*descriptorpb.FieldOptions)
f.L1.Options = func() protoreflect.ProtoMessage { return opts }
f.L1.IsWeak = opts.GetWeak()
f.L1.HasPacked = opts.Packed != nil
@@ -147,7 +148,7 @@
return nil, err
}
if opts := od.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.OneofOptions)
+ opts = proto.Clone(opts).(*descriptorpb.OneofOptions)
o.L1.Options = func() protoreflect.ProtoMessage { return opts }
}
}
@@ -163,7 +164,7 @@
return nil, err
}
if opts := xd.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.FieldOptions)
+ opts = proto.Clone(opts).(*descriptorpb.FieldOptions)
x.L2.Options = func() protoreflect.ProtoMessage { return opts }
x.L2.IsPacked = opts.GetPacked()
}
@@ -188,7 +189,7 @@
return nil, err
}
if opts := sd.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.ServiceOptions)
+ opts = proto.Clone(opts).(*descriptorpb.ServiceOptions)
s.L2.Options = func() protoreflect.ProtoMessage { return opts }
}
if s.L2.Methods.List, err = r.initMethodsFromDescriptorProto(sd.GetMethod(), s, sb); err != nil {
@@ -206,7 +207,7 @@
return nil, err
}
if opts := md.GetOptions(); opts != nil {
- opts = clone(opts).(*descriptorpb.MethodOptions)
+ opts = proto.Clone(opts).(*descriptorpb.MethodOptions)
m.L1.Options = func() protoreflect.ProtoMessage { return opts }
}
m.L1.IsStreamingClient = md.GetClientStreaming()
diff --git a/reflect/protodesc/file_test.go b/reflect/protodesc/file_test.go
index 9ee6c8d..934b32f 100644
--- a/reflect/protodesc/file_test.go
+++ b/reflect/protodesc/file_test.go
@@ -26,9 +26,7 @@
}
func cloneFile(in *descriptorpb.FileDescriptorProto) *descriptorpb.FileDescriptorProto {
- out := new(descriptorpb.FileDescriptorProto)
- proto.Merge(out, in)
- return out
+ return proto.Clone(in).(*descriptorpb.FileDescriptorProto)
}
var (
diff --git a/reflect/protodesc/proto.go b/reflect/protodesc/proto.go
index 380fd70..1b0de15 100644
--- a/reflect/protodesc/proto.go
+++ b/reflect/protodesc/proto.go
@@ -6,7 +6,6 @@
import (
"fmt"
- "reflect"
"strings"
"google.golang.org/protobuf/internal/encoding/defval"
@@ -22,7 +21,7 @@
p := &descriptorpb.FileDescriptorProto{
Name: proto.String(file.Path()),
Package: proto.String(string(file.Package())),
- Options: clone(file.Options()).(*descriptorpb.FileOptions),
+ Options: proto.Clone(file.Options()).(*descriptorpb.FileOptions),
}
for i, imports := 0, file.Imports(); i < imports.Len(); i++ {
imp := imports.Get(i)
@@ -79,7 +78,7 @@
func ToDescriptorProto(message protoreflect.MessageDescriptor) *descriptorpb.DescriptorProto {
p := &descriptorpb.DescriptorProto{
Name: proto.String(string(message.Name())),
- Options: clone(message.Options()).(*descriptorpb.MessageOptions),
+ Options: proto.Clone(message.Options()).(*descriptorpb.MessageOptions),
}
for i, fields := 0, message.Fields(); i < fields.Len(); i++ {
p.Field = append(p.Field, ToFieldDescriptorProto(fields.Get(i)))
@@ -98,7 +97,7 @@
p.ExtensionRange = append(p.ExtensionRange, &descriptorpb.DescriptorProto_ExtensionRange{
Start: proto.Int32(int32(xrange[0])),
End: proto.Int32(int32(xrange[1])),
- Options: clone(message.ExtensionRangeOptions(i)).(*descriptorpb.ExtensionRangeOptions),
+ Options: proto.Clone(message.ExtensionRangeOptions(i)).(*descriptorpb.ExtensionRangeOptions),
})
}
for i, oneofs := 0, message.Oneofs(); i < oneofs.Len(); i++ {
@@ -124,7 +123,7 @@
Name: proto.String(string(field.Name())),
Number: proto.Int32(int32(field.Number())),
Label: descriptorpb.FieldDescriptorProto_Label(field.Cardinality()).Enum(),
- Options: clone(field.Options()).(*descriptorpb.FieldOptions),
+ Options: proto.Clone(field.Options()).(*descriptorpb.FieldOptions),
}
if field.IsExtension() {
p.Extendee = fullNameOf(field.ContainingMessage())
@@ -161,7 +160,7 @@
func ToOneofDescriptorProto(oneof protoreflect.OneofDescriptor) *descriptorpb.OneofDescriptorProto {
return &descriptorpb.OneofDescriptorProto{
Name: proto.String(string(oneof.Name())),
- Options: clone(oneof.Options()).(*descriptorpb.OneofOptions),
+ Options: proto.Clone(oneof.Options()).(*descriptorpb.OneofOptions),
}
}
@@ -170,7 +169,7 @@
func ToEnumDescriptorProto(enum protoreflect.EnumDescriptor) *descriptorpb.EnumDescriptorProto {
p := &descriptorpb.EnumDescriptorProto{
Name: proto.String(string(enum.Name())),
- Options: clone(enum.Options()).(*descriptorpb.EnumOptions),
+ Options: proto.Clone(enum.Options()).(*descriptorpb.EnumOptions),
}
for i, values := 0, enum.Values(); i < values.Len(); i++ {
p.Value = append(p.Value, ToEnumValueDescriptorProto(values.Get(i)))
@@ -194,7 +193,7 @@
return &descriptorpb.EnumValueDescriptorProto{
Name: proto.String(string(value.Name())),
Number: proto.Int32(int32(value.Number())),
- Options: clone(value.Options()).(*descriptorpb.EnumValueOptions),
+ Options: proto.Clone(value.Options()).(*descriptorpb.EnumValueOptions),
}
}
@@ -203,7 +202,7 @@
func ToServiceDescriptorProto(service protoreflect.ServiceDescriptor) *descriptorpb.ServiceDescriptorProto {
p := &descriptorpb.ServiceDescriptorProto{
Name: proto.String(string(service.Name())),
- Options: clone(service.Options()).(*descriptorpb.ServiceOptions),
+ Options: proto.Clone(service.Options()).(*descriptorpb.ServiceOptions),
}
for i, methods := 0, service.Methods(); i < methods.Len(); i++ {
p.Method = append(p.Method, ToMethodDescriptorProto(methods.Get(i)))
@@ -218,7 +217,7 @@
Name: proto.String(string(method.Name())),
InputType: fullNameOf(method.Input()),
OutputType: fullNameOf(method.Output()),
- Options: clone(method.Options()).(*descriptorpb.MethodOptions),
+ Options: proto.Clone(method.Options()).(*descriptorpb.MethodOptions),
}
if method.IsStreamingClient() {
p.ClientStreaming = proto.Bool(true)
@@ -238,12 +237,3 @@
}
return proto.String("." + string(d.FullName()))
}
-
-func clone(src proto.Message) proto.Message {
- if reflect.ValueOf(src).IsNil() {
- return src
- }
- dst := src.ProtoReflect().New().Interface()
- proto.Merge(dst, src)
- return dst
-}