From da2447e1184716a4728d9d965e8d723dd80e45e5 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 9 Feb 2012 17:03:48 +0100 Subject: [PATCH] [Form] Fixed MergeCollectionListener when used with a custom property path --- .../EventListener/MergeCollectionListener.php | 41 +++++++----- .../Component/Form/Util/PropertyPath.php | 10 +++ .../MergeCollectionListenerTest.php | 65 ++++++++++++++++++- 3 files changed, 98 insertions(+), 18 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php index c7b5829b5f3a..b80c05c0a14d 100644 --- 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 @@ -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 index f29aa3d5156c..44087d933b20 100644 --- 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/tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php b/tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php index e29b40dff1b0..11256d77ee0e 100644 --- a/tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php +++ b/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 */