diff --git a/README.md b/README.md index 43c0b1c..f4769cd 100644 --- a/README.md +++ b/README.md @@ -11,20 +11,27 @@ It is ready for production use. It works fine after extensive use in the wild. [![Build Status][1]][2] -[![GoDoc](https://godoc.org/github.com/imdario/mergo?status.svg)](https://godoc.org/github.com/imdario/mergo) +[![GoDoc][3]][4] +[![GoCard][5]][6] [1]: https://travis-ci.org/imdario/mergo.png [2]: https://travis-ci.org/imdario/mergo +[3]: https://godoc.org/github.com/imdario/mergo?status.svg +[4]: https://godoc.org/github.com/imdario/mergo +[5]: https://goreportcard.com/badge/imdario/mergo +[6]: https://goreportcard.com/report/github.com/imdario/mergo ### Important note -Mergo is intended to merge **only** zero value fields on destination. Since April 6th it works like this. Before it didn't work properly, causing some random overwrites. After some issues and PRs I found it didn't merge as I designed it. Thanks to [imdario/mergo#8](https://github.com/imdario/mergo/pull/8) overwriting functions were added and the wrong behavior was clearly detected. +Mergo is intended to assign **only** zero value fields on destination with source value. Since April 6th it works like this. Before it didn't work properly, causing some random overwrites. After some issues and PRs I found it didn't merge as I designed it. Thanks to [imdario/mergo#8](https://github.com/imdario/mergo/pull/8) overwriting functions were added and the wrong behavior was clearly detected. If you were using Mergo **before** April 6th 2015, please check your project works as intended after updating your local copy with ```go get -u github.com/imdario/mergo```. I apologize for any issue caused by its previous behavior and any future bug that Mergo could cause (I hope it won't!) in existing projects after the change (release 0.2.0). ### Mergo in the wild +- [docker/docker](https://github.com/docker/docker/) - [GoogleCloudPlatform/kubernetes](https://github.com/GoogleCloudPlatform/kubernetes) +- [imdario/zas](https://github.com/imdario/zas) - [soniah/dnsmadeeasy](https://github.com/soniah/dnsmadeeasy) - [EagerIO/Stout](https://github.com/EagerIO/Stout) - [lynndylanhurley/defsynth-api](https://github.com/lynndylanhurley/defsynth-api) @@ -33,6 +40,16 @@ - [casualjim/exeggutor](https://github.com/casualjim/exeggutor) - [divshot/gitling](https://github.com/divshot/gitling) - [RWJMurphy/gorl](https://github.com/RWJMurphy/gorl) +- [andrerocker/deploy42](https://github.com/andrerocker/deploy42) +- [elwinar/rambler](https://github.com/elwinar/rambler) +- [tmaiaroto/gopartman](https://github.com/tmaiaroto/gopartman) +- [jfbus/impressionist](https://github.com/jfbus/impressionist) +- [Jmeyering/zealot](https://github.com/Jmeyering/zealot) +- [godep-migrator/rigger-host](https://github.com/godep-migrator/rigger-host) +- [Dronevery/MultiwaySwitch-Go](https://github.com/Dronevery/MultiwaySwitch-Go) +- [thoas/picfit](https://github.com/thoas/picfit) +- [mantasmatelis/whooplist-server](https://github.com/mantasmatelis/whooplist-server) +- [jnuthong/item_search](https://github.com/jnuthong/item_search) ## Installation @@ -90,7 +107,7 @@ fmt.Println(dest) // Will print - // {one 2} + // {two 2} } ``` diff --git a/issue17_test.go b/issue17_test.go new file mode 100644 index 0000000..0ee96f3 --- /dev/null +++ b/issue17_test.go @@ -0,0 +1,25 @@ +package mergo + +import ( + "encoding/json" + "testing" +) + +var ( + request = `{"timestamp":null, "name": "foo"}` + maprequest = map[string]interface{}{ + "timestamp": nil, + "name": "foo", + "newStuff": "foo", + } +) + +func TestIssue17MergeWithOverwrite(t *testing.T) { + var something map[string]interface{} + if err := json.Unmarshal([]byte(request), &something); err != nil { + t.Errorf("Error while Unmarshalling maprequest %s", err) + } + if err := MergeWithOverwrite(&something, maprequest); err != nil { + t.Errorf("Error while merging %s", err) + } +} diff --git a/map.go b/map.go index 1ed3d71..8e8c4ba 100644 --- a/map.go +++ b/map.go @@ -121,6 +121,8 @@ return _map(dst, src, false) } +// MapWithOverwrite will do the same as Map except that non-empty dst attributes will be overriden by +// non-empty src attribute values. func MapWithOverwrite(dst, src interface{}) error { return _map(dst, src, true) } diff --git a/merge.go b/merge.go index f2d0c38..11e55b1 100644 --- a/merge.go +++ b/merge.go @@ -46,15 +46,31 @@ continue } dstElement := dst.MapIndex(key) - switch reflect.TypeOf(srcElement.Interface()).Kind() { - case reflect.Struct: + switch srcElement.Kind() { + case reflect.Chan, reflect.Func, reflect.Map, reflect.Ptr, reflect.Interface, reflect.Slice: + if srcElement.IsNil() { + continue + } fallthrough - case reflect.Map: - if err = deepMerge(dstElement, srcElement, visited, depth+1, overwrite); err != nil { - return + default: + if !srcElement.CanInterface() { + continue + } + switch reflect.TypeOf(srcElement.Interface()).Kind() { + case reflect.Struct: + fallthrough + case reflect.Ptr: + fallthrough + case reflect.Map: + if err = deepMerge(dstElement, srcElement, visited, depth+1, overwrite); err != nil { + return + } } } if !isEmptyValue(srcElement) && (overwrite || (!dstElement.IsValid() || isEmptyValue(dst))) { + if dst.IsNil() { + dst.Set(reflect.MakeMap(dst.Type())) + } dst.SetMapIndex(key, srcElement) } } @@ -78,16 +94,16 @@ return } -// Merge sets fields' values in dst from src if they have a zero -// value of their type. -// dst and src must be valid same-type structs and dst must be -// a pointer to struct. -// It won't merge unexported (private) fields and will do recursively -// any exported field. +// Merge will fill any empty for value type attributes on the dst struct using corresponding +// src attributes if they themselves are not empty. dst and src must be valid same-type structs +// and dst must be a pointer to struct. +// It won't merge unexported (private) fields and will do recursively any exported field. func Merge(dst, src interface{}) error { return merge(dst, src, false) } +// MergeWithOverwrite will do the same as Merge except that non-empty dst attributes will be overriden by +// non-empty src attribute values. func MergeWithOverwrite(dst, src interface{}) error { return merge(dst, src, true) } diff --git a/mergo_test.go b/mergo_test.go index 877401c..dd2651b 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -9,6 +9,7 @@ "io/ioutil" "reflect" "testing" + "time" "gopkg.in/yaml.v1" ) @@ -20,7 +21,7 @@ type complexTest struct { St simpleTest sz int - Id string + ID string } type moreComplextText struct { @@ -102,7 +103,7 @@ func TestComplexStruct(t *testing.T) { a := complexTest{} - a.Id = "athing" + a.ID = "athing" b := complexTest{simpleTest{42}, 1, "bthing"} if err := Merge(&a, b); err != nil { t.FailNow() @@ -113,8 +114,8 @@ if a.sz == 1 { t.Fatalf("a's private field sz not preserved from merge: a.sz(%d) == b.sz(%d)", a.sz, b.sz) } - if a.Id == b.Id { - t.Fatalf("a's field Id merged unexpectedly: a.Id(%s) == b.Id(%s)", a.Id, b.Id) + if a.ID == b.ID { + t.Fatalf("a's field ID merged unexpectedly: a.ID(%s) == b.ID(%s)", a.ID, b.ID) } } @@ -245,23 +246,23 @@ func TestMapsWithOverwrite(t *testing.T) { m := map[string]simpleTest{ - "a": simpleTest{}, // overwritten by 16 - "b": simpleTest{42}, // not overwritten by empty value - "c": simpleTest{13}, // overwritten by 12 - "d": simpleTest{61}, + "a": {}, // overwritten by 16 + "b": {42}, // not overwritten by empty value + "c": {13}, // overwritten by 12 + "d": {61}, } n := map[string]simpleTest{ - "a": simpleTest{16}, - "b": simpleTest{}, - "c": simpleTest{12}, - "e": simpleTest{14}, + "a": {16}, + "b": {}, + "c": {12}, + "e": {14}, } expect := map[string]simpleTest{ - "a": simpleTest{16}, - "b": simpleTest{}, - "c": simpleTest{12}, - "d": simpleTest{61}, - "e": simpleTest{14}, + "a": {16}, + "b": {}, + "c": {12}, + "d": {61}, + "e": {14}, } if err := MergeWithOverwrite(&m, n); err != nil { @@ -275,23 +276,23 @@ func TestMaps(t *testing.T) { m := map[string]simpleTest{ - "a": simpleTest{}, - "b": simpleTest{42}, - "c": simpleTest{13}, - "d": simpleTest{61}, + "a": {}, + "b": {42}, + "c": {13}, + "d": {61}, } n := map[string]simpleTest{ - "a": simpleTest{16}, - "b": simpleTest{}, - "c": simpleTest{12}, - "e": simpleTest{14}, + "a": {16}, + "b": {}, + "c": {12}, + "e": {14}, } expect := map[string]simpleTest{ - "a": simpleTest{0}, - "b": simpleTest{42}, - "c": simpleTest{13}, - "d": simpleTest{61}, - "e": simpleTest{14}, + "a": {0}, + "b": {42}, + "c": {13}, + "d": {61}, + "e": {14}, } if err := Merge(&m, n); err != nil { @@ -341,7 +342,7 @@ func TestMap(t *testing.T) { a := complexTest{} - a.Id = "athing" + a.ID = "athing" c := moreComplextText{a, simpleTest{}, simpleTest{}} b := map[string]interface{}{ "ct": map[string]interface{}{ @@ -374,8 +375,8 @@ if c.Ct.sz == 1 { t.Fatalf("a's private field sz not preserved from merge: c.Ct.sz(%d) == b.Ct.sz(%d)", c.Ct.sz, m["sz"]) } - if c.Ct.Id == m["id"] { - t.Fatalf("a's field Id merged unexpectedly: c.Ct.Id(%s) == b.Ct.Id(%s)", c.Ct.Id, m["id"]) + if c.Ct.ID == m["id"] { + t.Fatalf("a's field ID merged unexpectedly: c.Ct.ID(%s) == b.Ct.ID(%s)", c.Ct.ID, m["id"]) } } @@ -433,9 +434,92 @@ } } +type structWithTimePointer struct { + Birth *time.Time +} + +func TestTime(t *testing.T) { + now := time.Now() + dataStruct := structWithTimePointer{ + Birth: &now, + } + dataMap := map[string]interface{}{ + "Birth": &now, + } + b := structWithTimePointer{} + if err := Merge(&b, dataStruct); err != nil { + t.FailNow() + } + if b.Birth.IsZero() { + t.Fatalf("time.Time not merged in properly: b.Birth(%v) != dataStruct['Birth'](%v)", b.Birth, dataStruct.Birth) + } + if b.Birth != dataStruct.Birth { + t.Fatalf("time.Time not merged in properly: b.Birth(%v) != dataStruct['Birth'](%v)", b.Birth, dataStruct.Birth) + } + b = structWithTimePointer{} + if err := Map(&b, dataMap); err != nil { + t.FailNow() + } + if b.Birth.IsZero() { + t.Fatalf("time.Time not merged in properly: b.Birth(%v) != dataMap['Birth'](%v)", b.Birth, dataMap["Birth"]) + } +} + +type simpleNested struct { + A int +} + +type structWithNestedPtrValueMap struct { + NestedPtrValue map[string]*simpleNested +} + +func TestNestedPtrValueInMap(t *testing.T) { + src := &structWithNestedPtrValueMap{ + NestedPtrValue: map[string]*simpleNested{ + "x": { + A: 1, + }, + }, + } + dst := &structWithNestedPtrValueMap{ + NestedPtrValue: map[string]*simpleNested{ + "x": {}, + }, + } + if err := Map(dst, src); err != nil { + t.FailNow() + } + if dst.NestedPtrValue["x"].A == 0 { + t.Fatalf("Nested Ptr value not merged in properly: dst.NestedPtrValue[\"x\"].A(%v) != src.NestedPtrValue[\"x\"].A(%v)", dst.NestedPtrValue["x"].A, src.NestedPtrValue["x"].A) + } +} + func loadYAML(path string) (m map[string]interface{}) { m = make(map[string]interface{}) raw, _ := ioutil.ReadFile(path) _ = yaml.Unmarshal(raw, &m) return } + +type structWithMap struct { + m map[string]structWithUnexportedProperty +} + +type structWithUnexportedProperty struct { + s string +} + +func TestUnexportedProperty(t *testing.T) { + a := structWithMap{map[string]structWithUnexportedProperty{ + "key": structWithUnexportedProperty{"hello"}, + }} + b := structWithMap{map[string]structWithUnexportedProperty{ + "key": structWithUnexportedProperty{"hi"}, + }} + defer func() { + if r := recover(); r != nil { + t.Errorf("Should not have panicked") + } + }() + Merge(&a, b) +}