Skip to content

Commit

Permalink
feat: use isReflectNil instead of isEmptyValue to gate Transformers
Browse files Browse the repository at this point in the history
Mergo handles merges with zero-value fields as follows:

	1. dst: true       src: false      ->     dst: true
	2. dst: nil        src: false      ->     dst: false
	3. dst: nil	   src: true	   ->	  dst: true
	4. dst: false	   src: true 	   ->	  dst: true

In some cases (darccio#166) it may be desirable
to change this default behaviour using a custom Transformer, for example
to not overwrite 'dst' when it's already set to 'false' (or '0' etc).
Unfortunately, in such cases (when dst holds a zero-value) no Transformers can
be applied due to the way these are gated (darccio#166).

Here we are changing how transformers are gated by using the isReflectNil function
insted of isEmptyValue.

Note that:
- this change doesn't affect the default behaviour of mergo, as it only affects
  how transformers are gated
- since custom transformers need to match data types in order to execute,
  this shouldn't affect any existing transformers either
  • Loading branch information
GGabriele committed Feb 1, 2022
1 parent fd3dfc9 commit 32bd3e7
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 1 deletion.
8 changes: 8 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
language: go

go:
- "1.13"
- "1.14"
- "1.15"
- "1.16"
- "1.17"

arch:
- amd64
- ppc64le
Expand Down
2 changes: 1 addition & 1 deletion merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co
visited[h] = &visit{addr, typ, seen}
}

if config.Transformers != nil && !isEmptyValue(dst) {
if config.Transformers != nil && !isReflectNil(dst) {
if fn := config.Transformers.Transformer(dst.Type()); fn != nil {
err = fn(dst, src)
return
Expand Down
131 changes: 131 additions & 0 deletions mergo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,10 @@ type structWithBoolPointer struct {
C *bool
}

type structWithIntPointer struct {
C *int
}

func TestBooleanPointer(t *testing.T) {
bt, bf := true, false
src := structWithBoolPointer{
Expand Down Expand Up @@ -938,3 +942,130 @@ func TestMergeSlicesIsNotSupported(t *testing.T) {
t.Errorf("expected %q, got %q", mergo.ErrNotSupported, err)
}
}

func Bool(v bool) *bool { return &v }
func Int(i int) *int { return &i }

type customTransformer struct{}

func (t customTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
if typ == reflect.TypeOf(Bool(false)) || typ == reflect.TypeOf(Int(0)) {
return func(dst, src reflect.Value) error {
if dst.CanSet() {
if src.IsNil() {
src.Set(dst)
}
}
return nil
}
}
return nil
}

func TestBooleanPointerWithTransformer(t *testing.T) {
testCases := []struct {
name string
src *bool
dst *bool
expected *bool
}{
{
name: "do not overwrite if dst is not empty (true)",
src: Bool(false),
dst: Bool(true),
expected: Bool(true),
},
{
name: "do not overwrite if dst is not empty (false)",
src: Bool(true),
dst: Bool(false),
expected: Bool(false),
},
{
name: "overwrite if dst is empty (true)",
src: Bool(true),
expected: Bool(true),
},
{
name: "overwrite if dst is empty (false)",
src: Bool(false),
expected: Bool(false),
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
var src, dst structWithBoolPointer
if tc.src != nil {
src = structWithBoolPointer{
C: tc.src,
}
}
if tc.dst != nil {
dst = structWithBoolPointer{
C: tc.dst,
}
}

if err := mergo.Merge(&dst, src, mergo.WithTransformers(customTransformer{})); err != nil {
t.Error(err)
}
if *dst.C != *tc.expected {
t.Errorf("expected %v, got %v", *tc.expected, *dst.C)
}
})
}
}

func TestIntPointerWithTransformer(t *testing.T) {
testCases := []struct {
name string
src *int
dst *int
expected *int
}{
{
name: "do not overwrite if dst is not empty (10)",
src: Int(100),
dst: Int(10),
expected: Int(10),
},
{
name: "do not overwrite if dst is not empty (0)",
src: Int(100),
dst: Int(0),
expected: Int(0),
},
{
name: "overwrite if dst is empty (100)",
src: Int(100),
expected: Int(100),
},
{
name: "overwrite if dst is empty (100)",
src: Int(100),
expected: Int(100),
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
var src, dst structWithIntPointer
if tc.src != nil {
src = structWithIntPointer{
C: tc.src,
}
}
if tc.dst != nil {
dst = structWithIntPointer{
C: tc.dst,
}
}

if err := mergo.Merge(&dst, src, mergo.WithTransformers(customTransformer{})); err != nil {
t.Error(err)
}
if *dst.C != *tc.expected {
t.Errorf("expected %v, got %v", *tc.expected, *dst.C)
}
})
}
}

0 comments on commit 32bd3e7

Please sign in to comment.