Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize CommandRunner and Commands #4683

Merged
merged 1 commit into from
May 17, 2021
Merged

Optimize CommandRunner and Commands #4683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 12 additions & 18 deletions system/CLI/BaseCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,16 @@
use Throwable;

/**
* Class BaseCommand
* BaseCommand is the base class used in creating CLI commands.
*
* @property string $group
* @property string $name
* @property string $usage
* @property string $description
* @property array $options
* @property array $arguments
* @property LoggerInterface $logger
* @property Commands $commands
*/
abstract class BaseCommand
{
Expand Down Expand Up @@ -78,7 +87,6 @@ abstract class BaseCommand
*/
protected $commands;

//--------------------------------------------------------------------
/**
* BaseCommand constructor.
*
Expand All @@ -91,8 +99,6 @@ public function __construct(LoggerInterface $logger, Commands $commands)
$this->commands = $commands;
}

//--------------------------------------------------------------------

/**
* Actually execute a command.
* This has to be over-ridden in any concrete implementation.
Expand All @@ -101,8 +107,6 @@ public function __construct(LoggerInterface $logger, Commands $commands)
*/
abstract public function run(array $params);

//--------------------------------------------------------------------

/**
* Can be used by a command to run other commands.
*
Expand All @@ -117,8 +121,6 @@ protected function call(string $command, array $params = [])
return $this->commands->run($command, $params);
}

//--------------------------------------------------------------------

/**
* A simple method to display an error with line/file, in child commands.
*
Expand All @@ -132,8 +134,6 @@ protected function showError(Throwable $e)
require APPPATH . 'Views/errors/cli/error_exception.php';
}

//--------------------------------------------------------------------

/**
* Show Help includes (Usage, Arguments, Description, Options).
*/
Expand Down Expand Up @@ -189,8 +189,6 @@ public function showHelp()
}
}

//--------------------------------------------------------------------

/**
* Pads our string out so that all titles are the same length to nicely line up descriptions.
*
Expand All @@ -208,8 +206,6 @@ public function setPad(string $item, int $max, int $extra = 2, int $indent = 0):
return str_pad(str_repeat(' ', $indent) . $item, $max);
}

//--------------------------------------------------------------------

/**
* Get pad for $key => $value array output
*
Expand All @@ -225,15 +221,15 @@ public function setPad(string $item, int $max, int $extra = 2, int $indent = 0):
public function getPad(array $array, int $pad): int
{
$max = 0;

foreach (array_keys($array) as $key)
{
$max = max($max, strlen($key));
}

return $max + $pad;
}

//--------------------------------------------------------------------

/**
* Makes it simple to access our protected properties.
*
Expand All @@ -251,8 +247,6 @@ public function __get(string $key)
return null;
}

//--------------------------------------------------------------------

/**
* Makes it simple to check our protected properties.
*
Expand Down
15 changes: 3 additions & 12 deletions system/CLI/CommandRunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@
class CommandRunner extends Controller
{
/**
* The Command Manager
* Instance of class managing the collection of commands
*
* @var Commands
*/
protected $commands;

//--------------------------------------------------------------------

/**
* Constructor
*/
Expand Down Expand Up @@ -58,8 +56,6 @@ public function _remap($method, ...$params)
return $this->index($params);
}

//--------------------------------------------------------------------

/**
* Default command.
*
Expand All @@ -70,14 +66,9 @@ public function _remap($method, ...$params)
*/
public function index(array $params)
{
$command = array_shift($params);

if (is_null($command))
{
$command = 'list';
}
$command = array_shift($params) ?? 'list';

return service('commands')->run($command, $params);
return $this->commands->run($command, $params);
}

/**
Expand Down
38 changes: 17 additions & 21 deletions system/CLI/Commands.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@

namespace CodeIgniter\CLI;

use CodeIgniter\Autoloader\FileLocator;
use CodeIgniter\Log\Logger;
use Config\Services;
use ReflectionClass;
use ReflectionException;

/**
* Class Commands
*
* Core functionality for running, listing, etc commands.
*/
class Commands
Expand All @@ -44,7 +42,8 @@ class Commands
*/
public function __construct($logger = null)
{
$this->logger = $logger ?? Services::logger();
$this->logger = $logger ?? service('logger');
$this->discoverCommands();
}

/**
Expand All @@ -55,8 +54,6 @@ public function __construct($logger = null)
*/
public function run(string $command, array $params)
{
$this->discoverCommands();

if (! $this->verifyCommand($command, $this->commands))
{
return;
Expand All @@ -77,39 +74,39 @@ public function run(string $command, array $params)
*/
public function getCommands()
{
$this->discoverCommands();

return $this->commands;
}

/**
* Discovers all commands in the framework and within user code,
* and collects instances of them to work with.
*
* @return void
*/
public function discoverCommands()
{
if (! empty($this->commands))
if ($this->commands !== [])
{
return;
}

$files = service('locator')->listFiles('Commands/');
/** @var FileLocator $locator */
$locator = service('locator');
$files = $locator->listFiles('Commands/');

// If no matching command files were found, bail
if (empty($files))
// This should never happen in unit testing.
if ($files === [])
{
// This should never happen in unit testing.
// if it does, we have far bigger problems!
// @codeCoverageIgnoreStart
return;
// @codeCoverageIgnoreEnd
return; // @codeCoverageIgnore
}

// Loop over each file checking to see if a command with that
// alias exists in the class. If so, return it. Otherwise, try the next.
// alias exists in the class.
foreach ($files as $file)
{
$className = Services::locator()->findQualifiedNameFromPath($file);
$className = $locator->findQualifiedNameFromPath($file);

if (empty($className) || ! class_exists($className))
{
continue;
Expand All @@ -124,10 +121,10 @@ public function discoverCommands()
continue;
}

/** @var BaseCommand $class */
$class = new $className($this->logger, $this);

// Store it!
if (! is_null($class->group))
if (isset($class->group))
{
$this->commands[$class->name] = [
'class' => $className,
Expand All @@ -137,7 +134,6 @@ public function discoverCommands()
];
}

$class = null;
unset($class);
}
catch (ReflectionException $e)
Expand Down
Loading