[Form] Made prefix of adder and remover method configurable. Adders and removers are not called if "by_reference" is disabled.

CHANGELOG-2.1.md

@@ -195,9 +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
+ * 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/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php

@@ -35,10 +35,31 @@ class MergeCollectionListener implements EventSubscriberInterface
      */
     private $allowDelete;
 
-    public function __construct($allowAdd = false, $allowDelete = false)
+    /**
+     * 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()
@@ -68,14 +89,14 @@ class MergeCollectionListener implements EventSubscriberInterface
         }
 
         // Check if the parent has matching methods to add/remove items
-        if (is_object($parentData)) {
+        if ($this->useAccessors && is_object($parentData)) {
             $plural = ucfirst($form->getName());
             $singulars = (array) FormUtil::singularify($plural);
             $reflClass = new \ReflectionClass($parentData);
 
             foreach ($singulars as $singular) {
-                $adderName = 'add' . $singular;
-                $removerName = 'remove' . $singular;
+                $adderName = $this->adderPrefix . $singular;
+                $removerName = $this->removerPrefix . $singular;
 
                 if ($reflClass->hasMethod($adderName) && $reflClass->hasMethod($removerName)) {
                     $adder = $reflClass->getMethod($adderName);

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

@@ -95,7 +95,17 @@ class ChoiceType extends AbstractType
             if ($options['multiple']) {
                 $builder
                     ->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']))
-                    ->addEventSubscriber(new MergeCollectionListener(true, true))
+                    ->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']));
@@ -146,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

@@ -40,7 +40,13 @@ class CollectionType extends AbstractType
 
         $mergeListener = new MergeCollectionListener(
             $options['allow_add'],
-            $options['allow_delete']
+            $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
@@ -84,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',

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

@@ -24,6 +24,13 @@ class MergeCollectionListenerTest_Car
     public function removeAxis($axis) {}
 }
 
+class MergeCollectionListenerTest_CarCustomPrefix
+{
+    public function fooAxis($axis) {}
+
+    public function barAxis($axis) {}
+}
+
 abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
 {
     private $dispatcher;
@@ -219,11 +226,12 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
     public function testCallAdderIfAllowAdd()
     {
         $parentData = $this->getMock(__CLASS__ . '_Car');
-        $parentForm = $this->getForm('article');
+        $parentForm = $this->getForm('car');
         $parentForm->setData($parentData);
         $parentForm->add($this->form);
 
-        $originalData = $this->getData(array(1 => 'second'));
+        $originalDataArray = array(1 => 'second');
+        $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
 
         $this->form->setData($originalData);
@@ -239,16 +247,47 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $listener = new MergeCollectionListener(true, false);
         $listener->onBindNormData($event);
 
-        // The original object was modified
         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('article');
+        $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);
 
@@ -261,23 +300,26 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
             ->method('addAxis');
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
+        $listener = new MergeCollectionListener(true, false, false);
         $listener->onBindNormData($event);
 
-        // The original object was modified
         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('article');
+        $parentForm = $this->getForm('car');
         $parentForm->setData($parentData);
         $parentForm->add($this->form);
 
-        $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
+        $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third');
+        $originalData = $this->getData($originalDataArray);
         $newData = $this->getData(array(1 => 'second'));
 
         $this->form->setData($originalData);
@@ -293,16 +335,47 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
         $listener = new MergeCollectionListener(false, true);
         $listener->onBindNormData($event);
 
-        // The original object was modified
         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('article');
+        $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);
 
@@ -315,12 +388,47 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
             ->method('removeAxis');
 
         $event = new FilterDataEvent($this->form, $newData);
-        $listener = new MergeCollectionListener(false, false);
+        $listener = new MergeCollectionListener(false, true, false);
         $listener->onBindNormData($event);
 
-        // The original object was modified
         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());
     }
 }