Skip to content

Commit

Permalink
bug #20793 [Validator] Fix caching of constraints derived from non-se…
Browse files Browse the repository at this point in the history
…rializable parents (uwej711)

This PR was squashed before being merged into the 2.7 branch (closes #20793).

Discussion
----------

[Validator] Fix caching of constraints derived from non-serializable parents

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12302
| License       | MIT
| Doc PR        | -

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.

Commits
-------

9dd6b0c [Validator] Fix caching of constraints derived from non-serializable parents
  • Loading branch information
nicolas-grekas committed Jan 12, 2017
2 parents 17ce5f5 + 9dd6b0c commit 4769ca2
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 20 deletions.
Expand Up @@ -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)) {
Expand All @@ -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));
Expand Down Expand Up @@ -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;
}

/**
Expand Down
Expand Up @@ -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;
Expand All @@ -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());
Expand All @@ -45,8 +46,6 @@ public function testMergeParentConstraints()
$constraints = array(
new ConstraintA(array('groups' => array(
'Default',
'EntityInterfaceA',
'EntityParent',
'Entity',
))),
new ConstraintA(array('groups' => array(
Expand All @@ -56,8 +55,8 @@ public function testMergeParentConstraints()
))),
new ConstraintA(array('groups' => array(
'Default',
'EntityParentInterface',
'EntityInterfaceB',
'EntityInterfaceA',
'EntityParent',
'Entity',
))),
new ConstraintA(array('groups' => array(
Expand All @@ -67,6 +66,8 @@ public function testMergeParentConstraints()
))),
new ConstraintA(array('groups' => array(
'Default',
'EntityParentInterface',
'EntityInterfaceB',
'Entity',
))),
);
Expand All @@ -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'))),
Expand Down Expand Up @@ -122,17 +123,51 @@ public function testReadMetadataFromCache()
$metadata = new ClassMetadata(self::PARENT_CLASS);
$metadata->addConstraint(new ConstraintA());

$parentClass = self::PARENT_CLASS;
$interfaceClass = self::INTERFACE_A_CLASS;

$loader->expects($this->never())
->method('loadClassMetadata');

$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, $parentClass, $interfaceClass) {
if ($parentClass == $name) {
return $metadata;
}

return new ClassMetadata($interfaceClass);
});

$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
Expand Down

0 comments on commit 4769ca2

Please sign in to comment.