8000 [Form] Fixed expanded choice field to be marked invalid when unknown … · symfony/symfony@6283b0e · GitHub
[go: up one dir, main page]
Skip to content

Commit 6283b0e

Browse files
webmozartwebmozart
committed
[Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted
1 parent 79a214f commit 6283b0e

File tree

8 files changed

+476
-116
lines changed

8 files changed

+476
-116
lines changed

‎src/Symfony/Component/Form/Extension/Core/DataTransformer/BooleanToStringTransformer.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,11 @@ public function __construct($trueValue)
4949
*/
5050
public function transform($value)
5151
{
52-
if (null === $value) {
53-
return null;
54-
}
55-
5652
if (!is_bool($value)) {
5753
throw new TransformationFailedException('Expected a Boolean.');
5854
}
5955

60-
return true === $value ? $this->trueValue : null;
56+
return $value ? $this->trueValue : null;
6157
}
6258

6359
/**

‎src/Symfony/Component/Form/Extension/Core/EventListener/FixCheckboxInputListener.php

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Form\Extension\Core\EventListener;
1313

14+
use Symfony\Component\Form\Exception\TransformationFailedException;
1415
use Symfony\Component\Form\FormEvents;
1516
use Symfony\Component\Form\FormEvent;
1617
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -38,10 +39,43 @@ public function __construct(ChoiceListInterface $choiceList)
3839

3940
public function preSubmit(FormEvent $event)
4041
{
41-
$values = (array) $event->getData();
42-
$indices = $this->choiceList->getIndicesForValues($values);
42+
$data = $event->getData();
4343

44-
$event->setData(count($indices) > 0 ? array_combine($indices, $values) : array());
44+
if (is_array($data)) {
45+
// Flip the submitted values for faster lookup
46+
// It's better to flip this array than $existingValues because
47+
// $submittedValues is generally smaller.
48+
$submittedValues = array_flip($data);
49+
50+
// Since expanded choice fields are completely loaded anyway, we
51+
// can just as well get the values again without losing performance.
52+
$existingValues = $this->choiceList->getValues();
53+
54+
// Clear the data array and fill it with correct indices
55+
$data = array();
56+
57+
foreach ($existingValues as $index => $value) {
58+
if (isset($submittedValues[$value])) {
59+
// Value was submitted
60+
$data[$index] = $value;
61+
unset($submittedValues[$value]);
62+
}
63+
}
64+
65+
if (count($submittedValues) > 0) {
66+
throw new TransformationFailedException(sprintf(
67+
'The following choices were not found: "%s"',
68+
implode('", "', array_keys($submittedValues))
69+
));
70+
}
71+
} elseif ('' === $data || null === $data) {
72+
// Empty values are always accepted.
73+
$data = array();
74+
}
75+
76+
// Else leave the data unchanged to provoke an error during submission
77+
78+
$event->setData($data);
4579
}
4680

4781
/**

‎src/Symfony/Component/Form/Extension/Core/EventListener/FixRadioInputListener.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,22 @@ public function __construct(ChoiceListInterface $choiceList, $placeholderPresent
4242

4343
public function preSubmit(FormEvent $event)
4444
{
45-
$value = $event->getData();
46-
$index = current($this->choiceList->getIndicesForValues(array($value)));
45+
$data = $event->getData();
4746

48-
$event->setData(false !== $index ? array($index => $value) : ($this->placeholderPresent ? array('placeholder' => '') : array())) ;
47+
// Since expanded choice fields are completely loaded anyway, we
48+
// can just as well get the values again without losing performance.
49+
$existingValues = $this->choiceList->getValues();
50+
51+
if (false !== ($index = array_search($data, $existingValues, true))) {
52+
$data = array($index => $data);
53+
} elseif ('' === $data || null === $data) {
54+
// Empty values are always accepted.
55+
$data = $this->placeholderPresent ? array('placeholder' => '') : array();
56+
}
57+
58+
// Else leave the data unchanged to provoke an error during submission
59+
60+
$event->setData($data);
4961
}
5062

5163
/**

‎src/Symfony/Component/Form/Extension/Core/Type/CheckboxType.php

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,14 @@ class CheckboxType extends AbstractType
2525
*/
2626
public function buildForm(FormBuilderInterface $builder, array $options)
2727
{
28-
$builder
29-
->addViewTransformer(new BooleanToStringTransformer($options['value']))
30-
;
28+
// Unlike in other types, where the data is NULL by default, it
29+
// needs to be a Boolean here. setData(null) is not acceptable
30+
// for checkboxes and radio buttons (unless a custom model
31+
// transformer handles this case).
32+
// We cannot solve this case via overriding the "data" option, because
33+
// doing so also calls setDataLocked(true).
34+
$builder->setData(isset($options['data']) ? $options['data'] : false);
35+
$builder->addViewTransformer(new BooleanToStringTransformer($options['value']));
3136
}
3237

3338
/**
@@ -46,8 +51,8 @@ public function buildView(FormView $view, FormInterface $form, array $options)
4651
*/
4752
public function setDefaultOptions(OptionsResolverInterface $resolver)
4853
{
49-
$emptyData = function (FormInterface $form, $clientData) {
50-
return $clientData;
54+
$emptyData = function (FormInterface $form, $viewData) {
55+
return $viewData;
5156
};
5257

5358
$resolver->setDefaults(array(

‎src/Symfony/Component/Form/Form.php

Lines changed: 77 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -522,83 +522,82 @@ public function submit($submittedData, $clearMissing = true)
522522

523523
$dispatcher = $this->config->getEventDispatcher();
524524

525-
// Hook to change content of the data submitted by the browser
526-
if ($dispatcher->hasListeners(FormEvents::PRE_SUBMIT)) {
527-
$event = new FormEvent($this, $submittedData);
528-
$dispatcher->dispatch(FormEvents::PRE_SUBMIT, $event);
529-
$submittedData = $event->getData();
530-
}
525+
$modelData = null;
526+
$normData = null;
527+
$viewData = null;
531528

532-
// Check whether the form is compound.
533-
// This check is preferable over checking the number of children,
534-
// since forms without children may also be compound.
535-
// (think of empty collection forms)
536-
if ($this->config->getCompound()) {
537-
if (!is_array($submittedData)) {
538-
$submittedData = array();
529+
try {
530+
// Hook to change content of the data submitted by the browser
531+
if ($dispatcher->hasListeners(FormEvents::PRE_SUBMIT)) {
532+
$event = new FormEvent($this, $submittedData);
533+
$dispatcher->dispatch(FormEvents::PRE_SUBMIT, $event);
534+
$submittedData = $event->getData();
539535
}
540536

541-
foreach ($this->children as $name => $child) {
542-
if (array_key_exists($name, $submittedData) || $clearMissing) {
543-
$child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
544-
unset($submittedData[$name]);
537+
// Check whether the form is compound.
538+
// This check is preferable over checking the number of children,
539+
// since forms without children may also be compound.
540+
// (think of empty collection forms)
541+
if ($this->config->getCompound()) {
542+
if (null === $submittedData) {
543+
$submittedData = array();
545544
}
546-
}
547-
548-
$this->extraData = $submittedData;
549-
}
550545

551-
// Forms that inherit their parents' data also are not processed,
552-
// because then it would be too difficult to merge the changes in
553-
// the child and the parent form. Instead, the parent form also takes
554-
// changes in the grandchildren (i.e. children of the form that inherits
555-
// its parent's data) into account.
556-
// (see InheritDataAwareIterator below)
557-
if ($this->config->getInheritData()) {
558-
$this->submitted = true;
546+
if (!is_array($submittedData)) {
547+
throw new TransformationFailedException('Compound forms expect an array or NULL on submission.');
548+
}
559549

560-
// When POST_SUBMIT is reached, the data is not yet updated, so pass
561-
// NULL to prevent hard-to-debug bugs.
562-
$dataForPostSubmit = null;
563-
} else {
564-
// If the form is compound, the default data in view format
565-
// is reused. The data of the children is merged into this
566-
// default data using the data mapper.
567-
// If the form is not compound, the submitted data is also the data in view format.
568-
$viewData = $this->config->getCompound() ? $this->viewData : $submittedData;
569-
570-
if (FormUtil::isEmpty($viewData)) {
571-
$emptyData = $this->config->getEmptyData();
572-
573-
if ($emptyData instanceof \Closure) {
574-
/* @var \Closure $emptyData */
575-
$emptyData = $emptyData($this, $viewData);
550+
foreach ($this->children as $name => $child) {
551+
if (isset($submittedData[$name]) || $clearMissing) {
552+
$child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
553+
unset($submittedData[$name]);
554+
}
576555
}
577556

578-
$viewData = $emptyData;
557+
$this->extraData = $submittedData;
579558
}
580559

581-
// Merge form data from children into existing view data
582-
// It is not necessary to invoke this method if the form has no children,
583-
// even if it is compound.
584-
if (count($this->children) > 0) {
585-
// Use InheritDataAwareIterator to process children of
586-
// descendants that inherit this form's data.
587-
// These descendants will not be submitted normally (see the check
588-
// for $this->config->getInheritData() above)
589-
$iterator = new InheritDataAwareIterator($this->children);
590-
$iterator = new \RecursiveIteratorIterator($iterator);
591-
$this->config->getDataMapper()->mapFormsToData($iterator, $viewData);
592-
}
560+
// Forms that inherit their parents' data also are not processed,
561+
// because then it would be too difficult to merge the changes in
562+
// the child and the parent form. Instead, the parent form also takes
563+
// changes in the grandchildren (i.e. children of the form that inherits
564+
// its parent's data) into account.
565+
// (see InheritDataAwareIterator below)
566+
if (!$this->config->getInheritData()) {
567+
// If the form is compound, the default data in view format
568+
// is reused. The data of the children is merged into this
569+
// default data using the data mapper.
570+
// If the form is not compound, the submitted data is also the data in view format.
571+
$viewData = $this->config->getCompound() ? $this->viewData : $submittedData;
572+
573+
if (FormUtil::isEmpty($viewData)) {
574+
$emptyData = $this->config->getEmptyData();
575+
576+
if ($emptyData instanceof \Closure) {
577+
/* @var \Closure $emptyData */
578+
$emptyData = $emptyData($this, $viewData);
579+
}
580+
581+
$viewData = $emptyData;
582+
}
593583

594-
$modelData = null;
595-
$normData = null;
584+
// Merge form data from children into existing view data
585+
// It is not necessary to invoke this method if the form has no children,
586+
// even if it is compound.
587+
if (count($this->children) > 0) {
588+
// Use InheritDataAwareIterator to process children of
589+
// descendants that inherit this form's data.
590+
// These descendants will not be submitted normally (see the check
591+
// for $this->config->getInheritData() above)
592+
$childrenIterator = new InheritDataAwareIterator($this->children);
593+
$childrenIterator = new \RecursiveIteratorIterator($childrenIterator);
594+
$this->config->getDataMapper()->mapFormsToData($childrenIterator, $viewData);
595+
}
596596

597-
try {
598597
// Normalize data to unified representation
599598
$normData = $this->viewToNorm($viewData);
600599

601-
// Hook to change content of the data into the normalized
600+
// Hook to change content of the data in the normalized
602601
// representation
603602
if ($dispatcher->hasListeners(FormEvents::SUBMIT)) {
604603
$event = new FormEvent($this, $normData);
@@ -609,20 +608,26 @@ public function submit($submittedData, $clearMissing = true)
609608
// Synchronize representations - must not change the content!
610609
$modelData = $this->normToModel($normData);
611610
$viewData = $this->normToView($normData);
612-
} catch (TransformationFailedException $e) {
613-
$this->synchronized = false;
614611
}
615-
616-
$this->submitted = true;
617-
$this->modelData = $modelData;
618-
$this->normData = $normData;
619-
$this->viewData = $viewData;
620-
621-
$dataForPostSubmit = $viewData;
612+
} catch (TransformationFailedException $e) {
613+
$this->synchronized = false;
614+
615+
// If $viewData was not yet set, set it to $submittedData so that
616+
// the erroneous data is accessible on the form.
617+
// Forms that inherit data never set any data, because the getters
618+
// forward to the parent form's getters anyway.
619+
if (null === $viewData && !$this->config->getInheritData()) {
620+
$viewData = $submittedData;
621+
}
622622
}
623623

624+
$this->submitted = true;
625+
$this->modelData = $modelData;
626+
$this->normData = $normData;
627+
$this->viewData = $viewData;
628+
624629
if ($dispatcher->hasListeners(FormEvents::POST_SUBMIT)) {
625-
$event = new FormEvent($this, $dataForPostSubmit);
630+
$event = new FormEvent($this, $viewData);
626631
$dispatcher->dispatch(FormEvents::POST_SUBMIT, $event);
627632
}
628633

‎src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/BooleanToStringTransformerTest.php

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ class BooleanToStringTransformerTest extends \PHPUnit_Framework_TestCase
1717
{
1818
const TRUE_VALUE = '1';
1919

20+
/**
21+
* @var BooleanToStringTransformer
22+
*/
2023
protected $transformer;
2124

2225
protected function setUp()
@@ -33,20 +36,29 @@ public function testTransform()
3336
{
3437
$this->assertEquals(self::TRUE_VALUE, $this->transformer->transform(true));
3538
$this->assertNull($this->transformer->transform(false));
36-
$this->assertNull($this->transformer->transform(null));
3739
}
3840

39-
public function testTransformExpectsBoolean()
41+
/**
42+
* @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
43+
*/
44+
public function testTransformFailsIfNull()
4045
{
41-
$this->setExpectedException('Symfony\Component\Form\Exception\TransformationFailedException');
46+
$this->transformer->transform(null);
47+
}
4248

49+
/**
50+
* @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
51+
*/
52+
public function testTransformFailsIfString()
53+
{
4354
$this->transformer->transform('1');
4455
}
4556

46-
public function testReverseTransformExpectsString()
57+
/**
58+
* @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
59+
*/
60+
public function testReverseTransformFailsIfInteger()
4761
{
48-
$this->setExpectedException('Symfony\Component\Form\Exception\TransformationFailedException');
49-
5062
$this->transformer->reverseTransform(1);
5163
}
5264

0 commit comments

Comments
 (0)
0