From 93878d420dee600e23f9a3081106ae73902824cf Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Sun, 20 Sep 2015 10:47:48 +0200 Subject: [PATCH 1/4] Add the support of RequestStack in the route voter --- .travis.yml | 1 + src/Knp/Menu/Matcher/Voter/RouteVoter.php | 52 +++++++++++--- .../Silex/KnpMenuServiceProviderTest.php | 9 ++- .../Tests/Matcher/Voter/RouteVoterTest.php | 72 ++++++++++++++++++- 4 files changed, 120 insertions(+), 14 deletions(-) diff --git a/.travis.yml b/.travis.yml index b5323d9a..82067827 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,6 +18,7 @@ php: env: global: - SYMFONY_PHPUNIT_DIR="$HOME/symfony-bridge/.phpunit" + - SYMFONY_PHPUNIT_REMOVE="symfony/yaml" # keep Prophecy matrix: include: diff --git a/src/Knp/Menu/Matcher/Voter/RouteVoter.php b/src/Knp/Menu/Matcher/Voter/RouteVoter.php index 78a9d502..212ca462 100644 --- a/src/Knp/Menu/Matcher/Voter/RouteVoter.php +++ b/src/Knp/Menu/Matcher/Voter/RouteVoter.php @@ -4,6 +4,7 @@ use Knp\Menu\ItemInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; /** * Voter based on the route @@ -11,27 +12,60 @@ class RouteVoter implements VoterInterface { /** - * @var Request + * @var RequestStack|null + */ + private $requestStack; + + /** + * @var Request|null */ private $request; - public function __construct(Request $request = null) + public function __construct($requestStack = null) { - $this->request = $request; + if ($requestStack instanceof RequestStack) { + $this->requestStack = $requestStack; + } elseif ($requestStack instanceof Request) { + @trigger_error(sprintf('Passing a Request as the first argument for "%s" constructor is deprecated since version 2.3 and wwon\'t be possible in 3.0. Pass a RequestStack instead.', __CLASS__), E_USER_DEPRECATED); + + // BC layer for the old API of the class + $this->request = $requestStack; + } elseif (null !== $requestStack) { + throw new \InvalidArgumentException('The first argument of %s must be null, a RequestStack or a Request. %s given', __CLASS__, is_object($requestStack) ? get_class($requestStack) : gettype($requestStack)); + } else { + @trigger_error(sprintf('Not passing a RequestStack as the first argument for "%s" constructor is deprecated since version 2.3 and won\'t be possible in 3.0.', __CLASS__), E_USER_DEPRECATED); + } } + /** + * Sets the request against which the menu should be matched. + * + * This Request is ignored in case a RequestStack is passed in the constructor. + * + * @deprecated since version 2.3. Pass a RequestStack to the constructor instead. + * + * @param Request $request + */ public function setRequest(Request $request) { + @trigger_error(sprintf('The %s() method is deprecated since version 2.3 and will be removed in 3.0. Pass a RequestStack in the constructor instead.', __METHOD__), E_USER_DEPRECATED); + $this->request = $request; } public function matchItem(ItemInterface $item) { - if (null === $this->request) { + if (null !== $this->requestStack) { + $request = $this->requestStack->getCurrentRequest(); + } else { + $request = $this->request; + } + + if (null === $request) { return null; } - $route = $this->request->attributes->get('_route'); + $route = $request->attributes->get('_route'); if (null === $route) { return null; } @@ -47,7 +81,7 @@ public function matchItem(ItemInterface $item) throw new \InvalidArgumentException('Routes extra items must be strings or arrays.'); } - if ($this->isMatchingRoute($testedRoute)) { + if ($this->isMatchingRoute($request, $testedRoute)) { return true; } } @@ -55,9 +89,9 @@ public function matchItem(ItemInterface $item) return null; } - private function isMatchingRoute(array $testedRoute) + private function isMatchingRoute(Request $request, array $testedRoute) { - $route = $this->request->attributes->get('_route'); + $route = $request->attributes->get('_route'); if (isset($testedRoute['route'])) { if ($route !== $testedRoute['route']) { @@ -75,7 +109,7 @@ private function isMatchingRoute(array $testedRoute) return true; } - $routeParameters = $this->request->attributes->get('_route_params', array()); + $routeParameters = $request->attributes->get('_route_params', array()); foreach ($testedRoute['parameters'] as $name => $value) { if (!isset($routeParameters[$name]) || $routeParameters[$name] !== (string) $value) { diff --git a/tests/Knp/Menu/Tests/Integration/Silex/KnpMenuServiceProviderTest.php b/tests/Knp/Menu/Tests/Integration/Silex/KnpMenuServiceProviderTest.php index d02e4841..e9d15f9f 100644 --- a/tests/Knp/Menu/Tests/Integration/Silex/KnpMenuServiceProviderTest.php +++ b/tests/Knp/Menu/Tests/Integration/Silex/KnpMenuServiceProviderTest.php @@ -93,8 +93,13 @@ private function bootstrapApp() }; $app['test.voter'] = $app->share(function (Application $app) { - $voter = new RouteVoter(); - $voter->setRequest($app['request']); + $requestStack = isset($app['request_stack']) ? $app['request_stack'] : null; + + $voter = new RouteVoter($requestStack); + + if (null === $requestStack) { + $voter->setRequest($app['request']); + } return $voter; }); diff --git a/tests/Knp/Menu/Tests/Matcher/Voter/RouteVoterTest.php b/tests/Knp/Menu/Tests/Matcher/Voter/RouteVoterTest.php index 6c1e136b..e667c72b 100644 --- a/tests/Knp/Menu/Tests/Matcher/Voter/RouteVoterTest.php +++ b/tests/Knp/Menu/Tests/Matcher/Voter/RouteVoterTest.php @@ -5,6 +5,7 @@ use Knp\Menu\Matcher\Voter\RouteVoter; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; class RouteVoterTest extends TestCase { @@ -15,7 +16,10 @@ protected function setUp() } } - public function testMatchingWithoutRequest() + /** + * @group legacy + */ + public function testMatchingWithoutRequestAndStack() { $item = $this->getMockBuilder('Knp\Menu\ItemInterface')->getMock(); $item->expects($this->never()) @@ -26,6 +30,41 @@ public function testMatchingWithoutRequest() $this->assertNull($voter->matchItem($item)); } + public function testMatchingWithoutRequestInStack() + { + if (!class_exists('Symfony\Component\HttpFoundation\RequestStack')) { + $this->markTestSkipped('The RequestStack is not available in this version of HttpFoundation.'); + } + + $item = $this->getMockBuilder('Knp\Menu\ItemInterface')->getMock(); + $item->expects($this->never()) + ->method('getExtra'); + + $voter = new RouteVoter(new RequestStack()); + + $this->assertNull($voter->matchItem($item)); + } + + /** + * @group legacy + */ + public function testMatchingWithoutStackRequestButLegacyRequest() + { + if (!class_exists('Symfony\Component\HttpFoundation\RequestStack')) { + $this->markTestSkipped('The RequestStack is not available in this version of HttpFoundation.'); + } + + $item = $this->getMockBuilder('Knp\Menu\ItemInterface')->getMock(); + $item->expects($this->never()) + ->method('getExtra'); + + $voter = new RouteVoter(new RequestStack()); + // the request set explicitly is ignored when a RequestStack is provided + $voter->setRequest(new Request()); + + $this->assertNull($voter->matchItem($item)); + } + /** * @expectedException \InvalidArgumentException */ @@ -41,7 +80,10 @@ public function testInvalidRouteConfig() $request->attributes->set('_route', 'foo'); $request->attributes->set('_route_params', array()); - $voter = new RouteVoter($request); + $requestStack = new RequestStack(); + $requestStack->push($request); + + $voter = new RouteVoter($requestStack); $voter->matchItem($item); } @@ -67,11 +109,35 @@ public function testMatching($route, array $parameters, $itemRoutes, $expected) $request->attributes->set('_route', $route); $request->attributes->set('_route_params', $parameters); - $voter = new RouteVoter($request); + $requestStack = new RequestStack(); + $requestStack->push($request); + + $voter = new RouteVoter($requestStack); $this->assertSame($expected, $voter->matchItem($item)); } + /** + * @group legacy + */ + public function testMatchingWithRequest() + { + if (!class_exists('Symfony\Component\HttpFoundation\RequestStack')) { + $this->markTestSkipped('The RequestStack is not available in this version of HttpFoundation.'); + } + + $item = $this->prophesize('Knp\Menu\ItemInterface'); + $item->getExtra('routes', array())->willReturn(array('foo')); + + $request = new Request(); + $request->attributes->set('_route', 'foo'); + $request->attributes->set('_route_params', array()); + + $voter = new RouteVoter($request); + + $this->assertTrue($voter->matchItem($item->reveal())); + } + public function provideData() { return array( From 6c4aaa5a9397939369f40b949b87bb7d899ce79d Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Tue, 22 Aug 2017 13:15:58 +0200 Subject: [PATCH 2/4] Use the master request for matching --- src/Knp/Menu/Matcher/Voter/RouteVoter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Knp/Menu/Matcher/Voter/RouteVoter.php b/src/Knp/Menu/Matcher/Voter/RouteVoter.php index 212ca462..6e7d4301 100644 --- a/src/Knp/Menu/Matcher/Voter/RouteVoter.php +++ b/src/Knp/Menu/Matcher/Voter/RouteVoter.php @@ -56,7 +56,7 @@ public function setRequest(Request $request) public function matchItem(ItemInterface $item) { if (null !== $this->requestStack) { - $request = $this->requestStack->getCurrentRequest(); + $request = $this->requestStack->getMasterRequest(); } else { $request = $this->request; } From 38c76634eebb77054df09391787b714bb8355079 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Tue, 22 Aug 2017 14:06:54 +0200 Subject: [PATCH 3/4] Add an explicit dev requirement on symfony/http-foundation Relying on the silex requirement to bring this package does not give full control over the version. --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 95899648..5319a318 100644 --- a/composer.json +++ b/composer.json @@ -24,6 +24,7 @@ }, "require-dev": { "pimple/pimple": "~1.0", + "symfony/http-foundation": "~2.4|~3.0", "symfony/phpunit-bridge": "~3.3", "symfony/routing": "~2.3|~3.0", "silex/silex": "~1.0", From e4837dc3d34125a97627b200a99948da187ce3b5 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Tue, 22 Aug 2017 14:46:53 +0200 Subject: [PATCH 4/4] Add a dev requirement to force Silex to use HttpKernel 2.4+ Using HttpKernel 2.3 with HttpFoundation 2.4 makes Silex create a RequestStack but which is not updated by the kernel, which breaks our lowest-bound test. --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 5319a318..538de51f 100644 --- a/composer.json +++ b/composer.json @@ -25,6 +25,7 @@ "require-dev": { "pimple/pimple": "~1.0", "symfony/http-foundation": "~2.4|~3.0", + "symfony/http-kernel": "~2.4|~3.0", "symfony/phpunit-bridge": "~3.3", "symfony/routing": "~2.3|~3.0", "silex/silex": "~1.0",