From aadefd378d594795471d3eae1595e6f8c524c762 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 20 Jan 2013 10:04:47 +0100 Subject: [PATCH] [HttpKernel] refactored the HTTP content renderer to make it easier to extend --- .../Extension/HttpKernelExtensionTest.php | 3 +- .../HttpKernel/HttpContentRenderer.php | 35 ++++++++++++++++--- .../DefaultRenderingStrategy.php | 18 ++-------- .../EsiRenderingStrategy.php | 5 ++- .../HIncludeRenderingStrategy.php | 3 +- .../RenderingStrategyInterface.php | 5 +-- .../Tests/HttpContentRendererTest.php | 3 +- .../DefaultRenderingStrategyTest.php | 10 +++--- .../EsiRenderingStrategyTest.php | 8 ++--- .../HIncludeRenderingStrategyTest.php | 12 +++---- 10 files changed, 60 insertions(+), 42 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php index 182c42d07715e..489d36bfb2051 100644 --- a/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php @@ -13,6 +13,7 @@ use Symfony\Bridge\Twig\Extension\HttpKernelExtension; use Symfony\Bridge\Twig\Tests\TestCase; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\HttpContentRenderer; class HttpKernelExtensionTest extends TestCase @@ -30,7 +31,7 @@ protected function setUp() public function testRenderWithoutMasterRequest() { - $kernel = $this->getHttpContentRenderer($this->returnValue('foo')); + $kernel = $this->getHttpContentRenderer($this->returnValue(new Response('foo'))); $this->assertEquals('foo', $this->renderTemplate($kernel)); } diff --git a/src/Symfony/Component/HttpKernel/HttpContentRenderer.php b/src/Symfony/Component/HttpKernel/HttpContentRenderer.php index ca35cd711ffd5..34b18e13d7490 100644 --- a/src/Symfony/Component/HttpKernel/HttpContentRenderer.php +++ b/src/Symfony/Component/HttpKernel/HttpContentRenderer.php @@ -12,6 +12,8 @@ namespace Symfony\Component\HttpKernel; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\StreamedResponse; use Symfony\Component\HttpKernel\Controller\ControllerReference; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; @@ -78,9 +80,6 @@ public function onKernelResponse(FilterResponseEvent $event) /** * Renders a URI and returns the Response content. * - * When the Response is a StreamedResponse, the content is streamed immediately - * instead of being returned. - * * Available options: * * * ignore_errors: true to return an empty string in case of an error @@ -92,6 +91,7 @@ public function onKernelResponse(FilterResponseEvent $event) * @return string|null The Response content or null when the Response is streamed * * @throws \InvalidArgumentException when the strategy does not exist + * @throws \RuntimeException when the Response is not successful */ public function render($uri, $strategy = 'default', array $options = array()) { @@ -103,7 +103,34 @@ public function render($uri, $strategy = 'default', array $options = array()) throw new \InvalidArgumentException(sprintf('The "%s" rendering strategy does not exist.', $strategy)); } - return $this->strategies[$strategy]->render($uri, $this->requests ? $this->requests[0] : null, $options); + $request = $this->requests ? $this->requests[0] : null; + + return $this->deliver($this->strategies[$strategy]->render($uri, $request, $options)); + } + + /** + * Delivers the Response as a string. + * + * When the Response is a StreamedResponse, the content is streamed immediately + * instead of being returned. + * + * @param Response $response A Response instance + * + * @return string|null The Response content or null when the Response is streamed + * + * @throws \RuntimeException when the Response is not successful + */ + protected function deliver(Response $response) + { + if (!$response->isSuccessful()) { + throw new \RuntimeException(sprintf('Error when rendering "%s" (Status code is %s).', $request->getUri(), $response->getStatusCode())); + } + + if (!$response instanceof StreamedResponse) { + return $response->getContent(); + } + + $response->sendContent(); } public static function getSubscribedEvents() diff --git a/src/Symfony/Component/HttpKernel/RenderingStrategy/DefaultRenderingStrategy.php b/src/Symfony/Component/HttpKernel/RenderingStrategy/DefaultRenderingStrategy.php index c0820b8504b3d..e1c737a05dc63 100644 --- a/src/Symfony/Component/HttpKernel/RenderingStrategy/DefaultRenderingStrategy.php +++ b/src/Symfony/Component/HttpKernel/RenderingStrategy/DefaultRenderingStrategy.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpKernel\RenderingStrategy; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Controller\ControllerReference; @@ -51,7 +52,7 @@ public function render($uri, Request $request = null, array $options = array()) $level = ob_get_level(); try { - return $this->handle($subRequest); + return $this->kernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST, false); } catch (\Exception $e) { // let's clean up the output buffers that were created by the sub-request while (ob_get_level() > $level) { @@ -68,22 +69,9 @@ public function render($uri, Request $request = null, array $options = array()) if (!isset($options['ignore_errors']) || !$options['ignore_errors']) { throw $e; } - } - } - - protected function handle(Request $request) - { - $response = $this->kernel->handle($request, HttpKernelInterface::SUB_REQUEST, false); - if (!$response->isSuccessful()) { - throw new \RuntimeException(sprintf('Error when rendering "%s" (Status code is %s).', $request->getUri(), $response->getStatusCode())); + return new Response(); } - - if (!$response instanceof StreamedResponse) { - return $response->getContent(); - } - - $response->sendContent(); } protected function createSubRequest($uri, Request $request = null) diff --git a/src/Symfony/Component/HttpKernel/RenderingStrategy/EsiRenderingStrategy.php b/src/Symfony/Component/HttpKernel/RenderingStrategy/EsiRenderingStrategy.php index f77669f484c62..e1ecc8a8cb1d3 100644 --- a/src/Symfony/Component/HttpKernel/RenderingStrategy/EsiRenderingStrategy.php +++ b/src/Symfony/Component/HttpKernel/RenderingStrategy/EsiRenderingStrategy.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpKernel\RenderingStrategy; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Controller\ControllerReference; use Symfony\Component\HttpKernel\HttpCache\Esi; @@ -69,7 +70,9 @@ public function render($uri, Request $request = null, array $options = array()) $alt = $this->generateProxyUri($alt, $request); } - return $this->esi->renderIncludeTag($uri, $alt, isset($options['ignore_errors']) ? $options['ignore_errors'] : false, isset($options['comment']) ? $options['comment'] : ''); + $tag = $this->esi->renderIncludeTag($uri, $alt, isset($options['ignore_errors']) ? $options['ignore_errors'] : false, isset($options['comment']) ? $options['comment'] : ''); + + return new Response($tag); } /** diff --git a/src/Symfony/Component/HttpKernel/RenderingStrategy/HIncludeRenderingStrategy.php b/src/Symfony/Component/HttpKernel/RenderingStrategy/HIncludeRenderingStrategy.php index 82abf3f71f248..cdcb6acee9109 100644 --- a/src/Symfony/Component/HttpKernel/RenderingStrategy/HIncludeRenderingStrategy.php +++ b/src/Symfony/Component/HttpKernel/RenderingStrategy/HIncludeRenderingStrategy.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpKernel\RenderingStrategy; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Templating\EngineInterface; use Symfony\Component\HttpKernel\Controller\ControllerReference; use Symfony\Component\HttpKernel\UriSigner; @@ -69,7 +70,7 @@ public function render($uri, Request $request = null, array $options = array()) $content = $template; } - return sprintf('%s', $uri, $content); + return new Response(sprintf('%s', $uri, $content)); } private function templateExists($template) diff --git a/src/Symfony/Component/HttpKernel/RenderingStrategy/RenderingStrategyInterface.php b/src/Symfony/Component/HttpKernel/RenderingStrategy/RenderingStrategyInterface.php index 36419c3792f94..fd795445e0d43 100644 --- a/src/Symfony/Component/HttpKernel/RenderingStrategy/RenderingStrategyInterface.php +++ b/src/Symfony/Component/HttpKernel/RenderingStrategy/RenderingStrategyInterface.php @@ -26,14 +26,11 @@ interface RenderingStrategyInterface /** * Renders a URI and returns the Response content. * - * When the Response is a StreamedResponse, the content is streamed immediately - * instead of being returned. - * * @param string|ControllerReference $uri A URI as a string or a ControllerReference instance * @param Request $request A Request instance * @param array $options An array of options * - * @return string|null The Response content or null when the Response is streamed + * @return Response A Response instance */ public function render($uri, Request $request = null, array $options = array()); diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpContentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpContentRendererTest.php index ff0d4cbdb0776..f97bf51451ede 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpContentRendererTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpContentRendererTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpKernel\Tests; use Symfony\Component\HttpKernel\HttpContentRenderer; +use Symfony\Component\HttpFoundation\Response; class HttpContentRendererTest extends \PHPUnit_Framework_TestCase { @@ -43,7 +44,7 @@ public function testRender() ->expects($this->any()) ->method('render') ->with('/', null, array('foo' => 'foo', 'ignore_errors' => true)) - ->will($this->returnValue('foo')) + ->will($this->returnValue(new Response('foo'))) ; $renderer = new HttpContentRenderer(); diff --git a/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/DefaultRenderingStrategyTest.php b/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/DefaultRenderingStrategyTest.php index 3c55c5905ce22..8a208f48e389f 100644 --- a/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/DefaultRenderingStrategyTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/DefaultRenderingStrategyTest.php @@ -35,7 +35,7 @@ public function testRender() { $strategy = new DefaultRenderingStrategy($this->getKernel($this->returnValue(new Response('foo')))); - $this->assertEquals('foo', $strategy->render('/')); + $this->assertEquals('foo', $strategy->render('/')->getContent()); } public function testRenderWithControllerReference() @@ -43,7 +43,7 @@ public function testRenderWithControllerReference() $strategy = new DefaultRenderingStrategy($this->getKernel($this->returnValue(new Response('foo')))); $strategy->setUrlGenerator($this->getUrlGenerator()); - $this->assertEquals('foo', $strategy->render(new ControllerReference('main_controller', array(), array()))); + $this->assertEquals('foo', $strategy->render(new ControllerReference('main_controller', array(), array()))->getContent()); } /** @@ -53,14 +53,14 @@ public function testRenderExceptionNoIgnoreErrors() { $strategy = new DefaultRenderingStrategy($this->getKernel($this->throwException(new \RuntimeException('foo')))); - $this->assertEquals('foo', $strategy->render('/')); + $this->assertEquals('foo', $strategy->render('/')->getContent()); } public function testRenderExceptionIgnoreErrors() { $strategy = new DefaultRenderingStrategy($this->getKernel($this->throwException(new \RuntimeException('foo')))); - $this->assertNull($strategy->render('/', null, array('ignore_errors' => true))); + $this->assertEmpty($strategy->render('/', null, array('ignore_errors' => true))->getContent()); } public function testRenderExceptionIgnoreErrorsWithAlt() @@ -70,7 +70,7 @@ public function testRenderExceptionIgnoreErrorsWithAlt() $this->returnValue(new Response('bar')) ))); - $this->assertEquals('bar', $strategy->render('/', null, array('ignore_errors' => true, 'alt' => '/foo'))); + $this->assertEquals('bar', $strategy->render('/', null, array('ignore_errors' => true, 'alt' => '/foo'))->getContent()); } private function getKernel($returnValue) diff --git a/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/EsiRenderingStrategyTest.php b/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/EsiRenderingStrategyTest.php index 513e30039c037..cea8aeee5d40a 100644 --- a/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/EsiRenderingStrategyTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/EsiRenderingStrategyTest.php @@ -49,10 +49,10 @@ public function testRender() $request = Request::create('/'); $request->headers->set('Surrogate-Capability', 'ESI/1.0'); - $this->assertEquals('', $strategy->render('/', $request)); - $this->assertEquals("\n", $strategy->render('/', $request, array('comment' => 'This is a comment'))); - $this->assertEquals('', $strategy->render('/', $request, array('alt' => 'foo'))); - $this->assertEquals('', $strategy->render(new ControllerReference('main_controller', array(), array()), $request, array('alt' => new ControllerReference('alt_controller', array(), array())))); + $this->assertEquals('', $strategy->render('/', $request)->getContent()); + $this->assertEquals("\n", $strategy->render('/', $request, array('comment' => 'This is a comment'))->getContent()); + $this->assertEquals('', $strategy->render('/', $request, array('alt' => 'foo'))->getContent()); + $this->assertEquals('', $strategy->render(new ControllerReference('main_controller', array(), array()), $request, array('alt' => new ControllerReference('alt_controller', array(), array())))->getContent()); } private function getDefaultStrategy($called = false) diff --git a/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/HIncludeRenderingStrategyTest.php b/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/HIncludeRenderingStrategyTest.php index ecc99665f8c70..8e81b3be2d415 100644 --- a/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/HIncludeRenderingStrategyTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/RenderingStrategy/HIncludeRenderingStrategyTest.php @@ -42,30 +42,30 @@ public function testRenderWithControllerAndSigner() { $strategy = new HIncludeRenderingStrategy(null, new UriSigner('foo')); $strategy->setUrlGenerator($this->getUrlGenerator()); - $this->assertEquals('', $strategy->render(new ControllerReference('main_controller', array(), array()))); + $this->assertEquals('', $strategy->render(new ControllerReference('main_controller', array(), array()))->getContent()); } public function testRenderWithUri() { $strategy = new HIncludeRenderingStrategy(); - $this->assertEquals('', $strategy->render('/foo')); + $this->assertEquals('', $strategy->render('/foo')->getContent()); $strategy = new HIncludeRenderingStrategy(null, new UriSigner('foo')); - $this->assertEquals('', $strategy->render('/foo')); + $this->assertEquals('', $strategy->render('/foo')->getContent()); } public function testRenderWhithDefault() { // only default $strategy = new HIncludeRenderingStrategy(); - $this->assertEquals('default', $strategy->render('/foo', null, array('default' => 'default'))); + $this->assertEquals('default', $strategy->render('/foo', null, array('default' => 'default'))->getContent()); // only global default $strategy = new HIncludeRenderingStrategy(null, null, 'global_default'); - $this->assertEquals('global_default', $strategy->render('/foo', null, array())); + $this->assertEquals('global_default', $strategy->render('/foo', null, array())->getContent()); // global default and default $strategy = new HIncludeRenderingStrategy(null, null, 'global_default'); - $this->assertEquals('default', $strategy->render('/foo', null, array('default' => 'default'))); + $this->assertEquals('default', $strategy->render('/foo', null, array('default' => 'default'))->getContent()); } }