Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d9379ad

Browse filesBrowse files
HeahDudeHeahDude
committed
[Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations
1 parent dab6732 commit d9379ad
Copy full SHA for d9379ad

File tree

9 files changed

+127
-137
lines changed
Filter options

9 files changed

+127
-137
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php
+11-66Lines changed: 11 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,20 @@
1212
namespace Symfony\Bridge\Doctrine\Form\ChoiceList;
1313

1414
use Doctrine\Common\Persistence\ObjectManager;
15-
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
16-
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
17-
use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
15+
use Symfony\Component\Form\ChoiceList\Loader\AbstractChoiceLoader;
1816

1917
/**
2018
* Loads choices using a Doctrine object manager.
2119
*
2220
* @author Bernhard Schussek <bschussek@gmail.com>
2321
*/
24-
class DoctrineChoiceLoader implements ChoiceLoaderInterface
22+
class DoctrineChoiceLoader extends AbstractChoiceLoader
2523
{
2624
private $manager;
2725
private $class;
2826
private $idReader;
2927
private $objectLoader;
3028

31-
/**
32-
* @var ChoiceListInterface
33-
*/
34-
private $choiceList;
35-
3629
/**
3730
* Creates a new choice loader.
3831
*
@@ -59,81 +52,33 @@ public function __construct(ObjectManager $manager, string $class, IdReader $idR
5952
/**
6053
* {@inheritdoc}
6154
*/
62-
public function loadChoiceList(callable $value = null)
55+
protected function loadChoices(): iterable
6356
{
64-
if ($this->choiceList) {
65-
return $this->choiceList;
66-
}
67-
68-
$objects = $this->objectLoader
57+
return $this->objectLoader
6958
? $this->objectLoader->getEntities()
70-
: $this->manager->getRepository($this->class)->findAll();
71-
72-
return $this->choiceList = new ArrayChoiceList($objects, $value);
59+
: $this->manager->getRepository($this->class)->findAll()
60+
;
7361
}
7462

7563
/**
7664
* {@inheritdoc}
7765
*/
78-
public function loadValuesForChoices(array $choices, callable $value = null)
66+
protected function doLoadChoicesForValues(array $values, ?callable $value): array
7967
{
80-
// Performance optimization
81-
if (empty($choices)) {
82-
return [];
83-
}
84-
85-
// Optimize performance for single-field identifiers. We already
86-
// know that the IDs are used as values
87-
$optimize = $this->idReader && (null === $value || \is_array($value) && $value[0] === $this->idReader);
88-
89-
// Attention: This optimization does not check choices for existence
90-
if ($optimize && !$this->choiceList && $this->idReader->isSingleId()) {
91-
$values = [];
92-
93-
// Maintain order and indices of the given objects
94-
foreach ($choices as $i => $object) {
95-
if ($object instanceof $this->class) {
96-
// Make sure to convert to the right format
97-
$values[$i] = (string) $this->idReader->getIdValue($object);
98-
}
99-
}
100-
101-
return $values;
102-
}
103-
104-
return $this->loadChoiceList($value)->getValuesForChoices($choices);
105-
}
106-
107-
/**
108-
* {@inheritdoc}
109-
*/
110-
public function loadChoicesForValues(array $values, callable $value = null)
111-
{
112-
// Performance optimization
113-
// Also prevents the generation of "WHERE id IN ()" queries through the
114-
// object loader. At least with MySQL and on the development machine
115-
// this was tested on, no exception was thrown for such invalid
116-
// statements, consequently no test fails when this code is removed.
117-
// https://github.com/symfony/symfony/pull/8981#issuecomment-24230557
118-
if (empty($values)) {
119-
return [];
120-
}
121-
12268
// Optimize performance in case we have an object loader and
12369
// a single-field identifier
124-
$optimize = $this->idReader && (null === $value || \is_array($value) && $this->idReader === $value[0]);
70+
$optimize = $this->idReader && (null === $value || (\is_array($value) && $this->idReader === $value[0]));
12571

126-
if ($optimize && !$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
127-
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
72+
if ($optimize && $this->objectLoader) {
12873
$objectsById = [];
12974
$objects = [];
13075

13176
// Maintain order and indices from the given $values
13277
// An alternative approach to the following loop is to add the
13378
// "INDEX BY" clause to the Doctrine query in the loader,
13479
// but I'm not sure whether that's doable in a generic fashion.
135-
foreach ($unorderedObjects as $object) {
136-
$objectsById[(string) $this->idReader->getIdValue($object)] = $object;
80+
foreach ($this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values) as $object) {
81+
$objectsById[$this->idReader->getIdValue($object)] = $object;
13782
}
13883

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

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function isIntId(): bool
8484
*
8585
* This method assumes that the object has a single-column ID.
8686
*
87-
* @return mixed The ID value
87+
* @return string|null The ID value
8888
*/
8989
public function getIdValue(object $object = null)
9090
{
@@ -104,7 +104,7 @@ public function getIdValue(object $object = null)
104104
$idValue = $this->associationIdReader->getIdValue($idValue);
105105
}
106106

107-
return $idValue;
107+
return (string) $idValue;
108108
}
109109

110110
/**

‎src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ abstract class DoctrineType extends AbstractType implements ResetInterface
5050
*
5151
* For backwards compatibility, objects are cast to strings by default.
5252
*
53-
*
5453
* @internal This method is public to be usable as callback. It should not
5554
* be used in user code.
5655
*/
@@ -143,8 +142,7 @@ public function configureOptions(OptionsResolver $resolver)
143142
$options['em'],
144143
$options['class'],
145144
$options['id_reader'],
146-
$entityLoader,
147-
false
145+
$entityLoader
148146
);
149147

150148
if (null !== $hash) {

‎src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
+16-9Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,18 @@ protected function setUp(): void
9494
$this->om->expects($this->any())
9595
->method('getRepository')
9696
->with($this->class)
97-
->willReturn($this->repository);
97+
->willReturn($this->repository)
98+
;
9899

99100
$this->om->expects($this->any())
100101
->method('getClassMetadata')
101102
->with($this->class)
102-
->willReturn(new ClassMetadata($this->class));
103+
->willReturn(new ClassMetadata($this->class))
104+
;
105+
$this->repository->expects($this->any())
106+
->method('findAll')
107+
->willReturn([$this->obj1, $this->obj2, $this->obj3])
108+
;
103109
}
104110

105111
public function testLoadChoiceList()
@@ -116,7 +122,8 @@ public function testLoadChoiceList()
116122

117123
$this->repository->expects($this->once())
118124
->method('findAll')
119-
->willReturn($choices);
125+
->willReturn($choices)
126+
;
120127

121128
$this->assertEquals($choiceList, $loader->loadChoiceList($value));
122129

@@ -202,10 +209,10 @@ public function testLoadValuesForChoicesDoesNotLoadIfSingleIntId()
202209
->with($this->obj2)
203210
->willReturn('2');
204211

205-
$this->assertSame(['2'], $loader->loadValuesForChoices([$this->obj2]));
212+
$this->assertSame(['2'], $loader->loadValuesForChoices([$this->obj2], [$this->idReader, 'getIdValue']));
206213
}
207214

208-
public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
215+
public function testLoadValuesForChoicesDoesNotLoadIfSingleIntIdAndValueGiven()
209216
{
210217
$loader = new DoctrineChoiceLoader(
211218
$this->om,
@@ -216,7 +223,7 @@ public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
216223
$choices = [$this->obj1, $this->obj2, $this->obj3];
217224
$value = function (\stdClass $object) { return $object->name; };
218225

219-
$this->repository->expects($this->once())
226+
$this->repository->expects($this->never())
220227
->method('findAll')
221228
->willReturn($choices);
222229

@@ -285,7 +292,7 @@ public function testLoadChoicesForValuesDoesNotLoadIfEmptyValues()
285292
$this->assertSame([], $loader->loadChoicesForValues([]));
286293
}
287294

288-
public function testLoadChoicesForValuesLoadsOnlyChoicesIfSingleIntId()
295+
public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader()
289296
{
290297
$loader = new DoctrineChoiceLoader(
291298
$this->om,
@@ -317,8 +324,8 @@ public function testLoadChoicesForValuesLoadsOnlyChoicesIfSingleIntId()
317324

318325
$this->assertSame(
319326
[4 => $this->obj3, 7 => $this->obj2],
320-
$loader->loadChoicesForValues([4 => '3', 7 => '2']
321-
));
327+
$loader->loadChoicesForValues([4 => '3', 7 => '2'])
328+
);
322329
}
323330

324331
public function testLoadChoicesForValuesLoadsAllIfSingleIntIdAndValueGiven()

‎src/Symfony/Bridge/Doctrine/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/composer.json
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"symfony/stopwatch": "^4.4|^5.0",
2828
"symfony/config": "^4.4|^5.0",
2929
"symfony/dependency-injection": "^4.4|^5.0",
30-
"symfony/form": "^5.0",
30+
"symfony/form": "^5.1",
3131
"symfony/http-kernel": "^5.0",
3232
"symfony/messenger": "^4.4|^5.0",
3333
"symfony/property-access": "^4.4|^5.0",

‎src/Symfony/Component/Form/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/CHANGELOG.md
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
5.1.0
5+
-----
6+
7+
* added an `AbstractChoiceLoader` to simplify implementations and handle global optimizations
8+
49
5.0.0
510
-----
611

‎src/Symfony/Component/Form/ChoiceList/Loader/AbstractChoiceLoader.php

Copy file name to clipboard
+86Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
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\Component\Form\ChoiceList\Loader;
13+
14+
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
15+
16+
/**
17+
* @author Jules Pietri <jules@heahprod.com>
18+
*/
19+
abstract class AbstractChoiceLoader implements ChoiceLoaderInterface
20+
{
21+
/**
22+
* The loaded choice list.
23+
*
24+
* @var ArrayChoiceList
25+
*/
26+
private $choiceList;
27+
28+
/**
29+
* {@inheritdoc}
30+
*/
31+
public function loadChoiceList(callable $value = null)
32+
{
33+
return $this->choiceList ?? ($this->choiceList = new ArrayChoiceList($this->loadChoices(), $value));
34+
}
35+
36+
/**
37+
* {@inheritdoc}
38+
*/
39+
public function loadChoicesForValues(array $values, callable $value = null)
40+
{
41+
// Optimize
42+
if (!$values) {
43+
return [];
44+
}
45+
46+
if ($this->choiceList) {
47+
return $this->choiceList->getChoicesForValues($values);
48+
}
49+
50+
return $this->doLoadChoicesForValues($values, $value);
51+
}
52+
53+
/**
54+
* {@inheritdoc}
55+
*/
56+
public function loadValuesForChoices(array $choices, callable $value = null)
57+
{
58+
// Optimize
59+
if (!$choices) {
60+
return [];
61+
}
62+
63+
if ($value) {
64+
// if a value callback exists, use it
65+
return array_map($value, $choices);
66+
}
67+
68+
if ($this->choiceList) {
69+
return $this->choiceList->getValuesForChoices($choices);
70+
}
71+
72+
return $this->doLoadValuesForChoices($choices);
73+
}
74+
75+
abstract protected function loadChoices(): iterable;
76+
77+
protected function doLoadChoicesForValues(array $values, ?callable $value): array
78+
{
79+
return $this->loadChoiceList($value)->getChoicesForValues($values);
80+
}
81+
82+
protected function doLoadValuesForChoices(array $choices): array
83+
{
84+
return $this->loadChoiceList()->getValuesForChoices($choices);
85+
}
86+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.