Skip to content

[FrameworkBundle] call setContainer() for autowired controllers #23239

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
Jul 3, 2017
Merged

[FrameworkBundle] call setContainer() for autowired controllers #23239

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
14 changes: 13 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,19 @@ protected function createController($controller)
$controller = $this->parser->parse($controller);
}

return parent::createController($controller);
$resolvedController = parent::createController($controller);

if (1 === substr_count($controller, ':') && is_array($resolvedController)) {
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be done in the parent class instead, which is the one supporting container injection ?

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO that would be inconsistent as the parent class isn't doing this for instances it creates itself either.

Copy link
Member

Choose a reason for hiding this comment

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

hmm indeed. I missed the fact that injection was done in this class in a separate method

if ($resolvedController[0] instanceof ContainerAwareInterface) {
$resolvedController[0]->setContainer($this->container);
}

if ($resolvedController[0] instanceof AbstractController && null !== $previousContainer = $resolvedController[0]->setContainer($this->container)) {
$resolvedController[0]->setContainer($previousContainer);
}
}

return $resolvedController;
}

/**
Expand Down
69 changes: 69 additions & 0 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 @@ -13,6 +13,7 @@

use Psr\Container\ContainerInterface as Psr11ContainerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser;
use Symfony\Bundle\FrameworkBundle\Controller\ControllerResolver;
use Symfony\Component\DependencyInjection\Container;
Expand Down Expand Up @@ -68,6 +69,24 @@ public function testGetControllerWithBundleNotation()
$this->assertSame('testAction', $controller[1]);
}

public function testContainerAwareControllerGetsContainerWhenNotSet()
{
class_exists(AbstractControllerTest::class);

$controller = new ContainerAwareController();

$container = new Container();
$container->set(TestAbstractController::class, $controller);

$resolver = $this->createControllerResolver(null, $container);

$request = Request::create('/');
$request->attributes->set('_controller', TestAbstractController::class.':testAction');

$this->assertSame(array($controller, 'testAction'), $resolver->getController($request));
$this->assertSame($container, $controller->getContainer());
}

public function testAbstractControllerGetsContainerWhenNotSet()
{
class_exists(AbstractControllerTest::class);
Expand All @@ -86,6 +105,24 @@ class_exists(AbstractControllerTest::class);
$this->assertSame($container, $controller->setContainer($container));
}

public function testAbstractControllerServiceWithFcqnIdGetsContainerWhenNotSet()
{
class_exists(AbstractControllerTest::class);

$controller = new DummyController();

$container = new Container();
$container->set(DummyController::class, $controller);

$resolver = $this->createControllerResolver(null, $container);

$request = Request::create('/');
$request->attributes->set('_controller', DummyController::class.':fooAction');

$this->assertSame(array($controller, 'fooAction'), $resolver->getController($request));
$this->assertSame($container, $controller->getContainer());
}

public function testAbstractControllerGetsNoContainerWhenSet()
{
class_exists(AbstractControllerTest::class);
Expand All @@ -106,6 +143,26 @@ class_exists(AbstractControllerTest::class);
$this->assertSame($controllerContainer, $controller->setContainer($container));
}

public function testAbstractControllerServiceWithFcqnIdGetsNoContainerWhenSet()
{
class_exists(AbstractControllerTest::class);

$controller = new DummyController();
$controllerContainer = new Container();
$controller->setContainer($controllerContainer);

$container = new Container();
$container->set(DummyController::class, $controller);

$resolver = $this->createControllerResolver(null, $container);

$request = Request::create('/');
$request->attributes->set('_controller', DummyController::class.':fooAction');

$this->assertSame(array($controller, 'fooAction'), $resolver->getController($request));
$this->assertSame($controllerContainer, $controller->getContainer());
}

protected function createControllerResolver(LoggerInterface $logger = null, Psr11ContainerInterface $container = null, ControllerNameParser $parser = null)
{
if (!$parser) {
Expand Down Expand Up @@ -152,3 +209,15 @@ public function __invoke()
{
}
}

class DummyController extends AbstractController
{
public function getContainer()
{
return $this->container;
}

public function fooAction()
{
}
}