From 6ea9af588b354065038d40a34cf5171e2b380097 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 24 Sep 2025 12:39:37 +0200 Subject: [PATCH] Replace `value` with `canonical` in `PersonIdentifier` unique constraint, repository logic, and tests - Update unique constraint on `PersonIdentifier` to use `canonical` instead of `value`. - Refactor repository method `findByDefinitionAndValue` to `findByDefinitionAndCanonical`, updating logic accordingly. - Adjust validation logic in `UniqueIdentifierConstraintValidator` to align with the new canonical-based approach. - Modify related integration and unit tests to support the changes. - Inject `PersonIdentifierManagerInterface` into the repository to handle canonical value generation. --- .../Entity/Identifier/PersonIdentifier.php | 4 ++-- .../Identifier/StringIdentifier.php | 4 +++- .../UniqueIdentifierConstraintValidator.php | 2 +- .../Identifier/PersonIdentifierRepository.php | 15 ++++++++++----- .../UniqueIdentifierConstraintValidatorTest.php | 4 ++-- .../Identifier/PersonIdentifierRepositoryTest.php | 14 +++++++++----- .../migrations/Version20250922151020.php | 2 +- 7 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php b/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php index bef721f83..0cd03472a 100644 --- a/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php +++ b/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php @@ -17,7 +17,7 @@ use Doctrine\ORM\Mapping as ORM; #[ORM\Entity] #[ORM\Table(name: 'chill_person_identifier')] -#[ORM\UniqueConstraint(name: 'chill_person_identifier_unique', columns: ['definition_id', 'value'])] +#[ORM\UniqueConstraint(name: 'chill_person_identifier_unique', columns: ['definition_id', 'canonical'])] #[UniqueIdentifierConstraint] class PersonIdentifier { @@ -38,7 +38,7 @@ class PersonIdentifier public function __construct( #[ORM\ManyToOne(targetEntity: PersonIdentifierDefinition::class)] - #[ORM\JoinColumn(name: 'definition_id', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')] + #[ORM\JoinColumn(name: 'definition_id', referencedColumnName: 'id', nullable: false, onDelete: 'RESTRICT')] private PersonIdentifierDefinition $definition, ) {} diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php index 2d003b9cf..4bc02c117 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php @@ -19,9 +19,11 @@ use Symfony\Component\Form\FormBuilderInterface; final readonly class StringIdentifier implements PersonIdentifierEngineInterface { + public const NAME = 'chill-person-bundle.string-identifier'; + public static function getName(): string { - return 'chill-person-bundle.string-identifier'; + return self::NAME; } public function canonicalizeValue(array $value, PersonIdentifierDefinition $definition): ?string diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php index 24035d01a..76364538b 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php @@ -36,7 +36,7 @@ class UniqueIdentifierConstraintValidator extends ConstraintValidator throw new UnexpectedValueException($value, PersonIdentifier::class); } - $identifiers = $this->personIdentifierRepository->findByDefinitionAndValue($value->getDefinition(), $value->getValue()); + $identifiers = $this->personIdentifierRepository->findByDefinitionAndCanonical($value->getDefinition(), $value->getValue()); if (count($identifiers) > 0) { $persons = array_map(fn (PersonIdentifier $idf): string => $this->personRender->renderString($idf->getPerson(), []), $identifiers); diff --git a/src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php b/src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php index 3a2da7ec7..e3af66e31 100644 --- a/src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php +++ b/src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php @@ -13,24 +13,29 @@ namespace Chill\PersonBundle\Repository\Identifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; +use Chill\PersonBundle\PersonIdentifier\PersonIdentifierManagerInterface; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; -use Doctrine\DBAL\Types\Types; use Doctrine\Persistence\ManagerRegistry; class PersonIdentifierRepository extends ServiceEntityRepository { - public function __construct(ManagerRegistry $registry) + public function __construct(ManagerRegistry $registry, private readonly PersonIdentifierManagerInterface $personIdentifierManager) { parent::__construct($registry, PersonIdentifier::class); } - public function findByDefinitionAndValue(PersonIdentifierDefinition $definition, array $value): array + public function findByDefinitionAndCanonical(PersonIdentifierDefinition $definition, array|string $valueOrCanonical): array { return $this->createQueryBuilder('p') ->where('p.definition = :definition') - ->andWhere('p.value = :value') + ->andWhere('p.canonical = :canonical') ->setParameter('definition', $definition) - ->setParameter('value', $value, Types::JSON) + ->setParameter( + 'canonical', + is_string($valueOrCanonical) ? + $valueOrCanonical : + $this->personIdentifierManager->buildWorkerByPersonIdentifierDefinition($definition)->canonicalizeValue($valueOrCanonical), + ) ->getQuery() ->getResult(); } diff --git a/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php index 2746b2298..6356178cf 100644 --- a/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php +++ b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php @@ -82,7 +82,7 @@ final class UniqueIdentifierConstraintValidatorTest extends ConstraintValidatorT $identifier->setValue(['value' => 'UNIQ']); // Configure repository mock to return empty array - $this->repository->findByDefinitionAndValue($definition, ['value' => 'UNIQ'])->willReturn([]); + $this->repository->findByDefinitionAndCanonical($definition, ['value' => 'UNIQ'])->willReturn([]); $this->validator->validate($identifier, new UniqueIdentifierConstraint()); $this->assertNoViolation(); @@ -108,7 +108,7 @@ final class UniqueIdentifierConstraintValidatorTest extends ConstraintValidatorT $dup2->setValue(['value' => '123']); // Repository returns duplicates - $this->repository->findByDefinitionAndValue($definition, ['value' => '123'])->willReturn([$dup1, $dup2]); + $this->repository->findByDefinitionAndCanonical($definition, ['value' => '123'])->willReturn([$dup1, $dup2]); // Person renderer returns names $this->personRender->renderString($personA, Argument::type('array'))->willReturn('Alice Anderson'); diff --git a/src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php b/src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php index 60585ad78..ab36fd872 100644 --- a/src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php +++ b/src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php @@ -14,6 +14,8 @@ namespace Chill\PersonBundle\Tests\Repository\Identifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; use Chill\PersonBundle\Entity\Person; +use Chill\PersonBundle\PersonIdentifier\Identifier\StringIdentifier; +use Chill\PersonBundle\PersonIdentifier\PersonIdentifierManagerInterface; use Chill\PersonBundle\Repository\Identifier\PersonIdentifierRepository; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; @@ -25,10 +27,12 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; */ class PersonIdentifierRepositoryTest extends KernelTestCase { - public function testFindByDefinitionAndValue(): void + public function testFindByDefinitionAndCanonical(): void { self::bootKernel(); $container = self::getContainer(); + /** @var PersonIdentifierManagerInterface $personIdentifierManager */ + $personIdentifierManager = $container->get(PersonIdentifierManagerInterface::class); /** @var EntityManagerInterface $em */ $em = $container->get(EntityManagerInterface::class); @@ -39,23 +43,23 @@ class PersonIdentifierRepositoryTest extends KernelTestCase self::assertNotNull($person, 'An existing Person is required for this integration test.'); // Create a definition - $definition = new PersonIdentifierDefinition(['en' => 'Test Identifier'], 'string'); + $definition = new PersonIdentifierDefinition(['en' => 'Test Identifier'], StringIdentifier::NAME); $em->persist($definition); $em->flush(); // Create an identifier attached to the person - $value = ['value' => 'ABC-'.bin2hex(random_bytes(4))]; + $value = ['content' => 'ABC-'.bin2hex(random_bytes(4))]; $identifier = new PersonIdentifier($definition); $identifier->setPerson($person); $identifier->setValue($value); - $identifier->setCanonical('canonical-'.$value['value']); + $identifier->setCanonical($personIdentifierManager->buildWorkerByPersonIdentifierDefinition($definition)->canonicalizeValue($identifier->getValue())); $em->persist($identifier); $em->flush(); // Use the repository to find by definition and value /** @var PersonIdentifierRepository $repo */ $repo = $container->get(PersonIdentifierRepository::class); - $results = $repo->findByDefinitionAndValue($definition, $value); + $results = $repo->findByDefinitionAndCanonical($definition, $value); self::assertNotEmpty($results, 'Repository should return at least one result.'); self::assertContainsOnlyInstancesOf(PersonIdentifier::class, $results); diff --git a/src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php b/src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php index 9d6030baa..709ecbf54 100644 --- a/src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php +++ b/src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php @@ -23,7 +23,7 @@ final class Version20250922151020 extends AbstractMigration public function up(Schema $schema): void { - $this->addSql('CREATE UNIQUE INDEX chill_person_identifier_unique ON chill_person_identifier (definition_id, value)'); + $this->addSql('CREATE UNIQUE INDEX chill_person_identifier_unique ON chill_person_identifier (definition_id, canonical)'); } public function down(Schema $schema): void