From 32bd3e72618fb72c82d7eea1437fea7ba505c2c3 Mon Sep 17 00:00:00 2001 From: Gabriele Gerbino Date: Wed, 26 Jan 2022 10:45:50 +0100 Subject: [PATCH] feat: use isReflectNil instead of isEmptyValue to gate Transformers 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 (https://github.com/imdario/mergo/issues/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 (https://github.com/imdario/mergo/issues/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 --- .travis.yml | 8 +++ merge.go | 2 +- mergo_test.go | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d324c43..acf3d76 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,12 @@ language: go + +go: +- "1.13" +- "1.14" +- "1.15" +- "1.16" +- "1.17" + arch: - amd64 - ppc64le diff --git a/merge.go b/merge.go index 8c2a8fc..5e58da6 100644 --- a/merge.go +++ b/merge.go @@ -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 diff --git a/mergo_test.go b/mergo_test.go index d69714c..bf0587d 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -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{ @@ -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) + } + }) + } +}