Merge values for provider group in the constructor (#48)

ProviderGroup can and should merge values on construction, instead of on value retrieval.
To do that we need need apply expand to all nested YAML nodes, instead of only top level ones.

CHANGELOG.md

@@ -14,7 +14,8 @@
   other cast libraries.
 - **[Breaking]** Removed `Value.IsDefault` method.
 - **[Breaking]** Removed Load* functions.
-- **[Breaking]** Removed NewYAMLProviderFromReader* functions.
+- **[Breaking]** Unexport NewYAMLProviderFromReader* functions.
+- **[Breaking]** `NewProviderGroup` returns an error.
 
 ## v1.0.0-rc1 (26 Jun 2017)
 

config_test.go

@@ -104,7 +104,7 @@ func TestDirectAccess(t *testing.T) {
 	p, err := NewYAMLProviderFromBytes(nestedYaml)
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	provider := NewProviderGroup("test", p)
+	provider, err := NewProviderGroup("test", p)
 	require.NoError(t, err)
 
 	v, err := provider.Get("n1.id1").WithDefault("xxx")
@@ -119,8 +119,7 @@ func TestScopedAccess(t *testing.T) {
 
 	p, err := NewYAMLProviderFromBytes(nestedYaml)
 	require.NoError(t, err, "Can't create a YAML provider")
-
-	provider := NewProviderGroup("test", p)
+	provider, err := NewProviderGroup("test", p)
 	require.NoError(t, err)
 
 	p1 := provider.Get("n1")
@@ -141,7 +140,7 @@ func TestSimpleConfigValues(t *testing.T) {
 	p, err := NewYAMLProviderFromBytes(yamlConfig3)
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	provider := NewProviderGroup("test", p)
+	provider, err := NewProviderGroup("test", p)
 	require.NoError(t, err)
 
 	assert.Equal(t, 123, provider.Get("int").Value())
@@ -165,7 +164,7 @@ func TestNestedStructs(t *testing.T) {
 	p, err := NewYAMLProviderFromBytes(nestedYaml)
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	provider := NewProviderGroup("test", p)
+	provider, err := NewProviderGroup("test", p)
 	require.NoError(t, err)
 
 	str := &root{}
@@ -189,7 +188,7 @@ func TestArrayOfStructs(t *testing.T) {
 	p, err := NewYAMLProviderFromBytes(structArrayYaml)
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	provider := NewProviderGroup("test", p)
+	provider, err := NewProviderGroup("test", p)
 	require.NoError(t, err)
 
 	target := &arrayOfStructs{}
@@ -208,7 +207,7 @@ func TestDefault(t *testing.T) {
 	p, err := NewYAMLProviderFromBytes(nest1)
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	provider := NewProviderGroup("test", p)
+	provider, err := NewProviderGroup("test", p)
 	require.NoError(t, err)
 
 	target := &nested{}

example_merge_test.go

@@ -54,7 +54,10 @@ config:
 
 	// Provider is going to keep name from the base provider,
 	// but ports and pool values will be overridden by prod.
-	p := config.NewProviderGroup("merge", base, prod)
+	p, err := config.NewProviderGroup("merge", base, prod)
+	if err != nil {
+		log.Fatal(err)
+	}
 
 	var c struct {
 		Name  string

provider_group.go

@@ -21,35 +21,29 @@
 package config
 
 type providerGroup struct {
-	name      string
 	providers []Provider
+	name      string
 }
 
 // NewProviderGroup creates a configuration provider from a group of providers.
 // The highest priority provider is the last.
-func NewProviderGroup(name string, providers ...Provider) Provider {
-	return providerGroup{
-		name:      name,
-		providers: providers,
-	}
-}
-
-// Get iterates through the providers and return the value merged from
-// underlying of providers.
-//
 // The merge strategy for two objects
 // from the first provider(A) and the last provider(B) is following:
-// * if A and B are maps, A and B will form a new map with keys from
+// If A and B are maps, A and B will form a new map with keys from
 // A and B and values from B will overwrite values of A. e.g.
 //   A:                B:                 merge(A, B):
 //     keep:A            new:B              keep:A
 //     update:fromA      update:fromB       update:fromB
 //                                          new:B
 //
-// * if A is a map and B is not, this function will panic,
-// e.g. key:value and -slice
+// If A is a map and B is not, this function will return a Value with
+// an error inside.
 //
-// * in all the remaining cases B will overwrite A.
+// In all the remaining cases B will overwrite A.
+func NewProviderGroup(name string, providers ...Provider) (Provider, error) {
+	return providerGroup{providers: providers, name: name}, nil
+}
+
 func (p providerGroup) Get(key string) Value {
 	var res interface{}
 	found := false

provider_group_test.go

@@ -33,7 +33,9 @@ func TestProviderGroup(t *testing.T) {
 	p, err := NewYAMLProviderFromBytes([]byte(`id: test`))
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	pg := NewProviderGroup("test-group", p)
+	pg, err := NewProviderGroup("test-group", p)
+	require.NoError(t, err)
+
 	assert.Equal(t, "test-group", pg.Name())
 	assert.Equal(t, "test", pg.Get("id").String())
 }
@@ -44,7 +46,9 @@ func TestProviderGroupScope(t *testing.T) {
 	p, err := NewStaticProvider(map[string]interface{}{"hello": map[string]int{"world": 42}})
 	require.NoError(t, err, "Can't create a static provider")
 
-	pg := NewProviderGroup("test-group", p)
+	pg, err := NewProviderGroup("test-group", p)
+	require.NoError(t, err)
+
 	assert.Equal(t, 42, pg.Get("hello").Get("world").Value())
 }
 
@@ -87,7 +91,9 @@ logging:
 	f, err := NewYAMLProviderFromBytes(fst)
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	pg := NewProviderGroup("group", s, f)
+	pg, err := NewProviderGroup("group", s, f)
+	require.NoError(t, err)
+
 	assert.True(t, pg.Get("logging").Get("enabled").Value().(bool))
 }
 
@@ -100,23 +106,10 @@ func TestProviderGroup_GetChecksAllProviders(t *testing.T) {
 	s, err := NewStaticProvider(map[string]string{"owner": "[email protected]", "name": "fx"})
 	require.NoError(t, err, "Can't create the second provider")
 
-	pg := NewProviderGroup("test-group", f, s)
-	require.NotNil(t, pg)
+	pg, err := NewProviderGroup("test-group", f, s)
+	require.NoError(t, err)
 
 	var svc map[string]string
 	require.NoError(t, pg.Get(Root).Populate(&svc))
 	assert.Equal(t, map[string]string{"name": "fx", "owner": "[email protected]", "desc": "test"}, svc)
 }
-
-func TestProviderGroupMergeFail(t *testing.T) {
-	t.Parallel()
-
-	m, err := NewStaticProvider(map[string]interface{}{"a": map[string]string{"b": "c"}})
-	require.NoError(t, err)
-	s, err := NewStaticProvider(map[string]interface{}{"a": []string{"b"}})
-	require.NoError(t, err)
-
-	g := NewProviderGroup("group", s, m)
-	v := g.Get("a")
-	assert.False(t, v.HasValue())
-}

value.go

@@ -71,7 +71,11 @@ func (cv Value) WithDefault(value interface{}) (Value, error) {
 		return cv, err
 	}
 
-	g := NewProviderGroup("withDefault", p, cv.provider)
+	g, err := NewProviderGroup("withDefault", p, cv.provider)
+	if err != nil {
+		return cv, err
+	}
+
 	return g.Get(cv.key), nil
 }
 

yaml.go

@@ -73,18 +73,23 @@ func newYAMLProviderCore(files ...io.ReadCloser) (*yamlConfigProvider, error) {
 	}, nil
 }
 
-// We need to have a custom merge map because yamlV2 doesn't unmarshal `map[interface{}]map[interface{}]interface{}`
-// as we expect: it will replace second level maps with new maps on each unmarshal call, instead of merging them.
+// We need to have a custom merge map because yamlV2 doesn't unmarshal
+// `map[interface{}]map[interface{}]interface{}` as we expect: it will
+// replace second level maps with new maps on each unmarshal call,
+// instead of merging them.
+//
 // The merge strategy for two objects A and B is following:
-// * if A and B are maps, A and B will form a new map with keys from A and B and values from B will overwrite values of A. e.g.
+// If A and B are maps, A and B will form a new map with keys from A and B and
+// values from B will overwrite values of A. e.g.:
 //   A:                B:                 merge(A, B):
 //     keep:A            new:B              keep:A
 //     update:fromA      update:fromB       update:fromB
 //                                          new:B
 //
-// * if A is a map and B is not, this function will panic, e.g. key:value and -slice
+// If A is a map and B is not, this function will return an error,
+// e.g. key:value and -slice.
 //
-// * in all the remaining cases B will overwrite A.
+// In all the remaining cases B will overwrite A.
 func mergeMaps(dst interface{}, src interface{}) (interface{}, error) {
 	if dst == nil {
 		return src, nil
@@ -321,6 +326,30 @@ func (n *yamlNode) Children() []*yamlNode {
 	return nodes
 }
 
+// Apply expand to all nested elements of a node.
+// There is no need to use reflection, because YAML unmarshaler is using
+// map[interface{}]interface{} to store objects and []interface{}
+// to store collections.
+func recursiveApply(node interface{}, expand func(string) string) interface{} {
+	if node == nil {
+		return nil
+	}
+	switch t := node.(type) {
+	case map[interface{}]interface{}:
+		for k := range t {
+			t[k] = recursiveApply(t[k], expand)
+		}
+		return t
+	case []interface{}:
+		for i := range t {
+			t[i] = recursiveApply(t[i], expand)
+		}
+		return t
+	}
+
+	return os.Expand(fmt.Sprint(node), expand)
+}
+
 func (n *yamlNode) applyOnAllNodes(expand func(string) string) (err error) {
 
 	defer func() {
@@ -333,9 +362,7 @@ func (n *yamlNode) applyOnAllNodes(expand func(string) string) (err error) {
 		}
 	}()
 
-	if n.nodeType == valueNode && n.value != nil {
-		n.value = os.Expand(fmt.Sprint(n.value), expand)
-	}
+	n.value = recursiveApply(n.value, expand)
 
 	for _, c := range n.Children() {
 		if err := c.applyOnAllNodes(expand); err != nil {

yaml_test.go

@@ -254,7 +254,9 @@ func withYamlBytes(yamlBytes []byte, f func(Provider), t *testing.T) {
 	y, err := NewYAMLProviderFromBytes(yamlBytes)
 	require.NoError(t, err, "Can't create a YAML provider")
 
-	provider := NewProviderGroup("global", y)
+	provider, err := NewProviderGroup("global", y)
+	require.NoError(t, err)
+
 	f(provider)
 }