Merge branch '4.4' into 5.0

* 4.4: [travis] fix CI (ter) Revert "[travis][appveyor] don't cache .phpunit" silence E_NOTICE triggered since PHP 7.4 [Form] Removed legacy check in `ValidationListener` [HttpClient] fix HTTP/2 support on non-SSL connections - CurlHttpClient only Force ping after transport Exception do not merge constraints within interfaces [Validator] Fixed default group for nested composite constraints
symfony · Apr 12, 2020 · d01fd93 · d01fd93
2 parents b6562e5 + fc33ffe
commit d01fd93
Show file tree

Hide file tree

Showing 10 changed files with 136 additions and 22 deletions.
diff --git a/Constraints/Composite.php b/Constraints/Composite.php
@@ -88,7 +88,8 @@ public function __construct($options = null)
                 }
             }
 
-            $this->groups = array_keys($mergedGroups);
+            // prevent empty composite constraint to have empty groups
+            $this->groups = array_keys($mergedGroups) ?: [self::DEFAULT_GROUP];
             $this->$compositeOption = $nestedConstraints;
 
             return;

diff --git a/Mapping/Factory/LazyLoadingMetadataFactory.php b/Mapping/Factory/LazyLoadingMetadataFactory.php
@@ -113,34 +113,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 (\ReflectionClass $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/Tests/Constraints/CollectionTest.php b/Tests/Constraints/CollectionTest.php
@@ -100,4 +100,16 @@ public function testAcceptRequiredConstraintAsOneElementArray()
 
         $this->assertEquals($collection1, $collection2);
     }
+
+    public function testConstraintHasDefaultGroupWithOptionalValues()
+    {
+        $constraint = new Collection([
+            'foo' => new Required(),
+            'bar' => new Optional(),
+        ]);
+
+        $this->assertEquals(['Default'], $constraint->groups);
+        $this->assertEquals(['Default'], $constraint->fields['foo']->groups);
+        $this->assertEquals(['Default'], $constraint->fields['bar']->groups);
+    }
 }
diff --git a/Tests/Constraints/CollectionValidatorTest.php b/Tests/Constraints/CollectionValidatorTest.php
@@ -143,6 +143,29 @@ public function testExtraFieldsDisallowed()
             ->assertRaised();
     }
 
+    public function testExtraFieldsDisallowedWithOptionalValues()
+    {
+        $constraint = new Optional();
+
+        $data = $this->prepareTestData([
+            'baz' => 6,
+        ]);
+
+        $this->validator->validate($data, new Collection([
+            'fields' => [
+                'foo' => $constraint,
+            ],
+            'extraFieldsMessage' => 'myMessage',
+        ]));
+
+        $this->buildViolation('myMessage')
+            ->setParameter('{{ field }}', '"baz"')
+            ->atPath('property.path[baz]')
+            ->setInvalidValue(6)
+            ->setCode(Collection::NO_SUCH_FIELD_ERROR)
+            ->assertRaised();
+    }
+
     // bug fix
     public function testNullNotConsideredExtraField()
     {

diff --git a/Tests/Constraints/CompositeTest.php b/Tests/Constraints/CompositeTest.php
@@ -19,7 +19,7 @@
 
 class ConcreteComposite extends Composite
 {
-    public $constraints;
+    public $constraints = [];
 
     protected function getCompositeOption(): string
     {
@@ -37,6 +37,30 @@ public function getDefaultOption(): ?string
  */
 class CompositeTest extends TestCase
 {
+    public function testConstraintHasDefaultGroup()
+    {
+        $constraint = new ConcreteComposite([
+            new NotNull(),
+            new NotBlank(),
+        ]);
+
+        $this->assertEquals(['Default'], $constraint->groups);
+        $this->assertEquals(['Default'], $constraint->constraints[0]->groups);
+        $this->assertEquals(['Default'], $constraint->constraints[1]->groups);
+    }
+
+    public function testNestedCompositeConstraintHasDefaultGroup()
+    {
+        $constraint = new ConcreteComposite([
+            new ConcreteComposite(),
+            new ConcreteComposite(),
+        ]);
+
+        $this->assertEquals(['Default'], $constraint->groups);
+        $this->assertEquals(['Default'], $constraint->constraints[0]->groups);
+        $this->assertEquals(['Default'], $constraint->constraints[1]->groups);
+    }
+
     public function testMergeNestedGroupsIfNoExplicitParentGroup()
     {
         $constraint = new ConcreteComposite([

diff --git a/Tests/Fixtures/AbstractPropertyGetter.php b/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/Tests/Fixtures/ChildGetterInterface.php b/Tests/Fixtures/ChildGetterInterface.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\Validator\Tests\Fixtures;
+
+interface ChildGetterInterface extends PropertyGetterInterface
+{
+}
diff --git a/Tests/Fixtures/PropertyGetter.php b/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/Tests/Fixtures/PropertyGetterInterface.php b/Tests/Fixtures/PropertyGetterInterface.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace Symfony\Component\Validator\Tests\Fixtures;
+
+interface PropertyGetterInterface
+{
+    public function getProperty();
+}
diff --git a/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php b/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php
@@ -15,10 +15,13 @@
 use Psr\Cache\CacheItemPoolInterface;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 use Symfony\Component\Validator\Constraints\Callback;
+use Symfony\Component\Validator\Constraints\NotBlank;
 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',
             ]]),
         ];
@@ -150,6 +152,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
@@ -161,3 +172,15 @@ public function loadClassMetadata(ClassMetadata $metadata): bool
         return true;
     }
 }
+
+class PropertyGetterInterfaceConstraintLoader implements LoaderInterface
+{
+    public function loadClassMetadata(ClassMetadata $metadata)
+    {
+        if (PropertyGetterInterface::class === $metadata->getClassName()) {
+            $metadata->addGetterConstraint('property', new NotBlank());
+        }
+
+        return true;
+    }
+}