diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php index c570cd79b..2f873a8ab 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/Identifier/StringIdentifier.php @@ -13,10 +13,10 @@ namespace Chill\PersonBundle\PersonIdentifier\Identifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; +use Chill\PersonBundle\PersonIdentifier\IdentifierViolationDTO; use Chill\PersonBundle\PersonIdentifier\PersonIdentifierEngineInterface; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormBuilderInterface; -use Symfony\Component\Validator\Context\ExecutionContextInterface; final readonly class StringIdentifier implements PersonIdentifierEngineInterface { @@ -50,20 +50,24 @@ final readonly class StringIdentifier implements PersonIdentifierEngineInterface return '' === trim($identifier->getValue()['content'] ?? ''); } - public function validate(ExecutionContextInterface $context, PersonIdentifier $identifier, PersonIdentifierDefinition $definition): void + public function validate(PersonIdentifier $identifier, PersonIdentifierDefinition $definition): array { $config = $definition->getData(); $content = (string) ($identifier->getValue()['content'] ?? ''); + $violations = []; if (($config[self::ONLY_NUMBERS] ?? false) && !preg_match('/^[0-9]+$/', $content)) { - $context->buildViolation('person_identifier.only_number') - ->addViolation(); + $violations[] = new IdentifierViolationDTO('person_identifier.only_number', '2a3352c0-a2b9-11f0-a767-b7a3f80e52f1'); } if (null !== ($config[self::FIXED_LENGTH] ?? null) && strlen($content) !== $config[self::FIXED_LENGTH]) { - $context->buildViolation('person_identifier.fixed_length') - ->setParameter('limit', (string) $config[self::FIXED_LENGTH]) - ->addViolation(); + $violations[] = new IdentifierViolationDTO( + 'person_identifier.fixed_length', + '2b02a8fe-a2b9-11f0-bfe5-033300972783', + ['limit' => (string) $config[self::FIXED_LENGTH]] + ); } + + return $violations; } } diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/IdentifierViolationDTO.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/IdentifierViolationDTO.php new file mode 100644 index 000000000..3a74a4b34 --- /dev/null +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/IdentifierViolationDTO.php @@ -0,0 +1,31 @@ + + */ + public array $parameters = [], + public string $messageDomain = 'validators', + ) {} +} diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php index f68dd86d7..4ebfbbabd 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierEngineInterface.php @@ -14,7 +14,6 @@ namespace Chill\PersonBundle\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; use Symfony\Component\Form\FormBuilderInterface; -use Symfony\Component\Validator\Context\ExecutionContextInterface; interface PersonIdentifierEngineInterface { @@ -34,5 +33,10 @@ interface PersonIdentifierEngineInterface */ public function isEmpty(PersonIdentifier $identifier): bool; - public function validate(ExecutionContextInterface $context, PersonIdentifier $identifier, PersonIdentifierDefinition $definition): void; + /** + * Return a list of @link{IdentifierViolationDTO} to generatie violation errors. + * + * @return list + */ + public function validate(PersonIdentifier $identifier, PersonIdentifierDefinition $definition): array; } diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php index 3bc2cf1be..71aa65468 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/PersonIdentifierWorker.php @@ -14,7 +14,6 @@ namespace Chill\PersonBundle\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; use Symfony\Component\Form\FormBuilderInterface; -use Symfony\Component\Validator\Context\ExecutionContextInterface; readonly class PersonIdentifierWorker { @@ -56,8 +55,11 @@ readonly class PersonIdentifierWorker return $this->identifierEngine->isEmpty($identifier); } - public function validate(ExecutionContextInterface $context, PersonIdentifier $identifier, PersonIdentifierDefinition $definition): void + /** + * @return list + */ + public function validate(PersonIdentifier $identifier, PersonIdentifierDefinition $definition): array { - $this->identifierEngine->validate($context, $identifier, $definition); + return $this->identifierEngine->validate($identifier, $definition); } } diff --git a/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/ValidIdentifierConstraintValidator.php b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/ValidIdentifierConstraintValidator.php index b6fbdc40c..296f5e3ee 100644 --- a/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/ValidIdentifierConstraintValidator.php +++ b/src/Bundle/ChillPersonBundle/PersonIdentifier/Validator/ValidIdentifierConstraintValidator.php @@ -34,6 +34,14 @@ final class ValidIdentifierConstraintValidator extends ConstraintValidator $worker = $this->personIdentifierManager->buildWorkerByPersonIdentifierDefinition($value->getDefinition()); - $worker->validate($this->context, $value, $value->getDefinition()); + $violations = $worker->validate($value, $value->getDefinition()); + + foreach ($violations as $violation) { + $this->context->buildViolation($violation->message) + ->setParameters($violation->parameters) + ->setParameter('{{ code }}', $violation->code) + ->setParameter('definition_id', (string) $value->getDefinition()->getId()) + ->addViolation(); + } } } diff --git a/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Identifier/StringIdentifierValidationTest.php b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Identifier/StringIdentifierValidationTest.php index 3b8190896..436c15589 100644 --- a/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Identifier/StringIdentifierValidationTest.php +++ b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Identifier/StringIdentifierValidationTest.php @@ -14,10 +14,7 @@ namespace Chill\PersonBundle\Tests\PersonIdentifier\Identifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; use Chill\PersonBundle\PersonIdentifier\Identifier\StringIdentifier; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Symfony\Component\Validator\Context\ExecutionContextInterface; -use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface; /** * @internal @@ -28,10 +25,15 @@ class StringIdentifierValidationTest extends TestCase { private function makeDefinition(array $data = []): PersonIdentifierDefinition { - return new PersonIdentifierDefinition(label: ['en' => 'String'], engine: StringIdentifier::getName(), data: $data); + $definition = new PersonIdentifierDefinition(label: ['en' => 'Test'], engine: StringIdentifier::NAME); + if ([] !== $data) { + $definition->setData($data); + } + + return $definition; } - private function makeIdentifier(PersonIdentifierDefinition $definition, string $content): PersonIdentifier + private function makeIdentifier(PersonIdentifierDefinition $definition, ?string $content): PersonIdentifier { $identifier = new PersonIdentifier($definition); $identifier->setValue(['content' => $content]); @@ -39,90 +41,71 @@ class StringIdentifierValidationTest extends TestCase return $identifier; } - /** - * Helper creating a context mock expecting zero violations. - */ - private function expectNoViolation(): ExecutionContextInterface + public function testValidateWithoutOptionsHasNoViolations(): void { - $context = $this->createMock(ExecutionContextInterface::class); - $context->expects(self::never())->method('buildViolation'); + $definition = $this->makeDefinition(); + $identifier = $this->makeIdentifier($definition, 'AB-123'); - return $context; - } - - /** - * Helper creating a context mock expecting a single violation with a given message and optional parameters chain. - */ - private function expectSingleViolation(string $message, array $parameters = []): ExecutionContextInterface - { - /** @var ConstraintViolationBuilderInterface&MockObject $builder */ - $builder = $this->createMock(ConstraintViolationBuilderInterface::class); - - // If parameters are provided, ensure setParameter is called with each, in any order, and returns builder for chaining - if ($parameters) { - foreach ($parameters as $name => $value) { - $builder->expects(self::atLeastOnce()) - ->method('setParameter') - ->with($name, $value) - ->willReturn($builder); - } - } else { - $builder->expects(self::any())->method('setParameter')->willReturn($builder); - } - - $builder->expects(self::once())->method('addViolation'); - - $context = $this->createMock(ExecutionContextInterface::class); - $context->expects(self::once()) - ->method('buildViolation') - ->with($message) - ->willReturn($builder); - - return $context; - } - - public function testNoOptionsAcceptsAnyString(): void - { $engine = new StringIdentifier(); - $definition = $this->makeDefinition([]); - $identifier = $this->makeIdentifier($definition, 'abc123'); + $violations = $engine->validate($identifier, $definition); - $engine->validate($this->expectNoViolation(), $identifier, $definition); + self::assertIsArray($violations); + self::assertCount(0, $violations); } - public function testOnlyNumbersValid(): void + public function testValidateOnlyNumbersOption(): void { - $engine = new StringIdentifier(); $definition = $this->makeDefinition(['only_numbers' => true]); - $identifier = $this->makeIdentifier($definition, '123456'); + $engine = new StringIdentifier(); - $engine->validate($this->expectNoViolation(), $identifier, $definition); + // valid numeric content + $identifierOk = $this->makeIdentifier($definition, '123456'); + $violationsOk = $engine->validate($identifierOk, $definition); + self::assertCount(0, $violationsOk); + + // invalid alphanumeric content + $identifierBad = $this->makeIdentifier($definition, '12AB'); + $violationsBad = $engine->validate($identifierBad, $definition); + self::assertCount(1, $violationsBad); + self::assertSame('person_identifier.only_number', $violationsBad[0]->message); + self::assertSame('2a3352c0-a2b9-11f0-a767-b7a3f80e52f1', $violationsBad[0]->code); } - public function testOnlyNumbersInvalid(): void + public function testValidateFixedLengthOption(): void { - $engine = new StringIdentifier(); - $definition = $this->makeDefinition(['only_numbers' => true]); - $identifier = $this->makeIdentifier($definition, '12AB'); - - $engine->validate($this->expectSingleViolation('person_identifier.only_number'), $identifier, $definition); - } - - public function testFixedLengthValid(): void - { - $engine = new StringIdentifier(); $definition = $this->makeDefinition(['fixed_length' => 5]); - $identifier = $this->makeIdentifier($definition, '12345'); + $engine = new StringIdentifier(); - $engine->validate($this->expectNoViolation(), $identifier, $definition); + // valid exact length + $identifierOk = $this->makeIdentifier($definition, 'ABCDE'); + $violationsOk = $engine->validate($identifierOk, $definition); + self::assertCount(0, $violationsOk); + + // invalid length (too short) + $identifierBad = $this->makeIdentifier($definition, 'AB'); + $violationsBad = $engine->validate($identifierBad, $definition); + self::assertCount(1, $violationsBad); + self::assertSame('person_identifier.fixed_length', $violationsBad[0]->message); + self::assertSame('2b02a8fe-a2b9-11f0-bfe5-033300972783', $violationsBad[0]->code); + self::assertSame(['limit' => '5'], $violationsBad[0]->parameters); } - public function testFixedLengthInvalid(): void + public function testValidateOnlyNumbersAndFixedLengthTogether(): void { + $definition = $this->makeDefinition(['only_numbers' => true, 'fixed_length' => 4]); $engine = new StringIdentifier(); - $definition = $this->makeDefinition(['fixed_length' => 5]); - $identifier = $this->makeIdentifier($definition, '1234'); - $engine->validate($this->expectSingleViolation('person_identifier.fixed_length', ['limit' => 5]), $identifier, $definition); + // valid: numeric and correct length + $identifierOk = $this->makeIdentifier($definition, '1234'); + $violationsOk = $engine->validate($identifierOk, $definition); + self::assertCount(0, $violationsOk); + + // invalid: non-numeric and wrong length -> two violations expected + $identifierBad = $this->makeIdentifier($definition, 'AB'); + $violationsBad = $engine->validate($identifierBad, $definition); + self::assertCount(2, $violationsBad); + // Order is defined by implementation: numbers check first, then length + self::assertSame('person_identifier.only_number', $violationsBad[0]->message); + self::assertSame('person_identifier.fixed_length', $violationsBad[1]->message); } } diff --git a/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/ValidIdentifierConstraintValidatorTest.php b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/ValidIdentifierConstraintValidatorTest.php index 8f698c497..e7860a2c0 100644 --- a/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/ValidIdentifierConstraintValidatorTest.php +++ b/src/Bundle/ChillPersonBundle/Tests/PersonIdentifier/Validator/ValidIdentifierConstraintValidatorTest.php @@ -13,12 +13,15 @@ namespace Chill\PersonBundle\Tests\PersonIdentifier\Validator; use Chill\PersonBundle\Entity\Identifier\PersonIdentifier; use Chill\PersonBundle\Entity\Identifier\PersonIdentifierDefinition; +use Chill\PersonBundle\PersonIdentifier\IdentifierViolationDTO; +use Chill\PersonBundle\PersonIdentifier\PersonIdentifierEngineInterface; use Chill\PersonBundle\PersonIdentifier\PersonIdentifierManagerInterface; use Chill\PersonBundle\PersonIdentifier\PersonIdentifierWorker; use Chill\PersonBundle\PersonIdentifier\Validator\ValidIdentifierConstraint; use Chill\PersonBundle\PersonIdentifier\Validator\ValidIdentifierConstraintValidator; use PHPUnit\Framework\Attributes\CoversClass; use Prophecy\PhpUnit\ProphecyTrait; +use Prophecy\Prophecy\ObjectProphecy; use Symfony\Component\Validator\Test\ConstraintValidatorTestCase; /** @@ -29,51 +32,79 @@ final class ValidIdentifierConstraintValidatorTest extends ConstraintValidatorTe { use ProphecyTrait; - private object $manager; - private PersonIdentifierManagerInterface|\Prophecy\Prophecy\ObjectProphecy $managerProphecy; - - protected function createValidator(): ValidIdentifierConstraintValidator - { - // $this->manager is set in setUp() before parent::setUp() calls this method - return new ValidIdentifierConstraintValidator($this->manager); - } + /** + * @var ObjectProphecy|PersonIdentifierManagerInterface + */ + private ObjectProphecy $manager; protected function setUp(): void { - // Prepare manager prophecy and reveal it before parent::setUp() - $managerProphecy = $this->prophesize(PersonIdentifierManagerInterface::class); - $this->manager = $managerProphecy->reveal(); - - // Store the prophecy itself for later configuration in tests - $this->managerProphecy = $managerProphecy; // dynamic property for test methods - + $this->manager = $this->prophesize(PersonIdentifierManagerInterface::class); parent::setUp(); } - public function testItCallsEngineValidateThroughWorker(): void + protected function createValidator(): ValidIdentifierConstraintValidator { - // Arrange a definition and corresponding identifier - $definition = new PersonIdentifierDefinition(label: ['en' => 'Any'], engine: 'any.engine'); + return new ValidIdentifierConstraintValidator($this->manager->reveal()); + } + + public function testAddsViolationFromWorker(): void + { + $definition = new PersonIdentifierDefinition(['en' => 'SSN'], 'string'); + // set definition id via reflection for definition_id parameter + $ref = new \ReflectionClass($definition); + $prop = $ref->getProperty('id'); + $prop->setValue($definition, 1); + $identifier = new PersonIdentifier($definition); + $identifier->setValue(['value' => 'bad']); - // Create an engine prophecy; we only care that validate() is called once with expected args - $engineProphecy = $this->prophesize(\Chill\PersonBundle\PersonIdentifier\PersonIdentifierEngineInterface::class); - $engineProphecy - ->validate($this->context, $identifier, $definition) - ->shouldBeCalled(); + $violation = new IdentifierViolationDTO('Invalid Identifier', '0000-1111-2222-3333', ['{{ foo }}' => 'bar']); - // Build a real worker that wraps our mocked engine and the concrete definition - $worker = new PersonIdentifierWorker($engineProphecy->reveal(), $definition); + // engine that returns one violation + $engine = new class ([$violation]) implements PersonIdentifierEngineInterface { + public function __construct(private array $violations) {} - // Configure the manager to return our worker for this definition - $this->managerProphecy + public static function getName(): string + { + return 'dummy'; + } + + public function canonicalizeValue(array $value, PersonIdentifierDefinition $definition): ?string + { + return null; + } + + public function buildForm(\Symfony\Component\Form\FormBuilderInterface $builder, PersonIdentifierDefinition $personIdentifierDefinition): void {} + + public function renderAsString(?PersonIdentifier $identifier, PersonIdentifierDefinition $definition): string + { + return ''; + } + + public function isEmpty(PersonIdentifier $identifier): bool + { + return false; + } + + public function validate(PersonIdentifier $identifier, PersonIdentifierDefinition $definition): array + { + return $this->violations; + } + }; + $worker = new PersonIdentifierWorker($engine, $definition); + + $this->manager ->buildWorkerByPersonIdentifierDefinition($definition) ->willReturn($worker); - // Act: run the validator - $this->validator->validate($identifier, new ValidIdentifierConstraint()); + $constraint = new ValidIdentifierConstraint(); + $this->validator->validate($identifier, $constraint); - // Assert: no explicit assertion needed; Prophecy will fail if method wasn't called - $this->assertNoViolation(); + $this->buildViolation('Invalid Identifier') + ->setParameters(['{{ foo }}' => 'bar']) + ->setParameter('{{ code }}', '0000-1111-2222-3333') + ->setParameter('definition_id', '1') + ->assertRaised(); } }