Showing posts with label PHP. Show all posts
Showing posts with label PHP. Show all posts

Saturday 1 April 2023

Symfony / Doctrine / DBAL: convincing/configuring it to use a PrimaryReadReplicaConnection connection

G'day:

A while back I documented how to create/configure a PrimaryReadReplicaConnection connection in PHP. PrimaryReadReplicaConnection is the replacement for MasterSlaveConnection, which has been retired due to socially-insensitive nomenclature. This is all in "PHP: PrimaryReadReplicaConnection - configuration / usage example".

Today's exercise is to get one to work in my Symfony project (adamcameron/SymfonyFastTrack).

Symfony's default DB connectivity is done via the DATABASE_URL environment variable which must be set (docs: Configuring Doctrine ORM). This is fine for simple situations, even though I personally think it's a daft way of handling connnection parameters: it's a bit "type couply" to using a URL to connect to a DB, which is not the only way of doing it. But falls flat pretty quickly once not in a simple situation. The problem is that the DATABASE_URL allows only for a single connection. It's gonna be pretty common to be using primary / replicas I think. I guess perhaps the Symfony thinking(?) is that this is better handled on a DB load balancer than in the app. However: I don't have one of those, and i have also seen enterprise-scale PHP-driven operations that also simply use a PrimaryReadReplicaConnection for this. I have a PrimaryReadReplicaConnection driver. I need to make it work. This is not well / clearly / at all documented.

Firstly: there is no escaping it. One needs to have DATABASE_URL set. And it needs to be valid. This is even if you remove references to it in config: the connection won't configure (and the application won't work) unless it exists. The Symfony docs say to use override_url to prevent this behaviour:

When specifying a url parameter, any information extracted from that URL will override explicitly set parameters unless override_url is set to true. An example database URL would be mysql://snoopy:redbaron@localhost/baseball, and any explicitly set driver, user, password and dbname parameter would be overridden by this URL. See the Doctrine DBAL documentation for more information.
Doctrine DBAL Configuration

However that link to the Doctrine docs is obsolete, and that setting has been deprecated since 2.4 (we're on 2.9 now):

UPGRADE FROM 2.3 to 2.4
=======================

Configuration
--------

 * Setting the `host`, `port`, `user`, `password`, `path`, `dbname`, `unix_socket`
   or `memory` configuration options while the `url` one is set has been deprecated.
 * The `override_url` configuration option has been deprecated.
DoctrineBundle/UPGRADE-2.4.md

OK. Great. Thanks for that. I must provide a DATABASE_URL. But any explicitly-set overrides I set in the actual connection config are ignored. And the functionality that used to permit me to actively say "FFS will you just do what yer told" (ie: override_url) has been deprecated. And the URL approach doesn't actually support the options I want to use, so it's completely useless to me. This seems pretty mickey mouse to me, from where I'm standing. But all right then; I'll play yer silly game.

After some trial end error (because there's no documentation that I can find), I came up with this (in config/packages/doctrine.yaml):

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                wrapper_class: Doctrine\DBAL\Connections\PrimaryReadReplicaConnection
                dbname: '%env(resolve:POSTGRES_PRIMARY_DB)%'
                host: '%env(resolve:POSTGRES_PRIMARY_HOST)%'
                port: '%env(resolve:POSTGRES_PRIMARY_PORT)%'
                user: '%env(resolve:POSTGRES_PRIMARY_USER)%'
                password: '%env(resolve:POSTGRES_PRIMARY_PASSWORD)%'
                driver: pdo_pgsql
                server_version: 15
                charset: utf8
                replicas:
                    replica1:
                        dbname: '%env(resolve:POSTGRES_REPLICA_DB)%'
                        host: '%env(resolve:POSTGRES_REPLICA_HOST)%'
                        port: '%env(resolve:POSTGRES_REPLICA_PORT)%'
                        user: '%env(resolve:POSTGRES_REPLICA_USER)%'
                        password: '%env(resolve:POSTGRES_REPLICA_PASSWORD)%'
                        charset: utf8


I have used the primary values in DATABASE_URL (in docker/php/envVars.public):

# …

DATABASE_URL="postgresql://${POSTGRES_PRIMARY_USER}:${POSTGRES_PRIMARY_PASSWORD}@${POSTGRES_PRIMARY_HOST}:${POSTGRES_PRIMARY_PORT}/${POSTGRES_PRIMARY_DB}?serverVersion=15&charset=utf8"

To test that this actually works, and it doesn't just send everything to the primary, I have set up two completely separate DBs, with different credentials (and host, port, and even database name). These are separate PostgreSQL containers. I'll push the config to Github, but I won't repeat it here as it's really just a duplication of what I already had in this codebase. But here's links to the relevant bits of various files:

It's important to note that in this experimentation the two DBs are completely separate, and there's no replication going on. I'm just testing the connection config is correctly switching between primary and replica, and it's easier to do when the databases have different data in them.

# primary.test
"id"    "value"
101    "Test row 1"
102    "Test row 2"
104    "PRIMARY"
105    "TEST_VALUE"
106    "TEST_VALUE"

(You can see I've already been running some tests there)

# replica.test
101    "Test row 1"
102    "Test row 2"
103    "REPLICA"
/** @testdox Writing to primary definitely does not impact the replica */
public function testWritingToPrimaryDoesNotImpactReplica()
{
    $sqlForCount = "SELECT COUNT(1) AS count FROM test";

    $primaryConnection = $this->getPrimaryConnection();
    $replicaConnection = $this->getReplicaConnection();

    $initialPrimaryCount = $primaryConnection->executeQuery($sqlForCount)->fetchOne();
    $initialReplicaCount = $replicaConnection->executeQuery($sqlForCount)->fetchOne();
    $this->assertNotEquals(
        $initialPrimaryCount,
        $initialReplicaCount,
        "Test aborted as the test requires the DBs to NOT be in sync (and they are)"
    );

    $defaultConnection = $this->getDefaultConnection();
    $this->assertFalse($defaultConnection->isConnectedToPrimary(), "Connection did not start on replica");

    $initialDefaultCount = $defaultConnection->executeQuery($sqlForCount)->fetchOne();
    $this->assertEquals($initialDefaultCount, $initialReplicaCount, "Row count from default should match replica");

    $defaultConnection->executeStatement("INSERT INTO test (value) VALUES (?)", ["TEST_VALUE"]);
    $this->assertTrue(
        $defaultConnection->isConnectedToPrimary(),
        "Connection did not switch to primary after INSERT"
    );

    $countFromDefault = $defaultConnection->executeQuery($sqlForCount)->fetchOne();
    $countFromPrimary = $primaryConnection->executeQuery($sqlForCount)->fetchOne();
    $countFromReplica = $replicaConnection->executeQuery($sqlForCount)->fetchOne();

    $this->assertEquals($countFromDefault, $countFromPrimary, "Row count from default should match primary");
    $this->assertEquals($initialReplicaCount, $countFromReplica, "Row count from replica should not have changed");
}

Here:

  • I'm creating three connections, one each that directly queries the primary and replica DBs respectively, and one that is the app's default connection (which is the PrimaryReadReplicaConnection one).
  • I get row counts from each, making sure they start off differently (otherwise it'll be harder to see the difference, further down).
  • I then check the row count from the default connection matches the replica (it should start using the replica connection.
  • Then I insert a row using the default connection. This has a dual effect:
  • The default connection should now be pointing to the primary database.
  • So a row count from that connection should match the primary now.
  • And the replica count should be unchanged.

And the helper functions:

private function getPrimaryConnection(): Connection
{
    return DriverManager::getConnection([
        "dbname" => getenv("POSTGRES_PRIMARY_DB"),
        "user" => getenv("POSTGRES_PRIMARY_USER"),
        "password" => getenv("POSTGRES_PRIMARY_PASSWORD"),
        "host" => getenv("POSTGRES_PRIMARY_HOST"),
        "port" => getenv("POSTGRES_PRIMARY_PORT"),
        "driver" => "pdo_pgsql"
    ]);
}

private function getReplicaConnection(): Connection
{
    return DriverManager::getConnection([
        "dbname" => getenv("POSTGRES_REPLICA_DB"),
        "user" => getenv("POSTGRES_REPLICA_USER"),
        "password" => getenv("POSTGRES_REPLICA_PASSWORD"),
        "host" => getenv("POSTGRES_REPLICA_HOST"),
        "port" => getenv("POSTGRES_REPLICA_PORT"),
        "driver" => "pdo_pgsql"
    ]);
}

public function getDefaultConnection(): PrimaryReadReplicaConnection
{
    $kernel = new Kernel("test", true);
    $kernel->boot();

    $container = $kernel->getContainer();

    return $container->get("doctrine.dbal.default_connection");
}

And that all worked, so that's good.

I'm gonna tag all this in GitHub as 1.6, but I'm gonna roll-back the two different DBs and just point the replica to the same DB as the primary from now on. I want the PrimaryReadReplicaConnection connection in place in this project, but I don't wanna horse around keeping throw-away DBs actually in sync. That version is tagged as 1.6.1.

And now tomorrow I can get on with what I actually wanted to be writing about today :-|

Righto.

--
Adam

Tuesday 28 March 2023

PHP / Symfony: working through "Symfony: The Fast Track", part 2: creating a controller (eventually)

G'day:

Sit. rep.

OK so last time (PHP / Symfony: working through "Symfony: The Fast Track", part 1: preparation and pre-requisites (and not actually any Symfony!)) I didn't make much Symfony progress, but I got my ducks (and my Docker containers) in a row, ready to start. I had worked through "Symfony: The Fast Track › Checking your Work Environment", and prepped as much as I could. Running symfony book:check-requirements yields this:

root:/var/www# symfony book:check-requirements
[OK] Git installed
[OK] PHP installed version 8.2.4 (/usr/local/bin/php)
[OK] PHP extension "xsl" installed - required
[OK] PHP extension "tokenizer" installed - required
[OK] PHP extension "xml" installed - required
[OK] PHP extension "redis" installed - optional - needed only for chapter 31
[OK] PHP extension "amqp" installed - optional - needed only for chapter 32
[OK] PHP extension "json" installed - required
[OK] PHP extension "session" installed - required
[OK] PHP extension "curl" installed - optional - needed only for chapter 17 (Panther)
[OK] PHP extension "pdo_pgsql" installed - required
[OK] PHP extension "mbstring" installed - required
[OK] PHP extension "openssl" installed - required
[OK] PHP extension "sodium" installed - required
[OK] PHP extension "zip" installed - optional - needed only for chapter 17 (Panther)
[OK] PHP extension "gd" installed - optional - needed only for chapter 23 (Imagine)
[OK] PHP extension "ctype" installed - required
[OK] PHP extension "intl" installed - required
[OK] Composer installed
[KO] Cannot find Docker, please install it https://www.docker.com/get-started
[KO] Cannot find Docker Compose, please install it https://docs.docker.com/compose/install/
[KO] Cannot find the npm package manager, please install it https://www.npmjs.com/
  
You should fix the reported issues before starting reading the book.
root:/var/www#

I'm not bothered about the last three as I'm already in a Docker container, and Node.js is running in a separate container, pointing at the same application directory. Thinking about it, it's quite odd they require using Docker, but it doesn't occur to them to run the app itself in a container. I hope my approach doesn't some how unstick me somewhere along the way.

Right. Let's crack on with it. Next page…


Introducing the project

Here they discuss "learning is doing", whilst not actually doing anything. But there's a diagram of where we get to in this project:

(this is no doubt © Symfony. I am using it in the context of critically evaluating their book, so I think this is "fair use". If someone from Symfony wishes to disagree: hit me up).

That's ambitious! Cool.

The also list how little code is needed to achieve this:

  • 20 PHP classes under src/ for the website;
  • 550 PHP Logical Lines of Code (LLOC) as reported by PHPLOC;
  • 40 lines of configuration tweaks in 3 files (via attributes and YAML), mainly to configure the backend design;
  • 20 lines of development infrastructure configuration (Docker);
  • 100 lines of production infrastructure configuration (Platform.sh);
  • 5 explicit environment variables.

ibid.

What's missing from that list? No tests. That's disappointing in 2023. I realise this book is about Symfony, but no code exercise in 2023 should normalise "no tests" as an approach. I will try to take a TDD approach with everything, if it seems viable (which it should be?).

There's instructions how to download / install / start the final working app. This seems a bit pre to me. I want to write the thing, not just look at it. "Learning is doing", I read somewhere, recently. I hope this step is not integral to the rest of the process cos I ain't doing it.

(Ah OK, a few paras on the author alludes to DIYing the code, so I guess this is just for reference. Good-o).


Going from Zero to Production

OK, so I now need to create a new Symfony app. This is done via a Symfony helper:

symfony new guestbook --version=6.2 --php=8.1 --webapp --docker --cloud

I know from past experience ("Part 6: Installing Symfony" and "Symfony: installing in my PHP8 container (for a second time, as it turns out)") that Symfony whines if I try to create a new project in a directory that already has stuff in it, so I'll do what I did last time: install it in a temp directory then migrate stuff back into my project directory. I will also lift the test I used last time to verify it's working OK. The code is in that second article I linked to above, I'll not repeat it here, but will link to it in the repo once I've tagged it.

I also know that Symfony uses a bad namespace for its apps (Symfony: getting rid of App namespace and using a well-formed one), so I'm going to sort that out straight away too. I'll not repeat any of the steps to make these changes and get that test passing, unless there's anything different I need to do this time. It's well-documented in that earlier article.

Bear with me for a bit. Back soon …

I ran this in a shell within my PHP container:

root:/var/www# cd /var/tmp
root:/var/tmp# symfony new SymfonyFastTrack --version=6.2 --php=8.2 --webapp --docker --cloud

I then copied the ensuing /var/tmp/SymfonyFastTrack (source) directory down to my host machine and did a file compare on that directory and my existing SymfonyFastTrack (destination) directory, doing the following:

  • Where the source directory had files not in destination, I simply copied them across.
  • Where there were clear additions to a file for the new Symfony app, I just merged it (sometimes by hand) into my own version of the file. EG this diff of the two versions of composer.json:

  • There was a docker-compose.yml and docker-compose-override.yml plonked into the root directory. These added containers for a stub/fake STMP server and a "BlackFire" server (whatever that is). I lifted the relevant bits of that out and put it in my own docker/docker-compose.yml file and deleted the ones in the root.
  • I followed the instructions about setting up a Blackfire account, got the various keys I needed and stuck them into a docker/blackfire/envVars.private (not source controlled) file which I got the docker/docker-compose.yml to load into the Blackfire container.
  • Symfony hard-codes its APP_SECRET value into the codebase. If it's supposed to be a secret then treat it that way: I lifted it out and set an env var for it (via docker/php/envVars.private (again, not source controlled)).
  • I set Symfony's DATABASE_URL as an environment variable instead of Symfony's config files. No reason to really, except every component of it (host, user, password, DB etc) came from adjacent environment variables, so I figured I'd keep it all in one place.
  • The /config/packages/doctrine.yaml file was setting a db_suffix value which had the effect of the Connection object trying to connect to a DB called db1test (it's just db1. I have no reason to add suffixes to my DB name because of the environment). I'm not sure why this would be out-of-the-box behaviour but… I suspect this is just one more example of me going "please stop trying to be helpful, Symfony", and I should get used to it. Oh yeah, this surfaced because the tests put the APP_ENV into test mode, and I have integration tests (see below) hitting the DB. Yay for tests.

I also wrote a few tests; or, as I said above: lifted them from another project ([during proofreading]: I also said I wasn't going to repeat them here… hrm…):

  • To test Symfony came up OK:

    /** @testdox It serves the Symfony welcome page after installation */
    public function testSymfonyWelcomeScreenDisplays()
    {
        $ch = curl_init();
        curl_setopt_array($ch, [
            CURLOPT_URL => "http://host.docker.internal:8062/",
            CURLOPT_RETURNTRANSFER => 1
        ]);
        $response = curl_exec($ch);
    
        if (curl_error($ch)) {
            $this->fail(sprintf("curl failed with [%s]", curl_error($ch)));
        }
        $this->assertEquals(Response::HTTP_NOT_FOUND, curl_getinfo($ch, CURLINFO_HTTP_CODE));
        $document = new \DOMDocument();
    
        $document->loadHTML($response, LIBXML_NOWARNING | LIBXML_NOERROR);
    
        $xpathDocument = new \DOMXPath($document);
    
        $hasTitle = $xpathDocument->query('/html/head/title[text() = "Welcome to Symfony!"]');
        $this->assertCount(1, $hasTitle, "Could not find title 'Welcome to Symfony!'");
    }
    
  • To test its console also runs OK:

    /** @testdox It can run the Symfony console in a shell */
    public function testSymfonyConsoleRuns()
    {
        $appRootDir = dirname(__DIR__, 3);
    
        exec("{$appRootDir}/bin/console --help", $output, $returnCode);
    
        $this->assertEquals(0, $returnCode);
        $this->assertNotEmpty($output);
    }
    
  • And to test that DATABASE_URL got loaded into the Connection object in Symfony's DI container:
    /** @testdox It has configured the Connection in the container with the correct DATABASE_URL */
    public function testContainerConnection()
    {
        $container = $this->getContainer();
        $connection = $container->get("database_connection");
        $result = $connection->executeQuery("SELECT version() AS version");
    
        $this->assertStringStartsWith("PostgreSQL 15", $result->fetchOne());
    }
    

I had the first two tests in there before I started the app installation process - a nod to TDD - but the third one I wrote after the fact to make sure my messing around had worked.

All my tests still/now pass, so I'm ready for the next step…

The rest of the page discussing pushing the "nothing" I so far have to production, under the guise of "deploy early and often". I have no aspirations to ship this to production, it's just a learning exercise. I guess there could be vagaries of deploying in "Production mode", but there's not even enough here yet to worry about that I think. I'll skip this for now.


Adopting a Methodology

A page dedicated to reminding one to commit one's work. I'm beginning to think the author was being paid by the word ;-)


Troubleshooting Problems

This page has some stuff about the debugging bumpf Symfony has at the bottom of the page (in dev mode wth debug on):

Handy.

It also discusses different environment modes. EG in dev mode the default first-install index page 404s with:

Switch to prod mode, and one gets this instead:

This is most easily done in .env:

# In all environments, the following files are loaded if they exist,
# the latter taking precedence over the former:
#
#  * .env                contains default values for the environment variables needed by the app
#  * .env.local          uncommitted file with local overrides
#  * .env.$APP_ENV       committed environment-specific defaults
#  * .env.$APP_ENV.local uncommitted environment-specific overrides
#
# Real environment variables win over .env files.
#
# …
APP_ENV=devprod

I left the guidance about the order of env file loading there, and I like how real env vars trump them all.

I tried to write functional tests loading the app in different modes and checking their output, but this won't work:

/** @testdox Testing variations of APP_ENV */
class AppEnvTest extends WebTestCase
{
    /** @testdox it shows the Welcome page for the / 404 if in dev mode */
    public function testDevMode()
    {
        $client = static::createClient([
            'environment' => 'dev',
            'debug' => false
        ]);

        $client->request('GET', '/');

        $this->assertResponseStatusCodeSame(Response::HTTP_NOT_FOUND);
        $this->assertSelectorTextContains('h1', 'Welcome to Symfony');
    }

    /** @testdox it shows a plain error for / 404 if in prod mode */
    public function testProdMode()
    {
        $client = static::createClient([
            'environment' => 'prod',
            'debug' => false
        ]);

        $client->request('GET', '/');

        $this->assertResponseStatusCodeSame(Response::HTTP_NOT_FOUND);
        $this->assertSelectorTextContains('h1', 'Oops! An Error Occurred');
    }
}

I got an initially enigmatic error:

LogicException: You cannot create the client used in functional tests if the "framework.test" config is not set to true.

But I tracked it back through the code and found this:

protected static function getContainer(): ContainerInterface
{
    if (!static::$booted) {
        static::bootKernel();
    }

    try {
        return self::$kernel->getContainer()->get('test.service_container');
    } catch (ServiceNotFoundException $e) {
        throw new \LogicException('Could not find service "test.service_container". Try updating the "framework.test" config to "true".', 0, $e);
    }
}

This is called as part of my createClient call in the test. Bottom line: the only mode one can do functional tests in is in test mode. I'm not entirely on board with this, if I'm honest. This is not Symfony application code; this is code they have specifically written for testing. If I wanna create a test client in other modes, that ought to be up to me, not Symfony. A problem I have with these large frameworks that tout themselves as being "opinionated" (as if this is a good thing) is that they start second-guessing stuff, and every second-guess they make closes a door on a situation that didn't occur to them. This is why I much preferred simpler frameworks like Silex (RIP), which just did routing, DI and exposed a coupla other things, but otherwise got out of my way. Never mind though, it's not a biggie.


Creating a Controller

Cool. Maybe I'll be able to write some code now.

Not yet.

They show some of the symfony console stuff which is very… thorough. Here's the list function showing all the options under the make functioanlity:

root:/var/www# symfony console list make
Symfony 6.2.7 (env: dev, debug: true) #StandWithUkraine https://sf.to/ukraine

Usage:
  command [options] [arguments]

Options:
  -h, --help            Display help for the given command. When no command is given display help for the list command
  -q, --quiet           Do not output any message
  -V, --version         Display this application version
      --ansi|--no-ansi  Force (or disable --no-ansi) ANSI output
  -n, --no-interaction  Do not ask any interactive question
  -e, --env=ENV         The Environment name. [default: "dev"]
      --no-debug        Switch off debug mode.
  -v|vv|vvv, --verbose  Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

Available commands for the "make" namespace:
  make:auth                   Creates a Guard authenticator of different flavors
  make:command                Creates a new console command class
  make:controller             Creates a new controller class
  make:crud                   Creates CRUD for Doctrine entity class
  make:docker:database        Adds a database container to your docker-compose.yaml file
  make:entity                 Creates or updates a Doctrine entity class, and optionally an API Platform resource
  make:fixtures               Creates a new class to load Doctrine fixtures
  make:form                   Creates a new form class
  make:functional-test        Creates a new test class
  make:message                Creates a new message and handler
  make:messenger-middleware   Creates a new messenger middleware
  make:migration              Creates a new migration based on database changes
  make:registration-form      Creates a new registration form system
  make:reset-password         Create controller, entity, and repositories for use with symfonycasts/reset-password-bundle
  make:serializer:encoder     Creates a new serializer encoder class
  make:serializer:normalizer  Creates a new serializer normalizer class
  make:stimulus-controller    Creates a new Stimulus controller
  make:subscriber             Creates a new event subscriber class
  make:test                   [make:unit-test|make:functional-test] Creates a new test class
  make:twig-component         Creates a twig (or live) component
  make:twig-extension         Creates a new Twig extension with its runtime class
  make:unit-test              Creates a new test class
  make:user                   Creates a new security user class
  make:validator              Creates a new validator and constraint class
  make:voter                  Creates a new security voter class
root:/var/www#
  

I'm buggered if I know what any of that means, but… yeah cool. All that stuff.

It looks like I'm going to be creating my controller with a wziard instead of just… creating/editing some files. I guess it saves me copying and pasting some boilerplate. Let's see what it does:

root:/var/www# symfony console make:controller ConferenceController
 ! [NOTE] It looks like your app may be using a namespace other than "App".
 !
 !        To configure this and make your life easier,
          see: https://symfony.com/doc/current/bundles/SymfonyMakerBundle/index.html#configuration


In Generator.php line 62:

Could not determine where to locate the new class "App\Controller\ConferenceController", maybe try with a full namespace like "\My\Full\Namespace\ConferenceController"
make:controller [--no-template] [--] [<controller-class>] root:/var/www#

That's actually quite helpful, cool.

Looking at those docs, I need to create config/packages/dev/maker.yaml:

# config/packages/dev/maker.yaml
# create this file if you need to configure anything
maker:
    # tell MakerBundle that all of your classes live in an
    # Acme namespace, instead of the default App
    # (e.g. Acme\Entity\Article, Acme\Command\MyCommand, etc)
    root_namespace: 'adamcameron\symfonythefasttrack'

Let's see if that helps.

After I saw a post on Stack Overflow advising that after adding the file I also needed to clear the cache, it all worked fine:

root@4b91bad4b422:/var/www# symfony console make:controller ConferenceController

 created: src/Controller/ConferenceController.php
 created: templates/conference/index.html.twig


Success!
Next: Open your new controller class and add some pages! root@4b91bad4b422:/var/www#

It just occurs to me that I've let down Team TDD a bit here. I shoulda had a test ready for this lot! I should have read ahead to see what I was gonna end up with, when running this code. I've written a test before I ran the code though:

/** @testdox Tests the functionality of ConferenceController */
class ConferenceControllerTest extends WebTestCase
{
    /** @testdox The index action returns a successful response */
    public function testIndex()
    {
        $client = static::createClient();
        $client->request("GET", "/conference");

        $this->assertResponseIsSuccessful();
        $this->assertSelectorTextContains("h1", "Hello ConferenceController!");
    }
}

(I checked the twig file for the file content).

This doesn't help with the "red" part of red-green-refactor, but it's something at least.

Looking at the generated controller:

class ConferenceController extends AbstractController
{
    #[Route('/conference', name: 'app_conference')]
    public function index(): Response
    {
        return $this->render('conference/index.html.twig', [
            'controller_name' => 'ConferenceController',
        ]);
    }
}

I'm not a fan of munging routing into controllers, so I'm gonna move that back into the routing config (routes.yaml):

controllers:
    resource:
        path: ../src/Controller/
        namespace: adamcameron\symfonythefasttrack\Controller
    type: attribute

conference:
    resource: routes/conference.yaml
    prefix: /conference/
conference:
    path: /
    methods: [GET]
    controller: adamcameron\symfonythefasttrack\Controller\ConferenceController::index

And here the test helps: I can refactor that routing config, and my test stays green!

Everything else seems fine.

The rest of the page messes with that controller to show using inline HTML in the controller (just why? would you even show that??), and output some debugging stuff. The debug stuff was interesting. One can put a dump in one's code and it will send the dump down to the browser, but different from var_dump it's not part of the main response, it's part of the debug bar (which I have only just clocked is a separate request, done via AJAX from the main response). Example (no link to this code as I'm not keeping it):

public function index(Request $request): Response
{
    dump($request);
    return new Response("<html><body></body></html>");
}

(note that for the debug bar to show up, it needs to be an "HTML" response with a body element)

Cool.


Right, that was a lot of effort to make not much progress. There was quite a bit of googling in places when things didn't quite go how I expected, which I just summarised in the text. Anyway, I'm nackered so am gonna draw a line under this one.

All the code for this lot is tagged as 1.5.

Oh and I've done the next article in this series: PHP / Symfony: working through "Symfony: The Fast Track", part 3: doing some ORM / DB config.

Righto.

--
Adam

Saturday 25 March 2023

PHP / Symfony: working through "Symfony: The Fast Track", part 1: preparation and pre-requisites (and not actually any Symfony!)

G'day:

This new PHP app we're shifting in at work is gonna be running Symfony. The other bods on the dev team have been working through "Symfony: The Fast Track", and I figured I pretty much ought to as well. So… here goes. It's a big "book", so am not sure how much of it I'll be able to get through in one go, so this will be a multi-part series. Or just one part if I get bored and move on to something else, as if often my wont on this blog. Hopefully it'll keep me motivated. The stuff I've seen from the other peeps on the team is interesting though, so who knows.


Pre-requisites

I need PHP and Nginx running so I'll install the config for those first. I've actually already done this, tested it, and tagged it up on GitHub as adamcameron/SymfonyFastTrack. The baseline for this was version 1.0 of my PHP 8 repo. This repo has moved on a bit since it was a bare baseline, but that v1.0 tag was round about right and with a minimum of unnecessary crap, so I started with that: cloned it, and slapped the files in my SymfonyFastTrack repo. I fiddled around with some stuff, but nothing of consequence. Anyhoo, there's an installation set of two Docker containers there: Nginx and PHP. They expose the website on http://localhost:8062/ on yer host machine. All the instructions are in the README.md file for the repo.

The only other thing I've done thusfar is set up a project in IntelliJ to talk to my PHP container so it can run tests and code quality tooling, etc.

There is nothing to do with Symfony in this codebase yet. This is next. When I start reading that book. Which'll be now.

Symfony: The Fast Track

I'm gonna start from the front and work my way to the back.

First coupla sections

There are "Acknowledgements" and "What is it about?" up front. Necessary I suppose, but no real value beyond the author's obligation to write them.

Checking your Work Environment

The interesting things in here is that they say up front I'm gonna need PostgreSQL and Node.js installed. Oh and the Symfony CLI (which I actually have installed in the PHP container already, as it turns out. I forgot about that).

Other than that the requirements for this - to start with - are:

  • An IDE - IntelliJ in my case.
  • A terminal. Well, um, yeah obviously (isn't it?).
  • Git. Yup, done.
  • PHP. The book is written for 8,1, and I have 8.2 installed.
  • Some PHP extensions, some of which I do not have installed yet.
  • Composer. Yup, obviously.
  • Docker.

One good thing the Symfony CLI does is have a check to see if I meet the book's prerequisites:

root:/var/www# symfony book:check-requirements
[OK] Git installed
[OK] PHP installed version 8.2.4 (/usr/local/bin/php)
[KO] PHP extension "xsl" not found, please install it - not found
[OK] PHP extension "tokenizer" installed - required
[OK] PHP extension "xml" installed - required
[KO] PHP extension "redis" not found - optional - needed only for chapter 31
[KO] PHP extension "amqp" not found - optional - needed only for chapter 32
[OK] PHP extension "json" installed - required
[OK] PHP extension "session" installed - required
[OK] PHP extension "curl" installed - optional - needed only for chapter 17 (Panther)
[KO] PHP extension "pdo_pgsql" not found, please install it - required
[OK] PHP extension "mbstring" installed - required
[OK] PHP extension "openssl" installed - required
[OK] PHP extension "sodium" installed - required
[OK] PHP extension "zip" installed - optional - needed only for chapter 17 (Panther)
[KO] PHP extension "gd" not found - optional - needed only for chapter 23 (Imagine)
[OK] PHP extension "ctype" installed - required
[OK] PHP extension "intl" installed - required
[OK] Composer installed
[KO] Cannot find Docker, please install it https://www.docker.com/get-started
[KO] Cannot find Docker Compose, please install it https://docs.docker.com/compose/install/
[KO] Cannot find the npm package manager, please install it https://www.npmjs.com/
  
You should fix the reported issues before starting reading the book.
root:/var/www#

It's not seeing Docker as I'm in a shell of a container already. But I'll get all that other stuff installed now.


PHP extensions

That was mostly googling how to install the various extensions that were missing, doing it manually in the running container to test they were working, then adding the steps to docker/php/Dockerfile so they're there when I rebuild the containers. These were the additions to the Dockerfile:

FROM php:8.2.4-fpm

RUN ["apt-get", "update"]
RUN ["apt-get", "install", "-y", "zip", "unzip", "git", "vim"]

COPY php.ini /usr/local/etc/php/php.ini
COPY home/.bash_history /root/.bash_history
COPY home/.bashrc /root/.bashrc

COPY --from=composer:latest /usr/bin/composer /usr/local/bin/composer

RUN pecl install xdebug && docker-php-ext-enable xdebug
COPY conf.d/xdebug.ini /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini
COPY conf.d/error_reporting.ini /usr/local/etc/php/conf.d/error_reporting.ini

RUN apt-get install -y libicu-dev && docker-php-ext-configure intl && docker-php-ext-install intl

RUN ["apt-get", "install", "-y", "libpng-dev", "libpq-dev", "librabbitmq-dev", "libxslt1-dev", "libz-dev", "libzip-dev"]
RUN docker-php-ext-configure bcmath && docker-php-ext-install bcmath
RUN docker-php-ext-configure gd && docker-php-ext-install gd
RUN docker-php-ext-configure opcache && docker-php-ext-install opcache
RUN docker-php-ext-configure pdo_mysql && docker-php-ext-install pdo_mysql
RUN docker-php-ext-configure pdo_pgsql && docker-php-ext-install pdo_pgsql
RUN docker-php-ext-configure xsl && docker-php-ext-install xsl
RUN docker-php-ext-configure zip && docker-php-ext-install zip

RUN pear config-set php_ini /usr/local/etc/php/php.ini && pecl install amqp && docker-php-ext-enable amqp
RUN pear config-set php_ini /usr/local/etc/php/php.ini && pecl install redis && docker-php-ext-enable redis

RUN curl -1sLf 'https://dl.cloudsmith.io/public/symfony/stable/setup.deb.sh' | bash
RUN ["apt-get", "install", "-y", "symfony-cli"]

WORKDIR /var/www
ENV COMPOSER_ALLOW_SUPERUSER 1

I also enforced as many of them as I could in composer.json, eg:

{
    "name" : "adamcameron/symfonythefasttrack",
    "description" : "Symfony: The Fast Track",
    "type" : "project",
    "license" : "LGPL-3.0-only",
    "require": {
        "php" : "^8.2",
        "ext-bcmath": "*",
        "ext-ctype": "*",
        "ext-curl": "*",
        "ext-dom": "*",
        "ext-gd": "*",
        "ext-iconv": "*",
        …

Which worked fine for all of them except the amqp: composer validate just went "what the hell is that?" I could not find out why so I omitted that one. It is installed though:

I googled around for what's different about the AMPQ extension compared to the others, and how I might handle it in composer.json, but drew a blank. I thought it might be because it's a PECL extension, not a "normal" one, but then I'd also expect to get the same grief with the redis one, which I didn't. It's no big thing, so I've just moved on.

Symfony is much happier now:

root:/var/www# symfony book:check-requirements
[OK] Git installed
[OK] PHP installed version 8.2.4 (/usr/local/bin/php)
[OK] PHP extension "xsl" installed - required
[OK] PHP extension "tokenizer" installed - required
[OK] PHP extension "xml" installed - required
[OK] PHP extension "redis" installed - optional - needed only for chapter 31
[OK] PHP extension "amqp" installed - optional - needed only for chapter 32
[OK] PHP extension "json" installed - required
[OK] PHP extension "session" installed - required
[OK] PHP extension "curl" installed - optional - needed only for chapter 17 (Panther)
[OK] PHP extension "pdo_pgsql" installed - required
[OK] PHP extension "mbstring" installed - required
[OK] PHP extension "openssl" installed - required
[OK] PHP extension "sodium" installed - required
[OK] PHP extension "zip" installed - optional - needed only for chapter 17 (Panther)
[OK] PHP extension "gd" installed - optional - needed only for chapter 23 (Imagine)
[OK] PHP extension "ctype" installed - required
[OK] PHP extension "intl" installed - required
[OK] Composer installed
[KO] Cannot find Docker, please install it https://www.docker.com/get-started
[KO] Cannot find Docker Compose, please install it https://docs.docker.com/compose/install/
[KO] Cannot find the npm package manager, please install it https://www.npmjs.com/
  
You should fix the reported issues before starting reading the book.
root:/var/www#

The changes to get this far are tagged as 1.1. I'll crack on with adding a Node.js and a PostgreSQL container to the mix now, before I turn the next page. I don't need the PostgreSQL one yet, but I might as well do them both now, whilst I'm dicking around with Docker.


Node.js

There was not much do doing this. I think. I had an old scratch Node.js repo I created a while back, and I know that works, so I co-opted the bits I wanted from that and that was it really.

Dockerfile:

FROM node:19-bullseye

RUN ["apt-get", "update"]
RUN ["apt-get", "install", "-y", "vim"]

COPY home/.bashrc /root/.bashrc
COPY home/.bash_history /root/.bash_history
COPY home/.vimrc /root/.vimrc

WORKDIR  /usr/share/nodejs/

Most of that is just puff to make my life easier when I'm ferretting around in the container shell. The only important bit is the FROM bit. Which seems to be the latest Debian-based image.

docker-compose.yml (just the relevant bit):

nodejs:
    build:
        context: nodejs
        dockerfile: Dockerfile

    stdin_open: true
    tty: true

    volumes:
        - ..:/usr/share/nodejs/

Nowt interesting there either.

package.json:

{
    "name": "symfony-the-fast-track",
    "description": "Symfony: The Fast Track requires Node.js for some reason. So here it is.",
    "version": "1.0.0",
    "author": "Adam Cameron",
    "license": "GPL-3.0-or-later",
    "devDependencies": {
        "chai": "^4.3.7",
        "mocha": "^10.2.0"
    },
    "scripts": {
        "test": "mocha tests/nodejs/**/*.js"
    }
}

And it wouldn't be me if I didn't have a test (NodeTest.js):

let chai = require("chai");
let expect = chai.expect;

describe("Test Node.js", () => {
    it("has the expected version",  () => {
        expect(process.version).to.be.match(/v19\.\d+\.\d+/);
    })
});

And having rebuild the containers and shelled into the nodejs one, I just did npm install, and my test was runnable:

root:/usr/share/nodejs# npm test

> symfony-the-fast-track@1.0.0 test
> mocha tests/nodejs/**/*.js



  Test Node.js
    ✔ has the expected version


  1 passing (4ms)

root:/usr/share/nodejs#

I'm content that's doing its job.

One other thing I needed to do was to rearrgange the tests subdirectory hierarchy a bit:

/usr/share/nodejs# tree tests
tests
|-- nodejs
|   `-- integration
|       `-- NodeTest.js
`-- php
    |-- Integration
    |   |-- InstallationTest.php
    |   `-- PhpTest.php
    |-- Unit
    `-- bootstrap.php

5 directories, 4 files
root:/usr/share/nodejs#

I did not want the PHP tests munged in with the JS tests. Obvs I re-ran the PHP tests after I rehomed them. They were fine.

I've tagged all that as 1.2


PostgreSQL

Oddly, I have another project with a PostgreSQL container in it. So I'm copying that too.

Dockerfile:

FROM postgres:15-bullseye

RUN ["apt-get", "update"]
RUN ["apt-get", "install", "-y", "vim"]

COPY postgres/home/.bash_history /root/.bash_history
COPY shared/home/.bashrc /root/.bashrc
COPY shared/home/.vimrc /root/.vimrc

COPY postgres/docker-entrypoint-initdb.d/ /docker-entrypoint-initdb.d/

EXPOSE 5432

As with the Node.js one: a bunch of housekeeping there. I'm running a script (1.createAndPopulateTestTable.sql) to install some data at least:

CREATE TABLE test (
    id int GENERATED ALWAYS AS IDENTITY (START WITH 101),
    value VARCHAR(50) NOT NULL
);

INSERT INTO test (value)
VALUES
    ('Test row 1'),
    ('Test row 2')
;

The docker-compose.yml bit for this one is more interesting:

    postgres:
        build:
            context: .
            dockerfile: postgres/Dockerfile

        env_file:
            - shared/envVars.public
            - shared/envVars.private

        ports:
            - "5432:5432"
        volumes:
            - postgresData:/var/lib/postgresql/data

        stdin_open: true
        tty: true

        networks:
            backend:
                aliases:
                    - database.backend

volumes:
    postgresData:

networks:
    backend:
        driver: "bridge"

I need to pass some env vars into the containers now, including a password. I've put that in shared/envVars.private, and have excluded it from source control (docker/shared/.gitignore):

envVars.private

The other env vars don't need securing (docker/shared/envVars.public)

POSTGRES_DB=db1
POSTGRES_USER=user1

I've also added an explicit network now (not for any specific reason).

You might note I've also changed the build context here. I've done this across the board. This is because all th containers have the same .bashrc and .vimrc requirements, so instead of having image-specific ones (eg: php/home/.bashrc and nginx/home/.bashrc having the same content), I've put them in that shared directory adjacent to the service-specific directories (I'm just showing the PHP and Postgres ones here; the Nginx and Node.js ones are equivalent):

root:/var/www/docker# tree -a
.
// …
|-- php
|   |-- Dockerfile
|   |-- conf.d
|   |   |-- error_reporting.ini
|   |   `-- xdebug.ini
|   |-- home
|   |   `-- .bash_history
|   `-- php.ini
|
|-- postgres
|   |-- Dockerfile
|   |-- docker-entrypoint-initdb.d
|   |   `-- 1.createAndPopulateTestTable.sql
|   `-- home
|       `-- .bash_history
`-- shared
    |-- .gitignore
    |-- envVars.private
    |-- envVars.public
    `-- home
        |-- .bashrc
        `-- .vimrc

I've also added a test in the PHP container to make sure it can reach the DB:

namespace adamcameron\symfonythefasttrack\tests\Integration;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\DriverManager;
use PHPUnit\Framework\TestCase;
use stdClass;

/** @testdox DB tests */
class DbTest extends TestCase
{

    /** @testdox It can connect to the DB using DBAL */
    public function testDbalConnection()
    {
        $connection = $this->getDbalConnection();
        $result = $connection->executeQuery("SELECT version() AS version");

        $this->assertStringStartsWith("PostgreSQL 15", $result->fetchOne());
    }

    private function getConnectionParameters() : stdClass
    {
        return (object) [
            "host" => "database.backend",
            "port" => "5432",
            "database" => getenv("POSTGRES_DB"),
            "username" => getenv("POSTGRES_USER"),
            "password" => getenv("POSTGRES_PASSWORD")
        ];
    }

    private function getDbalConnection() : Connection
    {
        $parameters = $this->getConnectionParameters();
        return DriverManager::getConnection([
            "dbname" => $parameters->database,
            "user" => $parameters->username,
            "password" => $parameters->password,
            "host" => $parameters->host,
            "port" => $parameters->port,
            "driver" => "pdo_pgsql"
        ]);
    }
}

It simply checks the major version of the DB. If it can get that: it's connected enough.

I did not do a similar test in the Node.js container as I don't know that it will need to talk to the DB. I am guessing it's only going to be used to do client-side code building and crap like that. If I need this later: I'll do it later.

Once I rebuild the containers, that new test runs:

root:/var/www# composer test
> phpunit tests --display-deprecations
PHPUnit 10.0.18 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.4 with Xdebug 3.2.1
Configuration: /var/www/phpunit.xml.dist

...                                                                 3 / 3 (100%)

Time: 00:05.572, Memory: 10.00 MB

DB tests
  It can connect to the DB using DBAL

Tests of the whole installation
  Nginx is serving PHP content on the host system

Tests of the PHP installation
  It has the expected PHP version

OK (3 tests, 5 assertions)

Generating code coverage report in HTML format ... done [00:01.721]

And now… I'm sick of doing this for the day! I'm got a busy day tomorrow during the day, but a quiet Sunday evening might be a good time to start doing some actual Symfony stuff.

I've linked to all the files as I mention them, and there's four different tags in play in this article, but the current state of everything is here: 1.3.

I continue in the next part: PHP / Symfony: working through "Symfony: The Fast Track", part 2: creating a controller (eventually).

Righto.

--
Adam

Saturday 11 March 2023

PHP / PHPUnit / TDD: unit testing abstract classes. Or not.

G'day:

One of my colleagues at work asked me about this, but it's a good topic to think about, so am gonna write about it here.

The question was pretty much as stated in the subject line there "if we have an abstract class… how do we go about unit testing that?". There's a coupla things to unpack here, one practical and one theoretical. I'll do the practical bit first.

Practical

I have got an abstract class (thanks to ChatGPT for writing the example code for me today, btw ;-))

abstract class Shape
{

    public function __construct(protected string $colour)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }

    abstract public function getArea();
}

And this will eventually have implementing classes like circles and squares and what not. The abstraction conceit being that all shapes have an area, but the algorithms for defining said area vary from shape to shape. That's easy to understand and pretty ubiquitous in "here's an example of abstract classes in action" situations. For now we're just testing the abstract class, so I'm not worrying about the implementations yet.

We want to test that getColour returns the shape's colour.

For a non-abstract class, we could do this:

/** @testdox getColour returns the colour set by the constructor */
public function testGetColour()
{
    $testColour = "red";
    $shape = new Shape($testColour);

    $actualColour = $shape->getColour();

    $this->assertEquals($testColour, $actualColour);
}

Job done. Except Shape is an abstract class, so we get this instead:

Error: Cannot instantiate abstract class adamcameron\php8\Shapes\Shape
/var/www/tests/Unit/Shapes/ShapeTest.php:15

All is not lost. Obviously this is well-trod ground and PHPUnit already deals with this sort of thing: We can use a partial mock to create an implemetation class at runtime:

public function testGetColour()
{
    $green = "karariki";
    $shape = $this->getMockForAbstractClass(Shape::class, [$green]);

    $actualColour = $shape->getColour();

    $this->assertEquals($green, $actualColour);
}

getMockForAbstractClass()

The getMockForAbstractClass() method returns a mock object for an abstract class. All abstract methods of the given abstract class are mocked. This allows for testing the concrete methods of an abstract class.

Easy. There's also a mock-builder variant of this too:

public function testGetColourUsingMockBuilder()
{
    $blue = "kikorangi";
    $shape = $this
        ->getMockBuilder(Shape::class)
        ->setConstructorArgs([$blue])
        ->getMockForAbstractClass();

    $actualColour = $shape->getColour();

    $this->assertEquals($blue, $actualColour);
}

That's it, really. Original question answered.


Theory

The problem is that "how to test methods of an abstract class" begs the question. If we're doing TDD: how do we get to a point where we have an abstract class to test anyhow? It sounds to me like we're getting ahead of ourselves. I hasten to add the question from my team mate was a theoretical one: something that just popped into his head, and it was a good thing to know the answer for. But it's also good to reason the situation through from a TDD perspective.

Let's say the end of the story is "an abstract Shape class, and concrete classes for Square and Circle". But at the beginning of the story we had no classes at all, and no code. We had a requirement, which was probably along the lines of "we need to be able to get the colour of our Circle". Why is it a Circle and not a Shape? Because it's unlikely we're going to have a real world requirement that deals in abstract terms. It's more likely there'll be a concrete requirement to start with. But it's a good question: I'll think about that. We write our first test, for the Circle:

/** @testdox getColour returns the Circle's colour */
public function testGetColour()
{
    $orange = "karaka";
    $circle = new Circle($orange, 1);

    $actualColour = $circle->getColour();

    $this->assertEquals($orange, $actualColour);
}

And once we see this failing, we implement what we need to get it to pass, which would be along these lines:

class Circle
{
    public function __construct(protected string $colour)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }
}

Cool.

The next requirement comes along, which is that we need to get the circle's area. Fine, more of the same sort of thing:

/** @testdocs getArea returns the Circle's area */
public function testGetArea()
{
    $circle = new Circle("NOT_TESTED", 2);

    $actualArea = $circle->getArea();

    $this->assertEquals(pi() * 4, $actualArea);
}

And implementation:

class Circle
{
    public function __construct(private readonly string $colour, private readonly float $radius)
    {
    }

    // …

    public function getArea(): float
    {
        return pi() * $this->radius ** 2;
    }
}

Now the twist comes in. The next requirement is "OK, sometimes the shapes will be squares instead of circles, but otherwise behave the same". Remember red-green-refactor here. After TDDing the Square's behaviour, we would end up with this implementation:

class Square
{

    public function __construct(private readonly string $colour, private readonly float $side)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }

    public function getArea(): float
    {
        return $this->side ** 2;
    }
}

That's "red" & "green" done. Now for "refactor". Clearly we come to the conclusion that Circles and Squares are both Shapes; the colour behaviour is identical in both, but whilst both have the concept of "area", how it's derived is different, and the naming of the property that we use to derive the area - radius or side, respectively - also differs. This is when we decide we need our Shape abstract class. During refactoring. But the thing is during refactoring, the tests don't change. We extract the colour-handling into a base class, but leave the area handling in the implementation classes. We just make sure the base class says "I don't care how you do it, but you need to be able to answer the question 'what is your area'".

abstract class Shape
{

    public function __construct(protected string $colour)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }

    abstract public function getArea();
}
class Circle extends Shape
{

    public function __construct(string $colour, protected float $radius)
    {
        parent::__construct($colour);
    }

    public function getArea(): float
    {
        return pi() * $this->radius ** 2;
    }
}
class Square extends Shape
{

    public function __construct(string $colour, protected float $side)
    {
        parent::__construct($colour);
    }

    public function getArea(): float
    {
        return $this->side ** 2;
    }
}

We're not done refactoring yet. And this comes back to the requirement to test methods of an abstarct class. Currently in CircleTest and SquareTest we have these:

public function testGetColour()
{
    $orange = "karaka";
    $circle = new Circle($orange, 1);

    $actualColour = $circle->getColour();

    $this->assertEquals($orange, $actualColour);
}
public function testGetColour()
{
    $red = "whero";
    $square = new Square($red, 1);

    $actualColour = $square->getColour();

    $this->assertEquals($red, $actualColour);
}

Other than the colours I've chosen to use and one creates a Circle and one creates a Square, these are identical. As they should be as they're testing behaviour of their base class. So it does make a kind of sense to de-dupe this stuff, and push the one test up into ShapeTest. The test is the one from the first section of this article.

However I'm split either way on this. That we decided to use an abstract Shape class here is - IMO - "implementation detail", and we don't generally directly test implementation detail. So if we refactor those two tests, we might be catering to implementation detail too directly. I'm unsure. Also these tests happen to be very simple, so I think in a way this refactoring would be a case of "bad DRY" (see my earlier article "DRY: don't repeat yourself"). Maybe if the tests were more complicated then there'd be a better case for de-duping the complexity ("good DRY"). I think in this case, either way would be A-OK. Personally I'm a pedant, and a bit "a place for everything, and everything in its place", so I'm gonna go ahead and do the refactoring, and have a unit test testing an abstract class.

The code for the tests is here: /tests/Unit/Shapes, and for the source code: /src/Shapes.

Righto.

--
Adam

Sunday 19 February 2023

PHP: looking at spatie/async some more

G'day:

I'm getting back to this spatie/async library today (see "PHP: looking at spatie/async" for the first part of this).

Previously I have just been passing a callback to the add function when adding a task to the pool:

$pool->add(function () use ($connection, $i, $startTime) {
    $result = $connection->executeQuery("CALL sleep_and_return(?)", [2]);
    return sprintf(
        "%d:%d:%d",
        $i,
        $result->fetchOne(),
        microtime(true) - $startTime
    );
});

This requires the task to be tightly coupled to the pool-handling, which is probably not what one wants to do. Instead, one can put the task logic into a Task class:

namespace adamcameron\php8\Task;

use adamcameron\php8\tests\Integration\Fixtures\Database as DB;
use Doctrine\DBAL\Connection;
use Spatie\Async\Task;

class SlowDbCallTask extends Task
{
    readonly private Connection $connection;

    public function __construct(
        readonly private int $i,
        readonly private float $startTime
    ) {
    }

    public function configure()
    {
        $this->connection = DB::getDbalConnection();
    }

    public function run()
    {
        $result = $this->connection->executeQuery("CALL sleep_and_return(?)", [2]);
        return sprintf(
            "%d:%d:%d",
            $this->i,
            $result->fetchOne(),
            microtime(true) - $this->startTime
        );
    }
}

So that's a bit nicer, especially when you see how the calling code in the test looks now:

$pool->add(new SlowDbCallTask($i, $startTime));

One thing that took me a while to straighten out in my head was the way the configure method works / needs to be used. Initially, my implementation of the SlowDbCallTask class was a bit literal, and my constructor was thus:

public function __construct(
    readonly private Connection $connection,
    readonly private int $i,
    readonly private float $startTime
) {
}

This errored-out when I ran the test:

Exception: Serialization of 'Closure' is not allowed
/var/www/vendor/spatie/async/src/Runtime/ParentRuntime.php:87
/var/www/vendor/spatie/async/src/Runtime/ParentRuntime.php:69
/var/www/vendor/spatie/async/src/Pool.php:140
/var/www/tests/Functional/SpatieAsync/TaskTest.php:20

I've run into this before: the object being used as the task handler needs to be serialised to get it into the PHP process that's running it, and not everything can be serialised. Less than ideal, but so be it. Then it occurred to me that I was being a div anyhow: I don't want to pass the Task's DB connection into it - there's no reason to - I can just initialise it in the Task's constructor, eg:

public function __construct(
    readonly private int $i,
    readonly private float $startTime
) {
    $this->connection = DB::getDbalConnection();
}

But same error, and same problem. When I handle this in the constructor, it's all being done in the calling code, so the same serialisation issue exists. I have to concede the penny did not drop without some googling, and I found this helpful comment on an issue on GitHub, which led me to this other comment:

It's important to remember that parallel processes are really separate processes. By using closures and a smart bootstrap, we're simulating the parent process as best as possible. There are however cases which won't work with that simple approach: not everything is serialisable.

That's where a Task comes in. Tasks allow more control over the bootstrap in the child process.

Start by taking a look at what the README says about tasks: https://github.com/spatie/async#working-with-tasks

The less objects and application things are passed to the child process, the better. […]

Whilst it doesn't directly say it, this made me click that that config method is for (I was previously going "not sure why I need that"); the docs - linked to above - are of the sort that state what thing are, but not why they are they way they are, so don't directly explain this either. Basically if there's anything the Task will need that won't be serialisable: sling it in the config method. Makes complete sense to me now.

If one looks at the base Task class, we can see how it works:

namespace Spatie\Async;

abstract class Task
{
    abstract public function configure();

    abstract public function run();

    public function __invoke()
    {
        $this->configure();

        return $this->run();
    }
}

There is a simplified way of implementing tasks too (not sure why this is needed: once one knows what's going on, it's not like the other approach is "complicated".

To prove I'm actually TDDing all this still, here's the test for this one:

/** @TestDox It supports a simplified version of a task */
public function testSimpleTask()
{
    $pool = Pool::create();

    $pool->add(new SimpleTask());

    $results = $pool->wait();

    $this->assertCount(1, $results);
    $this->assertEquals("G'day world from an async call", $results[0]);
}

And the implementation simply needs to be this:

class SimpleTask
{
    public function __invoke()
    {
        return "G'day world from an async call";
    }
}

(It's a bit mindless, fine. It's late).

As per the docs: provided one has an __invoke method: that's it.


Speaking of TDD: in the first part of this - despite not starting with the test code listing - I did TDD it. Although I took it as kind of a refactoring, so I duplicated the test for the inline-task-via-callback version, and changed the code to instantiate my SlowDbCallTask. I did not directly test the configure and run implementations (with their own tests I mean), because that literally is implementation detail. The feature here is "the slow DB call [blah blah, whatever the feature actually is]", and whether it's done by inline callback or a Task class is neither here nor there. And that test does actually exercise all the code in the task class anyhow, so: job done.

Interestingly, the test coverage report disagrees with me:

This is because the PHP process running that code is not the same one running the tests, I guess. Hrm. Maybe I should have separate tests. But: I am not of the disposition that I need to chase 100% LOC coverage … but (again) I discuss why ensuring 100% is something to aim for anyway in an article I wrote ages ago: "Yeah, you do want 100% test coverage" (which should then be measured against this other article on the topic: "Test coverage: it's not about lines of code"). I think in this case - as the class is so simple - I'd put a @codeCoverageIgnore annotation on it. If the task class ended up having a bunch of moving parts in it that became trickier to test via its calling context, then I might consider testing those discretely. "It depends" [etc].

OK. That's the end of that section of the docs (I have written more about it than there was actual documentation that said. Ha. So I'm gonna finish up here. I think I have another article or so to write on this stuff yet. Let's seem.

Righto.

--
Adam

Thursday 9 February 2023

PHP: looking at spatie/async

G'day:

For no good reason at all, other than it piquing my interest, I've decided to mess around with spatie/async, which is - in its own words:

[A] small and easy wrapper around PHP's PCNTL extension. It allows running of different processes in parallel, with an easy-to-use API.

I got onto it because after messing around with Guzzle the other day ("PHP: looking at ways of making HTTP requests"), I got to wondering how PHP is doing async stuff. It occurs to me now I didn't think to just check which lib(s) Guzzle was using; I just googled, and found a few options, and spatie/async seemed to be the simplest / most-no-nonsense of the lot, so thought would start here.

I'm gonna "experiment via test", as is my wont.


Set-up

First of all(*) though, I need to install PCNTL, which is not installed by default in PHP's stock Docker image. It's easy though, just add this to my Dockerfile and rebuild my container:

RUN docker-php-ext-configure pcntl && docker-php-ext-install pcntl

(*) I say "first of all", but - full disclosure - I missed the note in the first sentence of the docs (ie: that sentence I quoted above) that it was required, and wrote a whole bunch of code, and was left wondering "why is this not running async?". But only for like an hour or two. Ahem. Cough. Anyway. Moving right along.

First I have set up a proc that I can run that just takes ages to run. I'm gonna use this to tie-up PHP processing. Pardon my pathetic MySQL skillz:

USE db1;

DROP PROCEDURE IF EXISTS sleep_and_return;
DELIMITER //
CREATE PROCEDURE sleep_and_return(IN seconds INT)
BEGIN
    DO SLEEP(seconds);
    SELECT seconds;
END //
DELIMITER ;

Baseline: it runs stuff asynchronously

And now my first test:

/** @testdox It can call a slow proc multiple times async */
public function testSlowProcAsync()
{
    $connection = DB::getDbalConnection();

    $pool = Pool::create();

    $startTime = microtime(true);
    for ($i = 1; $i <= 3; $i++) {
        $pool->add(function () use ($connection, $i, $startTime) {
            $result = $connection->executeQuery("CALL sleep_and_return(?)", [2]);
            return sprintf(
                 "%d:%d:%d",
                 $i,
                 $result->fetchOne(),
                 microtime(true) - $startTime
             );
        });
    }
    $poolPopulationTime = microtime(true) - $startTime;
    $this->assertLessThanOrEqual(1, $poolPopulationTime);

    $startTime = microtime(true);
    $results = $pool->wait();
    $endTime = microtime(true);
    $executionTime = $endTime - $startTime;

    $this->assertLessThan(3, $executionTime);
    $this->assertCount(3, $results);
    $this->assertContains("1:2:2", $results, "1:2:2 not found in " . implode(",", $results));
    $this->assertContains("2:2:2", $results, "2:2:2 not found in " . implode(",", $results));
    $this->assertContains("3:2:2", $results, "3:2:2 not found in " . implode(",", $results));
}

There's a bit going on here (it's not my first iteration of this test, I have to admit).

  • The premise is that each task I need to process async is a call to that proc which I am making sleep for two seconds. Three tasks doing that is six seconds of synchronous work; or only around two seconds or so if each is running asynchronously.
  • This thing seems to work by having a pool, then adding processes to run asynchronously.
  • I take note of the time before I start adding processes to the pool, so I can take note of how long each takes to run (should be ~2sec).
  • I return those metrics, and they end up being what's returned from the wait call at the end. Easy.
  • I also note how long it took to populate the pool. This should be pretty quick (less than even one of the processes running).
  • I then time how long the pool takes to run. Each process is ~2sec, and if they're running asynchronously the whole thing should finish in less time than it would take two of them to run.
  • Lastly I verify the results are what I'd expect. Note that I'm not testing an exact array, but just looking for each element: as each process is async, I can't really assume that they'll complete (and populate that array) in an specific sequence.

The metrics I'm grabbing are not as clever as they could be. I'm assuming each process will finish around 2sec after I start the timer before adding them to the pool. There's overhead in the adding, and calling the code as well as the 2sec pause at the DB, and sometimes the latter 1-2 assertions fail. I've added the failure message to see what actually gets returned, but sadly I have not been able to see it fail since I added those. I imagine it's a case that ~3sec has passed between one of the latter process callbacks being added, and them actually finishing running. So like the metrics returned for that process might be 2:2:3, not 2:2:2: the last number is the duration since we started adding the processes to the pool.

Update

I'm writing this the day after I wrote the paragraph above.

I got the test failure, and this bears out my theory from before, pretty miuch:

tests of spatie/async (https://github.com/spatie/async)
  It can call a slow proc multiple times async
   
    1:2:2 not found in 3:2:3,1:2:3,2:2:3            
    Failed asserting that an array contains '1:2:2'.
   
    /var/www/tests/Functional/Async/SpatieAsyncTest.php:41
     
  

Note the array returned in the results: 3:2:3,1:2:3,2:2:3. In each of these the process took three seconds to complete. Probably because I had only just started my containers, and the PHP code was not compiled and cached, and the DB had only just started. So, yeah, not a great test, but it helped show this issue, which is something.


Adding a then handler

The next test is very similar, except I'm chaining a then handler onto the process:

/** @testdox It uses a then handler which acts on the result */
public function testSlowProcAsyncThen()
{
    $connection = DB::getDbalConnection();

    $pool = Pool::create();

    $startTime = microtime(true);
    for ($i = 1; $i <= 3; $i++) {
        $pool
            ->add(function () use ($connection) {
                return $connection->executeQuery("CALL sleep_and_return(?)", [2]);
            })
            ->then(function ($result) use ($i, $startTime) {
                return sprintf(
                    "%d:%d:%d",
                    $i,
                    $result->fetchOne(),
                    microtime(true) - $startTime
                );
            });
    }

    $startTime = microtime(true);
    $results = $pool->wait();
    $endTime = microtime(true);
    $executionTime = $endTime - $startTime;

    $this->assertLessThan(3, $executionTime);
    $this->assertCount(3, $executionTimes);
    $this->assertContains("1:2:2", $results, "1:2:2 not found in " . implode(",", $results));
    $this->assertContains("2:2:2", $results, "2:2:2 not found in " . implode(",", $results));
    $this->assertContains("3:2:2", $results, "3:2:2 not found in " . implode(",", $results));
}
  • The task returns the DB statement this time.
  • And the then handler receives that, and now it returns the metrics.

Everything else is the same.

And this test goes splat:

Exception : Serialization of 'PDOStatement' is not allowed

OK, fair enough. Not everything is serialisable, and I'm guessing this is how objects are being passed to different PHP processes. Good to know. I will update my test to pass the result, not the whole statement object:

$pool
    ->add(function () use ($connection) {
        return$result = $connection->executeQuery("CALL sleep_and_return(?)", [2]);
        return $result->fetchOne();
    })
    ->then(function ($result) use ($i, $startTime) {
        return sprintf(
            "%d:%d:%d",
            $i,
            $result,
            microtime(true) - $startTime
        );
    });

But this is wrong too:

1:2:2 not found in 2,2,2
Failed asserting that an array contains '1:2:2'.

What's going on here? Ah. OK, so the final result that is returned from $pool->wait() is the return values from the initial process callbacks, not the end result of the then handler. My process callbacks are returning the result of the proc call, which is just the number originally passed into it (2), hence the result array being [2,2,2]. If I want to access the result of the processing done in the then I'm gonna need to grab it separately:

public function testSlowProcAsyncThen()
{
    $connection = DB::getDbalConnection();

    $pool = Pool::create();

    $metrics = [];
    $startTime = microtime(true);
    for ($i = 1; $i <= 3; $i++) {
        $pool
            ->add(function () use ($connection) {
                $result = $connection->executeQuery("CALL sleep_and_return(?)", [2]);
                return $result->fetchOne();
            })
            ->then(function ($result) use (&$metrics, $i, $startTime) {
                $metrics[] = sprintf(
                    "%d:%d:%d",
                    $i,
                    $result,
                    microtime(true) - $startTime
                );
            });
    }

    $startTime = microtime(true);
    $pool->wait();
    $endTime = microtime(true);
    $executionTime = $endTime - $startTime;

    $this->assertLessThan(3, $executionTime);
    $this-->assertCount(3, $results);
    $this->assertCount(3, $metrics);
    $this->assertContains("1:2:2", $metrics, "1:2:2 not found in " . implode(",", $metrics));
    $this->assertContains("2:2:2", $metrics, "2:2:2 not found in " . implode(",", $metrics));
    $this->assertContains("3:2:2", $metrics, "3:2:2 not found in " . implode(",", $metrics));
}

And now this gives expected results.


Handling timeouts

Next up, I'm having a look at the timeout one can put on a pool. Similar code again:

/** @testdox It supports a timeout */
public function testSlowProcAsyncTimeout()
{
    $connection = DB::getDbalConnection();

    $pool = Pool::create();
    $pool->timeout(1);

    $timeOuts = [];

    $startTime = microtime(true);
    for ($i = 1; $i <= 3; $i++) {
        $pool
            ->add(function () use ($connection) {
                $result = $connection->executeQuery("CALL sleep_and_return(?)", [2]);
                return $result->fetchOne();
            })
            ->timeout(function () use (&$timeOuts, $i, $startTime) {
                $timeOuts[] = sprintf(
                    "TIMED OUT ON ITERATION %d after %d seconds",
                    $i,
                    microtime(true) - $startTime
                );
                return false;
            });
    }

    $results = $pool->wait();
    $endTime = microtime(true);
    $executionTime = $endTime - $startTime;

    $this->assertEquals(
        [
            "TIMED OUT ON ITERATION 1 after 1 seconds",
            "TIMED OUT ON ITERATION 2 after 1 seconds",
            "TIMED OUT ON ITERATION 3 after 1 seconds"
        ],
        $timeOuts
    );

    $this->assertLessThan(2, $executionTime);

    $this->assertEquals([], $results);
}
  • I set a timeout of 1sec.
  • I have a timeout handler that returns some metrics.
  • Note that the value returned from wait is an empty array this time.

Handling exceptions

Next a much more simplified test, looking at exception handling:

/** @testdox it supports exception handling */
public function testAsyncException()
{
    $pool = Pool::create();
    $pool
        ->add(function () {
            throw new \Exception("This is an exception");
        })
        ->catch(function (\Exception $exception) {
            $this->assertEquals("This is an exception", $exception->getMessage());
        });

    $pool->wait();
}

There's no surprises there, except the assertion fails, because $exception->getMessage() is not "This is an exception", it's this mess:

'This is an exception\n
\n
#0 [internal function]: adamcameron\php8\tests\Functional\Async\SpatieAsyncTest::{closure}()\n
#1 /var/www/vendor/laravel/serializable-closure/src/Serializers/Native.php(91): call_user_func_array(Object(Closure), Array)\n
#2 [internal function]: Laravel\SerializableClosure\Serializers\Native->__invoke()\n
#3 /var/www/vendor/laravel/serializable-closure/src/SerializableClosure.php(48): call_user_func_array(Object(Laravel\SerializableClosure\Serializers\Native), Array)\n
#4 /var/www/vendor/spatie/async/src/Runtime/ChildRuntime.php(26): Laravel\SerializableClosure\SerializableClosure->__invoke()\n
#5 {main}'

Exactly that. The exception message has somehow been polluted with a stack trace.

Initially I thought it might be because the library was wrapping my exception in another one (kinda fair enough), but it's not: it's just an exception. If I wasn't a complete n00b with this (and how PHP handles async stuff), and also wasn't incredibly rusty with PHP, I'd probably raise an issue with the library maintainers. But: I'll mull it over a bit first, to see if I can work out what dumbarsery I might be engaging in.


I decided to push the exception down a bit, and throw it in a then handler. Disappointing results here:

/** @testdox it does not support exception handling from a then handler */
public function testAsyncExceptionFromThen()
{
    $this->expectException(\Exception::class);
    $this->expectExceptionMessage("This is an exception");

    $pool = Pool::create();
    $pool
        ->add(function () {
            // do nothing
        })
        ->then(function () {
            throw new \Exception("This is an exception");
        })
        ->catch(function (\Exception $exception) {
            $this->assertStringStartsWith("This is an exception", $exception->getMessage());
        });

    $pool->wait();
}

The exception wasn't handled by the catch handler. It seems it'll only catch exceptions from the main process.


Stopping the pool in its tracks

I'm changing the test approach now, indeed I'm lifting their sample code from the docs and turning it into a test:

/**  @testdox a pool can be stopped */
public function testPoolStop()
{
    $pool = Pool::create();

    for ($i = 0; $i < 10000; $i++) {
        $pool->add(function () {
            return rand(0, 100);
        })->then(function ($output) use ($pool) {
            // If one of them randomly picks 100, end the pool early.
            if ($output === 100) {
                $pool->stop();
            }
        });
    }

    $results = $pool->wait();

    $this->assertLessThan(10000, count($results));
    $this->assertContains(100, $results);
}

This is obviously really contrived, but it's a way to see that the stop method does the trick.

I decided to see if it would also work in a catch handler:

/**  @testdox a pool can be stopped in a catch */
public function testPoolStopInCatch()
{
    $pool = Pool::create();

    for ($i = 0; $i < 10000; $i++) {
        $pool->add(function () {
            $result = rand(0, 100);
            if ($result === 100) {
                throw new \Exception("Something went wrong");
            }
            return $result;
        })->catch(function (\Exception $_) use ($pool) {
            $pool->stop();
        });
    }

    $results = $pool->wait();

    $this->assertLessThan(10000, count($results));
    $this->assertNotContains(100, $results);
}

Note that in this case, unlike the previous one, the results won't include the 100, because it never gets returned.


Stopping the Cameron in its tracks

There's more to look at, but this article is already quite long, and this is a reasonable stopping point because the next bit deals with alterntive syntaxes. I'll do that tomorrow (-ish).

This seems like pretty handy stuff so far, although there's that exception weirdness to mull over, and also I need to reason why I can't catch exceptions thrown in the then handlers.

All the methods are linked-to inline, but for the record they're all here: /tests/Functional/Async/SpatieAsyncTest.php.

Righto.

--
Adam