merged branch bschussek/issue1540 (PR #3239)

Commits
-------

9b0245b [Form] Made prefix of adder and remover method configurable. Adders and removers are not called if "by_reference" is disabled.
49d1464 [Form] Implemented MergeCollectionListener which calls addXxx() and removeXxx() in your model if found
7837f50 [Form] Added FormUtil::singularify()

Discussion
----------

[Form] Forms now call addXxx() and removeXxx() in your model

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1540
Todo: adapt documentation

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue1540)

Adds functionality for calling `addXxx` and `removeXxx` method in your model. All types returning a collection of values are affected: "collection", "choice" (with multiple selection) and "entity" (with multiple selection).

Example:

    class Article
    {
        public function addTag($tag) { ... }
        public function removeTag($tag) { ... }
        public function getTags($tag) { ... }
    }

And the controller:

    $form = $this->createFormBuilder($article)
        ->add('tags')
        ->getForm();

Upon modifying the form, addTag() and removeTag() are now called appropiately.

---------------------------------------------------------------------------

by stof at 2012-02-01T18:23:49Z

Really great

---------------------------------------------------------------------------

by vicb at 2012-02-01T18:24:04Z

Great !!

Two suggestions:

* make "add" and "remove" configurable,
* introduce a base class for the remove listeners with (final?) `::getSubscribedEvents()` and `::getEventPriorities()`

---------------------------------------------------------------------------

by haswalt at 2012-02-01T18:57:46Z

+1 this

---------------------------------------------------------------------------

by daFish at 2012-02-01T19:54:46Z

+1

---------------------------------------------------------------------------

by michelsalib at 2012-02-01T20:55:37Z

Can wait to have it!
It will save lots of time trying to solve WTF effects and making workarounds.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T09:37:12Z

@vicb: Your first point is done. The second, I don't understand.

---------------------------------------------------------------------------

by stof at 2012-02-02T09:40:50Z

@bschussek your branch conflicts with master according to github

---------------------------------------------------------------------------

by vicb at 2012-02-02T09:52:40Z

@bschussek my point is that I can stand hard-coded priorities which are error prone. A better solution might be to introduce constants (in `FormEvents` / `FormEventPriorities` ?) with meaningful names.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T10:21:52Z

@stof Rebased

@vicb I know, but who is responsible for managing priorities? There is no central entitty that can do this. (btw this is a general problem of the priority system of the EventDispatcher)

@fabpot Ready to merge.

---------------------------------------------------------------------------

by vicb at 2012-02-02T10:23:28Z

@bschussek doesn't each form has is own dispatcher so there is no need for a global registry here, something local to the form could be good enough.

CHANGELOG-2.1.md

@@ -195,6 +195,11 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
  * choice fields now throw a FormException if neither the "choices" nor the
    "choice_list" option is set
  * the radio type is now a child of the checkbox type
+ * the collection, choice (with multiple selection) and entity (with multiple
+   selection) types now make use of addXxx() and removeXxx() methods in your
+   model
+ * added options "adder_prefix" and "remover_prefix" to collection and choice
+   type
 
 ### HttpFoundation
 

src/Symfony/Bridge/Doctrine/Form/EventListener/MergeDoctrineCollectionListener.php

@@ -24,11 +24,13 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface;
  *
  * @see    Doctrine\Common\Collections\Collection
  */
-class MergeCollectionListener implements EventSubscriberInterface
+class MergeDoctrineCollectionListener implements EventSubscriberInterface
 {
     static public function getSubscribedEvents()
     {
-        return array(FormEvents::BIND_NORM_DATA => 'onBindNormData');
+        // Higher priority than core MergeCollectionListener so that this one
+        // is called before
+        return array(FormEvents::BIND_NORM_DATA => array('onBindNormData', 10));
     }
 
     public function onBindNormData(FilterDataEvent $event)
@@ -36,25 +38,10 @@ class MergeCollectionListener implements EventSubscriberInterface
         $collection = $event->getForm()->getData();
         $data = $event->getData();
 
-        if (!$collection) {
-            $collection = $data;
-        } elseif (count($data) === 0) {
+        // If all items were removed, call clear which has a higher
+        // performance on persistent collections
+        if ($collection && count($data) === 0) {
             $collection->clear();
-        } else {
-            // merge $data into $collection
-            foreach ($collection as $entity) {
-                if (!$data->contains($entity)) {
-                    $collection->removeElement($entity);
-                } else {
-                    $data->removeElement($entity);
-                }
-            }
-
-            foreach ($data as $entity) {
-                $collection->add($entity);
-            }
         }
-
-        $event->setData($collection);
     }
 }

src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php

@@ -16,7 +16,7 @@ use Doctrine\Common\Persistence\ObjectManager;
 use Symfony\Component\Form\FormBuilder;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
-use Symfony\Bridge\Doctrine\Form\EventListener\MergeCollectionListener;
+use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener;
 use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
 use Symfony\Component\Form\AbstractType;
 
@@ -36,7 +36,7 @@ abstract class DoctrineType extends AbstractType
     {
         if ($options['multiple']) {
             $builder
-                ->addEventSubscriber(new MergeCollectionListener())
+                ->addEventSubscriber(new MergeDoctrineCollectionListener())
                 ->prependClientTransformer(new CollectionToArrayTransformer())
             ;
         }

src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php

@@ -0,0 +1,179 @@
+<?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\Extension\Core\EventListener;
+
+use Symfony\Component\Form\Util\FormUtil;
+
+use Symfony\Component\Form\FormEvents;
+use Symfony\Component\Form\Event\FilterDataEvent;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
+
+/**
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+class MergeCollectionListener implements EventSubscriberInterface
+{
+    /**
+     * Whether elements may be added to the collection
+     * @var Boolean
+     */
+    private $allowAdd;
+
+    /**
+     * Whether elements may be removed from the collection
+     * @var Boolean
+     */
+    private $allowDelete;
+
+    /**
+     * Whether to search for and use adder and remover methods
+     * @var Boolean
+     */
+    private $useAccessors;
+
+    /**
+     * The prefix of the adder method to look for
+     * @var string
+     */
+    private $adderPrefix;
+
+    /**
+     * The prefix of the remover method to look for
+     * @var string
+     */
+    private $removerPrefix;
+
+    public function __construct($allowAdd = false, $allowDelete = false, $useAccessors = true, $adderPrefix = 'add', $removerPrefix = 'remove')
+    {
+        $this->allowAdd = $allowAdd;
+        $this->allowDelete = $allowDelete;
+        $this->useAccessors = $useAccessors;
+        $this->adderPrefix = $adderPrefix;
+        $this->removerPrefix = $removerPrefix;
+    }
+
+    static public function getSubscribedEvents()
+    {
+        return array(FormEvents::BIND_NORM_DATA => 'onBindNormData');
+    }
+
+    public function onBindNormData(FilterDataEvent $event)
+    {
+        $originalData = $event->getForm()->getData();
+        $form = $event->getForm();
+        $data = $event->getData();
+        $parentData = $form->hasParent() ? $form->getParent()->getData() : null;
+        $adder = null;
+        $remover = null;
+
+        if (null === $data) {
+            $data = array();
+        }
+
+        if (!is_array($data) && !($data instanceof \Traversable && $data instanceof \ArrayAccess)) {
+            throw new UnexpectedTypeException($data, 'array or (\Traversable and \ArrayAccess)');
+        }
+
+        if (null !== $originalData && !is_array($originalData) && !($originalData instanceof \Traversable && $originalData instanceof \ArrayAccess)) {
+            throw new UnexpectedTypeException($originalData, 'array or (\Traversable and \ArrayAccess)');
+        }
+
+        // Check if the parent has matching methods to add/remove items
+        if ($this->useAccessors && is_object($parentData)) {
+            $plural = ucfirst($form->getName());
+            $singulars = (array) FormUtil::singularify($plural);
+            $reflClass = new \ReflectionClass($parentData);
+
+            foreach ($singulars as $singular) {
+                $adderName = $this->adderPrefix . $singular;
+                $removerName = $this->removerPrefix . $singular;
+
+                if ($reflClass->hasMethod($adderName) && $reflClass->hasMethod($removerName)) {
+                    $adder = $reflClass->getMethod($adderName);
+                    $remover = $reflClass->getMethod($removerName);
+
+                    if ($adder->isPublic() && $adder->getNumberOfRequiredParameters() === 1
+                        && $remover->isPublic() && $remover->getNumberOfRequiredParameters() === 1) {
+
+                        // We found a public, one-parameter add and remove method
+                        break;
+                    }
+
+                    // False alert
+                    $adder = null;
+                    $remover = null;
+                }
+            }
+        }
+
+        // Check which items are in $data that are not in $originalData and
+        // vice versa
+        $itemsToDelete = array();
+        $itemsToAdd = is_object($data) ? clone $data : $data;
+
+        if ($originalData) {
+            foreach ($originalData as $originalKey => $originalItem) {
+                foreach ($data as $key => $item) {
+                    if ($item === $originalItem) {
+                        // Item found, next original item
+                        unset($itemsToAdd[$key]);
+                        continue 2;
+                    }
+                }
+
+                // Item not found, remember for deletion
+                $itemsToDelete[$originalKey] = $originalItem;
+            }
+        }
+
+        if ($adder && $remover) {
+            // If methods to add and to remove exist, call them now, if allowed
+            if ($this->allowDelete) {
+                foreach ($itemsToDelete as $item) {
+                    $remover->invoke($parentData, $item);
+                }
+            }
+
+            if ($this->allowAdd) {
+                foreach ($itemsToAdd as $item) {
+                    $adder->invoke($parentData, $item);
+                }
+            }
+        } elseif (!$originalData) {
+            // No original data was set. Set it if allowed
+            if ($this->allowAdd) {
+                $originalData = $data;
+            }
+        } else {
+            // Original data is an array-like structure
+            // Add and remove items in the original variable
+            if ($this->allowDelete) {
+                foreach ($itemsToDelete as $key => $item) {
+                    unset($originalData[$key]);
+                }
+            }
+
+            if ($this->allowAdd) {
+                foreach ($itemsToAdd as $key => $item) {
+                    if (!isset($originalData[$key])) {
+                        $originalData[$key] = $item;
+                    } else {
+                        $originalData[] = $item;
+                    }
+                }
+            }
+        }
+
+        $event->setData($originalData);
+    }
+}

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

@@ -66,7 +66,8 @@ class ResizeFormListener implements EventSubscriberInterface
         return array(
             FormEvents::PRE_SET_DATA => 'preSetData',
             FormEvents::PRE_BIND => 'preBind',
-            FormEvents::BIND_NORM_DATA => 'onBindNormData',
+            // (MergeCollectionListener, MergeDoctrineCollectionListener)
+            FormEvents::BIND_NORM_DATA => array('onBindNormData', 50),
         );
     }
 

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

@@ -19,6 +19,7 @@ use Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceList;
 use Symfony\Component\Form\Extension\Core\ChoiceList\SimpleChoiceList;
 use Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface;
 use Symfony\Component\Form\Extension\Core\EventListener\FixRadioInputListener;
+use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
 use Symfony\Component\Form\FormView;
 use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer;
 use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToBooleanArrayTransformer;
@@ -80,7 +81,10 @@ class ChoiceType extends AbstractType
 
         if ($options['expanded']) {
             if ($options['multiple']) {
-                $builder->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']));
+                $builder
+                    ->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']))
+                    ->addEventSubscriber(new MergeCollectionListener(true, true))
+                ;
             } else {
                 $builder
                     ->appendClientTransformer(new ChoiceToBooleanArrayTransformer($options['choice_list']))
@@ -89,12 +93,24 @@ class ChoiceType extends AbstractType
             }
         } else {
             if ($options['multiple']) {
-                $builder->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']));
+                $builder
+                    ->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']))
+                    ->addEventSubscriber(new MergeCollectionListener(
+                        true,
+                        true,
+                        // If "by_reference" is disabled (explicit calling of
+                        // the setter is desired), disable support for
+                        // adders/removers
+                        // Same as in CollectionType
+                        $options['by_reference'],
+                        $options['adder_prefix'],
+                        $options['remover_prefix']
+                    ))
+                ;
             } else {
                 $builder->appendClientTransformer(new ChoiceToValueTransformer($options['choice_list']));
             }
         }
-
     }
 
     /**
@@ -140,6 +156,8 @@ class ChoiceType extends AbstractType
             'empty_data'        => $multiple || $expanded ? array() : '',
             'empty_value'       => $multiple || $expanded || !isset($options['empty_value']) ? null : '',
             'error_bubbling'    => false,
+            'adder_prefix'      => 'add',
+            'remover_prefix'    => 'remove',
         );
     }
 

src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php

@@ -16,6 +16,7 @@ use Symfony\Component\Form\FormBuilder;
 use Symfony\Component\Form\FormView;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\Form\Extension\Core\EventListener\ResizeFormListener;
+use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
 
 class CollectionType extends AbstractType
 {
@@ -29,7 +30,7 @@ class CollectionType extends AbstractType
             $builder->setAttribute('prototype', $prototype->getForm());
         }
 
-        $listener = new ResizeFormListener(
+        $resizeListener = new ResizeFormListener(
             $builder->getFormFactory(),
             $options['type'],
             $options['options'],
@@ -37,8 +38,20 @@ class CollectionType extends AbstractType
             $options['allow_delete']
         );
 
+        $mergeListener = new MergeCollectionListener(
+            $options['allow_add'],
+            $options['allow_delete'],
+            // If "by_reference" is disabled (explicit calling of the setter
+            // is desired), disable support for adders/removers
+            // Same as in ChoiceType
+            $options['by_reference'],
+            $options['adder_prefix'],
+            $options['remover_prefix']
+        );
+
         $builder
-            ->addEventSubscriber($listener)
+            ->addEventSubscriber($resizeListener)
+            ->addEventSubscriber($mergeListener)
             ->setAttribute('allow_add', $options['allow_add'])
             ->setAttribute('allow_delete', $options['allow_delete'])
         ;
@@ -77,6 +90,8 @@ class CollectionType extends AbstractType
         return array(
             'allow_add'      => false,
             'allow_delete'   => false,
+            'adder_prefix'   => 'add',
+            'remover_prefix' => 'remove',
             'prototype'      => true,
             'prototype_name' => '__name__',
             'type'           => 'text',

src/Symfony/Component/Form/Util/FormUtil.php

@@ -16,6 +16,181 @@ namespace Symfony\Component\Form\Util;
  */
 abstract class FormUtil
 {
+    const PLURAL_SUFFIX = 0;
+
+    const PLURAL_SUFFIX_LENGTH = 1;
+
+    const PLURAL_SUFFIX_AFTER_VOCAL = 2;
+
+    const PLURAL_SUFFIX_AFTER_CONS = 3;
+
+    const SINGULAR_SUFFIX = 4;
+
+    /**
+     * Map english plural to singular suffixes
+     *
+     * @var array
+     *
+     * @see http://english-zone.com/spelling/plurals.html
+     * @see http://www.scribd.com/doc/3271143/List-of-100-Irregular-Plural-Nouns-in-English
+     */
+    static private $pluralMap = array(
+        // First entry: plural suffix, reversed
+        // Second entry: length of plural suffix
+        // Third entry: Whether the suffix may succeed a vocal
+        // Fourth entry: Whether the suffix may succeed a consonant
+        // Fifth entry: singular suffix, normal
+
+        // bacteria (bacterium), criteria (criterion), phenomena (phenomenon)
+        array('a', 1, true, true, array('on', 'um')),
+
+        // nebulae (nebula)
+        array('ea', 2, true, true, 'a'),
+
+        // mice (mouse), lice (louse)
+        array('eci', 3, false, true, 'ouse'),
+
+        // geese (goose)
+        array('esee', 4, false, true, 'oose'),
+
+        // fungi (fungus), alumni (alumnus), syllabi (syllabus), radii (radius)
+        array('i', 1, true, true, 'us'),
+
+        // men (man), women (woman)
+        array('nem', 3, true, true, 'man'),
+
+        // children (child)
+        array('nerdlihc', 8, true, true, 'child'),
+
+        // oxen (ox)
+        array('nexo', 4, false, false, 'ox'),
+
+        // indices (index), appendices (appendix)
+        array('seci', 4, false, true, array('ex', 'ix')),
+
+        // babies (baby)
+        array('sei', 3, false, true, 'y'),
+
+        // analyses (analysis), ellipses (ellipsis), funguses (fungus),
+        // neuroses (neurosis), theses (thesis), emphases (emphasis),
+        // oases (oasis), crises (crisis), houses (house), bases (base),
+        // atlases (atlas), kisses (kiss)
+        array('ses', 3, true, true, array('s', 'se', 'sis')),
+
+        // lives (life), wives (wife)
+        array('sevi', 4, false, true, 'ife'),
+
+        // hooves (hoof), dwarves (dwarf), elves (elf), leaves (leaf)
+        array('sev', 3, true, true, 'f'),
+
+        // axes (axis), axes (ax), axes (axe)
+        array('sexa', 4, false, false, array('ax', 'axe', 'axis')),
+
+        // indexes (index), matrixes (matrix)
+        array('sex', 3, true, false, 'x'),
+
+        // quizzes (quiz)
+        array('sezz', 4, true, false, 'z'),
+
+        // bureaus (bureau)
+        array('suae', 4, false, true, 'eau'),
+
+        // roses (rose), garages (garage), cassettes (cassette),
+        // waltzes (waltz), heroes (hero), bushes (bush), arches (arch),
+        // shoes (shoe)
+        array('se', 2, true, true, array('', 'e')),
+
+        // tags (tag)
+        array('s', 1, true, true, ''),
+
+        // chateaux (chateau)
+        array('xuae', 4, false, true, 'eau'),
+    );
+
+    /**
+     * Returns the singular form of a word
+     *
+     * If the method can't determine the form with certainty, an array of the
+     * possible singulars is returned.
+     *
+     * @param  string $plural A word in plural form
+     * @return string|array The singular form or an array of possible singular
+     *                      forms
+     */
+    static public function singularify($plural)
+    {
+        $pluralRev = strrev($plural);
+        $lowerPluralRev = strtolower($pluralRev);
+        $pluralLength = strlen($lowerPluralRev);
+
+        // The outer loop $i iterates over the entries of the plural table
+        // The inner loop $j iterates over the characters of the plural suffix
+        // in the plural table to compare them with the characters of the actual
+        // given plural suffix
+        for ($i = 0, $numPlurals = count(self::$pluralMap); $i < $numPlurals; ++$i) {
+            $suffix = self::$pluralMap[$i][self::PLURAL_SUFFIX];
+            $suffixLength = self::$pluralMap[$i][self::PLURAL_SUFFIX_LENGTH];
+            $j = 0;
+
+            // Compare characters in the plural table and of the suffix of the
+            // given plural one by one
+            while ($suffix[$j] === $lowerPluralRev[$j]) {
+                // Let $j point to the next character
+                ++$j;
+
+                // Successfully compared the last character
+                // Add an entry with the singular suffix to the singular array
+                if ($j === $suffixLength) {
+                    // Is there any character preceding the suffix in the plural string?
+                    if ($j < $pluralLength) {
+                        $nextIsVocal = false !== strpos('aeiou', $lowerPluralRev[$j]);
+
+                        if (!self::$pluralMap[$i][self::PLURAL_SUFFIX_AFTER_VOCAL] && $nextIsVocal) {
+                            break;
+                        }
+
+                        if (!self::$pluralMap[$i][self::PLURAL_SUFFIX_AFTER_CONS] && !$nextIsVocal) {
+                            break;
+                        }
+                    }
+
+                    $newBase = substr($plural, 0, $pluralLength - $suffixLength);
+                    $newSuffix = self::$pluralMap[$i][self::SINGULAR_SUFFIX];
+
+                    // Check whether the first character in the plural suffix
+                    // is uppercased. If yes, uppercase the first character in
+                    // the singular suffix too
+                    $firstUpper = ctype_upper($pluralRev[$j - 1]);
+
+                    if (is_array($newSuffix)) {
+                        $singulars = array();
+
+                        foreach ($newSuffix as $newSuffixEntry) {
+                            $singulars[] = $newBase . ($firstUpper ? ucfirst($newSuffixEntry) : $newSuffixEntry);
+                        }
+
+                        return $singulars;
+                    }
+
+                    return $newBase . ($firstUpper ? ucFirst($newSuffix) : $newSuffix);
+                }
+
+                // Suffix is longer than word
+                if ($j === $pluralLength) {
+                    break;
+                }
+            }
+        }
+
+        // Convert teeth to tooth, feet to foot
+        if (false !== ($pos = strpos($plural, 'ee'))) {
+            return substr_replace($plural, 'oo', $pos, 2);
+        }
+
+        // Assume that plural and singular is identical
+        return $plural;
+    }
+
     /**
      * Returns whether the given choice is a group.
      *

tests/Symfony/Tests/Bridge/Doctrine/Form/Type/EntityTypeTest.php

@@ -344,13 +344,13 @@ class EntityTypeTest extends TypeTestCase
             'property' => 'name',
         ));
 
-        $existing = new ArrayCollection(array($entity2));
+        $existing = new ArrayCollection(array(0 => $entity2));
 
         $field->setData($existing);
         $field->bind(array('1', '3'));
 
-        // entry with index 0 was removed
-        $expected = new ArrayCollection(array(1 => $entity1, 2 => $entity3));
+        // entry with index 0 ($entity2) was replaced
+        $expected = new ArrayCollection(array(0 => $entity1, 1 => $entity3));
 
         $this->assertTrue($field->isSynchronized());
         $this->assertEquals($expected, $field->getData());
@@ -406,8 +406,8 @@ class EntityTypeTest extends TypeTestCase
         $field->setData($existing);
         $field->bind(array('0', '2'));
 
-        // entry with index 0 was removed
-        $expected = new ArrayCollection(array(1 => $entity1, 2 => $entity3));
+        // entry with index 0 ($entity2) was replaced
+        $expected = new ArrayCollection(array(0 => $entity1, 1 => $entity3));
 
         $this->assertTrue($field->isSynchronized());
         $this->assertEquals($expected, $field->getData());

tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerArrayObjectTest.php

@@ -0,0 +1,20 @@
+<?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\Tests\Component\Form\Extension\Core\EventListener;
+
+class MergeCollectionListenerArrayObjectTest extends MergeCollectionListenerTest
+{
+    protected function getData(array $data)
+    {
+        return new \ArrayObject($data);
+    }
+}

tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerArrayTest.php

@@ -0,0 +1,20 @@
+<?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\Tests\Component\Form\Extension\Core\EventListener;
+
+class MergeCollectionListenerArrayTest extends MergeCollectionListenerTest
+{
+    protected function getData(array $data)
+    {
+        return $data;
+    }
+}

tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerCustomArrayObjectTest.php

@@ -0,0 +1,82 @@
+<?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\Tests\Component\Form\Extension\Core\EventListener;
+
+/**
+* This class is a hand written simplified version of PHP native `ArrayObject`
+* class, to show that it behaves differently than the PHP native implementation.
+*/
+class MergeCollectionListenerCustomArrayObjectTest_CustomArrayObject implements \ArrayAccess, \IteratorAggregate, \Countable, \Serializable
+{
+    private $array;
+
+    public function __construct(array $array = null)
+    {
+        $this->array = (array) ($array ?: array());
+    }
+
+    public function offsetExists($offset)
+    {
+        return array_key_exists($offset, $this->array);
+    }
+
+    public function offsetGet($offset)
+    {
+        return $this->array[$offset];
+    }
+
+    public function offsetSet($offset, $value)
+    {
+        if (null === $offset) {
+            $this->array[] = $value;
+        } else {
+            $this->array[$offset] = $value;
+        }
+    }
+
+    public function offsetUnset($offset)
+    {
+        if (array_key_exists($offset, $this->array)) {
+            unset($this->array[$offset]);
+        }
+    }
+
+    public function getIterator()
+    {
+        return new \ArrayIterator($this->array);
+    }
+
+    public function count()
+    {
+        return count($this->array);
+    }
+
+    public function serialize()
+    {
+        return serialize($this->array);
+    }
+
+    public function unserialize($serialized)
+    {
+        $this->array = (array) unserialize((string) $serialized);
+    }
+}
+
+class MergeCollectionListenerCustomArrayObjectTest extends MergeCollectionListenerTest
+{
+    protected function getData(array $data)
+    {
+        $class = __CLASS__ . '_CustomArrayObject';
+
+        return new $class($data);
+    }
+}

tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php

@@ -0,0 +1,434 @@
+<?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\Tests\Component\Form\Extension\Core\EventListener;
+
+use Symfony\Component\Form\Event\DataEvent;
+use Symfony\Component\Form\Event\FilterDataEvent;
+use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
+use Symfony\Component\Form\FormBuilder;
+
+class MergeCollectionListenerTest_Car
+{
+    // In the test, use a name that FormUtil can't uniquely singularify
+    public function addAxis($axis) {}
+
+    public function removeAxis($axis) {}
+}
+
+class MergeCollectionListenerTest_CarCustomPrefix
+{
+    public function fooAxis($axis) {}
+
+    public function barAxis($axis) {}
+}
+
+abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
+{
+    private $dispatcher;
+    private $factory;
+    private $form;
+
+    public function setUp()
+    {
+        $this->dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
+        $this->factory = $this->getMock('Symfony\Component\Form\FormFactoryInterface');
+        $this->form = $this->getForm('axes');
+    }
+
+    protected function tearDown()
+    {
+        $this->dispatcher = null;
+        $this->factory = null;
+        $this->form = null;
+    }
+
+    protected function getBuilder($name = 'name')
+    {
+        return new FormBuilder($name, $this->factory, $this->dispatcher);
+    }
+
+    protected function getForm($name = 'name')
+    {
+        return $this->getBuilder($name)->getForm();
+    }
+
+    protected function getMockForm()
+    {
+        return $this->getMock('Symfony\Tests\Component\Form\FormInterface');
+    }
+
+    abstract protected function getData(array $data);
+
+    public function testAddExtraEntriesIfAllowAdd()
+    {
+        $originalData = $this->getData(array(1 => 'second'));
+        $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(true, false);
+        $listener->onBindNormData($event);
+
+        // The original object was modified
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The original object matches the new object
+        $this->assertEquals($newData, $event->getData());
+    }
+
+    public function testAddExtraEntriesIfAllowAddDontOverwriteExistingIndices()
+    {
+        $originalData = $this->getData(array(1 => 'first'));
+        $newData = $this->getData(array(0 => 'first', 1 => 'second'));
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(true, false);
+        $listener->onBindNormData($event);
+
+        // The original object was modified
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The original object matches the new object
+        $this->assertEquals($this->getData(array(1 => 'first', 2 => 'second')), $event->getData());
+    }
+
+    public function testDoNothingIfNotAllowAdd()
+    {
+        $originalDataArray = array(1 => 'second');
+        $originalData = $this->getData($originalDataArray);
+        $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, false);
+        $listener->onBindNormData($event);
+
+        // We still have the original object
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // Nothing was removed
+        $this->assertEquals($this->getData($originalDataArray), $event->getData());
+    }
+
+    public function testRemoveMissingEntriesIfAllowDelete()
+    {
+        $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+        $newData = $this->getData(array(1 => 'second'));
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, true);
+        $listener->onBindNormData($event);
+
+        // The original object was modified
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The original object matches the new object
+        $this->assertEquals($newData, $event->getData());
+    }
+
+    public function testDoNothingIfNotAllowDelete()
+    {
+        $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third');
+        $originalData = $this->getData($originalDataArray);
+        $newData = $this->getData(array(1 => 'second'));
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, false);
+        $listener->onBindNormData($event);
+
+        // We still have the original object
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // Nothing was removed
+        $this->assertEquals($this->getData($originalDataArray), $event->getData());
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
+     */
+    public function testRequireArrayOrTraversable()
+    {
+        $newData = 'no array or traversable';
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, false);
+        $listener->onBindNormData($event);
+    }
+
+    public function testDealWithNullData()
+    {
+        $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+        $newData = null;
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, false);
+        $listener->onBindNormData($event);
+
+        $this->assertSame($originalData, $event->getData());
+    }
+
+    public function testDealWithNullOriginalDataIfAllowAdd()
+    {
+        $originalData = null;
+        $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(true, false);
+        $listener->onBindNormData($event);
+
+        $this->assertSame($newData, $event->getData());
+    }
+
+    public function testDontDealWithNullOriginalDataIfNotAllowAdd()
+    {
+        $originalData = null;
+        $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+
+        $this->form->setData($originalData);
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, false);
+        $listener->onBindNormData($event);
+
+        $this->assertNull($event->getData());
+    }
+
+    public function testCallAdderIfAllowAdd()
+    {
+        $parentData = $this->getMock(__CLASS__ . '_Car');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $originalDataArray = array(1 => 'second');
+        $originalData = $this->getData($originalDataArray);
+        $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+
+        $this->form->setData($originalData);
+
+        $parentData->expects($this->at(0))
+            ->method('addAxis')
+            ->with('first');
+        $parentData->expects($this->at(1))
+            ->method('addAxis')
+            ->with('third');
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(true, false);
+        $listener->onBindNormData($event);
+
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The data was not modified directly
+        // Thus it should not be written back into the parent data!
+        $this->assertEquals($this->getData($originalDataArray), $event->getData());
+    }
+
+    public function testDontCallAdderIfNotAllowAdd()
+    {
+        $parentData = $this->getMock(__CLASS__ . '_Car');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $originalDataArray = array(1 => 'second');
+        $originalData = $this->getData($originalDataArray);
+        $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+
+        $this->form->setData($originalData);
+
+        $parentData->expects($this->never())
+            ->method('addAxis');
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, false);
+        $listener->onBindNormData($event);
+
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The data was not modified
+        $this->assertEquals($this->getData($originalDataArray), $event->getData());
+    }
+
+    public function testDontCallAdderIfNotUseAccessors()
+    {
+        $parentData = $this->getMock(__CLASS__ . '_Car');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $originalData = $this->getData(array(1 => 'second'));
+        $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+
+        $this->form->setData($originalData);
+
+        $parentData->expects($this->never())
+            ->method('addAxis');
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(true, false, false);
+        $listener->onBindNormData($event);
+
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The data was modified without accessors
+        $this->assertEquals($newData, $event->getData());
+    }
+
+    public function testCallRemoverIfAllowDelete()
+    {
+        $parentData = $this->getMock(__CLASS__ . '_Car');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third');
+        $originalData = $this->getData($originalDataArray);
+        $newData = $this->getData(array(1 => 'second'));
+
+        $this->form->setData($originalData);
+
+        $parentData->expects($this->at(0))
+            ->method('removeAxis')
+            ->with('first');
+        $parentData->expects($this->at(1))
+            ->method('removeAxis')
+            ->with('third');
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, true);
+        $listener->onBindNormData($event);
+
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The data was not modified directly
+        // Thus it should not be written back into the parent data!
+        $this->assertEquals($this->getData($originalDataArray), $event->getData());
+    }
+
+    public function testDontCallRemoverIfNotAllowDelete()
+    {
+        $parentData = $this->getMock(__CLASS__ . '_Car');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third');
+        $originalData = $this->getData($originalDataArray);
+        $newData = $this->getData(array(1 => 'second'));
+
+        $this->form->setData($originalData);
+
+        $parentData->expects($this->never())
+            ->method('removeAxis');
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, false);
+        $listener->onBindNormData($event);
+
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The data was not modified
+        $this->assertEquals($this->getData($originalDataArray), $event->getData());
+    }
+
+    public function testDontCallRemoverIfNotUseAccessors()
+    {
+        $parentData = $this->getMock(__CLASS__ . '_Car');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+        $newData = $this->getData(array(1 => 'second'));
+
+        $this->form->setData($originalData);
+
+        $parentData->expects($this->never())
+            ->method('removeAxis');
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(false, true, false);
+        $listener->onBindNormData($event);
+
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The data was modified directly
+        $this->assertEquals($newData, $event->getData());
+    }
+
+    public function testCallAccessorsWithCustomPrefixes()
+    {
+        $parentData = $this->getMock(__CLASS__ . '_CarCustomPrefix');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $originalDataArray = array(1 => 'second');
+        $originalData = $this->getData($originalDataArray);
+        $newData = $this->getData(array(0 => 'first'));
+
+        $this->form->setData($originalData);
+
+        $parentData->expects($this->once())
+            ->method('fooAxis')
+            ->with('first');
+        $parentData->expects($this->once())
+            ->method('barAxis')
+            ->with('second');
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener = new MergeCollectionListener(true, true, true, 'foo', 'bar');
+        $listener->onBindNormData($event);
+
+        if (is_object($originalData)) {
+            $this->assertSame($originalData, $event->getData());
+        }
+
+        // The data was not modified directly
+        // Thus it should not be written back into the parent data!
+        $this->assertEquals($this->getData($originalDataArray), $event->getData());
+    }
+}

tests/Symfony/Tests/Component/Form/Extension/Core/Type/CollectionTypeTest.php

@@ -13,7 +13,7 @@ namespace Symfony\Tests\Component\Form\Extension\Core\Type;
 
 use Symfony\Component\Form\Form;
 
-class CollectionFormTest extends TypeTestCase
+class CollectionTypeTest extends TypeTestCase
 {
     public function testContainsNoFieldByDefault()
     {
@@ -80,12 +80,12 @@ class CollectionFormTest extends TypeTestCase
             'allow_delete' => true,
         ));
         $form->setData(array('foo@foo.com', 'bar@bar.com'));
-        $form->bind(array('foo@bar.com'));
+        $form->bind(array('foo@foo.com'));
 
         $this->assertTrue($form->has('0'));
         $this->assertFalse($form->has('1'));
-        $this->assertEquals('foo@bar.com', $form[0]->getData());
-        $this->assertEquals(array('foo@bar.com'), $form->getData());
+        $this->assertEquals('foo@foo.com', $form[0]->getData());
+        $this->assertEquals(array('foo@foo.com'), $form->getData());
     }
 
     public function testNotResizedIfBoundWithExtraData()
@@ -108,13 +108,13 @@ class CollectionFormTest extends TypeTestCase
             'allow_add' => true,
         ));
         $form->setData(array('foo@bar.com'));
-        $form->bind(array('foo@foo.com', 'bar@bar.com'));
+        $form->bind(array('foo@bar.com', 'bar@bar.com'));
 
         $this->assertTrue($form->has('0'));
         $this->assertTrue($form->has('1'));
-        $this->assertEquals('foo@foo.com', $form[0]->getData());
+        $this->assertEquals('foo@bar.com', $form[0]->getData());
         $this->assertEquals('bar@bar.com', $form[1]->getData());
-        $this->assertEquals(array('foo@foo.com', 'bar@bar.com'), $form->getData());
+        $this->assertEquals(array('foo@bar.com', 'bar@bar.com'), $form->getData());
     }
 
     public function testAllowAddButNoPrototype()

tests/Symfony/Tests/Component/Form/Util/FormUtilTest.php

@@ -77,4 +77,121 @@ class FormUtilTest extends \PHPUnit_Framework_TestCase
     {
         $this->assertSame($expected, FormUtil::isChoiceSelected($choice, $value));
     }
+
+    public function singularifyProvider()
+    {
+        // see http://english-zone.com/spelling/plurals.html
+        // see http://www.scribd.com/doc/3271143/List-of-100-Irregular-Plural-Nouns-in-English
+        return array(
+            array('tags', 'tag'),
+            array('alumni', 'alumnus'),
+            array('funguses', array('fungus', 'funguse', 'fungusis')),
+            array('fungi', 'fungus'),
+            array('axes', array('ax', 'axe', 'axis')),
+            array('appendices', array('appendex', 'appendix')),
+            array('indices', array('index', 'indix')),
+            array('indexes', 'index'),
+            array('children', 'child'),
+            array('men', 'man'),
+            array('women', 'woman'),
+            array('oxen', 'ox'),
+            array('bacteria', array('bacterion', 'bacterium')),
+            array('criteria', array('criterion', 'criterium')),
+            array('feet', 'foot'),
+            array('nebulae', 'nebula'),
+            array('babies', 'baby'),
+            array('hooves', 'hoof'),
+            array('chateaux', 'chateau'),
+            array('echoes', array('echo', 'echoe')),
+            array('analyses', array('analys', 'analyse', 'analysis')),
+            array('theses', array('thes', 'these', 'thesis')),
+            array('foci', 'focus'),
+            array('focuses', array('focus', 'focuse', 'focusis')),
+            array('oases', array('oas', 'oase', 'oasis')),
+            array('matrices', array('matrex', 'matrix')),
+            array('matrixes', 'matrix'),
+            array('bureaus', 'bureau'),
+            array('bureaux', 'bureau'),
+            array('beaux', 'beau'),
+            array('data', array('daton', 'datum')),
+            array('phenomena', array('phenomenon', 'phenomenum')),
+            array('strata', array('straton', 'stratum')),
+            array('geese', 'goose'),
+            array('teeth', 'tooth'),
+            array('antennae', 'antenna'),
+            array('antennas', 'antenna'),
+            array('houses', array('hous', 'house', 'housis')),
+            array('arches', array('arch', 'arche')),
+            array('atlases', array('atlas', 'atlase', 'atlasis')),
+            array('batches', array('batch', 'batche')),
+            array('bushes', array('bush', 'bushe')),
+            array('buses', array('bus', 'buse', 'busis')),
+            array('calves', 'calf'),
+            array('circuses', array('circus', 'circuse', 'circusis')),
+            array('crises', array('cris', 'crise', 'crisis')),
+            array('dwarves', 'dwarf'),
+            array('elves', 'elf'),
+            array('emphases', array('emphas', 'emphase', 'emphasis')),
+            array('faxes', 'fax'),
+            array('halves', 'half'),
+            array('heroes', array('hero', 'heroe')),
+            array('hoaxes', 'hoax'),
+            array('irises', array('iris', 'irise', 'irisis')),
+            array('kisses', array('kiss', 'kisse', 'kissis')),
+            array('knives', 'knife'),
+            array('lives', 'life'),
+            array('lice', 'louse'),
+            array('mice', 'mouse'),
+            array('neuroses', array('neuros', 'neurose', 'neurosis')),
+            array('plateaux', 'plateau'),
+            array('poppies', 'poppy'),
+            array('quizzes', 'quiz'),
+            array('scarves', 'scarf'),
+            array('spies', 'spy'),
+            array('stories', 'story'),
+            array('syllabi', 'syllabus'),
+            array('thieves', 'thief'),
+            array('waltzes', array('waltz', 'waltze')),
+            array('wharves', 'wharf'),
+            array('wives', 'wife'),
+            array('ions', 'ion'),
+            array('bases', array('bas', 'base', 'basis')),
+            array('cars', 'car'),
+            array('cassettes', array('cassett', 'cassette')),
+            array('lamps', 'lamp'),
+            array('hats', 'hat'),
+            array('cups', 'cup'),
+            array('boxes', 'box'),
+            array('sandwiches', array('sandwich', 'sandwiche')),
+            array('suitcases', array('suitcas', 'suitcase', 'suitcasis')),
+            array('roses', array('ros', 'rose', 'rosis')),
+            array('garages', array('garag', 'garage')),
+            array('shoes', array('sho', 'shoe')),
+            array('days', 'day'),
+            array('boys', 'boy'),
+            array('roofs', 'roof'),
+            array('cliffs', 'cliff'),
+            array('sheriffs', 'sheriff'),
+            array('discos', 'disco'),
+            array('pianos', 'piano'),
+            array('photos', 'photo'),
+            array('trees', array('tre', 'tree')),
+            array('bees', array('be', 'bee')),
+            array('cheeses', array('chees', 'cheese', 'cheesis')),
+            array('radii', 'radius'),
+
+            // test casing: if the first letter was uppercase, it should remain so
+            array('Men', 'Man'),
+            array('GrandChildren', 'GrandChild'),
+            array('SubTrees', array('SubTre', 'SubTree')),
+        );
+    }
+
+    /**
+     * @dataProvider singularifyProvider
+     */
+    public function testSingularify($plural, $singular)
+    {
+        $this->assertEquals($singular, FormUtil::singularify($plural));
+    }
 }