diff --git a/lib/classes/task/adhoc_task.php b/lib/classes/task/adhoc_task.php index 30ebc738a52..2e1e042d41b 100644 --- a/lib/classes/task/adhoc_task.php +++ b/lib/classes/task/adhoc_task.php @@ -40,6 +40,9 @@ abstract class adhoc_task extends task_base { /** @var integer|null $id - Adhoc tasks each have their own database record id. */ private $id = null; + /** @var integer|null $userid - Adhoc tasks may choose to run as a specific user. */ + private $userid = null; + /** * Setter for $id. * @param int|null $id @@ -49,11 +52,11 @@ abstract class adhoc_task extends task_base { } /** - * Getter for $id. - * @return int|null $id + * Getter for $userid. + * @return int|null $userid */ - public function get_id() { - return $this->id; + public function get_userid() { + return $this->userid; } /** @@ -88,5 +91,20 @@ abstract class adhoc_task extends task_base { return $this->customdata; } + /** + * Getter for $id. + * @return int|null $id + */ + public function get_id() { + return $this->id; + } + + /** + * Setter for $userid. + * @param int|null $userid + */ + public function set_userid($userid) { + $this->userid = $userid; + } } diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index b19b689890d..d7f4a11e5cf 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -137,6 +137,11 @@ class manager { $params = [$record->classname, $record->component, $record->customdata]; $sql = 'classname = ? AND component = ? AND ' . $DB->sql_compare_text('customdata', \core_text::strlen($record->customdata) + 1) . ' = ?'; + + if ($record->userid) { + $params[] = $record->userid; + $sql .= " AND userid = ? "; + } return $DB->record_exists_select('task_adhoc', $sql, $params); } @@ -151,6 +156,11 @@ class manager { public static function queue_adhoc_task(adhoc_task $task, $checkforexisting = false) { global $DB; + if ($userid = $task->get_userid()) { + // User found. Check that they are suitable. + \core_user::require_active_user(\core_user::get_user($userid, '*', MUST_EXIST), true, true); + } + $record = self::record_from_adhoc_task($task); // Schedule it immediately if nextruntime not explicitly set. if (!$task->get_next_run_time()) { @@ -239,6 +249,7 @@ class manager { $record->nextruntime = $task->get_next_run_time(); $record->faildelay = $task->get_fail_delay(); $record->customdata = $task->get_custom_data_as_string(); + $record->userid = $task->get_userid(); return $record; } @@ -276,6 +287,10 @@ class manager { $task->set_custom_data_as_string($record->customdata); } + if (isset($record->userid)) { + $task->set_userid($record->userid); + } + return $task; } diff --git a/lib/cronlib.php b/lib/cronlib.php index 55626a4c90d..4ce1c698836 100644 --- a/lib/cronlib.php +++ b/lib/cronlib.php @@ -71,44 +71,7 @@ function cron_run() { // Run all adhoc tasks. while (!\core\task\manager::static_caches_cleared_since($timenow) && $task = \core\task\manager::get_next_adhoc_task($timenow)) { - mtrace("Execute adhoc task: " . get_class($task)); - cron_trace_time_and_memory(); - $predbqueries = null; - $predbqueries = $DB->perf_get_queries(); - $pretime = microtime(1); - try { - get_mailer('buffer'); - $task->execute(); - if ($DB->is_transaction_started()) { - throw new coding_exception("Task left transaction open"); - } - if (isset($predbqueries)) { - mtrace("... used " . ($DB->perf_get_queries() - $predbqueries) . " dbqueries"); - mtrace("... used " . (microtime(1) - $pretime) . " seconds"); - } - mtrace("Adhoc task complete: " . get_class($task)); - \core\task\manager::adhoc_task_complete($task); - } catch (Exception $e) { - if ($DB && $DB->is_transaction_started()) { - error_log('Database transaction aborted automatically in ' . get_class($task)); - $DB->force_transaction_rollback(); - } - if (isset($predbqueries)) { - mtrace("... used " . ($DB->perf_get_queries() - $predbqueries) . " dbqueries"); - mtrace("... used " . (microtime(1) - $pretime) . " seconds"); - } - mtrace("Adhoc task failed: " . get_class($task) . "," . $e->getMessage()); - if ($CFG->debugdeveloper) { - if (!empty($e->debuginfo)) { - mtrace("Debug info:"); - mtrace($e->debuginfo); - } - mtrace("Backtrace:"); - mtrace(format_backtrace($e->getTrace(), true)); - } - \core\task\manager::adhoc_task_failed($task); - } - get_mailer('close'); + cron_run_inner_adhoc_task($task); unset($task); } @@ -171,6 +134,86 @@ function cron_run_inner_scheduled_task(\core\task\task_base $task) { get_mailer('close'); } +/** + * Shared code that handles running of a single adhoc task within the cron. + * + * @param \core\task\adhoc_task $task + */ +function cron_run_inner_adhoc_task(\core\task\adhoc_task $task) { + global $DB, $CFG; + mtrace("Execute adhoc task: " . get_class($task)); + cron_trace_time_and_memory(); + $predbqueries = null; + $predbqueries = $DB->perf_get_queries(); + $pretime = microtime(1); + + if ($userid = $task->get_userid()) { + // This task has a userid specified. + if ($user = \core_user::get_user($userid)) { + // User found. Check that they are suitable. + try { + \core_user::require_active_user($user, true, true); + } catch (moodle_exception $e) { + mtrace("User {$userid} cannot be used to run an adhoc task: " . get_class($task) . ". Cancelling task."); + $user = null; + } + } else { + // Unable to find the user for this task. + // A user missing in the database will never reappear. + mtrace("User {$userid} could not be found for adhoc task: " . get_class($task) . ". Cancelling task."); + } + + if (empty($user)) { + // A user missing in the database will never reappear so the task needs to be failed to ensure that locks are removed, + // and then removed to prevent future runs. + // A task running as a user should only be run as that user. + \core\task\manager::adhoc_task_failed($task); + $DB->delete_records('task_adhoc', ['id' => $task->get_id()]); + + return; + } + + cron_setup_user($user); + } + + try { + get_mailer('buffer'); + $task->execute(); + if ($DB->is_transaction_started()) { + throw new coding_exception("Task left transaction open"); + } + if (isset($predbqueries)) { + mtrace("... used " . ($DB->perf_get_queries() - $predbqueries) . " dbqueries"); + mtrace("... used " . (microtime(1) - $pretime) . " seconds"); + } + mtrace("Adhoc task complete: " . get_class($task)); + \core\task\manager::adhoc_task_complete($task); + } catch (Exception $e) { + if ($DB && $DB->is_transaction_started()) { + error_log('Database transaction aborted automatically in ' . get_class($task)); + $DB->force_transaction_rollback(); + } + if (isset($predbqueries)) { + mtrace("... used " . ($DB->perf_get_queries() - $predbqueries) . " dbqueries"); + mtrace("... used " . (microtime(1) - $pretime) . " seconds"); + } + mtrace("Adhoc task failed: " . get_class($task) . "," . $e->getMessage()); + if ($CFG->debugdeveloper) { + if (!empty($e->debuginfo)) { + mtrace("Debug info:"); + mtrace($e->debuginfo); + } + mtrace("Backtrace:"); + mtrace(format_backtrace($e->getTrace(), true)); + } + \core\task\manager::adhoc_task_failed($task); + } finally { + // Reset back to the standard admin user. + cron_setup_user(); + } + get_mailer('close'); +} + /** * Runs a single cron task. This function assumes it is displaying output in pseudo-CLI mode. * diff --git a/lib/db/install.xml b/lib/db/install.xml index 7e4b4f36254..880905b8310 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -3088,10 +3088,12 @@ + + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 1dc0391e739..df03600054a 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2475,5 +2475,24 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2017091200.00); } + if ($oldversion < 2017091201.00) { + // Define field userid to be added to task_adhoc. + $table = new xmldb_table('task_adhoc'); + $field = new xmldb_field('userid', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'customdata'); + + // Conditionally launch add field userid. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + $key = new xmldb_key('useriduser', XMLDB_KEY_FOREIGN, array('userid'), 'user', array('id')); + + // Launch add key userid_user. + $dbman->add_key($table, $key); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2017091201.00); + } + return true; } diff --git a/lib/tests/adhoc_task_test.php b/lib/tests/adhoc_task_test.php index e4ebcacc71e..2b8b7d1e19d 100644 --- a/lib/tests/adhoc_task_test.php +++ b/lib/tests/adhoc_task_test.php @@ -155,34 +155,103 @@ class core_adhoc_task_testcase extends advanced_testcase { */ public function test_queue_adhoc_task_if_not_scheduled() { $this->resetAfterTest(true); + $user = \core_user::get_user_by_username('admin'); // Schedule adhoc task. - $task1 = new \core\task\adhoc_test_task(); - $task1->set_custom_data(array('courseid' => 10)); - $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task1, true)); + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(array('courseid' => 10)); + $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task, true)); $this->assertEquals(1, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); - // Schedule same adhoc task with different custom data. - $task2 = new \core\task\adhoc_test_task(); - $task2->set_custom_data(array('courseid' => 1)); - $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task2, true)); + // Schedule adhoc task with a user. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(array('courseid' => 10)); + $task->set_userid($user->id); + $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task, true)); $this->assertEquals(2, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + // Schedule same adhoc task with different custom data. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(array('courseid' => 1)); + $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task, true)); + $this->assertEquals(3, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + // Schedule same adhoc task with same custom data. - $task3 = new \core\task\adhoc_test_task(); - $task3->set_custom_data(array('courseid' => 1)); - $this->assertEmpty(\core\task\manager::queue_adhoc_task($task3, true)); - $this->assertEquals(2, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(array('courseid' => 1)); + $this->assertEmpty(\core\task\manager::queue_adhoc_task($task, true)); + $this->assertEquals(3, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + + // Schedule same adhoc task with same custom data and a user. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(array('courseid' => 1)); + $task->set_userid($user->id); + $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task, true)); + $this->assertEquals(4, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); // Schedule same adhoc task without custom data. - $task4 = new \core\task\adhoc_test_task(); - $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task4, true)); - $this->assertEquals(3, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + // Note: This task was created earlier. + $task = new \core\task\adhoc_test_task(); + $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task, true)); + $this->assertEquals(5, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); // Schedule same adhoc task without custom data (again). $task5 = new \core\task\adhoc_test_task(); $this->assertEmpty(\core\task\manager::queue_adhoc_task($task5, true)); - $this->assertEquals(3, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + $this->assertEquals(5, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + // Schedule same adhoc task without custom data but with a userid. + $task6 = new \core\task\adhoc_test_task(); + $user = \core_user::get_user_by_username('admin'); + $task6->set_userid($user->id); + $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task6, true)); + $this->assertEquals(6, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + + // Schedule same adhoc task again without custom data but with a userid. + $task6 = new \core\task\adhoc_test_task(); + $user = \core_user::get_user_by_username('admin'); + $task6->set_userid($user->id); + $this->assertEmpty(\core\task\manager::queue_adhoc_task($task6, true)); + $this->assertEquals(6, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + } + + /** + * Test that when no userid is specified, it returns empty from the DB + * too. + */ + public function test_adhoc_task_user_empty() { + $this->resetAfterTest(true); + + // Create an adhoc task in future. + $task = new \core\task\adhoc_test_task(); + \core\task\manager::queue_adhoc_task($task); + + // Get it back from the scheduler. + $now = time(); + $task = \core\task\manager::get_next_adhoc_task($now); + \core\task\manager::adhoc_task_complete($task); + + $this->assertEmpty($task->get_userid()); + } + + /** + * Test that when a userid is specified, that userid is subsequently + * returned. + */ + public function test_adhoc_task_user_set() { + $this->resetAfterTest(true); + + // Create an adhoc task in future. + $task = new \core\task\adhoc_test_task(); + $user = \core_user::get_user_by_username('admin'); + $task->set_userid($user->id); + \core\task\manager::queue_adhoc_task($task); + + // Get it back from the scheduler. + $now = time(); + $task = \core\task\manager::get_next_adhoc_task($now); + \core\task\manager::adhoc_task_complete($task); + + $this->assertEquals($user->id, $task->get_userid()); } } diff --git a/version.php b/version.php index d7c15fd11b7..1bf9c604b8a 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2017091200.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2017091201.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.