Skip to content

Commit

Permalink
Potential fix for missing / broken course context id
Browse files Browse the repository at this point in the history
  • Loading branch information
@rrusso
rrusso committed Feb 3, 2024
1 parent 74ae443 commit d278f55
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 15 deletions.
7 changes: 6 additions & 1 deletion course/format/classes/stateactions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,13 @@ protected function set_cm_visibility(
$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);
}
}
Expand Down
22 changes: 15 additions & 7 deletions course/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -4005,7 +4014,6 @@ function course_get_user_administration_options($course, $context) {

// BEGIN LSU course restore limits to course ceators.
$teachersrestore = isset($CFG->teachersrestore) ? $CFG->teachersrestore : false;

if (!$teachersrestore) {
$options->restore = has_capability('moodle/restore:restorecourse', $context) && has_capability('moodle/course:create', $context);
} else {
Expand Down
47 changes: 47 additions & 0 deletions course/tests/courselib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,53 @@ public function test_module_visibility() {
}
}

/**
* 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);
Expand Down
5 changes: 5 additions & 0 deletions course/upgrade.txt
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
36 changes: 29 additions & 7 deletions lib/modinfolib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -742,19 +742,41 @@ public static function purge_course_section_cache_by_number(int $courseid, int $
* @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);
}

/**
Expand Down
82 changes: 82 additions & 0 deletions lib/tests/modinfolib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,88 @@ public function test_section_cache_by_number(): void {
$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.
*
Expand Down
4 changes: 4 additions & 0 deletions lib/upgrade.txt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
This files describes API changes in core libraries and APIs,
information provided here is intended especially for developers.

=== 4.1.7 ===
* 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
the site's fullname/shortname has been set. So there's no need to manually add the site name whenever calling $PAGE->set_title().
Expand Down

0 comments on commit d278f55

Please sign in to comment.