Merge branch 'MDL-59084-master' of git://github.com/andrewnicols/moodle

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.
+     * Getter for $userid.
-     * @return int|null $id
+     * @return int|null $userid
      */
-    public function get_id() {
+    public function get_userid() {
-        return $this->id;
+        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;
+    }
 
 }

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;
     }
 

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_run_inner_adhoc_task($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');
         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.
  *

lib/db/install.xml

@@ -3088,10 +3088,12 @@
         <FIELD NAME="nextruntime" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="faildelay" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="customdata" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="Custom data to be passed to the adhoc task. Must be serialisable using json_encode()"/>
+        <FIELD NAME="userid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="blocking" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
+        <KEY NAME="useriduser" TYPE="foreign" FIELDS="userid" REFTABLE="user" REFFIELDS="id"/>
       </KEYS>
       <INDEXES>
         <INDEX NAME="nextruntime_idx" UNIQUE="false" FIELDS="nextruntime"/>

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;
 }

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();
+        $task = new \core\task\adhoc_test_task();
-        $task1->set_custom_data(array('courseid' => 10));
+        $task->set_custom_data(array('courseid' => 10));
-        $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task1, true));
+        $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.
+        // Schedule adhoc task with a user.
-        $task2 = new \core\task\adhoc_test_task();
+        $task = new \core\task\adhoc_test_task();
-        $task2->set_custom_data(array('courseid' => 1));
+        $task->set_custom_data(array('courseid' => 10));
-        $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task2, true));
+        $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();
+        $task = new \core\task\adhoc_test_task();
-        $task3->set_custom_data(array('courseid' => 1));
+        $task->set_custom_data(array('courseid' => 1));
-        $this->assertEmpty(\core\task\manager::queue_adhoc_task($task3, true));
+        $this->assertEmpty(\core\task\manager::queue_adhoc_task($task, true));
-        $this->assertEquals(2, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task')));
+        $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();
+        // Note: This task was created earlier.
-        $this->assertNotEmpty(\core\task\manager::queue_adhoc_task($task4, true));
+        $task = new \core\task\adhoc_test_task();
-        $this->assertEquals(3, count(\core\task\manager::get_adhoc_tasks('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());
     }
 }

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.