Skip to content

Commit

Permalink
Switch order of checks in Unmarshaler hook
Browse files Browse the repository at this point in the history
  • Loading branch information
djaglowski committed Sep 9, 2022
1 parent 233c74d commit de697ef
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 11 deletions.
12 changes: 6 additions & 6 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,12 @@ func mapKeyStringToMapKeyTextUnmarshalerHookFunc() mapstructure.DecodeHookFuncTy
// by implementing the Unmarshaler interface.
func unmarshalerHookFunc(result interface{}) mapstructure.DecodeHookFuncValue {
return func(from reflect.Value, to reflect.Value) (interface{}, error) {
if _, ok := from.Interface().(map[string]interface{}); !ok {
toPtr := to.Addr().Interface()
if _, ok := toPtr.(Unmarshaler); !ok {
return from.Interface(), nil
}

toPtr := to.Addr().Interface()
if _, ok := toPtr.(Unmarshaler); !ok {
if _, ok := from.Interface().(map[string]interface{}); !ok {
return from.Interface(), nil
}

Expand All @@ -226,12 +226,12 @@ func unmarshalerHookFunc(result interface{}) mapstructure.DecodeHookFuncValue {
return from.Interface(), nil
}

unmarshaller := reflect.New(to.Type()).Interface().(Unmarshaler)
if err := unmarshaller.Unmarshal(NewFromStringMap(from.Interface().(map[string]interface{}))); err != nil {
unmarshaler := reflect.New(to.Type()).Interface().(Unmarshaler)
if err := unmarshaler.Unmarshal(NewFromStringMap(from.Interface().(map[string]interface{}))); err != nil {
return nil, err
}

return unmarshaller, nil
return unmarshaler, nil
}
}

Expand Down
37 changes: 32 additions & 5 deletions confmap/confmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (tc *testConfig) Unmarshal(component *Conf) error {
if err := component.UnmarshalExact(tc); err != nil {
return err
}
tc.Another += " is not called"
tc.Another += " is only called directly"
return nil
}

Expand All @@ -268,17 +268,37 @@ func (nc *nextConfig) Unmarshal(component *Conf) error {
return nil
}

func TestUnmarshallable(t *testing.T) {
func TestUnmarshaler(t *testing.T) {
cfgMap := NewFromStringMap(map[string]interface{}{
"next": map[string]interface{}{
"string": "make sure this",
},
"another": "make sure this",
})

tc := &testConfig{}
assert.NoError(t, cfgMap.UnmarshalExact(tc))
assert.NoError(t, cfgMap.Unmarshal(tc))
assert.Equal(t, "make sure this", tc.Another)
assert.Equal(t, "make sure this is called", tc.Next.String)

tce := &testConfig{}
assert.NoError(t, cfgMap.UnmarshalExact(tce))
assert.Equal(t, "make sure this", tce.Another)
assert.Equal(t, "make sure this is called", tce.Next.String)
}

func TestDirectUnmarshaler(t *testing.T) {
cfgMap := NewFromStringMap(map[string]interface{}{
"next": map[string]interface{}{
"string": "make sure this",
},
"another": "make sure this",
})

tc := &testConfig{}
assert.NoError(t, tc.Unmarshal(cfgMap))
assert.Equal(t, "make sure this is only called directly", tc.Another)
assert.Equal(t, "make sure this is called", tc.Next.String)
}

type testErrConfig struct {
Expand All @@ -297,13 +317,20 @@ func (tc *errConfig) Unmarshal(component *Conf) error {
return errors.New("never works")
}

func TestUnmarshallableErr(t *testing.T) {
func TestUnmarshalerErr(t *testing.T) {
cfgMap := NewFromStringMap(map[string]interface{}{
"err": map[string]interface{}{
"foo": "will not unmarshal due to error",
},
})

expectErr := "1 error(s) decoding:\n\n* error decoding 'err': never works"

tc := &testErrConfig{}
assert.EqualError(t, cfgMap.UnmarshalExact(tc), "1 error(s) decoding:\n\n* error decoding 'err': never works")
assert.EqualError(t, cfgMap.Unmarshal(tc), expectErr)
assert.Empty(t, tc.Err.Foo)

tce := &testErrConfig{}
assert.EqualError(t, cfgMap.UnmarshalExact(tce), expectErr)
assert.Empty(t, tc.Err.Foo)
}

0 comments on commit de697ef

Please sign in to comment.