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.

Sign up for GitHub

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

Jump to bottom

Closes #660 added excluded with and without #664

Merged
merged 3 commits into from Sep 27, 2020
Merged

Closes #660 added excluded with and without #664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
742006d
Closes #660 added excluded with and without
Sep 15, 2020
490e253
Closes #660 added excluded with and without tests
Sep 15, 2020
ca0533b
Closes #660 readme updates
Sep 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Expand Up @@ -201,6 +201,10 @@ Baked-in Validations
| required_with_all | Required With All |
| required_without | Required Without |
| required_without_all | Required Without All |
| excluded_with | Excluded With |
| excluded_with_all | Excluded With All |
| excluded_without | Excluded Without |
| excluded_without_all | Excluded Without All |
| unique | Unique |

Benchmarks
Expand Down
49 changes: 49 additions & 0 deletions baked_in.go
View file
Open in desktop
Expand Up @@ -68,6 +68,10 @@ var (
"required_with_all": requiredWithAll,
"required_without": requiredWithout,
"required_without_all": requiredWithoutAll,
"excluded_with": excludedWith,
"excluded_with_all": excludedWithAll,
"excluded_without": excludedWithout,
"excluded_without_all": excludedWithoutAll,
"isdefault": isDefault,
"len": hasLengthOf,
"min": hasMinOf,
Expand Down Expand Up @@ -1383,6 +1387,18 @@ func requireCheckFieldKind(fl FieldLevel, param string, defaultNotFoundValue boo
}
}

// ExcludedWith is the validation function
// The field under validation must not be present or is empty if any of the other specified fields are present.
func excludedWith(fl FieldLevel) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to use excluded_with and not getting the expected behaviour. I only passes the validation if I set all the fields with the excluded_with validation, which is kind of the opposite of what I expected. Example:

type Example struct {
	AAA  *aaaStruct  `validate:"excluded_with=BBB,omitempty,dive" json:"aaa,omitempty"`
	BBB  *bbbStruct  `validate:"excluded_with=AAA,omitempty,dive" json:"bbb,omitempty"`
}

If I set AAA, I get

"Key: 'Example.BBB' Error:Field validation for 'BBB' failed on the 'excluded_with' tag"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a play we could look at? Can you simplify it further (dive, omitempty) to expose the issue?

/cc @yaliv maybe you can spot the mistake?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll try to confirm it and make sure I'm not messing up somewhere. I'll get back to you tomorrow!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excluded_with validator works fine with basic/primitive fields (string, int).

I never set any validations for the sub-structs, because in my case I use json.Unmarshal() before validate.
Using json.Unmarshal() will always populate the sub-structs even if they do not present in the source JSON bytes.

This leads me to ask a fundamental question: "What is an empty struct value?"

  • Is it nil?
  • Is it struct{} (with zero fields)?

Unlike map[string]interface{}, which can hold "dynamic" fields, struct has a static set of fields.
If I define a struct with some fields in it (e.g. MyStruct), and then I create a struct value from it with its default (e.g. var test MyStruct), I still have access to those fields.

@jfrancisco0 if the built-in validators do not fit with your case, you can try creating some struct-level validations (see example). Plus, using this technique can minimize overhead, because you can validate multiple fields at once.

Copy link

@andig andig Oct 7, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yaliv in this case the struct pointers are nil and hence definitely empty. The source code/docs confirm this. This is similar to a string that is treated as „not existing“ when it has the zero value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm though that a field of type bool does not fail the validation, giving some strength to the hypothesis of structs behaving differently.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andig if I write just var test *MyStruct, then test would be nil, but if I write var test MyStruct, then &test would not be nil.
So struct pointers have possible nil value.

Yes, I can see that fields with struct type and struct pointer type have a complete opposite behavior against primitive types.
Using the excluded_with validator on 2 fields, each mentioning each other:

  • string type: both have values --> 2 validation errors; one has value --> validation passed 😃
  • struct type: they always have values --> validation passed ☹️
  • struct pointer type: both have values --> validation passed; one has value --> 1 validation error; both empty --> 2 validation errors ☹️

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so to summarise: there is an issue with struct pointers and excluded* validations. Sounds like it would be good to open a new issue with minimal example to reproduce. Might also be good to check required* behaviour for struct pointers too, as the pr code was a close copy.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yaliv did you by chance open follow-up issue?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andig sorry, I haven't done it yet.
I see this validator is quite complex, so I'm not sure if the problem is fixed it will not break other parts (e.g. the primitive types) or it will not cost performance.
But yes, it's still a good issue to be posted. Before it is fixed, maybe someone has a running workaround 😉

params := parseOneOfParam2(fl.Param())
for _, param := range params {
if !requireCheckFieldKind(fl, param, true) {
return !hasValue(fl)
}
}
return true
}

// RequiredWith is the validation function
// The field under validation must be present and not empty only if any of the other specified fields are present.
func requiredWith(fl FieldLevel) bool {
Expand All @@ -1395,6 +1411,18 @@ func requiredWith(fl FieldLevel) bool {
return true
}

// ExcludedWithAll is the validation function
// The field under validation must not be present or is empty if all of the other specified fields are present.
func excludedWithAll(fl FieldLevel) bool {
params := parseOneOfParam2(fl.Param())
for _, param := range params {
if requireCheckFieldKind(fl, param, true) {
return true
}
}
return !hasValue(fl)
}

// RequiredWithAll is the validation function
// The field under validation must be present and not empty only if all of the other specified fields are present.
func requiredWithAll(fl FieldLevel) bool {
Expand All @@ -1407,6 +1435,15 @@ func requiredWithAll(fl FieldLevel) bool {
return hasValue(fl)
}

// ExcludedWithout is the validation function
// The field under validation must not be present or is empty when any of the other specified fields are not present.
func excludedWithout(fl FieldLevel) bool {
if requireCheckFieldKind(fl, strings.TrimSpace(fl.Param()), true) {
return !hasValue(fl)
}
return true
}

// RequiredWithout is the validation function
// The field under validation must be present and not empty only when any of the other specified fields are not present.
func requiredWithout(fl FieldLevel) bool {
Expand All @@ -1416,6 +1453,18 @@ func requiredWithout(fl FieldLevel) bool {
return true
}

// RequiredWithoutAll is the validation function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ExcludedWithoutAll.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could open a PR ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll do it tomorrow :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#673

// The field under validation must not be present or is empty when all of the other specified fields are not present.
func excludedWithoutAll(fl FieldLevel) bool {
params := parseOneOfParam2(fl.Param())
for _, param := range params {
if !requireCheckFieldKind(fl, param, true) {
return true
}
}
return !hasValue(fl)
}

// RequiredWithoutAll is the validation function
// The field under validation must be present and not empty only when all of the other specified fields are not present.
func requiredWithoutAll(fl FieldLevel) bool {
Expand Down
262 changes: 262 additions & 0 deletions validator_test.go
View file
Open in desktop
Expand Up @@ -8750,6 +8750,268 @@ func TestRequiredWith(t *testing.T) {
AssertError(t, errs, "Field6", "Field6", "Field6", "Field6", "required_with")
}

func TestExcludedWith(t *testing.T) {
type Inner struct {
FieldE string
Field *string
}

fieldVal := "test"
test := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_with=FieldE" json:"field_1"`
Field2 *string `validate:"excluded_with=FieldE" json:"field_2"`
Field3 map[string]string `validate:"excluded_with=FieldE" json:"field_3"`
Field4 interface{} `validate:"excluded_with=FieldE" json:"field_4"`
Field5 string `validate:"excluded_with=Inner.FieldE" json:"field_5"`
Field6 string `validate:"excluded_with=Inner2.FieldE" json:"field_6"`
}{
Inner: &Inner{Field: &fieldVal},
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
Field6: "test",
}

validate := New()

errs := validate.Struct(test)
Equal(t, errs, nil)

test2 := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_with=Field" json:"field_1"`
Field2 *string `validate:"excluded_with=Field" json:"field_2"`
Field3 map[string]string `validate:"excluded_with=Field" json:"field_3"`
Field4 interface{} `validate:"excluded_with=Field" json:"field_4"`
Field5 string `validate:"excluded_with=Inner.Field" json:"field_5"`
Field6 string `validate:"excluded_with=Inner2.Field" json:"field_6"`
}{
Inner: &Inner{Field: &fieldVal},
Field: "populated",
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
Field6: "test",
}

errs = validate.Struct(test2)
NotEqual(t, errs, nil)

ve := errs.(ValidationErrors)
Equal(t, len(ve), 5)
for i := 1; i <= 5; i++ {
name := fmt.Sprintf("Field%d", i)
AssertError(t, errs, name, name, name, name, "excluded_with")
}
}

func TestExcludedWithout(t *testing.T) {
type Inner struct {
FieldE string
Field *string
}

fieldVal := "test"
test := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_without=Field" json:"field_1"`
Field2 *string `validate:"excluded_without=Field" json:"field_2"`
Field3 map[string]string `validate:"excluded_without=Field" json:"field_3"`
Field4 interface{} `validate:"excluded_without=Field" json:"field_4"`
Field5 string `validate:"excluded_without=Inner.Field" json:"field_5"`
}{
Inner: &Inner{Field: &fieldVal},
Field: "populated",
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
}

validate := New()

errs := validate.Struct(test)
Equal(t, errs, nil)

test2 := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_without=FieldE" json:"field_1"`
Field2 *string `validate:"excluded_without=FieldE" json:"field_2"`
Field3 map[string]string `validate:"excluded_without=FieldE" json:"field_3"`
Field4 interface{} `validate:"excluded_without=FieldE" json:"field_4"`
Field5 string `validate:"excluded_without=Inner.FieldE" json:"field_5"`
Field6 string `validate:"excluded_without=Inner2.FieldE" json:"field_6"`
}{
Inner: &Inner{Field: &fieldVal},
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
Field6: "test",
}

errs = validate.Struct(test2)
NotEqual(t, errs, nil)

ve := errs.(ValidationErrors)
Equal(t, len(ve), 6)
for i := 1; i <= 6; i++ {
name := fmt.Sprintf("Field%d", i)
AssertError(t, errs, name, name, name, name, "excluded_without")
}
}

func TestExcludedWithAll(t *testing.T) {
type Inner struct {
FieldE string
Field *string
}

fieldVal := "test"
test := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_with_all=FieldE Field" json:"field_1"`
Field2 *string `validate:"excluded_with_all=FieldE Field" json:"field_2"`
Field3 map[string]string `validate:"excluded_with_all=FieldE Field" json:"field_3"`
Field4 interface{} `validate:"excluded_with_all=FieldE Field" json:"field_4"`
Field5 string `validate:"excluded_with_all=Inner.FieldE" json:"field_5"`
Field6 string `validate:"excluded_with_all=Inner2.FieldE" json:"field_6"`
}{
Inner: &Inner{Field: &fieldVal},
Field: fieldVal,
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
Field6: "test",
}

validate := New()

errs := validate.Struct(test)
Equal(t, errs, nil)

test2 := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_with_all=Field FieldE" json:"field_1"`
Field2 *string `validate:"excluded_with_all=Field FieldE" json:"field_2"`
Field3 map[string]string `validate:"excluded_with_all=Field FieldE" json:"field_3"`
Field4 interface{} `validate:"excluded_with_all=Field FieldE" json:"field_4"`
Field5 string `validate:"excluded_with_all=Inner.Field" json:"field_5"`
Field6 string `validate:"excluded_with_all=Inner2.Field" json:"field_6"`
}{
Inner: &Inner{Field: &fieldVal},
Field: "populated",
FieldE: "populated",
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
Field6: "test",
}

errs = validate.Struct(test2)
NotEqual(t, errs, nil)

ve := errs.(ValidationErrors)
Equal(t, len(ve), 5)
for i := 1; i <= 5; i++ {
name := fmt.Sprintf("Field%d", i)
AssertError(t, errs, name, name, name, name, "excluded_with_all")
}
}

func TestExcludedWithoutAll(t *testing.T) {
type Inner struct {
FieldE string
Field *string
}

fieldVal := "test"
test := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_without_all=Field FieldE" json:"field_1"`
Field2 *string `validate:"excluded_without_all=Field FieldE" json:"field_2"`
Field3 map[string]string `validate:"excluded_without_all=Field FieldE" json:"field_3"`
Field4 interface{} `validate:"excluded_without_all=Field FieldE" json:"field_4"`
Field5 string `validate:"excluded_without_all=Inner.Field Inner.Field2" json:"field_5"`
}{
Inner: &Inner{Field: &fieldVal},
Field: "populated",
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
}

validate := New()

errs := validate.Struct(test)
Equal(t, errs, nil)

test2 := struct {
Inner *Inner
Inner2 *Inner
Field string `validate:"omitempty" json:"field"`
FieldE string `validate:"omitempty" json:"field_e"`
Field1 string `validate:"excluded_without_all=FieldE Field" json:"field_1"`
Field2 *string `validate:"excluded_without_all=FieldE Field" json:"field_2"`
Field3 map[string]string `validate:"excluded_without_all=FieldE Field" json:"field_3"`
Field4 interface{} `validate:"excluded_without_all=FieldE Field" json:"field_4"`
Field5 string `validate:"excluded_without_all=Inner.FieldE" json:"field_5"`
Field6 string `validate:"excluded_without_all=Inner2.FieldE" json:"field_6"`
}{
Inner: &Inner{Field: &fieldVal},
Field1: fieldVal,
Field2: &fieldVal,
Field3: map[string]string{"key": "val"},
Field4: "test",
Field5: "test",
Field6: "test",
}

errs = validate.Struct(test2)
NotEqual(t, errs, nil)

ve := errs.(ValidationErrors)
Equal(t, len(ve), 6)
for i := 1; i <= 6; i++ {
name := fmt.Sprintf("Field%d", i)
AssertError(t, errs, name, name, name, name, "excluded_without_all")
}
}

func TestRequiredWithAll(t *testing.T) {
type Inner struct {
Field *string
Expand Down