[Form] Improved test coverage of Form class

src/Symfony/Component/Form/Form.php

@@ -75,39 +75,132 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 class Form implements \IteratorAggregate, FormInterface
 {
     /**
-     * Contains all the children of this group
+     * The name of this form
+     * @var string
+     */
+    private $name;
+
+    /**
+     * The parent fo this form
+     * @var FormInterface
+     */
+    private $parent;
+
+    /**
+     * The children of this form
      * @var array
      */
     private $children = array();
 
+    /**
+     * The mapper for mapping data to children and back
+     * @var DataMapper\DataMapperInterface
+     */
     private $dataMapper;
+
+    /**
+     * The errors of this form
+     * @var array
+     */
     private $errors = array();
+
+    /**
+     * Whether added errors should bubble up to the parent
+     * @var Boolean
+     */
     private $errorBubbling;
-    private $name = '';
-    private $parent;
+
+    /**
+     * Whether this form is bound
+     * @var Boolean
+     */
     private $bound = false;
+
+    /**
+     * Whether this form may not be empty
+     * @var Boolean
+     */
     private $required;
+
+    /**
+     * The form data in application format
+     * @var mixed
+     */
     private $data;
+
+    /**
+     * The form data in normalized format
+     * @var mixed
+     */
     private $normData;
+
+    /**
+     * The form data in client format
+     * @var mixed
+     */
     private $clientData;
 
     /**
-     * Contains the names of bound values who don't belong to any children
+     * The names of bound values that don't belong to any children
      * @var array
      */
     private $extraData = array();
 
+    /**
+     * The transformer for transforming from application to normalized format
+     * and back
+     * @var DataTransformer\DataTransformerInterface
+     */
     private $normTransformer;
+
+    /**
+     * The transformer for transforming from normalized to client format and
+     * back
+     * @var DataTransformer\DataTransformerInterface
+     */
     private $clientTransformer;
+
+    /**
+     * Whether the data in application, normalized and client format is
+     * synchronized. Data may not be synchronized if transformation errors
+     * occur.
+     * @var Boolean
+     */
     private $synchronized = true;
+
+    /**
+     * The validators attached to this form
+     * @var array
+     */
     private $validators;
+
+    /**
+     * Whether this form may only be read, but not bound
+     * @var Boolean
+     */
     private $readOnly = false;
+
+    /**
+     * The dispatcher for distributing events of this form
+     * @var Symfony\Component\EventDispatcher\EventDispatcherInterface
+     */
     private $dispatcher;
+
+    /**
+     * Key-value store for arbitrary attributes attached to this form
+     * @var array
+     */
     private $attributes;
+
+    /**
+     * The FormTypeInterface instances used to create this form
+     * @var array
+     */
     private $types;
 
-    public function __construct($name, array $types,
+    public function __construct($name,
         EventDispatcherInterface $dispatcher,
+        array $types = array(),
         DataTransformerInterface $clientTransformer = null,
         DataTransformerInterface $normTransformer = null,
         DataMapperInterface $dataMapper = null, array $validators = array(),
@@ -382,6 +475,40 @@ class Form implements \IteratorAggregate, FormInterface
         }
     }
 
+    /**
+     * Binds a request to the form
+     *
+     * If the request was a POST request, the data is bound to the form,
+     * transformed and written into the form data (an object or an array).
+     * You can set the form data by passing it in the second parameter
+     * of this method or by passing it in the "data" option of the form's
+     * constructor.
+     *
+     * @param Request $request    The request to bind to the form
+     * @param array|object $data  The data from which to read default values
+     *                            and where to write bound values
+     */
+    public function bindRequest(Request $request)
+    {
+        // Store the bound data in case of a post request
+        switch ($request->getMethod()) {
+            case 'POST':
+            case 'PUT':
+                $data = array_replace_recursive(
+                    $request->request->get($this->getName(), array()),
+                    $request->files->get($this->getName(), array())
+                );
+                break;
+            case 'GET':
+                $data = $request->query->get($this->getName(), array());
+                break;
+            default:
+                throw new FormException(sprintf('The request method "%s" is not supported', $request->getMethod()));
+        }
+
+        $this->bind($data);
+    }
+
     /**
      * Returns the data in the format needed for the underlying object.
      *
@@ -484,7 +611,6 @@ class Form implements \IteratorAggregate, FormInterface
      */
     public function isValid()
     {
-        // TESTME
         if (!$this->isBound() || $this->hasErrors()) {
             return false;
         }
@@ -620,7 +746,7 @@ class Form implements \IteratorAggregate, FormInterface
      *
      * @param string $name The offset of the value to get
      *
-     * @return Field A form child instance
+     * @return FormInterface  A form instance
      */
     public function offsetGet($name)
     {
@@ -628,28 +754,24 @@ class Form implements \IteratorAggregate, FormInterface
     }
 
     /**
-     * Throws an exception saying that values cannot be set (implements the \ArrayAccess interface).
+     * Adds a child to the form (implements the \ArrayAccess interface).
      *
-     * @param string $offset (ignored)
-     * @param string $value (ignored)
-     *
-     * @throws \LogicException
+     * @param string $name Ignored. The name of the child is used.
+     * @param FormInterface $child  The child to be added
      */
     public function offsetSet($name, $child)
     {
-        throw new \BadMethodCallException('offsetSet() is not supported');
+        $this->add($child);
     }
 
     /**
-     * Throws an exception saying that values cannot be unset (implements the \ArrayAccess interface).
-     *
-     * @param string $name
+     * Removes the child with the given name from the form (implements the \ArrayAccess interface).
      *
-     * @throws \LogicException
+     * @param string $name  The name of the child to be removed
      */
     public function offsetUnset($name)
     {
-        throw new \BadMethodCallException('offsetUnset() is not supported');
+        $this->remove($name);
     }
 
     /**
@@ -733,38 +855,4 @@ class Form implements \IteratorAggregate, FormInterface
 
         return $this->clientTransformer->reverseTransform($value);
     }
-
-    /**
-     * Binds a request to the form
-     *
-     * If the request was a POST request, the data is bound to the form,
-     * transformed and written into the form data (an object or an array).
-     * You can set the form data by passing it in the second parameter
-     * of this method or by passing it in the "data" option of the form's
-     * constructor.
-     *
-     * @param Request $request    The request to bind to the form
-     * @param array|object $data  The data from which to read default values
-     *                            and where to write bound values
-     */
-    public function bindRequest(Request $request)
-    {
-        // Store the bound data in case of a post request
-        switch ($request->getMethod()) {
-            case 'POST':
-            case 'PUT':
-                $data = array_replace_recursive(
-                    $request->request->get($this->getName(), array()),
-                    $request->files->get($this->getName(), array())
-                );
-                break;
-            case 'GET':
-                $data = $request->query->get($this->getName(), array());
-                break;
-            default:
-                throw new FormException(sprintf('The request method "%s" is not supported', $request->getMethod()));
-        }
-
-        $this->bind($data);
-    }
 }

src/Symfony/Component/Form/FormBuilder.php

@@ -423,8 +423,8 @@ class FormBuilder
     {
         $instance = new Form(
             $this->getName(),
-            $this->getTypes(),
             $this->buildDispatcher(),
+            $this->getTypes(),
             $this->getClientTransformer(),
             $this->getNormTransformer(),
             $this->getDataMapper(),

src/Symfony/Component/Form/FormInterface.php

@@ -114,4 +114,8 @@ interface FormInterface extends \ArrayAccess, \Traversable, \Countable
     function hasAttribute($name);
 
     function getAttribute($name);
+
+    function getRoot();
+
+    function isRoot();
 }

tests/Symfony/Tests/Component/Form/FormTest.php

@@ -11,9 +11,12 @@
 
 namespace Symfony\Tests\Component\Form;
 
+require_once __DIR__.'/Fixtures/FixedDataTransformer.php';
+
 use Symfony\Component\Form\Form;
 use Symfony\Component\Form\FormBuilder;
 use Symfony\Component\Form\FormError;
+use Symfony\Tests\Component\Form\Fixtures\FixedDataTransformer;
 
 class FormTest extends \PHPUnit_Framework_TestCase
 {
@@ -26,15 +29,40 @@ class FormTest extends \PHPUnit_Framework_TestCase
     protected function setUp()
     {
         $this->dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
-        $this->builder = new FormBuilder('name', $this->dispatcher);
-        $this->form = $this->builder->getForm();
+        $this->form = $this->getBuilder()->getForm();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
+     */
+    public function testConstructExpectsValidValidators()
+    {
+        $validators = array(new \stdClass());
+
+        new Form('name', $this->dispatcher, array(), null, null, null, $validators);
+    }
+
+    public function testDataIsInitializedEmpty()
+    {
+        $norm = new FixedDataTransformer(array(
+            '' => 'foo',
+        ));
+        $client = new FixedDataTransformer(array(
+            'foo' => 'bar',
+        ));
+
+        $form = new Form('name', $this->dispatcher, array(), $client, $norm);
+
+        $this->assertNull($form->getData());
+        $this->assertSame('foo', $form->getNormData());
+        $this->assertSame('bar', $form->getClientData());
     }
 
     public function testErrorsBubbleUpIfEnabled()
     {
         $error = new FormError('Error!');
         $parent = $this->form;
-        $form = $this->builder->setErrorBubbling(true)->getForm();
+        $form = $this->getBuilder()->setErrorBubbling(true)->getForm();
 
         $form->setParent($parent);
         $form->addError($error);
@@ -47,7 +75,7 @@ class FormTest extends \PHPUnit_Framework_TestCase
     {
         $error = new FormError('Error!');
         $parent = $this->form;
-        $form = $this->builder->setErrorBubbling(false)->getForm();
+        $form = $this->getBuilder()->setErrorBubbling(false)->getForm();
 
         $form->setParent($parent);
         $form->addError($error);
@@ -110,18 +138,264 @@ class FormTest extends \PHPUnit_Framework_TestCase
         $this->form->bind(array());
     }
 
-    public function testAddSetsFieldParent()
+    public function testNeverRequiredIfParentNotRequired()
     {
-        $child = $this->getMockForm('firstName');
+        $parent = $this->getBuilder()->setRequired(false)->getForm();
+        $child = $this->getBuilder()->setRequired(true)->getForm();
+
+        $child->setParent($parent);
+
+        $this->assertFalse($child->isRequired());
+    }
+
+    public function testRequired()
+    {
+        $parent = $this->getBuilder()->setRequired(true)->getForm();
+        $child = $this->getBuilder()->setRequired(true)->getForm();
+
+        $child->setParent($parent);
+
+        $this->assertTrue($child->isRequired());
+    }
+
+    public function testNotRequired()
+    {
+        $parent = $this->getBuilder()->setRequired(true)->getForm();
+        $child = $this->getBuilder()->setRequired(false)->getForm();
+
+        $child->setParent($parent);
+
+        $this->assertFalse($child->isRequired());
+    }
+
+    public function testAlwaysReadOnlyIfParentReadOnly()
+    {
+        $parent = $this->getBuilder()->setReadOnly(true)->getForm();
+        $child = $this->getBuilder()->setReadOnly(false)->getForm();
+
+        $child->setParent($parent);
+
+        $this->assertTrue($child->isReadOnly());
+    }
+
+    public function testReadOnly()
+    {
+        $parent = $this->getBuilder()->setReadOnly(false)->getForm();
+        $child = $this->getBuilder()->setReadOnly(true)->getForm();
+
+        $child->setParent($parent);
+
+        $this->assertTrue($child->isReadOnly());
+    }
+
+    public function testNotReadOnly()
+    {
+        $parent = $this->getBuilder()->setReadOnly(false)->getForm();
+        $child = $this->getBuilder()->setReadOnly(false)->getForm();
+
+        $child->setParent($parent);
+
+        $this->assertFalse($child->isReadOnly());
+    }
+
+    public function testCloneChildren()
+    {
+        $child = $this->getBuilder('child')->getForm();
+        $this->form->add($child);
+
+        $clone = clone $this->form;
+
+        $this->assertNotSame($this->form, $clone);
+        $this->assertNotSame($child, $clone['child']);
+    }
+
+    public function testGetRootReturnsRootOfParent()
+    {
+        $parent = $this->getMockForm();
+        $parent->expects($this->once())
+            ->method('getRoot')
+            ->will($this->returnValue('ROOT'));
+
+        $this->form->setParent($parent);
+
+        $this->assertEquals('ROOT', $this->form->getRoot());
+    }
+
+    public function testGetRootReturnsSelfIfNoParent()
+    {
+        $this->assertSame($this->form, $this->form->getRoot());
+    }
+
+    public function testIsEmptyIfEmptyArray()
+    {
+        $this->form->setData(array());
+
+        $this->assertTrue($this->form->isEmpty());
+    }
+
+    public function testIsEmptyIfNull()
+    {
+        $this->form->setData(null);
+
+        $this->assertTrue($this->form->isEmpty());
+    }
+
+    public function testIsEmptyIfEmptyString()
+    {
+        $this->form->setData('');
+
+        $this->assertTrue($this->form->isEmpty());
+    }
+
+    public function testIsNotEmptyIfText()
+    {
+        $this->form->setData('foobar');
+
+        $this->assertFalse($this->form->isEmpty());
+    }
+
+    public function testIsNotEmptyIfChildNotEmpty()
+    {
+        $child = $this->getMockForm();
+        $child->expects($this->once())
+            ->method('isEmpty')
+            ->will($this->returnValue(false));
 
+        $this->form->setData(null);
+        $this->form->add($child);
+
+        $this->assertFalse($this->form->isEmpty());
+    }
+
+    public function testValidIfBound()
+    {
+        $this->form->bind('foobar');
+
+        $this->assertTrue($this->form->isValid());
+    }
+
+    public function testNotValidIfNotBound()
+    {
+        $this->assertFalse($this->form->isValid());
+    }
+
+    public function testNotValidIfErrors()
+    {
+        $this->form->bind('foobar');
+        $this->form->addError(new FormError('Error!'));
+
+        $this->assertFalse($this->form->isValid());
+    }
+
+    public function testNotValidIfChildNotValid()
+    {
+        $child = $this->getMockForm();
         $child->expects($this->once())
-            ->method('setParent')
-            ->with($this->equalTo($this->form));
+            ->method('isValid')
+            ->will($this->returnValue(false));
 
+        $this->form->bind('foobar');
         $this->form->add($child);
+
+        $this->assertFalse($this->form->isValid());
+    }
+
+    public function testHasErrors()
+    {
+        $this->form->addError(new FormError('Error!'));
+
+        $this->assertTrue($this->form->hasErrors());
+    }
+
+    public function testHasNoErrors()
+    {
+        $this->assertFalse($this->form->hasErrors());
+    }
+
+    public function testHasChildren()
+    {
+        $this->form->add($this->getBuilder()->getForm());
+
+        $this->assertTrue($this->form->hasChildren());
+    }
+
+    public function testHasNoChildren()
+    {
+        $this->assertFalse($this->form->hasChildren());
+    }
+
+    public function testAdd()
+    {
+        $child = $this->getBuilder('foo')->getForm();
+        $this->form->add($child);
+
+        $this->assertSame($this->form, $child->getParent());
+        $this->assertSame(array('foo' => $child), $this->form->getChildren());
+    }
+
+    public function testRemove()
+    {
+        $child = $this->getBuilder('foo')->getForm();
+        $this->form->add($child);
+        $this->form->remove('foo');
+
+        $this->assertNull($child->getParent());
+        $this->assertFalse($this->form->hasChildren());
+    }
+
+    public function testRemoveIgnoresUnknownName()
+    {
+        $this->form->remove('notexisting');
+    }
+
+    public function testArrayAccess()
+    {
+        $child = $this->getBuilder('foo')->getForm();
+
+        $this->form[] = $child;
+
+        $this->assertTrue(isset($this->form['foo']));
+        $this->assertSame($child, $this->form['foo']);
+
+        unset($this->form['foo']);
+
+        $this->assertFalse(isset($this->form['foo']));
+    }
+
+    public function testCountable()
+    {
+        $this->form->add($this->getBuilder('foo')->getForm());
+        $this->form->add($this->getBuilder('bar')->getForm());
+
+        $this->assertEquals(2, count($this->form));
+    }
+
+    public function testIterator()
+    {
+        $this->form->add($this->getBuilder('foo')->getForm());
+        $this->form->add($this->getBuilder('bar')->getForm());
+
+        $this->assertSame($this->form->getChildren(), iterator_to_array($this->form));
+    }
+
+    public function testIsBound()
+    {
+        $this->form->bind('foobar');
+
+        $this->assertTrue($this->form->isBound());
+    }
+
+    public function testIsNotBound()
+    {
+        $this->assertFalse($this->form->isBound());
+    }
+
+    protected function getBuilder($name = 'name')
+    {
+        return new FormBuilder($name, $this->dispatcher);
     }
 
-    protected function getMockForm($name)
+    protected function getMockForm($name = 'name')
     {
         $form = $this->getMock('Symfony\Tests\Component\Form\FormInterface');
 

tests/Symfony/Tests/Component/Form/Type/FormTypeTest.php

@@ -208,74 +208,6 @@ class FormTypeTest extends TestCase
         $this->assertEquals($file, $form['image']['file']->getData());
     }
 
-    public function testSupportsArrayAccess()
-    {
-        $builder = $this->factory->createBuilder('form', 'author');
-        $builder->add('firstName', 'field');
-        $form = $builder->getForm();
-
-        $this->assertEquals($form->get('firstName'), $form['firstName']);
-        $this->assertTrue(isset($form['firstName']));
-    }
-
-    /**
-     * @expectedException BadMethodCallException
-     */
-    public function testSupportsUnset()
-    {
-        $form = $this->factory->create('form', 'author');
-
-        unset($form['firstName']);
-    }
-
-    public function testDoesNotSupportAddingFields()
-    {
-        $form = $this->factory->create('form', 'author');
-
-        $this->setExpectedException('LogicException');
-
-        $form[] = $this->getMockForm('lastName');
-    }
-
-    public function testSupportsCountable()
-    {
-        $builder = $this->factory->createBuilder('form', 'group', array(
-            'csrf_protection' => false,
-        ));
-        $builder->add('firstName', 'field');
-        $builder->add('lastName', 'field');
-        $form = $builder->getForm();
-
-        $this->assertEquals(2, count($form));
-    }
-
-    public function testSupportsIterable()
-    {
-        $builder = $this->factory->createBuilder('form', 'group', array(
-            'csrf_protection' => false,
-        ));
-        $builder->add('field1', 'field');
-        $builder->add('field2', 'field');
-        $builder->add('field3', 'field');
-        $form = $builder->getForm();
-
-        $expected = array(
-            'field1' => $form->get('field1'),
-            'field2' => $form->get('field2'),
-            'field3' => $form->get('field3'),
-        );
-
-        $this->assertEquals($expected, iterator_to_array($form));
-    }
-
-    public function testIsBound()
-    {
-        $form = $this->factory->create('form', 'author');
-        $this->assertFalse($form->isBound());
-        $form->bind(array('firstName' => 'Bernhard'));
-        $this->assertTrue($form->isBound());
-    }
-
     public function testSetDataUpdatesAllFieldsFromTransformedData()
     {
         $originalAuthor = new Author();
@@ -587,26 +519,6 @@ class FormTypeTest extends TestCase
         $this->assertSame($ref2, $author['referenceCopy']);
     }
 
-    public function testIsEmptyReturnsTrueIfAllFieldsAreEmpty()
-    {
-        $builder = $this->factory->createBuilder('form', 'name');
-        $builder->add('foo', 'field', array('data' => ''));
-        $builder->add('bar', 'field', array('data' => null));
-        $form = $builder->getForm();
-
-        $this->assertTrue($form->isEmpty());
-    }
-
-    public function testIsEmptyReturnsFalseIfAnyFieldIsFilled()
-    {
-        $builder = $this->factory->createBuilder('form', 'name');
-        $builder->add('foo', 'field', array('data' => 'baz'));
-        $builder->add('bar', 'field', array('data' => null));
-        $form = $builder->getForm();
-
-        $this->assertFalse($form->isEmpty());
-    }
-
     /**
      * Create a group containing two fields, "visibleField" and "hiddenField"
      *