From 93cb5b090a046bf8a6fa8b301d8288b5078561d3 Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Thu, 23 Nov 2023 15:28:54 +0100 Subject: [PATCH] MDL-79999 course: save session cache by id The course modinfo cache now stores sections no matter the section number. This will be needed when delegated sections will be implemented. --- course/tests/courselib_test.php | 20 +++++----- lib/modinfolib.php | 70 +++++++++++++++++---------------- lib/tests/modinfolib_test.php | 45 ++++++++++++--------- 3 files changed, 73 insertions(+), 62 deletions(-) diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 2ac48b7937a..31f959b6fff 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1061,18 +1061,20 @@ class courselib_test extends advanced_testcase { rebuild_course_cache($course->id, true); // Build course cache. - get_fast_modinfo($course->id); + $modinfo = get_fast_modinfo($course->id); // Get the course modinfo cache. $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; + $numberedsections = $modinfo->get_section_info_all(); + // Make sure that we will have 4 section caches here. $this->assertCount(4, $sectioncaches); - $this->assertArrayHasKey(0, $sectioncaches); - $this->assertArrayHasKey(1, $sectioncaches); - $this->assertArrayHasKey(2, $sectioncaches); - $this->assertArrayHasKey(3, $sectioncaches); + $this->assertArrayHasKey($numberedsections[0]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[1]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[2]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[3]->id, $sectioncaches); // Move section. move_section_to($course, 2, 3); @@ -1083,10 +1085,10 @@ class courselib_test extends advanced_testcase { // Make sure that we will have 2 section caches left. $this->assertCount(2, $sectioncaches); - $this->assertArrayHasKey(0, $sectioncaches); - $this->assertArrayHasKey(1, $sectioncaches); - $this->assertArrayNotHasKey(2, $sectioncaches); - $this->assertArrayNotHasKey(3, $sectioncaches); + $this->assertArrayHasKey($numberedsections[0]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[1]->id, $sectioncaches); + $this->assertArrayNotHasKey($numberedsections[2]->id, $sectioncaches); + $this->assertArrayNotHasKey($numberedsections[3]->id, $sectioncaches); } /** diff --git a/lib/modinfolib.php b/lib/modinfolib.php index 9003f4a38fe..ff640ed1e89 100644 --- a/lib/modinfolib.php +++ b/lib/modinfolib.php @@ -581,12 +581,13 @@ class course_modinfo { // Expand section objects $this->sectioninfobynum = []; $this->sectioninfobyid = []; - foreach ($coursemodinfo->sectioncache as $number => $data) { - $sectioninfo = new section_info($data, $number, null, null, + foreach ($coursemodinfo->sectioncache as $data) { + $sectioninfo = new section_info($data, $data->section, null, null, $this, null); - $this->sectioninfobynum[$number] = $sectioninfo; - $this->sectioninfobyid[$data->id] = $this->sectioninfobynum[$number]; + $this->sectioninfobynum[$data->section] = $sectioninfo; + $this->sectioninfobyid[$data->id] = $sectioninfo; } + ksort($this->sectioninfobynum); } /** @@ -605,21 +606,25 @@ class course_modinfo { * the course cache. (Does not include information that is already cached * in some other way.) * - * @param stdClass $course Course object (must contain fields + * @param stdClass $course Course object (must contain fields id and cacherev) * @param boolean $usecache use cached section info if exists, use true for partial course rebuild - * @return array Information about sections, indexed by section number (not id) + * @return array Information about sections, indexed by section id (not number) */ protected static function build_course_section_cache(\stdClass $course, bool $usecache = false): array { global $DB; - // Get section data - $sections = $DB->get_records('course_sections', array('course' => $course->id), 'section', - 'section, id, course, name, summary, summaryformat, sequence, visible, availability'); + // Get section data. + $sections = $DB->get_records( + 'course_sections', + ['course' => $course->id], + 'section', + 'id, section, course, name, summary, summaryformat, sequence, visible, availability' + ); $compressedsections = []; $courseformat = course_get_format($course); if ($usecache) { - $cachecoursemodinfo = \cache::make('core', 'coursemodinfo'); + $cachecoursemodinfo = cache::make('core', 'coursemodinfo'); $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); if ($coursemodinfo !== false) { $compressedsections = $coursemodinfo->sectioncache; @@ -627,13 +632,14 @@ class course_modinfo { } $formatoptionsdef = course_get_format($course)->section_format_options(); - // Remove unnecessary data and add availability - foreach ($sections as $number => $section) { - $sectioninfocached = isset($compressedsections[$number]); + // Remove unnecessary data and add availability. + foreach ($sections as $section) { + $sectionid = $section->id; + $sectioninfocached = isset($compressedsections[$sectionid]); if ($sectioninfocached) { continue; } - // Add cached options from course format to $section object + // Add cached options from course format to $section object. foreach ($formatoptionsdef as $key => $option) { if (!empty($option['cache'])) { $formatoptions = $courseformat->get_format_options($section); @@ -642,12 +648,10 @@ class course_modinfo { } } } - // Clone just in case it is reused elsewhere - $compressedsections[$number] = clone($section); - section_info::convert_for_section_cache($compressedsections[$number]); + // Clone just in case it is reused elsewhere. + $compressedsections[$sectionid] = clone($section); + section_info::convert_for_section_cache($compressedsections[$sectionid]); } - - ksort($compressedsections); return $compressedsections; } @@ -730,15 +734,10 @@ class course_modinfo { $cache->acquire_lock($cachekey); try { $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev); - if ($coursemodinfo !== false) { - foreach ($coursemodinfo->sectioncache as $sectionno => $sectioncache) { - if ($sectioncache->id == $sectionid) { - $coursemodinfo->cacherev = -1; - unset($coursemodinfo->sectioncache[$sectionno]); - $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo); - break; - } - } + if ($coursemodinfo !== false && array_key_exists($sectionid, $coursemodinfo->sectioncache)) { + $coursemodinfo->cacherev = -1; + unset($coursemodinfo->sectioncache[$sectionid]); + $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo); } } finally { $cache->release_lock($cachekey); @@ -758,10 +757,15 @@ class course_modinfo { $cache->acquire_lock($cachekey); try { $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev); - if ($coursemodinfo !== false && array_key_exists($sectionno, $coursemodinfo->sectioncache)) { - $coursemodinfo->cacherev = -1; - unset($coursemodinfo->sectioncache[$sectionno]); - $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo); + if ($coursemodinfo !== false) { + foreach ($coursemodinfo->sectioncache as $sectionid => $sectioncache) { + if ($sectioncache->section == $sectionno) { + $coursemodinfo->cacherev = -1; + unset($coursemodinfo->sectioncache[$sectionid]); + $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo); + break; + } + } } } finally { $cache->release_lock($cachekey); @@ -3423,8 +3427,6 @@ class section_info implements IteratorAggregate { // Course id stored in course table unset($section->course); - // Section number stored in array key - unset($section->section); // Sequence stored implicity in modinfo $sections array unset($section->sequence); diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php index eab2bdf445c..e949586f77c 100644 --- a/lib/tests/modinfolib_test.php +++ b/lib/tests/modinfolib_test.php @@ -1119,21 +1119,26 @@ class modinfolib_test extends advanced_testcase { // Reset course cache. rebuild_course_cache($course->id, true); // Build course cache. - get_fast_modinfo($course->id); + $modinfo = get_fast_modinfo($course->id); // Get the course modinfo cache. $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; + $numberedsections = $modinfo->get_section_info_all(); + // Make sure that we will have 4 section caches here. $this->assertCount(4, $sectioncaches); - $this->assertArrayHasKey(0, $sectioncaches); - $this->assertArrayHasKey(1, $sectioncaches); - $this->assertArrayHasKey(2, $sectioncaches); - $this->assertArrayHasKey(3, $sectioncaches); + $this->assertArrayHasKey($numberedsections[0]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[1]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[2]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[3]->id, $sectioncaches); // Purge cache for the section by id. - course_modinfo::purge_course_section_cache_by_id($course->id, $sectioncaches[1]->id); + course_modinfo::purge_course_section_cache_by_id( + $course->id, + $numberedsections[1]->id + ); // Get the course modinfo cache. $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. @@ -1141,10 +1146,10 @@ class modinfolib_test extends advanced_testcase { // Make sure that we will have 3 section caches left. $this->assertCount(3, $sectioncaches); - $this->assertArrayNotHasKey(1, $sectioncaches); - $this->assertArrayHasKey(0, $sectioncaches); - $this->assertArrayHasKey(2, $sectioncaches); - $this->assertArrayHasKey(3, $sectioncaches); + $this->assertArrayNotHasKey($numberedsections[1]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[0]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[2]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[3]->id, $sectioncaches); // Make sure that the cacherev will be reset. $this->assertEquals(-1, $coursemodinfo->cacherev); } @@ -1165,18 +1170,20 @@ class modinfolib_test extends advanced_testcase { // Reset course cache. rebuild_course_cache($course->id, true); // Build course cache. - get_fast_modinfo($course->id); + $modinfo = get_fast_modinfo($course->id); // Get the course modinfo cache. $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; + $numberedsections = $modinfo->get_section_info_all(); + // Make sure that we will have 4 section caches here. $this->assertCount(4, $sectioncaches); - $this->assertArrayHasKey(0, $sectioncaches); - $this->assertArrayHasKey(1, $sectioncaches); - $this->assertArrayHasKey(2, $sectioncaches); - $this->assertArrayHasKey(3, $sectioncaches); + $this->assertArrayHasKey($numberedsections[0]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[1]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[2]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[3]->id, $sectioncaches); // Purge cache for the section with section number is 1. course_modinfo::purge_course_section_cache_by_number($course->id, 1); @@ -1187,10 +1194,10 @@ class modinfolib_test extends advanced_testcase { // Make sure that we will have 3 section caches left. $this->assertCount(3, $sectioncaches); - $this->assertArrayNotHasKey(1, $sectioncaches); - $this->assertArrayHasKey(0, $sectioncaches); - $this->assertArrayHasKey(2, $sectioncaches); - $this->assertArrayHasKey(3, $sectioncaches); + $this->assertArrayNotHasKey($numberedsections[1]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[0]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[2]->id, $sectioncaches); + $this->assertArrayHasKey($numberedsections[3]->id, $sectioncaches); // Make sure that the cacherev will be reset. $this->assertEquals(-1, $coursemodinfo->cacherev); }