From 0bdbff89ac65fdeb208ba6292de1cdf4d048c66a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krist=C3=B3f=20T=C3=B3th?= Date: Mon, 25 Oct 2021 18:28:36 +0200 Subject: [PATCH] Refactor config package --- cmd/after-lock/after-lock.go | 10 +-- config/config.go | 127 ++++++++++++++++++----------------- config/config_test.go | 89 +++++++++++++++++++----- 3 files changed, 144 insertions(+), 82 deletions(-) diff --git a/cmd/after-lock/after-lock.go b/cmd/after-lock/after-lock.go index d7cfbe5..7e66e6e 100644 --- a/cmd/after-lock/after-lock.go +++ b/cmd/after-lock/after-lock.go @@ -18,11 +18,11 @@ import ( type afterLockConfig struct { - INITIAL_DELAY int - LOOP_DELAY int - LOCK_PATH string + INITIAL_DELAY int `optional` + LOOP_DELAY int `optional` + LOCK_PATH string `optional` LOG_PATH string - PRINT_STACKTRACES bool + PRINT_STACKTRACES bool `optional` } var configuration = afterLockConfig{ @@ -46,7 +46,7 @@ func init() { func main() { defer handleErrors() - err := config.Build("AFL", &configuration) + err := config.NewBuilder().Build("AFL_", &configuration) if err != nil { panic(err) } diff --git a/config/config.go b/config/config.go index 773eaca..4583db7 100644 --- a/config/config.go +++ b/config/config.go @@ -1,99 +1,95 @@ package config import ( - "os" - "strings" "fmt" + "os" "reflect" - "errors" "strconv" + "strings" ) -var ConfigFetchMethod = os.Environ +type ConfigBuilder struct { + ConfigFetchMethod func()[]string + typeParsers map[reflect.Kind]func(string)(interface{}, error) +} +func NewBuilder() *ConfigBuilder { + return &ConfigBuilder{ + ConfigFetchMethod: os.Environ, + typeParsers: map[reflect.Kind]func(string)(interface{}, error) { + reflect.String: func(value string) (interface{}, error) { + return value, nil + }, + reflect.Int: func(value string) (interface{}, error) { + i, err := strconv.Atoi(value) + if err != nil { + return nil, err + } + return i, nil + }, + reflect.Bool: func(value string) (interface{}, error) { + b, err := strconv.ParseBool(value) + if err != nil { + return nil, err + } + return b, nil + }, + }, + } +} -func Build(prefix string, configStruct interface{}) (err error) { - defer func() { - if e := recover(); e != nil { - switch r := e.(type) { - case string: - err = errors.New(r) - case error: - err = r - default: - err = errors.New("Undefined panic") - } - } - }() +func (cb *ConfigBuilder) Build(prefix string, configStruct interface{}) error { assertIsPtr(configStruct) fieldNames := getConfigStructFieldNames(configStruct) - config := loadConfig(prefix) - + config := cb.loadConfig(prefix) structRef := reflect.ValueOf(configStruct) + for _, fieldName := range fieldNames { value, ok := config[fieldName] if !ok { - // no such field loaded, avoid overwriting default - continue + // no such field loaded, avoid overwriting default if field is optional + field, _ := reflect.TypeOf(configStruct).Elem().FieldByName(fieldName) + if strings.Contains(string(field.Tag), "optional") { + continue + } + return fmt.Errorf("no config found for required field '%s'", fieldName) } field := reflect.Indirect(structRef).FieldByName(fieldName) typeRef := field.Type() - ret, err := tryParseString(value, typeRef.Kind()) + typedValue, err := cb.tryParseString(value, typeRef.Kind()) if err != nil { return err } - field.Set(reflect.ValueOf(ret).Convert(typeRef)) + field.Set(reflect.ValueOf(typedValue).Convert(typeRef)) } return nil } -func tryParseString(what string, toType reflect.Kind) (interface{}, error) { - switch toType { - case reflect.String: - return what, nil - case reflect.Int: - i, err := strconv.Atoi(what) - if err != nil { - return nil, err - } - return i, nil - case reflect.Bool: - b, err := strconv.ParseBool(what) - if err != nil { - return nil, err - } - return b, nil - default: - return nil, fmt.Errorf("Failed to parse value %v", what) - } -} - -func assertIsPtr(what interface{}) { - r := reflect.ValueOf(what) +func assertIsPtr(configStruct interface{}) { + r := reflect.ValueOf(configStruct) if r.Kind() != reflect.Ptr { - panic(fmt.Errorf("Supplied value is not a pointer to a struct")) + panic(fmt.Errorf("supplied value is not a pointer to a struct")) } } func getConfigStructFieldNames(configStruct interface{}) []string { configFields := []string{} + structValue := reflect.ValueOf(configStruct).Elem() - val := reflect.ValueOf(configStruct).Elem() - for i := 0; i < val.NumField(); i++ { - configFields = append(configFields, val.Type().Field(i).Name) + for i := 0; i < structValue.NumField(); i++ { + configFields = append(configFields, structValue.Type().Field(i).Name) } return configFields } -func loadConfig(prefix string) map[string]string { - prefix = fmt.Sprintf("%s_", prefix) - +func (cb *ConfigBuilder) loadConfig(prefix string) map[string]string { config := map[string]string{} - for _, env := range ConfigFetchMethod() { + + for _, env := range cb.ConfigFetchMethod() { parts := strings.SplitN(env, "=", 2) key, value := parts[0], parts[1] if strings.HasPrefix(key, prefix) { @@ -101,15 +97,22 @@ func loadConfig(prefix string) map[string]string { config[key] = value } } + return config } -// CheckKeysExist returns an error if not all keys are present in config map -func CheckKeysExist(config map[string]string, keys ...string) error { - for _, key := range keys { - if _, ok := config[key]; ok { - return fmt.Errorf("Config key '%s' is not set", key) - } +func (cb *ConfigBuilder) tryParseString(value string, toType reflect.Kind) (interface{}, error) { + parser, ok := cb.typeParsers[toType] + if !ok { + return nil, fmt.Errorf("no parser found for type %v", toType) } - return nil -} \ No newline at end of file + parsed_val, err := parser(value) + if err != nil { + return nil, err + } + return parsed_val, nil +} + +func (cb *ConfigBuilder) RegisterParser(toType reflect.Kind, parser func(string)(interface{}, error)) { + cb.typeParsers[toType] = parser +} diff --git a/config/config_test.go b/config/config_test.go index d703771..b7cb8d2 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1,9 +1,11 @@ package config_test import ( - "testing" - "kdelsd/config" + "reflect" + "strings" + "strconv" + "testing" ) @@ -11,39 +13,96 @@ type TestConfig struct { INT int BOOL bool STR string - OTHER_STR string + OTHER_STR string `optional` + COMPLEX complex64 `optional` } -var testConfigInstance = TestConfig{ +var testConfigReference = TestConfig{ INT: 42, BOOL: true, STR: "sajtok", OTHER_STR: "default-value", } -var testEnv = []string { +var testEnv = []string{ "TEST_INT=42", "TEST_BOOL=true", "TEST_STR=sajtok", } -func setup() { - config.ConfigFetchMethod = func() []string { +func configBuilder() *config.ConfigBuilder { + cb := config.NewBuilder() + cb.ConfigFetchMethod = func()[]string { return testEnv } + + return cb +} + + +func TestConfigBuilder(t *testing.T) { + c := TestConfig{} + c.OTHER_STR = testConfigReference.OTHER_STR + builder := configBuilder() + + err := builder.Build("TEST_", &c) + if err != nil { + t.Fatalf("Config building should not fail\n") + } + + if c != testConfigReference { + t.Fatalf("Parsed config shoudl equal reference config\n%v != %v\n", c, testConfigReference) + } } -func TestBuildConfig(t *testing.T) { - setup() +func TestConfigBuilderParsers(t *testing.T) { c := TestConfig{} - c.OTHER_STR = testConfigInstance.OTHER_STR + builder := configBuilder() + builder.ConfigFetchMethod = func()[]string { + return append(testEnv, "TEST_COMPLEX=42+42i") + } - err := config.Build("TEST", &c) + err := builder.Build("TEST_", &c) + if err == nil { + t.Fatalf("CONFIG should not be parseable, since type has no default parser\n") + } + if !strings.Contains(err.Error(), "complex64") { + t.Fatalf("Error should contain the type that failed to parse\n") + } + + builder.RegisterParser(reflect.Complex64, func(value string)(interface{}, error) { + num, err := strconv.ParseComplex(value, 64) + if err != nil { + return nil, err + } + return num, nil + }) + err = builder.Build("TEST_", &c) if err != nil { - panic(err) + t.Fatalf("Parsing type should not fail after registering a parser for it\n") + } +} + +func TestConfigBuilderOptional(t *testing.T) { + type RequiredConfig struct { + REQUIRED_STR string + } + c := RequiredConfig{} + builder := config.NewBuilder() + + err := builder.Build("TEST_", &c) + if err == nil { + t.Fatalf("Fields not marked with the `optional` tag should be mandatory\n") + } + if !strings.Contains(err.Error(), "REQUIRED_STR") { + t.Fatalf("Error should contain the name of the field\n") } - if c != testConfigInstance { - t.Errorf("%v != %v", c, testConfigInstance) + builder.ConfigFetchMethod = func()[]string { + return append(testEnv, "TEST_REQUIRED_STR=prrrr") } -} \ No newline at end of file + err = builder.Build("TEST_", &c) + if err != nil { + t.Fatalf("Config building should not fail after providing the mandatory field\n") + } +}