8000 [Form] Fixed: error mapping aborts if reaching an unsynchronized form · symfony/symfony@59d6b55 · GitHub
[go: up one dir, main page]
Skip to content

Commit 59d6b55

Browse files
webmozartwebmozart
committed
[Form] Fixed: error mapping aborts if reaching an unsynchronized form
1 parent 9eda5f5 commit 59d6b55

File tree

2 files changed

+129
-50
lines changed

2 files changed

+129
-50
lines changed

‎src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
5454
$relativePath = $this->reconstructPath($violationPath, $form);
5555
$match = false;
5656

57+
// In general, mapping happens from the root form to the leaf forms
58+
// First, the rules of the root form are applied to determine
59+
// the subsequent descendant. The rules of this descendant are then
60+
// applied to find the next and so on, until we have found the
61+
// most specific form that matches the violation.
62+
63+
// If any of the forms found in this process is not synchronized,
64+
// mapping is aborted. Non-synchronized forms could not reverse
65+
// transform the value entered by the user, thus any further violations
66+
// caused by the (invalid) reverse transformed value should be
67+
// ignored.
68+
5769
if (null !== $relativePath) {
5870
// Set the scope to the root of the relative path
5971
// This root will usually be $form. If the path contains
@@ -62,13 +74,16 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
6274
$this->setScope($relativePath->getRoot());
6375
$it = new PropertyPathIterator($relativePath);
6476

65-
while (null !== ($child = $this->matchChild($it))) {
77+
while ($this->scope->isSynchronized() && null !== ($child = $this->matchChild($it))) {
6678
$this->setScope($child);
6779
$it->next();
6880
$match = true;
6981
}
7082
}
7183

84+
// This case happens if an error happened in the data under a
85+
// virtual form that does not match any of the children of
86+
// the virtual form.
7287
if (!$match) {
7388
// If we could not map the error to anything more specific
7489
// than the root element, map it to the innermost directly
@@ -80,7 +95,7 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
8095
// The overhead of setScope() is not needed anymore here
8196
$this->scope = $form;
8297

83-
while ($it->valid() && $it->mapsForm()) {
98+
while ($this->scope->isSynchronized() && $it->valid() && $it->mapsForm()) {
8499
if (!$this->scope->has($it->current())) {
85100
// Break if we find a reference to a non-existing child
86101
break;
@@ -94,17 +109,13 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
94109
// Follow dot rules until we have the final target
95110
$mapping = $this->scope->getAttribute('error_mapping');
96111

97-
while (isset($mapping['.'])) {
112+
while ($this->scope->isSynchronized() && isset($mapping['.'])) {
98113
$dotRule = new MappingRule($this->scope, '.', $mapping['.']);
99114
$this->scope = $dotRule->getTarget();
100115
$mapping = $this->scope->getAttribute('error_mapping');
101116
}
102117

103118
// Only add the error if the form is synchronized
104-
// If the form is not synchronized, it already contains an
105-
// error about being invalid. Further errors could be a result
106-
// of the failed transformation and thus should not be
107-
// displayed.
108119
if ($this->scope->isSynchronized()) {
109120
$this->scope->addError(new FormError(
110121
$violation->getMessageTemplate(),

‎src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php

Lines changed: 111 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,117 @@ protected function getFormError()
102102
return new FormError($this->message, $this->params);
103103
}
104104

105+
public function testMapToVirtualFormIfDataDoesNotMatch()
106+
{
107+
$violation = $this->getConstraintViolation('children[address].data.foo');
108+
$parent = $this->getForm('parent');
109+
$child = $this->getForm('address', 'address', null, array(), true);
110+
$grandChild = $this->getForm('street');
111+
112+
$parent->add($child);
113+
$child->add($grandChild);
114+
115+
$this->mapper->mapViolation($violation, $parent);
116+
117+
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one');
118+
$this->assertEquals(array($this->getFormError()), $child->getErrors(), $child->getName() . ' should have an error, but has none');
119+
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one');
120+
}
121+
122+
public function testFollowDotRules()
123+
{
124+
$violation = $this->getConstraintViolation('data.foo');
125+
$parent = $this->getForm('parent', null, null, array(
126+
'foo' => 'address',
127+
));
128+
$child = $this->getForm('address', null, null, array(
129+
'.' => 'street',
130+
));
131+
$grandChild = $this->getForm('street', null, null, array(
132+
'.' => 'name',
133+
));
134+
$grandGrandChild = $this->getForm('name');
135+
136+
$parent->add($child);
137+
$child->add($grandChild);
138+
$grandChild->add($grandGrandChild);
139+
140+
$this->mapper->mapViolation($violation, $parent);
141+
142+
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one');
143+
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one');
144+
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one');
145+
$this->assertEquals(array($this->getFormError()), $grandGrandChild->getErrors(), $grandGrandChild->getName() . ' should have an error, but has none');
146+
}
147+
148+
public function testAbortMappingIfNotSynchronized()
149+
{
150+
$violation = $this->getConstraintViolation('children[address].data.street');
151+
$parent = $this->getForm('parent');
152+
$child = $this->getForm('address', 'address', null, array(), false, false);
153+
// even though "street" is synchronized, it should not have any errors
154+
// due to its parent not being synchronized
155+
$grandChild = $this->getForm('street' , 'street');
156+
157+
$parent->add($child);
158+
$child->add($grandChild);
159+
160+
// bind to invoke the transformer and mark the form unsynchronized
161+
$parent->bind(array());
162+
163+
$this->mapper->mapViolation($violation, $parent);
164+
165+
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one');
166+
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one');
167+
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one');
168+
}
169+
170+
public function testAbortVirtualFormMappingIfNotSynchronized()
171+
{
172+
$violation = $this->getConstraintViolation('children[address].children[street].data.foo');
173+
$parent = $this->getForm('parent');
174+
$child = $this->getForm('address', 'address', null, array(), true, false);
175+
// even though "street" is synchronized, it should not have any errors
176+
// due to its parent not being synchronized
177+
$grandChild = $this->getForm('street' , 'street', null, array(), true);
178+
179+
$parent->add($child);
180+
$child->add($grandChild);
181+
182+
// bind to invoke the transformer and mark the form unsynchronized
183+
$parent->bind(array());
184+
185+
$this->mapper->mapViolation($violation, $parent);
186+
187+
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one');
188+
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one');
189+
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one');
190+
}
191+
192+
public function testAbortDotRuleMappingIfNotSynchronized()
193+
{
194+
$violation = $this->getConstraintViolation('data.address');
195+
$parent = $this->getForm('parent');
196+
$child = $this->getForm('address', 'address', null, array(
197+
'.' => 'street',
198+
), false, false);
199+
// even though "street" is synchronized, it should not have any errors
200+
// due to its parent not being synchronized
201+
$grandChild = $this->getForm('street');
202+
203+
$parent->add($child);
204+
$child->add($grandChild);
205+
206+
// bind to invoke the transformer and mark the form unsynchronized
207+
$parent->bind(array());
208+
209+
$this->mapper->mapViolation($violation, $parent);
210+
211+
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one');
212+
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one');
213+
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one');
214+
}
215+
105216
public function provideDefaultTests()
106217
{
107218
// The mapping must be deterministic! If a child has the property path "[street]",
@@ -1337,47 +1448,4 @@ public function testVirtualFormErrorMapping($target, $childName, $childPath, $gr
13371448
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none');
13381449
}
13391450
}
1340-
1341-
public function testDontMapToUnsynchronizedForms()
1342-
{
1343-
$violation = $this->getConstraintViolation('children[address].data.street');
1344-
$parent = $this->getForm('parent');
1345-
$child = $this->getForm('address', 'address', null, array(), false, false);
1346-
1347-
$parent->add($child);
1348-
1349-
// bind to invoke the transformer and mark the form unsynchronized
1350-
$parent->bind(array());
1351-
1352-
$this->mapper->mapViolation($violation, $parent);
1353-
1354-
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one');
1355-
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one');
1356-
}
1357-
1358-
public function testFollowDotRules()
1359-
{
1360-
$violation = $this->getConstraintViolation('data.foo');
1361-
$parent = $this->getForm('parent', null, null, array(
1362-
'foo' => 'address',
1363-
));
1364-
$child = $this->getForm('address', null, null, array(
1365-
'.' => 'street',
1366-
));
1367-
$grandChild = $this->getForm('street', null, null, array(
1368-
'.' => 'name',
1369-
));
1370-
$grandGrandChild = $this->getForm('name');
1371-
1372-
$parent->add($child);
1373-
$child->add($grandChild);
1374-
$grandChild->add($grandGrandChild);
1375-
1376-
$this->mapper->mapViolation($violation, $parent);
1377-
1378-
$this->assertFalse($parent->hasErrors(), $parent->getName 53C6 () . ' should not have an error, but has one');
1379-
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one');
1380-
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one');
1381-
$this->assertEquals(array($this->getFormError()), $grandGrandChild->getErrors(), $grandGrandChild->getName() . ' should have an error, but has none');
1382-
}
13831451
}

0 commit comments

Comments
 (0)
0