10000 [Form] Added an to simplify implementations and handle global optimi… · symfony/symfony@82e1931 · GitHub
[go: up one dir, main page]
Skip to content

Commit 82e1931

Browse files
HeahDudeHeahDude
committed
[Form] Added an to simplify implementations and handle global optimizations
1 parent c68ef46 commit 82e1931

File tree

7 files changed

+126
-128
lines changed

7 files changed

+126
-128
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
* changed guessing of DECIMAL to set the `input` option of `NumberType` to string
88
* deprecated not passing an `IdReader` to the `DoctrineChoiceLoader` when query can be optimized with a single id field
99
* deprecated passing an `IdReader` to the `DoctrineChoiceLoader` when entities have a composite id
10+
* added an `AbstractChoiceLoader` to simplify implementations and handle global optimizations
1011

1112
4.2.0
1213
-----

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

Lines changed: 6 additions & 61 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+
< 57AE span class="pl-k">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
*
@@ -75,71 +68,23 @@ public function __construct(ObjectManager $manager, string $class, IdReader $idR
7568
/**
7669
* {@inheritdoc}
7770
*/
78-
public function loadChoiceList($value = null)
71+
public function loadChoices(): array
7972
{
80-
if ($this->choiceList) {
81-
return $this->choiceList;
82-
}
83-
84-
$objects = $this->objectLoader
73+
return $this->objectLoader
8574
? $this->objectLoader->getEntities()
8675
: $this->manager->getRepository($this->class)->findAll();
87-
88-
return $this->choiceList = new ArrayChoiceList($objects, $value);
8976
}
9077

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

142-
if ($optimize && !$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
87+
if ($optimize && $this->objectLoader && $this->idReader->isSingleId()) {
14388
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
14489
$objectsById = [];
14590
$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;
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
@@ -94,12 +94,18 @@ protected function setUp()
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,7 +209,7 @@ 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

208215
public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
@@ -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'], [$this->idReader, 'getIdValue'])
328+
);
322329
}
323330

324331
public function testLoadChoicesForValuesLoadsAllIfSingleIntIdAndValueGiven()
@@ -426,8 +433,13 @@ public function testLoaderWithoutIdReaderCanBeOptimized()
426433

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

436+
$this->objectLoader->expects($this->never())
437+
->method('getEntities')
438+
;
439+
429440
$this->repository->expects($this->never())
430-
->method('findAll');
441+
->method('findAll')
442+
;
431443

432444
$this->objectLoader->expects($this->once())
433445
->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 (empty($values)) {
48+
return [];
49+
}
50+
51+
if ($this->choiceList) {
52+
return $this->choiceList->getChoicesForValues($values, $value);
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 (empty($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