This RFC discusses the possible enhancements to the current controller
implementation in Symfony 2. Feel free to give your opinion.

How does controllers work in PR1?
---------------------------------

When creating a controller, Symfony injects the container in the constructor:

    $controller = new Controller($container);

That's easy enough and means that you can use any service in your actions:

    function indexAction()
    {
      $this->container->getUserService()->setAttribute(...);
    }

Also, when executing the action, the method arguments are injected by matching
their names with the routing variables thanks to introspection:

    function showAction($id)
    {
    }

The id is injected if the corresponding route has an id variable:

    /article/:id

The rules are the following:

    * If the argument name matches a route variable, we use it ($id in the
      example)

* If not, and if there is a default value, and if the argument is optional,
      we use the default value ($pi in the example)

    * If not, we throw an exception

The main problems are the following:

  * The controller is tied with the container which is not a good idea
    (separation of concerns).

  * Using a service in an action is quite verbose
    ($this->container->getUserService())

  * To mitigate the previous problem, we provide extensions to the default
Controller, like DoctrineController, but of course, it's not possible to
    inherit from several base classes. So, it's only a partial solution.

How to test a controller?

To test a controller, we need to create a small container with the services or
mocks needed by the action to test. That's quite easy to do:

    $container = new Container();
    $container->setService('user', new UserMock());

    $controller = new ArticleController($container);

The main problem is that testing the controller involves a container, which is
not a very clean solution.

Possible changes for PR2
------------------------

Below are my proposals for Symfony 2 PR2.

### Inject the needed services in the constructor

As for the action, we can inject the services in the constructor by using
introspection:

    protected $user, $doctrineManager;

    function __construct($user, $doctrineManager)
    {
      $this->user = $user;
    }

    function showAction($id)
    {
      $this->user->setAttribute(...);
    }

By matching the service name with the argument name, that should be easy and
should only incur a small performance impact.

Problem: The constructor need all services needed by all methods, which is not
a good idea (we will create too many services most of the time). And then,
it's up to the developer to store the services in protected variables. That
means some boilerplate code.

Testing a controller is much cleaner than before though:

    $user = new UserMock();
    $controller = new ArticleController($user);

### Inject services as protected properties

To avoid the boilerplate code, we can inject the services directly by
introspecting the protected variables:

    protected $user;

    function showAction($id)
    {
      $this->user->setAttribute(...);
    }

The biggest problem here is the magic involved. And as the controller can have
non-service protected variables, we won't be able to thrown any exception if
the service does not exist:

    protected $user;
    protected $pi = 3.14;
    protected $someNonServiceVariable;

How do I know to not inject the id or someNonServiceVariable property?

Also, testing the controller is not easy as we need to inject protected
property, which means using introspection. Seems a bit ugly.

### Inject services in each controller

As we already do some introspection to pass arguments to the action, we can
also inject services:

    function showAction($id, $user, $doctrineManager, $pi = 3.14)
    {
    }

The rules can be the following:

  * If the argument name matches a route variable, we use it ($id in the
    example)

* If not, and if the argument name matches a service name, we use it ($user
    in the example)

* If not, and if there is a default value, and if the argument is optional,
    we use the default value ($pi in the example)

  * If not, we throw an exception

The action code is now very concise:

    function showAction($id, $user, $doctrineManager)
    {
      $user->setAttribute(...);
    }

The actions are still very easy to test and it looks more consistent. All the
variables needed by the action are passed as arguments.

There are two problems:

  * If you have a long list of routing variables and service names, the
    signature can be quite long (should be pretty rare):

function showAction($year, $month, $day, $slug, $user, $doctrineManager)
        {
        }

  * If a routing variable has the same name as a service name, the routing
variable wins but it can lead to WTF effects (if the documentation clearly
    states how it works, it should mitigate the problem)

Last thing, if we use the container as a way to store parameters (as the old
app.yml), which is I think a best practice, then we also need to be able to
pass container parameters:

function litsAction($category, $doctrineManager, $maxNumberOfArticles = 10)
    {
    }

Here, category comes from the routing, doctrineManager is a service, and
maxNumberOfArticles is a container parameter.

--
Fabien Potencier
Sensio CEO - symfony lead developer
sensiolabs.com | symfony-project.org | fabien.potencier.org
Tél: +33 1 40 99 80 80

--
If you want to report a vulnerability issue on symfony, please send it to 
security at symfony-project.com

You received this message because you are subscribed to the Google
Groups "symfony developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/symfony-devs?hl=en

To unsubscribe from this group, send email to symfony-devs+unsubscribegooglegroups.com or 
reply to this email with the words "REMOVE ME" as the subject.

Reply via email to