From 4d7d71fb37c13b3f08e950a52aedf4242366836c Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Fri, 24 Nov 2017 01:06:34 +0800 Subject: [PATCH] MDL-60188 groups: cache user's groups and groupings The function groups_get_user_groups is called too often both before rendering the page and after the page is rendered (using ajax). The function was executing a query joining 3 tables in each call. The plementation of the function has now modified to store the query result in a request cache. --- group/lib.php | 24 ++++++++++++++-- lang/en/cache.php | 1 + lib/db/caches.php | 8 ++++++ lib/enrollib.php | 6 +++- lib/grouplib.php | 72 +++++++++++++++++++++++++++++------------------ version.php | 2 +- 6 files changed, 81 insertions(+), 32 deletions(-) diff --git a/group/lib.php b/group/lib.php index 3f8de4e6804..374925d18a8 100644 --- a/group/lib.php +++ b/group/lib.php @@ -104,6 +104,9 @@ function groups_add_member($grouporid, $userorid, $component=null, $itemid=0) { $DB->set_field('groups', 'timemodified', $member->timeadded, array('id'=>$groupid)); $group->timemodified = $member->timeadded; + // Invalidate the group and grouping cache for users. + cache_helper::invalidate_by_definition('core', 'user_group_groupings', array(), array($userid)); + // Trigger group event. $params = array( 'context' => $context, @@ -205,6 +208,9 @@ function groups_remove_member($grouporid, $userorid) { $DB->set_field('groups', 'timemodified', $time, array('id' => $groupid)); $group->timemodified = $time; + // Invalidate the group and grouping cache for users. + cache_helper::invalidate_by_definition('core', 'user_group_groupings', array(), array($userid)); + // Trigger group event. $params = array( 'context' => context_course::instance($group->courseid), @@ -496,6 +502,8 @@ function groups_delete_group($grouporid) { // Invalidate the grouping cache for the course cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($group->courseid)); + // Purge the group and grouping cache for users. + cache_helper::purge_by_definition('core', 'user_group_groupings'); // Trigger group event. $params = array( @@ -547,6 +555,8 @@ function groups_delete_grouping($groupingorid) { // Invalidate the grouping cache for the course cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($grouping->courseid)); + // Purge the group and grouping cache for users. + cache_helper::purge_by_definition('core', 'user_group_groupings'); // Trigger group event. $params = array( @@ -621,6 +631,8 @@ function groups_delete_groupings_groups($courseid, $showfeedback=false) { // Invalidate the grouping cache for the course cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($courseid)); + // Purge the group and grouping cache for users. + cache_helper::purge_by_definition('core', 'user_group_groupings'); // TODO MDL-41312 Remove events_trigger_legacy('groups_groupings_groups_removed'). // This event is kept here for backwards compatibility, because it cannot be @@ -649,6 +661,8 @@ function groups_delete_groups($courseid, $showfeedback=false) { // Invalidate the grouping cache for the course cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($courseid)); + // Purge the group and grouping cache for users. + cache_helper::purge_by_definition('core', 'user_group_groupings'); // TODO MDL-41312 Remove events_trigger_legacy('groups_groups_deleted'). // This event is kept here for backwards compatibility, because it cannot be @@ -679,6 +693,8 @@ function groups_delete_groupings($courseid, $showfeedback=false) { // Invalidate the grouping cache for the course. cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($courseid)); + // Purge the group and grouping cache for users. + cache_helper::purge_by_definition('core', 'user_group_groupings'); // TODO MDL-41312 Remove events_trigger_legacy('groups_groupings_deleted'). // This event is kept here for backwards compatibility, because it cannot be @@ -818,7 +834,7 @@ function groups_parse_name($format, $groupnumber) { * @param int groupingid * @param int groupid * @param int $timeadded The time the group was added to the grouping. - * @param bool $invalidatecache If set to true the course group cache will be invalidated as well. + * @param bool $invalidatecache If set to true the course group cache and the user group cache will be invalidated as well. * @return bool true or exception */ function groups_assign_grouping($groupingid, $groupid, $timeadded = null, $invalidatecache = true) { @@ -841,6 +857,8 @@ function groups_assign_grouping($groupingid, $groupid, $timeadded = null, $inval if ($invalidatecache) { // Invalidate the grouping cache for the course cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($courseid)); + // Purge the group and grouping cache for users. + cache_helper::purge_by_definition('core', 'user_group_groupings'); } // Trigger event. @@ -860,7 +878,7 @@ function groups_assign_grouping($groupingid, $groupid, $timeadded = null, $inval * * @param int groupingid * @param int groupid - * @param bool $invalidatecache If set to true the course group cache will be invalidated as well. + * @param bool $invalidatecache If set to true the course group cache and the user group cache will be invalidated as well. * @return bool success */ function groups_unassign_grouping($groupingid, $groupid, $invalidatecache = true) { @@ -871,6 +889,8 @@ function groups_unassign_grouping($groupingid, $groupid, $invalidatecache = true if ($invalidatecache) { // Invalidate the grouping cache for the course cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($courseid)); + // Purge the group and grouping cache for users. + cache_helper::purge_by_definition('core', 'user_group_groupings'); } // Trigger event. diff --git a/lang/en/cache.php b/lang/en/cache.php index 0073e422291..2b6345fbab6 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -70,6 +70,7 @@ $string['cachedef_string'] = 'Language string cache'; $string['cachedef_tags'] = 'Tags collections and areas'; $string['cachedef_temp_tables'] = 'Temporary tables cache'; $string['cachedef_userselections'] = 'Data used to persist user selections throughout Moodle'; +$string['cachedef_user_group_groupings'] = 'User\'s groupings and groups per course'; $string['cachedef_yuimodules'] = 'YUI Module definitions'; $string['cachelock_file_default'] = 'Default file locking'; $string['cachestores'] = 'Cache stores'; diff --git a/lib/db/caches.php b/lib/db/caches.php index 45c1b14c807..4acfcca26e4 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -336,4 +336,12 @@ $definitions = array( 'simpledata' => true, 'staticacceleration' => false, ), + + // Caches grouping and group ids of a user. + 'user_group_groupings' => array( + 'mode' => cache_store::MODE_APPLICATION, + 'simplekeys' => true, + 'simpledata' => true, + 'staticacceleration' => true, + ), ); diff --git a/lib/enrollib.php b/lib/enrollib.php index 25f7c3f2728..5e8abcd0375 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -2216,7 +2216,11 @@ abstract class enrol_plugin { $participants->close(); // now clean up all remainders that were not removed correctly - $DB->delete_records('groups_members', array('itemid'=>$instance->id, 'component'=>'enrol_'.$name)); + if ($gms = $DB->get_records('groups_members', array('itemid' => $instance->id, 'component' => 'enrol_' . $name))) { + foreach ($gms as $gm) { + groups_remove_member($gm->groupid, $gm->userid); + } + } $DB->delete_records('role_assignments', array('itemid'=>$instance->id, 'component'=>'enrol_'.$name)); $DB->delete_records('user_enrolments', array('enrolid'=>$instance->id)); diff --git a/lib/grouplib.php b/lib/grouplib.php index 07bb4d99c24..8ef28ad170c 100644 --- a/lib/grouplib.php +++ b/lib/grouplib.php @@ -302,38 +302,54 @@ function groups_get_user_groups($courseid, $userid=0) { $userid = $USER->id; } - $sql = "SELECT g.id, gg.groupingid - FROM {groups} g - JOIN {groups_members} gm ON gm.groupid = g.id - LEFT JOIN {groupings_groups} gg ON gg.groupid = g.id - WHERE gm.userid = ? AND g.courseid = ?"; - $params = array($userid, $courseid); + $cache = cache::make('core', 'user_group_groupings'); - $rs = $DB->get_recordset_sql($sql, $params); + // Try to retrieve group ids from the cache. + $usergroups = $cache->get($userid); - if (!$rs->valid()) { - $rs->close(); // Not going to iterate (but exit), close rs + if ($usergroups === false) { + $sql = "SELECT g.id, g.courseid, gg.groupingid + FROM {groups} g + JOIN {groups_members} gm ON gm.groupid = g.id + LEFT JOIN {groupings_groups} gg ON gg.groupid = g.id + WHERE gm.userid = ?"; + + $rs = $DB->get_recordset_sql($sql, array($userid)); + + $usergroups = array(); + $allgroups = array(); + + foreach ($rs as $group) { + if (!array_key_exists($group->courseid, $allgroups)) { + $allgroups[$group->courseid] = array(); + } + $allgroups[$group->courseid][$group->id] = $group->id; + if (!array_key_exists($group->courseid, $usergroups)) { + $usergroups[$group->courseid] = array(); + } + if (is_null($group->groupingid)) { + continue; + } + if (!array_key_exists($group->groupingid, $usergroups[$group->courseid])) { + $usergroups[$group->courseid][$group->groupingid] = array(); + } + $usergroups[$group->courseid][$group->groupingid][$group->id] = $group->id; + } + $rs->close(); + + foreach (array_keys($allgroups) as $cid) { + $usergroups[$cid]['0'] = array_keys($allgroups[$cid]); // All user groups in the course. + } + + // Cache the data. + $cache->set($userid, $usergroups); + } + + if (array_key_exists($courseid, $usergroups)) { + return $usergroups[$courseid]; + } else { return array('0' => array()); } - - $result = array(); - $allgroups = array(); - - foreach ($rs as $group) { - $allgroups[$group->id] = $group->id; - if (is_null($group->groupingid)) { - continue; - } - if (!array_key_exists($group->groupingid, $result)) { - $result[$group->groupingid] = array(); - } - $result[$group->groupingid][$group->id] = $group->id; - } - $rs->close(); - - $result['0'] = array_keys($allgroups); // all groups - - return $result; } /** diff --git a/version.php b/version.php index 26a851ae18a..4e364d4a34b 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2017051503.03; // 20170515 = branching date YYYYMMDD - do not modify! +$version = 2017051503.04; // 20170515 = branching date YYYYMMDD - do not modify! // RR = release increments - 00 in DEV branches. // .XX = incremental changes.