From 968145ca2d76416afb0cb36dd7ad80c0f9b699ea Mon Sep 17 00:00:00 2001 From: grount Date: Tue, 23 Feb 2021 09:45:26 +0200 Subject: [PATCH 1/2] feature: control merging behavior issue #62 --- .gitignore | 5 ++- README.md | 78 ++++++++++++++++++++++++++++++++++++++++++++++- koanf.go | 76 ++++++++++++++++++++++++++++++++------------- koanf_test.go | 69 +++++++++++++++++++++++++++++++++++++++++ maps/maps.go | 58 +++++++++++++++++++++++++++++++++++ maps/maps_test.go | 35 +++++++++++++++++++++ 6 files changed, 298 insertions(+), 23 deletions(-) diff --git a/.gitignore b/.gitignore index 2eea525d..3777c0be 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,4 @@ -.env \ No newline at end of file +.env + +# IDE +.idea diff --git a/README.md b/README.md index 5cbf8364..3eed6a38 100644 --- a/README.md +++ b/README.md @@ -518,9 +518,72 @@ func main() { fmt.Printf("name is = `%s`\n", k.String("parent1.child1.name")) } ``` +### Merge behavior +#### Default behavior +The default behavior when you create Koanf this way is: `koanf.New(delim)` that the latest loaded configuration will +merge with the previous one. + +For example: +`first.yml` +```yaml +key: [1,2,3] +``` +`second.yml` +```yaml +key: 'string' +``` +When `second.yml` is loaded it will override the type of the `first.yml`. + +If this behavior is not desired, you can merge 'strictly'. In the same scenario, `Load` will return an error. + +```go +package main + +import ( + "errors" + "log" + + "github.com/knadh/koanf" + "github.com/knadh/koanf/maps" + "github.com/knadh/koanf/parsers/json" + "github.com/knadh/koanf/parsers/yaml" + "github.com/knadh/koanf/providers/file" +) +var conf = koanf.Conf{ + Delim: ".", + StrictMerge: true, +} +var k = koanf.NewWithConf(conf) + +func main() { + yamlPath := "mock/mock.yml" + if err := k.Load(file.Provider(yamlPath), yaml.Parser()); err != nil { + log.Fatalf("error loading config: %v", err) + } -### Order of merge and key case senstivity + jsonPath := "mock/mock.json" + if err := k.Load(file.Provider(jsonPath), json.Parser()); err != nil { + // go version <= v1.12 + if strictErr := err.(*maps.MergeStrictError); strictErr != nil { + log.Fatalf("error merging config %v: %v", jsonPath, err) + } + // go version > v1.12 + var strictErr *maps.MergeStrictError + if errors.As(err, &strictErr) { + log.Fatalf("error merging config %v: %v", jsonPath, err) + } + + log.Fatalf("error loading config: %v", err) + } +} +``` +**Note:** When merging different extensions, each parser can treat his types differently, + meaning even though you the load same types there is a probability that it will fail with `StrictMerge: true`. + +For example: merging JSON and YAML will most likely fail because JSON treats integers as float64 and YAML treats them as integers. + +### Order of merge and key case sensitivity - Config keys are case-sensitive in koanf. For example, `app.server.port` and `APP.SERVER.port` are not the same. - koanf does not impose any ordering on loading config from various providers. Every successive `Load()` or `Load()` merges new config into existing config. That means it is possible to load environment variables first, then files on top of it, and then command line variables on top of it, or any such order. @@ -533,6 +596,19 @@ Writing Providers and Parsers are easy. See the bundled implementations in the ` ## API +### Instantiation methods +| Method | Description | +| ------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `New(delim string) *Koanf` | Returns a new instance of Koanf. delim is the delimiter to use when specifying config key paths, for instance a `.` for `parent.child.key` or a `/` for `parent/child/key`. | +| `NewWithConf(conf Conf) *Koanf` | Returns a new instance of Koanf depending on the `Koanf.Conf` configurations. | + +### Structs +| Struct | Description | +| ------------------------------- | ---------------------------------------------------------------------------------------------------------------- | +| Koanf | Koanf is the configuration apparatus. | +| Conf | Conf is the Koanf configuration, that includes `Delim` and `StrictMerge`. | +| UnmarshalConf | UnmarshalConf represents configuration options used by `Unmarshal()` to unmarshal conf maps in arbitrary structs | + ### Bundled providers | Package | Provider | Description | diff --git a/koanf.go b/koanf.go index 75448cfb..e5e1832c 100644 --- a/koanf.go +++ b/koanf.go @@ -18,7 +18,21 @@ type Koanf struct { confMap map[string]interface{} confMapFlat map[string]interface{} keyMap KeyMap - delim string + conf Conf +} + +// Conf is the Koanf configuration. +type Conf struct { + // Delim is the delimiter to use + // when specifying config key paths, for instance a . for `parent.child.key` + // or a / for `parent/child/key`. + Delim string + + // StrictMerge makes the merging behavior strict. + // Meaning when loading two files that have the same key, + // the first loaded file will define the desired type, and if the second file loads + // a different type will cause an error. + StrictMerge bool } // KeyMap represents a map of flattened delimited keys and the non-delimited @@ -55,10 +69,23 @@ type UnmarshalConf struct { // or a / for `parent/child/key`. func New(delim string) *Koanf { return &Koanf{ - delim: delim, confMap: make(map[string]interface{}), confMapFlat: make(map[string]interface{}), keyMap: make(KeyMap), + conf: Conf{ + Delim: delim, + StrictMerge: false, + }, + } +} + +// NewWithConf returns a new instance of Koanf based on the Conf. +func NewWithConf(conf Conf) *Koanf { + return &Koanf{ + confMap: make(map[string]interface{}), + confMapFlat: make(map[string]interface{}), + keyMap: make(KeyMap), + conf: conf, } } @@ -94,8 +121,7 @@ func (ko *Koanf) Load(p Provider, pa Parser) error { } } - ko.merge(mp) - return nil + return ko.merge(mp) } // Keys returns the slice of all flattened keys in the loaded configuration @@ -164,8 +190,8 @@ func (ko *Koanf) Cut(path string) *Koanf { out = v } - n := New(ko.delim) - n.merge(out) + n := New(ko.conf.Delim) + _ = n.merge(out) return n } @@ -176,27 +202,26 @@ func (ko *Koanf) Copy() *Koanf { // Merge merges the config map of a given Koanf instance into // the current instance. -func (ko *Koanf) Merge(in *Koanf) { - ko.merge(in.Raw()) +func (ko *Koanf) Merge(in *Koanf) error { + return ko.merge(in.Raw()) } // MergeAt merges the config map of a given Koanf instance into // the current instance as a sub map, at the given key path. // If all or part of the key path is missing, it will be created. // If the key path is `""`, this is equivalent to Merge. -func (ko *Koanf) MergeAt(in *Koanf, path string) { +func (ko *Koanf) MergeAt(in *Koanf, path string) error { // No path. Merge the two config maps. if path == "" { - ko.Merge(in) - return + return ko.Merge(in) } // Unflatten the config map with the given key path. n := maps.Unflatten(map[string]interface{}{ path: in.Raw(), - }, ko.delim) + }, ko.conf.Delim) - ko.merge(n) + return ko.merge(n) } // Marshal takes a Parser implementation and marshals the config map into bytes, @@ -242,7 +267,7 @@ func (ko *Koanf) UnmarshalWithConf(path string, o interface{}, c UnmarshalConf) mp := ko.Get(path) if c.FlatPaths { if f, ok := mp.(map[string]interface{}); ok { - fmp, _ := maps.Flatten(f, nil, ko.delim) + fmp, _ := maps.Flatten(f, nil, ko.conf.Delim) mp = fmp } } @@ -270,8 +295,8 @@ func (ko *Koanf) Delete(path string) { maps.Delete(ko.confMap, p) // Update the flattened version as well. - ko.confMapFlat, ko.keyMap = maps.Flatten(ko.confMap, nil, ko.delim) - ko.keyMap = populateKeyParts(ko.keyMap, ko.delim) + ko.confMapFlat, ko.keyMap = maps.Flatten(ko.confMap, nil, ko.conf.Delim) + ko.keyMap = populateKeyParts(ko.keyMap, ko.conf.Delim) } // Get returns the raw, uncast interface{} value of a given key path @@ -328,7 +353,7 @@ func (ko *Koanf) Slices(path string) []*Koanf { continue } - k := New(ko.delim) + k := New(ko.conf.Delim) k.Load(confmap.Provider(v, ""), nil) out = append(out, k) } @@ -366,13 +391,22 @@ func (ko *Koanf) MapKeys(path string) []string { return out } -func (ko *Koanf) merge(c map[string]interface{}) { +func (ko *Koanf) merge(c map[string]interface{}) error{ maps.IntfaceKeysToStrings(c) - maps.Merge(c, ko.confMap) + if ko.conf.StrictMerge { + err := maps.MergeStrict(c, ko.confMap) + if err != nil { + return err + } + } else { + maps.Merge(c, ko.confMap) + } // Maintain a flattened version as well. - ko.confMapFlat, ko.keyMap = maps.Flatten(ko.confMap, nil, ko.delim) - ko.keyMap = populateKeyParts(ko.keyMap, ko.delim) + ko.confMapFlat, ko.keyMap = maps.Flatten(ko.confMap, nil, ko.conf.Delim) + ko.keyMap = populateKeyParts(ko.keyMap, ko.conf.Delim) + + return nil } // toInt64 takes an interface value and if it is an integer type, diff --git a/koanf_test.go b/koanf_test.go index 73257bb4..6f5670f2 100644 --- a/koanf_test.go +++ b/koanf_test.go @@ -4,6 +4,7 @@ import ( encjson "encoding/json" "flag" "fmt" + "github.com/knadh/koanf/maps" "io/ioutil" "log" "os" @@ -614,6 +615,74 @@ func TestMerge(t *testing.T) { assert.Equal(cut1.All(), k2.All(), "conf map mismatch") } +func TestMergeStrictError(t *testing.T) { + var ( + assert = assert.New(t) + ) + + ks := koanf.NewWithConf(koanf.Conf{ + Delim: delim, + StrictMerge: true, + }) + + assert.Nil(ks.Load(confmap.Provider(map[string]interface{}{ + "parent2": map[string]interface{}{ + "child2": map[string]interface{}{ + "grandchild2": map[string]interface{}{ + "ids": 123, + }, + }, + }, + }, delim), nil)) + + err := ks.Load(file.Provider(mockYAML), yaml.Parser()) + assert.Error(err) + assert.IsType(&maps.MergeStrictError{}, err) + assert.True(strings.HasPrefix(err.Error(), "incorrect types at key parent2.child2.grandchild2")) +} + +func TestMergeStrict(t *testing.T) { + var ( + assert = assert.New(t) + ) + + ks := koanf.NewWithConf(koanf.Conf{ + Delim: delim, + StrictMerge: true, + }) + + assert.Nil(ks.Load(confmap.Provider(map[string]interface{}{ + "parent2": map[string]interface{}{ + "child2": map[string]interface{}{ + "grandchild2": map[string]interface{}{ + "ids": []int{5,6,7}, + }, + }, + }, + }, delim), nil)) + + err := ks.Load(file.Provider(mockYAML), yaml.Parser()) + assert.NoError(err) +} + + +func TestMergeStrictJsonYamlError(t *testing.T) { + var ( + assert = assert.New(t) + ) + + ks := koanf.NewWithConf(koanf.Conf{ + Delim: delim, + StrictMerge: true, + }) + + + assert.NoError(ks.Load(file.Provider(mockJSON), json.Parser())) + err := ks.Load(file.Provider(mockYAML), yaml.Parser()) + assert.Error(err) + assert.IsType(&maps.MergeStrictError{}, err) +} + func TestMergeAt(t *testing.T) { var ( assert = assert.New(t) diff --git a/maps/maps.go b/maps/maps.go index 44b68fc3..8b2a28c2 100644 --- a/maps/maps.go +++ b/maps/maps.go @@ -6,9 +6,22 @@ package maps import ( "encoding/json" "fmt" + "reflect" "strings" ) +type MergeStrictError struct { + Errors []error +} + +func (m *MergeStrictError) Error() string { + var msg string + for _, err := range m.Errors { + msg += fmt.Sprintf("%v\n", err.Error()) + } + return msg +} + // Flatten takes a map[string]interface{} and traverses it and flattens // nested children into keys delimited by delim. // @@ -132,6 +145,51 @@ func Merge(a, b map[string]interface{}) { } } +func MergeStrict(a, b map[string]interface{}) error { + mergeError := MergeStrictError{Errors: []error{}} + mergeStrict(a, b, &mergeError, "") + if len(mergeError.Errors) > 0 { + return &mergeError + } + return nil +} + +func mergeStrict(a, b map[string]interface{}, mergeError *MergeStrictError, fullKey string) { + for key, val := range a { + // Does the key exist in the target map? + // If no, add it and move on. + bVal, ok := b[key] + if !ok { + b[key] = val + continue + } + + newFullKey := key + if fullKey != "" { + newFullKey = fmt.Sprintf("%v.%v", fullKey, key) + } + + // If the incoming val is not a map, do a direct merge between the same types. + if _, ok := val.(map[string]interface{}); !ok { + if reflect.TypeOf(b[key]) == reflect.TypeOf(val) { + b[key] = val + } else { + err := fmt.Errorf("incorrect types at key %v, type %T != %T", fullKey, b[key], val) + mergeError.Errors = append(mergeError.Errors, err) + } + continue + } + + // The source key and target keys are both maps. Merge them. + switch v := bVal.(type) { + case map[string]interface{}: + mergeStrict(val.(map[string]interface{}), v, mergeError, newFullKey) + default: + b[key] = val + } + } +} + // Delete removes the entry present at a given path, from the map. The path // is the key map slice, for eg:, parent.child.key -> [parent child key]. // Any empty, nested map on the path, is recursively deleted. diff --git a/maps/maps_test.go b/maps/maps_test.go index 9dcb7807..cbf903ab 100644 --- a/maps/maps_test.go +++ b/maps/maps_test.go @@ -104,6 +104,7 @@ func TestMerge(t *testing.T) { }, "top": 789, "empty": map[string]interface{}{}, + "key": 1, } m2 := map[string]interface{}{ "parent": map[string]interface{}{ @@ -117,6 +118,7 @@ func TestMerge(t *testing.T) { }, "newtop": 999, "empty": []int{1, 2, 3}, + "key": "string", } Merge(m2, m1) @@ -136,10 +138,43 @@ func TestMerge(t *testing.T) { "top": 789, "newtop": 999, "empty": []int{1, 2, 3}, + "key": "string", } assert.Equal(t, out, m1) } +func TestMergeStrict(t *testing.T) { + m1 := map[string]interface{}{ + "parent": map[string]interface{}{ + "child": map[string]interface{}{ + "key": "123", + }, + "child2": map[string]interface{}{ + "key": 123, + }, + }, + "top": 789, + "empty": []int{}, + "key": 1, + } + m2 := map[string]interface{}{ + "parent": map[string]interface{}{ + "child": map[string]interface{}{ + "key": 456, + "val": 789, + }, + }, + "child": map[string]interface{}{ + "key": 456, + }, + "newtop": 999, + "empty": []int{1, 2, 3}, + "key": "string", + } + err := MergeStrict(m2, m1) + assert.Error(t, err) +} + func TestDelete(t *testing.T) { testMap := map[string]interface{}{ "parent": map[string]interface{}{ From a05e2d502ca7d3069ffc14b454956853876e87ab Mon Sep 17 00:00:00 2001 From: Daniel Gront Date: Fri, 26 Mar 2021 17:38:57 +0300 Subject: [PATCH 2/2] MergeStrict returns the first error Signed-off-by: Daniel Gront --- README.md | 10 ---------- koanf.go | 16 +++++----------- koanf_test.go | 2 -- maps/maps.go | 34 +++++++++++++--------------------- 4 files changed, 18 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 3eed6a38..cf2ef797 100644 --- a/README.md +++ b/README.md @@ -564,16 +564,6 @@ func main() { jsonPath := "mock/mock.json" if err := k.Load(file.Provider(jsonPath), json.Parser()); err != nil { - // go version <= v1.12 - if strictErr := err.(*maps.MergeStrictError); strictErr != nil { - log.Fatalf("error merging config %v: %v", jsonPath, err) - } - // go version > v1.12 - var strictErr *maps.MergeStrictError - if errors.As(err, &strictErr) { - log.Fatalf("error merging config %v: %v", jsonPath, err) - } - log.Fatalf("error loading config: %v", err) } } diff --git a/koanf.go b/koanf.go index f75706ef..6c7c192c 100644 --- a/koanf.go +++ b/koanf.go @@ -68,15 +68,10 @@ type UnmarshalConf struct { // when specifying config key paths, for instance a . for `parent.child.key` // or a / for `parent/child/key`. func New(delim string) *Koanf { - return &Koanf{ - confMap: make(map[string]interface{}), - confMapFlat: make(map[string]interface{}), - keyMap: make(KeyMap), - conf: Conf{ - Delim: delim, - StrictMerge: false, - }, - } + return NewWithConf(Conf{ + Delim: delim, + StrictMerge: false, + }) } // NewWithConf returns a new instance of Koanf based on the Conf. @@ -393,8 +388,7 @@ func (ko *Koanf) MapKeys(path string) []string { func (ko *Koanf) merge(c map[string]interface{}) error{ maps.IntfaceKeysToStrings(c) if ko.conf.StrictMerge { - err := maps.MergeStrict(c, ko.confMap) - if err != nil { + if err := maps.MergeStrict(c, ko.confMap); err != nil { return err } } else { diff --git a/koanf_test.go b/koanf_test.go index 322f3644..ada64261 100644 --- a/koanf_test.go +++ b/koanf_test.go @@ -14,7 +14,6 @@ import ( "time" "github.com/knadh/koanf" - "github.com/knadh/koanf/maps" "github.com/knadh/koanf/parsers/dotenv" "github.com/knadh/koanf/parsers/hcl" "github.com/knadh/koanf/parsers/json" @@ -735,7 +734,6 @@ func TestMergeStrictError(t *testing.T) { err := ks.Load(file.Provider(mockYAML), yaml.Parser()) assert.Error(err) - assert.IsType(&maps.MergeStrictError{}, err) assert.True(strings.HasPrefix(err.Error(), "incorrect types at key parent2.child2.grandchild2")) } diff --git a/maps/maps.go b/maps/maps.go index c460eda7..219803ef 100644 --- a/maps/maps.go +++ b/maps/maps.go @@ -10,17 +10,6 @@ import ( "strings" ) -type MergeStrictError struct { - Errors []error -} - -func (m *MergeStrictError) Error() string { - var msg string - for _, err := range m.Errors { - msg += fmt.Sprintf("%v\n", err.Error()) - } - return msg -} // Flatten takes a map[string]interface{} and traverses it and flattens // nested children into keys delimited by delim. @@ -145,16 +134,19 @@ func Merge(a, b map[string]interface{}) { } } +// MergeStrict recursively merges map a into b (left to right), mutating +// and expanding map b. Note that there's no copying involved, so +// map b will retain references to map a. +// If an equal key in either of the maps has a different value type, it will return the first error. +// +// It's important to note that all nested maps should be +// map[string]interface{} and not map[interface{}]interface{}. +// Use IntfaceKeysToStrings() to convert if necessary. func MergeStrict(a, b map[string]interface{}) error { - mergeError := MergeStrictError{Errors: []error{}} - mergeStrict(a, b, &mergeError, "") - if len(mergeError.Errors) > 0 { - return &mergeError - } - return nil + return mergeStrict(a, b, "") } -func mergeStrict(a, b map[string]interface{}, mergeError *MergeStrictError, fullKey string) { +func mergeStrict(a, b map[string]interface{}, fullKey string) error { for key, val := range a { // Does the key exist in the target map? // If no, add it and move on. @@ -174,8 +166,7 @@ func mergeStrict(a, b map[string]interface{}, mergeError *MergeStrictError, full if reflect.TypeOf(b[key]) == reflect.TypeOf(val) { b[key] = val } else { - err := fmt.Errorf("incorrect types at key %v, type %T != %T", fullKey, b[key], val) - mergeError.Errors = append(mergeError.Errors, err) + return fmt.Errorf("incorrect types at key %v, type %T != %T", fullKey, b[key], val) } continue } @@ -183,11 +174,12 @@ func mergeStrict(a, b map[string]interface{}, mergeError *MergeStrictError, full // The source key and target keys are both maps. Merge them. switch v := bVal.(type) { case map[string]interface{}: - mergeStrict(val.(map[string]interface{}), v, mergeError, newFullKey) + return mergeStrict(val.(map[string]interface{}), v, newFullKey) default: b[key] = val } } + return nil } // Delete removes the entry present at a given path, from the map. The path