Fixing merge behaviour for booleans

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

coll/coll.go

@@ -5,8 +5,6 @@ import (
 	"reflect"
 	"sort"
 
-	"github.com/imdario/mergo"
-
 	"github.com/hairyhenderson/gomplate/conv"
 	"github.com/pkg/errors"
 )
@@ -169,14 +167,48 @@ func Reverse(list interface{}) ([]interface{}, error) {
 // the left-most values taking precedence over the right-most.
 func Merge(dst map[string]interface{}, srcs ...map[string]interface{}) (map[string]interface{}, error) {
 	for _, src := range srcs {
-		err := mergo.Merge(&dst, src)
-		if err != nil {
-			return nil, err
-		}
+		dst = mergeValues(src, dst)
 	}
 	return dst, nil
 }
 
+func copyMap(m map[string]interface{}) map[string]interface{} {
+	n := map[string]interface{}{}
+	for k, v := range m {
+		n[k] = v
+	}
+	return n
+}
+
+// Merges a default and override map
+func mergeValues(d map[string]interface{}, o map[string]interface{}) map[string]interface{} {
+	def := copyMap(d)
+	over := copyMap(o)
+	for k, v := range over {
+		// If the key doesn't exist already, then just set the key to that value
+		if _, exists := def[k]; !exists {
+			def[k] = v
+			continue
+		}
+		nextMap, ok := v.(map[string]interface{})
+		// If it isn't another map, overwrite the value
+		if !ok {
+			def[k] = v
+			continue
+		}
+		// Edge case: If the key exists in the default, but isn't a map
+		defMap, isMap := def[k].(map[string]interface{})
+		// If the override map has a map for this key, prefer it
+		if !isMap {
+			def[k] = v
+			continue
+		}
+		// If we got to this point, it is a map in both, so merge them
+		def[k] = mergeValues(defMap, nextMap)
+	}
+	return def
+}
+
 // Sort a given array or slice. Uses natural sort order if possible. If a
 // non-empty key is given and the list elements are maps, this will attempt to
 // sort by the values of those entries.

coll/coll_test.go

@@ -228,6 +228,56 @@ func TestMerge(t *testing.T) {
 	out, err = Merge(dst, src, src2)
 	assert.NoError(t, err)
 	assert.EqualValues(t, expected, out)
+
+	dst = map[string]interface{}{"a": false, "c": 5}
+	src = map[string]interface{}{"a": true, "b": 2, "c": 3}
+	src2 = map[string]interface{}{"a": true, "b": 2, "c": 3, "d": 4}
+	expected = map[string]interface{}{
+		"a": dst["a"], "b": src["b"], "c": dst["c"], "d": src2["d"],
+	}
+
+	out, err = Merge(dst, src, src2)
+	assert.NoError(t, err)
+	assert.EqualValues(t, expected, out)
+
+	dst = map[string]interface{}{"a": true, "c": 5}
+	src = map[string]interface{}{"a": false,
+		"b": map[string]interface{}{
+			"ca": "foo",
+		},
+	}
+	src2 = map[string]interface{}{"a": false, "b": 2, "c": 3, "d": 4}
+	expected = map[string]interface{}{
+		"a": dst["a"], "b": src["b"], "c": dst["c"], "d": src2["d"],
+	}
+
+	out, err = Merge(dst, src, src2)
+	assert.NoError(t, err)
+	assert.EqualValues(t, expected, out)
+
+	dst = map[string]interface{}{
+		"a": true,
+		"b": map[string]interface{}{
+			"ca": "foo",
+			"cb": "bar",
+		},
+		"c": 5}
+	src = map[string]interface{}{
+		"a": false,
+		"b": map[string]interface{}{
+			"ca": 8,
+		},
+	}
+	expected = map[string]interface{}{
+		"a": dst["a"], "b": map[string]interface{}{
+			"ca": "foo",
+			"cb": "bar",
+		}, "c": dst["c"],
+	}
+
+	out, err = Merge(dst, src)
+	assert.NoError(t, err)
+	assert.EqualValues(t, expected, out)
 }
 
 type coords struct {

data/datasource_merge.go

@@ -3,7 +3,7 @@ package data
 import (
 	"strings"
 
-	"github.com/imdario/mergo"
+	"github.com/hairyhenderson/gomplate/coll"
 
 	"github.com/pkg/errors"
 )
@@ -63,14 +63,13 @@ func (d *Data) readMerge(source *Source, args ...string) ([]byte, error) {
 	return b, nil
 }
 
-func mergeData(data []map[string]interface{}) ([]byte, error) {
+func mergeData(data []map[string]interface{}) (out []byte, err error) {
 	dst := data[0]
 	data = data[1:]
-	for _, datum := range data {
-		err := mergo.Merge(&dst, datum)
-		if err != nil {
-			return nil, errors.Wrap(err, "failed to merge datasources")
-		}
+
+	dst, err = coll.Merge(dst, data...)
+	if err != nil {
+		return nil, err
 	}
 
 	s, err := ToYAML(dst)

data/datasource_merge_test.go

@@ -10,23 +10,23 @@ import (
 )
 
 func TestReadMerge(t *testing.T) {
-	jsonContent := []byte(`{"hello": "world"}`)
-	yamlContent := []byte("hello: earth\ngoodnight: moon\n")
-	arrayContent := []byte(`["hello", "world"]`)
+	jsonContent := `{"hello": "world"}`
+	yamlContent := "hello: earth\ngoodnight: moon\n"
+	arrayContent := `["hello", "world"]`
 
-	mergedContent := []byte("goodnight: moon\nhello: world\n")
+	mergedContent := "goodnight: moon\nhello: world\n"
 
 	fs := afero.NewMemMapFs()
 
 	_ = fs.Mkdir("/tmp", 0777)
 	f, _ := fs.Create("/tmp/jsonfile.json")
-	_, _ = f.Write(jsonContent)
+	_, _ = f.WriteString(jsonContent)
 	f, _ = fs.Create("/tmp/array.json")
-	_, _ = f.Write(arrayContent)
+	_, _ = f.WriteString(arrayContent)
 	f, _ = fs.Create("/tmp/yamlfile.yaml")
-	_, _ = f.Write(yamlContent)
+	_, _ = f.WriteString(yamlContent)
 	f, _ = fs.Create("/tmp/textfile.txt")
-	_, _ = f.Write([]byte(`plain text...`))
+	_, _ = f.WriteString(`plain text...`)
 
 	source := &Source{Alias: "foo", URL: mustParseURL("merge:file:///tmp/jsonfile.json|file:///tmp/yamlfile.yaml")}
 	source.fs = fs
@@ -44,12 +44,12 @@ func TestReadMerge(t *testing.T) {
 
 	actual, err := d.readMerge(source)
 	assert.NoError(t, err)
-	assert.Equal(t, mergedContent, actual)
+	assert.Equal(t, mergedContent, string(actual))
 
 	source.URL = mustParseURL("merge:bar|baz")
 	actual, err = d.readMerge(source)
 	assert.NoError(t, err)
-	assert.Equal(t, mergedContent, actual)
+	assert.Equal(t, mergedContent, string(actual))
 
 	source.URL = mustParseURL("merge:file:///tmp/jsonfile.json")
 	_, err = d.readMerge(source)
@@ -71,3 +71,61 @@ func TestReadMerge(t *testing.T) {
 	_, err = d.readMerge(source)
 	assert.Error(t, err)
 }
+
+func TestMergeData(t *testing.T) {
+	def := map[string]interface{}{
+		"f": true,
+		"t": false,
+		"z": "def",
+	}
+	out, err := mergeData([]map[string]interface{}{def})
+	assert.NoError(t, err)
+	assert.Equal(t, "f: true\nt: false\nz: def\n", string(out))
+
+	over := map[string]interface{}{
+		"f": false,
+		"t": true,
+		"z": "over",
+	}
+	out, err = mergeData([]map[string]interface{}{over, def})
+	assert.NoError(t, err)
+	assert.Equal(t, "f: false\nt: true\nz: over\n", string(out))
+
+	over = map[string]interface{}{
+		"f": false,
+		"t": true,
+		"z": "over",
+		"m": map[string]interface{}{
+			"a": "aaa",
+		},
+	}
+	out, err = mergeData([]map[string]interface{}{over, def})
+	assert.NoError(t, err)
+	assert.Equal(t, "f: false\nm:\n  a: aaa\nt: true\nz: over\n", string(out))
+
+	uber := map[string]interface{}{
+		"z": "über",
+	}
+	out, err = mergeData([]map[string]interface{}{uber, over, def})
+	assert.NoError(t, err)
+	assert.Equal(t, "f: false\nm:\n  a: aaa\nt: true\nz: über\n", string(out))
+
+	uber = map[string]interface{}{
+		"m": "notamap",
+		"z": map[string]interface{}{
+			"b": "bbb",
+		},
+	}
+	out, err = mergeData([]map[string]interface{}{uber, over, def})
+	assert.NoError(t, err)
+	assert.Equal(t, "f: false\nm: notamap\nt: true\nz:\n  b: bbb\n", string(out))
+
+	uber = map[string]interface{}{
+		"m": map[string]interface{}{
+			"b": "bbb",
+		},
+	}
+	out, err = mergeData([]map[string]interface{}{uber, over, def})
+	assert.NoError(t, err)
+	assert.Equal(t, "f: false\nm:\n  a: aaa\n  b: bbb\nt: true\nz: over\n", string(out))
+}

docs-src/content/functions/coll.yml

@@ -238,7 +238,7 @@ funcs:
 
       Many source maps can be provided. Precedence is in left-to-right order.
 
-      Note that this function _changes_ the destination map.
+      _Note that this function does not modify the input._
     pipeline: true
     arguments:
       - name: dst

docs/content/datasources.md

@@ -61,7 +61,7 @@ Gomplate supports a number of datasources, each specified with a particular URL
 | [Environment](#using-env-datasources) | `env` | Environment variables can be used as datasources - useful for testing |
 | [File](#using-file-datasources) | `file` | Files can be read in any of the [supported formats](#mime-types), including by piping through standard input (`Stdin`). [Directories](#directory-datasources) are also supported. |
 | [HTTP](#using-http-datasources) | `http`, `https` | Data can be sourced from HTTP/HTTPS sites in many different formats. Arbitrary HTTP headers can be set with the [`--datasource-header`/`-H`][] flag |
-| [Merged Datasources](#using-merge-datasources) | `merge` | Merge two or more datasources together to produce the final value - useful for resolving defaults. |
+| [Merged Datasources](#using-merge-datasources) | `merge` | Merge two or more datasources together to produce the final value - useful for resolving defaults. Uses [`coll.Merge`][] for merging. |
 | [Stdin](#using-stdin-datasources) | `stdin` | A special case of the `file` datasource; allows piping through standard input (`Stdin`) |
 | [Vault](#using-vault-datasources) | `vault`, `vault+http`, `vault+https` | [HashiCorp Vault][] is an industry-leading open-source secret management tool. [List support](#directory-datasources) is also available. |
 
@@ -371,6 +371,8 @@ datasource values _override_ those to the right).
 Multiple different formats can be mixed, as long as they produce maps with string
 keys as their data type.
 
+The [`coll.Merge`][] function is used to perform the merge operation.
+
 ### Merging separately-defined datasources
 
 Consider this example:
@@ -528,6 +530,7 @@ The file `/tmp/vault-aws-nonce` will be created if it didn't already exist, and
 [`data.JSONArray`]: ../functions/data/#data-jsonarray
 [`data.TOML`]: ../functions/data/#data-toml
 [`data.YAML`]: ../functions/data/#data-yaml
+[`coll.Merge`]: ../functions/coll/#coll-merge
 
 [AWS SMP]: https://aws.amazon.com/systems-manager/features#Parameter_Store
 [BoltDB]: https://github.com/boltdb/bolt

docs/content/functions/coll.md

@@ -382,7 +382,7 @@ map can be configured the "overrides".
 
 Many source maps can be provided. Precedence is in left-to-right order.
 
-Note that this function _changes_ the destination map.
+_Note that this function does not modify the input._
 
 ### Usage
 

tests/integration/datasources_merge_test.go

@@ -22,8 +22,8 @@ var _ = Suite(&MergeDatasourceSuite{})
 func (s *MergeDatasourceSuite) SetUpSuite(c *C) {
 	s.tmpDir = fs.NewDir(c, "gomplate-inttests",
 		fs.WithFiles(map[string]string{
-			"config.json": `{"foo": {"bar": "baz"}}`,
-			"default.yml": "foo:\n  bar: qux\nother: true\n",
+			"config.json": `{"foo": {"bar": "baz"}, "isDefault": false, "isOverride": true}`,
+			"default.yml": "foo:\n  bar: qux\nother: true\nisDefault: true\nisOverride: false\n",
 		}),
 	)
 
@@ -49,21 +49,21 @@ func (s *MergeDatasourceSuite) TestMergeDatasource(c *C) {
 		"-d", "config=merge:user|default",
 		"-i", `{{ ds "config" | toJSON }}`,
 	)
-	result.Assert(c, icmd.Expected{ExitCode: 0, Out: `{"foo":{"bar":"baz"},"other":true}`})
+	result.Assert(c, icmd.Expected{ExitCode: 0, Out: `{"foo":{"bar":"baz"},"isDefault":false,"isOverride":true,"other":true}`})
 
 	result = icmd.RunCommand(GomplateBin,
 		"-d", "default="+s.tmpDir.Join("default.yml"),
 		"-d", "config=merge:user|default",
 		"-i", `{{ defineDatasource "user" `+"`"+s.tmpDir.Join("config.json")+"`"+` }}{{ ds "config" | toJSON }}`,
 	)
-	result.Assert(c, icmd.Expected{ExitCode: 0, Out: `{"foo":{"bar":"baz"},"other":true}`})
+	result.Assert(c, icmd.Expected{ExitCode: 0, Out: `{"foo":{"bar":"baz"},"isDefault":false,"isOverride":true,"other":true}`})
 
 	result = icmd.RunCommand(GomplateBin,
 		"-d", "default="+s.tmpDir.Join("default.yml"),
 		"-d", "config=merge:http://"+s.l.Addr().String()+"/foo.json|default",
 		"-i", `{{ ds "config" | toJSON }}`,
 	)
-	result.Assert(c, icmd.Expected{ExitCode: 0, Out: `{"foo":"bar","other":true}`})
+	result.Assert(c, icmd.Expected{ExitCode: 0, Out: `{"foo":"bar","isDefault":true,"isOverride":false,"other":true}`})
 
 	result = icmd.RunCommand(GomplateBin,
 		"-c", "merged=merge:http://"+s.l.Addr().String()+"/2.env|http://"+s.l.Addr().String()+"/1.env",