MDL-52503 mod_assign: Removing session cache
authorDavid Monllao <davidm@moodle.com>
Tue, 5 Jan 2016 07:04:32 +0000 (15:04 +0800)
committerDavid Monllao <davidm@moodle.com>
Tue, 5 Jan 2016 07:04:32 +0000 (15:04 +0800)
Replaced with a global $SESSION var, deferring cleanup to session
destroy.

mod/assign/locallib.php
mod/assign/tests/locallib_test.php

index aa70d7f..42fbba3 100644 (file)
@@ -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 */
@@ -170,6 +170,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;
 
@@ -184,6 +186,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 = [];
+        }
     }
 
     /**
@@ -3031,7 +3037,7 @@ class assign {
      * @return string
      */
     protected function view_single_grade_page($mform) {
-        global $DB, $CFG;
+        global $DB, $CFG, $SESSION;
 
         $o = '';
         $instance = $this->get_instance();
@@ -3054,12 +3060,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);
@@ -3277,7 +3283,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');
@@ -3438,11 +3444,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);
@@ -6121,7 +6126,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'];
@@ -6130,11 +6135,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;
@@ -6976,7 +6981,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');
 
@@ -6987,14 +6992,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;
index 30a55b1..ad92370 100644 (file)
@@ -2329,30 +2329,5 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $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')));
-    }
 }