knadh / koanf

Simple, extremely lightweight, extensible, configuration management library for Go. Support for JSON, TOML, YAML, env, command line, file, S3 etc. Alternative to viper.
MIT License
2.56k stars 146 forks source link

Bug in merge behavior due to bug in `fatih/structs` #299

Open gheinrich-dd opened 1 month ago

gheinrich-dd commented 1 month ago

Describe the bug Hi, recently started using your package and enjoy using it. I did unfortunately come across what seems to be a bug in fatih/structs, which ultimately ends up breaking some merge behavior for structs.Provider(). The tl;dr of the bug is that if you have a struct where all fields are default values, fatih/structs will think there was a problem representing the struct as a raw map and will leave the value in tact as the original struct value rather than turning it into map[string]any. The code that causes this can be found here.

To Reproduce

package example

import (
    "github.com/knadh/koanf"
    "github.com/knadh/koanf/providers/structs"
    "github.com/stretchr/testify/assert"
    "testing"
)

type Address struct {
    Street string `json:"street,omitempty"`
}

type Person struct {
    FirstName string   `json:"firstName,omitempty"`
    LastName  string   `json:"lastName,omitempty"`
    Address   *Address `json:"address,omitempty"`
}

func TestStructsProvider(t *testing.T) {
    expected := Person{
        FirstName: "Jane",
        LastName:  "Doe",
        Address: &Address{
            Street: "123 Street",
        },
    }

    override := Person{
        Address: &Address{
            Street: "",
        },
    }

    conf := koanf.New("::")
    err := conf.Load(structs.Provider(expected, "json"), nil)
    assert.NoError(t, err)

    err = conf.Load(structs.Provider(override, "json"), nil)
    assert.NoError(t, err)

    var actual Person
    err = conf.Unmarshal("", &actual)
    assert.NoError(t, err)

    // fails
    assert.Equal(t, expected, actual)
}

Expected behavior In the code above, the expected behavior is that the overrides don't change anything. The street value in the Address struct is empty, and so it should get dropped out because of omitempty, effectively meaning you're merging { "address": { } } into the existing value. Instead, fatih/structs leaves the Address struct value intact rather than converting it to map[string]any and dropping out the street field, causing the street value to get trampled by the empty value.

Please provide the following information):

Additional context In my company's codebase, I got around this issue by implementing a custom structs provider that does a json marshal and unmarshal into map[string]any.

knadh commented 2 weeks ago

Hi @gheinrich-dd. Apologies for the delayed reply. fatih/structs is frozen, so there is no possibility of getting a fix in. Perhaps a solution here is to use JSON marshalling to get a map[string]interface{} and to merge that into koanf directly without using the structs package. In fact, the structs provider could do this internally and get rid of fatih/structs.