diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index 4f1b61a0cd..71f54c5fba 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -37,13 +37,7 @@ class PropertyPathMapper implements DataMapperInterface $config = $form->getConfig(); if (null !== $propertyPath && $config->getMapped()) { - $propertyData = $propertyPath->getValue($data); - - if (is_object($propertyData) && !$form->getAttribute('by_reference')) { - $propertyData = clone $propertyData; - } - - $form->setData($propertyData); + $form->setData($propertyPath->getValue($data)); } } } @@ -68,9 +62,8 @@ class PropertyPathMapper implements DataMapperInterface // If the data is identical to the value in $data, we are // dealing with a reference $isReference = $form->getData() === $propertyPath->getValue($data); - $byReference = $form->getAttribute('by_reference'); - if (!(is_object($data) && $isReference && $byReference)) { + if (!is_object($data) || !$isReference || !$config->getByReference()) { $propertyPath->setValue($data, $form->getData()); } } diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php index 3ce2d18bcd..212675ee1a 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php @@ -46,9 +46,9 @@ class FormType extends AbstractType // BC compatibility, when "property_path" could be false ->setPropertyPath(is_string($options['property_path']) ? $options['property_path'] : null) ->setMapped($options['mapped']) + ->setByReference($options['by_reference']) ->setVirtual($options['virtual']) ->setAttribute('read_only', $options['read_only']) - ->setAttribute('by_reference', $options['by_reference']) ->setAttribute('max_length', $options['max_length']) ->setAttribute('pattern', $options['pattern']) ->setAttribute('label', $options['label'] ?: $this->humanize($builder->getName())) diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index fb3d66b17e..b9fe51376d 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -319,6 +319,10 @@ class Form implements \IteratorAggregate, FormInterface throw new AlreadyBoundException('You cannot change the data of a bound form'); } + if (is_object($appData) && !$this->config->getByReference()) { + $appData = clone $appData; + } + $event = new DataEvent($this, $appData); $this->config->getEventDispatcher()->dispatch(FormEvents::PRE_SET_DATA, $event); diff --git a/src/Symfony/Component/Form/FormConfig.php b/src/Symfony/Component/Form/FormConfig.php index 59a459367c..eae045323c 100644 --- a/src/Symfony/Component/Form/FormConfig.php +++ b/src/Symfony/Component/Form/FormConfig.php @@ -43,6 +43,11 @@ class FormConfig implements FormConfigInterface */ private $mapped = true; + /** + * @var Boolean + */ + private $byReference = true; + /** * @var Boolean */ @@ -294,6 +299,14 @@ class FormConfig implements FormConfigInterface return $this->mapped; } + /** + * {@inheritdoc} + */ + public function getByReference() + { + return $this->byReference; + } + /** * {@inheritdoc} */ @@ -550,6 +563,21 @@ class FormConfig implements FormConfigInterface return $this; } + /** + * Sets whether the form's data should be modified by reference. + * + * @param Boolean $byReference Whether the data should be + modified by reference. + * + * @return self The configuration object. + */ + public function setByReference($byReference) + { + $this->byReference = $byReference; + + return $this; + } + /** * Sets whether the form should be virtual. * diff --git a/src/Symfony/Component/Form/FormConfigInterface.php b/src/Symfony/Component/Form/FormConfigInterface.php index acceb84477..554a104667 100644 --- a/src/Symfony/Component/Form/FormConfigInterface.php +++ b/src/Symfony/Component/Form/FormConfigInterface.php @@ -47,6 +47,13 @@ interface FormConfigInterface */ function getMapped(); + /** + * Returns whether the form's data should be modified by reference. + * + * @return Boolean Whether to modify the form's data by reference. + */ + function getByReference(); + /** * Returns whether the form should be virtual. * diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index e6bb3a4b3e..040e9899dc 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -110,7 +110,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue($engine)); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $form = $this->getForm($config); @@ -133,7 +133,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue($engine)); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', false); + $config->setByReference(false); $config->setPropertyPath($propertyPath); $form = $this->getForm($config); @@ -148,7 +148,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase $car = new \stdClass(); $config = new FormConfig(null, '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $form = $this->getForm($config); $this->assertNull($form->getPropertyPath()); @@ -167,7 +167,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->method('getValue'); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setMapped(false); $config->setPropertyPath($propertyPath); $form = $this->getForm($config); @@ -185,7 +185,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->method('getValue'); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $form = $this->getForm($config); @@ -206,12 +206,12 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue($engine)); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setVirtual(true); $form = $this->getForm($config); $config = new FormConfig('engine', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $child = $this->getForm($config); @@ -234,7 +234,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->with($car, $engine); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', false); + $config->setByReference(false); $config->setPropertyPath($propertyPath); $config->setData($engine); $form = $this->getForm($config); @@ -253,7 +253,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->with($car, $engine); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $config->setData($engine); $form = $this->getForm($config); @@ -277,7 +277,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->method('setValue'); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $config->setData($engine); $form = $this->getForm($config); @@ -295,7 +295,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->method('setValue'); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $config->setData($engine); $config->setMapped(false); @@ -313,7 +313,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->method('setValue'); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $config->setData(null); $form = $this->getForm($config); @@ -331,7 +331,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->method('setValue'); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $config->setData($engine); $form = $this->getForm($config, false); @@ -349,7 +349,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase ->method('setValue'); $config = new FormConfig('name', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($propertyPath); $config->setData($engine); $config->setDisabled(true); @@ -380,7 +380,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase $form = $this->getForm($config); $config = new FormConfig('engine', '\stdClass', $this->dispatcher); - $config->setAttribute('by_reference', true); + $config->setByReference(true); $config->setPropertyPath($childPath); $config->setData($engine); $child = $this->getForm($config); diff --git a/src/Symfony/Component/Form/Tests/FormTest.php b/src/Symfony/Component/Form/Tests/FormTest.php index 6a0cd9a38f..e0188d9085 100644 --- a/src/Symfony/Component/Form/Tests/FormTest.php +++ b/src/Symfony/Component/Form/Tests/FormTest.php @@ -464,6 +464,25 @@ class FormTest extends \PHPUnit_Framework_TestCase $this->form->setData(null); } + public function testSetDataClonesObjectIfNotByReference() + { + $data = new \stdClass(); + $form = $this->getBuilder('name', null, '\stdClass')->setByReference(false)->getForm(); + $form->setData($data); + + $this->assertNotSame($data, $form->getData()); + $this->assertEquals($data, $form->getData()); + } + + public function testSetDataDoesNotCloneObjectIfByReference() + { + $data = new \stdClass(); + $form = $this->getBuilder('name', null, '\stdClass')->setByReference(true)->getForm(); + $form->setData($data); + + $this->assertSame($data, $form->getData()); + } + public function testSetDataExecutesTransformationChain() { // use real event dispatcher now diff --git a/src/Symfony/Component/Form/UnmodifiableFormConfig.php b/src/Symfony/Component/Form/UnmodifiableFormConfig.php index ab925316c1..3fdb963923 100644 --- a/src/Symfony/Component/Form/UnmodifiableFormConfig.php +++ b/src/Symfony/Component/Form/UnmodifiableFormConfig.php @@ -42,6 +42,11 @@ class UnmodifiableFormConfig implements FormConfigInterface */ private $mapped; + /** + * @var Boolean + */ + private $byReference; + /** * @var Boolean */ @@ -123,6 +128,7 @@ class UnmodifiableFormConfig implements FormConfigInterface $this->name = $config->getName(); $this->propertyPath = $config->getPropertyPath(); $this->mapped = $config->getMapped(); + $this->byReference = $config->getByReference(); $this->virtual = $config->getVirtual(); $this->types = $config->getTypes(); $this->clientTransformers = $config->getClientTransformers(); @@ -170,6 +176,14 @@ class UnmodifiableFormConfig implements FormConfigInterface return $this->mapped; } + /** + * {@inheritdoc} + */ + public function getByReference() + { + return $this->byReference; + } + /** * {@inheritdoc} */