8000 merged branch stof/doctrine_choice_list (PR #2921) · symfony/symfony@ddf6534 · GitHub
[go: up one dir, main page]
Skip to content

Commit ddf6534

Browse files
fabpotfabpot
committed
merged branch stof/doctrine_choice_list (PR #2921)
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.
2 parents 694c74d + 200ed54 commit ddf6534

File tree

9 files changed

+293
-160
lines changed

9 files changed

+293
-160
lines changed

‎src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php

Lines changed: 41 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,25 @@
1515
use Symfony\Component\Form\Exception\FormException;
1616
use Symfony\Component\Form\Exception\UnexpectedTypeException;
1717
use Symfony\Component\Form\Extension\Core\ChoiceList\ArrayChoiceList;
18-
use Doctrine\ORM\EntityManager;
19-
use Doctrine\ORM\QueryBuilder;
20-
use Doctrine\ORM\NoResultException;
18+
use Doctrine\Common\Persistence\ObjectManager;
2119

2220
class EntityChoiceList extends ArrayChoiceList
2321
{
2422
/**
25-
* @var Doctrine\ORM\EntityManager
23+
* @var ObjectManager
2624
*/
2725
private $em;
2826

2927
/**
30-
* @var Doctrine\ORM\Mapping\ClassMetadata
28+
* @var string
3129
*/
3230
private $class;
3331

32+
/**
33+
* @var \Doctrine\Common\Persistence\Mapping\ClassMetadata
34+
*/
35+
private $classMetadata;
36+
3437
/**
3538
* The entities from which the user can choose
3639
*
@@ -40,7 +43,7 @@ class EntityChoiceList extends ArrayChoiceList
4043
* This property is initialized by initializeChoices(). It should only
4144
* be accessed through getEntity() and getEntities().
4245
*
43-
* @var Collection
46+
* @var array
4447
*/
4548
private $entities = array();
4649

@@ -50,9 +53,9 @@ class EntityChoiceList extends ArrayChoiceList
5053
*
5154
* This property should only be accessed through queryBuilder.
5255
*
53-
* @var Doctrine\ORM\QueryBuilder
56+
* @var EntityLoaderInterface
5457
*/
55-
private $queryBuilder;
58+
private $entityLoader;
5659

5760
/**
5861
* The fields of which the identifier of the underlying class consists
@@ -64,21 +67,10 @@ class EntityChoiceList extends ArrayChoiceList
6467
private $identifier = array();
6568

6669
/**
67-
* A cache for \ReflectionProperty instances for the underlying class
70+
* Property path to access the key value of this choice-list.
6871
*
69-
* This property should only be accessed through getReflProperty().
70-
*
71-
* @var array
72+
* @var PropertyPath
7273
*/
73-
private $reflProperties = array();
74-
75-
/**
76-
* A cache for the UnitOfWork instance of Doctrine
77-
*
78-
* @var Doctrine\ORM\UnitOfWork
79-
*/
80-
private $unitOfWork;
81-
8274
private $propertyPath;
8375

8476
/**
@@ -91,39 +83,29 @@ class EntityChoiceList extends ArrayChoiceList
9183
/**
9284
* Constructor.
9385
*
94-
* @param EntityManager $em An EntityManager instance
86+
* @param ObjectManager $manager An EntityManager instance
9587
* @param string $class The class name
9688
* @param string $property The property name
97-
* @param QueryBuilder|\Closure $queryBuilder An optional query builder
89+
* @param EntityLoaderInterface $entityLoader An optional query builder
9890
* @param array|\Closure $choices An array of choices or a function returning an array
91+
* @param string $groupBy
9992
*/
100-
public function __construct(EntityManager $em, $class, $property = null, $queryBuilder = null, $choices = null, $groupBy = null)
93+
public function __construct(ObjectManager $manager, $class, $property = null, EntityLoaderInterface $entityLoader = null, $choices = null, $groupBy = null)
10194
{
102-
// If a query builder was passed, it must be a closure or QueryBuilder
103-
// instance
104-
if (!(null === $queryBuilder || $queryBuilder instanceof QueryBuilder || $queryBuilder instanceof \Closure)) {
105-
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure');
106-
}
107-
108-
if ($queryBuilder instanceof \Closure) {
109-
$queryBuilder = $queryBuilder($em->getRepository($class));
110-
111-
if (!$queryBuilder instanceof QueryBuilder) {
112-
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder');
113-
}
114-
}
115-
116-
$this->em = $em;
95+
$this->em = $manager;
11796
$this->class = $class;
118-
$this->queryBuilder = $queryBuilder;
119-
$this->unitOfWork = $em->getUnitOfWork();
120-
$this->identifier = $em->getClassMetadata($class)->getIdentifierFieldNames();
97+
$this->entityLoader = $entityLoader;
98+
$this->classMetadata = $manager->getClassMetadata($class);
99+
$this->identifier = $this->classMetadata->getIdentifierFieldNames();
121100
$this->groupBy = $groupBy;
122101

123102
// The property option defines, which property (path) is used for
124103
// displaying entities as strings
125104
if ($property) {
126105
$this->propertyPath = new PropertyPath($property);
106+
} elseif (!method_exists($this->class, '__toString')) {
107+
// Otherwise expect a __toString() method in the entity
108+
throw new FormException('Entities passed to the choice field must have a "__toString()" method defined (or you can also override the "property" option).');
127109
}
128110

129111
if (!is_array($choices) && !$choices instanceof \Closure && !is_null($choices)) {
@@ -150,8 +132,8 @@ protected function load()
150132

151133
if (is_array($this->choices)) {
152134
$entities = $this->choices;
153-
} elseif ($qb = $this->queryBuilder) {
154-
$entities = $qb->getQuery()->execute();
135+
} else if ($entityLoader = $this->entityLoader) {
136+
$entities = $entityLoader->getEntities();
155137
} else {
156138
$entities = $this->em->getRepository($this->class)->findAll();
157139
}
@@ -171,11 +153,11 @@ protected function load()
171153
private function groupEntities($entities, $groupBy)
172154
{
173155
$grouped = array();
156+
$path = new PropertyPath($groupBy);
174157

175158
foreach ($entities as $entity) {
176159
// Get group name from property path
177160
try {
178-
$path = new PropertyPath($groupBy);
179161
$group = (string) $path->getValue($entity);
180162
} catch (UnexpectedTypeException $e) {
181163
// PropertyPath cannot traverse entity
@@ -219,11 +201,6 @@ private function loadEntities($entities, $group = null)
219201
// If the property option was given, use it
220202
$value = $this->propertyPath->getValue($entity);
221203
} else {
222-
// Otherwise expect a __toString() method in the entity
223-
if (!method_exists($entity, '__toString')) {
224-
throw new FormException('Entities passed to the choice field must have a "__toString()" method defined (or you can also override the "property" option).');
225-
}
226-
227204
$value = (string) $entity;
228205
}
229206

@@ -278,7 +255,7 @@ public function getEntities()
278255
}
279256

280257
/**
281-
* Returns the entity for the given key.
258+
* Returns the entities for the given keys.
282259
*
283260
* If the underlying entities have composite identifiers, the choices
284261
* are initialized. The key is expected to be the index in the choices
@@ -287,55 +264,26 @@ public function getEntities()
287264
* If they have single identifiers, they are either fetched from the
288265
* internal entity cache (if filled) or loaded from the database.
289266
*
290-
* @param string $key The choice key (for entities with composite
291-
* identifiers) or entity ID (for entities with single
292-
* identifiers)
293-
*
294-
* @return object The matching entity
267+
* @param array $keys The choice key (for entities with composite
268+
* identifiers) or entity ID (for entities with single
269+
* identifiers)
270+
* @return object[] The matching entity
295271
*/
296-
public function getEntity($key)
272+
public function getEntitiesByKeys(array $keys)
297273
{
298274
if (!$this->loaded) {
299275
$this->load();
300276
}
301277

302-
try {
303-
if (count($this->identifier) > 1) {
304-
// $key is a collection index
305-
$entities = $this->getEntities();
306-
307-
return isset($entities[$key]) ? $entities[$key] : null;
308-
} elseif ($this->entities) {
309-
return isset($this->entities[$key]) ? $this->entities[$key] : null;
310-
} elseif ($qb = $this->queryBuilder) {
311-
// should we clone the builder?
312-
$alias = $qb->getRootAlias();
313-
$where = $qb->expr()->eq($alias.'.'.current($this->identifier), $key);
314-
315-
return $qb->andWhere($where)->getQuery()->getSingleResult();
316-
}
278+
$found = array();
317279

318-
return $this->em->find($this->class, $key);
319-
} catch (NoResultException $e) {
320-
return null;
321-
}
322-
}
323-
324-
/**
325-
* Returns the \ReflectionProperty instance for a property of the underlying class.
326-
*
327-
* @param string $property The name of the property
328-
*
329-
* @return \ReflectionProperty The reflection instance
330-
*/
331-
private function getReflProperty($property)
332-
{
333-
if (!isset($this->reflProperties[$property])) {
334-
$this->reflProperties[$property] = new \ReflectionProperty($this->class, $property);
335-
$this->reflProperties[$property]->setAccessible(true);
280+
foreach ($keys as $key) {
281+
if (isset($this->entities[$key])) {
282+
$found[] = $this->entities[$key];
283+
}
336284
}
337285

338-
return $this->reflProperties[$property];
286+
return $found;
339287
}
340288

341289
/**
@@ -353,10 +301,10 @@ private function getReflProperty($property)
353301
*/
354302
public function getIdentifierValues($entity)
355303
{
356-
if (!$this->unitOfWork->isInIdentityMap($entity)) {
304+
if (!$this->em->contains($entity)) {
357305
throw new FormException('Entities passed to the choice field must be managed');
358306
}
359307

360-
return $this->unitOfWork->getEntityIdentifier($entity);
308+
return $this->classMetadata->getIdentifierValues($entity);
361309
}
362310
}

‎src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityLoaderInterface.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bridge\Doctrine\Form\ChoiceList;
13+
14+
/**
15+
* Custom loader for entities in the choice list.
16+
*
17+
* @author Benjamin Eberlei <kontakt@beberlei.de>
18+
*/
19+
interface EntityLoaderInterface
20+
{
21+
/**
22+
* Return an array of entities that are valid choices in the corresponding choice list.
23+
*
24+
* @return array
25+
*/
26+
function getEntities();
27+
}

‎src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bridge\Doctrine\Form\ChoiceList;
13+
14+
use Doctrine\DBAL\Connection;
15+
use Symfony\Component\Form\Exception\FormException;
16+
use Symfony\Component\Form\Exception\UnexpectedTypeException;
17+
use Doctrine\ORM\QueryBuilder;
18+
19+
/**
20+
* Getting Entities through the ORM QueryBuilder
21+
*/
22+
class ORMQueryBuilderLoader implements EntityLoaderInterface
23+
{
24+
/**
25+
* Contains the query builder that builds the query for fetching the
26+
* entities
27+
*
28+
* This property should only be accessed through queryBuilder.
29+
*
30+
* @var Doctrine\ORM\QueryBuilder
31+
*/
32+
private $queryBuilder;
33+
34+
/**
35+
* Construct an ORM Query Builder Loader
36+
*
37+
* @param QueryBuilder $queryBuilder
38+
* @param EntityManager $manager
39+
* @param string $class
40+
*/
41+
public function __construct($queryBuilder, $manager = null, $class = null)
42+
{
43+
// If a query builder was passed, it must be a closure or QueryBuilder
44+
// instance
45+
if (!($queryBuilder instanceof QueryBuilder || $queryBuilder instanceof \Closure)) {
46+
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure');
47+
}
48+
49+
if ($queryBuilder instanceof \Closure) {
50+
$queryBuilder = $queryBuilder($manager->getRepository($class));
51+
52+
if (!$queryBuilder instanceof QueryBuilder) {
53+
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder');
54+
}
55+
}
56+
57+
$this->queryBuilder = $queryBuilder;
58+
}
59+
60+
/**
61+
* {@inheritDoc}
62+
*/
63+
public function getEntities()
64+
{
65+
return $this->queryBuilder->getQuery()->execute();
66+
}
67+
}

‎src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntitiesToArrayTransformer.php

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function transform($collection)
5252

5353
foreach ($collection as $entity) {
5454
// identify choices by their collection key
55-
$key = array_search($entity, $availableEntities);
55+
$key = array_search($entity, $availableEntities, true);
5656
$array[] = $key;
5757
}
5858
} else {
@@ -84,19 +84,13 @@ public function reverseTransform($keys)
8484
throw new UnexpectedTypeException($keys, 'array');
8585
}
8686

87-
$notFound = array();
88-
89-
// optimize this into a SELECT WHERE IN query
90-
foreach ($keys as $key) {
91-
if ($entity = $this->choiceList->getEntity($key)) {
92-
$collection->add($entity);
93-
} else {
94-
$notFound[] = $key;
95-
}
87+
$entities = $this->choiceList->getEntitiesByKeys($keys);
88+
if (count($keys) !== count($entities)) {
89+
throw new TransformationFailedException('Not all entities matching the keys were found.');
9690
}
9791

98-
if (count($notFound) > 0) {
99-
throw new TransformationFailedException(sprintf('The entities with keys "%s" could not be found', implode('", "', $notFound)));
92+
foreach ($entities as $entity) {
93+
$collection->add($entity);
10094
}
10195

10296
return $collection;

‎src/Symfony/Bridge/Doctrine/Form/DataTransformer/EntityToIdTransformer.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ public function reverseTransform($key)
7474
throw new UnexpectedTypeException($key, 'numeric');
7575
}
7676

77-
if (!($entity = $this->choiceList->getEntity($key))) {
77+
if (!($entities = $this->choiceList->getEntitiesByKeys(array($key)))) {
7878
throw new TransformationFailedException(sprintf('The entity with key "%s" could not be found', $key));
7979
}
8080

81-
return $entity;
81+
return $entities[0];
8282
}
8383
}

0 commit comments

Comments
 (0)
0