[Form] Fixed: submit() reacts to dynamic modifications of the form children

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;
     }

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;
     }

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;

src/Symfony/Component/Form/Tests/Util/VirtualFormAwareIteratorTest.php

@@ -0,0 +1,122 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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 <bschussek@gmail.com>
+ */
+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();
+    }
+}

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 <bschussek@gmail.com>
  */
-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();
     }
 }