Flat configuration does not work as expected when unmarshalling nested objects

[pkaila]

Describe the bug

If I specify my configuration using flat keys, I would still expect the unmarshalling to work with nested objects. Now it looks like flat keys (at least when parsed from JSON or HJSON) are not being unmarshalled to the nested objects.

To Reproduce

TLDR: https://go.dev/play/p/YfhKcAgPe-6

Define these structs:

type Config struct {
    TopLevelKey string    `koanf:"topLevelKey"`
    Sub         SubConfig `koanf:"sub"`
}

type SubConfig struct {
    SubLevelKey string `koanf:"subLevelKey"`
}

Define JSON string like this:

const flatJsonString = `
{
    "topLevelKey": "topLevelValue",
    "sub.subLevelKey": "subLevelValue"
}
`

Now run the following:

    k := koanf.New(".")
    _ = k.Load(rawbytes.Provider([]byte(flatJsonString)), json.Parser())

    c := Config{}
    _ = k.Unmarshal("", &c)

    fmt.Println(k.String("sub.subLevelKey") == "subLevelValue") // prints true, as expected
    fmt.Println(c.Sub.SubLevelKey == "subLevelValue")           // prints false, as not expected

Expected behavior

My expectation is that the configuration parsing would treat flat and non-flat configurations the same. As in:

{
    "sub": {
        "subLevelKey": "subLevelValue"
    }
}

and

{
    "sub.subLevelKey": "subLevelValue"
}

would be equivalent as long as I specify . as the delimiter.

When parsing configurations from non-structural sources like the environment variables it seems to be the case that the keys are always treated as flat keys and parsed accordingly.

Please provide the following information):

• OS: osx (or Go Playground)
• Koanf Version v1.5.0

Additional context

I have a layer of configurations where there is a default configuration which is defined with the proper structure. And then there are possibilities of adding overriding configuration files, which only override parts of the configuration. I bumped into this issue when I was trying to override one value in a deeply nested configuration and was doing it with a flat key as it seemed like the intuitive and easiest way to do it (also easiest to read).

I did find a workaround, which is to define a merge function for loading the information. The merge function I came up with is this:

func unFlatteningMergeFunc(src, dest map[string]interface{}) error {
    // Unflatten does not fully unflatten unless the config is first fully flattened.
    flat, _ := maps.Flatten(src, nil, ".")
    maps.Merge(maps.Unflatten(flat, "."), dest)
    return nil
}

Edit: Fixed the workaround as it failed to work in cases where the source config was not fully flattened or unflattened.

[rhnvrm]

I'm not sure if we can classify this as a bug, but it might be a good feature add nonetheless.

I think it may be better to add an option to unflatten keys before doing the decode and provide that as a config inside UnmarshalWithConf. This is not the default behavior so it would be easier to include as an opt-in utility feature.

diff --git a/koanf.go b/koanf.go
index 665f0df..a843263 100644
--- a/koanf.go
+++ b/koanf.go
@@ -60,8 +60,10 @@ type UnmarshalConf struct {
    //  Type       string `koanf:"json"`
    // }
    // ```
-   FlatPaths     bool
-   DecoderConfig *mapstructure.DecoderConfig
+   FlatPaths bool
+
+   UnflattenPaths bool
+   DecoderConfig  *mapstructure.DecoderConfig
 }

 // New returns a new instance of Koanf. delim is the delimiter to use
@@ -279,6 +281,14 @@ func (ko *Koanf) UnmarshalWithConf(path string, o interface{}, c UnmarshalConf)
        }
    }

+   if c.UnflattenPaths {
+       if f, ok := mp.(map[string]interface{}); ok {
+           fc := maps.Copy(f)
+           fmp := maps.Unflatten(fc, ko.conf.Delim)
+           mp = fmp
+       }
+   }
+
    return d.Decode(mp)
 }

// You can edit this code!
// Click here and start typing.
package main

import (
    "log"

    "github.com/knadh/koanf/parsers/json"
    "github.com/knadh/koanf/providers/rawbytes"
    "github.com/knadh/koanf/v2"
)

type Config struct {
    TopLevelKey string    `koanf:"topLevelKey"`
    Sub         SubConfig `koanf:"sub"`
}

type SubConfig struct {
    SubLevelKey string `koanf:"subLevelKey"`
}

const flatJsonString = `
{
    "topLevelKey": "topLevelValue",
    "sub.subLevelKey": "subLevelValue"
}`

const nonFlatJsonString = `
{
    "topLevelKey": "topLevelValue",
    "sub": {"subLevelKey": "subLevelValue"}
}`

func do(s []byte) {
    k := koanf.New(".")
    err := k.Load(rawbytes.Provider(s), json.Parser())
    if err != nil {
        log.Fatal(err)
    }

    log.Printf("%#v\n", k.Raw())

    c := Config{}
    err = k.UnmarshalWithConf("", &c, koanf.UnmarshalConf{UnflattenPaths: true})
    if err != nil {
        log.Fatal(err)
    }

    log.Printf("%#v\n", c)
}

func main() {
    do([]byte(flatJsonString))

    log.Println("\n0---")

    do([]byte(nonFlatJsonString))
}

Before this patch, we can see that the koanf map is different.

2023/10/13 13:35:15 map[string]interface {}{"sub.subLevelKey":"subLevelValue", "topLevelKey":"topLevelValue"}
2023/10/13 13:35:15 main.Config{TopLevelKey:"topLevelValue", Sub:main.SubConfig{SubLevelKey:""}}
2023/10/13 13:35:15 
0---
2023/10/13 13:35:15 map[string]interface {}{"sub":map[string]interface {}{"subLevelKey":"subLevelValue"}, "topLevelKey":"topLevelValue"}
2023/10/13 13:35:15 main.Config{TopLevelKey:"topLevelValue", Sub:main.SubConfig{SubLevelKey:"subLevelValue"}}

After this patch, we can see that the Unflatten step is able to convert it into an appropriate map.

2023/10/13 13:34:12 map[string]interface {}{"sub.subLevelKey":"subLevelValue", "topLevelKey":"topLevelValue"}
2023/10/13 13:34:12 main.Config{TopLevelKey:"topLevelValue", Sub:main.SubConfig{SubLevelKey:"subLevelValue"}}
2023/10/13 13:34:12 
0---
2023/10/13 13:34:12 map[string]interface {}{"sub":map[string]interface {}{"subLevelKey":"subLevelValue"}, "topLevelKey":"topLevelValue"}
2023/10/13 13:34:12 main.Config{TopLevelKey:"topLevelValue", Sub:main.SubConfig{SubLevelKey:"subLevelValue"}}

[pkaila]

Granted, I wouldn't maybe classify this as a bug either. The bug label seems to have been added automatically.

In the example you provided, how would the unmarshalling behave in the case where the configuration files are mixing the usage of flat/unflat keys? This is the case I have (the default configurations are using unflat keys, and the override is using flat keys). I think that should probably be handled already when loading the configuration, not when doing the unmarshalling? Because if you wait until the marshalling step you don't anymore know which of the configurations was loaded last and should take effect.

For example:

package main

import (
    "fmt"

    "github.com/knadh/koanf/parsers/json"
    "github.com/knadh/koanf/providers/rawbytes"
    "github.com/knadh/koanf/v2"
)

const jsonString = `
{
    "topLevelKey": "jsonTopValue",
    "sub": {
        "subLevelKey": "jsonSubValue"
    }
}`

const flatJsonString = `
{
    "topLevelKey": "flatJsonTopValue",
    "sub.subLevelKey": "flatJsonSubValue"
}`

func main() {
    k := koanf.New(".")
    _ = k.Load(rawbytes.Provider([]byte(jsonString)), json.Parser())
    _ = k.Load(rawbytes.Provider([]byte(flatJsonString)), json.Parser())

    fmt.Println(k.Raw())
}

This prints:

map[sub:map[subLevelKey:jsonSubValue] sub.subLevelKey:flatJsonSubValue topLevelKey:flatJsonTopValue]

As you can see from the output, if the unmarshalling step would use the unflat value for the subLevelKey it would differ from the flat value for the subLevelKey. And vice versa. This could provide quite unexpected results.

Thus, if you would want to make this an option, I think it should be an option on the koanf instance instead of the unmarshalling step.

Something like:

koanf.NewWithConf(koanf.Conf{
  Delim:       ".",
  StrictMerge: false,
  UnflattenPaths: true,
})

[pkaila]

If anyone bumped in to this issue earlier and used my workaround, I'd like to point that the workaround had a flaw. I've fixed my original workaround in the first post, but here is the new workaround again, just in case:

func unFlatteningMergeFunc(src, dest map[string]interface{}) error {
    // Unflatten does not fully unflatten unless the config is first fully flattened.
    flat, _ := maps.Flatten(src, nil, ".")
    maps.Merge(maps.Unflatten(flat, "."), dest)
    return nil
}

The previous version did not work in cases where the config was not totally flat as the unflatten function does not unflatten recursively.