From 02bb3263146dbff4e3b54016686c1d8a8a772f3d Mon Sep 17 00:00:00 2001
From: sam marshall <s.marshall@open.ac.uk>
Date: Wed, 12 Jan 2022 13:21:49 +0000
Subject: [PATCH] MDL-72837 core_cache: Use versioned cache for modinfo

Uses the new versioned cache feature for modinfo, which should make it
safe as a localisable cache.
---
 course/tests/courselib_test.php |  5 +++--
 lib/modinfolib.php              | 25 ++++++++++---------------
 lib/tests/modinfolib_test.php   | 16 ++++++++--------
 3 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php
index 51490347e3d..b02ca689253 100644
--- a/course/tests/courselib_test.php
+++ b/course/tests/courselib_test.php
@@ -884,8 +884,9 @@ class courselib_test extends advanced_testcase {
         $this->assertEquals($cmids[0] . ',' . $cmids[1], $sequence);
 
         // Check that modinfo cache was reset but not rebuilt (important for performance if calling repeatedly).
-        $this->assertGreaterThan($coursecacherev, $DB->get_field('course', 'cacherev', array('id' => $course->id)));
-        $this->assertEmpty(cache::make('core', 'coursemodinfo')->get($course->id));
+        $newcacherev = $DB->get_field('course', 'cacherev', ['id' => $course->id]);
+        $this->assertGreaterThan($coursecacherev, $newcacherev);
+        $this->assertEmpty(cache::make('core', 'coursemodinfo')->get_versioned($course->id, $newcacherev));
 
         // Add one to section that doesn't exist (this might rebuild modinfo).
         course_add_cm_to_section($course, $cmids[2], 2);
diff --git a/lib/modinfolib.php b/lib/modinfolib.php
index b962a133385..c3fe9ee17f4 100644
--- a/lib/modinfolib.php
+++ b/lib/modinfolib.php
@@ -475,14 +475,14 @@ class course_modinfo {
         $cachecoursemodinfo = cache::make('core', 'coursemodinfo');
 
         // Retrieve modinfo from cache. If not present or cacherev mismatches, call rebuild and retrieve again.
-        $coursemodinfo = $cachecoursemodinfo->get($course->id);
-        if ($coursemodinfo === false || ($course->cacherev != $coursemodinfo->cacherev)) {
+        $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev);
+        if (!$coursemodinfo) {
             $lock = self::get_course_cache_lock($course->id);
             try {
                 // Only actually do the build if it's still needed after getting the lock (not if
                 // somebody else, who might have been holding the lock, built it already).
-                $coursemodinfo = $cachecoursemodinfo->get($course->id);
-                if ($coursemodinfo === false || ($course->cacherev != $coursemodinfo->cacherev)) {
+                $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev);
+                if (!$coursemodinfo) {
                     $coursemodinfo = self::inner_build_course_cache($course, $lock);
                 }
             } finally {
@@ -681,17 +681,12 @@ class course_modinfo {
         global $DB, $CFG;
         require_once("{$CFG->dirroot}/course/lib.php");
 
-        // Ensure object has all necessary fields.
-        foreach (self::$cachedfields as $key) {
-            if (!isset($course->$key)) {
-                $course = $DB->get_record('course', array('id' => $course->id),
-                        implode(',', array_merge(array('id'), self::$cachedfields)), MUST_EXIST);
-                break;
-            }
-        }
+        // Always reload the course object from database to ensure we have the latest possible
+        // value for cacherev.
+        $course = $DB->get_record('course', ['id' => $course->id],
+                implode(',', array_merge(['id'], self::$cachedfields)), MUST_EXIST);
+
         // Retrieve all information about activities and sections.
-        // This may take time on large courses and it is possible that another user modifies the same course during this process.
-        // Field cacherev stored in both DB and cache will ensure that cached data matches the current course state.
         $coursemodinfo = new stdClass();
         $coursemodinfo->modinfo = get_array_of_activities($course->id);
         $coursemodinfo->sectioncache = self::build_course_section_cache($course);
@@ -700,7 +695,7 @@ class course_modinfo {
         }
         // Set the accumulated activities and sections information in cache, together with cacherev.
         $cachecoursemodinfo = cache::make('core', 'coursemodinfo');
-        $cachecoursemodinfo->set($course->id, $coursemodinfo);
+        $cachecoursemodinfo->set_versioned($course->id, $course->cacherev, $coursemodinfo);
         return $coursemodinfo;
     }
 }
diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php
index c4ab9a8ef3d..50d7431dbf9 100644
--- a/lib/tests/modinfolib_test.php
+++ b/lib/tests/modinfolib_test.php
@@ -260,28 +260,28 @@ class modinfolib_test extends advanced_testcase {
         rebuild_course_cache($course->id, true);
         $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id));
         $this->assertGreaterThan($prevcacherev, $cacherev);
-        $this->assertEmpty($cache->get($course->id));
+        $this->assertEmpty($cache->get_versioned($course->id, $prevcacherev));
         $prevcacherev = $cacherev;
 
         // Build course cache. Cacherev should not change but cache is now not empty. Make sure cacherev is the same everywhere.
         $modinfo = get_fast_modinfo($course->id);
         $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id));
         $this->assertEquals($prevcacherev, $cacherev);
-        $cachedvalue = $cache->get($course->id);
+        $cachedvalue = $cache->get_versioned($course->id, $cacherev);
         $this->assertNotEmpty($cachedvalue);
         $this->assertEquals($cacherev, $cachedvalue->cacherev);
         $this->assertEquals($cacherev, $modinfo->get_course()->cacherev);
         $prevcacherev = $cacherev;
 
         // Little trick to check that cache is not rebuilt druing the next step - substitute the value in MUC and later check that it is still there.
-        $cache->set($course->id, (object)array_merge((array)$cachedvalue, array('secretfield' => 1)));
+        $cache->set_versioned($course->id, $cacherev, (object)array_merge((array)$cachedvalue, array('secretfield' => 1)));
 
         // Clear static cache and call get_fast_modinfo() again (pretend we are in another request). Cache should not be rebuilt.
         course_modinfo::clear_instance_cache();
         $modinfo = get_fast_modinfo($course->id);
         $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id));
         $this->assertEquals($prevcacherev, $cacherev);
-        $cachedvalue = $cache->get($course->id);
+        $cachedvalue = $cache->get_versioned($course->id, $cacherev);
         $this->assertNotEmpty($cachedvalue);
         $this->assertEquals($cacherev, $cachedvalue->cacherev);
         $this->assertNotEmpty($cachedvalue->secretfield);
@@ -292,7 +292,7 @@ class modinfolib_test extends advanced_testcase {
         rebuild_course_cache($course->id);
         $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id));
         $this->assertGreaterThan($prevcacherev, $cacherev);
-        $cachedvalue = $cache->get($course->id);
+        $cachedvalue = $cache->get_versioned($course->id, $cacherev);
         $this->assertNotEmpty($cachedvalue);
         $this->assertEquals($cacherev, $cachedvalue->cacherev);
         $modinfo = get_fast_modinfo($course->id);
@@ -306,7 +306,7 @@ class modinfolib_test extends advanced_testcase {
         $modinfo = get_fast_modinfo($course->id);
         $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id));
         $this->assertGreaterThan($prevcacherev, $cacherev);
-        $cachedvalue = $cache->get($course->id);
+        $cachedvalue = $cache->get_versioned($course->id, $cacherev);
         $this->assertNotEmpty($cachedvalue);
         $this->assertEquals($cacherev, $cachedvalue->cacherev);
         $this->assertEquals($cacherev, $modinfo->get_course()->cacherev);
@@ -316,10 +316,10 @@ class modinfolib_test extends advanced_testcase {
         rebuild_course_cache(0, true);
         $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id));
         $this->assertGreaterThan($prevcacherev, $cacherev);
-        $this->assertEmpty($cache->get($course->id));
+        $this->assertEmpty($cache->get_versioned($course->id, $cacherev));
         // Rebuild again.
         $modinfo = get_fast_modinfo($course->id);
-        $cachedvalue = $cache->get($course->id);
+        $cachedvalue = $cache->get_versioned($course->id, $cacherev);
         $this->assertNotEmpty($cachedvalue);
         $this->assertEquals($cacherev, $cachedvalue->cacherev);
         $this->assertEquals($cacherev, $modinfo->get_course()->cacherev);