diff --git a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php index b2c52ae1f329e..7922bcc74c87f 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php +++ b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php @@ -41,7 +41,7 @@ public function validate($form, Constraint $formConstraint) $validator = $this->context->getValidator()->inContext($this->context); - if ($form->isSynchronized()) { + if ($form->isSubmitted() && $form->isSynchronized()) { // Validate the form data only if transformation succeeded $groups = self::getValidationGroups($form); $data = $form->getData(); @@ -90,7 +90,7 @@ public function validate($form, Constraint $formConstraint) } } } - } else { + } elseif (!$form->isSynchronized()) { $childrenSynchronized = true; /** @var FormInterface $child */ diff --git a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php index 86a77044d13d5..659c266ce6fa9 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php +++ b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php @@ -273,9 +273,6 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or */ private function acceptsErrors(FormInterface $form) { - // Ignore non-submitted forms. This happens, for example, in PATCH - // requests. - // https://github.com/symfony/symfony/pull/10567 - return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized()); + return $this->allowNonSynchronized || $form->isSynchronized(); } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php index 1dbac519fe754..7581f7698c12c 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php @@ -61,9 +61,8 @@ public function testValidate() { $object = new \stdClass(); $options = ['validation_groups' => ['group1', 'group2']]; - $form = $this->getBuilder('name', '\stdClass', $options) - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object, $options); + $form->submit([]); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); @@ -82,9 +81,8 @@ public function testValidateConstraints() 'validation_groups' => ['group1', 'group2'], 'constraints' => [$constraint1, $constraint2], ]; - $form = $this->getBuilder('name', '\stdClass', $options) - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object, $options); + $form->submit([]); // First default constraints $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); @@ -110,10 +108,9 @@ public function testValidateChildIfValidConstraint() 'validation_groups' => ['group1', 'group2'], 'constraints' => [new Valid()], ]; - $form = $this->getBuilder('name', '\stdClass', $options)->getForm(); + $form = $this->getCompoundForm($object, $options); $parent->add($form); - - $form->setData($object); + $parent->submit([]); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); @@ -146,8 +143,8 @@ public function testDontValidateIfParentWithoutValidConstraint() public function testMissingConstraintIndex() { $object = new \stdClass(); - $form = new FormBuilder('name', '\stdClass', $this->dispatcher, $this->factory); - $form = $form->setData($object)->getForm(); + $form = $this->getCompoundForm($object); + $form->submit([]); $this->expectValidateAt(0, 'data', $object, ['Default']); @@ -170,10 +167,9 @@ public function testValidateConstraintsOptionEvenIfNoValidConstraint() 'validation_groups' => ['group1', 'group2'], 'constraints' => [$constraint1, $constraint2], ]; - $form = $this->getBuilder('name', '\stdClass', $options) - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object, $options); $parent->add($form); + $parent->submit([]); $this->expectValidateValueAt(0, 'data', $object, $constraint1, 'group1'); $this->expectValidateValueAt(1, 'data', $object, $constraint2, 'group2'); @@ -361,9 +357,8 @@ public function testHandleGroupSequenceValidationGroups() { $object = new \stdClass(); $options = ['validation_groups' => new GroupSequence(['group1', 'group2'])]; - $form = $this->getBuilder('name', '\stdClass', $options) - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object, $options); + $form->submit([]); $this->expectValidateAt(0, 'data', $object, new GroupSequence(['group1', 'group2'])); $this->expectValidateAt(1, 'data', $object, new GroupSequence(['group1', 'group2'])); @@ -377,9 +372,8 @@ public function testHandleCallbackValidationGroups() { $object = new \stdClass(); $options = ['validation_groups' => [$this, 'getValidationGroups']]; - $form = $this->getBuilder('name', '\stdClass', $options) - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object, $options); + $form->submit([]); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); @@ -392,9 +386,8 @@ public function testDontExecuteFunctionNames() { $object = new \stdClass(); $options = ['validation_groups' => 'header']; - $form = $this->getBuilder('name', '\stdClass', $options) - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object, $options); + $form->submit([]); $this->expectValidateAt(0, 'data', $object, ['header']); @@ -409,9 +402,8 @@ public function testHandleClosureValidationGroups() $options = ['validation_groups' => function (FormInterface $form) { return ['group1', 'group2']; }]; - $form = $this->getBuilder('name', '\stdClass', $options) - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object, $options); + $form->submit([]); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); @@ -455,7 +447,7 @@ public function testDontUseValidationGroupOfUnclickedButton() ->setCompound(true) ->setDataMapper(new PropertyPathMapper()) ->getForm(); - $form = $this->getForm('name', '\stdClass', [ + $form = $this->getCompoundForm($object, [ 'validation_groups' => 'form_group', 'constraints' => [new Valid()], ]); @@ -465,7 +457,7 @@ public function testDontUseValidationGroupOfUnclickedButton() 'validation_groups' => 'button_group', ])); - $form->setData($object); + $parent->submit([]); $this->expectValidateAt(0, 'data', $object, ['form_group']); @@ -484,10 +476,9 @@ public function testUseInheritedValidationGroup() ->setDataMapper(new PropertyPathMapper()) ->getForm(); $formOptions = ['constraints' => [new Valid()]]; - $form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm(); + $form = $this->getCompoundForm($object, $formOptions); $parent->add($form); - - $form->setData($object); + $parent->submit([]); $this->expectValidateAt(0, 'data', $object, ['group']); @@ -506,10 +497,9 @@ public function testUseInheritedCallbackValidationGroup() ->setDataMapper(new PropertyPathMapper()) ->getForm(); $formOptions = ['constraints' => [new Valid()]]; - $form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm(); + $form = $this->getCompoundForm($object, $formOptions); $parent->add($form); - - $form->setData($object); + $parent->submit([]); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); @@ -523,7 +513,7 @@ public function testUseInheritedClosureValidationGroup() $object = new \stdClass(); $parentOptions = [ - 'validation_groups' => function (FormInterface $form) { + 'validation_groups' => function () { return ['group1', 'group2']; }, ]; @@ -532,10 +522,9 @@ public function testUseInheritedClosureValidationGroup() ->setDataMapper(new PropertyPathMapper()) ->getForm(); $formOptions = ['constraints' => [new Valid()]]; - $form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm(); + $form = $this->getCompoundForm($object, $formOptions); $parent->add($form); - - $form->setData($object); + $parent->submit([]); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); @@ -547,9 +536,8 @@ public function testUseInheritedClosureValidationGroup() public function testAppendPropertyPath() { $object = new \stdClass(); - $form = $this->getBuilder('name', '\stdClass') - ->setData($object) - ->getForm(); + $form = $this->getCompoundForm($object); + $form->submit([]); $this->expectValidateAt(0, 'data', $object, ['Default']); @@ -690,6 +678,15 @@ private function getForm($name = 'name', $dataClass = null, array $options = []) return $this->getBuilder($name, $dataClass, $options)->getForm(); } + private function getCompoundForm($data, array $options = []) + { + return $this->getBuilder('name', \get_class($data), $options) + ->setData($data) + ->setCompound(true) + ->setDataMapper(new PropertyPathMapper()) + ->getForm(); + } + private function getSubmitButton($name = 'name', array $options = []) { $builder = new SubmitButtonBuilder($name, $options); diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php index b526650ea054e..04036721bca5a 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php @@ -12,10 +12,15 @@ namespace Symfony\Component\Form\Tests\Extension\Validator; use PHPUnit\Framework\TestCase; +use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint; use Symfony\Component\Form\Extension\Validator\ValidatorExtension; use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser; use Symfony\Component\Form\Form; +use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\Form\FormFactoryBuilder; +use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\Component\Validator\Constraints\NotBlank; use Symfony\Component\Validator\Mapping\CascadingStrategy; use Symfony\Component\Validator\Mapping\ClassMetadata; use Symfony\Component\Validator\Mapping\TraversalStrategy; @@ -45,4 +50,64 @@ public function test2Dot5ValidationApi() $this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy); $this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy); } + + public function testDataConstraintsInvalidateFormEvenIfFieldIsNotSubmitted() + { + $form = $this->createForm(FooType::class); + $form->submit(['baz' => 'foobar'], false); + + $this->assertTrue($form->isSubmitted()); + $this->assertFalse($form->isValid()); + $this->assertFalse($form->get('bar')->isSubmitted()); + $this->assertCount(1, $form->get('bar')->getErrors()); + } + + public function testFieldConstraintsDoNotInvalidFormIfFieldIsNotSubmitted() + { + $form = $this->createForm(FooType::class); + $form->submit(['bar' => 'foobar'], false); + + $this->assertTrue($form->isSubmitted()); + $this->assertTrue($form->isValid()); + } + + private function createForm($type) + { + $validator = Validation::createValidatorBuilder() + ->enableAnnotationMapping() + ->getValidator(); + $formFactoryBuilder = new FormFactoryBuilder(); + $formFactoryBuilder->addExtension(new ValidatorExtension($validator)); + $formFactory = $formFactoryBuilder->getFormFactory(); + + return $formFactory->create($type); + } +} + +class Foo +{ + /** + * @NotBlank() + */ + public $bar; + + public $baz; +} + +class FooType extends AbstractType +{ + public function buildForm(FormBuilderInterface $builder, array $options) + { + $builder + ->add('bar') + ->add('baz', null, [ + 'constraints' => [new NotBlank()], + ]) + ; + } + + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefault('data_class', Foo::class); + } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php index 5d0afa238cede..2fa3e928926ee 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php @@ -205,7 +205,7 @@ public function testAbortDotRuleMappingIfNotSynchronized() $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); } - public function testAbortMappingIfNotSubmitted() + public function testMappingIfNotSubmitted() { $violation = $this->getConstraintViolation('children[address].data.street'); $parent = $this->getForm('parent'); @@ -225,10 +225,10 @@ public function testAbortMappingIfNotSubmitted() $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one'); - $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); + $this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error'); } - public function testAbortDotRuleMappingIfNotSubmitted() + public function testDotRuleMappingIfNotSubmitted() { $violation = $this->getConstraintViolation('data.address'); $parent = $this->getForm('parent'); @@ -250,7 +250,7 @@ public function testAbortDotRuleMappingIfNotSubmitted() $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one'); - $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); + $this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error'); } public function provideDefaultTests()