Refactoring a Symfony 2 Controller with PhpSpec

Sep 4th, 2013

This is the second post in a series that looks at working with PhpSpec and Symfony 2. In the first post we saw how it was difficult to manage (and mock) all the dependencies that come with a controller that extends the FrameworkBundle Controller (you can see all the code and commits for these posts on github).

namespace Peterjmit\BlogBundle\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\Controller;

class BlogController extends Controller
{
    public function indexAction()
    {
        $entityManager = $this->getDoctrine()->getManager();
        $posts = $entityManager->getRepository('PeterjmitBlogBundle:Blog')->findAll();

        return $this->render('PeterjmitBlogBundle:Blog:index.html.twig', array(
            'posts' => $posts
        ));
    }

    public function showAction($id)
    {
        $entityManager = $this->getDoctrine()->getManager();
        $post = $entityManager->getRepository('PeterjmitBlogBundle:Blog')->find($id);

        if (!$post) {
            throw $this->createNotFoundException(sprintf('Blog post %s was not found', $id));
        }

        return $this->render('PeterjmitBlogBundle:Blog:show.html.twig', array(
            'post' => $post
        ));
    }
}

BlogController knows too much

Since the previous post, I have added showAction to my controller, but I have not specced it because I know that I the mocks are going to be annoying to write, and I can see that I have duplicated some code from indexAction.

This is a good indicator for the need to refactor - in both the spec, and the controller. In order to decide how to proceed we need to identify what the behaviour of the controller should be, versus what it currently knows.

So we know that in our controller is called during a request, and should return a response object. We also want to show a list of, or a single blog post (depending on the action called).

However we can see that our controller contains more behaviour than that, it knows how to:

Get the templating and doctrine services from the container
Get a "manager" from the doctrine service
Get a named repository from the "manager"
Find blog post objects via the repository
Render a named template with the blog posts and return the result

Reviewing that list, only numbers 4 and 5 really seem to fall in line with our objective, so our goal will be to remove behaviours 1-3 from our controller ¹.

Let BlogController focus on responding to a request

Having the container is convenient when we aren't writing tests, but as soon as we start having to mock the container, our specs can get a lot less fun to write. Therefore we are going to lose our dependency on Symfony\Bundle\FrameworkBundle\Controller\Controller. To fix our failing tests we can construct our controller with templating and doctrine instead as shown in this commit.

That commit however does not go far enough, so lets get rid of doctrine too. We do this by introducing the repository BlogRepository directly, providing the controller with the blog posts that it needs.

edit: after some feedback I simplified the introduction of BlogManagerInterface by switching it to directly injecting the repository in this commit

// ...
use Peterjmit\BlogBundle\Doctrine\BlogRepository;
use Symfony\Bundle\FrameworkBundle\Templating\EngineInterface;

class BlogController
{
    private $repository;
    private $templating;

    public function __construct(BlogRepository $repository, EngineInterface $templating)
    {
        $this->repository = $repository;
        $this->templating = $templating;
    }

    public function indexAction()
    {
        $posts = $this->repository->findAll();

        return $this->templating->renderResponse('PeterjmitBlogBundle:Blog:index.html.twig', array(
            'posts' => $posts
        ));
    }

    public function showAction($id)
    {
        $post = $this->repository->find($id);

        // ...

        return $this->templating->renderResponse('PeterjmitBlogBundle:Blog:show.html.twig', array(
            'post' => $post
        ));
    }
}

By now our spec is looking a lot less difficult to work with, so we can introduce some examples for showAction

// ...

class BlogControllerSpec extends ObjectBehavior
{
    function let(
        BlogRepository $repository,
        EngineInterface $templating
    ) {
        $this->beConstructedWith($repository, $templating);
    }

    function it_is_initializable()
    {
        $this->shouldHaveType('Peterjmit\BlogBundle\Controller\BlogController');
    }

    function it_should_respond_to_index_action(
        BlogRepository $repository,
        EngineInterface $templating,
        Response $mockResponse
    ) {
        $repository->findAll()->willReturn(array('An array', 'of blog', 'posts!'));

        $templating
            ->renderResponse(
                'PeterjmitBlogBundle:Blog:index.html.twig',
                array('posts' => array('An array', 'of blog', 'posts!'))
            )
            ->willReturn($mockResponse)
        ;

        $response = $this->indexAction();

        $response->shouldHaveType('Symfony\Component\HttpFoundation\Response');
    }
}

Check out the full specification and controller after this round of refactoring.

The behaviour of our controller is now more tightly defined and our logic for fetching blog posts is now re-usable in other areas of the application. While some would argue this approach is a lot more verbose, we have gained increased testability, code re-use and we have gone some way to decoupling our Controller from Symfony 2.

If you hadn't already realised, in order for our controller to work within Symfony we will have to define it as a service and this should hopefully demonstrate some of the advantages (in terms of maintainability & testability) of doing so.

This is not the only possible approach to making controllers within Symfony more testable and arguably it is one of the more verbose, you can also look at the following for removing logic from your controllers:

BlogController knows too much

Let BlogController focus on responding to a request

See also