Skip to content

Commit

Permalink
Fully overrides 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 zeroes every slice before unmarshalling unless user provided slice is nil.
  • Loading branch information
mcdoker18 committed Sep 30, 2022
1 parent e9311f6 commit ca970c1
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
- Expose `AsRaw` and `FromRaw` `pcommon.Value` methods (#6090)
- Convert `ValueTypeBytes` attributes in logging exporter (#6153)
- Updated how `telemetryInitializer` is created so it's instanced per Collector instance rather than global to the process (#6138)
- Fully overrides default slice fields of config (#4001)

## v0.60.0 Beta

Expand Down
18 changes: 18 additions & 0 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,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 @@ -292,6 +293,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 zeroes 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
119 changes: 119 additions & 0 deletions confmap/confmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,122 @@ func TestUnmarshalerErr(t *testing.T) {
assert.EqualError(t, cfgMap.UnmarshalExact(tce), 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 zero 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.UnmarshalExact(tt.provided)
if assert.NoError(t, err) {
assert.Equal(t, tt.expected, tt.provided)
}
})
}
}

0 comments on commit ca970c1

Please sign in to comment.