bug #35591 [Validator] do not merge constraints within interfaces (greedyivan)

fabpot · fabpot · commit cd4a4bd3d103 · 2020-04-12T09:28:41.000+02:00
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
diff --git a/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php b/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php
@@ -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));
         }
     }
diff --git a/src/Symfony/Component/Validator/Tests/Fixtures/AbstractPropertyGetter.php b/src/Symfony/Component/Validator/Tests/Fixtures/AbstractPropertyGetter.php
@@ -0,0 +1,13 @@
+<?php
+
+namespace Symfony\Component\Validator\Tests\Fixtures;
+
+abstract class AbstractPropertyGetter implements PropertyGetterInterface
+{
+    private $property;
+
+    public function getProperty()
+    {
+        return $this->property;
+    }
+}
diff --git a/src/Symfony/Component/Validator/Tests/Fixtures/ChildGetterInterface.php b/src/Symfony/Component/Validator/Tests/Fixtures/ChildGetterInterface.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\Validator\Tests\Fixtures;
+
+interface ChildGetterInterface extends PropertyGetterInterface
+{
+}
diff --git a/src/Symfony/Component/Validator/Tests/Fixtures/PropertyGetter.php b/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
+{
+}
diff --git a/src/Symfony/Component/Validator/Tests/Fixtures/PropertyGetterInterface.php b/src/Symfony/Component/Validator/Tests/Fixtures/PropertyGetterInterface.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace Symfony\Component\Validator\Tests\Fixtures;
+
+interface PropertyGetterInterface
+{
+    public function getProperty();
+}
diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php
@@ -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
 {
@@ -70,7 +73,6 @@ public function testMergeParentConstraints()
             new ConstraintA(['groups' => [
                 'Default',
                 'EntityParentInterface',
-                'EntityInterfaceB',
                 'Entity',
             ]]),
         ];
@@ -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
@@ -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;
+    }
+}
