Skip to content

Commit

Permalink
Fully override default slice fields of config (open-telemetry#4001)
Browse files Browse the repository at this point in the history
mapstructure library doesn't override full slice during unmarshalling.
Origin issue: mitchellh/mapstructure#74 (comment)
To address this we empty every slice before unmarshalling unless user provided slice is nil.
  • Loading branch information
mcdoker18 committed Oct 17, 2022
1 parent b45d0ed commit cfebf0b
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .chloggen/config-slice-fields.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
change_type: "bug_fix"
component: "config"
note: "Fully override default slice fields of config"
issues: [4001]
subtext:
18 changes: 18 additions & 0 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ func decodeConfig(m *Conf, result interface{}, errorUnused bool) error {
mapstructure.StringToTimeDurationHookFunc(),
mapstructure.TextUnmarshallerHookFunc(),
unmarshalerHookFunc(result),
zeroesSliceHookFunc(),
),
}
decoder, err := mapstructure.NewDecoder(dc)
Expand Down Expand Up @@ -320,6 +321,23 @@ func marshalerHookFunc(orig interface{}) mapstructure.DecodeHookFuncValue {
}
}

// mapstructure library doesn't override full slice during unmarshalling.
// Origin issue: https://github.com/mitchellh/mapstructure/issues/74#issuecomment-279886492
// To address this we empty every slice before unmarshalling unless user provided slice is nil.
func zeroesSliceHookFunc() mapstructure.DecodeHookFuncValue {
return func(from reflect.Value, to reflect.Value) (interface{}, error) {
if from.Kind() == reflect.Slice && from.IsNil() {
return from.Interface(), nil
}

if to.CanSet() && to.Kind() == reflect.Slice {
to.Set(reflect.MakeSlice(reflect.SliceOf(to.Type().Elem()), 0, 0))
}

return from.Interface(), nil
}
}

// Unmarshaler interface may be implemented by types to customize their behavior when being unmarshaled from a Conf.
type Unmarshaler interface {
// Unmarshal a Conf into the struct in a custom way.
Expand Down
131 changes: 131 additions & 0 deletions confmap/confmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,3 +400,134 @@ func TestUnmarshalerErr(t *testing.T) {
assert.EqualError(t, cfgMap.Unmarshal(tc), expectErr)
assert.Empty(t, tc.Err.Foo)
}

func TestUnmarshalerSlices(t *testing.T) {
type innerStructWithSlices struct {
Ints []int `mapstructure:"ints"`
}
type structWithSlices struct {
Strings []string `mapstructure:"strings"`
Nested innerStructWithSlices `mapstructure:"nested"`
}

tests := []struct {
name string
cfg map[string]any
provided any
expected any
}{
{
name: "overridden by slice",
cfg: map[string]any{
"strings": []string{"111"},
},
provided: &structWithSlices{
Strings: []string{"xxx", "yyyy", "zzzz"},
},
expected: &structWithSlices{
Strings: []string{"111"},
},
},
{
name: "overridden by a bigger slice",
cfg: map[string]any{
"strings": []string{"111", "222", "333"},
},
provided: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
expected: &structWithSlices{
Strings: []string{"111", "222", "333"},
},
},
{
name: "overridden by an empty slice",
cfg: map[string]any{
"strings": []string{},
},
provided: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
expected: &structWithSlices{
Strings: []string{},
},
},
{
name: "not overridden by nil slice",
cfg: map[string]any{
"strings": []string(nil),
},
provided: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
expected: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
},
{
name: "not overridden by nil",
cfg: map[string]any{
"strings": nil,
},
provided: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
expected: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
},
{
name: "not overridden by missing value",
cfg: map[string]any{},
provided: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
expected: &structWithSlices{
Strings: []string{"xxx", "yyyy"},
},
},
{
name: "overridden by nested slice",
cfg: map[string]any{
"nested": map[string]any{
"ints": []int{777},
},
},
provided: &structWithSlices{
Nested: innerStructWithSlices{
Ints: []int{1, 2, 3},
},
},
expected: &structWithSlices{
Nested: innerStructWithSlices{
Ints: []int{777},
},
},
},
{
name: "overridden by weakly typed input",
cfg: map[string]any{
"strings": "111",
},
provided: &structWithSlices{
Strings: []string{"xxx", "yyyy", "zzzz"},
},
expected: &structWithSlices{
Strings: []string{"111"},
},
},
}

for _, tt := range tests {
tt := tt

t.Run(tt.name, func(t *testing.T) {
cfg := NewFromStringMap(tt.cfg)

err := cfg.Unmarshal(tt.provided)
if assert.NoError(t, err) {
assert.Equal(t, tt.expected, tt.provided)
}
})
}
}

0 comments on commit cfebf0b

Please sign in to comment.