Symfony2: Trimming Fat from Controllers

In this post I am going to look at some techniques for cleaning up Symfony2 controller actions. Some of this by using some of the extras that are part of the standard distribution and some by moving code out to be triggered by events.

So here is a fairly standard update action in a controller:

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Form\ProductType;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     */
    public function updateAction($id)
    {
        if (false === $this->get('security.context')->isGranted('ROLE_ADMIN')) {
            throw new AccessDeniedException();
        }

        $em = $this->getDoctrine()->getManager();
        $product = $em->getRepository('AcmeDemoBundle:Product')->find($id);

        if (!$product) {
            throw $this->createNotFoundException('Unable to find Product entity.');
        }

        $request = $this->getRequest();
        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $product->setLastUpdated(new \DateTime);
            $em->flush();

            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($this->container->getParameter('acme.product_email.from'))
                ->setTo($this->container->getParameter('acme.product_email.to'))
                ->setBody($this->renderView(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $product))
                )
            ;
            $this->get('mailer')->send($message);

            return $this->redirect(
                $this->generateUrl('product', array('id' => $id))
            );
        }

        return new Response(
            $this->renderView(
                'AcmeDemoBundle:Product:edit.html.twig',
                array(
                    'product'      => $product,
                    'edit_form'   => $editForm->createView(),
                )
            )
        );
    }

    //...
}

The controller action starts by checking if the current user has the ROLE_ADMIN role and throws and exception if they don't. The product entity is retrieved from the database based on the passed id. A form is created and the request retrieved from the container and bound to the form. If the form is valid the entity has its last updated field updated, the entity manager is flushed and an email sent to notify of the changes. If the product changes are not valid then the edit form is redisplayed.

The controller action is quite lengthy now and some of the work being done, in particular the sending of the email, does not really belong in it. Let's go through some steps we can take to arrive at a much leaner controller.

Starting with a task common to many actions, the rendering of the template and returning a Response object with the results can be made simpler. By adding the Template annotation we can just return the array of parameters to be passed to the template. With the annotation in place, when you return the parameters the template will be automatically rendered and a Response with the result sent. Note that the annotation is namespaced and requires the use statement to be added:

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Form\ProductType;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     * @Template("AcmeDemoBundle:Product:edit.html.twig")
     */
    public function updateAction($id)
    {
        if (false === $this->get('security.context')->isGranted('ROLE_ADMIN')) {
            throw new AccessDeniedException();
        }

        $em = $this->getDoctrine()->getManager();
        $product = $em->getRepository('AcmeDemoBundle:Product')->find($id);

        if (!$product) {
            throw $this->createNotFoundException('Unable to find Product entity.');
        }

        $request = $this->getRequest();
        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $product->setLastUpdated(new \DateTime);
            $em->flush();

            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($this->container->getParameter('acme.product_email.from'))
                ->setTo($this->container->getParameter('acme.product_email.to'))
                ->setBody($this->renderView(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $product))
                )
            ;
            $this->get('mailer')->send($message);

            return $this->redirect(
                $this->generateUrl('product', array('id' => $id))
            );
        }

        return array(
            'product'      => $product,
            'edit_form'   => $editForm->createView(),
        );
    }

    //...
}

A further annotation, the Secure annotation can be used to replace the permissions checking. By adding this annotation the check for the ROLE_ADMIN role and the exception throwing if necessary, will take place without the need for the code:

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Form\ProductType;
use JMS\SecurityExtraBundle\Annotation\Secure;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     * @Secure("ROLE_ADMIN")
     * @Template("AcmeDemoBundle:Product:edit.html.twig")
     */
    public function updateAction($id)
    {
        $em = $this->getDoctrine()->getManager();
        $product = $em->getRepository('AcmeDemoBundle:Product')->find($id);

        if (!$product) {
            throw $this->createNotFoundException('Unable to find Product entity.');
        }

        $request = $this->getRequest();
        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $product->setLastUpdated(new \DateTime);
            $em->flush();

            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($this->container->getParameter('acme.product_email.from'))
                ->setTo($this->container->getParameter('acme.product_email.to'))
                ->setBody($this->renderView(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $product))
                )
            ;
            $this->get('mailer')->send($message);

            return $this->redirect(
                $this->generateUrl('product', array('id' => $id))
            );
        }

        return array(
            'product'      => $product,
            'edit_form'   => $editForm->createView(),
        );
    }

    //...
}

We can use Symfony2 resolving of controller action arguments to remove more code from our action. First by having the request automatically injected as a method argument. By type hinting the argument as Request the current request will be passed to the action automatically and we can remove the line requesting it from the controller:

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Form\ProductType;
use JMS\SecurityExtraBundle\Annotation\Secure;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     * @Secure("ROLE_ADMIN")
     * @Template("AcmeDemoBundle:Product:edit.html.twig")
     */
    public function updateAction(Request $request, $id)
    {
        $em = $this->getDoctrine()->getManager();
        $product = $em->getRepository('AcmeDemoBundle:Product')->find($id);

        if (!$product) {
            throw $this->createNotFoundException('Unable to find Product entity.');
        }

        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $product->setLastUpdated(new \DateTime);
            $em->flush();

            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($this->container->getParameter('acme.product_email.from'))
                ->setTo($this->container->getParameter('acme.product_email.to'))
                ->setBody($this->renderView(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $product))
                )
            ;
            $this->get('mailer')->send($message);

            return $this->redirect(
                $this->generateUrl('product', array('id' => $id))
            );
        }

        return array(
            'product'      => $product,
            'edit_form'   => $editForm->createView(),
        );
    }

    //...
}

We can do something similar for the Product by using the ParamConverter annotation. So instead of the $id argument we have a $product type hinted as our Product entity. This is automatically retrieved from Doctrine or an exception thrown if it is not found, pretty much as we were doing in the code anyway, allowing us to remove the code for getting the entity from the database altogether:

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Entity\Product;
use Acme\DemoBundle\Form\ProductType;
use JMS\SecurityExtraBundle\Annotation\Secure;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     * @ParamConverter
     * @Secure("ROLE_ADMIN")
     * @Template("AcmeDemoBundle:Product:edit.html.twig")
     */
    public function updateAction(Request $request, Product $product)
    {
        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $product->setLastUpdated(new \DateTime);
            $em = $this->getDoctrine()->getManager();
            $em->flush();

            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($this->container->getParameter('acme.product_email.from'))
                ->setTo($this->container->getParameter('acme.product_email.to'))
                ->setBody($this->renderView(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $product))
                )
            ;
            $this->get('mailer')->send($message);

            return $this->redirect(
                $this->generateUrl('product', array('id' => $product->getId()))
            );            
        }

        return array(
            'product'      => $product,
            'edit_form'   => $editForm->createView(),
        );
    }

    //...
}

In fact in this case, where we are not setting any options on the ParamConverter annotation, we can omit it altogether:

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Entity\Product;
use Acme\DemoBundle\Form\ProductType;
use JMS\SecurityExtraBundle\Annotation\Secure;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     * @Secure("ROLE_ADMIN")
     * @Template("AcmeDemoBundle:Product:edit.html.twig")
     */
    public function updateAction(Request $request, Product $product)
    {
        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $product->setLastUpdated(new \DateTime);
            $em = $this->getDoctrine()->getManager();
            $em->flush();

            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($this->container->getParameter('acme.product_email.from'))
                ->setTo($this->container->getParameter('acme.product_email.to'))
                ->setBody($this->renderView(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $product))
                )
            ;
            $this->get('mailer')->send($message);

            return $this->redirect(
                $this->generateUrl('product', array('id' => $product->getId()))
            ); 
        }

        return array(
            'product'      => $product,
            'edit_form'   => $editForm->createView(),
        );
    }

    //...
}

If you have are not using Doctrine then you can still get similar results by creating your own ParamConverters.

We have now removed a lot of the boiler plate code that is needed in a lot of actions and we are starting to be able to see what our action is doing better. It looks like it is doing a bit too much around the saving of the product changes. Keeping track of when it was last updated is really a job for the entity itself rather than the controller. Fortunately with a Doctrine entity this can be easily achieved with Doctrine's lifecycle callbacks. By enabling these with an annotations and then annotating a method that updates the lastUpdated field before the update is persisted:

<?php

namespace Acme\DemoBundle\Entity;

use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity
 * @ORM\HasLifecycleCallbacks()
 */
class Product
{
    /**
     * @ORM\column
     */
    private $lastUpdated;

    //..

    /**
     * @ORM\preUpdate
     */
    public function setUpdated()
    {
        $this->lastUpdated = new \DateTime();
    }
}

We can now remove worrying about keeping this up to date from the controller. This time not only have we reduced the controller action we have ensured that the last update field is kept up to date however the entity is updated and not just from our controller action.

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Entity\Product;
use Acme\DemoBundle\Form\ProductType;
use JMS\SecurityExtraBundle\Annotation\Secure;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     * @Secure("ROLE_ADMIN")
     * @Template("AcmeDemoBundle:Product:edit.html.twig")
     */
    public function updateAction(Request $request, Product $product)
    {
        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $em = $this->getDoctrine()->getManager();
            $em->flush();

            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($this->container->getParameter('acme.product_email.from'))
                ->setTo($this->container->getParameter('acme.product_email.to'))
                ->setBody($this->renderView(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $product))
                )
            ;
            $this->get('mailer')->send($message);

            return $this->redirect(
                $this->generateUrl('product', array('id' => $product->getId()))
            ); 
        }

        return array(
            'product'      => $product,
            'edit_form'   => $editForm->createView(),
        );
    }

    //...
}

The last step is to remove the sending of the email, we can again do this with an event. In this case we need to use the mailer service and we do not want to have to inject it into our entity. We can use a standalone event listener instead and inject the services and parameters we need directly into the constructor:

<?php

namespace Acme\DemoBundle\Listener;

use Doctrine\ORM\Event\LifecycleEventArgs;
use Acme\DemoBundle\Entity\Product;

class ProductEmailler
{
    protected $mailer;
    protected $templating;
    protected $emailFrom;
    protected $emailTo;

    public function __construct($mailer, $templating, $emailFrom, $emailTo)
    {
        $this->mailer = $mailer;
        $this->templating = $templating;
        $this->emailFrom = $emailFrom;
        $this->emailTo = $emailTo;
    }

    public function postUpdate(LifecycleEventArgs $args)
    {
        $entity = $args->getEntity();

        if ($entity instanceof Product) {
            $message = \Swift_Message::newInstance()
                ->setSubject('Product updated')
                ->setFrom($emailFrom)
                ->setTo($emailTo)
                ->setBody($this->templating->render(
                    'AcmeDemoBundle:Product:email.txt.twig',
                    array('product' => $entity))
                )
            ;
            $this->mailer->send($message);
        }
    }
}

In the service configuration file we specify the services and parameters to pass into the listener and tag it as an Doctine event listener. The listener will now be called after an entity is updated:

<services>
    <service id="acme.listener.product_email"
             class="Acme\DemoBundle\Listener\ProductEmailler">
        <argument type="service" id="mailer"/>
        <argument type="service" id="templating"/>
        <argument>%acme.product_email.from%</argument>
        <argument>%acme.product_email.to%</argument>
        <tag name="doctrine.event_listener" event="postUpdate" />
     </service>
</services>

We can now remove the sending of the email from the controller. Again as well as reducing the size of our controller we have introduced flexibility to our application. It is now easy to change or remove the listener without having to touch the controller code. We could even add another listener to send SMS notifications without touching the controller. If you are not using Doctrine then you can still gain these benefits by raising your own events with the event dispatcher.

<?php

namespace Acme\DemoBundle\Controller;

use Acme\DemoBundle\Entity\Product;
use Acme\DemoBundle\Form\ProductType;
use JMS\SecurityExtraBundle\Annotation\Secure;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;

/**
 * @Route("/product")
 */
class ProductController extends Controller
{
    //...

    /**
     * @Route("/edit", name="product_update")
     * @Method("POST")
     * @Secure("ROLE_ADMIN")
     * @Template("AcmeDemoBundle:Product:edit.html.twig")
     */
    public function updateAction(Request $request, Product $product)
    {
        $editForm = $this->createForm(new ProductType(), $product);
        $editForm->bind($request);

        if ($editForm->isValid()) {
            $em = $this->getDoctrine()->getManager();
            $em->flush();
            return $this->redirect(
                $this->generateUrl('product', array('id' => $product->getId()))
            ); 
        }

        return array(
            'product' => $product,
            'edit_form' => $editForm->createView(),
        );
    }

    //...
}

Our controller is now much shorter than before by removing much of that boiler plate code and it is easier to see what is happening. The decision about what happens when a product is updated have been moved away from the controller and into the model layer.

Note: The example controller extends from the base controller and makes direct use of the service container. I have previously written about moving away from in favour of controllers as services. I do think there are advantages to that but since extending the base controller is more common, I have based my example for this post on that.