use the request stack instead of synchronized request

symfony · Tobion · Apr 17, 2012 · Sep 17, 2013 · Sep 18, 2013 · e8fda4d985885d54e9e5964dc6c78b7365b45dca
commit e8fda4d985885d54e9e5964dc6c78b7365b45dca
diff --git a/src/Symfony/Bridge/Twig/Extension/RoutingExtension.php b/src/Symfony/Bridge/Twig/Extension/RoutingExtension.php
@@ -11,7 +11,7 @@
 
 namespace Symfony\Bridge\Twig\Extension;
 
-use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
 /**
@@ -27,18 +27,20 @@ class RoutingExtension extends \Twig_Extension
     private $generator;
 
     /**
-     * @var Request|null
+     * @var RequestStack|null
      */
-    private $request;
+    private $requestStack;
 
     /**
      * Constructor.
      *
-     * @param UrlGeneratorInterface $generator A UrlGeneratorInterface instance
+     * @param UrlGeneratorInterface $generator    A UrlGeneratorInterface instance
+     * @param RequestStack|null     $requestStack An optional stack containing master/sub requests
      */
-    public function __construct(UrlGeneratorInterface $generator)
+    public function __construct(UrlGeneratorInterface $generator, RequestStack $requestStack = null)
     {
         $this->generator = $generator;
+        $this->requestStack = $requestStack;
     }
 
     /**
@@ -65,16 +67,6 @@ public function getPath($name, $parameters = array(), $relative = false)
         return $this->generator->generate($name, $parameters, $relative ? UrlGeneratorInterface::RELATIVE_PATH : UrlGeneratorInterface::ABSOLUTE_PATH);
     }
 
-    /**
-     * Sets the current request that is needed for the subpath function.
-     *
-     * @param Request|null $request
-     */
-    public function setRequest(Request $request = null)
-    {
-        $this->request = $request;
-    }
-
     /**
      * Returns the relative path to a route (defaults to current route) with all current route parameters merged
      * with the passed params. 
@@ -83,6 +75,10 @@ public function setRequest(Request $request = null)
      * params by passing null as value of a specific parameter. The path is a relative path to the
      * target URL, e.g. "../slug", based on the current request path.
      *
+     * Beware when using this method in a subrequest as it will use the params of the subrequest and will
+     * also generate a relative path based on it. The resulting relative reference is probably the wrong
+     * target when resolved by the user agent (browser) based on the main request.
+     *
      * @param string  $name         The route name (when empty it defaults to the current route)
      * @param array   $parameters   The parameters that should be added or overwrite existing params
      * @param Boolean $includeQuery Whether the current params in the query string should be included (disabled by default)
@@ -93,20 +89,20 @@ public function setRequest(Request $request = null)
      */
     public function getSubPath($name = '', array $parameters = array(), $includeQuery = false)
     {
-        if (null === $this->request) {
-            throw new \LogicException('The subpath function needs the current request to be set via setRequest().');
+        if (null === $this->requestStack || null === $request = $this->requestStack->getCurrentRequest()) {
+            throw new \LogicException('The subpath function needs the request to be set in the request stack.');
         }
 
         $parameters = array_replace(
-            $includeQuery ? $this->request->query->all() : array(),
-            $this->request->attributes->get('_route_params', array()),
+            $includeQuery ? $request->query->all() : array(),
+            $request->attributes->get('_route_params', array()),
             $parameters
         );
         $parameters = array_filter($parameters, function ($value) { 
             return null !== $value; 
         });
 
-        return $this->generator->generate($name ?: $this->request->attributes->get('_route', ''), $parameters, UrlGeneratorInterface::RELATIVE_PATH);
+        return $this->generator->generate($name ?: $request->attributes->get('_route', ''), $parameters, UrlGeneratorInterface::RELATIVE_PATH);
     }
 
     /**

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/RoutingExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/RoutingExtensionTest.php
@@ -13,6 +13,7 @@
 
 use Symfony\Bridge\Twig\Extension\RoutingExtension;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Routing\Generator\UrlGenerator;
 use Symfony\Component\Routing\RequestContext;
 use Symfony\Component\Routing\Route;
@@ -31,12 +32,14 @@ public function testUrlGeneration()
         $request->attributes->set('_route', 'page');
         $request->attributes->set('_route_params', array('dir' => 'dir', 'page' => 'page', '_format' => 'html'));
 
+        $requestStack = new RequestStack();
+        $requestStack->push($request);
+
         $context = new RequestContext();
         $context->fromRequest($request);
         $generator = new UrlGenerator($routes, $context);
 
-        $extension = new RoutingExtension($generator);
-        $extension->setRequest($request);
+        $extension = new RoutingExtension($generator, $requestStack);
 
         $this->assertSame('http://example.com/dir/page/comments', $extension->getUrl('comments', array('dir' => 'dir', 'page' => 'page')));
         $this->assertSame('//example.com/dir/page/comments', $extension->getUrl('comments', array('dir' => 'dir', 'page' => 'page'), true));
@@ -68,12 +71,14 @@ public function testPlaceholdersHaveHigherPriorityThanQueryInSubPath()
         $request->attributes->set('_route', 'page');
         $request->attributes->set('_route_params', array('page' => 'mypage'));
 
+        $requestStack = new RequestStack();
+        $requestStack->push($request);
+
         $context = new RequestContext();
         $context->fromRequest($request);
         $generator = new UrlGenerator($routes, $context);
 
-        $extension = new RoutingExtension($generator);
-        $extension->setRequest($request);
+        $extension = new RoutingExtension($generator, $requestStack);
 
         $this->assertStringStartsNotWith('querypage', $extension->getSubPath('', array(), true),
             'when the request query string has a parameter with the same name as a placeholder, the query param is ignored when includeQuery=true'
@@ -110,6 +115,8 @@ public function getEscapingTemplates()
             array('{{ path(name = "foo", parameters = { foo: ["foo", "bar"] }) }}', true),
             array('{{ path(name = "foo", parameters = { foo: foo }) }}', true),
             array('{{ path(name = "foo", parameters = { foo: "foo", bar: "bar" }) }}', true),
+
+            array('{{ subpath("foo") }}', true),
         );
     }
 }
diff --git a/src/Symfony/Bridge/Twig/composer.json b/src/Symfony/Bridge/Twig/composer.json
@@ -21,6 +21,7 @@
     },
     "require-dev": {
         "symfony/form": "~2.2",
+        "symfony/http-foundation": "~2.4",
         "symfony/http-kernel": "~2.2",
         "symfony/routing": "~2.2",
         "symfony/templating": "~2.1",
@@ -31,6 +32,7 @@
     },
     "suggest": {
         "symfony/form": "",
+        "symfony/http-foundation": "",
         "symfony/http-kernel": "",
         "symfony/routing": "",
         "symfony/templating": "",

diff --git a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
@@ -81,7 +81,7 @@
 
         <service id="twig.extension.routing" class="%twig.extension.routing.class%" public="false">
             <argument type="service" id="router" />
-            <call method="setRequest"><argument type="service" id="request" on-invalid="null" strict="false" /></call>
+            <argument type="service" id="request_stack" />
         </service>
 
         <service id="twig.extension.yaml" class="%twig.extension.yaml.class%" public="false">