Remove interfaces from xconfmap

Author: evan-bradley

config/configoptional/optional.go

@@ -150,8 +150,8 @@ func (o *Optional[T]) GetOrInsertDefault() *T {
 }
 
 var (
-	_ confmap.Unmarshaler        = (*Optional[any])(nil)
-	_ xconfmap.ScalarUnmarshaler = (*Optional[any])(nil)
+	_ confmap.Unmarshaler       = (*Optional[any])(nil)
+	_ confmap.ScalarUnmarshaler = (*Optional[any])(nil)
 )
 
 // Unmarshal the configuration into the Optional value.
@@ -211,7 +211,7 @@ func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error {
 // A `nil` value will set the Optional to None, disabling it as setting
 // `enabled: false` for a struct-type Optional or `null` for a pointer field
 // would.
-func (o *Optional[T]) UnmarshalScalar(scalarValue xconfmap.ScalarValue) error {
+func (o *Optional[T]) UnmarshalScalar(scalarValue confmap.ScalarValue) error {
 	if scalarValue.GetRaw() == nil {
 		if deref(reflect.TypeOf(o.value)).Kind() == reflect.Struct {
 			// Defer to Unmarshal behavior
@@ -234,8 +234,8 @@ func (o *Optional[T]) UnmarshalScalar(scalarValue xconfmap.ScalarValue) error {
 }
 
 var (
-	_ confmap.Marshaler        = (*Optional[any])(nil)
-	_ xconfmap.ScalarMarshaler = (*Optional[any])(nil)
+	_ confmap.Marshaler       = (*Optional[any])(nil)
+	_ confmap.ScalarMarshaler = (*Optional[any])(nil)
 )
 
 // Marshal the Optional value into the configuration.
@@ -257,7 +257,7 @@ func (o Optional[T]) Marshal(conf *confmap.Conf) error {
 	return nil
 }
 
-func (o Optional[T]) MarshalScalar(scalarValue xconfmap.ScalarValue) error {
+func (o Optional[T]) MarshalScalar(scalarValue confmap.ScalarValue) error {
 	if deref(reflect.TypeOf(o.value)).Kind() == reflect.Struct {
 		// Defer to Marshal behavior
 		return nil

confmap/confmap.go

@@ -49,3 +49,25 @@ type Unmarshaler = internal.Unmarshaler
 // A configuration struct can implement this interface to override the default
 // marshaling.
 type Marshaler = internal.Marshaler
+
+// ScalarValue provides access to a scalar configuration value and allows
+// calling back into the confmap decoding/encoding machinery.
+//
+// This interface is only provided to methods used for [ScalarUnmarshaler] and
+// [ScalarMarshaler] implementations and cannot be implemented by types outside
+// the confmap package.
+type ScalarValue = internal.ScalarValue
+
+// ScalarUnmarshaler is an interface which may be implemented by wrapper types
+// to customize their behavior when the type under the wrapper is a scalar
+// value.
+//
+// This should be used for types like `Wrapper[T]` where T is a scalar type, and
+// the wrapper type needs to implement custom logic for unmarshaling from a
+// scalar value (e.g. `5` for `Wrapper[int]`) into the wrapper type (e.g.
+// `Wrapper[int]{inner: 5}`).
+type ScalarUnmarshaler = internal.ScalarUnmarshaler
+
+// ScalarMarshaler is an interface which may be implemented by wrapper types
+// to customize their behavior when the type under the wrapper is a scalar value.
+type ScalarMarshaler = internal.ScalarMarshaler

confmap/xconfmap/scalarmarshaler_test.go confmap/internal/scalar_test.goconfmap/xconfmap/scalarmarshaler_test.go renamed to confmap/internal/scalar_test.go

@@ -1,19 +1,18 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-package xconfmap
+package internal
 
 import (
 	"bytes"
 	"errors"
 	"fmt"
 	"path/filepath"
+	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"go.opentelemetry.io/collector/confmap"
-	"go.opentelemetry.io/collector/confmap/confmaptest"
 )
 
 type textMarshalerStruct struct {
@@ -53,24 +52,24 @@ type NonImplWrapperType[T any] struct {
 }
 
 var (
-	_ confmap.Unmarshaler = (*wrapperType[any])(nil)
-	_ ScalarMarshaler     = wrapperType[any]{}
-	_ ScalarUnmarshaler   = (*wrapperType[any])(nil)
+	_ Unmarshaler       = (*wrapperType[any])(nil)
+	_ ScalarMarshaler   = wrapperType[any]{}
+	_ ScalarUnmarshaler = (*wrapperType[any])(nil)
 )
 
 type wrapperType[T any] struct {
 	inner T `mapstructure:"-"`
 }
 
-func (wt *wrapperType[T]) Unmarshal(conf *confmap.Conf) error {
+func (wt *wrapperType[T]) Unmarshal(conf *Conf) error {
 	if err := conf.Unmarshal(&wt.inner); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (wt wrapperType[T]) Marshal(conf *confmap.Conf) error {
+func (wt wrapperType[T]) Marshal(conf *Conf) error {
 	if err := conf.Marshal(wt.inner); err != nil {
 		return fmt.Errorf("failed to marshal wrapperType value: %w", err)
 	}
@@ -92,7 +91,7 @@ func (wt *wrapperType[T]) UnmarshalScalar(val ScalarValue) error {
 	return nil
 }
 
-type testConfig struct {
+type testScalarConf struct {
 	// Handled by confmap, treated as string
 	Tma         textMarshalerAlias                            `mapstructure:"text_marshaler_alias"`
 	Ntma        nonTextMarshalerAlias                         `mapstructure:"non_text_marshaler_alias"`
@@ -107,7 +106,7 @@ type testConfig struct {
 	Recursive   wrapperType[wrapperType[textMarshalerStruct]] `mapstructure:"recursive"`
 }
 
-func (cfg *testConfig) Unmarshal(conf *confmap.Conf) error {
+func (cfg *testScalarConf) Unmarshal(conf *Conf) error {
 	if err := conf.Unmarshal(cfg); err != nil {
 		return err
 	}
@@ -116,14 +115,13 @@ func (cfg *testConfig) Unmarshal(conf *confmap.Conf) error {
 }
 
 func TestMarshalConfig(t *testing.T) {
-	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml"))
-	require.NoError(t, err)
-	wantCfg := &testConfig{}
+	cm := NewFromStringMap(newConfFromFile(t, filepath.Join("testdata", "scalar.yaml")))
+	wantCfg := &testScalarConf{}
 	require.NoError(t, cm.Unmarshal(wantCfg))
 	require.NoError(t, cm.Marshal(wantCfg))
 
-	conf := confmap.New()
-	cfg := &testConfig{
+	conf := New()
+	cfg := &testScalarConf{
 		Tma:         textMarshalerAlias("test"),
 		Ntma:        nonTextMarshalerAlias("test"),
 		Nonimplint:  NonImplWrapperType[int]{inner: 1},
@@ -149,14 +147,14 @@ func (f failingScalarMarshaler) MarshalScalar(_ ScalarValue) error {
 }
 
 // TestMarshalScalarErrorPropagation verifies that an error returned by
-// MarshalScalar surfaces as an error from confmap.Marshal.
+// MarshalScalar surfaces as an error from Marshal.
 func TestMarshalScalarErrorPropagation(t *testing.T) {
 	type cfgWithFailing struct {
 		Val failingScalarMarshaler `mapstructure:"val"`
 	}
 
 	cfg := cfgWithFailing{Val: failingScalarMarshaler{}}
-	conf := confmap.New()
+	conf := New()
 	err := conf.Marshal(&cfg)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "marshal always fails")
@@ -178,7 +176,7 @@ func TestMarshalNonImplementingTypesUnaffected(t *testing.T) {
 		NonImpl: NonImplWrapperType[int]{inner: 7},
 		Plain:   99,
 	}
-	conf := confmap.New()
+	conf := New()
 	require.NoError(t, conf.Marshal(cfg))
 
 	m := conf.ToStringMap()
@@ -188,3 +186,110 @@ func TestMarshalNonImplementingTypesUnaffected(t *testing.T) {
 	_, ok := m["non_impl"]
 	require.True(t, ok, "non-implementing field should still appear in output")
 }
+
+type nullableWrapperType[T any] struct {
+	inner  T
+	wasNil bool
+}
+
+func (n *nullableWrapperType[T]) UnmarshalScalar(val ScalarValue) error {
+	raw := val.GetRaw()
+	if raw == nil || (reflect.ValueOf(raw).Kind() == reflect.Map && reflect.ValueOf(raw).IsNil()) {
+		n.wasNil = true
+		return nil
+	}
+	var v T
+	if err := val.Unmarshal(&v); err != nil {
+		return fmt.Errorf("nullableWrapperType: %w", err)
+	}
+	n.inner = v
+	return nil
+}
+
+type failingScalarUnmarshaler struct{}
+
+func (f *failingScalarUnmarshaler) UnmarshalScalar(_ ScalarValue) error {
+	return errors.New("always fails")
+}
+
+func TestUnmarshalConfig(t *testing.T) {
+	wantCfg := &testScalarConf{
+		Tma:       textMarshalerAlias("test"),
+		Ntma:      nonTextMarshalerAlias("test"),
+		Implint:   wrapperType[int]{inner: 1},
+		Implstr:   wrapperType[string]{inner: "test"},
+		Impltms:   wrapperType[textMarshalerStruct]{inner: textMarshalerStruct{id: 0, data: []byte{81}}},
+		Recursive: wrapperType[wrapperType[textMarshalerStruct]]{inner: wrapperType[textMarshalerStruct]{inner: textMarshalerStruct{id: 0, data: []byte{80}}}},
+	}
+
+	cm := NewFromStringMap(newConfFromFile(t, filepath.Join("testdata", "scalar.yaml")))
+	cfg := &testScalarConf{}
+	require.NoError(t, cm.Unmarshal(cfg))
+
+	require.Equal(t, wantCfg, cfg)
+}
+
+// TestUnmarshalScalarNullInput verifies that the hook calls UnmarshalScalar(nil)
+// when the source value is a nil map, which is how mapstructure represents a
+// YAML null for a map-typed value.
+func TestUnmarshalScalarNullInput(t *testing.T) {
+	type cfgWithNullable struct {
+		Val nullableWrapperType[int] `mapstructure:"val"`
+	}
+
+	// A nil map value triggers the `from.Kind() == reflect.Map && from.IsNil()` branch.
+	cm := NewFromStringMap(map[string]any{"val": map[string]any(nil)})
+	var cfg cfgWithNullable
+	require.NoError(t, cm.Unmarshal(&cfg))
+	assert.True(t, cfg.Val.wasNil, "expected UnmarshalScalar to be called with nil")
+	assert.Equal(t, 0, cfg.Val.inner, "inner value should remain zero after nil")
+}
+
+// TestUnmarshalScalarDecodeError verifies that errors from internal.Decode are
+// propagated when the source value cannot be decoded into ScalarType().
+func TestUnmarshalScalarDecodeError(t *testing.T) {
+	type cfgWithInt struct {
+		Val wrapperType[int] `mapstructure:"val"`
+	}
+
+	// A slice cannot be decoded into an int; this exercises the internal.Decode error path.
+	cm := NewFromStringMap(map[string]any{"val": []string{"a", "b"}})
+	cfg := cfgWithInt{}
+	err := cm.Unmarshal(&cfg)
+	require.Error(t, err)
+}
+
+// TestUnmarshalScalarErrorPropagation verifies that an error returned by
+// UnmarshalScalar surfaces as an error from Unmarshal.
+func TestUnmarshalScalarErrorPropagation(t *testing.T) {
+	type cfgWithFailing struct {
+		Val failingScalarUnmarshaler `mapstructure:"val"`
+	}
+
+	cm := NewFromStringMap(map[string]any{"val": 42})
+	var cfg cfgWithFailing
+	err := cm.Unmarshal(&cfg)
+	require.Error(t, err)
+	require.ErrorContains(t, err, "always fails")
+}
+
+// TestNonImplementingTypesUnaffected verifies that fields whose types do not
+// implement ScalarUnmarshaler are decoded normally by mapstructure, even when
+// implementing fields are present in the same struct.
+func TestNonImplementingTypesUnaffected(t *testing.T) {
+	type mixedCfg struct {
+		Impl    wrapperType[int]        `mapstructure:"impl"`
+		NonImpl NonImplWrapperType[int] `mapstructure:"non_impl"`
+		Plain   int                     `mapstructure:"plain"`
+	}
+
+	cm := NewFromStringMap(map[string]any{
+		"impl":  10,
+		"plain": 99,
+	})
+	var cfg mixedCfg
+	require.NoError(t, cm.Unmarshal(&cfg))
+	assert.Equal(t, 10, cfg.Impl.inner, "implementing field should be decoded via UnmarshalScalar")
+	assert.Equal(t, 99, cfg.Plain, "plain field should be decoded normally")
+	assert.Equal(t, 0, cfg.NonImpl.inner, "non-implementing field should remain zero")
+}