From 6ff41207843154c1f5546de54755e56bc9d9efd5 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Sun, 6 Feb 2011 18:04:20 +0100 Subject: [PATCH] [Form] Added Form option "by_reference" so that objects received from parent forms are modified by reference when this option is true (the default) The implication is that set() in the object of the parent form will not be called (and thus not has to be implemented/public). If you want to suppress this behaviour, manually set "by_reference" to false. --- src/Symfony/Component/Form/DateField.php | 13 ++ src/Symfony/Component/Form/DateTimeField.php | 13 ++ src/Symfony/Component/Form/Form.php | 16 +++ src/Symfony/Component/Form/TimeField.php | 13 ++ .../Symfony/Tests/Component/Form/FormTest.php | 118 ++++++++++++++++++ 5 files changed, 173 insertions(+) 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" *