Skip to content

Commit 4769ca2

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#20793 [Validator] Fix caching of constraints derived from non-serializable parents (uwej711)
This PR was squashed before being merged into the 2.7 branch (closes symfony#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 | symfony#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
2 parents 17ce5f5 + 9dd6b0c commit 4769ca2

File tree

2 files changed

+64
-20
lines changed

2 files changed

+64
-20
lines changed

‎src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,11 @@ public function getMetadataFor($value)
101101
return $this->loadedClasses[$class];
102102
}
103103

104-
if (null !== $this->cache && false !== ($this->loadedClasses[$class] = $this->cache->read($class))) {
105-
return $this->loadedClasses[$class];
104+
if (null !== $this->cache && false !== ($metadata = $this->cache->read($class))) {
105+
// Include constraints from the parent class
106+
$this->mergeConstraints($metadata);
107+
108+
return $this->loadedClasses[$class] = $metadata;
106109
}
107110

108111
if (!class_exists($class) && !interface_exists($class)) {
@@ -111,6 +114,22 @@ public function getMetadataFor($value)
111114

112115
$metadata = new ClassMetadata($class);
113116

117+
if (null !== $this->loader) {
118+
$this->loader->loadClassMetadata($metadata);
119+
}
120+
121+
if (null !== $this->cache) {
122+
$this->cache->write($metadata);
123+
}
124+
125+
// Include constraints from the parent class
126+
$this->mergeConstraints($metadata);
127+
128+
return $this->loadedClasses[$class] = $metadata;
129+
}
130+
131+
private function mergeConstraints(ClassMetadata $metadata)
132+
{
114133
// Include constraints from the parent class
115134
if ($parent = $metadata->getReflectionClass()->getParentClass()) {
116135
$metadata->mergeConstraints($this->getMetadataFor($parent->name));
@@ -141,16 +160,6 @@ public function getMetadataFor($value)
141160
}
142161
$metadata->mergeConstraints($this->getMetadataFor($interface->name));
143162
}
144-
145-
if (null !== $this->loader) {
146-
$this->loader->loadClassMetadata($metadata);
147-
}
148-
149-
if (null !== $this->cache) {
150-
$this->cache->write($metadata);
151-
}
152-
153-
return $this->loadedClasses[$class] = $metadata;
154163
}
155164

156165
/**

‎src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Validator\Tests\Mapping\Factory;
1313

14+
use Symfony\Component\Validator\Constraints\Callback;
1415
use Symfony\Component\Validator\Mapping\ClassMetadata;
1516
use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory;
1617
use Symfony\Component\Validator\Mapping\Loader\LoaderInterface;
@@ -30,8 +31,8 @@ public function testLoadClassMetadataWithInterface()
3031
$metadata = $factory->getMetadataFor(self::PARENT_CLASS);
3132

3233
$constraints = array(
33-
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))),
3434
new ConstraintA(array('groups' => array('Default', 'EntityParent'))),
35+
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))),
3536
);
3637

3738
$this->assertEquals($constraints, $metadata->getConstraints());
@@ -45,8 +46,6 @@ public function testMergeParentConstraints()
4546
$constraints = array(
4647
new ConstraintA(array('groups' => array(
4748
'Default',
48-
'EntityInterfaceA',
49-
'EntityParent',
5049
'Entity',
5150
))),
5251
new ConstraintA(array('groups' => array(
@@ -56,8 +55,8 @@ public function testMergeParentConstraints()
5655
))),
5756
new ConstraintA(array('groups' => array(
5857
'Default',
59-
'EntityParentInterface',
60-
'EntityInterfaceB',
58+
'EntityInterfaceA',
59+
'EntityParent',
6160
'Entity',
6261
))),
6362
new ConstraintA(array('groups' => array(
@@ -67,6 +66,8 @@ public function testMergeParentConstraints()
6766
))),
6867
new ConstraintA(array('groups' => array(
6968
'Default',
69+
'EntityParentInterface',
70+
'EntityInterfaceB',
7071
'Entity',
7172
))),
7273
);
@@ -80,8 +81,8 @@ public function testWriteMetadataToCache()
8081
$factory = new LazyLoadingMetadataFactory(new TestLoader(), $cache);
8182

8283
$parentClassConstraints = array(
83-
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))),
8484
new ConstraintA(array('groups' => array('Default', 'EntityParent'))),
85+
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))),
8586
);
8687
$interfaceAConstraints = array(
8788
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA'))),
@@ -122,17 +123,51 @@ public function testReadMetadataFromCache()
122123
$metadata = new ClassMetadata(self::PARENT_CLASS);
123124
$metadata->addConstraint(new ConstraintA());
124125

126+
$parentClass = self::PARENT_CLASS;
127+
$interfaceClass = self::INTERFACE_A_CLASS;
128+
125129
$loader->expects($this->never())
126130
->method('loadClassMetadata');
127131

128132
$cache->expects($this->never())
129133
->method('has');
130-
$cache->expects($this->once())
134+
$cache->expects($this->exactly(2))
131135
->method('read')
132-
->will($this->returnValue($metadata));
136+
->withConsecutive(
137+
array(self::PARENT_CLASS),
138+
array(self::INTERFACE_A_CLASS)
139+
)
140+
->willReturnCallback(function ($name) use ($metadata, $parentClass, $interfaceClass) {
141+
if ($parentClass == $name) {
142+
return $metadata;
143+
}
144+
145+
return new ClassMetadata($interfaceClass);
146+
});
133147

134148
$this->assertEquals($metadata, $factory->getMetadataFor(self::PARENT_CLASS));
135149
}
150+
151+
public function testMetadataCacheWithRuntimeConstraint()
152+
{
153+
$cache = $this->getMock('Symfony\Component\Validator\Mapping\Cache\CacheInterface');
154+
$factory = new LazyLoadingMetadataFactory(new TestLoader(), $cache);
155+
156+
$cache
157+
->expects($this->any())
158+
->method('write')
159+
->will($this->returnCallback(function ($metadata) { serialize($metadata); }))
160+
;
161+
162+
$cache->expects($this->any())
163+
->method('read')
164+
->will($this->returnValue(false));
165+
166+
$metadata = $factory->getMetadataFor(self::PARENT_CLASS);
167+
$metadata->addConstraint(new Callback(function () {}));
168+
169+
$metadata = $factory->getMetadataFor(self::CLASS_NAME);
170+
}
136171
}
137172

138173
class TestLoader implements LoaderInterface

0 commit comments

Comments
 (0)