Monday 1 May 2023

PHP / Symfony: working through "Symfony: The Fast Track", part 5: Twig stuff, and irritation

G'day:

I'm back on the case working through "Symfony: The Fast Track". This will be part five of this series, after the first four:

The page I'm starting on is Building the User Interface, which seems more interesting than the previous section. I also see mention of my old friend Twig in the first subsection of this, so it will be good to look at that some more and see how it's moved along since I last did PHP UI stuff about 7-8 years ago (god is it that long?).


Using Twig for the Templates

The first thing I need to do is to restore a twig file I deleted during the last exercise, thinking it was redundant. So: back comes templates/base.html.twig. I'll be referring back to this, so will dump it out here, even though it's boilerplate, and not my code:

<!DOCTYPE html>
<html>
<head>
    <meta charset="UTF-8">
    <title>{% block title %}Welcome!{% endblock %}</title>
    <link
    	rel="icon"
        href="
            data:image/svg+xml,
            <svg
            	xmlns=%22http://www.w3.org/2000/svg%22
                viewBox=%220 0 128 128%22
            >
                <text y=%221.2em%22 font-size=%2296%22>⚫️</text>
            </svg>
        "
    >
    {# Run `composer require symfony/webpack-encore-bundle` to start using Symfony UX #}
    {% block stylesheets %}
        {{ encore_entry_link_tags('app') }}
    {% endblock %}

    {% block javascripts %}
        {{ encore_entry_script_tags('app') }}
    {% endblock %}
</head>
<body>
{% block body %}{% endblock %}
</body>
</html>

And one for templates/conference/index.html.twig:

{% extends 'base.html.twig' %}

{% block title %}Conference Guestbook{% endblock %}

{% block body %}
    <h2>Give your feedback!</h2>

    {% for conference in conferences %}
        <h4>{{ conference }}</h4>
    {% endfor %}
{% endblock %}

Note that it inherits from the base one, and overrides its title and body blocks. I'll not discuss all the syntax vagaries as they're well documented: Twig Documentation. It's easy enough to read and infer what's going on, anyhow.

The next section is entitled "Using Twig in a Controller" and I panicked as I thought they meant they were gonna put Twig code into a controller method (remember they've been a bit woolly about what goes in a controller already, so it would not have surprised me), but fortunately no. It's just describing how to use the twig file:

public function index(Environment $twig, ConferenceRepository $conferenceRepository): Response
{
    // return of inline HTML (!!) removed

    return new Response($twig->render(
        "conference/index.html.twig",
        ["conferences" => $conferenceRepository->findAll()]
    ));
}

The conferences there are then used in the loop in the twig.


Creating the Page for a Conference

OK so here's a cool thing Symfony does. I am creating the page for the route /conference/{id}, and if my controller method takes a Conference object, Symfony is clever enough to go "I see what yer doing there" and goes and gets the conference for that ID. EG, I've added this routing to get to my controller method:

config/routes/conference.yaml

# …

show:
    path: /{id}
    methods: [ GET ]
    controller: adamcameron\symfonythefasttrack\Controller\ConferenceController::show

And the controller method the exercise has me write is this (nothing noteworthy really):

public function show(Environment $twig, Conference $conference, CommentRepository $commentRepository): Response
{
    return new Response($twig->render(
        "conference/show.html.twig",
        [
            "conference" => $conference,
            "comments" => $commentRepository->findBy(
                ["conference" => $conference],
                ["createdAt" => "DESC"]
            )
        ]
    ));
}

The routing takes an ID; the controller receives the equivalent Conference object.

Note that whilst it's neat that Symfony does this, that controller they've had me write is borderline not great MVC in my books. I don't really think controllers should be aware of things like "repositories". Repositories are a storage consideration, and should be hidden away with the model. What should be happening here I think is that the controller receives its Conference, but the Conference should implement whatever is necessary to get its Comment collection from wherever it put it (and via whatever mechanism it did it with). This is not the controller's job. This is a good example of how using generic ORM crap leads to subpar code. I realise this is a tutorial about using Twig, but I think on the whole this book is encouraging some pretty poor design decisions because that better shows-off Symfony's bells and whistles.

Ahem. Moving along…

For reference here's the twig template for the view:

{% extends 'base.html.twig' %}

{% block title %}Conference Guestbook - {{ conference }}{% endblock %}

{% block body %}
    <h2>{{ conference }} Conference</h2>

    {% if comments|length > 0 %}
        {% for comment in comments %}
            {% if comment.photofilename %}
                <img src="{{ asset('uploads/photos/' ~ comment.photofilename) }}"/>
            {% endif %}

            <h4>{{ comment.author }}</h4>
            <small>
                {{ comment.createdAt|format_datetime('medium', 'short') }}
            </small>

            <p>{{ comment.text }}</p>
        {% endfor %}
    {% else %}
        <div>No comments have been posted yet for this conference.</div>
    {% endif %}
{% endblock %}

And the test shows the initial part of the template implementation presents the detail from the correct conference correctly:

/** @testdox /conference/{id} end point returns a 200 OK for a valid conference */
public function testShow()
{
    $kernel = new Kernel("test", true);
    $kernel->boot();

    $container = $kernel->getContainer();

    $connection = $container->get("database_connection");
    $testConference = $connection->executeQuery("
        SELECT id, city, year
        FROM conference LIMIT 1
    ")->fetchAssociative();
    if (!$testConference) {
        $this->markTestSkipped("No conferences in database, test aborted");
    }

    $client = static::createClient();
    $client->request('GET', '/conference/' . $testConference['id']);

    $this->assertResponseIsSuccessful();

    $this->assertSelectorTextContains(
        'title',
        sprintf(
            "Conference Guestbook - %s %s",
            $testConference['city'],
            $testConference['year']
        )
    );
}

I presumed it would 404 if the {id} requested wasn't in the DB:

/** @testdocs /conference/{id} end point returns a 404 if the conference is not found */
public function testShowNotFound()
{
    $client = static::createClient();
    $client->request('GET', '/conference/999999');

    $this->assertResponseStatusCodeSame(Response::HTTP_NOT_FOUND);
}

And this passes. NB: the 404 page is a general Symfony one, but that's fine. It's handling it correctly.

It's important to note that I have not written any code relating to Conferences or the storage thereof in this chapter of the exercise. I'm simply using the stuff Symfony generated for me during the previous exercise.

Oh one additional thing I did need to do was to install the twig/intl-extra dependency, because I'm using that format_datetime Twig filter. How did I know I needed to do this? Symfony told me (well, OK, I have also just scrolled ahead in the lesson, and it also told me there):

That's quite cool. I note it's claiming it's a syntax error though, which it is not. If it was a syntax error, the code wouldn't compile, let alone run to the point to which it works out I have a Twig filter missing, and which one it is and where to get it from. It's a MissingTwigExtensionError or something. But a runtime error of some sort, not a compile-time one. Still: semantics aside, it's helpful. It does seem a bit odd to me though that something in either Twig or Symfony clearly knows what all the extensions are and what external libs they're in. This seems a bit of an inappropriate overreach to me, and a bit of a break of the Open-Closed Principle. Still: productivity over dogma, eh?

Oh! So far I have no comments in the DB, so all that Twig bit about rendering them doesn't do anything. All I'm getting is this:

Time to test the comments bit, before I move on.

[… more time passes than I would like to admit…]

I went round the houses with this a bit. I had to install a bunch of Node.js stuff in my Node container because one of the Twig function's internal workings checked for [something] and if it didn't exist: threw a PHP exception. I didn't preserve the error, but this code (in show.html.twig) was the culprit:

<img src="{{ asset('uploads/photos/' ~ comment.photofilename) }}"/>

The asset function was complaining that public/build/entrypoints.json was missing. I googled about and found out I needed to have symfony/webpack-encore-bundle installed, the instructions for which (that previous link) seem to not be as complete as they could be for codebases not using Symfony Flex. Still: the residual issues I had were all fairly googleable, and easily fixed. Sorry to be a bit vague there, but I did this lot on Fri/Sat - it's now Sun - and I can't be arsed digging through my browser history to recreate the - fairly unrewarding - sequence of hurdles I bumbled over to get my PHP app to stop erroring because I didn't have some webpack shit configured properly.

BTW: this is another thing I don't think a server-side MVC framework should be investing its time in. There's plenty of client-side frameworks for handling client-side stuff. Server-side frameworks should just busy themselves with their own job. Seems to me like another case of "if your only tool is a hammer, every problem starts looking like a nail".

Anyway, my test stopped erroring, and after some small effort I got it to go green. I actually ended up with two tests here:

/** @testdox /conference/{id} also displays any comment associated with the conference */
public function testShowComments()
{
    $testConference = $this->getConferenceToTestWith();

    $client = static::createClient();
    $client->request("GET", "/conference/" . $testConference->getId());

    $this->assertResponseIsSuccessful();
    $this->assertCommentersAreCorrect($client, $testConference);
}

private function getConferenceToTestWith(): Conference
{
    $container = Container::getContainer();

    $repo = $container->get("testing.ConferenceRepository");
    $qb = $repo->createQueryBuilder("cf");

    $qb->select("cf")
        ->setMaxResults(1)
        ->leftJoin("cf.comments", "cm")
        ->groupBy("cf.id")
        ->having("count(cm.id) > 1");
    $testConference = $qb->getQuery()->getResult();

    if (!$testConference) {
        $this->markTestSkipped("No suitable conferences in database, test aborted");
    }

    return $testConference[0];
}

private function getExpectedCommenters(Conference $testConference): array
{
    $expectedCommenters = $testConference->getComments()->toArray();
    usort($expectedCommenters, function ($e1, $e2) {
        return $e2->getCreatedAt() <=> $e1->getCreatedAt();
    });
    $expectedCommenters = array_map(function ($comment) {
        return $comment->getAuthor();
    }, $expectedCommenters);
    return $expectedCommenters;
}

private function assertCommentersAreCorrect(KernelBrowser $client, Conference $testConference): void
{
    $commenters = $client
        ->getCrawler()
        ->filter("body > h4")
        ->each(function ($node) {
            return $node->text();
        });

    $expectedCommenters = $this->getExpectedCommenters($testConference);
    $this->assertEquals($expectedCommenters, $commenters);
}

This one goes right through from the controller to the DB and back: comparing the data from the DB (expected) matching the data returned in the mark-up (actual).

I don't wanna be hitting the DB in my tests as a rule, especially if it's only a mechanism to get data to then see what the logic does with it. If I'm testing the logic, I want the expediency of using canned data that best exercises the logic. I tried to write a unit test of this, but it proved to be more difficult than I had patience for. The problem being that all the "logic" is in the controller method:

public function show(
    Environment $twig,
    Conference $conference,
    CommentRepository $commentRepository
): Response {
    return new Response($twig->render(
        "conference/show.html.twig",
        [
            "conference" => $conference,
            "comments" => $commentRepository->findBy(
                ["conference" => $conference],
                ["createdAt" => "DESC"]
            )
        ]
    ));
}

So to unit test that, I need to be able to mock the collaborators. Mocking my ones - the Conference and the CommentsRepository - was easy. Dealing with the Twig Environment was beyond my patience. And I wasn't even trying to do anything fancy with it: all I wanted was to spy on it so I could test the values exiting my test unit. But Symfony is very very convoluted in how it configures things, and I could not work out a minimum Environment that would actually initialise without erroring. Please note that I was very aware of "don't mock what you don't own" during all of this, and beyond a point I had decided that this was the wrong approach as well as being a fool's errand.

In the end I wrote a more functional test. I stuck with the Environment that Symfony had configured in its DI container, and the checked the results of its rendering. This is not so bad: it's all just done in code and no hits to external services (the DB), but it resulted in more code than I'm really happy with. There's no need to pore over this: I'm not gonna discuss it further. I only included to show the amount of effort involved.

/** @testdox /conference/{id} displays correct conference and comments */
public function testShow()
{
    $conferenceTitle = "MOCKED_CONFERENCE";
    list($twigEnvironment, $conference, $commentRepository) = $this->getTestDependencies($conferenceTitle);
    $controller = new ConferenceController();

    $result = $controller->show($twigEnvironment, $conference, $commentRepository);

    $this->assertResponseIsCorrect($result, $conferenceTitle, $commentRepository);
}

private function getTestDependencies(string $conferenceTitle): array
{
    $twigEnvironment = self::getContainer()->get(TwigEnvironment::class);

    $conference = $this->getMockedConference($conferenceTitle);

    $commentRepository = $this->getMockedCommentRepository();
    return array($twigEnvironment, $conference, $commentRepository);
}

private function getMockedConference(string $conferenceTitle): Conference
{
    $conference = $this->getMockBuilder(Conference::class)
        ->disableOriginalConstructor()
        ->getMock();
    $conference
        ->expects($this->any())
        ->method("__toString")
        ->willReturn($conferenceTitle);
    return $conference;
}

private function getMockedCommentRepository(): CommentRepository
{
    $commentCount = 3;
    $commentsToTestFor = range(1, $commentCount);

    $testComments = array_map(function ($i) {
        return (new Comment())
            ->setAuthor("COMMENTER" . $i)
            ->setText("COMMENT" . $i)
            ->setCreatedAt(new \DateTimeImmutable("2020-01-0{$i}"));
    }, $commentsToTestFor);

    $commentRepository = $this->getMockBuilder(CommentRepository::class)
        ->disableOriginalConstructor()
        ->getMock();
    $commentRepository
        ->method("findBy")
        ->willReturn($testComments);
    return $commentRepository;
}

private function assertResponseIsCorrect(Response $result, string $conferenceTitle, mixed $commentRepository): void
{
    $this->assertEquals(Response::HTTP_OK, $result->getStatusCode());

    $xpathDocument = $this->getContextAsXpathDocument($result);
    $this->assertTitleIsCorrect($xpathDocument);
    $this->assertSubheadingIsCorrect($xpathDocument, $conferenceTitle);
    $this->assertCommentsAreCorrect($xpathDocument, $commentRepository);
}

private function getContextAsXpathDocument(Response $result): DOMXPath
{
    $content = $result->getContent();
    $document = new DOMDocument();
    $document->loadHTML($content, LIBXML_NOWARNING | LIBXML_NOERROR);
    $xpathDocument = new DOMXPath($document);
    return $xpathDocument;
}

private function assertTitleIsCorrect(DOMXPath $xpathDocument): void
{
    $title = $xpathDocument->query('/html/head/title[text()]');
    $this->assertCount(1, $title);
    $this->assertEquals("Conference Guestbook - MOCKED_CONFERENCE", $title->item(0)->textContent);
}

private function assertSubheadingIsCorrect(DOMXPath $xpathDocument, string $conferenceTitle): void
{
    $h2 = $xpathDocument->query('/html/body/h2[text()]');
    $this->assertCount(1, $h2);
    $this->assertEquals("$conferenceTitle Conference", $h2->item(0)->textContent);
}

private function assertCommentsAreCorrect(DOMXPath $xpathDocument, CommentRepository $commentRepository): void
{
    $testComments = $commentRepository->findBy([], []);
    $commentCount = count($testComments);

    $commentAuthors = $xpathDocument->query('/html/body/h4[text()]');
    $this->assertCount($commentCount, $commentAuthors);
    $commentTexts = $xpathDocument->query('/html/body/p[text()]');
    $this->assertCount($commentCount, $commentTexts);
    $commentDates = $xpathDocument->query('/html/body/small[text()]');
    $this->assertCount($commentCount, $commentDates);

    array_walk($testComments, function ($comment, $i) use ($commentAuthors, $commentTexts, $commentDates) {
        $this->assertEquals($comment->getAuthor(), $commentAuthors->item($i)->textContent);
        $this->assertEquals($comment->getText(), $commentTexts->item($i)->textContent);

        $commentCreatedAtFormatted = $comment->getCreatedAt()->format("M j, Y, h:i A");
        $this->assertEquals($commentCreatedAtFormatted, trim($commentDates->item($i)->textContent));
    });
}

There's a moral to all this. "Don't mock what you don't own" issues aside, testing code that is tightly-coupled to the framework - like a controller method pretty much is - is gonna have overhead. As such: keep as little code in yer controllers as possible: have yer business logic within yer model, because you "own" this, and there will be less over-engineers framework code getting in the way.


Linking Pages Together & Pagination & some refactoring

Next they've got some more Twig advice about using its methods to build predictable URLs insted of hard-coding them, eg:

<a href="/conference/{{ conference.id }}">View</a>
<a href="{{ path('conference', { id: conference.id }) }}">View</a>

Good advice. I'm not gonna dwell on it further than that, other than sticking this code into my Twig template (after I test for it, obvs). I'll spare you the code, but it'll be in source control if you decide you do want to look at it. I got slightly tripped-up there as I had changed my route's name to be conference_show, and that reference to conference is the route's name. Easily fixed though.

Next it shows how to paginate record sets in the UI, which is pleasingly straight forward. A method in the repository:


public const PAGINATOR_PER_PAGE = 2;

// …

public function getPaginator(Conference $conference, int $offset): Paginator
{
    $query = $this->createQueryBuilder('c')
        ->andWhere('c.conference = :conference')
        ->setParameter('conference', $conference)
        ->orderBy('c.createdAt', 'DESC')
        ->setMaxResults(self::PAGINATOR_PER_PAGE)
        ->setFirstResult($offset)
        ->getQuery();

    return new Paginator($query);
}

Pass a paginator instead of the articles from the controller:

public function show(
    Request $request,
    Environment $twig,
    Conference $conference,
    CommentRepository $commentRepository
): Response {
    $offset = max(0, $request->query->getInt('offset', 0));
    $paginator = $commentRepository->getPaginator($conference, $offset);

    return new Response($twig->render(
        "conference/show.html.twig",
        [
            "conference" => $conference,
            "comments" => $commentRepository->findBy(["conference" => $conference], ["createdAt" => 'DESC']),
            "comments" => $paginator,
            'previous' => $offset - CommentRepository::PAGINATOR_PER_PAGE,
            'next' => min(count($paginator), $offset + CommentRepository::PAGINATOR_PER_PAGE)
        ]
    ));
}

And then add the pagination stuff to the view:

<div>There are {{ comments|length }} comments.</div>
{% for comment in comments %}
    {% if comment.photofilename %}
        <img src="{{ asset('uploads/photos/' ~ comment.photofilename) }}"/>
    {% endif %}

    <h4>{{ comment.author }}</h4>
    <small>
        {{ comment.createdAt|format_datetime('medium', 'short') }}
    </small>

    <p>{{ comment.text }}</p>
{% endfor %}

{% if previous >= 0 %}
    <a href="{{ path('conference_show', { id: conference.id, offset: previous }) }}">Previous</a>
{% endif %}
{% if next < comments|length %}
    <a href="{{ path('conference_show', { id: conference.id, offset: next }) }}">Next</a>
{% endif %}

The only change I made to this was that the suggested name for the pagination method was getCommentPaginator, but given it was in the CommentRepository, I thought repeating "comment" there was tautological, so changed it to getPaginator.

Oh, the page now looks like this:

Cool.

One thing to note here is that in refactoring my tests of the ConferenceController show method to accommodate the pagination, I pushed the coupling of the test class over the top of what PHPMD was happy with (the threshold is the default 13 classes), so I refactored that into three classes: Test, Assertions and Dependencies.

tests
`-- php
    `-- Functional
        `-- Controller
            `-- ConferenceController
                |-- Index
                |   `-- Test.php
                `-- Show
                    |-- Assertions.php
                    |-- Dependencies.php
                    `-- Test.php

I think this is all caused by testing code in a controller; having to mess around with Requests and Environments and having to test for rendered mark-up instead of just values-in and value-out adds "unnecessary" overhead. I am realy irked (in a laughing way) by what I just read in the next section which is the refactoring bit…

You might have noticed that both methods in ConferenceController take a Twig environment as an argument. Instead of injecting it into each method, let's leverage the render() helper method provided by the parent class

Symfony the Fast Track › Building the User Interface › Refactoring the Controller

Oh FFS. Ha. OK. Fine. Tell me that now.

So I can simplify things a bit:

public function show(
    Request $request,
    Environment $twig,
    Conference $conference,
    CommentRepository $commentRepository
): Response {
    $offset = max(0, $request->query->getInt('offset', 0));
    $paginator = $commentRepository->getPaginator($conference, $offset);

    return new Response($twig->render('conference/index.html.twig', [
    return $this->render(
        "conference/show.html.twig",
        [
            "conference" => $conference,
            "comments" => $paginator,
            'previous' => $offset - CommentRepository::PAGINATOR_PER_PAGE,
            'next' => min(count($paginator), $offset + CommentRepository::PAGINATOR_PER_PAGE)
        ]
    );
    ]));
}

That's good, but I really don't know why they didn't lead with that. There was nothing that relied on the Twig Environment in those methods, so why pass it in? Just to give an extra paragraph at the end?

Oh I also had to tweak a couple of the tests that broke because of this: for that render method to be available, I needed to initialise the controller class with the DI container. This was easy though:

$controller = new ConferenceController();
$container = self::getContainer();
$controller->setContainer($container);

End of section / article

Man. I only got through one page of the docs with this article, and it also took me four bites to write, even though it didn't really cover much ground. I think the issue is that a) I lack self-discipline; b) I'm not finding this stuff particularly compelling. I also got deeply frustrated with the amount of dicking around I had to do to try to convince Symfony to let me at its dependencies when I was trying to test stuff, and also the waste of time that was getting Encore working, because - as I said earlier - client-side libs are nothing to do with a server-side MVC framework, and should not get in the way. The more I use Symfony, the more I miss Silex.

I'm not looking forward to the re-drafting & read-throughs ofthis before I press "send", as I suspect it's a bit of a half-hearted mess. But I've come this far, so I'm buggered if I'm not publishing it. Well done if you read this far.

Righto.

--
Adam