diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php index 8249a81d2f..c7b5829b5f 100644 --- a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php @@ -13,6 +13,7 @@ namespace Symfony\Component\Form\Extension\Core\EventListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Form\FormEvents; +use Symfony\Component\Form\Event\DataEvent; use Symfony\Component\Form\Event\FilterDataEvent; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\Form\Exception\FormException; @@ -23,6 +24,21 @@ use Symfony\Component\Form\Util\FormUtil; */ class MergeCollectionListener implements EventSubscriberInterface { + /** + * Strategy for merging the new collection into the old collection + * + * @var integer + */ + const MERGE_NORMAL = 1; + + /** + * Strategy for calling add/remove methods on the parent data for all + * new/removed elements in the new collection + * + * @var integer + */ + const MERGE_INTO_PARENT = 2; + /** * Whether elements may be added to the collection * @var Boolean @@ -39,7 +55,7 @@ class MergeCollectionListener implements EventSubscriberInterface * Whether to search for and use adder and remover methods * @var Boolean */ - private $useAccessors; + private $mergeStrategy; /** * The name of the adder method to look for @@ -53,35 +69,91 @@ class MergeCollectionListener implements EventSubscriberInterface */ private $removeMethod; - public function __construct($allowAdd = false, $allowDelete = false, $useAccessors = true, $addMethod = null, $removeMethod = null) + /** + * A copy of the data before starting binding for this form + * @var mixed + */ + private $dataSnapshot; + + /** + * Creates a new listener. + * + * @param Boolean $allowAdd Whether values might be added to the + * collection. + * @param Boolean $allowDelete Whether values might be removed from the + * collection. + * @param integer $mergeStrategy Which strategy to use for merging the + * bound collection with the original + * collection. Might be any combination of + * MERGE_NORMAL and MERGE_INTO_PARENT. + * MERGE_INTO_PARENT has precedence over + * MERGE_NORMAL if an adder/remover method + * is found. The default strategy is to use + * both strategies. + * @param string $addMethod The name of the adder method to use. If + * not given, the listener tries to discover + * the method automatically. + * @param string $removeMethod The name of the remover method to use. If + * not given, the listener tries to discover + * the method automatically. + * + * @throws FormException If the given strategy is invalid. + */ + public function __construct($allowAdd = false, $allowDelete = false, $mergeStrategy = null, $addMethod = null, $removeMethod = null) { + if ($mergeStrategy && !($mergeStrategy & (self::MERGE_NORMAL | self::MERGE_INTO_PARENT))) { + throw new FormException('The merge strategy needs to be at least MERGE_NORMAL or MERGE_INTO_PARENT'); + } + $this->allowAdd = $allowAdd; $this->allowDelete = $allowDelete; - $this->useAccessors = $useAccessors; + $this->mergeStrategy = $mergeStrategy ?: self::MERGE_NORMAL | self::MERGE_INTO_PARENT; $this->addMethod = $addMethod; $this->removeMethod = $removeMethod; } static public function getSubscribedEvents() { - return array(FormEvents::BIND_NORM_DATA => 'onBindNormData'); + return array( + FormEvents::PRE_BIND => 'preBind', + FormEvents::BIND_NORM_DATA => 'onBindNormData', + ); + } + + public function preBind(DataEvent $event) + { + // Get a snapshot of the current state of the normalized data + // to compare against later + $this->dataSnapshot = $event->getForm()->getNormData(); + + if (is_object($this->dataSnapshot)) { + // Make sure the snapshot remains stable and doesn't change + $this->dataSnapshot = clone $this->dataSnapshot; + } + + if (null !== $this->dataSnapshot && !is_array($this->dataSnapshot) && !($this->dataSnapshot instanceof \Traversable && $this->dataSnapshot instanceof \ArrayAccess)) { + throw new UnexpectedTypeException($this->dataSnapshot, 'array or (\Traversable and \ArrayAccess)'); + } } public function onBindNormData(FilterDataEvent $event) { - $originalData = $event->getForm()->getData(); + $originalData = $event->getForm()->getNormData(); // If we are not allowed to change anything, return immediately if (!$this->allowAdd && !$this->allowDelete) { + // Don't set to the snapshot as then we are switching from the + // original object to its copy, which might break things $event->setData($originalData); return; } $form = $event->getForm(); $data = $event->getData(); - $parentData = $form->hasParent() ? $form->getParent()->getData() : null; + $parentData = $form->hasParent() ? $form->getParent()->getClientData() : null; $addMethod = null; $removeMethod = null; + $getMethod = null; if (null === $data) { $data = array(); @@ -96,24 +168,35 @@ class MergeCollectionListener implements EventSubscriberInterface } // Check if the parent has matching methods to add/remove items - if ($this->useAccessors && is_object($parentData)) { + 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; // Any of the two methods is required, but not yet known if ($addMethodNeeded || $removeMethodNeeded) { - $singulars = (array) FormUtil::singularify(ucfirst($form->getName())); + $singulars = (array) FormUtil::singularify($plural); foreach ($singulars as $singular) { // Try to find adder, but don't override preconfigured one if ($addMethodNeeded) { - $addMethod = $this->checkMethod($reflClass, 'add' . $singular); + $addMethod = 'add' . $singular; + + // False alert + if (!$this->isAccessible($reflClass, $addMethod, 1)) { + $addMethod = null; + } } // Try to find remover, but don't override preconfigured one if ($removeMethodNeeded) { - $removeMethod = $this->checkMethod($reflClass, 'remove' . $singular); + $removeMethod = 'remove' . $singular; + + // False alert + if (!$this->isAccessible($reflClass, $removeMethod, 1)) { + $removeMethod = null; + } } // Found all that we need. Abort search. @@ -129,12 +212,12 @@ class MergeCollectionListener implements EventSubscriberInterface // Set preconfigured adder if ($this->allowAdd && $this->addMethod) { - $addMethod = $this->checkMethod($reflClass, $this->addMethod); + $addMethod = $this->addMethod; - if (!$addMethod) { + if (!$this->isAccessible($reflClass, $addMethod, 1)) { throw new FormException(sprintf( - 'The method "%s" could not be found on class %s', - $this->addMethod, + 'The public method "%s" could not be found on class %s', + $addMethod, $reflClass->getName() )); } @@ -142,25 +225,36 @@ class MergeCollectionListener implements EventSubscriberInterface // Set preconfigured remover if ($this->allowDelete && $this->removeMethod) { - $removeMethod = $this->checkMethod($reflClass, $this->removeMethod); + $removeMethod = $this->removeMethod; - if (!$removeMethod) { + if (!$this->isAccessible($reflClass, $removeMethod, 1)) { throw new FormException(sprintf( - 'The method "%s" could not be found on class %s', - $this->removeMethod, + 'The public method "%s" could not be found on class %s', + $removeMethod, + $reflClass->getName() + )); + } + } + + 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() )); } } } - // Check which items are in $data that are not in $originalData and - // vice versa + // Calculate delta between $data and the snapshot created in PRE_BIND $itemsToDelete = array(); $itemsToAdd = is_object($data) ? clone $data : $data; - if ($originalData) { - foreach ($originalData as $originalKey => $originalItem) { + if ($this->dataSnapshot) { + foreach ($this->dataSnapshot as $originalItem) { foreach ($data as $key => $item) { if ($item === $originalItem) { // Item found, next original item @@ -170,7 +264,12 @@ class MergeCollectionListener implements EventSubscriberInterface } // Item not found, remember for deletion - $itemsToDelete[$originalKey] = $originalItem; + foreach ($originalData as $key => $item) { + if ($item === $originalItem) { + $itemsToDelete[$key] = $item; + continue 2; + } + } } } @@ -187,43 +286,47 @@ class MergeCollectionListener implements EventSubscriberInterface $parentData->$addMethod($item); } } - } elseif (!$originalData) { - // No original data was set. Set it if allowed - if ($this->allowAdd) { - $originalData = $data; - } - } else { - // Original data is an array-like structure - // Add and remove items in the original variable - if ($this->allowDelete) { - foreach ($itemsToDelete as $key => $item) { - unset($originalData[$key]); - } - } - if ($this->allowAdd) { - foreach ($itemsToAdd as $key => $item) { - if (!isset($originalData[$key])) { - $originalData[$key] = $item; - } else { - $originalData[] = $item; + $event->setData($parentData->$getMethod()); + } elseif ($this->mergeStrategy & self::MERGE_NORMAL) { + if (!$originalData) { + // No original data was set. Set it if allowed + if ($this->allowAdd) { + $originalData = $data; + } + } else { + // Original data is an array-like structure + // Add and remove items in the original variable + if ($this->allowDelete) { + foreach ($itemsToDelete as $key => $item) { + unset($originalData[$key]); + } + } + + if ($this->allowAdd) { + foreach ($itemsToAdd as $key => $item) { + if (!isset($originalData[$key])) { + $originalData[$key] = $item; + } else { + $originalData[] = $item; + } } } } - } - $event->setData($originalData); + $event->setData($originalData); + } } - private function checkMethod(\ReflectionClass $reflClass, $methodName) { + private function isAccessible(\ReflectionClass $reflClass, $methodName, $numberOfRequiredParameters) { if ($reflClass->hasMethod($methodName)) { $method = $reflClass->getMethod($methodName); - if ($method->isPublic() && $method->getNumberOfRequiredParameters() === 1) { - return $methodName; + if ($method->isPublic() && $method->getNumberOfRequiredParameters() === $numberOfRequiredParameters) { + return true; } } - return null; + return false; } } diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index ff4a4b5ee9..37c4c672cd 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -81,10 +81,7 @@ class ChoiceType extends AbstractType if ($options['expanded']) { if ($options['multiple']) { - $builder - ->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list'])) - ->addEventSubscriber(new MergeCollectionListener(true, true)) - ; + $builder->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list'])); } else { $builder ->appendClientTransformer(new ChoiceToBooleanArrayTransformer($options['choice_list'])) @@ -93,24 +90,31 @@ class ChoiceType extends AbstractType } } else { if ($options['multiple']) { - $builder - ->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list'])) - ->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['add_method'], - $options['remove_method'] - )) - ; + $builder->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list'])); } else { $builder->appendClientTransformer(new ChoiceToValueTransformer($options['choice_list'])); } } + + if ($options['multiple']) { + // Make sure the collection created during the client->norm + // transformation is merged back into the original collection + $mergeStrategy = MergeCollectionListener::MERGE_NORMAL; + + // Enable support for adders/removers unless "by_reference" is disabled + // (explicit calling of the setter is desired) + if ($options['by_reference']) { + $mergeStrategy = $mergeStrategy | MergeCollectionListener::MERGE_INTO_PARENT; + } + + $builder->addEventSubscriber(new MergeCollectionListener( + true, + true, + $mergeStrategy, + $options['add_method'], + $options['remove_method'] + )); + } } /** diff --git a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php index fbdb35383a..c094c2126c 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php @@ -38,23 +38,23 @@ class CollectionType extends AbstractType $options['allow_delete'] ); - $mergeListener = new MergeCollectionListener( - $options['allow_add'], - $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['add_method'], - $options['remove_method'] - ); - $builder ->addEventSubscriber($resizeListener) - ->addEventSubscriber($mergeListener) ->setAttribute('allow_add', $options['allow_add']) ->setAttribute('allow_delete', $options['allow_delete']) ; + + // Enable support for adders/removers unless "by_reference" is disabled + // (explicit calling of the setter is desired) + if ($options['by_reference']) { + $builder->addEventSubscriber(new MergeCollectionListener( + $options['allow_add'], + $options['allow_delete'], + MergeCollectionListener::MERGE_INTO_PARENT, + $options['add_method'], + $options['remove_method'] + )); + } } /** 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 02fd3e7300..e29b40dff1 100644 --- a/tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php +++ b/tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php @@ -22,6 +22,8 @@ class MergeCollectionListenerTest_Car public function addAxis($axis) {} public function removeAxis($axis) {} + + public function getAxes() {} } class MergeCollectionListenerTest_CarCustomNames @@ -29,16 +31,22 @@ class MergeCollectionListenerTest_CarCustomNames public function foo($axis) {} public function bar($axis) {} + + public function getAxes() {} } class MergeCollectionListenerTest_CarOnlyAdder { public function addAxis($axis) {} + + public function getAxes() {} } class MergeCollectionListenerTest_CarOnlyRemover { public function removeAxis($axis) {} + + public function getAxes() {} } abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase @@ -76,17 +84,55 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase return $this->getMock('Symfony\Tests\Component\Form\FormInterface'); } + public function getModesWithNormal() + { + return array( + array(MergeCollectionListener::MERGE_NORMAL), + array(MergeCollectionListener::MERGE_NORMAL | MergeCollectionListener::MERGE_INTO_PARENT), + ); + } + + public function getModesWithMergeIntoParent() + { + return array( + array(MergeCollectionListener::MERGE_INTO_PARENT), + array(MergeCollectionListener::MERGE_INTO_PARENT | MergeCollectionListener::MERGE_NORMAL), + ); + } + + public function getModesWithoutMergeIntoParent() + { + return array( + array(MergeCollectionListener::MERGE_NORMAL), + ); + } + + public function getInvalidModes() + { + return array( + // 0 is a valid mode, because it is treated as "default" (=3) + array(4), + array(8), + ); + } + abstract protected function getData(array $data); - public function testAddExtraEntriesIfAllowAdd() + /** + * @dataProvider getModesWithNormal + */ + public function testAddExtraEntriesIfAllowAdd($mode) { $originalData = $this->getData(array(1 => 'second')); $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); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false); $listener->onBindNormData($event); // The original object was modified @@ -98,15 +144,21 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($newData, $event->getData()); } - public function testAddExtraEntriesIfAllowAddDontOverwriteExistingIndices() + /** + * @dataProvider getModesWithNormal + */ + public function testAddExtraEntriesIfAllowAddDontOverwriteExistingIndices($mode) { $originalData = $this->getData(array(1 => 'first')); $newData = $this->getData(array(0 => 'first', 1 => 'second')); + $listener = new MergeCollectionListener(true, false, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false); $listener->onBindNormData($event); // The original object was modified @@ -118,16 +170,22 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($this->getData(array(1 => 'first', 2 => 'second')), $event->getData()); } - public function testDoNothingIfNotAllowAdd() + /** + * @dataProvider getModesWithNormal + */ + public function testDoNothingIfNotAllowAdd($mode) { $originalDataArray = array(1 => 'second'); $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); + $listener = new MergeCollectionListener(false, false, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, false); $listener->onBindNormData($event); // We still have the original object @@ -139,15 +197,21 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($this->getData($originalDataArray), $event->getData()); } - public function testRemoveMissingEntriesIfAllowDelete() + /** + * @dataProvider getModesWithNormal + */ + public function testRemoveMissingEntriesIfAllowDelete($mode) { $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); $newData = $this->getData(array(1 => 'second')); + $listener = new MergeCollectionListener(false, true, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, true); $listener->onBindNormData($event); // The original object was modified @@ -159,16 +223,22 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($newData, $event->getData()); } - public function testDoNothingIfNotAllowDelete() + /** + * @dataProvider getModesWithNormal + */ + public function testDoNothingIfNotAllowDelete($mode) { $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third'); $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(1 => 'second')); + $listener = new MergeCollectionListener(false, false, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, false); $listener->onBindNormData($event); // We still have the original object @@ -181,59 +251,81 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** + * @dataProvider getModesWithNormal * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException */ - public function testRequireArrayOrTraversable() + public function testRequireArrayOrTraversable($mode) { $newData = 'no array or traversable'; $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false); + $listener = new MergeCollectionListener(true, false, $mode); $listener->onBindNormData($event); } - public function testDealWithNullData() + /** + * @dataProvider getModesWithNormal + */ + public function testDealWithNullData($mode) { $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); $newData = null; + $listener = new MergeCollectionListener(false, false, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, false); $listener->onBindNormData($event); $this->assertSame($originalData, $event->getData()); } - public function testDealWithNullOriginalDataIfAllowAdd() + /** + * @dataProvider getModesWithNormal + */ + public function testDealWithNullOriginalDataIfAllowAdd($mode) { $originalData = null; $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); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false); $listener->onBindNormData($event); $this->assertSame($newData, $event->getData()); } - public function testDontDealWithNullOriginalDataIfNotAllowAdd() + /** + * @dataProvider getModesWithNormal + */ + public function testDontDealWithNullOriginalDataIfNotAllowAdd($mode) { $originalData = null; $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); + $listener = new MergeCollectionListener(false, false, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, false); $listener->onBindNormData($event); $this->assertNull($event->getData()); } - public function testCallAdderIfAllowAdd() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testCallAdderIfAllowAdd($mode) { $parentData = $this->getMock(__CLASS__ . '_CarOnlyAdder'); $parentForm = $this->getForm('car'); @@ -244,29 +336,77 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $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->at(0)) ->method('addAxis') ->with('first'); $parentData->expects($this->at(1)) ->method('addAxis') ->with('third'); + $parentData->expects($this->at(2)) + ->method('getAxes') + ->will($this->returnValue('RESULT')); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false); $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()); + $this->assertEquals('RESULT', $event->getData()); } - public function testDontCallAdderIfNotAllowAdd() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testCallAdderIfOriginalDataAlreadyModified($mode) + { + $parentData = $this->getMock(__CLASS__ . '_CarOnlyAdder'); + $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')); + + $listener = new MergeCollectionListener(true, false, $mode); + + $this->form->setData($originalData); + + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + + // The form already contains the new data + // This happens if the data mapper maps the data of the child forms + // back into the original collection. + // The original collection is then both already modified and passed + // as event argument. + $this->form->setData($newData); + + $parentData->expects($this->at(0)) + ->method('addAxis') + ->with('first'); + $parentData->expects($this->at(1)) + ->method('addAxis') + ->with('third'); + $parentData->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 + */ + public function testDontCallAdderIfNotAllowAdd($mode) { $parentData = $this->getMock(__CLASS__ . '_Car'); $parentForm = $this->getForm('car'); @@ -277,13 +417,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); + $listener = new MergeCollectionListener(false, false, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + $parentData->expects($this->never()) ->method('addAxis'); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, false); $listener->onBindNormData($event); if (is_object($originalData)) { @@ -294,7 +438,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($this->getData($originalDataArray), $event->getData()); } - public function testDontCallAdderIfNotUseAccessors() + /** + * @dataProvider getModesWithoutMergeIntoParent + */ + public function testDontCallAdderIfNotMergeIntoParent($mode) { $parentData = $this->getMock(__CLASS__ . '_Car'); $parentForm = $this->getForm('car'); @@ -304,13 +451,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData(array(1 => 'second')); $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->never()) ->method('addAxis'); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false, false); $listener->onBindNormData($event); if (is_object($originalData)) { @@ -321,7 +472,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($newData, $event->getData()); } - public function testCallRemoverIfAllowDelete() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testCallRemoverIfAllowDelete($mode) { $parentData = $this->getMock(__CLASS__ . '_CarOnlyRemover'); $parentForm = $this->getForm('car'); @@ -332,29 +486,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(1 => 'second')); + $listener = new MergeCollectionListener(false, true, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + $parentData->expects($this->at(0)) ->method('removeAxis') ->with('first'); $parentData->expects($this->at(1)) ->method('removeAxis') ->with('third'); + $parentData->expects($this->at(2)) + ->method('getAxes') + ->will($this->returnValue('RESULT')); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, true); $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()); + $this->assertEquals('RESULT', $event->getData()); } - public function testDontCallRemoverIfNotAllowDelete() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testDontCallRemoverIfNotAllowDelete($mode) { $parentData = $this->getMock(__CLASS__ . '_Car'); $parentForm = $this->getForm('car'); @@ -365,13 +523,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(1 => 'second')); + $listener = new MergeCollectionListener(false, false, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + $parentData->expects($this->never()) ->method('removeAxis'); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, false); $listener->onBindNormData($event); if (is_object($originalData)) { @@ -382,7 +544,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($this->getData($originalDataArray), $event->getData()); } - public function testDontCallRemoverIfNotUseAccessors() + /** + * @dataProvider getModesWithoutMergeIntoParent + */ + public function testDontCallRemoverIfNotMergeIntoParent($mode) { $parentData = $this->getMock(__CLASS__ . '_Car'); $parentForm = $this->getForm('car'); @@ -392,13 +557,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); $newData = $this->getData(array(1 => 'second')); + $listener = new MergeCollectionListener(false, true, $mode); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + $parentData->expects($this->never()) ->method('removeAxis'); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, true, false); $listener->onBindNormData($event); if (is_object($originalData)) { @@ -409,7 +578,10 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($newData, $event->getData()); } - public function testCallAdderAndDeleterIfAllowAll() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testCallAdderAndDeleterIfAllowAll($mode) { $parentData = $this->getMock(__CLASS__ . '_Car'); $parentForm = $this->getForm('car'); @@ -420,29 +592,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(0 => 'first')); + $listener = new MergeCollectionListener(true, true, $mode); + $this->form->setData($originalData); - $parentData->expects($this->once()) - ->method('addAxis') - ->with('first'); - $parentData->expects($this->once()) + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + + $parentData->expects($this->at(0)) ->method('removeAxis') ->with('second'); + $parentData->expects($this->at(1)) + ->method('addAxis') + ->with('first'); + $parentData->expects($this->at(2)) + ->method('getAxes') + ->will($this->returnValue('RESULT')); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, true, true); $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()); + $this->assertEquals('RESULT', $event->getData()); } - public function testCallAccessorsWithCustomNames() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testCallAccessorsWithCustomNames($mode) { $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); $parentForm = $this->getForm('car'); @@ -453,29 +629,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(0 => 'first')); + $listener = new MergeCollectionListener(true, true, $mode, 'foo', 'bar'); + $this->form->setData($originalData); - $parentData->expects($this->once()) - ->method('foo') - ->with('first'); - $parentData->expects($this->once()) + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + + $parentData->expects($this->at(0)) ->method('bar') ->with('second'); + $parentData->expects($this->at(1)) + ->method('foo') + ->with('first'); + $parentData->expects($this->at(2)) + ->method('getAxes') + ->will($this->returnValue('RESULT')); $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()); + $this->assertEquals('RESULT', $event->getData()); } - public function testDontCallAdderWithCustomNameIfDisallowed() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testDontCallAdderWithCustomNameIfDisallowed($mode) { $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); $parentForm = $this->getForm('car'); @@ -486,27 +666,32 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(0 => 'first')); + $listener = new MergeCollectionListener(false, true, $mode, 'foo', 'bar'); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + $parentData->expects($this->never()) ->method('foo'); - $parentData->expects($this->once()) + $parentData->expects($this->at(0)) ->method('bar') ->with('second'); + $parentData->expects($this->at(1)) + ->method('getAxes') + ->will($this->returnValue('RESULT')); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, true, true, 'foo', 'bar'); $listener->onBindNormData($event); - if (is_object($originalData)) { - $this->assertSame($originalData, $event->getData()); - } - - // The data was not modified - $this->assertEquals($this->getData($originalDataArray), $event->getData()); + $this->assertEquals('RESULT', $event->getData()); } - public function testDontCallRemoverWithCustomNameIfDisallowed() + /** + * @dataProvider getModesWithMergeIntoParent + */ + public function testDontCallRemoverWithCustomNameIfDisallowed($mode) { $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); $parentForm = $this->getForm('car'); @@ -517,30 +702,33 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(0 => 'first')); + $listener = new MergeCollectionListener(true, false, $mode, 'foo', 'bar'); + $this->form->setData($originalData); - $parentData->expects($this->once()) + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); + + $parentData->expects($this->at(0)) ->method('foo') ->with('first'); $parentData->expects($this->never()) ->method('bar'); + $parentData->expects($this->at(1)) + ->method('getAxes') + ->will($this->returnValue('RESULT')); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false, true, 'foo', 'bar'); $listener->onBindNormData($event); - if (is_object($originalData)) { - $this->assertSame($originalData, $event->getData()); - } - - // The data was not modified - $this->assertEquals($this->getData($originalDataArray), $event->getData()); + $this->assertEquals('RESULT', $event->getData()); } /** + * @dataProvider getModesWithMergeIntoParent * @expectedException Symfony\Component\Form\Exception\FormException */ - public function testThrowExceptionIfInvalidAdder() + public function testThrowExceptionIfInvalidAdder($mode) { $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); $parentForm = $this->getForm('car'); @@ -550,17 +738,21 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData(array(1 => 'second')); $newData = $this->getData(array(0 => 'first')); + $listener = new MergeCollectionListener(true, false, $mode, 'doesnotexist'); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false, true, 'doesnotexist'); $listener->onBindNormData($event); } /** + * @dataProvider getModesWithMergeIntoParent * @expectedException Symfony\Component\Form\Exception\FormException */ - public function testThrowExceptionIfInvalidRemover() + public function testThrowExceptionIfInvalidRemover($mode) { $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); $parentForm = $this->getForm('car'); @@ -570,10 +762,22 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase $originalData = $this->getData(array(1 => 'second')); $newData = $this->getData(array(0 => 'first')); + $listener = new MergeCollectionListener(false, true, $mode, null, 'doesnotexist'); + $this->form->setData($originalData); + $event = new DataEvent($this->form, $newData); + $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(false, true, true, null, 'doesnotexist'); $listener->onBindNormData($event); } + + /** + * @dataProvider getInvalidModes + * @expectedException Symfony\Component\Form\Exception\FormException + */ + public function testThrowExceptionIfInvalidMode($mode) + { + new MergeCollectionListener(true, true, $mode); + } }