Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nonzero validator to not validate nil array or slices. #148

Merged
merged 5 commits into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
### Removed

### Fixed
- Fixed nonzero validator to not fail on nil array or slice. #147
- Fixed nonzero validator to validate maps.
- Fixed required validator to validate maps.

## [0.8.1]

Expand Down
8 changes: 8 additions & 0 deletions error.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ var (
ErrRequired = errors.New("missing required field")

ErrEmpty = errors.New("empty field")

ErrArrayEmpty = errors.New("empty array")

ErrMapEmpty = errors.New("empty map")

ErrRegexEmpty = errors.New("regex value is not set")

ErrStringEmpty = errors.New("string value is not set")
)

// Error Classes
Expand Down
13 changes: 8 additions & 5 deletions reify.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func reifyInto(opts *options, to reflect.Value, from *Config) Error {

switch k {
case reflect.Map:
return reifyMap(opts, to, from)
return reifyMap(opts, to, from, nil)
case reflect.Struct:
return reifyStruct(opts, to, from)
case reflect.Slice, reflect.Array:
Expand All @@ -183,7 +183,7 @@ func reifyInto(opts *options, to reflect.Value, from *Config) Error {
return raiseInvalidTopLevelType(to.Interface(), opts.meta)
}

func reifyMap(opts *options, to reflect.Value, from *Config) Error {
func reifyMap(opts *options, to reflect.Value, from *Config, validators []validatorTag) Error {
parentFields := opts.activeFields
defer func() { opts.activeFields = parentFields }()

Expand All @@ -198,7 +198,7 @@ func reifyMap(opts *options, to reflect.Value, from *Config) Error {

fields := from.fields.dict()
if len(fields) == 0 {
if err := tryRecursiveValidate(to, opts, nil); err != nil {
if err := tryRecursiveValidate(to, opts, validators); err != nil {
return raiseValidation(from.ctx, from.metadata, "", err)
}
return nil
Expand All @@ -224,6 +224,9 @@ func reifyMap(opts *options, to reflect.Value, from *Config) Error {
to.SetMapIndex(key, v)
}

if err := runValidators(to.Interface(), validators); err != nil {
return raiseValidation(from.ctx, from.metadata, "", err)
}
if err := tryValidate(to); err != nil {
return raiseValidation(from.ctx, from.metadata, "", err)
}
Expand Down Expand Up @@ -321,7 +324,7 @@ func reifyGetField(
}

// Primitive types return early when it doesn't implement the Initializer interface.
if fieldType.Kind() != reflect.Map && fieldType.Kind() != reflect.Struct && !hasInitDefaults(fieldType) {
if fieldType.Kind() != reflect.Struct && !hasInitDefaults(fieldType) {
if err := tryRecursiveValidate(to, opts.opts, opts.validators); err != nil {
return raiseValidation(cfg.ctx, cfg.metadata, name, err)
}
Expand Down Expand Up @@ -479,7 +482,7 @@ func reifyMergeValue(
if err != nil {
return reflect.Value{}, raiseExpectedObject(opts.opts, val)
}
return old, reifyMap(opts.opts, old, sub)
return old, reifyMap(opts.opts, old, sub, opts.validators)

case reflect.Struct:
sub, err := val.toConfig(opts.opts)
Expand Down
34 changes: 28 additions & 6 deletions validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,31 +392,53 @@ func validateRequired(v interface{}, name string) error {
}
return nil
}
if err := validateNonEmpty(v, name); err != nil {
return ErrRequired
if err := validateNonEmptyWithAllowNil(v, name, false); err != nil {
return err
}
return nil
}

func validateNonEmpty(v interface{}, _ string) error {
func validateNonEmpty(v interface{}, name string) error {
return validateNonEmptyWithAllowNil(v, name, true)
}

func validateNonEmptyWithAllowNil(v interface{}, _ string, allowNil bool) error {
if s, ok := v.(string); ok {
if s == "" {
return ErrEmpty
return ErrStringEmpty
}
return nil
}

if r, ok := v.(regexp.Regexp); ok {
if r.String() == "" {
return ErrEmpty
return ErrRegexEmpty
}
return nil
}

val := reflect.ValueOf(v)
if val.Kind() == reflect.Array || val.Kind() == reflect.Slice {
if val.IsNil() {
if allowNil {
return nil
}
return ErrRequired
}
if val.Len() == 0 {
return ErrArrayEmpty
}
return nil
}
if val.Kind() == reflect.Map {
if val.IsNil() {
if allowNil {
return nil
}
return ErrRequired
}
if val.Len() == 0 {
return ErrEmpty
return ErrMapEmpty
}
return nil
}
Expand Down
83 changes: 71 additions & 12 deletions validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,12 @@ func TestValidationPass(t *testing.T) {
}{
X: &nestedNestedPtrStructValidator{},
},
&struct {
X []string `validate:"nonzero"` // field not present in config, nonzero (no error)
}{},
&struct {
X map[string]string `validate:"nonzero"` // field not present in config, nonzero (no error)
}{},
}

for i, test := range tests {
Expand Down Expand Up @@ -539,6 +545,7 @@ func TestValidateRequiredFailing(t *testing.T) {
"b": "",
"c": nil,
"d": []string{},
"f": map[string]string{},
})

tests := []struct {
Expand All @@ -552,7 +559,7 @@ func TestValidateRequiredFailing(t *testing.T) {
{ErrRequired, &struct {
A int `validate:"required"`
}{}},
{ErrRequired, &struct {
{ErrStringEmpty, &struct {
A string `validate:"required"`
}{}},
{ErrRequired, &struct {
Expand All @@ -563,13 +570,13 @@ func TestValidateRequiredFailing(t *testing.T) {
}{}},

// Access empty string field "b"
{ErrRequired, &struct {
{ErrStringEmpty, &struct {
B string `validate:"required"`
}{}},
{ErrRequired, &struct {
{ErrStringEmpty, &struct {
B *string `validate:"required"`
}{}},
{ErrRequired, &struct {
{ErrRegexEmpty, &struct {
B *regexp.Regexp `validate:"required"`
}{}},

Expand All @@ -580,7 +587,7 @@ func TestValidateRequiredFailing(t *testing.T) {
{ErrRequired, &struct {
C int `validate:"required"`
}{}},
{ErrRequired, &struct {
{ErrStringEmpty, &struct {
C string `validate:"required"`
}{}},
{ErrRequired, &struct {
Expand All @@ -590,10 +597,29 @@ func TestValidateRequiredFailing(t *testing.T) {
C time.Duration `validate:"required"`
}{}},

// Check empty []string field 'd'
{ErrRequired, &struct {
// Check required []string field 'd'
{ErrArrayEmpty, &struct {
D []string `validate:"required"`
}{}},

// Check required []string filed 'e' not in config (pre-initialized)
{ErrArrayEmpty, &struct {
E []string `validate:"required"`
}{
E: []string{},
}},

// Check empty map[string]string field 'f'
{ErrMapEmpty, &struct {
F map[string]string `validate:"required"`
}{}},

// Check required map[string] 'g' not in config (pre-initialized)
{ErrMapEmpty, &struct {
G map[string]string `validate:"required"`
}{
G: map[string]string{},
}},
}

for i, test := range tests {
Expand All @@ -616,6 +642,7 @@ func TestValidateNonzeroFailing(t *testing.T) {
"i": 0,
"s": "",
"a": []int{},
"c": map[string]string{},
})

tests := []struct {
Expand Down Expand Up @@ -663,23 +690,55 @@ func TestValidateNonzeroFailing(t *testing.T) {
}{}},

// test string types accessing 's'
{ErrEmpty, &struct {
{ErrStringEmpty, &struct {
S string `validate:"nonzero"`
}{}},
{ErrEmpty, &struct {
{ErrStringEmpty, &struct {
S *string `validate:"nonzero"`
}{}},
{ErrEmpty, &struct {
{ErrRegexEmpty, &struct {
S *regexp.Regexp `validate:"nonzero"`
}{}},

// test array type accessing 'a'
{ErrEmpty, &struct {
{ErrArrayEmpty, &struct {
A []int `validate:"nonzero"`
}{}},
{ErrEmpty, &struct {
{ErrArrayEmpty, &struct {
A []uint8 `validate:"nonzero"`
}{}},

// test array type accessing 'b' (pre-initialized)
{ErrArrayEmpty, &struct {
B []int `validate:"nonzero"`
}{
B: []int{},
}},
{ErrArrayEmpty, &struct {
B []uint8 `validate:"nonzero"`
}{
B: []uint8{},
}},

// test array type accessing 'c'
{ErrMapEmpty, &struct {
C map[string]string `validate:"nonzero"`
}{}},
{ErrMapEmpty, &struct {
C map[string]string `validate:"nonzero"`
}{}},

// test array type accessing 'd' (pre-initialized)
{ErrMapEmpty, &struct {
D map[string]string `validate:"nonzero"`
}{
D: map[string]string{},
}},
{ErrMapEmpty, &struct {
D map[string]string `validate:"nonzero"`
}{
D: map[string]string{},
}},
}

for i, test := range tests {
Expand Down