Add validation and support for identifiers in PersonJsonDenormalizer, enhance altNames handling, and update tests for improved coverage. Adjust PersonIdentifierManager to handle identifier definitions by ID.

2025-09-25 16:14:59 +00:00 · 2025-09-16 12:06:39 +02:00
parent bda0743c63
commit 8d29fb260a
4 changed files with 270 additions and 17 deletions
--- a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierManager.php
+++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierManager.php
@@ -13,6 +13,7 @@ namespace Chill\PersonBundle\PersonIdentifier;

 use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition;
 use Chill\PersonBundle\PersonIdentifier\Exception\EngineNotFoundException;
+use Chill\PersonBundle\PersonIdentifier\Exception\PersonIdentifierDefinitionNotFoundException;
 use Chill\PersonBundle\Repository\Identifier\PersonIdentifierDefinitionRepository;

 final readonly class PersonIdentifierManager implements PersonIdentifierManagerInterface
@@ -44,8 +45,16 @@ final readonly class PersonIdentifierManager implements PersonIdentifierManagerI
        return $workers;
    }

-    public function buildWorkerByPersonIdentifierDefinition(PersonIdentifierDefinition $personIdentifierDefinition): PersonIdentifierWorker
+    public function buildWorkerByPersonIdentifierDefinition(int|PersonIdentifierDefinition $personIdentifierDefinition): PersonIdentifierWorker
    {
+        if (is_int($personIdentifierDefinition)) {
+            $id = $personIdentifierDefinition;
+            $personIdentifierDefinition = $this->personIdentifierDefinitionRepository->find($id);
+            if (null === $personIdentifierDefinition) {
+                throw new PersonIdentifierDefinitionNotFoundException($id);
+            }
+        }
+
        return new PersonIdentifierWorker($this->getEngine($personIdentifierDefinition->getEngine()), $personIdentifierDefinition);
    }

--- a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierManagerInterface.php
+++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierManagerInterface.php
@@ -22,5 +22,8 @@ interface PersonIdentifierManagerInterface
     */
    public function getWorkers(): array;

-    public function buildWorkerByPersonIdentifierDefinition(PersonIdentifierDefinition $personIdentifierDefinition): PersonIdentifierWorker;
+    /**
+     * @param int|PersonIdentifierDefinition $personIdentifierDefinition an instance of PersonIdentifierDefinition, or his id
+     */
+    public function buildWorkerByPersonIdentifierDefinition(int|PersonIdentifierDefinition $personIdentifierDefinition): PersonIdentifierWorker;
 }
--- a/src/Bundle/ChillPersonBundle/Serializer/Normalizer/PersonJsonDenormalizer.php
+++ b/src/Bundle/ChillPersonBundle/Serializer/Normalizer/PersonJsonDenormalizer.php
@@ -14,9 +14,12 @@ namespace Chill\PersonBundle\Serializer\Normalizer;
 use Chill\MainBundle\Entity\Center;
 use Chill\MainBundle\Entity\Civility;
 use Chill\MainBundle\Entity\Gender;
+use Chill\PersonBundle\Entity\Identifier\PersonIdentifier;
 use Chill\PersonBundle\Entity\Person;
 use Chill\PersonBundle\Entity\PersonAltName;
+use Chill\PersonBundle\PersonIdentifier\PersonIdentifierManagerInterface;
 use libphonenumber\PhoneNumber;
+use Symfony\Component\Serializer\Exception\UnexpectedValueException;
 use Symfony\Component\Serializer\Normalizer\DenormalizerAwareInterface;
 use Symfony\Component\Serializer\Normalizer\DenormalizerAwareTrait;
 use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
@@ -27,11 +30,13 @@ use Symfony\Component\Serializer\Normalizer\ObjectToPopulateTrait;
 *
 * To find an existing instance by his id, see the @see{PersonJsonReadDenormalizer}.
 */
-class PersonJsonDenormalizer implements DenormalizerInterface, DenormalizerAwareInterface
+final class PersonJsonDenormalizer implements DenormalizerInterface, DenormalizerAwareInterface
 {
    use DenormalizerAwareTrait;
    use ObjectToPopulateTrait;

+    public function __construct(private readonly PersonIdentifierManagerInterface $personIdentifierManager) {}
+
    public function denormalize($data, string $type, ?string $format = null, array $context = []): Person
    {
        $person = $this->extractObjectToPopulate($type, $context);
@@ -78,19 +83,48 @@ class PersonJsonDenormalizer implements DenormalizerInterface, DenormalizerAware
        }

        if (\array_key_exists('altNames', $data)) {
-            foreach ($data['altNames'] as $altName) {
-                $oldAltName = $person
-                    ->getAltNames()
-                    ->filter(static fn (PersonAltName $n): bool => $n->getKey() === $altName['key'])->first();
-
-                if (false === $oldAltName) {
-                    $newAltName = new PersonAltName();
-                    $newAltName->setKey($altName['key']);
-                    $newAltName->setLabel($altName['label']);
-                    $person->addAltName($newAltName);
-                } else {
-                    $oldAltName->setLabel($altName['label']);
+            foreach ($data['altNames'] as $altNameData) {
+                if (!array_key_exists('key', $altNameData)
+                    || !array_key_exists('value', $altNameData)
+                    || '' === trim($altNameData['key'])
+                ) {
+                    throw new UnexpectedValueException('format for alt name is not correct');
                }
+                $altNameKey = $altNameData['key'];
+                $altNameValue = $altNameData['value'];
+
+                $altName = $person->getAltNames()->findFirst(fn (PersonAltName $personAltName) => $personAltName->getKey() === $altNameKey);
+                if (null === $altName) {
+                    $altName = new PersonAltName();
+                    $person->addAltName($altName);
+                }
+                $altName->setKey($altNameKey)->setLabel($altNameValue);
+            }
+        }
+
+        if (\array_key_exists('identifiers', $data)) {
+            foreach ($data['identifiers'] as $identifierData) {
+                if (!array_key_exists('definition_id', $identifierData)
+                    || !array_key_exists('value', $identifierData)
+                    || !is_int($identifierData['definition_id'])
+                    || !is_array($identifierData['value'])
+                ) {
+                    throw new UnexpectedValueException('format for identifiers is not correct');
+                }
+
+                $definitionId = $identifierData['definition_id'];
+                $value = $identifierData['value'];
+
+                $worker = $this->personIdentifierManager->buildWorkerByPersonIdentifierDefinition($definitionId);
+
+                $personIdentifier = $person->getIdentifiers()->findFirst(fn (PersonIdentifier $personIdentifier) => $personIdentifier->getId() === $definitionId);
+                if (null === $personIdentifier) {
+                    $personIdentifier = new PersonIdentifier($worker->getDefinition());
+                    $person->addIdentifier($personIdentifier);
+                }
+
+                $personIdentifier->setValue($value);
+                $personIdentifier->setCanonical($worker->canonicalizeValue($value));
            }
        }

--- a/src/Bundle/ChillPersonBundle/Tests/Serializer/Normalizer/PersonJsonDenormalizerTest.php
+++ b/src/Bundle/ChillPersonBundle/Tests/Serializer/Normalizer/PersonJsonDenormalizerTest.php
@@ -11,11 +11,218 @@ declare(strict_types=1);

 namespace Chill\PersonBundle\Tests\Serializer\Normalizer;

+use Chill\MainBundle\Entity\Center;
+use Chill\MainBundle\Entity\Civility;
+use Chill\MainBundle\Entity\Gender;
+use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition;
+use Chill\PersonBundle\Entity\Person;
+use Chill\PersonBundle\PersonIdentifier\PersonIdentifierEngineInterface;
+use Chill\PersonBundle\PersonIdentifier\PersonIdentifierManagerInterface;
+use Chill\PersonBundle\PersonIdentifier\PersonIdentifierWorker;
+use Chill\PersonBundle\Serializer\Normalizer\PersonJsonDenormalizer;
+use libphonenumber\PhoneNumber;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;

 /**
 * @internal
 *
- * @coversNothing
+ * @covers \Chill\PersonBundle\Serializer\Normalizer\PersonJsonDenormalizer
 */
-class PersonJsonDenormalizerTest extends TestCase {}
+final class PersonJsonDenormalizerTest extends TestCase
+{
+    private function createIdentifierManager(): PersonIdentifierManagerInterface
+    {
+        return new class () implements PersonIdentifierManagerInterface {
+            public function getWorkers(): array
+            {
+                return [];
+            }
+
+            public function buildWorkerByPersonIdentifierDefinition(int|PersonIdentifierDefinition $personIdentifierDefinition): PersonIdentifierWorker
+            {
+                if (is_int($personIdentifierDefinition)) {
+                    $definition = new PersonIdentifierDefinition(['en' => 'Test'], 'dummy');
+                    // Force the id for testing purposes
+                    $r = new \ReflectionProperty(PersonIdentifierDefinition::class, 'id');
+                    $r->setAccessible(true);
+                    $r->setValue($definition, $personIdentifierDefinition);
+                } else {
+                    $definition = $personIdentifierDefinition;
+                }
+
+                $engine = new class () implements PersonIdentifierEngineInterface {
+                    public static function getName(): string
+                    {
+                        return 'dummy';
+                    }
+
+                    public function canonicalizeValue(array $value, PersonIdentifierDefinition $definition): ?string
+                    {
+                        // trivial canonicalization for tests
+                        return isset($value['content']) ? (string) $value['content'] : null;
+                    }
+
+                    public function buildForm(\Symfony\Component\Form\FormBuilderInterface $builder, PersonIdentifierDefinition $personIdentifierDefinition): void {}
+
+                    public function renderAsString(?\Chill\PersonBundle\Entity\Identifier\PersonIdentifier $identifier, PersonIdentifierDefinition $definition): string
+                    {
+                        return '';
+                    }
+                };
+
+                return new PersonIdentifierWorker($engine, $definition);
+            }
+        };
+    }
+
+    public function testSupportsDenormalizationReturnsTrueForValidData(): void
+    {
+        $denormalizer = new PersonJsonDenormalizer($this->createIdentifierManager());
+
+        $data = [
+            'type' => 'person',
+            // important: new Person (creation) must not contain an id
+        ];
+
+        self::assertTrue($denormalizer->supportsDenormalization($data, Person::class));
+    }
+
+    public function testSupportsDenormalizationReturnsFalseForInvalidData(): void
+    {
+        $denormalizer = new PersonJsonDenormalizer($this->createIdentifierManager());
+
+        // not an array
+        self::assertFalse($denormalizer->supportsDenormalization('not-an-array', Person::class));
+
+        // missing type
+        self::assertFalse($denormalizer->supportsDenormalization([], Person::class));
+
+        // wrong type value
+        self::assertFalse($denormalizer->supportsDenormalization(['type' => 'not-person'], Person::class));
+
+        // id present means it's not a create payload for this denormalizer
+        self::assertFalse($denormalizer->supportsDenormalization(['type' => 'person', 'id' => 123], Person::class));
+
+        // wrong target class
+        self::assertFalse($denormalizer->supportsDenormalization(['type' => 'person'], \stdClass::class));
+    }
+
+    public function testDenormalizeMapsPayloadToPersonProperties(): void
+    {
+        $json = <<<'JSON'
+            {
+              "type": "person",
+              "firstName": "Jérome",
+              "lastName": "diallo",
+              "altNames": [
+                {
+                  "key": "jeune_fille",
+                  "value": "FJ"
+                }
+              ],
+              "birthdate": null,
+              "deathdate": null,
+              "phonenumber": "",
+              "mobilenumber": "",
+              "email": "",
+              "gender": {
+                "id": 5,
+                "type": "chill_main_gender"
+              },
+              "center": {
+                "id": 1,
+                "type": "center"
+              },
+              "civility": null,
+              "identifiers": [
+                {
+                  "type": "person_identifier",
+                  "value": {
+                    "content": "789456"
+                  },
+                  "definition_id": 5
+                }
+              ]
+            }
+            JSON;
+        $data = json_decode($json, true, 512, JSON_THROW_ON_ERROR);
+
+        $inner = new class () implements DenormalizerInterface {
+            public ?Gender $gender = null;
+            public ?Center $center = null;
+
+            public function denormalize($data, $type, $format = null, array $context = [])
+            {
+                if (PhoneNumber::class === $type) {
+                    return '' === $data ? null : new PhoneNumber();
+                }
+                if (\DateTime::class === $type || \DateTimeImmutable::class === $type) {
+                    return null === $data ? null : new \DateTimeImmutable((string) $data);
+                }
+                if (Gender::class === $type) {
+                    return $this->gender ??= new Gender();
+                }
+                if (Center::class === $type) {
+                    return $this->center ??= new Center();
+                }
+                if (Civility::class === $type) {
+                    return null; // input is null in our payload
+                }
+
+                return null;
+            }
+
+            public function supportsDenormalization($data, $type, $format = null)
+            {
+                return true;
+            }
+        };
+
+        $denormalizer = new PersonJsonDenormalizer($this->createIdentifierManager());
+        $denormalizer->setDenormalizer($inner);
+
+        $person = $denormalizer->denormalize($data, Person::class);
+
+        self::assertInstanceOf(Person::class, $person);
+        self::assertSame('Jérome', $person->getFirstName());
+        self::assertSame('diallo', $person->getLastName());
+
+        // phone numbers: empty strings map to null via the inner denormalizer stub
+        self::assertNull($person->getPhonenumber());
+        self::assertNull($person->getMobilenumber());
+
+        // email passes through as is
+        self::assertSame('', $person->getEmail());
+
+        // nested objects are provided by our inner denormalizer and must be set back on the Person
+        self::assertSame($inner->gender, $person->getGender());
+        self::assertSame($inner->center, $person->getCenter());
+
+        // dates are null in the provided payload
+        self::assertNull($person->getBirthdate());
+        self::assertNull($person->getDeathdate());
+
+        // civility is null as provided
+        self::assertNull($person->getCivility());
+
+        // altNames: make sure the alt name with key jeune_fille has label FJ
+        $found = false;
+        foreach ($person->getAltNames() as $altName) {
+            if ('jeune_fille' === $altName->getKey()) {
+                $found = true;
+                self::assertSame('FJ', $altName->getLabel());
+            }
+        }
+        self::assertTrue($found, 'Expected altNames to contain key "jeune_fille"');
+
+        $found = false;
+        foreach ($person->getIdentifiers() as $identifier) {
+            if (5 === $identifier->getDefinition()->getId()) {
+                $found = true;
+                self::assertSame(['content' => '789456'], $identifier->getValue());
+            }
+        }
+        self::assertTrue($found, 'Expected identifiers with definition id 5');
+    }
+}