From 7a080ada5ba072a91fd7f677bcf4d0e1aa286d39 Mon Sep 17 00:00:00 2001 From: joeybloggs Date: Thu, 14 Jul 2016 16:46:21 -0400 Subject: [PATCH] Fix issue with omitempty combined with nullable types in combination with a default static value set as the value fix for issue-#249 --- README.md | 2 +- util.go | 34 ++++++++++++++++++++-------------- validator.go | 5 +++-- validator_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index dd2a4f4..21693f6 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Package validator ================ [![Join the chat at https://gitter.im/bluesuncorp/validator](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/go-playground/validator?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) -![Project status](https://img.shields.io/badge/version-8.17.1-green.svg) +![Project status](https://img.shields.io/badge/version-8.17.3-green.svg) [![Build Status](https://semaphoreci.com/api/v1/projects/ec20115f-ef1b-4c7d-9393-cc76aba74eb4/530054/badge.svg)](https://semaphoreci.com/joeybloggs/validator) [![Coverage Status](https://coveralls.io/repos/go-playground/validator/badge.svg?branch=v8&service=github)](https://coveralls.io/github/go-playground/validator?branch=v8) [![Go Report Card](http://goreportcard.com/badge/go-playground/validator)](http://goreportcard.com/report/go-playground/validator) diff --git a/util.go b/util.go index ce01c7c..417a9fe 100644 --- a/util.go +++ b/util.go @@ -37,48 +37,54 @@ var ( // it is exposed for use within you Custom Functions func (v *Validate) ExtractType(current reflect.Value) (reflect.Value, reflect.Kind) { + val, k, _ := v.extractTypeInternal(current, false) + return val, k +} + +// only exists to not break backward compatibility, needed to return the third param for a bug fix internally +func (v *Validate) extractTypeInternal(current reflect.Value, nullable bool) (reflect.Value, reflect.Kind, bool) { + switch current.Kind() { case reflect.Ptr: + nullable = true + if current.IsNil() { - return current, reflect.Ptr + return current, reflect.Ptr, nullable } - return v.ExtractType(current.Elem()) + return v.extractTypeInternal(current.Elem(), nullable) case reflect.Interface: + nullable = true + if current.IsNil() { - return current, reflect.Interface + return current, reflect.Interface, nullable } - return v.ExtractType(current.Elem()) + return v.extractTypeInternal(current.Elem(), nullable) case reflect.Invalid: - return current, reflect.Invalid + return current, reflect.Invalid, nullable default: if v.hasCustomFuncs { - // fmt.Println("Type", current.Type()) - if fn, ok := v.customTypeFuncs[current.Type()]; ok { - - // fmt.Println("OK") - return v.ExtractType(reflect.ValueOf(fn(current))) + if fn, ok := v.customTypeFuncs[current.Type()]; ok { + return v.extractTypeInternal(reflect.ValueOf(fn(current)), nullable) } - - // fmt.Println("NOT OK") } - return current, current.Kind() + return current, current.Kind(), nullable } } // GetStructFieldOK traverses a struct to retrieve a specific field denoted by the provided namespace and // returns the field, field kind and whether is was successful in retrieving the field at all. // NOTE: when not successful ok will be false, this can happen when a nested struct is nil and so the field -// could not be retrived because it didnt exist. +// could not be retrived because it didn't exist. func (v *Validate) GetStructFieldOK(current reflect.Value, namespace string) (reflect.Value, reflect.Kind, bool) { current, kind := v.ExtractType(current) diff --git a/validator.go b/validator.go index 0e6db7e..2df68f0 100644 --- a/validator.go +++ b/validator.go @@ -586,7 +586,7 @@ func (v *Validate) traverseField(topStruct reflect.Value, currentStruct reflect. } } - current, kind := v.ExtractType(current) + current, kind, nullable := v.extractTypeInternal(current, false) var typ reflect.Type switch kind { @@ -671,9 +671,10 @@ func (v *Validate) traverseField(topStruct reflect.Value, currentStruct reflect. if valTag.tagVals[0][0] == omitempty { - if !HasValue(v, topStruct, currentStruct, current, typ, kind, blank) { + if !nullable && !HasValue(v, topStruct, currentStruct, current, typ, kind, blank) { return } + continue } diff --git a/validator_test.go b/validator_test.go index d4a8745..c3e746d 100644 --- a/validator_test.go +++ b/validator_test.go @@ -5775,3 +5775,48 @@ func TestCustomFieldName(t *testing.T) { Equal(t, errs["A.D"].Name, "D") Equal(t, errs["A.E"].Name, "E") } + +// Thanks @robbrockbank, see https://github.com/go-playground/validator/issues/249 +func TestPointerAndOmitEmpty(t *testing.T) { + + type Test struct { + MyInt *int `validate:"omitempty,gte=2,lte=255"` + } + + val1 := 0 + val2 := 256 + + t1 := Test{MyInt: &val1} // This should fail validation on gte because value is 0 + t2 := Test{MyInt: &val2} // This should fail validate on lte because value is 256 + t3 := Test{MyInt: nil} // This should succeed validation because pointer is nil + + errs := validate.Struct(t1) + NotEqual(t, errs, nil) + AssertError(t, errs, "Test.MyInt", "MyInt", "gte") + + errs = validate.Struct(t2) + NotEqual(t, errs, nil) + AssertError(t, errs, "Test.MyInt", "MyInt", "lte") + + errs = validate.Struct(t3) + Equal(t, errs, nil) + + type TestIface struct { + MyInt interface{} `validate:"omitempty,gte=2,lte=255"` + } + + ti1 := TestIface{MyInt: &val1} // This should fail validation on gte because value is 0 + ti2 := TestIface{MyInt: &val2} // This should fail validate on lte because value is 256 + ti3 := TestIface{MyInt: nil} // This should succeed validation because pointer is nil + + errs = validate.Struct(ti1) + NotEqual(t, errs, nil) + AssertError(t, errs, "TestIface.MyInt", "MyInt", "gte") + + errs = validate.Struct(ti2) + NotEqual(t, errs, nil) + AssertError(t, errs, "TestIface.MyInt", "MyInt", "lte") + + errs = validate.Struct(ti3) + Equal(t, errs, nil) +}