You've Got To Take The Power Back

Towards the end of the previous video we had seemingly coded ourselves into a corner.

Because of the way that \SplFileInfo works internally (read this if geeking) we can't mock it.

And anyway, we shouldn't.

We need to make Symfony bend to our will.

Our will here is that Symfony's UploadedFile should conform to the way our code works.

That means we need a standard way of working with uploaded files, of which Symfony's UploadedFile chicanery is just one possible implementation.

As Rage Against The Machine will tell you:

You gotta take the power back

(nerr nerr I know you're making guitar noises in your head. Eughhh, come-on!)

We need an interface.

What this interface does is standardises the way the system behaves, without worrying about the specifics of the implementation.

We're pushing the bounds of this being a unit test. It's pushing more towards being better served with an integration test. But we will persist.

Let's adapt our spec to describe what should happen:

// /spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

use AppBundle\Entity\Wallpaper;
// - use Symfony\Component\HttpFoundation\File\UploadedFile;
// + use AppBundle\Model\FileInterface;

// ...

    function it_can_prePersist(
        LifecycleEventArgs $eventArgs,
        FileInterface $file
    )
    {
        $fakeTempPath = '/tmp/some.file';
        $fakeRealPath = '/path/to/my/project/some.file';

        $path = $fakeTempPath;
        $originalName = 'some.file';

        $wallpaper = new Wallpaper();
        $wallpaper->setFile($file);

        $eventArgs->getEntity()->willReturn($wallpaper);

        $this->prePersist($eventArgs)->shouldReturn(true);

        $this->fileMover->move($fakeTempPath, $fakeRealPath)->shouldHaveBeenCalled();
    }

We've not change a huge amount.

All we've done is started mocking our own interface rather than relying on the specifics of the way Symfony's UploadedFile implementation works.

Where does that get us towards a passing test?

php vendor/bin/phpspec run spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

AppBundle/Event/Listener/WallpaperUploadListener
  45  - it can prePersist
      collaborator does not exist : AppBundle\Model\FileInterface

                            75%                                     25%          4
1 specs
4 examples (3 passed, 1 broken)
60ms

  Would you like me to generate an interface `AppBundle\Model\FileInterface`
  for you?
                                                                         [Y/n]
y

Interface AppBundle\Model\FileInterface created in /Users/Shared/Development/wallpaper/src/AppBundle/Model/FileInterface.php.

AppBundle/Event/Listener/WallpaperUploadListener
  45  - it can prePersist
      no calls have been made that match:
        Double\AppBundle\Service\FileMover\P4->move(exact("/tmp/some.file"), exact("/path/to/my/project/some.file"))
      but expected at least one.

                            75%                                     25%          4
1 specs
4 examples (3 passed, 1 failed)
24ms

Quite a bit further.

We no longer care about the specifics of how the file retrieval process actually takes place. We simply care it behaves like we expect it too.

And for testing purposes, we can make it behave however the heck we like.

Then, when it comes to creating any implementations, so long as they behave the same way, they can conform to our interface, and life is good.

But:

That's not to say this way isn't more code to write.

As ever, trade-offs.

Back To The Implementation

We've just spent a ton of time in the tests.

Now it's time to use all that specification goodness that we just stewed up.

<?php

// /src/AppBundle/Event/Listener/WallpaperUploadListener.php

namespace AppBundle\Event\Listener;

use AppBundle\Service\FileMover;
use Doctrine\ORM\Event\LifecycleEventArgs;
use Doctrine\ORM\Event\PreUpdateEventArgs;

class WallpaperUploadListener
{
    /**
     * @var FileMover
     */
    private $fileMover;

    public function __construct(FileMover $fileMover)
    {
        $this->fileMover = $fileMover;
    }

    public function prePersist(LifecycleEventArgs $eventArgs)
    {
        if (false === $eventArgs->getEntity() instanceof Wallpaper) {
            return false;
        }

        $file = $eventArgs->getEntity()->getFile();

        $this->fileMover->move(
            $file->getExistingFilePath(),
            $file->getNewFilePath()
        );

        return true;
    }

    public function preUpdate(PreUpdateEventArgs $eventArgs)
    {
    }
}

I've removed code hints here so be sure to watch the video for an IDE-warning friendly approach.

We now need to get back to PhpSpec and run our tests.

php vendor/bin/phpspec run spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

AppBundle/Event/Listener/WallpaperUploadListener
  45  - it can prePersist
      exception [err:TypeError("Argument 1 passed to AppBundle\Entity\Wallpaper::setFile() must implement interface AppBundle\Model\FileInterface, instance of PhpSpec\Wrapper\Collaborator given, called in /Users/Shared/Development/wallpaper/spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php on line 54")] has been thrown.

                            75%                                     25%          4
1 specs
4 examples (3 passed, 1 broken)
61ms

This one is a strange one on me.

To fix this we need to use a method called getWrappedObject.

The thing is, we aren't using an object. We are using an interface.

If we call to getWrappedObject then this will fix the problem.

I believe this fixes the problem because by calling getWrappedObject in PhpSpec we are really calling reveal in Prophecy. When we call reveal, this returns a dummy object that implements the interface. Such is my interpretation.

// /spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

use AppBundle\Entity\Wallpaper;
// - use Symfony\Component\HttpFoundation\File\UploadedFile;
// + use AppBundle\Model\FileInterface;

// ...

    function it_can_prePersist(
        LifecycleEventArgs $eventArgs,
        FileInterface $file
    )
    {
        $fakeTempPath = '/tmp/some.file';
        $fakeRealPath = '/path/to/my/project/some.file';

        $path = $fakeTempPath;
        $originalName = 'some.file';

        $wallpaper = new Wallpaper();
        // change here
        $wallpaper->setFile(
            $file->getWrappedObject()
        );

        $eventArgs->getEntity()->willReturn($wallpaper);

        $this->prePersist($eventArgs)->shouldReturn(true);

        $this->fileMover->move($fakeTempPath, $fakeRealPath)->shouldHaveBeenCalled();
    }

Now when running PhpSpec it's able to notice that the first of our interface's expected methods is not available on whatever is passing for a FileInterface implementation, behind the scenes.

We need to define this as a valid method on the interface itself.

Fix this, and you will be met with the second missing method. This would go on, and on until every part of your test requirements had been met.

php vendor/bin/phpspec run spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

AppBundle/Event/Listener/WallpaperUploadListener
  45  - it can prePersist
      method `Double\FileInterface\P5::getExistingFilePath()` not found.

                            75%                                     25%          4
1 specs
4 examples (3 passed, 1 broken)
22ms

➜  wallpaper git:(cf52be8) ✗ php vendor/bin/phpspec run spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

AppBundle/Event/Listener/WallpaperUploadListener
  45  - it can prePersist
      method `Double\FileInterface\P5::getNewFilePath()` not found.

                            75%                                     25%          4
1 specs
4 examples (3 passed, 1 broken)
61ms

To fix this:

<?php

// src/AppBundle/Model/FileInterface.php

namespace AppBundle\Model;

interface FileInterface
{
    public function getExistingFilePath();
    public function getNewFilePath();
}

To be clear here, we don't have a real implementation for this yet.

Adding our Symfony FileUpload based implementation will come shortly.

Running the tests now we see the file mover is being hit, but it doesn't appear to be getting the expected arguments:

php vendor/bin/phpspec run spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

AppBundle/Event/Listener/WallpaperUploadListener
  45  - it can prePersist
      no calls have been made that match:
        Double\AppBundle\Service\FileMover\P4->move(exact("/tmp/some.file"), exact("/path/to/my/project/some.file"))
      but expected at least one.
      Recorded `move(...)` calls:
        - move(null, null) @ src/AppBundle/Event/Listener/WallpaperUploadListener.php:43

                            75%                                     25%          4
1 specs
4 examples (3 passed, 1 failed)
65ms

This is because we haven't told our mock implementation how to respond when asked the two questions:

• getExistingFilePath
• getNewFilePath

To do so we need to tell our mock how to respond. For this, we can use willReturn:

// /spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php

    function it_can_prePersist(
        LifecycleEventArgs $eventArgs,
        FileInterface $file // new line
    )
    {
        $fakeTempPath = '/tmp/some.file';
        $fakeRealPath = '/path/to/my/project/some.file';

        // two new lines
        $file->getExistingFilePath()->willReturn($fakeTempPath);
        $file->getNewFilePath()->willReturn($fakeRealPath);

        $wallpaper = new Wallpaper();
        $wallpaper->setFile($file->getWrappedObject());

        $eventArgs->getEntity()->willReturn($wallpaper);

        $this->prePersist($eventArgs)->shouldReturn(true);

        $this->fileMover->move($fakeTempPath, $fakeRealPath)->shouldHaveBeenCalled();
    }

Finally, when we run our tests, they all pass:

php vendor/bin/phpspec run spec/AppBundle/Event/Listener/WallpaperUploadListenerSpec.php
                                      100%                                       4
1 specs
4 examples (4 passed)
62ms

Does that mean we have a working solution?

Unfortunately not. As mentioned, this approach is going to lead to us writing more lines of code.

A lot of this code will be test code.

For comparative purposes, let's look at how the implementation differs from the comparative point we were at in the un-tested approach:

<?php

// /src/AppBundle/Event/Listener/WallpaperUploadListener.php

namespace AppBundle\Event\Listener;

use AppBundle\Entity\Wallpaper;
use AppBundle\Service\FileMover;
use Doctrine\ORM\Event\LifecycleEventArgs;
use Doctrine\ORM\Event\PreUpdateEventArgs;

class WallpaperUploadListener
{
    /**
     * @var FileMover
     */
    private $fileMover;

    public function __construct(FileMover $fileMover)
    {
        $this->fileMover = $fileMover;
    }

    public function prePersist(LifecycleEventArgs $eventArgs)
    {
        if (false === $eventArgs->getEntity() instanceof Wallpaper) {
            return false;
        }

        /**
         * @var $entity Wallpaper
         */
        $entity = $eventArgs->getEntity();

        $filename = $entity->getFile()->getFilename();

        $existingFilePath = $entity->getFile()->getPathname();
        $newFilePath = '/Users/Shared/Development/wallpaper/web/images/' . $filename;

        $this->fileMover->move($existingFilePath, $newFilePath);

        $entity
            ->setFile($entity->getFile()->getPathname())
            ->setFilename($filename);

        return true;
    }

    public function preUpdate(PreUpdateEventArgs $eventArgs)
    {
    }
}

What are your thoughts so far?