[Form] Fixed MergeCollectionListener when used with a custom property…

… path
symfony · Feb 9, 2012 · da2447e · da2447e
1 parent b56502f
commit da2447e
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 18 deletions.
diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php
@@ -18,6 +18,7 @@
 use Symfony\Component\Form\Exception\UnexpectedTypeException;
 use Symfony\Component\Form\Exception\FormException;
 use Symfony\Component\Form\Util\FormUtil;
+use Symfony\Component\Form\Util\PropertyPath;
 
 /**
  * @author Bernhard Schussek <bschussek@gmail.com>
@@ -150,10 +151,31 @@ public function onBindNormData(FilterDataEvent $event)
 
         $form = $event->getForm();
         $data = $event->getData();
-        $parentData = $form->hasParent() ? $form->getParent()->getClientData() : null;
+        $childPropertyPath = null;
+        $parentData = null;
         $addMethod = null;
         $removeMethod = null;
-        $getMethod = null;
+        $propertyPath = null;
+        $plural = null;
+
+        if ($form->hasParent() && $form->getAttribute('property_path')) {
+            $propertyPath = new PropertyPath($form->getAttribute('property_path'));
+            $childPropertyPath = $propertyPath;
+            $parentData = $form->getParent()->getClientData();
+            $lastElement = $propertyPath->getElement($propertyPath->getLength() - 1);
+
+            // If the property path contains more than one element, the parent
+            // data is the object at the parent property path
+            if ($propertyPath->getLength() > 1) {
+                $parentData = $propertyPath->getParent()->getValue($parentData);
+
+                // Property path relative to $parentData
+                $childPropertyPath = new PropertyPath($lastElement);
+            }
+
+            // The plural form is the last element of the property path
+            $plural = ucfirst($lastElement);
+        }
 
         if (null === $data) {
             $data = array();
@@ -169,7 +191,6 @@ public function onBindNormData(FilterDataEvent $event)
 
         // Check if the parent has matching methods to add/remove items
         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;
@@ -235,18 +256,6 @@ public function onBindNormData(FilterDataEvent $event)
                     ));
                 }
             }
-
-            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()
-                    ));
-                }
-            }
         }
 
         // Calculate delta between $data and the snapshot created in PRE_BIND
@@ -287,7 +296,7 @@ public function onBindNormData(FilterDataEvent $event)
                 }
             }
 
-            $event->setData($parentData->$getMethod());
+            $event->setData($childPropertyPath->getValue($parentData));
         } elseif ($this->mergeStrategy & self::MERGE_NORMAL) {
             if (!$originalData) {
                 // No original data was set. Set it if allowed

diff --git a/src/Symfony/Component/Form/Util/PropertyPath.php b/src/Symfony/Component/Form/Util/PropertyPath.php
@@ -110,6 +110,16 @@ public function __toString()
         return $this->string;
     }
 
+    /**
+     * Returns the length of the property path.
+     *
+     * @return integer
+     */
+    public function getLength()
+    {
+        return $this->length;
+    }
+
     /**
      * Returns the parent property path.
      *

diff --git a/...Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php b/...Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php
@@ -49,6 +49,20 @@ public function removeAxis($axis) {}
     public function getAxes() {}
 }
 
+class MergeCollectionListenerTest_CompositeCar
+{
+    public function getStructure() {}
+}
+
+class MergeCollectionListenerTest_CarStructure
+{
+    public function addAxis($axis) {}
+
+    public function removeAxis($axis) {}
+
+    public function getAxes() {}
+}
+
 abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
 {
     private $dispatcher;
@@ -74,9 +88,11 @@ protected function getBuilder($name = 'name')
         return new FormBuilder($name, $this->factory, $this->dispatcher);
     }
 
-    protected function getForm($name = 'name')
+    protected function getForm($name = 'name', $propertyPath = null)
     {
-        return $this->getBuilder($name)->getForm();
+        $propertyPath = $propertyPath ?: $name;
+
+        return $this->getBuilder($name)->setAttribute('property_path', $propertyPath)->getForm();
     }
 
     protected function getMockForm()
@@ -359,6 +375,51 @@ public function testCallAdderIfAllowAdd($mode)
         $this->assertEquals('RESULT', $event->getData());
     }
 
+    /**
+     * @dataProvider getModesWithMergeIntoParent
+     */
+    public function testCallAdderIfCustomPropertyPath($mode)
+    {
+        $this->form = $this->getForm('structure_axes', 'structure.axes');
+
+        $parentData = $this->getMock(__CLASS__ . '_CompositeCar');
+        $parentForm = $this->getForm('car');
+        $parentForm->setData($parentData);
+        $parentForm->add($this->form);
+
+        $modifData = $this->getMock(__CLASS__ . '_CarStructure');
+
+        $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);
+
+        $parentData->expects($this->once())
+            ->method('getStructure')
+            ->will($this->returnValue($modifData));
+
+        $modifData->expects($this->at(0))
+            ->method('addAxis')
+            ->with('first');
+        $modifData->expects($this->at(1))
+            ->method('addAxis')
+            ->with('third');
+        $modifData->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
      */