From e9fc77c5b35634a587ee5aba2dcca2116fa0c4aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Ja=CC=88ger?= Date: Sat, 3 Dec 2016 12:44:28 +0100 Subject: [PATCH 1/3] [Validator] Fix issue 12302 - cache constraints This change allows to still cache constraints even when an uncachable constraint (i.e. Callback) is added to a parent class at runtime. This is achived by caching only the constraints that are loaded for the class in question only and merging parent and interface constraints after reading from cache. --- .../Factory/LazyLoadingMetadataFactory.php | 33 ++++++++----- .../LazyLoadingMetadataFactoryTest.php | 48 +++++++++++++++---- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php b/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php index 68236fc1eb069..82e1cf6bffd44 100644 --- a/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php +++ b/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php @@ -101,8 +101,11 @@ public function getMetadataFor($value) return $this->loadedClasses[$class]; } - if (null !== $this->cache && false !== ($this->loadedClasses[$class] = $this->cache->read($class))) { - return $this->loadedClasses[$class]; + if (null !== $this->cache && false !== ($metadata = $this->cache->read($class))) { + // Include constraints from the parent class + $this->mergeConstraints($metadata); + + return $this->loadedClasses[$class] = $metadata; } if (!class_exists($class) && !interface_exists($class)) { @@ -111,6 +114,22 @@ public function getMetadataFor($value) $metadata = new ClassMetadata($class); + if (null !== $this->loader) { + $this->loader->loadClassMetadata($metadata); + } + + if (null !== $this->cache) { + $this->cache->write($metadata); + } + + // Include constraints from the parent class + $this->mergeConstraints($metadata); + + return $this->loadedClasses[$class] = $metadata; + } + + private function mergeConstraints(ClassMetadata $metadata) + { // Include constraints from the parent class if ($parent = $metadata->getReflectionClass()->getParentClass()) { $metadata->mergeConstraints($this->getMetadataFor($parent->name)); @@ -141,16 +160,6 @@ public function getMetadataFor($value) } $metadata->mergeConstraints($this->getMetadataFor($interface->name)); } - - if (null !== $this->loader) { - $this->loader->loadClassMetadata($metadata); - } - - if (null !== $this->cache) { - $this->cache->write($metadata); - } - - return $this->loadedClasses[$class] = $metadata; } /** diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php index c1aaa9f8c7bf2..be73df4814b0f 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Validator\Tests\Mapping\Factory; +use Symfony\Component\Validator\Constraints\Callback; use Symfony\Component\Validator\Mapping\ClassMetadata; use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory; use Symfony\Component\Validator\Mapping\Loader\LoaderInterface; @@ -30,8 +31,8 @@ public function testLoadClassMetadataWithInterface() $metadata = $factory->getMetadataFor(self::PARENT_CLASS); $constraints = array( - new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))), new ConstraintA(array('groups' => array('Default', 'EntityParent'))), + new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))), ); $this->assertEquals($constraints, $metadata->getConstraints()); @@ -45,8 +46,6 @@ public function testMergeParentConstraints() $constraints = array( new ConstraintA(array('groups' => array( 'Default', - 'EntityInterfaceA', - 'EntityParent', 'Entity', ))), new ConstraintA(array('groups' => array( @@ -56,8 +55,8 @@ public function testMergeParentConstraints() ))), new ConstraintA(array('groups' => array( 'Default', - 'EntityParentInterface', - 'EntityInterfaceB', + 'EntityInterfaceA', + 'EntityParent', 'Entity', ))), new ConstraintA(array('groups' => array( @@ -67,6 +66,8 @@ public function testMergeParentConstraints() ))), new ConstraintA(array('groups' => array( 'Default', + 'EntityParentInterface', + 'EntityInterfaceB', 'Entity', ))), ); @@ -80,8 +81,8 @@ public function testWriteMetadataToCache() $factory = new LazyLoadingMetadataFactory(new TestLoader(), $cache); $parentClassConstraints = array( - new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))), new ConstraintA(array('groups' => array('Default', 'EntityParent'))), + new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))), ); $interfaceAConstraints = array( new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA'))), @@ -127,12 +128,43 @@ public function testReadMetadataFromCache() $cache->expects($this->never()) ->method('has'); - $cache->expects($this->once()) + $cache->expects($this->exactly(2)) ->method('read') - ->will($this->returnValue($metadata)); + ->withConsecutive( + array(self::PARENT_CLASS), + array(self::INTERFACE_A_CLASS) + ) + ->willReturnCallback(function ($name) use ($metadata) { + if (self::PARENT_CLASS == $name) { + return $metadata; + } + + return new ClassMetadata(self::INTERFACE_A_CLASS); + }); $this->assertEquals($metadata, $factory->getMetadataFor(self::PARENT_CLASS)); } + + public function testMetadataCacheWithRuntimeConstraint() + { + $cache = $this->getMock('Symfony\Component\Validator\Mapping\Cache\CacheInterface'); + $factory = new LazyLoadingMetadataFactory(new TestLoader(), $cache); + + $cache + ->expects($this->any()) + ->method('write') + ->will($this->returnCallback(function ($metadata) { serialize($metadata);})) + ; + + $cache->expects($this->any()) + ->method('read') + ->will($this->returnValue(false)); + + $metadata = $factory->getMetadataFor(self::PARENT_CLASS); + $metadata->addConstraint(new Callback(function () {})); + + $metadata = $factory->getMetadataFor(self::CLASS_NAME); + } } class TestLoader implements LoaderInterface From e61f311292954b5225d8691f52b50a0af6dd80ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Ja=CC=88ger?= Date: Wed, 7 Dec 2016 08:22:42 +0100 Subject: [PATCH 2/3] fix test for PHP 5.3 --- .../Mapping/Factory/LazyLoadingMetadataFactoryTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php index be73df4814b0f..2588c17c5197a 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php @@ -123,6 +123,8 @@ public function testReadMetadataFromCache() $metadata = new ClassMetadata(self::PARENT_CLASS); $metadata->addConstraint(new ConstraintA()); + $parentClass = self::PARENT_CLASS; + $loader->expects($this->never()) ->method('loadClassMetadata'); @@ -134,8 +136,8 @@ public function testReadMetadataFromCache() array(self::PARENT_CLASS), array(self::INTERFACE_A_CLASS) ) - ->willReturnCallback(function ($name) use ($metadata) { - if (self::PARENT_CLASS == $name) { + ->willReturnCallback(function ($name) use ($metadata, $parentClass) { + if ($parentClass == $name) { return $metadata; } From 427cc5cacef0cbef6318b43020179ccf4d413590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Ja=CC=88ger?= Date: Wed, 7 Dec 2016 08:51:11 +0100 Subject: [PATCH 3/3] fix test for PHP 5.3 --- .../Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php index 2588c17c5197a..a8295cd0ab8c0 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php @@ -124,6 +124,7 @@ public function testReadMetadataFromCache() $metadata->addConstraint(new ConstraintA()); $parentClass = self::PARENT_CLASS; + $interfaceClass = self::INTERFACE_A_CLASS; $loader->expects($this->never()) ->method('loadClassMetadata'); @@ -136,12 +137,12 @@ public function testReadMetadataFromCache() array(self::PARENT_CLASS), array(self::INTERFACE_A_CLASS) ) - ->willReturnCallback(function ($name) use ($metadata, $parentClass) { + ->willReturnCallback(function ($name) use ($metadata, $parentClass, $interfaceClass) { if ($parentClass == $name) { return $metadata; } - return new ClassMetadata(self::INTERFACE_A_CLASS); + return new ClassMetadata($interfaceClass); }); $this->assertEquals($metadata, $factory->getMetadataFor(self::PARENT_CLASS));