Skip to content

Commit

Permalink
MDL-46960 completionlib: Move completion cache to MUC.
Browse files Browse the repository at this point in the history
  • Loading branch information
@marinaglancy
Michael Wheeler authored and marinaglancy committed Mar 31, 2015
1 parent 20d3883 commit 3871db0
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 130 deletions.
3 changes: 2 additions & 1 deletion lang/en/cache.php
View file
Expand Up @@ -43,6 +43,7 @@
$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';
Expand Down Expand Up @@ -173,4 +174,4 @@
// 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.';
$string['lockmethod_help'] = 'This is the method used for locking when required of this store.';
91 changes: 19 additions & 72 deletions lib/completionlib.php
View file
Expand Up @@ -119,12 +119,6 @@
*/
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.
Expand Down Expand Up @@ -553,7 +547,7 @@ public function is_course_complete($user_id) {
* @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)) {
Expand Down Expand Up @@ -792,20 +786,13 @@ public function delete_course_completion_data() {
* 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();
}
Expand All @@ -818,19 +805,11 @@ public function delete_all_completion_data() {
* @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;
Expand Down Expand Up @@ -876,7 +855,7 @@ public function reset_all_state($cm) {
}
$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)
Expand All @@ -894,7 +873,7 @@ public function reset_all_state($cm) {

/**
* 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
Expand All @@ -908,39 +887,21 @@ public function reset_all_state($cm) {
* @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
Expand Down Expand Up @@ -976,14 +937,13 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null
$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
Expand All @@ -1000,13 +960,7 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null
}

// 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;
Expand All @@ -1022,7 +976,7 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null
* @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) {
Expand Down Expand Up @@ -1054,8 +1008,10 @@ public function internal_set_data($cm, $data) {
$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);
}
Expand Down Expand Up @@ -1341,13 +1297,4 @@ public function internal_systemerror($error) {
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);
}
}
7 changes: 7 additions & 0 deletions lib/db/caches.php
View file
Expand Up @@ -213,6 +213,13 @@
'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
Expand Down
86 changes: 29 additions & 57 deletions lib/tests/completionlib_test.php
View file
Expand Up @@ -325,7 +325,7 @@ public function test_count_user_data() {
}

public function test_delete_all_state() {
global $DB, $SESSION;
global $DB;
$this->mock_setup();

$course = (object)array('id'=>13);
Expand All @@ -339,21 +339,6 @@ public function test_delete_all_state() {
->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() {
Expand Down Expand Up @@ -396,9 +381,11 @@ public function test_reset_all_state() {
}

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);

Expand All @@ -412,18 +399,20 @@ public function test_get_data() {
->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))
Expand All @@ -432,34 +421,14 @@ public function test_get_data() {
->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);
Expand All @@ -476,15 +445,14 @@ public function test_get_data() {
$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);
Expand All @@ -505,10 +473,11 @@ public function test_internal_set_data() {
$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();
Expand All @@ -521,7 +490,10 @@ public function test_internal_set_data() {
$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.
Expand Down

0 comments on commit 3871db0

Please sign in to comment.