Always call decoders (#68)

This reverts part of #62 to always process decoder interfaces for struct fields. This removes a breaking change where certain built-in types like net/url.URL have a custom unmarshaling function for the empty string. The other rules for "overwrite" still apply.
sethvargo · Jul 27, 2022 · ce31e42 · ce31e42
1 parent 20ad4b3
commit ce31e42
Show file tree

Hide file tree

Showing 2 changed files with 136 additions and 23 deletions.
diff --git a/envconfig.go b/envconfig.go
@@ -330,20 +330,18 @@ func processWith(ctx context.Context, i interface{}, l Lookuper, parentNoInit bo
 			// Lookup the value, ignoring an error if the key isn't defined. This is
 			// required for nested structs that don't declare their own `env` keys,
 			// but have internal fields with an `env` defined.
-			val, found, usedDefault, err := lookup(key, opts, l)
+			val, _, _, err := lookup(key, opts, l)
 			if err != nil && !errors.Is(err, ErrMissingKey) {
 				return fmt.Errorf("%s: %w", tf.Name, err)
 			}
 
-			if found || usedDefault {
-				if ok, err := processAsDecoder(val, ef); ok {
-					if err != nil {
-						return err
-					}
-
-					setNilStruct(ef)
-					continue
+			if ok, err := processAsDecoder(val, ef); ok {
+				if err != nil {
+					return err
 				}
+
+				setNilStruct(ef)
+				continue
 			}
 
 			plu := l

diff --git a/envconfig_test.go b/envconfig_test.go
@@ -42,6 +42,31 @@ func (c *CustomDecoderType) EnvDecode(val string) error {
 	return nil
 }
 
+// Level mirrors Zap's level marshalling to reproduce an issue for tests.
+type Level int8
+
+const (
+	DebugLevel Level = 0
+	InfoLevel  Level = 5
+	ErrorLevel Level = 100
+)
+
+func (l *Level) UnmarshalText(text []byte) error {
+	switch string(text) {
+	case "debug":
+		*l = DebugLevel
+		return nil
+	case "info", "": // default
+		*l = InfoLevel
+		return nil
+	case "error":
+		*l = ErrorLevel
+		return nil
+	default:
+		return fmt.Errorf("unknown level %s", string(text))
+	}
+}
+
 var (
 	_ encoding.BinaryUnmarshaler = (*CustomStdLibDecodingType)(nil)
 	_ encoding.TextUnmarshaler   = (*CustomStdLibDecodingType)(nil)
@@ -1401,20 +1426,6 @@ func TestProcessWith(t *testing.T) {
 			}),
 			errMsg: "broken",
 		},
-		{
-			name: "custom_decoder/not_called_on_unset_envvar",
-			input: &struct {
-				Field CustomTypeError `env:"FIELD"`
-			}{},
-			exp: &struct {
-				Field CustomTypeError `env:"FIELD"`
-			}{
-				Field: CustomTypeError{},
-			},
-			lookuper: MapLookuper(nil),
-			// Note: We explicitly want no error here. The custom marshaller should
-			// not have been called, since the environment variables was not defined.
-		},
 
 		// Expand
 		{
@@ -2112,6 +2123,110 @@ func TestProcessWith(t *testing.T) {
 				"VCR_REMOTE_BUTTON_NAME": "button",
 			}),
 		},
+		{
+			// https://github.com/sethvargo/go-envconfig/issues/61
+			name: "custom_decoder_overwrite_uses_default",
+			input: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{},
+			exp: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{
+				Level: ErrorLevel,
+			},
+			lookuper: MapLookuper(nil),
+		},
+		{
+			// https://github.com/sethvargo/go-envconfig/issues/61
+			name: "custom_decoder_overwrite_unset",
+			input: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{},
+			exp: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{
+				Level: DebugLevel,
+			},
+			lookuper: MapLookuper(map[string]string{
+				"LEVEL": "debug",
+			}),
+		},
+		{
+			// https://github.com/sethvargo/go-envconfig/issues/61
+			name: "custom_decoder_overwrite_existing_value",
+			input: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{
+				Level: InfoLevel,
+			},
+			exp: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{
+				Level: InfoLevel,
+			},
+			lookuper: MapLookuper(nil),
+		},
+		{
+			// https://github.com/sethvargo/go-envconfig/issues/61
+			name: "custom_decoder_overwrite_existing_value_envvar",
+			input: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{
+				Level: InfoLevel,
+			},
+			exp: &struct {
+				Level Level `env:"LEVEL,overwrite,default=error"`
+			}{
+				Level: DebugLevel,
+			},
+			lookuper: MapLookuper(map[string]string{
+				"LEVEL": "debug",
+			}),
+		},
+		{
+			// https://github.com/sethvargo/go-envconfig/issues/64
+			name: "custom_decoder_uses_decoder_no_env",
+			input: &struct {
+				URL *url.URL
+			}{},
+			exp: &struct {
+				URL *url.URL
+			}{
+				URL: &url.URL{},
+			},
+			lookuper: MapLookuper(nil),
+		},
+		{
+			// https://github.com/sethvargo/go-envconfig/issues/64
+			name: "custom_decoder_uses_decoder_env_no_value",
+			input: &struct {
+				URL *url.URL `env:"URL"`
+			}{},
+			exp: &struct {
+				URL *url.URL `env:"URL"`
+			}{
+				URL: &url.URL{},
+			},
+			lookuper: MapLookuper(nil),
+		},
+		{
+			// https://github.com/sethvargo/go-envconfig/issues/64
+			name: "custom_decoder_uses_decoder_env_with_value",
+			input: &struct {
+				URL *url.URL `env:"URL"`
+			}{},
+			exp: &struct {
+				URL *url.URL `env:"URL"`
+			}{
+				URL: &url.URL{
+					Scheme: "https",
+					Host:   "foo.bar",
+				},
+			},
+			lookuper: MapLookuper(map[string]string{
+				"URL": "https://foo.bar",
+			}),
+		},
 	}
 
 	for _, tc := range cases {