5.3 refactoring fun-time party land

[willmot]

First bash at a major re-factor to introduce some more modern design patterns and conventions that are available to us now we support PHP 5.3 as a minimum.

What have we here?

• Moves HM Backup into this repo and renames it class-backup.php
• Moves the HM Backup tests into the BackUpWordPress tests directory
• Includes the changes introduced in Add support for copying and updating an existing backups #666 & Add support for updating an existing archive instead of always creating a new one humanmade/hm-backup#77
• Switches to using Namespaces (HM\BackUpWordPress\) instead of class prefixes throughout
• Rename some files and classes for consistency, split each class into it's own file
• De-couple Scheduled_Backup from Backup, the former no longer extends the latter, instead relying on dependancy injection.
• Required re-factoring several aspects of both classes so they could work together without being so tightly coupled, specifically
  • Correctly set Backup properties using the saved options in Scheduled_Backup::__construct
  • Update all calls to Backup methods that were being called directly through Scheduled_Backup relying on the fact it was a child, instead just call Backup directly
  • Backup::do_action now requires callbacks to be registered by the caller (which Scheduled_Backup has been updaed to do)
  • Get rid of the Backup::set_path && Scheduled_Backup::set_path methods as they were simply duplicating the functionality of Path::set_path
• Update all tests, they all pass!
• Also fixes up the issue where some tests were causing each other to fail becuase they weren't properly cleaning up after themselves (see Introduce a HMBKP_Path class to replace hmbkp_path(); #612 (comment))

Other ideas

Backup and Scheduled_Backup are both still large classes that should be modularised, some ways we could go are:

• Split filesystem stuff (get_files, excludes, filesize stuff) into it's own class
• Split the handling of the root path into it's own class (ala Path)
• We'd likely then have Backup_Path and Root_Path both extend a base class Path
• Move conform_dir to Path from Backup and probably rename it to normalise
• Split out the various archive and database dump methods into separate classes, each extending a base Backup_Archive & Database_Dump class
• Feels like Services should be renamed Extensions as that's really what they are
• Each Service should handle it's own do_action hooking instead of that being coupled in with Scheduled_Backup::do_action
• Potentially merge Setup into Plugin.

Before we merge

• Premium extensions will need updating as things have changed in non-backwards compatible ways, not sure how we handle that
• Moar testing.
• Fix up as much of the scrutinizer.io complaints
• Improve code coverage
• Move github issues from HM-Backup repo over to this repo.

[willmot]

Fix up as much of the scrutinizer.io complaints

Fixed up all the major issues reported, still a ton of minor issues.

[pdewouters]

Love this! There are a few in there that were also bothering me, others that I hadn't thought of at all...great stuff

Potentially merge Setup into Plugin.

What would be the benefit?

I agree Service classes can do with some re-architecturing, can be handled in separate PR. Take for example the empty Intercom methods that are just there to satisfy the subclassing

[pdewouters]

Instead of calling

    $schedule = new HM\BackUpWordPress\Scheduled_Backup( sanitize_text_field( urldecode( $_GET['hmbkp_schedule_id'] ) ) );

to run a schedule. can't we just do something like

Schedules->get_schedule( $id )->run();

[willmot]

Yeah that would be neater although would hit the DB twice instead of once.

[willmot]

I've moved all the issue over from HM Backup

[pdewouters]

De-couple Scheduled_Backup from Backup, the former no longer extends the latter, instead relying on dependancy injection.

As I understand it, dependency injection would require the BackUp instance to be passed as a parameter to the constructor

public function __construct( Backup $backup ) {
  $this->backup = $backup;
}

[willmot]

Yeah I ended up not doing that as I hadn't actually run into a situation where I needed to pass something different.

(One of the main reasons for dependancy injection is it means you can pass in mock objects for unit testing purposes)