From 02f61adcc54252d8828ac6629cb806bd20f8e7a1 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 2 Feb 2012 13:44:00 +0100 Subject: [PATCH] [Form] Renamed choice and collection options "adder_prefix" and "remover_prefix" to "add_method" and "remove_method" and allowed to specify full method names --- CHANGELOG-2.1.md | 3 +- .../EventListener/MergeCollectionListener.php | 119 +++++++++++------- .../Form/Extension/Core/Type/ChoiceType.php | 8 +- .../Extension/Core/Type/CollectionType.php | 8 +- .../MergeCollectionListenerTest.php | 54 ++++++-- 5 files changed, 133 insertions(+), 59 deletions(-) diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index 5799a29757..893255effe 100644 --- a/CHANGELOG-2.1.md +++ b/CHANGELOG-2.1.md @@ -198,8 +198,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c * 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 + * added options "add_method" and "remove_method" to collection and choice type ### HttpFoundation diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php index fcd7ed3099..35d086391b 100644 --- a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php @@ -11,12 +11,12 @@ namespace Symfony\Component\Form\Extension\Core\EventListener; -use Symfony\Component\Form\Util\FormUtil; - +use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\Event\FilterDataEvent; -use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Form\Exception\UnexpectedTypeException; +use Symfony\Component\Form\Exception\FormException; +use Symfony\Component\Form\Util\FormUtil; /** * @author Bernhard Schussek @@ -42,24 +42,24 @@ class MergeCollectionListener implements EventSubscriberInterface private $useAccessors; /** - * The prefix of the adder method to look for + * The name of the adder method to look for * @var string */ - private $adderPrefix; + private $addMethod; /** - * The prefix of the remover method to look for + * The name of the remover method to look for * @var string */ - private $removerPrefix; + private $removeMethod; - public function __construct($allowAdd = false, $allowDelete = false, $useAccessors = true, $adderPrefix = 'add', $removerPrefix = 'remove') + public function __construct($allowAdd = false, $allowDelete = false, $useAccessors = true, $addMethod = null, $removeMethod = null) { $this->allowAdd = $allowAdd; $this->allowDelete = $allowDelete; $this->useAccessors = $useAccessors; - $this->adderPrefix = $adderPrefix; - $this->removerPrefix = $removerPrefix; + $this->addMethod = $addMethod; + $this->removeMethod = $removeMethod; } static public function getSubscribedEvents() @@ -80,8 +80,8 @@ class MergeCollectionListener implements EventSubscriberInterface $form = $event->getForm(); $data = $event->getData(); $parentData = $form->hasParent() ? $form->getParent()->getData() : null; - $adderName = null; - $removerName = null; + $addMethod = null; + $removeMethod = null; if (null === $data) { $data = array(); @@ -97,39 +97,62 @@ class MergeCollectionListener implements EventSubscriberInterface // Check if the parent has matching methods to add/remove items if ($this->useAccessors && is_object($parentData)) { - $plural = ucfirst($form->getName()); - $singulars = (array) FormUtil::singularify($plural); $reflClass = new \ReflectionClass($parentData); - foreach ($singulars as $singular) { - $maybeAdderName = $this->adderPrefix . $singular; - $maybeRemoverName = $this->removerPrefix . $singular; + // Any of the two methods is required, but not yet known + if (($this->allowAdd && !$this->addMethod) || ($this->allowDelete && !$this->removeMethod)) { + $plural = ucfirst($form->getName()); + $singulars = (array) FormUtil::singularify($plural); - if ($this->allowAdd && $reflClass->hasMethod($maybeAdderName)) { - $adder = $reflClass->getMethod($maybeAdderName); - - if ($adder->isPublic() && $adder->getNumberOfRequiredParameters() === 1) { - $adderName = $maybeAdderName; + foreach ($singulars as $singular) { + // Try to find adder, but don't override preconfigured one + if ($this->allowAdd && !$this->addMethod) { + $addMethod = $this->checkMethod($reflClass, 'add' . $singular); } - } - if ($this->allowDelete && $reflClass->hasMethod($maybeRemoverName)) { - $remover = $reflClass->getMethod($maybeRemoverName); - - if ($remover->isPublic() && $remover->getNumberOfRequiredParameters() === 1) { - $removerName = $maybeRemoverName; + // Try to find remover, but don't override preconfigured one + if ($this->allowDelete && !$this->removeMethod) { + $removeMethod = $this->checkMethod($reflClass, 'remove' . $singular); } - } - // When we want to both add and delete, we look for an adder and - // remover with the same name - if (!($this->allowAdd && !$adderName) && !($this->allowDelete && !$removerName)) { - break; - } + $addMethodFound = !$this->allowAdd || $addMethod || $this->addMethod; + $removeMethodFound = !$this->allowDelete || $removeMethod || $this->removeMethod; - // False alert - $adderName = null; - $removerName = null; + // Found all that we need. Abort search. + if ($addMethodFound && $removeMethodFound) { + break; + } + + // False alert + $addMethod = null; + $removeMethod = null; + } + } + + // Set preconfigured adder + if ($this->addMethod) { + $addMethod = $this->checkMethod($reflClass, $this->addMethod); + + if (!$addMethod) { + throw new FormException(sprintf( + 'The method "%s" could not be found on class %s', + $this->addMethod, + $reflClass->getName() + )); + } + } + + // Set preconfigured remover + if ($this->removeMethod) { + $removeMethod = $this->checkMethod($reflClass, $this->removeMethod); + + if (!$removeMethod) { + throw new FormException(sprintf( + 'The method "%s" could not be found on class %s', + $this->removeMethod, + $reflClass->getName() + )); + } } } @@ -153,17 +176,17 @@ class MergeCollectionListener implements EventSubscriberInterface } } - if ($adderName || $removerName) { + if ($addMethod || $removeMethod) { // If methods to add and to remove exist, call them now, if allowed - if ($removerName) { + if ($removeMethod) { foreach ($itemsToDelete as $item) { - $parentData->$removerName($item); + $parentData->$removeMethod($item); } } - if ($adderName) { + if ($addMethod) { foreach ($itemsToAdd as $item) { - $parentData->$adderName($item); + $parentData->$addMethod($item); } } } elseif (!$originalData) { @@ -193,4 +216,16 @@ class MergeCollectionListener implements EventSubscriberInterface $event->setData($originalData); } + + private function checkMethod(\ReflectionClass $reflClass, $methodName) { + if ($reflClass->hasMethod($methodName)) { + $method = $reflClass->getMethod($methodName); + + if ($method->isPublic() && $method->getNumberOfRequiredParameters() === 1) { + return $methodName; + } + } + + return null; + } } diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index 574bf15aaf..e76967ce20 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -103,8 +103,8 @@ class ChoiceType extends AbstractType // adders/removers // Same as in CollectionType $options['by_reference'], - $options['adder_prefix'], - $options['remover_prefix'] + $options['add_method'], + $options['remove_method'] )) ; } else { @@ -156,8 +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', + 'add_method' => null, + 'remove_method' => null, ); } diff --git a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php index 13f6e3114e..fbdb35383a 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php @@ -45,8 +45,8 @@ class CollectionType extends AbstractType // is desired), disable support for adders/removers // Same as in ChoiceType $options['by_reference'], - $options['adder_prefix'], - $options['remover_prefix'] + $options['add_method'], + $options['remove_method'] ); $builder @@ -90,8 +90,8 @@ class CollectionType extends AbstractType return array( 'allow_add' => false, 'allow_delete' => false, - 'adder_prefix' => 'add', - 'remover_prefix' => 'remove', + 'add_method' => null, + 'remove_method' => null, 'prototype' => true, 'prototype_name' => '__name__', 'type' => 'text', 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 84f17ada78..9f81626b4c 100644 --- a/tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php +++ b/tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php @@ -24,11 +24,11 @@ class MergeCollectionListenerTest_Car public function removeAxis($axis) {} } -class MergeCollectionListenerTest_CarCustomPrefix +class MergeCollectionListenerTest_CarCustomNames { - public function fooAxis($axis) {} + public function foo($axis) {} - public function barAxis($axis) {} + public function bar($axis) {} } class MergeCollectionListenerTest_CarOnlyAdder @@ -442,9 +442,9 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($this->getData($originalDataArray), $event->getData()); } - public function testCallAccessorsWithCustomPrefixes() + public function testCallAccessorsWithCustomNames() { - $parentData = $this->getMock(__CLASS__ . '_CarCustomPrefix'); + $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); $parentForm = $this->getForm('car'); $parentForm->setData($parentData); $parentForm->add($this->form); @@ -456,10 +456,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->form->setData($originalData); $parentData->expects($this->once()) - ->method('fooAxis') + ->method('foo') ->with('first'); $parentData->expects($this->once()) - ->method('barAxis') + ->method('bar') ->with('second'); $event = new FilterDataEvent($this->form, $newData); @@ -474,4 +474,44 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase // Thus it should not be written back into the parent data! $this->assertEquals($this->getData($originalDataArray), $event->getData()); } + + /** + * @expectedException Symfony\Component\Form\Exception\FormException + */ + public function testThrowExceptionIfInvalidAdder() + { + $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); + $parentForm = $this->getForm('car'); + $parentForm->setData($parentData); + $parentForm->add($this->form); + + $originalData = $this->getData(array(1 => 'second')); + $newData = $this->getData(array(0 => 'first')); + + $this->form->setData($originalData); + + $event = new FilterDataEvent($this->form, $newData); + $listener = new MergeCollectionListener(true, false, true, 'doesnotexist'); + $listener->onBindNormData($event); + } + + /** + * @expectedException Symfony\Component\Form\Exception\FormException + */ + public function testThrowExceptionIfInvalidRemover() + { + $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); + $parentForm = $this->getForm('car'); + $parentForm->setData($parentData); + $parentForm->add($this->form); + + $originalData = $this->getData(array(1 => 'second')); + $newData = $this->getData(array(0 => 'first')); + + $this->form->setData($originalData); + + $event = new FilterDataEvent($this->form, $newData); + $listener = new MergeCollectionListener(false, true, true, null, 'doesnotexist'); + $listener->onBindNormData($event); + } }