From e893206c8589d6948b90dc90ab0918148dfe52e6 Mon Sep 17 00:00:00 2001 From: Bart van den Burg Date: Tue, 3 Apr 2012 21:00:45 +0200 Subject: [PATCH 1/3] [Form] added ChildDataEvent --- .../Component/Form/Event/ChildDataEvent.php | 48 +++++++++++++++++++ src/Symfony/Component/Form/Form.php | 23 ++++++--- src/Symfony/Component/Form/FormEvents.php | 2 + src/Symfony/Component/Form/Tests/FormTest.php | 35 ++++++++++++++ 4 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 src/Symfony/Component/Form/Event/ChildDataEvent.php diff --git a/src/Symfony/Component/Form/Event/ChildDataEvent.php b/src/Symfony/Component/Form/Event/ChildDataEvent.php new file mode 100644 index 0000000000000..651f7b62befaa --- /dev/null +++ b/src/Symfony/Component/Form/Event/ChildDataEvent.php @@ -0,0 +1,48 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Event; + +use Symfony\Component\Form\FormInterface; + +class ChildDataEvent extends DataEvent +{ + private $name; + + /** + * Constructs an event. + * + * @param FormInterface $form The associated form + * @param string $name The name of the field that was just set + * @param mixed $data The data of the field that was just set + */ + public function __construct(FormInterface $form, $name, $data) + { + $this->name = $name; + parent::__construct($form, $data); + } + + /** + * Gets the data of the field that was just set + */ + public function getData() + { + return parent::getData(); + } + + /** + * Gets the name of the field that was updated + */ + public function getName() + { + return $this->name; + } +} diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 0e68b88c01edf..ac823a47a0c1a 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -13,6 +13,7 @@ use Symfony\Component\Form\Event\DataEvent; use Symfony\Component\Form\Event\FilterDataEvent; +use Symfony\Component\Form\Event\ChildDataEvent; use Symfony\Component\Form\Exception\FormException; use Symfony\Component\Form\Exception\AlreadyBoundException; use Symfony\Component\Form\Exception\UnexpectedTypeException; @@ -506,13 +507,23 @@ public function bind($clientData) } } - foreach ($clientData as $name => $value) { - if ($this->has($name)) { - $this->children[$name]->bind($value); - } else { - $extraData[$name] = $value; + $extraData = $clientData; + $lastCount = count($this); + do { + foreach ($extraData as $name => $value) { + if ($this->has($name)) { + $this->children[$name]->bind($value); + unset($extraData[$name]); + + // every time we bind a child, we dispatch an event to allow + // listeners to add or remove field based on the result value + $event = new ChildDataEvent($this, $name, $this->children[$name]->getData()); + $this->dispatcher->dispatch(FormEvents::BIND_CHILD, $event); + } } - } + } while(count($extraData) && $lastCount != ($lastCount = count($this))); + + $clientData = array_diff_key($clientData, $extraData); // If we have a data mapper, use old client data and merge // data from the children into it later diff --git a/src/Symfony/Component/Form/FormEvents.php b/src/Symfony/Component/Form/FormEvents.php index a97337ec5a187..a30f66ab75623 100644 --- a/src/Symfony/Component/Form/FormEvents.php +++ b/src/Symfony/Component/Form/FormEvents.php @@ -26,6 +26,8 @@ final class FormEvents const BIND_CLIENT_DATA = 'form.bind_client_data'; + const BIND_CHILD = 'form.bind_child'; + const BIND_NORM_DATA = 'form.bind_norm_data'; const SET_DATA = 'form.set_data'; diff --git a/src/Symfony/Component/Form/Tests/FormTest.php b/src/Symfony/Component/Form/Tests/FormTest.php index d6784f4e52393..95006d86bf620 100644 --- a/src/Symfony/Component/Form/Tests/FormTest.php +++ b/src/Symfony/Component/Form/Tests/FormTest.php @@ -22,6 +22,8 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\Form\Tests\Fixtures\FixedDataTransformer; use Symfony\Component\Form\Tests\Fixtures\FixedFilterListener; +use Symfony\Component\Form\Event\ChildDataEvent; +use Symfony\Component\Form\FormEvents; class FormTest extends \PHPUnit_Framework_TestCase { @@ -1303,6 +1305,39 @@ public function testGetValidatorsReturnsValidators() $this->assertEquals(array($validator), $form->getValidators()); } + + public function testBindChild() + { + $field1builder = $this->getBuilder('field1'); + $field2builder = $this->getBuilder('field2'); + + $builder = $this->getBuilder('form', new EventDispatcher()) + ->addEventListener(FormEvents::BIND_CHILD, function(ChildDataEvent $e) use ($field2builder) { + if ($e->getName() == 'field1') { + if ($e->getData() == 'a') { + $e->getForm()->add($field2builder->getForm()); + } else { + $e->getForm()->remove('field2'); + } + } + }); + $form = $builder->getForm(); + $form->add($field1builder->getForm()); + + $form->bind(array('field1'=>'a', 'field2'=>'b')); + $this->assertTrue($form->has('field2')); + $this->assertEquals(array('field1'=>'a', 'field2'=>'b'), $form->getData()); + $this->assertEmpty($form->getExtraData()); + + $form = $builder->getForm(); + $form->add($field1builder->getForm()); + $form->add($field2builder->getForm()); + + $form->bind(array('field1'=>'b', 'field2'=>'a')); + $this->assertFalse($form->has('field2')); + $this->assertEquals(array('field1'=>'b'), $form->getData()); + $this->assertEquals(array('field2'=>'a'), $form->getExtraData()); + } protected function getBuilder($name = 'name', EventDispatcherInterface $dispatcher = null) { From 143bf992ec537d25e709496849b5c9069e9a4a39 Mon Sep 17 00:00:00 2001 From: Bart van den Burg Date: Tue, 3 Apr 2012 21:44:55 +0200 Subject: [PATCH 2/3] prevent infinite loop bug + tests --- src/Symfony/Component/Form/Form.php | 4 +-- src/Symfony/Component/Form/Tests/FormTest.php | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index ac823a47a0c1a..912e95515c6b7 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -508,8 +508,8 @@ public function bind($clientData) } $extraData = $clientData; - $lastCount = count($this); do { + $lastChildren = $this->children; foreach ($extraData as $name => $value) { if ($this->has($name)) { $this->children[$name]->bind($value); @@ -521,7 +521,7 @@ public function bind($clientData) $this->dispatcher->dispatch(FormEvents::BIND_CHILD, $event); } } - } while(count($extraData) && $lastCount != ($lastCount = count($this))); + } while(count($extraData) && $lastChildren !== $this->children); $clientData = array_diff_key($clientData, $extraData); diff --git a/src/Symfony/Component/Form/Tests/FormTest.php b/src/Symfony/Component/Form/Tests/FormTest.php index 95006d86bf620..dd70a1ca54984 100644 --- a/src/Symfony/Component/Form/Tests/FormTest.php +++ b/src/Symfony/Component/Form/Tests/FormTest.php @@ -1339,6 +1339,41 @@ public function testBindChild() $this->assertEquals(array('field2'=>'a'), $form->getExtraData()); } + public function testBindChildDoesntResultInEndlessLoop() + { + $field1builder = $this->getBuilder('field1'); + + $builder = $this->getBuilder('form'); + + $form = $builder->getForm(); + $form->add($field1builder->getForm()); + + $form->bind(array('field1'=>'a', 'field2'=>'b')); + } + + public function testBindChildKeepsLoopingUntilNoFieldsAreAdded() + { + $field2builder = $this->getBuilder('field2'); + $field3builder = $this->getBuilder('field3'); + + $builder = $this->getBuilder('form', new EventDispatcher()) + ->addEventListener(FormEvents::BIND_CHILD, function(ChildDataEvent $e) use ($field2builder, $field3builder) { + if ($e->getName() == 'field1') { + $e->getForm()->add($field2builder->getForm()); + } elseif ($e->getName() == 'field2') { + $e->getForm()->add($field3builder->getForm()); + } + }); + + $form = $builder->getForm(); + + $form->add($this->getBuilder('field1')->getForm()); + + $form->bind(array('field1'=>'a', 'field2'=>'b', 'field3'=>'c', 'field4'=>'d')); + $this->assertEquals(array('field1'=>'a', 'field2'=>'b', 'field3'=>'c'), $form->getData()); + $this->assertEquals(array('field4'=>'d'), $form->getExtraData()); + } + protected function getBuilder($name = 'name', EventDispatcherInterface $dispatcher = null) { return new FormBuilder($name, $this->factory, $dispatcher ?: $this->dispatcher); From 94b4d4975fc6f9665a35971dda783fd54180c342 Mon Sep 17 00:00:00 2001 From: Bart van den Burg Date: Tue, 3 Apr 2012 22:17:35 +0200 Subject: [PATCH 3/3] remove array_diff_key as it is a BC-break --- src/Symfony/Component/Form/Form.php | 2 -- src/Symfony/Component/Form/Tests/FormTest.php | 16 ++++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 912e95515c6b7..f886fff7043e3 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -523,8 +523,6 @@ public function bind($clientData) } } while(count($extraData) && $lastChildren !== $this->children); - $clientData = array_diff_key($clientData, $extraData); - // If we have a data mapper, use old client data and merge // data from the children into it later if ($this->dataMapper) { diff --git a/src/Symfony/Component/Form/Tests/FormTest.php b/src/Symfony/Component/Form/Tests/FormTest.php index dd70a1ca54984..e47e15318f430 100644 --- a/src/Symfony/Component/Form/Tests/FormTest.php +++ b/src/Symfony/Component/Form/Tests/FormTest.php @@ -1326,7 +1326,6 @@ public function testBindChild() $form->bind(array('field1'=>'a', 'field2'=>'b')); $this->assertTrue($form->has('field2')); - $this->assertEquals(array('field1'=>'a', 'field2'=>'b'), $form->getData()); $this->assertEmpty($form->getExtraData()); $form = $builder->getForm(); @@ -1335,7 +1334,6 @@ public function testBindChild() $form->bind(array('field1'=>'b', 'field2'=>'a')); $this->assertFalse($form->has('field2')); - $this->assertEquals(array('field1'=>'b'), $form->getData()); $this->assertEquals(array('field2'=>'a'), $form->getExtraData()); } @@ -1353,8 +1351,11 @@ public function testBindChildDoesntResultInEndlessLoop() public function testBindChildKeepsLoopingUntilNoFieldsAreAdded() { - $field2builder = $this->getBuilder('field2'); - $field3builder = $this->getBuilder('field3'); + $transformer = $this->getDataTransformer(); + $transformer->expects($this->exactly(3))->method('reverseTransform')->will($this->onConsecutiveCalls(array('abound', 'bbound', 'cbound'))); + + $field2builder = $this->getBuilder('field2')->appendClientTransformer($transformer); + $field3builder = $this->getBuilder('field3')->appendClientTransformer($transformer); $builder = $this->getBuilder('form', new EventDispatcher()) ->addEventListener(FormEvents::BIND_CHILD, function(ChildDataEvent $e) use ($field2builder, $field3builder) { @@ -1367,10 +1368,13 @@ public function testBindChildKeepsLoopingUntilNoFieldsAreAdded() $form = $builder->getForm(); - $form->add($this->getBuilder('field1')->getForm()); + $form->add($this->getBuilder('field1')->appendClientTransformer($transformer)->getForm()); $form->bind(array('field1'=>'a', 'field2'=>'b', 'field3'=>'c', 'field4'=>'d')); - $this->assertEquals(array('field1'=>'a', 'field2'=>'b', 'field3'=>'c'), $form->getData()); + + // issue #3770 + //$this->assertEquals(array('field1'=>'abound', 'field2'=>'bbound', 'field3'=>'cbound', 'field4'=>'d'), $form->getData()); + $this->assertEquals(array('field1'=>'a', 'field2'=>'b', 'field3'=>'c', 'field4'=>'d'), $form->getData()); $this->assertEquals(array('field4'=>'d'), $form->getExtraData()); }