-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DoctrineBridge] UniqueEntityValidator strictly compares ID fields that are objects (like UUID) #58883
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
Comments
Hello zavarock, I've tried to reproduce your issue but was not able to (see #59126) |
Hi @GiuseppeArcuti. Steps
It will throw a ValidationFailedException: This value is already used. |
Hi @zavarock If I understand correctly what you actually want to do is add a ignoreProperties: ['id'] on the ChangeBugLabelDto, because you are trying to edit an existing entity |
Let me try to explain better...
namespace App\Entity;
use App\Repository\PersonRepository;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Bridge\Doctrine\Types\UuidType;
use Symfony\Component\Uid\Uuid;
#[ORM\Entity(repositoryClass: PersonRepository::class)]
class Person
{
#[ORM\Id]
#[ORM\Column(type: UuidType::NAME, unique: true)]
#[ORM\GeneratedValue(strategy: 'CUSTOM')]
#[ORM\CustomIdGenerator(class: 'doctrine.uuid_generator')]
private ?Uuid $id = null;
#[ORM\Column(length: 255, unique: true)]
private ?string $fullName = null;
#[ORM\Column(length: 255, nullable: true)]
private ?string $address = null;
public function getId(): ?Uuid
{
return $this->id;
}
public function getFullName(): ?string
{
return $this->fullName;
}
public function setFullName(string $fullName): static
{
$this->fullName = $fullName;
return $this;
}
public function getAddress(): ?string
{
return $this->address;
}
public function setAddress(?string $address): static
{
$this->address = $address;
return $this;
}
}
namespace App\Dto;
use App\Entity\Person;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
use Symfony\Component\Validator\Constraints as Assert;
use Symfony\Component\Validator\Constraints\Uuid;
#[UniqueEntity(
fields: ['fullName'],
entityClass: Person::class,
identifierFieldNames: ['id'],
)]
class PersonDto
{
public ?Uuid $id;
#[Assert\NotBlank]
public ?string $fullName;
public ?string $address;
} The Person's table has already a few rows:
namespace App\Controller;
use App\Dto\PersonDto;
use App\Entity\Person;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Attribute\Route;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
class PersonController extends AbstractController
{
#[Route('/person/{id:person}', name: 'edit')]
public function edit(
Request $request,
Person $person,
NormalizerInterface $normalizer,
DenormalizerInterface $denormalizer,
EntityManagerInterface $entityManager,
): Response {
$personArray = $normalizer->normalize($person);
$personDto = $denormalizer->denormalize($personArray, PersonDto::class);
$form = $this
->createForm(PersonType::class, $personDto)
->handleRequest($request)
;
if ($form->isSubmitted() && $form->isValid()) {
$editedPersonArray = $normalizer->normalize($form->getData());
$denormalizer->denormalize($editedPersonArray, Person::class, context: [
AbstractNormalizer::OBJECT_TO_POPULATE => $person,
]);
$entityManager->flush();
$this->addFlash('success', 'Person edited successfully.');
}
return $this->render('person/edit.html.twig', [
'form' => $form,
]);
}
} You see.. when submitting this form, validation will fail on fullName because Serializer will create a completely new UUID object when denormalizing $personArray, but the real problem is the strict comparison here: symfony/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php Line 199 in ed0816a
Because of course, if we have this: $uuid1 = new UUID('ec562e21-1fc8-4e55-8de7-a42389ac75c5');
$uuid2 = new UUID('ec562e21-1fc8-4e55-8de7-a42389ac75c5');
$uuid1 === $uuid2; // false Then it won't work. I don't know if it is intended behavior and if yes, I'm sorry, but otherwise, maybe a solution could be checking if both $fieldValues[$identifierFieldName] and $propertyValue are Stringable and then converting it to a string before the comparison. |
Thanks for the detailed explanation. I think I've managed to reproduce with a unit test case. Please take a look |
…tifiers (GiuseppeArcuti, wkania) This PR was merged into the 7.2 branch. Discussion ---------- [DoctrineBridge] Fix UniqueEntityValidator Stringable identifiers | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #58883 | License | MIT `@GiuseppeArcuti` , thanks for creating the test in [PR](#59126). More info in the [comment](#59126 (comment)). Commits ------- b0f012f [DoctrineBridge] Fix UniqueEntityValidator Stringable identifiers 572ebe8 [DoctrineBridge] Fix UniqueEntity for non-integer identifiers
Uh oh!
There was an error while loading. Please reload this page.
Symfony version(s) affected
>=7.1
Description
I'm unsure if the approach taken by the UniqueEntityValidator is appropriate for all situations. Typically, we use the entity object to make changes with a FormType, which usually works well. However, in the case of DTOs, this can be a problem.
symfony/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php
Line 199 in ed0816a
How to reproduce
DTO sample:
Possible Solution
Maybe this is an edge case, but I wonder if it would be better to check if the field is an object and, if yes, make a loose one (!=) instead of (!==).
Additional Context
No response
The text was updated successfully, but these errors were encountered: