[Form] Fixed MergeCollectionListener for the case that the form's data is updated by the data mapper (as happening in CollectionType)

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

@@ -13,6 +13,7 @@ namespace Symfony\Component\Form\Extension\Core\EventListener;
 
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\Form\FormEvents;
+use Symfony\Component\Form\Event\DataEvent;
 use Symfony\Component\Form\Event\FilterDataEvent;
 use Symfony\Component\Form\Exception\UnexpectedTypeException;
 use Symfony\Component\Form\Exception\FormException;
@@ -23,6 +24,21 @@ use Symfony\Component\Form\Util\FormUtil;
  */
 class MergeCollectionListener implements EventSubscriberInterface
 {
+    /**
+     * Strategy for merging the new collection into the old collection
+     *
+     * @var integer
+     */
+    const MERGE_NORMAL = 1;
+
+    /**
+     * Strategy for calling add/remove methods on the parent data for all
+     * new/removed elements in the new collection
+     *
+     * @var integer
+     */
+    const MERGE_INTO_PARENT = 2;
+
     /**
      * Whether elements may be added to the collection
      * @var Boolean
@@ -39,7 +55,7 @@ class MergeCollectionListener implements EventSubscriberInterface
      * Whether to search for and use adder and remover methods
      * @var Boolean
      */
-    private $useAccessors;
+    private $mergeStrategy;
 
     /**
      * The name of the adder method to look for
@@ -53,35 +69,91 @@ class MergeCollectionListener implements EventSubscriberInterface
      */
     private $removeMethod;
 
-    public function __construct($allowAdd = false, $allowDelete = false, $useAccessors = true, $addMethod = null, $removeMethod = null)
+    /**
+     * A copy of the data before starting binding for this form
+     * @var mixed
+     */
+    private $dataSnapshot;
+
+    /**
+     * Creates a new listener.
+     *
+     * @param  Boolean $allowAdd      Whether values might be added to the
+     *                                collection.
+     * @param  Boolean $allowDelete   Whether values might be removed from the
+     *                                collection.
+     * @param  integer $mergeStrategy Which strategy to use for merging the
+     *                                bound collection with the original
+     *                                collection. Might be any combination of
+     *                                MERGE_NORMAL and MERGE_INTO_PARENT.
+     *                                MERGE_INTO_PARENT has precedence over
+     *                                MERGE_NORMAL if an adder/remover method
+     *                                is found. The default strategy is to use
+     *                                both strategies.
+     * @param  string $addMethod      The name of the adder method to use. If
+     *                                not given, the listener tries to discover
+     *                                the method automatically.
+     * @param  string $removeMethod   The name of the remover method to use. If
+     *                                not given, the listener tries to discover
+     *                                the method automatically.
+     *
+     * @throws FormException          If the given strategy is invalid.
+     */
+    public function __construct($allowAdd = false, $allowDelete = false, $mergeStrategy = null, $addMethod = null, $removeMethod = null)
     {
+        if ($mergeStrategy && !($mergeStrategy & (self::MERGE_NORMAL | self::MERGE_INTO_PARENT))) {
+            throw new FormException('The merge strategy needs to be at least MERGE_NORMAL or MERGE_INTO_PARENT');
+        }
+
         $this->allowAdd = $allowAdd;
         $this->allowDelete = $allowDelete;
-        $this->useAccessors = $useAccessors;
+        $this->mergeStrategy = $mergeStrategy ?: self::MERGE_NORMAL | self::MERGE_INTO_PARENT;
         $this->addMethod = $addMethod;
         $this->removeMethod = $removeMethod;
     }
 
     static public function getSubscribedEvents()
     {
-        return array(FormEvents::BIND_NORM_DATA => 'onBindNormData');
+        return array(
+            FormEvents::PRE_BIND => 'preBind',
+            FormEvents::BIND_NORM_DATA => 'onBindNormData',
+        );
+    }
+
+    public function preBind(DataEvent $event)
+    {
+        // Get a snapshot of the current state of the normalized data
+        // to compare against later
+        $this->dataSnapshot = $event->getForm()->getNormData();
+
+        if (is_object($this->dataSnapshot)) {
+            // Make sure the snapshot remains stable and doesn't change
+            $this->dataSnapshot = clone $this->dataSnapshot;
+        }
+
+        if (null !== $this->dataSnapshot && !is_array($this->dataSnapshot) && !($this->dataSnapshot instanceof \Traversable && $this->dataSnapshot instanceof \ArrayAccess)) {
+            throw new UnexpectedTypeException($this->dataSnapshot, 'array or (\Traversable and \ArrayAccess)');
+        }
     }
 
     public function onBindNormData(FilterDataEvent $event)
     {
-        $originalData = $event->getForm()->getData();
+        $originalData = $event->getForm()->getNormData();
 
         // If we are not allowed to change anything, return immediately
         if (!$this->allowAdd && !$this->allowDelete) {
+            // Don't set to the snapshot as then we are switching from the
+            // original object to its copy, which might break things
             $event->setData($originalData);
             return;
         }
 
         $form = $event->getForm();
         $data = $event->getData();
-        $parentData = $form->hasParent() ? $form->getParent()->getData() : null;
+        $parentData = $form->hasParent() ? $form->getParent()->getClientData() : null;
         $addMethod = null;
         $removeMethod = null;
+        $getMethod = null;
 
         if (null === $data) {
             $data = array();
@@ -96,24 +168,35 @@ class MergeCollectionListener implements EventSubscriberInterface
         }
 
         // Check if the parent has matching methods to add/remove items
-        if ($this->useAccessors && is_object($parentData)) {
+        if (($this->mergeStrategy & self::MERGE_INTO_PARENT) && is_object($parentData)) {
+            $plural = ucfirst($form->getName());
             $reflClass = new \ReflectionClass($parentData);
             $addMethodNeeded = $this->allowAdd && !$this->addMethod;
             $removeMethodNeeded = $this->allowDelete && !$this->removeMethod;
 
             // Any of the two methods is required, but not yet known
             if ($addMethodNeeded || $removeMethodNeeded) {
-                $singulars = (array) FormUtil::singularify(ucfirst($form->getName()));
+                $singulars = (array) FormUtil::singularify($plural);
 
                 foreach ($singulars as $singular) {
                     // Try to find adder, but don't override preconfigured one
                     if ($addMethodNeeded) {
-                        $addMethod = $this->checkMethod($reflClass, 'add' . $singular);
+                        $addMethod = 'add' . $singular;
+
+                        // False alert
+                        if (!$this->isAccessible($reflClass, $addMethod, 1)) {
+                            $addMethod = null;
+                        }
                     }
 
                     // Try to find remover, but don't override preconfigured one
                     if ($removeMethodNeeded) {
-                        $removeMethod = $this->checkMethod($reflClass, 'remove' . $singular);
+                        $removeMethod = 'remove' . $singular;
+
+                        // False alert
+                        if (!$this->isAccessible($reflClass, $removeMethod, 1)) {
+                            $removeMethod = null;
+                        }
                     }
 
                     // Found all that we need. Abort search.
@@ -129,12 +212,12 @@ class MergeCollectionListener implements EventSubscriberInterface
 
             // Set preconfigured adder
             if ($this->allowAdd && $this->addMethod) {
-                $addMethod = $this->checkMethod($reflClass, $this->addMethod);
+                $addMethod = $this->addMethod;
 
-                if (!$addMethod) {
+                if (!$this->isAccessible($reflClass, $addMethod, 1)) {
                     throw new FormException(sprintf(
-                        'The method "%s" could not be found on class %s',
+                        'The public method "%s" could not be found on class %s',
-                        $this->addMethod,
+                        $addMethod,
                         $reflClass->getName()
                     ));
                 }
@@ -142,25 +225,36 @@ class MergeCollectionListener implements EventSubscriberInterface
 
             // Set preconfigured remover
             if ($this->allowDelete && $this->removeMethod) {
-                $removeMethod = $this->checkMethod($reflClass, $this->removeMethod);
+                $removeMethod = $this->removeMethod;
 
-                if (!$removeMethod) {
+                if (!$this->isAccessible($reflClass, $removeMethod, 1)) {
                     throw new FormException(sprintf(
-                        'The method "%s" could not be found on class %s',
+                        'The public method "%s" could not be found on class %s',
-                        $this->removeMethod,
+                        $removeMethod,
+                        $reflClass->getName()
+                    ));
+                }
+            }
+
+            if ($addMethod || $removeMethod) {
+                $getMethod = 'get' . $plural;
+
+                if (!$this->isAccessible($reflClass, $getMethod, 0)) {
+                    throw new FormException(sprintf(
+                        'The public method "%s" could not be found on class %s',
+                        $getMethod,
                         $reflClass->getName()
                     ));
                 }
             }
         }
 
-        // Check which items are in $data that are not in $originalData and
+        // Calculate delta between $data and the snapshot created in PRE_BIND
-        // vice versa
         $itemsToDelete = array();
         $itemsToAdd = is_object($data) ? clone $data : $data;
 
-        if ($originalData) {
+        if ($this->dataSnapshot) {
-            foreach ($originalData as $originalKey => $originalItem) {
+            foreach ($this->dataSnapshot as $originalItem) {
                 foreach ($data as $key => $item) {
                     if ($item === $originalItem) {
                         // Item found, next original item
@@ -170,7 +264,12 @@ class MergeCollectionListener implements EventSubscriberInterface
                 }
 
                 // Item not found, remember for deletion
-                $itemsToDelete[$originalKey] = $originalItem;
+                foreach ($originalData as $key => $item) {
+                    if ($item === $originalItem) {
+                        $itemsToDelete[$key] = $item;
+                        continue 2;
+                    }
+                }
             }
         }
 
@@ -187,43 +286,47 @@ class MergeCollectionListener implements EventSubscriberInterface
                     $parentData->$addMethod($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) {
+            $event->setData($parentData->$getMethod());
-                foreach ($itemsToAdd as $key => $item) {
+        } elseif ($this->mergeStrategy & self::MERGE_NORMAL) {
-                    if (!isset($originalData[$key])) {
+            if (!$originalData) {
-                        $originalData[$key] = $item;
+                // No original data was set. Set it if allowed
-                    } else {
+                if ($this->allowAdd) {
-                        $originalData[] = $item;
+                    $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);
+            $event->setData($originalData);
+        }
     }
 
-    private function checkMethod(\ReflectionClass $reflClass, $methodName) {
+    private function isAccessible(\ReflectionClass $reflClass, $methodName, $numberOfRequiredParameters) {
         if ($reflClass->hasMethod($methodName)) {
             $method = $reflClass->getMethod($methodName);
 
-            if ($method->isPublic() && $method->getNumberOfRequiredParameters() === 1) {
+            if ($method->isPublic() && $method->getNumberOfRequiredParameters() === $numberOfRequiredParameters) {
-                return $methodName;
+                return true;
             }
         }
 
-        return null;
+        return false;
     }
 }

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

@@ -81,10 +81,7 @@ class ChoiceType extends AbstractType
 
         if ($options['expanded']) {
             if ($options['multiple']) {
-                $builder
+                $builder->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']));
-                    ->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']))
-                    ->addEventSubscriber(new MergeCollectionListener(true, true))
-                ;
             } else {
                 $builder
                     ->appendClientTransformer(new ChoiceToBooleanArrayTransformer($options['choice_list']))
@@ -93,24 +90,31 @@ class ChoiceType extends AbstractType
             }
         } else {
             if ($options['multiple']) {
-                $builder
+                $builder->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']));
-                    ->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['add_method'],
-                        $options['remove_method']
-                    ))
-                ;
             } else {
                 $builder->appendClientTransformer(new ChoiceToValueTransformer($options['choice_list']));
             }
         }
+
+        if ($options['multiple']) {
+            // Make sure the collection created during the client->norm
+            // transformation is merged back into the original collection
+            $mergeStrategy = MergeCollectionListener::MERGE_NORMAL;
+
+            // Enable support for adders/removers unless "by_reference" is disabled
+            // (explicit calling of the setter is desired)
+            if ($options['by_reference']) {
+                $mergeStrategy = $mergeStrategy | MergeCollectionListener::MERGE_INTO_PARENT;
+            }
+
+            $builder->addEventSubscriber(new MergeCollectionListener(
+                true,
+                true,
+                $mergeStrategy,
+                $options['add_method'],
+                $options['remove_method']
+            ));
+        }
     }
 
     /**

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

@@ -38,23 +38,23 @@ 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['add_method'],
-            $options['remove_method']
-        );
-
         $builder
             ->addEventSubscriber($resizeListener)
-            ->addEventSubscriber($mergeListener)
             ->setAttribute('allow_add', $options['allow_add'])
             ->setAttribute('allow_delete', $options['allow_delete'])
         ;
+
+        // Enable support for adders/removers unless "by_reference" is disabled
+        // (explicit calling of the setter is desired)
+        if ($options['by_reference']) {
+            $builder->addEventSubscriber(new MergeCollectionListener(
+                $options['allow_add'],
+                $options['allow_delete'],
+                MergeCollectionListener::MERGE_INTO_PARENT,
+                $options['add_method'],
+                $options['remove_method']
+            ));
+        }
     }
 
     /**

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

@@ -22,6 +22,8 @@ class MergeCollectionListenerTest_Car
     public function addAxis($axis) {}
 
     public function removeAxis($axis) {}
+
+    public function getAxes() {}
 }
 
 class MergeCollectionListenerTest_CarCustomNames
@@ -29,16 +31,22 @@ class MergeCollectionListenerTest_CarCustomNames
     public function foo($axis) {}
 
     public function bar($axis) {}
+
+    public function getAxes() {}
 }
 
 class MergeCollectionListenerTest_CarOnlyAdder
 {
     public function addAxis($axis) {}
+
+    public function getAxes() {}
 }
 
 class MergeCollectionListenerTest_CarOnlyRemover
 {
     public function removeAxis($axis) {}
+
+    public function getAxes() {}
 }
 
 abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
@@ -76,17 +84,55 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         return $this->getMock('Symfony\Tests\Component\Form\FormInterface');
     }
 
+    public function getModesWithNormal()
+    {
+        return array(
+            array(MergeCollectionListener::MERGE_NORMAL),
+            array(MergeCollectionListener::MERGE_NORMAL | MergeCollectionListener::MERGE_INTO_PARENT),
+        );
+    }
+
+    public function getModesWithMergeIntoParent()
+    {
+        return array(
+            array(MergeCollectionListener::MERGE_INTO_PARENT),
+            array(MergeCollectionListener::MERGE_INTO_PARENT | MergeCollectionListener::MERGE_NORMAL),
+        );
+    }
+
+    public function getModesWithoutMergeIntoParent()
+    {
+        return array(
+            array(MergeCollectionListener::MERGE_NORMAL),
+        );
+    }
+
+    public function getInvalidModes()
+    {
+        return array(
+            // 0 is a valid mode, because it is treated as "default" (=3)
+            array(4),
+            array(8),
+        );
+    }
+
     abstract protected function getData(array $data);
 
-    public function testAddExtraEntriesIfAllowAdd()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testAddExtraEntriesIfAllowAdd($mode)
     {
         $originalData = $this->getData(array(1 => 'second'));
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
+        $listener = new MergeCollectionListener(true, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, false);
         $listener->onBindNormData($event);
 
         // The original object was modified
@@ -98,15 +144,21 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($newData, $event->getData());
     }
 
-    public function testAddExtraEntriesIfAllowAddDontOverwriteExistingIndices()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testAddExtraEntriesIfAllowAddDontOverwriteExistingIndices($mode)
     {
         $originalData = $this->getData(array(1 => 'first'));
         $newData = $this->getData(array(0 => 'first', 1 => 'second'));
 
+        $listener = new MergeCollectionListener(true, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, false);
         $listener->onBindNormData($event);
 
         // The original object was modified
@@ -118,16 +170,22 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($this->getData(array(1 => 'first', 2 => 'second')), $event->getData());
     }
 
-    public function testDoNothingIfNotAllowAdd()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testDoNothingIfNotAllowAdd($mode)
     {
         $originalDataArray = array(1 => 'second');
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
+        $listener = new MergeCollectionListener(false, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
         $listener->onBindNormData($event);
 
         // We still have the original object
@@ -139,15 +197,21 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($this->getData($originalDataArray), $event->getData());
     }
 
-    public function testRemoveMissingEntriesIfAllowDelete()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testRemoveMissingEntriesIfAllowDelete($mode)
     {
         $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
         $newData = $this->getData(array(1 => 'second'));
 
+        $listener = new MergeCollectionListener(false, true, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, true);
         $listener->onBindNormData($event);
 
         // The original object was modified
@@ -159,16 +223,22 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($newData, $event->getData());
     }
 
-    public function testDoNothingIfNotAllowDelete()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testDoNothingIfNotAllowDelete($mode)
     {
         $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third');
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(1 => 'second'));
 
+        $listener = new MergeCollectionListener(false, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
         $listener->onBindNormData($event);
 
         // We still have the original object
@@ -181,59 +251,81 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
+     * @dataProvider getModesWithNormal
      * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
      */
-    public function testRequireArrayOrTraversable()
+    public function testRequireArrayOrTraversable($mode)
     {
         $newData = 'no array or traversable';
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, false);
+        $listener = new MergeCollectionListener(true, false, $mode);
         $listener->onBindNormData($event);
     }
 
-    public function testDealWithNullData()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testDealWithNullData($mode)
     {
         $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
         $newData = null;
 
+        $listener = new MergeCollectionListener(false, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
         $listener->onBindNormData($event);
 
         $this->assertSame($originalData, $event->getData());
     }
 
-    public function testDealWithNullOriginalDataIfAllowAdd()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testDealWithNullOriginalDataIfAllowAdd($mode)
     {
         $originalData = null;
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
+        $listener = new MergeCollectionListener(true, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, false);
         $listener->onBindNormData($event);
 
         $this->assertSame($newData, $event->getData());
     }
 
-    public function testDontDealWithNullOriginalDataIfNotAllowAdd()
+    /**
+     * @dataProvider getModesWithNormal
+     */
+    public function testDontDealWithNullOriginalDataIfNotAllowAdd($mode)
     {
         $originalData = null;
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
+        $listener = new MergeCollectionListener(false, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
         $listener->onBindNormData($event);
 
         $this->assertNull($event->getData());
     }
 
-    public function testCallAdderIfAllowAdd()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testCallAdderIfAllowAdd($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_CarOnlyAdder');
         $parentForm = $this->getForm('car');
@@ -244,29 +336,77 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
+        $listener = new MergeCollectionListener(true, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
         $parentData->expects($this->at(0))
             ->method('addAxis')
             ->with('first');
         $parentData->expects($this->at(1))
             ->method('addAxis')
             ->with('third');
+        $parentData->expects($this->at(2))
+            ->method('getAxes')
+            ->will($this->returnValue('RESULT'));
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, false);
         $listener->onBindNormData($event);
 
-        if (is_object($originalData)) {
+        $this->assertEquals('RESULT', $event->getData());
-            $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()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testCallAdderIfOriginalDataAlreadyModified($mode)
+    {
+        $parentData = $this->getMock(__CLASS__ . '_CarOnlyAdder');
+        $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'));
+
+        $listener = new MergeCollectionListener(true, false, $mode);
+
+        $this->form->setData($originalData);
+
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
+        // The form already contains the new data
+        // This happens if the data mapper maps the data of the child forms
+        // back into the original collection.
+        // The original collection is then both already modified and passed
+        // as event argument.
+        $this->form->setData($newData);
+
+        $parentData->expects($this->at(0))
+            ->method('addAxis')
+            ->with('first');
+        $parentData->expects($this->at(1))
+            ->method('addAxis')
+            ->with('third');
+        $parentData->expects($this->at(2))
+            ->method('getAxes')
+            ->will($this->returnValue('RESULT'));
+
+        $event = new FilterDataEvent($this->form, $newData);
+        $listener->onBindNormData($event);
+
+        $this->assertEquals('RESULT', $event->getData());
+    }
+
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testDontCallAdderIfNotAllowAdd($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_Car');
         $parentForm = $this->getForm('car');
@@ -277,13 +417,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
+        $listener = new MergeCollectionListener(false, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
         $parentData->expects($this->never())
             ->method('addAxis');
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
         $listener->onBindNormData($event);
 
         if (is_object($originalData)) {
@@ -294,7 +438,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($this->getData($originalDataArray), $event->getData());
     }
 
-    public function testDontCallAdderIfNotUseAccessors()
+    /**
+     * @dataProvider getModesWithoutMergeIntoParent
+     */
+    public function testDontCallAdderIfNotMergeIntoParent($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_Car');
         $parentForm = $this->getForm('car');
@@ -304,13 +451,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData(array(1 => 'second'));
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
+        $listener = new MergeCollectionListener(true, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
         $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)) {
@@ -321,7 +472,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($newData, $event->getData());
     }
 
-    public function testCallRemoverIfAllowDelete()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testCallRemoverIfAllowDelete($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_CarOnlyRemover');
         $parentForm = $this->getForm('car');
@@ -332,29 +486,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(1 => 'second'));
 
+        $listener = new MergeCollectionListener(false, true, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
         $parentData->expects($this->at(0))
             ->method('removeAxis')
             ->with('first');
         $parentData->expects($this->at(1))
             ->method('removeAxis')
             ->with('third');
+        $parentData->expects($this->at(2))
+            ->method('getAxes')
+            ->will($this->returnValue('RESULT'));
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, true);
         $listener->onBindNormData($event);
 
-        if (is_object($originalData)) {
+        $this->assertEquals('RESULT', $event->getData());
-            $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()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testDontCallRemoverIfNotAllowDelete($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_Car');
         $parentForm = $this->getForm('car');
@@ -365,13 +523,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(1 => 'second'));
 
+        $listener = new MergeCollectionListener(false, false, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
         $parentData->expects($this->never())
             ->method('removeAxis');
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
         $listener->onBindNormData($event);
 
         if (is_object($originalData)) {
@@ -382,7 +544,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($this->getData($originalDataArray), $event->getData());
     }
 
-    public function testDontCallRemoverIfNotUseAccessors()
+    /**
+     * @dataProvider getModesWithoutMergeIntoParent
+     */
+    public function testDontCallRemoverIfNotMergeIntoParent($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_Car');
         $parentForm = $this->getForm('car');
@@ -392,13 +557,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
         $newData = $this->getData(array(1 => 'second'));
 
+        $listener = new MergeCollectionListener(false, true, $mode);
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
         $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)) {
@@ -409,7 +578,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($newData, $event->getData());
     }
 
-    public function testCallAdderAndDeleterIfAllowAll()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testCallAdderAndDeleterIfAllowAll($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_Car');
         $parentForm = $this->getForm('car');
@@ -420,29 +592,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first'));
 
+        $listener = new MergeCollectionListener(true, true, $mode);
+
         $this->form->setData($originalData);
 
-        $parentData->expects($this->once())
+        $event = new DataEvent($this->form, $newData);
-            ->method('addAxis')
+        $listener->preBind($event);
-            ->with('first');
+
-        $parentData->expects($this->once())
+        $parentData->expects($this->at(0))
             ->method('removeAxis')
             ->with('second');
+        $parentData->expects($this->at(1))
+            ->method('addAxis')
+            ->with('first');
+        $parentData->expects($this->at(2))
+            ->method('getAxes')
+            ->will($this->returnValue('RESULT'));
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, true, true);
         $listener->onBindNormData($event);
 
-        if (is_object($originalData)) {
+        $this->assertEquals('RESULT', $event->getData());
-            $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 testCallAccessorsWithCustomNames()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testCallAccessorsWithCustomNames($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_CarCustomNames');
         $parentForm = $this->getForm('car');
@@ -453,29 +629,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first'));
 
+        $listener = new MergeCollectionListener(true, true, $mode, 'foo', 'bar');
+
         $this->form->setData($originalData);
 
-        $parentData->expects($this->once())
+        $event = new DataEvent($this->form, $newData);
-            ->method('foo')
+        $listener->preBind($event);
-            ->with('first');
+
-        $parentData->expects($this->once())
+        $parentData->expects($this->at(0))
             ->method('bar')
             ->with('second');
+        $parentData->expects($this->at(1))
+            ->method('foo')
+            ->with('first');
+        $parentData->expects($this->at(2))
+            ->method('getAxes')
+            ->will($this->returnValue('RESULT'));
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, true, true, 'foo', 'bar');
         $listener->onBindNormData($event);
 
-        if (is_object($originalData)) {
+        $this->assertEquals('RESULT', $event->getData());
-            $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 testDontCallAdderWithCustomNameIfDisallowed()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testDontCallAdderWithCustomNameIfDisallowed($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_CarCustomNames');
         $parentForm = $this->getForm('car');
@@ -486,27 +666,32 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first'));
 
+        $listener = new MergeCollectionListener(false, true, $mode, 'foo', 'bar');
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
         $parentData->expects($this->never())
             ->method('foo');
-        $parentData->expects($this->once())
+        $parentData->expects($this->at(0))
             ->method('bar')
             ->with('second');
+        $parentData->expects($this->at(1))
+            ->method('getAxes')
+            ->will($this->returnValue('RESULT'));
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, true, true, 'foo', 'bar');
         $listener->onBindNormData($event);
 
-        if (is_object($originalData)) {
+        $this->assertEquals('RESULT', $event->getData());
-            $this->assertSame($originalData, $event->getData());
-        }
-
-        // The data was not modified
-        $this->assertEquals($this->getData($originalDataArray), $event->getData());
     }
 
-    public function testDontCallRemoverWithCustomNameIfDisallowed()
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testDontCallRemoverWithCustomNameIfDisallowed($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_CarCustomNames');
         $parentForm = $this->getForm('car');
@@ -517,30 +702,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first'));
 
+        $listener = new MergeCollectionListener(true, false, $mode, 'foo', 'bar');
+
         $this->form->setData($originalData);
 
-        $parentData->expects($this->once())
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
+
+        $parentData->expects($this->at(0))
             ->method('foo')
             ->with('first');
         $parentData->expects($this->never())
             ->method('bar');
+        $parentData->expects($this->at(1))
+            ->method('getAxes')
+            ->will($this->returnValue('RESULT'));
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, false, true, 'foo', 'bar');
         $listener->onBindNormData($event);
 
-        if (is_object($originalData)) {
+        $this->assertEquals('RESULT', $event->getData());
-            $this->assertSame($originalData, $event->getData());
-        }
-
-        // The data was not modified
-        $this->assertEquals($this->getData($originalDataArray), $event->getData());
     }
 
     /**
+     * @dataProvider getModesWithMergeIntoParent
      * @expectedException Symfony\Component\Form\Exception\FormException
      */
-    public function testThrowExceptionIfInvalidAdder()
+    public function testThrowExceptionIfInvalidAdder($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_CarCustomNames');
         $parentForm = $this->getForm('car');
@@ -550,17 +738,21 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData(array(1 => 'second'));
         $newData = $this->getData(array(0 => 'first'));
 
+        $listener = new MergeCollectionListener(true, false, $mode, 'doesnotexist');
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(true, false, true, 'doesnotexist');
         $listener->onBindNormData($event);
     }
 
     /**
+     * @dataProvider getModesWithMergeIntoParent
      * @expectedException Symfony\Component\Form\Exception\FormException
      */
-    public function testThrowExceptionIfInvalidRemover()
+    public function testThrowExceptionIfInvalidRemover($mode)
     {
         $parentData = $this->getMock(__CLASS__ . '_CarCustomNames');
         $parentForm = $this->getForm('car');
@@ -570,10 +762,22 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $originalData = $this->getData(array(1 => 'second'));
         $newData = $this->getData(array(0 => 'first'));
 
+        $listener = new MergeCollectionListener(false, true, $mode, null, 'doesnotexist');
+
         $this->form->setData($originalData);
 
+        $event = new DataEvent($this->form, $newData);
+        $listener->preBind($event);
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, true, true, null, 'doesnotexist');
         $listener->onBindNormData($event);
     }
+
+    /**
+     * @dataProvider getInvalidModes
+     * @expectedException Symfony\Component\Form\Exception\FormException
+     */
+    public function testThrowExceptionIfInvalidMode($mode)
+    {
+        new MergeCollectionListener(true, true, $mode);
+    }
 }