-
Notifications
You must be signed in to change notification settings - Fork 9
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
Added support converting unmarshalled configs to interface{}
#460
Changes from 4 commits
c054635
a35fc0f
0c12668
46effb6
455d289
c07952d
f0f9da3
aafa9bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -15,6 +15,7 @@ import ( | |||||
|
||||||
"github.com/nil-go/konf/internal/assert" | ||||||
"github.com/nil-go/konf/internal/convert" | ||||||
"github.com/nil-go/konf/internal/maps" | ||||||
) | ||||||
|
||||||
func TestConverter(t *testing.T) { //nolint:maintidx | ||||||
|
@@ -835,34 +836,39 @@ func TestConverter(t *testing.T) { //nolint:maintidx | |||||
}), | ||||||
}, | ||||||
from: map[string]any{ | ||||||
"Enum": "sky", | ||||||
"OuterField": "outer", | ||||||
"PrivateField": "private", | ||||||
"InnerField": "squash", | ||||||
"Inner": map[string]any{"InnerField": "inner"}, | ||||||
"Enum": "sky", | ||||||
"OuterField": "outer", | ||||||
"PrivateField": "private", | ||||||
"InterfaceField": "interface{}", | ||||||
"InnerField": "squash", | ||||||
"Inner": map[string]any{"InnerField": "inner"}, | ||||||
}, | ||||||
to: pointer(OuterStruct{}), | ||||||
expected: pointer(OuterStruct{ | ||||||
Enum: Sky, | ||||||
OuterField: "outer", | ||||||
InnerStruct: InnerStruct{InnerField: "squash"}, | ||||||
Inner: &InnerStruct{InnerField: "inner"}, | ||||||
Enum: Sky, | ||||||
OuterField: "outer", | ||||||
InterfaceField: "interface{}", | ||||||
InnerStruct: InnerStruct{InnerField: "squash"}, | ||||||
Inner: &InnerStruct{InnerField: "inner"}, | ||||||
}), | ||||||
}, | ||||||
{ | ||||||
description: "map to struct (with keyMap)", | ||||||
opts: []convert.Option{ | ||||||
convert.WithKeyMapper(strings.ToLower), | ||||||
}, | ||||||
from: map[string]string{"innerfield": "inner"}, | ||||||
from: map[string]string{"innerfield": "inner", "interfacefield": "interface{}"}, | ||||||
to: pointer(struct { | ||||||
InnerField string | ||||||
InnerField string | ||||||
InterfaceField interface{} | ||||||
}{}), | ||||||
expected: pointer( | ||||||
struct { | ||||||
InnerField string | ||||||
InnerField string | ||||||
InterfaceField interface{} | ||||||
}{ | ||||||
InnerField: "inner", | ||||||
InnerField: "inner", | ||||||
InterfaceField: "interface{}", | ||||||
}), | ||||||
}, | ||||||
{ | ||||||
|
@@ -896,13 +902,85 @@ func TestConverter(t *testing.T) { //nolint:maintidx | |||||
to: pointer(OuterStruct{}), | ||||||
err: "'' expected a map, got 'string'", | ||||||
}, | ||||||
// unsupported. | ||||||
{ | ||||||
description: "to interface (unsupported)", | ||||||
description: "int to interface", | ||||||
from: 42, | ||||||
to: pointer(any(nil)), | ||||||
expected: pointer(any(42)), | ||||||
}, | ||||||
{ | ||||||
description: "string to interface", | ||||||
from: "str", | ||||||
to: pointer(any(nil)), | ||||||
err: ": unsupported type: interface", | ||||||
expected: pointer(any("str")), | ||||||
}, | ||||||
{ | ||||||
description: "float to interface", | ||||||
from: 42.42, | ||||||
to: pointer(any(nil)), | ||||||
expected: pointer(any(42.42)), | ||||||
}, | ||||||
{ | ||||||
description: "map to interface", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added few more tests to check if maps.Unpack call is required. Seems to be excessive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I may miss test for Unpack in convert package. Here is the code calls maps.Unpack konf/internal/convert/converter.go Line 355 in 857c68e
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just tested it from most outer https://github.com/nil-go/konf/blob/main/config_test.go#L102 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Seems to be tested indirectly. |
||||||
from: map[string]int{"key": 42, "keySensitive": 43}, | ||||||
to: pointer(any(nil)), | ||||||
expected: pointer(any(map[string]int{"key": 42, "keySensitive": 43})), | ||||||
}, | ||||||
{ | ||||||
description: "map to interface (with keyMap)", // Probably redundant. | ||||||
opts: []convert.Option{ | ||||||
convert.WithKeyMapper(strings.ToLower), | ||||||
}, | ||||||
from: map[string]int{"key": 42, "keysensitive": 43}, | ||||||
to: pointer(any(nil)), | ||||||
expected: pointer(any(map[string]int{"key": 42, "keysensitive": 43})), | ||||||
}, | ||||||
{ | ||||||
description: "packed KV and field to map[string]interface{}", | ||||||
from: map[string]interface{}{ | ||||||
"key1": maps.KeyValue{ | ||||||
Key: "key1", | ||||||
Value: "value1", | ||||||
}, | ||||||
"key2": "value2", | ||||||
}, | ||||||
to: pointer(map[string]interface{}{}), | ||||||
expected: pointer(map[string]interface{}{ | ||||||
"key1": "value1", | ||||||
"key2": "value2", | ||||||
}), | ||||||
}, | ||||||
{ | ||||||
description: "packed KV and field to struct (with keyMap)", | ||||||
opts: []convert.Option{ | ||||||
convert.WithKeyMapper(strings.ToLower), | ||||||
}, | ||||||
from: map[string]interface{}{ | ||||||
"key1": maps.KeyValue{ | ||||||
Key: "key1", | ||||||
Value: "value1", | ||||||
}, | ||||||
"key2": "value2", | ||||||
}, | ||||||
to: pointer(struct { | ||||||
Key1 interface{} | ||||||
Key2 interface{} | ||||||
}{}), | ||||||
expected: pointer(struct { | ||||||
Key1 interface{} | ||||||
Key2 interface{} | ||||||
}{ | ||||||
Key1: "value1", | ||||||
Key2: "value2", | ||||||
}), | ||||||
}, | ||||||
{ | ||||||
description: "slice to interface", | ||||||
from: []int{1, 2, 3}, | ||||||
to: pointer(any(nil)), | ||||||
expected: pointer(any([]int{1, 2, 3})), | ||||||
}, | ||||||
// unsupported. | ||||||
{ | ||||||
description: "to func (unsupported)", | ||||||
from: "str", | ||||||
|
@@ -966,9 +1044,10 @@ func (e *Enum) UnmarshalText(text []byte) error { | |||||
|
||||||
type ( | ||||||
OuterStruct struct { | ||||||
Enum Enum | ||||||
OuterField string | ||||||
privateField string //nolint:unused | ||||||
Enum Enum | ||||||
OuterField string | ||||||
privateField string //nolint:unused | ||||||
InterfaceField interface{} | ||||||
|
||||||
InnerStruct `konf:",squash"` | ||||||
Inner *InnerStruct | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs a special handling if fromVal is map since the keys in map may be wrapped for case sensitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't get your point. Special handling is required for maps, asaik, but it is the caller's responsibility to deal with keys and values if the destination contains fields of type interface{}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for confusion. To support case sensitive map key, the key in internal value has been wrapped with a special struct (https://github.com/nil-go/konf/blob/main/config.go#L166). So when unmarshaling back to map, it needs to unwrap the key back to string (case sensitive or not depends on configuration).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, there's no confusion. I just had doubts about whether special handling was really necessary here. I found a test case that clearly shows it is. =)