From 0ae28fad3ad6e72095f1918019df8e41bcc3b1fa Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Wed, 26 Jul 2017 11:20:35 +0800 Subject: [PATCH] MDL-59367 participants: Reduce db queries Fetch all role assignments for the course in a single query, instead of for each of the listed users. More data but less queries. --- lib/accesslib.php | 59 +++++++++++++++++++++++++++++ lib/tests/accesslib_test.php | 36 ++++++++++++++++++ user/classes/participants_table.php | 8 +++- 3 files changed, 102 insertions(+), 1 deletion(-) diff --git a/lib/accesslib.php b/lib/accesslib.php index 728b13c21ab..c60077d5f84 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -2664,6 +2664,65 @@ function get_archetype_roles($archetype) { return $DB->get_records('role', array('archetype'=>$archetype), 'sortorder ASC'); } +/** + * Gets all the user roles assigned in this context, or higher contexts for a list of users. + * + * @param context $context + * @param array $userids. An empty list means fetch all role assignments for the context. + * @param bool $checkparentcontexts defaults to true + * @param string $order defaults to 'c.contextlevel DESC, r.sortorder ASC' + * @return array + */ +function get_users_roles(context $context, $userids = [], $checkparentcontexts = true, $order = 'c.contextlevel DESC, r.sortorder ASC') { + global $USER, $DB; + + if ($checkparentcontexts) { + $contextids = $context->get_parent_context_ids(); + } else { + $contextids = array(); + } + $contextids[] = $context->id; + + list($contextids, $params) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED, 'con'); + + // If userids was passed as an empty array, we fetch all role assignments for the course. + if (empty($userids)) { + $useridlist = ' IS NOT NULL '; + $uparams = []; + } else { + list($useridlist, $uparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED, 'uids'); + } + + $sql = "SELECT ra.*, r.name, r.shortname, ra.userid + FROM {role_assignments} ra, {role} r, {context} c + WHERE ra.userid $useridlist + AND ra.roleid = r.id + AND ra.contextid = c.id + AND ra.contextid $contextids + ORDER BY $order"; + + $all = $DB->get_records_sql($sql , array_merge($params, $uparams)); + + // Return results grouped by userid. + $result = []; + foreach ($all as $id => $record) { + if (!isset($result[$record->userid])) { + $result[$record->userid] = []; + } + $result[$record->userid][$record->id] = $record; + } + + // Make sure all requested users are included in the result, even if they had no role assignments. + foreach ($userids as $id) { + if (!isset($result[$id])) { + $result[$id] = []; + } + } + + return $result; +} + + /** * Gets all the user roles assigned in this context, or higher contexts * this is mainly used when checking if a user can assign a role, or overriding a role diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index b9c32ad9e6b..426a45a5a01 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -1561,6 +1561,42 @@ class core_accesslib_testcase extends advanced_testcase { $this->assertSame('', $roles); } + /** + * Test get_user_roles and get_users_roles + */ + public function test_get_user_roles() { + global $DB, $CFG; + + $this->resetAfterTest(); + + $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'), '*', MUST_EXIST); + $studentrole = $DB->get_record('role', array('shortname'=>'student'), '*', MUST_EXIST); + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $teacherrename = (object)array('roleid'=>$teacherrole->id, 'name'=>'Učitel', 'contextid'=>$coursecontext->id); + $DB->insert_record('role_names', $teacherrename); + + $roleids = explode(',', $CFG->profileroles); // Should include teacher and student in new installs. + + $user1 = $this->getDataGenerator()->create_user(); + role_assign($teacherrole->id, $user1->id, $coursecontext->id); + role_assign($studentrole->id, $user1->id, $coursecontext->id); + $user2 = $this->getDataGenerator()->create_user(); + role_assign($studentrole->id, $user2->id, $coursecontext->id); + $user3 = $this->getDataGenerator()->create_user(); + + $u1roles = get_user_roles($coursecontext, $user1->id); + + $u2roles = get_user_roles($coursecontext, $user2->id); + + $allroles = get_users_roles($coursecontext); + $specificuserroles = get_users_roles($coursecontext, [$user1->id, $user2->id]); + $this->assertEquals($u1roles, $allroles[$user1->id]); + $this->assertEquals($u1roles, $specificuserroles[$user1->id]); + $this->assertEquals($u2roles, $allroles[$user2->id]); + $this->assertEquals($u2roles, $specificuserroles[$user2->id]); + } + /** * Test has_capability(), has_any_capability() and has_all_capabilities(). */ diff --git a/user/classes/participants_table.php b/user/classes/participants_table.php index becb5e45c0a..0364f547650 100644 --- a/user/classes/participants_table.php +++ b/user/classes/participants_table.php @@ -104,6 +104,11 @@ class participants_table extends \table_sql { */ protected $allroles; + /** + * @var \stdClass[] List of roles indexed by roleid. + */ + protected $allroleassignments; + /** * @var \stdClass[] Assignable roles in this course. */ @@ -201,6 +206,7 @@ class participants_table extends \table_sql { $this->context = $context; $this->groups = groups_get_all_groups($courseid, 0, 0, 'g.*', true); $this->allroles = role_fix_names(get_all_roles($this->context), $this->context); + $this->allroleassignments = get_users_roles($this->context, [], true, 'c.contextlevel DESC, r.sortorder ASC'); $this->assignableroles = get_assignable_roles($this->context, ROLENAME_ALIAS, false); } @@ -258,7 +264,7 @@ class participants_table extends \table_sql { public function col_roles($data) { global $OUTPUT; - $roles = get_user_roles($this->context, $data->id, true, 'c.contextlevel DESC, r.sortorder ASC'); + $roles = isset($this->allroleassignments[$data->id]) ? $this->allroleassignments[$data->id] : []; $getrole = function($role) { return $role->roleid; };