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 07b498c

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

File tree

8 files changed

+129
-127
lines changed
Filter options

8 files changed

+129
-127
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
+18-62Lines 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

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
+7-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/composer.json
+2-2Lines 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

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
+88Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/ChoiceList/Loader/CallbackChoiceLoader.php
+5-47Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.