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] Unique should support objects fields#47714
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:7.4
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This looks like a new feature to me and thus should target 6.2. |
Also, this makes PropertyAccess a required dependency. Better not if possible. And let's throw a useful LogicException when the dep is missing, suggesting to install it (see existing ones in the codebase.) Thanks! |
@nicolas-grekas it's done, not sure about the |
| if (isset($element[$field])) { | ||
| $output[$field] = $element[$field]; | ||
| // For no BC, because PropertyAccessor require brackets for array keys |
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.
For no BC, that simple case should be handled without requiring PropertyAccess
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.
In a first time this is what I've done: check if element is an array, then that the key do not contains any "[]." (PropertyAccessor syntax) and the key exists in the element.
In that case directly access to the array value via the key.
And add an Else statement (not array and/or PropertyPath syntax, that contains the if statement (because the actual implementation accept that the key can be missing).
At the end, find simpler to just transform the key in PrpertyPath compliant syntax.
BastienRimbert commentedNov 3, 2022
May I ask if someone is still working on this? As I opened an issue recently and learnt about this pull request and I could work on it if no one does. |
@BastienRimbert I’m waiting for some feedbacks/discussions. |
acantepie commentedDec 8, 2023
Is this MR will be merged before the next century ? |
@mpiot@nicolas-grekas it's a great feature for Unique validator, it would be great to see it merged. Today even wanted to create PR for same changes. |
| @@ -21,6 +25,13 @@ | |||
| */ | |||
| class UniqueValidator extends ConstraintValidator | |||
| { | |||
| private ?PropertyAccessorInterface $propertyAccessor; | |||
| public function __construct(PropertyAccessorInterface $propertyAccessor = null) | |||
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'd configurefactor for this method and onInvalid set null, otherwise it does not make any sense because PropertyAccess does not public constructor and only way to create Accessor is to call static methodcreatePropertyAccessor
| // For no BC, because PropertyAccessor require brackets for array keys | ||
| // Previous implementation, only check in array | ||
| if (\is_array($element) && !str_contains($field, '[')) { |
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'd do this:
if (is_object($element)) {if (!class_exists(PropertyAccess::class)) {thrownewLogicException('Using UniqueValidator on object requires "PropertyAccess" component. Try running "composer require symfony/property-access".'); }$this->accessor ??= PropertyAccess::createPropertyAccessor();if ($accessor->isReadable($element,$field)) {$output[$field] =$accessor->getValue($element,$field); }}
| // For no BC, because PropertyAccessor require brackets for array keys | ||
| // Previous implementation, only check in array | ||
| if (\is_array($element) && !str_contains($field, '[')) { |
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 don't think validating deeper than first level should be allowed.
Otherwise we may end up with validating uniqueness of other object.
nxtpge commentedJun 6, 2024
It looks like this PR has been superseded by#48951 |
Uh oh!
There was an error while loading.Please reload this page.
The Unique validator have a
fieldsoptions, to define whichfieldsof an array should be compared (if not set, the entire object or array is compared).But the
fieldsoption do not work if we defined object attributes (not the original implementation behavior), but should be logic to work with array and object.