From 30a3a5ad3db9cc75f9e08c95bdabd80819248d1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Tue, 4 Aug 2020 17:43:56 +0800 Subject: [PATCH 1/9] feat: log the names of environment variables - log the names of environment variables at debug level if logger is provided - use optional parameter to pass logger --- envconf.go | 33 ++++++++++++++++++++++++++++----- envconf_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/envconf.go b/envconf.go index ce38c6a..24bfd7d 100644 --- a/envconf.go +++ b/envconf.go @@ -8,6 +8,18 @@ import ( "syscall" ) +type Option func(*loader) + +func LoggerOption(logger logger) Option { + return func(l *loader) { + l.logger = logger + } +} + +type logger interface { + Debugf(string, ...interface{}) +} + // Load loads config from environment variables into the provided // pointer-to-struct `out`. // The names of loaded environment variables are uppercase and all start with the given `prefix`. @@ -15,18 +27,26 @@ import ( // Warning: // 1. Fields without env tag will be ignored. // 2. Duplicated field tags will be assigned with the same environment variable. -func Load(prefix string, out interface{}) { +func Load(prefix string, out interface{}, opts ...Option) { val := reflect.ValueOf(out).Elem() - loader := loader{ - usedKeys: map[string]struct{}{}, + loader := loader{} + loader.usedKeys = map[string]struct{}{} + for _, opt := range opts { + opt(&loader) } loader.loadStruct(strings.ToUpper(prefix), &val) } -var stringSliceType = reflect.TypeOf([]string{}) - type loader struct { usedKeys map[string]struct{} + + logger logger +} + +func (l *loader) logf(format string, args ...interface{}) { + if l.logger != nil { + l.logger.Debugf(format, args...) + } } func (l *loader) useKey(name string) error { @@ -34,9 +54,12 @@ func (l *loader) useKey(name string) error { return fmt.Errorf("Duplicated key %v", name) } l.usedKeys[name] = struct{}{} + l.logf("searching for environment variable: %s", name) return nil } +var stringSliceType = reflect.TypeOf([]string{}) + func (l *loader) loadField(name string, out *reflect.Value) { kind := out.Kind() if kind == reflect.Struct { diff --git a/envconf_test.go b/envconf_test.go index 963053a..b0a5d9f 100644 --- a/envconf_test.go +++ b/envconf_test.go @@ -1,6 +1,8 @@ package envconf import ( + "bytes" + "fmt" "os" "reflect" "testing" @@ -101,6 +103,18 @@ func TestTaggedUnsupportedTypeShouldPanic(t *testing.T) { Load("FAIL", &invalid) } +func TestUnsupportedSliceShouldPanic(t *testing.T) { + defer assertPanic(t) + + type Invalid struct { + Unsupported []map[string]string + } + + os.Setenv("FAIL_UNSUPPORTED", "[]") + var invalid Invalid + Load("FAIL", &invalid) +} + func TestInvalidIntShouldPanic(t *testing.T) { defer assertPanic(t) @@ -299,3 +313,29 @@ func TestDuplicatedKeysBetweenStructs(t *testing.T) { }{} Load("TEST", &config) } + +type customLogger struct { + buf bytes.Buffer +} + +func (l *customLogger) Debugf(format string, args ...interface{}) { + l.buf.WriteString(fmt.Sprintf(format, args...) + "\n") +} + +func TestLogger(t *testing.T) { + logger := customLogger{} + config := struct { + String string `env:"string"` + Integer int `env:"integer"` + }{} + Load("TEST", &config, LoggerOption(&logger)) + + expected := "searching for environment variable: TEST_STRING\nsearching for environment variable: TEST_INTEGER\n" + received := logger.buf.String() + if expected != received { + t.Errorf( + "expecting %s,\nreceiving %s", + expected, received, + ) + } +} From 3b81f5ac51d4cdfa7e95e40583e3b578a180832b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Thu, 6 Aug 2020 11:47:03 +0800 Subject: [PATCH 2/9] refactor: use callback - use callback and let caller decide how to deal with the status of environment variables --- envconf.go | 41 +++++++++++++++-------------------------- envconf_test.go | 36 +++++++++++++++--------------------- option.go | 9 +++++++++ 3 files changed, 39 insertions(+), 47 deletions(-) create mode 100644 option.go diff --git a/envconf.go b/envconf.go index 24bfd7d..ef93563 100644 --- a/envconf.go +++ b/envconf.go @@ -8,18 +8,6 @@ import ( "syscall" ) -type Option func(*loader) - -func LoggerOption(logger logger) Option { - return func(l *loader) { - l.logger = logger - } -} - -type logger interface { - Debugf(string, ...interface{}) -} - // Load loads config from environment variables into the provided // pointer-to-struct `out`. // The names of loaded environment variables are uppercase and all start with the given `prefix`. @@ -28,33 +16,29 @@ type logger interface { // 1. Fields without env tag will be ignored. // 2. Duplicated field tags will be assigned with the same environment variable. func Load(prefix string, out interface{}, opts ...Option) { - val := reflect.ValueOf(out).Elem() loader := loader{} - loader.usedKeys = map[string]struct{}{} + loader.envStatus = map[string]bool{} + loader.handleEnvironmentVariables = func(map[string]bool) {} for _, opt := range opts { opt(&loader) } - loader.loadStruct(strings.ToUpper(prefix), &val) -} -type loader struct { - usedKeys map[string]struct{} + val := reflect.ValueOf(out).Elem() + loader.loadStruct(strings.ToUpper(prefix), &val) - logger logger + loader.handleEnvironmentVariables(loader.envStatus) } -func (l *loader) logf(format string, args ...interface{}) { - if l.logger != nil { - l.logger.Debugf(format, args...) - } +type loader struct { + handleEnvironmentVariables func(map[string]bool) + envStatus map[string]bool } func (l *loader) useKey(name string) error { - if _, ok := l.usedKeys[name]; ok { + if _, ok := l.envStatus[name]; ok { return fmt.Errorf("Duplicated key %v", name) } - l.usedKeys[name] = struct{}{} - l.logf("searching for environment variable: %s", name) + l.envStatus[name] = false return nil } @@ -157,6 +141,7 @@ func (l *loader) loadString(name string, out *reflect.Value) { if !found { return } + l.envStatus[name] = true out.SetString(data) } @@ -166,6 +151,7 @@ func (l *loader) loadInt(name string, out *reflect.Value) { if !found { return } + l.envStatus[name] = true d, err := strconv.ParseInt(data, 10, 64) if err != nil { @@ -180,6 +166,7 @@ func (l *loader) loadUint(name string, out *reflect.Value) { if !found { return } + l.envStatus[name] = true d, err := strconv.ParseUint(data, 10, 64) if err != nil { @@ -194,6 +181,7 @@ func (l *loader) loadBool(name string, out *reflect.Value) { if !found { return } + l.envStatus[name] = true isTrue := data == "true" || data == "TRUE" || data == "True" || data == "1" isFalse := data == "false" || data == "FALSE" || data == "False" || data == "0" @@ -210,6 +198,7 @@ func (l *loader) loadStringSlice(name string, out *reflect.Value) { if !found { return } + l.envStatus[name] = true strListRaw := strings.Split(data, ",") strList := []string{} diff --git a/envconf_test.go b/envconf_test.go index b0a5d9f..f646d68 100644 --- a/envconf_test.go +++ b/envconf_test.go @@ -1,8 +1,6 @@ package envconf import ( - "bytes" - "fmt" "os" "reflect" "testing" @@ -314,28 +312,24 @@ func TestDuplicatedKeysBetweenStructs(t *testing.T) { Load("TEST", &config) } -type customLogger struct { - buf bytes.Buffer -} - -func (l *customLogger) Debugf(format string, args ...interface{}) { - l.buf.WriteString(fmt.Sprintf(format, args...) + "\n") -} - func TestLogger(t *testing.T) { - logger := customLogger{} + os.Setenv("TEST_INTEGER", "-3") + os.Setenv("TEST_UNSIGNED_INTEGER", "3") + + result := map[string]bool{} config := struct { String string `env:"string"` Integer int `env:"integer"` + Struct struct { + Unsigned uint `env:"unsigned_integer"` + Bool bool `env:"bool"` + StrSlice []string `env:"string_slice"` + } `env:",inline"` }{} - Load("TEST", &config, LoggerOption(&logger)) - - expected := "searching for environment variable: TEST_STRING\nsearching for environment variable: TEST_INTEGER\n" - received := logger.buf.String() - if expected != received { - t.Errorf( - "expecting %s,\nreceiving %s", - expected, received, - ) - } + Load("TEST", &config, CustomHandleEnvironmentVariablesOption(func(status map[string]bool) { + result = status + })) + + expected := map[string]bool{"TEST_STRING": false, "TEST_INTEGER": true, "TEST_UNSIGNED_INTEGER": true, "TEST_BOOL": false, "TEST_STRING_SLICE": false} + assertEqual(t, "", expected, result) } diff --git a/option.go b/option.go new file mode 100644 index 0000000..5ca4b01 --- /dev/null +++ b/option.go @@ -0,0 +1,9 @@ +package envconf + +type Option func(*loader) + +func CustomHandleEnvironmentVariablesOption(cb func(map[string]bool)) Option { + return func(l *loader) { + l.handleEnvironmentVariables = cb + } +} From e64d63ffe410092d8cf5de84ea8c3119297daaa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Thu, 6 Aug 2020 18:18:27 +0800 Subject: [PATCH 3/9] feat: predefined callback - log which environment variables are used by the given configuration and whether they are set or not --- options/predefined.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 options/predefined.go diff --git a/options/predefined.go b/options/predefined.go new file mode 100644 index 0000000..b23d6d2 --- /dev/null +++ b/options/predefined.go @@ -0,0 +1,19 @@ +package options + +import ( + "encoding/json" + "io" + "log" + + "gitlab.rayark.com/backend/envconf" +) + +func LogStatusOfEnvironmentVariables(w io.Writer) envconf.Option { + return envconf.CustomHandleEnvironmentVariablesOption(func(status map[string]bool) { + result := map[string]interface{}{} + result["message"] = "envconf: show environment variables used by configuration and whether they are set" + result["environment-variables"] = status + b, _ := json.MarshalIndent(result, "", " ") + log.New(w, "", 0).Printf(string(b)) + }) +} From b768b496ecb59bc16e681f9242b128faf7c17897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Fri, 7 Aug 2020 11:04:33 +0800 Subject: [PATCH 4/9] refactor: simplify predefined function - rename some functions --- option.go | 2 +- options/predefined.go | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/option.go b/option.go index 5ca4b01..ed37775 100644 --- a/option.go +++ b/option.go @@ -2,7 +2,7 @@ package envconf type Option func(*loader) -func CustomHandleEnvironmentVariablesOption(cb func(map[string]bool)) Option { +func CustomHandleEnvVarsOption(cb func(map[string]bool)) Option { return func(l *loader) { l.handleEnvironmentVariables = cb } diff --git a/options/predefined.go b/options/predefined.go index b23d6d2..e84db15 100644 --- a/options/predefined.go +++ b/options/predefined.go @@ -4,16 +4,22 @@ import ( "encoding/json" "io" "log" - - "gitlab.rayark.com/backend/envconf" ) -func LogStatusOfEnvironmentVariables(w io.Writer) envconf.Option { - return envconf.CustomHandleEnvironmentVariablesOption(func(status map[string]bool) { +// LogStatusOfEnvVars will log the status of environment variables used by given structure. +// The message is in JSON format. +func LogStatusOfEnvVars(w io.Writer) func(map[string]bool) { + return func(status map[string]bool) { + logger := log.New(w, "", 0) + result := map[string]interface{}{} result["message"] = "envconf: show environment variables used by configuration and whether they are set" result["environment-variables"] = status - b, _ := json.MarshalIndent(result, "", " ") - log.New(w, "", 0).Printf(string(b)) - }) + b, err := json.MarshalIndent(result, "", " ") + if err != nil { + logger.Printf("envconf encounters an error: %v", err.Error()) + return + } + logger.Printf(string(b)) + } } From 5a1b30f0d6ce5e6b4de0e723b28ff9f0c74e934e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Wed, 12 Aug 2020 10:15:10 +0800 Subject: [PATCH 5/9] chore: add comment - add comment for `CustomHandleEnvVarsOption` --- option.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/option.go b/option.go index ed37775..d87bc50 100644 --- a/option.go +++ b/option.go @@ -2,6 +2,11 @@ package envconf type Option func(*loader) +// CustomHandleEnvVarsOption creates an option that calls `cb` with a map +// indicating the environment variables checked by envconf. +// +// The map keys are the environment variable names that are checked by envconf, +// and the values present whether the corresponding environment variables are set. func CustomHandleEnvVarsOption(cb func(map[string]bool)) Option { return func(l *loader) { l.handleEnvironmentVariables = cb From 4a0ebb27eeb130b6aaacb72833c2e2e64a96c974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Wed, 12 Aug 2020 10:29:24 +0800 Subject: [PATCH 6/9] fix: update function name in tests --- envconf_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envconf_test.go b/envconf_test.go index f646d68..39f4a19 100644 --- a/envconf_test.go +++ b/envconf_test.go @@ -326,7 +326,7 @@ func TestLogger(t *testing.T) { StrSlice []string `env:"string_slice"` } `env:",inline"` }{} - Load("TEST", &config, CustomHandleEnvironmentVariablesOption(func(status map[string]bool) { + Load("TEST", &config, CustomHandleEnvVarsOption(func(status map[string]bool) { result = status })) From 994d9123b8833df4257806eaa9778ab77fff8ddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Wed, 12 Aug 2020 10:57:41 +0800 Subject: [PATCH 7/9] test: add test for predefined option --- options/predefined_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 options/predefined_test.go diff --git a/options/predefined_test.go b/options/predefined_test.go new file mode 100644 index 0000000..9231009 --- /dev/null +++ b/options/predefined_test.go @@ -0,0 +1,26 @@ +package options_test + +import ( + "bytes" + "os" + "testing" + + "gitlab.rayark.com/backend/envconf" + "gitlab.rayark.com/backend/envconf/options" +) + +func TestLogger(t *testing.T) { + os.Setenv("TEST_INTEGER", "-3") + os.Setenv("TEST_UNSIGNED_INTEGER", "3") + + config := struct { + String string `env:"string"` + Integer int `env:"integer"` + }{} + buf := bytes.NewBuffer(nil) + envconf.Load("TEST", &config, envconf.CustomHandleEnvVarsOption(options.LogStatusOfEnvVars(buf))) + + if buf.String() == "" { + t.Errorf("failed to log environment variable status") + } +} From b8ff7230cf21db75a3e0f917778b3d676b100aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Wed, 12 Aug 2020 13:27:10 +0800 Subject: [PATCH 8/9] test: fix error logs during tests --- envconf_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/envconf_test.go b/envconf_test.go index 39f4a19..5bdf319 100644 --- a/envconf_test.go +++ b/envconf_test.go @@ -36,6 +36,7 @@ type EmbeddedConfig struct { func assertEqual(t testing.TB, fieldName string, expected, received interface{}) { if !reflect.DeepEqual(expected, received) { + t.Helper() t.Errorf("%v is not loaded correctly.\nExpecting: %v %v\nReceived: %v %v", fieldName, reflect.TypeOf(expected), expected, @@ -45,6 +46,7 @@ func assertEqual(t testing.TB, fieldName string, expected, received interface{}) } func assertPanic(t testing.TB) { if r := recover(); r == nil { + t.Helper() t.Errorf("this test should panic") } } From 76f8d937bec370c7c2cfc1a8fa2628d62485b094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B3=88=E4=BB=81=E5=AE=87?= Date: Wed, 12 Aug 2020 13:33:19 +0800 Subject: [PATCH 9/9] refactor: use structure to present the status of env var --- envconf.go | 32 ++++++++++++++++++++------------ envconf_test.go | 12 +++++++++--- option.go | 2 +- options/predefined.go | 8 +++++--- options/predefined_test.go | 2 +- 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/envconf.go b/envconf.go index ef93563..86a04e3 100644 --- a/envconf.go +++ b/envconf.go @@ -17,8 +17,8 @@ import ( // 2. Duplicated field tags will be assigned with the same environment variable. func Load(prefix string, out interface{}, opts ...Option) { loader := loader{} - loader.envStatus = map[string]bool{} - loader.handleEnvironmentVariables = func(map[string]bool) {} + loader.envStatuses = map[string]*EnvStatus{} + loader.handleEnvironmentVariables = func(map[string]*EnvStatus) {} for _, opt := range opts { opt(&loader) } @@ -26,22 +26,30 @@ func Load(prefix string, out interface{}, opts ...Option) { val := reflect.ValueOf(out).Elem() loader.loadStruct(strings.ToUpper(prefix), &val) - loader.handleEnvironmentVariables(loader.envStatus) + loader.handleEnvironmentVariables(loader.envStatuses) } type loader struct { - handleEnvironmentVariables func(map[string]bool) - envStatus map[string]bool + handleEnvironmentVariables func(map[string]*EnvStatus) + envStatuses map[string]*EnvStatus } func (l *loader) useKey(name string) error { - if _, ok := l.envStatus[name]; ok { + if _, ok := l.envStatuses[name]; ok { return fmt.Errorf("Duplicated key %v", name) } - l.envStatus[name] = false + l.envStatuses[name] = &EnvStatus{} return nil } +type EnvStatus struct { + Loaded bool +} + +func (e *EnvStatus) SetLoaded() { + e.Loaded = true +} + var stringSliceType = reflect.TypeOf([]string{}) func (l *loader) loadField(name string, out *reflect.Value) { @@ -141,7 +149,7 @@ func (l *loader) loadString(name string, out *reflect.Value) { if !found { return } - l.envStatus[name] = true + l.envStatuses[name].SetLoaded() out.SetString(data) } @@ -151,7 +159,7 @@ func (l *loader) loadInt(name string, out *reflect.Value) { if !found { return } - l.envStatus[name] = true + l.envStatuses[name].SetLoaded() d, err := strconv.ParseInt(data, 10, 64) if err != nil { @@ -166,7 +174,7 @@ func (l *loader) loadUint(name string, out *reflect.Value) { if !found { return } - l.envStatus[name] = true + l.envStatuses[name].SetLoaded() d, err := strconv.ParseUint(data, 10, 64) if err != nil { @@ -181,7 +189,7 @@ func (l *loader) loadBool(name string, out *reflect.Value) { if !found { return } - l.envStatus[name] = true + l.envStatuses[name].SetLoaded() isTrue := data == "true" || data == "TRUE" || data == "True" || data == "1" isFalse := data == "false" || data == "FALSE" || data == "False" || data == "0" @@ -198,7 +206,7 @@ func (l *loader) loadStringSlice(name string, out *reflect.Value) { if !found { return } - l.envStatus[name] = true + l.envStatuses[name].SetLoaded() strListRaw := strings.Split(data, ",") strList := []string{} diff --git a/envconf_test.go b/envconf_test.go index 5bdf319..9686689 100644 --- a/envconf_test.go +++ b/envconf_test.go @@ -318,7 +318,7 @@ func TestLogger(t *testing.T) { os.Setenv("TEST_INTEGER", "-3") os.Setenv("TEST_UNSIGNED_INTEGER", "3") - result := map[string]bool{} + result := map[string]*EnvStatus{} config := struct { String string `env:"string"` Integer int `env:"integer"` @@ -328,10 +328,16 @@ func TestLogger(t *testing.T) { StrSlice []string `env:"string_slice"` } `env:",inline"` }{} - Load("TEST", &config, CustomHandleEnvVarsOption(func(status map[string]bool) { + Load("TEST", &config, CustomHandleEnvVarsOption(func(status map[string]*EnvStatus) { result = status })) - expected := map[string]bool{"TEST_STRING": false, "TEST_INTEGER": true, "TEST_UNSIGNED_INTEGER": true, "TEST_BOOL": false, "TEST_STRING_SLICE": false} + expected := map[string]*EnvStatus{ + "TEST_STRING": {false}, + "TEST_INTEGER": {true}, + "TEST_UNSIGNED_INTEGER": {true}, + "TEST_BOOL": {false}, + "TEST_STRING_SLICE": {false}, + } assertEqual(t, "", expected, result) } diff --git a/option.go b/option.go index d87bc50..f661287 100644 --- a/option.go +++ b/option.go @@ -7,7 +7,7 @@ type Option func(*loader) // // The map keys are the environment variable names that are checked by envconf, // and the values present whether the corresponding environment variables are set. -func CustomHandleEnvVarsOption(cb func(map[string]bool)) Option { +func CustomHandleEnvVarsOption(cb func(map[string]*EnvStatus)) Option { return func(l *loader) { l.handleEnvironmentVariables = cb } diff --git a/options/predefined.go b/options/predefined.go index e84db15..d90bb99 100644 --- a/options/predefined.go +++ b/options/predefined.go @@ -4,12 +4,14 @@ import ( "encoding/json" "io" "log" + + "gitlab.rayark.com/backend/envconf" ) -// LogStatusOfEnvVars will log the status of environment variables used by given structure. +// MakeJSONLogger will log the status of environment variables used by given structure. // The message is in JSON format. -func LogStatusOfEnvVars(w io.Writer) func(map[string]bool) { - return func(status map[string]bool) { +func MakeJSONLogger(w io.Writer) func(map[string]*envconf.EnvStatus) { + return func(status map[string]*envconf.EnvStatus) { logger := log.New(w, "", 0) result := map[string]interface{}{} diff --git a/options/predefined_test.go b/options/predefined_test.go index 9231009..13b6703 100644 --- a/options/predefined_test.go +++ b/options/predefined_test.go @@ -18,7 +18,7 @@ func TestLogger(t *testing.T) { Integer int `env:"integer"` }{} buf := bytes.NewBuffer(nil) - envconf.Load("TEST", &config, envconf.CustomHandleEnvVarsOption(options.LogStatusOfEnvVars(buf))) + envconf.Load("TEST", &config, envconf.CustomHandleEnvVarsOption(options.MakeJSONLogger(buf))) if buf.String() == "" { t.Errorf("failed to log environment variable status")