Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[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.

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

Open
wants to merge 2 commits into
base: 7.3
Choose a base branch
Loading
from
Open

[Validator] Unique should support objects fields #47714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
535c874
[Validator] Unique should support objects fields
mpiot Sep 28, 2022
889cb8d
Test if PropertyAccess component is available
mpiot Sep 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions 36 src/Symfony/Component/Validator/Constraints/UniqueValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@

namespace Symfony\Component\Validator\Constraints;

use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyAccess\PropertyAccessor;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Exception\LogicException;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
use Symfony\Component\Validator\Exception\UnexpectedValueException;

Expand All @@ -21,6 +25,13 @@
*/
class UniqueValidator extends ConstraintValidator
{
private ?PropertyAccessorInterface $propertyAccessor;

public function __construct(PropertyAccessorInterface $propertyAccessor = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd configure factor 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 method createPropertyAccessor

{
$this->propertyAccessor = $propertyAccessor;
}

public function validate(mixed $value, Constraint $constraint)
{
if (!$constraint instanceof Unique) {
Expand Down Expand Up @@ -69,18 +80,37 @@ private function getNormalizer(Unique $unique): callable
return $unique->normalizer;
}

private function reduceElementKeys(array $fields, array $element): array
private function reduceElementKeys(array $fields, array|object $element): array
{
$output = [];
foreach ($fields as $field) {
if (!\is_string($field)) {
throw new UnexpectedTypeException($field, 'string');
}
if (isset($element[$field])) {
$output[$field] = $element[$field];

// For no BC, because PropertyAccessor require brackets for array keys
Copy link
Member

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

Copy link
Contributor Author

@mpiot mpiot Sep 28, 2022 •
edited
Loading

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.

// Previous implementation, only check in array
if (\is_array($element) && !str_contains($field, '[')) {
Copy link
Contributor

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)) {
            throw new LogicException('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);
        }
}

Copy link
Contributor

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.

$field = "[{$field}]";
}

if (null !== $value = $this->getPropertyAccessor()->getValue($element, $field)) {
$output[$field] = $value;
}
}

return $output;
}

private function getPropertyAccessor(): PropertyAccessor
{
if (null === $this->propertyAccessor) {
if (!class_exists(PropertyAccess::class)) {
throw new LogicException('Unable to use property path as the Symfony PropertyAccess component is not installed.');
}
$this->propertyAccessor = PropertyAccess::createPropertyAccessor();
}

return $this->propertyAccessor;
}
}
21 changes: 21 additions & 0 deletions 21 src/Symfony/Component/Validator/Tests/Constraints/UniqueValidatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,27 @@ public function getInvalidCollectionValues(): array
['id' => 1, 'email' => 'bar@email.com'],
['id' => 1, 'email' => 'foo@email.com'],
], ['id']],
'unique string sub array' => [[
['id' => 1, 'translation' => ['lang' => 'eng', 'translation' => 'hi']],
['id' => 2, 'translation' => ['lang' => 'eng', 'translation' => 'hello']],
], ['[translation][lang]']],
'unique string attribute' => [[
(object) ['lang' => 'eng', 'translation' => 'hi'],
(object) ['lang' => 'eng', 'translation' => 'hello'],
], ['lang']],
'unique float attribute' => [[
(object) ['latitude' => 51.509865, 'longitude' => -0.118092, 'poi' => 'capital'],
(object) ['latitude' => 52.520008, 'longitude' => 13.404954],
(object) ['latitude' => 51.509865, 'longitude' => -0.118092],
], ['latitude', 'longitude']],
'unique int attribute' => [[
(object) ['id' => 1, 'email' => 'bar@email.com'],
(object) ['id' => 1, 'email' => 'foo@email.com'],
], ['id']],
'unique string sub object attribute' => [[
(object) ['id' => 1, 'translation' => (object) ['lang' => 'eng', 'translation' => 'hi']],
(object) ['id' => 2, 'translation' => (object) ['lang' => 'eng', 'translation' => 'hello']],
], ['translation.lang']],
];
}
}
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.