Skip to content

Commit b07fb3c

Browse files
fabpotfabpot
committed
merged branch bschussek/issue4385 (PR #4386)
Commits ------- bad6d04 [Form] Added accessor FormConfigInterface::getByReference() and let Form clone objects if not by reference fc23701 [Form] Correctly highlighted BC breaks in the CHANGELOG d1864c7 [Form] Fixed: Virtual forms are ignored when prepopulating a form Discussion ---------- [Form] Fixed: PropertyPathMapper did not always ignore virtual forms Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #4385 Todo: - --------------------------------------------------------------------------- by travisbot at 2012-05-23T12:13:49Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1410299) (merged a7f90944 into e023807). --------------------------------------------------------------------------- by travisbot at 2012-05-23T12:27:30Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1410430) (merged 52510fee into e023807). --------------------------------------------------------------------------- by travisbot at 2012-05-23T12:37:00Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1410485) (merged ca5aee9c into e023807). --------------------------------------------------------------------------- by travisbot at 2012-05-23T13:01:10Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1410669) (merged bad6d04 into e023807).
2 parents e023807 + bad6d04 commit b07fb3c

File tree

10 files changed

+282
-132
lines changed

10 files changed

+282
-132
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,20 @@ CHANGELOG
7171
* labels don't display field attributes anymore. Label attributes can be
7272
passed in the "label_attr" option/variable
7373
* added option "mapped" which should be used instead of setting "property_path" to false
74-
* "data_class" now *must* be set if a form maps to an object and should be left empty otherwise
74+
* [BC BREAK] "data_class" now *must* be set if a form maps to an object and should be left empty otherwise
7575
* improved error mapping on forms
7676
* dot (".") rules are now allowed to map errors assigned to a form to
7777
one of its children
7878
* errors are not mapped to unsynchronized forms anymore
79-
* changed Form constructor to accept a single `FormConfigInterface` object
80-
* changed argument order in the FormBuilder constructor
79+
* [BC BREAK] changed Form constructor to accept a single `FormConfigInterface` object
80+
* [BC BREAK] changed argument order in the FormBuilder constructor
8181
* deprecated Form methods
8282
* `getTypes`
8383
* `getErrorBubbling`
8484
* `getNormTransformers`
8585
* `getClientTransformers`
8686
* deprecated the option "validation_constraint" in favor of the new
8787
option "constraints"
88+
* removed superfluous methods from DataMapperInterface
89+
* `mapFormToData`
90+
* `mapDataToForm`

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,5 @@ interface DataMapperInterface
2121
*/
2222
function mapDataToForms($data, array $forms);
2323

24-
function mapDataToForm($data, FormInterface $form);
25-
2624
function mapFormsToData(array $forms, &$data);
27-
28-
function mapFormToData(FormInterface $form, &$data);
2925
}

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

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -32,28 +32,13 @@ public function mapDataToForms($data, array $forms)
3232
$iterator = new \RecursiveIteratorIterator($iterator);
3333

3434
foreach ($iterator as $form) {
35-
$this->mapDataToForm($data, $form);
36-
}
37-
}
38-
}
39-
40-
/**
41-
* {@inheritdoc}
42-
*/
43-
public function mapDataToForm($data, FormInterface $form)
44-
{
45-
if (!empty($data)) {
46-
$propertyPath = $form->getPropertyPath();
47-
$config = $form->getConfig();
48-
49-
if (null !== $propertyPath && $config->getMapped()) {
50-
$propertyData = $propertyPath->getValue($data);
35+
/* @var FormInterface $form */
36+
$propertyPath = $form->getPropertyPath();
37+
$config = $form->getConfig();
5138

52-
if (is_object($propertyData) && !$form->getAttribute('by_reference')) {
53-
$propertyData = clone $propertyData;
39+
if (null !== $propertyPath && $config->getMapped()) {
40+
$form->setData($propertyPath->getValue($data));
5441
}
55-
56-
$form->setData($propertyData);
5742
}
5843
}
5944
}
@@ -67,28 +52,20 @@ public function mapFormsToData(array $forms, &$data)
6752
$iterator = new \RecursiveIteratorIterator($iterator);
6853

6954
foreach ($iterator as $form) {
70-
$this->mapFormToData($form, $data);
71-
}
72-
}
73-
74-
/**
75-
* {@inheritdoc}
76-
*/
77-
public function mapFormToData(FormInterface $form, &$data)
78-
{
79-
$propertyPath = $form->getPropertyPath();
80-
$config = $form->getConfig();
55+
/* @var FormInterface $form */
56+
$propertyPath = $form->getPropertyPath();
57+
$config = $form->getConfig();
8158

82-
// Write-back is disabled if the form is not synchronized (transformation failed)
83-
// and if the form is disabled (modification not allowed)
84-
if (null !== $propertyPath && $config->getMapped() && $form->isSynchronized() && !$form->isDisabled()) {
85-
// If the data is identical to the value in $data, we are
86-
// dealing with a reference
87-
$isReference = $form->getData() === $propertyPath->getValue($data);
88-
$byReference = $form->getAttribute('by_reference');
59+
// Write-back is disabled if the form is not synchronized (transformation failed)
60+
// and if the form is disabled (modification not allowed)
61+
if (null !== $propertyPath && $config->getMapped() && $form->isSynchronized() && !$form->isDisabled()) {
62+
// If the data is identical to the value in $data, we are
63+
// dealing with a reference
64+
$isReference = $form->getData() === $propertyPath->getValue($data);
8965

90-
if (!(is_object($data) && $isReference && $byReference)) {
91-
$propertyPath->setValue($data, $form->getData());
66+
if (!is_object($data) || !$isReference || !$config->getByReference()) {
67+
$propertyPath->setValue($data, $form->getData());
68+
}
9269
}
9370
}
9471
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ public function buildForm(FormBuilder $builder, array $options)
4646
// BC compatibility, when "property_path" could be false
4747
->setPropertyPath(is_string($options['property_path']) ? $options['property_path'] : null)
4848
->setMapped($options['mapped'])
49+
->setByReference($options['by_reference'])
4950
->setVirtual($options['virtual'])
5051
->setAttribute('read_only', $options['read_only'])
51-
->setAttribute('by_reference', $options['by_reference'])
5252
->setAttribute('max_length', $options['max_length'])
5353
->setAttribute('pattern', $options['pattern'])
5454
->setAttribute('label', $options['label'] ?: $this->humanize($builder->getName()))

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,10 @@ public function setData($appData)
319319
throw new AlreadyBoundException('You cannot change the data of a bound form');
320320
}
321321

322+
if (is_object($appData) && !$this->config->getByReference()) {
323+
$appData = clone $appData;
324+
}
325+
322326
$event = new DataEvent($this, $appData);
323327
$this->config->getEventDispatcher()->dispatch(FormEvents::PRE_SET_DATA, $event);
324328

@@ -805,7 +809,7 @@ public function add(FormInterface $child)
805809
$child->setParent($this);
806810

807811
if ($this->config->getDataMapper()) {
808-
$this->config->getDataMapper()->mapDataToForm($this->getClientData(), $child);
812+
$this->config->getDataMapper()->mapDataToForms($this->getClientData(), array($child));
809813
}
810814

811815
return $this;

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

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,17 @@ class FormConfig implements FormConfigInterface
4141
/**
4242
* @var Boolean
4343
*/
44-
private $mapped;
44+
private $mapped = true;
4545

4646
/**
4747
* @var Boolean
4848
*/
49-
private $virtual;
49+
private $byReference = true;
50+
51+
/**
52+
* @var Boolean
53+
*/
54+
private $virtual = false;
5055

5156
/**
5257
* @var array
@@ -76,17 +81,17 @@ class FormConfig implements FormConfigInterface
7681
/**
7782
* @var Boolean
7883
*/
79-
private $required;
84+
private $required = true;
8085

8186
/**
8287
* @var Boolean
8388
*/
84-
private $disabled;
89+
private $disabled = false;
8590

8691
/**
8792
* @var Boolean
8893
*/
89-
private $errorBubbling;
94+
private $errorBubbling = false;
9095

9196
/**
9297
* @var mixed
@@ -294,6 +299,14 @@ public function getMapped()
294299
return $this->mapped;
295300
}
296301

302+
/**
303+
* {@inheritdoc}
304+
*/
305+
public function getByReference()
306+
{
307+
return $this->byReference;
308+
}
309+
297310
/**
298311
* {@inheritdoc}
299312
*/
@@ -550,6 +563,21 @@ public function setMapped($mapped)
550563
return $this;
551564
}
552565

566+
/**
567+
* Sets whether the form's data should be modified by reference.
568+
*
569+
* @param Boolean $byReference Whether the data should be
570+
modified by reference.
571+
*
572+
* @return self The configuration object.
573+
*/
574+
public function setByReference($byReference)
575+
{
576+
$this->byReference = $byReference;
577+
578+
return $this;
579+
}
580+
553581
/**
554582
* Sets whether the form should be virtual.
555583
*

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ function getPropertyPath();
4747
*/
4848
function getMapped();
4949

50+
/**
51+
* Returns whether the form's data should be modified by reference.
52+
*
53+
* @return Boolean Whether to modify the form's data by reference.
54+
*/
55+
function getByReference();
56+
5057
/**
5158
* Returns whether the form should be virtual.
5259
*

0 commit comments

Comments
 (0)