Showing posts with label Frameworks. Show all posts
Showing posts with label Frameworks. Show all posts

Tuesday 24 January 2023

Symfony: getting rid of App namespace and using a well-formed one

G'day:

This is a quick follow-on from the previous article, "Symfony: installing in my PHP8 container (for a second time, as it turns out)".

I was irritated that Symfony uses an invalid PSR-4 namespace for the app: App. That's like calling a variable "variable" and it's a bit shit. Plus, as indicated, it's not even valid, as a PSR-4 namespace is supposed to be <NamespaceName>(\<SubNamespaceNames>), where the first part reflects the vendor, and the (optional) second part is [something else, usually the name of the app]. So Symfony would be valid. Or Symfony/app would be valid, but just App is not valid. In my case I'm not "Symfony", so the namespace for this app should be (and is) adamcameron\php8 (OKOK, "PHP8" is not a great name for a web app, but this is my "messing around with PHP8 app", so kinda makes some sense. More than App does anyhow). </rant>.

How do I fix this? There's only a few touch points where I've needed to change references to App to adamcameron\php8:

That's it. As I had no test coverage of the console, I added one:

<?php

namespace adamcameron\php8\tests\integration;

use GuzzleHttp\Client;
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Response;

/** @testdox Tests of Symfony installation */
class SymfonyTest extends TestCase
{
    // …

    /** @testdox It can run the console in a shell */
    public function testSymfonyConsoleRuns()
    {
        $appRootDir = dirname(__DIR__, 2);

        exec("{$appRootDir}/bin/console --help", $output, $returnCode);

        $this->assertEquals(0, $returnCode);
    }
}

And that was it. Easy.

Righto.

--
Adam

Symfony: installing in my PHP8 container (for a second time, as it turns out)

G'day:

First up, I've messed around in the last coupla articles setting up some PHP8.2 containers (PHP: returning to PHP and setting up a PHP8 dev environment), adding MariaDB (Docker: adding a MariaDB container to my PHP & Nginx ones) etc and documenting it all… then I realised I've actually done this very exercise before! A coupla years ago when I was looking for PHP work and decided I had better get up to speed with Docker / Symfony / front-end dev. I had forgotten about a lot of it, only really remembering the VueJS part of it. Ha. Dammit. Oh well. Anyhow, that series - and it's def a series, there's a dozen articles - are all tagged with VueJs/Symfony/Docker/TDD series. Still, I am going to do a Symfony installation exercise again, cos I want it to be in this project this time. Because reasons.

Installing the baseline Symfony app

I'm working through Symfony › Installing & Setting up the Symfony Framework.

OK so I already have the Symfony CLI client installed during getting the PHP container up and running to my liking (first article linked-to above), and it seems happy:

/var/www# symfony check:requirement

Symfony Requirements Checker
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> PHP is using the following php.ini file:
/usr/local/etc/php/php.ini

> Checking Symfony requirements:

....................................

[OK] Your system is ready to run Symfony projects
Note The command console can use a different php.ini file ~~~~ than the one used by your web server. Please check that both the console and the web server are using the same PHP version and configuration. /var/www#

(note I'm running all this in a shell on the container, not on my host machine. I do not have PHP or Composer or anything like that installed on the host machine)

I already know Symfony / Composer will shit itself if I try to install Symfony in a non-empty directory, so I'm going to run the installer in /var/tmp, and I'll re-integrate the files I need into my app directory by hand.

/var/tmp# symfony new my_project_directory --version="6.2.*" --no-git
* Creating a new Symfony 6.2.* project with Composer
  (running /usr/local/bin/composer create-project symfony/skeleton /var/tmp/my_project_directory 6.2.* --no-interaction)

[OK] Your project is now ready in /var/tmp/my_project_directory
/var/tmp#

Note the --no-git there. Without that the installer wants my Git identification otherwise it can't run git init, and I don't need it to do that anyhow, so skip that bit. I found this out via trial and error.

What's installed:

/var/tmp# tree -L 2
.
`-- my_project_directory
    |-- bin
    |-- composer.json
    |-- composer.lock
    |-- config
    |-- public
    |-- src
    |-- symfony.lock
    |-- var
    `-- vendor

7 directories, 3 files
/var/tmp#

BTW I cheated and installed tree without telling you:

/var/tmp# apt-get update
[…]
Reading package lists... Done
/var/tmp# apt-get install tree

Right so a lot of that will copy across fine into my app dir, except I'll need to rename my /var/www/html to be /var/www/public. I'll also need to merge this composer.json file with my own one, as with the .gitignore. I'll just get rid of the vendor directory as I can regenerate all that with composer update. I don't currently know what the symfony.lock file is, so I'll copy it across. I presume it's something along the lines of a Symfony version of composer.lock, and is generated somehow. I'll find out later I guess.

Symfony sets the "PSR-4" namespaces to be App\\ and App\\Tests\\. I'm not having my app called "App": that's ridiculous, plus it's actually invalid according to the PSR-4 standard anyhow!

2. Specification

  1. The term "class" refers to classes, interfaces, traits, and other similar structures.
  2. A fully qualified class name has the following form:
      \<NamespaceName>(\<SubNamespaceNames>)*\<ClassName>
      
    1. The fully qualified class name MUST have a top-level namespace name, also known as a "vendor namespace".
    2. The fully qualified class name MAY have one or more sub-namespace names.

PSR-4, section 2 (part)

They're only using the \<SubNamespaceNames>.

Hopefully this is just a matter of renaming some namespace references in whatever stub / config files it's installed. However I'll actually rename my test directory to be tests to match the Symfony-idiomatic naming standard there.

File changes to make Symfony work with an existing PHP project

There was surprisingly little to do. I'll run through them, point by point


Rename of html directory to public

To fit with Nginx's default settings, I had my webroot set to be html, ie: /var/www/html. Symfony uses public, so to change that, I had to make the following file tweaks:

# docker/docker-compose.yml
version: "3"
services:
  nginx:
    build:
      context: nginx
      dockerfile: Dockerfile

    ports:
      - "8008:80"

    stdin_open: true
    tty: true

    volumes:
      - ../html:/usr/share/nginx/html/
      - ../public:/usr/share/nginx/html/

    depends_on:
      - php

Note that as far as Nginx is concerned, its web root is still /usr/share/nginx/html/, I'm just attaching the /public directory on the host machine to provide its files.

Also I've added the depends_on there because I was finding Nginx was now starting before PHP was ready, so Nginx was exiting due to not finding PHP to proxy to.

# docker/nginx/sites/default.conf
server {
    # …

    location ~ \.php$ {
        try_files $uri /index.php =404;
        fastcgi_pass php-upstream;
        fastcgi_index index.php;
        fastcgi_buffers 16 16k;
        fastcgi_buffer_size 32k;
        fastcgi_param SCRIPT_FILENAME /var/www/html/$fastcgi_script_name;
        fastcgi_param SCRIPT_FILENAME /var/www/public/$fastcgi_script_name;
        fastcgi_read_timeout 600;
        include fastcgi_params;
    }

    location ~ /\.ht {
        deny all;
    }
}

But I need to still pass the file from Nginx through to the /var/www/public/ directory now, in the PHP container.


.gitignore

/.idea
/vendor
/.phpunit.result.cache

###> symfony/framework-bundle ###
/.env.local
/.env.local.php
/.env.*.local
/config/secrets/prod/prod.decrypt.private.php
/public/bundles/
/var/
###< symfony/framework-bundle ###

###> phpunit/phpunit ###
/phpunit.xml
.phpunit.result.cache
###< phpunit/phpunit ###

A bunch of Symfony specific stuff. Seems inoccuous.


composer.json

{
    "name" : "adamcameron/php8",
    "description" : "PHP8 containers",
    "type" : "project",
    "license" : "LGPL-3.0-only",
    "require": {
        …
        "monolog/monolog": "^3.2.0",
        "symfony/console": "6.2.*",
        "symfony/dotenv": "6.2.*",
        "symfony/flex": "^2",
        "symfony/framework-bundle": "6.2.*",
        "symfony/http-client": "^6.2.2",
        "symfony/runtime": "6.2.*",
        "symfony/yaml": "6.2.*"
    },
    "require-dev": {
        "phpunit/phpunit": "^9.5.28",
        "phpmd/phpmd": "^2.13.0",
        "squizlabs/php_codesniffer": "^3.7.1"
    },
    "config": {
        "allow-plugins": {
            "symfony/flex": true,
            "symfony/runtime": true
        },
        "sort-packages": true
    },
    "autoload": {
        "psr-4": {
            "adamcameron\\php8\\": "src/",
            "App\\": "src/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "adamcameron\\php8\\testtests\\": "testtests/"
        }
    },
    "replace": {
        "symfony/polyfill-ctype": "*",
        "symfony/polyfill-iconv": "*",
        "symfony/polyfill-php72": "*",
        "symfony/polyfill-php73": "*",
        "symfony/polyfill-php74": "*",
        "symfony/polyfill-php80": "*",
        "symfony/polyfill-php81": "*"
    },
    "scripts" : {
        "test": "phpunit --testdox testtests",
        "phpmd": "phpmd src,testtests text phpmd.xml",
        "phpcs": "phpcs src testtests",
        "test-all": [
            "@test",
            "@phpmd",
            "@phpcs"
        ],
        "auto-scripts": {
            "cache:clear": "symfony-cmd",
            "assets:install %PUBLIC_DIR%": "symfony-cmd"
        },
        "post-install-cmd": [
            "@auto-scripts"
        ],
        "post-update-cmd": [
            "@auto-scripts"
        ]
    },
    "conflict": {
        "symfony/symfony": "*"
    },
    "extra": {
        "symfony": {
            "allow-contrib": false,
            "require": "6.2.*"
        }
    }
}

Here I took the stuff from the Symfony-install-generated composer.json file and merged it into mine.

I needed to continue to use Symfony's App namespace too, as it has hard dependencies on being able to find src/Kernel.php via that namespace. Suck. All my own code will continue to use a proper, correctly-defined namespace.

There's also a bit of the rename of the test directory to tests in the autoload-dev section. I also had to similarly update phpunit.xml.dist and all the namespace references. As that's just adding an s about the place, I'll not bore you with all those changes. For the completeists: it's all in Github for you to look at.


.env and .env.test

# 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.
#
# DO NOT DEFINE PRODUCTION SECRETS IN THIS FILE NOR IN ANY OTHER COMMITTED FILES.
# https://symfony.com/doc/current/configuration/secrets.html
#
# Run "composer dump-env prod" to compile .env files for production use (requires symfony/flex >=1.2).
# https://symfony.com/doc/current/best_practices.html#use-environment-variables-for-infrastructure-configuration

###> symfony/framework-bundle ###
APP_ENV=dev
APP_SECRET=369a2db7c3f19ae9cad11dd95777674e
###< symfony/framework-bundle ###

and

# define your env variables for the test env here
KERNEL_CLASS='App\Kernel'
APP_SECRET='$ecretf0rt3st'
SYMFONY_DEPRECATIONS_HELPER=999999
PANTHER_APP_ENV=panther
PANTHER_ERROR_SCREENSHOT_DIR=./var/error-screenshots

Symfony stuff. I will need to put those APP_SECRET values out of the codebase and put them into environment variables. They're not very "secret" sitting around in source control like that. I'll find out what they're for first. I am also looking forward to finding out what a "panther error" is. Ooh I wonder if I can change that KERNEL_CLASS reference to point to adamcameron\php8\Kernel instead, and get rid of that App namespace? Will look into that (might read my own article from last time I did this to see what I did about this…?).


symfony.lock

{
    "phpunit/phpunit": {
        "version": "9.5",
        "recipe": {
            "repo": "github.com/symfony/recipes",
            "branch": "main",
            "version": "9.3",
            "ref": "a6249a6c4392e9169b87abf93225f7f9f59025e6"
        },
        "files": [
            ".env.test",
            "phpunit.xml.dist",
            "tests/bootstrap.php"
        ]
    },
    "squizlabs/php_codesniffer": {
        "version": "3.7",
        "recipe": {
            "repo": "github.com/symfony/recipes-contrib",
            "branch": "main",
            "version": "3.6",
            "ref": "1019e5c08d4821cb9b77f4891f8e9c31ff20ac6f"
        }
    },
    "symfony/console": {
        "version": "6.2",
        "recipe": {
            "repo": "github.com/symfony/recipes",
            "branch": "main",
            "version": "5.3",
            "ref": "da0c8be8157600ad34f10ff0c9cc91232522e047"
        },
        "files": [
            "bin/console"
        ]
    },
    "symfony/flex": {
        "version": "2.2",
        "recipe": {
            "repo": "github.com/symfony/recipes",
            "branch": "main",
            "version": "1.0",
            "ref": "146251ae39e06a95be0fe3d13c807bcf3938b172"
        },
        "files": [
            ".env"
        ]
    },
    "symfony/framework-bundle": {
        "version": "6.2",
        "recipe": {
            "repo": "github.com/symfony/recipes",
            "branch": "main",
            "version": "6.2",
            "ref": "af47254c5e4cd543e6af3e4508298ffebbdaddd3"
        },
        "files": [
            "config/packages/cache.yaml",
            "config/packages/framework.yaml",
            "config/preload.php",
            "config/routes/framework.yaml",
            "config/services.yaml",
            "public/index.php",
            "src/Controller/.gitignore",
            "src/Kernel.php"
        ]
    },
    "symfony/routing": {
        "version": "6.2",
        "recipe": {
            "repo": "github.com/symfony/recipes",
            "branch": "main",
            "version": "6.2",
            "ref": "e0a11b4ccb8c9e70b574ff5ad3dfdcd41dec5aa6"
        },
        "files": [
            "config/packages/routing.yaml",
            "config/routes.yaml"
        ]
    }
}

This does seem like a Symfony-specific composer.lock file. I wonder why it needs one of its own?


bin/console

#!/usr/bin/env php
<?php

use App\Kernel;
use Symfony\Bundle\FrameworkBundle\Console\Application;

if (!is_file(dirname(__DIR__).'/vendor/autoload_runtime.php')) {
    throw new LogicException('Symfony Runtime is missing. Try running "composer require symfony/runtime".');
}

require_once dirname(__DIR__).'/vendor/autoload_runtime.php';

return function (array $context) {
    $kernel = new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']);

    return new Application($kernel);
};

I wonder why I need a console shell script?


config directory

There's a bunch of stuff in here, some obvious, some less so, but seems to be standard frameworked-app config like routing and the like. I'll not bother repeating it here, as I have nothing to add - commentary-wise - about any of it. Go have a look on Github perhaps.


public/index.php

<?php

use App\Kernel;

require_once dirname(__DIR__).'/vendor/autoload_runtime.php';

return function (array $context) {
    return new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']);
};

That's all that's needed in the public directory to load the app. Nice.


src/Kernel.php

<?php

namespace App;

use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
use Symfony\Component\HttpKernel\Kernel as BaseKernel;

class Kernel extends BaseKernel
{
    use MicroKernelTrait;
}

I guess I will need to change this to be app specific at some point, otherwise I don't really know why it needs to be in my app's src directory instead of Symfony's. Time will tell.


src/Controller/.gitignore

It's created an empty .gitignore here. Unsure why. If I was to guess, there's a reference to it here…

config/routes.yaml
controllers:
    resource:
        path: ../src/Controller/
        namespace: App\Controller
    type: attribute

… and the app will error if the directory doesn't exist?


tests/bootstrap.php

<?php

use Symfony\Component\Dotenv\Dotenv;

require dirname(__DIR__).'/vendor/autoload.php';

if (file_exists(dirname(__DIR__).'/config/bootstrap.php')) {
    require dirname(__DIR__).'/config/bootstrap.php';
} elseif (method_exists(Dotenv::class, 'bootEnv')) {
    (new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
}

I'm not actually loading a botostrap file in my phpunit.xml.dist yet as I didn't need one. I'm guessing I'll need this if I'm doing any functional tests that need the framework infrastructure?


tests/integration/SymfonyTest.php

This is my code, not Symfony's. I want a test to check the app is up and running. I'm checking I can curl the homepage, and get the Symfony splash screen.

<?php

namespace adamcameron\php8\tests\integration;

use GuzzleHttp\Client;
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Response;

/** @testdox Tests of Symfony installation */
class SymfonyTest extends TestCase
{
    /** @testdox It serves the default welcome page after installation */
    public function testSymfonyWelcomeScreenDisplays()
    {

        $client = new Client([
            'base_uri' => 'http://nginx/'
        ]);

        $response = $client->get(
            "/",
            ['http_errors' => false]
        );
        $this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode());

        $html = $response->getBody();
        $document = new \DOMDocument();
        $document->loadHTML($html, LIBXML_NOWARNING | LIBXML_NOERROR); // not ideal, but libxml can't handle the SVG in the Symfony logo

        $xpathDocument = new \DOMXPath($document);

        $hasTitle = $xpathDocument->query('/html/head/title[text() = "Welcome to Symfony!"]');
        $this->assertCount(1, $hasTitle);
    }
}

That's it. Everything else is just dealing with the rename of test to tests. I'm gonna push this lot and draw a line under this article, and then come back and see if I can get rid of that App namespace.

Righto.

--
Adam

Saturday 21 January 2023

PHP: looking at ways of making HTTP requests

G'day:

I'm reacquainting myself with PHP, and part of this process is chucking some tests together to demonstrate to myself how bits and pieces of it works. This has the added bonus of being able to put the code in front of my team, to help provide learning info for them. This article is pretty much just showing sample code, and it's for the reader to compare and contrast. There's likely not gonna be too much exposition from me once we get to the code. I'm sure I can pad things out by a few hundred words before we get there though. I am me after all.

This time, I've decided to revisit how to make HTTP requests.

I've got four candidate solutions to look at:

I am aware of PHP's curl extension always being available, but its API is a bit of a mess (it's been part of PHP since the bad old days).

I've also used Guzzle in the past, with mixed success. It started out being simple and handy, and I liked it. But then between a major version bump (I can't remember which versions this occurred between), the old API was basically dumped in favour of a new, non-backwards-compatible, and largely (and pointlessly IMO) overly complex promise-based approach. To provide asynchronicity in HTTP requests. Which was something I never needed and seemed like an odd addition to an HTTP library. I suspect the author had started to look at Node.jS with all its async HTTP shiz, and went "I know… I'll ruin something perfectly useful by adding this crap into it as well". Ugh. However I note Guzzle is still around, so - armed with an open mind - I'll look at that too.

During my googling I have also spied that Symfony has an HTTP client too. It probably always did, but in my last gig we went the Guzzle route, so I had not looked further afield.

Also during my googling (and reading the Guzzle docs), I discovered PHP's own streams extension can be used to make HTTP requests. That sounds interesting, so have decided to give that a go too.

My approach is to create a test class, and add a test for each of those four platforms, to do each of a GET and a POST. They are not complicated tests, it's just a case of getting the thing to do something simple that I can expect results from.

I will also concede that I used Copilot to do probably 80% of the work here, with my polishing that last 20%.

Installation

  • curl needs ext-curl installed. It ships with the Docker image, so I didn't have to do anything for this.
  • Installing Guzzle is a matter of adding it as a dependency in composer.json: "guzzlehttp/guzzle": "^7.5.0" at time of writing.
  • Similarly with Symfony's HTTP client: "symfony/http-client": "^6.2.2"
  • And PHP's streams lib is native to PHP. No installation necessary.

Curl

/** @testdox it can make a GET request */
public function testGet()
{
    $ch = curl_init();
    curl_setopt_array($ch, [
        CURLOPT_URL => 'https://api.github.com/users/adamcameron',
        CURLOPT_USERAGENT => $this->getUserAgentForCurl(),
        CURLOPT_RETURNTRANSFER => 1
    ]);
    $response = curl_exec($ch);
    curl_close($ch);

    $this->assertEquals(200, curl_getinfo($ch, CURLINFO_HTTP_CODE));
    $this->assertJson($response);
    $this->assertGitInfoIsCorrect($response);
}

It also uses these two helper methods:

private function getUserAgentForCurl(): string
{
    return sprintf("curl/%s", curl_version()['version']);
}
protected function assertGitInfoIsCorrect(string $response): void
{
    $myGitMetadata = json_decode($response);
    $this->assertEquals('adamcameron', $myGitMetadata->login);
    $this->assertEquals('Adam Cameron', $myGitMetadata->name);
}

(A bunch of the other tests below also use that one above).

The GET test in each case will be to get my own GitHub profile and to superficially check it's been fetched properly. BTW I needed that getUserAgentForCurl carry-on because curl by itself does notsent a user agent, and Github says "nuh-uh" if it doesn't get one. So I've just contrived a user agent that is the one that the underlying curl implementation would use (eg: ike if one was doing a curl from bash).

The POST test will post to https://httpbin.org/post. I only discovered httpbin.org when I was doing this exercise, and I wanted a simple (public) way of echoing back a post request. Handy.

/** @testdox it can make a POST request */
public function testPost()
{
    $ch = curl_init();
    curl_setopt_array($ch, [
        CURLOPT_URL => 'https://httpbin.org/post',
        CURLOPT_USERAGENT => $this->getUserAgentForCurl(),
        CURLOPT_RETURNTRANSFER => 1,
        CURLOPT_POST => 1,
        CURLOPT_POSTFIELDS => ['foo' => 'bar']
    ]);
    $response = curl_exec($ch);
    curl_close($ch);

    $this->assertEquals(200, curl_getinfo($ch, CURLINFO_HTTP_CODE));
    $this->assertJson($response);
    $httpBinResponse = json_decode($response);

    $this->assertEquals('bar', $httpBinResponse->form->foo);
}

An example of what https://httpbin.org/post returns is:

{
    "args":{
        
    },
    "data":"",
    "files":{
        
    },
    "form":{
        "foo":"bar"
    },
    "headers":{
        "Accept":"*/*",
        "Content-Length":"141",
        "Content-Type":"multipart/form-data; boundary=------------------------b0133bb008e6829b",
        "Host":"httpbin.org",
        "User-Agent":"curl/7.74.0",
        "X-Amzn-Trace-Id":"Root=1-63cc3251-4a3f92c809ad00f75261466a"
    },
    "json":null,
    "origin":"82.8.81.31",
    "url":"https://httpbin.org/post"
}

Guzzle

First up: I'm really pleased how compact and straight-forward Guzzle's code is for these exercises. And also that one is not forced to write async code for a non-async situation.

/** it can make a GET request */
public function testGet()
{
    $client = new Client();
    $response = $client->request('GET', 'https://api.github.com/users/adamcameron');
    $this->assertEquals(200, $response->getStatusCode());
    $this->assertJson($response->getBody());
    $this->assertGitInfoIsCorrect($response->getBody());
}
/** @testdox it can make a POST request */
public function testPost()
{
    $client = new Client();
    $response = $client->request(
        'POST',
        'https://httpbin.org/post',
        ['form_params' => ['foo' => 'bar']]
    );
    $this->assertEquals(200, $response->getStatusCode());
    $this->assertJson($response->getBody());
    $httpBinResponse = json_decode($response->getBody());
    $this->assertEquals('bar', $httpBinResponse->form->foo);
}

I also decided to revisit the async side of things:

/** it can make an asynchronous GET request */
public function testAsyncGet()
{
    $client = new Client();
    $promise = $client->requestAsync('GET', 'https://api.github.com/users/adamcameron');
    $response = $promise->wait();
    $this->assertEquals(200, $response->getStatusCode());
    $this->assertJson($response->getBody());
    $this->assertGitInfoIsCorrect($response->getBody());
}

Simple. It seems the current implementation is taking the "async-await" approach with these things like JS has these days.

That was not much of a test though. This time I am gonna make a bunch of requests (which are artificially slow) and make sure they do seem to run asynchronously. I've slung this in my web directory:

// html/test-fixtures/slow.php
$timeToWait = $_GET['timeToWait'] ?? 0;
sleep($timeToWait);
echo "waited $timeToWait seconds";

And calling that with varying delays:

/** it can make multiple asynchronous GET requests */
public function testMultipleAsyncGet()
{
    $client = new Client();
    $requestsToMakeConcurrently = [
        $client->getAsync('http://nginx/test-fixtures/slow.php?timeToWait=1'),
        $client->getAsync('http://nginx/test-fixtures/slow.php?timeToWait=2'),
        $client->getAsync('http://nginx/test-fixtures/slow.php?timeToWait=3')
    ];
    $startTime = microtime(true);
    $responses = Promise\Utils::unwrap($requestsToMakeConcurrently);
    $endTime = microtime(true);

    $totalTime = $endTime - $startTime;
    $this->assertGreaterThan(3, $totalTime);
    $this->assertLessThan(4, $totalTime);

    array_walk($responses, function ($response, $i) {
        $this->assertEquals(200, $response->getStatusCode());
        $this->assertEquals(sprintf("waited %d seconds", $i+1), $response->getBody());
    });
}

The assertions there are a bit woolly. I figured it should def take longer than 3sec cos at least one of the requests will take 3sec. Plus there'll be a wee bit of overhead. That overhead ought not be more than a second, so if the whole lot finishes in less than 4sec, it's a pretty good indicator that all three requests were being made simultaneously. It occurs to me now I could perhaps look @ the Nginx activity logs for when the requests come in. Please hold…

172.31.0.4 - - [21/Jan/2023:18:57:49 +0000] "GET /test-fixtures/slow.php?timeToWait=1 HTTP/1.1" 200 27 "-" "GuzzleHttp/7"
172.31.0.4 - - [21/Jan/2023:18:57:50 +0000] "GET /test-fixtures/slow.php?timeToWait=2 HTTP/1.1" 200 27 "-" "GuzzleHttp/7"
172.31.0.4 - - [21/Jan/2023:18:57:51 +0000] "GET /test-fixtures/slow.php?timeToWait=3 HTTP/1.1" 200 27 "-" "GuzzleHttp/7"

Now Nginx is logging when it responds to the request, not when it receives it, so what we can infer from this is that the requests all arrived at 18:57:48, and the 1sec request finished after 1sec at 18:57:49; the 2sec request finished after 2sec @ 18:57:50, and similarly the third one, 3sec, finished after 3 seconds at 18:57:51.

It's easier to see if I make the requests hang on for different periods of time. Here's an example where they take 1sec, 12sec and 23sec respectively:

172.31.0.4 - - [21/Jan/2023:18:59:07 +0000] "GET /test-fixtures/slow.php?timeToWait=1 HTTP/1.1" 200 27 "-" "GuzzleHttp/7"
172.31.0.4 - - [21/Jan/2023:18:59:18 +0000] "GET /test-fixtures/slow.php?timeToWait=12 HTTP/1.1" 200 28 "-" "GuzzleHttp/7"
172.31.0.4 - - [21/Jan/2023:18:59:29 +0000] "GET /test-fixtures/slow.php?timeToWait=23 HTTP/1.1" 200 28 "-" "GuzzleHttp/7"

We can infer they all arrived at 18:59:06. 1sec later at 18:59:07 the first request completed; 12sec later the second one completed at 18:59:18 (18:59:18 - 18:59:06 is 12sec); and lastly the third request - which will take 23sec to run - indeed finishes at 18:59:29 - 18:59:06 = 23sec later.

Excellent. Working as expected.

For completeness I also tested an async POST request:

/** @testdox it can make an asynchronous POST request */
public function testAsyncPost()
{
    $client = new Client();
    $promise = $client->requestAsync(
        'POST',
        'https://httpbin.org/post',
        ['form_params' => ['foo' => 'bar']]
    );
    $response = $promise->wait();
    $this->assertEquals(200, $response->getStatusCode());
    $this->assertJson($response->getBody());
    $httpBinResponse = json_decode($response->getBody());
    $this->assertEquals('bar', $httpBinResponse->form->foo);
}

No surprises.


Symfony

/** @testdox it can make a GET request */
public function testGet()
{
    $client = HttpClient::create();
    $response = $client->request('GET', 'https://api.github.com/users/adamcameron');
    $this->assertEquals(200, $response->getStatusCode());
    $this->assertJson($response->getContent());
    $this->assertGitInfoIsCorrect($response->getContent());
}

This is identical to the Guzzle example except Symfony uses a factory method to create the client object compared Guzzle just using new; and Guzzle uses getBody instead of Symfony's getContent.

/** @testdox it can make a POST request */
public function testPost()
{
    $client = HttpClient::create();
    $response = $client->request(
        'POST',
        'https://httpbin.org/post',
        ['body' => ['foo' => 'bar']]
    );
    $this->assertEquals(200, $response->getStatusCode());
    $this->assertJson($response->getContent());
    $httpBinResponse = json_decode($response->getContent());
    $this->assertEquals('bar', $httpBinResponse->form->foo);
}

It's just occurred to me that knowing Symfony, it can likely do async request collections too. And after some googling: sure enough I've found a way ("Symfony › HTTP Client › Concurrent Requests"):

/** it can make multiple asynchronous GET requests */
public function testMultipleAsyncGet()
{
    $client = HttpClient::create();
    $requestsToMakeConcurrently = [
        $client->request('GET', 'http://nginx/test-fixtures/slow.php?timeToWait=1'),
        $client->request('GET', 'http://nginx/test-fixtures/slow.php?timeToWait=2'),
        $client->request('GET', 'http://nginx/test-fixtures/slow.php?timeToWait=3')
    ];
    $stream = $client->stream($requestsToMakeConcurrently);

    $i = 1;
    $startTime = microtime(true);
    foreach ($stream as $response => $chunk) {
        if ($chunk->isLast()) {
            $this->assertEquals(200, $response->getStatusCode());
            $this->assertEquals("waited $i seconds", $response->getContent());
            $i++;
        }
    }
    $endTime = microtime(true);
    $totalTime = $endTime - $startTime;
    $this->assertGreaterThan(3, $totalTime);
    $this->assertLessThan(4, $totalTime);
}

This is analogous to the Guzzle version. It's implementation is not as nice though IMO.


PHP Streams

/** @testdox it can make a GET request */
public function testGet()
{
    $context = stream_context_create([
        'http' => [
            'method' => 'GET',
            'header' => ['User-Agent: ' . $this->getUserAgentForCurl()]
        ]
    ]);
    $response = file_get_contents('https://api.github.com/users/adamcameron', false, $context);
    $this->assertJson($response);
    $this->assertGitInfoIsCorrect($response);
}
/** @testdox it can make a POST request */
public function testPost()
{
    $context = stream_context_create([
        'http' => [
            'method' => 'POST',
            'header' => [
                'User-Agent: ' . $this->getUserAgentForCurl(),
                'Content-Type: application/x-www-form-urlencoded'
            ],
            'content' => http_build_query(['foo' => 'bar'])
        ]
    ]);
    $response = file_get_contents('https://httpbin.org/post', false, $context);
    $this->assertJson($response);
    $httpBinResponse = json_decode($response);
    $this->assertEquals('bar', $httpBinResponse->form->foo);
}

OK. It's poss just me being pedantic, but I get a bit itchy looking at file_get_contents on a URL. I mean I know an HTTP request is fetching a file - so semantically that's fine - but it still seems odd.


Conclusion

For these superficial test cases, I prefer Guzzle. Doubtless there more one can do with Symfony's HTTP client, because there's always more one can do with Symfony's stuff; but the same will apply with Guzzle too no doubt. I did not know about PHP's streams before, and whilst this might not be a good use of it, there'll likely be other situations to use them.

I'm mostly pleased that Guzzle seems easy to use again, and for both sync and async stuff. Cool.

All the code shown in here is @ /test/integration/http on Github.

Righto.

--
Adam

Thursday 8 September 2022

CFML: looking at how CFWheels messes up a loop

G'day:

This exercise came about from a bug in CFWheels we encountered today.

We got a notification that a user had received a 500-error, and I was looking into it. It was happening in /wheels/global/internal.cfm, in this bit of code:

for (local.key in application.wheels.cache[arguments.category]) {
    if (Now() > application.wheels.cache[arguments.category][local.key].expiresAt) {

On the second line we are getting an error: key [81AB90EB428410D6222B18AAE12EC014] doesn't exist.

It should be obvious to even a journeyman dev what the problem is here.

When the first line of code runs, there is a key 81AB90EB428410D6222B18AAE12EC014 in application.wheels.cache. By the time the second line of code runs, some other request has come along and removed 81AB90EB428410D6222B18AAE12EC014 from application.wheels.cache, and hence we get the error. This is why everyone (?) knows that one must synchronise access to shared scopes like the application scope, so that if a process is writing to it, then other processes can't read from it. Or competing writes can't be happening simultaneously. IE: Chuck a lock block around this sort of thing. This is CFML 101 sort of stuff, CFWheels Team.

On the face of it, this was just super bad timing. What are the odds that one request will run the first line, and then before that request gets to the second line, another request has blitzed what the first line "knew" was there to use.

But it's not really super bad timing. It's just bad coding. This code is a loop. When does a for (key in struct) loop decide which keys exist in the struct, and accordingly which keys to loop over? At the first time the loop is entered. And only then. So if there are 10000 items in that struct when our request first ran, any other process has from the first iteration until the 9999th iteration to delete the 10000th key, and we'll get this error. That's a lot of opportunity for failure. Also, due to the very nature of this code, there's a good chance that a bunch of requests are gonna be running it simultaneously: the key deletions come after a determination that a shared cache is full and needs culling (let's not get into matter of why a function called addToCache is deleting stuff anyhow. This is not the worst thing about this cluster🦆, I assure you), and at the moment in time the cache seems to be full, then every call to addToCache (sic) will try to delete stuff. Ugh.

To demonstrate what's going on here, I've knocked a quick example together (code on github):

// Application.cfc
component {
    this.name = "testLoopingOverAppScopedStructs"
}
// firstRequest.cfm
cfflush(interval=16)
application.numbers = [
    "one" = "tahi",
    "two" = "rua",
    "three" = "toru",
    "four" = "wha"
]

for (en in application.numbers) {
    writeOutput("keys @ top of loop: #application.numbers.keyList()#<br>")
    writeOutput("current key: #en#<br>")
    writeOutput("current value: #application.numbers[en]#<br>")
    sleep(2000)
    writeOutput("keys after sleep: #application.numbers.keyList()#<br>")
}
// secondRequest.cfm
application.numbers["five"]  = "rima"
application.numbers.delete("four")
writeDump(application.numbers)

All very straight forward. In secondRequest.cfm I only add a key so there's a visual cue in the output of firstRequest.cfm when the second request ran.

The test is to run firstRequest.cfm, then whist that's running, run secondRequest.cfm. The output from a typical run of firstRequest.cfm would be like:

keys @ top of loop: one,two,three,four
current key: one
current value: tahi
keys after sleep: one,two,three,four
keys @ top of loop: one,two,three,four
current key: two
current value: rua
keys after sleep: one,two,three,five <--- here is where I ran secondRequest.cfm
keys @ top of loop: one,two,three,five
current key: three
current value: toru
keys after sleep: one,two,three,five
keys @ top of loop: one,two,three,five
current key: four

[...]
Error Occurred While Processing Request
Element FOUR is undefined in a CFML structure referenced as part of an expression.

You can see that the keys the loop iterates over is set at the beginning, given it's unaffected by changes to the struct's keys during the loop.


What should the CFWheels codebase do to mitigate this. Well: here's a list of things that are… erm… "less good than they could be "… in that code.

  • Framework functionality should be in classes, not in include files. In this case cache-handing code should be in a CachingService or somesuch.
  • internal.cfm is an awful name for a library. A filename should describe what the code does (like, say "CachingService"), not how it's used ("internally", apparently. Sounds uncomfortable, not to mention a bit unhygienic).
  • Related to this, that file is a hotchpotch mess of all sorts of shite that should be homed somewhere else. This describes most files in the CFWheels codebase, unfortunately.
  • The only code that touches the application scope should be in an ApplicationScopeProxy object. BTW this makes controlling the scope-locking way easier to manage. See also: SessionScopeProxy / ServerScopeProxy / etc
  • Within that proxy, access to any shared scope should be appropriately synchronised (with a lock). This should also be extended to mitigating race conditions like we have here.
  • If your function is called addToCache then stick to doing that. Code that removes stuff from the cache does not belong there.
  • All the code in that method is conditional. All of it. There is no "mainline" or happy path. Simplify the conditions and bail out if they're not met. Don't put the code you actually expect to run inside a conditional block.
  • Connected to the two preceding points: most of the code in that method is not actually anything to do with adding something to a cache.
  • For the love of god stop prefixing functions with $ to indicate they're private. Just make them private.

I don't think anything there is particularly controversial or complicated. And whilst it's beyond "CFML 101", I think it's within the low bar of "Writing Frameworks 101".

Anyway. There you got. A coupla hours in the life of someone who has to use CFWheels.

Righto.

--
Adam

Monday 18 April 2022

CFML: Adding a LogBox logger to a CFWheels app via dependency injection

G'day:

This follows on from CFML: implementing dependency injection in a CFWheels web site. In that article I got the DI working, but only with a test scenario. For the sake of completeness, I'm gonna continue with the whole point of the exercise, which is getting a logger service into my model objects, via DI.


0.6 adding LogBox

I ran into some "issues" yesterday whilst trying to write this article. Documented here: "A day in the life of trying to write a blog article in the CFML ecosystem", and there's some file changes committed as 0.5.1. So that killed my motivation to continue this work once I'd waded through that. I appear to be back on track now though.

Right so I'll add a quick test to check LogBox is installed. It's a bit of a contrived no-brainer, but this is playing to my strengths. So be it:

describe("Tests the LogBox install", () => {
    it("is instantiable", () => {
        expect(() => getComponentMetadata("LogBox")).notToThrow(regex="^.*can't find component \[LogBox\].*$")
    })
})

If LogBox is where it's supposed to be: it'll pass. Initially I had a new LogBox() in there, but it needs some config to work, and that requires a bit of horsing around, so I'll deal with that next. For now: is it installed? Test sez "no":

Test sez… yes.

OK. That was unexpected. Why did that pass? I have checked that LogBox is not installed, so WTH??

After a coupla hours of horsing about looking at TestBox code, I worked out there's a logic… um… shortfall (shall I say) in its implementation of that regex param, which is a bit wayward. The code in question is this (from /system/Assertion.cfc):

 if (
    len( arguments.regex ) AND
    (
        !arrayLen( reMatchNoCase( arguments.regex, e.message ) )
        OR
        !arrayLen( reMatchNoCase( arguments.regex, e.detail ) )
    )
) {
    return this;
}

Basically this requires both of message and detail to not match the regex for it to be considered "the same" exception. This is a bit rigorous as it's really unlikely for this to be the case in the real world. I've raised it with Ortus (TESTBOX-349), but for now I'll just work around it. Oh yeah, there's a Lucee bug interfering with this too. When an exception does have the same message and details, Lucee ignores the details. I've not raised a bug for this yet: I'm waiting for them to fee-back as to whether I'm missing something. When there's a ticket, I'll cross-post it here.

Anyway, moving on, I'll just check for any exception, and that'll do:

expect(() => getComponentMetadata("LogBox")).notToThrow()

That was a lot more faff to get to this point than I expected. Anyway. Now I install LogBox:

root@60d2920f4c60:/var/www# box install logbox
√ | Installing package [forgebox:logbox]

0.7 wiring LogBox into the DependencyInjectionService

One of the reasons the previous step really didn't push the boat out with testing if LogBox was working, is that to actually create a working LogBox logger takes some messing about; and I wanted to separate that from the installation. And also to give me some time to come up with the next test case. I want to avoid this sort of thing:

Possibly © 9gag.com - I am using it without permission. If you are the copyright owner and wish me to remove this, please let me know.

I don't want to skip to a test that is "it can log stuff that happens in the Test model object". I guess it is part ofthe requiremnt that the logger is handled via dependency injection into the model, so we can first get it set up and ready to go in the DependencyInjectionService. I mean the whole thing here is about DI: the logger is just an example usage. I think the next step is legit.

I've never used LogBox before, so I am RTFMing all this as I type (docs: "Configuring LogBox"). It seems I need to pass a Config object to my LogBox object, then get the root logger from said object… and that's my logger. Al of that can go in a factory method in configureDependencies, adn I'll just put the resultant logger into the IoC container.

it("loads a logger", () => {
    diService = new DependencyInjectionService()

    logger = diService.getBean("Logger")

    expect(logger).toBeInstanceOf("logbox.system.logging.Logger")

    expect(() => logger.info("TEST")).notToThrow()
})

I'm grabbing a logger and logging a message with it. The expectation is simply that the act of logging doesn't error. For now.

First here's the most minimal config I seem to be able to get away with:

component {
    function configure() {
        variables.logBox = {
            appenders = {
                DummyAppender = {
                    class = "logbox.system.logging.appenders.DummyAppender"
                }
            }
        }
    }
}

The docs ("LogBox DSL ") seemed to indicate I only needed the logBox struct, but it errored when I used it unless I had at least one appender. I'm just using a dummy one for now because I'm testing config, not operations. And there's nothing to test there: it's all implementation, so I think it's fine to create that in the "green" phase of "red-green-refactor" from that test above (currently red). With TDD the red phase is just to do the minimum code to make the test pass. That doesn't mean it needs to be one line of code, or one function or whatever. If my code needed to call a method on this Config object: then I'd test that separately. But I'm happy that this is - well - config. It's just data.

Once we have that I can write my factory method on DependencyInjectionService:

private function configureDependencies() {
    variables.container.declareBean("DependencyInjectionService", "services.DependencyInjectionService")
    variables.container.declareBean("TestDependency", "services.TestDependency")

    variables.container.factoryBean("Logger", () => {
        config = new Config()
        logboxConfig = new LogBoxConfig(config)

        logbox = new LogBox(logboxConfig)
        logger = logbox.getRootLogger()

        return logger
    })
}

I got all that from the docs, and I have nothing to add: it's pretty straight forward. Let's see if the test passes:

Cool.

Now I need to get my Test model to inject the logger into itself, and verify I can use it:

it("logs calls", () => {
    test = model("Test").new()
    prepareMock(test)
    logger = test.$getProperty("logger")
    prepareMock(logger)
    logger.$("debug")

    test.getMessage()

    loggerCallLog = logger.$callLog()
    expect(loggerCallLog).toHaveKey("debug")
    expect(loggerCallLog.debug).toHaveLength(1)
    expect(loggerCallLog.debug[1]).toBe(["getMessage was called"])
})

Here I am mocking the logger's debug method, just so I can check it's being called, and with what. Having done this, I am now wondering about "don't mock what you don't own", but I suspect in this case I'm OK because whilst the nomenclature is all "mock", I'm actually just spying on the method that "I don't own". IE: it's LogBox's method, not my application's method. I'll have to think about that a bit.

And the implementation for this is way easier than the test:

// models/Test.cfc
private function setDependencies() {
    variables.dependency = variables.diService.getBean("TestDependency")
    variables.logger = variables.diService.getBean("Logger")
}

public function getMessage() {
    variables.logger.debug("getMessage was called")
    return variables.dependency.getMessage()
}

Just for the hell of it, I also wrote a functional test to check the append was getting the expected info:

it("logs via the correct appender", () => {
    test = model("Test").new()

    prepareMock(test)
    logger = test.$getProperty("logger")

    appenders = logger.getAppenders()
    expect(appenders).toHaveKey("DummyAppender", "Logger is not configured with the correct appender. Test aborted.")

    appender = logger.getAppenders().DummyAppender
    prepareMock(appender)
    appender.$("logMessage").$results(appender)

    test.getMessage()

    appenderCallLog = appender.$callLog()

    expect(appenderCallLog).toHaveKey("logMessage")
    expect(appenderCallLog.logMessage).toHaveLength(1)
    expect(appenderCallLog.logMessage[1]).toSatisfy((actual) => {
        expect(actual[1].getMessage()).toBe("getMessage was called")
        expect(actual[1].getSeverity()).toBe(logger.logLevels.DEBUG)
        expect(actual[1].getTimestamp()).toBeCloseTo(now(), 2, "s")
        return true
    }, "Log entry is not correct")
})

It's largely the same as the unit test, except it spies on the appender instead of the logger. There's no good reason for doing this, I was just messing around.


And that's it. I'm pushing that up to Git as 0.7

… and go to bed.

Righto.

--
Adam

Sunday 17 April 2022

A day in the life of trying to write a blog article in the CFML ecosystem

G'day:

This is not the article I intended to write today. That article was gonna be titled "CFML: Adding a LogBox logger to a CFWheels app via dependency injection", but I'll need to get to that another day now.

Here's how far that article got before the wheels fell off:

And that was it.

Why? Well I started by writing an integration test just to check that box install logbox did what I expected:

import test.BaseSpec
import logbox.system.logging.LogBox

component extends=BaseSpec {

    function run() {
        describe("Tests the LogBox install", () => {
            it("is instantiable", () => {
                expect(() => getComponentMetadata("LogBox")).notToThrow()
            })
        })
    }
}

Simple enough. It'll throws an exception if LogBox ain't there, and I'm expecting that. It's a dumb test but it's a reasonable first step to build on.

I run the test:

Err… come again? I ain't installed it yet. I lifted the code from the expect callback out and run it "raw" in the body ofthe test case: predictable exception. I put it back in the callback. Test passes. I change the matcher to be toThrow. Test still passed. So this code both throws and exception and doesn't throw an exception. This is pleasingly Schrödingeresque, but not helpful.

The weird thing is I know this is not a bug in TestBox, cos we use notToThrow in our tests at work. I port the test over to my work codebase: test fails (remember: this is what I want ATM, we're still at the "red" of "red-green-refactor").

I noticed that we were running a slightly different version of Testbox in the work codebase: 4.4.0-snapshot compared to my 4.5.0+5. Maybe there's been a regression. I changed my TestBox version in box.json and - without thinking things through - went box install again (not just box install testbox which is all I really needed to do), and was greeted with this:

That's reasonably bemusing as I had just used box install fw1 to install it in the first place, and that went fine. And I have not touched it since. I checked what version I already had installed (in framework/box.json), and it claims 4.3.0. So… ForgeBox… I beg to differ pal. You found this version y/day, why can't you find it today? I check on ForgeBox, and for 4.x I see versions 4.0.0, 4.1.0, 4.2.0, 4.5.0-SNAPSHOT. OK, so granted: no 4.3.0. Except that's what it installed for me yesterday. Maybe 4.3.0 has issues and got taken down in the last 24h (doubtful, but hey), so I blow away my /framework directory, and remove the entry from box.json, and box install fw1 again. This is interesting:

root@280d80cf28c6:/var/www# box install fw1
√ | Installing package [forgebox:fw1]
|------------------------------------------------
| Verifying package 'fw1' in forgebox, please wait...
| Installing version [4.2.0].

4.2.0. But its entry in its own box.json is 4.3.0, and the constraint it put in my box.json is ^4.3.0.

I do not have time or inclination for any of this, so I just stick a constraint of ~4.2.0 in my box.json, and that seems to have solved it. I mean the error went away: it's still installing 4.3.0. Even with a hard-coded 4.2.0 it's still installing 4.3.0.

Brad Wood from Ortus/CommandBox had a look at this, nutted-out that there was something wrong with the way the FW/1 package on ForgeBox was configured, and he in turn pinged Steve Neiland who looks after FW/1 these days, and he got this sorted. I'm now on 4.3.0, and it says it's 4.2.0. And box install no longer complains at me. Cheers fellas.

Then I noticed that because of the stupid way CFWheels "organises" itself in the file system, I have inadvertantly overwritten a bunch of my own CFWheels files. Sigh. CFWheels doesn't bother to package itself up as "app" (its stuff) and "implementation" (my code that uses their app), it just has "here's some files: some you should change (anything outside the wheels subdirectory), some you probably shouldn't (the stuff in the wheels subdirectory)", but there's no differentiation when it comes to installation: all the files are deployed. Overwriting all the user-space files with their original defaults. Sorry but this is just dumbarsey. Hurrah for source control and small commit iterations is all I can say, as I could just revert some files and I was all good.

Right so now I have the same version of TestBox installed here as in our app at work (remember how this was all I was tring to do? Update testbox. Nothing to do with FW/1, and nothing to do with CFWheels. But there's an hour gone cocking around with that lot).

And the test still doesn't work. Ballocks.

I notice the Lucee version is also different. We're locked into an older version of Lucee at work due to bugs and incompats in newer versions that we're still waiting on to be fixed, so the work app is running 5.3.7.47, and I am on 5.3.8.206. Surely it's not that? I rolled my app's Lucee version back to 5.3.7.47 and the test started failing (correctly). OK, so it's a Lucee issue.

I spent about an hour messing around doing a binary search of Lucee container versions until I identified the last version that wasn't broken (5.3.8.3) and the next version - a big jump here - 5.3.8.42 that was broken. I looked at a diff of the code but nothing leapt out at me. This was slightly daft as I had no idea what I was looking for, so that was probably half an hour of time looking at Lucee's source code in an aimless fashion. I actually saw the change that was the problem, but didn't clock that that is what caused it at the time.

Having drawn a blank, I slapped my forehead, called myself a dick, and went back to the code in TestBox that was behaving differently. That would obviously tell me where to look for the issue.

I tracked the problem down to here, in system/Assertion.cfc:

    // Message+Detail regex must not match
    if (
        len( arguments.regex ) AND
        (
            !arrayLen( reMatchNoCase( arguments.regex, e.message ) ) OR !arrayLen(
                reMatchNoCase( arguments.regex, e.detail )
            )
        )
    ) {
        return this;
    }

    fail( arguments.message );
}

I distilled that down into a portable repro case:

s = ""
pattern = ".*"
writeDump([
    reMatch = s.reMatchNoCase(pattern),
    matches = s.matches(pattern),
    split = s.split(pattern)
])

There are some Java method calls there to act as controls, but on Lucee's current version, we get this:

And on earlier versions it's this:

(Full disclosure: I'm using Lucee 4.5 on trycf.com for that second dump, but it's the same results in earlier versions of Lucee 5, up to the point where it starts going wrong)

Note how previously a regex match of .* matches an empty string? This is correct. It does. In all regex engines I know of. Yet in Lucee's current versions, it returns a completely empty array. This indicates no match, and it's wrong. Simple as that. So there's the bug.

I was pointed in the direction of an existing issue for this: LDEV-3703. Depsite being a regression they know they caused, Lucee have decided to only fix it in 6.x. Not the version they actually broke. Less than ideal, but so be it.

There were a coupla of Regex issues dealt with between those Lucee versions I mentioned before. Here's a Jira search for "fixversion >= 5.3.8.4 and fixversion < 5.3.8.42 and text ~ regex". I couldn't be arsed tracking back through the code, but I did find something in LDEV-3009 mentioning a new Application.cfc setting:

this.useJavaAsRegexEngine = true

This is documented for ColdFusion in Application variables, and… absolutely frickin' nowhere in the Lucee docs, as far as I can see.

On a whim I stuck that setting in my Application.cfc and re-ran the test. If the setting was false: the test doesn't work. If it was true: the test does work. That's something, but Lucee is not off the hook here. The behaviour of that regex match does not change between the old and new regex engines! .* always matches an empty string! So there's still a bug.

However, being pragmatic, I figured "problem solved" (for now), and moved on. For some reason I restarted my container, and re-hit my tests:

I switched the setting to this.useJavaAsRegexEngine = false and the tests ran again (failed incorrectly, but ran). So… let me get this straight. For TestBox to work, I need to set that setting to true. To get CFWheels to work, I need to set it to false.

For pete's sake.

As I said on the Lucee subchannel on the CFML Slack:

Do ppl recall how I've always said these stupid flags to change behaviour of the language were a fuckin dumb idea, and are basically unusable in a day and age where we all rely on third-party libs to do our jobs?

Exhibit. Fucking. A.

Every single one of these stupid, toxic, setting doubles the overhead for library providers to make their code work. I do not fault TestBox or CFWheels one bit here. They can't be expected to support the exponential number of variations each one of those settings accumulates. I can firmly say that no CFML code should ever be written that depends on any of these settings. And no library or third-party code should ever be tested with the setting variation on. Just ignore them. The settings should not exist. Anyway: this is an editorial digression. "We are where we are" as the over-used saying goes.


Screw all this. Seriously. All I wanted to do is to do a blog article about perhaps 50-odd new lines of code in my example app. Instead I spent four hours untangling this shite. And my blog article has not progressed.

Here's what I needed to do to my app to work around these various issues:

This is all committed to GitHub as 0.5.1

Writing this sure was cathartic. I think I was my own audience for this one. Ah well. Good on you if you got to here.

Righto.

--
Adam

Friday 15 April 2022

CFML: implementing dependency injection in a CFWheels web site

G'day:

Recently I wanted to abstract some logic out of one of our CFWheels model classes, into its own representation. Code had grown organically over time, with logic being inlined in functions, making a bunch of methods a bit weighty, and had drifted well away from the notion of following the "Single Responsibility Principle". Looking at the code in question, even if I separated it out into a bunch of private methods (it was a chunk of code, and refactoring into a single private method would not have worked), it was clear that this was just shifting the SRP violation out of the method, and into the class. This code did not belong in this class at all. Not least of all because we also needed to use some of it in another class. This is a pretty common refactoring exercise in OOP land.

I'm going to be a bit woolly in my descriptions of the functionality I'm talking about here, because the code in question is pretty business-domain-specific, and would not make a lot of sense to people outside our team. Let's just say it was around logging requirements. It wasn't, but that's a general notion that everyone gets. We need to log stuff in one of our classes, and currently it's all baked directly into that class. It's not. But let's pretend.

I could see what I needed to do: I'll just rip this lot out into another service class, and then use DI to… bugger. CFWheels does not have any notion of dependency injection. I mean... I'm fairly certain it doesn't even use the notion of constructors when creating objects. If one wants to use some code in a CFWheels model, one writes the code in the model. This is pretty much why we got to where we are. If one wants to use code from another class in one's model... one uses the model factory method to get a different model (eg: myOtherModel = model("Other")). Inline in one's code. There's a few drawbacks here:

  • In CFWheels parlance, "model" means "an ORM representation of a DB row". The hard-coupling between models and a DB table is innate to CFWheels. It didn't seem to occur to the architects of CFWheels that not all domain model objects map to underlying DB storage. One can have a "tableless model", but it still betrays the inappropriate coupling between domain model and storage representation. A logging service is a prime example of this. It is part of the domain model. It does not represent persisted objects. In complex applications, the storage considerations around the logic is just a tier underneath the domain model. It's not baked right into the model. I've just found a telling quote on the CFWheels website:
    Model: Just another name for the representation of data, usually a database table.
    Frameworks and CFWheels › Model-View-Controller (MVC)
    That's not correct. That is not what the model is. But explains a lot about CFWheels.
  • Secondly, it's still a bit of a fail of separation of concerns / IoC if the "calling code" hard-codes which implementation of a concern it is going to use.
  • If one is writing testable code, one doesn't want that second-point tight-coupling going on. If I'm testing features of my model, I want to stub out the logger it's using, for example. This is awkward to do if the decision as to which logger implementation we're using is baked-into the code I'm testing.

Anyway, you probably get it. Dependency injection exists for a reason. And this is something CFWheels appears to have overlooked.


0.1 - Baseline container

I have worked through the various bits and pieces I'm going to discuss already, to make sure it all works. But as I write this I am starting out with a bare-bones Lucee container, and a bare-bones MariaDB container (from my lucee_and_mariadb repo. I've also gone ahead and installed TestBox, and my baseline tests are passing:

Yes. I have a test that TestBox is installed and running correctly.

We have a green light, so that's a baseline to start with. I've tagged that in GitHub as 0.1.


0.2 - CFWheels operational

OK, I'll install CFWheels. But first my requirement here is "CFWheels is working". I will take this to mean that it displays its homepage after install, so I can test for that pretty easily:

it("displays the welcome page", () => {
    cfhttp(url="http://lucee:8888/index.cfm", result="httpResponse");

    expect(httpResponse.statusCode).toBe(200)
    expect(httpResponse.fileContent).toInclude("<title>CFWheels</title>")
})

I'm using TDD for even mundane stuff like this so I don't get ahead of myself, and miss bits I need to do to get things working.

This test fails as one would expect. Installing CFWheels is easy: just box install cfwheels. This installs everything in the public web root which is not great, but it's the way CFWheels works. I've written another series about how to get a CFWheels-driven web app working whilst also putting the code in a sensible place, summarised here: Short version: getting CFWheels working outside the context of a web-browsable directory, but life's too short to do all that horsing around today, so we'll just use the default install pattern. Note: I do not consider this approach to be appropriate for production, but it'll do for this demonstration.

After the CFWheels installation I do still have to fix-up a few things:

  • It steamrolled my existing Application.cfc, so I had to merge the CFWheels bit with my bit again.
  • Anything in CFWheels will only work properly if it's called in the context of a CFWheels app, so I need to tweak my test config slightly to accommodate that.
  • And that CFWheels "context" only works if it's called from index.cfm. So I need to rename my test/runTests.cfm to be index.cfm.

Having done that:

A passing test is all good, but I also made sure the thing did work. By actually looking at it:

I've tagged that lot again, as 0.2.

0.3 - A working model

I want to mess around with models, so I need to create one. I have a stub DB configured with this app, and it has a table test with a coupla test rows in it. I'll create a CFWheels model that maps to that. CFWheels expects plural table names, but mine's singular so I need a config tweak there. I will test that I can retrieve test records from it.

it("can find test records from the DB", () => {
    tests = model("Test").findAll(returnAs="object")

    expect(tests).notToBeEmpty()

    tests.each((test) => {
        expect(test).toBeInstanceOf("models.Test")
        expect(test.properties().keyArray().sort("text")).toBe(["id", "value"])
    })
})

And the implementation to make it pass:

import wheels.Model

component extends=Model {

    function config() {
        table(name="test")
    }
}

Good. Next I wanted to check when CFWheels calls my Test class's constructor. Given one needs to use that factory method (eg: model("Test").etc) to do anything relating to model objects / collections . etc, I was not sure whether the constructor comes into play. Why do i care? Because when using dependency injection, one generally passes the dependencies in as constructor arguments. This is not the only way of doing it, but it's the most obvious ("KISS" / "Principle of Least Astonishment") approach. So let's at least check.

it("has its constructor called when it is instantiated by CFWheels", () => {
    test = model("Test").new()

    expect(test.getFindMe()).toBe("FOUND")
})

Implementation:

public function init() {
    variables.findMe = "FOUND"
}

public string function getFindMe() {
    return variables.findMe
}

Result:

OK so scratch that idea. CFWheels does not call the model class's constructor. Initially I was annoyed about this as it seems bloody stupid. But then I recalled that when one is using a factory method to create objects, it's not unusual to not use the public constructor to do so. OK fair enough.

I asked around, and (sorry I forget who told me, or where they told me) found out that CFWheels does provide an event hook I can leverage for when an model object is created: model.afterInitialization. I already have my test set up to manage my expectations, so I can just change my implementation:

function config() {
    table(name="test")
    afterInitialization("setFindMe")
}

public function setFindMe() {
    variables.findMe = "FOUND"
}

And that passed this time. Oh I changed the test label from "has its constructor called…" to be "has its afterInitialization handler called…". But the rest of the test stays the same. This is an example of how with TDD we are testing the desired outcome rather than the implementation. It doesn't matter whether the value is set by a constructor or by an event handler: it's the end result of being able to use the value that matters.

At the moment I have found my "way in" to each object as they are created. I reckon from here I can have a DependencyInjectionService that I can call upon from the afterInitialization handler so the model can get the dependencies it needs. This is not exactly "dependency injection", it's more "dependency self-medication", but it should work.

I'll tag here before I move on. 0.3

0.4 integrating DI/1

My DI requirements ATM are fairly minimal, but I am not going to reinvent the wheel. I'm gonna use DI/1 to handle the dependencies. I've had a look at it before, and it's straight forward enough, and is solid.

My tests are pretty basic to start with: I just want to know it's installed properly and operations:

it("can be instantiated", () => {
    container = new framework.ioc("/services")

    expect(container).toBeInstanceOf("framework.ioc")
})

And now to install it: box install fw1

And we have a passing test (NB: I'm not showing you the failures necessarily, but I do always actually not proceed with anything until I see the test failing):

It's not much use unless it loads up some stuff, so I'll test that it can:

it("loads services with dependencies", () => {
    container = new framework.ioc("/services")

    testService = container.getBean("TestService")
    expect(testService.getDependency()).toBeInstanceOf("services.TestDependency")
})

I'm gonna show the failures this time. First up:

This is reasonable because the TestService class isn't there yet, so we'd expect DI/1 to complain. The good news is it's complaining in the way we'd want it to. TestService is simple:

component {

    public function init(required TestDependency testDependency) {
        variables.dependency = arguments.testDependency
    }

    public TestDependency function getDependency() {
        return variables.dependency
    }
}

Now the failure changes:

This is still a good sign: DI/1 is doing what it's supposed to. Well: trying to. And reporting back with exactly what's wrong. Let's put it (and, I imagine: you) out of its misery and give it the code it wants. TestDependency:

component {
}

And now DI/1 has wired everything together properly:

As well as creating a DI/1 instance and pointing it at a directory (well: actually I won't be doing that), I need to hand-crank some dependency creation as they are not just a matter of something DI/1 can autowire. So I'm gonna wrap-up all that in a service too, so the app can just use a DependencyInjectionService, and not need to know what its internal workings are.

To start with, I'll just make sure the wrapper can do the same thing we just did with the raw IoC object from the previous tests:

describe("Tests for DependencyInjectionService", () => {
    it("loads the DI/1 IoC container and its configuration", () => {
        diService = new DependencyInjectionService()

        testService = diService.getBean("DependencyInjectionService")

        expect(testService).toBeInstanceOf("services.DependencyInjectionService")
    })
})

Instead of testing the TestService here, I decided to use DependencyInjectionService to test it can… load itself

There's a bit more code this time for the implementation, but not much.

import framework.ioc

component {

    public function init() {
        variables.container = new ioc("")
        configureDependencies()
    }

    private function configureDependencies() {
        variables.container.declareBean("DependencyInjectionService", "services.DependencyInjectionService")
    }

    public function onMissingMethod(required string missingMethodName, required struct missingMethodArguments) {
        return variables.container[missingMethodName](argumentCollection=missingMethodArguments)
    }
}
  • It creates an IOC container object, but doesn't scan any directories for autowiring opportunities this time.
  • It hand-cranks the loading of the DependencyInjectionService object.
  • It also acts as a decorator for the underlying IOC instance, so calling code just calls getBean (for example) on a DependencyInjectionService instance, and this is passed straight on to the IOC object to do the work.

And we have a passing test:

Now we can call our DI service in our model, and the model can use it to configure its dependencies. First we need to configure the DependencyInjectionService with another bean:

it("loads TestDependency", () => {
    diService = new DependencyInjectionService()

    testDependency = diService.getBean("TestDependency")

    expect(testDependency).toBeInstanceOf("services.TestDependency")
})
private function configureDependencies() {
    variables.container.declareBean("DependencyInjectionService", "services.DependencyInjectionService")
    variables.container.declareBean("TestDependency", "services.TestDependency")
}
describe("Tests for TestDependency", () => {
    describe("Tests for getMessage method")
        it("returns SET_BY_DEPENDENCY", () => {
            testDependency = new TestDependency()

            expect(testDependency.getMessage()).toBe("SET_BY_DEPENDENCY")
        })
    })
})
// TestDependency.cfc
component {

    public string function getMessage() {
        return "SET_BY_DEPENDENCY"
    }
}

That's not quite the progression of the code there. I had to create TestDependency first, so I did its test and it first; then wired it into DependencyInjectionService.

Now we need to wire that into the model class. But first a test to show it's worked:

describe("Tests for Test model", () => {
    describe("Tests of getMessage method", () => {
        it("uses an injected dependency to provide a message", () => {
            test = model("Test").new()

            expect(test.getMessage()).toBe("SET_BY_DEPENDENCY")
        })
    })
})

Hopefully that speaks for itself: we're gonna get that getMessage method in Test to call the equivalent method from TestDependency. And to do that, we need to wire an instance of TestDependency into our instance of the Test model. I should have thought of better names for these classes, eh?

// /models/Test.cfc
import services.DependencyInjectionService
import wheels.Model

component extends=Model {

    function config() {
        table(name="test")
        afterInitialization("setFindMe,loadIocContainer")
    }

    public function setFindMe() {
        variables.findMe = "FOUND"
    }

    public string function getFindMe() {
        return variables.findMe
    }

    private function loadIocContainer() {
        variables.diService = new DependencyInjectionService()
        setDependencies()
    }

    private function setDependencies() {
        variables.dependency = variables.diService.getBean("TestDependency")
    }

    public function getMessage() {
        return variables.dependency.getMessage()
    }

}

That works…

…but it needs some adjustment.

Firstly I want the dependency injection stuff being done for all models, not just this one. So I'm going to shove some of that code up into the Model base class:

// /models/Model.cfc
/**
 * This is the parent model file that all your models should extend.
 * You can add functions to this file to make them available in all your models.
 * Do not delete this file.
 */

import services.DependencyInjectionService

 component extends=wheels.Model {

    function config() {
        afterInitialization("loadIocContainer")
    }

    private function loadIocContainer() {
        variables.diService = new DependencyInjectionService()
        setDependencies()
    }

    private function setDependencies() {
        // OVERRIDE IN SUBCLASS
    }
}
// models/Test.cfc
import wheels.Model

component extends=Model {

    function config() {
        super.config()
        table(name="test")
        afterInitialization("setFindMe")
    }
    
    // ...


    private function setDependencies() {
        variables.dependency = variables.diService.getBean("TestDependency")
    }
    
    // ...

}

Now the base model handles the loading of the DependencyInjectionService, and calls a setDependencies method. Its own method does nothing, but if a subclass has an override of it, then that will run instead.

I will quickly tag that lot before I continue. 0.4.

But…

0.5 Dealing with the hard-coded DependencyInjectionService initialisation

The second problem is way more significant. Model is creating and initialising that DependencyInjectionService object every time a model object is created. That's not great. All that stuff only needs to be done once for the life of the application. I need to do that bit onApplicationStart (or whatever approximation of that CFWheels supports), and then I need to somehow expose the resultant object in Model.cfc. A crap way of doing it would be to just stick it in application.dependencyInjectionService and have Model look for that. But that's a bit "global variable" for my liking. I wonder if CFWheels has an object cache that it intrinsically passes around the place, and exposes to its inner workings. I sound vague because I had pre-baked all the code up to where I am now a week or two ago, and it was not until I was writing this article I went "oh well that is shit, I can't be having that stuff in there". And I don't currently know the answer.

Let's take the red-green-refactor route, and at least get the initialisation out of Model, and into the application lifecycle.

Ugh. Looking through the CFWheels codebase is not for the faint-hearted. Unfortunately the "architecture" of CFWheels is such that it's about one million (give or take) individual functions, and no real sense of cohesion to anything other than a set of functions might be in the same .cfm (yes: .cfm file :-| ), which then gets arbitrarily included all over the place. If I dump out the variables scope of my Test model class, it has 291 functions. Sigh.

There's a bunch of functions possibly relating to caching, but there's no Cache class or CacheService or anything like that... there's just some functions that act upon a bunch of application-scoped variable that are not connected in any way other than having the word "cache" in them. I feel like I have fallen back through time to the days of CF4.5. Ah well.

I'll chance my arm creating my DependencyInjectionService object in my onApplicationStart handler, use the $addToCache function to maybe put it into a cache… and then pull it back out in Model. Please hold.

[about an hour passes. It was mostly swearing]

Okey doke, so first things first: obviously there's a new test:

describe("Tests for onApplicationStart", () => {
    it("puts an instance of DependencyInjectionService into cache", () => {
        diService = $getFromCache("diService")

        expect(diService).toBeInstanceOf("services.DependencyInjectionService")
    })
})

The implementation for this was annoying. I could not use the onApplicationStart handler in my own Application.cfc because CFWheels steamrolls it with its own one. Rather than using the CFML lifecycle event handlers the way they were intended, and also using inheritance when an application and an application framework might have their own work to do, CFWheels just makes you write its handler methods into your Application.cfc. This sounds ridiculous, but this is what CFWheels does in the application's own Application.cfc. I'm going to follow-up on this stupidity in a separate article, perhaps. But suffice it to say that instead of using my onApplicationStart method, I had to do it the CFWheels way. which is … wait for it… to put the code in events/onapplicationstart.cfm. Yuh. Another .cfm file. Oh well. Anyway, here it is:

<cfscript>
// Place code here that should be executed on the "onApplicationStart" event.
import services.DependencyInjectionService

setDependencyInjectionService()

private void function setDependencyInjectionService() {
    diService = new DependencyInjectionService()
    $addToCache("diService", diService)
}
</cfscript>

And then in models/Model.cfc I make this adjustment:

private function loadIocContainer() {
    variables.diService = new DependencyInjectionService()
    variables.diService = $getFromCache("diService")
    setDependencies()
}

And then…

I consider that a qualified sucessful exercise in "implementing dependency injection in a CFWheels web site". I mean I shouldn't have to hand-crank stuff like this. This article should not need to be written. This is something that any framework still in use in 2022 should do out of the box. But… well… here we are. It's all a wee bit Heath Robinson, but I don't think it's so awful that it's something one oughtn't do.

And now I'm gonna push the code up to github (0.5), press "send" on this, and go pretend none of it ever happened.

Righto.

--
Adam