Fix: Get() and IsSet() work correctly on nested values

Rewriting of find(), which now checks for in-depth values,
and ignores shadowed ones.

* When looking for "foo.bar":
  - before: if "foo" was defined in some map, "foo.*" was looked for
            only in this map
  - now: "foo.*" is looked for in all maps, the first one with a value is
         used (even if "foo" was defined elsewhere, in a higher-priority map).

* Also, find() tests that the requested key is not shadowed somewhere
  on its path.
  e.g., {"foo.bar": 1} in the config map shadows "foo.bar.baz" in
  the defaults map

* Lastly, if in the config map, config["foo"]["bar"] and config["foo.bar"]
  coexist, the latter value is used.
  (this should not be necessary for other maps, since by construction
  their keys should not contain dots)

=> Get() and IsSet() corrected and simplified, since all the logic
   about value retrieval has been put in find()

+ README.md modified accordingly:
  In Section “Accessing nested keys”, to reflect the corrected behavior of find():
  - paths searched for at all levels, not only the one of the first sub-key;
  - paths may be shadowed by a shorter, higher priority, path.

+ tests modified accordingly

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
+This obeys the precedence rules established above; the search for the path
-(in this example, `datastore`) will cascade through the remaining configuration
+will cascade through the remaining configuration registries until found.
-registries until found. The search for the sub-keys (`metric` and `host`),
-however, will not.
 
-For example, if the `metric` key was not defined in the configuration loaded
+For example, given this configuration file, both `datastore.metric.host` and
-from file, but was defined in the defaults, Viper would return the zero value.
+`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
+However, if `datastore.metric` was overridden (by a flag, an environment variable,
-the remaining registries looking for it.
+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

viper.go

@@ -399,8 +399,9 @@ func (v *Viper) providerPathExists(p *defaultRemoteProvider) bool {
 	return false
 }
 
+// searchMap recursively searches for a value for path in source map.
+// Returns nil if not found.
 func (v *Viper) searchMap(source map[string]interface{}, path []string) interface{} {
-
 	if len(path) == 0 {
 		return source
 	}
@@ -430,9 +431,127 @@ func (v *Viper) searchMap(source map[string]interface{}, path []string) interfac
 			// got a value but nested key expected, return "nil" for not found
 			return nil
 		}
-	} else {
-		return nil
 	}
+	return nil
+}
+
+// searchMapWithPathPrefixes recursively searches for a value for path in source map.
+//
+// While searchMap() considers each path element as a single map key, this
+// function searches for, and prioritizes, merged path elements.
+// e.g., if in the source, "foo" is defined with a sub-key "bar", and "foo.bar"
+// is also defined, this latter value is returned for path ["foo", "bar"].
+//
+// This should be useful only at config level (other maps may not contain dots
+// in their keys).
+func (v *Viper) searchMapWithPathPrefixes(source map[string]interface{}, path []string) interface{} {
+	if len(path) == 0 {
+		return source
+	}
+
+	// search for path prefixes, starting from the longest one
+	for i := len(path); i > 0; i-- {
+		prefixKey := strings.ToLower(strings.Join(path[0:i], v.keyDelim))
+
+		var ok bool
+		var next interface{}
+		for k, v := range source {
+			if strings.ToLower(k) == prefixKey {
+				ok = true
+				next = v
+				break
+			}
+		}
+
+		if ok {
+			var val interface{}
+			switch next.(type) {
+			case map[interface{}]interface{}:
+				val = v.searchMapWithPathPrefixes(cast.ToStringMap(next), path[i:])
+			case map[string]interface{}:
+				// Type assertion is safe here since it is only reached
+				// if the type of `next` is the same as the type being asserted
+				val = v.searchMapWithPathPrefixes(next.(map[string]interface{}), path[i:])
+			default:
+				if len(path) == i {
+					val = next
+				}
+				// got a value but nested key expected, do nothing and look for next prefix
+			}
+			if val != nil {
+				return val
+			}
+		}
+	}
+
+	// not found
+	return nil
+}
+
+// isPathShadowedInDeepMap makes sure the given path is not shadowed somewhere
+// on its path in the map.
+// e.g., if "foo.bar" has a value in the given map, it “shadows”
+//       "foo.bar.baz" in a lower-priority map
+func (v *Viper) isPathShadowedInDeepMap(path []string, m map[string]interface{}) string {
+	var parentVal interface{}
+	for i := 1; i < len(path); i++ {
+		parentVal = v.searchMap(m, path[0:i])
+		if parentVal == nil {
+			// not found, no need to add more path elements
+			return ""
+		}
+		switch parentVal.(type) {
+		case map[interface{}]interface{}:
+			continue
+		case map[string]interface{}:
+			continue
+		default:
+			// parentVal is a regular value which shadows "path"
+			return strings.Join(path[0:i], v.keyDelim)
+		}
+	}
+	return ""
+}
+
+// isPathShadowedInFlatMap makes sure the given path is not shadowed somewhere
+// in a sub-path of the map.
+// e.g., if "foo.bar" has a value in the given map, it “shadows”
+//       "foo.bar.baz" in a lower-priority map
+func (v *Viper) isPathShadowedInFlatMap(path []string, mi interface{}) string {
+	// unify input map
+	var m map[string]interface{}
+	switch mi.(type) {
+	case map[string]string, map[string]FlagValue:
+		m = cast.ToStringMap(mi)
+	default:
+		return ""
+	}
+
+	// scan paths
+	var parentKey string
+	for i := 1; i < len(path); i++ {
+		parentKey = strings.Join(path[0:i], v.keyDelim)
+		if _, ok := m[parentKey]; ok {
+			return parentKey
+		}
+	}
+	return ""
+}
+
+// isPathShadowedInAutoEnv makes sure the given path is not shadowed somewhere
+// in the environment, when automatic env is on.
+// e.g., if "foo.bar" has a value in the environment, it “shadows”
+//       "foo.bar.baz" in a lower-priority map
+func (v *Viper) isPathShadowedInAutoEnv(path []string) string {
+	var parentKey string
+	var val string
+	for i := 1; i < len(path); i++ {
+		parentKey = strings.Join(path[0:i], v.keyDelim)
+		if val = v.getEnv(v.mergeWithEnvPrefix(parentKey)); val != "" {
+			return parentKey
+		}
+	}
+	return ""
 }
 
 // SetTypeByDefaultValue enables or disables the inference of a key value's
@@ -469,46 +588,16 @@ func Get(key string) interface{} { return v.Get(key) }
 func (v *Viper) Get(key string) interface{} {
 	lcaseKey := strings.ToLower(key)
 	val := v.find(lcaseKey)
-
-	if val == nil {
-		path := strings.Split(key, v.keyDelim)
-		source := v.find(strings.ToLower(path[0]))
-		if source != nil {
-			if reflect.TypeOf(source).Kind() == reflect.Map {
-				val = v.searchMap(cast.ToStringMap(source), path[1:])
-			}
-		}
-	}
-
-	// if no other value is returned and a flag does exist for the value,
-	// get the flag's value even if the flag's value has not changed
-	if val == nil {
-		if flag, exists := v.pflags[lcaseKey]; exists {
-			jww.TRACE.Println(key, "get pflag default", val)
-			switch flag.ValueType() {
-			case "int", "int8", "int16", "int32", "int64":
-				val = cast.ToInt(flag.ValueString())
-			case "bool":
-				val = cast.ToBool(flag.ValueString())
-			default:
-				val = flag.ValueString()
-			}
-		}
-	}
-
 	if val == nil {
 		return nil
 	}
 
-	var valType interface{}
+	valType := val
-	if !v.typeByDefValue {
+	if v.typeByDefValue {
-		valType = val
+		path := strings.Split(lcaseKey, v.keyDelim)
-	} else {
+		defVal := v.searchMap(v.defaults, path)
-		defVal, defExists := v.defaults[lcaseKey]
+		if defVal != nil {
-		if defExists {
 			valType = defVal
-		} else {
-			valType = val
 		}
 	}
 
@@ -756,14 +845,25 @@ func (v *Viper) find(key string) interface{} {
 	var val interface{}
 	var exists bool
 
+	// compute the path through the nested maps to the nested value
+	path := strings.Split(key, v.keyDelim)
+	if shadow := v.isPathShadowedInDeepMap(path, castMapStringToMapInterface(v.aliases)); shadow != "" {
+		return nil
+	}
+
 	// if the requested key is an alias, then return the proper key
 	key = v.realKey(key)
+	// re-compute the path
+	path = strings.Split(key, v.keyDelim)
 
 	// Set() override first
-	val, exists = v.override[key]
+	val = v.searchMap(v.override, path)
-	if exists {
+	if val != nil {
 		return val
 	}
+	if shadow := v.isPathShadowedInDeepMap(path, v.override); shadow != "" {
+		return nil
+	}
 
 	// PFlag override next
 	flag, exists := v.pflags[key]
@@ -780,18 +880,20 @@ func (v *Viper) find(key string) interface{} {
 			return flag.ValueString()
 		}
 	}
-
+	if shadow := v.isPathShadowedInFlatMap(path, v.pflags); shadow != "" {
-	val, exists = v.override[key]
+		return nil
-	if exists {
-		return val
 	}
 
+	// Env override next
 	if v.automaticEnvApplied {
 		// even if it hasn't been registered, if automaticEnv is used,
 		// check any Get request
 		if val = v.getEnv(v.mergeWithEnvPrefix(key)); val != "" {
 			return val
 		}
+		if shadow := v.isPathShadowedInAutoEnv(path); shadow != "" {
+			return nil
+		}
 	}
 	envkey, exists := v.env[key]
 	if exists {
@@ -799,39 +901,53 @@ func (v *Viper) find(key string) interface{} {
 			return val
 		}
 	}
-
+	if shadow := v.isPathShadowedInFlatMap(path, v.env); shadow != "" {
-	// Config file next
+		return nil
-	val, exists = v.config[key]
-	if exists {
-		return val
 	}
 
-	//   test for nested config parameter
+	// Config file next
-	if strings.Contains(key, v.keyDelim) {
+	val = v.searchMapWithPathPrefixes(v.config, path)
-		path := strings.Split(key, v.keyDelim)
-
-		source := v.find(path[0])
-		if source != nil {
-			if reflect.TypeOf(source).Kind() == reflect.Map {
-				val := v.searchMap(cast.ToStringMap(source), path[1:])
 	if val != nil {
 		return val
 	}
-			}
+	if shadow := v.isPathShadowedInDeepMap(path, v.config); shadow != "" {
-		}
+		return nil
 	}
 
 	// K/V store next
-	val, exists = v.kvstore[key]
+	val = v.searchMap(v.kvstore, path)
-	if exists {
+	if val != nil {
 		return val
 	}
+	if shadow := v.isPathShadowedInDeepMap(path, v.kvstore); shadow != "" {
+		return nil
+	}
 
-	// Default as last chance
+	// Default next
-	val, exists = v.defaults[key]
+	val = v.searchMap(v.defaults, path)
-	if exists {
+	if val != nil {
 		return val
 	}
+	if shadow := v.isPathShadowedInDeepMap(path, v.defaults); shadow != "" {
+		return nil
+	}
+
+	// last chance: if no other value is returned and a flag does exist for the value,
+	// get the flag's value even if the flag's value has not changed
+	if flag, exists := v.pflags[key]; exists {
+		switch flag.ValueType() {
+		case "int", "int8", "int16", "int32", "int64":
+			return cast.ToInt(flag.ValueString())
+		case "bool":
+			return cast.ToBool(flag.ValueString())
+		case "stringSlice":
+			s := strings.TrimPrefix(flag.ValueString(), "[")
+			return strings.TrimSuffix(s, "]")
+		default:
+			return flag.ValueString()
+		}
+	}
+	// last item, no need to check shadowing
 
 	return nil
 }
@@ -839,20 +955,8 @@ func (v *Viper) find(key string) interface{} {
 // IsSet checks to see if the key has been set in any of the data locations.
 func IsSet(key string) bool { return v.IsSet(key) }
 func (v *Viper) IsSet(key string) bool {
-	path := strings.Split(key, v.keyDelim)
-
 	lcaseKey := strings.ToLower(key)
 	val := v.find(lcaseKey)
-
-	if val == nil {
-		source := v.find(strings.ToLower(path[0]))
-		if source != nil {
-			if reflect.TypeOf(source).Kind() == reflect.Map {
-				val = v.searchMap(cast.ToStringMap(source), path[1:])
-			}
-		}
-	}
-
 	return val != nil
 }
 
@@ -1037,6 +1141,14 @@ func castToMapStringInterface(
 	return tgt
 }
 
+func castMapStringToMapInterface(src map[string]string) map[string]interface{} {
+	tgt := map[string]interface{}{}
+	for k, v := range src {
+		tgt[k] = v
+	}
+	return tgt
+}
+
 // mergeMaps merges two maps. The `itgt` parameter is for handling go-yaml's
 // insistence on parsing nested structures as `map[interface{}]interface{}`
 // instead of using a `string` as the key for nest structures beyond one level

viper_test.go

@@ -261,7 +261,7 @@ func TestUnmarshalling(t *testing.T) {
 	assert.False(t, InConfig("state"))
 	assert.Equal(t, "steve", Get("name"))
 	assert.Equal(t, []interface{}{"skateboarding", "snowboarding", "go"}, Get("hobbies"))
-	assert.Equal(t, map[interface{}]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[interface{}]interface{}{"size": "large"}}, Get("clothing"))
+	assert.Equal(t, map[string]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[interface{}]interface{}{"size": "large"}}, Get("clothing"))
 	assert.Equal(t, 35, Get("age"))
 }
 
@@ -424,7 +424,7 @@ func TestAllKeys(t *testing.T) {
 
 	ks := sort.StringSlice{"title", "newkey", "owner", "name", "beard", "ppu", "batters", "hobbies", "clothing", "age", "hacker", "id", "type", "eyes", "p_id", "p_ppu", "p_batters", "p_type", "p_name", "foos"}
 	dob, _ := time.Parse(time.RFC3339, "1979-05-27T07:32:00Z")
-	all := map[string]interface{}{"owner": map[string]interface{}{"organization": "MongoDB", "Bio": "MongoDB Chief Developer Advocate & Hacker at Large", "dob": dob}, "title": "TOML Example", "ppu": 0.55, "eyes": "brown", "clothing": map[string]interface{}{"trousers": "denim", "jacket": "leather", "pants": map[string]interface{}{"size": "large"}}, "id": "0001", "batters": map[string]interface{}{"batter": []interface{}{map[string]interface{}{"type": "Regular"}, map[string]interface{}{"type": "Chocolate"}, map[string]interface{}{"type": "Blueberry"}, map[string]interface{}{"type": "Devil's Food"}}}, "hacker": true, "beard": true, "hobbies": []interface{}{"skateboarding", "snowboarding", "go"}, "age": 35, "type": "donut", "newkey": "remote", "name": "Cake", "p_id": "0001", "p_ppu": "0.55", "p_name": "Cake", "p_batters": map[string]interface{}{"batter": map[string]interface{}{"type": "Regular"}}, "p_type": "donut", "foos": []map[string]interface{}{map[string]interface{}{"foo": []map[string]interface{}{map[string]interface{}{"key": 1}, map[string]interface{}{"key": 2}, map[string]interface{}{"key": 3}, map[string]interface{}{"key": 4}}}}}
+	all := map[string]interface{}{"owner": map[string]interface{}{"organization": "MongoDB", "Bio": "MongoDB Chief Developer Advocate & Hacker at Large", "dob": dob}, "title": "TOML Example", "ppu": 0.55, "eyes": "brown", "clothing": map[string]interface{}{"trousers": "denim", "jacket": "leather", "pants": map[interface{}]interface{}{"size": "large"}}, "id": "0001", "batters": map[string]interface{}{"batter": []interface{}{map[string]interface{}{"type": "Regular"}, map[string]interface{}{"type": "Chocolate"}, map[string]interface{}{"type": "Blueberry"}, map[string]interface{}{"type": "Devil's Food"}}}, "hacker": true, "beard": true, "hobbies": []interface{}{"skateboarding", "snowboarding", "go"}, "age": 35, "type": "donut", "newkey": "remote", "name": "Cake", "p_id": "0001", "p_ppu": "0.55", "p_name": "Cake", "p_batters": map[string]interface{}{"batter": map[string]interface{}{"type": "Regular"}}, "p_type": "donut", "foos": []map[string]interface{}{map[string]interface{}{"foo": []map[string]interface{}{map[string]interface{}{"key": 1}, map[string]interface{}{"key": 2}, map[string]interface{}{"key": 3}, map[string]interface{}{"key": 4}}}}}
 
 	var allkeys sort.StringSlice
 	allkeys = AllKeys()
@@ -644,7 +644,7 @@ func TestFindsNestedKeys(t *testing.T) {
 		"name":      "Cake",
 		"hacker":    true,
 		"ppu":       0.55,
-		"clothing": map[interface{}]interface{}{
+		"clothing": map[string]interface{}{
 			"jacket":   "leather",
 			"trousers": "denim",
 			"pants": map[interface{}]interface{}{
@@ -693,7 +693,7 @@ func TestReadBufConfig(t *testing.T) {
 	assert.False(t, v.InConfig("state"))
 	assert.Equal(t, "steve", v.Get("name"))
 	assert.Equal(t, []interface{}{"skateboarding", "snowboarding", "go"}, v.Get("hobbies"))
-	assert.Equal(t, map[interface{}]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[interface{}]interface{}{"size": "large"}}, v.Get("clothing"))
+	assert.Equal(t, map[string]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[interface{}]interface{}{"size": "large"}}, v.Get("clothing"))
 	assert.Equal(t, 35, v.Get("age"))
 }