From a1fd3958680018193293dc0f525b405dea3896ef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julien=20Fastr=C3=A9?= <julien.fastre@champs-libres.coop>
Date: Tue, 23 Sep 2025 11:56:55 +0200
Subject: [PATCH] Add unique constraint for `PersonIdentifier`, implement
 `UniqueIdentifierConstraint` with validation logic, and include supporting
 tests

- Introduce `UniqueIdentifierConstraint` and its validator for ensuring identifier uniqueness.
- Add a database-level unique constraint on `PersonIdentifier` (`definition_id`, `value`).
- Implement repository method to fetch identifiers by definition and value.
- Include integration and unit tests for validation and repository functionality.
- Update `Person` entity with `Assert\Valid` annotation for `identifiers`.
---
 .../Entity/Identifier/PersonIdentifier.php    |   3 +
 .../ChillPersonBundle/Entity/Person.php       |   1 +
 .../Validator/UniqueIdentifierConstraint.php  |  25 ++++
 .../UniqueIdentifierConstraintValidator.php   |  50 +++++++
 .../Identifier/PersonIdentifierRepository.php |  37 +++++
 ...niqueIdentifierConstraintValidatorTest.php | 130 ++++++++++++++++++
 .../PersonIdentifierRepositoryTest.php        |  72 ++++++++++
 .../migrations/Version20250922151020.php      |  33 +++++
 8 files changed, 351 insertions(+)
 create mode 100644 src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraint.php
 create mode 100644 src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php
 create mode 100644 src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php
 create mode 100644 src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php
 create mode 100644 src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php
 create mode 100644 src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php

diff --git a/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php b/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php
index 0e2b0d15b..bef721f83 100644
--- a/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php
+++ b/src/Bundle/ChillPersonBundle/Entity/Identifier/PersonIdentifier.php
@@ -12,10 +12,13 @@ declare(strict_types=1);
 namespace Chill\PersonBundle\Entity\Identifier;
 
 use Chill\PersonBundle\Entity\Person;
+use Chill\PersonBundle\PersonIdentifier\Validator\UniqueIdentifierConstraint;
 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'])]
+#[UniqueIdentifierConstraint]
 class PersonIdentifier
 {
     #[ORM\Id]
diff --git a/src/Bundle/ChillPersonBundle/Entity/Person.php b/src/Bundle/ChillPersonBundle/Entity/Person.php
index 82224d40f..af9135177 100644
--- a/src/Bundle/ChillPersonBundle/Entity/Person.php
+++ b/src/Bundle/ChillPersonBundle/Entity/Person.php
@@ -275,6 +275,7 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI
 
     #[ORM\OneToMany(mappedBy: 'person', targetEntity: PersonIdentifier::class, cascade: ['persist', 'remove'], orphanRemoval: true)]
     #[RequiredIdentifierConstraint]
+    #[Assert\Valid]
     private Collection $identifiers;
 
     /**
diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraint.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraint.php
new file mode 100644
index 000000000..b15c4a8b2
--- /dev/null
+++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraint.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\PersonBundle\PersonIdentifier\Validator;
+
+use Symfony\Component\Validator\Constraint;
+
+#[\Attribute]
+class UniqueIdentifierConstraint extends Constraint
+{
+    public string $message = 'person_identifier.Identifier must be unique. The same identifier already exists for {{ persons }}';
+
+    public function getTargets(): string
+    {
+        return self::CLASS_CONSTRAINT;
+    }
+}
diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php
new file mode 100644
index 000000000..24035d01a
--- /dev/null
+++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/UniqueIdentifierConstraintValidator.php
@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\PersonBundle\PersonIdentifier\Validator;
+
+use Chill\PersonBundle\Entity\Identifier\PersonIdentifier;
+use Chill\PersonBundle\Repository\Identifier\PersonIdentifierRepository;
+use Chill\PersonBundle\Templating\Entity\PersonRenderInterface;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+use Symfony\Component\Validator\Exception\UnexpectedValueException;
+
+class UniqueIdentifierConstraintValidator extends ConstraintValidator
+{
+    public function __construct(
+        private readonly PersonIdentifierRepository $personIdentifierRepository,
+        private readonly PersonRenderInterface $personRender,
+    ) {}
+
+    public function validate($value, Constraint $constraint): void
+    {
+        if (!$constraint instanceof UniqueIdentifierConstraint) {
+            throw new UnexpectedTypeException($constraint, UniqueIdentifierConstraint::class);
+        }
+
+        if (!$value instanceof PersonIdentifier) {
+            throw new UnexpectedValueException($value, PersonIdentifier::class);
+        }
+
+        $identifiers = $this->personIdentifierRepository->findByDefinitionAndValue($value->getDefinition(), $value->getValue());
+
+        if (count($identifiers) > 0) {
+            $persons = array_map(fn (PersonIdentifier $idf): string => $this->personRender->renderString($idf->getPerson(), []), $identifiers);
+
+            $this->context->buildViolation($constraint->message)
+                ->setParameter('{{ persons }}', implode(', ', $persons))
+                ->setParameter('definition_id', (string) $value->getDefinition()->getId())
+                ->addViolation();
+        }
+    }
+}
diff --git a/src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php b/src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php
new file mode 100644
index 000000000..3a2da7ec7
--- /dev/null
+++ b/src/Bundle/ChillPersonBundle/Repository/Identifier/PersonIdentifierRepository.php
@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\PersonBundle\Repository\Identifier;
+
+use Chill\PersonBundle\Entity\Identifier\PersonIdentifier;
+use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition;
+use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
+use Doctrine\DBAL\Types\Types;
+use Doctrine\Persistence\ManagerRegistry;
+
+class PersonIdentifierRepository extends ServiceEntityRepository
+{
+    public function __construct(ManagerRegistry $registry)
+    {
+        parent::__construct($registry, PersonIdentifier::class);
+    }
+
+    public function findByDefinitionAndValue(PersonIdentifierDefinition $definition, array $value): array
+    {
+        return $this->createQueryBuilder('p')
+            ->where('p.definition = :definition')
+            ->andWhere('p.value = :value')
+            ->setParameter('definition', $definition)
+            ->setParameter('value', $value, Types::JSON)
+            ->getQuery()
+            ->getResult();
+    }
+}
diff --git a/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php
new file mode 100644
index 000000000..2746b2298
--- /dev/null
+++ b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/UniqueIdentifierConstraintValidatorTest.php
@@ -0,0 +1,130 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\PersonBundle\Tests\PersonIdentifier\Validator;
+
+use Chill\PersonBundle\Entity\Identifier\PersonIdentifier;
+use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition;
+use Chill\PersonBundle\Entity\Person;
+use Chill\PersonBundle\PersonIdentifier\Validator\UniqueIdentifierConstraint;
+use Chill\PersonBundle\PersonIdentifier\Validator\UniqueIdentifierConstraintValidator;
+use Chill\PersonBundle\Repository\Identifier\PersonIdentifierRepository;
+use Chill\PersonBundle\Templating\Entity\PersonRenderInterface;
+use PHPUnit\Framework\Attributes\CoversClass;
+use Prophecy\Argument;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Prophecy\Prophecy\ObjectProphecy;
+use Symfony\Component\Validator\Constraints\NotBlank;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+use Symfony\Component\Validator\Exception\UnexpectedValueException;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+
+/**
+ * @internal
+ */
+#[CoversClass(UniqueIdentifierConstraintValidator::class)]
+final class UniqueIdentifierConstraintValidatorTest extends ConstraintValidatorTestCase
+{
+    use ProphecyTrait;
+
+    /**
+     * @var ObjectProphecy|PersonIdentifierRepository
+     */
+    private ObjectProphecy $repository;
+
+    /**
+     * @var ObjectProphecy|PersonRenderInterface
+     */
+    private ObjectProphecy $personRender;
+
+    protected function setUp(): void
+    {
+        $this->repository = $this->prophesize(PersonIdentifierRepository::class);
+        $this->personRender = $this->prophesize(PersonRenderInterface::class);
+        parent::setUp();
+    }
+
+    protected function createValidator(): UniqueIdentifierConstraintValidator
+    {
+        return new UniqueIdentifierConstraintValidator($this->repository->reveal(), $this->personRender->reveal());
+    }
+
+    public function testThrowsOnInvalidConstraintType(): void
+    {
+        $this->expectException(UnexpectedTypeException::class);
+
+        // Provide a valid value so execution reaches the constraint type check
+        $definition = new PersonIdentifierDefinition(['en' => 'Test'], 'string');
+        $identifier = new PersonIdentifier($definition);
+        $identifier->setValue(['value' => 'ABC']);
+
+        $this->validator->validate($identifier, new NotBlank());
+    }
+
+    public function testThrowsOnInvalidValueType(): void
+    {
+        $this->expectException(UnexpectedValueException::class);
+        $this->validator->validate(new \stdClass(), new UniqueIdentifierConstraint());
+    }
+
+    public function testNoViolationWhenNoDuplicate(): void
+    {
+        $definition = new PersonIdentifierDefinition(['en' => 'Test'], 'string');
+        $identifier = new PersonIdentifier($definition);
+        $identifier->setValue(['value' => 'UNIQ']);
+
+        // Configure repository mock to return empty array
+        $this->repository->findByDefinitionAndValue($definition, ['value' => 'UNIQ'])->willReturn([]);
+
+        $this->validator->validate($identifier, new UniqueIdentifierConstraint());
+        $this->assertNoViolation();
+    }
+
+    public function testViolationWhenDuplicateFound(): void
+    {
+        $definition = new PersonIdentifierDefinition(['en' => 'SSN'], 'string');
+        $reflectionClass = new \ReflectionClass($definition);
+        $reflectionId = $reflectionClass->getProperty('id');
+        $reflectionId->setValue($definition, 1);
+
+        $personA = new Person();
+        $personA->setFirstName('Alice')->setLastName('Anderson');
+        $personB = new Person();
+        $personB->setFirstName('Bob')->setLastName('Brown');
+
+        $dup1 = new PersonIdentifier($definition);
+        $dup1->setPerson($personA);
+        $dup1->setValue(['value' => '123']);
+        $dup2 = new PersonIdentifier($definition);
+        $dup2->setPerson($personB);
+        $dup2->setValue(['value' => '123']);
+
+        // Repository returns duplicates
+        $this->repository->findByDefinitionAndValue($definition, ['value' => '123'])->willReturn([$dup1, $dup2]);
+
+        // Person renderer returns names
+        $this->personRender->renderString($personA, Argument::type('array'))->willReturn('Alice Anderson');
+        $this->personRender->renderString($personB, Argument::type('array'))->willReturn('Bob Brown');
+
+        $identifier = new PersonIdentifier($definition);
+        $identifier->setPerson(new Person());
+        $identifier->setValue(['value' => '123']);
+
+        $constraint = new UniqueIdentifierConstraint();
+
+        $this->validator->validate($identifier, $constraint);
+
+        $this->buildViolation($constraint->message)
+            ->setParameter('{{ persons }}', 'Alice Anderson, Bob Brown')
+            ->setParameter('definition_id', '1')
+            ->assertRaised();
+    }
+}
diff --git a/src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php b/src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php
new file mode 100644
index 000000000..60585ad78
--- /dev/null
+++ b/src/Bundle/ChillPersonBundle/Tests/Repository/Identifier/PersonIdentifierRepositoryTest.php
@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+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\Repository\Identifier\PersonIdentifierRepository;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+/**
+ * @internal
+ *
+ * @coversNothing
+ */
+class PersonIdentifierRepositoryTest extends KernelTestCase
+{
+    public function testFindByDefinitionAndValue(): void
+    {
+        self::bootKernel();
+        $container = self::getContainer();
+
+        /** @var EntityManagerInterface $em */
+        $em = $container->get(EntityManagerInterface::class);
+
+        // Get a random existing person from fixtures
+        /** @var Person|null $person */
+        $person = $em->getRepository(Person::class)->findOneBy([]);
+        self::assertNotNull($person, 'An existing Person is required for this integration test.');
+
+        // Create a definition
+        $definition = new PersonIdentifierDefinition(['en' => 'Test Identifier'], 'string');
+        $em->persist($definition);
+        $em->flush();
+
+        // Create an identifier attached to the person
+        $value = ['value' => 'ABC-'.bin2hex(random_bytes(4))];
+        $identifier = new PersonIdentifier($definition);
+        $identifier->setPerson($person);
+        $identifier->setValue($value);
+        $identifier->setCanonical('canonical-'.$value['value']);
+        $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);
+
+        self::assertNotEmpty($results, 'Repository should return at least one result.');
+        self::assertContainsOnlyInstancesOf(PersonIdentifier::class, $results);
+        self::assertTrue(in_array($identifier->getId(), array_map(static fn (PersonIdentifier $pi) => $pi->getId(), $results), true));
+
+        // Cleanup
+        foreach ($results as $res) {
+            $em->remove($res);
+        }
+        $em->flush();
+        $em->remove($definition);
+        $em->flush();
+    }
+}
diff --git a/src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php b/src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php
new file mode 100644
index 000000000..9d6030baa
--- /dev/null
+++ b/src/Bundle/ChillPersonBundle/migrations/Version20250922151020.php
@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\Migrations\Person;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20250922151020 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Add unique constraint for person identifiers';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $this->addSql('CREATE UNIQUE INDEX chill_person_identifier_unique ON chill_person_identifier (definition_id, value)');
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('DROP INDEX chill_person_identifier_unique');
+    }
+}