From 8a7a921ee1a7fc5ee5dd23a4d33dc816eec69b7f Mon Sep 17 00:00:00 2001 From: Adirelle Date: Mon, 27 Apr 2015 08:51:45 +0200 Subject: [PATCH] Refactor the plugin interface, added more interfaces. --- PHPCI/Plugin.php | 24 ++++++++ PHPCI/Plugin/AbstractExecutingPlugin.php | 8 +-- PHPCI/Plugin/AbstractInterpolatingPlugin.php | 24 ++------ PHPCI/Plugin/AbstractPlugin.php | 55 ++++--------------- PHPCI/Plugin/Atoum.php | 4 +- PHPCI/Plugin/Codeception.php | 5 +- PHPCI/Plugin/Composer.php | 4 +- PHPCI/Plugin/Email.php | 6 +- PHPCI/Plugin/Lint.php | 2 +- PHPCI/Plugin/Mysql.php | 27 ++++----- PHPCI/Plugin/Pdepend.php | 2 +- PHPCI/Plugin/Pgsql.php | 13 ++--- PHPCI/Plugin/Phar.php | 21 ++----- PHPCI/Plugin/PhpCodeSniffer.php | 2 +- PHPCI/Plugin/PhpCpd.php | 2 +- PHPCI/Plugin/PhpMessDetector.php | 2 +- PHPCI/Plugin/PhpUnit.php | 1 - PHPCI/Plugin/Sqlite.php | 13 ++--- PHPCI/Plugin/TechnicalDebt.php | 2 +- PHPCI/Plugin/Util/Executor.php | 51 ++++++++++------- PHPCI/Plugin/Util/Factory.php | 9 +++ .../Util/InterpolatorAwareInterface.php | 27 +++++++++ 22 files changed, 149 insertions(+), 155 deletions(-) create mode 100644 PHPCI/Plugin/Util/InterpolatorAwareInterface.php diff --git a/PHPCI/Plugin.php b/PHPCI/Plugin.php index ac2960cd..4bc9674e 100644 --- a/PHPCI/Plugin.php +++ b/PHPCI/Plugin.php @@ -15,5 +15,29 @@ namespace PHPCI; */ interface Plugin { + /** + * Sets the options for the current build stage. + * + * @param array $options + * + * @throws \Exception If the options are invalid. + */ + public function setOptions(array $options); + + /** + * Sets the settings for all stages. + * + * @param array $settings + * + * @throws \Exception If the settings are invalid. + */ + public function setCommonSettings(array $settings); + + /** + * Execute the plugin. + * + * @return bool true if all went nice, false if the plugin ended normally but failed. + * @throws \Exception If something prevented the plugin to end normally. + */ public function execute(); } diff --git a/PHPCI/Plugin/AbstractExecutingPlugin.php b/PHPCI/Plugin/AbstractExecutingPlugin.php index 9c21a4a8..ad57a161 100644 --- a/PHPCI/Plugin/AbstractExecutingPlugin.php +++ b/PHPCI/Plugin/AbstractExecutingPlugin.php @@ -33,18 +33,14 @@ abstract class AbstractExecutingPlugin extends AbstractPlugin * * @param Builder $builder * @param Build $build - * @param BuildLogger $logger * @param CommandExecutor $executor - * @param array $options */ public function __construct( Builder $builder, Build $build, - BuildLogger $logger, - CommandExecutor $executor, - array $options = array() + CommandExecutor $executor ) { $this->executor = $executor; - parent:__construct($builder, $build, $logger, $options); + parent:__construct($builder, $build); } } diff --git a/PHPCI/Plugin/AbstractInterpolatingPlugin.php b/PHPCI/Plugin/AbstractInterpolatingPlugin.php index 1efb1355..0883290e 100644 --- a/PHPCI/Plugin/AbstractInterpolatingPlugin.php +++ b/PHPCI/Plugin/AbstractInterpolatingPlugin.php @@ -11,39 +11,25 @@ namespace PHPCI\Plugin; -use PHPCI\Builder; use PHPCI\Helper\BuildInterpolator; -use PHPCI\Helper\CommandExecutor; -use PHPCI\Model\Build; +use PHPCI\Plugin\Util\InterpolatorAwareInterface; /** * Asbtract plugin which uses a BuildInterpolator. */ -abstract class AbstractInterpolatingPlugin extends AbstractExecutingPlugin +abstract class AbstractInterpolatingPlugin extends AbstractExecutingPlugin implements InterpolatorAwareInterface { /** * @var BuildInterpolator */ protected $interpolator; - /** Standard constructor. + /** * - * @param Builder $phpci - * @param Build $build - * @param BuildLogger $logger - * @param CommandExecutor $executor * @param BuildInterpolator $interpolator - * @param array $options */ - public function __construct( - Builder $phpci, - Build $build, - BuildLogger $logger, - CommandExecutor $executor, - BuildInterpolator $interpolator, - array $options = array() - ) { + public function setInterpolator(BuildInterpolator $interpolator) + { $this->interpolator = $interpolator; - parent::__construct($phpci, $build, $logger, $executor, $options); } } diff --git a/PHPCI/Plugin/AbstractPlugin.php b/PHPCI/Plugin/AbstractPlugin.php index 50e29a77..83d1274f 100644 --- a/PHPCI/Plugin/AbstractPlugin.php +++ b/PHPCI/Plugin/AbstractPlugin.php @@ -14,13 +14,15 @@ use PHPCI\Builder; use PHPCI\Logging\BuildLogger; use PHPCI\Model\Build; use PHPCI\Plugin; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerInterface; /** * Asbtract plugin. * * Holds helper for the subclasses. */ -abstract class AbstractPlugin implements Plugin +abstract class AbstractPlugin implements Plugin, LoggerAwareInterface { /** * @var Build @@ -33,7 +35,7 @@ abstract class AbstractPlugin implements Plugin protected $phpci; /** - * @var BuildLogger + * @var LoggerInterface */ protected $logger; @@ -43,30 +45,23 @@ abstract class AbstractPlugin implements Plugin * @param Builder $builder * @param Build $build * @param BuildLogger $logger - * @param array $options */ - public function __construct(Builder $builder, Build $build, BuildLogger $logger, array $options = array()) + public function __construct(Builder $builder, Build $build) { $this->phpci = $builder; $this->build = $build; - $this->logger = $logger; - $this->setup($options); + $this->setBuildPath($builder->buildPath); + $this->setIgnorePath((array) $builder->ignore); } /** - * Return the key used for options in phpci.yml for this plugin. * - * @return string + * @param LoggerInterface $logger */ - public function getPluginKey() + public function setLogger(LoggerInterface $logger) { - $matches = null; - $className = get_class($this); - if (!preg_match('/^PHPCI\\Plguin\\(\w+)$/', $className, $matches)) { - return $className; - } - return strtolower(preg_replace('/[[:lower:]][[:upper:]]/g', '$1_$2', $matches[1])); + $this->logger = $logger; } /** @@ -100,34 +95,4 @@ abstract class AbstractPlugin implements Plugin { // NOOP } - - /** - * Configure the plugin for a given stage. - * - * @param array $options - */ - abstract protected function setOptions(array $options); - - /** - * Post-constructor setup. - * - * TODO: expose setOptions and setCommonSettings in Plugin and get rid of this method. - * - * @param array $options - */ - private function setup(array $options) - { - $this->setBuildPath($this->phpci->buildPath); - $this->setIgnorePath((array) $this->phpci->ignore); - - $settings = $this->phpci->getConfig('build_settings'); - $key = $this->getPluginKey(); - if ($settings && isset($settings[$key])) { - $this->setCommonSettings((array) $settings[$key]); - } else { - $this->setCommonSettings(array()); - } - - $this->setOptions($options); - } } diff --git a/PHPCI/Plugin/Atoum.php b/PHPCI/Plugin/Atoum.php index 95bc9b08..a690bfba 100644 --- a/PHPCI/Plugin/Atoum.php +++ b/PHPCI/Plugin/Atoum.php @@ -72,11 +72,11 @@ class Atoum extends AbstractExecutingPlugin if (count(preg_grep("/Success \(/", $output)) == 0) { $status = false; - $this->logger->log($output); + $this->logger->notice($output); } if (count($output) == 0) { $status = false; - $this->logger->log(Lang::get('no_tests_performed')); + $this->logger->warning(Lang::get('no_tests_performed')); } return $status; diff --git a/PHPCI/Plugin/Codeception.php b/PHPCI/Plugin/Codeception.php index 64d3f4b0..a04d51f8 100644 --- a/PHPCI/Plugin/Codeception.php +++ b/PHPCI/Plugin/Codeception.php @@ -129,10 +129,7 @@ class Codeception extends AbstractExecutingPlugin implements PHPCI\ZeroConfigPlu $configPath = $this->buildPath . $configPath; $success = $this->executor->executeCommand($cmd, $this->buildPath, $configPath); - $this->logger->log( - 'Codeception XML path: '. $this->buildPath . $this->path . '_output/report.xml', - Loglevel::DEBUG - ); + $this->logger->debug('Codeception XML path: '. $this->buildPath . $this->path . '_output/report.xml'); $xml = file_get_contents($this->buildPath . $this->path . '_output/report.xml', false); $parser = new Parser($this->phpci, $xml); diff --git a/PHPCI/Plugin/Composer.php b/PHPCI/Plugin/Composer.php index 4aadd4a3..978c0b53 100644 --- a/PHPCI/Plugin/Composer.php +++ b/PHPCI/Plugin/Composer.php @@ -85,10 +85,10 @@ class Composer extends AbstractExecutingPlugin implements PHPCI\ZeroConfigPlugin $cmd .= $composerLocation . ' --no-ansi --no-interaction '; if ($this->preferDist) { - $this->logger->log('Using --prefer-dist flag'); + $this->logger->notice('Using --prefer-dist flag'); $cmd .= '--prefer-dist'; } else { - $this->logger->log('Using --prefer-source flag'); + $this->logger->notice('Using --prefer-source flag'); $cmd .= '--prefer-source'; } diff --git a/PHPCI/Plugin/Email.php b/PHPCI/Plugin/Email.php index 9d1f48ba..ecea9361 100644 --- a/PHPCI/Plugin/Email.php +++ b/PHPCI/Plugin/Email.php @@ -65,8 +65,10 @@ class Email extends AbstractPlugin ); // This is a success if we've not failed to send anything. - $this->logger->log(sprintf("%d emails sent", (count($addresses) - $sendFailures))); - $this->logger->log(sprintf("%d emails failed to send", $sendFailures)); + $this->logger->notice(sprintf("%d emails sent", (count($addresses) - $sendFailures))); + if ($sendFailures > 0) { + $this->logger->error(sprintf("%d emails failed to send", $sendFailures)); + } return ($sendFailures === 0); } diff --git a/PHPCI/Plugin/Lint.php b/PHPCI/Plugin/Lint.php index 7aef2520..1ce833bc 100644 --- a/PHPCI/Plugin/Lint.php +++ b/PHPCI/Plugin/Lint.php @@ -126,7 +126,7 @@ class Lint extends AbstractExecutingPlugin $success = true; if (!$this->executor->executeCommand($php . ' -l "%s"', $this->buildPath . $path)) { - $this->logger->logFailure($path); + $this->logger->error($path); $success = false; } diff --git a/PHPCI/Plugin/Mysql.php b/PHPCI/Plugin/Mysql.php index 3439d7e8..8466ff66 100644 --- a/PHPCI/Plugin/Mysql.php +++ b/PHPCI/Plugin/Mysql.php @@ -75,24 +75,19 @@ class Mysql extends AbstractInterpolatingPlugin */ public function execute() { - try { - $opts = array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION); - $pdo = new PDO('mysql:host=' . $this->host, $this->user, $this->pass, $opts); + $opts = array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION); + $pdo = new PDO('mysql:host=' . $this->host, $this->user, $this->pass, $opts); - foreach ($this->queries as $query) { - if (!is_array($query)) { - // Simple query - $pdo->query($this->interpolator->interpolate($query)); - } elseif (isset($query['import'])) { - // SQL file execution - $this->executeFile($query['import']); - } else { - throw new \Exception(Lang::get('invalid_command')); - } + foreach ($this->queries as $query) { + if (!is_array($query)) { + // Simple query + $pdo->query($this->interpolator->interpolate($query)); + } elseif (isset($query['import'])) { + // SQL file execution + $this->executeFile($query['import']); + } else { + throw new \Exception(Lang::get('invalid_command')); } - } catch (\Exception $ex) { - $this->logger->logFailure($ex->getMessage()); - return false; } return true; } diff --git a/PHPCI/Plugin/Pdepend.php b/PHPCI/Plugin/Pdepend.php index 98c63093..9bd7b73b 100644 --- a/PHPCI/Plugin/Pdepend.php +++ b/PHPCI/Plugin/Pdepend.php @@ -92,7 +92,7 @@ class Pdepend extends AbstractExecutingPlugin $config = $this->phpci->getSystemConfig('phpci'); if ($success) { - $this->logger->logSuccess( + $this->logger->notice( sprintf( "Pdepend successful. You can use %s\n, ![Chart](%s \"Pdepend Chart\")\n and ![Pyramid](%s \"Pdepend Pyramid\")\n diff --git a/PHPCI/Plugin/Pgsql.php b/PHPCI/Plugin/Pgsql.php index 1f597bfc..8c276e80 100644 --- a/PHPCI/Plugin/Pgsql.php +++ b/PHPCI/Plugin/Pgsql.php @@ -65,16 +65,11 @@ class Pgsql extends AbstractInterpolatingPlugin */ public function execute() { - try { - $opts = array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION); - $pdo = new PDO('pgsql:host=' . $this->host, $this->user, $this->pass, $opts); + $opts = array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION); + $pdo = new PDO('pgsql:host=' . $this->host, $this->user, $this->pass, $opts); - foreach ($this->queries as $query) { - $pdo->query($this->interpolator->interpolate($query)); - } - } catch (\Exception $ex) { - $this->logger->logFailure($ex->getMessage()); - return false; + foreach ($this->queries as $query) { + $pdo->query($this->interpolator->interpolate($query)); } return true; } diff --git a/PHPCI/Plugin/Phar.php b/PHPCI/Plugin/Phar.php index 139c6ac7..e140ae8b 100644 --- a/PHPCI/Plugin/Phar.php +++ b/PHPCI/Plugin/Phar.php @@ -200,23 +200,14 @@ class Phar extends AbstractPlugin */ public function execute() { - $success = false; + $phar = new PHPPhar($this->getDirectory() . '/' . $this->getFilename(), 0, $this->getFilename()); + $phar->buildFromDirectory($this->getPHPCI()->buildPath, $this->getRegExp()); - try { - $phar = new PHPPhar($this->getDirectory() . '/' . $this->getFilename(), 0, $this->getFilename()); - $phar->buildFromDirectory($this->getPHPCI()->buildPath, $this->getRegExp()); - - $stub = $this->getStubContent(); - if ($stub) { - $phar->setStub($stub); - } - - $success = true; - } catch (Exception $e) { - $this->getPHPCI()->log(Lang::get('phar_internal_error')); - $this->getPHPCI()->log($e->getMessage()); + $stub = $this->getStubContent(); + if ($stub) { + $phar->setStub($stub); } - return $success; + return true; } } diff --git a/PHPCI/Plugin/PhpCodeSniffer.php b/PHPCI/Plugin/PhpCodeSniffer.php index ad91bd00..073db941 100644 --- a/PHPCI/Plugin/PhpCodeSniffer.php +++ b/PHPCI/Plugin/PhpCodeSniffer.php @@ -197,7 +197,7 @@ class PhpCodeSniffer extends AbstractExecutingPlugin implements PHPCI\ZeroConfig $data = json_decode(trim($output), true); if (!is_array($data)) { - $this->logger->log($output); + $this->logger->error($output); throw new \Exception(PHPCI\Helper\Lang::get('could_not_process_report')); } diff --git a/PHPCI/Plugin/PhpCpd.php b/PHPCI/Plugin/PhpCpd.php index f4647be9..dcc37b02 100644 --- a/PHPCI/Plugin/PhpCpd.php +++ b/PHPCI/Plugin/PhpCpd.php @@ -103,7 +103,7 @@ class PhpCpd extends AbstractExecutingPlugin $xml = simplexml_load_string($xmlString); if ($xml === false) { - $this->logger->log($xmlString); + $this->logger->error($xmlString); throw new \Exception(Lang::get('could_not_process_report')); } diff --git a/PHPCI/Plugin/PhpMessDetector.php b/PHPCI/Plugin/PhpMessDetector.php index 4b9f873d..1f6f1d41 100644 --- a/PHPCI/Plugin/PhpMessDetector.php +++ b/PHPCI/Plugin/PhpMessDetector.php @@ -128,7 +128,7 @@ class PhpMessDetector extends AbstractExecutingPlugin implements PHPCI\ZeroConfi $xml = simplexml_load_string($xmlString); if ($xml === false) { - $this->logger->log($xmlString); + $this->logger->error($xmlString); throw new \Exception('Could not process PHPMD report XML.'); } diff --git a/PHPCI/Plugin/PhpUnit.php b/PHPCI/Plugin/PhpUnit.php index ff430e0d..bd52e83d 100644 --- a/PHPCI/Plugin/PhpUnit.php +++ b/PHPCI/Plugin/PhpUnit.php @@ -156,7 +156,6 @@ class PhpUnit extends AbstractInterpolatingPlugin implements PHPCI\ZeroConfigPlu $tapParser = new TapParser($tapString); $output = $tapParser->parse(); } catch (\Exception $ex) { - $this->logger->logFailure($tapString); throw $ex; } diff --git a/PHPCI/Plugin/Sqlite.php b/PHPCI/Plugin/Sqlite.php index 1094f3e9..24fa6471 100644 --- a/PHPCI/Plugin/Sqlite.php +++ b/PHPCI/Plugin/Sqlite.php @@ -53,16 +53,11 @@ class Sqlite extends AbstractPlugin */ public function execute() { - try { - $opts = array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION); - $pdo = new PDO('sqlite:' . $this->path, $opts); + $opts = array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION); + $pdo = new PDO('sqlite:' . $this->path, $opts); - foreach ($this->queries as $query) { - $pdo->query($this->interpolator->interpolate($query)); - } - } catch (\Exception $ex) { - $this->logger->logFailure($ex->getMessage()); - return false; + foreach ($this->queries as $query) { + $pdo->query($this->interpolator->interpolate($query)); } return true; } diff --git a/PHPCI/Plugin/TechnicalDebt.php b/PHPCI/Plugin/TechnicalDebt.php index 24d222ae..7925ae58 100755 --- a/PHPCI/Plugin/TechnicalDebt.php +++ b/PHPCI/Plugin/TechnicalDebt.php @@ -111,7 +111,7 @@ class TechnicalDebt extends AbstractPlugin implements PHPCI\ZeroConfigPlugin list($errorCount, $data) = $this->getErrorList(); - $this->logger->log("Found $errorCount instances of " . implode(', ', $this->searches)); + $this->logger->notice("Found $errorCount instances of " . implode(', ', $this->searches)); $this->build->storeMeta('technical_debt-warnings', $errorCount); $this->build->storeMeta('technical_debt-data', $data); diff --git a/PHPCI/Plugin/Util/Executor.php b/PHPCI/Plugin/Util/Executor.php index 8c74707e..3e48469e 100644 --- a/PHPCI/Plugin/Util/Executor.php +++ b/PHPCI/Plugin/Util/Executor.php @@ -48,8 +48,10 @@ class Executor foreach ($config[$stage] as $plugin => $options) { $this->logger->log(Lang::get('running_plugin', $plugin)); + $settings = isset($config['build_settings'][$plugin]) ? $config['build_settings'][$plugin] : array(); + // Try and execute it: - if ($this->executePlugin($plugin, $options)) { + if ($this->executePlugin($plugin, $options, $settings)) { // Execution was successful: $this->logger->logSuccess(Lang::get('plugin_success')); } elseif ($stage == 'setup') { @@ -72,8 +74,34 @@ class Executor /** * Executes a given plugin, with options and returns the result. + * + * @param string $plugin Plugin name or class. + * @param array $options Stage options. + * @param array $settings Common options. + * @return boolean true if plugin ended successfully, false if something went wrong. */ - public function executePlugin($plugin, $options) + public function executePlugin($plugin, array $options, array $settings) + { + try { + $obj = $this->buildPlugin($plugin); + $obj->setCommonSettings($settings); + $obj->setOptions($options); + + return $obj->execute(); + } catch (\Exception $ex) { + $this->logger->logFailure(Lang::get('exception') . $ex->getMessage(), $ex); + return false; + } + } + + /** + * Create a plugin instance of the given name. + * + * @param string $plugin Plugin class or name + * @return \PHPCI\Plugin + * @throws \Exception If the plugin could not be created. + */ + protected function buildPlugin($plugin) { // Any plugin name without a namespace separator is a PHPCI built in plugin // if not we assume it's a fully name-spaced class name that implements the plugin interface. @@ -87,24 +115,9 @@ class Executor } if (!class_exists($class)) { - $this->logger->logFailure(Lang::get('plugin_missing', $plugin)); - return false; + throw new \Exception(Lang::get('plugin_missing', $plugin)); } - $rtn = true; - - // Try running it: - try { - $obj = $this->pluginFactory->buildPlugin($class, $options); - - if (!$obj->execute()) { - $rtn = false; - } - } catch (\Exception $ex) { - $this->logger->logFailure(Lang::get('exception') . $ex->getMessage(), $ex); - $rtn = false; - } - - return $rtn; + return $this->pluginFactory->buildPlugin($class); } } diff --git a/PHPCI/Plugin/Util/Factory.php b/PHPCI/Plugin/Util/Factory.php index 76de2b7d..0aeb0c8f 100644 --- a/PHPCI/Plugin/Util/Factory.php +++ b/PHPCI/Plugin/Util/Factory.php @@ -2,6 +2,7 @@ namespace PHPCI\Plugin\Util; +use Psr\Log\LoggerAwareInterface; /** * Plugin Factory - Loads Plugins and passes required dependencies. * @package PHPCI\Plugin\Util @@ -104,6 +105,14 @@ class Factory $plugin = $reflectedPlugin->newInstance(); } + if ($plugin instanceof InterpolatorAwareInterface) { + $plugin->setInterpolator($this->getResourceFor('PHPCI\Helper\BuildInterpolator')); + } + + if ($plugin instanceof LoggerAwareInterface) { + $plugin->setLogger($this->getResourceFor('Psr\Log\LoggerInterface')); + } + return $plugin; } diff --git a/PHPCI/Plugin/Util/InterpolatorAwareInterface.php b/PHPCI/Plugin/Util/InterpolatorAwareInterface.php new file mode 100644 index 00000000..76f07d90 --- /dev/null +++ b/PHPCI/Plugin/Util/InterpolatorAwareInterface.php @@ -0,0 +1,27 @@ + + */ +interface InterpolatorAwareInterface +{ + /** + * Sets the BuildInterpolator. + * + * @param BuildInterpolator $interpolator + */ + public function setInterpolator(BuildInterpolator $interpolator); +}