8000 bug #15623 Fix the retrieval of the value with property path when usi… · symfony/symfony@c9927bf · GitHub
[go: up one dir, main page]
Skip to content

Commit c9927bf

Browse files
TobionTobion
committed
bug #15623 Fix the retrieval of the value with property path when using a loader (stof)
This PR was merged into the 2.7 branch. Discussion ---------- Fix the retrieval of the value with property path when using a loader | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a the ChoiceType is transforming the form data to its value in case of non-expanded fields. When creating the choice list from choices, this works fine when using a property path for the choice_value, because it has a safeguard against values which are not object or arrays (a typical case being ``null`` because nothing is selected yet). However, when loading from a ChoiceLoader, the generated closure was missing the same safeguard, breaking the usage of ChoiceLoader with property path. This went unnoticed because the only usage of choice_loader in core is for the doctrine type, and this one uses its own logic to get the choice value I added test on the PropertyAccessDecorator directly, to have fast tests. We could imagine adding tests in the ChoiceTypeTest using a ChoiceLoader too, matching all existing tests for other ways to specify arguments, but I'm not sure it is worth it. Commits ------- 5df64dc Fix the retrieval of the value with property path when using a loader
2 parents fac7f21 + 5df64dc commit c9927bf

File tree

2 files changed

+37
-1
lines changed

2 files changed

+37
-1
lines changed

‎src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,13 @@ public function createListFromLoader(ChoiceLoaderInterface $loader, $value = nul
141141
if ($value instanceof PropertyPath) {
142142
$accessor = $this->propertyAccessor;
143143
$value = function ($choice) use ($accessor, $value) {
144-
return $accessor->getValue($choice, $value);
144+
// The callable may be invoked with a non-object/array value
145+
// when such values are passed to
146+
// ChoiceListInterface::getValuesForChoices(). Handle this case
147+
// so that the call to getValue() doesn't break.
148+
if (is_object($choice) || is_array($choice)) {
149+
return $accessor->getValue($choice, $value);
150+
}
145151
};
146152
}
147153

‎src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,36 @@ public function testCreateFromLoaderPropertyPath()
9393
$this->assertSame('value', $this->factory->createListFromLoader($loader, 'property'));
9494
}
9595

96+
// https://github.com/symfony/symfony/issues/5494
97+
public function testCreateFromChoicesAssumeNullIfValuePropertyPathUnreadable()
98+
{
99+
$choices = array(null);
100+
101+
$this->decoratedFactory->expects($this->once())
102+
->method('createListFromChoices')
103+
->with($choices, $this->isInstanceOf('\Closure'))
104+
->will($this->returnCallback(function ($choices, $callback) {
105+
return array_map($callback, $choices);
106+
}));
107+
108+
$this->assertSame(array(null), $this->factory->createListFromChoices($choices, 'property'));
109+
}
110+
111+
// https://github.com/symfony/symfony/issues/5494
112+
public function testCreateFromChoiceLoaderAssumeNullIfValuePropertyPathUnreadable()
113+
{
114+
$loader = $this->getMock('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface');
115+
116+
$this->decoratedFactory->expects($this->once())
117+
->method('createListFromLoader')
118+
->with($loader, $this->isInstanceOf('\Closure'))
119+
->will($this->returnCallback(function ($loader, $callback) {
120+
return $callback(null);
121+
}));
122+
123+
$this->assertNull($this->factory->createListFromLoader($loader, 'property'));
124+
}
125+
96126
public function testCreateFromLoaderPropertyPathInstance()
97127
{
98128
$loader = $this->getMock('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface');

0 commit comments

Comments
 (0)
0