Skip to content

Commit

Permalink
[confmap] confmap honors Unmarshal methods on config embedded struc…
Browse files Browse the repository at this point in the history
…ts. (open-telemetry#9635)

**Description:**
This implements support for calling `Unmarshal` on embedded structs of
structs being decoded.

**Link to tracking Issue:**
Fixes open-telemetry#6671

**Testing:**
Unit tests.

Contrib fix is open:
open-telemetry/opentelemetry-collector-contrib#31406
  • Loading branch information
atoulme authored and tomasmota committed Mar 14, 2024
1 parent b0fedad commit 59f3fc7
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 2 deletions.
25 changes: 25 additions & 0 deletions .chloggen/support_embedded_structs_confmap.yaml
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: confmap

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: confmap honors `Unmarshal` methods on config embedded structs.

# One or more tracking issues or pull requests related to the change
issues: [6671]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
27 changes: 27 additions & 0 deletions component/config_test.go
Expand Up @@ -11,6 +11,8 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/confmap"
)

var _ fmt.Stringer = Type{}
Expand Down Expand Up @@ -417,3 +419,28 @@ func TestNewType(t *testing.T) {
})
}
}

type configWithEmbeddedStruct struct {
String string `mapstructure:"string"`
Num int `mapstructure:"num"`
embeddedUnmarshallingConfig
}

type embeddedUnmarshallingConfig struct {
}

func (euc *embeddedUnmarshallingConfig) Unmarshal(_ *confmap.Conf) error {
return nil // do nothing.
}
func TestStructWithEmbeddedUnmarshaling(t *testing.T) {
t.Skip("Skipping, to be fixed with https://github.com/open-telemetry/opentelemetry-collector/issues/7102")
cfgMap := confmap.NewFromStringMap(map[string]any{
"string": "foo",
"num": 123,
})
tc := &configWithEmbeddedStruct{}
err := UnmarshalConfig(cfgMap, tc)
require.NoError(t, err)
assert.Equal(t, "foo", tc.String)
assert.Equal(t, 123, tc.Num)
}
39 changes: 39 additions & 0 deletions confmap/confmap.go
Expand Up @@ -157,6 +157,9 @@ func decodeConfig(m *Conf, result any, errorUnused bool) error {
mapstructure.StringToTimeDurationHookFunc(),
mapstructure.TextUnmarshallerHookFunc(),
unmarshalerHookFunc(result),
// after the main unmarshaler hook is called,
// we unmarshal the embedded structs if present to merge with the result:
unmarshalerEmbeddedStructsHookFunc(),
zeroSliceHookFunc(),
),
}
Expand Down Expand Up @@ -261,6 +264,42 @@ func mapKeyStringToMapKeyTextUnmarshalerHookFunc() mapstructure.DecodeHookFuncTy
}
}

// unmarshalerEmbeddedStructsHookFunc provides a mechanism for embedded structs to define their own unmarshal logic,
// by implementing the Unmarshaler interface.
func unmarshalerEmbeddedStructsHookFunc() mapstructure.DecodeHookFuncValue {
return func(from reflect.Value, to reflect.Value) (any, error) {
if to.Type().Kind() != reflect.Struct {
return from.Interface(), nil
}
fromAsMap, ok := from.Interface().(map[string]any)
if !ok {
return from.Interface(), nil
}
for i := 0; i < to.Type().NumField(); i++ {
// embedded structs passed in via `squash` cannot be pointers. We just check if they are structs:
if to.Type().Field(i).IsExported() && to.Type().Field(i).Anonymous {
if unmarshaler, ok := to.Field(i).Addr().Interface().(Unmarshaler); ok {
if err := unmarshaler.Unmarshal(NewFromStringMap(fromAsMap)); err != nil {
return nil, err
}
// the struct we receive from this unmarshaling only contains fields related to the embedded struct.
// we merge this partially unmarshaled struct with the rest of the result.
// note we already unmarshaled the main struct earlier, and therefore merge with it.
conf := New()
if err := conf.Marshal(unmarshaler); err != nil {
return nil, err
}
resultMap := conf.ToStringMap()
for k, v := range resultMap {
fromAsMap[k] = v
}
}
}
}
return fromAsMap, nil
}
}

// Provides a mechanism for individual structs to define their own unmarshal logic,
// by implementing the Unmarshaler interface.
func unmarshalerHookFunc(result any) mapstructure.DecodeHookFuncValue {
Expand Down
121 changes: 119 additions & 2 deletions confmap/confmap_test.go
Expand Up @@ -309,8 +309,78 @@ func newConfFromFile(t testing.TB, fileName string) map[string]any {
}

type testConfig struct {
Next *nextConfig `mapstructure:"next"`
Another string `mapstructure:"another"`
Next *nextConfig `mapstructure:"next"`
Another string `mapstructure:"another"`
EmbeddedConfig `mapstructure:",squash"`
EmbeddedConfig2 `mapstructure:",squash"`
}

type testConfigWithoutUnmarshaler struct {
Next *nextConfig `mapstructure:"next"`
Another string `mapstructure:"another"`
EmbeddedConfig `mapstructure:",squash"`
EmbeddedConfig2 `mapstructure:",squash"`
}

type testConfigWithEmbeddedError struct {
Next *nextConfig `mapstructure:"next"`
Another string `mapstructure:"another"`
EmbeddedConfigWithError `mapstructure:",squash"`
}

type testConfigWithMarshalError struct {
Next *nextConfig `mapstructure:"next"`
Another string `mapstructure:"another"`
EmbeddedConfigWithMarshalError `mapstructure:",squash"`
}

func (tc *testConfigWithEmbeddedError) Unmarshal(component *Conf) error {
if err := component.Unmarshal(tc, WithIgnoreUnused()); err != nil {
return err
}
return nil
}

type EmbeddedConfig struct {
Some string `mapstructure:"some"`
}

func (ec *EmbeddedConfig) Unmarshal(component *Conf) error {
if err := component.Unmarshal(ec, WithIgnoreUnused()); err != nil {
return err
}
ec.Some += " is also called"
return nil
}

type EmbeddedConfig2 struct {
Some2 string `mapstructure:"some_2"`
}

func (ec *EmbeddedConfig2) Unmarshal(component *Conf) error {
if err := component.Unmarshal(ec, WithIgnoreUnused()); err != nil {
return err
}
ec.Some2 += " also called2"
return nil
}

type EmbeddedConfigWithError struct {
}

func (ecwe *EmbeddedConfigWithError) Unmarshal(_ *Conf) error {
return errors.New("embedded error")
}

type EmbeddedConfigWithMarshalError struct {
}

func (ecwe EmbeddedConfigWithMarshalError) Marshal(_ *Conf) error {
return errors.New("marshaling error")
}

func (ecwe EmbeddedConfigWithMarshalError) Unmarshal(_ *Conf) error {
return nil
}

func (tc *testConfig) Unmarshal(component *Conf) error {
Expand Down Expand Up @@ -340,12 +410,59 @@ func TestUnmarshaler(t *testing.T) {
"string": "make sure this",
},
"another": "make sure this",
"some": "make sure this",
"some_2": "this better be",
})

tc := &testConfig{}
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)
assert.Equal(t, "make sure this is also called", tc.EmbeddedConfig.Some)
assert.Equal(t, "this better be also called2", tc.EmbeddedConfig2.Some2)
}

func TestEmbeddedUnmarshaler(t *testing.T) {
cfgMap := NewFromStringMap(map[string]any{
"next": map[string]any{
"string": "make sure this",
},
"another": "make sure this",
"some": "make sure this",
"some_2": "this better be",
})

tc := &testConfigWithoutUnmarshaler{}
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)
assert.Equal(t, "make sure this is also called", tc.EmbeddedConfig.Some)
assert.Equal(t, "this better be also called2", tc.EmbeddedConfig2.Some2)
}

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

tc := &testConfigWithEmbeddedError{}
assert.EqualError(t, cfgMap.Unmarshal(tc), "error decoding '': embedded error")
}

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

tc := &testConfigWithMarshalError{}
assert.EqualError(t, cfgMap.Unmarshal(tc), "error decoding '': error running encode hook: marshaling error")
}

func TestUnmarshalerKeepAlreadyInitialized(t *testing.T) {
Expand Down

0 comments on commit 59f3fc7

Please sign in to comment.