Skip to content

Commit

Permalink
Add options to confmap Marshal/Unmarshal, deprecat UnmarshalExact
Browse files Browse the repository at this point in the history
Officially this can be considered breaking change because of a use in a `var foo func(interface{}) error = conf.Unmarshal`, not in a normal calling usage, but this is an edgecase that we can except and move forward with this.

Signed-off-by: Bogdan <[email protected]>
  • Loading branch information
bogdandrutu committed Oct 4, 2022
1 parent b1e1614 commit 4417d70
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- Deprecate `p[metric|log|trace]otlp.RegiserServer` in favor of `p[metric|log|trace]otlp.RegiserGRPCServer` (#6180)
- Deprecate `pcommon.Map.PutString` in favor of `pcommon.Map.PutStr` (#6210)
- Deprecate `pcommon.NewValueString` in favor of `pcommon.NewValueStr` (#6209)
- Deprecate `confmap.Conf.UnmarshalExact` in favor of `confmap.Conf.Unmarshal(.., WithErrorUnused)` (#6231)

### 💡 Enhancements 💡

Expand Down
2 changes: 1 addition & 1 deletion config/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,5 @@ func unmarshal(componentSection *confmap.Conf, intoCfg interface{}) error {
return cu.Unmarshal(componentSection)
}

return componentSection.UnmarshalExact(intoCfg)
return componentSection.Unmarshal(intoCfg, confmap.WithErrorUnused())
}
48 changes: 42 additions & 6 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,55 @@ func (l *Conf) AllKeys() []string {
return l.k.Keys()
}

// Unmarshal unmarshalls the config into a struct.
type UnmarshalOption interface {
apply(*unmarshalOption)
}

type unmarshalOption struct {
// If ErrorUnused is true, then it is an error for there to exist
// keys in the original Conf that were unused in the decoding process
// (extra keys).
errorUnused bool
}

// WithErrorUnused then it is an error for there to exist
// keys in the original Conf that were unused in the decoding process
// (extra keys).
func WithErrorUnused() UnmarshalOption {
return unmarshalOptionFunc(func(uo *unmarshalOption) {
uo.errorUnused = true
})
}

type unmarshalOptionFunc func(*unmarshalOption)

func (fn unmarshalOptionFunc) apply(set *unmarshalOption) {
fn(set)
}

// Unmarshal unmarshalls the config into a struct using the given options.
// Tags on the fields of the structure must be properly set.
func (l *Conf) Unmarshal(result interface{}) error {
return decodeConfig(l, result, false)
func (l *Conf) Unmarshal(result interface{}, opts ...UnmarshalOption) error {
set := unmarshalOption{}
for _, opt := range opts {
opt.apply(&set)
}
return decodeConfig(l, result, set.errorUnused)
}

// UnmarshalExact unmarshalls the config into a struct, erroring if a field is nonexistent.
// Deprecated: [v0.62.0] use Unmarshal.
func (l *Conf) UnmarshalExact(result interface{}) error {
return decodeConfig(l, result, true)
return l.Unmarshal(result, WithErrorUnused())
}

type marshalOption struct{}

type MarshalOption interface {
apply(*marshalOption)
}

// Marshal encodes the config and merges it into the Conf.
func (l *Conf) Marshal(rawVal interface{}) error {
func (l *Conf) Marshal(rawVal interface{}, opts ...MarshalOption) error {
enc := encoder.New(encoderConfig(rawVal))
data, err := enc.Encode(rawVal)
if err != nil {
Expand Down
39 changes: 14 additions & 25 deletions confmap/confmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestExpandNilStructPointersHookFunc(t *testing.T) {
conf := NewFromStringMap(stringMap)
cfg := &TestConfig{}
assert.Nil(t, cfg.Struct)
assert.NoError(t, conf.UnmarshalExact(cfg))
assert.NoError(t, conf.Unmarshal(cfg))
assert.Nil(t, cfg.Boolean)
// assert.False(t, *cfg.Boolean)
assert.Nil(t, cfg.Struct)
Expand All @@ -144,7 +144,7 @@ func TestExpandNilStructPointersHookFuncDefaultNotNilConfigNil(t *testing.T) {
Struct: s1,
MapStruct: map[string]*Struct{"struct": s2},
}
assert.NoError(t, conf.UnmarshalExact(cfg))
assert.NoError(t, conf.Unmarshal(cfg))
assert.NotNil(t, cfg.Boolean)
assert.True(t, *cfg.Boolean)
assert.NotNil(t, cfg.Struct)
Expand All @@ -154,6 +154,15 @@ func TestExpandNilStructPointersHookFuncDefaultNotNilConfigNil(t *testing.T) {
assert.Equal(t, &Struct{}, cfg.MapStruct["struct"])
}

func TestUnmarshalWithErrorUnused(t *testing.T) {
stringMap := map[string]interface{}{
"boolean": true,
"string": "this is a string",
}
conf := NewFromStringMap(stringMap)
assert.Error(t, conf.Unmarshal(&TestIDConfig{}, WithErrorUnused()))
}

type TestConfig struct {
Boolean *bool `mapstructure:"boolean"`
Struct *Struct `mapstructure:"struct"`
Expand Down Expand Up @@ -208,11 +217,6 @@ func TestMapKeyStringToMapKeyTextUnmarshalerHookFunc(t *testing.T) {
}
conf := NewFromStringMap(stringMap)

cfgExact := &TestIDConfig{}
assert.NoError(t, conf.UnmarshalExact(cfgExact))
assert.True(t, cfgExact.Boolean)
assert.Equal(t, map[TestID]string{"string": "this is a string"}, cfgExact.Map)

cfg := &TestIDConfig{}
assert.NoError(t, conf.Unmarshal(cfg))
assert.True(t, cfg.Boolean)
Expand All @@ -229,9 +233,6 @@ func TestMapKeyStringToMapKeyTextUnmarshalerHookFuncDuplicateID(t *testing.T) {
}
conf := NewFromStringMap(stringMap)

cfgExact := &TestIDConfig{}
assert.Error(t, conf.UnmarshalExact(cfgExact))

cfg := &TestIDConfig{}
assert.Error(t, conf.Unmarshal(cfg))
}
Expand All @@ -245,9 +246,6 @@ func TestMapKeyStringToMapKeyTextUnmarshalerHookFuncErrorUnmarshal(t *testing.T)
}
conf := NewFromStringMap(stringMap)

cfgExact := &TestIDConfig{}
assert.Error(t, conf.UnmarshalExact(cfgExact))

cfg := &TestIDConfig{}
assert.Error(t, conf.Unmarshal(cfg))
}
Expand Down Expand Up @@ -326,7 +324,7 @@ type testConfig struct {
}

func (tc *testConfig) Unmarshal(component *Conf) error {
if err := component.UnmarshalExact(tc); err != nil {
if err := component.Unmarshal(tc); err != nil {
return err
}
tc.Another += " is only called directly"
Expand All @@ -338,7 +336,7 @@ type nextConfig struct {
}

func (nc *nextConfig) Unmarshal(component *Conf) error {
if err := component.UnmarshalExact(nc); err != nil {
if err := component.Unmarshal(nc); err != nil {
return err
}
nc.String += " is called"
Expand All @@ -357,11 +355,6 @@ func TestUnmarshaler(t *testing.T) {
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) {
Expand All @@ -383,7 +376,7 @@ type testErrConfig struct {
}

func (tc *testErrConfig) Unmarshal(component *Conf) error {
return component.UnmarshalExact(tc)
return component.Unmarshal(tc)
}

type errConfig struct {
Expand All @@ -406,8 +399,4 @@ func TestUnmarshalerErr(t *testing.T) {
tc := &testErrConfig{}
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)
}
2 changes: 1 addition & 1 deletion receiver/otlpreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
return errors.New("empty config for OTLP receiver")
}
// first load the config normally
err := componentParser.UnmarshalExact(cfg)
err := componentParser.Unmarshal(cfg, confmap.WithErrorUnused())
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions service/internal/configunmarshaler/defaultunmarshaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (ConfigUnmarshaler) Unmarshal(v *confmap.Conf, factories component.Factorie

// Unmarshal top level sections and validate.
rawCfg := configSettings{}
if err := v.UnmarshalExact(&rawCfg); err != nil {
if err := v.Unmarshal(&rawCfg, confmap.WithErrorUnused()); err != nil {
return nil, configError{
error: fmt.Errorf("error reading top level configuration sections: %w", err),
code: errUnmarshalTopLevelStructure,
Expand Down Expand Up @@ -185,7 +185,7 @@ func unmarshalService(srvRaw map[string]interface{}) (config.Service, error) {
},
}

if err := confmap.NewFromStringMap(srvRaw).UnmarshalExact(&srv); err != nil {
if err := confmap.NewFromStringMap(srvRaw).Unmarshal(&srv, confmap.WithErrorUnused()); err != nil {
return srv, fmt.Errorf("error reading service configuration: %w", err)
}

Expand Down

0 comments on commit 4417d70

Please sign in to comment.