From 35d63df044cba20cdf441963ca85a7f4d51200cc Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 13 Dec 2012 15:44:37 +0100 Subject: [PATCH 1/4] removed the dependency on the container for exception handling --- src/Symfony/Bundle/TwigBundle/CHANGELOG.md | 1 + .../Controller/ExceptionController.php | 46 +++++++++++-------- .../DependencyInjection/Configuration.php | 2 +- .../TwigBundle/Resources/config/twig.xml | 6 +++ .../Controller/ExceptionController.php | 32 ++++++++----- .../Resources/config/profiler.xml | 6 +++ .../views/Collector/exception.html.twig | 2 +- 7 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/Symfony/Bundle/TwigBundle/CHANGELOG.md b/src/Symfony/Bundle/TwigBundle/CHANGELOG.md index 01834ca16c..8c03bdbbf1 100644 --- a/src/Symfony/Bundle/TwigBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/TwigBundle/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 2.2.0 ----- + * moved the exception controller to be a service (`twig.controller.exception:showAction` vs `Symfony\\Bundle\\TwigBundle\\Controller\\ExceptionController::showAction`) * added support for multiple loaders via the "twig.loader" tag. * added automatic registration of namespaced paths for registered bundles * added support for namespaced paths diff --git a/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php b/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php index 54be66a8fc..6039fb883c 100644 --- a/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php +++ b/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php @@ -13,9 +13,9 @@ namespace Symfony\Bundle\TwigBundle\Controller; use Symfony\Bundle\FrameworkBundle\Templating\EngineInterface; use Symfony\Bundle\FrameworkBundle\Templating\TemplateReference; -use Symfony\Component\DependencyInjection\ContainerAware; use Symfony\Component\HttpKernel\Exception\FlattenException; use Symfony\Component\HttpKernel\Log\DebugLoggerInterface; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; /** @@ -23,8 +23,17 @@ use Symfony\Component\HttpFoundation\Response; * * @author Fabien Potencier */ -class ExceptionController extends ContainerAware +class ExceptionController { + protected $twig; + protected $debug; + + public function __construct(\Twig_Environment $twig, $debug) + { + $this->twig = $twig; + $this->debug = $debug; + } + /** * Converts an Exception to a Response. * @@ -36,17 +45,16 @@ class ExceptionController extends ContainerAware * * @throws \InvalidArgumentException When the exception template does not exist */ - public function showAction(FlattenException $exception, DebugLoggerInterface $logger = null, $format = 'html') + public function showAction(Request $request, FlattenException $exception, DebugLoggerInterface $logger = null, $format = 'html') { - $this->container->get('request')->setRequestFormat($format); + $request->setRequestFormat($format); - $currentContent = $this->getAndCleanOutputBuffering(); + $currentContent = $this->getAndCleanOutputBuffering($request->headers->get('X-Php-Ob-Level', -1)); - $templating = $this->container->get('templating'); $code = $exception->getStatusCode(); - return $templating->renderResponse( - $this->findTemplate($templating, $format, $code, $this->container->get('kernel')->isDebug()), + return new Response($this->twig->render( + $this->findTemplate($request, $format, $code, $this->debug), array( 'status_code' => $code, 'status_text' => isset(Response::$statusTexts[$code]) ? Response::$statusTexts[$code] : '', @@ -54,19 +62,17 @@ class ExceptionController extends ContainerAware 'logger' => $logger, 'currentContent' => $currentContent, ) - ); + )); } /** * @return string */ - protected function getAndCleanOutputBuffering() + protected function getAndCleanOutputBuffering($startObLevel) { // ob_get_level() never returns 0 on some Windows configurations, so if // the level is the same two times in a row, the loop should be stopped. $previousObLevel = null; - $startObLevel = $this->container->get('request')->headers->get('X-Php-Ob-Level', -1); - $currentContent = ''; while (($obLevel = ob_get_level()) > $startObLevel && $obLevel !== $previousObLevel) { @@ -78,14 +84,14 @@ class ExceptionController extends ContainerAware } /** - * @param EngineInterface $templating - * @param string $format - * @param integer $code An HTTP response status code - * @param Boolean $debug + * @param Request $request + * @param string $format + * @param integer $code An HTTP response status code + * @param Boolean $debug * * @return TemplateReference */ - protected function findTemplate($templating, $format, $code, $debug) + protected function findTemplate(Request $request, $format, $code, $debug) { $name = $debug ? 'exception' : 'error'; if ($debug && 'html' == $format) { @@ -95,19 +101,19 @@ class ExceptionController extends ContainerAware // when not in debug, try to find a template for the specific HTTP status code and format if (!$debug) { $template = new TemplateReference('TwigBundle', 'Exception', $name.$code, $format, 'twig'); - if ($templating->exists($template)) { + if ($this->twig->getLoader()->exists($template)) { return $template; } } // try to find a template for the given format $template = new TemplateReference('TwigBundle', 'Exception', $name, $format, 'twig'); - if ($templating->exists($template)) { + if ($this->twig->getLoader()->exists($template)) { return $template; } // default to a generic HTML exception - $this->container->get('request')->setRequestFormat('html'); + $request->setRequestFormat('html'); return new TemplateReference('TwigBundle', 'Exception', $name, 'html', 'twig'); } diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php index 3b3c8ed6e4..c327037e2b 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php @@ -34,7 +34,7 @@ class Configuration implements ConfigurationInterface $rootNode ->children() - ->scalarNode('exception_controller')->defaultValue('Symfony\\Bundle\\TwigBundle\\Controller\\ExceptionController::showAction')->end() + ->scalarNode('exception_controller')->defaultValue('twig.controller.exception:showAction')->end() ->end() ; diff --git a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml index d54035b425..5b62b01c24 100644 --- a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml +++ b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml @@ -21,6 +21,7 @@ Symfony\Bridge\Twig\Form\TwigRenderer Symfony\Bridge\Twig\Translation\TwigExtractor Symfony\Component\HttpKernel\EventListener\ExceptionListener + Symfony\Bundle\TwigBundle\Controller\ExceptionController @@ -111,5 +112,10 @@ %twig.exception_listener.controller% + + + + %kernel.debug% + diff --git a/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php b/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php index f646d50a4c..0ff7492b7c 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php @@ -12,27 +12,37 @@ namespace Symfony\Bundle\WebProfilerBundle\Controller; use Symfony\Component\HttpKernel\Exception\FlattenException; -use Symfony\Component\HttpKernel\Log\DebugLoggerInterface; use Symfony\Component\HttpFoundation\Response; -use Symfony\Bundle\TwigBundle\Controller\ExceptionController as BaseExceptionController; /** * ExceptionController. * * @author Fabien Potencier */ -class ExceptionController extends BaseExceptionController +class ExceptionController { - /** - * {@inheritdoc} - */ - public function showAction(FlattenException $exception, DebugLoggerInterface $logger = null, $format = 'html') + protected $twig; + protected $debug; + + public function __construct(\Twig_Environment $twig, $debug) + { + $this->twig = $twig; + $this->debug = $debug; + } + + /** + * Converts an Exception to a Response. + * + * @param FlattenException $exception A FlattenException instance + * + * @return Response + */ + public function showAction(FlattenException $exception) { - $template = $this->container->get('kernel')->isDebug() ? 'exception' : 'error'; $code = $exception->getStatusCode(); - return $this->container->get('templating')->renderResponse( - 'TwigBundle:Exception:'.$template.'.html.twig', + return new Response($this->twig->render( + '@Twig/Exception/'.($this->debug ? 'exception' : 'error').'.html.twig', array( 'status_code' => $code, 'status_text' => Response::$statusTexts[$code], @@ -40,6 +50,6 @@ class ExceptionController extends BaseExceptionController 'logger' => null, 'currentContent' => '', ) - ); + )); } } diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml index c06f0dbd21..02aa6bd3e3 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml @@ -7,6 +7,7 @@ Symfony\Bundle\WebProfilerBundle\Controller\ProfilerController Symfony\Bundle\WebProfilerBundle\Controller\RouterController + Symfony\Bundle\WebProfilerBundle\Controller\ExceptionController @@ -23,5 +24,10 @@ + + + + %kernel.debug% + diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig index 6dbfef293c..b498188be5 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig @@ -28,7 +28,7 @@

{% else %}
- {% render 'WebProfilerBundle:Exception:show' with { 'exception': collector.exception, 'format': 'html' } %} + {% render 'web_profiler.controller.exception:showAction' with { 'exception': collector.exception, 'format': 'html' } %}
{% endif %} {% endblock %} From f0056493153a8958c572ac30a32f2ca2c80a3c9f Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 13 Dec 2012 16:01:47 +0100 Subject: [PATCH 2/4] [WebProfilerBundle] decoupled the bundle from TwigBundle --- .../Controller/ExceptionController.php | 55 +++++++++++++++++-- .../Resources/config/profiler.xml | 1 + .../views/Collector/exception.html.twig | 4 +- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php b/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php index 0ff7492b7c..170d61a251 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php @@ -12,6 +12,8 @@ namespace Symfony\Bundle\WebProfilerBundle\Controller; use Symfony\Component\HttpKernel\Exception\FlattenException; +use Symfony\Component\HttpKernel\Profiler\Profiler; +use Symfony\Component\HttpKernel\Debug\ExceptionHandler; use Symfony\Component\HttpFoundation\Response; /** @@ -23,26 +25,39 @@ class ExceptionController { protected $twig; protected $debug; + protected $profiler; - public function __construct(\Twig_Environment $twig, $debug) + public function __construct(Profiler $profiler, \Twig_Environment $twig, $debug) { + $this->profiler = $profiler; $this->twig = $twig; $this->debug = $debug; } /** - * Converts an Exception to a Response. + * Renders the exception panel for the given token. * - * @param FlattenException $exception A FlattenException instance + * @param string $token The profiler token * - * @return Response + * @return Response A Response instance */ - public function showAction(FlattenException $exception) + public function showAction($token) { + $this->profiler->disable(); + + $exception = $this->profiler->loadProfile($token)->getCollector('exception')->getException(); + $template = $this->getTemplate(); + + if (!$this->twig->getLoader()->exists($template)) { + $handler = new ExceptionHandler(); + + return new Response($handler->getContent($exception)); + } + $code = $exception->getStatusCode(); return new Response($this->twig->render( - '@Twig/Exception/'.($this->debug ? 'exception' : 'error').'.html.twig', + $template, array( 'status_code' => $code, 'status_text' => Response::$statusTexts[$code], @@ -52,4 +67,32 @@ class ExceptionController ) )); } + + /** + * Renders the exception panel stylesheet for the given token. + * + * @param string $token The profiler token + * + * @return Response A Response instance + */ + public function cssAction($token) + { + $this->profiler->disable(); + + $exception = $this->profiler->loadProfile($token)->getCollector('exception')->getException(); + $template = $this->getTemplate(); + + if (!$this->twig->getLoader()->exists($template)) { + $handler = new ExceptionHandler(); + + return new Response($handler->getStylesheet($exception)); + } + + return new Response($this->twig->render('@WebProfiler/Collector/exception.css.twig')); + } + + protected function getTemplate() + { + return '@Twig/Exception/'.($this->debug ? 'exception' : 'error').'.html.twig'; + } } diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml index 02aa6bd3e3..22d12409cd 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml @@ -26,6 +26,7 @@ + %kernel.debug% diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig index b498188be5..fa1e75ec96 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig @@ -2,7 +2,7 @@ {% block head %} {{ parent() }} {% endblock %} @@ -28,7 +28,7 @@

{% else %}
- {% render 'web_profiler.controller.exception:showAction' with { 'exception': collector.exception, 'format': 'html' } %} + {% render 'web_profiler.controller.exception:showAction' with { 'token': token } %}
{% endif %} {% endblock %} From fc444f1a55c449ac672be9420eff3c1ca131be6d Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 13 Dec 2012 17:11:23 +0100 Subject: [PATCH 3/4] fixed support for Twig loaders when they do not extend Twig_ExistsLoaderInterface --- .../Controller/ExceptionController.php | 22 +++++++++++++++++-- .../Controller/ExceptionController.php | 20 ++++++++++++++++- .../Profiler/TemplateManager.php | 20 ++++++++++++++++- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php b/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php index 6039fb883c..8c07b90b91 100644 --- a/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php +++ b/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php @@ -101,14 +101,14 @@ class ExceptionController // when not in debug, try to find a template for the specific HTTP status code and format if (!$debug) { $template = new TemplateReference('TwigBundle', 'Exception', $name.$code, $format, 'twig'); - if ($this->twig->getLoader()->exists($template)) { + if ($this->templateExists($template)) { return $template; } } // try to find a template for the given format $template = new TemplateReference('TwigBundle', 'Exception', $name, $format, 'twig'); - if ($this->twig->getLoader()->exists($template)) { + if ($this->templateExists($template)) { return $template; } @@ -117,4 +117,22 @@ class ExceptionController return new TemplateReference('TwigBundle', 'Exception', $name, 'html', 'twig'); } + + // to be removed when the minimum required version of Twig is >= 2.0 + protected function templateExists($template) + { + $loader = $this->twig->getLoader(); + if ($loader instanceof \Twig_ExistsLoaderInterface && $loader->exists($template)) { + return true; + } + + try { + $loader->getSource($template); + + return true; + } catch (Twig_Error_Loader $e) { + } + + return false; + } } diff --git a/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php b/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php index 170d61a251..9a760c9f32 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php @@ -82,7 +82,7 @@ class ExceptionController $exception = $this->profiler->loadProfile($token)->getCollector('exception')->getException(); $template = $this->getTemplate(); - if (!$this->twig->getLoader()->exists($template)) { + if (!$this->templateExists($template)) { $handler = new ExceptionHandler(); return new Response($handler->getStylesheet($exception)); @@ -95,4 +95,22 @@ class ExceptionController { return '@Twig/Exception/'.($this->debug ? 'exception' : 'error').'.html.twig'; } + + // to be removed when the minimum required version of Twig is >= 2.0 + protected function templateExists($template) + { + $loader = $this->twig->getLoader(); + if ($loader instanceof \Twig_ExistsLoaderInterface && $loader->exists($template)) { + return true; + } + + try { + $loader->getSource($template); + + return true; + } catch (Twig_Error_Loader $e) { + } + + return false; + } } diff --git a/src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php b/src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php index da59efb57b..87842dbdbc 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php @@ -107,7 +107,7 @@ class TemplateManager $template = substr($template, 0, -10); } - if (!$this->twig->getLoader()->exists($template.'.html.twig')) { + if (!$this->templateExists($template.'.html.twig')) { throw new \UnexpectedValueException(sprintf('The profiler template "%s.html.twig" for data collector "%s" does not exist.', $template, $name)); } @@ -116,4 +116,22 @@ class TemplateManager return $templates; } + + // to be removed when the minimum required version of Twig is >= 2.0 + protected function templateExists($template) + { + $loader = $this->twig->getLoader(); + if ($loader instanceof \Twig_ExistsLoaderInterface && $loader->exists($template)) { + return true; + } + + try { + $loader->getSource($template); + + return true; + } catch (Twig_Error_Loader $e) { + } + + return false; + } } From 142cffbc4f8d4cfaa206b826014afc72120444e0 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 13 Dec 2012 17:59:38 +0100 Subject: [PATCH 4/4] fixed unit tests --- .../Controller/ExceptionControllerTest.php | 67 ++++--------------- .../Tests/Profiler/TemplateManagerTest.php | 7 +- 2 files changed, 19 insertions(+), 55 deletions(-) diff --git a/src/Symfony/Bundle/TwigBundle/Tests/Controller/ExceptionControllerTest.php b/src/Symfony/Bundle/TwigBundle/Tests/Controller/ExceptionControllerTest.php index 394008332e..18523eaa76 100644 --- a/src/Symfony/Bundle/TwigBundle/Tests/Controller/ExceptionControllerTest.php +++ b/src/Symfony/Bundle/TwigBundle/Tests/Controller/ExceptionControllerTest.php @@ -12,72 +12,33 @@ namespace Symfony\Bundle\TwigBundle\Tests\Controller; use Symfony\Bundle\TwigBundle\Tests\TestCase; - use Symfony\Bundle\TwigBundle\Controller\ExceptionController; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Scope; use Symfony\Component\HttpFoundation\Request; class ExceptionControllerTest extends TestCase { - protected $controller; - protected $container; - protected $flatten; - protected $templating; - protected $kernel; - - protected function setUp() + public function testOnlyClearOwnOutputBuffers() { - parent::setUp(); - - $this->flatten = $this->getMock('Symfony\Component\HttpKernel\Exception\FlattenException'); - $this->flatten + $flatten = $this->getMock('Symfony\Component\HttpKernel\Exception\FlattenException'); + $flatten ->expects($this->once()) ->method('getStatusCode') ->will($this->returnValue(404)); - $this->controller = new ExceptionController(); - $this->kernel = $this->getMock('Symfony\\Component\\HttpKernel\\KernelInterface'); - $this->templating = $this->getMockBuilder('Symfony\\Bundle\\TwigBundle\\TwigEngine') + $twig = $this->getMockBuilder('\Twig_Environment') ->disableOriginalConstructor() ->getMock(); - $this->templating + $twig ->expects($this->any()) - ->method('renderResponse') + ->method('render') ->will($this->returnValue($this->getMock('Symfony\Component\HttpFoundation\Response'))); - $this->request = Request::create('/'); - $this->container = $this->getContainer(); - } + $twig + ->expects($this->any()) + ->method('getLoader') + ->will($this->returnValue($this->getMock('\Twig_LoaderInterface'))); + $request = Request::create('/'); + $request->headers->set('X-Php-Ob-Level', 1); - protected function tearDown() - { - parent::tearDown(); - - $this->controller = null; - $this->container = null; - $this->flatten = null; - $this->templating = null; - $this->kernel = null; - } - - public function testOnlyClearOwnOutputBuffers() - { - $this->request->headers->set('X-Php-Ob-Level', 1); - - $this->controller->setContainer($this->container); - $this->controller->showAction($this->flatten); - } - - private function getContainer() - { - $container = new ContainerBuilder(); - $container->addScope(new Scope('request')); - $container->set('request', $this->request); - $container->set('templating', $this->templating); - $container->setParameter('kernel.bundles', array()); - $container->setParameter('kernel.cache_dir', __DIR__); - $container->setParameter('kernel.root_dir', __DIR__); - $container->set('kernel', $this->kernel); - - return $container; + $controller = new ExceptionController($twig, false); + $controller->showAction($request, $flatten); } } diff --git a/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php b/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php index eacfbe9a6d..488f2eadb8 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php @@ -80,7 +80,7 @@ class TemplateManagerTest extends TestCase ->method('hasCollector') ->will($this->returnCallback(array($this, 'profileHasCollectorCallback'))); - $this->assertEquals('FooBundle:Collector:foo.html.twig', $this->templateManager->getName($profile, 'foo')); + $this->assertEquals('FooBundle:Collector:foo.html.twig', $this->templateManager->getName($profile, 'foo')); } /** @@ -89,7 +89,6 @@ class TemplateManagerTest extends TestCase */ public function testGetTemplates() { - $profile = $this->mockProfile(); $profile->expects($this->any()) ->method('hasCollector') @@ -145,6 +144,10 @@ class TemplateManagerTest extends TestCase ->method('loadTemplate') ->will($this->returnValue('loadedTemplate')); + $this->twigEnvironment->expects($this->any()) + ->method('getLoader') + ->will($this->returnValue($this->getMock('\Twig_LoaderInterface'))); + return $this->twigEnvironment; }