Skip to content

Fix #19943 Make sure to process each interface metadata only once #20078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 30, 2016
Merged

Fix #19943 Make sure to process each interface metadata only once #20078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,27 @@ public function getMetadataFor($value)
$metadata->mergeConstraints($this->getMetadataFor($parent->name));
}

// Include constraints from all implemented interfaces that have not been processed via parent class yet
foreach ($metadata->getReflectionClass()->getInterfaces() as $interface) {
if ('Symfony\Component\Validator\GroupSequenceProviderInterface' === $interface->name || ($parent && $parent->implementsInterface($interface->name))) {
$interfaces = $metadata->getReflectionClass()->getInterfaces();

$interfaces = array_filter($interfaces, function ($interface) use ($parent, $interfaces) {
$interfaceName = $interface->getName();

if ($parent && $parent->implementsInterface($interfaceName)) {
return false;
}

foreach ($interfaces as $i) {
if ($i !== $interface && $i->implementsInterface($interfaceName)) {
return false;
}
}

return true;
});

// Include constraints from all directly implemented interfaces
foreach ($interfaces as $interface) {
if ('Symfony\Component\Validator\GroupSequenceProviderInterface' === $interface->name) {
continue;
}
$metadata->mergeConstraints($this->getMetadataFor($interface->name));
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/Tests/Fixtures/Entity.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* @Assert\GroupSequence({"Foo", "Entity"})
* @Assert\Callback({"Symfony\Component\Validator\Tests\Fixtures\CallbackClass", "callback"})
*/
class Entity extends EntityParent
class Entity extends EntityParent implements EntityInterfaceB
{
/**
* @Assert\NotNull
Expand Down
2 changes: 1 addition & 1 deletion ...idator/Tests/Fixtures/EntityInterface.php → ...dator/Tests/Fixtures/EntityInterfaceA.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@

namespace Symfony\Component\Validator\Tests\Fixtures;

interface EntityInterface
interface EntityInterfaceA
{
}
16 changes: 16 additions & 0 deletions src/Symfony/Component/Validator/Tests/Fixtures/EntityInterfaceB.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Validator\Tests\Fixtures;

interface EntityInterfaceB extends EntityParentInterface
{
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/Tests/Fixtures/EntityParent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

use Symfony\Component\Validator\Constraints\NotNull;

class EntityParent implements EntityInterface
class EntityParent implements EntityInterfaceA
{
protected $firstName;
private $internal;
Expand Down
16 changes: 16 additions & 0 deletions src/Symfony/Component/Validator/Tests/Fixtures/EntityParentInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Validator\Tests\Fixtures;

interface EntityParentInterface
{
}
49 changes: 32 additions & 17 deletions src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,19 @@

class LazyLoadingMetadataFactoryTest extends \PHPUnit_Framework_TestCase
{
const CLASSNAME = 'Symfony\Component\Validator\Tests\Fixtures\Entity';
const PARENTCLASS = 'Symfony\Component\Validator\Tests\Fixtures\EntityParent';
const INTERFACECLASS = 'Symfony\Component\Validator\Tests\Fixtures\EntityInterface';
const CLASS_NAME = 'Symfony\Component\Validator\Tests\Fixtures\Entity';
const PARENT_CLASS = 'Symfony\Component\Validator\Tests\Fixtures\EntityParent';
const INTERFACE_A_CLASS = 'Symfony\Component\Validator\Tests\Fixtures\EntityInterfaceA';
const INTERFACE_B_CLASS = 'Symfony\Component\Validator\Tests\Fixtures\EntityInterfaceB';
const PARENT_INTERFACE_CLASS = 'Symfony\Component\Validator\Tests\Fixtures\EntityParentInterface';

public function testLoadClassMetadataWithInterface()
{
$factory = new LazyLoadingMetadataFactory(new TestLoader());
$metadata = $factory->getMetadataFor(self::PARENTCLASS);
$metadata = $factory->getMetadataFor(self::PARENT_CLASS);

$constraints = array(
new ConstraintA(array('groups' => array('Default', 'EntityInterface', 'EntityParent'))),
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))),
new ConstraintA(array('groups' => array('Default', 'EntityParent'))),
);

Expand All @@ -38,12 +40,12 @@ public function testLoadClassMetadataWithInterface()
public function testMergeParentConstraints()
{
$factory = new LazyLoadingMetadataFactory(new TestLoader());
$metadata = $factory->getMetadataFor(self::CLASSNAME);
$metadata = $factory->getMetadataFor(self::CLASS_NAME);

$constraints = array(
new ConstraintA(array('groups' => array(
'Default',
'EntityInterface',
'EntityInterfaceA',
'EntityParent',
'Entity',
))),
Expand All @@ -52,6 +54,17 @@ public function testMergeParentConstraints()
'EntityParent',
'Entity',
))),
new ConstraintA(array('groups' => array(
'Default',
'EntityParentInterface',
'EntityInterfaceB',
'Entity',
))),
new ConstraintA(array('groups' => array(
'Default',
'EntityInterfaceB',
'Entity',
))),
new ConstraintA(array('groups' => array(
'Default',
'Entity',
Expand All @@ -67,34 +80,36 @@ public function testWriteMetadataToCache()
$factory = new LazyLoadingMetadataFactory(new TestLoader(), $cache);

$parentClassConstraints = array(
new ConstraintA(array('groups' => array('Default', 'EntityInterface', 'EntityParent'))),
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA', 'EntityParent'))),
new ConstraintA(array('groups' => array('Default', 'EntityParent'))),
);
$interfaceConstraints = array(new ConstraintA(array('groups' => array('Default', 'EntityInterface'))));
$interfaceAConstraints = array(
new ConstraintA(array('groups' => array('Default', 'EntityInterfaceA'))),
);

$cache->expects($this->never())
->method('has');
$cache->expects($this->exactly(2))
->method('read')
->withConsecutive(
array($this->equalTo(self::PARENTCLASS)),
array($this->equalTo(self::INTERFACECLASS))
array($this->equalTo(self::PARENT_CLASS)),
array($this->equalTo(self::INTERFACE_A_CLASS))
)
->will($this->returnValue(false));
$cache->expects($this->exactly(2))
->method('write')
->withConsecutive(
$this->callback(function ($metadata) use ($interfaceConstraints) {
return $interfaceConstraints == $metadata->getConstraints();
$this->callback(function ($metadata) use ($interfaceAConstraints) {
return $interfaceAConstraints == $metadata->getConstraints();
}),
$this->callback(function ($metadata) use ($parentClassConstraints) {
return $parentClassConstraints == $metadata->getConstraints();
})
);

$metadata = $factory->getMetadataFor(self::PARENTCLASS);
$metadata = $factory->getMetadataFor(self::PARENT_CLASS);

$this->assertEquals(self::PARENTCLASS, $metadata->getClassName());
$this->assertEquals(self::PARENT_CLASS, $metadata->getClassName());
$this->assertEquals($parentClassConstraints, $metadata->getConstraints());
}

Expand All @@ -104,7 +119,7 @@ public function testReadMetadataFromCache()
$cache = $this->getMock('Symfony\Component\Validator\Mapping\Cache\CacheInterface');
$factory = new LazyLoadingMetadataFactory($loader, $cache);

$metadata = new ClassMetadata(self::PARENTCLASS);
$metadata = new ClassMetadata(self::PARENT_CLASS);
$metadata->addConstraint(new ConstraintA());

$loader->expects($this->never())
Expand All @@ -116,7 +131,7 @@ public function testReadMetadataFromCache()
->method('read')
->will($this->returnValue($metadata));

$this->assertEquals($metadata, $factory->getMetadataFor(self::PARENTCLASS));
$this->assertEquals($metadata, $factory->getMetadataFor(self::PARENT_CLASS));
}
}

Expand Down