Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0f1a5c4

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #36343 [Form] Fixed handling groups sequence validation (HeahDude)
This PR was merged into the 3.4 branch. Discussion ---------- [Form] Fixed handling groups sequence validation | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | FIx #9939 (comment), Fix #35556 | License | MIT | Doc PR | ~ This is not the same as the original issue fixed by #36245, that was reported in #9939 (comment). The form also fails to cascade sequence validation properly because each nested field is validated against the sequence, and one can fail at a step independently from another which could failed in another step. I've added a lot of tests to ensure this is working properly and tested in a website skeleton too. This PR aims to close #35556 which tries to fix the same issue but afterwards in its implementation as said in #35556 (comment). Commits ------- dfb61c2 [Form] Fixed handling groups sequence validation
2 parents 17bbaa5 + dfb61c2 commit 0f1a5c4
Copy full SHA for 0f1a5c4

File tree

5 files changed

+212
-29
lines changed
Filter options

5 files changed

+212
-29
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php
+59-20Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Resources/config/validation.xml
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php
+36-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php
+62-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php
+54-2Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.