The Great Backup Refactor of 2015

[willmot]

This PR introduces a major refactor of the Backups class with the express aim of separating concerns, reducing complexity and improving testability.

This is achieved by splitting the Backups class into several smaller classes, each responsible for different parts of the backup process.

The core backup functionality (mysqldump, zip, zipArchive) is now handled by a set of Backup_Engine's, one for each backup method.

These Backup_Engines are then brought together in Site_Backup, which takes responsibility for combining the database and file backups as well as looping through the available backup engines.

The Refactor

Backup Engines

1. Backup_Engine

The backup engine concept has two levels of inheritance. Starting at it's base with the Backup_Engine class. This is both a utility class and a template and is responsible for:

• Setting the memory_limit and set_timeout settings so the backup can run uninterrupted.
• Storing the backup filename and filepath.
• Setting the directory which the backup will be stored in.
• It defines an abstract function verify_backup

2. File_Backup_Engine & Database_Backup_Engine

The next 2 classes both extend Backup_Engine. These are each responsible for generic functionality related to Database and File backups.

• Database_Backup_Engine is responsible for parsing database connection details and making them available to be used by the eventual backup methods. It's also defines the verify_backup method which is used to verify that any database backup methods completed successfully.
• File_Backup_Engine is responsible managing the excludes functionality including default excludes. It provides methods for dealing with the files intended for backup and provides a file specific implementation of verify_backup.

3. Zip_File_Backup_Engine, Zip_Archive_File_Backup_Engine, Mysqldump_Database_Backup_Engine, IMysqldump_Database_Backup_Engine.

With all the shared functionality handled by the parent classes we're left with very lean final backup method implementations. At their core a backup method need only implement the backup method which should return $this->verify_backup.

One huge benefit to this structure is that it's now much, much easier to add new backup methods. We could easily add Tar support, or bring back PclZip with much less overhead and no increase in code complexity outside of the specific Backup_Method implementations. For example

class Tar_File_Backup_Engine extends File_Backup_Engine {

    public function __construct() {
        parent::__construct();
    }

    public function backup() {
        // Perform Tar Backup...
        return $this->verify_backup();
    }
}

This PR also splits out a bunch of other functionality from both the Scheduled_Backup and Backup classes:

• The various static utility methods have been split out into a Backup_Utilities class.
• Excludes are now handled by Excludes.
• The site size functionality is now handled by Site_Size.
• The backup status tracking is now handled by Backup_Status.

Unit Testing

A huge driver behind this refactor was a desire to make the core backups functionality much more testable. The current implementation has been written entirely to tests.

The abstract Backup_Engine, File_Backup_Engine and Database_Backup_Engine classes are all tested indirectly using mock backup engine implementations.

Another key aim was that we should run the same set of common backup engine unit tests across all backup method implementations without having to duplicate the tests, this is achieved with the Common_Database_Backup_Engine_Tests & Common_File_Backup_Engine_Tests parent test cases. These test cases are not run directly, instead the are extended by a specific backup engine implementations tests. For example:

<?php

namespace HM\BackUpWordPress;

class Zip_File_Backup_Engine_Tests extends File_Backup_Engine_Common_Tests {

    protected $backup;

    public function setUp() {
        $this->backup = new Zip_File_Backup_Engine;
        parent::setUp();
    }


}

This test case is then run directly. Any tests unique to the Zip_File_Backup_Method would be implemented here. The specific backup engine is then passed up to the parent by calling parent::setUp. Again the real benefit here is that a new backup engine automatically get's a whole suit of unit tests just be implementing the above barebones test case. This hugely reduces the amount of copy paste tests that we currently have.

As part of implementing this unit tests I've already discovered and fixed several bugs that are present in the current Backup class.

The test data is now created and destroyed programatically instead of being stored on the filesystem, this ensures we can rely on the data being exactly as we expect.

TODO

There is a still a bunch of things todo to finish off the work here:

• Dockblocks need moving over from the existing Backup class, and in a lot of cases, updating.
• The top level Backup_Engine class currently has all the error and warning handling. This should probably be moved to a separate Backup_Errors class and be refactored to use WP_Error.
• There is no backwards compatibility with the old hmbkp_ actions.
• We could consider splitting the static utility functions into their own Backup_Utility class.
• Back compat with the old get_archive_method and get_mysqldump_method methods (if they are needed).
• Site size calculations
• Site file list folder sizes
• Backup status
• Backup errors and warnings
• We should aim for at least 80% coverage of this new code before we merge.
• Move over the remaining unit tests from the old Backup class.
• Remove any old unit tests that are no longer relevant.
• Fixup issues reported by Scrutiniser
• Add in a filter so that 3rd parties can add their own Backup_Engine implementations into the list.
• The database backup is not currently passed into the file backups, need to find a way to do this.
• The Backup_Director needs to be able to take arguments that need to be set on the Backup_Engine implementations (for example setting new excludes).
• Integrate with Scheduled_Backup so that the main plugin uses the new code.
• Remove the old Backup class.
• PATH shouldn't be a singleton and should instead be passed around.
• File_Backup_Engines should take the directory to backup and the output filepath as constructor args. Database_Backup_Engines should just take the output filepath.
• The root Backup_Engine class should probably go away (error handling should be a separate class that's injected as a dependancy and upping the memory limit should be handled by the caller.
• Database_Backup_Engine and File_Backup_Engine should maybe be implementations. The shared functionality could be passed in by the caller.
• There should be a Files class which handles interactions with Finder.
• Fixup the Email & Webhook Services.
• No longer firing Services::action, is that an issue?
• No longer storing errors and warnings in .backup_errors and .backup_warnings files, were they used?

[willmot]

Put in a bunch of effort to get the unit tests running cleanly and passing on Windows. I'm also going to writeup a guide on how to get unit testing running on Windows as there isn't much out there.

[willmot]

Chatted to @pdewouters, we're going to introduce back combat methods on Site_Backup so that extensions continue to work without changes.

[willmot]

In 7031a2b we're now 100% back compatible with existing extensions.

[willmot]

\0/