From 5a8b6b15e76b53172a637bf0b62f1b9becaa97a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 11 Apr 2024 15:48:16 +0200 Subject: [PATCH 1/9] add support for maps to DecodeInto --- config/config.go | 52 +++++++++++++++++++++++++++++++++++++++++++ config/config_test.go | 31 ++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/config/config.go b/config/config.go index ec9789f..b596312 100644 --- a/config/config.go +++ b/config/config.go @@ -160,6 +160,8 @@ func (c Config) DecodeInto(target any, hookFunc ...mapstructure.DecodeHookFunc) DecodeHook: mapstructure.ComposeDecodeHookFunc( append( hookFunc, + mapStringHookFunc(), + mapStructHookFunc(), emptyStringToZeroValueHookFunc(), mapstructure.StringToTimeDurationHookFunc(), mapstructure.StringToSliceHookFunc(","), @@ -215,3 +217,53 @@ func emptyStringToZeroValueHookFunc() mapstructure.DecodeHookFunc { return reflect.New(t).Elem().Interface(), nil } } + +func mapStringHookFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.Map || f.Elem().Kind() != reflect.Interface || + t.Kind() != reflect.Map || t.Elem().Kind() != reflect.String { + return data, nil + } + + // no need to assert, we know it's a map[string]any + dataMap := data.(map[string]any) + + // remove all keys with maps + for k, v := range dataMap { + if reflect.TypeOf(v).Kind() == reflect.Map { + delete(dataMap, k) + } + } + + return dataMap, nil + } +} + +func mapStructHookFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.Map || f.Elem().Kind() != reflect.Interface || + t.Kind() != reflect.Map || t.Elem().Kind() != reflect.Struct { + return data, nil + } + + // no need to assert, we know it's a map[string]any + dataMap := data.(map[string]any) + + // remove all keys with a dot that contains a value with a string + for k, v := range dataMap { + _, isString := v.(string) + if !isString || !strings.Contains(k, ".") { + continue + } + delete(dataMap, k) + } + + return dataMap, nil + } +} diff --git a/config/config_test.go b/config/config_test.go index 217974a..def0366 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -429,6 +429,10 @@ func TestParseConfig_Embedded_Struct(t *testing.T) { func TestParseConfig_All_Types(t *testing.T) { is := is.New(t) + type structMapVal struct { + MyString string + MyInt int + } type testCfg struct { MyString string MyBool1 bool @@ -459,6 +463,12 @@ func TestParseConfig_All_Types(t *testing.T) { MySlice []string MyIntSlice []int MyFloatSlice []float32 + + Nested struct { + MyString string + } + StringMap map[string]string + StructMap map[string]structMapVal } input := Config{ @@ -488,6 +498,17 @@ func TestParseConfig_All_Types(t *testing.T) { "myslice": "1,2,3,4", "myIntSlice": "1,2,3,4", "myFloatSlice": "1.1,2.2", + + "nested.mystring": "string", + + "stringmap.foo": "1", + "stringmap.bar": "2", + "stringmap.baz.qux": "3", + + "structmap.foo.mystring": "foo-name", + "structmap.foo.myint": "1", + "structmap.bar.mystring": "bar-name", + "structmap.bar.myint": "-1", } want := testCfg{ MyString: "string", @@ -514,6 +535,16 @@ func TestParseConfig_All_Types(t *testing.T) { MySlice: []string{"1", "2", "3", "4"}, MyIntSlice: []int{1, 2, 3, 4}, MyFloatSlice: []float32{1.1, 2.2}, + Nested: struct{ MyString string }{MyString: "string"}, + StringMap: map[string]string{ + "foo": "1", + "bar": "2", + "baz.qux": "3", + }, + StructMap: map[string]structMapVal{ + "foo": {MyString: "foo-name", MyInt: 1}, + "bar": {MyString: "bar-name", MyInt: -1}, + }, } var result testCfg From ff89be19073cc7a3f2f09df3ca1e59fd95f58d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 11 Apr 2024 16:39:07 +0200 Subject: [PATCH 2/9] get key for dynamic parameter --- config/config.go | 60 +++++++++++++++++++++++++-- config/config_test.go | 96 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index b596312..5e8cf30 100644 --- a/config/config.go +++ b/config/config.go @@ -49,7 +49,7 @@ func (c Config) Sanitize() Config { // configuration, the default value is applied. func (c Config) ApplyDefaults(params Parameters) Config { for key, param := range params { - if strings.TrimSpace(c[key]) == "" { + if strings.TrimSpace(c[key]) == "" { // TODO get value for dynamic key c[key] = param.Default } } @@ -92,9 +92,9 @@ func (c Config) validateUnrecognizedParameters(params Parameters) []error { // validateParamType validates that a parameter value is parsable to its assigned type. func (c Config) validateParamType(key string, param Parameter) error { - value := c[key] + value := c[key] // TODO get value for dynamic key // empty value is valid for all types - if c[key] == "" { + if value == "" { return nil } @@ -127,7 +127,7 @@ func (c Config) validateParamType(key string, param Parameter) error { // validateParamValue validates that a configuration value matches all the // validations required for the parameter. func (c Config) validateParamValue(key string, param Parameter) error { - value := c[key] + value := c[key] // TODO get value for dynamic key var errs []error isRequired := false @@ -148,6 +148,58 @@ func (c Config) validateParamValue(key string, param Parameter) error { return errors.Join(errs...) } +func (c Config) getKeysForParameter(key string) []string { + // First break up the key into tokens. + tokens := strings.Split(key, "*") + if len(tokens) == 1 { + // No wildcard in the key, return the key directly. + return []string{key} + } + + consume := func(s, prefix string) (string, bool) { + if !strings.HasPrefix(s, prefix) { + // The key does not start with the token, it does not match the pattern. + return "", false + } + return strings.TrimPrefix(s, prefix), true + } + + // There is at least one wildcard in the key, we need to manually find all + // the keys that match the pattern. + var keys []string + for k := range c { + fullKey := k + for i, token := range tokens { + var ok bool + k, ok = consume(k, token) + if !ok { + // The key does not start with the token, it does not match the pattern. + break + } + if i == len(tokens)-1 { + if k != "" && token != "" { + // The key is not fully consumed and last token is not a wildcard, + // it does not match the pattern. + break + } + // We checked all tokens and the key matches the pattern. + keys = append(keys, fullKey) + break + } + + // Between tokens there is a wildcard, we need to strip the key until + // the next ".". + _, k, ok = strings.Cut(k, ".") + if !ok { + // The key does not have a "." after the token, it does not match the pattern. + break + } + k = "." + k // Add the "." back to the key. + } + } + return keys +} + // DecodeInto copies configuration values into the target object. // Under the hood, this function uses github.com/mitchellh/mapstructure, with // the "mapstructure" tag renamed to "json". To rename a key, use the "json" diff --git a/config/config_test.go b/config/config_test.go index def0366..21b7dbd 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -17,6 +17,7 @@ package config import ( "errors" "regexp" + "sort" "testing" "time" @@ -588,3 +589,98 @@ func TestBreakUpConfig_Conflict_Value(t *testing.T) { got := input.breakUp() is.Equal(want, got) } + +func TestConfig_getValuesForParameter(t *testing.T) { + cfg := Config{ + "ignore": "me", + + // foo + "test.foo.val": "0", + + "test.foo.format.baz.type": "0", + "test.foo.format.baz.options": "0", + + "test.foo.format.qux.type": "0", + "test.foo.format.qux.options": "0", + + // bar + "test.bar.val": "0", + + "test.bar.format.baz.type": "0", + "test.bar.format.baz.options": "0", + + "test.bar.format.qux.type": "0", + "test.bar.format.qux.options": "0", + } + + testCases := []struct { + key string + want []string + }{{ + key: "test.foo.val", + want: []string{"test.foo.val"}, + }, { + key: "blah", + want: []string{"blah"}, + }, { + key: "test.*.blah", + want: nil, + }, { + key: "test.*", + want: []string{ + "test.foo.val", + "test.foo.format.baz.type", + "test.foo.format.baz.options", + "test.foo.format.qux.type", + "test.foo.format.qux.options", + "test.bar.val", + "test.bar.format.baz.type", + "test.bar.format.baz.options", + "test.bar.format.qux.type", + "test.bar.format.qux.options", + }, + }, { + key: "test.*.val", + want: []string{"test.foo.val", "test.bar.val"}, + }, { + key: "test.*.format.*", + want: []string{ + "test.foo.format.baz.type", + "test.foo.format.baz.options", + "test.foo.format.qux.type", + "test.foo.format.qux.options", + "test.bar.format.baz.type", + "test.bar.format.baz.options", + "test.bar.format.qux.type", + "test.bar.format.qux.options", + }, + }, { + key: "test.*.format.*.type", + want: []string{ + "test.foo.format.baz.type", + "test.foo.format.qux.type", + "test.bar.format.baz.type", + "test.bar.format.qux.type", + }, + }, { + key: "test.*.format.*.options", + want: []string{ + "test.foo.format.baz.options", + "test.foo.format.qux.options", + "test.bar.format.baz.options", + "test.bar.format.qux.options", + }, + }} + + for _, tc := range testCases { + t.Run(tc.key, func(t *testing.T) { + is := is.New(t) + got := cfg.getKeysForParameter(tc.key) + + sort.Strings(tc.want) + sort.Strings(got) + is.Equal(tc.want, got) + }) + } + +} From 784e856bae62f49f8ce1192563008b5f60b94221 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 11 Apr 2024 16:55:35 +0200 Subject: [PATCH 3/9] add support for wildcards in validations and defaults --- config/config.go | 96 +++++++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 41 deletions(-) diff --git a/config/config.go b/config/config.go index 5e8cf30..089df27 100644 --- a/config/config.go +++ b/config/config.go @@ -49,8 +49,12 @@ func (c Config) Sanitize() Config { // configuration, the default value is applied. func (c Config) ApplyDefaults(params Parameters) Config { for key, param := range params { - if strings.TrimSpace(c[key]) == "" { // TODO get value for dynamic key - c[key] = param.Default + for _, key := range c.getKeysForParameter(key) { + // TODO it's not that easy, we need to check for all keys with the + // same pattern and create them if they don't exist + if strings.TrimSpace(c[key]) == "" { + c[key] = param.Default + } } } return c @@ -92,57 +96,67 @@ func (c Config) validateUnrecognizedParameters(params Parameters) []error { // validateParamType validates that a parameter value is parsable to its assigned type. func (c Config) validateParamType(key string, param Parameter) error { - value := c[key] // TODO get value for dynamic key - // empty value is valid for all types - if value == "" { - return nil - } + keys := c.getKeysForParameter(key) - //nolint:exhaustive // type ParameterTypeFile and ParameterTypeString don't need type validations (both are strings or byte slices) - switch param.Type { - case ParameterTypeInt: - _, err := strconv.Atoi(value) - if err != nil { - return fmt.Errorf("error validating %q: %q value is not an integer: %w", key, value, ErrInvalidParameterType) - } - case ParameterTypeFloat: - _, err := strconv.ParseFloat(value, 64) - if err != nil { - return fmt.Errorf("error validating %q: %q value is not a float: %w", key, value, ErrInvalidParameterType) - } - case ParameterTypeDuration: - _, err := time.ParseDuration(value) - if err != nil { - return fmt.Errorf("error validating %q: %q value is not a duration: %w", key, value, ErrInvalidParameterType) + var errs []error + for _, k := range keys { + value := c[k] + // empty value is valid for all types + if value == "" { + continue } - case ParameterTypeBool: - _, err := strconv.ParseBool(value) - if err != nil { - return fmt.Errorf("error validating %q: %q value is not a boolean: %w", key, value, ErrInvalidParameterType) + //nolint:exhaustive // type ParameterTypeFile and ParameterTypeString don't need type validations (both are strings or byte slices) + switch param.Type { + case ParameterTypeInt: + _, err := strconv.Atoi(value) + if err != nil { + errs = append(errs, fmt.Errorf("error validating %q: %q value is not an integer: %w", k, value, ErrInvalidParameterType)) + } + case ParameterTypeFloat: + _, err := strconv.ParseFloat(value, 64) + if err != nil { + errs = append(errs, fmt.Errorf("error validating %q: %q value is not a float: %w", k, value, ErrInvalidParameterType)) + } + case ParameterTypeDuration: + _, err := time.ParseDuration(value) + if err != nil { + errs = append(errs, fmt.Errorf("error validating %q: %q value is not a duration: %w", k, value, ErrInvalidParameterType)) + } + case ParameterTypeBool: + _, err := strconv.ParseBool(value) + if err != nil { + errs = append(errs, fmt.Errorf("error validating %q: %q value is not a boolean: %w", k, value, ErrInvalidParameterType)) + } } } - return nil + return errors.Join(errs...) } // validateParamValue validates that a configuration value matches all the // validations required for the parameter. func (c Config) validateParamValue(key string, param Parameter) error { - value := c[key] // TODO get value for dynamic key - var errs []error + keys := c.getKeysForParameter(key) - isRequired := false - for _, v := range param.Validations { - if _, ok := v.(ValidationRequired); ok { - isRequired = true + var errs []error + for _, k := range keys { + value := c[k] + var valErrs []error + + isRequired := false + for _, v := range param.Validations { + if _, ok := v.(ValidationRequired); ok { + isRequired = true + } + err := v.Validate(value) + if err != nil { + valErrs = append(valErrs, fmt.Errorf("error validating %q: %w", k, err)) + continue + } } - err := v.Validate(value) - if err != nil { - errs = append(errs, fmt.Errorf("error validating %q: %w", key, err)) - continue + if value == "" && !isRequired { + continue // empty optional parameter is valid } - } - if value == "" && !isRequired { - return nil // empty optional parameter is valid + errs = append(errs, valErrs...) } return errors.Join(errs...) From 05db2b5c1d6db882b86c9221b1ee87d8bb5d5837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 11 Apr 2024 18:39:25 +0200 Subject: [PATCH 4/9] correctly populate default values --- config/config.go | 48 +++++++++++++++++++++++++------------------ config/config_test.go | 31 +++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/config/config.go b/config/config.go index 089df27..9ee2b17 100644 --- a/config/config.go +++ b/config/config.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "reflect" + "slices" "strconv" "strings" "time" @@ -50,8 +51,6 @@ func (c Config) Sanitize() Config { func (c Config) ApplyDefaults(params Parameters) Config { for key, param := range params { for _, key := range c.getKeysForParameter(key) { - // TODO it's not that easy, we need to check for all keys with the - // same pattern and create them if they don't exist if strings.TrimSpace(c[key]) == "" { c[key] = param.Default } @@ -87,6 +86,7 @@ func (c Config) Validate(params Parameters) error { func (c Config) validateUnrecognizedParameters(params Parameters) []error { var errs []error for key := range c { + // TODO dynamic parameters if _, ok := params[key]; !ok { errs = append(errs, fmt.Errorf("%q: %w", key, ErrUnrecognizedParameter)) } @@ -135,10 +135,8 @@ func (c Config) validateParamType(key string, param Parameter) error { // validateParamValue validates that a configuration value matches all the // validations required for the parameter. func (c Config) validateParamValue(key string, param Parameter) error { - keys := c.getKeysForParameter(key) - var errs []error - for _, k := range keys { + for _, k := range c.getKeysForParameter(key) { value := c[k] var valErrs []error @@ -184,34 +182,44 @@ func (c Config) getKeysForParameter(key string) []string { for k := range c { fullKey := k for i, token := range tokens { - var ok bool - k, ok = consume(k, token) - if !ok { - // The key does not start with the token, it does not match the pattern. - break - } if i == len(tokens)-1 { - if k != "" && token != "" { - // The key is not fully consumed and last token is not a wildcard, - // it does not match the pattern. + if k == "" && token != "" { + // The key is consumed, but the token is not, it does not match the pattern. + // This happens when the last token is not a wildcard and + // the key is a leaf. break } - // We checked all tokens and the key matches the pattern. + // The last token doesn't matter, if the key matched so far, all + // wildcards have matched and we can potentially expect a match. + // The reason for this is so that we can apply defaults to the + // wildcard keys, even if they don't contain a value in the + // configuration. + if token != "" { + // Build potential key + fullKey = strings.TrimSuffix(fullKey, k) + fullKey += token + } keys = append(keys, fullKey) break } + var ok bool + k, ok = consume(k, token) + if !ok { + // The key does not start with the token, it does not match the pattern. + break + } + // Between tokens there is a wildcard, we need to strip the key until // the next ".". _, k, ok = strings.Cut(k, ".") - if !ok { - // The key does not have a "." after the token, it does not match the pattern. - break + if ok { + k = "." + k // Add the "." back to the key. } - k = "." + k // Add the "." back to the key. } } - return keys + slices.Sort(keys) + return slices.Compact(keys) } // DecodeInto copies configuration values into the target object. diff --git a/config/config_test.go b/config/config_test.go index 21b7dbd..ad15a6e 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/matryer/is" ) @@ -611,6 +612,12 @@ func TestConfig_getValuesForParameter(t *testing.T) { "test.bar.format.qux.type": "0", "test.bar.format.qux.options": "0", + + // include + "test.include.me": "yes", + + // ignore this, it's not nested + "test.ignore": "0", } testCases := []struct { @@ -623,8 +630,16 @@ func TestConfig_getValuesForParameter(t *testing.T) { key: "blah", want: []string{"blah"}, }, { - key: "test.*.blah", - want: nil, + key: "test.*.blah", + want: []string{ + // Note that the function returns keys that don't exist in the config, + // it figures out the potential keys based on matched wildcards. + // However, it does not return test.ignore.blah, as test.ignore does + // not contain any nested keys. + "test.foo.blah", + "test.bar.blah", + "test.include.blah", + }, }, { key: "test.*", want: []string{ @@ -638,10 +653,16 @@ func TestConfig_getValuesForParameter(t *testing.T) { "test.bar.format.baz.options", "test.bar.format.qux.type", "test.bar.format.qux.options", + "test.include.me", + "test.ignore", }, }, { - key: "test.*.val", - want: []string{"test.foo.val", "test.bar.val"}, + key: "test.*.val", + want: []string{ + "test.foo.val", + "test.bar.val", + "test.include.val", + }, }, { key: "test.*.format.*", want: []string{ @@ -679,7 +700,7 @@ func TestConfig_getValuesForParameter(t *testing.T) { sort.Strings(tc.want) sort.Strings(got) - is.Equal(tc.want, got) + is.Equal(cmp.Diff(tc.want, got), "") }) } From 343926100fb5c0e265877691dbf0b3443f4a2fb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 11 Apr 2024 19:27:24 +0200 Subject: [PATCH 5/9] correctly detect unrecognized parameters, lots of tests --- config/config.go | 61 ++++++-- config/config_test.go | 314 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 363 insertions(+), 12 deletions(-) diff --git a/config/config.go b/config/config.go index 9ee2b17..7372e77 100644 --- a/config/config.go +++ b/config/config.go @@ -86,8 +86,24 @@ func (c Config) Validate(params Parameters) error { func (c Config) validateUnrecognizedParameters(params Parameters) []error { var errs []error for key := range c { - // TODO dynamic parameters - if _, ok := params[key]; !ok { + if _, ok := params[key]; ok { + // Direct match. + continue + } + // Check if the key is a wildcard key. + match := false + for pattern := range params { + if !strings.Contains(pattern, "*") { + continue + } + // Check if the key matches the wildcard key. + if c.matchParameterKey(key, pattern) { + match = true + break + } + } + + if !match { errs = append(errs, fmt.Errorf("%q: %w", key, ErrUnrecognizedParameter)) } } @@ -168,14 +184,6 @@ func (c Config) getKeysForParameter(key string) []string { return []string{key} } - consume := func(s, prefix string) (string, bool) { - if !strings.HasPrefix(s, prefix) { - // The key does not start with the token, it does not match the pattern. - return "", false - } - return strings.TrimPrefix(s, prefix), true - } - // There is at least one wildcard in the key, we need to manually find all // the keys that match the pattern. var keys []string @@ -187,6 +195,7 @@ func (c Config) getKeysForParameter(key string) []string { // The key is consumed, but the token is not, it does not match the pattern. // This happens when the last token is not a wildcard and // the key is a leaf. + // e.g. param: "collection.*.format", key: "collection.foo" break } // The last token doesn't matter, if the key matched so far, all @@ -222,6 +231,30 @@ func (c Config) getKeysForParameter(key string) []string { return slices.Compact(keys) } +func (c Config) matchParameterKey(key, pattern string) bool { + tokens := strings.Split(pattern, "*") + if len(tokens) == 1 { + // No wildcard in the key, compare the key directly. + return key == pattern + } + k := key + for _, token := range tokens { + var ok bool + k, ok = consume(k, token) + if !ok { + return false + } + + // Between tokens there is a wildcard, we need to strip the key until + // the next ".". + _, k, ok = strings.Cut(k, ".") + if ok { + k = "." + k // Add the "." back to the key. + } + } + return true +} + // DecodeInto copies configuration values into the target object. // Under the hood, this function uses github.com/mitchellh/mapstructure, with // the "mapstructure" tag renamed to "json". To rename a key, use the "json" @@ -341,3 +374,11 @@ func mapStructHookFunc() mapstructure.DecodeHookFunc { return dataMap, nil } } + +func consume(s, prefix string) (string, bool) { + if !strings.HasPrefix(s, prefix) { + // The key does not start with the token, it does not match the pattern. + return "", false + } + return strings.TrimPrefix(s, prefix), true +} diff --git a/config/config_test.go b/config/config_test.go index ad15a6e..57cae0c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -105,6 +105,72 @@ func TestConfig_Validate_ParameterType(t *testing.T) { config: Config{"param1": "some-data"}, params: Parameters{"param1": {Type: ParameterTypeFile}}, wantErr: false, + }, { + // ---------------------- DYNAMIC PARAMETER TESTS ---------------------- + name: "dynamic: valid type number", + config: Config{"foo.0.param1": "3"}, + params: Parameters{"foo.*.param1": {Default: "3.3", Type: ParameterTypeFloat}}, + wantErr: false, + }, { + name: "dynamic: invalid type float", + config: Config{"foo.0.param1": "not-a-number"}, + params: Parameters{"foo.*.param1": {Default: "3.3", Type: ParameterTypeFloat}}, + wantErr: true, + }, { + name: "dynamic: valid default type float", + config: Config{"foo.0.param1": ""}, + params: Parameters{"foo.*.param1": {Default: "3", Type: ParameterTypeFloat}}, + wantErr: false, + }, { + name: "dynamic: valid type int", + config: Config{"foo.0.param1": "3"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeInt}}, + wantErr: false, + }, { + name: "dynamic: invalid type int", + config: Config{"foo.0.param1": "3.3"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeInt}}, + wantErr: true, + }, { + name: "dynamic: valid type bool", + config: Config{"foo.0.param1": "1"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeBool}}, + wantErr: false, + }, { + name: "dynamic: valid type bool", + config: Config{"foo.0.param1": "true"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeBool}}, + wantErr: false, + }, { + name: "dynamic: invalid type bool", + config: Config{"foo.0.param1": "not-a-bool"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeBool}}, + wantErr: true, + }, { + name: "dynamic: valid type duration", + config: Config{"foo.0.param1": "1s"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeDuration}}, + wantErr: false, + }, { + name: "dynamic: empty value is valid for all types", + config: Config{"foo.0.param1": ""}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeDuration}}, + wantErr: false, + }, { + name: "dynamic: invalid type duration", + config: Config{"foo.0.param1": "not-a-duration"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeDuration}}, + wantErr: true, + }, { + name: "dynamic: valid type string", + config: Config{"foo.0.param1": "param"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeString}}, + wantErr: false, + }, { + name: "dynamic: valid type file", + config: Config{"foo.0.param1": "some-data"}, + params: Parameters{"foo.*.param1": {Type: ParameterTypeFile}}, + wantErr: false, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -116,7 +182,7 @@ func TestConfig_Validate_ParameterType(t *testing.T) { if err != nil && tt.wantErr { is.True(errors.Is(err, ErrInvalidParameterType)) } else if err != nil || tt.wantErr { - t.Errorf("UtilityFunc() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("error = %v, wantErr %v", err, tt.wantErr) } }) } @@ -266,6 +332,144 @@ func TestConfig_Validate_Validations(t *testing.T) { }}, }, wantErr: false, + }, { + // ---------------------- DYNAMIC PARAMETER TESTS ---------------------- + name: "dynamic: required validation failed", + config: Config{"foo.0.param1": ""}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + }}, + }, + wantErr: true, + err: ErrRequiredParameterMissing, + }, { + name: "dynamic: required validation pass", + config: Config{"foo.0.param1": "value"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + }}, + }, + wantErr: false, + }, { + name: "dynamic: less than validation failed", + config: Config{"foo.0.param1": "20"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationLessThan{10}, + }}, + }, + wantErr: true, + err: ErrLessThanValidationFail, + }, { + name: "dynamic: less than validation pass", + config: Config{"foo.0.param1": "0"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationLessThan{10}, + }}, + }, + wantErr: false, + }, { + name: "dynamic: greater than validation failed", + config: Config{"foo.0.param1": "0"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationGreaterThan{10}, + }}, + }, + wantErr: true, + err: ErrGreaterThanValidationFail, + }, { + name: "dynamic: greater than validation failed", + config: Config{"foo.0.param1": "20"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationGreaterThan{10}, + }}, + }, + wantErr: false, + }, { + name: "dynamic: inclusion validation failed", + config: Config{"foo.0.param1": "three"}, + params: Parameters{ + "param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationInclusion{[]string{"one", "two"}}, + }}, + }, + wantErr: true, + err: ErrInclusionValidationFail, + }, { + name: "dynamic: inclusion validation pass", + config: Config{"foo.0.param1": "two"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationInclusion{[]string{"one", "two"}}, + }}, + }, + wantErr: false, + }, { + name: "dynamic: exclusion validation failed", + config: Config{"foo.0.param1": "one"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationExclusion{[]string{"one", "two"}}, + }}, + }, + wantErr: true, + err: ErrExclusionValidationFail, + }, { + name: "dynamic: exclusion validation pass", + config: Config{"foo.0.param1": "three"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationExclusion{[]string{"one", "two"}}, + }}, + }, + wantErr: false, + }, { + name: "dynamic: regex validation failed", + config: Config{"foo.0.param1": "a-a"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationRegex{regexp.MustCompile("[a-z]-[1-9]")}, + }}, + }, + wantErr: true, + err: ErrRegexValidationFail, + }, { + name: "dynamic: regex validation pass", + config: Config{"foo.0.param1": "a-8"}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationRequired{}, + ValidationRegex{regexp.MustCompile("[a-z]-[1-9]")}, + }}, + }, + wantErr: false, + }, { + name: "dynamic: optional validation pass", + config: Config{"foo.0.param1": ""}, + params: Parameters{ + "foo.*.param1": {Validations: []Validation{ + ValidationInclusion{[]string{"one", "two"}}, + ValidationExclusion{[]string{"three", "four"}}, + ValidationRegex{regexp.MustCompile("[a-z]")}, + ValidationGreaterThan{10}, + ValidationLessThan{20}, + }}, + }, + wantErr: false, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -277,7 +481,45 @@ func TestConfig_Validate_Validations(t *testing.T) { if err != nil && tt.wantErr { is.True(errors.Is(err, tt.err)) } else if err != nil || tt.wantErr { - t.Errorf("UtilityFunc() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestConfig_Validate_Unrecognized(t *testing.T) { + tests := []struct { + name string + config Config + params Parameters + wantErr bool + }{{ + name: "parameters empty", + config: Config{"param1": "3"}, + params: Parameters{}, + wantErr: true, + }, { + name: "static parameter unrecognized", + config: Config{"param1": "not-a-number"}, + params: Parameters{"param2": {Type: ParameterTypeFloat}}, + wantErr: true, + }, { + name: "dynamic parameter unrecognized", + config: Config{"foo.0.param1.": ""}, + params: Parameters{"foo.*.param2": {Type: ParameterTypeFloat}}, + wantErr: true, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + is := is.New(t) + err := tt.config.Sanitize(). + ApplyDefaults(tt.params). + Validate(tt.params) + + if err != nil && tt.wantErr { + is.True(errors.Is(err, ErrUnrecognizedParameter)) + } else if err != nil || tt.wantErr { + t.Errorf("error = %v, wantErr %v", err, tt.wantErr) } }) } @@ -343,6 +585,74 @@ OUTER: } } +func TestConfig_ApplyDefaults(t *testing.T) { + params := map[string]Parameter{ + "limit": {Type: ParameterTypeInt, Default: "1"}, + "foo.*.param1": {Type: ParameterTypeString, Default: "foo"}, + "foo.*.param2": {Type: ParameterTypeString}, + } + + testCases := []struct { + name string + have Config + want Config + }{{ + name: "empty", + have: Config{}, + want: Config{ + "limit": "1", + }, + }, { + name: "foo.0.param2", + have: Config{ + "foo.0.param2": "bar", + }, + want: Config{ + "limit": "1", + "foo.0.param1": "foo", + "foo.0.param2": "bar", + }, + }, { + name: "foo.0.param1", + have: Config{ + "limit": "-1", + "foo.0.param1": "custom", + }, + want: Config{ + "limit": "-1", + "foo.0.param1": "custom", + "foo.0.param2": "", + }, + }, { + name: "multiple dynamic params", + have: Config{ + "limit": "-1", + "foo.0.param1": "parameter", + "foo.1.param2": "custom", + "foo.2.does-not-exist": "unrecognized key still triggers creation of defaults", + }, + want: Config{ + "limit": "-1", + "foo.0.param1": "parameter", + "foo.0.param2": "", + "foo.1.param1": "foo", + "foo.1.param2": "custom", + "foo.2.param1": "foo", + "foo.2.param2": "", + "foo.2.does-not-exist": "unrecognized key still triggers creation of defaults", + }, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + is := is.New(t) + got := tc.have.Sanitize(). + ApplyDefaults(params) + is.Equal(tc.want, got) + }) + } +} + // unwrapErrors recursively unwraps all errors combined using errors.Join. func unwrapErrors(err error) []error { errs, ok := err.(interface{ Unwrap() []error }) From aa9546ae9f82cfa6ee50d0eabb845351a03e869f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 11 Apr 2024 19:29:45 +0200 Subject: [PATCH 6/9] fix linter warnings --- .golangci.yml | 1 + config/config.go | 10 ++++++---- config/config_test.go | 1 - 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3261131..66453fb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -40,6 +40,7 @@ issues: - funlen - goerr113 - dupl + - maintidx linters: # please, do not use `enable-all`: it's deprecated and will be removed soon. diff --git a/config/config.go b/config/config.go index 7372e77..4987271 100644 --- a/config/config.go +++ b/config/config.go @@ -329,13 +329,14 @@ func mapStringHookFunc() mapstructure.DecodeHookFunc { return func( f reflect.Type, t reflect.Type, - data interface{}) (interface{}, error) { + data interface{}, + ) (interface{}, error) { if f.Kind() != reflect.Map || f.Elem().Kind() != reflect.Interface || t.Kind() != reflect.Map || t.Elem().Kind() != reflect.String { return data, nil } - // no need to assert, we know it's a map[string]any + //nolint:forcetypeassert // We checked in the condition above and know it's a map[string]any dataMap := data.(map[string]any) // remove all keys with maps @@ -353,13 +354,14 @@ func mapStructHookFunc() mapstructure.DecodeHookFunc { return func( f reflect.Type, t reflect.Type, - data interface{}) (interface{}, error) { + data interface{}, + ) (interface{}, error) { if f.Kind() != reflect.Map || f.Elem().Kind() != reflect.Interface || t.Kind() != reflect.Map || t.Elem().Kind() != reflect.Struct { return data, nil } - // no need to assert, we know it's a map[string]any + //nolint:forcetypeassert // We checked in the condition above and know it's a map[string]any dataMap := data.(map[string]any) // remove all keys with a dot that contains a value with a string diff --git a/config/config_test.go b/config/config_test.go index 57cae0c..2d67f46 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1013,5 +1013,4 @@ func TestConfig_getValuesForParameter(t *testing.T) { is.Equal(cmp.Diff(tc.want, got), "") }) } - } From e564966b97f1fde81dcc327722a5b0e29f57d0d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Fri, 12 Apr 2024 12:57:31 +0200 Subject: [PATCH 7/9] fix test names --- config/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 2d67f46..4c210af 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -247,7 +247,7 @@ func TestConfig_Validate_Validations(t *testing.T) { wantErr: true, err: ErrGreaterThanValidationFail, }, { - name: "greater than validation failed", + name: "greater than validation pass", config: Config{"param1": "20"}, params: Parameters{ "param1": {Validations: []Validation{ @@ -385,7 +385,7 @@ func TestConfig_Validate_Validations(t *testing.T) { wantErr: true, err: ErrGreaterThanValidationFail, }, { - name: "dynamic: greater than validation failed", + name: "dynamic: greater than validation pass", config: Config{"foo.0.param1": "20"}, params: Parameters{ "foo.*.param1": {Validations: []Validation{ From 3d53547518c01b28594c93c00644d08b68fb49bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Mon, 15 Apr 2024 13:33:36 +0200 Subject: [PATCH 8/9] document behavior --- config/config.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/config/config.go b/config/config.go index 4987271..06763c7 100644 --- a/config/config.go +++ b/config/config.go @@ -219,11 +219,14 @@ func (c Config) getKeysForParameter(key string) []string { break } - // Between tokens there is a wildcard, we need to strip the key until - // the next ".". - _, k, ok = strings.Cut(k, ".") - if ok { - k = "." + k // Add the "." back to the key. + // Between tokens there is a wildcard, we need to consume the key + // until the next ".". If there is no next ".", the whole key is + // consumed. + // e.g. "foo.format" -> ".format" or "foo" -> "" + if index := strings.IndexRune(k, '.'); index != -1 { + k = k[index:] + } else { + k = "" } } } From f78d5d693470b151946654007cc0d0b0fb3b2a8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Mon, 15 Apr 2024 13:38:14 +0200 Subject: [PATCH 9/9] add test for wildcard at the start --- config/config_test.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/config/config_test.go b/config/config_test.go index 4c210af..b554591 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -903,7 +903,8 @@ func TestBreakUpConfig_Conflict_Value(t *testing.T) { func TestConfig_getValuesForParameter(t *testing.T) { cfg := Config{ - "ignore": "me", + "ignore": "me", + "ignore.foo.this": "me", // foo "test.foo.val": "0", @@ -1001,6 +1002,34 @@ func TestConfig_getValuesForParameter(t *testing.T) { "test.bar.format.baz.options", "test.bar.format.qux.options", }, + }, { + key: "*", + want: []string{ + "ignore", + "ignore.foo.this", + "test.foo.val", + "test.foo.format.baz.type", + "test.foo.format.baz.options", + "test.foo.format.qux.type", + "test.foo.format.qux.options", + "test.bar.val", + "test.bar.format.baz.type", + "test.bar.format.baz.options", + "test.bar.format.qux.type", + "test.bar.format.qux.options", + "test.include.me", + "test.ignore", + }, + }, { + key: "*.foo.*", + want: []string{ + "ignore.foo.this", + "test.foo.val", + "test.foo.format.baz.type", + "test.foo.format.baz.options", + "test.foo.format.qux.type", + "test.foo.format.qux.options", + }, }} for _, tc := range testCases {