From 64d43c806bc1c00dda536305565d8aaa3d07b699 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 17 Dec 2012 15:17:23 +0100 Subject: [PATCH] restricted to only URIs the first argument of the render tag --- UPGRADE-2.2.md | 18 ++++ src/Symfony/Bridge/Twig/CHANGELOG.md | 1 + .../Bundle/FrameworkBundle/CHANGELOG.md | 4 + .../Controller/InternalController.php | 75 --------------- .../Bundle/FrameworkBundle/HttpKernel.php | 94 +++---------------- .../Resources/config/routing/internal.xml | 18 ---- .../FrameworkBundle/Tests/HttpKernelTest.php | 55 ++--------- .../TwigBundle/Extension/ActionsExtension.php | 13 ++- .../Bundle/TwigBundle/Node/RenderNode.php | 8 +- .../TokenParser/RenderTokenParser.php | 17 +--- 10 files changed, 54 insertions(+), 249 deletions(-) delete mode 100644 src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php delete mode 100644 src/Symfony/Bundle/FrameworkBundle/Resources/config/routing/internal.xml diff --git a/UPGRADE-2.2.md b/UPGRADE-2.2.md index 864cf983f9..4fbfe5cc64 100644 --- a/UPGRADE-2.2.md +++ b/UPGRADE-2.2.md @@ -1,6 +1,24 @@ UPGRADE FROM 2.1 to 2.2 ======================= +### TwigBridge + + * The `render` tag signature and arguments changed. + + Before: + + ``` + {% render 'BlogBundle:Post:list' with { 'limit': 2 }, { 'alt': 'BlogBundle:Post:error' } %} + ``` + + After: + + ``` + {% render url('post_list', { 'limit': 2 }), { 'alt': 'BlogBundle:Post:error' } %} + ``` + + where `post_list` is the route name for the `BlogBundle:Post:list` controller. + ### HttpFoundation * The MongoDbSessionHandler default field names and timestamp type have changed. diff --git a/src/Symfony/Bridge/Twig/CHANGELOG.md b/src/Symfony/Bridge/Twig/CHANGELOG.md index 5513c8b4ff..644a130e2a 100644 --- a/src/Symfony/Bridge/Twig/CHANGELOG.md +++ b/src/Symfony/Bridge/Twig/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 2.2.0 ----- + * [BC BREAK] restricted the `render` tag to only accept URIs as reference (the signature changed) * added a render function to render a request * The `app` global variable is now injected even when using the twig service directly. diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index c89be59cff..edca8d858e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -4,6 +4,10 @@ CHANGELOG 2.2.0 ----- + * [BC BREAK] restricted the `Symfony\Bundle\FrameworkBundle\HttpKernel::render()` method to only accept URIs as reference + * `Symfony\Bundle\FrameworkBundle\HttpKernel::render()` method signature changed and the first argument + must now be a URI (the `generateInternalUri()` method was removed) + * The internal routes have been removed (`Resources/config/routing/internal.xml`) * replaced Symfony\Bundle\FrameworkBundle\Controller\TraceableControllerResolver by Symfony\Component\HttpKernel\Controller\TraceableControllerResolver * replaced Symfony\Component\HttpKernel\Debug\ContainerAwareTraceableEventDispatcher by Symfony\Component\HttpKernel\Debug\TraceableEventDispatcher * added Client::enableProfiler() diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php deleted file mode 100644 index ce674b190c..0000000000 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php +++ /dev/null @@ -1,75 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Bundle\FrameworkBundle\Controller; - -use Symfony\Component\DependencyInjection\ContainerAware; -use Symfony\Component\HttpFoundation\Response; - -/** - * InternalController. - * - * @author Fabien Potencier - */ -class InternalController extends ContainerAware -{ - /** - * Forwards to the given controller with the given path. - * - * @param string $path The path - * @param string $controller The controller name - * - * @return Response A Response instance - */ - public function indexAction($path, $controller) - { - // safeguard - if (!is_string($controller)) { - throw new \RuntimeException('A Controller must be a string.'); - } - - // check that the controller looks like a controller - if (false === strpos($controller, '::')) { - $count = substr_count($controller, ':'); - if (2 == $count) { - // the convention already enforces the Controller suffix - } elseif (1 == $count) { - // controller in the service:method notation - list($service, $method) = explode(':', $controller, 2); - $class = get_class($this->container->get($service)); - - if (!preg_match('/Controller$/', $class)) { - throw new \RuntimeException('A Controller class name must end with Controller.'); - } - } else { - throw new \LogicException('Unable to parse the Controller name.'); - } - } else { - list($class, $method) = explode('::', $controller, 2); - - if (!preg_match('/Controller$/', $class)) { - throw new \RuntimeException('A Controller class name must end with Controller.'); - } - } - - $request = $this->container->get('request'); - $attributes = $request->attributes; - - $attributes->remove('path'); - $attributes->remove('controller'); - if ('none' !== $path) { - parse_str($path, $tmp); - $attributes->add($tmp); - } - - return $this->container->get('http_kernel')->forward($controller, $attributes->all(), $request->query->all()); - } -} diff --git a/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php b/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php index 6aba2501d6..4f2d2a19ec 100644 --- a/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php +++ b/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php @@ -84,54 +84,40 @@ class HttpKernel extends BaseHttpKernel * * Available options: * - * * attributes: An array of request attributes (only when the first argument is a controller) - * * query: An array of request query parameters (only when the first argument is a controller) * * ignore_errors: true to return an empty string in case of an error - * * alt: an alternative controller to execute in case of an error (can be a controller, a URI, or an array with the controller, the attributes, and the query arguments) + * * alt: an alternative URI to execute in case of an error * * standalone: whether to generate an esi:include tag or not when ESI is supported * * comment: a comment to add when returning an esi:include tag * - * @param string $controller A controller name to execute (a string like BlogBundle:Post:index), or a relative URI - * @param array $options An array of options + * @param string $uri A URI + * @param array $options An array of options * * @return string The Response content * * @throws \RuntimeException * @throws \Exception */ - public function render($controller, array $options = array()) + public function render($uri, array $options = array()) { + $request = $this->container->get('request'); + $options = array_merge(array( - 'attributes' => array(), - 'query' => array(), 'ignore_errors' => !$this->container->getParameter('kernel.debug'), - 'alt' => array(), + 'alt' => null, 'standalone' => false, 'comment' => '', 'default' => null, ), $options); - if (!is_array($options['alt'])) { - $options['alt'] = array($options['alt']); - } - if (null === $this->esiSupport) { - $this->esiSupport = $this->container->has('esi') && $this->container->get('esi')->hasSurrogateEsiCapability($this->container->get('request')); + $this->esiSupport = $this->container->has('esi') && $this->container->get('esi')->hasSurrogateEsiCapability($request); } if ($this->esiSupport && (true === $options['standalone'] || 'esi' === $options['standalone'])) { - $uri = $this->generateInternalUri($controller, $options['attributes'], $options['query']); - - $alt = ''; - if ($options['alt']) { - $alt = $this->generateInternalUri($options['alt'][0], isset($options['alt'][1]) ? $options['alt'][1] : array(), isset($options['alt'][2]) ? $options['alt'][2] : array()); - } - - return $this->container->get('esi')->renderIncludeTag($uri, $alt, $options['ignore_errors'], $options['comment']); + return $this->container->get('esi')->renderIncludeTag($uri, $options['alt'], $options['ignore_errors'], $options['comment']); } if ('js' === $options['standalone']) { - $uri = $this->generateInternalUri($controller, $options['attributes'], $options['query'], false); $defaultContent = null; $templating = $this->container->get('templating'); @@ -149,29 +135,9 @@ class HttpKernel extends BaseHttpKernel return $this->renderHIncludeTag($uri, $defaultContent); } - $request = $this->container->get('request'); - - // controller or URI or path? - if (0 === strpos($controller, 'http://') || 0 === strpos($controller, 'https://')) { - $subRequest = Request::create($controller, 'get', array(), $request->cookies->all(), array(), $request->server->all()); - if ($session = $request->getSession()) { - $subRequest->setSession($session); - } - } elseif (0 === strpos($controller, '/')) { - $subRequest = Request::create($request->getUriForPath($controller), 'get', array(), $request->cookies->all(), array(), $request->server->all()); - if ($session = $request->getSession()) { - $subRequest->setSession($session); - } - } else { - $options['attributes']['_controller'] = $controller; - - if (!isset($options['attributes']['_format'])) { - $options['attributes']['_format'] = $request->getRequestFormat(); - } - - $options['attributes']['_route'] = '_internal'; - $subRequest = $request->duplicate($options['query'], null, $options['attributes']); - $subRequest->setMethod('GET'); + $subRequest = Request::create($uri, 'get', array(), $request->cookies->all(), array(), $request->server->all()); + if ($session = $request->getSession()) { + $subRequest->setSession($session); } $level = ob_get_level(); @@ -191,10 +157,8 @@ class HttpKernel extends BaseHttpKernel if ($options['alt']) { $alt = $options['alt']; unset($options['alt']); - $options['attributes'] = isset($alt[1]) ? $alt[1] : array(); - $options['query'] = isset($alt[2]) ? $alt[2] : array(); - return $this->render($alt[0], $options); + return $this->render($alt, $options); } if (!$options['ignore_errors']) { @@ -208,38 +172,6 @@ class HttpKernel extends BaseHttpKernel } } - /** - * Generates an internal URI for a given controller. - * - * This method uses the "_internal" route, which should be available. - * - * @param string $controller A controller name to execute (a string like BlogBundle:Post:index), or a relative URI - * @param array $attributes An array of request attributes - * @param array $query An array of request query parameters - * @param boolean $secure - * - * @return string An internal URI - */ - public function generateInternalUri($controller, array $attributes = array(), array $query = array(), $secure = true) - { - if (0 === strpos($controller, '/')) { - return $controller; - } - - $path = http_build_query($attributes, '', '&'); - $uri = $this->container->get('router')->generate($secure ? '_internal' : '_internal_public', array( - 'controller' => $controller, - 'path' => $path ?: 'none', - '_format' => $this->container->get('request')->getRequestFormat(), - )); - - if ($queryString = http_build_query($query, '', '&')) { - $uri .= '?'.$queryString; - } - - return $uri; - } - /** * Renders an HInclude tag. * diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing/internal.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing/internal.xml deleted file mode 100644 index 556c45b301..0000000000 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing/internal.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - FrameworkBundle:Internal:index - .+ - [^.]+ - - - - FrameworkBundle:Internal:index - .+ - [^.]+ - - diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/HttpKernelTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/HttpKernelTest.php index 3c02f4fe50..c7587f6b4b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/HttpKernelTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/HttpKernelTest.php @@ -116,47 +116,6 @@ class HttpKernelTest extends \PHPUnit_Framework_TestCase } } - public function testGenerateInternalUriHandlesNullValues() - { - $request = new Request(); - - $router = $this->getMock('Symfony\\Component\\Routing\\RouterInterface'); - $container = $this->getMock('Symfony\\Component\\DependencyInjection\\ContainerInterface'); - $container - ->expects($this->at(0)) - ->method('get') - ->with($this->equalTo('router')) - ->will($this->returnValue($router)) - ; - $container - ->expects($this->at('1')) - ->method('get') - ->with($this->equalTo('request')) - ->will($this->returnValue($request)) - ; - - $controller = 'AController'; - $attributes = array('anAttribute' => null); - $query = array('aQueryParam' => null); - - $expectedPath = 'none'; - - $routeParameters = array('controller' => $controller, 'path' => $expectedPath, '_format' => 'html'); - $router - ->expects($this->once()) - ->method('generate') - ->with($this->equalTo('_internal'), $this->equalTo($routeParameters)) - ->will($this->returnValue('GENERATED_URI')) - ; - - $dispatcher = new EventDispatcher(); - $resolver = $this->getMock('Symfony\\Component\\HttpKernel\\Controller\\ControllerResolverInterface'); - $kernel = new HttpKernel($dispatcher, $container, $resolver); - - $uri = $kernel->generateInternalUri($controller, $attributes, $query); - $this->assertEquals('GENERATED_URI', $uri); - } - public function getProviderTypes() { return array( @@ -172,22 +131,22 @@ class HttpKernelTest extends \PHPUnit_Framework_TestCase $container = $this->getMock('Symfony\\Component\\DependencyInjection\\ContainerInterface'); $container ->expects($this->at(0)) + ->method('get') + ->with($this->equalTo('request')) + ->will($this->returnValue($request)) + ; + $container + ->expects($this->at(1)) ->method('getParameter') ->with($this->equalTo('kernel.debug')) ->will($this->returnValue(false)) ; $container - ->expects($this->at(1)) + ->expects($this->at(2)) ->method('has') ->with($this->equalTo('esi')) ->will($this->returnValue(false)) ; - $container - ->expects($this->at(2)) - ->method('get') - ->with($this->equalTo('request')) - ->will($this->returnValue($request)) - ; $dispatcher = new EventDispatcher(); $resolver = $this->getMock('Symfony\\Component\\HttpKernel\\Controller\\ControllerResolverInterface'); diff --git a/src/Symfony/Bundle/TwigBundle/Extension/ActionsExtension.php b/src/Symfony/Bundle/TwigBundle/Extension/ActionsExtension.php index 017bfb4311..4f88c6ca15 100644 --- a/src/Symfony/Bundle/TwigBundle/Extension/ActionsExtension.php +++ b/src/Symfony/Bundle/TwigBundle/Extension/ActionsExtension.php @@ -34,17 +34,16 @@ class ActionsExtension extends \Twig_Extension } /** - * Returns the Response content for a given controller or URI. + * Returns the Response content for a given URI. * - * @param string $controller A controller name to execute (a string like BlogBundle:Post:index), or a relative URI - * @param array $attributes An array of request attributes - * @param array $options An array of options + * @param string $uri A URI + * @param array $options An array of options * * @see Symfony\Bundle\FrameworkBundle\Controller\ControllerResolver::render() */ - public function renderAction($controller, array $attributes = array(), array $options = array()) + public function renderUri($uri, array $options = array()) { - return $this->container->get('templating.helper.actions')->render($controller, $attributes, $options); + return $this->container->get('templating.helper.actions')->render($uri, $options); } /** @@ -55,7 +54,7 @@ class ActionsExtension extends \Twig_Extension public function getTokenParsers() { return array( - // {% render 'BlogBundle:Post:list' with { 'limit': 2 }, { 'alt': 'BlogBundle:Post:error' } %} + // {% render url('post_list', { 'limit': 2 }), { 'alt': 'BlogBundle:Post:error' } %} new RenderTokenParser(), ); } diff --git a/src/Symfony/Bundle/TwigBundle/Node/RenderNode.php b/src/Symfony/Bundle/TwigBundle/Node/RenderNode.php index 3c40f3c8db..7f4c385723 100644 --- a/src/Symfony/Bundle/TwigBundle/Node/RenderNode.php +++ b/src/Symfony/Bundle/TwigBundle/Node/RenderNode.php @@ -18,9 +18,9 @@ namespace Symfony\Bundle\TwigBundle\Node; */ class RenderNode extends \Twig_Node { - public function __construct(\Twig_Node_Expression $expr, \Twig_Node_Expression $attributes, \Twig_Node_Expression $options, $lineno, $tag = null) + public function __construct(\Twig_Node_Expression $expr, \Twig_Node_Expression $options, $lineno, $tag = null) { - parent::__construct(array('expr' => $expr, 'attributes' => $attributes, 'options' => $options), array(), $lineno, $tag); + parent::__construct(array('expr' => $expr, 'options' => $options), array(), $lineno, $tag); } /** @@ -32,11 +32,9 @@ class RenderNode extends \Twig_Node { $compiler ->addDebugInfo($this) - ->write("echo \$this->env->getExtension('actions')->renderAction(") + ->write("echo \$this->env->getExtension('actions')->renderUri(") ->subcompile($this->getNode('expr')) ->raw(', ') - ->subcompile($this->getNode('attributes')) - ->raw(', ') ->subcompile($this->getNode('options')) ->raw(");\n") ; diff --git a/src/Symfony/Bundle/TwigBundle/TokenParser/RenderTokenParser.php b/src/Symfony/Bundle/TwigBundle/TokenParser/RenderTokenParser.php index 272324bef4..74177f0596 100644 --- a/src/Symfony/Bundle/TwigBundle/TokenParser/RenderTokenParser.php +++ b/src/Symfony/Bundle/TwigBundle/TokenParser/RenderTokenParser.php @@ -31,29 +31,16 @@ class RenderTokenParser extends \Twig_TokenParser { $expr = $this->parser->getExpressionParser()->parseExpression(); - // attributes - if ($this->parser->getStream()->test(\Twig_Token::NAME_TYPE, 'with')) { - $this->parser->getStream()->next(); - - $hasAttributes = true; - $attributes = $this->parser->getExpressionParser()->parseExpression(); - } else { - $hasAttributes = false; - $attributes = new \Twig_Node_Expression_Array(array(), $token->getLine()); - } - // options - if ($hasAttributes && $this->parser->getStream()->test(\Twig_Token::PUNCTUATION_TYPE, ',')) { + if ($this->parser->getStream()->test(\Twig_Token::PUNCTUATION_TYPE, ',')) { $this->parser->getStream()->next(); $options = $this->parser->getExpressionParser()->parseExpression(); - } else { - $options = new \Twig_Node_Expression_Array(array(), $token->getLine()); } $this->parser->getStream()->expect(\Twig_Token::BLOCK_END_TYPE); - return new RenderNode($expr, $attributes, $options, $token->getLine(), $this->getTag()); + return new RenderNode($expr, $options, $token->getLine(), $this->getTag()); } /**