From b7686086601e57211794d77e8be6229612398db5 Mon Sep 17 00:00:00 2001 From: Jules Pietri Date: Thu, 9 Apr 2020 18:38:25 +0200 Subject: [PATCH] fixup mixing groups in class and fields --- .../Validator/Constraints/FormValidator.php | 37 ++++++++++++----- .../Constraints/FormValidatorTest.php | 5 +-- .../Type/FormTypeValidatorExtensionTest.php | 40 ++++++++++++++++--- 3 files changed, 63 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php index b300f1fcdaf3b..5708ab54aa705 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php +++ b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php @@ -53,27 +53,31 @@ public function validate($form, Constraint $formConstraint) } $data = $form->getData(); - // Validate the data against its own constraints - if ($form->isRoot() && (\is_object($data) || \is_array($data))) { - if (($groups && \is_array($groups)) || ($groups instanceof GroupSequence && $groups->groups)) { - $validator->atPath('data')->validate($data, null, $groups); - } - } + $validateDataGraph = $form->isRoot() + && (\is_object($data) || \is_array($data)) + && (($groups && \is_array($groups)) || ($groups instanceof GroupSequence && $groups->groups)) + ; // Validate the data against the constraints defined in the form /** @var Constraint[] $constraints */ $constraints = $config->getOption('constraints', []); if ($groups instanceof GroupSequence) { - // Validate the form AND nested fields in sequence - $violationsCount = \count($this->context->getViolations()); + // Validate the data, the form AND nested fields in sequence + $violationsCount = $this->context->getViolations()->count(); $fieldPropertyPath = \is_object($data) ? 'data.%s' : '%s'; - $hasChildren = \count($form) > 0; + $hasChildren = $form->count() > 0; $this->resolvedGroups = $hasChildren ? new \SplObjectStorage() : null; foreach ($groups->groups as $group) { - $validator->atPath('data')->validate($data, $constraints, $group); + if ($validateDataGraph) { + $validator->atPath('data')->validate($data, null, $group); + } + + if ($groupedConstraints = self::getConstraintsInGroups($constraints, $group)) { + $validator->atPath('data')->validate($data, $groupedConstraints, $group); + } foreach ($form->all() as $field) { if ($field->isSubmitted()) { @@ -86,7 +90,7 @@ public function validate($form, Constraint $formConstraint) } } - if ($violationsCount < \count($this->context->getViolations())) { + if ($violationsCount < $this->context->getViolations()->count()) { break; } } @@ -96,6 +100,10 @@ public function validate($form, Constraint $formConstraint) $this->resolvedGroups = null; } } else { + if ($validateDataGraph) { + $validator->atPath('data')->validate($data, null, $groups); + } + foreach ($constraints as $constraint) { // For the "Valid" constraint, validate the data in all groups if ($constraint instanceof Valid) { @@ -221,4 +229,11 @@ private static function resolveValidationGroups($groups, FormInterface $form) return (array) $groups; } + + private static function getConstraintsInGroups($constraints, $group) + { + return array_filter($constraints, static function (Constraint $constraint) use ($group) { + return \in_array($group, $constraint->groups, true); + }); + } } 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 a20b25a4a6671..5181e4122516e 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php @@ -401,9 +401,8 @@ public function testHandleGroupSequenceValidationGroups() $form = $this->getCompoundForm($object, $options); $form->submit([]); - $this->expectValidateAt(0, 'data', $object, new GroupSequence(['group1', 'group2'])); - $this->expectValidateAt(1, 'data', $object, 'group1'); - $this->expectValidateAt(2, 'data', $object, 'group2'); + $this->expectValidateAt(0, 'data', $object, 'group1'); + $this->expectValidateAt(1, 'data', $object, 'group2'); $this->validator->validate($form, new Form()); diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php index b2f0c820d8c6f..13b5a885abe85 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php @@ -12,15 +12,20 @@ namespace Symfony\Component\Form\Tests\Extension\Validator\Type; use Symfony\Component\Form\Extension\Validator\ValidatorExtension; +use Symfony\Component\Form\Form; use Symfony\Component\Form\Forms; use Symfony\Component\Form\Test\Traits\ValidatorExtensionTrait; use Symfony\Component\Form\Tests\Extension\Core\Type\FormTypeTest; use Symfony\Component\Form\Tests\Extension\Core\Type\TextTypeTest; +use Symfony\Component\Form\Tests\Fixtures\Author; use Symfony\Component\Validator\Constraints\GroupSequence; use Symfony\Component\Validator\Constraints\Length; use Symfony\Component\Validator\Constraints\NotBlank; +use Symfony\Component\Validator\Constraints\NotNull; use Symfony\Component\Validator\Constraints\Valid; use Symfony\Component\Validator\ConstraintViolationList; +use Symfony\Component\Validator\Mapping\ClassMetadata; +use Symfony\Component\Validator\Mapping\Factory\MetadataFactoryInterface; use Symfony\Component\Validator\Validation; class FormTypeValidatorExtensionTest extends BaseValidatorExtensionTest @@ -79,23 +84,48 @@ public function testGroupSequenceWithConstraintsOption() public function testManyFieldsGroupSequenceWithConstraintsOption() { + $formMetadata = new ClassMetadata(Form::class); + $authorMetadata = (new ClassMetadata(Author::class)) + ->addPropertyConstraint('firstName', new NotNull(['groups' => 'Second'])) + ; + $metadataFactory = $this->createMock(MetadataFactoryInterface::class); + $metadataFactory->expects($this->any()) + ->method('getMetadataFor') + ->willReturnCallback(static function ($classOrObject) use ($formMetadata, $authorMetadata) { + if (Author::class === $classOrObject || $classOrObject instanceof Author) { + return $authorMetadata; + } + + if (Form::class === $classOrObject || $classOrObject instanceof Form) { + return $formMetadata; + } + + return new ClassMetadata(\is_string($classOrObject) ? $classOrObject : \get_class($classOrObject)); + }) + ; + + $validator = Validation::createValidatorBuilder() + ->setMetadataFactory($metadataFactory) + ->getValidator() + ; $form = Forms::createFormFactoryBuilder() - ->addExtension(new ValidatorExtension(Validation::createValidator())) + ->addExtension(new ValidatorExtension($validator)) ->getFormFactory() - ->create(FormTypeTest::TESTED_TYPE, null, (['validation_groups' => new GroupSequence(['First', 'Second'])])) - ->add('field1', TextTypeTest::TESTED_TYPE, [ + ->create(FormTypeTest::TESTED_TYPE, new Author(), (['validation_groups' => new GroupSequence(['First', 'Second'])])) + ->add('firstName', TextTypeTest::TESTED_TYPE) + ->add('lastName', TextTypeTest::TESTED_TYPE, [ 'constraints' => [ new Length(['min' => 10, 'groups' => ['First']]), ], ]) - ->add('field2', TextTypeTest::TESTED_TYPE, [ + ->add('australian', TextTypeTest::TESTED_TYPE, [ 'constraints' => [ new NotBlank(['groups' => ['Second']]), ], ]) ; - $form->submit(['field1' => 'wrong_1', 'field2' => '']); + $form->submit(['firstName' => '', 'lastName' => 'wrong_1', 'australian' => '']); $errors = $form->getErrors(true);