From ebbb58556ac4634dffab5777986d01384b8a93ed Mon Sep 17 00:00:00 2001 From: David Monllao Date: Tue, 5 Jan 2016 15:04:32 +0800 Subject: [PATCH] MDL-52503 mod_assign: Removing session cache Replaced with a global $SESSION var, deferring cleanup to session destroy. --- mod/assign/locallib.php | 43 +++++++++++++++++------------- mod/assign/tests/locallib_test.php | 25 ----------------- 2 files changed, 24 insertions(+), 44 deletions(-) diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 1ee5e58919b..48890832113 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -140,7 +140,7 @@ class assign { /** @var bool whether to exclude users with inactive enrolment */ private $showonlyactiveenrol = null; - /** @var string A key used to identify cached userlists created by this object. */ + /** @var string A key used to identify userlists created by this object. */ private $useridlistid = null; /** @var array cached list of participants for this assignment. The cache key will be group, showactive and the context id */ @@ -167,6 +167,8 @@ class assign { * otherwise this class will load one from the context as required. */ public function __construct($coursemodulecontext, $coursemodule, $course) { + global $SESSION; + $this->context = $coursemodulecontext; $this->course = $course; @@ -181,6 +183,10 @@ class assign { // Extra entropy is required for uniqid() to work on cygwin. $this->useridlistid = clean_param(uniqid('', true), PARAM_ALPHANUM); + + if (!isset($SESSION->mod_assign_useridlist)) { + $SESSION->mod_assign_useridlist = []; + } } /** @@ -2971,7 +2977,7 @@ class assign { * @return string */ protected function view_single_grade_page($mform) { - global $DB, $CFG; + global $DB, $CFG, $SESSION; $o = ''; $instance = $this->get_instance(); @@ -2994,12 +3000,12 @@ class assign { $userid = optional_param('userid', 0, PARAM_INT); $attemptnumber = optional_param('attemptnumber', -1, PARAM_INT); - $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist'); if (!$userid) { - if (!$useridlist = $cache->get($this->get_useridlist_key($useridlistid))) { - $useridlist = $this->get_grading_userid_list(); + $useridlistkey = $this->get_useridlist_key($useridlistid); + if (empty($SESSION->mod_assign_useridlist[$useridlistkey])) { + $SESSION->mod_assign_useridlist[$useridlistkey] = $this->get_grading_userid_list(); } - $cache->set($this->get_useridlist_key($useridlistid), $useridlist); + $useridlist = $SESSION->mod_assign_useridlist[$useridlistkey]; } else { $rownum = 0; $useridlist = array($userid); @@ -3215,7 +3221,7 @@ class assign { * @return string */ protected function view_grading_table() { - global $USER, $CFG; + global $USER, $CFG, $SESSION; // Include grading options form. require_once($CFG->dirroot . '/mod/assign/gradingoptionsform.php'); @@ -3376,11 +3382,10 @@ class assign { } if ($this->can_grade()) { - // We need to cache the order of uses in the table as the person may wish to grade them. + // We need to store the order of uses in the table as the person may wish to grade them. // This is done based on the row number of the user. - $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist'); $useridlist = $gradingtable->get_column_data('userid'); - $cache->set($this->get_useridlist_key(), $useridlist); + $SESSION->mod_assign_useridlist[$this->get_useridlist_key()] = $useridlist; } $currentgroup = groups_get_activity_group($this->get_course_module(), true); @@ -6058,7 +6063,7 @@ class assign { * @return void */ public function add_grade_form_elements(MoodleQuickForm $mform, stdClass $data, $params) { - global $USER, $CFG; + global $USER, $CFG, $SESSION; $settings = $this->get_instance(); $rownum = $params['rownum']; @@ -6067,11 +6072,11 @@ class assign { $userid = $params['userid']; $attemptnumber = $params['attemptnumber']; if (!$userid) { - $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist'); - if (!$useridlist = $cache->get($this->get_useridlist_key($useridlistid))) { - $useridlist = $this->get_grading_userid_list(); - $cache->set($this->get_useridlist_key($useridlistid), $useridlist); + $useridlistkey = $this->get_useridlist_key($useridlistid); + if (empty($SESSION->mod_assign_useridlist[$useridlistkey])) { + $SESSION->mod_assign_useridlist[$useridlistkey] = $this->get_grading_userid_list(); } + $useridlist = $SESSION->mod_assign_useridlist[$useridlistkey]; } else { $useridlist = array($userid); $rownum = 0; @@ -6913,7 +6918,7 @@ class assign { * @return bool - was the grade saved */ protected function process_save_grade(&$mform) { - global $CFG; + global $CFG, $SESSION; // Include grade form. require_once($CFG->dirroot . '/mod/assign/gradeform.php'); @@ -6924,14 +6929,14 @@ class assign { $attemptnumber = optional_param('attemptnumber', -1, PARAM_INT); $useridlistid = optional_param('useridlistid', $this->get_useridlist_key_id(), PARAM_ALPHANUM); $userid = optional_param('userid', 0, PARAM_INT); - $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist'); if (!$userid) { - if (!$useridlist = $cache->get($this->get_useridlist_key($useridlistid))) { - // If the userid list is not cached we must not save, as it is possible that the user in a + if (empty($SESSION->mod_assign_useridlist[$this->get_useridlist_key($useridlistid)])) { + // If the userid list is not stored we must not save, as it is possible that the user in a // given row position may not be the same now as when the grading page was generated. $url = new moodle_url('/mod/assign/view.php', array('id' => $this->get_course_module()->id)); throw new moodle_exception('useridlistnotcached', 'mod_assign', $url); } + $useridlist = $SESSION->mod_assign_useridlist[$this->get_useridlist_key($useridlistid)]; } else { $useridlist = array($userid); $rownum = 0; diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index 30a55b1ba0a..ad92370e3b5 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -2329,30 +2329,5 @@ Anchor link 2:Link text $this->assertTrue(in_array($this->extrastudents[0]->id, $allgroupmembers)); $this->assertTrue(in_array($this->extrastudents[1]->id , $allgroupmembers)); } - - /** - * Test that the useridlist cache will retive the correct values - * when using assign::get_useridlist_key and assign::get_useridlist_key_id. - */ - public function test_useridlist_cache() { - // Create an assignment object, we will use this to test the key generation functions. - $course = self::getDataGenerator()->create_course(); - $assign = self::getDataGenerator()->create_module('assign', array('course' => $course->id)); - list($courserecord, $cm) = get_course_and_cm_from_instance($assign->id, 'assign'); - $context = context_module::instance($cm->id); - $assign = new assign($context, $cm, $courserecord); - // Create the cache. - $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist'); - // Create an entry that we will insert into the cache. - $entry = array(0 => '5', 1 => '6325', 2 => '67783'); - // Insert the value into the cache. - $cache->set($assign->get_useridlist_key(), $entry); - // Now test we can retrive the entry. - $this->assertEquals($entry, $cache->get($assign->get_useridlist_key())); - $useridlistid = clean_param($assign->get_useridlist_key_id(), PARAM_ALPHANUM); - $this->assertEquals($entry, $cache->get($assign->get_useridlist_key($useridlistid))); - // Check it will not retrive anything on an invalid key. - $this->assertFalse($cache->get($assign->get_useridlist_key('notvalid'))); - } }