diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index 587c2fe595..ebb276024c 100644 --- a/CHANGELOG-2.1.md +++ b/CHANGELOG-2.1.md @@ -252,8 +252,8 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c * the radio type is now a child of the checkbox type * 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 "add_method" and "remove_method" to collection and choice type + model. For a custom, non-recognized singular form, set the "property_path" + option like this: "plural|singular" * forms now don't create an empty object anymore if they are completely empty and not required. The empty value for such forms is null. * added constant Guess::VERY_HIGH_CONFIDENCE diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index e9c3c126e4..04135af5b5 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -59,8 +59,16 @@ class PropertyPathMapper implements DataMapperInterface public function mapDataToForm($data, FormInterface $form) { if (!empty($data)) { - if (null !== $form->getAttribute('property_path')) { - $form->setData($form->getAttribute('property_path')->getValue($data)); + $propertyPath = $form->getAttribute('property_path'); + + if (null !== $propertyPath) { + $propertyData = $propertyPath->getValue($data); + + if (is_object($propertyData) && !$form->getAttribute('by_reference')) { + $propertyData = clone $propertyData; + } + + $form->setData($propertyData); } } } @@ -77,9 +85,9 @@ class PropertyPathMapper implements DataMapperInterface public function mapFormToData(FormInterface $form, &$data) { - if (null !== $form->getAttribute('property_path') && $form->isSynchronized()) { - $propertyPath = $form->getAttribute('property_path'); + $propertyPath = $form->getAttribute('property_path'); + if (null !== $propertyPath && $form->isSynchronized()) { // If the data is identical to the value in $data, we are // dealing with a reference $isReference = $form->getData() === $propertyPath->getValue($data); diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php index 5b162c43f2..75940c6f84 100644 --- a/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php @@ -25,21 +25,6 @@ use Symfony\Component\Form\Util\PropertyPath; */ 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 @@ -52,30 +37,6 @@ class MergeCollectionListener implements EventSubscriberInterface */ private $allowDelete; - /** - * Whether to search for and use adder and remover methods - * @var Boolean - */ - private $mergeStrategy; - - /** - * The name of the adder method to look for - * @var string - */ - private $addMethod; - - /** - * The name of the remover method to look for - * @var string - */ - private $removeMethod; - - /** - * A copy of the data before starting binding for this form - * @var mixed - */ - private $dataSnapshot; - /** * Creates a new listener. * @@ -83,100 +44,26 @@ class MergeCollectionListener implements EventSubscriberInterface * 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) + public function __construct($allowAdd = false, $allowDelete = false) { - 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->mergeStrategy = $mergeStrategy ?: self::MERGE_NORMAL | self::MERGE_INTO_PARENT; - $this->addMethod = $addMethod; - $this->removeMethod = $removeMethod; } static public function getSubscribedEvents() { 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()->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; - } + $dataToMergeInto = $event->getForm()->getNormData(); $form = $event->getForm(); $data = $event->getData(); - $childPropertyPath = null; - $parentData = null; - $addMethod = null; - $removeMethod = 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(); @@ -186,157 +73,60 @@ class MergeCollectionListener implements EventSubscriberInterface throw new UnexpectedTypeException($data, 'array or (\Traversable and \ArrayAccess)'); } - if (null !== $originalData && !is_array($originalData) && !($originalData instanceof \Traversable && $originalData instanceof \ArrayAccess)) { - throw new UnexpectedTypeException($originalData, 'array or (\Traversable and \ArrayAccess)'); + if (null !== $dataToMergeInto && !is_array($dataToMergeInto) && !($dataToMergeInto instanceof \Traversable && $dataToMergeInto instanceof \ArrayAccess)) { + throw new UnexpectedTypeException($dataToMergeInto, 'array or (\Traversable and \ArrayAccess)'); } - // Check if the parent has matching methods to add/remove items - if (($this->mergeStrategy & self::MERGE_INTO_PARENT) && is_object($parentData)) { - $reflClass = new \ReflectionClass($parentData); - $addMethodNeeded = $this->allowAdd && !$this->addMethod; - $removeMethodNeeded = $this->allowDelete && !$this->removeMethod; + // If we are not allowed to change anything, return immediately + if ((!$this->allowAdd && !$this->allowDelete) || $data === $dataToMergeInto) { + $event->setData($dataToMergeInto); - // Any of the two methods is required, but not yet known - if ($addMethodNeeded || $removeMethodNeeded) { - $singulars = (array) FormUtil::singularify($plural); - - foreach ($singulars as $singular) { - // Try to find adder, but don't override preconfigured one - if ($addMethodNeeded) { - $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 = 'remove' . $singular; - - // False alert - if (!$this->isAccessible($reflClass, $removeMethod, 1)) { - $removeMethod = null; - } - } - - // Found all that we need. Abort search. - if ((!$addMethodNeeded || $addMethod) && (!$removeMethodNeeded || $removeMethod)) { - break; - } - - // False alert - $addMethod = null; - $removeMethod = null; - } - } - - // Set preconfigured adder - if ($this->allowAdd && $this->addMethod) { - $addMethod = $this->addMethod; - - if (!$this->isAccessible($reflClass, $addMethod, 1)) { - throw new FormException(sprintf( - 'The public method "%s" could not be found on class %s', - $addMethod, - $reflClass->getName() - )); - } - } - - // Set preconfigured remover - if ($this->allowDelete && $this->removeMethod) { - $removeMethod = $this->removeMethod; - - if (!$this->isAccessible($reflClass, $removeMethod, 1)) { - throw new FormException(sprintf( - 'The public method "%s" could not be found on class %s', - $removeMethod, - $reflClass->getName() - )); - } - } + return; } - // Calculate delta between $data and the snapshot created in PRE_BIND - $itemsToDelete = array(); - $itemsToAdd = is_object($data) ? clone $data : $data; + if (!$dataToMergeInto) { + // No original data was set. Set it if allowed + if ($this->allowAdd) { + $dataToMergeInto = $data; + } + } else { + // Calculate delta + $itemsToAdd = is_object($data) ? clone $data : $data; + $itemsToDelete = array(); - if ($this->dataSnapshot) { - foreach ($this->dataSnapshot as $originalItem) { - foreach ($data as $key => $item) { - if ($item === $originalItem) { + foreach ($dataToMergeInto as $beforeKey => $beforeItem) { + foreach ($data as $afterKey => $afterItem) { + if ($afterItem === $beforeItem) { // Item found, next original item - unset($itemsToAdd[$key]); + unset($itemsToAdd[$afterKey]); continue 2; } } // Item not found, remember for deletion - foreach ($originalData as $key => $item) { - if ($item === $originalItem) { - $itemsToDelete[$key] = $item; - continue 2; + $itemsToDelete[] = $beforeKey; + } + + // Remove deleted items before adding to free keys that are to be + // replaced + if ($this->allowDelete) { + foreach ($itemsToDelete as $key) { + unset($dataToMergeInto[$key]); + } + } + + // Add remaining items + if ($this->allowAdd) { + foreach ($itemsToAdd as $key => $item) { + if (!isset($dataToMergeInto[$key])) { + $dataToMergeInto[$key] = $item; + } else { + $dataToMergeInto[] = $item; } } } } - if ($addMethod || $removeMethod) { - // If methods to add and to remove exist, call them now, if allowed - if ($removeMethod) { - foreach ($itemsToDelete as $item) { - $parentData->$removeMethod($item); - } - } - - if ($addMethod) { - foreach ($itemsToAdd as $item) { - $parentData->$addMethod($item); - } - } - - $event->setData($childPropertyPath->getValue($parentData)); - } 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); - } - } - - private function isAccessible(\ReflectionClass $reflClass, $methodName, $numberOfRequiredParameters) { - if ($reflClass->hasMethod($methodName)) { - $method = $reflClass->getMethod($methodName); - - if ($method->isPublic() && $method->getNumberOfRequiredParameters() === $numberOfRequiredParameters) { - return true; - } - } - - return false; + $event->setData($dataToMergeInto); } } diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php index 5ac3994ff9..d934680c9c 100644 --- a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php @@ -144,6 +144,8 @@ class ResizeFormListener implements EventSubscriberInterface throw new UnexpectedTypeException($data, 'array or (\Traversable and \ArrayAccess)'); } + // The data mapper only adds, but does not remove items, so do this + // here if ($this->allowDelete) { foreach ($data as $name => $child) { if (!$form->has($name)) { diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index b79d85347a..0b7c45053c 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -98,24 +98,10 @@ class ChoiceType extends AbstractType } } - if ($options['multiple']) { + if ($options['multiple'] && $options['by_reference']) { // 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'] - )); + $builder->addEventSubscriber(new MergeCollectionListener(true, true)); } } @@ -180,8 +166,6 @@ class ChoiceType extends AbstractType 'empty_data' => $multiple || $expanded ? array() : '', 'empty_value' => $multiple || $expanded || !isset($options['empty_value']) ? null : '', 'error_bubbling' => false, - '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 ec20ec617d..5eb8285d31 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php @@ -45,18 +45,6 @@ class CollectionType extends AbstractType ->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'] - )); - } } /** @@ -92,8 +80,6 @@ class CollectionType extends AbstractType return array( 'allow_add' => false, 'allow_delete' => false, - 'add_method' => null, - 'remove_method' => null, 'prototype' => true, 'prototype_name' => '__name__', 'type' => 'text', diff --git a/src/Symfony/Component/Form/FormBuilder.php b/src/Symfony/Component/Form/FormBuilder.php index 6bb38b7579..d0caa65d0e 100644 --- a/src/Symfony/Component/Form/FormBuilder.php +++ b/src/Symfony/Component/Form/FormBuilder.php @@ -459,7 +459,7 @@ class FormBuilder * * @return FormBuilder The current builder */ - public function setDataMapper(DataMapperInterface $dataMapper) + public function setDataMapper(DataMapperInterface $dataMapper = null) { $this->dataMapper = $dataMapper; diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index 91f3ea2b36..e7e23ee21a 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -11,80 +11,191 @@ namespace Symfony\Component\Form\Tests\Extension\Core\DataMapper; +use Symfony\Component\Form\Tests\FormInterface; use Symfony\Component\Form\Util\PropertyPath; use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper; +abstract class PropertyPathMapperTest_Form implements FormInterface +{ + private $attributes = array(); + + private $data; + + public function setAttribute($name, $value) + { + $this->attribute[$name] = $value; + } + + public function getAttribute($name) + { + return isset($this->attribute[$name]) ? $this->attribute[$name] : null; + } + + public function setData($data) + { + $this->data = $data; + } + + public function getData() + { + return $this->data; + } +} + class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase { private $mapper; - private $propertyPath; - protected function setUp() { $this->mapper = new PropertyPathMapper(); - $this->propertyPath = $this->getMockBuilder('Symfony\Component\Form\Util\PropertyPath') - ->disableOriginalConstructor() - ->getMock(); } protected function tearDown() { $this->mapper = null; - $this->propertyPath = null; } - private function getForm(PropertyPath $propertyPath = null) + private function getPropertyPath($path) { - $form = $this->getMock('Symfony\Component\Form\Tests\FormInterface'); + return $this->getMockBuilder('Symfony\Component\Form\Util\PropertyPath') + ->setConstructorArgs(array($path)) + ->setMethods(array('getValue', 'setValue')) + ->getMock(); + } + + private function getForm(PropertyPath $propertyPath = null, $byReference, $synchronized = true) + { + $form = $this->getMockBuilder(__CLASS__ . '_Form') + // PHPUnit's getMockForAbstractClass does not behave like in the docs.. + // If the array is empty, all methods are mocked. If it is not + // empty, only abstract methods and the methods in the array are + // mocked. + ->setMethods(array('foo')) + ->getMockForAbstractClass(); + + $form->setAttribute('property_path', $propertyPath); + $form->setAttribute('by_reference', $byReference); $form->expects($this->any()) - ->method('getAttribute') - ->with('property_path') - ->will($this->returnValue($propertyPath)); + ->method('isSynchronized') + ->will($this->returnValue($synchronized)); return $form; } - public function testMapDataToForm() + public function testMapDataToFormPassesObjectRefIfByReference() { - $data = new \stdClass(); + $car = new \stdClass(); + $engine = new \stdClass(); + $propertyPath = $this->getPropertyPath('engine'); - $this->propertyPath->expects($this->once()) + $propertyPath->expects($this->once()) ->method('getValue') - ->with($data) - ->will($this->returnValue('foobar')); + ->with($car) + ->will($this->returnValue($engine)); - $form = $this->getForm($this->propertyPath); + $form = $this->getForm($propertyPath, true); - $form->expects($this->once()) - ->method('setData') - ->with('foobar'); + $this->mapper->mapDataToForm($car, $form); - $this->mapper->mapDataToForm($data, $form); + // Can't use isIdentical() above because mocks always clone their + // arguments which can't be disabled in PHPUnit 3.6 + $this->assertSame($engine, $form->getData()); + } + + public function testMapDataToFormPassesObjectCloneIfNotByReference() + { + $car = new \stdClass(); + $engine = new \stdClass(); + $propertyPath = $this->getPropertyPath('engine'); + + $propertyPath->expects($this->once()) + ->method('getValue') + ->with($car) + ->will($this->returnValue($engine)); + + $form = $this->getForm($propertyPath, false); + + $this->mapper->mapDataToForm($car, $form); + + $this->assertNotSame($engine, $form->getData()); + $this->assertEquals($engine, $form->getData()); } public function testMapDataToFormIgnoresEmptyPropertyPath() { - $data = new \stdClass(); + $car = new \stdClass(); - $form = $this->getForm(null); + $form = $this->getForm(null, true); $form->expects($this->never()) ->method('setData'); - $this->mapper->mapDataToForm($data, $form); + $this->mapper->mapDataToForm($car, $form); } public function testMapDataToFormIgnoresEmptyData() { - $form = $this->getForm($this->propertyPath); + $propertyPath = $this->getPropertyPath('engine'); + $form = $this->getForm($propertyPath, true); $form->expects($this->never()) ->method('setData'); - $form->getAttribute('property_path'); // <- weird PHPUnit bug if I don't do this - $this->mapper->mapDataToForm(null, $form); } + + public function testMapFormToDataWritesBackIfNotByReference() + { + $car = new \stdClass(); + $engine = new \stdClass(); + $propertyPath = $this->getPropertyPath('engine'); + + $propertyPath->expects($this->once()) + ->method('setValue') + ->with($car, $engine); + + $form = $this->getForm($propertyPath, false); + $form->setData($engine); + + $this->mapper->mapFormToData($form, $car); + } + + public function testMapFormToDataWritesBackIfByReferenceButNoReference() + { + $car = new \stdClass(); + $engine = new \stdClass(); + $propertyPath = $this->getPropertyPath('engine'); + + $propertyPath->expects($this->once()) + ->method('setValue') + ->with($car, $engine); + + $form = $this->getForm($propertyPath, true); + $form->setData($engine); + + $this->mapper->mapFormToData($form, $car); + } + + public function testMapFormToDataWritesBackIfByReferenceAndReference() + { + $car = new \stdClass(); + $engine = new \stdClass(); + $propertyPath = $this->getPropertyPath('engine'); + + // $car already contains the reference of $engine + $propertyPath->expects($this->once()) + ->method('getValue') + ->with($car) + ->will($this->returnValue($engine)); + + $propertyPath->expects($this->never()) + ->method('setValue'); + + $form = $this->getForm($propertyPath, true); + $form->setData($engine); + + $this->mapper->mapFormToData($form, $car); + } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerCustomArrayObjectTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerCustomArrayObjectTest.php index 0dd7c3db2e..368cdb6aaa 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerCustomArrayObjectTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerCustomArrayObjectTest.php @@ -10,71 +10,13 @@ */ namespace Symfony\Component\Form\Tests\Extension\Core\EventListener; - -/** - * This class is a hand written simplified version of PHP native `ArrayObject` - * class, to show that it behaves differently than the PHP native implementation. - */ -class MergeCollectionListenerCustomArrayObjectTest_CustomArrayObject implements \ArrayAccess, \IteratorAggregate, \Countable, \Serializable -{ - private $array; - - public function __construct(array $array = null) - { - $this->array = $array ?: array(); - } - - public function offsetExists($offset) - { - return array_key_exists($offset, $this->array); - } - - public function offsetGet($offset) - { - return $this->array[$offset]; - } - - public function offsetSet($offset, $value) - { - if (null === $offset) { - $this->array[] = $value; - } else { - $this->array[$offset] = $value; - } - } - - public function offsetUnset($offset) - { - unset($this->array[$offset]); - } - - public function getIterator() - { - return new \ArrayIterator($this->array); - } - - public function count() - { - return count($this->array); - } - - public function serialize() - { - return serialize($this->array); - } - - public function unserialize($serialized) - { - $this->array = (array) unserialize((string) $serialized); - } -} + +use Symfony\Component\Form\Tests\Fixtures\CustomArrayObject; class MergeCollectionListenerCustomArrayObjectTest extends MergeCollectionListenerTest { protected function getData(array $data) { - $class = __CLASS__ . '_CustomArrayObject'; - - return new $class($data); + return new CustomArrayObject($data); } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerTest.php index 10f34153f9..8be2a62f18 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/MergeCollectionListenerTest.php @@ -16,53 +16,6 @@ use Symfony\Component\Form\Event\FilterDataEvent; use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener; use Symfony\Component\Form\FormBuilder; -class MergeCollectionListenerTest_Car -{ - // In the test, use a name that FormUtil can't uniquely singularify - public function addAxis($axis) {} - - public function removeAxis($axis) {} - - public function getAxes() {} -} - -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() {} -} - -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; @@ -104,66 +57,38 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase return $this->getMock('Symfony\Component\Form\Tests\FormInterface'); } - public function getModesWithNormal() + public function getBooleanMatrix1() { - if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) { - return array(null); - } - return array( - array(MergeCollectionListener::MERGE_NORMAL), - array(MergeCollectionListener::MERGE_NORMAL | MergeCollectionListener::MERGE_INTO_PARENT), + array(true), + array(false), ); } - public function getModesWithMergeIntoParent() - { - if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) { - return array(null); - } - - return array( - array(MergeCollectionListener::MERGE_INTO_PARENT), - array(MergeCollectionListener::MERGE_INTO_PARENT | MergeCollectionListener::MERGE_NORMAL), - ); - } - - public function getModesWithoutMergeIntoParent() - { - if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) { - return array(null); - } - - return array( - array(MergeCollectionListener::MERGE_NORMAL), - ); - } - - public function getInvalidModes() + public function getBooleanMatrix2() { return array( - // 0 is a valid mode, because it is treated as "default" (=3) - array(4), - array(8), + array(true, true), + array(true, false), + array(false, true), + array(false, false), ); } abstract protected function getData(array $data); /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix1 */ - public function testAddExtraEntriesIfAllowAdd($mode) + public function testAddExtraEntriesIfAllowAdd($allowDelete) { $originalData = $this->getData(array(1 => 'second')); $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); - $listener = new MergeCollectionListener(true, false, $mode); + $listener = new MergeCollectionListener(true, $allowDelete); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); @@ -177,19 +102,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix1 */ - public function testAddExtraEntriesIfAllowAddDontOverwriteExistingIndices($mode) + public function testAddExtraEntriesIfAllowAddDontOverwriteExistingIndices($allowDelete) { $originalData = $this->getData(array(1 => 'first')); $newData = $this->getData(array(0 => 'first', 1 => 'second')); - $listener = new MergeCollectionListener(true, false, $mode); + $listener = new MergeCollectionListener(true, $allowDelete); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); @@ -203,20 +126,18 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix1 */ - public function testDoNothingIfNotAllowAdd($mode) + public function testDoNothingIfNotAllowAdd($allowDelete) { $originalDataArray = array(1 => 'second'); $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); - $listener = new MergeCollectionListener(false, false, $mode); + $listener = new MergeCollectionListener(false, $allowDelete); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); @@ -230,19 +151,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix1 */ - public function testRemoveMissingEntriesIfAllowDelete($mode) + public function testRemoveMissingEntriesIfAllowDelete($allowAdd) { $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); $newData = $this->getData(array(1 => 'second')); - $listener = new MergeCollectionListener(false, true, $mode); + $listener = new MergeCollectionListener($allowAdd, true); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); @@ -256,20 +175,18 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix1 */ - public function testDoNothingIfNotAllowDelete($mode) + public function testDoNothingIfNotAllowDelete($allowAdd) { $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third'); $originalData = $this->getData($originalDataArray); $newData = $this->getData(array(1 => 'second')); - $listener = new MergeCollectionListener(false, false, $mode); + $listener = new MergeCollectionListener($allowAdd, false); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); @@ -283,31 +200,26 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix2 * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException */ - public function testRequireArrayOrTraversable($mode) + public function testRequireArrayOrTraversable($allowAdd, $allowDelete) { $newData = 'no array or traversable'; $event = new FilterDataEvent($this->form, $newData); - $listener = new MergeCollectionListener(true, false, $mode); + $listener = new MergeCollectionListener($allowAdd, $allowDelete); $listener->onBindNormData($event); } - /** - * @dataProvider getModesWithNormal - */ - public function testDealWithNullData($mode) + public function testDealWithNullData() { $originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); $newData = null; - $listener = new MergeCollectionListener(false, false, $mode); + $listener = new MergeCollectionListener(false, false); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); @@ -315,19 +227,17 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix1 */ - public function testDealWithNullOriginalDataIfAllowAdd($mode) + public function testDealWithNullOriginalDataIfAllowAdd($allowDelete) { $originalData = null; $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); - $listener = new MergeCollectionListener(true, false, $mode); + $listener = new MergeCollectionListener(true, $allowDelete); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); @@ -335,526 +245,20 @@ abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getModesWithNormal + * @dataProvider getBooleanMatrix1 */ - public function testDontDealWithNullOriginalDataIfNotAllowAdd($mode) + public function testDontDealWithNullOriginalDataIfNotAllowAdd($allowDelete) { $originalData = null; $newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third')); - $listener = new MergeCollectionListener(false, false, $mode); + $listener = new MergeCollectionListener(false, $allowDelete); $this->form->setData($originalData); - $event = new DataEvent($this->form, $newData); - $listener->preBind($event); $event = new FilterDataEvent($this->form, $newData); $listener->onBindNormData($event); $this->assertNull($event->getData()); } - - /** - * @dataProvider getModesWithMergeIntoParent - */ - public function testCallAdderIfAllowAdd($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); - - $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 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 - */ - 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'); - $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(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->onBindNormData($event); - - if (is_object($originalData)) { - $this->assertSame($originalData, $event->getData()); - } - - // The data was not modified - $this->assertEquals($this->getData($originalDataArray), $event->getData()); - } - - /** - * @dataProvider getModesWithoutMergeIntoParent - */ - public function testDontCallAdderIfNotMergeIntoParent($mode) - { - $parentData = $this->getMock(__CLASS__ . '_Car'); - $parentForm = $this->getForm('car'); - $parentForm->setData($parentData); - $parentForm->add($this->form); - - $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->onBindNormData($event); - - if (is_object($originalData)) { - $this->assertSame($originalData, $event->getData()); - } - - // The data was modified without accessors - $this->assertEquals($newData, $event->getData()); - } - - /** - * @dataProvider getModesWithMergeIntoParent - */ - public function testCallRemoverIfAllowDelete($mode) - { - $parentData = $this->getMock(__CLASS__ . '_CarOnlyRemover'); - $parentForm = $this->getForm('car'); - $parentForm->setData($parentData); - $parentForm->add($this->form); - - $originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third'); - $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->onBindNormData($event); - - $this->assertEquals('RESULT', $event->getData()); - } - - /** - * @dataProvider getModesWithMergeIntoParent - */ - public function testDontCallRemoverIfNotAllowDelete($mode) - { - $parentData = $this->getMock(__CLASS__ . '_Car'); - $parentForm = $this->getForm('car'); - $parentForm->setData($parentData); - $parentForm->add($this->form); - - $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); - - $parentData->expects($this->never()) - ->method('removeAxis'); - - $event = new FilterDataEvent($this->form, $newData); - $listener->onBindNormData($event); - - if (is_object($originalData)) { - $this->assertSame($originalData, $event->getData()); - } - - // The data was not modified - $this->assertEquals($this->getData($originalDataArray), $event->getData()); - } - - /** - * @dataProvider getModesWithoutMergeIntoParent - */ - public function testDontCallRemoverIfNotMergeIntoParent($mode) - { - $parentData = $this->getMock(__CLASS__ . '_Car'); - $parentForm = $this->getForm('car'); - $parentForm->setData($parentData); - $parentForm->add($this->form); - - $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->onBindNormData($event); - - if (is_object($originalData)) { - $this->assertSame($originalData, $event->getData()); - } - - // The data was modified directly - $this->assertEquals($newData, $event->getData()); - } - - /** - * @dataProvider getModesWithMergeIntoParent - */ - public function testCallAdderAndDeleterIfAllowAll($mode) - { - $parentData = $this->getMock(__CLASS__ . '_Car'); - $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')); - - $listener = new MergeCollectionListener(true, true, $mode); - - $this->form->setData($originalData); - - $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->onBindNormData($event); - - $this->assertEquals('RESULT', $event->getData()); - } - - /** - * @dataProvider getModesWithMergeIntoParent - */ - public function testCallAccessorsWithCustomNames($mode) - { - $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); - $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')); - - $listener = new MergeCollectionListener(true, true, $mode, 'foo', 'bar'); - - $this->form->setData($originalData); - - $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->onBindNormData($event); - - $this->assertEquals('RESULT', $event->getData()); - } - - /** - * @dataProvider getModesWithMergeIntoParent - */ - public function testDontCallAdderWithCustomNameIfDisallowed($mode) - { - $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); - $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')); - - $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->at(0)) - ->method('bar') - ->with('second'); - $parentData->expects($this->at(1)) - ->method('getAxes') - ->will($this->returnValue('RESULT')); - - $event = new FilterDataEvent($this->form, $newData); - $listener->onBindNormData($event); - - $this->assertEquals('RESULT', $event->getData()); - } - - /** - * @dataProvider getModesWithMergeIntoParent - */ - public function testDontCallRemoverWithCustomNameIfDisallowed($mode) - { - $parentData = $this->getMock(__CLASS__ . '_CarCustomNames'); - $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')); - - $listener = new MergeCollectionListener(true, false, $mode, 'foo', 'bar'); - - $this->form->setData($originalData); - - $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->onBindNormData($event); - - $this->assertEquals('RESULT', $event->getData()); - } - - /** - * @dataProvider getModesWithMergeIntoParent - * @expectedException Symfony\Component\Form\Exception\FormException - */ - public function testThrowExceptionIfInvalidAdder($mode) - { - $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')); - - $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->onBindNormData($event); - } - - /** - * @dataProvider getModesWithMergeIntoParent - * @expectedException Symfony\Component\Form\Exception\FormException - */ - public function testThrowExceptionIfInvalidRemover($mode) - { - $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')); - - $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->onBindNormData($event); - } - - /** - * @dataProvider getInvalidModes - * @expectedException Symfony\Component\Form\Exception\FormException - */ - public function testThrowExceptionIfInvalidMode($mode) - { - new MergeCollectionListener(true, true, $mode); - } } diff --git a/src/Symfony/Component/Form/Tests/Fixtures/CustomArrayObject.php b/src/Symfony/Component/Form/Tests/Fixtures/CustomArrayObject.php new file mode 100644 index 0000000000..b4224b3f3e --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Fixtures/CustomArrayObject.php @@ -0,0 +1,70 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Fixtures; + +/** + * This class is a hand written simplified version of PHP native `ArrayObject` + * class, to show that it behaves differently than the PHP native implementation. + */ +class CustomArrayObject implements \ArrayAccess, \IteratorAggregate, \Countable, \Serializable +{ + private $array; + + public function __construct(array $array = null) + { + $this->array = $array ?: array(); + } + + public function offsetExists($offset) + { + return array_key_exists($offset, $this->array); + } + + public function offsetGet($offset) + { + return $this->array[$offset]; + } + + public function offsetSet($offset, $value) + { + if (null === $offset) { + $this->array[] = $value; + } else { + $this->array[$offset] = $value; + } + } + + public function offsetUnset($offset) + { + unset($this->array[$offset]); + } + + public function getIterator() + { + return new \ArrayIterator($this->array); + } + + public function count() + { + return count($this->array); + } + + public function serialize() + { + return serialize($this->array); + } + + public function unserialize($serialized) + { + $this->array = (array) unserialize((string) $serialized); + } +} \ No newline at end of file diff --git a/src/Symfony/Component/Form/Tests/Util/PropertyPathArrayObjectTest.php b/src/Symfony/Component/Form/Tests/Util/PropertyPathArrayObjectTest.php new file mode 100644 index 0000000000..96ddc6d6f0 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Util/PropertyPathArrayObjectTest.php @@ -0,0 +1,20 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Util; + +class PropertyPathArrayObjectTest extends PropertyPathCollectionTest +{ + protected function getCollection(array $array) + { + return new \ArrayObject($array); + } +} diff --git a/src/Symfony/Component/Form/Tests/Util/PropertyPathArrayTest.php b/src/Symfony/Component/Form/Tests/Util/PropertyPathArrayTest.php new file mode 100644 index 0000000000..ce726c80f8 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Util/PropertyPathArrayTest.php @@ -0,0 +1,20 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Util; + +class PropertyPathArrayTest extends PropertyPathCollectionTest +{ + protected function getCollection(array $array) + { + return $array; + } +} diff --git a/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php b/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php new file mode 100644 index 0000000000..af4c03dd1d --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php @@ -0,0 +1,185 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Util; + +use Symfony\Component\Form\Util\PropertyPath; +use Symfony\Component\Form\Tests\Fixtures\Author; +use Symfony\Component\Form\Tests\Fixtures\Magician; + +class PropertyPathCollectionTest_Car +{ + // In the test, use a name that FormUtil can't uniquely singularify + public function addAxis($axis) {} + + public function removeAxis($axis) {} + + public function getAxes() {} +} + +class PropertyPathCollectionTest_CarCustomSingular +{ + public function addFoo($axis) {} + + public function removeFoo($axis) {} + + public function getAxes() {} +} + +class PropertyPathCollectionTest_Engine +{ +} + +class PropertyPathCollectionTest_CarOnlyAdder +{ + public function addAxis($axis) {} + + public function getAxes() {} +} + +class PropertyPathCollectionTest_CarOnlyRemover +{ + public function removeAxis($axis) {} + + public function getAxes() {} +} + +class PropertyPathCollectionTest_CompositeCar +{ + public function getStructure() {} +} + +class PropertyPathCollectionTest_CarStructure +{ + public function addAxis($axis) {} + + public function removeAxis($axis) {} + + public function getAxes() {} +} + +abstract class PropertyPathCollectionTest extends \PHPUnit_Framework_TestCase +{ + abstract protected function getCollection(array $array); + + public function testSetValueCallsAdderAndRemoverForCollections() + { + $car = $this->getMock(__CLASS__ . '_Car'); + $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); + $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); + + $path = new PropertyPath('axes'); + + $car->expects($this->at(0)) + ->method('getAxes') + ->will($this->returnValue($axesBefore)); + $car->expects($this->at(1)) + ->method('removeAxis') + ->with('fourth'); + $car->expects($this->at(2)) + ->method('addAxis') + ->with('first'); + $car->expects($this->at(3)) + ->method('addAxis') + ->with('third'); + + $path->setValue($car, $axesAfter); + } + + public function testSetValueCallsAdderAndRemoverForNestedCollections() + { + $car = $this->getMock(__CLASS__ . '_CompositeCar'); + $structure = $this->getMock(__CLASS__ . '_CarStructure'); + $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); + $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); + + $path = new PropertyPath('structure.axes'); + + $car->expects($this->any()) + ->method('getStructure') + ->will($this->returnValue($structure)); + + $structure->expects($this->at(0)) + ->method('getAxes') + ->will($this->returnValue($axesBefore)); + $structure->expects($this->at(1)) + ->method('removeAxis') + ->with('fourth'); + $structure->expects($this->at(2)) + ->method('addAxis') + ->with('first'); + $structure->expects($this->at(3)) + ->method('addAxis') + ->with('third'); + + $path->setValue($car, $axesAfter); + } + + public function testSetValueCallsCustomAdderAndRemover() + { + $car = $this->getMock(__CLASS__ . '_CarCustomSingular'); + $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); + $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); + + $path = new PropertyPath('axes|foo'); + + $car->expects($this->at(0)) + ->method('getAxes') + ->will($this->returnValue($axesBefore)); + $car->expects($this->at(1)) + ->method('removeFoo') + ->with('fourth'); + $car->expects($this->at(2)) + ->method('addFoo') + ->with('first'); + $car->expects($this->at(3)) + ->method('addFoo') + ->with('third'); + + $path->setValue($car, $axesAfter); + } + + /** + * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException + */ + public function testMapFormToDataFailsIfOnlyAdderFound() + { + $car = $this->getMock(__CLASS__ . '_CarOnlyAdder'); + $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); + $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); + + $path = new PropertyPath('axes'); + + $car->expects($this->any()) + ->method('getAxes') + ->will($this->returnValue($axesBefore)); + + $path->setValue($car, $axesAfter); + } + + /** + * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException + */ + public function testMapFormToDataFailsIfOnlyRemoverFound() + { + $car = $this->getMock(__CLASS__ . '_CarOnlyRemover'); + $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); + $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); + + $path = new PropertyPath('axes'); + + $car->expects($this->any()) + ->method('getAxes') + ->will($this->returnValue($axesBefore)); + + $path->setValue($car, $axesAfter); + } +} diff --git a/src/Symfony/Component/Form/Tests/Util/PropertyPathCustomArrayObjectTest.php b/src/Symfony/Component/Form/Tests/Util/PropertyPathCustomArrayObjectTest.php new file mode 100644 index 0000000000..a8c6476372 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Util/PropertyPathCustomArrayObjectTest.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Util; + +use Symfony\Component\Form\Tests\Fixtures\CustomArrayObject; + +class PropertyPathCustomArrayObjectTest extends PropertyPathCollectionTest +{ + protected function getCollection(array $array) + { + return new CustomArrayObject($array); + } +} diff --git a/src/Symfony/Component/Form/Tests/PropertyPathTest.php b/src/Symfony/Component/Form/Tests/Util/PropertyPathTest.php similarity index 95% rename from src/Symfony/Component/Form/Tests/PropertyPathTest.php rename to src/Symfony/Component/Form/Tests/Util/PropertyPathTest.php index 25086b6bc8..0b6326da72 100644 --- a/src/Symfony/Component/Form/Tests/PropertyPathTest.php +++ b/src/Symfony/Component/Form/Tests/Util/PropertyPathTest.php @@ -9,7 +9,7 @@ * file that was distributed with this source code. */ -namespace Symfony\Component\Form\Tests; +namespace Symfony\Component\Form\Tests\Util; use Symfony\Component\Form\Util\PropertyPath; use Symfony\Component\Form\Tests\Fixtures\Author; @@ -26,6 +26,15 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase $this->assertEquals('Bernhard', $path->getValue($array)); } + public function testGetValueIgnoresSingular() + { + $array = array('children' => 'Many'); + + $path = new PropertyPath('children|child'); + + $this->assertEquals('Many', $path->getValue($array)); + } + public function testGetValueReadsZeroIndex() { $array = array('Bernhard'); @@ -37,27 +46,27 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase public function testGetValueReadsIndexWithSpecialChars() { - $array = array('#!@$.' => 'Bernhard'); + $array = array('%!@$§.' => 'Bernhard'); - $path = new PropertyPath('[#!@$.]'); + $path = new PropertyPath('[%!@$§.]'); $this->assertEquals('Bernhard', $path->getValue($array)); } - public function testGetValueReadsElementWithSpecialCharsExceptDOt() + public function testGetValueReadsElementWithSpecialCharsExceptDot() { - $array = array('#!@$' => 'Bernhard'); + $array = array('%!@$§' => 'Bernhard'); - $path = new PropertyPath('#!@$'); + $path = new PropertyPath('%!@$§'); $this->assertEquals('Bernhard', $path->getValue($array)); } public function testGetValueReadsNestedIndexWithSpecialChars() { - $array = array('root' => array('#!@$.' => 'Bernhard')); + $array = array('root' => array('%!@$§.' => 'Bernhard')); - $path = new PropertyPath('root[#!@$.]'); + $path = new PropertyPath('root[%!@$§.]'); $this->assertEquals('Bernhard', $path->getValue($array)); } diff --git a/src/Symfony/Component/Form/Util/PropertyPath.php b/src/Symfony/Component/Form/Util/PropertyPath.php index 4babb2a5ea..197a8e0a7e 100644 --- a/src/Symfony/Component/Form/Util/PropertyPath.php +++ b/src/Symfony/Component/Form/Util/PropertyPath.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Form\Util; +use Traversable; +use ReflectionClass; use Symfony\Component\Form\Exception\InvalidPropertyPathException; use Symfony\Component\Form\Exception\InvalidPropertyException; use Symfony\Component\Form\Exception\PropertyAccessDeniedException; @@ -23,12 +25,24 @@ use Symfony\Component\Form\Exception\UnexpectedTypeException; */ class PropertyPath implements \IteratorAggregate { + /** + * Character used for separating between plural and singular of an element. + * @var string + */ + const SINGULAR_SEPARATOR = '|'; + /** * The elements of the property path * @var array */ private $elements = array(); + /** + * The singular forms of the elements in the property path. + * @var array + */ + private $singulars = array(); + /** * The number of elements in the property path * @var integer @@ -76,13 +90,24 @@ class PropertyPath implements \IteratorAggregate $this->positions[] = $position; if ('' !== $matches[2]) { - $this->elements[] = $matches[2]; + $element = $matches[2]; $this->isIndex[] = false; } else { - $this->elements[] = $matches[3]; + $element = $matches[3]; $this->isIndex[] = true; } + $pos = strpos($element, self::SINGULAR_SEPARATOR); + $singular = null; + + if (false !== $pos) { + $singular = substr($element, $pos + 1); + $element = substr($element, 0, $pos); + } + + $this->elements[] = $element; + $this->singulars[] = $singular; + $position += strlen($matches[1]); $remaining = $matches[4]; $pattern = '/^(\.(\w+)|\[([^\]]+)\])(.*)/'; @@ -141,6 +166,7 @@ class PropertyPath implements \IteratorAggregate --$parent->length; $parent->string = substr($parent->string, 0, $parent->positions[$parent->length]); array_pop($parent->elements); + array_pop($parent->singulars); array_pop($parent->isIndex); array_pop($parent->positions); @@ -329,7 +355,7 @@ class PropertyPath implements \IteratorAggregate } } else { $camelProp = $this->camelize($property); - $reflClass = new \ReflectionClass($object); + $reflClass = new ReflectionClass($object); $getter = 'get'.$camelProp; $isser = 'is'.$camelProp; $hasser = 'has'.$camelProp; @@ -388,10 +414,104 @@ class PropertyPath implements \IteratorAggregate $objectOrArray[$property] = $value; } elseif (is_object($objectOrArray)) { - $reflClass = new \ReflectionClass($objectOrArray); + $reflClass = new ReflectionClass($objectOrArray); $setter = 'set'.$this->camelize($property); + $addMethod = null; + $removeMethod = null; + $plural = null; - if ($reflClass->hasMethod($setter)) { + // Check if the parent has matching methods to add/remove items + if (is_array($value) || $value instanceof Traversable) { + $singular = $this->singulars[$currentIndex]; + if (null !== $singular) { + $addMethod = 'add' . ucfirst($singular); + $removeMethod = 'remove' . ucfirst($singular); + + if (!$this->isAccessible($reflClass, $addMethod, 1)) { + throw new InvalidPropertyException(sprintf( + 'The public method "%s" with exactly one required parameter was not found on class %s', + $addMethod, + $reflClass->getName() + )); + } + + if (!$this->isAccessible($reflClass, $removeMethod, 1)) { + throw new InvalidPropertyException(sprintf( + 'The public method "%s" with exactly one required parameter was not found on class %s', + $removeMethod, + $reflClass->getName() + )); + } + } else { + // The plural form is the last element of the property path + $plural = ucfirst($this->elements[$this->length - 1]); + + // Any of the two methods is required, but not yet known + $singulars = (array) FormUtil::singularify($plural); + + foreach ($singulars as $singular) { + $addMethodName = 'add' . $singular; + $removeMethodName = 'remove' . $singular; + + if ($this->isAccessible($reflClass, $addMethodName, 1)) { + $addMethod = $addMethodName; + } + + if ($this->isAccessible($reflClass, $removeMethodName, 1)) { + $removeMethod = $removeMethodName; + } + + if ($addMethod && !$removeMethod) { + throw new InvalidPropertyException(sprintf( + 'Found the public method "%s", but did not find a public "%s" on class %s', + $addMethodName, + $removeMethodName, + $reflClass->getName() + )); + } + + if ($removeMethod && !$addMethod) { + throw new InvalidPropertyException(sprintf( + 'Found the public method "%s", but did not find a public "%s" on class %s', + $removeMethodName, + $addMethodName, + $reflClass->getName() + )); + } + + if ($addMethod && $removeMethod) { + break; + } + } + } + } + + // Collection with matching adder/remover in $objectOrArray + if ($addMethod && $removeMethod) { + $itemsToAdd = is_object($value) ? clone $value : $value; + $previousValue = $this->readProperty($objectOrArray, $currentIndex); + + if (is_array($previousValue) || $previousValue instanceof Traversable) { + foreach ($previousValue as $previousItem) { + foreach ($value as $key => $item) { + if ($item === $previousItem) { + // Item found, don't add + unset($itemsToAdd[$key]); + + // Next $previousItem + continue 2; + } + } + + // Item not found, remove + $objectOrArray->$removeMethod($previousItem); + } + } + + foreach ($itemsToAdd as $item) { + $objectOrArray->$addMethod($item); + } + } elseif ($reflClass->hasMethod($setter)) { if (!$reflClass->getMethod($setter)->isPublic()) { throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $setter, $reflClass->getName())); } @@ -421,4 +541,17 @@ class PropertyPath implements \IteratorAggregate { return preg_replace_callback('/(^|_|\.)+(.)/', function ($match) { return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); }, $property); } + + private function isAccessible(ReflectionClass $reflClass, $methodName, $numberOfRequiredParameters) + { + if ($reflClass->hasMethod($methodName)) { + $method = $reflClass->getMethod($methodName); + + if ($method->isPublic() && $method->getNumberOfRequiredParameters() === $numberOfRequiredParameters) { + return true; + } + } + + return false; + } }