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

Author: webmozart

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

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.
  *

tests/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
  */