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

[DoctrineBridge] Fixed: don't cache choice lists if query builders are constructed dynamically

Browse Source
This commit is contained in:
Bernhard Schussek 2015-03-25 11:41:36 +01:00
parent 03efce1b56
commit 3846b3750a
4 changed files with 97 additions and 134 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceLoader.php → src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php
View File

@ -23,7 +23,7 @@ use Symfony\Component\Form\Exception\RuntimeException;
* *
* @author Bernhard Schussek <bschussek@gmail.com> * @author Bernhard Schussek <bschussek@gmail.com>
*/ */
class EntityChoiceLoader implements ChoiceLoaderInterface class DoctrineChoiceLoader implements ChoiceLoaderInterface
{ {
/** /**
* @var ChoiceListFactoryInterface * @var ChoiceListFactoryInterface
@ -48,7 +48,7 @@ class EntityChoiceLoader implements ChoiceLoaderInterface
/** /**
* @var null|EntityLoaderInterface * @var null|EntityLoaderInterface
*/ */
private $entityLoader; private $objectLoader;
/** /**
* The identifier field, unless the identifier is composite * The identifier field, unless the identifier is composite
@ -70,20 +70,20 @@ class EntityChoiceLoader implements ChoiceLoaderInterface
private $choiceList; private $choiceList;
/** /**
* Returns the value of the identifier field of an entity. * Returns the value of the identifier field of an object.
* *
* Doctrine must know about this entity, that is, the entity must already * Doctrine must know about this object, that is, the object must already
* be persisted or added to the identity map before. Otherwise an * be persisted or added to the identity map before. Otherwise an
* exception is thrown. * exception is thrown.
* *
* This method assumes that the entity has a single-column identifier and * This method assumes that the object has a single-column identifier and
* will return a single value instead of an array. * will return a single value instead of an array.
* *
* @param object $object The entity for which to get the identifier * @param object $object The object for which to get the identifier
* *
* @return int|string The identifier value * @return int|string The identifier value
* *
* @throws RuntimeException If the entity does not exist in Doctrine's identity map * @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 * @internal Should not be accessed by user-land code. This method is public
* only to be usable as callback. * only to be usable as callback.
@ -106,22 +106,23 @@ class EntityChoiceLoader implements ChoiceLoaderInterface
* Creates a new choice loader. * Creates a new choice loader.
* *
* Optionally, an implementation of {@link EntityLoaderInterface} can be * Optionally, an implementation of {@link EntityLoaderInterface} can be
* passed which optimizes the entity loading for one of the Doctrine * passed which optimizes the object loading for one of the Doctrine
* mapper implementations. * mapper implementations.
* *
* @param ChoiceListFactoryInterface $factory The factory for creating * @param ChoiceListFactoryInterface $factory The factory for creating
* the loaded choice list * the loaded choice list
* @param ObjectManager $manager The object manager * @param ObjectManager $manager The object manager
* @param string $class The entity class name * @param string $class The class name of the
* @param null|EntityLoaderInterface $entityLoader The entity loader * loaded objects
* @param null|EntityLoaderInterface $objectLoader The objects loader
*/ */
public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, EntityLoaderInterface $entityLoader = null) public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, EntityLoaderInterface $objectLoader = null)
{ {
$this->factory = $factory; $this->factory = $factory;
$this->manager = $manager; $this->manager = $manager;
$this->classMetadata = $manager->getClassMetadata($class); $this->classMetadata = $manager->getClassMetadata($class);
$this->class = $this->classMetadata->getName(); $this->class = $this->classMetadata->getName();
$this->entityLoader = $entityLoader; $this->objectLoader = $objectLoader;
$identifier = $this->classMetadata->getIdentifierFieldNames(); $identifier = $this->classMetadata->getIdentifierFieldNames();
@ -140,51 +141,51 @@ class EntityChoiceLoader implements ChoiceLoaderInterface
return $this->choiceList; return $this->choiceList;
} }
$entities = $this->entityLoader $objects = $this->objectLoader
? $this->entityLoader->getEntities() ? $this->objectLoader->getEntities()
: $this->manager->getRepository($this->class)->findAll(); : $this->manager->getRepository($this->class)->findAll();
// If the class has a multi-column identifier, we cannot index the // If the class has a multi-column identifier, we cannot index the
// entities by their IDs // objects by their IDs
if ($this->compositeId) { if ($this->compositeId) {
$this->choiceList = $this->factory->createListFromChoices($entities, $value); $this->choiceList = $this->factory->createListFromChoices($objects, $value);
return $this->choiceList; return $this->choiceList;
} }
// Index the entities by ID // Index the objects by ID
$entitiesById = array(); $objectsById = array();
foreach ($entities as $entity) { foreach ($objects as $object) {
$id = self::getIdValue($this->manager, $this->classMetadata, $entity); $id = self::getIdValue($this->manager, $this->classMetadata, $object);
$entitiesById[$id] = $entity; $objectsById[$id] = $object;
} }
$this->choiceList = $this->factory->createListFromChoices($entitiesById, $value); $this->choiceList = $this->factory->createListFromChoices($objectsById, $value);
return $this->choiceList; return $this->choiceList;
} }
/** /**
* Loads the values corresponding to the given entities. * Loads the values corresponding to the given objects.
* *
* The values are returned with the same keys and in the same order as the * The values are returned with the same keys and in the same order as the
* corresponding entities in the given array. * corresponding objects in the given array.
* *
* Optionally, a callable can be passed for generating the choice values. * Optionally, a callable can be passed for generating the choice values.
* The callable receives the entity as first and the array key as the second * The callable receives the object as first and the array key as the second
* argument. * argument.
* *
* @param array $entities An array of entities. Non-existing entities * @param array $objects An array of objects. Non-existing objects in
* in this array are ignored * this array are ignored
* @param null|callable $value The callable generating the choice values * @param null|callable $value The callable generating the choice values
* *
* @return string[] An array of choice values * @return string[] An array of choice values
*/ */
public function loadValuesForChoices(array $entities, $value = null) public function loadValuesForChoices(array $objects, $value = null)
{ {
// Performance optimization // Performance optimization
if (empty($entities)) { if (empty($objects)) {
return array(); return array();
} }
@ -195,41 +196,41 @@ class EntityChoiceLoader implements ChoiceLoaderInterface
if (!$this->choiceList && !$this->compositeId) { if (!$this->choiceList && !$this->compositeId) {
$values = array(); $values = array();
// Maintain order and indices of the given entities // Maintain order and indices of the given objects
foreach ($entities as $i => $entity) { foreach ($objects as $i => $object) {
if ($entity instanceof $this->class) { if ($object instanceof $this->class) {
// Make sure to convert to the right format // Make sure to convert to the right format
$values[$i] = (string) self::getIdValue($this->manager, $this->classMetadata, $entity); $values[$i] = (string) self::getIdValue($this->manager, $this->classMetadata, $object);
} }
} }
return $values; return $values;
} }
return $this->loadChoiceList($value)->getValuesForChoices($entities); return $this->loadChoiceList($value)->getValuesForChoices($objects);
} }
/** /**
* Loads the entities corresponding to the given values. * Loads the objects corresponding to the given values.
* *
* The entities are returned with the same keys and in the same order as the * The objects are returned with the same keys and in the same order as the
* corresponding values in the given array. * corresponding values in the given array.
* *
* Optionally, a callable can be passed for generating the choice values. * Optionally, a callable can be passed for generating the choice values.
* The callable receives the entity as first and the array key as the second * The callable receives the object as first and the array key as the second
* argument. * argument.
* *
* @param string[] $values An array of choice values. Non-existing * @param string[] $values An array of choice values. Non-existing
* values in this array are ignored * values in this array are ignored
* @param null|callable $value The callable generating the choice values * @param null|callable $value The callable generating the choice values
* *
* @return array An array of entities * @return array An array of objects
*/ */
public function loadChoicesForValues(array $values, $value = null) public function loadChoicesForValues(array $values, $value = null)
{ {
// Performance optimization // Performance optimization
// Also prevents the generation of "WHERE id IN ()" queries through the // Also prevents the generation of "WHERE id IN ()" queries through the
// entity loader. At least with MySQL and on the development machine // object loader. At least with MySQL and on the development machine
// this was tested on, no exception was thrown for such invalid // this was tested on, no exception was thrown for such invalid
// statements, consequently no test fails when this code is removed. // statements, consequently no test fails when this code is removed.
// https://github.com/symfony/symfony/pull/8981#issuecomment-24230557 // https://github.com/symfony/symfony/pull/8981#issuecomment-24230557
@ -237,29 +238,29 @@ class EntityChoiceLoader implements ChoiceLoaderInterface
return array(); return array();
} }
// Optimize performance in case we have an entity loader and // Optimize performance in case we have an object loader and
// a single-field identifier // a single-field identifier
if (!$this->choiceList && !$this->compositeId && $this->entityLoader) { if (!$this->choiceList && !$this->compositeId && $this->objectLoader) {
$unorderedEntities = $this->entityLoader->getEntitiesByIds($this->idField, $values); $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idField, $values);
$entitiesById = array(); $objectsById = array();
$entities = array(); $objects = array();
// Maintain order and indices from the given $values // Maintain order and indices from the given $values
// An alternative approach to the following loop is to add the // An alternative approach to the following loop is to add the
// "INDEX BY" clause to the Doctrine query in the loader, // "INDEX BY" clause to the Doctrine query in the loader,
// but I'm not sure whether that's doable in a generic fashion. // but I'm not sure whether that's doable in a generic fashion.
foreach ($unorderedEntities as $entity) { foreach ($unorderedObjects as $object) {
$id = self::getIdValue($this->manager, $this->classMetadata, $entity); $id = self::getIdValue($this->manager, $this->classMetadata, $object);
$entitiesById[$id] = $entity; $objectsById[$id] = $object;
} }
foreach ($values as $i => $id) { foreach ($values as $i => $id) {
if (isset($entitiesById[$id])) { if (isset($objectsById[$id])) {
$entities[$i] = $entitiesById[$id]; $objects[$i] = $objectsById[$id];
} }
} }
return $entities; return $objects;
} }
return $this->loadChoiceList($value)->getChoicesForValues($values); return $this->loadChoiceList($value)->getChoicesForValues($values);

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

@ -24,7 +24,7 @@ use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
* @author Bernhard Schussek <bschussek@gmail.com> * @author Bernhard Schussek <bschussek@gmail.com>
* *
* @deprecated Deprecated since Symfony 2.7, to be removed in Symfony 3.0. * @deprecated Deprecated since Symfony 2.7, to be removed in Symfony 3.0.
* Use {@link EntityChoiceLoader} instead. * Use {@link DoctrineChoiceLoader} instead.
*/ */
class EntityChoiceList extends ObjectChoiceList class EntityChoiceList extends ObjectChoiceList
{ {

63
src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
View File

@ -13,7 +13,7 @@ namespace Symfony\Bridge\Doctrine\Form\Type;
use Doctrine\Common\Persistence\ManagerRegistry; use Doctrine\Common\Persistence\ManagerRegistry;
use Doctrine\Common\Persistence\ObjectManager; use Doctrine\Common\Persistence\ObjectManager;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceLoader; use Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface; use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer; use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener; use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener;
@ -41,7 +41,7 @@ abstract class DoctrineType extends AbstractType
private $choiceListFactory; private $choiceListFactory;
/** /**
* @var EntityChoiceLoader[] * @var DoctrineChoiceLoader[]
*/ */
private $choiceLoaders = array(); private $choiceLoaders = array();
@ -71,32 +71,43 @@ abstract class DoctrineType extends AbstractType
$choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) { $choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) {
// Unless the choices are given explicitly, load them on demand // Unless the choices are given explicitly, load them on demand
if (null === $options['choices']) { if (null === $options['choices']) {
$hash = CachingFactoryDecorator::generateHash(array( // Don't cache if the query builder is constructed dynamically
$options['em'], if ($options['query_builder'] instanceof \Closure) {
$options['class'], $hash = null;
$options['query_builder'], } else {
$options['loader'], $hash = CachingFactoryDecorator::generateHash(array(
));
if (!isset($choiceLoaders[$hash])) {
if ($options['loader']) {
$loader = $options['loader'];
} elseif (null !== $options['query_builder']) {
$loader = $type->getLoader($options['em'], $options['query_builder'], $options['class']);
} else {
$queryBuilder = $options['em']->getRepository($options['class'])->createQueryBuilder('e');
$loader = $type->getLoader($options['em'], $queryBuilder, $options['class']);
}
$choiceLoaders[$hash] = new EntityChoiceLoader(
$choiceListFactory,
$options['em'], $options['em'],
$options['class'], $options['class'],
$loader $options['query_builder'],
); $options['loader'],
));
if (isset($choiceLoaders[$hash])) {
return $choiceLoaders[$hash];
}
} }
return $choiceLoaders[$hash]; if ($options['loader']) {
$entityLoader = $options['loader'];
} elseif (null !== $options['query_builder']) {
$entityLoader = $type->getLoader($options['em'], $options['query_builder'], $options['class']);
} else {
$queryBuilder = $options['em']->getRepository($options['class'])->createQueryBuilder('e');
$entityLoader = $type->getLoader($options['em'], $queryBuilder, $options['class']);
}
$choiceLoader = new DoctrineChoiceLoader(
$choiceListFactory,
$options['em'],
$options['class'],
$entityLoader
);
if (null !== $hash) {
$choiceLoaders[$hash] = $choiceLoader;
}
return $choiceLoader;
} }
}; };
@ -131,7 +142,7 @@ abstract class DoctrineType extends AbstractType
}; };
// The choices are always indexed by ID (see "choices" normalizer // The choices are always indexed by ID (see "choices" normalizer
// and EntityChoiceLoader), unless the ID is composite. Then they // and DoctrineChoiceLoader), unless the ID is composite. Then they
// are indexed by an incrementing integer. // are indexed by an incrementing integer.
// Use the ID/incrementing integer as choice value. // Use the ID/incrementing integer as choice value.
$choiceValue = function ($entity, $key) { $choiceValue = function ($entity, $key) {
@ -181,7 +192,7 @@ abstract class DoctrineType extends AbstractType
$entitiesById = array(); $entitiesById = array();
foreach ($entities as $entity) { foreach ($entities as $entity) {
$id = EntityChoiceLoader::getIdValue($om, $classMetadata, $entity); $id = DoctrineChoiceLoader::getIdValue($om, $classMetadata, $entity);
$entitiesById[$id] = $entity; $entitiesById[$id] = $entity;
} }

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

@ -17,71 +17,22 @@ use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
class EntityType extends DoctrineType class EntityType extends DoctrineType
{ {
/**
* @var ORMQueryBuilderLoader[]
*/
private $loaderCache = array();
/** /**
* Return the default loader object. * Return the default loader object.
* *
* @param ObjectManager $manager * @param ObjectManager $manager
* @param mixed $queryBuilder * @param QueryBuilder|\Closure $queryBuilder
* @param string $class * @param string $class
* *
* @return ORMQueryBuilderLoader * @return ORMQueryBuilderLoader
*/ */
public function getLoader(ObjectManager $manager, $queryBuilder, $class) public function getLoader(ObjectManager $manager, $queryBuilder, $class)
{ {
if (!$queryBuilder instanceof QueryBuilder) { return new ORMQueryBuilderLoader(
return new ORMQueryBuilderLoader( $queryBuilder,
$queryBuilder, $manager,
$manager, $class
$class );
);
}
$queryBuilderHash = $this->getQueryBuilderHash($queryBuilder);
$loaderHash = $this->getLoaderHash($manager, $queryBuilderHash, $class);
if (!isset($this->loaderCache[$loaderHash])) {
$this->loaderCache[$loaderHash] = new ORMQueryBuilderLoader(
$queryBuilder,
$manager,
$class
);
}
return $this->loaderCache[$loaderHash];
}
/**
* @param QueryBuilder $queryBuilder
*
* @return string
*/
private function getQueryBuilderHash(QueryBuilder $queryBuilder)
{
return hash('sha256', json_encode(array(
'sql' => $queryBuilder->getQuery()->getSQL(),
'parameters' => $queryBuilder->getParameters(),
)));
}
/**
* @param ObjectManager $manager
* @param string $queryBuilderHash
* @param string $class
*
* @return string
*/
private function getLoaderHash(ObjectManager $manager, $queryBuilderHash, $class)
{
return hash('sha256', json_encode(array(
'manager' => spl_object_hash($manager),
'queryBuilder' => $queryBuilderHash,
'class' => $class,
)));
} }
public function getName() public function getName()