Nested maps (#195)

Fixes #71, #93, #158, #168, #209, #141, #160, #162, #190 * Fixed: indentation in comment * Fixed: Get() returns nil when nested element not found * Fixed: insensitiviseMaps() made recursive so that nested keys are lowercased * Fixed: order of expected<=>actual in assert.Equal() statements * Fixed: find() looks into "overrides" first * Fixed: TestBindPFlags() to use a new Viper instance * Fixed: removed extra aliases from display in Debug() * Added: test for checking precedence of dot-containing keys. * Fixed: Set() and SetDefault() insert nested values * Added: tests for overriding nested values * Changed: AllKeys() includes all keys / AllSettings() includes overridden nested values * Added: test for shadowed nested key * Fixed: properties parsing generates nested maps * Fixed: Get() and IsSet() work correctly on nested values * Changed: modifier README.md to reflect changes
spf13 · Oct 8, 2016 · ec4eb2f · ec4eb2f
1 parent 670c42a
commit ec4eb2f
Show file tree

Hide file tree

Showing 5 changed files with 578 additions and 150 deletions.
diff --git a/README.md b/README.md
@@ -458,16 +458,17 @@ Viper can access a nested field by passing a `.` delimited path of keys:
 GetString("datastore.metric.host") // (returns "127.0.0.1")
 ```
 
-This obeys the precedence rules established above; the search for the root key
-(in this example, `datastore`) will cascade through the remaining configuration
-registries until found. The search for the sub-keys (`metric` and `host`),
-however, will not.
+This obeys the precedence rules established above; the search for the path
+will cascade through the remaining configuration registries until found.
 
-For example, if the `metric` key was not defined in the configuration loaded
-from file, but was defined in the defaults, Viper would return the zero value.
+For example, given this configuration file, both `datastore.metric.host` and
+`datastore.metric.port` are already defined (and may be overridden). If in addition
+`datastore.metric.protocol` was defined in the defaults, Viper would also find it.
 
-On the other hand, if the primary key was not defined, Viper would go through
-the remaining registries looking for it.
+However, if `datastore.metric` was overridden (by a flag, an environment variable,
+the `Set()` method, …) with an immediate value, then all sub-keys of
+`datastore.metric` become undefined, they are “shadowed” by the higher-priority
+configuration level.
 
 Lastly, if there exists a key that matches the delimited key path, its value
 will be returned instead. E.g.
@@ -491,7 +492,7 @@ will be returned instead. E.g.
     }
 }
 
-GetString("datastore.metric.host") //returns "0.0.0.0"
+GetString("datastore.metric.host") // returns "0.0.0.0"
 ```
 
 ### Extract sub-tree

diff --git a/overrides_test.go b/overrides_test.go
@@ -0,0 +1,173 @@
+package viper
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cast"
+	"github.com/stretchr/testify/assert"
+)
+
+type layer int
+
+const (
+	defaultLayer layer = iota + 1
+	overrideLayer
+)
+
+func TestNestedOverrides(t *testing.T) {
+	assert := assert.New(t)
+	var v *Viper
+
+	// Case 0: value overridden by a value
+	overrideDefault(assert, "tom", 10, "tom", 20) // "tom" is first given 10 as default value, then overridden by 20
+	override(assert, "tom", 10, "tom", 20)        // "tom" is first given value 10, then overridden by 20
+	overrideDefault(assert, "tom.age", 10, "tom.age", 20)
+	override(assert, "tom.age", 10, "tom.age", 20)
+	overrideDefault(assert, "sawyer.tom.age", 10, "sawyer.tom.age", 20)
+	override(assert, "sawyer.tom.age", 10, "sawyer.tom.age", 20)
+
+	// Case 1: key:value overridden by a value
+	v = overrideDefault(assert, "tom.age", 10, "tom", "boy") // "tom.age" is first given 10 as default value, then "tom" is overridden by "boy"
+	assert.Nil(v.Get("tom.age"))                             // "tom.age" should not exist anymore
+	v = override(assert, "tom.age", 10, "tom", "boy")
+	assert.Nil(v.Get("tom.age"))
+
+	// Case 2: value overridden by a key:value
+	overrideDefault(assert, "tom", "boy", "tom.age", 10) // "tom" is first given "boy" as default value, then "tom" is overridden by map{"age":10}
+	override(assert, "tom.age", 10, "tom", "boy")
+
+	// Case 3: key:value overridden by a key:value
+	v = overrideDefault(assert, "tom.size", 4, "tom.age", 10)
+	assert.Equal(4, v.Get("tom.size")) // value should still be reachable
+	v = override(assert, "tom.size", 4, "tom.age", 10)
+	assert.Equal(4, v.Get("tom.size"))
+	deepCheckValue(assert, v, overrideLayer, []string{"tom", "size"}, 4)
+
+	// Case 4: key:value overridden by a map
+	v = overrideDefault(assert, "tom.size", 4, "tom", map[string]interface{}{"age": 10}) // "tom.size" is first given "4" as default value, then "tom" is overridden by map{"age":10}
+	assert.Equal(4, v.Get("tom.size"))                                                   // "tom.size" should still be reachable
+	assert.Equal(10, v.Get("tom.age"))                                                   // new value should be there
+	deepCheckValue(assert, v, overrideLayer, []string{"tom", "age"}, 10)                 // new value should be there
+	v = override(assert, "tom.size", 4, "tom", map[string]interface{}{"age": 10})
+	assert.Nil(v.Get("tom.size"))
+	assert.Equal(10, v.Get("tom.age"))
+	deepCheckValue(assert, v, overrideLayer, []string{"tom", "age"}, 10)
+
+	// Case 5: array overridden by a value
+	overrideDefault(assert, "tom", []int{10, 20}, "tom", 30)
+	override(assert, "tom", []int{10, 20}, "tom", 30)
+	overrideDefault(assert, "tom.age", []int{10, 20}, "tom.age", 30)
+	override(assert, "tom.age", []int{10, 20}, "tom.age", 30)
+
+	// Case 6: array overridden by an array
+	overrideDefault(assert, "tom", []int{10, 20}, "tom", []int{30, 40})
+	override(assert, "tom", []int{10, 20}, "tom", []int{30, 40})
+	overrideDefault(assert, "tom.age", []int{10, 20}, "tom.age", []int{30, 40})
+	v = override(assert, "tom.age", []int{10, 20}, "tom.age", []int{30, 40})
+	// explicit array merge:
+	s, ok := v.Get("tom.age").([]int)
+	if assert.True(ok, "tom[\"age\"] is not a slice") {
+		v.Set("tom.age", append(s, []int{50, 60}...))
+		assert.Equal([]int{30, 40, 50, 60}, v.Get("tom.age"))
+		deepCheckValue(assert, v, overrideLayer, []string{"tom", "age"}, []int{30, 40, 50, 60})
+	}
+}
+
+func overrideDefault(assert *assert.Assertions, firstPath string, firstValue interface{}, secondPath string, secondValue interface{}) *Viper {
+	return overrideFromLayer(defaultLayer, assert, firstPath, firstValue, secondPath, secondValue)
+}
+func override(assert *assert.Assertions, firstPath string, firstValue interface{}, secondPath string, secondValue interface{}) *Viper {
+	return overrideFromLayer(overrideLayer, assert, firstPath, firstValue, secondPath, secondValue)
+}
+
+// overrideFromLayer performs the sequential override and low-level checks.
+//
+// First assignment is made on layer l for path firstPath with value firstValue,
+// the second one on the override layer (i.e., with the Set() function)
+// for path secondPath with value secondValue.
+//
+// firstPath and secondPath can include an arbitrary number of dots to indicate
+// a nested element.
+//
+// After each assignment, the value is checked, retrieved both by its full path
+// and by its key sequence (successive maps).
+func overrideFromLayer(l layer, assert *assert.Assertions, firstPath string, firstValue interface{}, secondPath string, secondValue interface{}) *Viper {
+	v := New()
+	firstKeys := strings.Split(firstPath, v.keyDelim)
+	if assert == nil ||
+		len(firstKeys) == 0 || len(firstKeys[0]) == 0 {
+		return v
+	}
+
+	// Set and check first value
+	switch l {
+	case defaultLayer:
+		v.SetDefault(firstPath, firstValue)
+	case overrideLayer:
+		v.Set(firstPath, firstValue)
+	default:
+		return v
+	}
+	assert.Equal(firstValue, v.Get(firstPath))
+	deepCheckValue(assert, v, l, firstKeys, firstValue)
+
+	// Override and check new value
+	secondKeys := strings.Split(secondPath, v.keyDelim)
+	if len(secondKeys) == 0 || len(secondKeys[0]) == 0 {
+		return v
+	}
+	v.Set(secondPath, secondValue)
+	assert.Equal(secondValue, v.Get(secondPath))
+	deepCheckValue(assert, v, overrideLayer, secondKeys, secondValue)
+
+	return v
+}
+
+// deepCheckValue checks that all given keys correspond to a valid path in the
+// configuration map of the given layer, and that the final value equals the one given
+func deepCheckValue(assert *assert.Assertions, v *Viper, l layer, keys []string, value interface{}) {
+	if assert == nil || v == nil ||
+		len(keys) == 0 || len(keys[0]) == 0 {
+		return
+	}
+
+	// init
+	var val interface{}
+	var ms string
+	switch l {
+	case defaultLayer:
+		val = v.defaults
+		ms = "v.defaults"
+	case overrideLayer:
+		val = v.override
+		ms = "v.override"
+	}
+
+	// loop through map
+	var m map[string]interface{}
+	err := false
+	for _, k := range keys {
+		if val == nil {
+			assert.Fail(fmt.Sprintf("%s is not a map[string]interface{}", ms))
+			return
+		}
+
+		// deep scan of the map to get the final value
+		switch val.(type) {
+		case map[interface{}]interface{}:
+			m = cast.ToStringMap(val)
+		case map[string]interface{}:
+			m = val.(map[string]interface{})
+		default:
+			assert.Fail(fmt.Sprintf("%s is not a map[string]interface{}", ms))
+			return
+		}
+		ms = ms + "[\"" + k + "\"]"
+		val = m[k]
+	}
+	if !err {
+		assert.Equal(value, val)
+	}
+}
diff --git a/util.go b/util.go
@@ -45,6 +45,10 @@ func insensitiviseMap(m map[string]interface{}) {
 		if key != lower {
 			delete(m, key)
 			m[lower] = val
+			if m2, ok := val.(map[string]interface{}); ok {
+				// nested map: recursively insensitivise
+				insensitiviseMap(m2)
+			}
 		}
 	}
 }
@@ -149,7 +153,12 @@ func unmarshallConfigReader(in io.Reader, c map[string]interface{}, configType s
 		}
 		for _, key := range p.Keys() {
 			value, _ := p.Get(key)
-			c[key] = value
+			// recursively build nested maps
+			path := strings.Split(key, ".")
+			lastKey := strings.ToLower(path[len(path)-1])
+			deepestMap := deepSearch(c, path[0:len(path)-1])
+			// set innermost value
+			deepestMap[lastKey] = value
 		}
 	}
 
@@ -199,3 +208,34 @@ func parseSizeInBytes(sizeStr string) uint {
 
 	return safeMul(uint(size), multiplier)
 }
+
+// deepSearch scans deep maps, following the key indexes listed in the
+// sequence "path".
+// The last value is expected to be another map, and is returned.
+//
+// In case intermediate keys do not exist, or map to a non-map value,
+// a new map is created and inserted, and the search continues from there:
+// the initial map "m" may be modified!
+func deepSearch(m map[string]interface{}, path []string) map[string]interface{} {
+	for _, k := range path {
+		m2, ok := m[k]
+		if !ok {
+			// intermediate key does not exist
+			// => create it and continue from there
+			m3 := make(map[string]interface{})
+			m[k] = m3
+			m = m3
+			continue
+		}
+		m3, ok := m2.(map[string]interface{})
+		if !ok {
+			// intermediate key is a value
+			// => replace with a new map
+			m3 = make(map[string]interface{})
+			m[k] = m3
+		}
+		// continue search from here
+		m = m3
+	}
+	return m
+}