Skip to content

Commit

Permalink
Merge pull request #910 from eugef/patch-1
Browse files Browse the repository at this point in the history 
Fixed validation for Request parameter that is an array without default value
  • Loading branch information
@lsmith77
lsmith77 committed Dec 2, 2014
2 parents 221ceea + ec90c29 commit 6714a42
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 35 deletions.
10 changes: 5 additions & 5 deletions Request/ParamFetcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,14 @@ public function get($name, $strict = null)
$default = $config->default;
$paramType = $config instanceof QueryParam ? 'Query' : 'Request';

if ($config->array) {
$default = (array) $default;
}

if (null === $strict) {
$strict = $config->strict;
}

if ($config->array && (null !== $default || !$strict)) {
$default = (array) $default;
}

if ($config instanceof RequestParam) {
$param = $this->request->request->get($config->getKey(), $default);
} elseif ($config instanceof QueryParam) {
Expand All @@ -130,7 +130,7 @@ public function get($name, $strict = null)
if (!is_array($param)) {
if ($strict) {
throw new BadRequestHttpException(
sprintf("% parameter value of '%s' is not an array", $paramType, $name)
sprintf("%s parameter value of '%s' is not an array", $paramType, $name)
);
}

Expand Down
64 changes: 34 additions & 30 deletions Tests/Request/ParamFetcherTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ public function setup()
$annotations['biz']->nullable = true;
$annotations['biz']->description = 'A scalar param with an explicitly defined null default';

$annotations['arr'] = new RequestParam();
$annotations['arr']->name = 'arr';
$annotations['arr']->array = true;

$this->paramReader
->expects($this->any())
->method('read')
Expand Down Expand Up @@ -174,23 +178,23 @@ public static function validatesConfiguredParamDataProvider()
array( // check that non-strict missing params take default value
'foo',
'1',
array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array(),
array('bar' => '2', 'baz' => '4'),
array('bar' => '2', 'baz' => '4', 'arr' => array()),
),
array( // pass Param in GET
'foo',
'42',
array('foo' => '42', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '42', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('foo' => '42'),
array('bar' => '2', 'baz' => '4'),
array('bar' => '2', 'baz' => '4', 'arr' => array()),
),
array( // check that invalid non-strict params take default value
'foo',
'1',
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('foo' => 'bar'),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) {
$errors = new ConstraintViolationList(array(
new ConstraintViolation("expected error", null, array(), null, null, null),
Expand All @@ -210,31 +214,31 @@ function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framewor
array( // invalid array
'buzz',
array(1),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => 'invaliddata'),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),
array( // invalid array (multiple depth)
'buzz',
array(1),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => array(array(1))),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),

array( // multiple array
'buzz',
array(2, 3, 4),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => array(2, 3, 4)),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),
array( // multiple array with one invalid value
'buzz',
array(2, 1, 4),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 1, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 1, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => array(2, 'invaliddata', 4)),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) {
$errors = new ConstraintViolationList(array(
new ConstraintViolation("expected error", null, array(), null, null, null),
Expand All @@ -254,44 +258,44 @@ function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framewor
array( // Array not provided in GET query
'boo',
array(),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => array(2, 3, 4)),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),
array( // QueryParam provided in GET query but as a scalar
'boo',
array(),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => array(2, 3, 4), 'boo' => 'scalar'),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),
array( // QueryParam provided in GET query with valid values
'boo',
array('1', 'foo', 5),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),
array( // QueryParam provided in GET query with valid values
'boozz',
null,
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array()),
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)),
array('bar' => '1', 'baz' => '4'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),
array( // QueryParam provided in GET query with valid values
'boozz',
5,
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null, 'arr' => array()),
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5),
array('bar' => '1', 'baz' => '4', 'boozz' => 5),
array('bar' => '1', 'baz' => '4', 'boozz' => 5, 'arr' => array()),
),
);
}

public function testValidatesAddParam()
{
$queryFetcher = $this->getParamFetcher(array(), array('bar' => '2', 'baz' => '4','bub' => '10'));
$queryFetcher = $this->getParamFetcher(array(), array('bar' => '2', 'baz' => '4','bub' => '10', 'arr' => array()));
$queryFetcher->setController($this->controller);

$runtimeParam = new RequestParam();
Expand All @@ -301,7 +305,7 @@ public function testValidatesAddParam()
$queryFetcher->addParam($runtimeParam);

$this->assertEquals(10, $queryFetcher->get('bub'));
$this->assertEquals(array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null,'bub' => 10), $queryFetcher->all());
$this->assertEquals(array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'bub' => 10, 'arr' => array()), $queryFetcher->all());
}

public function testValidatesConfiguredParamStrictly()
Expand Down Expand Up @@ -370,10 +374,10 @@ public function testExceptionOnValidatesFailure($query, $request, $param, \Closu
public static function exceptionOnValidatesFailureDataProvider()
{
return array(
array( // test missing strict param
array(),
array(),
'bar',
array( // test missing 'arr' request param of array type
array('boozz' => 'foo'),
array('bar' => 'foo', 'baz' => 'foo'),
'arr',
),
array( // test invalid strict param
array(),
Expand Down

0 comments on commit 6714a42

Please sign in to comment.