From 3a232840a5550d53392b11a01f7559e229da93a8 Mon Sep 17 00:00:00 2001 From: Jamie Stamp Date: Wed, 7 Oct 2020 17:27:35 +0100 Subject: [PATCH] MDL-65843 tasks: Allow schedules to be overridden in config --- admin/tool/task/renderer.php | 11 ++- admin/tool/task/scheduledtasks.php | 2 +- config-dist.php | 40 +++++++++ lib/classes/task/manager.php | 109 ++++++++++++++++++++++-- lib/classes/task/scheduled_task.php | 19 +++++ lib/tests/scheduled_task_test.php | 123 ++++++++++++++++++++++++++++ 6 files changed, 294 insertions(+), 10 deletions(-) diff --git a/admin/tool/task/renderer.php b/admin/tool/task/renderer.php index ec85231c5a3..d69092470d3 100644 --- a/admin/tool/task/renderer.php +++ b/admin/tool/task/renderer.php @@ -82,7 +82,7 @@ class tool_task_renderer extends plugin_renderer_base { $defaulttask = \core\task\manager::get_default_scheduled_task($classname, false); $customised = $task->is_customised() ? $no : $yes; - if (empty($CFG->preventscheduledtaskchanges)) { + if (empty($CFG->preventscheduledtaskchanges) && !$task->is_overridden()) { $configureurl = new moodle_url('/admin/tool/task/scheduledtasks.php', ['action' => 'edit', 'task' => $classname]); $editlink = $this->output->action_icon($configureurl, new pix_icon('t/edit', @@ -100,8 +100,13 @@ class tool_task_renderer extends plugin_renderer_base { )); } - $namecell = new html_table_cell($task->get_name() . "\n" . - html_writer::span('\\' . $classname, 'task-class text-ltr')); + $namecellcontent = $task->get_name() . "\n" . + html_writer::span('\\' . $classname, 'task-class text-ltr'); + if ($task->is_overridden()) { + // Let the user know the scheduled task is defined in config. + $namecellcontent .= "\n" . html_writer::div(get_string('configoverride', 'admin'), 'alert-info'); + } + $namecell = new html_table_cell($namecellcontent); $namecell->header = true; $plugininfo = core_plugin_manager::instance()->get_plugin_info($task->get_component()); diff --git a/admin/tool/task/scheduledtasks.php b/admin/tool/task/scheduledtasks.php index 243039e6b6d..85a4055380e 100644 --- a/admin/tool/task/scheduledtasks.php +++ b/admin/tool/task/scheduledtasks.php @@ -53,7 +53,7 @@ if ($task) { $renderer = $PAGE->get_renderer('tool_task'); -if ($mform && ($mform->is_cancelled() || !empty($CFG->preventscheduledtaskchanges))) { +if ($mform && ($mform->is_cancelled() || !empty($CFG->preventscheduledtaskchanges) || $task->is_overridden())) { redirect($nexturl); } else if ($action == 'edit' && empty($CFG->preventscheduledtaskchanges)) { diff --git a/config-dist.php b/config-dist.php index 083f2174a24..e35e19b7649 100644 --- a/config-dist.php +++ b/config-dist.php @@ -1084,6 +1084,46 @@ $CFG->admin = 'admin'; // $CFG->alternative_cache_factory_class = 'tool_alternativecache_cache_factory'; // //========================================================================= +// 17. SCHEDULED TASK OVERRIDES +//========================================================================= +// +// It is now possible to define scheduled tasks directly within config. +// The overridden value will take precedence over the values that have been set VIA the UI from the +// next time the task is run. +// +// Tasks are configured as an array of tasks that can override a task's schedule, as well as setting +// the task as disabled. I.e: +// +// $CFG->scheduled_tasks = [ +// '\local_plugin\task\my_task' => [ +// 'schedule' => '*/15 0 0 0 0', +// 'disabled' => 0, +// ], +// ]; +// +// The format for the schedule definition is: '{minute} {hour} {day} {dayofweek} {month}'. +// +// The classname of the task also supports wildcards: +// +// $CFG->scheduled_tasks = [ +// '\local_plugin\*' => [ +// 'schedule' => '*/15 0 0 0 0', +// 'disabled' => 0, +// ], +// '*' => [ +// 'schedule' => '0 0 0 0 0', +// 'disabled' => 0, +// ], +// ]; +// +// In this example, any task classnames matching '\local_plugin\*' would match the first rule and +// use that schedule the next time the task runs. Note that even though the 'local_plugin' tasks match +// the second rule as well, the highest rule takes precedence. Therefore, the second rule would be +// applied to all tasks, except for tasks within '\local_plugin\'. +// +// When the full classname is used, this rule always takes priority over any wildcard rules. +// +//========================================================================= // ALL DONE! To continue installation, visit your main page with a browser //========================================================================= diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index 08b624222ac..559c68f7676 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -67,7 +67,7 @@ class manager { foreach ($tasks as $task) { $record = (object) $task; - $scheduledtask = self::scheduled_task_from_record($record, $expandr); + $scheduledtask = self::scheduled_task_from_record($record, $expandr, false); // Safety check in case the task in the DB does not match a real class (maybe something was uninstalled). if ($scheduledtask) { $scheduledtask->set_component($componentname); @@ -338,9 +338,10 @@ class manager { * @param \stdClass $record * @param bool $expandr - if true (default) an 'R' value in a time is expanded to an appropriate int. * If false, they are left as 'R' + * @param bool $override - if true loads overridden settings from config. * @return \core\task\scheduled_task|false */ - public static function scheduled_task_from_record($record, $expandr = true) { + public static function scheduled_task_from_record($record, $expandr = true, $override = true) { $classname = self::get_canonical_class_name($record->classname); if (!class_exists($classname)) { debugging("Failed to load task: " . $classname, DEBUG_DEVELOPER); @@ -348,6 +349,12 @@ class manager { } /** @var \core\task\scheduled_task $task */ $task = new $classname; + + if ($override) { + // Update values with those defined in the config, if any are set. + $record = self::get_record_with_config_overrides($record); + } + if (isset($record->lastruntime)) { $task->set_last_run_time($record->lastruntime); } @@ -391,6 +398,7 @@ class manager { if (isset($record->pid)) { $task->set_pid($record->pid); } + $task->set_overridden(self::scheduled_task_has_override($classname)); return $task; } @@ -701,10 +709,12 @@ class manager { } } - // Make sure the task data is unchanged. - if (!$DB->record_exists('task_scheduled', (array) $record)) { - $lock->release(); - continue; + if (!self::scheduled_task_has_override($record->classname)) { + // Make sure the task data is unchanged unless an override is being used. + if (!$DB->record_exists('task_scheduled', (array)$record)) { + $lock->release(); + continue; + } } // The global cron lock is under the most contention so request it @@ -1106,4 +1116,91 @@ class manager { return true; } + + /** + * For a given scheduled task record, this method will check to see if any overrides have + * been applied in config and return a copy of the record with any overridden values. + * + * The format of the config value is: + * $CFG->scheduled_tasks = array( + * '$classname' => array( + * 'schedule' => '* * * * *', + * 'disabled' => 1, + * ), + * ); + * + * Where $classname is the value of the task's classname, i.e. '\core\task\grade_cron_task'. + * + * @param \stdClass $record scheduled task record + * @return \stdClass scheduled task with any configured overrides + */ + protected static function get_record_with_config_overrides(\stdClass $record): \stdClass { + global $CFG; + + $scheduledtaskkey = self::scheduled_task_get_override_key($record->classname); + $overriddenrecord = $record; + + if ($scheduledtaskkey) { + $overriddenrecord->customised = true; + $taskconfig = $CFG->scheduled_tasks[$scheduledtaskkey]; + + if (isset($taskconfig['disabled'])) { + $overriddenrecord->disabled = $taskconfig['disabled']; + } + if (isset($taskconfig['schedule'])) { + list ( + $overriddenrecord->minute, + $overriddenrecord->hour, + $overriddenrecord->day, + $overriddenrecord->dayofweek, + $overriddenrecord->month) = explode(' ', $taskconfig['schedule']); + } + } + + return $overriddenrecord; + } + + /** + * This checks whether or not there is a value set in config + * for a scheduled task. + * + * @param string $classname Scheduled task's classname + * @return bool true if there is an entry in config + */ + public static function scheduled_task_has_override(string $classname): bool { + return self::scheduled_task_get_override_key($classname) !== null; + } + + /** + * Get the key within the scheduled tasks config object that + * for a classname. + * + * @param string $classname the scheduled task classname to find + * @return string the key if found, otherwise null + */ + public static function scheduled_task_get_override_key(string $classname): ?string { + global $CFG; + + if (isset($CFG->scheduled_tasks)) { + // Firstly, attempt to get a match against the full classname. + if (isset($CFG->scheduled_tasks[$classname])) { + return $classname; + } + + // Check to see if there is a wildcard matching the classname. + foreach (array_keys($CFG->scheduled_tasks) as $key) { + if (strpos($key, '*') === false) { + continue; + } + + $pattern = '/' . str_replace('\\', '\\\\', str_replace('*', '.*', $key)) . '/'; + + if (preg_match($pattern, $classname)) { + return $key; + } + } + } + + return null; + } } diff --git a/lib/classes/task/scheduled_task.php b/lib/classes/task/scheduled_task.php index 492e4c502e8..04fbd1feba6 100644 --- a/lib/classes/task/scheduled_task.php +++ b/lib/classes/task/scheduled_task.php @@ -67,6 +67,9 @@ abstract class scheduled_task extends task_base { /** @var boolean $customised - Has this task been changed from it's default schedule? */ private $customised = false; + /** @var boolean $overridden - Does the task have values set VIA config? */ + private $overridden = false; + /** @var int $disabled - Is this task disabled in cron? */ private $disabled = false; @@ -102,6 +105,22 @@ abstract class scheduled_task extends task_base { $this->customised = $customised; } + /** + * Has this task been changed from it's default config? + * @return bool + */ + public function is_overridden(): bool { + return $this->overridden; + } + + /** + * Set the overridden value. + * @param bool $overridden + */ + public function set_overridden(bool $overridden): void { + $this->overridden = $overridden; + } + /** * Setter for $minute. Accepts a special 'R' value * which will be translated to a random minute. diff --git a/lib/tests/scheduled_task_test.php b/lib/tests/scheduled_task_test.php index 56f6915a80d..a16e8e0bb48 100644 --- a/lib/tests/scheduled_task_test.php +++ b/lib/tests/scheduled_task_test.php @@ -517,6 +517,129 @@ class core_scheduled_task_testcase extends advanced_testcase { $this->assertLessThan($before + 70, $task->get_next_run_time()); } + /** + * Data provider for test_tool_health_category_find_missing_parents. + */ + public static function provider_schedule_overrides(): array { + return array( + array( + 'scheduled_tasks' => array( + '\core\task\scheduled_test_task' => array( + 'schedule' => '10 13 1 2 4', + 'disabled' => 0, + ), + '\core\task\scheduled_test2_task' => array( + 'schedule' => '* * * * *', + 'disabled' => 1, + ), + ), + 'task_full_classnames' => array( + '\core\task\scheduled_test_task', + '\core\task\scheduled_test2_task', + ), + 'expected' => array( + '\core\task\scheduled_test_task' => array( + 'min' => '10', + 'hour' => '13', + 'day' => '1', + 'week' => '2', + 'month' => '4', + 'disabled' => 0, + ), + '\core\task\scheduled_test2_task' => array( + 'min' => '*', + 'hour' => '*', + 'day' => '*', + 'week' => '*', + 'month' => '*', + 'disabled' => 1, + ), + ) + ), + array( + 'scheduled_tasks' => array( + '\core\task\*' => array( + 'schedule' => '1 2 3 4 5', + 'disabled' => 0, + ) + ), + 'task_full_classnames' => array( + '\core\task\scheduled_test_task', + '\core\task\scheduled_test2_task', + ), + 'expected' => array( + '\core\task\scheduled_test_task' => array( + 'min' => '1', + 'hour' => '2', + 'day' => '3', + 'week' => '4', + 'month' => '5', + 'disabled' => 0, + ), + '\core\task\scheduled_test2_task' => array( + 'min' => '1', + 'hour' => '2', + 'day' => '3', + 'week' => '4', + 'month' => '5', + 'disabled' => 0, + ), + ) + ) + ); + } + + + /** + * Test to ensure scheduled tasks are updated by values set in config. + * + * @param array $overrides + * @param array $tasks + * @param array $expected + * @dataProvider provider_schedule_overrides + */ + public function test_scheduled_task_override_values(array $overrides, array $tasks, array $expected): void { + global $CFG, $DB; + + $this->resetAfterTest(); + + // Add overrides to the config. + $CFG->scheduled_tasks = $overrides; + + // Set up test scheduled task record. + $record = new stdClass(); + $record->component = 'test_scheduled_task'; + + foreach ($tasks as $task) { + $record->classname = $task; + $DB->insert_record('task_scheduled', $record); + + $scheduledtask = \core\task\manager::get_scheduled_task($task); + $expectedresults = $expected[$task]; + + // Check that the task is actually overridden. + $this->assertTrue($scheduledtask->is_overridden(), 'Is overridden'); + + // Check minute is correct. + $this->assertEquals($expectedresults['min'], $scheduledtask->get_minute(), 'Minute check'); + + // Check day is correct. + $this->assertEquals($expectedresults['day'], $scheduledtask->get_day(), 'Day check'); + + // Check hour is correct. + $this->assertEquals($expectedresults['hour'], $scheduledtask->get_hour(), 'Hour check'); + + // Check week is correct. + $this->assertEquals($expectedresults['week'], $scheduledtask->get_day_of_week(), 'Day of week check'); + + // Check week is correct. + $this->assertEquals($expectedresults['month'], $scheduledtask->get_month(), 'Month check'); + + // Check to see if the task is disabled. + $this->assertEquals($expectedresults['disabled'], $scheduledtask->get_disabled(), 'Disabled check'); + } + } + /** * Assert that the specified tasks are equal. *