Merge pull request KnpLabs#222 from stof/use_request_stack

Add the support of RequestStack in the route voter
ph-kvieth · Aug 22, 2017 · f49faca · f49faca
2 parents 928df8e + e4837dc
commit f49faca
Show file tree

Hide file tree

Showing 5 changed files with 122 additions and 14 deletions.
diff --git 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/composer.json b/composer.json
@@ -24,6 +24,8 @@
     },
     "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",

diff --git a/src/Knp/Menu/Matcher/Voter/RouteVoter.php b/src/Knp/Menu/Matcher/Voter/RouteVoter.php
@@ -4,34 +4,68 @@
 
 use Knp\Menu\ItemInterface;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 
 /**
  * Voter based on the route
  */
 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->getMasterRequest();
+        } 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,17 +81,17 @@ 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;
             }
         }
 
         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
@@ -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
@@ -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(
-Original file line number
+Diff line change
@@ Expand Up / @@ -18,6 +18,7 @@ php: @@
     env:
         global:
             - SYMFONY_PHPUNIT_DIR="$HOME/symfony-bridge/.phpunit"
+            - SYMFONY_PHPUNIT_REMOVE="symfony/yaml" # keep Prophecy
     matrix:
         include:
@@ Expand Down @@