Skip to content

[HttpKernel] Allow to inject PSR-7 ServerRequest in controllers and to return a PSR-7 response. #14751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
"homepage": "https://symfony.com/contributors"
}
],
"repositories": [
{
"type": "vcs",
"url": "https://github.com/dunglas/psr-http-message-bridge"
}
],
"require": {
"php": ">=5.3.9",
"symfony/asset": "~2.7|~3.0.0",
Expand Down Expand Up @@ -46,7 +52,9 @@
"symfony/expression-language": "~2.6|~3.0.0",
"symfony/process": "~2.0,>=2.0.5|~3.0.0",
"symfony/validator": "~2.5|~3.0.0",
"symfony/yaml": "~2.0,>=2.0.5|~3.0.0"
"symfony/yaml": "~2.0,>=2.0.5|~3.0.0",
"symfony/psr-http-message-bridge": "dev-wip",
"zendframework/zend-diactoros": "~1.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those 2 deps are actually soft ones, not hard ones.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in the require-dev section (just for tests).

},
"suggest": {
"symfony/console": "For using the console commands",
Expand Down
58 changes: 44 additions & 14 deletions src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
namespace Symfony\Component\HttpKernel\Controller;

use Psr\Log\LoggerInterface;
use Symfony\Bridge\PsrHttpMessage\Factory\DiactorosFactory;
use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface;
use Symfony\Component\HttpFoundation\Request;

/**
Expand All @@ -22,21 +24,27 @@
* the controller method arguments.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Kévin Dunglas <dunglas@gmail.com>
*
* @api
*/
class ControllerResolver implements ControllerResolverInterface
{
private $logger;
private $httpMessageFactory;

/**
* Constructor.
*
* @param LoggerInterface $logger A LoggerInterface instance
*/
public function __construct(LoggerInterface $logger = null)
public function __construct(LoggerInterface $logger = null, HttpMessageFactoryInterface $httpMessageFactory = null)
{
$this->logger = $logger;

if (null === $httpMessageFactory && class_exists('Symfony\Bridge\PsrHttpMessage\Factory\DiactorosFactory')) {
$this->httpMessageFactory = new DiactorosFactory();
}
}

/**
Expand Down Expand Up @@ -112,21 +120,43 @@ protected function doGetArguments(Request $request, $controller, array $paramete
foreach ($parameters as $param) {
if (array_key_exists($param->name, $attributes)) {
$arguments[] = $attributes[$param->name];
} elseif ($param->getClass() && $param->getClass()->isInstance($request)) {
$arguments[] = $request;
} elseif ($param->isDefaultValueAvailable()) {
$arguments[] = $param->getDefaultValue();
} else {
if (is_array($controller)) {
$repr = sprintf('%s::%s()', get_class($controller[0]), $controller[1]);
} elseif (is_object($controller)) {
$repr = get_class($controller);
} else {
$repr = $controller;

continue;
}

if ($class = $param->getClass()) {
if ($class->isInstance($request)) {
$arguments[] = $request;

continue;
}

throw new \RuntimeException(sprintf('Controller "%s" requires that you provide a value for the "$%s" argument (because there is no default value or because there is a non optional argument after this one).', $repr, $param->name));
if ($class->implementsInterface('Psr\Http\Message\ServerRequestInterface')) {
if (null === $this->httpMessageFactory) {
throw new \RuntimeException('The PSR-7 Bridge must be installed to inject instances of Psr\Http\Message\ServerRequestInterface in controllers.');
}

$arguments[] = $this->httpMessageFactory->createRequest($request);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how this works. The Symfony Request won't be in the RequestStack.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand your concern well. The Symfony request is added to the request stack here: https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/HttpKernel/HttpKernel.php#L124

So it is already in the stack when it is converted to a PSR request here.


continue;
}
}

if ($param->isDefaultValueAvailable()) {
$arguments[] = $param->getDefaultValue();

continue;
}

if (is_array($controller)) {
$repr = sprintf('%s::%s()', get_class($controller[0]), $controller[1]);
} elseif (is_object($controller)) {
$repr = get_class($controller);
} else {
$repr = $controller;
}

throw new \RuntimeException(sprintf('Controller "%s" requires that you provide a value for the "$%s" argument (because there is no default value or because there is a non optional argument after this one).', $repr, $param->name));
}

return $arguments;
Expand Down Expand Up @@ -157,7 +187,7 @@ protected function createController($controller)
}

/**
* Returns an instantiated controller
* Returns an instantiated controller.
*
* @param string $class A class name
*
Expand Down
18 changes: 17 additions & 1 deletion src/Symfony/Component/HttpKernel/HttpKernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\HttpKernel;

use Psr\Http\Message\ResponseInterface;
use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory;
use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
Expand All @@ -35,6 +37,7 @@
*/
class HttpKernel implements HttpKernelInterface, TerminableInterface
{
private $httpFoundationFactory;
protected $dispatcher;
protected $resolver;
protected $requestStack;
Expand All @@ -48,11 +51,15 @@ class HttpKernel implements HttpKernelInterface, TerminableInterface
*
* @api
*/
public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver, RequestStack $requestStack = null)
public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver, RequestStack $requestStack = null, HttpFoundationFactory $httpFoundationFactory = null)
{
$this->dispatcher = $dispatcher;
$this->resolver = $resolver;
$this->requestStack = $requestStack ?: new RequestStack();

if (null === $httpFoundationFactory && class_exists('Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory')) {
$this->httpFoundationFactory = new HttpFoundationFactory();
}
}

/**
Expand Down Expand Up @@ -146,6 +153,15 @@ private function handleRaw(Request $request, $type = self::MASTER_REQUEST)
// call controller
$response = call_user_func_array($controller, $arguments);

// handle PSR-7 responses
if ($response instanceof ResponseInterface) {
if (null === $this->httpFoundationFactory) {
throw new \RuntimeException('The PSR-7 Bridge must be installed to handle instances of Psr\Http\Message\ResponseInterface.');
}

$response = $this->httpFoundationFactory->createResponse($response);
}

// view
if (!$response instanceof Response) {
$event = new GetResponseForControllerResultEvent($this, $request, $type, $response);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\HttpKernel\Tests\Controller;

use Psr\Http\Message\ServerRequestInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Controller\ControllerResolver;
use Symfony\Component\HttpFoundation\Request;
Expand Down Expand Up @@ -195,6 +196,11 @@ public function testGetArguments()
$request = Request::create('/');
$controller = array(new self(), 'controllerMethod5');
$this->assertEquals(array($request), $resolver->getArguments($request, $controller), '->getArguments() injects the request');

$request = Request::create('/');
$controller = array(new self(), 'controllerMethod6');
$args = $resolver->getArguments($request, $controller);
$this->assertInstanceOf('Psr\Http\Message\ServerRequestInterface', $args[0], '->getArguments() injects the PSR ServerRequest');
}

public function testCreateControllerCanReturnAnyCallable()
Expand Down Expand Up @@ -235,6 +241,10 @@ protected static function controllerMethod4()
protected function controllerMethod5(Request $request)
{
}

protected function controllerMethod6(ServerRequestInterface $request)
{
}
}

function some_controller_function($foo, $foobar)
Expand Down
15 changes: 15 additions & 0 deletions src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\HttpKernel\Tests;

use Symfony\Bridge\PsrHttpMessage\Tests\Fixtures\Response as PsrResponse;
use Symfony\Bridge\PsrHttpMessage\Tests\Fixtures\Stream as PsrStream;
use Symfony\Component\HttpKernel\HttpKernel;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelEvents;
Expand Down Expand Up @@ -206,6 +208,14 @@ public function testHandleWhenTheControllerIsAStaticArray()
$this->assertResponseEquals(new Response('foo'), $kernel->handle(new Request()));
}

public function testHandleWhenTheControllerReturnsAPsrResponse()
{
$kernel = new HttpKernel(new EventDispatcher(), $this->getResolver(array(new Controller(), 'psrController')));

$response = $kernel->handle(new Request());
$this->assertResponseEquals(new Response('Les-Tilleuls.coop'), $response);
}

/**
* @expectedException \LogicException
*/
Expand Down Expand Up @@ -307,6 +317,11 @@ public function controller()
return new Response('foo');
}

public function psrController()
{
return new PsrResponse('1.0', array(), new PsrStream('Les-Tilleuls.coop'), 200);
}

public static function staticController()
{
return new Response('foo');
Expand Down
14 changes: 12 additions & 2 deletions src/Symfony/Component/HttpKernel/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
"homepage": "https://symfony.com/contributors"
}
],
"repositories": [
{
"type": "vcs",
"url": "https://github.com/dunglas/psr-http-message-bridge"
}
],
"require": {
"php": ">=5.3.9",
"symfony/event-dispatcher": "~2.5.9|~2.6,>=2.6.2|~3.0.0",
Expand All @@ -38,7 +44,9 @@
"symfony/stopwatch": "~2.3|~3.0.0",
"symfony/templating": "~2.2|~3.0.0",
"symfony/translation": "~2.0,>=2.0.5|~3.0.0",
"symfony/var-dumper": "~2.6|~3.0.0"
"symfony/var-dumper": "~2.6|~3.0.0",
"symfony/psr-http-message-bridge": "dev-wip",
"zendframework/zend-diactoros": "~1.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, those are soft deps (dev ones possibily).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here (in require-dev).

},
"conflict": {
"symfony/config": "<2.7"
Expand All @@ -50,7 +58,9 @@
"symfony/console": "",
"symfony/dependency-injection": "",
"symfony/finder": "",
"symfony/var-dumper": ""
"symfony/var-dumper": "",
"symfony/psr-http-message-bridge": "To enable PSR-7 support.",
"zendframework/zend-diactoros": "To enable PSR-7 support."
},
"autoload": {
"psr-4": { "Symfony\\Component\\HttpKernel\\": "" }
Expand Down