8000 [Form] Fixed handling groups sequence validation · symfony/symfony@dfb61c2 · GitHub
[go: up one dir, main page]
Skip to content

Commit dfb61c2

Browse files
HeahDudeHeahDude
committed
[Form] Fixed handling groups sequence validation
1 parent 9b41a32 commit dfb61c2

File tree

5 files changed

+212
-29
lines changed

5 files changed

+212
-29
lines changed

‎src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
*/
2525
class FormValidator extends ConstraintValidator
2626
{
27+
private $resolvedGroups;
28+
2729
/**
2830
* {@inheritdoc}
2931
*/
@@ -44,42 +46,68 @@ public function validate($form, Constraint $formConstraint)
4446

4547
if ($form->isSubmitted() && $form->isSynchronized()) {
4648
// Validate the form data only if transformation succeeded
47-
$groups = self::getValidationGroups($form);
49+
$groups = $this->getValidationGroups($form);
4850

4951
if (!$groups) {
5052
return;
5153
}
5254

5355
$data = $form->getData();
54-
5556
// Validate the data against its own constraints
56-
if ($form->isRoot() && (\is_object($data) || \is_array($data))) {
57-
if (($groups && \is_array($groups)) || ($groups instanceof GroupSequence && $groups->groups)) {
58-
$validator->atPath('data')->validate($form->getData(), null, $groups);
59-
}
60-
}
57+
$validateDataGraph = $form->isRoot()
58+
&& (\is_object($data) || \is_array($data))
59+
&& (($groups && \is_array($groups)) || ($groups instanceof GroupSequence && $groups->groups))
60+
;
6161

62-
// Validate the data against the constraints defined
63-
// in the form
62+
// Validate the data against the constraints defined in the form
63+
/** @var Constraint[] $constraints */
6464
$constraints = $config->getOption('constraints', []);
6565

6666
if ($groups instanceof GroupSequence) {
67-
$validator->atPath('data')->validate($form->getData(), $constraints, $groups);
68-
// Otherwise validate a constraint only once for the first
69-
// matching group
70-
foreach ($groups as $group) {
71-
if (\in_array($group, $formConstraint->groups)) {
72-
$validator->atPath('data')->validate($form->getData(), $formConstraint, $group);
73-
if (\count($this->context->getViolations()) > 0) {
74-
break;
67+
// Validate the data, the form AND nested fields in sequence
68+
$violationsCount = $this->context->getViolations()->count();
69+
$fieldPropertyPath = \is_object($data) ? 'children[%s]' : 'children%s';
70+
$hasChildren = $form->count() > 0;
71+
$this->resolvedGroups = $hasChildren ? new \SplObjectStorage() : null;
72+
73+
foreach ($groups->groups as $group) {
74+
if ($validateDataGraph) {
75+
$validator->atPath('data')->validate($data, null, $group);
76+
}
77+
78+
if ($groupedConstraints = self::getConstraintsInGroups($constraints, $group)) {
79+
$validator->atPath('data')->validate($data, $groupedConstraints, $group);
80+
}
81+
82+
foreach ($form->all() as $field) {
83+
if ($field->isSubmitted()) {
84+
// remember to validate this field is one group only
85+
// otherwise resolving the groups would reuse the same
86+
// sequence recursively, thus some fields could fail
87+
// in different steps without breaking early enough
88+
$this->resolvedGroups[$field] = (array) $group;
89+
$validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $formConstraint);
7590
}
7691
}
92+
93+
if ($violationsCount < $this->context->getViolations()->count()) {
94+
break;
95+
}
96+
}
97+
98+
if ($hasChildren) {
99+
// destroy storage at the end of the sequence to avoid memory leaks
100+
$this->resolvedGroups = null;
77101
}
78102
} else {
103+
if ($validateDataGraph) {
104+
$validator->atPath('data')->validate($data, null, $groups);
105+
}
106+
79107
foreach ($constraints as $constraint) {
80108
// For the "Valid" constraint, validate the data in all groups
81109
if ($constraint instanceof Valid) {
82-
$validator->atPath('data')->validate($form->getData(), $constraint, $groups);
110+
$validator->atPath('data')->validate($data, $constraint, $groups);
83111

84112
continue;
85113
}
@@ -88,7 +116,7 @@ public function validate($form, Constraint $formConstraint)
88116
// matching group
89117
foreach ($groups as $group) {
90118
if (\in_array($group, $constraint->groups)) {
91-
$validator->atPath('data')->validate($form->getData(), $constraint, $group);
119+
$validator->atPath('data')->validate($data, $constraint, $group);
92120

93121
// Prevent duplicate validation
94122
if (!$constraint instanceof Composite) {
@@ -147,7 +175,7 @@ public function validate($form, Constraint $formConstraint)
147175
*
148176
* @return string|GroupSequence|(string|GroupSequence)[] The validation groups
149177
*/
150-
private static function getValidationGroups(FormInterface $form)
178+
private function getValidationGroups(FormInterface $form)
151179
{
152180
// Determine the clicked button of the complete form tree
153181
$clickedButton = null;
@@ -171,6 +199,10 @@ private static function getValidationGroups(FormInterface $form)
171199
return self::resolveValidationGroups($groups, $form);
172200
}
173201

202+
if (isset($this->resolvedGroups[$form])) {
203+
return $this->resolvedGroups[$form];
204+
}
205+
174206
$form = $form->getParent();
175207
} while (null !== $form);
176208

@@ -197,4 +229,11 @@ private static function resolveValidationGroups($groups, FormInterface $form)
197229

198230
return (array) $groups;
199231
}
232+
233+
private static function getConstraintsInGroups($constraints, $group)
234+
{
235+
return array_filter($constraints, static function (Constraint $constraint) use ($group) {
236+
return \in_array($group, $constraint->groups, true);
237+
});
238+
}
200239
}

‎src/Symfony/Component/Form/Resources/config/validation.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
<class name="Symfony\Component\Form\Form">
88
<constraint name="Symfony\Component\Form\Extension\Validator\Constraints\Form" />
99
<property name="children">
10-
<constraint name="Valid" />
10+
<constraint name="Valid" />
1111
</property>
1212
</class>
1313
</constraint-mapping>

‎src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -401,8 +401,8 @@ public function testHandleGroupSequenceValidationGroups()
401401
$form = $this->getCompoundForm($object, $options);
402402
$form->submit([]);
403403

404-
$this->expectValidateAt(0, 'data', $object, new GroupSequence(['group1', 'group2']));
405-
$this->expectValidateAt(1, 'data', $object, new GroupSequence(['group1', 'group2']));
404+
$this->expectValidateAt(0, 'data', $object, 'group1');
405+
$this->expectValidateAt(1, 'data', $object, 'group2');
406406

407407
$this->validator->validate($form, new Form());
408408

@@ -756,6 +756,39 @@ public function testCompositeConstraintValidatedInEachGroup()
756756
$this->assertSame('data[field2]', $context->getViolations()[1]->getPropertyPath());
757757
}
758758

759+
public function testCompositeConstraintValidatedInSequence()
760+
{
761+
$form = $this->getCompoundForm([], [
762+
'constraints' => [
763+
new Collection([
764+
'field1' => new NotBlank([
765+
'groups' => ['field1'],
766+
]),
767+
'field2' => new NotBlank([
768+
'groups' => ['field2'],
769+
]),
770+
]),
771+
],
772+
'validation_groups' => new GroupSequence(['field1', 'field2']),
773+
])
774+
->add($this->getForm('field1'))
775+
->add($this->getForm('field2'))
776+
;
777+
778+
$form->submit([
779+
'field1' => '',
780+
'field2' => '',
781+
]);
782+
783+
$context = new ExecutionContext(Validation::createValidator(), $form, new IdentityTranslator());
784+
$this->validator->initialize($context);
785+
$this->validator->validate($form, new Form());
786+
787+
$this->assertCount(1, $context->getViolations());
788+
$this->assertSame('This value should not be blank.', $context->getViolations()[0]->getMessage());
789+
$this->assertSame('data[field1]', $context->getViolations()[0]->getPropertyPath());
790+
}
791+
759792
protected function createValidator()
760793
{
761794
return new FormValidator();
@@ -784,7 +817,7 @@ private function getForm($name = 'name', $dataClass = null, array $options = [])
784817

785818
private function getCompoundForm($data, array $options = [])
786819
{
787-
return $this->getBuilder('name', \get_class($data), $options)
820+
return $this->getBuilder('name', \is_object($data) ? \get_class($data) : null, $options)
788821
->setData($data)
789822
->setCompound(true)
790823
->setDataMapper(new PropertyPathMapper())

‎src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,19 @@
1212
namespace Symfony\Component\Form\Tests\Extension\Validator\Type;
1313

1414
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
15+
use Symfony\Component\Form\Form;
1516
use Symfony\Component\Form\Forms;
1617
use Symfony\Component\Form\Test\Traits\ValidatorExtensionTrait;
1718
use Symfony\Component\Form\Tests\Extension\Core\Type\FormTypeTest;
1819
use Symfony\Component\Form\Tests\Extension\Core\Type\TextTypeTest;
19-
use Symfony\Component\Validator\Constraints\Email;
20+
use Symfony\Component\Form\Tests\Fixtures\Author;
2021
use Symfony\Component\Validator\Constraints\GroupSequence;
2122
use Symfony\Component\Validator\Constraints\Length;
23+
use Symfony\Component\Validator\Constraints\NotBlank;
2224
use Symfony\Component\Validator\Constraints\Valid;
2325
use Symfony\Component\Validator\ConstraintViolationList;
26+
use Symfony\Component\Validator\Mapping\ClassMetadata;
27+
use Symfony\Component\Validator\Mapping\Factory\MetadataFactoryInterface;
2428
use Symfony\Component\Validator\Validation;
2529

2630
class FormTypeValidatorExtensionTest extends BaseValidatorExtensionTest
@@ -64,14 +68,69 @@ public function testGroupSequenceWithConstraintsOption()
6468
->add('field', TextTypeTest::TESTED_TYPE, [
6569
'constraints' => [
6670
new Length(['min' => 10, 'groups' => ['First']]),
67-
new Email(['groups' => ['Second']]),
71+
new NotBlank(['groups' => ['Second']]),
6872
],
6973
])
7074
;
7175

7276
$form->submit(['field' => 'wrong']);
7377

74-
$this->assertCount(1, $form->getErrors(true));
78+
$errors = $form->getErrors(true);
79+
80+
$this->assertCount(1, $errors);
81+
$this->assertInstanceOf(Length::class, $errors[0]->getCause()->getConstraint());
82+
}
83+
84+
public function testManyFieldsGroupSequenceWithConstraintsOption()
85+
{
86+
$formMetadata = new ClassMetadata(Form::class);
87+
$authorMetadata = (new ClassMetadata(Author::class))
88+
->addPropertyConstraint('firstName', new NotBlank(['groups' => 'Second']))
89+
;
90+
$metadataFactory = $this->createMock(MetadataFactoryInterface::class);
91+
$metadataFactory->expects($this->any())
92+
->method('getMetadataFor')
93+
->willReturnCallback(static function ($classOrObject) use ($formMetadata, $authorMetadata) {
94+
if (Author::class === $classOrObject || $classOrObject instanceof Author) {
95+
return $authorMetadata;
96+
}
97+
98+
if (Form::class === $classOrObject || $classOrObject instanceof Form) {
99+
return $formMetadata;
100+
}
101+
102+
return new ClassMetadata(\is_string($classOrObject) ? $classOrObject : \get_class($classOrObject));
103+
})
104+
;
105+
106+
$validator = Validation::createValidatorBuilder()
107+
->setMetadataFactory($metadataFactory)
108+
->getValidator()
109+
;
110+
$form = Forms::createFormFactoryBuilder()
111+
->addExtension(new ValidatorExtension($validator))
112+
->getFormFactory()
113+
->create(FormTypeTest::TESTED_TYPE, new Author(), (['validation_groups' => new GroupSequence(['First', 'Second'])]))
114+
->add('firstName', TextTypeTest::TESTED_TYPE)
115+
->add('lastName', TextTypeTest::TESTED_TYPE, [
116+
'constraints' => [
117+
new Length(['min' => 10, 'groups' => ['First']]),
118+
],
119+
])
120+
->add('australian', TextTypeTest::TESTED_TYPE, [
121+
'constraints' => [
122+
new NotBlank(['groups' => ['Second']]),
123+
],
124+
])
125+
;
126+
127+
$form->submit(['firstName' => '', 'lastName' => 'wrong_1', 'australian' => '']);
128+
129+
$errors = $form->getErrors(true);
130+
131+
$this->assertCount(1, $errors);
132+
$this->assertInstanceOf(Length::class, $errors[0]->getCause()->getConstraint());
133+
$this->assertSame('children[lastName].data', $errors[0]->getCause()->getPropertyPath());
75134
}
76135

77136
protected function createForm(array $options = [])

‎src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,17 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Form\AbstractType;
16+
use Symfony\Component\Form\Extension\Core\Type\FormType;
17+
use Symfony\Component\Form\Extension\Core\Type\TextType;
1618
use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint;
1719
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
1820
use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser;
1921
use Symfony\Component\Form\Form;
2022
use Symfony\Component\Form\FormBuilderInterface;
2123
use Symfony\Component\Form\FormFactoryBuilder;
2224
use Symfony\Component\OptionsResolver\OptionsResolver;
25+
use Symfony\Component\Validator\Constraints\GroupSequence;
26+
use Symfony\Component\Validator\Constraints\Length;
2327
use Symfony\Component\Validator\Constraints\NotBlank;
2428
use Symfony\Component\Validator\Mapping\CascadingStrategy;
2529
use Symfony\Component\Validator\Mapping\ClassMetadata;
@@ -49,6 +53,8 @@ public function test2Dot5ValidationApi()
4953
$this->assertCount(1, $metadata->getConstraints());
5054
$this->assertInstanceOf(FormConstraint::class, $metadata->getConstraints()[0]);
5155

56+
$this->assertSame(CascadingStrategy::NONE, $metadata->cascadingStrategy);
57+
$this->assertSame(TraversalStrategy::IMPLICIT, $metadata->traversalStrategy);
5258
$this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy);
5359
$this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy);
5460
}
@@ -86,7 +92,53 @@ public function testFieldConstraintsInvalidateFormIfFieldIsSubmitted()
8692
$this->assertFalse($form->get('baz')->isValid());
8793
}
8894

89-
private function createForm($type)
95+
public function testFieldsValidateInSequence()
96+
{
97+
$form = $this->createForm(FormType::class, null, [
98+
'validation_groups' => new GroupSequence(['group1', 'group2']),
99+
])
100+
->add('foo', TextType::class, [
101+
'constraints' => [new Length(['min' => 10, 'groups' => ['group1']])],
102+
])
103+
->add('bar', TextType::class, [
104+
'constraints' => [new NotBlank(['groups' => ['group2']])],
105+
])
106+
;
107+
108+
$form->submit(['foo' => 'invalid', 'bar' => null]);
109+
110+
$errors = $form->getErrors(true);
111+
112+
$this->assertCount(1, $errors);
113+
$this->assertInstanceOf(Length::class, $errors[0]->getCause()->getConstraint());
114+
}
115+
116+
public function testFieldsValidateInSequenceWithNestedGroupsArray()
117+
{
118+
$form = $this->createForm(FormType::class, null, [
119+
'validation_groups' => new GroupSequence([['group1', 'group2'], 'group3']),
120+
])
121+
->add('foo', TextType::class, [
122+
'constraints' => [new Length(['min' => 10, 'groups' => ['group1']])],
123+
])
124+
->add('bar', TextType::class, [
125+
'constraints' => [new Length(['min' => 10, 'groups' => ['group2']])],
126+
])
127+
->add('baz', TextType::class, [
128+
'constraints' => [new NotBlank(['groups' => ['group3']])],
129+
])
130+
;
131+
132+
$form->submit(['foo' => 'invalid', 'bar' => 'invalid', 'baz' => null]);
133+
134+
$errors = $form->getErrors(true);
135+
136+
$this->assertCount(2, $errors);
137+
$this->assertInstanceOf(Length::class, $errors[0]->getCause()->getConstraint());
138+
$this->assertInstanceOf(Length::class, $errors[1]->getCause()->getConstraint());
139+
}
140+
141+
private function createForm($type, $data = null, array $options = [])
90142
{
91143
$validator = Validation::createValidatorBuilder()
92144
->setMetadataFactory(new LazyLoadingMetadataFactory(new StaticMethodLoader()))
@@ -95,7 +147,7 @@ private function createForm($type)
95147
$formFactoryBuilder->addExtension(new ValidatorExtension($validator));
96148
$formFactory = $formFactoryBuilder->getFormFactory();
97149

98-
return $formFactory->create($type);
150+
return $formFactory->create($type, $data, $options);
99151
}
100152
}
101153

0 commit comments

Comments
 (0)
0