diff --git a/envconfig.go b/envconfig.go index 3719648..145d285 100644 --- a/envconfig.go +++ b/envconfig.go @@ -330,18 +330,20 @@ 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, err := lookup(key, opts, l) + val, found, usedDefault, err := lookup(key, opts, l) if err != nil && !errors.Is(err, ErrMissingKey) { return fmt.Errorf("%s: %w", tf.Name, err) } - if ok, err := processAsDecoder(val, ef); ok { - if err != nil { - return err - } + if found || usedDefault { + if ok, err := processAsDecoder(val, ef); ok { + if err != nil { + return err + } - setNilStruct(ef) - continue + setNilStruct(ef) + continue + } } plu := l @@ -368,23 +370,34 @@ func processWith(ctx context.Context, i interface{}, l Lookuper, parentNoInit bo continue } - // The field already has a non-zero value and overwrite is false, do not overwrite. + // The field already has a non-zero value and overwrite is false, do not + // overwrite. if !ef.IsZero() && !opts.Overwrite { continue } - val, err := lookup(key, opts, l) + val, found, usedDefault, err := lookup(key, opts, l) if err != nil { return fmt.Errorf("%s: %w", tf.Name, err) } + // If the field already has a non-zero value and there was no value directly + // specified, do not overwrite the existing field. We only want to overwrite + // when the envvar was provided directly. + if !ef.IsZero() && !found { + continue + } + // Apply any mutators. Mutators are applied after the lookup, but before any - // type conversions. They always resolve to a string (or error) - for _, fn := range fns { - if fn != nil { - val, err = fn(ctx, key, val) - if err != nil { - return fmt.Errorf("%s: %w", tf.Name, err) + // type conversions. They always resolve to a string (or error), so we don't + // call mutators when the environment variable was not set. + if found || usedDefault { + for _, fn := range fns { + if fn != nil { + val, err = fn(ctx, key, val) + if err != nil { + return fmt.Errorf("%s: %w", tf.Name, err) + } } } } @@ -450,29 +463,32 @@ LOOP: return key, &opts, nil } -// lookup looks up the given key using the provided Lookuper and options. -func lookup(key string, opts *options, l Lookuper) (string, error) { +// lookup looks up the given key using the provided Lookuper and options. The +// first boolean parameter indicates whether the value was found in the +// lookuper. The second boolean parameter indicates whether the default value +// was used. +func lookup(key string, opts *options, l Lookuper) (string, bool, bool, error) { if key == "" { // The struct has something like `env:",required"`, which is likely a // mistake. We could try to infer the envvar from the field name, but that // feels too magical. - return "", ErrMissingKey + return "", false, false, ErrMissingKey } if opts.Required && opts.Default != "" { // Having a default value on a required value doesn't make sense. - return "", ErrRequiredAndDefault + return "", false, false, ErrRequiredAndDefault } // Lookup value. - val, ok := l.Lookup(key) - if !ok { + val, found := l.Lookup(key) + if !found { if opts.Required { if pl, ok := l.(*prefixLookuper); ok { key = pl.prefix + key } - return "", fmt.Errorf("%w: %s", ErrMissingRequired, key) + return "", false, false, fmt.Errorf("%w: %s", ErrMissingRequired, key) } if opts.Default != "" { @@ -485,10 +501,12 @@ func lookup(key string, opts *options, l Lookuper) (string, error) { } return "" }) + + return val, false, true, nil } } - return val, nil + return val, found, false, nil } // processAsDecoder processes the given value as a decoder or custom diff --git a/envconfig_test.go b/envconfig_test.go index 64fe875..d7e9cc6 100644 --- a/envconfig_test.go +++ b/envconfig_test.go @@ -876,6 +876,106 @@ func TestProcessWith(t *testing.T) { "FIELD": "foo", }), }, + { + name: "overwrite/does_not_overwrite_no_value", + input: &struct { + Field string `env:"FIELD, overwrite"` + }{ + Field: "inside", + }, + exp: &struct { + Field string `env:"FIELD, overwrite"` + }{ + Field: "inside", + }, + lookuper: MapLookuper(nil), + }, + { + name: "overwrite/env_overwrites_existing", + input: &struct { + Field string `env:"FIELD, overwrite"` + }{ + Field: "inside", + }, + exp: &struct { + Field string `env:"FIELD, overwrite"` + }{ + Field: "env", + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "env", + }), + }, + { + name: "overwrite/env_overwrites_empty", + input: &struct { + Field string `env:"FIELD, overwrite"` + }{}, + exp: &struct { + Field string `env:"FIELD, overwrite"` + }{ + Field: "env", + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "env", + }), + }, + { + name: "overwrite/default_does_not_overwrite_no_value", + input: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{ + Field: "inside", + }, + exp: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{ + Field: "inside", + }, + lookuper: MapLookuper(nil), + }, + { + name: "overwrite/default_env_overwrites_existing", + input: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{ + Field: "inside", + }, + exp: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{ + Field: "env", + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "env", + }), + }, + { + name: "overwrite/default_env_overwrites_empty", + input: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{}, + exp: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{ + Field: "env", + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "env", + }), + }, + { + name: "overwrite/default_uses_default_when_unspecified", + input: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{}, + exp: &struct { + Field string `env:"FIELD, overwrite, default=default"` + }{ + Field: "default", + }, + lookuper: MapLookuper(nil), + }, // Required { @@ -1276,16 +1376,44 @@ func TestProcessWith(t *testing.T) { input: &struct { field *CustomDecoderType `env:"FIELD"` }{}, - lookuper: MapLookuper(map[string]string{}), - err: ErrPrivateField, + lookuper: MapLookuper(map[string]string{ + "FIELD": "foo", + }), + err: ErrPrivateField, }, { name: "custom_decoder/error", input: &struct { Field CustomTypeError `env:"FIELD"` }{}, + lookuper: MapLookuper(map[string]string{ + "FIELD": "foo", + }), + errMsg: "broken", + }, + { + name: "custom_decoder/called_for_empty_string", + input: &struct { + Field CustomTypeError `env:"FIELD"` + }{}, + lookuper: MapLookuper(map[string]string{ + "FIELD": "", + }), + 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(map[string]string{}), - errMsg: "broken", + // Note: We explicitly want no error here. The custom marshaller should + // not have been called, since the environment variables was not defined. }, // Expand diff --git a/go.mod b/go.mod index b2ff131..d2c2d9c 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,4 @@ module github.com/sethvargo/go-envconfig go 1.17 -require github.com/google/go-cmp v0.4.1 +require github.com/google/go-cmp v0.5.8 diff --git a/go.sum b/go.sum index 210ab77..e9b099c 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,2 @@ -github.com/google/go-cmp v0.4.1 h1:/exdXoGamhu5ONeUJH0deniYLWYvQwW66yvlfiiKTu0= -github.com/google/go-cmp v0.4.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=