From 64e2f08de091389a06799dd02fee4169ed2858ee Mon Sep 17 00:00:00 2001 From: Morozov Roman Date: Thu, 2 May 2024 10:18:42 +0400 Subject: [PATCH 1/6] errors --- cleanenv.go | 17 +++++------- cleanenv_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ errors.go | 46 +++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 10 deletions(-) create mode 100644 errors.go diff --git a/cleanenv.go b/cleanenv.go index ce0d0d2..1197bc5 100644 --- a/cleanenv.go +++ b/cleanenv.go @@ -428,11 +428,13 @@ func readEnvVars(cfg interface{}, update bool) error { } } + var envName string + if len(meta.envList) > 0 { + envName = meta.envList[0] + } + if rawValue == nil && meta.required && meta.isFieldValueZero() { - return fmt.Errorf( - "field %q is required but the value is not provided", - meta.fieldName, - ) + return newRequireError(meta.fieldName, envName) } if rawValue == nil && meta.isFieldValueZero() { @@ -443,13 +445,8 @@ func readEnvVars(cfg interface{}, update bool) error { continue } - var envName string - if len(meta.envList) > 0 { - envName = meta.envList[0] - } - if err := parseValue(meta.fieldValue, *rawValue, meta.separator, meta.layout); err != nil { - return fmt.Errorf("parsing field %v env %v: %v", meta.fieldName, envName, err) + return newParsingError(meta.fieldName, envName, err) } } diff --git a/cleanenv_test.go b/cleanenv_test.go index 0a13fcb..a92c14a 100644 --- a/cleanenv_test.go +++ b/cleanenv_test.go @@ -2,6 +2,7 @@ package cleanenv import ( "bytes" + "encoding/json" "errors" "fmt" "io" @@ -323,6 +324,77 @@ func TestReadEnvVars(t *testing.T) { } } +func TestReadEnvErrors(t *testing.T) { + type testEnvErrors struct { + Queue struct { + Host string `env:"HOST"` + } `env-prefix:"TEST_ERRORS_"` + Database struct { + Host string `env:"HOST" env-required:"true"` + TTL time.Duration `env:"TTL"` + } `env-prefix:"TEST_ERRORS_DATABASE_"` + } + + tests := []struct { + name string + env map[string]string + cfg interface{} + errorAs interface{} + errorWant interface{} + }{ + { + name: "required error", + env: nil, + cfg: &testEnvErrors{}, + errorAs: RequireError{}, + errorWant: RequireError{ + FieldName: "Host", + EnvName: "TEST_ERRORS_DATABASE_HOST", + }, + }, + { + name: "parsing error", + env: map[string]string{ + "TEST_ERRORS_DATABASE_HOST": "localhost", + "TEST_ERRORS_DATABASE_TTL": "bad-value", + }, + cfg: &testEnvErrors{}, + errorAs: ParsingError{}, + errorWant: ParsingError{ + Err: fmt.Errorf("time: invalid duration \"bad-value\""), + FieldName: "TTL", + EnvName: "TEST_ERRORS_DATABASE_TTL", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for env, val := range tt.env { + os.Setenv(env, val) + } + defer os.Clearenv() + + err := readEnvVars(tt.cfg, false) + if err == nil { + t.Errorf("wrong behavior %v, want error", err) + } + + if !errors.As(err, &tt.errorAs) { + t.Errorf("wrong error as %T, want %T", tt.errorAs, err) + } + + if tt.errorWant != nil && !reflect.DeepEqual(tt.errorAs, tt.errorWant) { + // not using error interface for printing value + bytes1, _ := json.Marshal(tt.errorAs) + bytes2, _ := json.Marshal(tt.errorWant) + + t.Errorf("wrong error data %s, want %s", bytes1, bytes2) + } + }) + } +} + func TestReadEnvVarsURL(t *testing.T) { urlFunc := func(u string) url.URL { parsed, err := url.Parse(u) diff --git a/errors.go b/errors.go new file mode 100644 index 0000000..a9281e4 --- /dev/null +++ b/errors.go @@ -0,0 +1,46 @@ +package cleanenv + +import ( + "fmt" +) + +type RequireError struct { + FieldName string + EnvName string +} + +func newRequireError(fieldName string, envName string) RequireError { + return RequireError{ + FieldName: fieldName, + EnvName: envName, + } +} + +func (r RequireError) Error() string { + return fmt.Sprintf( + "field %q is required but the value is not provided", + r.FieldName, + ) +} + +type ParsingError struct { + Err error + FieldName string + EnvName string +} + +func newParsingError(fieldName string, envName string, err error) ParsingError { + return ParsingError{ + FieldName: fieldName, + EnvName: envName, + Err: err, + } +} + +func (p ParsingError) Error() string { + return fmt.Sprintf("parsing field %v env %v: %v", p.FieldName, p.EnvName, p.Err) +} + +func (p ParsingError) Unwrap() error { + return p.Err +} From 9e9a15904d17ba8802044631e2363cc605f19dff Mon Sep 17 00:00:00 2001 From: Morozov Roman Date: Thu, 2 May 2024 19:31:36 +0400 Subject: [PATCH 2/6] field path --- cleanenv.go | 19 +++++++++++++------ cleanenv_test.go | 1 + errors.go | 18 ++++++++++++++---- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/cleanenv.go b/cleanenv.go index 1197bc5..a7dc346 100644 --- a/cleanenv.go +++ b/cleanenv.go @@ -249,6 +249,7 @@ type structMeta struct { description string updatable bool required bool + path []string } // isFieldValueZero determines if fieldValue empty or not @@ -302,9 +303,10 @@ func readStructMetadata(cfgRoot interface{}) ([]structMeta, error) { type cfgNode struct { Val interface{} Prefix string + Path []string } - cfgStack := []cfgNode{{cfgRoot, ""}} + cfgStack := []cfgNode{{cfgRoot, "", nil}} metas := make([]structMeta, 0) for i := 0; i < len(cfgStack); i++ { @@ -342,7 +344,11 @@ func readStructMetadata(cfgRoot interface{}) ([]structMeta, error) { // add structure to parsing stack if _, found := validStructs[fld.Type()]; !found { prefix, _ := fType.Tag.Lookup(TagEnvPrefix) - cfgStack = append(cfgStack, cfgNode{fld.Addr().Interface(), sPrefix + prefix}) + cfgStack = append(cfgStack, cfgNode{ + Val: fld.Addr().Interface(), + Prefix: sPrefix + prefix, + Path: append(cfgStack[i].Path, fType.Name), + }) continue } @@ -392,6 +398,7 @@ func readStructMetadata(cfgRoot interface{}) ([]structMeta, error) { description: fType.Tag.Get(TagEnvDescription), updatable: upd, required: required, + path: cfgStack[i].Path, }) } @@ -408,7 +415,7 @@ func readEnvVars(cfg interface{}, update bool) error { } if updater, ok := cfg.(Updater); ok { - if err := updater.Update(); err != nil { + if err = updater.Update(); err != nil { return err } } @@ -434,7 +441,7 @@ func readEnvVars(cfg interface{}, update bool) error { } if rawValue == nil && meta.required && meta.isFieldValueZero() { - return newRequireError(meta.fieldName, envName) + return newRequireError(meta.fieldName, meta.path, envName) } if rawValue == nil && meta.isFieldValueZero() { @@ -445,8 +452,8 @@ func readEnvVars(cfg interface{}, update bool) error { continue } - if err := parseValue(meta.fieldValue, *rawValue, meta.separator, meta.layout); err != nil { - return newParsingError(meta.fieldName, envName, err) + if err = parseValue(meta.fieldValue, *rawValue, meta.separator, meta.layout); err != nil { + return newParsingError(meta.fieldName, meta.path, envName, err) } } diff --git a/cleanenv_test.go b/cleanenv_test.go index a92c14a..ad32d4d 100644 --- a/cleanenv_test.go +++ b/cleanenv_test.go @@ -348,6 +348,7 @@ func TestReadEnvErrors(t *testing.T) { cfg: &testEnvErrors{}, errorAs: RequireError{}, errorWant: RequireError{ + FieldPath: []string{"Database"}, FieldName: "Host", EnvName: "TEST_ERRORS_DATABASE_HOST", }, diff --git a/errors.go b/errors.go index a9281e4..c072be2 100644 --- a/errors.go +++ b/errors.go @@ -2,16 +2,19 @@ package cleanenv import ( "fmt" + "strings" ) type RequireError struct { FieldName string + FieldPath []string EnvName string } -func newRequireError(fieldName string, envName string) RequireError { +func newRequireError(fieldName string, fieldPath []string, envName string) RequireError { return RequireError{ FieldName: fieldName, + FieldPath: fieldPath, EnvName: envName, } } @@ -19,26 +22,33 @@ func newRequireError(fieldName string, envName string) RequireError { func (r RequireError) Error() string { return fmt.Sprintf( "field %q is required but the value is not provided", - r.FieldName, + strings.Join(append(r.FieldPath, r.FieldName), "."), ) } type ParsingError struct { Err error FieldName string + FieldPath []string EnvName string } -func newParsingError(fieldName string, envName string, err error) ParsingError { +func newParsingError(fieldName string, fieldPath []string, envName string, err error) ParsingError { return ParsingError{ FieldName: fieldName, + FieldPath: fieldPath, EnvName: envName, Err: err, } } func (p ParsingError) Error() string { - return fmt.Sprintf("parsing field %v env %v: %v", p.FieldName, p.EnvName, p.Err) + return fmt.Sprintf( + "parsing field %v env %v: %v", + strings.Join(append(p.FieldPath, p.FieldName), "."), + p.EnvName, + p.Err, + ) } func (p ParsingError) Unwrap() error { From e269dff6b6fdeec2e2eb30e35b1d375b36b59e89 Mon Sep 17 00:00:00 2001 From: Morozov Roman Date: Thu, 2 May 2024 19:54:44 +0400 Subject: [PATCH 3/6] []string to string for safety append --- cleanenv.go | 8 ++++---- cleanenv_test.go | 6 +++++- errors.go | 15 +++++++-------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/cleanenv.go b/cleanenv.go index a7dc346..0bca32b 100644 --- a/cleanenv.go +++ b/cleanenv.go @@ -249,7 +249,7 @@ type structMeta struct { description string updatable bool required bool - path []string + path string } // isFieldValueZero determines if fieldValue empty or not @@ -303,10 +303,10 @@ func readStructMetadata(cfgRoot interface{}) ([]structMeta, error) { type cfgNode struct { Val interface{} Prefix string - Path []string + Path string } - cfgStack := []cfgNode{{cfgRoot, "", nil}} + cfgStack := []cfgNode{{cfgRoot, "", ""}} metas := make([]structMeta, 0) for i := 0; i < len(cfgStack); i++ { @@ -347,7 +347,7 @@ func readStructMetadata(cfgRoot interface{}) ([]structMeta, error) { cfgStack = append(cfgStack, cfgNode{ Val: fld.Addr().Interface(), Prefix: sPrefix + prefix, - Path: append(cfgStack[i].Path, fType.Name), + Path: fmt.Sprintf("%s%s.", cfgStack[i].Path, fType.Name), }) continue } diff --git a/cleanenv_test.go b/cleanenv_test.go index ad32d4d..67f7b64 100644 --- a/cleanenv_test.go +++ b/cleanenv_test.go @@ -333,6 +333,9 @@ func TestReadEnvErrors(t *testing.T) { Host string `env:"HOST" env-required:"true"` TTL time.Duration `env:"TTL"` } `env-prefix:"TEST_ERRORS_DATABASE_"` + ThirdStruct struct { + Host string `env:"HOST"` + } `env-prefix:"TEST_ERRORS_THIRD_"` } tests := []struct { @@ -348,7 +351,7 @@ func TestReadEnvErrors(t *testing.T) { cfg: &testEnvErrors{}, errorAs: RequireError{}, errorWant: RequireError{ - FieldPath: []string{"Database"}, + FieldPath: "Database.", FieldName: "Host", EnvName: "TEST_ERRORS_DATABASE_HOST", }, @@ -364,6 +367,7 @@ func TestReadEnvErrors(t *testing.T) { errorWant: ParsingError{ Err: fmt.Errorf("time: invalid duration \"bad-value\""), FieldName: "TTL", + FieldPath: "Database.", EnvName: "TEST_ERRORS_DATABASE_TTL", }, }, diff --git a/errors.go b/errors.go index c072be2..ada6b5c 100644 --- a/errors.go +++ b/errors.go @@ -2,16 +2,15 @@ package cleanenv import ( "fmt" - "strings" ) type RequireError struct { FieldName string - FieldPath []string + FieldPath string EnvName string } -func newRequireError(fieldName string, fieldPath []string, envName string) RequireError { +func newRequireError(fieldName string, fieldPath string, envName string) RequireError { return RequireError{ FieldName: fieldName, FieldPath: fieldPath, @@ -22,18 +21,18 @@ func newRequireError(fieldName string, fieldPath []string, envName string) Requi func (r RequireError) Error() string { return fmt.Sprintf( "field %q is required but the value is not provided", - strings.Join(append(r.FieldPath, r.FieldName), "."), + r.FieldPath+r.FieldName, ) } type ParsingError struct { Err error FieldName string - FieldPath []string + FieldPath string EnvName string } -func newParsingError(fieldName string, fieldPath []string, envName string, err error) ParsingError { +func newParsingError(fieldName string, fieldPath string, envName string, err error) ParsingError { return ParsingError{ FieldName: fieldName, FieldPath: fieldPath, @@ -44,8 +43,8 @@ func newParsingError(fieldName string, fieldPath []string, envName string, err e func (p ParsingError) Error() string { return fmt.Sprintf( - "parsing field %v env %v: %v", - strings.Join(append(p.FieldPath, p.FieldName), "."), + "parsing field %q env %q: %v", + p.FieldPath+p.FieldName, p.EnvName, p.Err, ) From d9c82a109a90b630e09e35ade5cf3e10a46731c4 Mon Sep 17 00:00:00 2001 From: Morozov Roman Date: Fri, 3 May 2024 10:30:27 +0400 Subject: [PATCH 4/6] test more levels --- cleanenv_test.go | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/cleanenv_test.go b/cleanenv_test.go index 67f7b64..5c0f957 100644 --- a/cleanenv_test.go +++ b/cleanenv_test.go @@ -325,7 +325,11 @@ func TestReadEnvVars(t *testing.T) { } func TestReadEnvErrors(t *testing.T) { - type testEnvErrors struct { + type testOneLevel struct { + Host string `env:"HOST" env-required:"true"` + } + + type testTwoLevels struct { Queue struct { Host string `env:"HOST"` } `env-prefix:"TEST_ERRORS_"` @@ -338,6 +342,14 @@ func TestReadEnvErrors(t *testing.T) { } `env-prefix:"TEST_ERRORS_THIRD_"` } + type testThreeLevels struct { + Database struct { + URL struct { + Host string `env:"HOST" env-required:"true"` + } + } + } + tests := []struct { name string env map[string]string @@ -346,9 +358,31 @@ func TestReadEnvErrors(t *testing.T) { errorWant interface{} }{ { - name: "required error", + name: "required error - one level", + env: nil, + cfg: &testOneLevel{}, + errorAs: RequireError{}, + errorWant: RequireError{ + FieldPath: "", + FieldName: "Host", + EnvName: "HOST", + }, + }, + { + name: "required error - three levels", + env: nil, + cfg: &testThreeLevels{}, + errorAs: RequireError{}, + errorWant: RequireError{ + FieldPath: "Database.URL.", + FieldName: "Host", + EnvName: "HOST", + }, + }, + { + name: "required error - two levels", env: nil, - cfg: &testEnvErrors{}, + cfg: &testTwoLevels{}, errorAs: RequireError{}, errorWant: RequireError{ FieldPath: "Database.", @@ -362,7 +396,7 @@ func TestReadEnvErrors(t *testing.T) { "TEST_ERRORS_DATABASE_HOST": "localhost", "TEST_ERRORS_DATABASE_TTL": "bad-value", }, - cfg: &testEnvErrors{}, + cfg: &testTwoLevels{}, errorAs: ParsingError{}, errorWant: ParsingError{ Err: fmt.Errorf("time: invalid duration \"bad-value\""), From 46c18dfaf9ef6332d7758d1bf3a2ac95bfba0746 Mon Sep 17 00:00:00 2001 From: Ilya Kaznacheev Date: Sun, 5 Jan 2025 22:48:31 +0100 Subject: [PATCH 5/6] Simplify error handling by removing error types Signed-off-by: Ilya Kaznacheev --- cleanenv.go | 12 +++++--- cleanenv_test.go | 74 ++++++++++++++---------------------------------- errors.go | 55 ----------------------------------- 3 files changed, 30 insertions(+), 111 deletions(-) delete mode 100644 errors.go diff --git a/cleanenv.go b/cleanenv.go index 0bca32b..1e2577e 100644 --- a/cleanenv.go +++ b/cleanenv.go @@ -49,7 +49,7 @@ const ( // TagEnvRequired flag to mark a field as required TagEnvRequired = "env-required" - // TagEnvPrefix аlag to specify prefix for structure fields + // TagEnvPrefix flag to specify prefix for structure fields TagEnvPrefix = "env-prefix" ) @@ -113,7 +113,7 @@ func UpdateEnv(cfg interface{}) error { return readEnvVars(cfg, true) } -// parseFile parses configuration file according to it's extension +// parseFile parses configuration file according to its extension // // Currently following file extensions are supported: // @@ -441,7 +441,9 @@ func readEnvVars(cfg interface{}, update bool) error { } if rawValue == nil && meta.required && meta.isFieldValueZero() { - return newRequireError(meta.fieldName, meta.path, envName) + return fmt.Errorf("field %q is required but the value is not provided", + meta.path+meta.fieldName, + ) } if rawValue == nil && meta.isFieldValueZero() { @@ -453,7 +455,9 @@ func readEnvVars(cfg interface{}, update bool) error { } if err = parseValue(meta.fieldValue, *rawValue, meta.separator, meta.layout); err != nil { - return newParsingError(meta.fieldName, meta.path, envName, err) + return fmt.Errorf("parsing field %q env %q: %v", + meta.path+meta.fieldName, envName, err, + ) } } diff --git a/cleanenv_test.go b/cleanenv_test.go index 5c0f957..c2f0fba 100644 --- a/cleanenv_test.go +++ b/cleanenv_test.go @@ -2,7 +2,6 @@ package cleanenv import ( "bytes" - "encoding/json" "errors" "fmt" "io" @@ -351,44 +350,28 @@ func TestReadEnvErrors(t *testing.T) { } tests := []struct { - name string - env map[string]string - cfg interface{} - errorAs interface{} - errorWant interface{} + name string + env map[string]string + cfg interface{} + expectedError string }{ { - name: "required error - one level", - env: nil, - cfg: &testOneLevel{}, - errorAs: RequireError{}, - errorWant: RequireError{ - FieldPath: "", - FieldName: "Host", - EnvName: "HOST", - }, + name: "required error - one level", + env: nil, + cfg: &testOneLevel{}, + expectedError: `field "Host" is required but the value is not provided`, }, { - name: "required error - three levels", - env: nil, - cfg: &testThreeLevels{}, - errorAs: RequireError{}, - errorWant: RequireError{ - FieldPath: "Database.URL.", - FieldName: "Host", - EnvName: "HOST", - }, + name: "required error - two levels", + env: nil, + cfg: &testTwoLevels{}, + expectedError: `field "Database.Host" is required but the value is not provided`, }, { - name: "required error - two levels", - env: nil, - cfg: &testTwoLevels{}, - errorAs: RequireError{}, - errorWant: RequireError{ - FieldPath: "Database.", - FieldName: "Host", - EnvName: "TEST_ERRORS_DATABASE_HOST", - }, + name: "required error - three levels", + env: nil, + cfg: &testThreeLevels{}, + expectedError: `field "Database.URL.Host" is required but the value is not provided`, }, { name: "parsing error", @@ -396,14 +379,8 @@ func TestReadEnvErrors(t *testing.T) { "TEST_ERRORS_DATABASE_HOST": "localhost", "TEST_ERRORS_DATABASE_TTL": "bad-value", }, - cfg: &testTwoLevels{}, - errorAs: ParsingError{}, - errorWant: ParsingError{ - Err: fmt.Errorf("time: invalid duration \"bad-value\""), - FieldName: "TTL", - FieldPath: "Database.", - EnvName: "TEST_ERRORS_DATABASE_TTL", - }, + cfg: &testTwoLevels{}, + expectedError: `parsing field "Database.TTL" env "TEST_ERRORS_DATABASE_TTL": time: invalid duration "bad-value"`, }, } @@ -415,20 +392,13 @@ func TestReadEnvErrors(t *testing.T) { defer os.Clearenv() err := readEnvVars(tt.cfg, false) - if err == nil { - t.Errorf("wrong behavior %v, want error", err) - } - if !errors.As(err, &tt.errorAs) { - t.Errorf("wrong error as %T, want %T", tt.errorAs, err) + if err == nil { + t.Fatalf("expected error but got nil") } - if tt.errorWant != nil && !reflect.DeepEqual(tt.errorAs, tt.errorWant) { - // not using error interface for printing value - bytes1, _ := json.Marshal(tt.errorAs) - bytes2, _ := json.Marshal(tt.errorWant) - - t.Errorf("wrong error data %s, want %s", bytes1, bytes2) + if err.Error() != tt.expectedError { + t.Errorf("unexpected error message: got %q, want %q", err.Error(), tt.expectedError) } }) } diff --git a/errors.go b/errors.go deleted file mode 100644 index ada6b5c..0000000 --- a/errors.go +++ /dev/null @@ -1,55 +0,0 @@ -package cleanenv - -import ( - "fmt" -) - -type RequireError struct { - FieldName string - FieldPath string - EnvName string -} - -func newRequireError(fieldName string, fieldPath string, envName string) RequireError { - return RequireError{ - FieldName: fieldName, - FieldPath: fieldPath, - EnvName: envName, - } -} - -func (r RequireError) Error() string { - return fmt.Sprintf( - "field %q is required but the value is not provided", - r.FieldPath+r.FieldName, - ) -} - -type ParsingError struct { - Err error - FieldName string - FieldPath string - EnvName string -} - -func newParsingError(fieldName string, fieldPath string, envName string, err error) ParsingError { - return ParsingError{ - FieldName: fieldName, - FieldPath: fieldPath, - EnvName: envName, - Err: err, - } -} - -func (p ParsingError) Error() string { - return fmt.Sprintf( - "parsing field %q env %q: %v", - p.FieldPath+p.FieldName, - p.EnvName, - p.Err, - ) -} - -func (p ParsingError) Unwrap() error { - return p.Err -} From f7cb9dde8217d64eaaee086c4831cb4d085cc231 Mon Sep 17 00:00:00 2001 From: Ilya Kaznacheev Date: Sun, 5 Jan 2025 23:57:37 +0100 Subject: [PATCH 6/6] Bump go version Signed-off-by: Ilya Kaznacheev --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f35d0d1..29d7223 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,13 +15,13 @@ jobs: strategy: matrix: go-version: + - '1.23' + - '1.22' - '1.21' - '1.20' - '1.19' - '1.18' - '1.17' - - '1.16' - - '1.15' os: - ubuntu-latest - macos-latest