From 4ee23c10bb365bd23ba26464268230a86e8d0700 Mon Sep 17 00:00:00 2001 From: Damien Bezborodov Date: Wed, 25 Jun 2014 17:33:10 +0930 Subject: [PATCH] MDL-45678 assignsubmission_comments: Improve performance for permissions checking --- lang/en/cache.php | 1 + lib/accesslib.php | 19 ++++++++++++++++++- lib/db/caches.php | 9 ++++++++- lib/enrollib.php | 3 +++ mod/assign/locallib.php | 8 +------- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/lang/en/cache.php b/lang/en/cache.php index 603746947aa..c033cbcaf5e 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -46,6 +46,7 @@ $string['cachedef_coursemodinfo'] = 'Accumulated information about modules and s $string['cachedef_databasemeta'] = 'Database meta information'; $string['cachedef_eventinvalidation'] = 'Event invalidation'; $string['cachedef_externalbadges'] = 'External badges for particular user'; +$string['cachedef_get_suspended_userids'] = 'List of suspended user identifiers per course'; $string['cachedef_gradecondition'] = 'User grades cached for evaluating conditional availability'; $string['cachedef_groupdata'] = 'Course group information'; $string['cachedef_htmlpurifier'] = 'HTML Purifier - cleaned content'; diff --git a/lib/accesslib.php b/lib/accesslib.php index abcb825587e..37a20769d9d 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -7445,11 +7445,21 @@ function extract_suspended_users($context, &$users, $ignoreusers=array()) { * or enrolment has expired or not started. * * @param context $context context in which user enrolment is checked. + * @param bool $context Enable or disable (default) the request cache * @return array list of suspended user id's. */ -function get_suspended_userids($context){ +function get_suspended_userids(context $context, $usecache = false) { global $DB; + // Check the cache first for performance reasons if enabled. + if ($usecache) { + $cache = cache::make('core', 'get_suspended_userids'); + $susers = $cache->get($context->id); + if ($susers !== false) { + return $susers; + } + } + // Get all enrolled users. list($sql, $params) = get_enrolled_sql($context); $users = $DB->get_records_sql($sql, $params); @@ -7466,5 +7476,12 @@ function get_suspended_userids($context){ } } } + + // Cache results for the remainder of this request. + if ($usecache) { + $cache->set($context->id, $susers); + } + + // Return. return $susers; } diff --git a/lib/db/caches.php b/lib/db/caches.php index c2f99cfd767..24543b9be44 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -221,5 +221,12 @@ $definitions = array( 'mode' => cache_store::MODE_SESSION, 'simplekeys' => true, 'simpledata' => true - ) + ), + + // For the function get_suspended_userids() in core_access. + 'get_suspended_userids' => array( + 'mode' => cache_store::MODE_REQUEST, + 'simplekeys' => true, + 'simpledata' => true, + ), ); diff --git a/lib/enrollib.php b/lib/enrollib.php index df880d877f0..ce130660812 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -1404,6 +1404,9 @@ abstract class enrol_plugin { $DB->update_record('user_enrolments', $ue); context_course::instance($instance->courseid)->mark_dirty(); // reset enrol caches + // Invalidate core_access cache for get_suspended_userids. + cache_helper::invalidate_by_definition('core', 'get_suspended_userids', array(), array($instance->courseid)); + // Trigger event. $event = \core\event\user_enrolment_updated::create( array( diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 8a33ff31144..6b475038776 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -128,9 +128,6 @@ class assign { /** @var bool whether to exclude users with inactive enrolment */ private $showonlyactiveenrol = null; - /** @var array list of suspended user IDs in form of ([id1] => id1) */ - public $susers = null; - /** @var array cached list of participants for this assignment. The cache key will be group, showactive and the context id */ private $participants = array(); @@ -6873,10 +6870,7 @@ class assign { * @return bool true is user is active in course. */ public function is_active_user($userid) { - if (is_null($this->susers) && !is_null($this->context)) { - $this->susers = get_suspended_userids($this->context); - } - return !in_array($userid, $this->susers); + return !in_array($userid, get_suspended_userids($this->context, true)); } }