Skip to content

Commit

Permalink
MDL-65843 tasks: Allow schedules to be overridden in config
Browse files Browse the repository at this point in the history
  • Loading branch information
jamie-catalyst committed Nov 18, 2020
1 parent 6ef4e66 commit 3a23284
Show file tree
Hide file tree
Showing 6 changed files with 294 additions and 10 deletions.
11 changes: 8 additions & 3 deletions admin/tool/task/renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public function scheduled_tasks_table($tasks, $lastchanged = '') {
$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',
Expand All @@ -100,8 +100,13 @@ public function scheduled_tasks_table($tasks, $lastchanged = '') {
));
}

$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());
Expand Down
2 changes: 1 addition & 1 deletion admin/tool/task/scheduledtasks.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@

$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)) {

Expand Down
40 changes: 40 additions & 0 deletions config-dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,46 @@
// $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
//=========================================================================

Expand Down
109 changes: 103 additions & 6 deletions lib/classes/task/manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public static function load_default_scheduled_tasks_for_component($componentname

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);
Expand Down Expand Up @@ -338,16 +338,23 @@ public static function adhoc_task_from_record($record) {
* @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);
return false;
}
/** @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);
}
Expand Down Expand Up @@ -391,6 +398,7 @@ public static function scheduled_task_from_record($record, $expandr = true) {
if (isset($record->pid)) {
$task->set_pid($record->pid);
}
$task->set_overridden(self::scheduled_task_has_override($classname));

return $task;
}
Expand Down Expand Up @@ -701,10 +709,12 @@ public static function get_next_scheduled_task($timestart) {
}
}

// 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
Expand Down Expand Up @@ -1106,4 +1116,91 @@ public static function run_from_cli(\core\task\task_base $task):bool {

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;
}
}
19 changes: 19 additions & 0 deletions lib/classes/task/scheduled_task.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -102,6 +105,22 @@ public function set_customised($customised) {
$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.
Expand Down
123 changes: 123 additions & 0 deletions lib/tests/scheduled_task_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,129 @@ public function test_clear_fail_delay() {
$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.
*
Expand Down

0 comments on commit 3a23284

Please sign in to comment.