MDL-68388 core_grades: Use MUC for grade letters
authorMarcus Boon <marcus@catalyst-au.net>
Thu, 9 Apr 2020 03:00:29 +0000 (13:00 +1000)
committerMarcus Boon <marcus@catalyst-au.net>
Mon, 25 May 2020 23:06:50 +0000 (09:06 +1000)
In the get_grade_letters there is a static variable that is used
to cache grade letters, we should use MUC for this so that it is
reset properly between unit tests.

grade/edit/letter/index.php
lang/en/cache.php
lib/db/caches.php
lib/gradelib.php
lib/tests/gradelib_test.php

index 9f4e075..b72e203 100644 (file)
@@ -137,6 +137,10 @@ if (!$edit) {
         redirect($returnurl);
 
     } else if ($data = $mform->get_data()) {
+
+        // Make sure we are updating the cache.
+        $cache = cache::make('core', 'grade_letters');
+
         if (!$admin and empty($data->override)) {
             $records = $DB->get_records('grade_letters', array('contextid' => $context->id));
             foreach ($records as $record) {
@@ -148,6 +152,9 @@ if (!$edit) {
                 ));
                 $event->trigger();
             }
+
+            // Make sure we clear the cache for this context.
+            $cache->delete($context->id);
             redirect($returnurl);
         }
 
@@ -222,6 +229,15 @@ if (!$edit) {
             }
         }
 
+        // Cache the changed letters.
+        if (!empty($letters)) {
+
+            // For some reason, the cache saves it in the order in which they were entered
+            // but we really want to order them in descending order so we sort it here.
+            krsort($letters);
+            $cache->set($context->id, $letters);
+        }
+
         // Delete the unused records.
         foreach($pool as $leftover) {
             $DB->delete_records('grade_letters', array('id' => $leftover->id));
index cb59517..6c24f1b 100644 (file)
@@ -78,6 +78,7 @@ $string['cachedef_recommendation_favourite_course_content_items'] = 'Recommendat
 $string['cachedef_repositories'] = 'Repositories instances data';
 $string['cachedef_roledefs'] = 'Role definitions';
 $string['cachedef_grade_categories'] = 'Grade category queries';
+$string['cachedef_grade_letters'] = 'Grade letters queries';
 $string['cachedef_string'] = 'Language string cache';
 $string['cachedef_tags'] = 'Tags collections and areas';
 $string['cachedef_temp_tables'] = 'Temporary tables cache';
index a5bd37e..ac2fa19 100644 (file)
@@ -454,4 +454,12 @@ $definitions = array(
         'mode' => cache_store::MODE_APPLICATION,
         'simpledata' => true,
     ],
+
+    // Cache the grade letters for faster retrival.
+    'grade_letters' => [
+        'mode'                   => cache_store::MODE_REQUEST,
+        'simplekeys'             => true,
+        'staticacceleration'     => true,
+        'staticaccelerationsize' => 100
+    ],
 );
index ef19298..02919f1 100644 (file)
@@ -955,14 +955,11 @@ function grade_get_letters($context=null) {
         return array('93'=>'A', '90'=>'A-', '87'=>'B+', '83'=>'B', '80'=>'B-', '77'=>'C+', '73'=>'C', '70'=>'C-', '67'=>'D+', '60'=>'D', '0'=>'F');
     }
 
-    static $cache = array();
-
-    if (array_key_exists($context->id, $cache)) {
-        return $cache[$context->id];
-    }
+    $cache = cache::make('core', 'grade_letters');
+    $data = $cache->get($context->id);
 
-    if (count($cache) > 100) {
-        $cache = array(); // cache size limit
+    if (!empty($data)) {
+        return $data;
     }
 
     $letters = array();
@@ -978,13 +975,15 @@ function grade_get_letters($context=null) {
         }
 
         if (!empty($letters)) {
-            $cache[$context->id] = $letters;
+            // Cache the grade letters for this context.
+            $cache->set($context->id, $letters);
             return $letters;
         }
     }
 
     $letters = grade_get_letters(null);
-    $cache[$context->id] = $letters;
+    // Cache the grade letters for this context.
+    $cache->set($context->id, $letters);
     return $letters;
 }
 
@@ -1430,6 +1429,9 @@ function remove_grade_letters($context, $showfeedback) {
     if ($showfeedback) {
         echo $OUTPUT->notification($strdeleted.' - '.get_string('letters', 'grades'), 'notifysuccess');
     }
+
+    $cache = cache::make('core', 'grade_letters');
+    $cache->delete($context->id);
 }
 
 /**
index a3e2b3d..737aca9 100644 (file)
@@ -76,10 +76,28 @@ class core_gradelib_testcase extends advanced_testcase {
         $letter->contextid = $context->id;
         $DB->insert_record('grade_letters', $letter);
 
+        // Pre-warm the cache, ensure that that the letter is cached.
+        $cache = cache::make('core', 'grade_letters');
+
+        // Check that the cache is empty beforehand.
+        $letters = $cache->get($context->id);
+        $this->assertFalse($letters);
+
+        // Call the function.
+        grade_get_letters($context);
+
+        $letters = $cache->get($context->id);
+        $this->assertEquals(1, count($letters));
+        $this->assertEquals($letter->letter, $letters['100.00000']);
+
         remove_grade_letters($context, false);
 
         // Confirm grade letter was deleted.
         $this->assertEquals(0, $DB->count_records('grade_letters'));
+
+        // Confirm grade letter is also deleted from cache.
+        $letters = $cache->get($context->id);
+        $this->assertFalse($letters);
     }
 
     /**
@@ -228,4 +246,59 @@ class core_gradelib_testcase extends advanced_testcase {
             ],
         ];
     }
+
+    /**
+     * Test the caching of grade letters.
+     */
+    public function test_get_grade_letters() {
+
+        $this->resetAfterTest();
+
+        // Setup some basics.
+        $course = $this->getDataGenerator()->create_course();
+        $context = context_course::instance($course->id);
+
+        $cache = cache::make('core', 'grade_letters');
+        $letters = $cache->get($context->id);
+
+        // Make sure the cache is empty.
+        $this->assertFalse($letters);
+
+        // Now check to see if the letters get cached.
+        $actual = grade_get_letters($context);
+
+        $expected = $cache->get($context->id);
+
+        $this->assertEquals($expected, $actual);
+    }
+
+    /**
+     * Test custom letters.
+     */
+    public function test_get_grade_letters_custom() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $context = context_course::instance($course->id);
+
+        $cache = cache::make('core', 'grade_letters');
+        $letters = $cache->get($context->id);
+
+        // Make sure the cache is empty.
+        $this->assertFalse($letters);
+
+        // Add a grade letter to the course.
+        $letter = new stdClass();
+        $letter->letter = 'M';
+        $letter->lowerboundary = '100';
+        $letter->contextid = $context->id;
+        $DB->insert_record('grade_letters', $letter);
+
+        $actual = grade_get_letters($context);
+        $expected = $cache->get($context->id);
+
+        $this->assertEquals($expected, $actual);
+    }
 }