Skip to content

Commit

Permalink
Fix: Get() and IsSet() work correctly on nested values
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@benoitmasson
benoitmasson committed Sep 29, 2016
1 parent f513f5f commit ae72cd4
Show file tree
Hide file tree
Showing 3 changed files with 203 additions and 90 deletions.
19 changes: 10 additions & 9 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
266 changes: 189 additions & 77 deletions viper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{}
if !v.typeByDefValue {
valType = val
} else {
defVal, defExists := v.defaults[lcaseKey]
if defExists {
valType := val
if v.typeByDefValue {
path := strings.Split(lcaseKey, v.keyDelim)
defVal := v.searchMap(v.defaults, path)
if defVal != nil {
valType = defVal
} else {
valType = val
}
}

Expand Down Expand Up @@ -752,14 +841,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]
if exists {
val = v.searchMap(v.override, path)
if val != nil {
return val
}
if shadow := v.isPathShadowedInDeepMap(path, v.override); shadow != "" {
return nil
}

// PFlag override next
flag, exists := v.pflags[key]
Expand All @@ -776,79 +876,83 @@ func (v *Viper) find(key string) interface{} {
return flag.ValueString()
}
}

val, exists = v.override[key]
if exists {
return val
if shadow := v.isPathShadowedInFlatMap(path, v.pflags); shadow != "" {
return nil
}

// 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 {
if val = v.getEnv(envkey); val != "" {
return val
}
}
if shadow := v.isPathShadowedInFlatMap(path, v.env); shadow != "" {
return nil
}

// Config file next
val, exists = v.config[key]
if exists {
val = v.searchMapWithPathPrefixes(v.config, path)
if val != nil {
return val
}

// test for nested config parameter
if strings.Contains(key, v.keyDelim) {
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]
if exists {
val = v.searchMap(v.kvstore, path)
if val != nil {
return val
}
if shadow := v.isPathShadowedInDeepMap(path, v.kvstore); shadow != "" {
return nil
}

// Default as last chance
val, exists = v.defaults[key]
if exists {
// Default next
val = v.searchMap(v.defaults, path)
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
}

// 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
}

Expand Down Expand Up @@ -1033,6 +1137,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
Expand Down
Loading

0 comments on commit ae72cd4

Please sign in to comment.