Skip to content

Commit

Permalink
feature #22157 [FrameworkBundle] Introduce AbstractController, replac…
Browse files Browse the repository at this point in the history
…ing ControllerTrait (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[FrameworkBundle] Introduce AbstractController, replacing ControllerTrait

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no (master only)
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Basically reverts and replaces #18193.

Instead of using getter injection to provide our controller helpers, let's leverage the new `ServiceSubscriberInterface` (see #21708).
This is what the proposed `AbstractController` class provides.

So, instead of extending `Controller`, this would encourage extending `AbstractController`.
This provides almost the same experience, but makes the container private, thus not usable by userland (this safeguard was already provided by `ControllerTrait`).

I did not deprecate `Controller`, but I think we should. Now that we also have "controller.service_arguments" (see #21771), we have everything in place to encourage *not* using the container in controllers directly anymore.

My target in doing so is removing getter injection, which won't have any use case in core anymore.

The wiring for this could be:

```yaml
services:
    _instanceof:
        Symfony\Bundle\FrameworkBundle\Controller\AbstractController:
            calls: [ [ setContainer, [ '@container' ] ] ]
            tags: [ container.service_subscriber ]
````

But this is optional, because we don't really need to inject a scoped service locator: injecting the real container is fine also, since everything is private. And this is done automatically on ControllerResolver.

Commits
-------

a93f059 [FrameworkBundle] Introduce AbstractController, replacing ControllerTrait
  • Loading branch information
fabpot committed Mar 25, 2017
2 parents 8901c7e + a93f059 commit 3306dc2
Show file tree
Hide file tree
Showing 10 changed files with 736 additions and 1,502 deletions.
8 changes: 2 additions & 6 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Expand Up @@ -6,6 +6,8 @@ CHANGELOG

* Added a new new version strategy option called json_manifest_path
that allows you to use the `JsonManifestVersionStrategy`.
* Added `Symfony\Bundle\FrameworkBundle\Controller\AbstractController`. It provides the same helpers than the `Controller` class,
but does not allow accessing the dependency injection container, in order to encourage explicit dependency declarations.
* Added support for the `controller.service_arguments` tag, for injecting services into controllers' actions
* Deprecated `cache:clear` with warmup (always call it with `--no-warmup`)
* Deprecated the "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter
Expand All @@ -26,12 +28,6 @@ CHANGELOG
Use `Symfony\Component\Console\DependencyInjection\ConfigCachePass` instead.
* Deprecated `PropertyInfoPass`, use `Symfony\Component\PropertyInfo\DependencyInjection\PropertyInfoPass` instead
* Deprecated extending `ConstraintValidatorFactory`
* Added `Symfony\Bundle\FrameworkBundle\Controller\ControllerTrait` (requires PHP 7). Unlike the `Symfony\Bundle\FrameworkBundle\Controller\Controller`
class, this trait does not have access to the dependency injection container. Its dependencies are explicitly and lazily
injected using getter injection.
`render()`, `renderView()` and `stream()` methods can only use Twig (using the Templating component is not supported).
The `json()` method requires the Serializer component (use `Symfony\Component\HttpFoundation\JsonResponse` directly if
you do not want to use the Serializer).
* Deprecated `ControllerArgumentValueResolverPass`. Use
`Symfony\Component\HttpKernel\DependencyInjection\ControllerArgumentValueResolverPass` instead
* Deprecated `RoutingResolverPass`, use `Symfony\Component\Routing\DependencyInjection\RoutingResolverPass` instead
Expand Down
@@ -0,0 +1,69 @@
<?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\Bundle\FrameworkBundle\Controller;

use Psr\Container\ContainerInterface;
use Doctrine\Common\Persistence\ManagerRegistry;
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
use Symfony\Component\Form\FormFactoryInterface;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Routing\RouterInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Serializer\SerializerInterface;
use Symfony\Component\Templating\EngineInterface;

/**
* Provides common features needed in controllers.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
abstract class AbstractController implements ServiceSubscriberInterface
{
use ControllerTrait;

private $container;

/**
* @internal
* @required
*/
public function setContainer(ContainerInterface $container)
{
$previous = $this->container;
$this->container = $container;

return $previous;
}

public static function getSubscribedServices()
{
return array(
'router' => '?'.RouterInterface::class,
'request_stack' => '?'.RequestStack::class,
'http_kernel' => '?'.HttpKernelInterface::class,
'serializer' => '?'.SerializerInterface::class,
'session' => '?'.SessionInterface::class,
'security.authorization_checker' => '?'.AuthorizationCheckerInterface::class,
'templating' => '?'.EngineInterface::class,
'twig' => '?'.\Twig_Environment::class,
'doctrine' => '?'.ManagerRegistry::class,
'form.factory' => '?'.FormFactoryInterface::class,
'security.token_storage' => '?'.TokenStorageInterface::class,
'security.csrf.token_manager' => '?'.CsrfTokenManagerInterface::class,
);
}
}

0 comments on commit 3306dc2

Please sign in to comment.