Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Validator] Fix type error for non-array items whenUnique::fields is set#52722
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:6.3
Are you sure you want to change the base?
Conversation
Unique::fields is setThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you please rebase to get rid of the first commit?
src/Symfony/Component/Validator/Tests/Constraints/UniqueValidatorTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
7ee329e tof7158bdCompare| @@ -43,7 +43,7 @@ public function validate(mixed $value, Constraint $constraint) | |||
| $collectionElements = []; | |||
| $normalizer = $this->getNormalizer($constraint); | |||
| foreach ($value as $element) { | |||
| if ($fields && !$element= $this->reduceElementKeys($fields, $element)) { | |||
| if ($fields && !(\is_array($element) && $element= $this->reduceElementKeys($fields, $element))) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I wonder if we shouldn't rather throw anUnexpectedValueException instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Good point!
Checking types is not this constraint validator responsibility, as it doesn't need a certain type to do its validation.
Whenfields is passed, and an item doesn't have them, even if it's an array, it is ignored. Why not do the same when the item is not an array?
Every item in the following collection is unique, event when passingfields: 'a' to the constraint:
[ ['a' =>1,'b' =>1], ['a' =>2,'b' =>2], ['b' =>3],null,]
Btw, I'll be happy with any solution, as long as it doesn't crash.
On a wider thought, I think theUnexpectedValueException should not exist, as we already have theType constraint.
A value that can not be validated by a constraint should be ignored (ie:EmailValidator should skip anything that's not a string)
But this is another topic, and probably I lack the knowledge to suggest so 😅
f7158bd to6b7df72Compare
Uh oh!
There was an error while loading.Please reload this page.
The
UniqueValidatorcrashes if:fieldsargument is setvalueis an array but some of the elements are notExample value:
Example constraint config:
Error thrown:
Solution:
Looking at the actual behavior, if one of its items, being an array, has none of the
fields, is ignored.So, continuing on this premise, if one of its items, is not an array, it should be ignored too.
That's what this PR is about (hope so)!