From 32d3f4f2af91d7db7b3de708c116991cfaa70e25 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Thu, 26 Sep 2024 17:22:22 +1200 Subject: [PATCH] API Update API to reflect changes in CLI interaction (#93) --- README.md | 10 +- _config/crontask.yml | 13 +- src/Cli/CronTaskCommand.php | 117 +++++++++++++++ src/Controllers/CronTaskController.php | 192 ------------------------- src/Interfaces/CronTask.php | 4 +- tests/CronTaskControllerTest.php | 43 +++--- 6 files changed, 147 insertions(+), 232 deletions(-) create mode 100644 src/Cli/CronTaskCommand.php delete mode 100644 src/Controllers/CronTaskController.php diff --git a/README.md b/README.md index 216c300..570c280 100644 --- a/README.md +++ b/README.md @@ -76,14 +76,14 @@ class TestCron implements CronTask } ``` -Run `vendor/bin/sake dev/build flush=1` to make Silverstripe aware of the new +Run `vendor/bin/sake db:build --flush` to make Silverstripe aware of the new module. Then execute the crontask controller, it's preferable you do this via the CLI since that is how the server will execute it. ``` -vendor/bin/sake dev/cron +vendor/bin/sake cron-task ``` Server configuration @@ -96,7 +96,7 @@ most common way is by adding a file to the `/etc/cron.d/` directory. First find the correct command to execute, for example: ``` -/usr/bin/php /path/to/silverstripe/docroot/vendor/bin/sake dev/cron +/usr/bin/php /path/to/silverstripe/docroot/vendor/bin/sake cron-task ``` Then find out which user the webserver is running on, for example `www-data`. @@ -110,7 +110,7 @@ sudo vim /etc/cron.d/silverstripe-crontask The content of that file should be: ``` -* * * * * www-data /usr/bin/php /path/to/silverstripe/docroot/vendor/bin/sake dev/cron +* * * * * www-data /usr/bin/php /path/to/silverstripe/docroot/vendor/bin/sake cron-task ``` This will run every minute as the www-data user and check if there are any @@ -122,7 +122,7 @@ adding quiet=1 - for example ``` MAILTO=admin@example.com -* * * * * www-data /usr/bin/php /path/to/silverstripe/docroot/framework/cli-script.php dev/cron quiet=1 +* * * * * www-data /usr/bin/php /path/to/silverstripe/docroot/vendor/bin/sake cron-task --quiet ``` **Warning**: Observe that the crontask module doesn't do any checking. If diff --git a/_config/crontask.yml b/_config/crontask.yml index 0b1ee82..404aa27 100644 --- a/_config/crontask.yml +++ b/_config/crontask.yml @@ -1,13 +1,6 @@ --- Name: crontask --- -SilverStripe\Dev\DevelopmentAdmin: - registered_controllers: - cron: - controller: SilverStripe\CronTask\Controllers\CronTaskController - links: - cron: 'Run registered SilverStripe cron tasks' - -SilverStripe\Control\Director: - rules: - 'dev/cron/$Action': SilverStripe\CronTask\Controllers\CronTaskController +SilverStripe\Cli\Sake: + commands: + cron-task: 'SilverStripe\CronTask\Cli\CronTaskCommand' diff --git a/src/Cli/CronTaskCommand.php b/src/Cli/CronTaskCommand.php new file mode 100644 index 0000000..f69fbaa --- /dev/null +++ b/src/Cli/CronTaskCommand.php @@ -0,0 +1,117 @@ +getValue()); + if ($cron->isDue($now)) { + if (empty($status) || empty($status->LastRun)) { + return true; + } + // In case this process is invoked twice in one minute, supress subsequent executions + $lastRun = new DateTime($status->LastRun); + return $lastRun->format('Y-m-d H:i') != $now->format('Y-m-d H:i'); + } + + // If this is the first time this task is ever checked, no way to detect postponed execution + if (empty($status) || empty($status->LastChecked)) { + return false; + } + + // Determine if we have passed the last expected run time + $nextExpectedDate = $cron->getNextRunDate($status->LastChecked); + return $nextExpectedDate <= $now; + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + // Check each task + $tasks = ClassInfo::implementorsOf(CronTask::class); + if (empty($tasks)) { + $this->output( + _t( + CronTaskCommand::class . '.NO_IMPLEMENTERS', + 'There are no implementators of CronTask to run' + ), + $output + ); + return Command::SUCCESS; + } + foreach ($tasks as $subclass) { + $task = Injector::inst()->create($subclass); + // falsey schedule = don't run task + if ($task->getSchedule()) { + $this->runTask($task, $output); + } + } + return Command::SUCCESS; + } + + /** + * Checks and runs a single CronTask + */ + public function runTask(CronTask $task, OutputInterface $output): void + { + $cron = CronExpression::factory($task->getSchedule()); + $isDue = $this->isTaskDue($task, $cron); + // Update status of this task prior to execution in case of interruption + CronTaskStatus::update_status(get_class($task), $isDue); + if ($isDue) { + $this->output( + _t( + CronTaskCommand::class . '.WILL_START_NOW', + '{task} will start now.', + ['task' => get_class($task)] + ), + $output + ); + $task->process(); + } else { + $this->output( + _t( + CronTaskCommand::class . '.WILL_RUN_AT', + '{task} will run at {time}.', + ['task' => get_class($task), 'time' => $cron->getNextRunDate()->format('Y-m-d H:i:s')] + ), + $output, + OutputInterface::VERBOSITY_VERBOSE + ); + } + } + + /** + * Output a message including the timestamp + */ + public function output(string $message, OutputInterface $output, int $verbosity = 0): void + { + $timestamp = DBDatetime::now()->Rfc2822(); + $output->writeln($timestamp . ' - ' . $message, $verbosity); + } +} diff --git a/src/Controllers/CronTaskController.php b/src/Controllers/CronTaskController.php deleted file mode 100644 index 8690be0..0000000 --- a/src/Controllers/CronTaskController.php +++ /dev/null @@ -1,192 +0,0 @@ -verbosity = (int) $verbosity; - } - - /** - * Checks for cli or admin permissions and include the library - * - * @throws Exception - */ - public function init() - { - parent::init(); - - // Unless called from the command line, we need ADMIN privileges - if (!Director::is_cli() && !Permission::check('ADMIN')) { - Security::permissionFailure(); - } - - // set quiet flag based on CLI parameter - if ($this->getRequest()->getVar('quiet')) { - $this->setVerbosity(0); - } - if ($this->getRequest()->getVar('debug')) { - $this->setVerbosity(2); - } - } - - /** - * Determine if a task should be run - * - * @param CronTask $task - * @param CronExpression $cron - */ - public function isTaskDue(CronTask $task, CronExpression $cron) - { - // Get last run status - $status = CronTaskStatus::get_status(get_class($task)); - - // If the cron is due immediately, then run it - $now = new DateTime(DBDatetime::now()->getValue()); - if ($cron->isDue($now)) { - if (empty($status) || empty($status->LastRun)) { - return true; - } - // In case this process is invoked twice in one minute, supress subsequent executions - $lastRun = new DateTime($status->LastRun); - return $lastRun->format('Y-m-d H:i') != $now->format('Y-m-d H:i'); - } - - // If this is the first time this task is ever checked, no way to detect postponed execution - if (empty($status) || empty($status->LastChecked)) { - return false; - } - - // Determine if we have passed the last expected run time - $nextExpectedDate = $cron->getNextRunDate($status->LastChecked); - return $nextExpectedDate <= $now; - } - - /** - * Default controller action - * - * @param HTTPRequest $request - */ - public function index(HTTPRequest $request) - { - // Show more debug info with ?debug=1 - $isDebug = (bool)$request->getVar('debug'); - - // Check each task - $tasks = ClassInfo::implementorsOf(CronTask::class); - if (empty($tasks)) { - $this->output( - _t( - CronTaskController::class . '.NO_IMPLEMENTERS', - 'There are no implementators of CronTask to run' - ), - 2 - ); - return; - } - foreach ($tasks as $subclass) { - $task = Injector::inst()->create($subclass); - // falsey schedule = don't run task - if ($task->getSchedule()) { - $this->runTask($task, $isDebug); - } - } - } - - /** - * Checks and runs a single CronTask - * - * @param CronTask $task - */ - public function runTask(CronTask $task) - { - $cron = CronExpression::factory($task->getSchedule()); - $isDue = $this->isTaskDue($task, $cron); - // Update status of this task prior to execution in case of interruption - CronTaskStatus::update_status(get_class($task), $isDue); - if ($isDue) { - $this->output(_t( - CronTaskController::class . '.WILL_START_NOW', - '{task} will start now.', - ['task' => get_class($task)] - )); - $task->process(); - } else { - $this->output( - _t( - CronTaskController::class . '.WILL_RUN_AT', - '{task} will run at {time}.', - ['task' => get_class($task), 'time' => $cron->getNextRunDate()->format('Y-m-d H:i:s')] - ), - 2 - ); - } - } - - /** - * Output a message to the browser or CLI - * - * @param string $message - */ - public function output($message, $minVerbosity = 1) - { - if ($this->verbosity < $minVerbosity) { - return; - } - $timestamp = DBDatetime::now()->Rfc2822(); - if (Director::is_cli()) { - echo $timestamp . ' - ' . $message . PHP_EOL; - } else { - echo Convert::raw2xml($timestamp . ' - ' . $message) . '
' . PHP_EOL; - } - } -} diff --git a/src/Interfaces/CronTask.php b/src/Interfaces/CronTask.php index 40ac4cd..a980454 100644 --- a/src/Interfaces/CronTask.php +++ b/src/Interfaces/CronTask.php @@ -3,8 +3,8 @@ namespace SilverStripe\CronTask\Interfaces; /** - * By implementing this interface a /dev/cron will be able to start in on the - * expression that you return frmo getSchedule(); + * By implementing this interface `sake cron-task` will be able to start in on the + * expression that you return from getSchedule(); * * @package crontask */ diff --git a/tests/CronTaskControllerTest.php b/tests/CronTaskControllerTest.php index 3298e95..fdad6a3 100644 --- a/tests/CronTaskControllerTest.php +++ b/tests/CronTaskControllerTest.php @@ -3,12 +3,13 @@ namespace SilverStripe\CronTask\Tests; use Cron\CronExpression; -use SilverStripe\CronTask\Controllers\CronTask; -use SilverStripe\CronTask\Controllers\CronTaskController; +use SilverStripe\CronTask\Cli\CronTaskCommand; use SilverStripe\CronTask\CronTaskStatus; use SilverStripe\Dev\FunctionalTest; use SilverStripe\ORM\FieldType\DBDate; use SilverStripe\ORM\FieldType\DBDatetime; +use Symfony\Component\Console\Input\ArrayInput; +use Symfony\Component\Console\Output\BufferedOutput; /** * @package crontask @@ -35,7 +36,7 @@ protected function setUp(): void */ public function testIsTaskDue() { - $runner = CronTaskController::create(); + $runner = new CronTaskCommand(); $task = new CronTaskTest\TestCron(); $cron = CronExpression::factory($task->getSchedule()); @@ -71,57 +72,53 @@ public function testIsTaskDue() */ public function testRunTask() { - $runner = CronTaskController::create(); - $runner->setVerbosity(0); + $runner = new CronTaskCommand(); + $output = new BufferedOutput(); $task = new CronTaskTest\TestCron(); // Assuming first run, match the exact time (seconds are ignored) $this->assertEquals(0, CronTaskTest\TestCron::$times_run); DBDatetime::set_mock_now('2010-06-20 13:00:10'); - $runner->runTask($task); + $runner->runTask($task, $output); $this->assertEquals(1, CronTaskTest\TestCron::$times_run); // Test that re-requsting the task in the same minute do not retrigger another run DBDatetime::set_mock_now('2010-06-20 13:00:40'); - $runner->runTask($task); + $runner->runTask($task, $output); $this->assertEquals(1, CronTaskTest\TestCron::$times_run); // Job prior to next hour mark should not run DBDatetime::set_mock_now('2010-06-20 13:40:00'); - $runner->runTask($task); + $runner->runTask($task, $output); $this->assertEquals(1, CronTaskTest\TestCron::$times_run); // Jobs just after the next hour mark should run DBDatetime::set_mock_now('2010-06-20 14:10:00'); - $runner->runTask($task); + $runner->runTask($task, $output); $this->assertEquals(2, CronTaskTest\TestCron::$times_run); // Jobs run on the exact next expected date should run DBDatetime::set_mock_now('2010-06-20 15:00:00'); - $runner->runTask($task); + $runner->runTask($task, $output); $this->assertEquals(3, CronTaskTest\TestCron::$times_run); // Jobs somehow delayed a whole day should be run DBDatetime::set_mock_now('2010-06-21 13:40:00'); - $runner->runTask($task); + $runner->runTask($task, $output); $this->assertEquals(4, CronTaskTest\TestCron::$times_run); } - // normal cron output includes the current date/time - we check for that // the exact output here could vary depending on what other modules are installed public function testDefaultQuietFlagOutput() { - $this->loginWithPermission('ADMIN'); - $this->expectOutputRegex('#' . DBDatetime::now()->Format(DBDate::ISO_DATE) . '#'); - $this->get('dev/cron?debug=1'); - } - - // with the flag set we want no output - public function testQuietFlagOnOutput() - { - $this->loginWithPermission('ADMIN'); - $this->expectOutputString(''); - $this->get('dev/cron?quiet=1'); + $input = new ArrayInput([]); + $output = new BufferedOutput(); + $command = new CronTaskCommand(); + $command->run($input, $output); + $this->assertMatchesRegularExpression( + '#' . DBDatetime::now()->Format(DBDate::ISO_DATE) . '#', + $output->fetch() + ); } }