From 022d6cbc0ee26153470de2d6846cddbb81b5e746 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 30 May 2025 00:40:01 +0530 Subject: [PATCH 01/12] fix: add map serializer --- env.go | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++ env_test.go | 24 ++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/env.go b/env.go index c928b66..2ba6f25 100644 --- a/env.go +++ b/env.go @@ -415,6 +415,10 @@ func doParseField( return doParseSlice(refField, processField, optionsWithEnvPrefix(refTypeField, opts)) } + if isMapOfStructs(refTypeField) { + return doParseMap(refField, processField, optionsWithEnvPrefix(refTypeField, opts)) + } + return nil } @@ -437,6 +441,20 @@ func isSliceOfStructs(refTypeField reflect.StructField) bool { return false } +func isMapOfStructs(refTypeField reflect.StructField) bool { + field := refTypeField.Type + + if field.Kind() == reflect.Ptr { + field = field.Elem() + } + + if field.Kind() == reflect.Map && field.Elem().Kind() == reflect.Struct { + return true + } + + return false +} + func doParseSlice(ref reflect.Value, processField processFieldFn, opts Options) error { if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { opts.Prefix += string(underscore) @@ -501,6 +519,82 @@ func doParseSlice(ref reflect.Value, processField processFieldFn, opts Options) return nil } +func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) error { + // Ensure prefix ends with underscore + if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { + opts.Prefix += string(underscore) + } + + // Collect all environment variables with the prefix + var environments []string + for environment := range opts.Environment { + if strings.HasPrefix(environment, opts.Prefix) { + environments = append(environments, environment) + } + } + + if len(environments) > 0 { + // Create a new map if it's nil + if ref.IsNil() { + ref.Set(reflect.MakeMap(ref.Type())) + } + + // Get the key and value types + keyType := ref.Type().Key() + valueType := ref.Type().Elem() + + // Group environment variables by key + keyGroups := make(map[string][]string) + for _, env := range environments { + // Remove prefix and split by first underscore + key := strings.TrimPrefix(env, opts.Prefix) + parts := strings.SplitN(key, string(underscore), 2) + if len(parts) >= 2 { + mapKey := parts[0] + keyGroups[mapKey] = append(keyGroups[mapKey], env) + } + } + + // Process each key group + for mapKey := range keyGroups { + // Create a new value for this key + value := reflect.New(valueType).Elem() + + // Create options with the key prefix + keyOpts := Options{ + Environment: opts.Environment, + TagName: opts.TagName, + PrefixTagName: opts.PrefixTagName, + DefaultValueTagName: opts.DefaultValueTagName, + RequiredIfNoDef: opts.RequiredIfNoDef, + OnSet: opts.OnSet, + Prefix: fmt.Sprintf("%s%s_", opts.Prefix, mapKey), + UseFieldNameByDefault: opts.UseFieldNameByDefault, + SetDefaultsForZeroValuesOnly: opts.SetDefaultsForZeroValuesOnly, + FuncMap: opts.FuncMap, + rawEnvVars: opts.rawEnvVars, + } + + // Parse the value + if err := doParse(value, processField, keyOpts); err != nil { + return err + } + + // Convert the map key to the correct type + keyValue := reflect.ValueOf(mapKey) + if keyType.Kind() != reflect.String { + // Try to convert the key to the required type + keyValue = keyValue.Convert(keyType) + } + + // Set the value in the map + ref.SetMapIndex(keyValue, value) + } + } + + return nil +} + func setField(refField reflect.Value, refTypeField reflect.StructField, opts Options, fieldParams FieldParams) error { value, err := get(fieldParams, opts) if err != nil { diff --git a/env_test.go b/env_test.go index 9a348f8..194b59e 100644 --- a/env_test.go +++ b/env_test.go @@ -2411,3 +2411,27 @@ func TestEnvBleed(t *testing.T) { isEqual(t, "", cfg.Foo) }) } + +func TestComplexConfigWithMap(t *testing.T) { + type Test struct { + Str string `env:"STR"` + Num int `env:"NUM"` + } + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` + } + + t.Setenv("BAR_KEY1_STR", "b1t") + t.Setenv("BAR_KEY1_NUM", "201") + t.Setenv("BAR_KEY2_STR", "b2t") + t.Setenv("BAR_KEY2_NUM", "202") + + cfg := ComplexConfig{Bar: make(map[string]Test)} + + isNoErr(t, Parse(&cfg)) + + isEqual(t, "b1t", cfg.Bar["KEY1"].Str) + isEqual(t, 201, cfg.Bar["KEY1"].Num) + isEqual(t, "b2t", cfg.Bar["KEY2"].Str) + isEqual(t, 202, cfg.Bar["KEY2"].Num) +} From a76adb32ad80859fd7312b9e7a935254210f0bbc Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 30 May 2025 01:38:48 +0530 Subject: [PATCH 02/12] fix: changes --- env.go | 120 +++++++++++++++++++++++----------------------------- env_test.go | 51 ++++++++++++++-------- 2 files changed, 87 insertions(+), 84 deletions(-) diff --git a/env.go b/env.go index 2ba6f25..9c257bd 100644 --- a/env.go +++ b/env.go @@ -240,7 +240,7 @@ func customOptions(opts Options) Options { return defOpts } -func optionsWithSliceEnvPrefix(opts Options, index int) Options { +func optionsWithPrefix(opts Options, prefix string) Options { return Options{ Environment: opts.Environment, TagName: opts.TagName, @@ -248,7 +248,7 @@ func optionsWithSliceEnvPrefix(opts Options, index int) Options { DefaultValueTagName: opts.DefaultValueTagName, RequiredIfNoDef: opts.RequiredIfNoDef, OnSet: opts.OnSet, - Prefix: fmt.Sprintf("%s%d_", opts.Prefix, index), + Prefix: prefix, UseFieldNameByDefault: opts.UseFieldNameByDefault, SetDefaultsForZeroValuesOnly: opts.SetDefaultsForZeroValuesOnly, FuncMap: opts.FuncMap, @@ -256,20 +256,16 @@ func optionsWithSliceEnvPrefix(opts Options, index int) Options { } } +func optionsWithSliceEnvPrefix(opts Options, index int) Options { + return optionsWithPrefix(opts, fmt.Sprintf("%s%d_", opts.Prefix, index)) +} + +func optionsWithMapEnvPrefix(opts Options, mapKey string) Options { + return optionsWithPrefix(opts, fmt.Sprintf("%s%s_", opts.Prefix, mapKey)) +} + func optionsWithEnvPrefix(field reflect.StructField, opts Options) Options { - return Options{ - Environment: opts.Environment, - TagName: opts.TagName, - PrefixTagName: opts.PrefixTagName, - DefaultValueTagName: opts.DefaultValueTagName, - RequiredIfNoDef: opts.RequiredIfNoDef, - OnSet: opts.OnSet, - Prefix: opts.Prefix + field.Tag.Get(opts.PrefixTagName), - UseFieldNameByDefault: opts.UseFieldNameByDefault, - SetDefaultsForZeroValuesOnly: opts.SetDefaultsForZeroValuesOnly, - FuncMap: opts.FuncMap, - rawEnvVars: opts.rawEnvVars, - } + return optionsWithPrefix(opts, opts.Prefix+field.Tag.Get(opts.PrefixTagName)) } // Parse parses a struct containing `env` tags and loads its values from @@ -520,12 +516,10 @@ func doParseSlice(ref reflect.Value, processField processFieldFn, opts Options) } func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) error { - // Ensure prefix ends with underscore if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { opts.Prefix += string(underscore) } - // Collect all environment variables with the prefix var environments []string for environment := range opts.Environment { if strings.HasPrefix(environment, opts.Prefix) { @@ -533,63 +527,55 @@ func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) er } } - if len(environments) > 0 { - // Create a new map if it's nil - if ref.IsNil() { - ref.Set(reflect.MakeMap(ref.Type())) - } - - // Get the key and value types - keyType := ref.Type().Key() - valueType := ref.Type().Elem() - - // Group environment variables by key - keyGroups := make(map[string][]string) - for _, env := range environments { - // Remove prefix and split by first underscore - key := strings.TrimPrefix(env, opts.Prefix) - parts := strings.SplitN(key, string(underscore), 2) - if len(parts) >= 2 { - mapKey := parts[0] - keyGroups[mapKey] = append(keyGroups[mapKey], env) - } + // There are no map keys that match + if len(environments) == 0 { + return nil + } + + // Create a new map if it's nil + if ref.IsNil() { + ref.Set(reflect.MakeMap(ref.Type())) + } + + // Get the key and value types + keyType := ref.Type().Key() + valueType := ref.Type().Elem() + + keyGroups := make(map[string]bool) + for _, env := range environments { + key := strings.TrimPrefix(env, opts.Prefix) + parts := strings.SplitN(key, string(underscore), 2) + if len(parts) == 2 { + keyGroups[parts[0]] = true } + } - // Process each key group - for mapKey := range keyGroups { - // Create a new value for this key - value := reflect.New(valueType).Elem() - - // Create options with the key prefix - keyOpts := Options{ - Environment: opts.Environment, - TagName: opts.TagName, - PrefixTagName: opts.PrefixTagName, - DefaultValueTagName: opts.DefaultValueTagName, - RequiredIfNoDef: opts.RequiredIfNoDef, - OnSet: opts.OnSet, - Prefix: fmt.Sprintf("%s%s_", opts.Prefix, mapKey), - UseFieldNameByDefault: opts.UseFieldNameByDefault, - SetDefaultsForZeroValuesOnly: opts.SetDefaultsForZeroValuesOnly, - FuncMap: opts.FuncMap, - rawEnvVars: opts.rawEnvVars, - } + // Process each key group + for mapKey := range keyGroups { + value := reflect.New(valueType).Elem() + keyOpts := optionsWithMapEnvPrefix(opts, mapKey) - // Parse the value - if err := doParse(value, processField, keyOpts); err != nil { - return err - } + err := doParse(value, processField, keyOpts) + if err != nil { + return err + } - // Convert the map key to the correct type - keyValue := reflect.ValueOf(mapKey) - if keyType.Kind() != reflect.String { - // Try to convert the key to the required type - keyValue = keyValue.Convert(keyType) + keyValue := reflect.ValueOf(mapKey) + parserFunc, ok := opts.FuncMap[keyType] + if !ok { + kind := keyType.Kind() + if parserFunc, ok = defaultBuiltInParsers[kind]; !ok { + return fmt.Errorf("unsupported map key type: %s", kind.String()) } + } - // Set the value in the map - ref.SetMapIndex(keyValue, value) + parsedKey, err := parserFunc(mapKey) + if err != nil { + return fmt.Errorf("failed to parse map key %q: %w", mapKey, err) } + keyValue = reflect.ValueOf(parsedKey).Convert(keyType) + + ref.SetMapIndex(keyValue, value) } return nil diff --git a/env_test.go b/env_test.go index 194b59e..2cb4365 100644 --- a/env_test.go +++ b/env_test.go @@ -2413,25 +2413,42 @@ func TestEnvBleed(t *testing.T) { } func TestComplexConfigWithMap(t *testing.T) { - type Test struct { - Str string `env:"STR"` - Num int `env:"NUM"` - } - type ComplexConfig struct { - Bar map[string]Test `envPrefix:"BAR_"` - } + t.Run("Default with string key", func(t *testing.T) { - t.Setenv("BAR_KEY1_STR", "b1t") - t.Setenv("BAR_KEY1_NUM", "201") - t.Setenv("BAR_KEY2_STR", "b2t") - t.Setenv("BAR_KEY2_NUM", "202") + type Test struct { + Str string `env:"DAT_STR"` + Num int `env:"DAT_NUM"` + } + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` + } - cfg := ComplexConfig{Bar: make(map[string]Test)} + t.Setenv("BAR_KEY1_T_DAT_STR", "b1t") + t.Setenv("BAR_KEY1_T_DAT_NUM", "201") - isNoErr(t, Parse(&cfg)) + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) - isEqual(t, "b1t", cfg.Bar["KEY1"].Str) - isEqual(t, 201, cfg.Bar["KEY1"].Num) - isEqual(t, "b2t", cfg.Bar["KEY2"].Str) - isEqual(t, 202, cfg.Bar["KEY2"].Num) + isEqual(t, "b1t", cfg.Bar["KEY1_T"].Str) + }) + + t.Run("Default with float key", func(t *testing.T) { + type Test struct { + Str string `env:"STR"` + Num int `env:"NUM"` + } + type ComplexConfig struct { + Bar map[float64]Test `envPrefix:"BAR_"` + } + + t.Setenv("BAR_10.17_STR", "b1t") + t.Setenv("BAR_7.9_NUM", "201") + + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + + isEqual(t, "b1t", cfg.Bar[10.17].Str) + }) } From ee3c7da623e6ae770b4f418890fdd8960f894148 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Mon, 2 Jun 2025 16:00:51 +0530 Subject: [PATCH 03/12] fix: updates --- env.go | 248 ++++++++++++++++++++++++++++++++++++++-------------- env_test.go | 75 ++++++++++++++-- 2 files changed, 250 insertions(+), 73 deletions(-) diff --git a/env.go b/env.go index 9c257bd..24b9473 100644 --- a/env.go +++ b/env.go @@ -21,6 +21,7 @@ import ( "net/url" "os" "reflect" + "slices" "strconv" "strings" "time" @@ -444,8 +445,18 @@ func isMapOfStructs(refTypeField reflect.StructField) bool { field = field.Elem() } - if field.Kind() == reflect.Map && field.Elem().Kind() == reflect.Struct { - return true + if field.Kind() == reflect.Map { + kind := field.Elem().Kind() + if kind == reflect.Struct { + return true + } + + if kind == reflect.Ptr { + ptrField := field.Elem() + if ptrField.Elem().Kind() == reflect.Struct { + return true + } + } } return false @@ -515,72 +526,6 @@ func doParseSlice(ref reflect.Value, processField processFieldFn, opts Options) return nil } -func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) error { - if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { - opts.Prefix += string(underscore) - } - - var environments []string - for environment := range opts.Environment { - if strings.HasPrefix(environment, opts.Prefix) { - environments = append(environments, environment) - } - } - - // There are no map keys that match - if len(environments) == 0 { - return nil - } - - // Create a new map if it's nil - if ref.IsNil() { - ref.Set(reflect.MakeMap(ref.Type())) - } - - // Get the key and value types - keyType := ref.Type().Key() - valueType := ref.Type().Elem() - - keyGroups := make(map[string]bool) - for _, env := range environments { - key := strings.TrimPrefix(env, opts.Prefix) - parts := strings.SplitN(key, string(underscore), 2) - if len(parts) == 2 { - keyGroups[parts[0]] = true - } - } - - // Process each key group - for mapKey := range keyGroups { - value := reflect.New(valueType).Elem() - keyOpts := optionsWithMapEnvPrefix(opts, mapKey) - - err := doParse(value, processField, keyOpts) - if err != nil { - return err - } - - keyValue := reflect.ValueOf(mapKey) - parserFunc, ok := opts.FuncMap[keyType] - if !ok { - kind := keyType.Kind() - if parserFunc, ok = defaultBuiltInParsers[kind]; !ok { - return fmt.Errorf("unsupported map key type: %s", kind.String()) - } - } - - parsedKey, err := parserFunc(mapKey) - if err != nil { - return fmt.Errorf("failed to parse map key %q: %w", mapKey, err) - } - keyValue = reflect.ValueOf(parsedKey).Convert(keyType) - - ref.SetMapIndex(keyValue, value) - } - - return nil -} - func setField(refField reflect.Value, refTypeField reflect.StructField, opts Options, fieldParams FieldParams) error { value, err := get(fieldParams, opts) if err != nil { @@ -934,3 +879,170 @@ func ToMap(env []string) map[string]string { func isInvalidPtr(v reflect.Value) bool { return reflect.Ptr == v.Kind() && v.Elem().Kind() == reflect.Invalid } + +func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) error { + if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { + opts.Prefix += string(underscore) + } + + var environments []string + for environment := range opts.Environment { + if strings.HasPrefix(environment, opts.Prefix) { + environments = append(environments, environment) + } + } + + // There are no map keys that match + if len(environments) == 0 { + return nil + } + + // Create a new map if it's nil + if ref.IsNil() { + ref.Set(reflect.MakeMap(ref.Type())) + } + + // Get the key and value types + keyType := ref.Type().Key() + valueType := ref.Type().Elem() + + keyGroups := make(map[string]bool) + + environmentVars := getPossibleEnvVars(valueType, opts) + + // We identify the key by the prefix and the suffix + // If there are multiple matches the first match in alphabetical order + // will be chosen + for _, env := range environments { + key := strings.TrimPrefix(env, opts.Prefix) + + for _, envVar := range environmentVars { + if strings.HasSuffix(key, envVar) { + if key == envVar { + // If the key is exactly the envVar, this means that the env var was malformed + // TODO: Look at using the newParseError function + return fmt.Errorf(`malformed complex map struct for %q`, key) + } + newKey := strings.TrimSuffix(key, "_"+envVar) + keyGroups[newKey] = true + } + } + } + + // Process each key group + for mapKey := range keyGroups { + value := reflect.New(valueType).Elem() + keyOpts := optionsWithMapEnvPrefix(opts, mapKey) + + initialKind := value.Kind() + if initialKind == reflect.Ptr { + if value.IsNil() { + value.Set(reflect.New(valueType.Elem())) + } + value = value.Elem() + } + + err := doParse(value, processField, keyOpts) + if err != nil { + return err + } + + keyValue := reflect.ValueOf(mapKey) + parserFunc, ok := opts.FuncMap[keyType] + if !ok { + kind := keyType.Kind() + if parserFunc, ok = defaultBuiltInParsers[kind]; !ok { + return fmt.Errorf("unsupported map key type: %s", kind.String()) + } + } + + parsedKey, err := parserFunc(mapKey) + if err != nil { + return fmt.Errorf("failed to parse map key %q: %w", mapKey, err) + } + keyValue = reflect.ValueOf(parsedKey).Convert(keyType) + + if initialKind == reflect.Ptr { + valuePtr := reflect.New(valueType.Elem()) + valuePtr.Elem().Set(value) + value = valuePtr + } + + ref.SetMapIndex(keyValue, value) + } + + return nil +} + +// getPossibleEnvVars returns all possible environment variables that could be set for a given struct type. +func getPossibleEnvVars(v reflect.Type, opts Options) []string { + envVars := make(map[string]struct{}) + + if v.Kind() == reflect.Ptr { + v = v.Elem() + } + + traverseStruct(v, "", opts, envVars) + + // Convert map keys to slice and sort for deterministic order + result := make([]string, 0, len(envVars)) + for k := range envVars { + result = append(result, k) + } + slices.Sort(result) + + return result +} + +type EnvSuffixType struct { + useContains bool + value string +} + +// traverseStruct recursively traverses a struct type and collects all possible environment variables. +func traverseStruct(t reflect.Type, prefix string, opts Options, envVars map[string]struct{}) { + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + + // Get field prefix if exists + fieldPrefix := field.Tag.Get(opts.PrefixTagName) + if fieldPrefix != "" { + prefix = prefix + fieldPrefix + } + + // Get env tag if exists + envTag := field.Tag.Get(opts.TagName) + if envTag != "" { + envVars[prefix+envTag] = struct{}{} + } + + // Handle nested structs and maps of structs + fieldType := field.Type + if fieldType.Kind() == reflect.Ptr { + fieldType = fieldType.Elem() + } + + if fieldType.Kind() == reflect.Struct { + traverseStruct(fieldType, prefix, opts, envVars) + } + + if fieldType.Kind() == reflect.Map { + elemType := fieldType.Elem() + if elemType.Kind() == reflect.Ptr { + elemType = elemType.Elem() + } + if elemType.Kind() == reflect.Struct { + // If the type is a map of structs, we don't know what are the possible keys + // When we don't have nested map.Struct types we can easily determine the key + // But this mean that we can have two dynamic keys instead of one + // Let's say that the parent map key is MAP and the envPrefix for the inner map is + // also MAP, and user enters MAP for both keys + // BAR_ "MAP_MAP" _MAP_"STRING"_SUBGRAPHS : Map_Map is Key one and "String" is key 2 + // BAR_ "MAP" _MAP_"MAP"_FIELD : Map is the Key 1 and Map is the Key 2 + // BAR_ "MAP_MAP" _MAP_"MAP"_FIELD : Map_Map is the Key 1 and Map is the Key 2 + // If we look at the above example, we don't know how to dermine which is the prefix or which is the key then + + } + } + } +} diff --git a/env_test.go b/env_test.go index 2cb4365..14ca0ac 100644 --- a/env_test.go +++ b/env_test.go @@ -2413,24 +2413,64 @@ func TestEnvBleed(t *testing.T) { } func TestComplexConfigWithMap(t *testing.T) { + t.Run("Default with empty key", func(t *testing.T) { + type Test struct { + Str string `env:"DAT_STR"` + } + + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` + } + + t.Setenv("BAR_DAT_STR", "b1t") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + isErrorWithMessage(t, err, `env: malformed complex map struct for "DAT_STR"`) + }) + + t.Run("Default with struct without any env", func(t *testing.T) { + type Test struct { + Str string + } + + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` + } + + t.Setenv("BAR_E_DAT_STR", "b1t") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + isNoErr(t, err) + }) + t.Run("Default with string key", func(t *testing.T) { + type SubTest struct { + Str string `env:"NEW_STR"` + } type Test struct { - Str string `env:"DAT_STR"` - Num int `env:"DAT_NUM"` + Str string `env:"DAT_STR"` + Num int `env:"DAT_NUM"` + Sub SubTest `envPrefix:"SUB_"` } + type ComplexConfig struct { Bar map[string]Test `envPrefix:"BAR_"` } - t.Setenv("BAR_KEY1_T_DAT_STR", "b1t") - t.Setenv("BAR_KEY1_T_DAT_NUM", "201") + t.Setenv("BAR_KEY1_DAT_STR", "b1t") + t.Setenv("BAR_KEY1_DAT_NUM", "201") + t.Setenv("BAR_KEY1_SUB_NEW_STR", "sub_b1t") cfg := ComplexConfig{} isNoErr(t, Parse(&cfg)) - isEqual(t, "b1t", cfg.Bar["KEY1_T"].Str) + isEqual(t, "b1t", cfg.Bar["KEY1"].Str) }) t.Run("Default with float key", func(t *testing.T) { @@ -2451,4 +2491,29 @@ func TestComplexConfigWithMap(t *testing.T) { isEqual(t, "b1t", cfg.Bar[10.17].Str) }) + + t.Run("Default with string key", func(t *testing.T) { + type Employee struct { + Name string `env:"NAME"` + } + + type Sub struct { + Name string `env:"NAME"` + NewEmployee Employee `envPrefix:"EMP_"` + } + + type Test struct { + Str Sub `envPrefix:"SUB_"` + ReStr Sub `envPrefix:"SUB_"` + Num int `env:"DAT_NUM"` + } + + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` + } + + // 1. The type has to be a struct internally + // there is no way to figure out the type for this map[string]struct{} + + }) } From 1aaecefa618fa017d29cfc95d585c95aa2bb84b9 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 03:09:55 +0530 Subject: [PATCH 04/12] fix: refactoring --- env.go | 155 +++++++++++++++++++------------ env_test.go | 262 ++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 327 insertions(+), 90 deletions(-) diff --git a/env.go b/env.go index 24b9473..df9d8ff 100644 --- a/env.go +++ b/env.go @@ -438,30 +438,6 @@ func isSliceOfStructs(refTypeField reflect.StructField) bool { return false } -func isMapOfStructs(refTypeField reflect.StructField) bool { - field := refTypeField.Type - - if field.Kind() == reflect.Ptr { - field = field.Elem() - } - - if field.Kind() == reflect.Map { - kind := field.Elem().Kind() - if kind == reflect.Struct { - return true - } - - if kind == reflect.Ptr { - ptrField := field.Elem() - if ptrField.Elem().Kind() == reflect.Struct { - return true - } - } - } - - return false -} - func doParseSlice(ref reflect.Value, processField processFieldFn, opts Options) error { if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { opts.Prefix += string(underscore) @@ -880,6 +856,30 @@ func isInvalidPtr(v reflect.Value) bool { return reflect.Ptr == v.Kind() && v.Elem().Kind() == reflect.Invalid } +func isMapOfStructs(refTypeField reflect.StructField) bool { + field := refTypeField.Type + + if field.Kind() == reflect.Ptr { + field = field.Elem() + } + + if field.Kind() == reflect.Map { + kind := field.Elem().Kind() + if kind == reflect.Struct { + return true + } + + if kind == reflect.Ptr { + ptrField := field.Elem() + if ptrField.Elem().Kind() == reflect.Struct { + return true + } + } + } + + return false +} + func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) error { if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { opts.Prefix += string(underscore) @@ -908,25 +908,49 @@ func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) er keyGroups := make(map[string]bool) - environmentVars := getPossibleEnvVars(valueType, opts) + structInnerSubEnvVars := getPossibleEnvVars(valueType, opts) - // We identify the key by the prefix and the suffix - // If there are multiple matches the first match in alphabetical order - // will be chosen for _, env := range environments { + currKey := "" key := strings.TrimPrefix(env, opts.Prefix) - for _, envVar := range environmentVars { - if strings.HasSuffix(key, envVar) { - if key == envVar { - // If the key is exactly the envVar, this means that the env var was malformed + // A user can have multiple environment variables which match to multiple keys + // for example BAR_KEY_STR and BAR_KEY_NEW_STR are valid envars + // If the struct has both "STR" and "NEW_STR" this would mean that + // "STR" matches to both as a suffix and would result in two map keys + // KEY_NEW and KEY, thus we match the suffix that would give the smallest key + // since the smallest suffix that gives the largest key may have its own + // different environment variable + for _, innerEnvVar := range structInnerSubEnvVars { + // If we are using a map of a map (we don't use the absolute path value, instead we use the prefix value) + suffix := string(underscore) + innerEnvVar.value + if innerEnvVar.useContains { + idx := strings.LastIndex(key, suffix) + if idx != -1 { + newKey := key[:idx] + // We had a better match which trimmed the key further + if newKey != "" && (currKey == "" || len(currKey) > len(newKey)) { + currKey = newKey + } + } + } else if strings.HasSuffix(key, innerEnvVar.value) { + if key == innerEnvVar.value { + // If the key is exactly the innerEnvVar, this means that the env var was malformed // TODO: Look at using the newParseError function return fmt.Errorf(`malformed complex map struct for %q`, key) } - newKey := strings.TrimSuffix(key, "_"+envVar) - keyGroups[newKey] = true + newKey := strings.TrimSuffix(key, suffix) + // We had a better match which trimmed the key further + if newKey != "" && (currKey == "" || len(currKey) > len(newKey)) { + currKey = newKey + } } } + + // If a key match has been found + if currKey != "" { + keyGroups[currKey] = true + } } // Process each key group @@ -947,12 +971,11 @@ func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) er return err } - keyValue := reflect.ValueOf(mapKey) parserFunc, ok := opts.FuncMap[keyType] if !ok { kind := keyType.Kind() if parserFunc, ok = defaultBuiltInParsers[kind]; !ok { - return fmt.Errorf("unsupported map key type: %s", kind.String()) + return newNoParserError(sf) } } @@ -960,7 +983,8 @@ func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) er if err != nil { return fmt.Errorf("failed to parse map key %q: %w", mapKey, err) } - keyValue = reflect.ValueOf(parsedKey).Convert(keyType) + + keyValue := reflect.ValueOf(parsedKey).Convert(keyType) if initialKind == reflect.Ptr { valuePtr := reflect.New(valueType.Elem()) @@ -974,33 +998,52 @@ func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) er return nil } +type SuffixType struct { + useContains bool + value string +} + // getPossibleEnvVars returns all possible environment variables that could be set for a given struct type. -func getPossibleEnvVars(v reflect.Type, opts Options) []string { - envVars := make(map[string]struct{}) +func getPossibleEnvVars(v reflect.Type, opts Options) []SuffixType { + envVars := make(map[string]bool) if v.Kind() == reflect.Ptr { v = v.Elem() } + // The lib does not allow recursive structs technically + // Recursive structs need to have the parent reference type as Pointer, + // which means since pointer struct types do not get initialized by the parser by default, + // and only with the `env:,init` tag. However, when the `init` attribute is set + // the lib goes into an infinite loop because it does not support recursive structs + // Thus we do not handle recursive structs here traverseStruct(v, "", opts, envVars) // Convert map keys to slice and sort for deterministic order - result := make([]string, 0, len(envVars)) - for k := range envVars { - result = append(result, k) + result := make([]SuffixType, 0, len(envVars)) + for k, val := range envVars { + entry := SuffixType{ + value: k, + useContains: val, + } + result = append(result, entry) } - slices.Sort(result) - return result -} + slices.SortFunc(result, func(i, j SuffixType) int { + if i.useContains != j.useContains { + if i.useContains { + return 1 + } + return -1 + } + return strings.Compare(i.value, j.value) + }) -type EnvSuffixType struct { - useContains bool - value string + return result } // traverseStruct recursively traverses a struct type and collects all possible environment variables. -func traverseStruct(t reflect.Type, prefix string, opts Options, envVars map[string]struct{}) { +func traverseStruct(t reflect.Type, prefix string, opts Options, envVars map[string]bool) { for i := 0; i < t.NumField(); i++ { field := t.Field(i) @@ -1012,8 +1055,9 @@ func traverseStruct(t reflect.Type, prefix string, opts Options, envVars map[str // Get env tag if exists envTag := field.Tag.Get(opts.TagName) + key := prefix + envTag if envTag != "" { - envVars[prefix+envTag] = struct{}{} + envVars[key] = false } // Handle nested structs and maps of structs @@ -1032,16 +1076,7 @@ func traverseStruct(t reflect.Type, prefix string, opts Options, envVars map[str elemType = elemType.Elem() } if elemType.Kind() == reflect.Struct { - // If the type is a map of structs, we don't know what are the possible keys - // When we don't have nested map.Struct types we can easily determine the key - // But this mean that we can have two dynamic keys instead of one - // Let's say that the parent map key is MAP and the envPrefix for the inner map is - // also MAP, and user enters MAP for both keys - // BAR_ "MAP_MAP" _MAP_"STRING"_SUBGRAPHS : Map_Map is Key one and "String" is key 2 - // BAR_ "MAP" _MAP_"MAP"_FIELD : Map is the Key 1 and Map is the Key 2 - // BAR_ "MAP_MAP" _MAP_"MAP"_FIELD : Map_Map is the Key 1 and Map is the Key 2 - // If we look at the above example, we don't know how to dermine which is the prefix or which is the key then - + envVars[key] = true } } } diff --git a/env_test.go b/env_test.go index 14ca0ac..9746ef8 100644 --- a/env_test.go +++ b/env_test.go @@ -2413,7 +2413,8 @@ func TestEnvBleed(t *testing.T) { } func TestComplexConfigWithMap(t *testing.T) { - t.Run("Default with empty key", func(t *testing.T) { + + t.Run("Should not parse struct map with empty key", func(t *testing.T) { type Test struct { Str string `env:"DAT_STR"` } @@ -2430,7 +2431,7 @@ func TestComplexConfigWithMap(t *testing.T) { isErrorWithMessage(t, err, `env: malformed complex map struct for "DAT_STR"`) }) - t.Run("Default with struct without any env", func(t *testing.T) { + t.Run("Should parse map with struct without any matching env", func(t *testing.T) { type Test struct { Str string } @@ -2439,7 +2440,7 @@ func TestComplexConfigWithMap(t *testing.T) { Bar map[string]Test `envPrefix:"BAR_"` } - t.Setenv("BAR_E_DAT_STR", "b1t") + t.Setenv("BAR_E_TEST_DAT_STR", "b1t") cfg := ComplexConfig{} @@ -2447,7 +2448,7 @@ func TestComplexConfigWithMap(t *testing.T) { isNoErr(t, err) }) - t.Run("Default with string key", func(t *testing.T) { + t.Run("Should parse non pointer struct map with", func(t *testing.T) { type SubTest struct { Str string `env:"NEW_STR"` } @@ -2462,58 +2463,259 @@ func TestComplexConfigWithMap(t *testing.T) { Bar map[string]Test `envPrefix:"BAR_"` } - t.Setenv("BAR_KEY1_DAT_STR", "b1t") - t.Setenv("BAR_KEY1_DAT_NUM", "201") - t.Setenv("BAR_KEY1_SUB_NEW_STR", "sub_b1t") + t.Run("valid values", func(t *testing.T) { + t.Setenv("BAR_KEY1_TEST_DAT_STR", "b1t") + t.Setenv("BAR_KEY1_DAT_NUM", "201") + t.Setenv("BAR_KEY1_SUB_NEW_STR", "sub_b1t") - cfg := ComplexConfig{} + cfg := ComplexConfig{} - isNoErr(t, Parse(&cfg)) + isNoErr(t, Parse(&cfg)) + + isEqual(t, "b1t", cfg.Bar["KEY1_TEST"].Str) + isEqual(t, 201, cfg.Bar["KEY1"].Num) + isEqual(t, "sub_b1t", cfg.Bar["KEY1"].Sub.Str) + }) + + t.Run("no values", func(t *testing.T) { + cfg := ComplexConfig{} - isEqual(t, "b1t", cfg.Bar["KEY1"].Str) + isNoErr(t, Parse(&cfg)) + + isEqual(t, 0, len(cfg.Bar)) + isEqual(t, "", cfg.Bar["KEY1_TEST"].Str) + isEqual(t, 0, cfg.Bar["KEY1"].Num) + isEqual(t, "", cfg.Bar["KEY1"].Sub.Str) + }) }) - t.Run("Default with float key", func(t *testing.T) { + t.Run("Should parse pointer struct map with", func(t *testing.T) { + type SubTest struct { + Str string `env:"NEW_STR"` + } + type Test struct { - Str string `env:"STR"` - Num int `env:"NUM"` + Str string `env:"DAT_STR"` + Num int `env:"DAT_NUM"` + Sub SubTest `envPrefix:"SUB_"` } + type ComplexConfig struct { - Bar map[float64]Test `envPrefix:"BAR_"` + Bar map[string]*Test `envPrefix:"BAR_"` } - t.Setenv("BAR_10.17_STR", "b1t") - t.Setenv("BAR_7.9_NUM", "201") + t.Run("valid values", func(t *testing.T) { + t.Setenv("BAR_KEY1_TEST_DAT_STR", "b1t") + t.Setenv("BAR_KEY1_DAT_NUM", "201") + t.Setenv("BAR_KEY1_SUB_NEW_STR", "sub_b1t") - cfg := ComplexConfig{} + cfg := ComplexConfig{} - isNoErr(t, Parse(&cfg)) + isNoErr(t, Parse(&cfg)) - isEqual(t, "b1t", cfg.Bar[10.17].Str) + isEqual(t, "b1t", cfg.Bar["KEY1_TEST"].Str) + isEqual(t, 201, cfg.Bar["KEY1"].Num) + isEqual(t, "sub_b1t", cfg.Bar["KEY1"].Sub.Str) + }) + + t.Run("no values", func(t *testing.T) { + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + + isEqual(t, nil, cfg.Bar) + }) }) - t.Run("Default with string key", func(t *testing.T) { - type Employee struct { - Name string `env:"NAME"` + t.Run("Parse struct map nested in struct maps without any duplicate field names", func(t *testing.T) { + type SubTest struct { + Str string `env:"NEW_STR"` + } + + type Test struct { + Foo map[string]SubTest `envPrefix:"FOO_"` + } + + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` } - type Sub struct { - Name string `env:"NAME"` - NewEmployee Employee `envPrefix:"EMP_"` + t.Run("Should succeed with valid values", func(t *testing.T) { + t.Setenv("BAR_KEY1_FOO_KEY2_NEW_STR", "b1t") + + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + isEqual(t, "b1t", cfg.Bar["KEY1"].Foo["KEY2"].Str) + }) + + t.Run("Should succeed with envPrefix as key but nested map having unique key", func(t *testing.T) { + map1Key := "BAR_FOO_FOO_FOO_FOO_FOO_FOO" + map2Key := "KEY1" + t.Setenv(`BAR_`+map1Key+`_FOO_`+map2Key+`_NEW_STR`, "b1t") + + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + isEqual(t, "b1t", cfg.Bar[map1Key].Foo[map2Key].Str) + }) + + t.Run("Should fail where nested map prefix is key for for parent map and nested map keys", func(t *testing.T) { + map1Key := "FOO_FOO_FOO_FOO_FOO_FOO" + map2Key := "FOO" + t.Setenv(`BAR_`+map1Key+`_FOO_`+map2Key+`_NEW_STR`, "b1t") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + isErrorWithMessage(t, err, `env: malformed complex map struct for "NEW_STR"`) + }) + }) + + // We identify the key by the prefix and the suffix + // We go through all environments and try to find the best match + // For example a struct could look like + // type SubStruct struct { + // String string `env:"STR"` + // NewString string `env:"NEW_STR"` + // } + // Map[string]SubStruct `env:"BAR_"` + // which could have the following environment variable keys + // BAR_STR and BAR_NEW_STR + // we would have the suffixes STR and NEW_STR, thus when we are + // processing `BAR_NEW_STR` both suffixes will be a match + // thus we need to find the best match + t.Run("Verify matching of similar names", func(t *testing.T) { + type SubTest struct { + Str1 string `env:"STR"` } type Test struct { - Str Sub `envPrefix:"SUB_"` - ReStr Sub `envPrefix:"SUB_"` - Num int `env:"DAT_NUM"` + Str1 string `env:"NEW_STR"` + Str2 string `env:"STR"` + Str3 string `env:"NEW_STRING"` + Str4 string `env:"STR_NEWER"` + Str5 string `env:"NEW_STR_STR"` + + SubStr map[string]SubTest `envPrefix:"SUBBER_"` } type ComplexConfig struct { Bar map[string]Test `envPrefix:"BAR_"` } - // 1. The type has to be a struct internally - // there is no way to figure out the type for this map[string]struct{} + t.Run("partial environment variables", func(t *testing.T) { + cfg := ComplexConfig{} + + t.Setenv("BAR_FOO_NEW_STR", "a1") + t.Setenv("BAR_FOO_NEW_STRING", "a3") + isNoErr(t, Parse(&cfg)) + isEqual(t, "a1", cfg.Bar["FOO"].Str1) + isEqual(t, "", cfg.Bar["FOO"].Str2) + isEqual(t, "a3", cfg.Bar["FOO"].Str3) + isEqual(t, "", cfg.Bar["FOO"].Str4) + isEqual(t, "", cfg.Bar["FOO"].Str5) + isEqual(t, 0, len(cfg.Bar["FOO"].SubStr)) + }) + + t.Run("full environment variables", func(t *testing.T) { + cfg := ComplexConfig{} + + t.Setenv("BAR_FOO_NEW_STR", "a1") + t.Setenv("BAR_FOO_STR", "a2") + t.Setenv("BAR_FOO_NEW_STRING", "a3") + t.Setenv("BAR_FOO_STR_NEWER", "a4") + t.Setenv("BAR_FOO_NEW_STR_STR", "a5") + + isNoErr(t, Parse(&cfg)) + isEqual(t, "a1", cfg.Bar["FOO"].Str1) + isEqual(t, "a2", cfg.Bar["FOO"].Str2) + isEqual(t, "a3", cfg.Bar["FOO"].Str3) + isEqual(t, "a4", cfg.Bar["FOO"].Str4) + isEqual(t, "a5", cfg.Bar["FOO"].Str5) + isEqual(t, 0, len(cfg.Bar["FOO"].SubStr)) + }) + + t.Run("the map", func(t *testing.T) { + cfg := ComplexConfig{} + + t.Setenv("BAR_FOO_SUBBER_NEWER_STR", "a1") + + isNoErr(t, Parse(&cfg)) + isEqual(t, "a1", cfg.Bar["FOO"].SubStr["NEWER"].Str1) + }) + }) + + t.Run("Parse struct map nested in struct maps with duplicate field names", func(t *testing.T) { + type SubTest struct { + Str1 string `env:"STR"` + } + + type Test struct { + Str1 string `env:"NEW_STR"` + Foo map[string]SubTest `envPrefix:"FOO_"` + } + + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` + } + + cfg := ComplexConfig{} + + t.Setenv("BAR_FOO_FOO_THERE_STR", "a1") + t.Setenv("BAR_FOO_NEW_STR", "a2") + + isNoErr(t, Parse(&cfg)) + isEqual(t, "b1t", "ee") + }) + + t.Run("Parse struct map nested in struct maps with duplicate field names", func(t *testing.T) { + type SubTest struct { + Str string `env:"FOO_FOO"` + } + + type Test struct { + Foo map[string]SubTest `envPrefix:"FOO_"` + } + + type ComplexConfig struct { + Bar map[string]Test `envPrefix:"BAR_"` + } + + map1Key := "FOO_FOO_FOO_FOO_FOO_FOO" + map2Key := "FOO" + t.Setenv(`BAR_`+map1Key+`_FOO_`+map2Key+`_FOO_FOO`, "b1t") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + fmt.Println(err) + + invalidAssumedKey := map1Key + "_FOO_FOO" + result, ok := cfg.Bar[invalidAssumedKey] + isTrue(t, ok) + // The map would be initialized but no values set + isEqual(t, 0, len(result.Foo)) + }) + + t.Run("Should parse struct map with float key", func(t *testing.T) { + type Test struct { + Str string `env:"STR"` + Num int `env:"NUM"` + } + type ComplexConfig struct { + Bar map[float64]Test `envPrefix:"BAR_"` + } + + t.Setenv("BAR_10.17_STR", "b1t") + t.Setenv("BAR_7.9_NUM", "201") + + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + + isEqual(t, "b1t", cfg.Bar[10.17].Str) }) + } From 4252f76c49aabfcaa8fb8611e10a675407028902 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 03:41:26 +0530 Subject: [PATCH 05/12] fix: refactoring --- env.go | 9 ++++----- env_test.go | 43 +++---------------------------------------- 2 files changed, 7 insertions(+), 45 deletions(-) diff --git a/env.go b/env.go index df9d8ff..aeb816a 100644 --- a/env.go +++ b/env.go @@ -413,7 +413,7 @@ func doParseField( } if isMapOfStructs(refTypeField) { - return doParseMap(refField, processField, optionsWithEnvPrefix(refTypeField, opts)) + return doParseMap(refField, processField, optionsWithEnvPrefix(refTypeField, opts), refTypeField) } return nil @@ -880,7 +880,7 @@ func isMapOfStructs(refTypeField reflect.StructField) bool { return false } -func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) error { +func doParseMap(ref reflect.Value, processField processFieldFn, opts Options, sf reflect.StructField) error { if opts.Prefix != "" && !strings.HasSuffix(opts.Prefix, string(underscore)) { opts.Prefix += string(underscore) } @@ -936,8 +936,7 @@ func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) er } else if strings.HasSuffix(key, innerEnvVar.value) { if key == innerEnvVar.value { // If the key is exactly the innerEnvVar, this means that the env var was malformed - // TODO: Look at using the newParseError function - return fmt.Errorf(`malformed complex map struct for %q`, key) + return newParseError(sf, fmt.Errorf(`malformed complex map struct for %q`, key)) } newKey := strings.TrimSuffix(key, suffix) // We had a better match which trimmed the key further @@ -981,7 +980,7 @@ func doParseMap(ref reflect.Value, processField processFieldFn, opts Options) er parsedKey, err := parserFunc(mapKey) if err != nil { - return fmt.Errorf("failed to parse map key %q: %w", mapKey, err) + return newParseError(sf, fmt.Errorf("failed to parse map key %q: %w", mapKey, err)) } keyValue := reflect.ValueOf(parsedKey).Convert(keyType) diff --git a/env_test.go b/env_test.go index 9746ef8..93c3426 100644 --- a/env_test.go +++ b/env_test.go @@ -2413,7 +2413,6 @@ func TestEnvBleed(t *testing.T) { } func TestComplexConfigWithMap(t *testing.T) { - t.Run("Should not parse struct map with empty key", func(t *testing.T) { type Test struct { Str string `env:"DAT_STR"` @@ -2428,7 +2427,7 @@ func TestComplexConfigWithMap(t *testing.T) { cfg := ComplexConfig{} err := Parse(&cfg) - isErrorWithMessage(t, err, `env: malformed complex map struct for "DAT_STR"`) + isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Test\": malformed complex map struct for \"DAT_STR\"") }) t.Run("Should parse map with struct without any matching env", func(t *testing.T) { @@ -2568,23 +2567,10 @@ func TestComplexConfigWithMap(t *testing.T) { cfg := ComplexConfig{} err := Parse(&cfg) - isErrorWithMessage(t, err, `env: malformed complex map struct for "NEW_STR"`) + isErrorWithMessage(t, err, "env: parse error on field \"Foo\" of type \"map[string]env.SubTest\": malformed complex map struct for \"NEW_STR\"") }) }) - // We identify the key by the prefix and the suffix - // We go through all environments and try to find the best match - // For example a struct could look like - // type SubStruct struct { - // String string `env:"STR"` - // NewString string `env:"NEW_STR"` - // } - // Map[string]SubStruct `env:"BAR_"` - // which could have the following environment variable keys - // BAR_STR and BAR_NEW_STR - // we would have the suffixes STR and NEW_STR, thus when we are - // processing `BAR_NEW_STR` both suffixes will be a match - // thus we need to find the best match t.Run("Verify matching of similar names", func(t *testing.T) { type SubTest struct { Str1 string `env:"STR"` @@ -2647,29 +2633,6 @@ func TestComplexConfigWithMap(t *testing.T) { }) }) - t.Run("Parse struct map nested in struct maps with duplicate field names", func(t *testing.T) { - type SubTest struct { - Str1 string `env:"STR"` - } - - type Test struct { - Str1 string `env:"NEW_STR"` - Foo map[string]SubTest `envPrefix:"FOO_"` - } - - type ComplexConfig struct { - Bar map[string]Test `envPrefix:"BAR_"` - } - - cfg := ComplexConfig{} - - t.Setenv("BAR_FOO_FOO_THERE_STR", "a1") - t.Setenv("BAR_FOO_NEW_STR", "a2") - - isNoErr(t, Parse(&cfg)) - isEqual(t, "b1t", "ee") - }) - t.Run("Parse struct map nested in struct maps with duplicate field names", func(t *testing.T) { type SubTest struct { Str string `env:"FOO_FOO"` @@ -2690,7 +2653,7 @@ func TestComplexConfigWithMap(t *testing.T) { cfg := ComplexConfig{} err := Parse(&cfg) - fmt.Println(err) + isNoErr(t, err) invalidAssumedKey := map1Key + "_FOO_FOO" result, ok := cfg.Bar[invalidAssumedKey] From a342cd542a4a418b602d1725d0d02ce509d0e200 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 04:06:46 +0530 Subject: [PATCH 06/12] fix: readme --- README.md | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/README.md b/README.md index de6f824..f569e46 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,71 @@ Pointers, slices and slices of pointers, and maps of those types are also suppor You may also add custom parsers for your types. +### Supported Complex Types + +#### Slices of structs +Use the `envPrefix` tag to define slices of structs. For example: +```go +type Server struct { + Host string `env:"HOST"` + Port int `env:"PORT"` +} +type Config struct { + Servers []Server `envPrefix:"SERVER_"` +} +``` + +Set environment variables with zero-based indices: +``` +SERVER_0_HOST=localhost +SERVER_0_PORT=8080 +SERVER_1_HOST=example.com +SERVER_1_PORT=80 +``` + +#### Maps of structs +Use the `envPrefix` tag to define maps of structs. The library automatically handles the mapping: +```go +type Server struct { + Host string `env:"HOST"` + Port int `env:"PORT"` +} +type Config struct { + Servers map[string]Server `envPrefix:"SERVER_"` +} +``` + +Set environment variables using your chosen keys: +```bash +export SERVER_FOO_HOST=localhost +export SERVER_FOO_PORT=8080 +``` + +> [!IMPORTANT] +> For nested maps, be careful with key naming to avoid conflicts. + +Example of nested maps: +```go +type Sub struct { + Value string `env:"VALUE"` +} +type Server struct { + Sub map[string]Sub `envPrefix:"SERVER_"` +} +type Config struct { + Entries map[string]Server `envPrefix:"ENTRIES_"` +} +``` + +The library uses the rightmost `envPrefix` match as the key. For example: +```bash +# This won't work - VALUE becomes a key instead of a field +export ENTRIES_FOO_SERVER_SERVER_SERVER_VALUE=foo + +# This works - KEY1 is the map key +export ENTRIES_FOO_SERVER_SERVER_SERVER_KEY1_VALUE=foo +``` + ### Tags The following tags are provided: From 8b2c943ed162331f48cb7aeda6a0d10201dd851f Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 14:19:26 +0530 Subject: [PATCH 07/12] fix: tag bug and added more tests --- env.go | 7 ++++--- env_test.go | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/env.go b/env.go index aeb816a..b213ff2 100644 --- a/env.go +++ b/env.go @@ -1052,10 +1052,11 @@ func traverseStruct(t reflect.Type, prefix string, opts Options, envVars map[str prefix = prefix + fieldPrefix } + ownKey, _ := parseKeyForOption(field.Tag.Get(opts.TagName)) + // Get env tag if exists - envTag := field.Tag.Get(opts.TagName) - key := prefix + envTag - if envTag != "" { + key := prefix + ownKey + if ownKey != "" { envVars[key] = false } diff --git a/env_test.go b/env_test.go index 93c3426..9f0b64b 100644 --- a/env_test.go +++ b/env_test.go @@ -2681,4 +2681,64 @@ func TestComplexConfigWithMap(t *testing.T) { isEqual(t, "b1t", cfg.Bar[10.17].Str) }) + t.Run("Should parse struct map with required key", func(t *testing.T) { + type Test struct { + Str string `env:"STR,required"` + Num int `env:"NUM"` + } + type ComplexConfig struct { + Bar map[int64]Test `envPrefix:"BAR_"` + } + + t.Run("Where required is present", func(t *testing.T) { + t.Setenv("BAR_7_STR", "b1t") + t.Setenv("BAR_7_NUM", "201") + t.Setenv("BAR_9_STR", "b1t") + + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + isEqual(t, "b1t", cfg.Bar[7].Str) + isEqual(t, 201, cfg.Bar[7].Num) + isEqual(t, "b1t", cfg.Bar[9].Str) + }) + + t.Run("Where required is missing", func(t *testing.T) { + t.Setenv("BAR_7_STR", "b1t") + t.Setenv("BAR_7_NUM", "201") + t.Setenv("BAR_9_NUM", "7") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + isErrorWithMessage(t, err, "env: required environment variable \"BAR_9_STR\" is not set") + }) + }) + + t.Run("Should parse we map with init and envDefault", func(t *testing.T) { + type SubStruct struct { + Str string `env:"STR"` + } + + type Test struct { + Str string `env:"STR,required"` + Num int `envDefault:"17" env:"NUM"` + Sub *SubStruct `env:",init" envPrefix:"SUB_"` + } + + type ComplexConfig struct { + Bar map[int64]Test `envPrefix:"BAR_"` + } + + t.Setenv("BAR_7_STR", "b1t") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + isNoErr(t, err) + + isFalse(t, cfg.Bar[7].Sub == nil) + isEqual(t, 17, cfg.Bar[7].Num) + }) + } From 126b18ad0e035e70a2ef99f86ce29d3146f99b35 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 16:58:54 +0530 Subject: [PATCH 08/12] fix: add examples --- env.go | 27 +++++++++----- env_test.go | 96 +++++++++++++++++++++++++++++++++++++++++++++++-- example_test.go | 36 +++++++++++++++++++ 3 files changed, 148 insertions(+), 11 deletions(-) diff --git a/env.go b/env.go index b213ff2..bc6322d 100644 --- a/env.go +++ b/env.go @@ -412,7 +412,10 @@ func doParseField( return doParseSlice(refField, processField, optionsWithEnvPrefix(refTypeField, opts)) } - if isMapOfStructs(refTypeField) { + isValidMapOfStructs, err := isMapOfStructs(refTypeField, opts) + if err != nil { + return err + } else if isValidMapOfStructs { return doParseMap(refField, processField, optionsWithEnvPrefix(refTypeField, opts), refTypeField) } @@ -856,7 +859,7 @@ func isInvalidPtr(v reflect.Value) bool { return reflect.Ptr == v.Kind() && v.Elem().Kind() == reflect.Invalid } -func isMapOfStructs(refTypeField reflect.StructField) bool { +func isMapOfStructs(refTypeField reflect.StructField, opts Options) (bool, error) { field := refTypeField.Type if field.Kind() == reflect.Ptr { @@ -865,19 +868,25 @@ func isMapOfStructs(refTypeField reflect.StructField) bool { if field.Kind() == reflect.Map { kind := field.Elem().Kind() - if kind == reflect.Struct { - return true - } - if kind == reflect.Ptr { ptrField := field.Elem() - if ptrField.Elem().Kind() == reflect.Struct { - return true + kind = ptrField.Elem().Kind() + } + + if kind == reflect.Struct { + val, _ := parseKeyForOption(refTypeField.Tag.Get(opts.TagName)) + if val != "" { + return false, newParseError(refTypeField, fmt.Errorf(`env key unsupported for struct map %q`, refTypeField.Name)) + } + // Only process if the env prefix tag is present + // This avoids the lib trying to set keys for a map without any prefix given + if refTypeField.Tag.Get(opts.PrefixTagName) != "" { + return true, nil } } } - return false + return false, nil } func doParseMap(ref reflect.Value, processField processFieldFn, opts Options, sf reflect.StructField) error { diff --git a/env_test.go b/env_test.go index 9f0b64b..47ab215 100644 --- a/env_test.go +++ b/env_test.go @@ -2163,6 +2163,36 @@ func TestIssue298ErrorNestedFieldRequiredNotSet(t *testing.T) { isTrue(t, errors.Is(err, VarIsNotSetError{})) } +func TestIssue298VerifyPrefixUsingEnvTagIsNotSupported(t *testing.T) { + type Test struct { + Str string `env:"STR"` + Num int `env:"NUM"` + } + type ComplexConfig struct { + Foo *[]Test `env:"FOO_"` + Bar []Test `env:"BAR"` + Baz []Test `env:",init"` + } + + t.Setenv("FOO_0_STR", "f0t") + t.Setenv("FOO_0_NUM", "101") + t.Setenv("FOO_1_STR", "f1t") + t.Setenv("FOO_1_NUM", "111") + + t.Setenv("BAR_0_STR", "b0t") + t.Setenv("BAR_0_NUM", "127") + t.Setenv("BAR_1_STR", "b1t") + t.Setenv("BAR_1_NUM", "212") + + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + + isEqual(t, nil, cfg.Foo) + isEqual(t, nil, cfg.Baz) + isEqual(t, nil, cfg.Bar) +} + func TestIssue320(t *testing.T) { type Test struct { Str string `env:"STR"` @@ -2427,7 +2457,7 @@ func TestComplexConfigWithMap(t *testing.T) { cfg := ComplexConfig{} err := Parse(&cfg) - isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Test\": malformed complex map struct for \"DAT_STR\"") + isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Host\": malformed complex map struct for \"DAT_STR\"") }) t.Run("Should parse map with struct without any matching env", func(t *testing.T) { @@ -2488,6 +2518,68 @@ func TestComplexConfigWithMap(t *testing.T) { }) }) + t.Run("Should skip processing map when no tags are present on the map", func(t *testing.T) { + type SubTest struct { + Str string `env:"NEW_STR"` + } + + type Test struct { + Str string `env:"DAT_STR"` + Num int `env:"DAT_NUM"` + Sub SubTest `envPrefix:"SUB_"` + } + + type ComplexConfig struct { + Bar map[string]Test + } + + t.Setenv("KEY1_TEST_DAT_STR", "b1t") + t.Setenv("KEY1_DAT_NUM", "201") + t.Setenv("KEY1_SUB_NEW_STR", "sub_b1t") + + cfg := ComplexConfig{} + + isNoErr(t, Parse(&cfg)) + + isEqual(t, nil, cfg.Bar) + }) + + t.Run("Should not allow an env tag with a value to be set on the map", func(t *testing.T) { + type Test struct { + Str string `env:"DAT_STR"` + } + + type ComplexConfig struct { + Bar map[string]Test `env:"VALUE,init"` + } + + t.Setenv("KEY1_DAT_STR", "b1t") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + isEqual(t, nil, cfg.Bar) + isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Host\": env key unsupported for struct map \"Bar\"") + }) + + t.Run("Should allow an env tag without a value to be set on the map", func(t *testing.T) { + type Test struct { + Str string `env:"DAT_STR"` + } + + type ComplexConfig struct { + Bar map[string]Test `env:",init"` + } + + t.Setenv("KEY1_DAT_STR", "b1t") + + cfg := ComplexConfig{} + + err := Parse(&cfg) + isNoErr(t, err) + isEqual(t, nil, cfg.Bar) + }) + t.Run("Should parse pointer struct map with", func(t *testing.T) { type SubTest struct { Str string `env:"NEW_STR"` @@ -2715,7 +2807,7 @@ func TestComplexConfigWithMap(t *testing.T) { }) }) - t.Run("Should parse we map with init and envDefault", func(t *testing.T) { + t.Run("Should parse struct map which contains fields with init and envDefault", func(t *testing.T) { type SubStruct struct { Str string `env:"STR"` } diff --git a/example_test.go b/example_test.go index 2562a83..94ed430 100644 --- a/example_test.go +++ b/example_test.go @@ -480,3 +480,39 @@ func Example_setDefaultsForZeroValuesOnly() { // Without SetDefaultsForZeroValuesOnly, the username would have been 'admin'. // Output: {Username:root Password:qwerty} } + +func Example_useComplexStructMaps() { + type Service struct { + Username string `env:"USERNAME"` + } + + type Host struct { + Address string `env:"ADDRESS"` + Port int `env:"PORT"` + // For nested maps, be careful with key naming to avoid conflicts. + // The library uses the rightmost `envPrefix` match as the key. For example: + // This means that won't work - USERNAME becomes a key instead of a field + // export HOST_aws_server_SERVICE_SERVICE_USERNAME=Jakinson + // Thus you must make sure to name the keys appropriately correctly + Services map[string]Service `envPrefix:"SERVICE_"` + } + + type Config struct { + // In the case of struct of slices, you can use it without a prefix + // however if a prefix is not specified processing this map will be skipped + Hosts map[string]Host `envPrefix:"HOST_"` + } + + os.Setenv("HOST_aws_server_ADDRESS", "127.0.0.1") + os.Setenv("HOST_aws_server_PORT", "79") + os.Setenv("HOST_aws_server_SERVICE_Ec2_USERNAME", "Jakinson") + os.Setenv("HOST_aws_server_SERVICE_cloudfront_USERNAME", "Arnold") + + cfg := Config{} + + if err := Parse(&cfg); err != nil { + fmt.Println(err) + } + + fmt.Println(cfg) +} From 06e0e49a875e6addfab057bd64825b3ab636e654 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 16:58:59 +0530 Subject: [PATCH 09/12] fix: update readme --- README.md | 70 +++++-------------------------------------------------- 1 file changed, 6 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index f569e46..731b603 100644 --- a/README.md +++ b/README.md @@ -88,70 +88,12 @@ Pointers, slices and slices of pointers, and maps of those types are also suppor You may also add custom parsers for your types. -### Supported Complex Types - -#### Slices of structs -Use the `envPrefix` tag to define slices of structs. For example: -```go -type Server struct { - Host string `env:"HOST"` - Port int `env:"PORT"` -} -type Config struct { - Servers []Server `envPrefix:"SERVER_"` -} -``` - -Set environment variables with zero-based indices: -``` -SERVER_0_HOST=localhost -SERVER_0_PORT=8080 -SERVER_1_HOST=example.com -SERVER_1_PORT=80 -``` - -#### Maps of structs -Use the `envPrefix` tag to define maps of structs. The library automatically handles the mapping: -```go -type Server struct { - Host string `env:"HOST"` - Port int `env:"PORT"` -} -type Config struct { - Servers map[string]Server `envPrefix:"SERVER_"` -} -``` - -Set environment variables using your chosen keys: -```bash -export SERVER_FOO_HOST=localhost -export SERVER_FOO_PORT=8080 -``` +Additionally, the following are also supported +- Slices of Structs +- Map of Structs > [!IMPORTANT] -> For nested maps, be careful with key naming to avoid conflicts. - -Example of nested maps: -```go -type Sub struct { - Value string `env:"VALUE"` -} -type Server struct { - Sub map[string]Sub `envPrefix:"SERVER_"` -} -type Config struct { - Entries map[string]Server `envPrefix:"ENTRIES_"` -} -``` - -The library uses the rightmost `envPrefix` match as the key. For example: -```bash -# This won't work - VALUE becomes a key instead of a field -export ENTRIES_FOO_SERVER_SERVER_SERVER_VALUE=foo - -# This works - KEY1 is the map key -export ENTRIES_FOO_SERVER_SERVER_SERVER_KEY1_VALUE=foo -``` +> For nested maps (i.e. map of structs inside map of structs), be careful with key naming to avoid conflicts. ### Tags @@ -160,8 +102,8 @@ The following tags are provided: - `env`: sets the environment variable name and optionally takes the tag options described below - `envDefault`: sets the default value for the field - `envPrefix`: can be used in a field that is a complex type to set a prefix to all environment variables used in it -- `envSeparator`: sets the character to be used to separate items in slices and maps (default: `,`) -- `envKeyValSeparator`: sets the character to be used to separate keys and their values in maps (default: `:`) +- `envSeparator`: sets the character to be used to separate items in slices and maps (which do not have structs as the value type) (default: `,`) +- `envKeyValSeparator`: sets the character to be used to separate keys and their values in maps (which do not have structs as the value type) (default: `:`) ### `env` tag options From f225e7d8a337b8d0a670e5d756dc3ed9a362ac05 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 17:04:44 +0530 Subject: [PATCH 10/12] fix: tests --- env_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env_test.go b/env_test.go index 47ab215..c4c13f5 100644 --- a/env_test.go +++ b/env_test.go @@ -2559,7 +2559,7 @@ func TestComplexConfigWithMap(t *testing.T) { err := Parse(&cfg) isEqual(t, nil, cfg.Bar) - isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Host\": env key unsupported for struct map \"Bar\"") + isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Test\": malformed complex map struct for \"DAT_STR\"") }) t.Run("Should allow an env tag without a value to be set on the map", func(t *testing.T) { From 54953fb222cea19130b8775d0722e43c36117d24 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 17:06:12 +0530 Subject: [PATCH 11/12] fix: tests --- env_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/env_test.go b/env_test.go index c4c13f5..c94d030 100644 --- a/env_test.go +++ b/env_test.go @@ -2553,13 +2553,11 @@ func TestComplexConfigWithMap(t *testing.T) { Bar map[string]Test `env:"VALUE,init"` } - t.Setenv("KEY1_DAT_STR", "b1t") - cfg := ComplexConfig{} err := Parse(&cfg) isEqual(t, nil, cfg.Bar) - isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Test\": malformed complex map struct for \"DAT_STR\"") + isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Test\": env key unsupported for struct map \"Bar\"") }) t.Run("Should allow an env tag without a value to be set on the map", func(t *testing.T) { From 486f24cb14baca38467205991ccf153ce23a0ec0 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 3 Jun 2025 17:06:52 +0530 Subject: [PATCH 12/12] fix: tests --- env_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env_test.go b/env_test.go index c94d030..7db027b 100644 --- a/env_test.go +++ b/env_test.go @@ -2457,7 +2457,7 @@ func TestComplexConfigWithMap(t *testing.T) { cfg := ComplexConfig{} err := Parse(&cfg) - isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Host\": malformed complex map struct for \"DAT_STR\"") + isErrorWithMessage(t, err, "env: parse error on field \"Bar\" of type \"map[string]env.Test\": malformed complex map struct for \"DAT_STR\"") }) t.Run("Should parse map with struct without any matching env", func(t *testing.T) {