Skip to content

[HttpKernel] Add a ContainerControllerResolver (psr-11) #21768

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 28, 2017
Merged

[HttpKernel] Add a ContainerControllerResolver (psr-11) #21768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
[HttpKernel] Add a ContainerControllerResolver (psr-11)
  • Loading branch information
@ogizanagi
ogizanagi committed Feb 28, 2017
commit 7bbae4159bcd5e49fb2303a7af6af99b45da498d
39 changes: 7 additions & 32 deletions src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,17 @@
namespace Symfony\Bundle\FrameworkBundle\Controller;

use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Controller\ControllerResolver as BaseControllerResolver;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\HttpKernel\Controller\ContainerControllerResolver;

/**
* ControllerResolver.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class ControllerResolver extends BaseControllerResolver
class ControllerResolver extends ContainerControllerResolver
{
protected $container;
protected $parser;

/**
Expand All @@ -35,39 +34,19 @@ class ControllerResolver extends BaseControllerResolver
*/
public function __construct(ContainerInterface $container, ControllerNameParser $parser, LoggerInterface $logger = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just changing the type hint and make the parser optional?

Copy link
Contributor Author

@ogizanagi ogizanagi Feb 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Because the new class is available in the HttpKernel component, whereas this one is in the framework bundle, as well as the ControllerNameParser.
I'm fine with this class as is in the framework. :) I just think having a base ControllerResolver usable with psr-11 shipped with the component makes sense.

{
$this->container = $container;
$this->parser = $parser;

parent::__construct($logger);
parent::__construct($container, $logger);
}

/**
* Returns a callable for the given controller.
*
* @param string $controller A Controller string
*
* @return mixed A PHP callable
*
* @throws \LogicException When the name could not be parsed
* @throws \InvalidArgumentException When the controller class does not exist
* {@inheritdoc}
*/
protected function createController($controller)
{
if (false === strpos($controller, '::')) {
$count = substr_count($controller, ':');
if (2 == $count) {
// controller in the a:b:c notation then
$controller = $this->parser->parse($controller);
} elseif (1 == $count) {
// controller in the service:method notation
list($service, $method) = explode(':', $controller, 2);

return array($this->container->get($service), $method);
} elseif ($this->container->has($controller) && method_exists($service = $this->container->get($controller), '__invoke')) {
return $service;
} else {
throw new \LogicException(sprintf('Unable to parse the controller name "%s".', $controller));
}
if (false === strpos($controller, '::') && 2 === substr_count($controller, ':')) {
// controller in the a:b:c notation then
$controller = $this->parser->parse($controller);
}

return parent::createController($controller);
Expand All @@ -78,10 +57,6 @@ protected function createController($controller)
*/
protected function instantiateController($class)
{
if ($this->container->has($class)) {
return $this->container->get($class);
}

$controller = parent::instantiateController($class);

if ($controller instanceof ContainerAwareInterface) {
Expand Down
123 changes: 5 additions & 118 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@

namespace Symfony\Bundle\FrameworkBundle\Tests\Controller;

use Psr\Container\ContainerInterface as Psr11ContainerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser;
use Symfony\Bundle\FrameworkBundle\Controller\ControllerResolver;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest as BaseControllerResolverTest;
use Symfony\Component\HttpKernel\Tests\Controller\ContainerControllerResolverTest;

class ControllerResolverTest extends BaseControllerResolverTest
class ControllerResolverTest extends ContainerControllerResolverTest
{
public function testGetControllerOnContainerAware()
{
Expand Down Expand Up @@ -55,7 +56,7 @@ public function testGetControllerWithBundleNotation()
->will($this->returnValue('Symfony\Bundle\FrameworkBundle\Tests\Controller\ContainerAwareController::testAction'))
;

$resolver = $this->createControllerResolver(null, $parser);
$resolver = $this->createControllerResolver(null, null, $parser);
$request = Request::create('/');
$request->attributes->set('_controller', $shortName);

Expand All @@ -66,110 +67,7 @@ public function testGetControllerWithBundleNotation()
$this->assertSame('testAction', $controller[1]);
}

public function testGetControllerService()
{
$container = $this->createMockContainer();
$container->expects($this->once())
->method('get')
->with('foo')
->will($this->returnValue($this))
;

$resolver = $this->createControllerResolver(null, null, $container);
$request = Request::create('/');
$request->attributes->set('_controller', 'foo:controllerMethod1');

$controller = $resolver->getController($request);

$this->assertInstanceOf(get_class($this), $controller[0]);
$this->assertSame('controllerMethod1', $controller[1]);
}

public function testGetControllerInvokableService()
{
$invokableController = new InvokableController('bar');

$container = $this->createMockContainer();
$container->expects($this->once())
->method('has')
->with('foo')
->will($this->returnValue(true))
;
$container->expects($this->once())
->method('get')
->with('foo')
->will($this->returnValue($invokableController))
;

$resolver = $this->createControllerResolver(null, null, $container);
$request = Request::create('/');
$request->attributes->set('_controller', 'foo');

$controller = $resolver->getController($request);

$this->assertEquals($invokableController, $controller);
}

public function testGetControllerInvokableServiceWithClassNameAsName()
{
$invokableController = new InvokableController('bar');
$className = __NAMESPACE__.'\InvokableController';

$container = $this->createMockContainer();
$container->expects($this->once())
->method('has')
->with($className)
->will($this->returnValue(true))
;
$container->expects($this->once())
->method('get')
->with($className)
->will($this->returnValue($invokableController))
;

$resolver = $this->createControllerResolver(null, null, $container);
$request = Request::create('/');
$request->attributes->set('_controller', $className);

$controller = $resolver->getController($request);

$this->assertEquals($invokableController, $controller);
}

/**
* @dataProvider getUndefinedControllers
*/
public function testGetControllerOnNonUndefinedFunction($controller, $exceptionName = null, $exceptionMessage = null)
{
// All this logic needs to be duplicated, since calling parent::testGetControllerOnNonUndefinedFunction will override the expected excetion and not use the regex
$resolver = $this->createControllerResolver();
if (method_exists($this, 'expectException')) {
$this->expectException($exceptionName);
$this->expectExceptionMessageRegExp($exceptionMessage);
} else {
$this->setExpectedExceptionRegExp($exceptionName, $exceptionMessage);
}

$request = Request::create('/');
$request->attributes->set('_controller', $controller);
$resolver->getController($request);
}

public function getUndefinedControllers()
{
return array(
array('foo', '\LogicException', '/Unable to parse the controller name "foo"\./'),
array('oof::bar', '\InvalidArgumentException', '/Class "oof" does not exist\./'),
array('stdClass', '\LogicException', '/Unable to parse the controller name "stdClass"\./'),
array(
'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::bar',
'\InvalidArgumentException',
'/.?[cC]ontroller(.*?) for URI "\/" is not callable\.( Expected method(.*) Available methods)?/',
),
);
}

protected function createControllerResolver(LoggerInterface $logger = null, ControllerNameParser $parser = null, ContainerInterface $container = null)
protected function createControllerResolver(LoggerInterface $logger = null, Psr11ContainerInterface $container = null, ControllerNameParser $parser = null)
{
if (!$parser) {
$parser = $this->createMockParser();
Expand Down Expand Up @@ -215,14 +113,3 @@ public function __invoke()
{
}
}

class InvokableController
{
public function __construct($bar) // mandatory argument to prevent automatic instantiation
{
}

public function __invoke()
{
}
}
76 changes: 76 additions & 0 deletions src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpKernel\Controller;

use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;

/**
* A controller resolver searching for a controller in a psr-11 container when using the "service:method" notation.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
*/
class ContainerControllerResolver extends ControllerResolver
{
protected $container;

public function __construct(ContainerInterface $container, LoggerInterface $logger = null)
{
$this->container = $container;

parent::__construct($logger);
}

/**
* Returns a callable for the given controller.
*
* @param string $controller A Controller string
*
* @return mixed A PHP callable
*
* @throws \LogicException When the name could not be parsed
* @throws \InvalidArgumentException When the controller class does not exist
*/
protected function createController($controller)
{
if (false !== strpos($controller, '::')) {
return parent::createController($controller);
}

if (1 == substr_count($controller, ':')) {
// controller in the "service:method" notation
list($service, $method) = explode(':', $controller, 2);

return array($this->container->get($service), $method);
}

if ($this->container->has($controller) && method_exists($service = $this->container->get($controller), '__invoke')) {
// invokable controller in the "service" notation
return $service;
}

throw new \LogicException(sprintf('Unable to parse the controller name "%s".', $controller));
}

/**
* {@inheritdoc}
*/
protected function instantiateController($class)
{
if ($this->container->has($class)) {
return $this->container->get($class);
}

return parent::instantiateController($class);
}
}
Loading