8000 [Form] Added an AbstractChoiceLoader to simplify implementations and … · symfony/symfony@07b498c · GitHub
[go: up one dir, main page]
Skip to content

Commit 07b498c

Browse files
HeahDudeHeahDude
committed
[Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations
1 parent ae00ff4 commit 07b498c

File tree

8 files changed

+129
-127
lines changed

8 files changed

+129
-127
lines changed

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

Lines changed: 18 additions & 62 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,44 @@ 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

75-
/**
76-
* {@inheritdoc}
77-
*/
78-
public function loadValuesForChoices(array $choices, callable $value = null)
63+
protected function doLoadValuesForChoices(array $choices): array
7964
{
80-
// Performance optimization
81-
if (empty($choices)) {
82-
return [];
65+
if ($this->idReader) {
66+
return array_filter(array_map(function ($choice): ?string {
67+
return $choice instanceof $this->class ? $this->idReader->getIdValue($choice) : null;
68+
}, $choices));
8369
}
8470

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) {
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);
71+
return parent::doLoadValuesForChoices($choices);
10572
}
10673

10774
/**
10875
* {@inheritdoc}
10976
*/
110-
public function loadChoicesForValues(array $values, callable $value = null)
77+
protected function doLoadChoicesForValues(array $values, ?callable $value): array
11178
{
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-
12279
// Optimize performance in case we have an object loader and
12380
// a single-field identifier
124-
$optimize = $this->idReader && (null === $value || \is_array($value) && $this->idReader === $value[0]);
81+
$optimize = $this->idReader && (null === $value || (\is_array($value) && $this->idReader === $value[0]));
12582

126-
if ($optimize && !$this->choiceList && $this->objectLoader) {
127-
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
83+
if ($optimize && $this->objectLoader) {
12884
$objectsById = [];
12985
$objects = [];
13086

13187
// Maintain order and indices from the given $values
13288
// An alternative approach to the following loop is to add the
13389
// "INDEX BY" clause to the Doctrine query in the loader,
13490
// 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;
91+
foreach ($this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values) as $object) {
92+
$objectsById[$this->idReader->getIdValue($object)] = $object;
13793
}
13894

13995
foreach ($values as $i => $id) {
@@ -145,6 +101,6 @@ public function loadChoicesForValues(array $values, callable $value = null)
145101
return $objects;
146102
}
147103

148-
return $this->loadChoiceList($value)->getChoicesForValues($values);
104+
return parent::doLoadChoicesForValues($values, $value);
149105
}
150106
}

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

Lines 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/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ protected function setUp(): void
100100
->method('getClassMetadata')
101101
->with($this->class)
102102
->willReturn(new ClassMetadata($this->class));
103+
$this->repository->expects($this->any())
104+
->method('findAll')
105+
->willReturn([$this->obj1, $this->obj2, $this->obj3])
106+
;
103107
}
104108

105109
public function testLoadChoiceList()
@@ -205,7 +209,7 @@ public function testLoadValuesForChoicesDoesNotLoadIfSingleIntId()
205209
$this->assertSame(['2'], $loader->loadValuesForChoices([$this->obj2]));
206210
}
207211

208-
public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
212+
public function testLoadValuesForChoicesDoesNotLoadIfSingleIntIdAndValueGiven()
209213
{
210214
$loader = new DoctrineChoiceLoader(
211215
$this->om,
@@ -216,7 +220,7 @@ public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
216220
$choices = [$this->obj1, $this->obj2, $this->obj3];
217221
$value = function (\stdClass $object) { return $object->name; };
218222

219-
$this->repository->expects($this->once())
223+
$this->repository->expects($this->never())
220224
->method('findAll')
221225
->willReturn($choices);
222226

@@ -285,7 +289,7 @@ public function testLoadChoicesForValuesDoesNotLoadIfEmptyValues()
285289
$this->assertSame([], $loader->loadChoicesForValues([]));
286290
}
287291

288-
public function testLoadChoicesForValuesLoadsOnlyChoicesIfSingleIntId()
292+
public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader()
289293
{
290294
$loader = new DoctrineChoiceLoader(
291295
$this->om,

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

Lines changed: 2 additions & 2 deletions
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",
@@ -49,7 +49,7 @@
4949
"conflict": {
5050
"phpunit/phpunit": "<5.4.3",
5151
"symfony/dependency-injection": "<4.4",
52-
"symfony/form": "<5",
52+
"symfony/form": "<5.1",
5353
"symfony/http-kernel": "<5",
5454
"symfony/messenger": "<4.4",
5555
"symfony/property-info": "<5",

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

Lines 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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
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+
* @final
30+
*
31+
* {@inheritdoc}
32+
*/
33+
public function loadChoiceList(callable $value = null)
34+
{
35+
return $this->choiceList ?? ($this->choiceList = new ArrayChoiceList($this->loadChoices(), $value));
36+
}
37+
38+
/**
39+
* {@inheritdoc}
40+
*/
41+
public function loadChoicesForValues(array $values, callable $value = null)
42+
{
43+
// Optimize
44+
if (!$values) {
45+
return [];
46+
}
47+
48+
if ($this->choiceList) {
49+
return $this->choiceList->getChoicesForValues($values);
50+
}
51+
52+
return $this->doLoadChoicesForValues($values, $value);
53+
}
54+
55+
/**
56+
* {@inheritdoc}
57+
*/
58+
public function loadValuesForChoices(array $choices, callable $value = null)
59+
{
60+
// Optimize
61+
if (!$choices) {
62+
return [];
63+
}
64+
65+
if ($value) {
66+
// if a value callback exists, use it
67+
return array_map($value, $choices);
68+
}
69+
70+
if ($this->choiceList) {
71+
return $this->choiceList->getValuesForChoices($choices);
72+
}
73+
74+
return $this->doLoadValuesForChoices($choices);
75+
}
76+
77+
abstract protected function loadChoices(): iterable;
78+
79+
protected function doLoadChoicesForValues(array $values, ?callable $value): array
80+
{
81+
return $this->loadChoiceList($value)->getChoicesForValues($values);
82+
}
83+
84+
protected function doLoadValuesForChoices(array $choices): array
85+
{
86+
return $this->loadChoiceList()->getValuesForChoices($choices);
87+
}
88+
}

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

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -11,67 +11,25 @@
1111

1212
namespace Symfony\Component\Form\ChoiceList\Loader;
1313

14-
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
15-
1614
/**
17-
* Loads an {@link ArrayChoiceList} instance from a callable returning an array of choices.
15+
* Loads an {@link ArrayChoiceList} instance from a callable returning iterable choices.
1816
*
1917
* @author Jules Pietri <jules@heahprod.com>
2018
*/
21-
class CallbackChoiceLoader implements ChoiceLoaderInterface
19+
class CallbackChoiceLoader extends AbstractChoiceLoader
2220
{
2321
private $callback;
2422

2523
/**
26-
* The loaded choice list.
27-
*
28-
* @var ArrayChoiceList
29-
*/
30-
private $choiceList;
31-
32-
/**
33-
* @param callable $callback The callable returning an array of choices
24+
* @param callable $callback The callable returning iterable choices
3425
*/
3526
public function __construct(callable $callback)
3627
{
3728
$this->callback = $callback;
3829
}
3930

40-
/**
41-
* {@inheritdoc}
42-
*/
43-
public function loadChoiceList(callable $value = null)
31+
protected function loadChoices(): iterable
4432
{
45-
if (null !== $this->choiceList) {
46-
return $this->choiceList;
47-
}
48-
49-
return $this->choiceList = new ArrayChoiceList(($this->callback)(), $value);
50-
}
51-
52-
/**
53-
* {@inheritdoc}
54-
*/
55-
public function loadChoicesForValues(array $values, callable $value = null)
56-
{
57-
// Optimize
58-
if (empty($values)) {
59-
return [];
60-
}
61-
62-
return $this->loadChoiceList($value)->getChoicesForValues($values);
63-
}
64-
65-
/**
66-
* {@inheritdoc}
67-
*/
68-
public function loadValuesForChoices(array $choices, callable $value = null)
69-
{
70-
// Optimize
71-
if (empty($choices)) {
72-
return [];
73-
}
74-
75-
return $this->loadChoiceList($value)->getValuesForChoices($choices);
33+
return ($this->callback)();
7634
}
7735
}

0 commit comments

Comments
 (0)
0