Refactoring for the future and edge cases.
Part of a series
This is the sixth post in a series of posts chronicling my attempt to use test-driven development techniques to develop the “Gattiny” WordPress plugin.
The plugin will take care of resizing animated GIF images preserving the animations when uploading them through the WordPress Media screen.
The first post is probably a better starting point than this one.
Dependency injection, service providers and stuff like that
In my previous post I’ve hinted at the refactoring process needed to move forward.
Given the plugin currently amounts to two files, the move could look like an overkill: it’s not going to be in my personal perception, but it could use some context and explanations.
As a first think I’m adding DI52, a PHP 5.2 compatible dependency injection container, to the mix:
composer require lucatume/di52
While di52 code is PHP 5.2 compatible, the autoload code Composer is generating is not; to make sure it is I’m adding the xrstf/composer-php52 package as a requirement for the project:
composer require xrstf/composer-php52
The xrstf/composer-php52 package requires modifying Composer configuration file, composer.json, to generate a PHP 5.2 compatible autoloader alongside the one it usually generates; I add the ``requireandscriptlines, and the ones needed to autoload Gattiny own classes. The former will create the PHP 5.2 compatible autoloader on the next call to thecomposer updateandcomposer dump-autoloadcommand and the latter will make sure thegattiny_` class prefix is one Composer autoloading component will recognize and manage:
{
"name": "lucatume/gattiny",
"description": "Resize animated GIF images on upload.",
"type": "wordpress-plugin",
"license": "GPL-2.0",
"authors": [
{
"name": "Luca Tumedei",
"email": "luca@theaveragedev.com"
}
],
"require": {
"lucatume/di52": "2.0.6",
"xrstf/composer-php52": "^1.0"
},
"require-dev": {
"lucatume/wp-browser": "^1.19"
},
"autoload": {
"psr-0": {
"gattiny_": [
"src/"
]
}
},
"scripts": {
"post-install-cmd": [
"xrstf\\Composer52\\Generator::onPostInstallCmd"
],
"post-update-cmd": [
"xrstf\\Composer52\\Generator::onPostInstallCmd"
],
"post-autoload-dump": [
"xrstf\\Composer52\\Generator::onPostInstallCmd"
]
}
}
After the update operation I include the Composer generated autoload file first thing in the main plugin file and remove my own autoloading solution:
<?php
/*
Plugin Name: Gattiny
Plugin URI: https://wordpress.org/plugins/gattiny/
Description: Resize animated GIF images on upload.
Version: 0.1.0
Author: Luca Tumedei
Author URI: http://theaveragedev.local
Text Domain: gattiny
Domain Path: /languages
*/
include_once dirname(__FILE__) . '/vendor/autoload_52.php';
Hooking stuff
The actions and filters the plugin needs to hook on to have not changed, but I’m refactoring the code to move to an object-oriented structure managed by means of a dependency injection container; this will make the code easier to test in its functions and easier to maintain in the long run.
Here is the updated code of the main plugin file:
<?php
// file gattiny.php
/*
Plugin Name: Gattiny
Plugin URI: https://wordpress.org/plugins/gattiny/
Description: Resize animated GIF images on upload.
Version: 0.1.0
Author: Luca Tumedei
Author URI: http://theaveragedev.local
Text Domain: gattiny
Domain Path: /languages
*/
// include the Composer generated autoload facility
include_once dirname(__FILE__) . '/vendor/autoload_52.php';
// instance the dependency-injection container
$di = new tad_DI52_Container();
$di->singleton('system', 'gattiny_System');
$di->singleton('image-editors', 'gattiny_ImageEditors');
add_action('admin_init', $di->callback('system', 'maybeDeactivate'));
add_filter('wp_image_editors', $di->callback('image-editors', 'filterImageEditors'));
Now the two final lines of the code deserve an explanation; since the same ratio applies to both I’m using the first one as an example.
First of all I bind the gattiny_System class to the system slug in the container; this means that when the container will be required to make an instance of the system object it will return a ready to use instance of the gattiny_System class.
I’m also saying the container the instance should be built just once when first requested implementing, in the context of the dependency injection container, a singleton.
In the next line I’m not using the $di->make('system') method but the callback one:
add_action('admin_init', $di->callback('system', 'maybeDeactivate'));
I’m still referring to the system slug but leveraging a lazy instantiation possibility offered by the container.
If I was writing PHP 5.3+ compatible code I would write the above code using a closure like this:
add_action('admin_init', function() use($container) {
// the "system" object is built **when** and **if** required
$system = $container->make('system');
$system->maybeDeactivate();
});
Under the hood DI52 is doing the same providing a eval-uated function as a callback to the WordPress action.
The alternative, for the gattiny_System class and any other class that needs to hook to WordPress actions and filters, would be eager instantiation or the use of functions; the former is inefficient, the latter is not my style. Running the tests I’ve put in place so far again reassures me the refactoring did not break anything:
Defensive testing
The idea of defensive programming is to make each component of a software capable of handling weird and unusual circumstances.
The word “circumstances” might be translated, in the context of a PHP application, in terms of inputs and context; keeping this in mind what could be a “weird” situation for the function in charge of filtering the wp_image_editors array?
The current function code is simple enough: it just prepends the gattiny_GifEditor class to the list of available image editors:
<?php
/**
* Class gattiny_ImageEditors
*
* Manages the image editors provided by the plugin.
*/
class gattiny_ImageEditors {
public function filterImageEditors(array $imageEditors) {
array_unshift($imageEditors, 'gattiny_GifEditor');
return $imageEditors;
}
}
Woudl it make sense to add the gattiny_GifEditor class to the list of available image editors if the the Imagick extension is not loaded? Not only it would not make sense but it would also be harmful and potentially fatal to the application.
To avoid such a case I’ve put a check in place in the gattiny_System class to avoid activating the plugin if it is not suppported; yet this does not cover the scenario where the plugin is activated silently, without calling its activation method, using a function like this activate_plugin('gattiny/gattiny.php', '', is_multisite(), true); where the last true argument means “silently”.
I write a test to cover the case where this could be the case in the tests/integration/FiltersTest test case:
/**
* It should not filter the image editors if the plugin is not supported
*
* @test
*/
public function it_should_not_filter_the_image_editors_if_the_plugin_is_not_supported() {
update_option('gattiny_supported', '0');
$imageEditors = new gattiny_ImageEditors();
$filtered = $imageEditors->filterImageEditors([]);
$this->assertNotContains('gattiny_GifEditor', $filtered);
}
And watch it fail as expected:
Time to correct the behaviour and make the test pass:
<?php
/**
* Class gattiny_ImageEditors
*
* Manages the image editors provided by the plugin.
*/
class gattiny_ImageEditors {
public function filterImageEditors(array $imageEditors) {
if (get_option('gattiny_supported') === '0') {
return $imageEditors;
}
array_unshift($imageEditors, 'gattiny_GifEditor');
return $imageEditors;
}
}
But where is that gattiny_supported option set to 0 when and if the Imagick extension is not loaded? That should ideally happen as early as possible and when it’s relevant to check for it; I update the gattiny_System::maybeDeactivate method:
public function maybeDeactivate() {
if ('0' === get_option('gattiny_supported') || !extension_loaded('imagick')) {
unset($_GET['activate']);
add_action('admin_notices', array($this, 'unsupportedNotice'));
}
}
And the gattiny_ImageEditors class again:
<?php
/**
* Class gattiny_ImageEditors
*
* Manages the image editors provided by the plugin.
*/
class gattiny_ImageEditors {
public function filterImageEditors(array $imageEditors) {
if ('0' === get_option('gattiny_supported') || !extension_loaded('imagick')) {
return $imageEditors;
}
array_unshift($imageEditors, 'gattiny_GifEditor');
return $imageEditors;
}
}
Next
The code shown in this post is on GitHub tagged post-06.
Next I’m getting down from integration to unit testing to make sure the atomic pieces of the code behave as intended.