diff --git a/course/format/classes/stateactions.php b/course/format/classes/stateactions.php index 0f1a6ffc362..4150dbf6032 100644 --- a/course/format/classes/stateactions.php +++ b/course/format/classes/stateactions.php @@ -416,8 +416,13 @@ class stateactions { $allowstealth = !empty($CFG->allowstealth) && $format->allow_stealth_module_visibility($cm, $section); $coursevisible = ($allowstealth) ? 0 : 1; } - set_coursemodule_visible($cm->id, $visible, $coursevisible); + set_coursemodule_visible($cm->id, $visible, $coursevisible, false); course_module_updated::create_from_cm($cm, $modcontext)->trigger(); + } + course_modinfo::purge_course_modules_cache($course->id, $ids); + rebuild_course_cache($course->id, false, true); + + foreach ($cms as $cm) { $updates->add_cm_put($cm->id); } } diff --git a/course/lib.php b/course/lib.php index 505411acfac..19c2700889b 100644 --- a/course/lib.php +++ b/course/lib.php @@ -709,12 +709,17 @@ function set_downloadcontent(int $id, bool $downloadcontent): bool { * has been moved to {@link set_section_visible()} which was the only place from which * the parameter was used. * + * If $rebuildcache is set to false, the calling code is responsible for ensuring the cache is purged + * and rebuilt as appropriate. Consider using this if set_coursemodule_visible is called multiple times + * (e.g. in a loop). + * * @param int $id of the module * @param int $visible state of the module * @param int $visibleoncoursepage state of the module on the course page + * @param bool $rebuildcache If true (default), perform a partial cache purge and rebuild. * @return bool false when the module was not found, true otherwise */ -function set_coursemodule_visible($id, $visible, $visibleoncoursepage = 1) { +function set_coursemodule_visible($id, $visible, $visibleoncoursepage = 1, bool $rebuildcache = true) { global $DB, $CFG; require_once($CFG->libdir.'/gradelib.php'); require_once($CFG->dirroot.'/calendar/lib.php'); @@ -771,8 +776,10 @@ function set_coursemodule_visible($id, $visible, $visibleoncoursepage = 1) { } } - \course_modinfo::purge_course_module_cache($cm->course, $cm->id); - rebuild_course_cache($cm->course, false, true); + if ($rebuildcache) { + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + rebuild_course_cache($cm->course, false, true); + } return true; } @@ -1457,20 +1464,22 @@ function course_update_section($course, $section, $data) { // If section visibility was changed, hide the modules in this section too. if ($changevisibility && !empty($section->sequence)) { $modules = explode(',', $section->sequence); + $cmids = []; foreach ($modules as $moduleid) { if ($cm = get_coursemodule_from_id(null, $moduleid, $courseid)) { + $cmids[] = $cm->id; if ($data['visible']) { // As we unhide the section, we use the previously saved visibility stored in visibleold. - set_coursemodule_visible($moduleid, $cm->visibleold, $cm->visibleoncoursepage); + set_coursemodule_visible($moduleid, $cm->visibleold, $cm->visibleoncoursepage, false); } else { // We hide the section, so we hide the module but we store the original state in visibleold. - set_coursemodule_visible($moduleid, 0, $cm->visibleoncoursepage); + set_coursemodule_visible($moduleid, 0, $cm->visibleoncoursepage, false); $DB->set_field('course_modules', 'visibleold', $cm->visible, ['id' => $moduleid]); - \course_modinfo::purge_course_module_cache($cm->course, $cm->id); } \core\event\course_module_updated::create_from_cm($cm)->trigger(); } } + \course_modinfo::purge_course_modules_cache($courseid, $cmids); rebuild_course_cache($courseid, false, true); } } diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index a811dd38b07..860c814ce2e 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1333,6 +1333,53 @@ class courselib_test extends advanced_testcase { } } + /** + * Test rebuildcache = false behaviour. + * + * When we pass rebuildcache = false to set_coursemodule_visible, the corusemodinfo cache will still contain + * the original visibility until we trigger a rebuild. + * + * @return void + * @covers ::set_coursemodule_visible + */ + public function test_module_visibility_no_rebuild(): void { + $this->setAdminUser(); + $this->resetAfterTest(true); + + // Create course and modules. + $course = $this->getDataGenerator()->create_course(['numsections' => 5]); + $forum = $this->getDataGenerator()->create_module('forum', ['course' => $course->id]); + $assign = $this->getDataGenerator()->create_module('assign', ['duedate' => time(), 'course' => $course->id]); + $modules = compact('forum', 'assign'); + + // Hiding the modules. + foreach ($modules as $mod) { + set_coursemodule_visible($mod->cmid, 0, 1, false); + // The modinfo cache still has the original visibility until we manually trigger a rebuild. + $cm = get_fast_modinfo($mod->course)->get_cm($mod->cmid); + $this->assertEquals(1, $cm->visible); + } + + rebuild_course_cache($course->id); + + foreach ($modules as $mod) { + $this->check_module_visibility($mod, 0, 0); + } + + // Showing the modules. + foreach ($modules as $mod) { + set_coursemodule_visible($mod->cmid, 1, 1, false); + $cm = get_fast_modinfo($mod->course)->get_cm($mod->cmid); + $this->assertEquals(0, $cm->visible); + } + + rebuild_course_cache($course->id); + + foreach ($modules as $mod) { + $this->check_module_visibility($mod, 1, 1); + } + } + public function test_section_visibility_events() { $this->setAdminUser(); $this->resetAfterTest(true); diff --git a/course/upgrade.txt b/course/upgrade.txt index bfdc7f62979..c066c42c279 100644 --- a/course/upgrade.txt +++ b/course/upgrade.txt @@ -1,6 +1,11 @@ This files describes API changes in /course/*, information provided here is intended especially for developers. +=== 4.1.7 === +* set_coursemodule_visible() has a new $rebuildcache parameter. If this is being called multiple times in the same request, + consider passing `false` for this parameter and rebuilding the cache once after all the course modules have been updated. + See course_update_section() for an example. + === 4.1 === * The function course_modchooser() has been finally deprecated and can not be used anymore. Please use course_activitychooser() instead. diff --git a/lib/modinfolib.php b/lib/modinfolib.php index d64bc4dfbce..3e855a7c61e 100644 --- a/lib/modinfolib.php +++ b/lib/modinfolib.php @@ -742,19 +742,41 @@ class course_modinfo { * @param int $cmid Course module id */ public static function purge_course_module_cache(int $courseid, int $cmid): void { + self::purge_course_modules_cache($courseid, [$cmid]); + } + + /** + * Purge the cache of multiple course modules. + * + * @param int $courseid Course id + * @param int[] $cmids List of course module ids + * @return void + */ + public static function purge_course_modules_cache(int $courseid, array $cmids): void { $course = get_course($courseid); $cache = cache::make('core', 'coursemodinfo'); $cachekey = $course->id; $cache->acquire_lock($cachekey); - $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev); - $hascache = ($coursemodinfo !== false) && array_key_exists($cmid, $coursemodinfo->modinfo); - if ($hascache) { - $coursemodinfo->cacherev = -1; - unset($coursemodinfo->modinfo[$cmid]); - $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo); + try { $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev); + $hascache = ($coursemodinfo !== false); + $updatedcache = false; + if ($hascache) { + foreach ($cmids as $cmid) { + if (array_key_exists($cmid, $coursemodinfo->modinfo)) { + unset($coursemodinfo->modinfo[$cmid]); + $updatedcache = true; + } + } + if ($updatedcache) { + $coursemodinfo->cacherev = -1; + $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo); + $cache->get_versioned($cachekey, $course->cacherev); + } + } + } finally { + $cache->release_lock($cachekey); } - $cache->release_lock($cachekey); } /** diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php index f99049a1d1f..fbd2ed54278 100644 --- a/lib/tests/modinfolib_test.php +++ b/lib/tests/modinfolib_test.php @@ -1099,6 +1099,88 @@ class modinfolib_test extends advanced_testcase { $this->assertEquals(-1, $coursemodinfo->cacherev); } + /** + * Purge a single course module from the cache. + * + * @return void + * @covers \course_modinfo::purge_course_module_cache + */ + public function test_purge_course_module(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $cache = cache::make('core', 'coursemodinfo'); + + // Generate the course and pre-requisite section. + $course = $this->getDataGenerator()->create_course(); + $cm1 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + $cm2 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + $cm3 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + $cm4 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + // Reset course cache. + rebuild_course_cache($course->id, true); + // Build course cache. + get_fast_modinfo($course->id); + // Get the course modinfo cache. + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); + $this->assertCount(4, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm1->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm2->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm3->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm4->cmid, $coursemodinfo->modinfo); + + course_modinfo::purge_course_module_cache($course->id, $cm1->cmid); + + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); + $this->assertCount(3, $coursemodinfo->modinfo); + $this->assertArrayNotHasKey($cm1->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm2->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm3->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm4->cmid, $coursemodinfo->modinfo); + // Make sure that the cacherev will be reset. + $this->assertEquals(-1, $coursemodinfo->cacherev); + } + + /** + * Purge a multiple course modules from the cache. + * + * @return void + * @covers \course_modinfo::purge_course_modules_cache + */ + public function test_purge_multiple_course_modules(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $cache = cache::make('core', 'coursemodinfo'); + + // Generate the course and pre-requisite section. + $course = $this->getDataGenerator()->create_course(); + $cm1 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + $cm2 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + $cm3 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + $cm4 = $this->getDataGenerator()->create_module('page', ['course' => $course]); + // Reset course cache. + rebuild_course_cache($course->id, true); + // Build course cache. + get_fast_modinfo($course->id); + // Get the course modinfo cache. + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); + $this->assertCount(4, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm1->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm2->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm3->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm4->cmid, $coursemodinfo->modinfo); + + course_modinfo::purge_course_modules_cache($course->id, [$cm2->cmid, $cm3->cmid]); + + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); + $this->assertCount(2, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm1->cmid, $coursemodinfo->modinfo); + $this->assertArrayNotHasKey($cm2->cmid, $coursemodinfo->modinfo); + $this->assertArrayNotHasKey($cm3->cmid, $coursemodinfo->modinfo); + $this->assertArrayHasKey($cm4->cmid, $coursemodinfo->modinfo); + // Make sure that the cacherev will be reset. + $this->assertEquals(-1, $coursemodinfo->cacherev); + } + /** * Test get_cm() method to output course module id in the exception text. * diff --git a/lib/upgrade.txt b/lib/upgrade.txt index d868f1ceb82..ca72da46848 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -6,6 +6,8 @@ information provided here is intended especially for developers. (via options.pending). This is a backport of patch MDL-78779. * Add a new parameter to the debounce (core/utils) function to allow for cancellation. * Add a new method core_user::get_initials to get the initials of a user in a way compatible with internationalisation. +* course_modinfo now has a purge_course_modules_cache() method, which takes a list of cmids and purges + them all in a single cache set. === 4.1.6 === * \moodle_page::set_title() has been updated to append the site name depending on the value of $CFG->sitenameintitle and whether