From 4207efd6bfcf2875e3302b0e508c020915b99901 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 18 Sep 2025 13:32:09 +0200 Subject: [PATCH] Remove empty `PersonIdentifier` values during denormalization and add `isEmpty` logic to `PersonIdentifierWorker`. Include tests for empty value handling. --- .../Identifier/StringIdentifier.php | 5 ++ .../PersonIdentifierEngineInterface.php | 8 ++ .../PersonIdentifierWorker.php | 8 ++ .../Normalizer/PersonJsonDenormalizer.php | 4 + .../Normalizer/PersonJsonDenormalizerTest.php | 76 ++++++++++++++++++- 5 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php index 0801d4947..9979d0f83 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php @@ -38,4 +38,9 @@ final readonly class StringIdentifier implements PersonIdentifierEngineInterface { return $identifier?->getValue()['content'] ?? ''; } + + public function isEmpty(PersonIdentifier $identifier): bool + { + return '' !== trim($identifier->getValue()['content'] ?? ''); + } } diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php index 6c75f263e..b467cf3d0 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php @@ -24,4 +24,12 @@ interface PersonIdentifierEngineInterface public function buildForm(FormBuilderInterface $builder, PersonIdentifierDefinition $personIdentifierDefinition): void; public function renderAsString(?PersonIdentifier $identifier, PersonIdentifierDefinition $definition): string; + + /** + * Return true if the identifier must be considered as empty. + * + * This is in use when the identifier is validated and must be required. If the identifier is empty and is required + * by the definition, the validation will fails. + */ + public function isEmpty(PersonIdentifier $identifier): bool; } diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php index 94702b8eb..dc0279a21 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php @@ -46,4 +46,12 @@ final readonly class PersonIdentifierWorker { return $this->identifierEngine->renderAsString($identifier, $this->definition); } + + /** + * Return true if the identifier must be considered as empty. + */ + public function isEmpty(PersonIdentifier $identifier): bool + { + return $this->identifierEngine->isEmpty($identifier); + } } diff --git a/src/Bundle/ChillPersonBundle/Serializer/Normalizer/PersonJsonDenormalizer.php b/src/Bundle/ChillPersonBundle/Serializer/Normalizer/PersonJsonDenormalizer.php index cfd43ca6d..a65a84261 100644 --- a/src/Bundle/ChillPersonBundle/Serializer/Normalizer/PersonJsonDenormalizer.php +++ b/src/Bundle/ChillPersonBundle/Serializer/Normalizer/PersonJsonDenormalizer.php @@ -129,6 +129,10 @@ final class PersonJsonDenormalizer implements DenormalizerInterface, Denormalize $personIdentifier->setValue($value); $personIdentifier->setCanonical($worker->canonicalizeValue($value)); + + if ($worker->isEmpty($personIdentifier)) { + $person->removeIdentifier($personIdentifier); + } } } diff --git a/src/Bundle/ChillPersonBundle/Tests/Serializer/Normalizer/PersonJsonDenormalizerTest.php b/src/Bundle/ChillPersonBundle/Tests/Serializer/Normalizer/PersonJsonDenormalizerTest.php index aec345ff6..cfa947d44 100644 --- a/src/Bundle/ChillPersonBundle/Tests/Serializer/Normalizer/PersonJsonDenormalizerTest.php +++ b/src/Bundle/ChillPersonBundle/Tests/Serializer/Normalizer/PersonJsonDenormalizerTest.php @@ -14,6 +14,7 @@ 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\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; use Chill\PersonBundle\Entity\Person; use Chill\PersonBundle\PersonIdentifier\PersonIdentifierEngineInterface; @@ -23,6 +24,7 @@ use Chill\PersonBundle\Serializer\Normalizer\PersonJsonDenormalizer; use libphonenumber\PhoneNumber; use PHPUnit\Framework\TestCase; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; +use Symfony\Component\Serializer\Normalizer\AbstractNormalizer; /** * @internal @@ -65,10 +67,18 @@ final class PersonJsonDenormalizerTest extends TestCase public function buildForm(\Symfony\Component\Form\FormBuilderInterface $builder, PersonIdentifierDefinition $personIdentifierDefinition): void {} - public function renderAsString(?\Chill\PersonBundle\Entity\Identifier\PersonIdentifier $identifier, PersonIdentifierDefinition $definition): string + public function renderAsString(?PersonIdentifier $identifier, PersonIdentifierDefinition $definition): string { return ''; } + + public function isEmpty(PersonIdentifier $identifier): bool + { + $value = $identifier->getValue(); + $content = isset($value['content']) ? trim((string) $value['content']) : ''; + + return '' === $content; + } }; return new PersonIdentifierWorker($engine, $definition); @@ -225,4 +235,68 @@ final class PersonJsonDenormalizerTest extends TestCase } self::assertTrue($found, 'Expected identifiers with definition id 5'); } + + public function testDenormalizeRemovesEmptyIdentifier(): void + { + $data = [ + 'type' => 'person', + 'firstName' => 'Alice', + 'lastName' => 'Smith', + 'identifiers' => [ + [ + 'type' => 'person_identifier', + 'value' => ['content' => ''], + 'definition_id' => 7, + ], + ], + ]; + + $denormalizer = new PersonJsonDenormalizer($this->createIdentifierManager()); + + $person = $denormalizer->denormalize($data, Person::class); + + // The identifier with empty content must be considered empty and removed + self::assertSame(0, $person->getIdentifiers()->count(), 'Expected no identifiers to remain on the person'); + } + + public function testDenormalizeRemovesPreviouslyExistingIdentifierWhenIncomingValueIsEmpty(): void + { + // Prepare an existing Person with a pre-existing identifier (definition id = 9) + $definition = new PersonIdentifierDefinition(['en' => 'Test'], 'dummy'); + $ref = new \ReflectionProperty(PersonIdentifierDefinition::class, 'id'); + $ref->setValue($definition, 9); + + $existingIdentifier = new PersonIdentifier($definition); + $existingIdentifier->setValue(['content' => 'ABC']); + + $person = new Person(); + $person->addIdentifier($existingIdentifier); + + // Also set the identifier's own id = 9 so that the denormalizer logic matches it + // (the current denormalizer matches by PersonIdentifier->getId() === definition_id) + $refId = new \ReflectionProperty(PersonIdentifier::class, 'id'); + $refId->setValue($existingIdentifier, 9); + + // Incoming payload sets the same definition id with an empty value + $data = [ + 'type' => 'person', + 'identifiers' => [ + [ + 'type' => 'person_identifier', + 'value' => ['content' => ''], + 'definition_id' => 9, + ], + ], + ]; + + $denormalizer = new PersonJsonDenormalizer($this->createIdentifierManager()); + + // Use AbstractNormalizer::OBJECT_TO_POPULATE to update the existing person + $result = $denormalizer->denormalize($data, Person::class, null, [ + AbstractNormalizer::OBJECT_TO_POPULATE => $person, + ]); + + self::assertSame($person, $result, 'Denormalizer should update and return the provided Person instance'); + self::assertSame(0, $person->getIdentifiers()->count(), 'The previously existing identifier should be removed when incoming value is empty'); + } }