diff --git a/src/Symfony/Component/Form/DateField.php b/src/Symfony/Component/Form/DateField.php index 3330bad81f..f08c4733e9 100644 --- a/src/Symfony/Component/Form/DateField.php +++ b/src/Symfony/Component/Form/DateField.php @@ -89,6 +89,19 @@ class DateField extends HybridField */ protected $formatter; + /** + * {@inheritDoc} + */ + public function __construct($key, array $options = array()) + { + // Override parent option + // \DateTime objects are never edited by reference, because + // we treat them like value objects + $this->addOption('by_reference', false); + + parent::__construct($key, $options); + } + protected function configure() { $this->addOption('widget', self::CHOICE, self::$widgets); diff --git a/src/Symfony/Component/Form/DateTimeField.php b/src/Symfony/Component/Form/DateTimeField.php index 3002b7f2bf..5f9d96ed8d 100644 --- a/src/Symfony/Component/Form/DateTimeField.php +++ b/src/Symfony/Component/Form/DateTimeField.php @@ -75,6 +75,19 @@ class DateTimeField extends Form TimeField::INPUT, ); + /** + * {@inheritDoc} + */ + public function __construct($key, array $options = array()) + { + // Override parent option + // \DateTime objects are never edited by reference, because + // we treat them like value objects + $this->addOption('by_reference', false); + + parent::__construct($key, $options); + } + protected function configure() { $this->addOption('date_widget', DateField::CHOICE, self::$dateWidgets); diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 820772bd1b..6b0886bf82 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -93,6 +93,7 @@ class Form extends Field implements \IteratorAggregate, FormInterface $this->addOption('virtual', false); $this->addOption('validator'); $this->addOption('context'); + $this->addOption('by_reference', true); if (isset($options['validation_groups'])) { $options['validation_groups'] = (array)$options['validation_groups']; @@ -880,6 +881,21 @@ class Form extends Field implements \IteratorAggregate, FormInterface } } + /** + * {@inheritDoc} + */ + public function writeProperty(&$objectOrArray) + { + $data = $this->getData(); + + // Don't update parent if data is a composite type (object or array) + // and "by_reference" option is true, because then we expect that + // we are working with a reference to the parent's data + if (!(is_object($data) || is_array($data)) || !$this->getOption('by_reference')) { + parent::writeProperty($objectOrArray); + } + } + /** * Merges two arrays without reindexing numeric keys. * diff --git a/src/Symfony/Component/Form/TimeField.php b/src/Symfony/Component/Form/TimeField.php index 3f221db2b4..a8961fe7aa 100644 --- a/src/Symfony/Component/Form/TimeField.php +++ b/src/Symfony/Component/Form/TimeField.php @@ -59,6 +59,19 @@ class TimeField extends Form self::RAW, ); + /** + * {@inheritDoc} + */ + public function __construct($key, array $options = array()) + { + // Override parent option + // \DateTime objects are never edited by reference, because + // we treat them like value objects + $this->addOption('by_reference', false); + + parent::__construct($key, $options); + } + /** * {@inheritDoc} */ diff --git a/tests/Symfony/Tests/Component/Form/FormTest.php b/tests/Symfony/Tests/Component/Form/FormTest.php index 128fd50f64..e23cc3212d 100644 --- a/tests/Symfony/Tests/Component/Form/FormTest.php +++ b/tests/Symfony/Tests/Component/Form/FormTest.php @@ -41,6 +41,59 @@ class FormTest_PreconfiguredForm extends Form } } +// behaves like a form with a value transformer that transforms into +// a specific format +class FormTest_FormThatReturns extends Form +{ + protected $returnValue; + + public function setReturnValue($returnValue) + { + $this->returnValue = $returnValue; + } + + public function setData($data) + { + } + + public function getData() + { + return $this->returnValue; + } +} + +class FormTest_AuthorWithoutRefSetter +{ + protected $reference; + + protected $referenceCopy; + + public function __construct($reference) + { + $this->reference = $reference; + $this->referenceCopy = $reference; + } + + // The returned object should be modified by reference without having + // to provide a setReference() method + public function getReference() + { + return $this->reference; + } + + // The returned object is a copy, so setReferenceCopy() must be used + // to update it + public function getReferenceCopy() + { + return is_object($this->referenceCopy) ? clone $this->referenceCopy : $this->referenceCopy; + } + + public function setReferenceCopy($reference) + { + $this->referenceCopy = $reference; + } +} + class TestSetDataBeforeConfigureForm extends Form { protected $testCase; @@ -1130,6 +1183,71 @@ class FormTest extends \PHPUnit_Framework_TestCase $form->validateData($context); } + public function testSubformDoesntCallSetters() + { + $author = new FormTest_AuthorWithoutRefSetter(new Author()); + + $form = new Form('author', array('validator' => $this->createMockValidator())); + $form->setData($author); + $refForm = new Form('reference'); + $refForm->add(new TestField('firstName')); + $form->add($refForm); + + $form->bind($this->createPostRequest(array( + 'author' => array( + // reference has a getter, but not setter + 'reference' => array( + 'firstName' => 'Foo', + ) + ) + ))); + + $this->assertEquals('Foo', $author->getReference()->firstName); + } + + public function testSubformCallsSettersIfByReferenceIsFalse() + { + $author = new FormTest_AuthorWithoutRefSetter(new Author()); + + $form = new Form('author', array('validator' => $this->createMockValidator())); + $form->setData($author); + $refForm = new Form('referenceCopy', array('by_reference' => false)); + $refForm->add(new TestField('firstName')); + $form->add($refForm); + + $form->bind($this->createPostRequest(array( + 'author' => array( + // referenceCopy has a getter that returns a copy + 'referenceCopy' => array( + 'firstName' => 'Foo', + ) + ) + ))); + + // firstName can only be updated if setReferenceCopy() was called + $this->assertEquals('Foo', $author->getReferenceCopy()->firstName); + } + + public function testSubformCallsSettersIfReferenceIsScalar() + { + $author = new FormTest_AuthorWithoutRefSetter('scalar'); + + $form = new Form('author', array('validator' => $this->createMockValidator())); + $form->setData($author); + $refForm = new FormTest_FormThatReturns('referenceCopy'); + $refForm->setReturnValue('foobar'); + $form->add($refForm); + + $form->bind($this->createPostRequest(array( + 'author' => array( + 'referenceCopy' => array(), // doesn't matter actually + ) + ))); + + // firstName can only be updated if setReferenceCopy() was called + $this->assertEquals('foobar', $author->getReferenceCopy()); + } + /** * Create a group containing two fields, "visibleField" and "hiddenField" *