diff --git a/issue121_test.go b/issue121_test.go index d4cf101..bcec280 100644 --- a/issue121_test.go +++ b/issue121_test.go @@ -4,102 +4,30 @@ import ( "testing" ) -func TestIssue121WithSliceDeepMerge(t *testing.T) { - dst := map[string]interface{}{ - "a": "1", - "b": []map[string]interface{}{ - {"c": "2"}, - }, - } - src := map[string]interface{}{ - "b": []map[string]interface{}{ - {"c": "3", "d": "1"}, - {"e": "1", "f": "1", "g": []string{"1", "2"}}, - }, - } - if err := Merge(&dst, src, WithDeepMergeSlice); err != nil { - t.Fatalf("Error during the merge: %v", err) - } - if dst["a"].(string) != "1" { - t.Error("a should equal '1'") - } - if dst["b"].([]map[string]interface{})[0]["c"] != "2" { - t.Error("b.[0].c should equal '2'") - } - if dst["b"].([]map[string]interface{})[0]["d"] != "1" { - t.Error("b.[0].d should equal '2'") - } - if dst["b"].([]map[string]interface{})[1]["e"] != "1" { - t.Error("b.[1].e should equal '1'") - } - if dst["b"].([]map[string]interface{})[1]["g"].([]string)[0] != "1" { - t.Error("b.[1].g[0] should equal '1'") - } -} - -func TestIssue121WithSliceDeepMergeFromPR126(t *testing.T) { +func TestIssue121WithSliceDeepCopy(t *testing.T) { dst := map[string]interface{}{ "inter": map[string]interface{}{ "a": "1", "b": "2", }, } + src := map[string]interface{}{ "inter": map[string]interface{}{ "a": "3", "c": "4", }, } - if err := Merge(&dst, src, WithDeepMergeSlice); err != nil { - t.Fatalf("Error during the merge: %v", err) - } - if dst["inter"].(map[string]interface{})["a"].(string) != "1" { - t.Error("inter.a should equal '1'") + + if err := Merge(&dst, src, WithSliceDeepCopy); err != nil { + t.Errorf("Error during the merge: %v", err) } - if dst["inter"].(map[string]interface{})["b"].(string) != "2" { - t.Error("inter.a should equal '2'") + + if dst["inter"].(map[string]interface{})["a"].(string) != "3" { + t.Error("inter.a should equal '3'") } + if dst["inter"].(map[string]interface{})["c"].(string) != "4" { t.Error("inter.c should equal '4'") } } - -type order struct { - A string - B int64 - Details []detail -} - -type detail struct { - A string - B string -} - -func TestIssue121(t *testing.T) { - src := order{ - A: "one", - B: 2, - Details: []detail{ - { - B: "B", - }, - }, - } - dst := order{ - A: "two", - Details: []detail{ - { - A: "one", - }, - }, - } - if err := Merge(&dst, src, WithDeepMergeSlice); err != nil { - t.Fatalf("Error during the merge: %v", err) - } - if len(dst.Details) != 1 { - t.Fatalf("B was not properly merged: %+v", dst.Details) - } - if dst.Details[0].B != "B" { - t.Fatalf("B was not properly merged: %+v", dst.Details[0].B) - } -} diff --git a/issue131_test.go b/issue131_test.go index 5e42a17..4f2d6ca 100644 --- a/issue131_test.go +++ b/issue131_test.go @@ -18,7 +18,9 @@ func TestIssue131MergeWithOverwriteWithEmptyValue(t *testing.T) { A: func(v bool) *bool { return &v }(true), B: "dest", } - Merge(&dest, src, WithOverwriteWithEmptyValue) + if err := Merge(&dest, src, WithOverwriteWithEmptyValue); err != nil { + t.Error(err) + } if *src.A != *dest.A { t.Errorf("dest.A not merged in properly: %v != %v", *src.A, *dest.A) } diff --git a/map.go b/map.go index a549073..4823546 100644 --- a/map.go +++ b/map.go @@ -171,9 +171,8 @@ func MapWithOverwrite(dst, src interface{}, opts ...func(*Config)) error { func _map(dst, src interface{}, opts ...func(*Config)) error { if dst != nil && reflect.ValueOf(dst).Kind() != reflect.Ptr { - return ErrNonPointerArgument + return ErrNonPointerAgument } - var ( vDst, vSrc reflect.Value err error diff --git a/merge.go b/merge.go index df7c753..21085bd 100644 --- a/merge.go +++ b/merge.go @@ -14,11 +14,13 @@ import ( "unsafe" ) -func hasExportedField(dst reflect.Value) bool { +func isMergeableField(dst reflect.Value) (exported bool) { for i, n := 0, dst.NumField(); i < n; i++ { field := dst.Type().Field(i) - if isExportedComponent(&field) { - return true + if field.Anonymous && dst.Field(i).Kind() == reflect.Struct { + exported = exported || isMergeableField(dst.Field(i)) + } else if isExportedComponent(&field) { + exported = exported || len(field.PkgPath) == 0 } } @@ -30,24 +32,20 @@ func isExportedComponent(field *reflect.StructField) bool { if len(pkgPath) > 0 { return false } - c := field.Name[0] if 'a' <= c && c <= 'z' || c == '_' { return false } - return true } -// Config allows to customize Mergo's behaviour. type Config struct { Overwrite bool AppendSlice bool TypeCheck bool overwriteWithEmptyValue bool overwriteSliceWithEmptyValue bool - deepMergeSlice bool - Transformers Transformers + sliceDeepCopy bool } // Transformers allow to merge specific types differently than in the default behavior. @@ -65,6 +63,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, typeCheck := config.TypeCheck overwriteWithEmptySrc := config.overwriteWithEmptyValue overwriteSliceWithEmptySrc := config.overwriteSliceWithEmptyValue + sliceDeepCopy := config.sliceDeepCopy if !src.IsValid() { return @@ -100,9 +99,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, switch dst.Kind() { case reflect.Struct: - if hasExportedField(dst) { - dstCp := reflect.New(dst.Type()).Elem() - + if isMergeableField(dst) { for i, n := 0, dst.NumField(); i < n; i++ { dstField := dst.Field(i) structField := dst.Type().Field(i) @@ -129,11 +126,9 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, dstCp.Field(i).Set(dstField) } - - if dst.CanSet() { - dst.Set(dstCp) - } else { - dst = dstCp + } else { + if (isReflectNil(dst) || overwrite) && (!isEmptyValue(src) || overwriteWithEmptySrc) { + dst.Set(src) } return @@ -204,15 +199,35 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, dst.SetMapIndex(key, newSlice) dstElement = newSlice } - } - } else { - if overwrite || overwriteSliceWithEmptySrc || overwriteWithEmptySrc || dst.IsNil() { - dst.SetMapIndex(key, srcElement) - dstElement = srcElement - } else { - if typeCheck { - err = fmt.Errorf("cannot append two different types (%s, %s)", srcElement, dstElement) - return + + if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice && !sliceDeepCopy { + if typeCheck && srcSlice.Type() != dstSlice.Type() { + return fmt.Errorf("cannot override two slices with different type (%s, %s)", srcSlice.Type(), dstSlice.Type()) + } + dstSlice = srcSlice + } else if config.AppendSlice { + if srcSlice.Type() != dstSlice.Type() { + return fmt.Errorf("cannot append two slices with different type (%s, %s)", srcSlice.Type(), dstSlice.Type()) + } + dstSlice = reflect.AppendSlice(dstSlice, srcSlice) + } else if sliceDeepCopy { + i := 0 + for ; i < srcSlice.Len() && i < dstSlice.Len(); i++ { + srcElement := srcSlice.Index(i) + dstElement := dstSlice.Index(i) + + if srcElement.CanInterface() { + srcElement = reflect.ValueOf(srcElement.Interface()) + } + if dstElement.CanInterface() { + dstElement = reflect.ValueOf(dstElement.Interface()) + } + + if err = deepMerge(dstElement, srcElement, visited, depth+1, config); err != nil { + return + } + } + } } } @@ -225,15 +240,11 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, dst.SetMapIndex(key, dstElement) } case reflect.Slice: - newSlice := dst - - if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice { - if typeCheck && src.Type() != dst.Type() { - err = fmt.Errorf("cannot override two slices with different type (%s, %s)", src.Type(), dst.Type()) - return - } - - newSlice = src + if !dst.CanSet() { + break + } + if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice && !sliceDeepCopy { + dst.Set(src) } else if config.AppendSlice { if typeCheck && src.Type() != dst.Type() { err = fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type()) @@ -245,6 +256,29 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, err = fmt.Errorf("cannot deep merge two slice with different type (%s, %s)", src.Type(), dst.Type()) return } + dst.Set(reflect.AppendSlice(dst, src)) + } else if sliceDeepCopy { + for i := 0; i < src.Len() && i < dst.Len(); i++ { + srcElement := src.Index(i) + dstElement := dst.Index(i) + if srcElement.CanInterface() { + srcElement = reflect.ValueOf(srcElement.Interface()) + } + if dstElement.CanInterface() { + dstElement = reflect.ValueOf(dstElement.Interface()) + } + + if err = deepMerge(dstElement, srcElement, visited, depth+1, config); err != nil { + return + } + } + } + case reflect.Ptr: + fallthrough + case reflect.Interface: + if src.IsNil() { + break + } if src.Len() > dst.Len() { newSlice = reflect.MakeSlice(dst.Type(), src.Len(), src.Len()) @@ -407,14 +441,15 @@ func WithTypeCheck(config *Config) { config.TypeCheck = true } -// WithDeepMergeSlice will make merge deep merge slice elements pairwise (resizing dst slice if needed) -func WithDeepMergeSlice(config *Config) { - config.deepMergeSlice = true +// WithSliceDeepCopy will merge slice element one by one with Overwrite flag. +func WithSliceDeepCopy(config *Config) { + config.sliceDeepCopy = true + config.Overwrite = true } func merge(dst, src interface{}, opts ...func(*Config)) error { if dst != nil && reflect.ValueOf(dst).Kind() != reflect.Ptr { - return ErrNonPointerArgument + return ErrNonPointerAgument } var ( vDst, vSrc reflect.Value @@ -452,3 +487,16 @@ func isReflectNil(v reflect.Value) bool { return false } } + +// IsReflectNil is the reflect value provided nil +func isReflectNil(v reflect.Value) bool { + k := v.Kind() + switch k { + case reflect.Interface, reflect.Slice, reflect.Chan, reflect.Func, reflect.Map, reflect.Ptr: + // Both interface and slice are nil if first word is 0. + // Both are always bigger than a word; assume flagIndir. + return v.IsNil() + default: + return false + } +} diff --git a/merge_test.go b/merge_test.go index 19f0caf..356d623 100644 --- a/merge_test.go +++ b/merge_test.go @@ -62,7 +62,7 @@ func TestMergeNonPointer(t *testing.T) { "a": "1", }, } - want := ErrNonPointerArgument + want := ErrNonPointerAgument if got := merge(dst, src); got != want { t.Errorf("want: %s, got: %s", want, got) @@ -79,7 +79,7 @@ func TestMapNonPointer(t *testing.T) { }, }, } - want := ErrNonPointerArgument + want := ErrNonPointerAgument if got := merge(dst, src); got != want { t.Errorf("want: %s, got: %s", want, got) } diff --git a/mergo.go b/mergo.go index 46a9b04..3cc926c 100644 --- a/mergo.go +++ b/mergo.go @@ -20,7 +20,7 @@ var ( ErrNotSupported = errors.New("only structs and maps are supported") ErrExpectedMapAsDestination = errors.New("dst was expected to be a map") ErrExpectedStructAsDestination = errors.New("dst was expected to be a struct") - ErrNonPointerArgument = errors.New("dst must be a pointer") + ErrNonPointerAgument = errors.New("dst must be a pointer") ) // During deepMerge, must keep track of checks that are diff --git a/mergo_test.go b/mergo_test.go index b638df1..926738c 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -365,12 +365,12 @@ func TestMapsWithOverwrite(t *testing.T) { "e": {14}, } - if err := MergeWithOverwrite(&dst, src); err != nil { + if err := MergeWithOverwrite(&m, n); err != nil { t.Errorf(err.Error()) } - if !reflect.DeepEqual(dst, expect) { - t.Errorf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", dst, expect) + if !reflect.DeepEqual(m, expect) { + t.Errorf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", m, expect) } } @@ -552,7 +552,7 @@ func TestMaps(t *testing.T) { if !reflect.DeepEqual(m, expect) { t.Errorf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", m, expect) } - if m["a"].Value != 16 { + if m["a"].Value != 0 { t.Errorf(`n merged in m because I solved non-addressable map values TODO: m["a"].Value(%d) != n["a"].Value(%d)`, m["a"].Value, n["a"].Value) } if m["b"].Value != 42 { diff --git a/v039_bugs_test.go b/v039_bugs_test.go new file mode 100644 index 0000000..db0cca9 --- /dev/null +++ b/v039_bugs_test.go @@ -0,0 +1,90 @@ +package mergo + +import ( + "testing" +) + +type inner struct { + A int +} + +type outer struct { + inner + B int +} + +func TestV039Issue139(t *testing.T) { + dst := outer{ + inner: inner{A: 1}, + B: 2, + } + src := outer{ + inner: inner{A: 10}, + B: 20, + } + err := MergeWithOverwrite(&dst, src) + if err != nil { + panic(err.Error()) + } + if dst.inner.A == 1 { + t.Errorf("expected %d, got %d", src.inner.A, dst.inner.A) + } +} + +func TestV039Issue152(t *testing.T) { + dst := map[string]interface{}{ + "properties": map[string]interface{}{ + "field1": map[string]interface{}{ + "type": "text", + }, + "field2": "ohai", + }, + } + src := map[string]interface{}{ + "properties": map[string]interface{}{ + "field1": "wrong", + }, + } + if err := Map(&dst, src, WithOverride); err != nil { + t.Error(err) + } +} + +type issue146Foo struct { + A string + B map[string]issue146Bar +} + +type issue146Bar struct { + C *string + D *string +} + +func TestV039Issue146(t *testing.T) { + var ( + s1 = "asd" + s2 = "sdf" + ) + dst := issue146Foo{ + A: "two", + B: map[string]issue146Bar{ + "foo": { + C: &s1, + }, + }, + } + src := issue146Foo{ + A: "one", + B: map[string]issue146Bar{ + "foo": { + D: &s2, + }, + }, + } + if err := Merge(&dst, src, WithOverride); err != nil { + t.Error(err) + } + if dst.B["foo"].D == nil { + t.Errorf("expected %v, got nil", &s2) + } +}