symfony: [DoctrineBridge] UniqueEntity validator problem with entityClass

The UniqueEntity does not work on non-entity objects afaik. However, I think a proper solution would would be to just try and insert, then catch the unique constraint exception and add a form error:

try {
    $this->em->flush();
} catch (UniqueConstraintViolationException $exception) {
    $form->get('username')->addError(new FormError(
        $this->translator->trans('authentication.username.taken', [], 'validators')
    ));
}

Even when the purpose of the entityClass was to execute the query in a different repository (in some cases, such as when using Doctrine inheritance mapping), I wonder if we could expand its scope to apply UniqueEntity() for non-entity objects (being entityClass mandatory in these case).

The target of this constraint is validate that a particular field (or fields) in a Doctrine entity is (are) unique, so what if:

• we use entityClass option (if set) instead of get_class($entity),
• we use PropertyAccess component instead of ReflectionProperty to read the field value from current object (maintain the metadata verification and the DTO fields must be readable too),
• we remove the check about the repository being able to support the current object,

We would have DTO’s with multiple UniqueEntity constraints so:

/**
 * @UniqueEntity(fields={"email"}, entityClass="AppBundle\Entity\User")
 * @UniqueEntity(fields={"phone"}, entityClass="AppBundle\Entity\Phone", em="custom")
 */
class RegisterDTO
{
    /** @var string */
    private $email;

    /** @var string */
    private $phone;
    
    //...
}

Is it something we want/can to support?

https://github.com/symfony/symfony/issues/14917 related

ping @ogizanagi, @HeahDude

@yceruto I considered something similar in the past; I wasn’t sure about reusing the same constraint but now I’d say it makes sense. And the feature would be particularly useful for validating commands (command bus) for instance.

But we will also need a mapping between the DTOs fields and the targeted entity fields as their naming might slightly differ.

Foolish example:

/**
 * @UniqueEntity(fields={"userEmail": "email"}, entityClass="AppBundle\Entity\User")
 * @UniqueEntity(fields={"userPhone": "phoneNumber"}, entityClass="AppBundle\Entity\Phone", em="custom")
 */
class RegisterDTO
{
    /** @var string */
    private $userEmail;

    /** @var string */
    private $userPhone;
    
    //...
}

Facing the same issue and definitely interested 👍

Personally I still think this is quite an important feature.

I’m coming back on this issue since I’m having it right away.

While working on my side-project, I wanted to create a DTO that would contain the UniqueEntity constraint associated with another class.

For now, as a first solution, I suggest that we provide a quick “fix” (I think it’s an issue and not a feature to make sure we can properly use the constraint with DTO-like classes.

Next, on a second plan, I think we can provide another format for the fields mapping option in order to be able to associated DTO fields with entity fields if names differ.

I can make a PR for the first one, since I started the work on it before discovering this issue.

Friendly ping @nicolas-grekas for a potential decision on the core-team side

That’s probably not enough. You also need to consider a way to determine whether or not a possible result is the object currently being modified (the current implementation checks for the object being returned to be the same as the one being validated).

@ogizanagi Perhaps this could help?

https://gist.github.com/webbertakken/569409670bfc7c079e276f79260105ed

A working mapping like you suggested.

ogizanagi commented on May 1, 2017 • @yceruto I considered something similar in the past; I wasn’t sure about reusing the same constraint but now I’d say it makes sense. And the feature would be particularly useful for validating commands (command bus) for instance.

But we will also need a mapping between the DTOs fields and the targeted entity fields as their naming might slightly differ. Foolish example:

/**
 * @UniqueEntity(fields={"userEmail": "email"}, entityClass="AppBundle\Entity\User")
 * @UniqueEntity(fields={"userPhone": "phoneNumber"}, entityClass="AppBundle\Entity\Phone", em="custom")
 */
class RegisterDTO
{
    /** @var string */
    private $userEmail;

    /** @var string */
    private $userPhone;
    
    //...
}

While it may not display the errors in the WDT, the error will still be shown when you render the form.

But at the point of calling $this->em->flush(); the form should be in a submitted and valid state.

This is true. However, 1ms after the form was valid (it was unique at the moment of validation) and after that moment, someone else just flushed their entity with the exact same value, your flush will now fail with a UniqueConstraintException.

Another possible solution: https://gist.github.com/Xymanek/369f468d02069090770a1e4626d9f1e9

I can make a PR if it looks good

This breaks my DDD

I made my own like this. Not perfect, but it works fine so far.

class UniqueDTO extends Constraint
{
    public ?string $atPath = null;
    public string $entityClass;
    public string|array $fields;
    public string $message = 'error.duplicate';

    public function getTargets(): string
    {
        return parent::CLASS_CONSTRAINT;
    }
}

class UniqueDTOValidator extends ConstraintValidator
{
    private EntityManagerInterface $em;
    private PropertyAccessorInterface $accessor;

    public function __construct(EntityManagerInterface $em, PropertyAccessorInterface $accessor)
    {
        $this->em = $em;
        $this->accessor = $accessor;
    }

    public function validate($value, Constraint $constraint): void
    {
        if (!$constraint instanceof UniqueDTO) {
            throw new UnexpectedTypeException($constraint, UniqueDTO::class);
        }

        if(!$constraint->entityClass) {
            throw new InvalidArgumentException('Entity class is required.');
        }

        $repository = $this->em->getRepository($constraint->entityClass);

        $fields = (array) $constraint->fields;
        $criteria = [];

        foreach ($fields as $from => $to) {
            $criteria[$to] = $this->accessor->getValue($value, $from);
        }

        if ($repository->count($criteria)) {
            $cvb = $this->context->buildViolation($constraint->message);

            if ($constraint->atPath) {
                $cvb->atPath($constraint->atPath);
            }

            $cvb->addViolation();
        }
    }
}

$metadata->addConstraint(new UniqueDTO([
    'atPath' => 'email',
    'entityClass' => User::class,
    'fields' => ['email' => 'email'],
    'message' => 'email.duplicate',
]));

Cheers

Hello @nickicool,

This won’t work if you intend to make an update of your entity.

Another good solution for validate DTO’s on unique field here

I just encountered the same issue and it should definitely be fixed as using a DTO is just cleaner than using the entity directly, especially with multiple APIs and possible necessary transformations.

Would be great if it would make it into 5.4

And the PR above https://github.com/symfony/symfony/pull/38662 seems to address and fix this issue already, it even takes possible property / field mappings into account, would be nice if it is merged or, if necessary, adjusted and merged so it can make it into 5.4 and 6.0

Wondering if this could be worked out (or worked-around) on Doctrine side.

Wouldn’t it make sense to configure/extend from Doctrine Bridge to make it aware that a DTO corresponds to a specific Entity?

This way the UniqueEntity constraints would work transparently and wouldn’t need to care if the variable is an actual Entity or a representation of that (DTO).

Facing the very same issue, this would be a must have feature