From 26eba769b5a1f9a13df71675f80f0269d89b1c2b Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 26 Mar 2015 10:52:07 +0100 Subject: [PATCH] [Form] Fixed regression: Choices are compared by their values if a value callback is given --- .../Form/ChoiceList/DoctrineChoiceLoader.php | 97 ++--------- .../Doctrine/Form/ChoiceList/IdReader.php | 125 ++++++++++++++ .../Doctrine/Form/Type/DoctrineType.php | 130 +++++++++----- .../Tests/Form/Type/EntityTypeTest.php | 1 - .../Form/ChoiceList/ArrayChoiceList.php | 22 ++- .../Factory/DefaultChoiceListFactory.php | 28 +-- .../Factory/PropertyAccessDecorator.php | 10 +- .../Form/ChoiceList/LazyChoiceList.php | 10 +- .../Tests/ChoiceList/ArrayChoiceListTest.php | 24 +-- .../ChoiceList/ArrayKeyChoiceListTest.php | 10 +- .../Factory/DefaultChoiceListFactoryTest.php | 163 +++++++++--------- 11 files changed, 361 insertions(+), 259 deletions(-) create mode 100644 src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php index c00c258ca5..4b10b45855 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php @@ -11,12 +11,10 @@ namespace Symfony\Bridge\Doctrine\Form\ChoiceList; -use Doctrine\Common\Persistence\Mapping\ClassMetadata; use Doctrine\Common\Persistence\ObjectManager; use Symfony\Component\Form\ChoiceList\ChoiceListInterface; use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface; use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface; -use Symfony\Component\Form\Exception\RuntimeException; /** * Loads choices using a Doctrine object manager. @@ -41,67 +39,20 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface private $class; /** - * @var ClassMetadata + * @var IdReader */ - private $classMetadata; + private $idReader; /** * @var null|EntityLoaderInterface */ private $objectLoader; - /** - * The identifier field, unless the identifier is composite - * - * @var null|string - */ - private $idField = null; - - /** - * Whether to use the identifier for value generation - * - * @var bool - */ - private $compositeId = true; - /** * @var ChoiceListInterface */ private $choiceList; - /** - * Returns the value of the identifier field of an object. - * - * Doctrine must know about this object, that is, the object must already - * be persisted or added to the identity map before. Otherwise an - * exception is thrown. - * - * This method assumes that the object has a single-column identifier and - * will return a single value instead of an array. - * - * @param object $object The object for which to get the identifier - * - * @return int|string The identifier value - * - * @throws RuntimeException If the object does not exist in Doctrine's identity map - * - * @internal Should not be accessed by user-land code. This method is public - * only to be usable as callback. - */ - public static function getIdValue(ObjectManager $om, ClassMetadata $classMetadata, $object) - { - if (!$om->contains($object)) { - throw new RuntimeException( - 'Entities passed to the choice field must be managed. Maybe '. - 'persist them in the entity manager?' - ); - } - - $om->initializeObject($object); - - return current($classMetadata->getIdentifierValues($object)); - } - /** * Creates a new choice loader. * @@ -114,22 +65,17 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface * @param ObjectManager $manager The object manager * @param string $class The class name of the * loaded objects + * @param IdReader $idReader The reader for the object + * IDs. * @param null|EntityLoaderInterface $objectLoader The objects loader */ - public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, EntityLoaderInterface $objectLoader = null) + public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, IdReader $idReader, EntityLoaderInterface $objectLoader = null) { $this->factory = $factory; $this->manager = $manager; - $this->classMetadata = $manager->getClassMetadata($class); - $this->class = $this->classMetadata->getName(); + $this->class = $manager->getClassMetadata($class)->getName(); + $this->idReader = $idReader; $this->objectLoader = $objectLoader; - - $identifier = $this->classMetadata->getIdentifierFieldNames(); - - if (1 === count($identifier)) { - $this->idField = $identifier[0]; - $this->compositeId = false; - } } /** @@ -145,23 +91,7 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface ? $this->objectLoader->getEntities() : $this->manager->getRepository($this->class)->findAll(); - // If the class has a multi-column identifier, we cannot index the - // objects by their IDs - if ($this->compositeId) { - $this->choiceList = $this->factory->createListFromChoices($objects, $value); - - return $this->choiceList; - } - - // Index the objects by ID - $objectsById = array(); - - foreach ($objects as $object) { - $id = self::getIdValue($this->manager, $this->classMetadata, $object); - $objectsById[$id] = $object; - } - - $this->choiceList = $this->factory->createListFromChoices($objectsById, $value); + $this->choiceList = $this->factory->createListFromChoices($objects, $value); return $this->choiceList; } @@ -193,14 +123,14 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface // know that the IDs are used as values // Attention: This optimization does not check choices for existence - if (!$this->choiceList && !$this->compositeId) { + if (!$this->choiceList && $this->idReader->isSingleId()) { $values = array(); // Maintain order and indices of the given objects foreach ($objects as $i => $object) { if ($object instanceof $this->class) { // Make sure to convert to the right format - $values[$i] = (string) self::getIdValue($this->manager, $this->classMetadata, $object); + $values[$i] = (string) $this->idReader->getIdValue($object); } } @@ -240,8 +170,8 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface // Optimize performance in case we have an object loader and // a single-field identifier - if (!$this->choiceList && !$this->compositeId && $this->objectLoader) { - $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idField, $values); + if (!$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) { + $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values); $objectsById = array(); $objects = array(); @@ -250,8 +180,7 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface // "INDEX BY" clause to the Doctrine query in the loader, // but I'm not sure whether that's doable in a generic fashion. foreach ($unorderedObjects as $object) { - $id = self::getIdValue($this->manager, $this->classMetadata, $object); - $objectsById[$id] = $object; + $objectsById[$this->idReader->getIdValue($object)] = $object; } foreach ($values as $i => $id) { diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php new file mode 100644 index 0000000000..f6164725fd --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php @@ -0,0 +1,125 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Doctrine\Form\ChoiceList; + +use Doctrine\Common\Persistence\Mapping\ClassMetadata; +use Doctrine\Common\Persistence\ObjectManager; +use Symfony\Component\Form\Exception\RuntimeException; + +/** + * A utility for reading object IDs. + * + * @since 1.0 + * @author Bernhard Schussek + * + * @internal This class is meant for internal use only. + */ +class IdReader +{ + /** + * @var ObjectManager + */ + private $om; + + /** + * @var ClassMetadata + */ + private $classMetadata; + + /** + * @var bool + */ + private $singleId; + + /** + * @var bool + */ + private $intId; + + /** + * @var string + */ + private $idField; + + public function __construct(ObjectManager $om, ClassMetadata $classMetadata) + { + $ids = $classMetadata->getIdentifierFieldNames(); + $idType = $classMetadata->getTypeOfField(current($ids)); + + $this->om = $om; + $this->classMetadata = $classMetadata; + $this->singleId = 1 === count($ids); + $this->intId = $this->singleId && 1 === count($ids) && in_array($idType, array('integer', 'smallint', 'bigint')); + $this->idField = current($ids); + } + + /** + * Returns whether the class has a single-column ID. + * + * @return bool Returns `true` if the class has a single-column ID and + * `false` otherwise. + */ + public function isSingleId() + { + return $this->singleId; + } + + /** + * Returns whether the class has a single-column integer ID. + * + * @return bool Returns `true` if the class has a single-column integer ID + * and `false` otherwise. + */ + public function isIntId() + { + return $this->intId; + } + + /** + * Returns the ID value for an object. + * + * This method assumes that the object has a single-column ID. + * + * @param object $object The object. + * + * @return mixed The ID value. + */ + public function getIdValue($object) + { + if (!$object) { + return null; + } + + if (!$this->om->contains($object)) { + throw new RuntimeException( + 'Entities passed to the choice field must be managed. Maybe '. + 'persist them in the entity manager?' + ); + } + + $this->om->initializeObject($object); + + return current($this->classMetadata->getIdentifierValues($object)); + } + + /** + * Returns the name of the ID field. + * + * This method assumes that the object has a single-column ID. + * + * @return string The name of the ID field. + */ + public function getIdField() + { + return $this->idField; + } +} diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php index ed8ded5bad..6020d95e92 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php @@ -16,6 +16,7 @@ use Doctrine\Common\Persistence\ObjectManager; use Doctrine\ORM\QueryBuilder; use Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader; use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface; +use Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader; use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer; use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener; use Symfony\Component\Form\AbstractType; @@ -42,11 +43,55 @@ abstract class DoctrineType extends AbstractType */ private $choiceListFactory; + /** + * @var IdReader[] + */ + private $idReaders = array(); + /** * @var DoctrineChoiceLoader[] */ private $choiceLoaders = array(); + /** + * Creates the label for a choice. + * + * For backwards compatibility, objects are cast to strings by default. + * + * @param object $choice The object. + * + * @return string The string representation of the object. + * + * @internal This method is public to be usable as callback. It should not + * be used in user code. + */ + public static function createChoiceLabel($choice) + { + return (string) $choice; + } + + /** + * Creates the field name for a choice. + * + * This method is used to generate field names if the underlying object has + * a single-column integer ID. In that case, the value of the field is + * the ID of the object. That ID is also used as field name. + * + * @param object $choice The object. + * @param int|string $key The choice key. + * @param string $value The choice value. Corresponds to the object's + * ID here. + * + * @return string The field name. + * + * @internal This method is public to be usable as callback. It should not + * be used in user code. + */ + public static function createChoiceName($choice, $key, $value) + { + return (string) $value; + } + public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null) { $this->registry = $registry; @@ -67,9 +112,30 @@ abstract class DoctrineType extends AbstractType { $registry = $this->registry; $choiceListFactory = $this->choiceListFactory; + $idReaders = &$this->idReaders; $choiceLoaders = &$this->choiceLoaders; $type = $this; + $idReader = function (Options $options) use (&$idReaders) { + $hash = CachingFactoryDecorator::generateHash(array( + $options['em'], + $options['class'], + )); + + // The ID reader is a utility that is needed to read the object IDs + // when generating the field values. The callback generating the + // field values has no access to the object manager or the class + // of the field, so we store that information in the reader. + // The reader is cached so that two choice lists for the same class + // (and hence with the same reader) can successfully be cached. + if (!isset($idReaders[$hash])) { + $classMetadata = $options['em']->getClassMetadata($options['class']); + $idReaders[$hash] = new IdReader($options['em'], $classMetadata); + } + + return $idReaders[$hash]; + }; + $choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) { // Unless the choices are given explicitly, load them on demand if (null === $options['choices']) { @@ -106,6 +172,7 @@ abstract class DoctrineType extends AbstractType $choiceListFactory, $options['em'], $options['class'], + $options['id_reader'], $entityLoader ); @@ -120,24 +187,18 @@ abstract class DoctrineType extends AbstractType } // BC: use __toString() by default - return function ($entity) { - return (string) $entity; - }; + return array(__CLASS__, 'createChoiceLabel'); }; $choiceName = function (Options $options) { - /** @var ObjectManager $om */ - $om = $options['em']; - $classMetadata = $om->getClassMetadata($options['class']); - $ids = $classMetadata->getIdentifierFieldNames(); - $idType = $classMetadata->getTypeOfField(current($ids)); + /** @var IdReader $idReader */ + $idReader = $options['id_reader']; - // If the entity has a single-column, numeric ID, use that ID as - // field name - if (1 === count($ids) && in_array($idType, array('integer', 'smallint', 'bigint'))) { - return function ($entity, $id) { - return $id; - }; + // If the object has a single-column, numeric ID, use that ID as + // field name. We can only use numeric IDs as names, as we cannot + // guarantee that a non-numeric ID contains a valid form name + if ($idReader->isIntId()) { + return array(__CLASS__, 'createChoiceName'); } // Otherwise, an incrementing integer is used as name automatically @@ -147,8 +208,16 @@ abstract class DoctrineType extends AbstractType // and DoctrineChoiceLoader), unless the ID is composite. Then they // are indexed by an incrementing integer. // Use the ID/incrementing integer as choice value. - $choiceValue = function ($entity, $key) { - return $key; + $choiceValue = function (Options $options) { + /** @var IdReader $idReader */ + $idReader = $options['id_reader']; + + // If the entity has a single-column ID, use that ID as value + if ($idReader->isSingleId()) { + return array($idReader, 'getIdValue'); + } + + // Otherwise, an incrementing integer is used as value automatically }; $emNormalizer = function (Options $options, $em) use ($registry) { @@ -174,33 +243,6 @@ abstract class DoctrineType extends AbstractType return $em; }; - $choicesNormalizer = function (Options $options, $entities) { - if (null === $entities || 0 === count($entities)) { - return $entities; - } - - // Make sure that the entities are indexed by their ID - /** @var ObjectManager $om */ - $om = $options['em']; - $classMetadata = $om->getClassMetadata($options['class']); - $ids = $classMetadata->getIdentifierFieldNames(); - - // We cannot use composite IDs as indices. In that case, keep the - // given indices - if (count($ids) > 1) { - return $entities; - } - - $entitiesById = array(); - - foreach ($entities as $entity) { - $id = DoctrineChoiceLoader::getIdValue($om, $classMetadata, $entity); - $entitiesById[$id] = $entity; - } - - return $entitiesById; - }; - // Invoke the query builder closure so that we can cache choice lists // for equal query builders $queryBuilderNormalizer = function (Options $options, $queryBuilder) { @@ -226,12 +268,12 @@ abstract class DoctrineType extends AbstractType 'choice_label' => $choiceLabel, 'choice_name' => $choiceName, 'choice_value' => $choiceValue, + 'id_reader' => $idReader, )); $resolver->setRequired(array('class')); $resolver->setNormalizer('em', $emNormalizer); - $resolver->setNormalizer('choices', $choicesNormalizer); $resolver->setNormalizer('query_builder', $queryBuilderNormalizer); $resolver->setAllowedTypes('em', array('null', 'string', 'Doctrine\Common\Persistence\ObjectManager')); diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php index 7d80819f6b..95e11aff6f 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php @@ -29,7 +29,6 @@ use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView; use Symfony\Component\Form\ChoiceList\View\ChoiceView; use Symfony\Component\Form\Forms; use Symfony\Component\Form\Test\TypeTestCase; -use Symfony\Component\OptionsResolver\Options; use Symfony\Component\PropertyAccess\PropertyAccess; class EntityTypeTest extends TypeTestCase diff --git a/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php b/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php index a3987cc02c..515cd15a83 100644 --- a/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php +++ b/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php @@ -51,15 +51,12 @@ class ArrayChoiceList implements ChoiceListInterface * * The given choice array must have the same array keys as the value array. * - * @param array $choices The selectable choices - * @param callable $value The callable for creating the value for a - * choice. If `null` is passed, incrementing - * integers are used as values - * @param bool $compareByValue Whether to use the value callback to - * compare choices. If `null`, choices are - * compared by identity + * @param array $choices The selectable choices + * @param callable $value The callable for creating the value for a + * choice. If `null` is passed, incrementing + * integers are used as values */ - public function __construct(array $choices, $value = null, $compareByValue = false) + public function __construct(array $choices, $value = null) { if (null !== $value && !is_callable($value)) { throw new UnexpectedTypeException($value, 'null or callable'); @@ -67,7 +64,7 @@ class ArrayChoiceList implements ChoiceListInterface $this->choices = $choices; $this->values = array(); - $this->valueCallback = $compareByValue ? $value : null; + $this->valueCallback = $value; if (null === $value) { $i = 0; @@ -76,7 +73,7 @@ class ArrayChoiceList implements ChoiceListInterface } } else { foreach ($choices as $key => $choice) { - $this->values[$key] = (string) call_user_func($value, $choice, $key); + $this->values[$key] = (string) call_user_func($value, $choice); } } } @@ -132,8 +129,9 @@ class ArrayChoiceList implements ChoiceListInterface // Use the value callback to compare choices by their values, if present if ($this->valueCallback) { $givenValues = array(); - foreach ($choices as $key => $choice) { - $givenValues[$key] = (string) call_user_func($this->valueCallback, $choice, $key); + + foreach ($choices as $i => $givenChoice) { + $givenValues[$i] = (string) call_user_func($this->valueCallback, $givenChoice); } return array_intersect($givenValues, $this->values); diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php b/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php index f0ea07cdf6..d974bf7d4f 100644 --- a/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php +++ b/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php @@ -191,10 +191,7 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface // The names are generated from an incrementing integer by default if (null === $index) { - $i = 0; - $index = function () use (&$i) { - return $i++; - }; + $index = 0; } // If $groupBy is not given, no grouping is done @@ -267,27 +264,30 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface return new ChoiceListView($otherViews, $preferredViews); } - private static function addChoiceView($choice, $key, $label, $values, $index, $attr, $isPreferred, &$preferredViews, &$otherViews) + private static function addChoiceView($choice, $key, $label, $values, &$index, $attr, $isPreferred, &$preferredViews, &$otherViews) { + $value = $values[$key]; + $nextIndex = is_int($index) ? $index++ : call_user_func($index, $choice, $key, $value); + $view = new ChoiceView( // If the labels are null, use the choice key by default - null === $label ? (string) $key : (string) call_user_func($label, $choice, $key), - $values[$key], + null === $label ? (string) $key : (string) call_user_func($label, $choice, $key, $value), + $value, $choice, // The attributes may be a callable or a mapping from choice indices // to nested arrays - is_callable($attr) ? call_user_func($attr, $choice, $key) : (isset($attr[$key]) ? $attr[$key] : array()) + is_callable($attr) ? call_user_func($attr, $choice, $key, $value) : (isset($attr[$key]) ? $attr[$key] : array()) ); // $isPreferred may be null if no choices are preferred - if ($isPreferred && call_user_func($isPreferred, $choice, $key)) { - $preferredViews[call_user_func($index, $choice, $key)] = $view; + if ($isPreferred && call_user_func($isPreferred, $choice, $key, $value)) { + $preferredViews[$nextIndex] = $view; } else { - $otherViews[call_user_func($index, $choice, $key)] = $view; + $otherViews[$nextIndex] = $view; } } - private static function addChoiceViewsGroupedBy($groupBy, $label, $choices, $values, $index, $attr, $isPreferred, &$preferredViews, &$otherViews) + private static function addChoiceViewsGroupedBy($groupBy, $label, $choices, $values, &$index, $attr, $isPreferred, &$preferredViews, &$otherViews) { foreach ($groupBy as $key => $content) { // Add the contents of groups to new ChoiceGroupView instances @@ -333,9 +333,9 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface } } - private static function addChoiceViewGroupedBy($groupBy, $choice, $key, $label, $values, $index, $attr, $isPreferred, &$preferredViews, &$otherViews) + private static function addChoiceViewGroupedBy($groupBy, $choice, $key, $label, $values, &$index, $attr, $isPreferred, &$preferredViews, &$otherViews) { - $groupLabel = call_user_func($groupBy, $choice, $key); + $groupLabel = call_user_func($groupBy, $choice, $key, $values[$key]); if (null === $groupLabel) { // If the callable returns null, don't group the choice diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php b/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php index bf91d85eea..131690a6ff 100644 --- a/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php +++ b/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php @@ -91,7 +91,15 @@ class PropertyAccessDecorator implements ChoiceListFactoryInterface if ($value instanceof PropertyPath) { $accessor = $this->propertyAccessor; $value = function ($choice) use ($accessor, $value) { - return $accessor->getValue($choice, $value); + // The callable may be invoked with a non-object/array value + // when such values are passed to + // ChoiceListInterface::getValuesForChoices(). Handle this case + // so that the call to getValue() doesn't break. + if (is_object($choice) || is_array($choice)) { + return $accessor->getValue($choice, $value); + } + + return null; }; } diff --git a/src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php b/src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php index 91e6bfe408..3dea398c6d 100644 --- a/src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php +++ b/src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php @@ -43,6 +43,13 @@ class LazyChoiceList implements ChoiceListInterface */ private $value; + /** + * Whether to use the value callback to compare choices. + * + * @var bool + */ + private $compareByValue; + /** * @var ChoiceListInterface */ @@ -59,10 +66,11 @@ class LazyChoiceList implements ChoiceListInterface * @param null|callable $value The callable generating the choice * values */ - public function __construct(ChoiceLoaderInterface $loader, $value = null) + public function __construct(ChoiceLoaderInterface $loader, $value = null, $compareByValue = false) { $this->loader = $loader; $this->value = $value; + $this->compareByValue = $compareByValue; } /** diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php index 0dffd08374..129a093b89 100644 --- a/src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php +++ b/src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php @@ -54,15 +54,15 @@ class ArrayChoiceListTest extends AbstractChoiceListTest public function testCreateChoiceListWithValueCallback() { - $callback = function ($choice, $key) { - return $key.':'.$choice; + $callback = function ($choice) { + return ':'.$choice; }; $choiceList = new ArrayChoiceList(array(2 => 'foo', 7 => 'bar', 10 => 'baz'), $callback); - $this->assertSame(array(2 => '2:foo', 7 => '7:bar', 10 => '10:baz'), $choiceList->getValues()); - $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz'))); - $this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz'))); + $this->assertSame(array(2 => ':foo', 7 => ':bar', 10 => ':baz'), $choiceList->getValues()); + $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => ':foo', 2 => ':baz'))); + $this->assertSame(array(1 => ':foo', 2 => ':baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz'))); } public function testCompareChoicesByIdentityByDefault() @@ -76,20 +76,6 @@ class ArrayChoiceListTest extends AbstractChoiceListTest $choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback); $this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2))); - $this->assertSame(array(), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2')))); - } - - public function testCompareChoicesWithValueCallbackIfCompareByValue() - { - $callback = function ($choice) { - return $choice->value; - }; - - $obj1 = (object) array('value' => 'value1'); - $obj2 = (object) array('value' => 'value2'); - - $choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback, true); - $this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2))); $this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2')))); } } diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/ArrayKeyChoiceListTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/ArrayKeyChoiceListTest.php index 78263502d6..5024a60db7 100644 --- a/src/Symfony/Component/Form/Tests/ChoiceList/ArrayKeyChoiceListTest.php +++ b/src/Symfony/Component/Form/Tests/ChoiceList/ArrayKeyChoiceListTest.php @@ -183,14 +183,14 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest public function testCreateChoiceListWithValueCallback() { - $callback = function ($choice, $key) { - return $key.':'.$choice; + $callback = function ($choice) { + return ':'.$choice; }; $choiceList = new ArrayKeyChoiceList(array(2 => 'foo', 7 => 'bar', 10 => 'baz'), $callback); - $this->assertSame(array(2 => '2:foo', 7 => '7:bar', 10 => '10:baz'), $choiceList->getValues()); - $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz'))); - $this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz'))); + $this->assertSame(array(2 => ':foo', 7 => ':bar', 10 => ':baz'), $choiceList->getValues()); + $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => ':foo', 2 => ':baz'))); + $this->assertSame(array(1 => ':foo', 2 => ':baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz'))); } } diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php index b144699892..360d46729f 100644 --- a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php +++ b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php @@ -150,23 +150,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertObjectListWithCustomValues($list); } - public function testCreateFromChoicesFlatValuesClosureReceivesKey() - { - $list = $this->factory->createListFromChoices( - array('A' => $this->obj1, 'B' => $this->obj2, 'C' => $this->obj3, 'D' => $this->obj4), - function ($object, $key) { - switch ($key) { - case 'A': return 'a'; - case 'B': return 'b'; - case 'C': return '1'; - case 'D': return '2'; - } - } - ); - - $this->assertObjectListWithCustomValues($list); - } - public function testCreateFromChoicesGrouped() { $list = $this->factory->createListFromChoices( @@ -217,26 +200,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertObjectListWithCustomValues($list); } - public function testCreateFromChoicesGroupedValuesAsClosureReceivesKey() - { - $list = $this->factory->createListFromChoices( - array( - 'Group 1' => array('A' => $this->obj1, 'B' => $this->obj2), - 'Group 2' => array('C' => $this->obj3, 'D' => $this->obj4), - ), - function ($object, $key) { - switch ($key) { - case 'A': return 'a'; - case 'B': return 'b'; - case 'C': return '1'; - case 'D': return '2'; - } - } - ); - - $this->assertObjectListWithCustomValues($list); - } - /** * @expectedException \Symfony\Component\Form\Exception\UnexpectedTypeException */ @@ -306,23 +269,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertScalarListWithCustomValues($list); } - public function testCreateFromFlippedChoicesFlatValuesClosureReceivesKey() - { - $list = $this->factory->createListFromFlippedChoices( - array('a' => 'A', 'b' => 'B', 'c' => 'C', 'd' => 'D'), - function ($choice, $key) { - switch ($key) { - case 'A': return 'a'; - case 'B': return 'b'; - case 'C': return '1'; - case 'D': return '2'; - } - } - ); - - $this->assertScalarListWithCustomValues($list); - } - public function testCreateFromFlippedChoicesGrouped() { $list = $this->factory->createListFromFlippedChoices( @@ -380,26 +326,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertScalarListWithCustomValues($list); } - public function testCreateFromFlippedChoicesGroupedValuesAsClosureReceivesKey() - { - $list = $this->factory->createListFromFlippedChoices( - array( - 'Group 1' => array('a' => 'A', 'b' => 'B'), - 'Group 2' => array('c' => 'C', 'd' => 'D'), - ), - function ($choice, $key) { - switch ($key) { - case 'A': return 'a'; - case 'B': return 'b'; - case 'C': return '1'; - case 'D': return '2'; - } - } - ); - - $this->assertScalarListWithCustomValues($list); - } - public function testCreateFromLoader() { $loader = $this->getMock('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface'); @@ -537,12 +463,9 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase public function testCreateViewFlatPreferredChoicesClosureReceivesKey() { - $obj2 = $this->obj2; - $obj3 = $this->obj3; - $view = $this->factory->createView( $this->list, - function ($object, $key) use ($obj2, $obj3) { + function ($object, $key) { return 'B' === $key || 'C' === $key; } ); @@ -550,6 +473,18 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertFlatView($view); } + public function testCreateViewFlatPreferredChoicesClosureReceivesValue() + { + $view = $this->factory->createView( + $this->list, + function ($object, $key, $value) { + return '1' === $value || '2' === $value; + } + ); + + $this->assertFlatView($view); + } + public function testCreateViewFlatLabelAsCallable() { $view = $this->factory->createView( @@ -587,6 +522,24 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertFlatView($view); } + public function testCreateViewFlatLabelClosureReceivesValue() + { + $view = $this->factory->createView( + $this->list, + array($this->obj2, $this->obj3), + function ($object, $key, $value) { + switch ($value) { + case '0': return 'A'; + case '1': return 'B'; + case '2': return 'C'; + case '3': return 'D'; + } + } + ); + + $this->assertFlatView($view); + } + public function testCreateViewFlatIndexAsCallable() { $view = $this->factory->createView( @@ -632,6 +585,25 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertFlatViewWithCustomIndices($view); } + public function testCreateViewFlatIndexClosureReceivesValue() + { + $view = $this->factory->createView( + $this->list, + array($this->obj2, $this->obj3), + null, // label + function ($object, $key, $value) { + switch ($value) { + case '0': return 'w'; + case '1': return 'x'; + case '2': return 'y'; + case '3': return 'z'; + } + } + ); + + $this->assertFlatViewWithCustomIndices($view); + } + public function testCreateViewFlatGroupByAsArray() { $view = $this->factory->createView( @@ -724,6 +696,21 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertGroupedView($view); } + public function testCreateViewFlatGroupByClosureReceivesValue() + { + $view = $this->factory->createView( + $this->list, + array($this->obj2, $this->obj3), + null, // label + null, // index + function ($object, $key, $value) { + return '0' === $value || '1' === $value ? 'Group 1' : 'Group 2'; + } + ); + + $this->assertGroupedView($view); + } + public function testCreateViewFlatAttrAsArray() { $view = $this->factory->createView( @@ -805,6 +792,26 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase $this->assertFlatViewWithAttr($view); } + public function testCreateViewFlatAttrClosureReceivesValue() + { + $view = $this->factory->createView( + $this->list, + array($this->obj2, $this->obj3), + null, // label + null, // index + null, // group + function ($object, $key, $value) { + switch ($value) { + case '1': return array('attr1' => 'value1'); + case '2': return array('attr2' => 'value2'); + default: return array(); + } + } + ); + + $this->assertFlatViewWithAttr($view); + } + public function testCreateViewForLegacyChoiceList() { $preferred = array(new ChoiceView('Preferred', 'x', 'x'));