Validate nested structs

[zenovich]

Fixes #367 .

Make sure that you've checked the boxes below before you submit PR:

• Tests exist or have been written that cover this particular change.

Change Details:
This change allows using (any: custom or backed in) validations on nested structs.

@go-playground/admins

[deankarn]

Hey @zenovich finally found some time to take a look at this, nice work! I've wanted to make all validations run the same way whether structs or fields.

There's only one thing I'm concerned about and that's the order the validations are done.

If I've had validations on a struct and it's fields I would expect that the validations run on the struct first and then the fields, but with these changes looks like is the opposite.

I'll keep thinking about it, if you can come up with something please let me know :)

[MissingRoberto]

Hitting the same issue as @zenovich

[zenovich]

Thanks, @deankarn. Sorry for the delay. I think we cannot treat a piece of data as a struct if it doesn't pass the fields validation, otherwise there can be a lot of suprises for struct validators. For the reason, I implemented it in the way the struct's fields are validated before the struct itself. I hope it makes sense.

[deankarn]

Hey @zenovich sorry for the delay. I really do like the change but my thoughts are that the struct validations still need to come first before the fields.

My reasoning is that a struct comes first in the hierarchical eg. you can't have fields to validate if the struct say is nil. just like not being able to validate the values of an array or map before checking if there are values.

[zenovich]

Hey @zenovich sorry for the delay. I really do like the change but my thoughts are that the struct validations still need to come first before the fields.

My reasoning is that a struct comes first in the hierarchical eg. you can't have fields to validate if the struct say is nil. just like not being able to validate the values of an array or map before checking if there are values.

I still think we cannot consider data as a struct before we validate its fields. For the situation where we have nil instead of the data for the struct, we can always use 'required'/'omitempty' tag or check if the data is nil in the validators.

[deankarn]

I guess what you said is exactly what I mean, you have to do checks/validations on the struct first to ensure existence of data to check. If it’s nil then there are no fields to validate and is why I believe the order needs to run validations on the struct itself first.

[deankarn]

Closing until the matter of order can be addressed.

[zenovich]

@deankarn, sorry for the delay. I didn't have enough time for investigation to answer your question till now. I've just rechecked that actually everything works exactly in the way you want: the validators are run on the structs first, than on the fields, so validators are not run for the fields when the struct is null.

Sorry for the confusion. Can you reopen the PR please?

[zenovich]

func TestStructValidation_SkipsNotGivenNestedStructs(t *testing.T) {
	type inner struct {
		Test string `validate:"required"`
	}
	type TestStruct struct {
		Test *struct {
			Test string `validate:"required"`
		} `validate:"one,two,dive"`
	}

	ts := TestStruct{}
	val := New()

	fn1 := func(fl FieldLevel) bool {
		return fl.Field().FieldByName("Test").String() == "some value"
	}
	fn2 := func(fl FieldLevel) bool {
		return fl.Field().FieldByName("Test").String() == "another value"
	}

	val.RegisterValidation("one", fn1)
	val.RegisterValidation("two", fn2)
	errs := val.Struct(ts)
	Equal(t, len(errs.(ValidationErrors)), 1)
	AssertError(t, errs, "TestStruct.Test", "TestStruct.Test", "Test", "Test", "one")
}

Note that the validator are not called for TestStruct.Test.Test because TestStruct.Test is null.

[zenovich]

So, in the code, it looks like the validators of inner fields of a nested struct are being run before validators of the nested struct, but this never happens if the nested struct is null.

[zenovich]

IMO the order for nested structs should be:

1. check if the nested struct is null, otherwise skip this struct,
2. validate all the fields of the nested struct,
3. if there are no errors inside the nested struct, run validators for the nested struct itself.

The idea is that validators for the whole struct can only make sense if each part of the struct makes sense.

But there is a bug in this PR: on the step 3 validators validators for the nested struct get run even if there were errors inside :(
I'll fix this soon.

[deankarn]

The reason is nils are handled a little differently in the code that shortcuts the logic.

Also would it work if a StructLevel validation was registered/used?

By all means please submit another PR and I’ll take a look at that one :)

[zenovich]

@deankarn all the existing tests related to StructLevel are successful. Should we check any special cases?

[zenovich]

By all means please submit another PR and I’ll take a look at that one :)

Done