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

Commit 1d30019

Browse files
HeahDudeyceruto
HeahDude
authored and
yceruto
committed
[Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations
1 parent 940eabb commit 1d30019

File tree

7 files changed

+127
-129
lines changed

7 files changed

+127
-129
lines changed

‎src/Symfony/Bridge/Doctrine/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ CHANGELOG
77
* added `DoctrineClearEntityManagerMiddleware`
88
* deprecated `RegistryInterface`, use `Doctrine\Common\Persistence\ManagerRegistry`
99
* added support for invokable event listeners
10+
* added an `AbstractChoiceLoader` to simplify implementations and handle global optimizations
1011

1112
4.3.0
1213
-----

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

Lines changed: 7 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
*
@@ -72,71 +65,23 @@ public function __construct(ObjectManager $manager, string $class, IdReader $idR
7265
/**
7366
* {@inheritdoc}
7467
*/
75-
public function loadChoiceList($value = null)
68+
protected function loadChoices(): array
7669
{
77-
if ($this->choiceList) {
78-
return $this->choiceList;
79-
}
80-
81-
$objects = $this->objectLoader
70+
return $this->objectLoader
8271
? $this->objectLoader->getEntities()
8372
: $this->manager->getRepository($this->class)->findAll();
84-
85-
return $this->choiceList = new ArrayChoiceList($objects, $value);
8673
}
8774

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

139-
if ($optimize && !$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
84+
if ($optimize && $this->objectLoader && $this->idReader->isSingleId()) {
14085
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
14186
$objectsById = [];
14287
$objects = [];

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public function isIntId(): bool
9191
public function getIdValue($object)
9292
{
9393
if (!$object) {
94-
return null;
94+
return '';
9595
}
9696

9797
if (!$this->om->contains($object)) {
@@ -106,7 +106,7 @@ public function getIdValue($object)
106106
$idValue = $this->associationIdReader->getIdValue($idValue);
107107
}
108108

109-
return $idValue;
109+
return (string) $idValue;
110110
}
111111

112112
/**

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

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,18 @@ protected function setUp(): void
9595
$this->om->expects($this->any())
9696
->method('getRepository')
9797
->with($this->class)
98-
->willReturn($this->repository);
98+
->willReturn($this->repository)
99+
;
99100

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

106112
public function testLoadChoiceList()
@@ -117,7 +123,8 @@ public function testLoadChoiceList()
117123

118124
$this->repository->expects($this->once())
119125
->method('findAll')
120-
->willReturn($choices);
126+
->willReturn($choices)
127+
;
121128

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

@@ -203,7 +210,7 @@ public function testLoadValuesForChoicesDoesNotLoadIfSingleIntId()
203210
->with($this->obj2)
204211
->willReturn('2');
205212

206-
$this->assertSame(['2'], $loader->loadValuesForChoices([$this->obj2]));
213+
$this->assertSame(['2'], $loader->loadValuesForChoices([$this->obj2], [$this->idReader, 'getIdValue']));
207214
}
208215

209216
public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
@@ -217,7 +224,7 @@ public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
217224
$choices = [$this->obj1, $this->obj2, $this->obj3];
218225
$value = function (\stdClass $object) { return $object->name; };
219226

220-
$this->repository->expects($this->once())
227+
$this->repository->expects($this->never())
221228
->method('findAll')
222229
->willReturn($choices);
223230

@@ -286,7 +293,7 @@ public function testLoadChoicesForValuesDoesNotLoadIfEmptyValues()
286293
$this->assertSame([], $loader->loadChoicesForValues([]));
287294
}
288295

289-
public function testLoadChoicesForValuesLoadsOnlyChoicesIfSingleIntId()
296+
public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader()
290297
{
291298
$loader = new DoctrineChoiceLoader(
292299
$this->om,
@@ -318,8 +325,8 @@ public function testLoadChoic 10000 esForValuesLoadsOnlyChoicesIfSingleIntId()
318325

319326
$this->assertSame(
320327
[4 => $this->obj3, 7 => $this->obj2],
321-
$loader->loadChoicesForValues([4 => '3', 7 => '2']
322-
));
328+
$loader->loadChoicesForValues([4 => '3', 7 => '2'], [$this->idReader, 'getIdValue'])
329+
);
323330
}
324331

325332
public function testLoadChoicesForValuesLoadsAllIfSingleIntIdAndValueGiven()
@@ -427,8 +434,13 @@ public function testLoaderWithoutIdReaderCanBeOptimized()
427434

428435
$choices = [$obj1, $obj2];
429436

437+
$this->objectLoader->expects($this->never())
438+
->method('getEntities')
439+
;
440+
430441
$this->repository->expects($this->never())
431-
->method('findAll');
442+
->method('findAll')
443+
;
432444

433445
$this->objectLoader->expects($this->once())
434446
->method('getEntitiesByIds')

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

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

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

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,15 @@
1111

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

14-
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
15-
1614
/**
1715
* Loads an {@link ArrayChoiceList} instance from a callable returning an array of 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

25-
/**
26-
* The loaded choice list.
27-
*
28-
* @var ArrayChoiceList
29-
*/
30-
private $choiceList;
31-
3223
/**
3324
* @param callable $callback The callable returning an array of choices
3425
*/
@@ -37,41 +28,8 @@ public function __construct(callable $callback)
3728
$this->callback = $callback;
3829
}
3930

40-
/**
41-
* {@inheritdoc}
42-
*/
43-
public function loadChoiceList($value = null)
31+
protected function loadChoices(): array
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, $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, $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