From 3defce44b45d69a2678f9d57a3e06667f83acb06 Mon Sep 17 00:00:00 2001 From: Geoff Watson Date: Mon, 18 Mar 2024 20:23:45 -0700 Subject: [PATCH 1/3] Add support for nested message overwrite --- fmutils.go | 65 ++++++++++++++++++++-------- fmutils_test.go | 113 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 19 deletions(-) diff --git a/fmutils.go b/fmutils.go index d73cdcd..c942e23 100644 --- a/fmutils.go +++ b/fmutils.go @@ -170,30 +170,57 @@ func (mask NestedMask) Overwrite(src, dest proto.Message) { mask.overwrite(src.ProtoReflect(), dest.ProtoReflect()) } -func (mask NestedMask) overwrite(src, dest protoreflect.Message) { - for k, v := range mask { - srcFD := src.Descriptor().Fields().ByName(protoreflect.Name(k)) - destFD := dest.Descriptor().Fields().ByName(protoreflect.Name(k)) - if srcFD == nil || destFD == nil { - continue - } - - // Leaf mask -> copy value from src to dest - if len(v) == 0 { - if srcFD.Kind() == destFD.Kind() { // TODO: Full type equality check - val := src.Get(srcFD) - if isValid(srcFD, val) { - dest.Set(destFD, val) +func (mask NestedMask) overwrite(srcRft, destRft protoreflect.Message) { + for srcFDName, submask := range mask { + srcFD := srcRft.Descriptor().Fields().ByName(protoreflect.Name(srcFDName)) + srcVal := srcRft.Get(srcFD) + if len(submask) == 0 { + if isValid(srcFD, srcVal) { + destRft.Set(srcFD, srcVal) + } else { + destRft.Clear(srcFD) + } + } else if srcFD.IsMap() && srcFD.Kind() == protoreflect.MessageKind { + srcMap := srcRft.Get(srcFD).Map() + destMap := destRft.Get(srcFD).Map() + srcMap.Range(func(mk protoreflect.MapKey, mv protoreflect.Value) bool { + if mi, ok := submask[mk.String()]; ok { + if i, ok := mv.Interface().(protoreflect.Message); ok && len(mi) > 0 { + destVal := protoreflect.ValueOf(mv) + destMap.Set(mk, destVal) + mi.overwrite(i, destVal.Message()) + } else { + destMap.Set(mk, mv) + } + } + return true + }) + } else if srcFD.IsList() && srcFD.Kind() == protoreflect.MessageKind { + srcList := srcRft.Get(srcFD).List() + destList := destRft.Mutable(srcFD).List() + // Truncate anything in dest that exceeds the length of src + if srcList.Len() < destList.Len() { + destList.Truncate(srcList.Len()) + } + for i := 0; i < srcList.Len(); i++ { + srcListItem := srcList.Get(i) + var destListItem protoreflect.Message + if destList.Len() > i { + // Overwrite existing items. + destListItem = destList.Get(i).Message() } else { - dest.Clear(destFD) + // Append new items to overwrite. + destListItem = destList.AppendMutable().Message() } + submask.overwrite(srcListItem.Message(), destListItem) } + } else if srcFD.Kind() == protoreflect.MessageKind { - // If dest field is nil - if !dest.Get(destFD).Message().IsValid() { - dest.Set(destFD, protoreflect.ValueOf(dest.Get(destFD).Message().New())) + // If the dest field is nil + if !destRft.Get(srcFD).Message().IsValid() { + destRft.Set(srcFD, protoreflect.ValueOf(destRft.Get(srcFD).Message().New())) } - v.overwrite(src.Get(srcFD).Message(), dest.Get(destFD).Message()) + submask.overwrite(srcRft.Get(srcFD).Message(), destRft.Get(srcFD).Message()) } } } diff --git a/fmutils_test.go b/fmutils_test.go index d3b4add..0993be7 100644 --- a/fmutils_test.go +++ b/fmutils_test.go @@ -883,6 +883,119 @@ func TestOverwrite(t *testing.T) { }, }, }, + { + name: "overwrite repeated message fields", + paths: []string{"gallery.path"}, + src: &testproto.Profile{ + User: &testproto.User{ + UserId: 567, + Name: "different-name", + }, + Photo: &testproto.Photo{ + Path: "photo-path", + }, + LoginTimestamps: []int64{1, 2, 3}, + Attributes: map[string]*testproto.Attribute{ + "src": {}, + }, + Gallery: []*testproto.Photo{ + { + PhotoId: 123, + Path: "test-path-1", + Dimensions: &testproto.Dimensions{ + Width: 345, + Height: 456, + }, + }, + { + PhotoId: 234, + Path: "test-path-2", + Dimensions: &testproto.Dimensions{ + Width: 3456, + Height: 4567, + }, + }, + { + PhotoId: 345, + Path: "test-path-3", + Dimensions: &testproto.Dimensions{ + Width: 34567, + Height: 45678, + }, + }, + }, + }, + dest: &testproto.Profile{ + User: &testproto.User{ + Name: "name", + }, + Gallery: []*testproto.Photo{ + { + PhotoId: 123, + Path: "test-path-7", + Dimensions: &testproto.Dimensions{ + Width: 345, + Height: 456, + }, + }, + { + PhotoId: 234, + Path: "test-path-6", + Dimensions: &testproto.Dimensions{ + Width: 3456, + Height: 4567, + }, + }, + { + PhotoId: 345, + Path: "test-path-5", + Dimensions: &testproto.Dimensions{ + Width: 34567, + Height: 45678, + }, + }, + { + PhotoId: 345, + Path: "test-path-4", + Dimensions: &testproto.Dimensions{ + Width: 34567, + Height: 45678, + }, + }, + }, + }, + want: &testproto.Profile{ + User: &testproto.User{ + Name: "name", + }, + Gallery: []*testproto.Photo{ + { + PhotoId: 123, + Path: "test-path-1", + Dimensions: &testproto.Dimensions{ + Width: 345, + Height: 456, + }, + }, + { + PhotoId: 234, + Path: "test-path-2", + Dimensions: &testproto.Dimensions{ + Width: 3456, + Height: 4567, + }, + }, + { + PhotoId: 345, + Path: "test-path-3", + Dimensions: &testproto.Dimensions{ + Width: 34567, + Height: 45678, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From a7a12695a3fc12278ce10808c5be3caa31a6f3e2 Mon Sep 17 00:00:00 2001 From: Geoff Watson Date: Tue, 19 Mar 2024 10:50:49 -0700 Subject: [PATCH 2/3] Add map test case --- fmutils.go | 5 ++--- fmutils_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/fmutils.go b/fmutils.go index c942e23..81e08b3 100644 --- a/fmutils.go +++ b/fmutils.go @@ -186,9 +186,8 @@ func (mask NestedMask) overwrite(srcRft, destRft protoreflect.Message) { srcMap.Range(func(mk protoreflect.MapKey, mv protoreflect.Value) bool { if mi, ok := submask[mk.String()]; ok { if i, ok := mv.Interface().(protoreflect.Message); ok && len(mi) > 0 { - destVal := protoreflect.ValueOf(mv) - destMap.Set(mk, destVal) - mi.overwrite(i, destVal.Message()) + destMap.Set(mk, mv) + mi.overwrite(i, mv.Message()) } else { destMap.Set(mk, mv) } diff --git a/fmutils_test.go b/fmutils_test.go index 0993be7..e655687 100644 --- a/fmutils_test.go +++ b/fmutils_test.go @@ -883,6 +883,47 @@ func TestOverwrite(t *testing.T) { }, }, }, + { + name: "overwrite map with message values", + paths: []string{"attributes.src1.tags", "attributes.src2.tags"}, + src: &testproto.Profile{ + User: nil, + Attributes: map[string]*testproto.Attribute{ + "src1": { + Tags: map[string]string{"key1": "value1", "key2": "value2"}, + }, + "src2": { + Tags: map[string]string{"key3": "value3"}, + }, + }, + }, + dest: &testproto.Profile{ + User: &testproto.User{ + Name: "name", + }, + Attributes: map[string]*testproto.Attribute{ + "dest1": { + Tags: map[string]string{"key4": "value5"}, + }, + }, + }, + want: &testproto.Profile{ + User: &testproto.User{ + Name: "name", + }, + Attributes: map[string]*testproto.Attribute{ + "src1": { + Tags: map[string]string{"key1": "value1", "key2": "value2"}, + }, + "src2": { + Tags: map[string]string{"key3": "value3"}, + }, + "dest1": { + Tags: map[string]string{"key4": "value5"}, + }, + }, + }, + }, { name: "overwrite repeated message fields", paths: []string{"gallery.path"}, From 4300539142fb19954d921c11ebe3cd06f0cbd142 Mon Sep 17 00:00:00 2001 From: Geoff Watson Date: Tue, 19 Mar 2024 11:49:11 -0700 Subject: [PATCH 3/3] Additional tests and minor fix --- fmutils.go | 12 +++++++-- fmutils_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/fmutils.go b/fmutils.go index 81e08b3..2f5d101 100644 --- a/fmutils.go +++ b/fmutils.go @@ -183,14 +183,22 @@ func (mask NestedMask) overwrite(srcRft, destRft protoreflect.Message) { } else if srcFD.IsMap() && srcFD.Kind() == protoreflect.MessageKind { srcMap := srcRft.Get(srcFD).Map() destMap := destRft.Get(srcFD).Map() + if !destMap.IsValid() { + destRft.Set(srcFD, protoreflect.ValueOf(srcMap)) + destMap = destRft.Get(srcFD).Map() + } srcMap.Range(func(mk protoreflect.MapKey, mv protoreflect.Value) bool { if mi, ok := submask[mk.String()]; ok { if i, ok := mv.Interface().(protoreflect.Message); ok && len(mi) > 0 { - destMap.Set(mk, mv) - mi.overwrite(i, mv.Message()) + newVal := protoreflect.ValueOf(i.New()) + destMap.Set(mk, newVal) + mi.overwrite(mv.Message(), newVal.Message()) } else { + destMap.Set(mk, mv) } + } else { + destMap.Clear(mk) } return true }) diff --git a/fmutils_test.go b/fmutils_test.go index e655687..3953d5f 100644 --- a/fmutils_test.go +++ b/fmutils_test.go @@ -885,7 +885,7 @@ func TestOverwrite(t *testing.T) { }, { name: "overwrite map with message values", - paths: []string{"attributes.src1.tags", "attributes.src2.tags"}, + paths: []string{"attributes.src1.tags.key1", "attributes.src2"}, src: &testproto.Profile{ User: nil, Attributes: map[string]*testproto.Attribute{ @@ -903,7 +903,7 @@ func TestOverwrite(t *testing.T) { }, Attributes: map[string]*testproto.Attribute{ "dest1": { - Tags: map[string]string{"key4": "value5"}, + Tags: map[string]string{"key4": "value4"}, }, }, }, @@ -913,13 +913,13 @@ func TestOverwrite(t *testing.T) { }, Attributes: map[string]*testproto.Attribute{ "src1": { - Tags: map[string]string{"key1": "value1", "key2": "value2"}, + Tags: map[string]string{"key1": "value1"}, }, "src2": { Tags: map[string]string{"key3": "value3"}, }, "dest1": { - Tags: map[string]string{"key4": "value5"}, + Tags: map[string]string{"key4": "value4"}, }, }, }, @@ -1037,6 +1037,63 @@ func TestOverwrite(t *testing.T) { }, }, }, + { + name: "overwrite repeated message fields to empty list", + paths: []string{"gallery.path"}, + src: &testproto.Profile{ + User: &testproto.User{ + UserId: 567, + Name: "different-name", + }, + Photo: &testproto.Photo{ + Path: "photo-path", + }, + LoginTimestamps: []int64{1, 2, 3}, + Attributes: map[string]*testproto.Attribute{ + "src": {}, + }, + Gallery: []*testproto.Photo{ + { + PhotoId: 123, + Path: "test-path-1", + Dimensions: &testproto.Dimensions{ + Width: 345, + Height: 456, + }, + }, + { + PhotoId: 234, + Path: "test-path-2", + Dimensions: &testproto.Dimensions{ + Width: 3456, + Height: 4567, + }, + }, + { + PhotoId: 345, + Path: "test-path-3", + Dimensions: &testproto.Dimensions{ + Width: 34567, + Height: 45678, + }, + }, + }, + }, + dest: &testproto.Profile{}, + want: &testproto.Profile{ + Gallery: []*testproto.Photo{ + { + Path: "test-path-1", + }, + { + Path: "test-path-2", + }, + { + Path: "test-path-3", + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {