Skip to content

Commit

Permalink
bug #35591 [Validator] do not merge constraints within interfaces (gr…
Browse files Browse the repository at this point in the history
…eedyivan)

This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] do not merge constraints within interfaces

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #22538
| License       | MIT
| Doc PR        |

This fix disables merge constraints within interfaces.

There is no reason to merge constraints from one interface to another because each class merges the constraints of all its interfaces. Only one check is needed is to eliminate all interfaces that comes from parent class to avoid duplication.

Commits
-------

67f336b do not merge constraints within interfaces
  • Loading branch information
fabpot committed Apr 12, 2020
2 parents db733da + 67f336b commit cd4a4bd
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 20 deletions.
Expand Up @@ -117,34 +117,25 @@ public function getMetadataFor($value)

private function mergeConstraints(ClassMetadata $metadata)
{
if ($metadata->getReflectionClass()->isInterface()) {
return;
}

// Include constraints from the parent class
if ($parent = $metadata->getReflectionClass()->getParentClass()) {
$metadata->mergeConstraints($this->getMetadataFor($parent->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) {
foreach ($metadata->getReflectionClass()->getInterfaces() as $interface) {
if ('Symfony\Component\Validator\GroupSequenceProviderInterface' === $interface->name) {
continue;
}

if ($parent && \in_array($interface->getName(), $parent->getInterfaceNames(), true)) {
continue;
}

$metadata->mergeConstraints($this->getMetadataFor($interface->name));
}
}
Expand Down
@@ -0,0 +1,13 @@
<?php

namespace Symfony\Component\Validator\Tests\Fixtures;

abstract class AbstractPropertyGetter implements PropertyGetterInterface
{
private $property;

public function getProperty()
{
return $this->property;
}
}
@@ -0,0 +1,7 @@
<?php

namespace Symfony\Component\Validator\Tests\Fixtures;

interface ChildGetterInterface extends PropertyGetterInterface
{
}
12 changes: 12 additions & 0 deletions src/Symfony/Component/Validator/Tests/Fixtures/PropertyGetter.php
@@ -0,0 +1,12 @@
<?php

namespace Symfony\Component\Validator\Tests\Fixtures;

/**
* This class has two paths to PropertyGetterInterface:
* PropertyGetterInterface <- AbstractPropertyGetter <- PropertyGetter
* PropertyGetterInterface <- ChildGetterInterface <- PropertyGetter
*/
class PropertyGetter extends AbstractPropertyGetter implements ChildGetterInterface
{
}
@@ -0,0 +1,8 @@
<?php

namespace Symfony\Component\Validator\Tests\Fixtures;

interface PropertyGetterInterface
{
public function getProperty();
}
Expand Up @@ -14,11 +14,14 @@
use PHPUnit\Framework\TestCase;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
use Symfony\Component\Validator\Constraints\Callback;
use Symfony\Component\Validator\Constraints\NotBlank;
use Symfony\Component\Validator\Mapping\Cache\Psr6Cache;
use Symfony\Component\Validator\Mapping\ClassMetadata;
use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory;
use Symfony\Component\Validator\Mapping\Loader\LoaderInterface;
use Symfony\Component\Validator\Tests\Fixtures\ConstraintA;
use Symfony\Component\Validator\Tests\Fixtures\PropertyGetter;
use Symfony\Component\Validator\Tests\Fixtures\PropertyGetterInterface;

class LazyLoadingMetadataFactoryTest extends TestCase
{
Expand Down Expand Up @@ -70,7 +73,6 @@ public function testMergeParentConstraints()
new ConstraintA(['groups' => [
'Default',
'EntityParentInterface',
'EntityInterfaceB',
'Entity',
]]),
];
Expand Down Expand Up @@ -186,6 +188,15 @@ public function testGroupsFromParent()
$this->assertContains('EntityStaticCar', $groups);
$this->assertContains('EntityStaticVehicle', $groups);
}

public function testMultipathInterfaceConstraint()
{
$factory = new LazyLoadingMetadataFactory(new PropertyGetterInterfaceConstraintLoader());
$metadata = $factory->getMetadataFor(PropertyGetter::class);
$constraints = $metadata->getPropertyMetadata('property');

$this->assertCount(1, $constraints);
}
}

class TestLoader implements LoaderInterface
Expand All @@ -195,3 +206,15 @@ public function loadClassMetadata(ClassMetadata $metadata)
$metadata->addConstraint(new ConstraintA());
}
}

class PropertyGetterInterfaceConstraintLoader implements LoaderInterface
{
public function loadClassMetadata(ClassMetadata $metadata)
{
if (PropertyGetterInterface::class === $metadata->getClassName()) {
$metadata->addGetterConstraint('property', new NotBlank());
}

return true;
}
}

0 comments on commit cd4a4bd

Please sign in to comment.