Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: control merging behavior #66

Merged
merged 3 commits into from
Apr 10, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
.env
.env

# IDE
.idea
78 changes: 77 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 |
Expand Down
76 changes: 55 additions & 21 deletions koanf.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -55,10 +69,23 @@ type UnmarshalConf struct {
// or a / for `parent/child/key`.
func New(delim string) *Koanf {
return &Koanf{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can just return this:

return NewWithConf(Conf{Delim: delim})

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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand All @@ -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,
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -327,7 +352,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)
}
Expand Down Expand Up @@ -365,13 +390,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 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err check can be in-lined.

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,
Expand Down
27 changes: 27 additions & 0 deletions koanf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ 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"
Expand Down Expand Up @@ -712,6 +713,32 @@ func TestRaw_JsonTypes(t *testing.T) {
}
}

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 TestMergeAt(t *testing.T) {
var (
assert = assert.New(t)
Expand Down
58 changes: 58 additions & 0 deletions maps/maps.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,22 @@ package maps
import (
"fmt"
"github.com/mitchellh/copystructure"
"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.
//
Expand Down Expand Up @@ -132,6 +145,51 @@ func Merge(a, b map[string]interface{}) {
}
}

func MergeStrict(a, b map[string]interface{}) error {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing comment.

mergeError := MergeStrictError{Errors: []error{}}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if skipping failures and collecting all errors is beneficial. We can just keep this idiomatic and return on the first error. That'll also combine MergeStrict() and mergeStrict().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming I have 10 errors, that would require 10 runs to figure out all the errors.
That way he can fix all the possible errors.
WDYT?

Copy link
Owner

@knadh knadh Mar 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could be said of any function that does multiple things, but collecting errors like that is an uncommon pattern. For instance, a JSON payload passed to json.Unmarshal() could have an indefinite number of errors, but the execution halts at the first error which is then immediately returned. We should keep this simple and return at the first error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

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 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep mergeStrict() exactly like merge() except for the additional type check that'll be added here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still need the fullKey to return the error message with the path to the key, otherwise it would be misleading.

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.
Expand Down
Loading