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 9146ade

Browse filesBrowse files
fabpotfabpot
committed
merged branch bschussek/issue4715 (PR #4771)
Commits ------- 5fe3f39 [Form] Made data mappers completely responsible for dealing with empty values. Removed duplication of code. 9bf6e8b [Form] Compound forms now always need a data mapper. Otherwise an exception is thrown. Discussion ---------- [Form] Made requirements for data mappers stricter Bug fix: no Feature addition: no Backwards compatibility break: (yes) Symfony2 tests pass: yes Fixes the following tickets: - Todo: - This cleanup was done while trying to fix #4715, which is not easily fixable right now. It breaks BC for those people who do not extend `FormType` and manually construct `Form` instances, which hopefully nobody does as it is absolutely *not* recommended at this time.
2 parents 98b6fc7 + 5fe3f39 commit 9146ade
Copy full SHA for 9146ade

File tree

Expand file treeCollapse file tree

14 files changed

+1633
-1421
lines changed
Filter options
Expand file treeCollapse file tree

14 files changed

+1633
-1421
lines changed

‎src/Symfony/Component/Form/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/CHANGELOG.md
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,4 @@ CHANGELOG
139139
* deprecated `getChildren` in Form and FormBuilder in favor of `all`
140140
* deprecated `hasChildren` in Form and FormBuilder in favor of `count`
141141
* FormBuilder now implements \IteratorAggregate
142+
* [BC BREAK] compound forms now always need a data mapper

‎src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php
+21-11Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,24 @@ class PropertyPathMapper implements DataMapperInterface
2323
*/
2424
public function mapDataToForms($data, array $forms)
2525
{
26-
if (!empty($data) && !is_array($data) && !is_object($data)) {
26+
if (null === $data || array() === $data) {
27+
return;
28+
}
29+
30+
if (!is_array($data) && !is_object($data)) {
2731
throw new UnexpectedTypeException($data, 'object, array or empty');
2832
}
2933

30-
if (!empty($data)) {
31-
$iterator = new VirtualFormAwareIterator($forms);
32-
$iterator = new \RecursiveIteratorIterator($iterator);
34+
$iterator = new VirtualFormAwareIterator($forms);
35+
$iterator = new \RecursiveIteratorIterator($iterator);
3336

34-
foreach ($iterator as $form) {
35-
/* @var FormInterface $form */
36-
$propertyPath = $form->getPropertyPath();
37-
$config = $form->getConfig();
37+
foreach ($iterator as $form) {
38+
/* @var FormInterface $form */
39+
$propertyPath = $form->getPropertyPath();
40+
$config = $form->getConfig();
3841

39-
if (null !== $propertyPath && $config->getMapped()) {
40-
$form->setData($propertyPath->getValue($data));
41-
}
42+
if (null !== $propertyPath && $config->getMapped()) {
43+
$form->setData($propertyPath->getValue($data));
4244
}
4345
}
4446
}
@@ -48,6 +50,14 @@ public function mapDataToForms($data, array $forms)
4850
*/
4951
public function mapFormsToData(array $forms, &$data)
5052
{
53+
if (null === $data) {
54+
return;
55+
}
56+
57+
if (!is_array($data) && !is_object($data)) {
58+
throw new UnexpectedTypeException($data, 'object, array or empty');
59+
}
60+
5161
$iterator = new VirtualFormAwareIterator($forms);
5262
$iterator = new \RecursiveIteratorIterator($iterator);
5363

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/Type/FormType.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function buildForm(FormBuilderInterface $builder, array $options)
4444
->setVirtual($options['virtual'])
4545
->setCompound($options['compound'])
4646
->setData($options['data'])
47-
->setDataMapper(new PropertyPathMapper())
47+
->setDataMapper($options['compound'] ? new PropertyPathMapper() : null)
4848
;
4949

5050
if ($options['trim']) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Form.php
+31-24Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Form\Exception\AlreadyBoundException;
1616
use Symfony\Component\Form\Exception\UnexpectedTypeException;
1717
use Symfony\Component\Form\Exception\TransformationFailedException;
18+
use Symfony\Component\Form\Util\FormUtil;
1819
use Symfony\Component\Form\Util\PropertyPath;
1920
use Symfony\Component\HttpFoundation\Request;
2021
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -130,6 +131,13 @@ public function __construct(FormConfigInterface $config)
130131
$config = new UnmodifiableFormConfig($config);
131132
}
132133

134+
// Compound forms always need a data mapper, otherwise calls to
135+
// `setData` and `add` will not lead to the correct population of
136+
// the child forms.
137+
if ($config->getCompound() && !$config->getDataMapper()) {
138+
throw new FormException('Compound forms need a data mapper');
139+
}
140+
133141
$this->config = $config;
134142

135143
$this->setData($config->getData());
@@ -345,7 +353,7 @@ public function setData($modelData)
345353
$viewData = $this->normToView($normData);
346354

347355
// Validate if view data matches data class (unless empty)
348-
if ('' !== $viewData && null !== $viewData) {
356+
if (!FormUtil::isEmpty($viewData)) {
349357
$dataClass = $this->config->getDataClass();
350358

351359
$actualType = is_object($viewData) ? 'an instance of class ' . get_class($viewData) : ' a(n) ' . gettype($viewData);
@@ -378,7 +386,7 @@ public function setData($modelData)
378386
$this->viewData = $viewData;
379387
$this->synchronized = true;
380388

381-
if ($this->config->getCompound() && $this->config->getDataMapper()) {
389+
if ($this->config->getCompound()) {
382390
// Update child forms from the data
383391
$this->config->getDataMapper()->mapDataToForms($viewData, $this->children);
384392
}
@@ -477,17 +485,20 @@ public function bind($submittedData)
477485
$this->config->getEventDispatcher()->dispatch(FormEvents::BIND_CLIENT_DATA, $event);
478486
$submittedData = $event->getData();
479487

488+
// By default, the submitted data is also the data in view format
489+
$viewData = $submittedData;
490+
480491
// Check whether the form is compound.
481492
// This check is preferrable over checking the number of children,
482493
// since forms without children may also be compound.
483494
// (think of empty collection forms)
484495
if ($this->config->getCompound()) {
485-
if (null === $submittedData || '' === $submittedData) {
486-
$submittedData = array();
487-
}
488-
489496
if (!is_array($submittedData)) {
490-
throw new UnexpectedTypeException($submittedData, 'array');
497+
if (!FormUtil::isEmpty($submittedData)) {
498+
throw new UnexpectedTypeException($submittedData, 'array');
499+
}
500+
501+
$submittedData = array();
491502
}
492503

493504
foreach ($this->children as $name => $child) {
@@ -503,19 +514,14 @@ public function bind($submittedData)
503514
$extraData[$name] = $value;
504515
}
505516
}
506-
}
507517

508-
// By default, the submitted data is also the data in view format
509-
$viewData = $submittedData;
510-
511-
// If the form is compound, the default data in view format
512-
// is reused. The data of the children is merged into this
513-
// default data using the data mapper.
514-
if ($this->config->getCompound() && $this->config->getDataMapper()) {
518+
// If the form is compound, the default data in view format
519+
// is reused. The data of the children is merged into this
520+
// default data using the data mapper.
515521
$viewData = $this->getViewData();
516522
}
517523

518-
if (null === $viewData || '' === $viewData) {
524+
if (FormUtil::isEmpty($viewData)) {
519525
$emptyData = $this->config->getEmptyData();
520526

521527
if ($emptyData instanceof \Closure) {
@@ -527,7 +533,7 @@ public function bind($submittedData)
527533
}
528534

529535
// Merge form data from children into existing view data
530-
if ($this->config->getCompound() && $this->config->getDataMapper() && null !== $viewData) {
536+
if ($this->config->getCompound()) {
531537
$this->config->getDataMapper()->mapFormsToData($this->children, $viewData);
532538
}
533539

@@ -696,7 +702,7 @@ public function isEmpty()
696702
}
697703
}
698704

699-
return array() === $this->modelData || null === $this->modelData || '' === $this->modelData;
705+
return FormUtil::isEmpty($this->modelData) || array() === $this->modelData;
700706
}
701707

702708
/**
@@ -853,9 +859,7 @@ public function add(FormInterface $child)
853859

854860
$child->setParent($this);
855861

856-
if ($this->config->getDataMapper()) {
857-
$this->config->getDataMapper()->mapDataToForms($this->getViewData(), array($child));
858-
}
862+
$this->config->getDataMapper()->mapDataToForms($this->getViewData(), array($child));
859863

860864
return $this;
861865
}
@@ -1045,9 +1049,12 @@ private function normToModel($value)
10451049
*/
10461050
private function normToView($value)
10471051
{
1048-
if (!$this->config->getViewTransformers()) {
1049-
// Scalar values should always be converted to strings to
1050-
// facilitate differentiation between empty ("") and zero (0).
1052+
// Scalar values should be converted to strings to
1053+
// facilitate differentiation between empty ("") and zero (0).
1054+
// Only do this for simple forms, as the resulting value in
1055+
// compound forms is passed to the data mapper and thus should
1056+
// not be converted to a string before.
1057+
if (!$this->config->getViewTransformers() && !$this->config->getCompound()) {
10511058
return null === $value || is_scalar($value) ? (string) $value : $value;
10521059
}
10531060

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/FormConfig.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class FormConfig implements FormConfigEditorInterface
5656
/**
5757
* @var Boolean
5858
*/
59-
private $compound = true;
59+
private $compound = false;
6060

6161
/**
6262
* @var array

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

Copy file name to clipboard
+141Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Form\Tests;
13+
14+
use Symfony\Component\Form\FormError;
15+
use Symfony\Component\Form\FormBuilder;
16+
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
17+
18+
abstract class AbstractFormTest extends \PHPUnit_Framework_TestCase
19+
{
20+
/**
21+
* @var EventDispatcherInterface
22+
*/
23+
protected $dispatcher;
24+
25+
/**
26+
* @var \Symfony\Component\Form\FormFactoryInterface
27+
*/
28+
protected $factory;
29+
30+
/**
31+
* @var \Symfony\Component\Form\FormInterface
32+
*/
33+
protected $form;
34+
35+
protected function setUp()
36+
{
37+
if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) {
38+
$this->markTestSkipped('The "EventDispatcher" component is not available');
39+
}
40+
41+
$this->dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
42+
$this->factory = $this->getMock('Symfony\Component\Form\FormFactoryInterface');
43+
$this->form = $this->createForm();
44+
}
45+
46+
protected function tearDown()
47+
{
48+
$this->dispatcher = null;
49+
$this->factory = null;
50+
$this->form = null;
51+
}
52+
53+
/**
54+
* @return \Symfony\Component\Form\FormInterface
55+
*/
56+
abstract protected function createForm();
57+
58+
/**
59+
* @param string $name
60+
* @param EventDispatcherInterface $dispatcher
61+
* @param string $dataClass
62+
*
63+
* @return FormBuilder
64+
*/
65+
protected function getBuilder($name = 'name', EventDispatcherInterface $dispatcher = null, $dataClass = null)
66+
{
67+
return new FormBuilder($name, $dataClass, $dispatcher ?: $this->dispatcher, $this->factory);
68+
}
69+
70+
/**
71+
* @param string $name
72+
*
73+
* @return \PHPUnit_Framework_MockObject_MockObject
74+
*/
75+
protected function getMockForm($name = 'name')
76+
{
77+
$form = $this->getMock('Symfony\Component\Form\Tests\FormInterface');
78+
79+
$form->expects($this->any())
80+
->method('getName')
81+
->will($this->returnValue($name));
82+
83+
return $form;
84+
}
85+
86+
/**
87+
* @param string $name
88+
*
89+
* @return \PHPUnit_Framework_MockObject_MockObject
90+
*/
91+
protected function getValidForm($name)
92+
{
93+
$form = $this->getMockForm($name);
94+
95+
$form->expects($this->any())
96+
->method('isValid')
97+
->will($this->returnValue(true));
98+
99+
return $form;
100+
}
101+
102+
/**
103+
* @param string $name
104+
*
105+
* @return \PHPUnit_Framework_MockObject_MockObject
106+
*/
107+
protected function getInvalidForm($name)
108+
{
109+
$form = $this->getMockForm($name);
110+
111+
$form->expects($this->any())
112+
->method('isValid')
113+
->will($this->returnValue(false));
114+
115+
return $form;
116+
}
117+
118+
/**
119+
* @return \PHPUnit_Framework_MockObject_MockObject
120+
*/
121+
protected function getDataMapper()
122+
{
123+
return $this->getMock('Symfony\Component\Form\DataMapperInterface');
124+
}
125+
126+
/**
127+
* @return \PHPUnit_Framework_MockObject_MockObject
128+
*/
129+
protected function getDataTransformer()
130+
{
131+
return $this->getMock('Symfony\Component\Form\DataTransformerInterface');
132+
}
133+
134+
/**
135+
* @return \PHPUnit_Framework_MockObject_MockObject
136+
*/
137+
protected function getFormValidator()
138+
{
139+
return $this->getMock('Symfony\Component\Form\FormValidatorInterface');
140+
}
141+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.