From 2b9fcfaffcbf0bb51b69cd6c748d876f999bbfae Mon Sep 17 00:00:00 2001 From: dmitri Date: Mon, 23 Nov 2020 20:28:27 +0100 Subject: [PATCH] Add support for merging messages that implement Merger but are embedded by value. --- proto/clone.go | 16 ++++++++----- proto/clone_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/proto/clone.go b/proto/clone.go index a26b046d94..eb2e0ecf9d 100644 --- a/proto/clone.go +++ b/proto/clone.go @@ -138,13 +138,17 @@ func mergeStruct(out, in reflect.Value) { // viaPtr indicates whether the values were indirected through a pointer (implying proto2). // prop is set if this is a struct field (it may be nil). func mergeAny(out, in reflect.Value, viaPtr bool, prop *Properties) { - if in.Type() == protoMessageType { - if !in.IsNil() { - if out.IsNil() { - out.Set(reflect.ValueOf(Clone(in.Interface().(Message)))) - } else { - Merge(out.Interface().(Message), in.Interface().(Message)) + if in.Type() == protoMessageType || (in.CanAddr() && in.Addr().Type().Implements(protoMessageType)) { + if in.Kind() == reflect.Ptr { + if !in.IsNil() { + if out.IsNil() { + out.Set(reflect.ValueOf(Clone(in.Interface().(Message)))) + } else { + Merge(out.Interface().(Message), in.Interface().(Message)) + } } + } else { + Merge(out.Addr().Interface().(Message), in.Addr().Interface().(Message)) } return } diff --git a/proto/clone_test.go b/proto/clone_test.go index ac4b919b28..d604d67d4d 100644 --- a/proto/clone_test.go +++ b/proto/clone_test.go @@ -33,6 +33,7 @@ package proto_test import ( "testing" + "time" "github.com/gogo/protobuf/proto" @@ -380,6 +381,15 @@ var mergeTests = []struct { Others: []*pb.OtherMessage{}, }, }, + { + src: &myMessage{}, + dst: &myMessage{ + Sub: Sub{Timestamp: newTimestampPtr(time.Date(1984, time.April, 4, 0, 0, 0, 0, time.UTC))}, + }, + want: &myMessage{ + Sub: Sub{Timestamp: newTimestampPtr(time.Date(1984, time.April, 4, 0, 0, 0, 0, time.UTC))}, + }, + }, } func TestMerge(t *testing.T) { @@ -395,3 +405,50 @@ func TestMerge(t *testing.T) { } } } + +func (r *myMessage) Reset() { *r = myMessage{} } +func (r *myMessage) String() string { return proto.CompactTextString(r) } +func (r *myMessage) ProtoMessage() {} + +func (r *Sub) Reset() { *r = Sub{} } +func (r *Sub) String() string { return proto.CompactTextString(r) } +func (r *Sub) ProtoMessage() {} + +// Merge merges this value with src. +// src is assumed to be of type *Sub +// Implements proto.Merger +func (r *Sub) Merge(src proto.Message) { + s, ok := src.(*Sub) + if !ok { + return + } + if s.Timestamp != nil { + t := *s.Timestamp + r.Timestamp = &t + } +} + +// myMessage describes a custom protobuf type that contains +// a field using a non-protobuf type which does not lend itself +// to reflect-style cloning due to the use of unexported fields. +// custom implementation of the Merge interface is required +// for Clone to succeed +type myMessage struct { + // Sub describes a field embedded as value instead of a pointer + Sub Sub `protobuf:"bytes,1,opt,name=Sub,stdtime" json:"sub"` +} + +type Sub struct { + // Timestamp specifies a timestamp as a pointer to a time.Time value. + // This is similar to the following protobuf definition: + // google.protobuf.Timestamp Tiestamp = 1 [ + // (gogoproto.stdtime) = true, + // (gogoproto.nullable) = false, + // (gogoproto.jsontag) = "timestamp" + // ]; + Timestamp *time.Time `protobuf:"bytes,1,opt,name=Timestamp,stdtime" json:"timestamp"` +} + +func newTimestampPtr(t time.Time) *time.Time { + return &t +}