Description
Q | A |
---|---|
Bug report? | yes/no |
Feature request? | no |
BC Break report? | yes/no |
RFC? | yes |
Symfony version | 3.2.0 |
I wonder how this should work for very common example using DTOs for forms. I have applied validation to my DTO
AppBundle\Command\Register:
constraints:
- Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity:
fields: email
entityClass: AppBundle\Entity\User
em: default
With this configuration I get error: "The class 'AppBundle\Command\Register' was not found in the chain configured namespaces AppBundle\Entity"
The flow currently works like this I set em
for my entity: AppBundle\Entity\User
but in validator we have getting current class metadata for main entity (in my example AppBundle\Command\Register this is not an entity) $em->getClassMetadata(get_class($entity)); so this is why it fails.
# Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntityValidator
if ($constraint->em) {
$em = $this->registry->getManager($constraint->em);
if (!$em) {
throw new ConstraintDefinitionException(sprintf('Object manager "%s" does not exist.', $constraint->em));
}
} else {
$em = $this->registry->getManagerForClass(get_class($entity));
if (!$em) {
throw new ConstraintDefinitionException(sprintf('Unable to find the object manager associated with an entity of class "%s".', get_class($entity)));
}
}
$class = $em->getClassMetadata(get_class($entity));
/* @var $class \Doctrine\Common\Persistence\Mapping\ClassMetadata */
IMHO if we specify entityClass we should expect that everything will concern this entityClass. Another example of failure is when two entities: entity1 and entity2 belong to different entity managers. If this is an expected behaviour (IMHO it should work like this) we should at least add this information to documentation ?
Activity
linaori commentedon May 1, 2017
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:yceruto commentedon May 1, 2017
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 applyUniqueEntity()
for non-entity objects (beingentityClass
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:
entityClass
option (if set) instead ofget_class($entity)
,PropertyAccess
component instead ofReflectionProperty
to read the field value from current object (maintain the metadata verification and the DTO fields must be readable too),We would have DTO's with multiple
UniqueEntity
constraints so:Is it something we want/can to support?
#14917 related
ping @ogizanagi, @HeahDude
ogizanagi commentedon 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:
xabbuh commentedon May 1, 2017
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).
linaori commentedon May 2, 2017
Please also note that validating unique fields might still trigger the case I've listed before, so you'll need to add that either way. If so, you might just as well omit the validation imo, saves you a lot of work.
HeahDude commentedon Jul 30, 2017
While this is not properly supported in core, one may consider using custom constraints to do so (example https://github.com/EnMarche/en-marche.fr/blob/master/src/Validator/UniqueMembershipValidator.php).
ostrolucky commentedon Dec 17, 2017
Here's more generic solution we are using https://gist.github.com/ostrolucky/7b4a7133e51f6b5ffcb598a287acb5f7
Also, can we give this issue more appropriate title please?
Xymanek commentedon Jan 21, 2018
Another possible solution: https://gist.github.com/Xymanek/369f468d02069090770a1e4626d9f1e9
I can make a PR if it looks good
syther101 commentedon Feb 27, 2018
@iltar I'm unsure how your proposed solution would work. Or maybe I'm confused. But at the point of calling
$this->em->flush();
the form should be in a submitted and valid state.How would you then add errors to a field of an already submitted form?
linaori commentedon Feb 27, 2018
While it may not display the errors in the WDT, the error will still be shown when you render the form.
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
.20 remaining items
Pictor13 commentedon May 17, 2021
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).
AndreasA commentedon Sep 30, 2021
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 #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
CaptainFalcon92 commentedon Nov 28, 2021
Resurrecting that issue; i just came across this need today myself.
Having the ability to call UniqueEntity constraint from outisde an entity would be the perfect answer.
Especially at the time of writing with ApiPlatform and their DTO implementation.
I'm totally for it too :
And by doing an assoc array we could eventually indicate the input value to field mapping :
Reading the error message, it sounds like the usage of UniqueEntity is being unnecessary restricted to within an entity.
carsonbot commentedon May 29, 2022
Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?
AndreasA commentedon May 30, 2022
Personally I still think this is quite an important feature.
CaptainFalcon92 commentedon May 30, 2022
I made my own like this. Not perfect, but it works fine so far.
Cheers
nickicool commentedon Dec 4, 2022
Another good solution for validate DTO's on unique field here
ii02735 commentedon Feb 17, 2023
Hello @nickicool,
This won't work if you intend to make an update of your entity.
janklan commentedon Apr 3, 2023
It's pretty odd the
entityClass
is not used to fetch the correct entity manager etc.As for my solution, I decorated the standard
UniqueEntityValidator
, and when my decorator detects the$value
to be one of my DTOs, I apply custom logic; otherwise, I pass it on to the standard validator:I looked at the stock validator, and the way it mingles
$value::class
and$constraint->entityClass
doesn't make much sense. Does anyone know why$constraint->entityClass
is not used exclusively when provided?feature #38662 [DoctrineBridge][Validator] Allow validating every cla…
feature #38662 [DoctrineBridge][Validator] Allow validating every cla…