Added support converting unmarshalled configs to interface{}
#460
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #460 +/- ##
=====================================
Coverage 82.8% 82.9%
=====================================
Files 62 62
Lines 2651 2664 +13
=====================================
+ Hits 2197 2210 +13
Misses 371 371
Partials 83 83
|
BoskyWSMFN
changed the title
interface{}interface{}
internal/convert/converter.go
Outdated
| @@ -91,6 +91,10 @@ func (c Converter) convert(name string, from any, toVal reflect.Value) error { / | |||
| return c.convertString(name, fromVal, toVal) | |||
| case toVal.Kind() == reflect.Struct: | |||
| return c.convertStruct(name, fromVal, toVal) | |||
| case toVal.Kind() == reflect.Interface: // Right after all other checks. | |||
| toVal.Set(fromVal) | |||
There was a problem hiding this comment.
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.
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.
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.
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. =)
| }, | ||
| { | ||
| description: "map to interface", |
There was a problem hiding this comment.
Added few more tests to check if maps.Unpack call is required. Seems to be excessive.
There was a problem hiding this comment.
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.
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.
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
Seems to be tested indirectly.
| @@ -554,6 +552,55 @@ func (c Converter) convertStruct(name string, fromVal, toVal reflect.Value) erro | |||
| } | |||
| } | |||
|
|
|||
| func (c Converter) convertInterface(name string, fromVal, toVal reflect.Value) error { | |||
| switch fromVal.Kind() { | |||
| case reflect.Map: | |||
There was a problem hiding this comment.
is it possible just creating a Map[string]any and then return the result from convert with new created map?
There was a problem hiding this comment.
Oh... Right, it is. Haste makes waste.
| }), | ||
| }, | ||
| { | ||
| description: "packed KV and field to interface{}", |
There was a problem hiding this comment.
This test case shows that special handling is necessary. Now, all complex data types will be treated as map[string]interface{}.
|
@BoskyWSMFN may I merge it now? |
Sure. |
Fixed problem described in #459.
Nothing is broken so far.