From d0696e385deb4a4a18fb88661fda01c4f03abde0 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 26 Nov 2011 17:36:55 +0100 Subject: [PATCH 1/6] [DoctrineBridge] Refactor entity choice list to be ORM independant using an EntityLoader interface. --- .../Form/ChoiceList/EntityChoiceList.php | 74 ++++------------ .../Form/ChoiceList/EntityLoaderInterface.php | 36 ++++++++ .../Form/ChoiceList/ORMQueryBuilderLoader.php | 86 +++++++++++++++++++ .../Bridge/Doctrine/Form/Type/EntityType.php | 15 +++- 4 files changed, 153 insertions(+), 58 deletions(-) create mode 100644 src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php create mode 100644 src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php index 3367e95bfb3ef..9afb0239588e6 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php @@ -15,7 +15,7 @@ use Symfony\Component\Form\Exception\FormException; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\Form\Extension\Core\ChoiceList\ArrayChoiceList; -use Doctrine\ORM\EntityManager; +use Doctrine\Common\Persistence\ObjectManager; use Doctrine\ORM\QueryBuilder; use Doctrine\ORM\NoResultException; @@ -52,7 +52,7 @@ class EntityChoiceList extends ArrayChoiceList * * @var Doctrine\ORM\QueryBuilder */ - private $queryBuilder; + private $entityLoader; /** * The fields of which the identifier of the underlying class consists @@ -63,15 +63,6 @@ class EntityChoiceList extends ArrayChoiceList */ private $identifier = array(); - /** - * A cache for \ReflectionProperty instances for the underlying class - * - * This property should only be accessed through getReflProperty(). - * - * @var array - */ - private $reflProperties = array(); - /** * A cache for the UnitOfWork instance of Doctrine * @@ -79,6 +70,11 @@ class EntityChoiceList extends ArrayChoiceList */ private $unitOfWork; + /** + * Property path to access the key value of this choice-list. + * + * @var PropertyPath + */ private $propertyPath; /** @@ -88,27 +84,13 @@ class EntityChoiceList extends ArrayChoiceList */ private $groupBy; - public function __construct(EntityManager $em, $class, $property = null, $queryBuilder = null, $choices = array(), $groupBy = null) + public function __construct(ObjectManager $manager, $class, $property = null, EntityLoaderInterface $entityLoader = null, $choices = array(), $groupBy = null) { - // If a query builder was passed, it must be a closure or QueryBuilder - // instance - if (!(null === $queryBuilder || $queryBuilder instanceof QueryBuilder || $queryBuilder instanceof \Closure)) { - throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure'); - } - - if ($queryBuilder instanceof \Closure) { - $queryBuilder = $queryBuilder($em->getRepository($class)); - - if (!$queryBuilder instanceof QueryBuilder) { - throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder'); - } - } - - $this->em = $em; + $this->em = $manager; $this->class = $class; - $this->queryBuilder = $queryBuilder; - $this->unitOfWork = $em->getUnitOfWork(); - $this->identifier = $em->getClassMetadata($class)->getIdentifierFieldNames(); + $this->entityLoader = $entityLoader; + $this->unitOfWork = $manager->getUnitOfWork(); + $this->identifier = $manager->getClassMetadata($class)->getIdentifierFieldNames(); $this->groupBy = $groupBy; // The property option defines, which property (path) is used for @@ -137,8 +119,8 @@ protected function load() if ($this->choices) { $entities = $this->choices; - } else if ($qb = $this->queryBuilder) { - $entities = $qb->getQuery()->execute(); + } else if ($entityLoader = $this->entityLoader) { + $entities = $entityLoader->getEntities(); } else { $entities = $this->em->getRepository($this->class)->findAll(); } @@ -158,11 +140,11 @@ protected function load() private function groupEntities($entities, $groupBy) { $grouped = array(); + $path = new PropertyPath($groupBy); foreach ($entities as $entity) { // Get group name from property path try { - $path = new PropertyPath($groupBy); $group = (string) $path->getValue($entity); } catch (UnexpectedTypeException $e) { // PropertyPath cannot traverse entity @@ -286,12 +268,9 @@ public function getEntity($key) return isset($entities[$key]) ? $entities[$key] : null; } else if ($this->entities) { return isset($this->entities[$key]) ? $this->entities[$key] : null; - } else if ($qb = $this->queryBuilder) { - // should we clone the builder? - $alias = $qb->getRootAlias(); - $where = $qb->expr()->eq($alias.'.'.current($this->identifier), $key); - - return $qb->andWhere($where)->getQuery()->getSingleResult(); + } else if ($entityLoader = $this->entityLoader) { + $entities = $entityLoader->getEntitiesByKeys($this->identifier, array($key)); + return (isset($entities[0])) ? $entities[0] : false; } return $this->em->find($this->class, $key); @@ -300,23 +279,6 @@ public function getEntity($key) } } - /** - * Returns the \ReflectionProperty instance for a property of the - * underlying class - * - * @param string $property The name of the property - * @return \ReflectionProperty The reflection instance - */ - private function getReflProperty($property) - { - if (!isset($this->reflProperties[$property])) { - $this->reflProperties[$property] = new \ReflectionProperty($this->class, $property); - $this->reflProperties[$property]->setAccessible(true); - } - - return $this->reflProperties[$property]; - } - /** * Returns the values of the identifier fields of an entity * diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php new file mode 100644 index 0000000000000..5c6454900ee4c --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php @@ -0,0 +1,36 @@ + + * + * 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; + +/** + * Custom loader for entities in the choice list. + * + * @author Benjamin Eberlei + */ +interface EntityLoaderInterface +{ + /** + * Given choice list values this method returns the appropriate entities for it. + * + * @param array $identifier + * @param array $choiceListKeys Array of values of the select option, checkbox or radio button. + * @return object[] + */ + function getEntitiesByKeys(array $identifier, array $choiceListKeys); + + /** + * Return an array of entities that are valid choices in the corresponding choice list. + * + * @return array + */ + function getEntities(); +} diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php new file mode 100644 index 0000000000000..298378242fddd --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php @@ -0,0 +1,86 @@ + + * + * 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\DBAL\Connection; +use Symfony\Component\Form\Exception\FormException; +use Symfony\Component\Form\Exception\UnexpectedTypeException; +use Doctrine\ORM\QueryBuilder; + +/** + * Getting Entities through the ORM QueryBuilder + */ +class ORMQueryBuilderLoader implements EntityLoaderInterface +{ + /** + * Contains the query builder that builds the query for fetching the + * entities + * + * This property should only be accessed through queryBuilder. + * + * @var Doctrine\ORM\QueryBuilder + */ + private $queryBuilder; + + /** + * Construct an ORM Query Builder Loader + * + * @param QueryBuilder $queryBuilder + * @param EntityManager $manager + * @param string $class + */ + public function __construct($queryBuilder, $manager = null, $class = null) + { + // If a query builder was passed, it must be a closure or QueryBuilder + // instance + if (!(null === $queryBuilder || $queryBuilder instanceof QueryBuilder || $queryBuilder instanceof \Closure)) { + throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure'); + } + + if ($queryBuilder instanceof \Closure) { + $queryBuilder = $queryBuilder($manager->getRepository($class)); + + if (!$queryBuilder instanceof QueryBuilder) { + throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder'); + } + } + + $this->queryBuilder = $queryBuilder; + } + + /** + * {@inheritDoc} + */ + public function getEntities() + { + return $this->queryBuilder->getQuery()->execute(); + } + + /** + * {@inheritDoc} + */ + public function getEntitiesByKeys(array $identifier, array $choiceListKeys) + { + if (count($identifier) != 1) { + throw new FormException("Only entities with one identifier supported by ORM QueryBuilder."); + } + + $qb = clone ($this->queryBuilder); + $alias = $qb->getRootAlias(); + $where = $qb->expr()->in($alias.'.'.current($identifier), "?1"); + + return $qb->andWhere($where) + ->getQuery() + ->setParameter(1, $choiceListKeys, Connection::PARAM_STR_ARRAY) + ->getResult(); + } +} \ No newline at end of file diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php index 81f6657f00163..5e4a2491e4282 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php @@ -14,6 +14,7 @@ use Doctrine\Common\Persistence\ManagerRegistry; use Symfony\Component\Form\FormBuilder; use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList; +use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader; use Symfony\Bridge\Doctrine\Form\EventListener\MergeCollectionListener; use Symfony\Bridge\Doctrine\Form\DataTransformer\EntitiesToArrayTransformer; use Symfony\Bridge\Doctrine\Form\DataTransformer\EntityToIdTransformer; @@ -47,6 +48,7 @@ public function getDefaultOptions(array $options) 'class' => null, 'property' => null, 'query_builder' => null, + 'loader' => null, 'choices' => array(), 'group_by' => null, ); @@ -54,11 +56,20 @@ public function getDefaultOptions(array $options) $options = array_replace($defaultOptions, $options); if (!isset($options['choice_list'])) { + $manager = $this->registry->getManager($options['em']); + if (isset($options['query_builder'])) { + $options['loader'] = new ORMQueryBuilderLoader( + $options['query_builder'], + $manager, + $options['class'] + ); + } + $defaultOptions['choice_list'] = new EntityChoiceList( - $this->registry->getManager($options['em']), + $manager, $options['class'], $options['property'], - $options['query_builder'], + $options['loader'], $options['choices'], $options['group_by'] ); From 2c841bc8a73a1fae1ebe6dfe3eae3f5a09728926 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 26 Nov 2011 17:59:47 +0100 Subject: [PATCH 2/6] [DoctrineBridge] Optimize fetching of entities to use WHERE IN and fix other inefficencies. --- .../Form/ChoiceList/EntityChoiceList.php | 55 ++++++++++--------- .../EntitiesToArrayTransformer.php | 18 ++---- .../DataTransformer/EntityToIdTransformer.php | 4 +- .../Doctrine/Fixtures/SingleIdentEntity.php | 5 ++ 4 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php index 9afb0239588e6..b910c45e70b90 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php @@ -97,6 +97,9 @@ public function __construct(ObjectManager $manager, $class, $property = null, En // displaying entities as strings if ($property) { $this->propertyPath = new PropertyPath($property); + } else if (!method_exists($this->class, '__toString')) { + // Otherwise expect a __toString() method in the entity + throw new FormException('Entities passed to the choice field must have a "__toString()" method defined (or you can also override the "property" option).'); } parent::__construct($choices); @@ -186,11 +189,6 @@ private function loadEntities($entities, $group = null) // If the property option was given, use it $value = $this->propertyPath->getValue($entity); } else { - // Otherwise expect a __toString() method in the entity - if (!method_exists($entity, '__toString')) { - throw new FormException('Entities passed to the choice field must have a "__toString()" method defined (or you can also override the "property" option).'); - } - $value = (string)$entity; } @@ -240,7 +238,7 @@ public function getEntities() } /** - * Returns the entity for the given key + * Returns the entities for the given keys * * If the underlying entities have composite identifiers, the choices * are initialized. The key is expected to be the index in the choices @@ -249,34 +247,41 @@ public function getEntities() * If they have single identifiers, they are either fetched from the * internal entity cache (if filled) or loaded from the database. * - * @param string $key The choice key (for entities with composite + * @param array $keys The choice key (for entities with composite * identifiers) or entity ID (for entities with single * identifiers) - * @return object The matching entity + * @return object[] The matching entity */ - public function getEntity($key) + public function getEntitiesByKeys($keys) { if (!$this->loaded) { $this->load(); } - - try { - if (count($this->identifier) > 1) { - // $key is a collection index - $entities = $this->getEntities(); - - return isset($entities[$key]) ? $entities[$key] : null; - } else if ($this->entities) { - return isset($this->entities[$key]) ? $this->entities[$key] : null; - } else if ($entityLoader = $this->entityLoader) { - $entities = $entityLoader->getEntitiesByKeys($this->identifier, array($key)); - return (isset($entities[0])) ? $entities[0] : false; + $found = array(); + + if (count($this->identifier) > 1) { + // $key is a collection index + $entities = $this->getEntities(); + foreach ($keys as $key) { + if (isset($entities[$key])) { + $found[] = $entities[$key]; + } } - - return $this->em->find($this->class, $key); - } catch (NoResultException $e) { - return null; + } else if ($this->entities) { + foreach ($keys as $key) { + if (isset($this->entities[$key])) { + $found[] = $this->entities[$key]; + } + } + } else if ($entityLoader = $this->entityLoader) { + $found = $entityLoader->getEntitiesByKeys($this->identifier, $keys); + } else if ($keys) { + $identifier = current($this->identifier); + $found = $this->em->getRepository($this->class) + ->findBy(array($identifier => $keys)); } + + return $found; } /** diff --git a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php index cca5fde11c55a..57914babf32b0 100644 --- a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php +++ b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php @@ -84,19 +84,13 @@ public function reverseTransform($keys) throw new UnexpectedTypeException($keys, 'array'); } - $notFound = array(); - - // optimize this into a SELECT WHERE IN query - foreach ($keys as $key) { - if ($entity = $this->choiceList->getEntity($key)) { - $collection->add($entity); - } else { - $notFound[] = $key; - } + $entities = $this->choiceList->getEntitiesByKeys($keys); + if (count($keys) != count($entities)) { + throw new TransformationFailedException('Not all entities matching the keys were found.'); } - - if (count($notFound) > 0) { - throw new TransformationFailedException(sprintf('The entities with keys "%s" could not be found', implode('", "', $notFound))); + + foreach ($entities as $entity) { + $collection->add($entity); } return $collection; diff --git a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php index 2193a24e3cd3a..7d2df4cd727e3 100644 --- a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php +++ b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php @@ -74,10 +74,10 @@ public function reverseTransform($key) throw new UnexpectedTypeException($key, 'numeric'); } - if (!($entity = $this->choiceList->getEntity($key))) { + if (!($entities = $this->choiceList->getEntitiesByKeys(array($key)))) { throw new TransformationFailedException(sprintf('The entity with key "%s" could not be found', $key)); } - return $entity; + return $entities[0]; } } diff --git a/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php b/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php index 69c74406e096e..b860b06022e84 100644 --- a/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php +++ b/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php @@ -19,4 +19,9 @@ public function __construct($id, $name) { $this->id = $id; $this->name = $name; } + + public function __toString() + { + return (string)$this->id; + } } From 468e03dd9a6c7f968a21eecb00a9a11321e15f0e Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 26 Nov 2011 18:21:26 +0100 Subject: [PATCH 3/6] [DoctrineBridge] Remove large parts of the EntityChoiceList code that were completly unnecessary (code was unreachable). --- .../Form/ChoiceList/EntityChoiceList.php | 24 +++---------- .../Form/ChoiceList/EntityLoaderInterface.php | 11 +----- .../Form/ChoiceList/ORMQueryBuilderLoader.php | 19 ---------- .../Doctrine/Fixtures/SingleIdentEntity.php | 2 +- .../Doctrine/Form/Type/EntityTypeTest.php | 35 +++++++++++++++++++ 5 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php index b910c45e70b90..80585914bcf70 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php @@ -252,33 +252,17 @@ public function getEntities() * identifiers) * @return object[] The matching entity */ - public function getEntitiesByKeys($keys) + public function getEntitiesByKeys(array $keys) { if (!$this->loaded) { $this->load(); } $found = array(); - if (count($this->identifier) > 1) { - // $key is a collection index - $entities = $this->getEntities(); - foreach ($keys as $key) { - if (isset($entities[$key])) { - $found[] = $entities[$key]; - } + foreach ($keys as $key) { + if (isset($this->entities[$key])) { + $found[] = $this->entities[$key]; } - } else if ($this->entities) { - foreach ($keys as $key) { - if (isset($this->entities[$key])) { - $found[] = $this->entities[$key]; - } - } - } else if ($entityLoader = $this->entityLoader) { - $found = $entityLoader->getEntitiesByKeys($this->identifier, $keys); - } else if ($keys) { - $identifier = current($this->identifier); - $found = $this->em->getRepository($this->class) - ->findBy(array($identifier => $keys)); } return $found; diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php index 5c6454900ee4c..18a231d2c1b76 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php @@ -17,16 +17,7 @@ * @author Benjamin Eberlei */ interface EntityLoaderInterface -{ - /** - * Given choice list values this method returns the appropriate entities for it. - * - * @param array $identifier - * @param array $choiceListKeys Array of values of the select option, checkbox or radio button. - * @return object[] - */ - function getEntitiesByKeys(array $identifier, array $choiceListKeys); - +{ /** * Return an array of entities that are valid choices in the corresponding choice list. * diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php index 298378242fddd..5f901f0189a67 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php @@ -64,23 +64,4 @@ public function getEntities() { return $this->queryBuilder->getQuery()->execute(); } - - /** - * {@inheritDoc} - */ - public function getEntitiesByKeys(array $identifier, array $choiceListKeys) - { - if (count($identifier) != 1) { - throw new FormException("Only entities with one identifier supported by ORM QueryBuilder."); - } - - $qb = clone ($this->queryBuilder); - $alias = $qb->getRootAlias(); - $where = $qb->expr()->in($alias.'.'.current($identifier), "?1"); - - return $qb->andWhere($where) - ->getQuery() - ->setParameter(1, $choiceListKeys, Connection::PARAM_STR_ARRAY) - ->getResult(); - } } \ No newline at end of file diff --git a/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php b/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php index b860b06022e84..2e2c74465fe26 100644 --- a/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php +++ b/tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php @@ -22,6 +22,6 @@ public function __construct($id, $name) { public function __toString() { - return (string)$this->id; + return (string)$this->name; } } diff --git a/tests/Symfony/Tests/Bridge/Doctrine/Form/Type/EntityTypeTest.php b/tests/Symfony/Tests/Bridge/Doctrine/Form/Type/EntityTypeTest.php index 3f6230065df68..b1a21713a5b41 100644 --- a/tests/Symfony/Tests/Bridge/Doctrine/Form/Type/EntityTypeTest.php +++ b/tests/Symfony/Tests/Bridge/Doctrine/Form/Type/EntityTypeTest.php @@ -111,6 +111,41 @@ public function testSetDataToUninitializedEntityWithNonRequired() $this->assertEquals(array(1 => 'Foo', 2 => 'Bar'), $field->createView()->get('choices')); } + + public function testSetDataToUninitializedEntityWithNonRequiredToString() + { + $entity1 = new SingleIdentEntity(1, 'Foo'); + $entity2 = new SingleIdentEntity(2, 'Bar'); + + $this->persist(array($entity1, $entity2)); + + $field = $this->factory->createNamed('entity', 'name', null, array( + 'em' => 'default', + 'class' => self::SINGLE_IDENT_CLASS, + 'required' => false, + )); + + $this->assertEquals(array("1" => 'Foo', "2" => 'Bar'), $field->createView()->get('choices')); + } + + public function testSetDataToUninitializedEntityWithNonRequiredQueryBuilder() + { + $entity1 = new SingleIdentEntity(1, 'Foo'); + $entity2 = new SingleIdentEntity(2, 'Bar'); + + $this->persist(array($entity1, $entity2)); + $qb = $this->em->createQueryBuilder()->select('e')->from(self::SINGLE_IDENT_CLASS, 'e'); + + $field = $this->factory->createNamed('entity', 'name', null, array( + 'em' => 'default', + 'class' => self::SINGLE_IDENT_CLASS, + 'required' => false, + 'property' => 'name', + 'query_builder' => $qb + )); + + $this->assertEquals(array(1 => 'Foo', 2 => 'Bar'), $field->createView()->get('choices')); + } /** * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException From a2fc7d36205ec06c225da44d2f55f78ec483a854 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 12 Dec 2011 22:09:04 +0100 Subject: [PATCH 4/6] Adjust check --- .../Form/DataTransformer/EntitiesToArrayTransformer.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php index 57914babf32b0..c207801273847 100644 --- a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php +++ b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php @@ -85,10 +85,10 @@ public function reverseTransform($keys) } $entities = $this->choiceList->getEntitiesByKeys($keys); - if (count($keys) != count($entities)) { + if (count($keys) !== count($entities)) { throw new TransformationFailedException('Not all entities matching the keys were found.'); } - + foreach ($entities as $entity) { $collection->add($entity); } From d7e1f337b1a66adf465b44ba5746525955f58277 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 12 Dec 2011 22:12:59 +0100 Subject: [PATCH 5/6] [Doctrine] Dont allow null in ORMQueryBuilderLoader --- .../Form/ChoiceList/ORMQueryBuilderLoader.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php index 5f901f0189a67..c772ed9c3217c 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php @@ -30,19 +30,19 @@ class ORMQueryBuilderLoader implements EntityLoaderInterface * @var Doctrine\ORM\QueryBuilder */ private $queryBuilder; - + /** * Construct an ORM Query Builder Loader - * + * * @param QueryBuilder $queryBuilder * @param EntityManager $manager - * @param string $class + * @param string $class */ public function __construct($queryBuilder, $manager = null, $class = null) { // If a query builder was passed, it must be a closure or QueryBuilder // instance - if (!(null === $queryBuilder || $queryBuilder instanceof QueryBuilder || $queryBuilder instanceof \Closure)) { + if (!($queryBuilder instanceof QueryBuilder || $queryBuilder instanceof \Closure)) { throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure'); } @@ -53,10 +53,10 @@ public function __construct($queryBuilder, $manager = null, $class = null) throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder'); } } - + $this->queryBuilder = $queryBuilder; } - + /** * {@inheritDoc} */ @@ -64,4 +64,4 @@ public function getEntities() { return $this->queryBuilder->getQuery()->execute(); } -} \ No newline at end of file +} From c4b127afb789020fdb8aec13932f5212384e0cbb Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 12 Dec 2011 22:55:53 +0100 Subject: [PATCH 6/6] [Doctrine] Cleanup and move loader into its own method --- .../Form/ChoiceList/EntityChoiceList.php | 2 +- .../EntitiesToArrayTransformer.php | 2 +- .../Bridge/Doctrine/Form/Type/EntityType.php | 27 ++++++++++++++----- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php index 90c7291f136c8..f7c9d8874a63e 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php @@ -72,7 +72,7 @@ class EntityChoiceList extends ArrayChoiceList /** * Property path to access the key value of this choice-list. - * + * * @var PropertyPath */ private $propertyPath; diff --git a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php index c207801273847..6f043641523b4 100644 --- a/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php +++ b/src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php @@ -52,7 +52,7 @@ public function transform($collection) foreach ($collection as $entity) { // identify choices by their collection key - $key = array_search($entity, $availableEntities); + $key = array_search($entity, $availableEntities, true); $array[] = $key; } } else { diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php index 5e4a2491e4282..52e8f9330d56c 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php @@ -22,6 +22,9 @@ class EntityType extends AbstractType { + /** + * @var ManagerRegistry + */ protected $registry; public function __construct(ManagerRegistry $registry) @@ -58,13 +61,9 @@ public function getDefaultOptions(array $options) if (!isset($options['choice_list'])) { $manager = $this->registry->getManager($options['em']); if (isset($options['query_builder'])) { - $options['loader'] = new ORMQueryBuilderLoader( - $options['query_builder'], - $manager, - $options['class'] - ); + $options['loader'] = $this->getLoader($manager, $options); } - + $defaultOptions['choice_list'] = new EntityChoiceList( $manager, $options['class'], @@ -78,6 +77,22 @@ public function getDefaultOptions(array $options) return $defaultOptions; } + /** + * Return the default loader object. + * + * @param ObjectManager + * @param array $options + * @return ORMQueryBuilderLoader + */ + protected function getLoader($manager, $options) + { + return new ORMQueryBuilderLoader( + $options['query_builder'], + $manager, + $options['class'] + ); + } + public function getParent(array $options) { return 'choice';