Add support for @Template annotation

[bpolaszek]

Since all actions must be located in the Action directory, we lose native support for @Template annotation (I mean, the @Template annotation allows to not specify a template file, then it'll guess that a simple @Template annotation on a AppBundle\Controller\ExampleController::viewAction() should render AppBundle:Example:view.html.twig).

I had to override Symfony's TemplateGuesser for this to work, but I'm not sure it's the best solution.

class TemplateGuesser extends \Sensio\Bundle\FrameworkExtraBundle\Templating\TemplateGuesser {

    /**
     * Guesses and returns the template name to render based on the controller
     * and action names.
     *
     * @param array   $controller An array storing the controller object and action method
     * @param Request $request    A Request instance
     * @param string  $engine
     *
     * @return TemplateReference template reference
     *
     * @throws \InvalidArgumentException
     */
    public function guessTemplateName($controller, Request $request, $engine = 'twig')
    {
        $className = class_exists('Doctrine\Common\Util\ClassUtils') ? ClassUtils::getClass($controller[0]) : get_class($controller[0]);

        if (!preg_match('/Action\\\(.+)Action/', $className, $matchController)) {
            throw new \InvalidArgumentException(sprintf('The "%s" class does not look like a controller class (it must be in a "Action" sub-namespace and the class name must end with "Action")', get_class($controller[0])));
        }
        if (!preg_match('/^(.+)Action$/', $controller[1], $matchAction)) {
            throw new \InvalidArgumentException(sprintf('The "%s" method does not look like an action method (it does not end with Action)', $controller[1]));
        }

        $bundle = $this->getBundleForClass($className);

        if ($bundle) {
            while ($bundleName = $bundle->getName()) {
                if (null === $parentBundleName = $bundle->getParent()) {
                    $bundleName = $bundle->getName();

                    break;
                }

                $bundles = $this->kernel->getBundle($parentBundleName, false);
                $bundle = array_pop($bundles);
            }
        } else {
            $bundleName = null;
        }

        return new TemplateReference($bundleName, $matchController[1], $matchAction[1], $request->getRequestFormat(), $engine);
    }

}

<?php
# AppBundle/Action/ExampleAction.php

namespace AppBundle\Action;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Component\HttpFoundation\Request;

class ExampleAction {

    /**
     * @Route("/example", name="example")
     * @Template
     * @param Request $request
     */
    public function viewAction(Request $request) {
        // awesome code
    }

}

Or maybe would it be possible to store Actions in the Controller directory instead of their own?

I don't see any advantage to have both an Action and a Controller directory into the same bundle. You have to choose wether you use the standard MVC pattern, or the ADR pattern for gluing your views and your model. The directory name, in fine, doesn't matter at all.

Despite they don't obey the same design pattern, the "action" concept is pretty close to the "controller" concept (It looks like a duck, swims like a duck, and quacks like a duck, then it probably is a duck). Call them actions, I call them controllers.

Anyway, I played a little bit with this bundle, and it should definitely replace Symfony's standard controller architecture :)

[dunglas]

Hi @bpolaszek, thanks for the report.

ADR terminology apart, the Action/ directory is basically used to avoid breaking existing third party bundles and "legacy" apps using the standard controller system: the bundle iterate over registered bundles (even third party ones line SonataAdmin or FOSUser providing their own controllers) and register all classes in the Action/ directory as services. If the default is Controller it will fail because standard controllers are not designed to be registered as a service and are already registered by the FrameworkBundle.

That being said, the directory where to store actions is now configurable: https://github.com/dunglas/DunglasActionBundle#configuration If you're sure that you'r app do not use "legacy" controllers, you can change it to the Controller directory (but I never tried it, maybe this will conflict with how FrameworkBundle registers standards controllers.

About the issue with @Template, ActionBundle should definitely include you're patched guesser (with some minor changes such as using the configured directory name and not requiring that methods end with Action as it's not the case for __invoke for instance). Are you motivated to work on a PR to add it? If you're out of time or not interested I'll do it.

[bpolaszek]

Hello Kévin,

Thanks for your explanations. Everything is clearer now :)

Since I use, as many of us around here, the FOSUserBundle and the FOSRestBundle, it indeed may not be a good idea to consider Actions as Controllers.

I'd like to do a PR but I'm clearly out of time, and I'm not sure to do things right, since I never worked on an "exportable" bundle and I'm not comfortable with tests. So, I won't tell you, yeah, I'll do it, and finally don't, because I'll certainly struggle on things I'm not familiar with, and give up for priority reasons.

I'm also not sure if overriding the TemplateGuesser is the best option: it actually works, but I find this a bit tricky, and it may conflict with potential users who developed their own. Perhaps another @Template annotation would be more framework-friendly?

You're the master :)

Last question, but not least: why are the action services named like action.Acme\AppBundle\Action\ExampleAction instead of something action.acme.app_bundle.action.example_action? It's a little uncommon to have antislashes and uppercase in service names.

Cheers, Ben

[dunglas]

For the naming convention, it's for 2 reasons:

• The Symfony autowiring system use a similar one (autowired.The\Class\Name)
• It's easier to debug (the service name is action. + paste the FQN here, no camelcase to snake case conversion + underscore replacement needed)

[bpolaszek]

Everything's clear now.

[GuilhemN]

please see https://github.com/sensiolabs/SensioFrameworkExtraBundle/issues/398

[GuilhemN]

BTW https://github.com/sensiolabs/SensioFrameworkExtraBundle/pull/398 is merged so this is fixed ;-)

Edit: not completely, we missed a fix here

[dunglas]

Great!