Skip to content

Commit 9be4952

Browse files
GuilhemNGuilhemN
committed
Merge pull request #1263 from FriendsOfSymfony/FIXBUG
 Fix a bug in the ParamFetcher
2 parents 03b4b39 + 59c43d3 commit 9be4952

File tree

8 files changed

+255
-38
lines changed

8 files changed

+255
-38
lines changed

‎Request/ParamFetcher.php‎

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use FOS\RestBundle\Controller\Annotations\Param;
1616
use FOS\RestBundle\Controller\Annotations\RequestParam;
1717
use FOS\RestBundle\Util\ViolationFormatterInterface;
18-
use Doctrine\Common\Util\ClassUtils;
1918
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
2019
use Symfony\Component\DependencyInjection\ContainerInterface;
2120
use Symfony\Component\HttpFoundation\Request;
@@ -36,17 +35,14 @@
3635
*/
3736
class ParamFetcher implements ParamFetcherInterface, ContainerAwareInterface
3837
{
39-
private $paramReader;
38+
private $parameterBag;
4039
private $requestStack;
41-
private $params;
4240
private $validator;
4341
private $violationFormatter;
44-
4542
/**
4643
* @var callable
4744
*/
4845
private $controller;
49-
5046
/**
5147
* @var ContainerInterface
5248
*/
@@ -68,11 +64,12 @@ public function __construct(ParamReader $paramReader, $requestStack = null, Viol
6864
throw new \InvalidArgumentException(sprintf('Argument 3 of %s constructor must be either an instance of Symfony\Component\HttpFoundation\Request or Symfony\Component\HttpFoundation\RequestStack.', get_class($this)));
6965
}
7066

71-
$this->paramReader = $paramReader;
7267
$this->requestStack = $requestStack;
7368
$this->violationFormatter = $violationFormatter;
7469
$this->validator = $validator;
7570

71+
$this->parameterBag = new ParameterBag($paramReader);
72+
7673
if ($validator !== null && !$validator instanceof LegacyValidatorInterface && !$validator instanceof ValidatorInterface) {
7774
throw new \InvalidArgumentException(sprintf(
7875
'Validator has expected to be an instance of %s or %s, "%s" given',
@@ -98,7 +95,7 @@ public function setContainer(ContainerInterface $container = null)
9895
*/
9996
public function setController($controller)
10097
{
101-
$this->controller = $controller;
98+
$this->parameterBag->setController($this->getRequest(), $controller);
10299
}
103100

104101
/**
@@ -110,20 +107,15 @@ public function setController($controller)
110107
*/
111108
public function addParam(Param $param)
112109
{
113-
$this->getParams(); // init params
114-
$this->params[$param->name] = $param;
110+
$this->parameterBag->addParam($this->getRequest(), $param);
115111
}
116112

117113
/**
118114
* @return Param[]
119115
*/
120116
public function getParams()
121117
{
122-
if (null === $this->params) {
123-
$this->initParams();
124-
}
125-
126-
return $this->params;
118+
return $this->parameterBag->getParams($this->getRequest());
127119
}
128120

129121
/**
@@ -293,29 +285,6 @@ public function all($strict = null)
293285
return $params;
294286
}
295287

296-
/**
297-
* Initialize the parameters.
298-
*
299-
* @throws \InvalidArgumentException
300-
*/
301-
private function initParams()
302-
{
303-
if (empty($this->controller)) {
304-
throw new \InvalidArgumentException('Controller and method needs to be set via setController');
305-
}
306-
307-
if (!is_array($this->controller) || empty($this->controller[0]) || !is_object($this->controller[0])) {
308-
throw new \InvalidArgumentException(
309-
'Controller needs to be set as a class instance (closures/functions are not supported)'
310-
);
311-
}
312-
313-
$this->params = $this->paramReader->read(
314-
new \ReflectionClass(ClassUtils::getClass($this->controller[0])),
315-
$this->controller[1]
316-
);
317-
}
318-
319288
/**
320289
* Check if current param is not in conflict with other parameters
321290
* according to the "incompatibles" field.

‎Request/ParameterBag.php‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the FOSRestBundle package.
5+
*
6+
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace FOS\RestBundle\Request;
13+
14+
use Doctrine\Common\Util\ClassUtils;
15+
use FOS\RestBundle\Controller\Annotations\Param;
16+
use Symfony\Component\HttpFoundation\Request;
17+
18+
/**
19+
* Contains the {@link ParamFetcher} params and links them to a request.
20+
*
21+
* @internal
22+
*/
23+
final class ParameterBag
24+
{
25+
private $paramReader;
26+
private $params = array();
27+
28+
public function __construct(ParamReaderInterface $paramReader)
29+
{
30+
$this->paramReader = $paramReader;
31+
}
32+
33+
public function getParams(Request $request)
34+
{
35+
$requestId = spl_object_hash($request);
36+
if (!isset($this->params[$requestId]) || empty($this->params[$requestId]['controller'])) {
37+
throw new \InvalidArgumentException('Controller and method needs to be set via setController.');
38+
}
39+
if ($this->params[$requestId]['params'] === null) {
40+
return $this->initParams($requestId);
41+
}
42+
43+
return $this->params[$requestId]['params'];
44+
}
45+
46+
public function addParam(Request $request, Param $param)
47+
{
48+
$requestId = spl_object_hash($request);
49+
$this->getParams($request);
50+
51+
$this->params[$requestId]['params'][$param->name] = $param;
52+
}
53+
54+
public function setController(Request $request, $controller)
55+
{
56+
$requestId = spl_object_hash($request);
57+
$this->params[$requestId] = array(
58+
'controller' => $controller,
59+
'params' => null,
60+
);
61+
}
62+
63+
/**
64+
* Initialize the parameters.
65+
*
66+
* @param string $requestId
67+
*
68+
* @throws \InvalidArgumentException
69+
*/
70+
private function initParams($requestId)
71+
{
72+
$controller = $this->params[$requestId]['controller'];
73+
if (!is_array($controller) || empty($controller[0]) || !is_object($controller[0])) {
74+
throw new \InvalidArgumentException(
75+
'Controller needs to be set as a class instance (closures/functions are not supported)'
76+
);
77+
}
78+
79+
return $this->params[$requestId]['params'] = $this->paramReader->read(
80+
new \ReflectionClass(ClassUtils::getClass($controller[0])),
81+
$controller[1]
82+
);
83+
}
84+
}

‎Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the FOSRestBundle package.
5+
*
6+
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace FOS\RestBundle\Tests\Functional\Bundle\TestBundle\Controller;
13+
14+
use FOS\RestBundle\Controller\Annotations\RequestParam;
15+
use FOS\RestBundle\Controller\Annotations\QueryParam;
16+
use FOS\RestBundle\Request\ParamFetcherInterface;
17+
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
18+
use Symfony\Component\HttpFoundation\JsonResponse;
19+
use Symfony\Component\HttpFoundation\Request;
20+
use Symfony\Component\HttpKernel\HttpKernelInterface;
21+
use Symfony\Component\Validator\Constraints\IdenticalTo;
22+
23+
class ParamFetcherController extends Controller
24+
{
25+
/**
26+
* @RequestParam(name="raw", requirements=@IdenticalTo(value="fooraw", message="foo"), default="invalid")
27+
* @RequestParam(name="map", array=true, requirements=@IdenticalTo(value={"foo"="map", "foobar"="foo"}, message="foo"), default="invalid2")
28+
*/
29+
public function paramsAction(ParamFetcherInterface $fetcher)
30+
{
31+
return new JsonResponse($fetcher->all(false));
32+
}
33+
34+
/**
35+
* @QueryParam(name="foo", default="invalid")
36+
* @RequestParam(name="bar", default="foo")
37+
*/
38+
public function testAction(Request $request, ParamFetcherInterface $fetcher)
39+
{
40+
$paramsBefore = $fetcher->all();
41+
42+
$newRequest = new Request();
43+
$newRequest->query = $request->query;
44+
$newRequest->request = $request->request;
45+
$newRequest->attributes->set('_controller', sprintf('%s::paramsAction', __CLASS__));
46+
$response = $this->container->get('http_kernel')->handle($newRequest, HttpKernelInterface::SUB_REQUEST, false);
47+
48+
$paramsAfter = $fetcher->all(false);
49+
50+
return new JsonResponse(array(
51+
'before' => $paramsBefore,
52+
'during' => json_decode($response->getContent(), true),
53+
'after' => $paramsAfter,
54+
));
55+
}
56+
}

‎Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,11 @@ test_serializer_error_exception:
99
test_serializer_error_invalid_form:
1010
path: /serializer-error/invalid-form.{_format}
1111
defaults: { _controller: TestBundle:SerializerError:invalidForm }
12+
13+
test_param_fetcher:
14+
path: /params
15+
defaults: { _controller: TestBundle:ParamFetcher:params }
16+
17+
test_param_fetcher_test:
18+
path: /params/test
19+
defaults: { _controller: TestBundle:ParamFetcher:test }

‎Tests/Functional/ParamFetcherTest.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the FOSRestBundle package.
5+
*
6+
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace FOS\RestBundle\Tests\Functional;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
16+
/**
17+
* @author Ener-Getick <[email protected]>
18+
*/
19+
class ParamFetcherTest extends WebTestCase
20+
{
21+
private $validRaw = 'fooraw';
22+
private $validMap = array(
23+
'foo' => 'map',
24+
'foobar' => 'foo',
25+
);
26+
27+
public function setUp()
28+
{
29+
$this->client = $this->createClient(array('test_case' => 'ParamFetcher'));
30+
}
31+
32+
public function testDefaultParameters()
33+
{
34+
$this->client->request('POST', '/params');
35+
36+
$this->assertEquals(array('raw' => 'invalid', 'map' => array('invalid2')), $this->getData());
37+
}
38+
39+
public function testValidRawParameter()
40+
{
41+
$this->client->request('POST', '/params', array('raw' => $this->validRaw, 'map' => $this->validMap));
42+
43+
$this->assertEquals(array('raw' => $this->validRaw, 'map' => array('foo' => 'invalid2', 'foobar' => 'invalid2')), $this->getData());
44+
}
45+
46+
public function testValidMapParameter()
47+
{
48+
$map = array(
49+
'foo' => $this->validMap,
50+
'bar' => $this->validMap,
51+
);
52+
$this->client->request('POST', '/params', array('raw' => 'bar', 'map' => $map));
53+
54+
$this->assertEquals(array('raw' => 'invalid', 'map' => $map), $this->getData());
55+
}
56+
57+
public function testWithSubRequests()
58+
{
59+
$this->client->request('POST', '/params/test?foo=quz', array('raw' => $this->validRaw));
60+
$this->assertEquals(array(
61+
'before' => array('foo' => 'quz', 'bar' => 'foo'),
62+
'during' => array('raw' => $this->validRaw, 'map' => array('invalid2')),
63+
'after' => array('foo' => 'quz', 'bar' => 'foo'),
64+
), $this->getData());
65+
}
66+
67+
protected function getData()
68+
{
69+
return json_decode($this->client->getResponse()->getContent(), true);
70+
}
71+
}

‎Tests/Functional/app/ParamFetcher/bundles.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the FOSRestBundle package.
5+
*
6+
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
return array(
13+
new \Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
14+
new \FOS\RestBundle\FOSRestBundle(),
15+
new \FOS\RestBundle\Tests\Functional\Bundle\TestBundle\TestBundle(),
16+
new \Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle(),
17+
);

‎Tests/Functional/app/ParamFetcher/config.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
imports:
2+
- { resource: ../config/default.yml }
3+
4+
framework:
5+
serializer:
6+
enabled: true
7+
8+
fos_rest:
9+
param_fetcher_listener: true

‎Tests/Request/ParamFetcherTest.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,10 @@ public function testExceptionOnRequestWithoutController()
504504
$this->markTestSkipped('RequestStack unvailable.');
505505
}
506506

507-
$queryFetcher = new ParamFetcher($this->paramReader, new RequestStack(), $this->violationFormatter, $this->validator);
507+
$requestStack = new RequestStack();
508+
$requestStack->push(new Request());
509+
510+
$queryFetcher = new ParamFetcher($this->paramReader, $requestStack, $this->violationFormatter, $this->validator);
508511
$queryFetcher->get('none', '42');
509512
}
510513

0 commit comments

Comments
 (0)