[Form] Improved test coverage of ResizeFormListener. Fixed https://github.com/symfony/symfony/pull/493

src/Symfony/Component/Form/EventListener/ResizeFormListener.php

@@ -60,23 +60,25 @@ class ResizeFormListener implements EventSubscriberInterface
     public function preSetData(DataEvent $event)
     {
         $form = $event->getForm();
-        $collection = $event->getData();
+        $data = $event->getData();
 
-        if (null === $collection) {
-            $collection = array();
+        if (null === $data) {
+            $data = array();
         }
 
-        if (!is_array($collection) && !$collection instanceof \Traversable) {
-            throw new UnexpectedTypeException($collection, 'array or \Traversable');
+        if (!is_array($data) && !$data instanceof \Traversable) {
+            throw new UnexpectedTypeException($data, 'array or \Traversable');
         }
 
+        // First remove all rows except for the prototype row
         foreach ($form as $name => $child) {
-            if (!$this->resizeOnBind || '$$name$$' != $name) {
+            if (!($this->resizeOnBind && '$$name$$' === $name)) {
                 $form->remove($name);
             }
         }
 
-        foreach ($collection as $name => $value) {
+        // Then add all rows again in the correct order
+        foreach ($data as $name => $value) {
             $form->add($this->factory->create($this->type, $name, array(
                 'property_path' => '['.$name.']',
             )));
@@ -96,12 +98,18 @@ class ResizeFormListener implements EventSubscriberInterface
             $data = array();
         }
 
+        if (!is_array($data) && !$data instanceof \Traversable) {
+            throw new UnexpectedTypeException($data, 'array or \Traversable');
+        }
+
+        // Remove all empty rows except for the prototype row
         foreach ($form as $name => $child) {
-            if (!isset($data[$name]) && '$$name$$' != $name) {
+            if (!isset($data[$name]) && '$$name$$' !== $name) {
                 $form->remove($name);
             }
         }
 
+        // Add all additional rows
         foreach ($data as $name => $value) {
             if (!$form->has($name)) {
                 $form->add($this->factory->create($this->type, $name, array(
@@ -118,14 +126,22 @@ class ResizeFormListener implements EventSubscriberInterface
         }
 
         $form = $event->getForm();
-        $collection = $event->getData();
+        $data = $event->getData();
+
+        if (null === $data) {
+            $data = array();
+        }
+
+        if (!is_array($data) && !$data instanceof \Traversable) {
+            throw new UnexpectedTypeException($data, 'array or \Traversable');
+        }
 
-        foreach ($collection as $name => $child) {
+        foreach ($data as $name => $child) {
             if (!$form->has($name)) {
-                unset($collection[$name]);
+                unset($data[$name]);
             }
         }
 
-        $event->setData($collection);
+        $event->setData($data);
     }
 }

src/Symfony/Component/Form/FormInterface.php

@@ -32,6 +32,12 @@ interface FormInterface extends \ArrayAccess, \Traversable, \Countable
      */
     function getParent();
 
+    function add(FormInterface $child);
+
+    function has($name);
+
+    function remove($name);
+
     function getChildren();
 
     function hasChildren();

tests/Symfony/Tests/Component/Form/EventListener/ResizeFormListenerTest.php

@@ -11,9 +11,10 @@
 
 namespace Symfony\Tests\Component\Form\EventListener;
 
+use Symfony\Component\Form\Event\DataEvent;
 use Symfony\Component\Form\Event\FilterDataEvent;
 use Symfony\Component\Form\EventListener\ResizeFormListener;
-use Symfony\Component\Form\Event\DataEvent;
+use Symfony\Component\Form\FormBuilder;
 
 class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 {
@@ -22,63 +23,210 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 
     public function setUp()
     {
+        $this->dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
         $this->factory = $this->getMock('Symfony\Component\Form\FormFactoryInterface');
-        $this->form = $this->getMock('Symfony\Component\Form\Form', array('add', 'has'), array(), '', false);
+        $this->form = $this->getForm();
+    }
+
+    protected function getBuilder($name = 'name')
+    {
+        return new FormBuilder($name, $this->factory, $this->dispatcher);
     }
 
-    public function testResizePreSetData()
+    protected function getForm($name = 'name')
     {
-        $expectedType = "text";
+        return $this->getBuilder($name)->getForm();
+    }
+
+    protected function getMockForm()
+    {
+        return $this->getMock('Symfony\Tests\Component\Form\FormInterface');
+    }
+
+    public function testPreSetDataResizesForm()
+    {
+        $this->form->add($this->getForm('0'));
+        $this->form->add($this->getForm('1'));
 
         $this->factory->expects($this->at(0))
-                      ->method('create')
-                      ->with($this->equalTo($expectedType), $this->equalTo( 0 ), array('property_path' => '[0]'))
-                      ->will($this->returnValue($this->getMock('Symfony\Tests\Component\Form\FormInterface')));
+            ->method('create')
+            ->with('text', 1, array('property_path' => '[1]'))
+            ->will($this->returnValue($this->getForm('1')));
         $this->factory->expects($this->at(1))
-                      ->method('create')
-                      ->with($this->equalTo($expectedType), $this->equalTo( 1 ), array('property_path' => '[1]'))
-                      ->will($this->returnValue($this->getMock('Symfony\Tests\Component\Form\FormInterface')));
+            ->method('create')
+            ->with('text', 2, array('property_path' => '[2]'))
+            ->will($this->returnValue($this->getForm('2')));
 
-        $data = array("string", "string");
+        $data = array(1 => 'string', 2 => 'string');
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, $expectedType, false);
+        $listener = new ResizeFormListener($this->factory, 'text', false);
         $listener->preSetData($event);
+
+        $this->assertFalse($this->form->has('0'));
+        $this->assertTrue($this->form->has('1'));
+        $this->assertTrue($this->form->has('2'));
     }
 
-    public function testResizePreSetDataNoArrayThrowsException()
+    public function testPreSetDataRemovesPrototypeRowIfNotResizeOnBind()
     {
-        $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
-        
-        $data = "no array or traversable";
+        $this->form->add($this->getForm('$$name$$'));
+
+        $data = array();
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, "text", false);
+        $listener = new ResizeFormListener($this->factory, 'text', false);
         $listener->preSetData($event);
+
+        $this->assertFalse($this->form->has('$$name$$'));
     }
 
-    public function testResizePreSetDataNull()
+    public function testPreSetDataDoesNotRemovePrototypeRowIfResizeOnBind()
+    {
+        $this->form->add($this->getForm('$$name$$'));
+
+        $data = array();
+        $event = new DataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener->preSetData($event);
+
+        $this->assertTrue($this->form->has('$$name$$'));
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
+     */
+    public function testPreSetDataRequiresArrayOrTraversable()
+    {
+        $data = 'no array or traversable';
+        $event = new DataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener->preSetData($event);
+    }
+
+    public function testPreSetDataDealsWithNullData()
     {
         $this->factory->expects($this->never())->method('create');
 
         $data = null;
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, "text", false);
+        $listener = new ResizeFormListener($this->factory, 'text', false);
         $listener->preSetData($event);
     }
 
-    public function testPreBind()
+    public function testPreBindResizesFormIfResizable()
     {
-        $expectedType = "text";
+        $this->form->add($this->getForm('0'));
+        $this->form->add($this->getForm('1'));
 
-        $this->form->expects($this->once())->method('has')->with($this->equalTo('foo'))->will($this->returnValue( false ));
-        $this->form->expects($this->once())->method('add')->with($this->isInstanceOf('Symfony\Tests\Component\Form\FormInterface'));
-        $this->factory->expects($this->at(0))
-                      ->method('create')
-                      ->with($this->equalTo($expectedType), $this->equalTo('foo'), $this->equalTo(array('property_path' => '[foo]')))
-                      ->will($this->returnValue( $this->getMock('Symfony\Tests\Component\Form\FormInterface') ));
+        $this->factory->expects($this->once())
+            ->method('create')
+            ->with('text', 2, array('property_path' => '[2]'))
+            ->will($this->returnValue($this->getForm('2')));
+
+        $data = array(0 => 'string', 2 => 'string');
+        $event = new DataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener->preBind($event);
+
+        $this->assertTrue($this->form->has('0'));
+        $this->assertFalse($this->form->has('1'));
+        $this->assertTrue($this->form->has('2'));
+    }
+
+    // fix for https://github.com/symfony/symfony/pull/493
+    public function testPreBindRemovesZeroKeys()
+    {
+        $this->form->add($this->getForm('0'));
+
+        $data = array();
+        $event = new DataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener->preBind($event);
 
-        $data = array("foo" => "bar");
+        $this->assertFalse($this->form->has('0'));
+    }
+
+    public function testPreBindDoesNothingIfNotResizable()
+    {
+        $this->form->add($this->getForm('0'));
+        $this->form->add($this->getForm('1'));
+
+        $data = array(0 => 'string', 2 => 'string');
+        $event = new DataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener->preBind($event);
+
+        $this->assertTrue($this->form->has('0'));
+        $this->assertTrue($this->form->has('1'));
+        $this->assertFalse($this->form->has('2'));
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
+     */
+    public function testPreBindRequiresArrayOrTraversable()
+    {
+        $data = 'no array or traversable';
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, "text", true);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
         $listener->preBind($event);
     }
+
+    public function testPreBindDealsWithNullData()
+    {
+        $this->form->add($this->getForm('1'));
+
+        $data = null;
+        $event = new DataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener->preBind($event);
+
+        $this->assertFalse($this->form->has('1'));
+    }
+
+    public function testOnBindNormDataRemovesEntriesMissingInTheFormIfResizable()
+    {
+        $this->form->add($this->getForm('1'));
+
+        $data = array(0 => 'first', 1 => 'second', 2 => 'third');
+        $event = new FilterDataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener->onBindNormData($event);
+
+        $this->assertEquals(array(1 => 'second'), $event->getData());
+    }
+
+    public function testOnBindNormDataDoesNothingIfNotResizable()
+    {
+        $this->form->add($this->getForm('1'));
+
+        $data = array(0 => 'first', 1 => 'second', 2 => 'third');
+        $event = new FilterDataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener->onBindNormData($event);
+
+        $this->assertEquals($data, $event->getData());
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
+     */
+    public function testOnBindNormDataRequiresArrayOrTraversable()
+    {
+        $data = 'no array or traversable';
+        $event = new FilterDataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener->onBindNormData($event);
+    }
+
+    public function testOnBindNormDataDealsWithNullData()
+    {
+        $this->form->add($this->getForm('1'));
+
+        $data = null;
+        $event = new FilterDataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener->onBindNormData($event);
+
+        $this->assertEquals(array(), $event->getData());
+    }
 }