[Form] Fixed new ArrayChoiceList to compare choices by their values, if enabled
This commit is contained in:
Bernhard Schussek
parent
e6739bf05e
commit
a289deb973
@ -11,7 +11,7 @@
|
||||
|
||||
namespace Symfony\Component\Form\ChoiceList;
|
||||
|
||||
use Symfony\Component\Form\Exception\InvalidArgumentException;
|
||||
use Symfony\Component\Form\Exception\UnexpectedTypeException;
|
||||
|
||||
/**
|
||||
* A list of choices with arbitrary data types.
|
||||
@ -39,33 +39,46 @@ class ArrayChoiceList implements ChoiceListInterface
|
||||
*/
|
||||
protected $values = array();
|
||||
|
||||
/**
|
||||
* The callback for creating the value for a choice.
|
||||
*
|
||||
* @var callable
|
||||
*/
|
||||
protected $valueCallback;
|
||||
|
||||
/**
|
||||
* Creates a list with the given choices and values.
|
||||
*
|
||||
* The given choice array must have the same array keys as the value array.
|
||||
*
|
||||
* @param array $choices The selectable choices
|
||||
* @param string[] $values The string values of the choices
|
||||
*
|
||||
* @throws InvalidArgumentException If the keys of the choices don't match
|
||||
* the keys of the values
|
||||
* @param array $choices The selectable choices
|
||||
* @param callable $value The callable for creating the value for a
|
||||
* choice. If `null` is passed, incrementing
|
||||
* integers are used as values
|
||||
* @param bool $compareByValue Whether to use the value callback to
|
||||
* compare choices. If `null`, choices are
|
||||
* compared by identity
|
||||
*/
|
||||
public function __construct(array $choices, array $values)
|
||||
public function __construct(array $choices, $value = null, $compareByValue = false)
|
||||
{
|
||||
$choiceKeys = array_keys($choices);
|
||||
$valueKeys = array_keys($values);
|
||||
|
||||
if ($choiceKeys !== $valueKeys) {
|
||||
throw new InvalidArgumentException(sprintf(
|
||||
'The keys of the choices and the values must match. The choice '.
|
||||
'keys are: "%s". The value keys are: "%s".',
|
||||
implode('", "', $choiceKeys),
|
||||
implode('", "', $valueKeys)
|
||||
));
|
||||
if (null !== $value && !is_callable($value)) {
|
||||
throw new UnexpectedTypeException($value, 'null or callable');
|
||||
}
|
||||
|
||||
$this->choices = $choices;
|
||||
$this->values = array_map('strval', $values);
|
||||
$this->values = array();
|
||||
$this->valueCallback = $compareByValue ? $value : null;
|
||||
|
||||
if (null === $value) {
|
||||
$i = 0;
|
||||
foreach ($this->choices as $key => $choice) {
|
||||
$this->values[$key] = (string) $i++;
|
||||
}
|
||||
} else {
|
||||
foreach ($choices as $key => $choice) {
|
||||
$this->values[$key] = (string) call_user_func($value, $choice, $key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -116,6 +129,17 @@ class ArrayChoiceList implements ChoiceListInterface
|
||||
{
|
||||
$values = array();
|
||||
|
||||
// Use the value callback to compare choices by their values, if present
|
||||
if ($this->valueCallback) {
|
||||
$givenValues = array();
|
||||
foreach ($choices as $key => $choice) {
|
||||
$givenValues[$key] = (string) call_user_func($this->valueCallback, $choice, $key);
|
||||
}
|
||||
|
||||
return array_intersect($givenValues, $this->values);
|
||||
}
|
||||
|
||||
// Otherwise compare choices by identity
|
||||
foreach ($choices as $i => $givenChoice) {
|
||||
foreach ($this->choices as $j => $choice) {
|
||||
if ($choice !== $givenChoice) {
|
||||
|
||||
@ -43,21 +43,14 @@ use Symfony\Component\Form\Exception\InvalidArgumentException;
|
||||
* @deprecated Added for backwards compatibility in Symfony 2.7, to be removed
|
||||
* in Symfony 3.0.
|
||||
*/
|
||||
class ArrayKeyChoiceList implements ChoiceListInterface
|
||||
class ArrayKeyChoiceList extends ArrayChoiceList
|
||||
{
|
||||
/**
|
||||
* The selectable choices.
|
||||
* Whether the choices are used as values.
|
||||
*
|
||||
* @var array
|
||||
* @var bool
|
||||
*/
|
||||
private $choices = array();
|
||||
|
||||
/**
|
||||
* The values of the choices.
|
||||
*
|
||||
* @var string[]
|
||||
*/
|
||||
private $values = array();
|
||||
private $useChoicesAsValues = false;
|
||||
|
||||
/**
|
||||
* Casts the given choice to an array key.
|
||||
@ -100,51 +93,26 @@ class ArrayKeyChoiceList implements ChoiceListInterface
|
||||
* values.
|
||||
*
|
||||
* @param array $choices The selectable choices
|
||||
* @param string[] $values Optional. The string values of the choices
|
||||
* @param callable $value The callable for creating the value for a
|
||||
* choice. If `null` is passed, the choices are
|
||||
* cast to strings and used as values
|
||||
*
|
||||
* @throws InvalidArgumentException If the keys of the choices don't match
|
||||
* the keys of the values or if any of the
|
||||
* choices is not scalar
|
||||
*/
|
||||
public function __construct(array $choices, array $values = array())
|
||||
public function __construct(array $choices, $value = null)
|
||||
{
|
||||
if (empty($values)) {
|
||||
// The cast to strings happens later
|
||||
$values = $choices;
|
||||
} else {
|
||||
$choiceKeys = array_keys($choices);
|
||||
$valueKeys = array_keys($values);
|
||||
$choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
|
||||
|
||||
if ($choiceKeys !== $valueKeys) {
|
||||
throw new InvalidArgumentException(
|
||||
sprintf(
|
||||
'The keys of the choices and the values must match. The choice '.
|
||||
'keys are: "%s". The value keys are: "%s".',
|
||||
implode('", "', $choiceKeys),
|
||||
implode('", "', $valueKeys)
|
||||
)
|
||||
);
|
||||
}
|
||||
if (null === $value) {
|
||||
$value = function ($choice) {
|
||||
return (string) $choice;
|
||||
};
|
||||
$this->useChoicesAsValues = true;
|
||||
}
|
||||
|
||||
$this->choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
|
||||
$this->values = array_map('strval', $values);
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function getChoices()
|
||||
{
|
||||
return $this->choices;
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function getValues()
|
||||
{
|
||||
return $this->values;
|
||||
parent::__construct($choices, $value);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -152,11 +120,15 @@ class ArrayKeyChoiceList implements ChoiceListInterface
|
||||
*/
|
||||
public function getChoicesForValues(array $values)
|
||||
{
|
||||
$values = array_map('strval', $values);
|
||||
if ($this->useChoicesAsValues) {
|
||||
$values = array_map('strval', $values);
|
||||
|
||||
// The values are identical to the choices, so we can just return them
|
||||
// to improve performance a little bit
|
||||
return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, $this->values));
|
||||
// If the values are identical to the choices, so we can just return
|
||||
// them to improve performance a little bit
|
||||
return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, $this->values));
|
||||
}
|
||||
|
||||
return parent::getChoicesForValues($values);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -166,8 +138,12 @@ class ArrayKeyChoiceList implements ChoiceListInterface
|
||||
{
|
||||
$choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
|
||||
|
||||
// The choices are identical to the values, so we can just return them
|
||||
// to improve performance a little bit
|
||||
return array_map('strval', array_intersect($choices, $this->choices));
|
||||
if ($this->useChoicesAsValues) {
|
||||
// If the choices are identical to the values, we can just return
|
||||
// them to improve performance a little bit
|
||||
return array_map('strval', array_intersect($choices, $this->choices));
|
||||
}
|
||||
|
||||
return parent::getValuesForChoices($choices);
|
||||
}
|
||||
}
|
||||
|
||||
@ -89,10 +89,6 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
|
||||
throw new UnexpectedTypeException($choices, 'array or \Traversable');
|
||||
}
|
||||
|
||||
if (null !== $value && !is_callable($value)) {
|
||||
throw new UnexpectedTypeException($value, 'null or callable');
|
||||
}
|
||||
|
||||
if ($choices instanceof \Traversable) {
|
||||
$choices = iterator_to_array($choices);
|
||||
}
|
||||
@ -102,29 +98,7 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
|
||||
// in the view only.
|
||||
self::flatten($choices, $flatChoices);
|
||||
|
||||
// If no values are given, use incrementing integers as values
|
||||
// We can not use the choices themselves, because we don't know whether
|
||||
// choices can be converted to (duplicate-free) strings
|
||||
if (null === $value) {
|
||||
$values = $flatChoices;
|
||||
$i = 0;
|
||||
|
||||
foreach ($values as $key => $value) {
|
||||
$values[$key] = (string) $i++;
|
||||
}
|
||||
|
||||
return new ArrayChoiceList($flatChoices, $values);
|
||||
}
|
||||
|
||||
// Can't use array_map(), because array_map() doesn't pass the key
|
||||
// Can't use array_walk(), which ignores the return value of the
|
||||
// closure
|
||||
$values = array();
|
||||
foreach ($flatChoices as $key => $choice) {
|
||||
$values[$key] = call_user_func($value, $choice, $key);
|
||||
}
|
||||
|
||||
return new ArrayChoiceList($flatChoices, $values);
|
||||
return new ArrayChoiceList($flatChoices, $value);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -139,10 +113,6 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
|
||||
throw new UnexpectedTypeException($choices, 'array or \Traversable');
|
||||
}
|
||||
|
||||
if (null !== $value && !is_callable($value)) {
|
||||
throw new UnexpectedTypeException($value, 'null or callable');
|
||||
}
|
||||
|
||||
if ($choices instanceof \Traversable) {
|
||||
$choices = iterator_to_array($choices);
|
||||
}
|
||||
@ -157,20 +127,12 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
|
||||
// strings or integers, we are guaranteed to be able to convert them
|
||||
// to strings
|
||||
if (null === $value) {
|
||||
$values = array_map('strval', $flatChoices);
|
||||
|
||||
return new ArrayKeyChoiceList($flatChoices, $values);
|
||||
$value = function ($choice) {
|
||||
return (string) $choice;
|
||||
};
|
||||
}
|
||||
|
||||
// Can't use array_map(), because array_map() doesn't pass the key
|
||||
// Can't use array_walk(), which ignores the return value of the
|
||||
// closure
|
||||
$values = array();
|
||||
foreach ($flatChoices as $key => $choice) {
|
||||
$values[$key] = call_user_func($value, $choice, $key);
|
||||
}
|
||||
|
||||
return new ArrayKeyChoiceList($flatChoices, $values);
|
||||
return new ArrayKeyChoiceList($flatChoices, $value);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -29,7 +29,9 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
|
||||
|
||||
protected function createChoiceList()
|
||||
{
|
||||
return new ArrayChoiceList($this->getChoices(), $this->getValues());
|
||||
$i = 0;
|
||||
|
||||
return new ArrayChoiceList($this->getChoices());
|
||||
}
|
||||
|
||||
protected function getChoices()
|
||||
@ -49,4 +51,45 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
|
||||
{
|
||||
new ArrayChoiceList(array(0 => 'a', 1 => 'b'), array(1 => 'a', 2 => 'b'));
|
||||
}
|
||||
|
||||
public function testCreateChoiceListWithValueCallback()
|
||||
{
|
||||
$callback = function ($choice, $key) {
|
||||
return $key.':'.$choice;
|
||||
};
|
||||
|
||||
$choiceList = new ArrayChoiceList(array(2 => 'foo', 7 => 'bar', 10 => 'baz'), $callback);
|
||||
|
||||
$this->assertSame(array(2 => '2:foo', 7 => '7:bar', 10 => '10:baz'), $choiceList->getValues());
|
||||
$this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz')));
|
||||
$this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
|
||||
}
|
||||
|
||||
public function testCompareChoicesByIdentityByDefault()
|
||||
{
|
||||
$callback = function ($choice) {
|
||||
return $choice->value;
|
||||
};
|
||||
|
||||
$obj1 = (object) array('value' => 'value1');
|
||||
$obj2 = (object) array('value' => 'value2');
|
||||
|
||||
$choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback);
|
||||
$this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2)));
|
||||
$this->assertSame(array(), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2'))));
|
||||
}
|
||||
|
||||
public function testCompareChoicesWithValueCallbackIfCompareByValue()
|
||||
{
|
||||
$callback = function ($choice) {
|
||||
return $choice->value;
|
||||
};
|
||||
|
||||
$obj1 = (object) array('value' => 'value1');
|
||||
$obj2 = (object) array('value' => 'value2');
|
||||
|
||||
$choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback, true);
|
||||
$this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2)));
|
||||
$this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2'))));
|
||||
}
|
||||
}
|
||||
|
||||
@ -29,7 +29,7 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
|
||||
|
||||
protected function createChoiceList()
|
||||
{
|
||||
return new ArrayKeyChoiceList($this->getChoices(), $this->getValues());
|
||||
return new ArrayKeyChoiceList($this->getChoices());
|
||||
}
|
||||
|
||||
protected function getChoices()
|
||||
@ -42,14 +42,6 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
|
||||
return array('0', '1', 'a', 'b', '');
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\Form\Exception\InvalidArgumentException
|
||||
*/
|
||||
public function testFailIfKeyMismatch()
|
||||
{
|
||||
new ArrayKeyChoiceList(array(0 => 'a', 1 => 'b'), array(1 => 'a', 2 => 'b'));
|
||||
}
|
||||
|
||||
public function testUseChoicesAsValuesByDefault()
|
||||
{
|
||||
$list = new ArrayKeyChoiceList(array(1 => '', 3 => 0, 7 => '1', 10 => 1.23));
|
||||
@ -102,7 +94,7 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
|
||||
*/
|
||||
public function testConvertChoicesIfNecessary(array $choices, array $converted)
|
||||
{
|
||||
$list = new ArrayKeyChoiceList($choices, range(0, count($choices) - 1));
|
||||
$list = new ArrayKeyChoiceList($choices);
|
||||
|
||||
$this->assertSame($converted, $list->getChoices());
|
||||
}
|
||||
@ -134,7 +126,7 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
|
||||
*/
|
||||
public function testFailIfInvalidChoices(array $choices)
|
||||
{
|
||||
new ArrayKeyChoiceList($choices, range(0, count($choices) - 1));
|
||||
new ArrayKeyChoiceList($choices);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -157,31 +149,48 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
|
||||
/**
|
||||
* @dataProvider provideConvertibleValues
|
||||
*/
|
||||
public function testConvertValuesToStrings(array $values, array $converted)
|
||||
public function testConvertValuesToStrings($value, $converted)
|
||||
{
|
||||
$list = new ArrayKeyChoiceList(range(0, count($values) - 1), $values);
|
||||
$callback = function () use ($value) {
|
||||
return $value;
|
||||
};
|
||||
|
||||
$this->assertSame($converted, $list->getValues());
|
||||
$list = new ArrayKeyChoiceList(array('choice'), $callback);
|
||||
|
||||
$this->assertSame(array($converted), $list->getValues());
|
||||
}
|
||||
|
||||
public function provideConvertibleValues()
|
||||
{
|
||||
return array(
|
||||
array(array(0), array('0')),
|
||||
array(array(1), array('1')),
|
||||
array(array('0'), array('0')),
|
||||
array(array('1'), array('1')),
|
||||
array(array('1.23'), array('1.23')),
|
||||
array(array('foobar'), array('foobar')),
|
||||
array(0, '0'),
|
||||
array(1, '1'),
|
||||
array('0', '0'),
|
||||
array('1', '1'),
|
||||
array('1.23', '1.23'),
|
||||
array('foobar', 'foobar'),
|
||||
// The default value of choice fields is NULL. It should be treated
|
||||
// like the empty value for this choice list type
|
||||
array(array(null), array('')),
|
||||
array(array(1.23), array('1.23')),
|
||||
array(null, ''),
|
||||
array(1.23, '1.23'),
|
||||
// Always cast booleans to 0 and 1, because:
|
||||
// array(true => 'Yes', false => 'No') === array(1 => 'Yes', 0 => 'No')
|
||||
// see ChoiceTypeTest::testSetDataSingleNonExpandedAcceptsBoolean
|
||||
array(array(true), array('1')),
|
||||
array(array(false), array('')),
|
||||
array(true, '1'),
|
||||
array(false, ''),
|
||||
);
|
||||
}
|
||||
|
||||
public function testCreateChoiceListWithValueCallback()
|
||||
{
|
||||
$callback = function ($choice, $key) {
|
||||
return $key.':'.$choice;
|
||||
};
|
||||
|
||||
$choiceList = new ArrayKeyChoiceList(array(2 => 'foo', 7 => 'bar', 10 => 'baz'), $callback);
|
||||
|
||||
$this->assertSame(array(2 => '2:foo', 7 => '7:bar', 10 => '10:baz'), $choiceList->getValues());
|
||||
$this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz')));
|
||||
$this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
|
||||
}
|
||||
}
|
||||
|
||||
@ -83,8 +83,7 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
|
||||
$this->obj3 = (object) array('label' => 'C', 'index' => 'y', 'value' => 1, 'preferred' => true, 'group' => 'Group 2', 'attr' => array('attr2' => 'value2'));
|
||||
$this->obj4 = (object) array('label' => 'D', 'index' => 'z', 'value' => 2, 'preferred' => false, 'group' => 'Group 2', 'attr' => array());
|
||||
$this->list = new ArrayChoiceList(
|
||||
array('A' => $this->obj1, 'B' => $this->obj2, 'C' => $this->obj3, 'D' => $this->obj4),
|
||||
array('A' => '0', 'B' => '1', 'C' => '2', 'D' => '3')
|
||||
array('A' => $this->obj1, 'B' => $this->obj2, 'C' => $this->obj3, 'D' => $this->obj4)
|
||||
);
|
||||
$this->factory = new DefaultChoiceListFactory();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user