From 00bc2708bce4f1c16a77c371de871c922c31079f Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 15 Aug 2013 21:04:26 +0200 Subject: [PATCH] [Form] Fixed: submit() reacts to dynamic modifications of the form children --- src/Symfony/Component/Form/Form.php | 9 +- .../Component/Form/Tests/AbstractFormTest.php | 3 + .../Component/Form/Tests/CompoundFormTest.php | 55 ++++++-- .../Util/VirtualFormAwareIteratorTest.php | 122 ++++++++++++++++++ .../Form/Util/VirtualFormAwareIterator.php | 74 ++++++++++- 5 files changed, 242 insertions(+), 21 deletions(-) create mode 100644 src/Symfony/Component/Form/Tests/Util/VirtualFormAwareIteratorTest.php diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 55d80c2f9f..2cf0c9dff8 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -552,7 +552,12 @@ class Form implements \IteratorAggregate, FormInterface $submittedData = array(); } - foreach ($this->children as $name => $child) { + reset($this->children); + + for (reset($this->children); false !== current($this->children); next($this->children)) { + $child = current($this->children); + $name = key($this->children); + $child->bind(isset($submittedData[$name]) ? $submittedData[$name] : null); unset($submittedData[$name]); } @@ -829,7 +834,7 @@ class Form implements \IteratorAggregate, FormInterface /** * {@inheritdoc} */ - public function all() + public function &all() { return $this->children; } diff --git a/src/Symfony/Component/Form/Tests/AbstractFormTest.php b/src/Symfony/Component/Form/Tests/AbstractFormTest.php index 5e13863aa9..46be43eb0d 100644 --- a/src/Symfony/Component/Form/Tests/AbstractFormTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractFormTest.php @@ -81,6 +81,9 @@ abstract class AbstractFormTest extends \PHPUnit_Framework_TestCase $form->expects($this->any()) ->method('getName') ->will($this->returnValue($name)); + $form->expects($this->any()) + ->method('getConfig') + ->will($this->returnValue($this->getMock('Symfony\Component\Form\FormConfigInterface'))); return $form; } diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php index ca97c8ec2a..d719133da1 100644 --- a/src/Symfony/Component/Form/Tests/CompoundFormTest.php +++ b/src/Symfony/Component/Form/Tests/CompoundFormTest.php @@ -11,7 +11,6 @@ namespace Symfony\Component\Form\Tests; -use Symfony\Component\Form\Form; use Symfony\Component\Form\FormError; use Symfony\Component\Form\Extension\HttpFoundation\EventListener\BindRequestListener; use Symfony\Component\HttpFoundation\Request; @@ -46,19 +45,6 @@ class CompoundFormTest extends AbstractFormTest $this->assertFalse($this->form->isValid()); } - public function testBindForwardsNullIfValueIsMissing() - { - $child = $this->getMockForm('firstName'); - - $this->form->add($child); - - $child->expects($this->once()) - ->method('bind') - ->with($this->equalTo(null)); - - $this->form->bind(array()); - } - public function testCloneChildren() { $child = $this->getBuilder('child')->getForm(); @@ -322,6 +308,47 @@ class CompoundFormTest extends AbstractFormTest $form->setData('foo'); } + public function testBindForwardsNullIfValueIsMissing() + { + $child = $this->getMockForm('firstName'); + + $this->form->add($child); + + $child->expects($this->once()) + ->method('bind') + ->with($this->equalTo(null)); + + $this->form->bind(array()); + } + + public function testBindSupportsDynamicAdditionAndRemovalOfChildren() + { + $child = $this->getMockForm('child'); + $childToBeRemoved = $this->getMockForm('removed'); + $childToBeAdded = $this->getMockForm('added'); + + $this->form->add($child); + $this->form->add($childToBeRemoved); + + $form = $this->form; + + $child->expects($this->once()) + ->method('bind') + ->will($this->returnCallback(function () use ($form, $childToBeAdded) { + $form->remove('removed'); + $form->add($childToBeAdded); + })); + + $childToBeRemoved->expects($this->never()) + ->method('bind'); + + $childToBeAdded->expects($this->once()) + ->method('bind'); + + // pass NULL to all children + $this->form->bind(array()); + } + public function testBindMapsBoundChildrenOntoExistingViewData() { $test = $this; diff --git a/src/Symfony/Component/Form/Tests/Util/VirtualFormAwareIteratorTest.php b/src/Symfony/Component/Form/Tests/Util/VirtualFormAwareIteratorTest.php new file mode 100644 index 0000000000..805acad903 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Util/VirtualFormAwareIteratorTest.php @@ -0,0 +1,122 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Util; + +use Symfony\Component\Form\Util\VirtualFormAwareIterator; + +/** + * @author Bernhard Schussek + */ +class VirtualFormAwareIteratorTest extends \PHPUnit_Framework_TestCase +{ + public function testSupportDynamicModification() + { + $form = $this->getMockForm('form'); + $formToBeAdded = $this->getMockForm('added'); + $formToBeRemoved = $this->getMockForm('removed'); + + $forms = array('form' => $form, 'removed' => $formToBeRemoved); + $iterator = new VirtualFormAwareIterator($forms); + + $iterator->rewind(); + $this->assertTrue($iterator->valid()); + $this->assertSame('form', $iterator->key()); + $this->assertSame($form, $iterator->current()); + + // dynamic modification + unset($forms['removed']); + $forms['added'] = $formToBeAdded; + + // continue iteration + $iterator->next(); + $this->assertTrue($iterator->valid()); + $this->assertSame('added', $iterator->key()); + $this->assertSame($formToBeAdded, $iterator->current()); + + // end of array + $iterator->next(); + $this->assertFalse($iterator->valid()); + } + + public function testSupportDynamicModificationInRecursiveCall() + { + $virtualForm = $this->getMockForm('virtual'); + $form = $this->getMockForm('form'); + $formToBeAdded = $this->getMockForm('added'); + $formToBeRemoved = $this->getMockForm('removed'); + + $virtualForm->getConfig()->expects($this->any()) + ->method('getVirtual') + ->will($this->returnValue(true)); + + $virtualForm->add($form); + $virtualForm->add($formToBeRemoved); + + $forms = array('virtual' => $virtualForm); + $iterator = new VirtualFormAwareIterator($forms); + + $iterator->rewind(); + $this->assertTrue($iterator->valid()); + $this->assertSame('virtual', $iterator->key()); + $this->assertSame($virtualForm, $iterator->current()); + $this->assertTrue($iterator->hasChildren()); + + // enter nested iterator + $nestedIterator = $iterator->getChildren(); + $this->assertSame('form', $nestedIterator->key()); + $this->assertSame($form, $nestedIterator->current()); + $this->assertFalse($nestedIterator->hasChildren()); + + // dynamic modification + $virtualForm->remove('removed'); + $virtualForm->add($formToBeAdded); + + // continue iteration - nested iterator discovers change in the form + $nestedIterator->next(); + $this->assertTrue($nestedIterator->valid()); + $this->assertSame('added', $nestedIterator->key()); + $this->assertSame($formToBeAdded, $nestedIterator->current()); + + // end of array + $nestedIterator->next(); + $this->assertFalse($nestedIterator->valid()); + } + + /** + * @param string $name + * + * @return \PHPUnit_Framework_MockObject_MockObject + */ + protected function getMockForm($name = 'name') + { + $config = $this->getMock('Symfony\Component\Form\FormConfigInterface'); + + $config->expects($this->any()) + ->method('getName') + ->will($this->returnValue($name)); + $config->expects($this->any()) + ->method('getCompound') + ->will($this->returnValue(true)); + $config->expects($this->any()) + ->method('getDataMapper') + ->will($this->returnValue($this->getMock('Symfony\Component\Form\DataMapperInterface'))); + $config->expects($this->any()) + ->method('getEventDispatcher') + ->will($this->returnValue($this->getMock('Symfony\Component\EventDispatcher\EventDispatcher'))); + + return $this->getMockBuilder('Symfony\Component\Form\Form') + ->setConstructorArgs(array($config)) + ->disableArgumentCloning() + ->setMethods(array('getViewData')) + ->getMock(); + } +} diff --git a/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php b/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php index c1322bb35d..9b9abb36e7 100644 --- a/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php +++ b/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php @@ -12,22 +12,86 @@ namespace Symfony\Component\Form\Util; /** - * Iterator that traverses fields of a field group + * Iterator that traverses an array of forms. * - * If the iterator encounters a virtual field group, it enters the field - * group and traverses its children as well. + * Contrary to \ArrayIterator, this iterator recognizes changes in the original + * array during iteration. + * + * You can wrap the iterator into a {@link \RecursiveIterator} in order to + * enter any virtual child form and iterate the children of that virtual form. * * @author Bernhard Schussek */ -class VirtualFormAwareIterator extends \ArrayIterator implements \RecursiveIterator +class VirtualFormAwareIterator implements \RecursiveIterator { + /** + * @var \Symfony\Component\Form\FormInterface[] + */ + private $forms; + + /** + * Creates a new iterator. + * + * @param \Symfony\Component\Form\FormInterface[] $forms An array of forms + */ + public function __construct(array &$forms) + { + $this->forms = &$forms; + } + + /** + *{@inheritdoc} + */ + public function current() + { + return current($this->forms); + } + + /** + *{@inheritdoc} + */ + public function next() + { + next($this->forms); + } + + /** + *{@inheritdoc} + */ + public function key() + { + return key($this->forms); + } + + /** + *{@inheritdoc} + */ + public function valid() + { + return null !== key($this->forms); + } + + /** + *{@inheritdoc} + */ + public function rewind() + { + reset($this->forms); + } + + /** + *{@inheritdoc} + */ public function getChildren() { return new self($this->current()->all()); } + /** + *{@inheritdoc} + */ public function hasChildren() { - return $this->current()->getConfig()->getVirtual(); + return (bool) $this->current()->getConfig()->getVirtual(); } }