Skip to content

Commit

Permalink
bug #23239 [FrameworkBundle] call setContainer() for autowired contro…
Browse files Browse the repository at this point in the history
…llers (xabbuh)

This PR was merged into the 3.3 branch.

Discussion
----------

[FrameworkBundle] call setContainer() for autowired controllers

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23200, FriendsOfSymfony/FOSRestBundle#1719
| License       | MIT
| Doc PR        |

Previously, you either did not use controllers as services or you explicitly wired everything yourself. In case of a non-service controller the FrameworkBundle took care of calling `setContainer()` inside the `instantiateController()` method:

```php
protected function instantiateController($class)
{
    $controller = parent::instantiateController($class);

    if ($controller instanceof ContainerAwareInterface) {
        $controller->setContainer($this->container);
    }
    if ($controller instanceof AbstractController && null !== $previousContainer = $controller->setContainer($this->container)) {
        $controller->setContainer($previousContainer);
    }

    return $controller;
}
```

With the new autowired controllers as services this is no longer happening as controllers do not need to be instantiated anymore (the container already returns fully built objects).

Commits
-------

1d07a28 call setContainer() for autowired controllers
  • Loading branch information
nicolas-grekas committed Jul 3, 2017
2 parents a40b29b + 1d07a28 commit 966662d
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 1 deletion.
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)) {
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
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()
{
}
}

0 comments on commit 966662d

Please sign in to comment.