Tuesday 30 March 2021

Laravel: circumventing insecure default guidance about needing to have writeable files in the application code directory

G'day:

Sorry, that was a bit of a mouthful. I'm having to look at Laravel for [reasons I won't go into here], and have recently waded through setting up a default web app with it.

I won't go into the installation process as I faffed around a fair bit and didn't really have any useful findings other than Laravel's whole approach to things seems a bit bloated. I mean it's just an MVC framework for webapps. Why does it need a special application to even install? Blimey. But anyway.

The thing I have found most horrifying about Laravel so far is this error message I received in the browser when I first cranked-up the application:

UnexpectedValueException
The stream or file "/usr/share/laravelExampleApp/storage/logs/laravel.log" could not be opened in append mode: Failed to open stream: Permission denied

Just to be clear: /usr/share/laravelExampleApp/ is my application directory. IE: where all the code is. Why the hell is Laravel trying to write to a log file in there? Nothing should ever be writing to an application directory.

Initially I thought I'd messed something up, but after a bunch of googling and reading stuff on Stack Overflow and issues in Github, I discovered that Laravel actually did this by "design". They actually expect the application to write temporary files to your application directory. They have this storage directory subtree in the root of the application:

adam@DESKTOP-QV1A45U:/mnt/c/src/laravel-example-app$ tree storage
storage
├── app
│   └── public
├── framework
│   ├── cache
│   │   └── data
│   ├── sessions
│   ├── testing
│   └── views
└── logs

9 directories, 0 files
adam@DESKTOP-QV1A45U:/mnt/c/src/laravel-example-app$

And it's for writing logs, cached shite, and other temporary files. What?

In addition to this, there's a second subdirectory structure that also needs to be writable, within one of the code subdirectories: <appDir>/bootstrap/cache. the <appDir>/bootstrap directory has application code in it, btw.

Well: we're not having that. Temporary files can go in the /var/tmp directory where they belong.

Now this is why I'm writing this article: it took me quite a while to work out how to change this: as far as I can tell none of it is documented, one just needs to wade through the code. So just in case someone (/ everyone ~) else has this issue, here's what to do.

Dealing with the bootstrap/cache stuff is super easy; hardly even an inconvenience. There's a bunch of environment variables Laravel pays attention to that define where to write temp files that by default go into bootstrap/cache. I've just chucked this lot into my .env file:

# The APP_STORAGE_PATH value below is for reference only.
# It can't be set here because it's needed in bootstrap/app.php before this file is loaded
# It needs to be set in the environment
APP_STORAGE_PATH=/var/tmp/storage

APP_SERVICES_CACHE=${APP_STORAGE_PATH}/bootstrap/cache/services.php
APP_PACKAGES_CACHE=${APP_STORAGE_PATH}/bootstrap/cache/packages.php
APP_CONFIG_CACHE=${APP_STORAGE_PATH}/bootstrap/cache/config.php
APP_ROUTES_CACHE=${APP_STORAGE_PATH}/bootstrap/cache/routes.php
APP_EVENTS_CACHE=${APP_STORAGE_PATH}/bootstrap/cache/events.php

(.env itself is not in source control, but it's based on this: .env.example).

Each of those environment variables define where to write their relevant file.

That comment about APP_STORAGE_PATH is part of the solution for relocating the storage directory. I've tried to follow the same approach as relocating the bootstrap/cache files, and had partial success, but unfortunately Laravel needs to know where that directory is before it's read its .env file. Of course it does. However the fix is a one-liner in bootstrap/app.php

$app = new Illuminate\Foundation\Application(
    $_ENV['APP_BASE_PATH'] ?? dirname(__DIR__)
);

$app->useStoragePath( env( 'APP_STORAGE_PATH', base_path() . '/storage' ) );

Having done that, Laravel is not tempted to try to write files to my application directory any more. Now I know that I should try to segregate out caching stuff from logging stuff from session stuff from random-other-shite stuff that Laravel mungs together in that storage directory, and I might get to that later if I need to care more about it; but for now at least it's out of the application directory.

Anyway. That's that. I needed something written down to point someone to, and this is it, and this is sufficient.

Righto.

--
Adam

Saturday 27 March 2021

How to waste one's morning working around the shitfuckery that is the ever-shifting sands of Docker, Node, VueJS and Vue Test Utils

Sigh.

In case you were wondering how my Saturday morning and early afternoon has gone, it is summed up by this README.md I just had to add to the project for my ongoing blog article series (VueJs/Symfony/Docker/TDD series):

Warning

2021-03-27

Note that in package.json both vue and @vue/compiler-sfc and currently version-limited to exactly 3.0.6. I realise this is less than ideal.

This is because something about later versions (current is 3.0.8 at time of writing) screws up how Vue Test Utils mounts <select> options when the <select> has a v-model (or possibly because the v-model is asynchronously loadedfrom a remote call? Dunno).

This results in "Cannot read property 'length' of undefined" in setSelected in node_modules/@vue/runtime-dom/dist/runtime-dom.esm-bundler.jsruntime-dom.esm-bundler.js when it evaluates el.options.length, when the component is first shallowMount-ed. Around line 2021 in the version of that file I'm looking at.

This is because the options array apparently is not even defined if it's empty (which seems like a bug to me).

That specific code is the same in 3.0.6 as it is in 3.0.8, so it must be some upstream code that's messing it up. I CBA digging into it further.

Note that the component works A-OK in a browser environment, so am guessing Vue itself is not doing anything wrong; it's how the test utils are mounting the component. Although I guess if Vue has changed things so that that options array might not now exist, then that's not so cool.

I'll give it a month or so and try with a more liberal version constraint, to see if it starts working again.

All this happened because I happened to rebuild my NodeJS container from scratch before starting on my next blog article this morning. Dumb-arse.

Oh I also suddendly needed to add global.SVGElement = Element; to the top of my test spec because reasons. Thanks to Testing with react-md for being the only resource I could find explaining the "ReferenceError: SVGElement is not defined" my tests started throwing when I ran them after the rebuild. Fortunately now I'm back on Vue 3.0.6 that error has gone, so I backed-out that change.

All this was after I spent half a day yesterday wondering why npm-installing Vue CLI globally in my Docker container suddenly started to freeze and chew up 50% of my PC's disk and memory resources. Building the same container on a different PC (but still with largely the same environment: Windows, WSL, same version of Docker etc) worked fine. I never worked that one out; I just stopped installing Vue CLI globally and problem went away. Except it started all this other bullshit.

So instead of another article today, I think Docker and VueJS can just get tae fuck. I'm gonna play computer games instead. Something with lots of bullets and explosions (OKOK, it'll be FallOut 4. It's the only shooting game I play).

Fuck sake.

--
Adam

Wednesday 17 March 2021

Symfony and TDD: adding server-side POST request validation

G'day:

In the previous article ("Vue.js and TDD: adding client-side form field validation") I added the small amount of client-side validation this data entry form I'm working on needs (full mise-en-scène can be caught-up-with via a bunch of articles tagged with "VueJs/Symfony/Docker/TDD series"). In this article I'm returning to the back-end to implement validation for the data the front-end will be passing to a web service endpoint.

Even to start with I taught myself a TDD lesson in "let's not get ahead of ourselves". I leapt in and started writing test case names (note the plural), and ran the incomplete tests to show myself how clever I was:

Unit tests of WorkshopsController::doPost
  It returns a 400 status if fullName is greater than 100 characters
  It returns a 400 status if phoneNumber is greater than 50 characters
  It returns a 400 status if emailAddress is greater than 320 characters
  It returns a 400 status if password is greater than 255 characters

And then I thought "OK I better add in the controller method for that, so I can get cracking with the coding". And then I thought "erm… actually a route would be nice. And… that's not even the right controller I'm mentioning there". And I'm not sure I should be mentioning a controller in a test case anyhow. The POST requests aren't for /workshops/ (hence WorkshopsController): POSTing to /workshops/ would be for adding a new Workshop. What I'm wanting to do here is to add a new WorkshopRegistration. So: /workshop-registrations/ and WorkshopRegistrationsController. But now that I'm thinking about it, I think mentioning WorkshopRegistrationsController is a bit implementation-specific and not relevant to the user of the test case. What they're doing is POSTing to /workshop-registrations/, and getting the results they want. How that's implemented is irrelevant. So - lesson learned about getting ahead of myself - the first case is "it will accept POST requests on /workshop-registrations/, and return a 201 CREATED if successful". Let's just write that test first. Let's only write one case, one test, and one implementation at a time in this exercise, eh?

I had not committed any of that code above, so I just reverted it.


It will accept POST requests on /workshop-registrations/, and return a 201 CREATED if successful

This is straight forward, and we already have a very similar test in tests\functional\Controller\WorkshopsControllerTest for the /workshops/ route. Here's the one for the new requirement:

namespace adamCameron\fullStackExercise\tests\functional\Controller;

use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\Dotenv\Dotenv;
use Symfony\Component\HttpFoundation\Response;

/** @testdox Functional tests of /workshop-registrations/ endpoint */
class WorkshopRegistrationsControllerTest extends WebTestCase
{
    private KernelBrowser $client;

    public static function setUpBeforeClass(): void
    {
        $dotenv = new Dotenv();
        $dotenv->load(dirname(__DIR__, 3) . "/.env.test");
    }

    protected function setUp(): void
    {
        $this->client = static::createClient(['debug' => false]);
    }

    /**
     * @testdox it needs to return a 201-CREATED status for successful POST requests
     * @covers \adamCameron\fullStackExercise\Controller\WorkshopRegistrationsController
     */
    public function testDoPostReturns201(): void
    {
        $this->client->request('POST', '/workshop-registrations/');

        $this->assertEquals(Response::HTTP_CREATED, $this->client->getResponse()->getStatusCode());
    }
}

(I've been slightly naughty and already introduced a slight refactoring there to have the setUpBeforeClass and setUp methods, based on the earlier work demonstrating we'll need those. Also cos I copy and pasted the whole WorkshopsControllerTestclass and just changed some bits). Running that fails for obvious reasons (the code it is calling not existing yet being the main one), so we're good to implement that code now.

namespace adamCameron\fullStackExercise\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Response;

class WorkshopRegistrationsController extends AbstractController
{

    public function doPost() : JsonResponse
    {
        return new JsonResponse(null, Response::HTTP_CREATED);
    }
}

And the test passes. Now I can start thinking about other cases that involve behaviour of hitting that end point.

It must receive a JSON object with fullName, phoneNumber, workshopsToAttend, emailAddress and password properties, otherwise will return a 400-BAD-REQUEST status

Before we start worrying about length-checks and other validation rules; let's just ease into things: we receive an object with the correct schema.

One interesting thing that popped up when I started doing the test case for this is that the test case description is over the length that PHPCS is happy with, and I was getting a warning:

root@12a5e50e1652:/usr/share/fullstackExercise# composer phpcs
> vendor/bin/phpcs --standard=phpcs.xml.dist

FILE: tests/functional/Controller/WorkshopRegistrationsControllerTest.php
-------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------
48 | WARNING | Line exceeds 120 characters; contains 177 characters
-------------------------------------------------------------------------

I tried to work out how to split that description over multiple lines, but seems there's a "shortfall" in PHPUnit that prohibits dealing with this: `@testdox` annotations should be allowed to be multiline. #4511. This was closed without being considered with a rather subpar comment of "I do not think so.", which is disappointing. Obviously me being me: I put my oar in there. However as it happens once I implemented the test logic, the length came back within 120chars, so PHPCS was happy. Here's the test code:

/**
 * @testdox It must receive a JSON object with a $property property, otherwise will return a 400-BAD-REQUEST status
 * @dataProvider provideSchemaPropertyCheckTestCases
 * @covers \adamCameron\fullStackExercise\Controller\WorkshopRegistrationsController
 */
public function testRequiredPropertiesArePresentInBody($property)
{
    $testBody = $this->getValidObjectForTestRequest();
    unset($testBody->$property);
    $this->client->request(
        'POST',
        '/workshop-registrations/',
        [],
        [],
        [],
        json_encode($testBody)
    );

    $this->assertEquals(Response::HTTP_BAD_REQUEST, $this->client->getResponse()->getStatusCode());
}

public function provideSchemaPropertyCheckTestCases()
{
    return [
        ['property' => 'fullName'],
        ['property' => 'phoneNumber'],
        ['property' => 'workshopsToAttend'],
        ['property' => 'emailAddress'],
        ['property' => 'password']
    ];
}

private function getValidObjectForTestRequest()
{
    return (object) [
        'fullName' => static::UNTESTED_VALUE,
        'phoneNumber' => static::UNTESTED_VALUE,
        'workshopsToAttend' => static::UNTESTED_VALUE,
        'emailAddress' => static::UNTESTED_VALUE,
        'password' => static::UNTESTED_VALUE
    ];
}

(note that - offscreen - I have also updated the previous test to use that getValidObjectForTestRequest method).

The implementation for this will be more complicated than that for the first test.

First: an aside. As I am not a lunatic, I am not going to hand-crank my own validation. I am going to use Symfony Validation. Validation can be tricky, and it's critical so it's not a good idea to hand-crank it. Use an established library. My axiom for stuff like this is to consider "what I am in the business of?" In this context I am in the business of registering workshops. I am not in the business of writing validation libraries. Therefore: I should not be engaging in writing validation libraries; I'll leave that to someone who is in the business of writing validation libraries.

Anyway… here's the implementation code:

class WorkshopRegistrationsController extends AbstractController
{

    private WorkshopRegistrationValidationService $validator;

    public function __construct(WorkshopRegistrationValidationService $validator)
    {
        $this->validator = $validator;
    }

    public function doPost(Request $request) : JsonResponse
    {
        try {
            $this->validator->validate($request);
        } catch (WorkshopRegistrationValidationException $e) {
            return new JsonResponse(null, Response::HTTP_BAD_REQUEST);
        }
        return new JsonResponse(null, Response::HTTP_CREATED);
    }
}

Note we've added a dependency of a WorkshopRegistrationValidationService there, which we use to validate the incoming request. If it throws a WorkshopRegistrationValidationException then we had some issues, so we return a 400.

WorkshopRegistrationValidationException is just a placeholder at the moment:

class WorkshopRegistrationValidationException extends DomainException
{
}

WorkshopRegistrationValidationService is more interesting:

class WorkshopRegistrationValidationService
{

    private ValidatorInterface $validator;

    public function __construct(ValidatorInterface $validator)
    {
        $this->validator = $validator;
    }

    public function validate(Request $request): void
    {
        $content = $request->getContent();
        $values = json_decode($content, true);
        $constraints = $this->getConstraints();

        $violations = $this->validator->validate($values, $constraints);

        if (count($violations) > 0) {
            throw new WorkshopRegistrationValidationException();
        }
    }

    private function getConstraints()
    {
        return new Assert\Collection([
            'fullName' => new Assert\NotBlank(),
            'phoneNumber' => new Assert\NotBlank(),
            'workshopsToAttend' => new Assert\NotBlank(),
            'emailAddress' => new Assert\NotBlank(),
            'password' => new Assert\NotBlank()
        ]);
    }
}

Notes:

  • It takes a ValidatorInterface, which will be a Symfony Validator implementation.
  • Its validate method passes the JSON from the request body to Symfony's validate method, along with the constrainst to validate against.
  • If that call returns any constraint violations, we throw that exception we saw above.
  • And it contains the constraints we are using for validation.

It's not doing much yet, and doing nothing with the values except validate them, but that's all we need to do for the current test case.

To get the Symfony Validator object into the WorkshopRegistrationValidationService we need to use a factory again, like we did in an earlier article for the WorkshopCollection (this is in services.yaml)

adamCameron\fullStackExercise\Factory\WorkshopCollectionFactory: ~
adamCameron\fullStackExercise\Model\WorkshopCollection:
    factory: ['@adamCameron\fullStackExercise\Factory\WorkshopCollectionFactory', 'getWorkshopCollection']

adamCameron\fullStackExercise\Factory\ValidatorFactory: ~
Symfony\Component\Validator\Validator\ValidatorInterface:
    factory: ['@adamCameron\fullStackExercise\Factory\ValidatorFactory', 'getValidator']

The factory itself is simple. We just need it to make the dependency injection work with that method chaining we need to do. Basically some complexity Symfony has created for itself. And by association: me :-(

class ValidatorFactory
{

    public function getValidator() : ValidatorInterface
    {
        return Validation::createValidatorBuilder()->getValidator();
    }
}

And that sorts out the tests.

It returns details of the validation failures in the body of the 400 response

Test:

/**
 * @testdox It returns details of the validation failures in the body of the 400 response
 * @covers ::doPost
 */
public function testValidationFailsAreReturned()
{
    $testBody = $this->getValidObjectForTestRequest();
    unset($testBody->fullName);
    unset($testBody->password);

    $this->client->request('POST', '/workshop-registrations/', [], [], [], json_encode($testBody));
    $response = $this->client->getResponse();
    $this->assertEquals(Response::HTTP_BAD_REQUEST, $response->getStatusCode());
    $content = json_decode($response->getContent());

    $this->assertObjectHasAttribute('errors', $content);
    $this->assertEquals(
        (object) [
            'errors' => [
                (object) ['field' => '[fullName]', 'message' => 'This field is missing.'],
                (object) ['field' => '[password]', 'message' => 'This field is missing.']
            ]
        ],
        $content
    );
}

This is possibly a wee bit fragile because I'm relying on the order Symfony is validating things, but this'll works now, so it's fine. I have to admit my first pass of the test of this was using completely wrong values in there - by design - so I could see what the right values in the failure was once I did my implementation, and then fixed the test to match the values. IE I had no idea that Symfony returned This field is missing. as the constraint violation message, so my assertion initially checked for 'NOT_THE_CORRECT_MESSAGE'.

Implementation:

// WorkshopRegistrationsController
//...
return new JsonResponse(null, Response::HTTP_BAD_REQUEST);
return new JsonResponse(['errors' => $e->getErrors()], Response::HTTP_BAD_REQUEST);
// WorkshopRegistrationValidationService
// ...
if (count($violations) > 0) {
    throw new WorkshopRegistrationValidationException();
    $errors = array_map(function ($violation) {
        return [
            'field' => $violation->getPropertyPath(),
            'message' => $violation->getMessage()
        ];
    }, $violations->getIterator()->getArrayCopy());
    throw new WorkshopRegistrationValidationException($errors);
}
class WorkshopRegistrationValidationException extends DomainException
{
    public function __construct($errors)
    {
        $this->errors = $errors;
    }

    public function getErrors()
    {
        return $this->errors;
    }
}

That's all pretty self-explanatory.

It [checks a bunch of Other Stuff]

I started to do this on a case by case basis, but everything else is just configuration of that getConstraints method of WorkshopRegistrationValidationService that I mentioned a bit further up. Instead of doing a heading-line for each, I'll list the rest of the test cases I've added, and show you the code for the whole lot. All the tests are very very similar, and don't need much specific focus. So I added these cases:

  • It should not accept a fullName with length greater than 100 characters
  • It should not accept a phoneNumber with length greater than 50 characters
  • It should not accept a emailAddress with length greater than 320 characters
  • It should not accept an emailAddress with length less than 3 characters
  • It cannot·have·fewer·than·8·characters for password
  • It can·have·exactly·8·characters for password
  • It can·have·more·than·8·characters for password
  • It must·have·at·least·one·lowercase·letter for password
  • It must·have·at·least·one·uppercase·letter for password
  • It must·have·at·least·one·digit for password
  • It must·have·at·least·one·non-alphanumeric·character for password
  • It cannot have embedded XSS risk for fullName
  • It cannot have embedded XSS risk for phoneNumber
  • It cannot have embedded XSS risk for emailAddress

For the sake of clarity, I'll dump out the final constrains first up here, as it'll make it easier to see what the tests are doing. But I did make sure to write the tests before adding any new constraints in!

private function getConstraints()
{
    return new Assert\Collection([
        'fields' => [
            'fullName' => [
                new Assert\Length(['min' => 1, 'max' => 100]),
                new ContainsXssRiskConstraint()
            ],
            'phoneNumber' => [
                new Assert\Length(['min' => 1, 'max' => 50]),
                new ContainsXssRiskConstraint()
            ],
            'workshopsToAttend' => [
                new Assert\NotBlank(),
                new Assert\All([
                    new Assert\Type('int')
                ])
            ],
            'emailAddress' => [
                new Assert\Length(['min' => 3, 'max' => 320]),
                new ContainsXssRiskConstraint()
            ],
            'password' => [
                new Assert\Regex([
                    'pattern' => '/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*\W)(?:.){8,}$/',
                    'message' => 'Failed complexity validation'
                ])
            ]
        ],
        'allowMissingFields' => false,
        'allowExtraFields' => true
    ]);
}

That's all pretty straight forward I think? I've had to create a custom constraint in there, the code for which is just this lot:

class ContainsXssRiskConstraint extends Constraint
{
    public $message = 'The string contains illegal content';
}

class ContainsXssRiskConstraintValidator extends ConstraintValidator
{
    public function validate($value, Constraint $constraint)
    {
        if (htmlspecialchars($value) !== $value) {
            $this->context->buildViolation($constraint->message)
                ->setParameter('{{ string }}', $value)
                ->addViolation();
        }
    }
}

(I've stripped some Symfony boilerplate out of the second one, but the if statement is the bit that does the validation). For the constraint array and these two classes, I just read along with the Symfony Validation docs. They've made it very easy.

Now for the test code.

/**
 * @testdox It should not accept a $property with length greater than $length characters
 * @dataProvider provideStringLengthCheckTestCases
 */
public function testStringLengthValidation($property, $length)
{
    $testBody = $this->getValidObjectForTestRequest();
    $testBody->$property = str_repeat('X', $length);

    $this->client->request('POST', '/workshop-registrations/', [], [], [], json_encode($testBody));
    $response = $this->client->getResponse();
    $this->assertEquals(Response::HTTP_CREATED, $response->getStatusCode());

    $testBody = $this->getValidObjectForTestRequest();
    $testBody->$property = str_repeat('X', $length + 1);

    $this->client->request('POST', '/workshop-registrations/', [], [], [], json_encode($testBody));
    $response = $this->client->getResponse();
    $this->assertEquals(Response::HTTP_BAD_REQUEST, $response->getStatusCode());
}

public function provideStringLengthCheckTestCases()
{
    return [
        ['property' => 'fullName', 'length' => 100],
        ['property' => 'phoneNumber', 'length' => 50],
        ['property' => 'emailAddress', 'length' => 320]
    ];
}

Here I'm cheating slightly and testing two things: that the maximum length is OK, and that over that is not OK. You might note that I am not length-checking the password here. We're not gonna be putting the clear-text password into the DB, we'll be hashing it and the hashes are always a set length, so we don't need to care how long the original password is.

It's the same deal with this next one which is just testing that the email address minimum is 3:

/**
 * @testdox It should not accept an emailAddress with length less than 3 characters
 */
public function testEmailMinLengthValidation()
{
    $testBody = $this->getValidObjectForTestRequest();
    $testBody->emailAddress = 'a@b';

    $this->client->request('POST', '/workshop-registrations/', [], [], [], json_encode($testBody));
    $response = $this->client->getResponse();
    $this->assertEquals(Response::HTTP_CREATED, $response->getStatusCode());

    $testBody = $this->getValidObjectForTestRequest();
    $testBody->emailAddress = 'a@';

    $this->client->request('POST', '/workshop-registrations/', [], [], [], json_encode($testBody));
    $response = $this->client->getResponse();
    $this->assertEquals(Response::HTTP_BAD_REQUEST, $response->getStatusCode());
}

Next I'm testing the password complexity (the cases are lifted straight from the client-side validation code, in frontend/test/unit/workshopRegistration.spec.js):

/**
 * @testdox It $_dataName for password
 * @dataProvider providePasswordTestCases
 */
public function testPasswordValidation($testValue, $expectedErrors)
{
    $testBody = $this->getValidObjectForTestRequest();
    $testBody->password = $testValue;

    $this->client->request('POST', '/workshop-registrations/', [], [], [], json_encode($testBody));
    $response = $this->client->getResponse();

    if (!count($expectedErrors)) {
        $this->assertEquals(Response::HTTP_CREATED, $response->getStatusCode());
        return;
    }
    $this->assertEquals(Response::HTTP_BAD_REQUEST, $response->getStatusCode());
    $content = json_decode($response->getContent());

    $this->assertEquals($expectedErrors, $content->errors);
}

public function providePasswordTestCases()
{
    $expectedErrors = [(object) ['field' => '[password]', 'message' => 'Failed complexity validation']];
    return [
        'cannot have fewer than 8 characters' => [
            'password' => 'Aa1!567',
            'expectedErrors' => $expectedErrors
        ],
        'can have exactly 8 characters' => [
            'password' => 'Aa1!5678',
            'expectedErrors' => []
        ],
        'can have more than 8 characters' => [
            'password' => 'Aa1!56789',
            'expectedErrors' => []
        ],
        'must have at least one lowercase letter' => [
            'password' => 'A_1!56789',
            'expectedErrors' => $expectedErrors
        ],
        'must have at least one uppercase letter' => [
            'password' => '_a1!56789',
            'expectedErrors' => $expectedErrors
        ],
        'must have at least one digit' => [
            'password' => 'Aa_!efghi',
            'expectedErrors' => $expectedErrors
        ],
        'must have at least one non-alphanumeric character' => [
            'password' => 'Aa1x56789',
            'expectedErrors' => $expectedErrors
        ]
    ];
}

Notice how I'm using the test case label in the @testDox annotation value. That's quite cool.

Finally, and partially as an excuse to create that custom constraint, I just check if the text fields have any nasty surprises in them:

/**
 * @testdox It cannot have embedded XSS risk for $property
 * @testWith    ["fullName"]
 *              ["phoneNumber"]
 *              ["emailAddress"]
 */
public function testXssInTextFieldValidation($property)
{
    $testBody = $this->getValidObjectForTestRequest();
    $testBody->$property = '<script>hijackTheirSession()</script>';

    $this->client->request('POST', '/workshop-registrations/', [], [], [], json_encode($testBody));
    $response = $this->client->getResponse();
    $this->assertEquals(Response::HTTP_BAD_REQUEST, $response->getStatusCode());

    $content = json_decode($response->getContent());

    $this->assertEquals(
        [
            (object) ['field' => "[$property]", 'message' => 'The string contains illegal content']
        ],
        $content->errors
    );
}

(Note I'm using a different tactic with the data provider here too. And this is still supported by the @testDox).

The other change I needed to make was to update the wee method I'm using to provide that initial getValidObjectForTestRequest at the beginning of all the tests:

class WorkshopRegistrationsControllerTest extends WebTestCase
{
    // ...

    private const UNTESTED_VALUE = 'UNTESTED_VALUE';
    private const UNTESTED_INT_VALUE = -1;
    private const UNTESTED_PASSWORD_VALUE = 'aA1!1234';

    // ...

    private function getValidObjectForTestRequest()
    {
        return (object) [
            'fullName' => static::UNTESTED_VALUE,
            'phoneNumber' => static::UNTESTED_VALUE,
            'workshopsToAttend' => [static::UNTESTED_INT_VALUE],
            'emailAddress' => static::UNTESTED_VALUE,
            'password' => static::UNTESTED_PASSWORD_VALUE
        ];
    }
}

This is just so all the earlier tests keep passing as I toughen up the validation constraints.

Now I can run all the tests, and they all pass:

> vendor/bin/phpunit --testdox 'tests/functional/Controller/WorkshopRegistrationsControllerTest.php'
PHPUnit 9.5.2 by Sebastian Bergmann and contributors.

Functional tests of /workshop-registrations/ endpoint
it needs to return a 201-CREATED status for successful POST requests
It must receive a JSON object with a fullName property, otherwise will return a 400-BAD-REQUEST status
It must receive a JSON object with a phoneNumber property, otherwise will return a 400-BAD-REQUEST status
It must receive a JSON object with a workshopsToAttend property, otherwise will return a 400-BAD-REQUEST status
It must receive a JSON object with a emailAddress property, otherwise will return a 400-BAD-REQUEST status
It must receive a JSON object with a password property, otherwise will return a 400-BAD-REQUEST status
It returns details of the validation failures in the body of the 400 response
It should not accept a fullName with length greater than 100 characters
It should not accept a phoneNumber with length greater than 50 characters
It should not accept a emailAddress with length greater than 320 characters
It should not accept an emailAddress with length less than 3 characters
It cannot·have·fewer·than·8·characters for password
It can·have·exactly·8·characters for password
It can·have·more·than·8·characters for password
It must·have·at·least·one·lowercase·letter for password
It must·have·at·least·one·uppercase·letter for password
It must·have·at·least·one·digit for password
It must·have·at·least·one·non-alphanumeric·character for password
It cannot have embedded XSS risk for fullName
It cannot have embedded XSS risk for phoneNumber
It cannot have embedded XSS risk for emailAddress

Time: 00:00.419, Memory: 22.00 MB

OK (21 tests, 35 assertions)

Generating code coverage report in HTML format ... done [00:00.929]
root@12a5e50e1652:/usr/share/fullstackExercise#

Excellent, eh? Um. Kinda.

It doesn't frickin test everything it should

That's not another test case, it's a statement of fact. The next thing I did was to triumphantly go to my browser's REST-testing app (Talend API Tester - Free Edition),and hit the endpoint with a POST request. To start with I didn't send a body, cos I was gonna build it up key by key. But… erm… I got this:

Huh?

I'll spare you the details of my hour of googling and frustration. It turns out that if you give the Symfony validator a NULL, then it skips the validation. By design. Unfrickinbelievable.

I needed another few test cases then:

/**
 * @testdox it will not accept $_dataName for the body
 * @covers ::doPost
 * @dataProvider provideUnexpectedBodyTestCases
 */
public function testDoPostUnexpectedBodyErrorsOut($body): void
{
    $this->client->request('POST', '/workshop-registrations/', [], [], [], $body);
    $response = $this->client->getResponse();

    $this->assertEquals(Response::HTTP_BAD_REQUEST, $response->getStatusCode());

    $content = json_decode($response->getContent());
    $this->assertObjectHasAttribute('errors', $content);
    $this->assertEquals(
        (object) [
            'errors' => [
                (object) ['field' => '[fullName]', 'message' => 'This field is missing.'],
                (object) ['field' => '[phoneNumber]', 'message' => 'This field is missing.'],
                (object) ['field' => '[workshopsToAttend]', 'message' => 'This field is missing.'],
                (object) ['field' => '[emailAddress]', 'message' => 'This field is missing.'],
                (object) ['field' => '[password]', 'message' => 'This field is missing.']
            ]
        ],
        $content
    );
}

public function provideUnexpectedBodyTestCases()
{
    return [
        'nothing' => ['body' => json_encode(null)],
        'empty object' => ['body' => json_encode((object)[])],
        'not JSON' => ['body' => 'NOT_JSON']
    ];
}

(I noticed that third case whilst testing too: json_decode silently fails if the input you give it isn't JSON, and it just returns null).

The fix was simple (in backend/src/Service/WorkshopRegistrationValidationService.php):

public function validate(Request $request)
{
    $content = $request->getContent();
    $values = json_decode($content, true) ?? []; // Symfony won't validate null (apparently by "design")
    $constraints = $this->getConstraints();
    $violations = $this->validator->validate($values, $constraints);

    if (count($violations) > 0) {
        $errors = array_map(function ($violation) {
            return [
                'field' => $violation->getPropertyPath(),
                'message' => $violation->getMessage()
            ];
        }, $violations->getIterator()->getArrayCopy());
        throw new WorkshopRegistrationValidationException($errors);
    }
}

It must allow underscore as the one non-alphanumeric character for password

I also notice this one when testing by hand. I had a bug in my regex pattern for the password in that it did not consider underscore to be punctuation. I appended a quick test case:

'must have at least one non-alphanumeric character' => [
    'password' => 'Aa1x56789',
    'expectedErrors' => $expectedErrors
],
'must allow underscore as the one non-alphanumeric character' => [
    'password' => 'Aa1_56789',
    'expectedErrors' => []
]

The fix was dead easy:

'password' => [
    new Assert\Regex([
        'pattern' => '/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*\W)(?:.){8,}$/',
        'pattern' => '/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[\W_])(?:.){8,}$/',
        'message' => 'Failed complexity validation'
    ])
]

Stupid me went "yeah \W (non-word character) covers all punctuation". But underscore is a word character. For some reason. I knew this, I just didn't think. Anyway, fixed now. It does just go to show that with the best will in the world, one will never think of all test cases for a given situation. I'm glad I found these two though. Also glad I had all the other test cases in place so when I fixed these two I could be confident they didn't break any of the other case.

OK so barring other bugs I didn't spot, I'm done with the Symfony side of the validation now. I need to go back to the front end and deal with situations when the server comes back with a 400. Currently it assumes success. And then after I do that work, I can start thinking about pushing stuff into the database. Hopefully. So I guess two more articles to go…

Righto.

--
Adam

Friday 12 March 2021

Vue.js and TDD: adding client-side form field validation

G'day:

In the previous article ("TDDing the reading of data from a web service to populate elements of a Vue JS component"), I started connecting my front-end form to the back-end web service that does its data-handling. The first step was easy: just fetching some data the form needed to display a multi-select:

The next step for the back-end will be to receive the form-field values, but before we do that we need to ensure we're doing a superficial level of value validation on the front-end. I say "superficial" because this sort of validation is only there to guide the user, not prevent any nefarious activity: any attempt to prevent nefarious activity on the front-end is too easy to circumvent by the baddy, so it's not worth sinking too much time into. The reallyreally validation needs to be done on the back-end, and that will be done in the next article.

The first TDD step is to identify a test case. I'm gonna cheat a bit here and list the cases I'm gonna address once, here. This is kinda getting ahead of myself, but it makes things clearer for the article. I'll only be addressing one of them at a time though. One thing to note with TDD: don't try to compile an exhaustive list of test cases first, then work through them: that'll just delay you getting on with it, and there's no need to identify every case before you start anyhow: sometimes that can seem like a daunting task in and of itself (not so much with this exercise, granted, as it's so trivial). Identify a case. Test it. Implement it. Identify the next case. Test it. Implement it. Maybe see if there's any refactoring that is bubbling-up; but probably wait until your third case for those refactoring opportunities to surface more clearly. Rinse and repeat. Eat that elephant one bite at a time.

Actually before I get on with the new validation cases, we've already got some covered:

  • should have a required text input for fullName, maxLength 100, and label 'Full name'
  • should have a required text input for phoneNumber, maxLength 50, and label 'Phone number'
  • should have a required text input for emailAddress, maxLength 320, and label 'Email address'
  • should have a required password input for password, maxLength 255, and label 'Password'
  • should have a required workshopsToAttend multiple-select box, with label 'Workshops to attend'
  • should list the workshop options fetched from the back-end
  • should have a button to submit the registration
  • should leave the submit button disabled until the form is filled
  • should disable the form and indicate data is processing when the form is submitted
  • should send the form values to WorkshopService.saveWorkshopRegistration when the form is submitted
  • should display the registration summary 'template' after the registration has been submitted
  • should display the summary values in the registration summary

We're at least validating requiredness and (max) string-length. And we're part way there with "should leave the submit button disabled until the form is filled", if we just update that to be suffixed with "… with valid data".

Now that I look at the form, I can only see one validation case we need to address: password strength: "it has a password that is at least 8 characters long, has at least one each of lowercase letter, uppercase letter, digit, and one other character not in those subsets (eg: punctuation etc)". One might think we need to validate that the email address is indeed an email address, but trying to get that 100% right is a mug's game, and anyhow there's no way in form field validation to check whether it's their email address - ie: they can receive email at it - anyhow. And we're not asking for an RFC-5322-compliant email address, we're asking for their email address. We need to trust that if they want to engage with us, they'll allow us to communicate with them. That said, I am at least going to change the input type to email for that field; first having updated the relevant test to expect it. This is a matter of semantics, not validation though. They can still type-in anything they like there.

For the password validation, I am going to test by example:

describe("Password-strength validation tests", () => {
    const examples = [
        {case: "cannot have fewer than 8 characters", password: "Aa1!567", valid: false},
        {case: "can have exactly 8 characters", password: "Aa1!5678", valid: true},
        {case: "can have more than 8 characters", password: "Aa1!56789", valid: true},
        {case: "must have at least one lowercase letter", password: "A_1!56789", valid: false},
        {case: "must have at least one uppercase letter", password: "_a1!56789", valid: false},
        {case: "must have at least one digit", password: "Aa_!efghi", valid: false},
        {case: "must have at least one non-alphanumeric character", password: "Aa1x56789", valid: false}
    ];

    examples.forEach((testCase) => {
        it(testCase.case, async () => {
            await populateForm(testCase.password);
            await flushPromises();

            let buttonDisabledAttribute = component.find("form.workshopRegistration button").attributes("disabled");
            let failureMessage = `${testCase.password} should be ${testCase.valid ? "valid" : "invalid"}`;

            testCase.valid
                ? expect(buttonDisabledAttribute, failureMessage).to.not.exist
                : expect(buttonDisabledAttribute, failureMessage).to.exist;
        });
    });
});

This:

  • specifies some examples (both valid and invalid examples);
  • loops over them;
  • populates the form with otherwise known-good values (so, all things being equal, the form can be submitted);
  • sets the password to be the test password;
  • tests whether the form can be submitted (which it only should be able to be if the password is valid).

I've just thought of another case: we better explain to the user why the form is only submittable when their password follows our rules. We'll deal with that once we get the functionality operational.

I've also refactored the populateForm function slightly:

let populateForm = async (password = VALID_PASSWORD) => {
    let form = component.find("form.workshopRegistration");
    form.find("input[name='fullName").setValue(TEST_INPUT_VALUE +"_fullName");
    form.find("input[name='phoneNumber").setValue(TEST_INPUT_VALUE +"_phoneNumber");
    form.find("input[name='emailAddress").setValue(TEST_INPUT_VALUE +"_emailAddress");
    form.find("input[name='password").setValue(password);
    form.find("select").setValue(TEST_SELECT_VALUE);
    await flushPromises();
};

Previously it was like this:

let populateForm = async () => {
    let form = component.find("form.workshopRegistration")
    form.findAll("input").forEach((input) => {
        let name = input.attributes("name");
        input.setValue(TEST_INPUT_VALUE + name);
    });
    form.find("select").setValue(TEST_SELECT_VALUE);

    await flushPromises();
};

I was being lazy and looping over the input fields setting dummy values, but now I wanted to set the password with a different sort of value I could not just set all the input elements; it was only the text and email ones. I could make my element selector more clever, or… I could just make the whole thing more explicit. Part of the reason for simplification was that I messed up the refactor of this three times trying to get the loop to hit only the correct inputs, so I decided just to keep it simple. Conveniently, this tweak also meant that the other tests would not start failing due to not being able to submit the form due to not having a valid password :-)

Cool so when I run my tests now, all the ones expecting the invalid passwords to prevent the form from being submitted now fail. The two cases with valid passwords do not fail, because they meet the current validation requirements already: they have length, and that's all we're validating. I'm currently thinking whether it's OK that we have passing test cases here already. I'm not sure. As long as they keep passing once we have the correct validation in, I guess that's all right.

The implementation here is easy, I just add a function that checks the password follows the rules, and then use that in the isFormUnready function instead of the length check:

computed : {
    isFormUnready: function () {
        let unready = this.formValues.fullName.length === 0
            || this.formValues.phoneNumber.length === 0
            || this.formValues.workshopsToAttend.length === 0
            || this.formValues.emailAddress.length === 0
            || this.formValues.password.length === 0;
            || !this.isPasswordValid;

        return unready;
    },
    // ...
    isPasswordValid: function () {
        const validPasswordPattern = new RegExp("^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)(?=.*\\W)(?:.){8,}$");
        return validPasswordPattern.test(this.formValues.password);
    }
}

Actually I had a sudden thought how I could feel more happy about those cases that are currently passing. I updated isPasswordValid to just return false for a moment, and verified those cases now failed. It seems pedantic and obvious, but it does demonstrate they're using the correct logic flow, and the behaviour is working. I could also have pushed the validation rule out to only accept say 10-character passwords, and note those cases failing, or something like that too.

Anyway, that implementation worked, and the tests all pass, and the UI behaviour also seems correct.

Next we need to address this other case I spotted: "it shows the user a message if the password value is not valid after they have typed one in". Although as I was writing the test, it occurred to me there are three cases to address here:

  • it does not show the user a message if the password value is valid after they have typed one in
  • it shows the user a message if the password value is not valid after they have typed one in
  • hides the previously displayed bad password message if the password is updated to be valid

The message will be implemented as an <aside> element, and the current password will be evaluated for validity when the keyup event fires. So it won't display until the user starts typing in the box; and will re-evaluate every keystroke. The code below shows the final rendition of these tests. Initially I had all the code inline for each of them, but each of them followed a similar sequence of the same two steps, so I extracted those:


it("does not show the user a message if the password value is valid after they have typed one in", async () => {
    confirmMessageVisibility(false);

    await enterAPassword(VALID_PASSWORD);
    confirmMessageVisibility(false);
});

it("shows the user a message if the password value is not valid after they have typed one in", async () => {
    confirmMessageVisibility(false);

    await enterAPassword(INVALID_PASSWORD);
    confirmMessageVisibility(true);
});

it("hides the previously displayed bad password message if the password is updated to be valid", async () => {
    confirmMessageVisibility(false);

    await enterAPassword(INVALID_PASSWORD);
    confirmMessageVisibility(true);

    await enterAPassword(VALID_PASSWORD);
    confirmMessageVisibility(false);
});

let enterAPassword = async function (password) {
    let passwordField = component.find("form.workshopRegistration input[type='password']");
    await passwordField.setValue(password);
    await passwordField.trigger("keyup");

    await flushPromises();
};

let confirmMessageVisibility = function (visible) {
    let messageField = component.find("form.workshopRegistration aside.passwordMessage");
    visible
        ? expect(messageField.exists(), "message should be visible").to.be.true
        : expect(messageField.exists(), "message should not be visible").to.be.false;
};
  • Each test confirms the message is not displayed to start with.
  • I mimic keying in a password by setting the value, then triggering a keyup event;
  • and once that's done, I check whether I can now see the message.
  • The last test checks first an invalid, then a valid password.

The implementation is really easy. Here's the template changes. I just added an event listener, and the <aside/>, which is set to be visible based on showPasswordMessage:

<input @keyup="checkPassword" type="password" name="password" required="required" maxlength="255" id="password" v-model="formValues.password" autocomplete="new-password">

<aside class="passwordMessage" v-if="showPasswordMessage">
    Password be at least eight characters long
    and must comprise at least one uppercase letter, one lowercase letter, one digit, and one other non-alphanumeric character
</aside>

And in the code part, we just check if the password is legit, and compute that variable accordingly:

checkPassword() {
    this.showPasswordMessage = !this.isPasswordValid;
}

That seems way too easy, but it all works. The tests pass, and it works on-screen too:

 

Haha. You know what? I wondered if I should test the text of the message as well, and went "nah, screw that". And now that I have that image in there, I see the typo in it. Ahem.

That will do for that lot. I quite enjoyed working out how to test the password changes there (how I trigger the keyup event. And doing stuff in Vue.js is pretty easy.

Tomorrow I'll go back to th web service end of things and do the back-end validation, which will need to be a lot more comprehensive than this. But it'll be easier to test as it's all PHP which I know better, and it's just value checking, not messing around with a UI.

Oh. The code for the two files I was working in is here, on Github:

Righto.

--
Adam

Wednesday 10 March 2021

TDDing the reading of data from a web service to populate elements of a Vue JS component

G'day:

This article leads on from "Symfony & TDD: adding endpoints to provide data for front-end workshop / registration requirements", which itself continues a saga I started back with "Creating a web site with Vue.js, Nginx, Symfony on PHP8 & MariaDB running in Docker containers - Part 1: Intro & Nginx" (that's a 12-parter), and a coupla other intermediary articles also related to this body of work. What I'm doing here specifically leads on from "Vue.js: using TDD to develop a data-entry form". In that article I built a front-end Vue.js component for a "workshop registration" form, submission, and summary:


Currently the front-end transitions work, but it's not connected to the back-end at all. It needs to read that list of "Workshops to attend" from the DB, and it needs to save all the data between form and summary.

In the immediately previous article I set up the endpoint the front end needs to call to fetch that list of workshops, and today I'm gonna remove the mocked data the form is using, and get it to use reallyreally data from the DB, via that end point.

To recap, this is what we currently have:

In the <template/> section of WorkshopRegistrationForm.vue:

<label for="workshopsToAttend" class="required">Workshops to attend:</label>

<select name="workshopsToAttend[]" multiple="true" required="required" id="workshopsToAttend" v-model="formValues.workshopsToAttend">
    <option v-for="workshop in workshops" :value="workshop.value" :key="workshop.value">{{workshop.text}}</option>
</select>

And in the code part of it:

mounted() {
      this.workshops = this.workshopService.getWorkshops();
},

And WorkshopService just has some mocked data:

getWorkshops() {
    return [
        {value: 2, text:"Workshop 1"},
        {value: 3, text:"Workshop 2"},
        {value: 5, text:"Workshop 3"},
        {value: 7, text:"Workshop 4"}
    ];
}

In this exercise I will be taking very small increments in my test / code / test / code cycle here. It's slightly arbitrary, but it's just because I like to keep separate the "refactoring" parts from the "new development parts", and I'll be doing both. The small increments help surface problems quickly, and also assist my focus so that I'm less likely to stray off-plan and start implementing stuff that is not part of the increment's requirements. This might, however, make for tedious reading. Shrug.

We're gonna refactor things a bit to start with, to push the mocked data closer to the boundary between the application and the DB. We're gonna follow a similar tiered application approach as the back-end work, in that we're going to have these components:

WorkshopRegistrationForm.vue
The Vue component providing the view part of the workshop registration form.
WorkshopService
This is the boundary between WorkshopRegistrationForm and the application code. The component calls it to get / process data. In this exercise, all it needs to do is to return a WorkshopCollection.
WorkshopCollection
This is the model for the collection of Workshops we display. It loads its data via WorkshopRepository
Workshop
This is the model for a single workshop. Just an ID and name.
WorkshopsRepository
This deals with fetching data from storage, and modelling it for the application to use. It knows about the application domain, and it knows about the DB schema. And translates between the two.
WorkshopsDAO
Because the repository has logic in it that we will be testing, our code that actually makes the calls to the DB connector has been extracted into this DAO class, so that when testing the repository it can be mocked-out.
Client
This is not out code. We're using Axios to make calls to the API, and the DAO is a thin layer around this.

As a first step we are going to push that mocked data back to the repository. We can do this without having to change our tests or writing any data manipulation logic.

Here's the current test run:

  Tests of WorkshopRegistrationForm component
     should have a required text input for fullName, maxLength 100, and label 'Full name'
     should have a required text input for phoneNumber, maxLength 50, and label 'Phone number'
     should have a required text input for emailAddress, maxLength 320, and label 'Email address'
     should have a required password input for password, maxLength 255, and label 'Password'
     should have a required workshopsToAttend multiple-select box, with label 'Workshops to attend'
     should list the workshop options fetched from the back-end
     should have a button to submit the registration
     should leave the submit button disabled until the form is filled
     should disable the form and indicate data is processing when the form is submitted
     should send the form values to WorkshopService.saveWorkshopRegistration when the form is submitted
     should display the registration summary 'template' after the registration has been submitted
     should display the summary values in the registration summary

We have to keep that green, and all we can do is move code around. No new code for now (other than a wee bit of scaffolding to let the app know about the new classes). Here are all the first round of changes.

Currently the deepest we go in the new code is the repository, which just returns the mocked data at the moment:

class WorkshopsRepository {

    selectAll() {
        return [
            {value: 2, text:"Workshop 1"},
            {value: 3, text:"Workshop 2"},
            {value: 5, text:"Workshop 3"},
            {value: 7, text:"Workshop 4"}
        ];
    }
}

export default WorkshopsRepository;

The WorkshopCollection grabs that data and uses it to populate itself. It extends the native array class so can be used as an array by the Vue template code:

class WorkshopCollection extends Array {

    constructor(repository) {
        super();
        this.repository = repository;
    }

    loadAll() {
        let workshops = this.repository.selectAll();

        this.length = 0;
        this.push(...workshops);
    }
}

module.exports = WorkshopCollection;

And the WorkshopService has had the mocked values removed, an in their place it tells its WorkshopCollection to load itself, and it returns the populated collection:

class WorkshopService {

    constructor(workshopCollection) {
        this.workshopCollection = workshopCollection;
    }

    getWorkshops() {
        this.workshopCollection.loadAll();
        return this.workshopCollection;
    }

    // ... rest of it unchanged...
}

module.exports = WorkshopService;

WorkshopRegistrationForm.vue has not changed, as it still receives an array of data to its this.workshops property, which is still used in the same way by the template code:

<template>
    <form method="post" action="" class="workshopRegistration" v-if="registrationState !== REGISTRATION_STATE_SUMMARY">
        <fieldset :disabled="isFormDisabled">

            <!-- ... -->

            <select name="workshopsToAttend[]" multiple="true" required="required" id="workshopsToAttend" v-model="formValues.workshopsToAttend">
                <option v-for="workshop in workshops" :value="workshop.value" :key="workshop.value">{{workshop.text}}</option>
            </select>

            <!-- ... -->

        </fieldset>
    </form>

    <!-- ... -->
</template>

<script>
// ...

export default {
    // ...
    mounted() {
        this.workshops = this.workshopService.getWorkshops();
    },
    // ...
}
</script>

Before we wire this up, we need to change our test to reflect where the data is coming from now. It used to be just stubbing the WorkshopService's getWorkshops method, but now we will need to be mocking the WorkshopRepository's selectAll method instead:

workshopService = new WorkshopService();
sinon.stub(workshopService, "getWorkshops").returns(expectedOptions);
sinon.stub(repository, "selectAll").returns(expectedOptions);

The tests now break as we have not yet implemented this change. We need to update App.vue, which has a bit more work to do to initialise that WorkshopService now, with its dependencies:

<template>
    <workshop-registration-form></workshop-registration-form>
</template>

<script>
import WorkshopRegistrationForm from "./WorkshopRegistrationForm";
import WorkshopService from "./WorkshopService";
import WorkshopCollection from "./WorkshopCollection";
import WorkshopsRepository from "./WorkshopsRepository";

let workshopService = new WorkshopService(
    new WorkshopCollection(
        new WorkshopsRepository()
    )
);

export default {
    name: 'App',
    components: {
        WorkshopRegistrationForm
    },
    provide: {
        workshopService: workshopService
    }
}
</script>

Having done this, the tests are passing again.

Next we need to deal with the fact that so far the mocked data we're passing around is keyed on how it is being used in the <option> tags it's populating:

return [
    {value: 2, text:"Workshop 1"},
    {value: 3, text:"Workshop 2"},
    {value: 5, text:"Workshop 3"},
    {value: 7, text:"Workshop 4"}
]

value and text are derived from the values' use in the mark-up, whereas what we ought to be mocking is an array of Workshop objects, which have keys id and name:

class Workshop {

    constructor(id, name) {
        this.id = id;
        this.name = name;
    }
}

module.exports = Workshop;

We will need to update our tests to expect this change:

sinon.stub(workshopService, "getWorkshops").returns(expectedOptions);
sinon.stub(repository, "selectAll").returns(
    expectedOptions.map((option) => new Workshop(option.value, option.text))
);

The tests fail again, so we're ready to change the code to make the tests pass. The repo now returns objects:

class WorkshopsRepository {

    selectAll() {
        return [
            new Workshop(2, "Workshop 1"),
            new Workshop(3, "Workshop 2"),
            new Workshop(5, "Workshop 3"),
            new Workshop(7, "Workshop 4")
        ];
    }
}

And we need to make a quick change to the stubbed saveWorkshopRegistration method in WorkshopService too, so the selectedWorkshops are now filtered on id, not value:

saveWorkshopRegistration(details) {
    let allWorkshops = this.getWorkshops();
    let selectedWorkshops = allWorkshops.filter((workshop) => {
        return details.workshopsToAttend.indexOf(workshop.name) >= 0;
        return details.workshopsToAttend.indexOf(workshop.id) >= 0;
    });
    // ...

And the template now expects them:

<option v-for="workshop in workshops" :value="workshop.value" :key="workshop.value">{{workshop.text}}</option>
<option v-for="workshop in workshops" :value="workshop.id" :key="workshop.id">{{workshop.name}}</option>

I almost forgot about this, but the summary section also uses the WorkshopCollection data, and currently it's also coded to expect mocked workshops with value/text properties, rather than id/name one. So the test needs updating:

it("should display the summary values in the registration summary", async () => {
    const summaryValues = {
        registrationCode : "TEST_registrationCode",
        fullName : "TEST_fullName",
        phoneNumber : "TEST_phoneNumber",
        emailAddress : "TEST_emailAddress",
        workshopsToAttend : [{value: "TEST_workshopToAttend_VALUE", text:"TEST_workshopToAttend_TEXT"}]
        workshopsToAttend : [{id: "TEST_workshopToAttend_ID", name:"TEST_workshopToAttend_NAME"}]
    };
    
	// ...

    let ddValue = actualWorkshopValue.find("ul>li");
    expect(ddValue.exists()).to.be.true;
    expect(ddValue.text()).to.equal(expectedWorkshopValue[0].name);
    expect(ddValue.text()).to.equal(expectedWorkshopValue[0].value);

	// ...
});

And the code change in the template:

<li v-for="workshop in summaryValues.workshopsToAttend" :key="workshop.value">{{workshop.text}}</li>
<li v-for="workshop in summaryValues.workshopsToAttend" :key="workshop.id">{{workshop.name}}</li>

And the tests pass again.

Now we will introduce the WorkshopsDAO with the data that it will get back from the API call mocked. It'll return this to the WorkshopsRepository which will implement the modelling now:

class WorkshopDAO {

    selectAll() {
        return [
            {id: 2, name: "Workshop 1"},
            {id: 3, name: "Workshop 2"},
            {id: 5, name: "Workshop 3"},
            {id: 7, name: "Workshop 4"}
        ];
    }
}

export default WorkshopDAO;

We also now need to update the test initialisation to pass a DAO instance into the repository, and for now we can remove the Sinon stubbing because the DAO is appropriately stubbed already:

let repository = new WorkshopsRepository();
sinon.stub(repository, "selectAll").returns(
    expectedOptions.map((option) => new Workshop(option.value, option.text))
);

workshopService = new WorkshopService(
    new WorkshopCollection(
        repository
        new WorkshopsRepository(
            new WorkshopsDAO()
        )
    )
);

That's the only test change here (and the tests now break). We'll update the code to also pass-in a DAO when we initialise WorkshopService's dependencies, and also implement the modelling code in the WorkshopRepository class's selectAll method:

let workshopService = new WorkshopService(
    new WorkshopCollection(
        new WorkshopsRepository()
        new WorkshopsRepository(
            new WorkshopsDAO()
        )
    )
);
class WorkshopRepository {

    constructor(dao) {
        this.dao = dao;
    }

    selectAll() {
        return this.dao.selectAll().map((unmodelledWorkshop) => {
            return new Workshop(unmodelledWorkshop.id, unmodelledWorkshop.name);
        });
    }
}

We're stable again. The next bit of development is the important bit: add the code in the DAO that actually makes the DB call. We will be changing the DAO to be this:

class WorkshopDAO {

    constructor(client, config) {
        this.client = client;
        this.config = config;
    }

    selectAll() {
        return this.client.get(this.config.workshopsUrl)
            .then((response) => {
                return response.data;
            });
    }
}

export default WorkshopDAO;

And that Config class will be this:

class Config {
    static workshopsUrl = "http://fullstackexercise.backend/workshops/";
}

module.exports = Config;

Now. Because Axios's get method returns a promise, we're going to need to cater to this in the up-stream methods that need manipulate the data being promised. Plus, ultimately, WorkshopRegistration.vue's code is going to receive a promise, not just the raw data. IE, this code:

mounted() {
    this.workshops = this.workshopService.getWorkshops();
    this.workshops = this.workshopService.getWorkshops()
        .then((workshops) => {
            this.workshops = workshops;
        });
},

A bunch of the tests rely on the state of the component after the data has been received:

  • should have a required text input for fullName, maxLength 100, and label 'Full name'
  • should have a required text input for phoneNumber, maxLength 50, and label 'Phone number'
  • should have a required text input for emailAddress, maxLength 320, and label 'Email address'
  • should have a required password input for password, maxLength 255, and label 'Password'
  • should have a required workshopsToAttend multiple-select box, with label 'Workshops to attend'
  • should list the workshop options fetched from the back-end
  • should have a button to submit the registration
  • should leave the submit button disabled until the form is filled
  • should disable the form and indicate data is processing when the form is submitted
  • should send the form values to WorkshopService.saveWorkshopRegistration when the form is submitted
  • should display the registration summary 'template' after the registration has been submitted
  • should display the summary values in the registration summary

In these tests we are gonna have to put the test code within a watch handler, so it waits until the promise returns the data (and accordingly workshops gets set, and the watch-handler fires) before trying to test with it, eg:

it("should list the workshop options fetched from the back-end", () => {
    component.vm.$watch("workshops", async () => {
    	await flushPromises();
        let options = component.findAll("form.workshopRegistration select[name='workshopsToAttend[]']>option");

        expect(options).to.have.length(expectedOptions.length);
        options.forEach((option, i) => {
            expect(option.attributes("value"), `option[${i}] value incorrect`).to.equal(expectedOptions[i].value.toString());
            expect(option.text(), `option[${i}] text incorrect`).to.equal(expectedOptions[i].text);
        });
    });
});

Other than that the tests shouldn't need changing. Also now that the DAO is not itself a mock as it was in the last iteration, we need to go back to mocking it again, this time to return a promise of things to come:

sinon.stub(dao, "selectAll").returns(Promise.resolve(
    expectedOptions.map((option) => ({id: option.value, name: option.text}))
));

It might seem odd that we are making changes to the DAO class but we're not unit testing those changes: the test changes here are only to accommodate the other objects' changes from being synchronous to being asynchrous. Bear in mind that these are unit tests and the DAO only exists as a thin wrapper around the Axios Client object, and its purpose is to give us some of our code that we can mock to prevent Axios from making an actual API call when we test the objects above it. We'll do a in integration test of the DAO separately after we deal with the unit testing and implementation.

After updating those tests to deal with the promises, the tests collapse in a heap, but this is to be expected. We'll make them pass again by bubbling-back the promise from returned from the DAO.

For the code revisions to the other classes I'm just going to show an image of the diff between the files from my IDE. I hate using pictures of code, but this is the clearest way of showing the diffs. All the actual code is on Github @ frontend/src/workshopRegistration

Here in the repository we need to use the values from the promise, so we need to wait for them to be resolved.

Similarly with the collection, service, and component (in two places), as per below:



It's important to note that the template code in the component did not need changing at all: it was happy to wait until the promise resolved before rendering the workshops in the select box.

Having done all this, the tests pass wonderfully, but the UI breaks. Doh! But in an "OK" way:

 


Entirely fair enough. I need to send that header back with the response.

class WorkshopControllerTest extends TestCase
{

    private WorkshopsController $controller;
    private WorkshopCollection $collection;

    protected function setUp(): void
    {
        $this->collection = $this->createMock(WorkshopCollection::class);
        $this->controller = new WorkshopsController($this->collection);
    }

    /**
     * @testdox It makes sure the CORS header returns with the origin's address
     * @covers ::doGet
     */
    public function testDoGetSetsCorsHeader()
    {
        $testOrigin = 'TEST_ORIGIN';
        $request = new Request();
        $request->headers->set('origin', $testOrigin);

        $response = $this->controller->doGet($request);

        $this->assertSame($testOrigin, $response->headers->get('Access-Control-Allow-Origin'));
    }
}
class WorkshopsController extends AbstractController
{

    // ...

    public function doGet(Request $request) : JsonResponse
    {
        $this->workshops->loadAll();

        $origin = $request->headers->get('origin');
        return new JsonResponse(
            $this->workshops,
            Response::HTTP_OK,
            [
                'Access-Control-Allow-Origin' => $origin
            ]
        );
    }
}

That sorts it out. And… we're code complete.

But before I finish, I'm gonna do an integration test of that DAO method, just to automate proof that it does what it needs do. I don't count "looking at the UI and going 'seems legit'" as testing.

import {expect} from "chai";
import Config from "../../src/workshopRegistration/Config";
import WorkshopsDAO from "../../src/workshopRegistration/WorkshopsDAO";
const client = require('axios').default;

describe("Integration tests of WorkshopsDAO", () => {

    it("returns the correct records from the API", async () => {
        let directCallObjects = await client.get(Config.workshopsUrl);

        let daoCallObjects = await new WorkshopsDAO(client, Config).selectAll();

        expect(daoCallObjects).to.eql(directCallObjects.data);
    });
});

And that passes.

OK we're done here. I learned a lot about JS promises in this exercise: I hid a lot of it from you here, but working out how to get those tests working for async stuff coming from the DAO took me an embarassing amount of time and frustration. Once I nailed it it all made sense though, and I was kinda like "duh, Cameron". So that's… erm… good…?

The next thing we need to do is to sort out how to write the data to the DB now. But before we do that, I'm feeling a bit naked without having any data validation on either the form or on the web service. Well: the web service end of things doesn't exist yet, but I'm going to start that work with putting some expected validation in. But I'll put some on the client-side first. As with most of the stuff in this current wodge of work: I do not have the slightest idea of how to do form validation with Vue.js. But tomorrow I will start to find out (see "Vue.js and TDD: adding client-side form field validation" for the results of that). Once again, I have a beer appointment to get myself too for now.

Righto.

--
Adam