Adrian Tiliță

Just sharing...

  • About Me
  • Just code

NeedleProject/FileIo – Step 3: Fix issues

January 16, 2017 / Leave a Comment

In the previous post we established some issues that can be viewed here here.

In this post I want to cover some of the issues, but, before that, let’s create an example to see our current covered usage.

First, I will write an index.php file in a new directory called example.

The file looks like this:

<?php
require_once dirname(__FILE__) . '/../vendor/autoload.php';

use NeedleProject\FileIo\File;
use NeedleProject\FileIo\Content\Content;

$filename = __DIR__ . DIRECTORY_SEPARATOR . 'my_first_file';

/**
 * Writing content to a file
 */
$content = new Content('Hello world!');
$file = new File(__DIR__ . DIRECTORY_SEPARATOR . 'my_first_file');
if (true === $file->write($content)) {
    echo "I wrote my first file!\n";
}

/**
 * Verifing file
 */
if (true === $file->exists()) {
    echo sprintf('File %s exists on disk!', $filename) . "\n";
}

if (true === $file->isWritable()) {
    echo sprintf('File %s can be written!', $filename) . "\n";
}

if (true === $file->isReadable()) {
    echo sprintf('File %s can be read!', $filename) . "\n";
}

/**
 * Read content from a file
 */
echo "File content:\n";
echo $file->getContent()->get();
echo "\nEnd of file content!\n";

/**
 * Delete the file
 */
if (true === $file->delete()) {
    echo "The file was successfully deleted!" . "\n";
}

By running the code, we get block in ::isWritable issue so let’s try to first fix this issue:

<?php
namespace NeedleProject\FileIo;

use NeedleProject\FileIo\Content\Content;
use NeedleProject\FileIo\Content\ContentInterface;
use NeedleProject\FileIo\Exception\FileNotFoundException;
use NeedleProject\FileIo\Exception\PermissionDeniedException;

class File
{
    // [...]
    /**
     * File constructor.
     *
     * @param string $filename
     */
    public function __construct(string $filename)
    {
        $this->filename = preg_replace('#(\\\|\/)#', DIRECTORY_SEPARATOR, $filename);
    }
    // [...]
    /**
     * @return bool
     */
    public function isWritable(): bool
    {
        if ($this->exists()) {
            return is_writable($this->filename);
        }
        $parts = explode(DIRECTORY_SEPARATOR, $this->filename);
        array_pop($parts);
        return is_writable(implode(DIRECTORY_SEPARATOR, $parts));
    }
    // [...]
}

By doing so, we have applied a working, but ugly, patch. That what happens eventually in every project: you write things to work but not always in the best way in terms of “clean code”. This is where refactoring will come into action but I will explain that in future posts.

First, on construction we modify the filename to have a path using the current’s system directory separator. We used that so we can retrieve handle the path inside is writable.
Now, we can write a file, even if it does not exists, by validating that that the path to that file is writable.

Running the example now will output like this:

php index.php
File /home/adrian.tilita/fileio/example/my_first_file exists on disk!
File /home/adrian.tilita/fileio/example/my_first_file can be written!
File /home/adrian.tilita/fileio/example/my_first_file can be read!
File content:
Hello world!
End of file content!
The file was successfully deleted!

Now the test should be updated to cover our current development. Given the fact that I will write into FileTest.php, I decided to do some rewriting as well: data providers should be all prefixed using the same keyword (I used both provision and provide and decided I should stick to one: provide).
Also, in many cases, the test scenario can be covered with only one provided item (data providers returns only one scenario) so I ended up removing some “@dataProviders” and scope the data inside the given test method.

Afterwards, I continue by handling issue #1

The File’s ::getContent end up looking like this:

/**
 * @return \NeedleProject\FileIo\Content\ContentInterface
 * @throws \NeedleProject\FileIo\Exception\FileNotFoundException
 * @throws \NeedleProject\FileIo\Exception\IOException
 * @throws \NeedleProject\FileIo\Exception\PermissionDeniedException
 */
public function getContent(): ContentInterface
{
    if ($this->exists() === false) {
        throw new FileNotFoundException(sprintf("%s does not exists!", $this->filename));
    }
    if ($this->isReadable() === false) {
        throw new PermissionDeniedException(
            sprintf("You do not have permissions to read file %s!", $this->filename)
        );
    }
    $stringContent = file_get_contents($this->filename);
    if (false === $stringContent) {
        throw new IOException(
            sprintf("Could not retrieve content! Error message: %s", error_get_last()['message'])
        );
    }
    return new Content($stringContent);
}

This approach will cause some issues: I really don’t like to apply error suppressing using the @ symbol, and, also, the test will be a real pain. Usually this kind of tests can be excluded or the logic can be spitted in a separate mock-able layer.
To further explain: The file_get_contents is strictly coupled inside ::getContent, and, being a builtin function, it will be hard to mock a create the scenario where it will return false as a response. There are some possibilities:

  1. exclude test (the library is at a very low stage, excluding the test will be a big draw-bag for me)
  2. “ninja” the test – create a slightly complicated testing scenario and stubs
  3. Refactor this using Extract method and Move method ending up with a separate layer/class that contain that can encapsulate and handle the file_get_contents response

Usually I would chose #3 but in this context I don’t want to modify the logic for the testing purpose and also I don’t want to exclude tests so will go for “ninja”:

<?php
namespace NeedleProject\FileIo;

use NeedleProject\FileIo\Content\Content;
use Symfony\Component\DependencyInjection\Tests\Compiler\F;

class FileTest extends \PHPUnit_Framework_TestCase
{
    /**
     * @var bool    If we should apply a stub
     */
    static public $applyStub = false;

    /**
     * @var bool    If the the stub should trigger an error
     */
    static public $disableStubsError = false;

    // [...]


    /**
     * @param $providedFile
     * @dataProvider provideRealFiles
     * @expectedException \NeedleProject\FileIo\Exception\IOException
     * @expectedExceptionMessage Dummy error!
     */
    public function testGetContentConvertedException($providedFile)
    {
        require_once 'stub/file_get_contents.php';
        static::$applyStub = true;

        $file = new File($providedFile);
        $file->getContent();
    }


    /**
     * @param $providedFile
     * @dataProvider provideRealFiles
     * @expectedException \NeedleProject\FileIo\Exception\IOException
     * @expectedExceptionMessageRegExp /Could not retrieve content! Error message:/
     */
    public function testGetContentFalse($providedFile)
    {
        require_once 'stub/file_get_contents.php';
        static::$applyStub = true;
        static::$disableStubsError = true;

        $file = new File($providedFile);
        $file->getContent();
    }
    // [...]
}

So, what does file_get_contents.php contains and what’s with the static $applyStub.
Well, we will create a new “file_get_contents” function, but, given the fact that in our code we are not using the function by “rooting” it’s namespace (Ex: \file_get_contents) we can declare the function inside our namespace. I think the code will speak for itself:

<?php
namespace NeedleProject\FileIo;

/**
 * Stub file_get_contents
 * @param $filename
 * @return bool
 */
function file_get_contents($filename) {
    if (true === FileTest::$applyStub) {
        FileTest::$applyStub = false;
        if (false === FileTest::$disableStubsError) {
            trigger_error("Dummy error!", E_USER_WARNING);
        }
        return false;
    }
    return \file_get_contents($filename);
}
Note: This is neither “elegant” or recommended. Tests with this level of complexity might end-up being themselves prone to errors and can cause other tests to behave unexpected. In this case it is an exception I am willing to take responsibility for it.
I realize that eventually I will end-up refactoring to the 3rd form presented previously.

Now, we created the exact scenario and then we run the tests and see that the tests encounters an error: that Dummy Error that the code should cover. For that I will create a helper (in a new directory called Util) that will replace errors into Expections:

<?php
namespace NeedleProject\FileIo\Util;

use NeedleProject\FileIo\Exception\IOException;

class ErrorHandler
{
    /**
     * States that the current object is the current error handler
     * @var bool
     */
    protected static $isHandledLocal = false;

    /**
     * @param int|null $level   The error level to be handled
     */
    public static function convertErrorsToExceptions(int $level = null)
    {
        static::$isHandledLocal = true;
        if (is_null($level)) {
            $level = E_ALL;
        }
        set_error_handler(function ($errorNumber, $errorMessage, $errorFile, $errorLine, $errorContext) {
            throw new IOException($errorMessage, $errorNumber);
        }, $level);
    }

    /**
     * Restore predefined error handlers
     */
    public static function restoreErrorHandler()
    {
        if (true === static::$isHandledLocal) {
            \restore_error_handler();
        }
    }
}

Now let’s include the ErrorHandler in the ::getContent() method:
The File’s ::getContent end up looking like this:

/**
 * @return \NeedleProject\FileIo\Content\ContentInterface
 * @throws \NeedleProject\FileIo\Exception\FileNotFoundException
 * @throws \NeedleProject\FileIo\Exception\IOException
 * @throws \NeedleProject\FileIo\Exception\PermissionDeniedException
 */
public function getContent(): ContentInterface
{
    if ($this->exists() === false) {
        throw new FileNotFoundException(sprintf("%s does not exists!", $this->filename));
    }
    if ($this->isReadable() === false) {
        throw new PermissionDeniedException(
            sprintf("You do not have permissions to read file %s!", $this->filename)
        );
    }
    ErrorHandler::convertErrorsToExceptions();
    $stringContent = file_get_contents($this->filename);
    ErrorHandler::restoreErrorHandler();
    if (false === $stringContent) {
        throw new IOException(
            sprintf("Could not retrieve content! Error message: %s", error_get_last()['message'])
        );
    }
    return new Content($stringContent);
}

Now, given that we have this utility, let’s also fix the Issue #2:

/**
 * Deletes the current file
 * @return bool
 * @throws \NeedleProject\FileIo\Exception\IOException
 */
public function delete(): bool
{
    if ($this->exists() === false) {
        return false;
    }
    ErrorHandler::convertErrorsToExceptions();
    $unlinkResult = unlink($this->filename);
    ErrorHandler::restoreErrorHandler();
    return $unlinkResult;
}

So let’s commit our changes and solve our issues. The commit can be viewed here.

To conclude:

  • We’ve fixed issues #1 and #2
  • We’ve created an example of usage

But we will not stop here, we still have a lot of work to do that will present in future posts!

Share This:

Posted in: Just code Tagged: error handling, fileio, needle-project, php, tdd

My shared code

adrian-tilita (Tilita Adrian)
Bucharest, Romania
Joined on Nov 25, 2013
11 Public Repositories
0 Public Gists

Copyright © 2023 Adrian Tiliță.

Me WordPress Theme by themehall.com