diff --git a/lang/en/cache.php b/lang/en/cache.php index 5fac49e8984..3e633792143 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -43,6 +43,7 @@ $string['cachedef_coursecatrecords'] = 'Course categories records'; $string['cachedef_coursecontacts'] = 'List of course contacts'; $string['cachedef_coursecattree'] = 'Course categories tree'; $string['cachedef_coursemodinfo'] = 'Accumulated information about modules and sections for each course'; +$string['cachedef_completion'] = 'Activity completion status'; $string['cachedef_databasemeta'] = 'Database meta information'; $string['cachedef_eventinvalidation'] = 'Event invalidation'; $string['cachedef_externalbadges'] = 'External badges for particular user'; @@ -173,4 +174,4 @@ $string['userinputsharingkey_help'] = 'Enter your own private key here. When you // Deprecated since 2.9. $string['lockingmeans'] = 'Locking mechanism'; $string['lockmethod'] = 'Lock method'; -$string['lockmethod_help'] = 'This is the method used for locking when required of this store.'; \ No newline at end of file +$string['lockmethod_help'] = 'This is the method used for locking when required of this store.'; diff --git a/lib/completionlib.php b/lib/completionlib.php index 2e83745084b..b7af0b2783b 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -119,12 +119,6 @@ define('COMPLETION_VIEWED', 1); */ define('COMPLETION_NOT_VIEWED', 0); -/** - * Cache expiry time in seconds (10 minutes) - * Completion cacheing - */ -define('COMPLETION_CACHE_EXPIRY', 10*60); - /** * Completion details should be ORed together and you should return false if * none apply. @@ -553,7 +547,7 @@ class completion_info { * @return void */ public function update_state($cm, $possibleresult=COMPLETION_UNKNOWN, $userid=0) { - global $USER, $SESSION; + global $USER; // Do nothing if completion is not enabled for that activity if (!$this->is_enabled($cm)) { @@ -792,20 +786,13 @@ class completion_info { * Used by course reset page. */ public function delete_all_completion_data() { - global $DB, $SESSION; + global $DB; // Delete from database. $DB->delete_records_select('course_modules_completion', 'coursemoduleid IN (SELECT id FROM {course_modules} WHERE course=?)', array($this->course_id)); - // Reset cache for current user. - if (isset($SESSION->completioncache) && - array_key_exists($this->course_id, $SESSION->completioncache)) { - - unset($SESSION->completioncache[$this->course_id]); - } - // Wipe course completion data too. $this->delete_course_completion_data(); } @@ -818,19 +805,11 @@ class completion_info { * @param stdClass|cm_info $cm Activity */ public function delete_all_state($cm) { - global $SESSION, $DB; + global $DB; // Delete from database $DB->delete_records('course_modules_completion', array('coursemoduleid'=>$cm->id)); - // Erase cache data for current user if applicable - if (isset($SESSION->completioncache) && - array_key_exists($cm->course, $SESSION->completioncache) && - array_key_exists($cm->id, $SESSION->completioncache[$cm->course])) { - - unset($SESSION->completioncache[$cm->course][$cm->id]); - } - // Check if there is an associated course completion criteria $criteria = $this->get_criteria(COMPLETION_CRITERIA_TYPE_ACTIVITY); $acriteria = false; @@ -876,7 +855,7 @@ class completion_info { } $rs->close(); - // Delete all existing state [also clears session cache for current user] + // Delete all existing state. $this->delete_all_state($cm); // Merge this with list of planned users (according to roles) @@ -894,7 +873,7 @@ class completion_info { /** * Obtains completion data for a particular activity and user (from the - * session cache if available, or by SQL query) + * completion cache if available, or by SQL query) * * @param stcClass|cm_info $cm Activity; only required field is ->id * @param bool $wholecourse If true (default false) then, when necessary to @@ -908,39 +887,21 @@ class completion_info { * @return object Completion data (record from course_modules_completion) */ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null) { - global $USER, $CFG, $SESSION, $DB; + global $USER, $CFG, $DB; + $completioncache = cache::make('core', 'completion'); // Get user ID if (!$userid) { $userid = $USER->id; } - // Is this the current user? - $currentuser = $userid==$USER->id; - - if ($currentuser && is_object($SESSION)) { - // Make sure cache is present and is for current user (loginas - // changes this) - if (!isset($SESSION->completioncache) || $SESSION->completioncacheuserid!=$USER->id) { - $SESSION->completioncache = array(); - $SESSION->completioncacheuserid = $USER->id; - } - // Expire any old data from cache - foreach ($SESSION->completioncache as $courseid=>$activities) { - if (empty($activities['updated']) || $activities['updated'] < time()-COMPLETION_CACHE_EXPIRY) { - unset($SESSION->completioncache[$courseid]); - } - } - // See if requested data is present, if so use cache to get it - if (isset($SESSION->completioncache) && - array_key_exists($this->course->id, $SESSION->completioncache) && - array_key_exists($cm->id, $SESSION->completioncache[$this->course->id])) { - return $SESSION->completioncache[$this->course->id][$cm->id]; - } + // See if requested data is present in cache + if ($cacheddata = $completioncache->get($userid . '_' . $this->course->id . '_' . $cm->id)) { + return $cacheddata; } // Not there, get via SQL - if ($currentuser && $wholecourse) { + if ($wholecourse) { // Get whole course data for cache $alldatabycmc = $DB->get_records_sql(" SELECT @@ -976,14 +937,13 @@ class completion_info { $data->viewed = 0; $data->timemodified = 0; } - $SESSION->completioncache[$this->course->id][$othercm->id] = $data; + $completioncache->set($userid . '_' . $this->course->id . '_' . $othercm->id, $data); } - $SESSION->completioncache[$this->course->id]['updated'] = time(); - if (!isset($SESSION->completioncache[$this->course->id][$cm->id])) { + if (!$completiondata = $completioncache->get($userid . '_' . $this->course->id . '_' . $cm->id)) { $this->internal_systemerror("Unexpected error: course-module {$cm->id} could not be found on course {$this->course->id}"); } - return $SESSION->completioncache[$this->course->id][$cm->id]; + return $completiondata; } else { // Get single record @@ -1000,13 +960,7 @@ class completion_info { } // Put in cache - if ($currentuser) { - $SESSION->completioncache[$this->course->id][$cm->id] = $data; - // For single updates, only set date if it was empty before - if (empty($SESSION->completioncache[$this->course->id]['updated'])) { - $SESSION->completioncache[$this->course->id]['updated'] = time(); - } - } + $completioncache->set($userid . '_' . $this->course->id . '_' . $cm->id, $data); } return $data; @@ -1022,7 +976,7 @@ class completion_info { * @param stdClass $data Data about completion for that user */ public function internal_set_data($cm, $data) { - global $USER, $SESSION, $DB; + global $USER, $DB; $transaction = $DB->start_delegated_transaction(); if (!$data->id) { @@ -1054,8 +1008,10 @@ class completion_info { $event->add_record_snapshot('course_modules_completion', $data); $event->trigger(); + $completioncache = cache::make('core', 'completion'); + $completioncache->set($data->userid . '_' . $cm->course . '_' . $cm->id, $data); + // TODO under what circumstances should I call get_fast_modinfo()? if ($data->userid == $USER->id) { - $SESSION->completioncache[$cm->course][$cm->id] = $data; // reset modinfo for user (no need to call rebuild_course_cache()) get_fast_modinfo($cm->course, 0, true); } @@ -1341,13 +1297,4 @@ class completion_info { throw new moodle_exception('err_system','completion', $CFG->wwwroot.'/course/view.php?id='.$this->course->id,null,$error); } - - /** - * For testing only. Wipes information cached in user session. - */ - public static function wipe_session_cache() { - global $SESSION; - unset($SESSION->completioncache); - unset($SESSION->completioncacheuserid); - } } diff --git a/lib/db/caches.php b/lib/db/caches.php index e71dc577fee..2b08e21f4c2 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -213,6 +213,13 @@ $definitions = array( 'ttl' => 3600, ), + // Used to cache activity completion status. + 'completion' => array( + 'mode' => cache_store::MODE_APPLICATION, + 'simplekeys' => true, + 'ttl' => 10*60, + ), + // A simple cache that stores whether a user can expand a course in the navigation. // The key is the course ID and the value will either be 1 or 0 (cast to bool). // The cache isn't always up to date, it should only ever be used to save a costly call to diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index 150bafc789d..927ae65c6c2 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -325,7 +325,7 @@ class core_completionlib_testcase extends advanced_testcase { } public function test_delete_all_state() { - global $DB, $SESSION; + global $DB; $this->mock_setup(); $course = (object)array('id'=>13); @@ -339,21 +339,6 @@ class core_completionlib_testcase extends advanced_testcase { ->with('course_modules_completion', array('coursemoduleid'=>42)) ->will($this->returnValue(true)); $c->delete_all_state($cm); - - // Build up a session to check it deletes the right bits from it - // (and not other bits). - $SESSION->completioncache = array(); - $SESSION->completioncache[13] = array(); - $SESSION->completioncache[13][42] = 'foo'; - $SESSION->completioncache[13][43] = 'foo'; - $SESSION->completioncache[14] = array(); - $SESSION->completioncache[14][42] = 'foo'; - $DB->expects($this->at(0)) - ->method('delete_records') - ->with('course_modules_completion', array('coursemoduleid'=>42)) - ->will($this->returnValue(true)); - $c->delete_all_state($cm); - $this->assertEquals(array(13=>array(43=>'foo'), 14=>array(42=>'foo')), $SESSION->completioncache); } public function test_reset_all_state() { @@ -396,9 +381,11 @@ class core_completionlib_testcase extends advanced_testcase { } public function test_get_data() { - global $DB, $SESSION; + global $DB; $this->mock_setup(); + $cache = cache::make('core', 'completion'); + $c = new completion_info((object)array('id'=>42)); $cm = (object)array('id'=>13, 'course'=>42); @@ -412,18 +399,20 @@ class core_completionlib_testcase extends advanced_testcase { ->will($this->returnValue($sillyrecord)); $result = $c->get_data($cm, false, 123); $this->assertEquals($sillyrecord, $result); - $this->assertFalse(isset($SESSION->completioncache)); + $this->assertEquals($cache->get('123_42_13'), $sillyrecord); - // 2. Not current user, default record, whole course (ignored). + // 2. Not current user, default record, whole course. + $cache->purge(); $DB->expects($this->at(0)) - ->method('get_record') - ->with('course_modules_completion', array('coursemoduleid'=>13, 'userid'=>123)) - ->will($this->returnValue(false)); - $result=$c->get_data($cm, true, 123); + ->method('get_records_sql') + ->will($this->returnValue(array())); + $modinfo = new stdClass(); + $modinfo->cms = array((object)array('id'=>13)); + $result=$c->get_data($cm, true, 123, $modinfo); $this->assertEquals((object)array( 'id'=>'0', 'coursemoduleid'=>13, 'userid'=>123, 'completionstate'=>0, 'viewed'=>0, 'timemodified'=>0), $result); - $this->assertFalse(isset($SESSION->completioncache)); + $this->assertEquals($cache->get('123_42_13'), $result); // 3. Current user, single record, not from cache. $DB->expects($this->at(0)) @@ -432,34 +421,14 @@ class core_completionlib_testcase extends advanced_testcase { ->will($this->returnValue($sillyrecord)); $result = $c->get_data($cm); $this->assertEquals($sillyrecord, $result); - $this->assertEquals($sillyrecord, $SESSION->completioncache[42][13]); - // When checking time(), allow for second overlaps. - $this->assertTrue(time()-$SESSION->completioncache[42]['updated']<2); + $this->assertEquals($sillyrecord, $cache->get('314159_42_13')); // 4. Current user, 'whole course', but from cache. $result = $c->get_data($cm, true); $this->assertEquals($sillyrecord, $result); - // 5. Current user, single record, cache expired - $SESSION->completioncache[42]['updated']=37; // Quite a long time ago. - $now = time(); - $SESSION->completioncache[17]['updated']=$now; - $SESSION->completioncache[39]['updated']=72; // Also a long time ago. - $DB->expects($this->at(0)) - ->method('get_record') - ->with('course_modules_completion', array('coursemoduleid'=>13, 'userid'=>314159)) - ->will($this->returnValue($sillyrecord)); - $result = $c->get_data($cm, false); - $this->assertEquals($sillyrecord, $result); - - // Check that updated value is right, then fudge it to make next compare work. - $this->assertTrue(time()-$SESSION->completioncache[42]['updated']<2); - $SESSION->completioncache[42]['updated']=$now; - // Check things got expired from cache. - $this->assertEquals(array(42=>array(13=>$sillyrecord, 'updated'=>$now), 17=>array('updated'=>$now)), $SESSION->completioncache); - - // 6. Current user, 'whole course' and record not in cache. - unset($SESSION->completioncache); + // 5. Current user, 'whole course' and record not in cache. + $cache->purge(); // Scenario: Completion data exists for one CMid. $basicrecord = (object)array('coursemoduleid'=>13); @@ -476,15 +445,14 @@ class core_completionlib_testcase extends advanced_testcase { $this->assertEquals($basicrecord, $result); // Check the cache contents. - $this->assertTrue(time()-$SESSION->completioncache[42]['updated']<2); - $SESSION->completioncache[42]['updated'] = $now; - $this->assertEquals(array(42=>array(13=>$basicrecord, 14=>(object)array( - 'id'=>'0', 'coursemoduleid'=>14, 'userid'=>314159, 'completionstate'=>0, - 'viewed'=>0, 'timemodified'=>0), 'updated'=>$now)), $SESSION->completioncache); + $this->assertEquals($basicrecord, $cache->get('314159_42_13')); + $this->assertEquals((object)array('id'=>'0', 'coursemoduleid'=>14, + 'userid'=>314159, 'completionstate'=>0, 'viewed'=>0, 'timemodified'=>0), + $cache->get('314159_42_14')); } public function test_internal_set_data() { - global $DB, $SESSION; + global $DB; $this->setup_data(); $this->setUser($this->user); @@ -505,10 +473,11 @@ class core_completionlib_testcase extends advanced_testcase { $c->internal_set_data($cm, $data); $d1 = $DB->get_field('course_modules_completion', 'id', array('coursemoduleid' => $cm->id)); $this->assertEquals($d1, $data->id); - $this->assertEquals(array($this->course->id => array($cm->id => $data)), $SESSION->completioncache); + $cache = cache::make('core', 'completion'); + $this->assertEquals($cache->get($data->userid . '_' . $cm->course . '_' . $cm->id), + $data); - // 2) Test with existing data and for different user (not cached). - unset($SESSION->completioncache); + // 2) Test with existing data and for different user. $forum2 = $this->getDataGenerator()->create_module('forum', array('course' => $this->course->id), $completionauto); $cm2 = get_coursemodule_from_instance('forum', $forum2->id); $newuser = $this->getDataGenerator()->create_user(); @@ -521,7 +490,10 @@ class core_completionlib_testcase extends advanced_testcase { $d2->timemodified = time(); $d2->viewed = COMPLETION_NOT_VIEWED; $c->internal_set_data($cm2, $d2); - $this->assertFalse(isset($SESSION->completioncache)); + $this->assertEquals($cache->get($data->userid . '_' . $cm->course . '_' . $cm->id), + $data); + $this->assertEquals($cache->get($d2->userid . '_' . $cm2->course . '_' . $cm2->id), + $d2); // 3) Test where it THINKS the data is new (from cache) but actually // in the database it has been set since.