From b819d94d1413e426237f35e6b6eddbc380dbccf9 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 19 May 2020 08:35:15 +0200 Subject: [PATCH] validate subforms in all validation groups --- .../Validator/Constraints/FormValidator.php | 31 +++++++++++++----- .../Validator/ValidatorExtension.php | 4 +-- .../Form/Resources/config/validation.xml | 6 ++-- .../Constraints/FormValidatorTest.php | 6 ++-- .../Validator/ValidatorExtensionTest.php | 32 +++++++++++++++++-- 5 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php index 14158f4c1cf7..6a8923ecbf0a 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php +++ b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php @@ -63,12 +63,16 @@ public function validate($form, Constraint $formConstraint) /** @var Constraint[] $constraints */ $constraints = $config->getOption('constraints', []); + $hasChildren = $form->count() > 0; + + if ($hasChildren && $form->isRoot()) { + $this->resolvedGroups = new \SplObjectStorage(); + } + if ($groups instanceof GroupSequence) { // Validate the data, the form AND nested fields in sequence $violationsCount = $this->context->getViolations()->count(); $fieldPropertyPath = \is_object($data) ? 'children[%s]' : 'children%s'; - $hasChildren = $form->count() > 0; - $this->resolvedGroups = $hasChildren ? new \SplObjectStorage() : null; foreach ($groups->groups as $group) { if ($validateDataGraph) { @@ -86,7 +90,8 @@ public function validate($form, Constraint $formConstraint) // sequence recursively, thus some fields could fail // in different steps without breaking early enough $this->resolvedGroups[$field] = (array) $group; - $validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $formConstraint); + $fieldFormConstraint = new Form(); + $validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $fieldFormConstraint); } } @@ -94,12 +99,9 @@ public function validate($form, Constraint $formConstraint) break; } } - - if ($hasChildren) { - // destroy storage at the end of the sequence to avoid memory leaks - $this->resolvedGroups = null; - } } else { + $fieldPropertyPath = \is_object($data) ? 'children[%s]' : 'children%s'; + if ($validateDataGraph) { $validator->atPath('data')->validate($data, null, $groups); } @@ -125,6 +127,19 @@ public function validate($form, Constraint $formConstraint) } } } + + foreach ($form->all() as $field) { + if ($field->isSubmitted()) { + $this->resolvedGroups[$field] = $groups; + $fieldFormConstraint = new Form(); + $validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $fieldFormConstraint); + } + } + } + + if ($hasChildren && $form->isRoot()) { + // destroy storage to avoid memory leaks + $this->resolvedGroups = new \SplObjectStorage(); } } elseif (!$form->isSynchronized()) { $childrenSynchronized = true; diff --git a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php index a5e38859c088..ac2d61238feb 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php @@ -13,7 +13,7 @@ use Symfony\Component\Form\AbstractExtension; use Symfony\Component\Form\Extension\Validator\Constraints\Form; -use Symfony\Component\Validator\Constraints\Valid; +use Symfony\Component\Validator\Constraints\Traverse; use Symfony\Component\Validator\Mapping\ClassMetadata; use Symfony\Component\Validator\Validator\ValidatorInterface; @@ -37,7 +37,7 @@ public function __construct(ValidatorInterface $validator) /* @var $metadata ClassMetadata */ $metadata->addConstraint(new Form()); - $metadata->addPropertyConstraint('children', new Valid()); + $metadata->addConstraint(new Traverse(false)); $this->validator = $validator; } diff --git a/src/Symfony/Component/Form/Resources/config/validation.xml b/src/Symfony/Component/Form/Resources/config/validation.xml index b2b935442d46..918f101f4266 100644 --- a/src/Symfony/Component/Form/Resources/config/validation.xml +++ b/src/Symfony/Component/Form/Resources/config/validation.xml @@ -6,8 +6,8 @@ - - - + + + 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 5181e4122516..3d7111f85f3c 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php @@ -615,7 +615,8 @@ public function testViolationIfExtraData() $this->assertTrue($form->isSubmitted()); $this->assertTrue($form->isSynchronized()); - $this->expectNoValidate(); + + $this->expectValidateValueAt(0, 'children[child]', $form->get('child'), new Form()); $this->validator->validate($form, new Form()); @@ -638,7 +639,8 @@ public function testViolationFormatIfMultipleExtraFields() $this->assertTrue($form->isSubmitted()); $this->assertTrue($form->isSynchronized()); - $this->expectNoValidate(); + + $this->expectValidateValueAt(0, 'children[child]', $form->get('child'), new Form()); $this->validator->validate($form, new Form()); diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php index cb9b93abdbf6..9793bd78e69e 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php @@ -54,9 +54,8 @@ public function test2Dot5ValidationApi() $this->assertInstanceOf(FormConstraint::class, $metadata->getConstraints()[0]); $this->assertSame(CascadingStrategy::NONE, $metadata->cascadingStrategy); - $this->assertSame(TraversalStrategy::IMPLICIT, $metadata->traversalStrategy); - $this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy); - $this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy); + $this->assertSame(TraversalStrategy::NONE, $metadata->traversalStrategy); + $this->assertCount(0, $metadata->getPropertyMetadata('children')); } public function testDataConstraintsInvalidateFormEvenIfFieldIsNotSubmitted() @@ -138,6 +137,33 @@ public function testFieldsValidateInSequenceWithNestedGroupsArray() $this->assertInstanceOf(Length::class, $errors[1]->getCause()->getConstraint()); } + public function testConstraintsInDifferentGroupsOnSingleField() + { + $form = $this->createForm(FormType::class, null, [ + 'validation_groups' => new GroupSequence(['group1', 'group2']), + ]) + ->add('foo', TextType::class, [ + 'constraints' => [ + new NotBlank([ + 'groups' => ['group1'], + ]), + new Length([ + 'groups' => ['group2'], + 'max' => 3, + ]), + ], + ]); + $form->submit([ + 'foo' => 'test@example.com', + ]); + + $errors = $form->getErrors(true); + + $this->assertFalse($form->isValid()); + $this->assertCount(1, $errors); + $this->assertInstanceOf(Length::class, $errors[0]->getCause()->getConstraint()); + } + private function createForm($type, $data = null, array $options = []) { $validator = Validation::createValidatorBuilder()