Unexpected behavior of `Get...Map...` and `Sub` functions when a nested key is overridden with `Set` function

[olegch]

It appears that when nested keys are overridden with Set function, structures returned for nesting keys only include override layer values.

This behavior affects all Get...Map... and Sub functions.

At the same time Merge... and AllSettings behavior is not affected.

The unit test to show this behavior is as follows:

package viper

import (
    "bytes"
    "testing"

    "github.com/stretchr/testify/assert"
)

func TestStructureOverrides(t *testing.T) {
    // we will verify that MergeConfig works as expected, but overriding values
    // with Set function sometimes leads to unexpected results

    // create a new viper object and prepare it for loading yaml-formatted configration
    vpr := New()
    vpr.SetConfigType("yaml")

    // load first set of values
    err := vpr.ReadConfig(bytes.NewReader([]byte(`
a:
  k1: val1
`)))
    assert.Nil(t, err)

    // load second set of values
    err = vpr.MergeConfig(bytes.NewReader([]byte(`
a:
  k2: val2
`)))
    assert.Nil(t, err)

    // GetString behaves as expected
    assert.Equal(t, "val1", vpr.GetString("a.k1"))
    assert.Equal(t, "val2", vpr.GetString("a.k2"))
    assert.Equal(t, "", vpr.GetString("a.k3"))
    assert.Equal(t, "", vpr.GetString("a.k4"))

    // AllSettings behaves as expected
    assert.Equal(t, map[string]interface{}{
        "k1": "val1",
        "k2": "val2",
    }, vpr.AllSettings()["a"])

    // GetStringMapString behaves as expected
    assert.Equal(t, map[string]string{
        "k1": "val1",
        "k2": "val2",
    }, vpr.GetStringMapString("a"))

    // Sub.AllSettings behaves as expected
    assert.Equal(t, map[string]interface{}{
        "k1": "val1",
        "k2": "val2",
    }, vpr.Sub("a").AllSettings())

    // Now add a new value using Set function
    // The value goes into override layer
    vpr.Set("a.k3", "val3")

    // GetString behaves as expected
    assert.Equal(t, "val1", vpr.GetString("a.k1"))
    assert.Equal(t, "val2", vpr.GetString("a.k2"))
    assert.Equal(t, "val3", vpr.GetString("a.k3"))
    assert.Equal(t, "", vpr.GetString("a.k4"))

    // AllSettings on root behaves as expected
    assert.Equal(t, map[string]interface{}{
        "k1": "val1",
        "k2": "val2",
        "k3": "val3",
    }, vpr.AllSettings()["a"])

    // FAIL: GetStringMapString does not behave as expected!
    // Apparently it only returns map with values stored in the "override" layer;
    // it does not merge them with values from underlying layers
    assert.Equal(t, map[string]string{
        "k1": "val1",
        "k2": "val2",
        "k3": "val3",
    }, vpr.GetStringMapString("a"))

    // FAIL: Sub.AllSettings does not work as expected either
    // It seems that Sub only takes into account override layer map,
    // it does not merge it with underlying values, although underlying
    // layers may contain keys that are not overridden in the override layer.
    assert.Equal(t, map[string]interface{}{
        "k1": "val1",
        "k2": "val2",
        "k3": "val3",
    }, vpr.Sub("a").AllSettings())
}

[mxork]

another failing example:

package main

import (
    "fmt"
    "strings"

    "github.com/spf13/viper"
)

var config = `[parent]
child1 = "alice"
child2 = "bob"
`

func main() {
    viper.SetConfigType("toml")
    if err := viper.ReadConfig(strings.NewReader(config)); err != nil {
        panic(err)
    }

    viper.Set("parent.child1", "apple")

    fmt.Println(viper.AllSettings())
    fmt.Println(viper.Sub("parent").AllSettings())
}

This behavior seems to contradict the README (the bit with datastore.metric) which suggests that Set will clobber a value rather than merging, and not that it will clobber sibling values as well.

Sub is broken and should either be tagged as such in the godoc, or removed from the API.

[Thynix]

We've run into this, too - it appeared as overwriting other subkeys when calling Sub() after overriding with Set().

It looks like Get() uses find(), and Sub(), Get*Map*() do too. Where I'm currently confused is Get*() also uses Get() and it considers more than the overrides layer. I don't yet see a clear way to fix this, but having a test case is a good start.

In the hope that it's helpful, I have test cases demonstrating the problem given Set() and lack of problem given MergeConfigMap():

package main

import (
        "testing"

        "github.com/spf13/viper"
        "github.com/stretchr/testify/require"
        "gopkg.in/yaml.v2"
)

func Test_Sub_DoesNotDropsFirstChildFromParent_GivenSet(t *testing.T) {
        config := viper.New()
        err := mergeYAML(config, `
Parent:
    FirstChild: FirstValue
`)
        require.NoError(t, err)

        require.Equal(t, "FirstValue", config.GetString("Parent.FirstChild"))

        config.Set("Parent.SecondChild", "SecondValue")
        require.Equal(t, "FirstValue", config.GetString("Parent.FirstChild"))
        require.Equal(t, "SecondValue", config.GetString("Parent.SecondChild"))

        childrenConfig := config.Sub("Parent")
        require.Equal(t, "SecondValue", config.GetString("Parent.SecondChild"))
        require.Equal(t, "FirstValue", config.GetString("Parent.FirstChild"))
        require.Equal(t, "SecondValue", childrenConfig.GetString("SecondChild"))

        // Fails
        require.True(t, childrenConfig.IsSet("FirstChild"))
        // Fails with ""
        require.Equal(t, "FirstValue", childrenConfig.GetString("FirstChild"))
}

func Test_Sub_DoesNotDropsFirstChildFromParent_GivenMerge(t *testing.T) {
        config := viper.New()
        err := mergeYAML(config, `
Parent:
    FirstChild: FirstValue
`)
        require.NoError(t, err)

        require.Equal(t, "FirstValue", config.GetString("Parent.FirstChild"))

        err = mergeYAML(config, `
Parent:
    SecondChild: SecondValue
`)
        require.NoError(t, err)

        require.Equal(t, "FirstValue", config.GetString("Parent.FirstChild"))
        require.Equal(t, "SecondValue", config.GetString("Parent.SecondChild"))

        childrenConfig := config.Sub("Parent")
        require.Equal(t, "SecondValue", config.GetString("Parent.SecondChild"))
        require.Equal(t, "FirstValue", config.GetString("Parent.FirstChild"))
        require.Equal(t, "SecondValue", childrenConfig.GetString("SecondChild"))
        require.Equal(t, "FirstValue", childrenConfig.GetString("FirstChild"))
}

func mergeYAML(v *viper.Viper, input string) error {
        m := make(map[string]interface{})
        err := yaml.Unmarshal([]byte(input), &m)
        if err != nil {
                return nil
        }
        return v.MergeConfigMap(m)
}