Skip to content

Navigation Menu

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.

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

Conversation

mpiot
Copy link
Contributor

@mpiot mpiot commented Sep 28, 2022

Q A
Branch? 6.2
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR

The Unique validator have a fields options, to define which fields of an array should be compared (if not set, the entire object or array is compared).

But the fields option do not work if we defined object attributes (not the original implementation behavior), but should be logic to work with array and object.

@xabbuh
Copy link
Member

xabbuh commented Sep 28, 2022

This looks like a new feature to me and thus should target 6.2.

@nicolas-grekas
Copy link
Member

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 nicolas-grekas modified the milestones: 6.1, 6.2 Sep 28, 2022
@mpiot mpiot changed the base branch from 6.1 to 6.2 September 28, 2022 14:07
@mpiot
Copy link
Contributor Author

mpiot commented Sep 28, 2022

@nicolas-grekas it's done, not sure about the __construct method.

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

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
Copy link

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.

@mpiot
Copy link
Contributor Author

mpiot commented Nov 3, 2022

@BastienRimbert I’m waiting for some feedbacks/discussions.

@acantepie
Copy link

Is this MR will be merged before the next century ?

@oleg-andreyev
Copy link
Contributor

@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)
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


// For no BC, because PropertyAccessor require brackets for array keys
// 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);
        }
}


// For no BC, because PropertyAccessor require brackets for array keys
// 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 don't think validating deeper than first level should be allowed.
Otherwise we may end up with validating uniqueness of other object.

@nxtpge
Copy link

nxtpge commented Jun 6, 2024

Hello @xabbuh and @stof, do you plan to release this feature for the version 7.2?

@magikid
Copy link
Contributor

magikid commented Mar 31, 2025

It looks like this PR has been superseded by #48951

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.