8000 [Form] Fixed regression: Choices are compared by their values if a va… · symfony/symfony@26eba76 · GitHub
[go: up one dir, main page]
Skip to content

Commit 26eba76

Browse files
webmozartwebmozart
committed
[Form] Fixed regression: Choices are compared by their values if a value callback is given
1 parent a289deb commit 26eba76

File tree

11 files changed

+362
-260
lines changed

11 files changed

+362
-260
lines changed

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

Lines changed: 13 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@
1111

1212
namespace Symfony\Bridge\Doctrine\Form\ChoiceList;
1313

14-
use Doctrine\Common\Persistence\Mapping\ClassMetadata;
1514
use Doctrine\Common\Persistence\ObjectManager;
1615
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
1716
use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface;
1817
use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
19-
use Symfony\Component\Form\Exception\RuntimeException;
2018

2119
/**
2220
* Loads choices using a Doctrine object manager.
@@ -41,67 +39,20 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
4139
private $class;
4240

4341
/**
44-
* @var ClassMetadata
42+
* @var IdReader
4543
*/
46-
private $classMetadata;
44+
private $idReader;
4745

4846
/**
4947
* @var null|EntityLoaderInterface
5048
*/
5149
private $objectLoader;
5250

53-
/**
54-
* The identifier field, unless the identifier is composite
55-
*
56-
* @var null|string
57-
*/
58-
private $idField = null;
59-
60-
/**
61-
* Whether to use the identifier for value generation
62-
*
63-
* @var bool
64-
*/
65-
private $compositeId = true;
66-
6751
/**
6852
* @var ChoiceListInterface
6953
*/
7054
private $choiceList;
7155

72-
/**
73-
* Returns the value of the identifier field of an object.
74-
*
75-
* Doctrine must know about this object, that is, the object must already
76-
* be persisted or added to the identity map before. Otherwise an
77-
* exception is thrown.
78-
*
79-
* This method assumes that the object has a single-column identifier and
80-
* will return a single value instead of an array.
81-
*
82-
* @param object $object The object for which to get the identifier
83-
*
84-
* @return int|string The identifier value
85-
*
86-
* @throws RuntimeException If the object does not exist in Doctrine's identity map
87-
*
88-
* @internal Should not be accessed by user-land code. This method is public
89-
* only to be usable as callback.
90-
*/
91-
public static function getIdValue(ObjectManager $om, ClassMetadata $classMetadata, $object)
92-
{
93-
if (!$om->contains($object)) {
94-
throw new RuntimeException(
95-
'Entities passed to the choice field must be managed. Maybe '.
96-
'persist them in the entity manager?'
97-
);
98-
}
99-
100-
$om->initializeObject($object);
101-
102-
return current($classMetadata->getIdentifierValues($object));
103-
}
104-
10556
/**
10657
* Creates a new choice loader.
10758
*
@@ -114,22 +65,17 @@ public static function getIdValue(ObjectManager $om, ClassMetadata $classMetadat
11465
* @param ObjectManager $manager The object manager
11566
* @param string $class The class name of the
11667
* loaded objects
68+
* @param IdReader $idReader The reader for the object
69+
* IDs.
11770
* @param null|EntityLoaderInterface $objectLoader The objects loader
11871
*/
119-
public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, EntityLoaderInterface $objectLoader = null)
72+
public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, IdReader $idReader, EntityLoaderInterface $objectLoader = null)
12073
{
12174
$this->factory = $factory;
12275
$this->manager = $manager;
123-
$this->classMetadata = $manager->getClassMetadata($class);
124-
$this->class = $this->classMetadata->getName();
76+
$this->class = $manager->getClassMetadata($class)->getName();
77+
$this->idReader = $idReader;
12578
$this->objectLoader = $objectLoader;
126-
127-
$identifier = $this->classMetadata->getIdentifierFieldNames();
128-
129-
if (1 === count($identifier)) {
130-
$this->idField = $identifier[0];
131-
$this->compositeId = false;
132-
}
13379
}
13480

13581
/**
@@ -145,23 +91,7 @@ public function loadChoiceList($value = null)
14591
? $this->objectLoader->getEntities()
14692
: $this->manager->getRepository($this->class)->findAll();
14793

148-
// If the class has a multi-column identifier, we cannot index the
149-
// objects by their IDs
150-
if ($this->compositeId) {
151-
$this->choiceList = $this->factory->createListFromChoices($objects, $value);
152-
153-
return $this->choiceList;
154-
}
155-
156-
// Index the objects by ID
157-
$objectsById = array();
158-
159-
foreach ($objects as $object) {
160-
$id = self::getIdValue($this->manager, $this->classMetadata, $object);
161-
$objectsById[$id] = $object;
162-
}
163-
164-
$this->choiceList = $this->factory->createListFromChoices($objectsById, $value);
94+
$this->choiceList = $this->factory->createListFromChoices($objects, $value);
16595

16696
return $this->choiceList;
16797
}
@@ -193,14 +123,14 @@ public function loadValuesForChoices(array $objects, $value = null)
193123
// know that the IDs are used as values
194124

195125
// Attention: This optimization does not check choices for existence
196-
if (!$this->choiceList && !$this->compositeId) {
126+
if (!$this->choiceList && $this->idReader->isSingleId()) {
197127
$values = array();
198128

199129
// Maintain order and indices of the given objects
200130
foreach ($objects as $i => $object) {
201131
if ($object instanceof $this->class) {
202132
// Make sure to convert to the right format
203-
$values[$i] = (string) self::getIdValue($this->manager, $this->classMetadata, $object);
133+
$values[$i] = (string) $this->idReader->getIdValue($object);
204134
}
205135
}
206136

@@ -240,8 +170,8 @@ public function loadChoicesForValues(array $values, $value = null)
240170

241171
// Optimize performance in case we have an object loader and
242172
// a single-field identifier
243-
if (!$this->choiceList && !$this->compositeId && $this->objectLoader) {
244-
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idField, $values);
173+
if (!$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
174+
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
245175
$objectsById = array();
246176
$objects = array();
247177

@@ -250,8 +180,7 @@ public function loadChoicesForValues(array $values, $value = null)
250180
// "INDEX BY" clause to the Doctrine query in the loader,
251181
// but I'm not sure whether that's doable in a generic fashion.
252182
foreach ($unorderedObjects as $object) {
253-
$id = self::getIdValue($this->manager, $this->classMetadata, $object);
254-
$objectsById[$id] = $object;
183+
$objectsById[$this->idReader->getIdValue($object)] = $object;
255184
}
256185

257186
foreach ($values as $i => $id) {

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

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
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\Common\Persistence\Mapping\ClassMetadata;
15+
use Doctrine\Common\Persistence\ObjectManager;
16+
use Symfony\Component\Form\Exception\RuntimeException;
17+
18+
/**
19+
* A utility for reading object IDs.
20+
*
21+
* @since 1.0
22+
* @author Bernhard Schussek <bschussek@gmail.com>
23+
*
24+
* @internal This class is meant for internal use only.
25+
*/
26+
class IdReader
27+
{
28+
/**
29+
* @var ObjectManager
30+
*/
31+
private $om;
32+
33+
/**
34+
* @var ClassMetadata
35+
*/
36+
private $classMetadata;
37+
38+
/**
39+
* @var bool
40+
*/
41+
private $singleId;
42+
43+
/**
44+
* @var bool
45+
*/
46+
private $intId;
47+
48+
/**
49+
* @var string
50+
*/
51+
private $idField;
52+
53+
public function __construct(ObjectManager $om, ClassMetadata $classMetadata)
54+
{
55+
$ids = $classMetadata->getIdentifierFieldNames();
56+
$idType = $classMetadata->getTypeOfField(current($ids));
57+
58+
$this->om = $om;
59+
$this->classMetadata = $classMetadata;
60+
$this->singleId = 1 === count($ids);
61+
$this->intId = $this->singleId && 1 === count($ids) && in_array($idType, array('integer', 'smallint', 'bigint'));
62+
$this->idField = current($ids);
63+
}
64+
65+
/**
66+
* Returns whether the class has a single-column ID.
67+
*
68+
* @return bool Returns `true` if the class has a single-column ID and
69+
* `false` otherwise.
70+
*/
71+
public function isSingleId()
72+
{
73+
return $this->singleId;
74+
}
75+
76+
/**
77+
* Returns whether the class has a single-column integer ID.
78+
*
79+
* @return bool Returns `true` if the class has a single-column integer ID
80+
* and `false` otherwise.
81+
*/
82+
public function isIntId()
83+
{
84+
return $this->intId;
85+
}
86+
87+
/**
88+
* Returns the ID value for an object.
89+
*
90+
* This method assumes that the object has a single-column ID.
91+
*
92+
* @param object $object The object.
93+
*
94+
* @return mixed The ID value.
95+
*/
96+
public function getIdValue($object)
97+
{
98+
if (!$object) {
99+
return null;
100+
}
101+
102+
if (!$this->om->contains($object)) {
103+
throw new RuntimeException(
104+
'Entities passed to the choice field must be managed. Maybe '.
105+
'persist them in the entity manager?'
106+
);
107+
}
108+
109+
$this->om->initializeObject($object);
110+
111+
return current($this->classMetadata->getIdentifierValues($object));
112+
}
113+
114+
/**
115+
* Returns the name of the ID field.
116+
*
117+
* This method assumes that the object has a single-column ID.
118+
*
119+
* @return string The name of the ID field.
120+
*/
121+
public function getIdField()
122+
{
123+
return $this->idField;
124+
}
125+
}

0 commit comments

Comments
 (0)
0