8000 [Form] Fix same choice loader with different choice values · symfony/symfony@b41d306 · GitHub
[go: up one dir, main page]
Skip to content

Commit b41d306

Browse files
HeahDudeHeahDude
committed
[Form] Fix same choice loader with different choice values
1 parent 01a8e72 commit b41d306

File tree

9 files changed

+101
-32
lines changed

9 files changed

+101
-32
lines changed

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use Doctrine\Persistence\ObjectManager;
1515
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
16-
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
1716
use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
1817

1918
/**
@@ -29,9 +28,9 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
2928
private $objectLoader;
3029

3130
/**
32-
* @var ChoiceListInterface
31+
* @var array
3332
*/
34-
private $choiceList;
33+
private $choices;
3534

3635
/**
3736
* Creates a new choice loader.
@@ -74,15 +73,13 @@ public function __construct(ObjectManager $manager, string $class, IdReader $idR
7473
*/
7574
public function loadChoiceList($value = null)
7675
{
77-
if ($this->choiceList) {
78-
return $this->choiceList;
76+
if (null === $this->choices) {
77+
$this->choices = $this->objectLoader
78+
? $this->objectLoader->getEntities()
79+
: $this->manager->getRepository($this->class)->findAll();;
7980
}
8081

81-
$objects = $this->objectLoader
82-
? $this->objectLoader->getEntities()
83-
: $this->manager->getRepository($this->class)->findAll();
84-
85-
return $this->choiceList = new ArrayChoiceList($objects, $value);
82+
return new ArrayChoiceList($this->choices, $value);
8683
}
8784

8885
/**
@@ -100,7 +97,7 @@ public function loadValuesForChoices(array $choices, $value = null)
10097
$optimize = $this->idReader && (null === $value || \is_array($value) && $value[0] === $this->idReader);
10198

10299
// Attention: This optimization does not check choices for existence
103-
if ($optimize && !$this->choiceList && $this->idReader->isSingleId()) {
100+
if ($optimize && !$this->choices && $this->idReader->isSingleId()) {
104101
$values = [];
105102

106103
// Maintain order and indices of the given objects
@@ -136,7 +133,7 @@ public function loadChoicesForValues(array $values, $value = null)
136133
// a single-field identifier
137134
$optimize = $this->idReader && (null === $value || \is_array($value) && $this->idReader === $value[0]);
138135

139-
if ($optimize && !$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
136+
if ($optimize && !$this->choices && $this->objectLoader && $this->idReader->isSingleId()) {
140137
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
141138
$objectsById = [];
142139
$objects = [];

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,7 @@ public function testLoadChoiceListUsesObjectLoaderIfAvailable()
146146
$this->assertEquals($choiceList, $loaded = $loader->loadChoiceList());
147147

148148
// no further loads on subsequent calls
149-
150-
$this->assertSame($loaded, $loader->loadChoiceList());
149+
$this->assertEquals($loaded, $loader->loadChoiceList());
151150
}
152151

153152
public function testLoadValuesForChoices()

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"symfony/stopwatch": "^3.4|^4.0|^5.0",
3030
"symfony/config": "^4.2|^5.0",
3131
"symfony/dependency-injection": "^3.4|^4.0|^5.0",
32-
"symfony/form": "^4.4.11|^5.0.11",
32+
"symfony/form": "^4.4.41|^5.0.11",
3333
"symfony/http-kernel": "^4.3.7",
3434
"symfony/messenger": "^4.4|^5.0",
3535
"symfony/property-access": "^3.4|^4.0|^5.0",

‎src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ class LazyChoiceList implements ChoiceListInterface
3838
*/
3939
private $value;
4040

41+
/**
42+
* @var ChoiceListInterface|null
43+
*/
44+
private $loadedList;
45+
4146
/**
4247
* Creates a lazily-loaded list using the given loader.
4348
*
@@ -58,38 +63,58 @@ public function __construct(ChoiceLoaderInterface $loader, callable $value = nul
5863
*/
5964
public function getChoices()
6065
{
61-
return $this->loader->loadChoiceList($this->value)->getChoices();
66+
if ($this->loadedList) {
67+
return $this->loadedList->getChoices();
68+
}
69+
70+
return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getChoices();
6271
}
6372

6473
/**
6574
* {@inheritdoc}
6675
*/
6776
public function getValues()
6877
{
69-
return $this->loader->loadChoiceList($this->value)->getValues();
78+
if ($this->loadedList) {
79+
return $this->loadedList->getValues();
80+
}
81+
82+
return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getValues();
7083
}
7184

7285
/**
7386
* {@inheritdoc}
7487
*/
7588
public function getStructuredValues()
7689
{
77-
return $this->loader->loadChoiceList($this->value)->getStructuredValues();
90+
if ($this->loadedList) {
91+
return $this->loadedList->getStructuredValues();
92+
}
93+
94+
return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getStructuredValues();
7895
}
7996

8097
/**
8198
* {@inheritdoc}
8299
*/
83100
public function getOriginalKeys()
84101
{
85-
return $this->loader->loadChoiceList($this->value)->getOriginalKeys();
102+
if ($this->loadedList) {
103+
return $this->loadedList->getOriginalKeys();
104+
}
105+
106+
return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getOriginalKeys();
86107
}
87108

88109
/**
89110
* {@inheritdoc}
90111
*/
91112
public function getChoicesForValues(array $values)
92113
{
114+
if ($this->loadedList) {
115+
return $this->loadedList->getChoicesForValues($values);
116+
}
117+
93118
return $this->loader->loadChoicesForValues($values, $this->value);
94119
}
95120

@@ -98,6 +123,10 @@ public function getChoicesForValues(array $values)
98123
*/
99124
public function getValuesForChoices(array $choices)
100125
{
126+
if ($this->loadedList) {
127+
return $this->loadedList->getValuesForChoices($choices);
128+
}
129+
101130
return $this->loader->loadValuesForChoices($choices, $this->value);
102131
}
103132
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ class CallbackChoiceLoader implements ChoiceLoaderInterface
2323
private $callback;
2424

2525
/**
26-
* The loaded choice list.
26+
* The loaded choices.
2727
*
28-
* @var ArrayChoiceList
28+
* @var array|null
2929
*/
30-
private $choiceList;
30+
private $choices;
3131

3232
/**
3333
* @param callable $callback The callable returning an array of choices
@@ -42,11 +42,11 @@ public function __construct(callable $callback)
4242
*/
4343
public function loadChoiceList($value = null)
4444
{
45-
if (null !== $this->choiceList) {
46-
return $this->choiceList;
45+
if (null === $this->choices) {
46+
$this->choices = ($this->callback)();
4747
}
4848

49-
return $this->choiceList = new ArrayChoiceList(($this->callback)(), $value);
49+
return new ArrayChoiceList($this->choices, $value);
5050
}
5151

5252
/**

‎src/Symfony/Component/Form/Tests/ChoiceList/LazyChoiceListTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public function testGetChoicesForValuesForwardsCallIfListNotLoaded()
9898

9999
$this->assertSame(['foo', 'bar'], $list->getChoicesForValues(['a', 'b']));
100100
$this->assertSame(['foo', 'bar'], $list->getChoicesForValues(['a', 'b']));
101-
$this->assertSame(3, $calls);
101+
$this->assertSame(6, $calls);
102102
}
103103

104104
public function testGetChoicesForValuesUsesLoadedList()

‎src/Symfony/Component/Form/Tests/ChoiceList/Loader/CallbackChoiceLoaderTest.php

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,18 @@ public function testLoadChoiceList()
6767
$this->assertInstanceOf(ChoiceListInterface::class, self::$loader->loadChoiceList(self::$value));
6868
}
6969

70-
public function testLoadChoiceListOnlyOnce()
70+
public function testLoadChoicesOnlyOnce()
7171
{
72-
$loadedChoiceList = self::$loader->loadChoiceList(self::$value);
72+
$calls = 0;
73+
$loader = new CallbackChoiceLoader(function () use (&$calls) {
74+
++$calls;
7375

74-
$this->assertSame($loadedChoiceList, self::$loader->loadChoiceList(self::$value));
76+
return [1];
77+
});
78+
$loadedChoiceList = $loader->loadChoiceList();
79+
80+
$this->assertNotSame($loadedChoiceList, $loader->loadChoiceList());
81+
$this->assertSame(1, $calls);
7582
}
7683

7784
public function testLoadChoicesForValuesLoadsChoiceListOnFirstCall()

‎src/Symfony/Component/Form/Tests/ChoiceList/Loader/IntlCallbackChoiceLoaderTest.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,19 @@ public function testLoadChoiceList()
6868
$this->assertInstanceOf(ChoiceListInterface::class, self::$loader->loadChoiceList(self::$value));
6969
}
7070

71-
public function testLoadChoiceListOnlyOnce()
71+
public function testLoadChoicesOnlyOnce()
7272
{
73-
$loadedChoiceList = self::$loader->loadChoiceList(self::$value);
73+
$calls = 0;
74+
$loader = new IntlCallbackChoiceLoader(function () use (&$calls) {
75+
++$calls;
7476

75-
$this->assertSame($loadedChoiceList, self::$loader->loadChoiceList(self::$value));
77+
return self::$choices;
78+
});
79+
80+
$loadedChoiceList = $loader->loadChoiceList(self::$value);
81+
82+
$this->assertNotSame($loadedChoiceList, $loader->loadChoiceList(self::$value));
83+
$this->assertSame(1, $calls);
7684
}
7785

7886
public function testLoadChoicesForValuesLoadsChoiceListOnFirstCall()

‎src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php

Lines changed: 29 additions & 0 deletions
10000
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Form\Tests\Extension\Core\Type;
1313

14+
use Symfony\Component\Form\ChoiceList\Loader\CallbackChoiceLoader;
1415
use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
1516
use Symfony\Component\Form\ChoiceList\View\ChoiceView;
1617
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
@@ -2165,4 +2166,32 @@ public function expandedIsEmptyWhenNoRealChoiceIsSelectedProvider()
21652166
'Placeholder submitted / single / not required / with a placeholder -> should not be empty' => [false, '', false, false, 'ccc'], // The placeholder is a selected value
21662167
];
21672168
}
2169+
2170+
public function testWithSameLoaderAndDifferentChoiceValueCallbacks()
2171+
{
2172+
$choiceLoader = new CallbackChoiceLoader(function () {
2173+
return [1, 2, 3];
2174+
});
2175+
2176+
$view = $this->factory->create(FormTypeTest::TESTED_TYPE)
2177+
->add('choice_one', self::TESTED_TYPE, [
2178+
'choice_loader' => $choiceLoader,
2179+
])
2180+
->add('choice_two', self::TESTED_TYPE, [
2181+
'choice_loader' => $choiceLoader,
2182+
'choice_value' => function ($choice) {
2183+
return $choice ? (string) $choice * 10 : '';
2184+
},
2185+
])
2186+
->createView()
2187+
;
2188+
2189+
$this->assertSame('1', $view['choice_one']->vars['choices'][0]->value);
2190+
$this->assertSame('2', $view['choice_one']->vars['choices'][1]->value);
2191+
$this->assertSame('3', $view['choice_one']->vars['choices'][2]->value);
2192+
2193+
$this->assertSame('10', $view['choice_two']->vars['choices 5240 9;][0]->value);
2194+
$this->assertSame('20', $view['choice_two']->vars['choices'][1]->value);
2195+
$this->assertSame('30', $view['choice_two']->vars['choices'][2]->value);
2196+
}
21682197
}

0 commit comments

Comments
 (0)
0