From ae380f98023d27711b3d571af18823247813eec1 Mon Sep 17 00:00:00 2001 From: Sylvain Fabre Date: Fri, 21 Aug 2026 11:23:28 +0200 Subject: [PATCH 1/5] Memoize constraint resolution and attribute lookups in EntityValidator Cache per (class, field): the resolved constraint set and the OnlyValidateOnUpdate attribute presence, and reuse a single PropertyAccessor instance, since all three only depend on static class metadata. Co-Authored-By: Claude Fable 5 --- src/Validator/Constraints/EntityValidator.php | 44 ++++- .../Constraints/EntityValidatorTest.php | 179 +++++++++++++++++- 2 files changed, 217 insertions(+), 6 deletions(-) diff --git a/src/Validator/Constraints/EntityValidator.php b/src/Validator/Constraints/EntityValidator.php index 86bbff1..6671b46 100644 --- a/src/Validator/Constraints/EntityValidator.php +++ b/src/Validator/Constraints/EntityValidator.php @@ -9,6 +9,7 @@ use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException; use Symfony\Component\PropertyAccess\PropertyAccess; +use Symfony\Component\PropertyAccess\PropertyAccessorInterface; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\All; use Symfony\Component\Validator\Constraints\NotNull; @@ -22,6 +23,11 @@ class EntityValidator extends ConstraintValidator private EntityManagerInterface $em; /** @var FieldConstraintsSetProviderInterface[] */ private iterable $fieldConstraintsSetFactories; + private ?PropertyAccessorInterface $propertyAccessor = null; + /** @var array>> */ + private array $constraintsCache = []; + /** @var array> */ + private array $onlyValidateOnUpdateCache = []; /** * @param FieldConstraintsSetProviderInterface[] $fieldConstraintsSetFactories @@ -48,7 +54,7 @@ public function validate(mixed $entity, Constraint $constraint): void $metadata = $this->em->getClassMetadata($class); $fields = array_keys(self::reflectionPropertiesToArray($metadata->getReflectionProperties())); $validator = $this->context->getValidator()->inContext($this->context); - $propertyAccessor = PropertyAccess::createPropertyAccessor(); + $propertyAccessor = $this->propertyAccessor ??= PropertyAccess::createPropertyAccessor(); foreach ($fields as $field) { if (!$this->checkIfFieldNeedsToBeValidated($entity, $field)) { @@ -89,9 +95,20 @@ public function getConstraintsForType(array $fieldMapping): array } /** + * Constraints only depend on the Doctrine mapping, so they are memoized per class and field. + * Sharing the constraint instances is safe: constraints are immutable value objects. + * * @return array */ public function getConstraints(string $class, string $field): array + { + return $this->constraintsCache[$class][$field] ??= $this->buildConstraints($class, $field); + } + + /** + * @return array + */ + private function buildConstraints(string $class, string $field): array { $metadata = $this->em->getClassMetadata($class); @@ -148,15 +165,32 @@ private static function reflectionPropertiesToArray(iterable $properties): array private function checkIfFieldNeedsToBeValidated(object $entity, string $field): bool { - $reflectionClass = new \ReflectionClass($entity::class); - $fieldAttributes = $this->getFieldAttributes($reflectionClass, $field); + $class = $entity::class; + $hasOnlyValidateOnUpdate = $this->onlyValidateOnUpdateCache[$class][$field] + ??= $this->hasOnlyValidateOnUpdateAttribute($class, $field); + + if ($hasOnlyValidateOnUpdate) { + return in_array($field, array_keys($this->em->getUnitOfWork()->getEntityChangeSet($entity)), true); + } + return true; + } + + /** + * Whether the field carries the OnlyValidateOnUpdate attribute only depends on the class + * definition, so it is memoized per class and field to avoid repeated reflection. + * + * @param class-string $class + */ + private function hasOnlyValidateOnUpdateAttribute(string $class, string $field): bool + { + $fieldAttributes = $this->getFieldAttributes(new \ReflectionClass($class), $field); foreach ($fieldAttributes as $attribute) { if (OnlyValidateOnUpdate::class === $attribute->getName()) { - return in_array($field, array_keys($this->em->getUnitOfWork()->getEntityChangeSet($entity)), true); + return true; } } - return true; + return false; } /** diff --git a/tests/Validator/Constraints/EntityValidatorTest.php b/tests/Validator/Constraints/EntityValidatorTest.php index 1150226..c5cd1af 100644 --- a/tests/Validator/Constraints/EntityValidatorTest.php +++ b/tests/Validator/Constraints/EntityValidatorTest.php @@ -9,6 +9,7 @@ use AssoConnect\ValidatorBundle\Test\Functional\App\Entity\MyEntityParent; use AssoConnect\ValidatorBundle\Validator\Constraints\Entity; use AssoConnect\ValidatorBundle\Validator\Constraints\EntityValidator; +use AssoConnect\ValidatorBundle\Validator\Constraints\OnlyValidateOnUpdate; use AssoConnect\ValidatorBundle\Validator\Constraints\Phone; use AssoConnect\ValidatorBundle\Validator\ConstraintsSetProvider\Field\PhoneProvider; use Doctrine\ORM\EntityManagerInterface; @@ -18,6 +19,8 @@ use Doctrine\ORM\Mapping\ManyToManyOwningSideMapping; use Doctrine\ORM\Mapping\ManyToOneAssociationMapping; use Doctrine\ORM\Mapping\OneToManyAssociationMapping; +use Doctrine\ORM\UnitOfWork; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\All; use Symfony\Component\Validator\Constraints\NotNull; @@ -30,7 +33,7 @@ */ class EntityValidatorTest extends ConstraintValidatorTestCase { - protected EntityManagerInterface $em; + protected EntityManagerInterface&MockObject $em; /** @var ClassMetadata */ protected ClassMetadata $classMetadata; @@ -94,6 +97,14 @@ public function testGetConstraintsForNotNullableField(): void ); } + public function testGetConstraintsIsMemoizedPerClassAndField(): void + { + $constraints = $this->validator->getConstraints('class', 'notnullable'); + + self::assertSame($constraints, $this->validator->getConstraints('class', 'notnullable')); + self::assertNotSame($constraints, $this->validator->getConstraints('class', 'nullable')); + } + public function testGetConstraintsForEmbeddable(): void { self::assertArrayContainsSameObjects( @@ -164,6 +175,172 @@ public function __construct() $this->validator->validate($entity, new Entity()); } + public function testValidateAppliesTheSameConstraintsToEveryInstanceOfAClass(): void + { + $createEntity = static fn(): object => new class { + public readonly ?string $nullable; + + public function __construct() + { + $this->nullable = '+33611223344'; + } + }; + $first = $createEntity(); + $second = $createEntity(); + $this->classMetadata->reflFields = [ + 'nullable' => new \ReflectionProperty($first, 'nullable'), + ]; + + $this->expectValidateValueAt(0, 'nullable', '+33611223344', [new Phone()]); + $this->expectValidateValueAt(1, 'nullable', '+33611223344', [new Phone()]); + + $this->validator->validate($first, new Entity()); + $this->validator->validate($second, new Entity()); + + self::assertSame( + $this->validator->getConstraints($first::class, 'nullable'), + $this->validator->getConstraints($second::class, 'nullable') + ); + } + + public function testValidateSkipsOnlyValidateOnUpdateFieldsOnInsert(): void + { + $entity = new class { + #[OnlyValidateOnUpdate] + public readonly ?string $nullable; + + public function __construct() + { + $this->nullable = '+33611223344'; + } + }; + $this->classMetadata->reflFields = [ + 'nullable' => new \ReflectionProperty($entity, 'nullable'), + ]; + $this->mockEntityChangeSet([]); + + $this->expectNoValidate(); + + $this->validator->validate($entity, new Entity()); + + self::assertNoViolation(); + } + + public function testValidateChecksOnlyValidateOnUpdateFieldsPartOfTheChangeSet(): void + { + $entity = new class { + #[OnlyValidateOnUpdate] + public readonly ?string $nullable; + + public function __construct() + { + $this->nullable = '+33611223344'; + } + }; + $this->classMetadata->reflFields = [ + 'nullable' => new \ReflectionProperty($entity, 'nullable'), + ]; + $this->mockEntityChangeSet(['nullable' => [null, '+33611223344']]); + + $this->expectValidateValueAt(0, 'nullable', '+33611223344', [new Phone()]); + + $this->validator->validate($entity, new Entity()); + } + + /** + * @param array $changeSet + */ + private function mockEntityChangeSet(array $changeSet): void + { + $unitOfWork = self::createStub(UnitOfWork::class); + $unitOfWork->method('getEntityChangeSet')->willReturn($changeSet); + $this->em->method('getUnitOfWork')->willReturn($unitOfWork); + } + + public function testGetConstraintsForNotNullableFieldWithOrm3MappingObject(): void + { + self::skipUnlessOrm3(); + + $metadata = new ClassMetadata(MyEntityParent::class); + $metadata->fieldMappings = [ + 'notnullable' => FieldMapping::fromMappingArray([ + 'type' => 'phone', + 'fieldName' => 'notnullable', + 'columnName' => 'notnullable', + 'nullable' => false, + ]), + ]; + + self::assertArrayContainsSameObjects( + $this->createValidatorForMetadata($metadata)->getConstraints('class', 'notnullable'), + [new NotNull(), new Phone()] + ); + } + + public function testGetConstraintsForRelationsWithOrm3MappingObjects(): void + { + self::skipUnlessOrm3(); + + $metadata = new ClassMetadata(MyEntityParent::class); + $metadata->associationMappings = [ + 'notowning' => OneToManyAssociationMapping::fromMappingArray([ + 'fieldName' => 'notowning', + 'sourceEntity' => MyEntityParent::class, + 'targetEntity' => MyEntityParent::class, + 'mappedBy' => 'parent', + ]), + 'owningToOne' => ManyToOneAssociationMapping::fromMappingArray([ + 'fieldName' => 'owningToOne', + 'sourceEntity' => MyEntityParent::class, + 'targetEntity' => MyEntityParent::class, + ]), + 'owningToOneNotNull' => ManyToOneAssociationMapping::fromMappingArray([ + 'fieldName' => 'owningToOneNotNull', + 'sourceEntity' => MyEntityParent::class, + 'targetEntity' => MyEntityParent::class, + 'joinColumns' => [['name' => 'parent_id', 'referencedColumnName' => 'id', 'nullable' => false]], + ]), + 'owningToMany' => ManyToManyOwningSideMapping::fromMappingArray([ + 'fieldName' => 'owningToMany', + 'sourceEntity' => MyEntityParent::class, + 'targetEntity' => MyEntityParent::class, + ]), + ]; + $validator = $this->createValidatorForMetadata($metadata); + + self::assertEmpty($validator->getConstraints('class', 'notowning')); + self::assertArrayContainsSameObjects( + $validator->getConstraints('class', 'owningToOne'), + [new Type(MyEntityParent::class)] + ); + self::assertArrayContainsSameObjects( + $validator->getConstraints('class', 'owningToOneNotNull'), + [new Type(MyEntityParent::class), new NotNull()] + ); + self::assertArrayContainsSameObjects( + $validator->getConstraints('class', 'owningToMany'), + [new All(constraints: [new Type(MyEntityParent::class)])] + ); + } + + private static function skipUnlessOrm3(): void + { + if (!class_exists(FieldMapping::class)) { + self::markTestSkipped('Requires the doctrine/orm 3 mapping objects'); + } + } + + /** + * @param ClassMetadata $metadata + */ + private function createValidatorForMetadata(ClassMetadata $metadata): EntityValidator + { + $em = $this->createMock(EntityManagerInterface::class); + $em->method('getClassMetadata')->willReturn($metadata); + + return new EntityValidator($em, [new PhoneProvider()]); + } + public static function providerInvalidValues(): iterable { return []; From c56577e9cc14b26bc161b05c27a0772944924538 Mon Sep 17 00:00:00 2001 From: Sylvain Fabre Date: Sat, 22 Aug 2026 13:39:49 +0200 Subject: [PATCH 2/5] Require shipmonk/phpstan-rules ^4.4 so the lowest-deps run supports PHPStan 2.2 The CI upgrades PHPStan to latest after the prefer-lowest install; shipmonk 4.2 predates the ExpressionResultStorage API and crashes on files using callables. Co-Authored-By: Claude Fable 5 --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 381c659..6edb6f9 100644 --- a/composer.json +++ b/composer.json @@ -56,7 +56,8 @@ "assoconnect/php-quality-config": "^2.2", "guzzlehttp/guzzle": "^7.7", "guzzlehttp/psr7": "^2.4.5", - "phpstan/phpstan-symfony": "^2" + "phpstan/phpstan-symfony": "^2", + "shipmonk/phpstan-rules": "^4.4" }, "config": { "allow-plugins": { From 120a028b00d6c50a5aed3371ab9be81f52cc0a71 Mon Sep 17 00:00:00 2001 From: Sylvain Fabre Date: Sun, 23 Aug 2026 22:25:00 +0200 Subject: [PATCH 3/5] Require php-quality-config ^2.4 so lowest-deps runs work with latest Rector Older releases register StaticArrowFunctionRector, which current Rector rejects. Co-Authored-By: Claude Fable 5 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 6edb6f9..99a8d48 100644 --- a/composer.json +++ b/composer.json @@ -53,7 +53,7 @@ "symfony/var-exporter": "^7.0", "symfony/yaml": "^7.0", "dg/bypass-finals": "^1.1", - "assoconnect/php-quality-config": "^2.2", + "assoconnect/php-quality-config": "^2.4", "guzzlehttp/guzzle": "^7.7", "guzzlehttp/psr7": "^2.4.5", "phpstan/phpstan-symfony": "^2", From 010433f916fc286e6e63b910d2a7bd84b2268ede Mon Sep 17 00:00:00 2001 From: Sylvain Fabre Date: Mon, 24 Aug 2026 10:29:08 +0200 Subject: [PATCH 4/5] Keep phpstan/phpstan-phpunit in sync with the forced phpunit/phpunit upgrade on lowest-deps CI Co-Authored-By: Claude Sonnet 5 --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5c70faa..cf09314 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -31,7 +31,7 @@ jobs: - name: Upgrade analysis tools to latest if: ${{ matrix.dependency-versions == 'lowest' }} - run: composer update phpstan/phpstan rector/rector phpunit/phpunit squizlabs/php_codesniffer --with-all-dependencies --no-interaction + run: composer update phpstan/phpstan phpstan/phpstan-phpunit rector/rector phpunit/phpunit squizlabs/php_codesniffer --with-all-dependencies --no-interaction - run: vendor/bin/phpcs if: ${{ failure() || success() }} From 8b168e6b0248d42ca47dc30b1bbae9c5379bfe74 Mon Sep 17 00:00:00 2001 From: Sylvain Fabre Date: Mon, 24 Aug 2026 11:13:47 +0200 Subject: [PATCH 5/5] Revert composer.json floor bumps for php-quality-config and shipmonk/phpstan-rules; fold them into the lowest-deps tool-upgrade step instead Co-Authored-By: Claude Sonnet 5 --- .github/workflows/build.yml | 2 +- composer.json | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cf09314..a7adce9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -31,7 +31,7 @@ jobs: - name: Upgrade analysis tools to latest if: ${{ matrix.dependency-versions == 'lowest' }} - run: composer update phpstan/phpstan phpstan/phpstan-phpunit rector/rector phpunit/phpunit squizlabs/php_codesniffer --with-all-dependencies --no-interaction + run: composer update phpstan/phpstan phpstan/phpstan-phpunit shipmonk/phpstan-rules rector/rector assoconnect/php-quality-config phpunit/phpunit squizlabs/php_codesniffer --with-all-dependencies --no-interaction - run: vendor/bin/phpcs if: ${{ failure() || success() }} diff --git a/composer.json b/composer.json index 99a8d48..381c659 100644 --- a/composer.json +++ b/composer.json @@ -53,11 +53,10 @@ "symfony/var-exporter": "^7.0", "symfony/yaml": "^7.0", "dg/bypass-finals": "^1.1", - "assoconnect/php-quality-config": "^2.4", + "assoconnect/php-quality-config": "^2.2", "guzzlehttp/guzzle": "^7.7", "guzzlehttp/psr7": "^2.4.5", - "phpstan/phpstan-symfony": "^2", - "shipmonk/phpstan-rules": "^4.4" + "phpstan/phpstan-symfony": "^2" }, "config": { "allow-plugins": {