merged branch canni/fix_collection_validator (PR #3078)
Commits -------7f7c2a7
Add prof-of-concept test, this test will fail without changes in previous commit253eeba
[BugFix][Validator] Fix for PHP incosistent behaviour of ArrayAccess Discussion ---------- [BugFix][Validator] Fix for PHP incosistent behaviour of ArrayAccess Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #2779 Todo: - [![Build Status](https://secure.travis-ci.org/canni/symfony.png)](http://travis-ci.org/canni/symfony) Because PHP function `array_key_exists` is buggy, it works great with native PHP `ArrayObject` instances, but hand written implementations of `ArrayAccess` and `Traversable` objects will fail to work with `CollectionValidator` Tests from second commit are valid use cases, but without this change, they will fail.
This commit is contained in:
commit
c185302c0d
@ -52,7 +52,11 @@ class CollectionValidator extends ConstraintValidator
|
|||||||
}
|
}
|
||||||
|
|
||||||
foreach ($constraint->fields as $field => $constraints) {
|
foreach ($constraint->fields as $field => $constraints) {
|
||||||
if (array_key_exists($field, $value)) {
|
if (
|
||||||
|
// bug fix issue #2779
|
||||||
|
(is_array($value) && array_key_exists($field, $value)) ||
|
||||||
|
($value instanceof \ArrayAccess && $value->offsetExists($field))
|
||||||
|
) {
|
||||||
// cannot simply cast to array, because then the object is converted to an
|
// cannot simply cast to array, because then the object is converted to an
|
||||||
// array instead of wrapped inside
|
// array instead of wrapped inside
|
||||||
$constraints = is_array($constraints) ? $constraints : array($constraints);
|
$constraints = is_array($constraints) ? $constraints : array($constraints);
|
||||||
|
@ -13,9 +13,71 @@ namespace Symfony\Tests\Component\Validator\Constraints;
|
|||||||
|
|
||||||
use Symfony\Component\Validator\ExecutionContext;
|
use Symfony\Component\Validator\ExecutionContext;
|
||||||
use Symfony\Component\Validator\Constraints\Min;
|
use Symfony\Component\Validator\Constraints\Min;
|
||||||
|
use Symfony\Component\Validator\Constraints\NotBlank;
|
||||||
|
use Symfony\Component\Validator\Constraints\NotNull;
|
||||||
use Symfony\Component\Validator\Constraints\Collection;
|
use Symfony\Component\Validator\Constraints\Collection;
|
||||||
use Symfony\Component\Validator\Constraints\CollectionValidator;
|
use Symfony\Component\Validator\Constraints\CollectionValidator;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* This class is a hand written simplified version of PHP native `ArrayObject`
|
||||||
|
* class, to show that it behaves different than PHP native implementation.
|
||||||
|
*/
|
||||||
|
class TestArrayObject implements \ArrayAccess, \IteratorAggregate, \Countable, \Serializable
|
||||||
|
{
|
||||||
|
private $array;
|
||||||
|
|
||||||
|
public function __construct(array $array = null)
|
||||||
|
{
|
||||||
|
$this->array = (array) ($array ?: array());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function offsetExists($offset)
|
||||||
|
{
|
||||||
|
return array_key_exists($offset, $this->array);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function offsetGet($offset)
|
||||||
|
{
|
||||||
|
return $this->array[$offset];
|
||||||
|
}
|
||||||
|
|
||||||
|
public function offsetSet($offset, $value)
|
||||||
|
{
|
||||||
|
if (null === $offset) {
|
||||||
|
$this->array[] = $value;
|
||||||
|
} else {
|
||||||
|
$this->array[$offset] = $value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public function offsetUnset($offset)
|
||||||
|
{
|
||||||
|
if (array_key_exists($offset, $this->array)) {
|
||||||
|
unset($this->array[$offset]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getIterator()
|
||||||
|
{
|
||||||
|
return new \ArrayIterator($this->array);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function count()
|
||||||
|
{
|
||||||
|
return count($this->array);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function serialize()
|
||||||
|
{
|
||||||
|
return serialize($this->array);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function unserialize($serialized)
|
||||||
|
{
|
||||||
|
$this->array = (array) unserialize((string) $serialized);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
||||||
{
|
{
|
||||||
protected $validator;
|
protected $validator;
|
||||||
@ -49,15 +111,22 @@ class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
|||||||
|
|
||||||
public function testFieldsAsDefaultOption()
|
public function testFieldsAsDefaultOption()
|
||||||
{
|
{
|
||||||
$this->validator->isValid(array('foo' => 'foobar'), new Collection(array(
|
$this->assertTrue($this->validator->isValid(array('foo' => 'foobar'), new Collection(array(
|
||||||
'foo' => new Min(4),
|
'foo' => new Min(4),
|
||||||
)));
|
))));
|
||||||
|
$this->assertTrue($this->validator->isValid(new \ArrayObject(array('foo' => 'foobar')), new Collection(array(
|
||||||
|
'foo' => new Min(4),
|
||||||
|
))));
|
||||||
|
$this->assertTrue($this->validator->isValid(new TestArrayObject(array('foo' => 'foobar')), new Collection(array(
|
||||||
|
'foo' => new Min(4),
|
||||||
|
))));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException Symfony\Component\Validator\Exception\UnexpectedTypeException
|
||||||
|
*/
|
||||||
public function testThrowsExceptionIfNotTraversable()
|
public function testThrowsExceptionIfNotTraversable()
|
||||||
{
|
{
|
||||||
$this->setExpectedException('Symfony\Component\Validator\Exception\UnexpectedTypeException');
|
|
||||||
|
|
||||||
$this->validator->isValid('foobar', new Collection(array('fields' => array(
|
$this->validator->isValid('foobar', new Collection(array('fields' => array(
|
||||||
'foo' => new Min(4),
|
'foo' => new Min(4),
|
||||||
))));
|
))));
|
||||||
@ -112,13 +181,11 @@ class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
|||||||
))));
|
))));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testExtraFieldsDisallowed()
|
/**
|
||||||
|
* @dataProvider getArgumentsWithExtraFields
|
||||||
|
*/
|
||||||
|
public function testExtraFieldsDisallowed($array)
|
||||||
{
|
{
|
||||||
$array = array(
|
|
||||||
'foo' => 5,
|
|
||||||
'bar' => 6,
|
|
||||||
);
|
|
||||||
|
|
||||||
$this->assertFalse($this->validator->isValid($array, new Collection(array(
|
$this->assertFalse($this->validator->isValid($array, new Collection(array(
|
||||||
'fields' => array(
|
'fields' => array(
|
||||||
'foo' => new Min(4),
|
'foo' => new Min(4),
|
||||||
@ -132,12 +199,15 @@ class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
|||||||
$array = array(
|
$array = array(
|
||||||
'foo' => null,
|
'foo' => null,
|
||||||
);
|
);
|
||||||
|
$collection = new Collection(array(
|
||||||
$this->assertTrue($this->validator->isValid($array, new Collection(array(
|
|
||||||
'fields' => array(
|
'fields' => array(
|
||||||
'foo' => new Min(4),
|
'foo' => new Min(4),
|
||||||
),
|
),
|
||||||
))));
|
));
|
||||||
|
|
||||||
|
$this->assertTrue($this->validator->isValid($array, $collection));
|
||||||
|
$this->assertTrue($this->validator->isValid(new \ArrayObject($array), $collection));
|
||||||
|
$this->assertTrue($this->validator->isValid(new TestArrayObject($array), $collection));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testExtraFieldsAllowed()
|
public function testExtraFieldsAllowed()
|
||||||
@ -146,13 +216,16 @@ class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
|||||||
'foo' => 5,
|
'foo' => 5,
|
||||||
'bar' => 6,
|
'bar' => 6,
|
||||||
);
|
);
|
||||||
|
$collection = new Collection(array(
|
||||||
$this->assertTrue($this->validator->isValid($array, new Collection(array(
|
|
||||||
'fields' => array(
|
'fields' => array(
|
||||||
'foo' => new Min(4),
|
'foo' => new Min(4),
|
||||||
),
|
),
|
||||||
'allowExtraFields' => true,
|
'allowExtraFields' => true,
|
||||||
))));
|
));
|
||||||
|
|
||||||
|
$this->assertTrue($this->validator->isValid($array, $collection));
|
||||||
|
$this->assertTrue($this->validator->isValid(new \ArrayObject($array), $collection));
|
||||||
|
$this->assertTrue($this->validator->isValid(new TestArrayObject($array), $collection));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testMissingFieldsDisallowed()
|
public function testMissingFieldsDisallowed()
|
||||||
@ -162,6 +235,16 @@ class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
|||||||
'foo' => new Min(4),
|
'foo' => new Min(4),
|
||||||
),
|
),
|
||||||
))));
|
))));
|
||||||
|
$this->assertFalse($this->validator->isValid(new \ArrayObject(array()), new Collection(array(
|
||||||
|
'fields' => array(
|
||||||
|
'foo' => new Min(4),
|
||||||
|
),
|
||||||
|
))));
|
||||||
|
$this->assertFalse($this->validator->isValid(new TestArrayObject(array()), new Collection(array(
|
||||||
|
'fields' => array(
|
||||||
|
'foo' => new Min(4),
|
||||||
|
),
|
||||||
|
))));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testMissingFieldsAllowed()
|
public function testMissingFieldsAllowed()
|
||||||
@ -172,16 +255,58 @@ class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
|||||||
),
|
),
|
||||||
'allowMissingFields' => true,
|
'allowMissingFields' => true,
|
||||||
))));
|
))));
|
||||||
|
$this->assertTrue($this->validator->isValid(new \ArrayObject(array()), new Collection(array(
|
||||||
|
'fields' => array(
|
||||||
|
'foo' => new Min(4),
|
||||||
|
),
|
||||||
|
'allowMissingFields' => true,
|
||||||
|
))));
|
||||||
|
$this->assertTrue($this->validator->isValid(new TestArrayObject(array()), new Collection(array(
|
||||||
|
'fields' => array(
|
||||||
|
'foo' => new Min(4),
|
||||||
|
),
|
||||||
|
'allowMissingFields' => true,
|
||||||
|
))));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getValidArguments()
|
public function testArrayAccessObject() {
|
||||||
{
|
$value = new TestArrayObject();
|
||||||
return array(
|
$value['foo'] = 12;
|
||||||
// can only test for one entry, because PHPUnits mocking does not allow
|
$value['asdf'] = 'asdfaf';
|
||||||
// to expect multiple method calls with different arguments
|
|
||||||
array(array('foo' => 3)),
|
$this->assertTrue(isset($value['asdf']));
|
||||||
array(new \ArrayObject(array('foo' => 3))),
|
$this->assertTrue(isset($value['foo']));
|
||||||
);
|
$this->assertFalse(empty($value['asdf']));
|
||||||
|
$this->assertFalse(empty($value['foo']));
|
||||||
|
|
||||||
|
$result = $this->validator->isValid($value, new Collection(array(
|
||||||
|
'fields' => array(
|
||||||
|
'foo' => new NotBlank(),
|
||||||
|
'asdf' => new NotBlank()
|
||||||
|
)
|
||||||
|
)));
|
||||||
|
|
||||||
|
$this->assertTrue($result);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testArrayObject() {
|
||||||
|
$value = new \ArrayObject(array());
|
||||||
|
$value['foo'] = 12;
|
||||||
|
$value['asdf'] = 'asdfaf';
|
||||||
|
|
||||||
|
$this->assertTrue(isset($value['asdf']));
|
||||||
|
$this->assertTrue(isset($value['foo']));
|
||||||
|
$this->assertFalse(empty($value['asdf']));
|
||||||
|
$this->assertFalse(empty($value['foo']));
|
||||||
|
|
||||||
|
$result = $this->validator->isValid($value, new Collection(array(
|
||||||
|
'fields' => array(
|
||||||
|
'foo' => new NotBlank(),
|
||||||
|
'asdf' => new NotBlank()
|
||||||
|
)
|
||||||
|
)));
|
||||||
|
|
||||||
|
$this->assertTrue($result);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testObjectShouldBeLeftUnchanged()
|
public function testObjectShouldBeLeftUnchanged()
|
||||||
@ -199,4 +324,34 @@ class CollectionValidatorTest extends \PHPUnit_Framework_TestCase
|
|||||||
'foo' => 3
|
'foo' => 3
|
||||||
), (array) $value);
|
), (array) $value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getValidArguments()
|
||||||
|
{
|
||||||
|
return array(
|
||||||
|
// can only test for one entry, because PHPUnits mocking does not allow
|
||||||
|
// to expect multiple method calls with different arguments
|
||||||
|
array(array('foo' => 3)),
|
||||||
|
array(new \ArrayObject(array('foo' => 3))),
|
||||||
|
array(new TestArrayObject(array('foo' => 3))),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getArgumentsWithExtraFields()
|
||||||
|
{
|
||||||
|
return array(
|
||||||
|
array(array(
|
||||||
|
'foo' => 5,
|
||||||
|
'bar' => 6,
|
||||||
|
)),
|
||||||
|
array(new \ArrayObject(array(
|
||||||
|
'foo' => 5,
|
||||||
|
'bar' => 6,
|
||||||
|
))),
|
||||||
|
array(new TestArrayObject(array(
|
||||||
|
'foo' => 5,
|
||||||
|
'bar' => 6,
|
||||||
|
)))
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user