Closes #660 added excluded with and without #664
Conversation
|
Ping @deankarn any chance this PR could get merged? |
|
Is there any chance to consider this PR for merging? |
|
Much appreciated, thank you! |
| @@ -1416,6 +1453,18 @@ func requiredWithout(fl FieldLevel) bool { | |||
| return true | |||
| } | |||
|
|
|||
| // RequiredWithoutAll is the validation function | |||
There was a problem hiding this comment.
This should be ExcludedWithoutAll.
There was a problem hiding this comment.
Sure, I'll do it tomorrow :)
| @@ -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 { | |||
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Ok, I'll try to confirm it and make sure I'm not messing up somewhere. I'll get back to you tomorrow!
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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
☹️
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@yaliv did you by chance open follow-up issue?
There was a problem hiding this comment.
@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 😉
Fixes #660
Make sure that you've checked the boxes below before you submit PR:
Change Details:
@go-playground/admins