Skip to content

Commit

Permalink
Previous fix was uncomplete
Browse files Browse the repository at this point in the history 
#960
  • Loading branch information
@eliecharra @lsmith77
eliecharra authored and lsmith77 committed Feb 9, 2015
1 parent dcd379b commit cfc2f0e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 19 deletions.
12 changes: 7 additions & 5 deletions Request/ParamFetcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,6 @@ public function get($name, $strict = null)
$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 @@ -136,8 +132,14 @@ public function get($name, $strict = null)
}

if ($config->array) {

if (($default !== null || !$strict) || $nullable)
{
$default = (array) $default;
}

if (!is_array($param)) {
if (!$nullable) {
if ($strict && !$nullable) {
throw new BadRequestHttpException(
sprintf("%s parameter value of '%s' is not an array", $paramType, $name)
);
Expand Down
41 changes: 27 additions & 14 deletions Tests/Request/ParamFetcherTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ public function setup()
$annotations['arr']->name = 'arr';
$annotations['arr']->array = true;

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

$annotations['moo'] = new QueryParam();
$annotations['moo']->name = 'mooh';
$annotations['moo']->key = 'moo';
Expand Down Expand Up @@ -187,21 +193,21 @@ 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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array(),
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, 'arr' => array(), 'moo' => null),
array('foo' => '42', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('foo' => '42'),
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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('foo' => 'bar'),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) {
Expand All @@ -220,32 +226,39 @@ function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framewor

},
),
array( // nullable array with strict
'arr_null_strict',
array(),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array(),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
),
array( // invalid array
'buzz',
array(1),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => 'invaliddata'),
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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(array(1))),
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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(2, 3, 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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 1, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(2, 'invaliddata', 4)),
array('bar' => '1', 'baz' => '4', 'arr' => array()),
function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) {
Expand All @@ -267,42 +280,42 @@ 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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(2, 3, 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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(2, 3, 4), 'boo' => 'scalar'),
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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)),
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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)),
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, 'arr' => array(), 'moo' => null),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => null),
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5),
array('bar' => '1', 'baz' => '4', 'boozz' => 5, 'arr' => array()),
),
array( // QueryParam provided in GET query with valid values
'moo',
'string',
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null, 'arr' => array(), 'moo' => 'string'),
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => 'string'),
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'moo' => 'string'),
array('bar' => '1', 'baz' => '4', 'boozz' => 5, 'arr' => array()),
)
Expand All @@ -321,7 +334,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, 'arr' => array(), 'moo' => ''), $queryFetcher->all());
$this->assertEquals(array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'bub' => 10, 'arr' => array(), 'arr_null_strict' => array(), 'moo' => ''), $queryFetcher->all());
}

public function testValidatesConfiguredParamStrictly()
Expand Down

0 comments on commit cfc2f0e

Please sign in to comment.