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 98621f4

Browse filesBrowse files
HeahDudeHeahDude
committed
[Form] optimized LazyChoiceList
1 parent 86b2ff1 commit 98621f4
Copy full SHA for 98621f4

File tree

2 files changed

+136
-38
lines changed
Filter options

2 files changed

+136
-38
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php
+78-14Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,18 @@ class LazyChoiceList implements ChoiceListInterface
4545

4646
/**
4747
* @var ChoiceListInterface|null
48+
*
49+
* @deprecated Since 3.1, to be remove in 4.0. Cache the choice list in the {@link ChoiceLoaderInterface} instead.
4850
*/
4951
private $loadedList;
5052

53+
/**
54+
* @var bool
55+
*
56+
* @deprecated Flag used for BC layer since 3.1. To be removed in 4.0.
57+
*/
58+
private $loaded = false;
59+
5160
/**
5261
* Creates a lazily-loaded list using the given loader.
5362
*
@@ -70,70 +79,125 @@ public function __construct(ChoiceLoaderInterface $loader, callable $value = nul
7079
*/
7180
public function getChoices()
7281
{
73-
if (!$this->loadedList) {
74-
$this->loadedList = $this->loader->loadChoiceList($this->value);
82+
if ($this->loaded) {
83+
// We can safely invoke the {@link ChoiceLoaderInterface} assuming it has the list
84+
// in cache when the lazy list is already loaded
85+
if ($this->loadedList !== $this->loader->loadChoiceList($this->value)) {
86+
@trigger_error(sprintf('Caching the choice list in %s is deprecated since 3.1 and will not happen in 4.0. Cache the list in the %s instead.', __CLASS__, ChoiceLoaderInterface::class));
87+
}
88+
89+
return $this->loadedList->getChoices();
7590
}
7691

92+
// BC
93+
$this->loadedList = $this->loader->loadChoiceList($this->value);
94+
$this->loaded = true;
95+
7796
return $this->loadedList->getChoices();
97+
// In 4.0 keep the following line only:
98+
// return $this->loader->loadChoiceList($this->value)->getChoices()
7899
}
79100

80101
/**
81102
* {@inheritdoc}
82103
*/
83104
public function getValues()
84105
{
85-
if (!$this->loadedList) {
86-
$this->loadedList = $this->loader->loadChoiceList($this->value);
106+
if ($this->loaded) {
107+
// Check whether the loader has the same cache
108+
if ($this->loadedList !== $this->loader->loadChoiceList($this->value)) {
109+
@trigger_error(sprintf('Caching the choice list in %s is deprecated since 3.1 and will not happen in 4.0. Cache the list in the %s instead.', __CLASS__, ChoiceLoaderInterface::class));
110+
}
111+
112+
return $this->loadedList->getValues();
87113
}
88114

115+
// BC
116+
$this->loadedList = $this->loader->loadChoiceList($this->value);
117+
$this->loaded = true;
118+
89119
return $this->loadedList->getValues();
120+
// In 4.0 keep the following line only:
121+
// return $this->loader->loadChoiceList($this->value)->getValues()
90122
}
91123

92124
/**
93125
* {@inheritdoc}
94126
*/
95127
public function getStructuredValues()
96128
{
97-
if (!$this->loadedList) {
98-
$this->loadedList = $this->loader->loadChoiceList($this->value);
129+
if ($this->loaded) {
130+
// Check whether the loader has the same cache
131+
if ($this->loadedList !== $this->loader->loadChoiceList($this->value)) {
132+
@trigger_error(sprintf('Caching the choice list in %s is deprecated since 3.1 and will not happen in 4.0. Cache the list in the %s instead.', __CLASS__, ChoiceLoaderInterface::class));
133+
}
134+
135+
return $this->loadedList->getStructuredValues();
99136
}
100137

138+
// BC
139+
$this->loadedList = $this->loader->loadChoiceList($this->value);
140+
$this->loaded = true;
141+
101142
return $this->loadedList->getStructuredValues();
143+
// In 4.0 keep the following line only:
144+
// return $this->loader->loadChoiceList($this->value)->getStructuredValues();
102145
}
103146

104147
/**
105148
* {@inheritdoc}
106149
*/
107150
public function getOriginalKeys()
108151
{
109-
if (!$this->loadedList) {
110-
$this->loadedList = $this->loader->loadChoiceList($this->value);
152+
if ($this->loaded) {
153+
// Check whether the loader has the same cache
154+
if ($this->loadedList !== $this->loader->loadChoiceList($this->value)) {
155+
@trigger_error(sprintf('Caching the choice list in %s is deprecated since 3.1 and will not happen in 4.0. Cache the list in the %s instead.', __CLASS__, ChoiceLoaderInterface::class));
156+
}
157+
158+
return $this->loadedList->getOriginalKeys();
111159
}
112160

161+
// BC
162+
$this->loadedList = $this->loader->loadChoiceList($this->value);
163+
$this->loaded = true;
164+
113165
return $this->loadedList->getOriginalKeys();
166+
// In 4.0 keep the following line only:
167+
// return $this->loader->loadChoiceList($this->value)->getOriginalKeys();
114168
}
115169

116170
/**
117171
* {@inheritdoc}
118172
*/
119173
public function getChoicesForValues(array $values)
120174
{
121-
if (!$this->loadedList) {
122-
return $this->loader->loadChoicesForValues($values, $this->value);
175+
if ($this->loaded) {
176+
// Check whether the loader has the same cache
177+
if ($this->loadedList !== $this->loader->loadChoiceList($this->value)) {
178+
@trigger_error(sprintf('Caching the choice list in %s is deprecated since 3.1 and will not happen in 4.0. Cache the list in the %s instead.', __CLASS__, ChoiceLoaderInterface::class));
179+
}
180+
181+
return $this->loadedList->getChoicesForValues($values);
123182
}
124183

125-
return $this->loadedList->getChoicesForValues($values);
184+
return $this->loader->loadChoicesForValues($values, $this->value);
126185
}
127186

128187
/**
129188
* {@inheritdoc}
130189
*/
131190
public function getValuesForChoices(array $choices)
132191
{
133-
if (!$this->loadedList) {
134-
return $this->loader->loadValuesForChoices($choices, $this->value);
192+
if ($this->loaded) {
193+
// Check whether the loader has the same cache
194+
if ($this->loadedList !== $this->loader->loadChoiceList($this->value)) {
195+
@trigger_error(sprintf('Caching the choice list in %s is deprecated since 3.1 and will not happen in 4.0. Cache the list in the %s instead.', __CLASS__, ChoiceLoaderInterface::class));
196+
}
197+
198+
return $this->loadedList->getValuesForChoices($choices);
135199
}
136200

137-
return $this->loadedList->getValuesForChoices($choices);
201+
return $this->loader->loadValuesForChoices($choices, $this->value);
138202
}
139203
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/ChoiceList/LazyChoiceListTest.php
+58-24Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

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

14+
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
1415
use Symfony\Component\Form\ChoiceList\LazyChoiceList;
1516

1617
/**
@@ -26,7 +27,7 @@ class LazyChoiceListTest extends \PHPUnit_Framework_TestCase
2627
/**
2728
* @var \PHPUnit_Framework_MockObject_MockObject
2829
*/
29-
private $innerList;
30+
private $loadedList;
3031

3132
/**
3233
* @var \PHPUnit_Framework_MockObject_MockObject
@@ -37,65 +38,93 @@ class LazyChoiceListTest extends \PHPUnit_Framework_TestCase
3738

3839
protected function setUp()
3940
{
40-
$this->innerList = $this->getMock('Symfony\Component\Form\ChoiceList\ChoiceListInterface');
41+
$this->loadedList = $this->getMock('Symfony\Component\Form\ChoiceList\ChoiceListInterface');
4142
$this->loader = $this->getMock('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface');
4243
$this->value = function () {};
4344
$this->list = new LazyChoiceList($this->loader, $this->value);
4445
}
4546

46-
public function testGetChoicesLoadsInnerListOnFirstCall()
47+
public function testGetChoiceLoadersLoadsLoadedListOnFirstCall()
4748
{
48-
$this->loader->expects($this->once())
49+
$this->loader->expects($this->exactly(2))
50+
->method('loadChoiceList')
51+
->with($this->value)
52+
->will($this->returnValue($this->loadedList));
53+
54+
// The same list is returned by the loader
55+
$this->loadedList->expects($this->exactly(2))
56+
->method('getChoices')
57+
->will($this->returnValue('RESULT'));
58+
59+
$this->assertSame('RESULT', $this->list->getChoices());
60+
$this->assertSame('RESULT', $this->list->getChoices());
61+
}
62+
63+
/**
64+
* @group legacy
65+
*/
66+
public function testGetChoicesUsesLoadedListWhenLoaderDoesNotCacheChoiceListOnFirstCall()
67+
{
68+
$this->loader->expects($this->at(0))
69+
->method('loadChoiceList')
70+
->with($this->value)
71+
->willReturn($this->loadedList);
72+
73+
$this->loader->expects($this->at(1))
4974
->method('loadChoiceList')
5075
->with($this->value)
51-
->will($this->returnValue($this->innerList));
76+
->willReturn(new ArrayChoiceList(array('a', 'b')));
5277

53-
$this->innerList->expects($this->exactly(2))
78+
// The same list is returned by the lazy choice list
79+
$this->loadedList->expects($this->exactly(2))
5480
->method('getChoices')
5581
->will($this->returnValue('RESULT'));
5682

5783
$this->assertSame('RESULT', $this->list->getChoices());
5884
$this->assertSame('RESULT', $this->list->getChoices());
5985
}
6086

61-
public function testGetValuesLoadsInnerListOnFirstCall()
87+
public function testGetValuesLoadsLoadedListOnFirstCall()
6288
{
63-
$this->loader->expects($this->once())
89+
$this->loader->expects($this->exactly(2))
6490
->method('loadChoiceList')
6591
->with($this->value)
66-
->will($this->returnValue($this->innerList));
92+
->will($this->returnValue($this->loadedList));
6793

68-
$this->innerList->expects($this->exactly(2))
94+
// The same list is returned by the loader
95+
$this->loadedList->expects($this->exactly(2))
6996
->method('getValues')
7097
->will($this->returnValue('RESULT'));
7198

7299
$this->assertSame('RESULT', $this->list->getValues());
73100
$this->assertSame('RESULT', $this->list->getValues());
74101
}
75102

76-
public function testGetStructuredValuesLoadsInnerListOnFirstCall()
103+
public function testGetStructuredValuesLoadsLoadedListOnFirstCall()
77104
{
78-
$this->loader->expects($this->once())
105+
$this->loader->expects($this->exactly(2))
79106
->method('loadChoiceList')
80107
->with($this->value)
81-
->will($this->returnValue($this->innerList));
108+
->will($this->returnValue($this->loadedList));
82109

83-
$this->innerList->expects($this->exactly(2))
110+
// The same list is returned by the loader
111+
$this->loadedList->expects($this->exactly(2))
84112
->method('getStructuredValues')
85113
->will($this->returnValue('RESULT'));
86114

87115
$this->assertSame('RESULT', $this->list->getStructuredValues());
88116
$this->assertSame('RESULT', $this->list->getStructuredValues());
89117
}
90118

91-
public function testGetOriginalKeysLoadsInnerListOnFirstCall()
119+
public function testGetOriginalKeysLoadsLoadedListOnFirstCall()
92120
{
93-
$this->loader->expects($this->once())
121+
$this->loader->expects($this->exactly(2))
94122
->method('loadChoiceList')
95123
->with($this->value)
96-
->will($this->returnValue($this->innerList));
124+
->will($this->returnValue($this->loadedList));
97125

98-
$this->innerList->expects($this->exactly(2))
126+
// The same list is returned by the loader
127+
$this->loadedList->expects($this->exactly(2))
99128
->method('getOriginalKeys')
100129
->will($this->returnValue('RESULT'));
101130

@@ -116,15 +145,17 @@ public function testGetChoicesForValuesForwardsCallIfListNotLoaded()
116145

117146
public function testGetChoicesForValuesUsesLoadedList()
118147
{
119-
$this->loader->expects($this->once())
148+
$this->loader->expects($this->exactly(3))
120149
->method('loadChoiceList')
121150
->with($this->value)
122-
->will($this->returnValue($this->innerList));
151+
// For BC, the same choice loaded list is returned 3 times
152+
// It should only twice in 4.0
153+
->will($this->returnValue($this->loadedList));
123154

124155
$this->loader->expects($this->never())
125156
->method('loadChoicesForValues');
126157

127-
$this->innerList->expects($this->exactly(2))
158+
$this->loadedList->expects($this->exactly(2))
128159
->method('getChoicesForValues')
129160
->with(array('a', 'b'))
130161
->will($this->returnValue('RESULT'));
@@ -136,6 +167,7 @@ public function testGetChoicesForValuesUsesLoadedList()
136167
$this->assertSame('RESULT', $this->list->getChoicesForValues(array('a', 'b')));
137168
}
138169

170+
// To be remove in 4.0
139171
public function testGetValuesForChoicesForwardsCallIfListNotLoaded()
140172
{
141173
$this->loader->expects($this->exactly(2))
@@ -149,15 +181,17 @@ public function testGetValuesForChoicesForwardsCallIfListNotLoaded()
149181

150182
public function testGetValuesForChoicesUsesLoadedList()
151183
{
152-
$this->loader->expects($this->once())
184+
$this->loader->expects($this->exactly(3))
153185
->method('loadChoiceList')
154186
->with($this->value)
155-
->will($this->returnValue($this->innerList));
187+
// For BC, the same choice loaded list is returned 3 times
188+
// It should only twice in 4.0
189+
->will($this->returnValue($this->loadedList));
156190

157191
$this->loader->expects($this->never())
158192
->method('loadValuesForChoices');
159193

160-
$this->innerList->expects($this->exactly(2))
194+
$this->loadedList->expects($this->exactly(2))
161195
->method('getValuesForChoices')
162196
->with(array('a', 'b'))
163197
->will($this->returnValue('RESULT'));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.