Skip to content

Commit

Permalink
use the request stack instead of synchronized request
Browse files Browse the repository at this point in the history
  • Loading branch information
Tobion committed Sep 18, 2013
1 parent b3e68d0 commit e8fda4d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 25 deletions.
36 changes: 16 additions & 20 deletions src/Symfony/Bridge/Twig/Extension/RoutingExtension.php
Expand Up @@ -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;

/**
Expand All @@ -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;
}

/**
Expand All @@ -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.
Expand All @@ -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)
Expand All @@ -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);
}

/**
Expand Down
15 changes: 11 additions & 4 deletions src/Symfony/Bridge/Twig/Tests/Extension/RoutingExtensionTest.php
Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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),
);
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/Twig/composer.json
Expand Up @@ -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",
Expand All @@ -31,6 +32,7 @@
},
"suggest": {
"symfony/form": "",
"symfony/http-foundation": "",
"symfony/http-kernel": "",
"symfony/routing": "",
"symfony/templating": "",
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
Expand Up @@ -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">
Expand Down

0 comments on commit e8fda4d

Please sign in to comment.