diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch stof/doctrine_choice_list (PR #2921)

Browse Source 
Commits
-------

200ed54 [DoctrineBridge] Extracted the common type and made the choice list generic
3c81b62 [Doctrine] Cleanup and move loader into its own method
7646a5b [Doctrine] Dont allow null in ORMQueryBuilderLoader
988c2a5 Adjust check
3b5c617 [DoctrineBridge] Remove large parts of the EntityChoiceList code that were completly unnecessary (code was unreachable).
b919d92 [DoctrineBridge] Optimize fetching of entities to use WHERE IN and fix other inefficencies.
517eebc [DoctrineBridge] Refactor entity choice list to be ORM independant using an EntityLoader interface.

Discussion
----------

[DoctrineBridge] Refactor EntityChoiceList

Bug fix: no
Feature addition: yes
Backwards compatibility break: no (99%)
Symfony2 tests pass: yes

This decouples the ORM from the EntityChoiceList and makes its much more flexible. Instead of having a "query_builder" to do smart or complex queries you can now create "loader" instances which can arbitrarily help loading entities.

Additionally i removed lots of code that was unnecessary and not used by the current code.

There is a slight BC break in that the EntityChoiceList class is now accepting an EntityLoaderInterface instead of a querybuilder. However that class was nested inside the EntityType and should not be widely used or overwritten.

The abstract class DoctrineType is meant to be used as base class by other Doctrine project to share the logic by simply using a different type name and a different loader implementation.

This PR replaces #2728.

---------------------------------------------------------------------------

by beberlei at 2011/12/19 09:20:43 -0800

Thanks for doing the last refactorings, this is now fine from my side as well.
This commit is contained in:
Fabien Potencier 2011-12-19 19:46:30 +01:00
commit ddf6534cc6
9 changed files with 292 additions and 159 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

132
src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php
View File

@ -15,22 +15,25 @@ use Symfony\Component\Form\Util\PropertyPath;
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\ORM\QueryBuilder;
use Doctrine\ORM\NoResultException;
use Doctrine\Common\Persistence\ObjectManager;
class EntityChoiceList extends ArrayChoiceList
{
/**
* @var Doctrine\ORM\EntityManager
* @var ObjectManager
*/
private $em;
/**
* @var Doctrine\ORM\Mapping\ClassMetadata
* @var string
*/
private $class;
/**
* @var \Doctrine\Common\Persistence\Mapping\ClassMetadata
*/
private $classMetadata;
/**
* The entities from which the user can choose
*
@ -40,7 +43,7 @@ class EntityChoiceList extends ArrayChoiceList
* This property is initialized by initializeChoices(). It should only
* be accessed through getEntity() and getEntities().
*
* @var Collection
* @var array
*/
private $entities = array();
@ -50,9 +53,9 @@ class EntityChoiceList extends ArrayChoiceList
*
* This property should only be accessed through queryBuilder.
*
* @var Doctrine\ORM\QueryBuilder
* @var EntityLoaderInterface
*/
private $queryBuilder;
private $entityLoader;
/**
* The fields of which the identifier of the underlying class consists
@ -64,21 +67,10 @@ class EntityChoiceList extends ArrayChoiceList
private $identifier = array();
/**
* A cache for \ReflectionProperty instances for the underlying class
* Property path to access the key value of this choice-list.
*
* This property should only be accessed through getReflProperty().
*
* @var array
* @var PropertyPath
*/
private $reflProperties = array();
/**
* A cache for the UnitOfWork instance of Doctrine
*
* @var Doctrine\ORM\UnitOfWork
*/
private $unitOfWork;
private $propertyPath;
/**
@ -91,39 +83,29 @@ class EntityChoiceList extends ArrayChoiceList
/**
* Constructor.
*
* @param EntityManager $em An EntityManager instance
* @param ObjectManager $manager An EntityManager instance
* @param string $class The class name
* @param string $property The property name
* @param QueryBuilder|\Closure $queryBuilder An optional query builder
* @param EntityLoaderInterface $entityLoader An optional query builder
* @param array|\Closure $choices An array of choices or a function returning an array
* @param string $groupBy
*/
public function __construct(EntityManager $em, $class, $property = null, $queryBuilder = null, $choices = null, $groupBy = null)
public function __construct(ObjectManager $manager, $class, $property = null, EntityLoaderInterface $entityLoader = null, $choices = null, $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->classMetadata = $manager->getClassMetadata($class);
$this->identifier = $this->classMetadata->getIdentifierFieldNames();
$this->groupBy = $groupBy;
// The property option defines, which property (path) is used for
// displaying entities as strings
if ($property) {
$this->propertyPath = new PropertyPath($property);
} elseif (!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).');
}
if (!is_array($choices) && !$choices instanceof \Closure && !is_null($choices)) {
@ -150,8 +132,8 @@ class EntityChoiceList extends ArrayChoiceList
if (is_array($this->choices)) {
$entities = $this->choices;
} elseif ($qb = $this->queryBuilder) {
$entities = $qb->getQuery()->execute();
} else if ($entityLoader = $this->entityLoader) {
$entities = $entityLoader->getEntities();
} else {
$entities = $this->em->getRepository($this->class)->findAll();
}
@ -171,11 +153,11 @@ class EntityChoiceList extends ArrayChoiceList
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
@ -219,11 +201,6 @@ class EntityChoiceList extends ArrayChoiceList
// 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;
}
@ -278,7 +255,7 @@ class EntityChoiceList extends ArrayChoiceList
}
/**
* 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
@ -287,55 +264,26 @@ class EntityChoiceList extends ArrayChoiceList
* 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
* identifiers) or entity ID (for entities with single
* identifiers)
*
* @return object The matching entity
* @param array $keys The choice key (for entities with composite
* identifiers) or entity ID (for entities with single
* identifiers)
* @return object[] The matching entity
*/
public function getEntity($key)
public function getEntitiesByKeys(array $keys)
{
if (!$this->loaded) {
$this->load();
}
try {
if (count($this->identifier) > 1) {
// $key is a collection index
$entities = $this->getEntities();
$found = array();
return isset($entities[$key]) ? $entities[$key] : null;
} elseif ($this->entities) {
return isset($this->entities[$key]) ? $this->entities[$key] : null;
} elseif ($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();
foreach ($keys as $key) {
if (isset($this->entities[$key])) {
$found[] = $this->entities[$key];
}
return $this->em->find($this->class, $key);
} catch (NoResultException $e) {
return null;
}
}
/**
* 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];
return $found;
}
/**
@ -353,10 +301,10 @@ class EntityChoiceList extends ArrayChoiceList
*/
public function getIdentifierValues($entity)
{
if (!$this->unitOfWork->isInIdentityMap($entity)) {
if (!$this->em->contains($entity)) {
throw new FormException('Entities passed to the choice field must be managed');
}
return $this->unitOfWork->getEntityIdentifier($entity);
return $this->classMetadata->getIdentifierValues($entity);
}
}

27
src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php Normal file
View File

@ -0,0 +1,27 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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 <kontakt@beberlei.de>
*/
interface EntityLoaderInterface
{
/**
* Return an array of entities that are valid choices in the corresponding choice list.
*
* @return array
*/
function getEntities();
}

67
src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php Normal file
View File

@ -0,0 +1,67 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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 (!($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();
}
}

18
src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php
View File

@ -52,7 +52,7 @@ class EntitiesToArrayTransformer implements DataTransformerInterface
foreach ($collection as $entity) {
// identify choices by their collection key
$key = array_search($entity, $availableEntities);
$key = array_search($entity, $availableEntities, true);
$array[] = $key;
}
} else {
@ -84,19 +84,13 @@ class EntitiesToArrayTransformer implements DataTransformerInterface
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;

4
src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php
View File

@ -74,10 +74,10 @@ class EntityToIdTransformer implements DataTransformerInterface
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];
}
}

94
src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php Normal file
View File

@ -0,0 +1,94 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Bridge\Doctrine\Form\Type;
use Doctrine\Common\Persistence\ManagerRegistry;
use Doctrine\Common\Persistence\ObjectManager;
use Symfony\Component\Form\FormBuilder;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
use Symfony\Bridge\Doctrine\Form\EventListener\MergeCollectionListener;
use Symfony\Bridge\Doctrine\Form\DataTransformer\EntitiesToArrayTransformer;
use Symfony\Bridge\Doctrine\Form\DataTransformer\EntityToIdTransformer;
use Symfony\Component\Form\AbstractType;
abstract class DoctrineType extends AbstractType
{
/**
* @var ManagerRegistry
*/
protected $registry;
public function __construct(ManagerRegistry $registry)
{
$this->registry = $registry;
}
public function buildForm(FormBuilder $builder, array $options)
{
if ($options['multiple']) {
$builder
->addEventSubscriber(new MergeCollectionListener())
->prependClientTransformer(new EntitiesToArrayTransformer($options['choice_list']))
;
} else {
$builder->prependClientTransformer(new EntityToIdTransformer($options['choice_list']));
}
}
public function getDefaultOptions(array $options)
{
$defaultOptions = array(
'em' => null,
'class' => null,
'property' => null,
'query_builder' => null,
'loader' => null,
'choices' => null,
'group_by' => null,
);
$options = array_replace($defaultOptions, $options);
if (!isset($options['choice_list'])) {
$manager = $this->registry->getManager($options['em']);
if (isset($options['query_builder'])) {
$options['loader'] = $this->getLoader($manager, $options);
}
$defaultOptions['choice_list'] = new EntityChoiceList(
$manager,
$options['class'],
$options['property'],
$options['loader'],
$options['choices'],
$options['group_by']
);
}
return $defaultOptions;
}
/**
* Return the default loader object.
*
* @param ObjectManager $manager
* @param array $options
* @return EntityLoaderInterface
*/
abstract protected function getLoader(ObjectManager $manager, array $options);
public function getParent(array $options)
{
return 'choice';
}
}

69
src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php
View File

@ -11,65 +11,28 @@
namespace Symfony\Bridge\Doctrine\Form\Type;
use Doctrine\Common\Persistence\ManagerRegistry;
use Doctrine\Common\Persistence\ObjectManager;
use Symfony\Component\Form\FormBuilder;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;
use Symfony\Bridge\Doctrine\Form\EventListener\MergeCollectionListener;
use Symfony\Bridge\Doctrine\Form\DataTransformer\EntitiesToArrayTransformer;
use Symfony\Bridge\Doctrine\Form\DataTransformer\EntityToIdTransformer;
use Symfony\Component\Form\AbstractType;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
class EntityType extends AbstractType
class EntityType extends DoctrineType
{
protected $registry;
public function __construct(ManagerRegistry $registry)
/**
* Return the default loader object.
*
* @param ObjectManager $manager
* @param array $options
* @return ORMQueryBuilderLoader
*/
protected function getLoader(ObjectManager $manager, array $options)
{
$this->registry = $registry;
}
public function buildForm(FormBuilder $builder, array $options)
{
if ($options['multiple']) {
$builder
->addEventSubscriber(new MergeCollectionListener())
->prependClientTransformer(new EntitiesToArrayTransformer($options['choice_list']))
;
} else {
$builder->prependClientTransformer(new EntityToIdTransformer($options['choice_list']));
}
}
public function getDefaultOptions(array $options)
{
$defaultOptions = array(
'em' => null,
'class' => null,
'property' => null,
'query_builder' => null,
'choices' => null,
'group_by' => null,
return new ORMQueryBuilderLoader(
$options['query_builder'],
$manager,
$options['class']
);
$options = array_replace($defaultOptions, $options);
if (!isset($options['choice_list'])) {
$defaultOptions['choice_list'] = new EntityChoiceList(
$this->registry->getManager($options['em']),
$options['class'],
$options['property'],
$options['query_builder'],
$options['choices'],
$options['group_by']
);
}
return $defaultOptions;
}
public function getParent(array $options)
{
return 'choice';
}
public function getName()

5
tests/Symfony/Tests/Bridge/Doctrine/Fixtures/SingleIdentEntity.php
View File

@ -19,4 +19,9 @@ class SingleIdentEntity
$this->id = $id;
$this->name = $name;
}
public function __toString()
{
return (string)$this->name;
}
}

35
tests/Symfony/Tests/Bridge/Doctrine/Form/Type/EntityTypeTest.php
View File

@ -111,6 +111,41 @@ class EntityTypeTest extends TypeTestCase
$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