[HttpKernel] made the Request required when using rendering strategies

The previous code allowed to pass null as a Request but that does not
really make sense as rendering a sub-request can only happen from a
master request. This was done to ease testing but that was a mistake.
This commit is contained in:
Fabien Potencier 2013-01-22 09:06:13 +01:00
parent 8a351f07cb
commit b9f0e17e67
12 changed files with 64 additions and 50 deletions

View File

@ -13,6 +13,7 @@ namespace Symfony\Bridge\Twig\Tests\Extension;
use Symfony\Bridge\Twig\Extension\HttpKernelExtension;
use Symfony\Bridge\Twig\Tests\TestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\HttpContentRenderer;
@ -29,13 +30,6 @@ class HttpKernelExtensionTest extends TestCase
}
}
public function testRenderWithoutMasterRequest()
{
$kernel = $this->getHttpContentRenderer($this->returnValue(new Response('foo')));
$this->assertEquals('foo', $this->renderTemplate($kernel));
}
/**
* @expectedException \Twig_Error_Runtime
*/
@ -56,7 +50,18 @@ class HttpKernelExtensionTest extends TestCase
$strategy->expects($this->once())->method('getName')->will($this->returnValue('default'));
$strategy->expects($this->once())->method('render')->will($return);
return new HttpContentRenderer(array($strategy));
// simulate a master request
$event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')->disableOriginalConstructor()->getMock();
$event
->expects($this->once())
->method('getRequest')
->will($this->returnValue(Request::create('/')))
;
$renderer = new HttpContentRenderer(array($strategy));
$renderer->onKernelRequest($event);
return $renderer;
}
protected function renderTemplate(HttpContentRenderer $renderer, $template = '{{ render("foo") }}')

View File

@ -23,7 +23,13 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
* Renders a URI using different strategies.
*
* This class handles sub-requests. The response content from the sub-request
* is then embedded into a master request. The handling of the sub-request
* is managed by rendering strategies.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @see RenderingStrategyInterface
*/
class HttpContentRenderer implements EventSubscriberInterface
{
@ -103,9 +109,7 @@ class HttpContentRenderer implements EventSubscriberInterface
throw new \InvalidArgumentException(sprintf('The "%s" rendering strategy does not exist.', $strategy));
}
$request = $this->requests ? $this->requests[0] : null;
return $this->deliver($this->strategies[$strategy]->render($uri, $request, $options));
return $this->deliver($this->strategies[$strategy]->render($uri, $this->requests[0], $options));
}
/**

View File

@ -42,7 +42,7 @@ class DefaultRenderingStrategy extends GeneratorAwareRenderingStrategy
*
* * alt: an alternative URI to render in case of an error
*/
public function render($uri, Request $request = null, array $options = array())
public function render($uri, Request $request, array $options = array())
{
if ($uri instanceof ControllerReference) {
$uri = $this->generateProxyUri($uri, $request);
@ -74,21 +74,16 @@ class DefaultRenderingStrategy extends GeneratorAwareRenderingStrategy
}
}
protected function createSubRequest($uri, Request $request = null)
protected function createSubRequest($uri, Request $request)
{
if (null !== $request) {
$cookies = $request->cookies->all();
$server = $request->server->all();
$cookies = $request->cookies->all();
$server = $request->server->all();
// the sub-request is internal
$server['REMOTE_ADDR'] = '127.0.0.1';
} else {
$cookies = array();
$server = array();
}
// the sub-request is internal
$server['REMOTE_ADDR'] = '127.0.0.1';
$subRequest = Request::create($uri, 'get', array(), $cookies, array(), $server);
if (null !== $request && $session = $request->getSession()) {
if ($session = $request->getSession()) {
$subRequest->setSession($session);
}

View File

@ -55,9 +55,9 @@ class EsiRenderingStrategy extends GeneratorAwareRenderingStrategy
*
* @see Symfony\Component\HttpKernel\HttpCache\ESI
*/
public function render($uri, Request $request = null, array $options = array())
public function render($uri, Request $request, array $options = array())
{
if (null === $request || !$this->esi->hasSurrogateEsiCapability($request)) {
if (!$this->esi->hasSurrogateEsiCapability($request)) {
return $this->defaultStrategy->render($uri, $request, $options);
}

View File

@ -50,7 +50,7 @@ abstract class GeneratorAwareRenderingStrategy implements RenderingStrategyInter
* @throws \LogicException when the _proxy route is not available
* @throws \LogicException when there is no registered route generator
*/
protected function generateProxyUri(ControllerReference $reference, Request $request = null)
protected function generateProxyUri(ControllerReference $reference, Request $request)
{
if (null === $this->generator) {
throw new \LogicException('Unable to generate a proxy URL as there is no registered route generator.');
@ -59,10 +59,8 @@ abstract class GeneratorAwareRenderingStrategy implements RenderingStrategyInter
if (isset($reference->attributes['_format'])) {
$format = $reference->attributes['_format'];
unset($reference->attributes['_format']);
} elseif (null !== $request) {
$format = $request->getRequestFormat();
} else {
$format = 'html';
$format = $request->getRequestFormat();
}
try {

View File

@ -53,7 +53,7 @@ class HIncludeRenderingStrategy extends GeneratorAwareRenderingStrategy
*
* * default: The default content (it can be a template name or the content)
*/
public function render($uri, Request $request = null, array $options = array())
public function render($uri, Request $request, array $options = array())
{
if ($uri instanceof ControllerReference) {
if (null === $this->signer) {

View File

@ -32,7 +32,7 @@ interface RenderingStrategyInterface
*
* @return Response A Response instance
*/
public function render($uri, Request $request = null, array $options = array());
public function render($uri, Request $request, array $options = array());
/**
* Gets the name of the strategy.

View File

@ -12,6 +12,7 @@
namespace Symfony\Component\HttpKernel\Tests;
use Symfony\Component\HttpKernel\HttpContentRenderer;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
class HttpContentRendererTest extends \PHPUnit_Framework_TestCase
@ -34,6 +35,8 @@ class HttpContentRendererTest extends \PHPUnit_Framework_TestCase
public function testRender()
{
$request = Request::create('/');
$strategy = $this->getMock('Symfony\Component\HttpKernel\RenderingStrategy\RenderingStrategyInterface');
$strategy
->expects($this->any())
@ -43,13 +46,22 @@ class HttpContentRendererTest extends \PHPUnit_Framework_TestCase
$strategy
->expects($this->any())
->method('render')
->with('/', null, array('foo' => 'foo', 'ignore_errors' => true))
->with('/', $request, array('foo' => 'foo', 'ignore_errors' => true))
->will($this->returnValue(new Response('foo')))
;
$renderer = new HttpContentRenderer();
$renderer->addStrategy($strategy);
// simulate a master request
$event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')->disableOriginalConstructor()->getMock();
$event
->expects($this->once())
->method('getRequest')
->will($this->returnValue(Request::create('/')))
;
$renderer->onKernelRequest($event);
$this->assertEquals('foo', $renderer->render('/', 'foo', array('foo' => 'foo')));
}

View File

@ -35,7 +35,7 @@ class DefaultRenderingStrategyTest extends AbstractRenderingStrategyTest
{
$strategy = new DefaultRenderingStrategy($this->getKernel($this->returnValue(new Response('foo'))));
$this->assertEquals('foo', $strategy->render('/')->getContent());
$this->assertEquals('foo', $strategy->render('/', Request::create('/'))->getContent());
}
public function testRenderWithControllerReference()
@ -43,7 +43,7 @@ class DefaultRenderingStrategyTest extends AbstractRenderingStrategyTest
$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()))->getContent());
$this->assertEquals('foo', $strategy->render(new ControllerReference('main_controller', array(), array()), Request::create('/'))->getContent());
}
/**
@ -53,14 +53,14 @@ class DefaultRenderingStrategyTest extends AbstractRenderingStrategyTest
{
$strategy = new DefaultRenderingStrategy($this->getKernel($this->throwException(new \RuntimeException('foo'))));
$this->assertEquals('foo', $strategy->render('/')->getContent());
$this->assertEquals('foo', $strategy->render('/', Request::create('/'))->getContent());
}
public function testRenderExceptionIgnoreErrors()
{
$strategy = new DefaultRenderingStrategy($this->getKernel($this->throwException(new \RuntimeException('foo'))));
$this->assertEmpty($strategy->render('/', null, array('ignore_errors' => true))->getContent());
$this->assertEmpty($strategy->render('/', Request::create('/'), array('ignore_errors' => true))->getContent());
}
public function testRenderExceptionIgnoreErrorsWithAlt()
@ -70,7 +70,7 @@ class DefaultRenderingStrategyTest extends AbstractRenderingStrategyTest
$this->returnValue(new Response('bar'))
)));
$this->assertEquals('bar', $strategy->render('/', null, array('ignore_errors' => true, 'alt' => '/foo'))->getContent());
$this->assertEquals('bar', $strategy->render('/', Request::create('/'), array('ignore_errors' => true, 'alt' => '/foo'))->getContent());
}
private function getKernel($returnValue)

View File

@ -32,7 +32,7 @@ class EsiRenderingStrategyTest extends AbstractRenderingStrategyTest
public function testRenderFallbackToDefaultStrategyIfNoRequest()
{
$strategy = new EsiRenderingStrategy(new Esi(), $this->getDefaultStrategy(true));
$strategy->render('/');
$strategy->render('/', Request::create('/'));
}
public function testRenderFallbackToDefaultStrategyIfEsiNotSupported()

View File

@ -32,7 +32,7 @@ class GeneratorAwareRenderingStrategyTest extends AbstractRenderingStrategyTest
public function testGenerateProxyUriWithNoGenerator()
{
$strategy = new Strategy();
$strategy->doGenerateProxyUri(new ControllerReference('controller', array(), array()));
$strategy->doGenerateProxyUri(new ControllerReference('controller', array(), array()), Request::create('/'));
}
/**
@ -49,7 +49,7 @@ class GeneratorAwareRenderingStrategyTest extends AbstractRenderingStrategyTest
$strategy = new Strategy();
$strategy->setUrlGenerator($generator);
$strategy->doGenerateProxyUri(new ControllerReference('controller', array(), array()));
$strategy->doGenerateProxyUri(new ControllerReference('controller', array(), array()), Request::create('/'));
}
/**
@ -57,7 +57,7 @@ class GeneratorAwareRenderingStrategyTest extends AbstractRenderingStrategyTest
*/
public function testGenerateProxyUri($uri, $controller)
{
$this->assertEquals($uri, $this->getStrategy()->doGenerateProxyUri($controller));
$this->assertEquals($uri, $this->getStrategy()->doGenerateProxyUri($controller, Request::create('/')));
}
public function getGeneratorProxyUriData()
@ -91,10 +91,10 @@ class GeneratorAwareRenderingStrategyTest extends AbstractRenderingStrategyTest
class Strategy extends GeneratorAwareRenderingStrategy
{
public function render($uri, Request $request = null, array $options = array()) {}
public function render($uri, Request $request, array $options = array()) {}
public function getName() {}
public function doGenerateProxyUri(ControllerReference $reference, Request $request = null)
public function doGenerateProxyUri(ControllerReference $reference, Request $request)
{
return parent::generateProxyUri($reference, $request);
}

View File

@ -35,37 +35,37 @@ class HIncludeRenderingStrategyTest extends AbstractRenderingStrategyTest
public function testRenderExceptionWhenControllerAndNoSigner()
{
$strategy = new HIncludeRenderingStrategy();
$strategy->render(new ControllerReference('main_controller', array(), array()));
$strategy->render(new ControllerReference('main_controller', array(), array()), Request::create('/'));
}
public function testRenderWithControllerAndSigner()
{
$strategy = new HIncludeRenderingStrategy(null, new UriSigner('foo'));
$strategy->setUrlGenerator($this->getUrlGenerator());
$this->assertEquals('<hx:include src="/main_controller.html?_hash=6MuxpWUHcqIddMMmoN36uPsEjws%3D"></hx:include>', $strategy->render(new ControllerReference('main_controller', array(), array()))->getContent());
$this->assertEquals('<hx:include src="/main_controller.html?_hash=6MuxpWUHcqIddMMmoN36uPsEjws%3D"></hx:include>', $strategy->render(new ControllerReference('main_controller', array(), array()), Request::create('/'))->getContent());
}
public function testRenderWithUri()
{
$strategy = new HIncludeRenderingStrategy();
$this->assertEquals('<hx:include src="/foo"></hx:include>', $strategy->render('/foo')->getContent());
$this->assertEquals('<hx:include src="/foo"></hx:include>', $strategy->render('/foo', Request::create('/'))->getContent());
$strategy = new HIncludeRenderingStrategy(null, new UriSigner('foo'));
$this->assertEquals('<hx:include src="/foo"></hx:include>', $strategy->render('/foo')->getContent());
$this->assertEquals('<hx:include src="/foo"></hx:include>', $strategy->render('/foo', Request::create('/'))->getContent());
}
public function testRenderWhithDefault()
{
// only default
$strategy = new HIncludeRenderingStrategy();
$this->assertEquals('<hx:include src="/foo">default</hx:include>', $strategy->render('/foo', null, array('default' => 'default'))->getContent());
$this->assertEquals('<hx:include src="/foo">default</hx:include>', $strategy->render('/foo', Request::create('/'), array('default' => 'default'))->getContent());
// only global default
$strategy = new HIncludeRenderingStrategy(null, null, 'global_default');
$this->assertEquals('<hx:include src="/foo">global_default</hx:include>', $strategy->render('/foo', null, array())->getContent());
$this->assertEquals('<hx:include src="/foo">global_default</hx:include>', $strategy->render('/foo', Request::create('/'), array())->getContent());
// global default and default
$strategy = new HIncludeRenderingStrategy(null, null, 'global_default');
$this->assertEquals('<hx:include src="/foo">default</hx:include>', $strategy->render('/foo', null, array('default' => 'default'))->getContent());
$this->assertEquals('<hx:include src="/foo">default</hx:include>', $strategy->render('/foo', Request::create('/'), array('default' => 'default'))->getContent());
}
}