From f97b86e841dfec3741e88098809686dc0cc8fe27 Mon Sep 17 00:00:00 2001 From: Dongsheng Cai Date: Fri, 16 Jul 2021 13:49:35 +1000 Subject: [PATCH 1/5] MDL-72991 Course: Partial course cache rebuild When updating/deleting a section/module, the system now only invalidate of the element (section/module), not the whole course cache Also, the system now only recalculate the cache for element (section/module) if necessary, not the whole course cache Move module/section purging to course_modinfo: + course_modinfo::purge_course_section_cache_by_id was created to purge section by id + course_modinfo::purge_course_section_cache_by_number was created to purge section by number + course_modinfo::purge_course_module_cache was created to purge module --- .../behat/availability_completion.feature | 55 +++ .../condition/date/classes/condition.php | 6 +- course/dnduploadlib.php | 4 +- course/format/classes/base.php | 17 +- course/lib.php | 316 ++++++++++-------- course/mod.php | 4 +- course/modlib.php | 3 +- course/tests/courselib_test.php | 1 + course/tests/externallib_test.php | 6 +- course/upgrade.txt | 3 + lib/db/caches.php | 12 +- lib/deprecatedlib.php | 41 +++ lib/modinfolib.php | 130 ++++++- lib/tests/modinfolib_test.php | 2 +- lib/upgrade.txt | 4 +- mod/chat/lib.php | 14 +- mod/lti/locallib.php | 21 +- 17 files changed, 458 insertions(+), 181 deletions(-) diff --git a/availability/condition/completion/tests/behat/availability_completion.feature b/availability/condition/completion/tests/behat/availability_completion.feature index 0f4c121d968..2ac19496286 100644 --- a/availability/condition/completion/tests/behat/availability_completion.feature +++ b/availability/condition/completion/tests/behat/availability_completion.feature @@ -54,3 +54,58 @@ Feature: availability_completion # Mark page 1 complete When I toggle the manual completion state of "Page 1" Then I should see "Page 2" in the "region-main" "region" + + @javascript + Scenario: Test completion and course cache rebuild + Given the following "activities" exist: + | activity | name | intro | course | idnumber | + | forum | forum 1 | forum 1 | C1 | forum1 | + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I open "forum 1" actions menu + And I click on "Edit settings" "link" in the "forum 1" activity + And I set the following fields to these values: + | Completion tracking | Show activity as complete when conditions are met | + | completionview | 1 | + | completionpostsenabled | 1 | + | completionposts | 2 | + And I press "Save and return to course" + And I add a new discussion to "forum 1" forum with: + | Subject | Forum post 1 | + | Message | This is the body | + And I am on "Course 1" course homepage with editing mode on + And I add a "Page" to section "2" + And I set the following fields to these values: + | Name | Page 2 | + | Description | Test | + | Page content | Test | + And I expand all fieldsets + And I press "Add restriction..." + And I click on "Activity completion" "button" in the "Add restriction..." "dialogue" + And I click on ".availability-item .availability-eye img" "css_element" + And I set the following fields to these values: + | Required completion status | must be marked complete | + | cm | forum 1 | + And I press "Save and return to course" + And I log out + And I log in as "student1" + When I am on "Course 1" course homepage + # Page 2 should not appear yet. + Then I should not see "Page 2" in the "region-main" "region" + And I click on "forum 1" "link" in the "region-main" "region" + # Page 2 should not appear yet. + And I should not see "Page 2" in the "region-main" "region" + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I am on the "forum 1" "forum activity editing" page + And I expand all fieldsets + And I set the following fields to these values: + | completionpostsenabled | 0 | + And I press "Save and display" + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I click on "forum 1" "link" in the "region-main" "region" + And I am on "Course 1" course homepage + And I should see "Page 2" in the "region-main" "region" diff --git a/availability/condition/date/classes/condition.php b/availability/condition/date/classes/condition.php index 63b61b4ebce..f7422ebbf74 100644 --- a/availability/condition/date/classes/condition.php +++ b/availability/condition/date/classes/condition.php @@ -292,14 +292,16 @@ class condition extends \core_availability\condition { $updatesection->availability = json_encode($tree->save()); $updatesection->timemodified = time(); $DB->update_record('course_sections', $updatesection); + // Invalidate the section cache by given section id. + \course_modinfo::purge_course_section_cache_by_id($courseid, $section->id); $anychanged = true; } } - // Ensure course cache is cleared if required. if ($anychanged) { - rebuild_course_cache($courseid, true); + // Partial rebuild the sections which have been invalidated. + rebuild_course_cache($courseid, true, true); } } } diff --git a/course/dnduploadlib.php b/course/dnduploadlib.php index cf23b5da001..d3be854817a 100644 --- a/course/dnduploadlib.php +++ b/course/dnduploadlib.php @@ -614,8 +614,10 @@ class dndupload_ajax_processor { $visible = get_fast_modinfo($this->course)->get_section_info($this->section)->visible; $DB->set_field('course_modules', 'instance', $instanceid, array('id' => $this->cm->id)); + + \course_modinfo::purge_course_module_cache($this->course->id, $this->cm->id); // Rebuild the course cache after update action - rebuild_course_cache($this->course->id, true); + rebuild_course_cache($this->course->id, true, true); $sectionid = course_add_cm_to_section($this->course, $this->cm->id, $this->section); diff --git a/course/format/classes/base.php b/course/format/classes/base.php index a88695341fd..cc83f570698 100644 --- a/course/format/classes/base.php +++ b/course/format/classes/base.php @@ -1095,7 +1095,15 @@ abstract class base { } } if ($needrebuild) { - rebuild_course_cache($this->courseid, true); + if ($sectionid) { + // Invalidate the section cache by given section id. + course_modinfo::purge_course_section_cache_by_id($this->courseid, $sectionid); + // Partial rebuild sections that have been invalidated. + rebuild_course_cache($this->courseid, true, true); + } else { + // Full rebuild if sectionid is null. + rebuild_course_cache($this->courseid); + } } if ($changed) { // Reset internal caches. @@ -1385,7 +1393,7 @@ abstract class base { } if (!is_object($section)) { $section = $DB->get_record('course_sections', array('course' => $this->get_courseid(), 'section' => $section), - 'id,section,sequence,summary'); + 'id,course,section,sequence,summary'); } if (!$section || !$section->section) { // Not possible to delete 0-section. @@ -1422,7 +1430,10 @@ abstract class base { // Delete section and it's format options. $DB->delete_records('course_format_options', array('sectionid' => $section->id)); $DB->delete_records('course_sections', array('id' => $section->id)); - rebuild_course_cache($course->id, true); + // Invalidate the section cache by given section id. + course_modinfo::purge_course_section_cache_by_id($course->id, $section->id); + // Partial rebuild section cache that has been purged. + rebuild_course_cache($course->id, true, true); // Delete section summary files. $context = \context_course::instance($course->id); diff --git a/course/lib.php b/course/lib.php index 59b6c1daf9f..dc663aa38ea 100644 --- a/course/lib.php +++ b/course/lib.php @@ -389,8 +389,12 @@ function course_integrity_check($courseid, $rawmods = null, $sections = null, $f /** * For a given course, returns an array of course activity objects * Each item in the array contains he following properties: + * + * @param int $courseid course id + * @param bool $usecache get activities from cache if modinfo exists when $usecache is true + * @return array list of activities */ -function get_array_of_activities($courseid) { +function get_array_of_activities(int $courseid, bool $usecache = false): array { // cm - course module id // mod - name of the module (eg forum) // section - the number of the section (eg week or topic) @@ -406,12 +410,21 @@ function get_array_of_activities($courseid) { throw new moodle_exception('courseidnotfound'); } - $mod = array(); - $rawmods = get_course_mods($courseid); if (empty($rawmods)) { - return $mod; // always return array + return []; } + + $mods = []; + if ($usecache) { + // Get existing cache. + $cachecoursemodinfo = cache::make('core', 'coursemodinfo'); + $coursemodinfo = $cachecoursemodinfo->get($courseid); + if ($coursemodinfo !== false) { + $mods = $coursemodinfo->modinfo; + } + } + $courseformat = course_get_format($course); if ($sections = $DB->get_records('course_sections', array('course' => $courseid), @@ -424,139 +437,151 @@ function get_array_of_activities($courseid) { 'section ASC', 'id,section,sequence,visible'); } // Build array of activities. - foreach ($sections as $section) { - if (!empty($section->sequence)) { - $sequence = explode(",", $section->sequence); - foreach ($sequence as $seq) { - if (empty($rawmods[$seq])) { - continue; - } - // Adjust visibleoncoursepage, value in DB may not respect format availability. - $rawmods[$seq]->visibleoncoursepage = (!$rawmods[$seq]->visible - || $rawmods[$seq]->visibleoncoursepage - || empty($CFG->allowstealth) - || !$courseformat->allow_stealth_module_visibility($rawmods[$seq], $section)) ? 1 : 0; + foreach ($sections as $section) { + if (!empty($section->sequence)) { + $cmids = explode(",", $section->sequence); + $numberofmods = count($cmids); + for ($order = 0; $order < $numberofmods; $order++) { + $cmid = $cmids[$order]; + // Activity does not exist in the database. + $notexistindb = empty($rawmods[$cmid]); + $activitycached = isset($mods[$cmid]); + if ($activitycached || $notexistindb) { + continue; + } + $modposition = ($order === 0) ? 0 : array_search($cmids[$order - 1], array_keys($mods)) + 1; + $mods = array_slice($mods, 0, $modposition, true) + + [$cmid => new stdClass()] + + array_slice($mods, $modposition, null, true); - // Create an object that will be cached. - $mod[$seq] = new stdClass(); - $mod[$seq]->id = $rawmods[$seq]->instance; - $mod[$seq]->cm = $rawmods[$seq]->id; - $mod[$seq]->mod = $rawmods[$seq]->modname; + // Adjust visibleoncoursepage, value in DB may not respect format availability. + $rawmods[$cmid]->visibleoncoursepage = (!$rawmods[$cmid]->visible + || $rawmods[$cmid]->visibleoncoursepage + || empty($CFG->allowstealth) + || !$courseformat->allow_stealth_module_visibility($rawmods[$cmid], $section)) ? 1 : 0; + + $mods[$cmid]->id = $rawmods[$cmid]->instance; + $mods[$cmid]->cm = $rawmods[$cmid]->id; + $mods[$cmid]->mod = $rawmods[$cmid]->modname; // Oh dear. Inconsistent names left here for backward compatibility. - $mod[$seq]->section = $section->section; - $mod[$seq]->sectionid = $rawmods[$seq]->section; + $mods[$cmid]->section = $section->section; + $mods[$cmid]->sectionid = $rawmods[$cmid]->section; - $mod[$seq]->module = $rawmods[$seq]->module; - $mod[$seq]->added = $rawmods[$seq]->added; - $mod[$seq]->score = $rawmods[$seq]->score; - $mod[$seq]->idnumber = $rawmods[$seq]->idnumber; - $mod[$seq]->visible = $rawmods[$seq]->visible; - $mod[$seq]->visibleoncoursepage = $rawmods[$seq]->visibleoncoursepage; - $mod[$seq]->visibleold = $rawmods[$seq]->visibleold; - $mod[$seq]->groupmode = $rawmods[$seq]->groupmode; - $mod[$seq]->groupingid = $rawmods[$seq]->groupingid; - $mod[$seq]->indent = $rawmods[$seq]->indent; - $mod[$seq]->completion = $rawmods[$seq]->completion; - $mod[$seq]->extra = ""; - $mod[$seq]->completiongradeitemnumber = - $rawmods[$seq]->completiongradeitemnumber; - $mod[$seq]->completionpassgrade = $rawmods[$seq]->completionpassgrade; - $mod[$seq]->completionview = $rawmods[$seq]->completionview; - $mod[$seq]->completionexpected = $rawmods[$seq]->completionexpected; - $mod[$seq]->showdescription = $rawmods[$seq]->showdescription; - $mod[$seq]->availability = $rawmods[$seq]->availability; - $mod[$seq]->deletioninprogress = $rawmods[$seq]->deletioninprogress; - $mod[$seq]->downloadcontent = $rawmods[$seq]->downloadcontent; - $modname = $mod[$seq]->mod; - $functionname = $modname."_get_coursemodule_info"; + $mods[$cmid]->module = $rawmods[$cmid]->module; + $mods[$cmid]->added = $rawmods[$cmid]->added; + $mods[$cmid]->score = $rawmods[$cmid]->score; + $mods[$cmid]->idnumber = $rawmods[$cmid]->idnumber; + $mods[$cmid]->visible = $rawmods[$cmid]->visible; + $mods[$cmid]->visibleoncoursepage = $rawmods[$cmid]->visibleoncoursepage; + $mods[$cmid]->visibleold = $rawmods[$cmid]->visibleold; + $mods[$cmid]->groupmode = $rawmods[$cmid]->groupmode; + $mods[$cmid]->groupingid = $rawmods[$cmid]->groupingid; + $mods[$cmid]->indent = $rawmods[$cmid]->indent; + $mods[$cmid]->completion = $rawmods[$cmid]->completion; + $mods[$cmid]->extra = ""; + $mods[$cmid]->completiongradeitemnumber = + $rawmods[$cmid]->completiongradeitemnumber; + $mods[$cmid]->completionpassgrade = $rawmods[$cmid]->completionpassgrade; + $mods[$cmid]->completionview = $rawmods[$cmid]->completionview; + $mods[$cmid]->completionexpected = $rawmods[$cmid]->completionexpected; + $mods[$cmid]->showdescription = $rawmods[$cmid]->showdescription; + $mods[$cmid]->availability = $rawmods[$cmid]->availability; + $mods[$cmid]->deletioninprogress = $rawmods[$cmid]->deletioninprogress; + $mods[$cmid]->downloadcontent = $rawmods[$cmid]->downloadcontent; - if (!file_exists("$CFG->dirroot/mod/$modname/lib.php")) { - continue; - } + $modname = $mods[$cmid]->mod; + $functionname = $modname . "_get_coursemodule_info"; - include_once("$CFG->dirroot/mod/$modname/lib.php"); + if (!file_exists("$CFG->dirroot/mod/$modname/lib.php")) { + continue; + } - if ($hasfunction = function_exists($functionname)) { - if ($info = $functionname($rawmods[$seq])) { - if (!empty($info->icon)) { - $mod[$seq]->icon = $info->icon; - } - if (!empty($info->iconcomponent)) { - $mod[$seq]->iconcomponent = $info->iconcomponent; - } - if (!empty($info->name)) { - $mod[$seq]->name = $info->name; - } - if ($info instanceof cached_cm_info) { - // When using cached_cm_info you can include three new fields - // that aren't available for legacy code - if (!empty($info->content)) { - $mod[$seq]->content = $info->content; - } - if (!empty($info->extraclasses)) { - $mod[$seq]->extraclasses = $info->extraclasses; - } - if (!empty($info->iconurl)) { - // Convert URL to string as it's easier to store. Also serialized object contains \0 byte and can not be written to Postgres DB. - $url = new moodle_url($info->iconurl); - $mod[$seq]->iconurl = $url->out(false); - } - if (!empty($info->onclick)) { - $mod[$seq]->onclick = $info->onclick; - } - if (!empty($info->customdata)) { - $mod[$seq]->customdata = $info->customdata; - } - } else { - // When using a stdclass, the (horrible) deprecated ->extra field - // is available for BC - if (!empty($info->extra)) { - $mod[$seq]->extra = $info->extra; - } - } - } - } - // When there is no modname_get_coursemodule_info function, - // but showdescriptions is enabled, then we use the 'intro' - // and 'introformat' fields in the module table - if (!$hasfunction && $rawmods[$seq]->showdescription) { - if ($modvalues = $DB->get_record($rawmods[$seq]->modname, - array('id' => $rawmods[$seq]->instance), 'name, intro, introformat')) { - // Set content from intro and introformat. Filters are disabled - // because we filter it with format_text at display time - $mod[$seq]->content = format_module_intro($rawmods[$seq]->modname, - $modvalues, $rawmods[$seq]->id, false); + include_once("$CFG->dirroot/mod/$modname/lib.php"); - // To save making another query just below, put name in here - $mod[$seq]->name = $modvalues->name; - } - } - if (!isset($mod[$seq]->name)) { - $mod[$seq]->name = $DB->get_field($rawmods[$seq]->modname, "name", array("id"=>$rawmods[$seq]->instance)); - } + if ($hasfunction = function_exists($functionname)) { + if ($info = $functionname($rawmods[$cmid])) { + if (!empty($info->icon)) { + $mods[$cmid]->icon = $info->icon; + } + if (!empty($info->iconcomponent)) { + $mods[$cmid]->iconcomponent = $info->iconcomponent; + } + if (!empty($info->name)) { + $mods[$cmid]->name = $info->name; + } + if ($info instanceof cached_cm_info) { + // When using cached_cm_info you can include three new fields. + // That aren't available for legacy code. + if (!empty($info->content)) { + $mods[$cmid]->content = $info->content; + } + if (!empty($info->extraclasses)) { + $mods[$cmid]->extraclasses = $info->extraclasses; + } + if (!empty($info->iconurl)) { + // Convert URL to string as it's easier to store. + // Also serialized object contains \0 byte, + // ... and can not be written to Postgres DB. + $url = new moodle_url($info->iconurl); + $mods[$cmid]->iconurl = $url->out(false); + } + if (!empty($info->onclick)) { + $mods[$cmid]->onclick = $info->onclick; + } + if (!empty($info->customdata)) { + $mods[$cmid]->customdata = $info->customdata; + } + } else { + // When using a stdclass, the (horrible) deprecated ->extra field, + // ... that is available for BC. + if (!empty($info->extra)) { + $mods[$cmid]->extra = $info->extra; + } + } + } + } + // When there is no modname_get_coursemodule_info function, + // ... but showdescriptions is enabled, then we use the 'intro', + // ... and 'introformat' fields in the module table. + if (!$hasfunction && $rawmods[$cmid]->showdescription) { + if ($modvalues = $DB->get_record($rawmods[$cmid]->modname, + ['id' => $rawmods[$cmid]->instance], 'name, intro, introformat')) { + // Set content from intro and introformat. Filters are disabled. + // Because we filter it with format_text at display time. + $mods[$cmid]->content = format_module_intro($rawmods[$cmid]->modname, + $modvalues, $rawmods[$cmid]->id, false); - // Minimise the database size by unsetting default options when they are - // 'empty'. This list corresponds to code in the cm_info constructor. - foreach (array('idnumber', 'groupmode', 'groupingid', + // To save making another query just below, put name in here. + $mods[$cmid]->name = $modvalues->name; + } + } + if (!isset($mods[$cmid]->name)) { + $mods[$cmid]->name = $DB->get_field($rawmods[$cmid]->modname, "name", + ["id" => $rawmods[$cmid]->instance]); + } + + // Minimise the database size by unsetting default options when they are 'empty'. + // This list corresponds to code in the cm_info constructor. + foreach (['idnumber', 'groupmode', 'groupingid', 'indent', 'completion', 'extra', 'extraclasses', 'iconurl', 'onclick', 'content', 'icon', 'iconcomponent', 'customdata', 'availability', 'completionview', - 'completionexpected', 'score', 'showdescription', 'deletioninprogress') as $property) { - if (property_exists($mod[$seq], $property) && - empty($mod[$seq]->{$property})) { - unset($mod[$seq]->{$property}); - } - } - // Special case: this value is usually set to null, but may be 0 - if (property_exists($mod[$seq], 'completiongradeitemnumber') && - is_null($mod[$seq]->completiongradeitemnumber)) { - unset($mod[$seq]->completiongradeitemnumber); - } - } + 'completionexpected', 'score', 'showdescription', 'deletioninprogress'] as $property) { + if (property_exists($mods[$cmid], $property) && + empty($mods[$cmid]->{$property})) { + unset($mods[$cmid]->{$property}); + } + } + // Special case: this value is usually set to null, but may be 0. + if (property_exists($mods[$cmid], 'completiongradeitemnumber') && + is_null($mods[$cmid]->completiongradeitemnumber)) { + unset($mods[$cmid]->completiongradeitemnumber); + } + } } } } - return $mod; + return $mods; } /** @@ -818,11 +843,7 @@ function course_add_cm_to_section($courseorid, $cmid, $sectionnum, $beforemod = } $DB->set_field("course_sections", "sequence", $newsequence, array("id" => $section->id)); $DB->set_field('course_modules', 'section', $section->id, array('id' => $cmid)); - if (is_object($courseorid)) { - rebuild_course_cache($courseorid->id, true); - } else { - rebuild_course_cache($courseorid, true); - } + rebuild_course_cache($courseid, true); return $section->id; // Return course_sections ID that was used. } @@ -841,7 +862,8 @@ function set_coursemodule_groupmode($id, $groupmode) { $cm = $DB->get_record('course_modules', array('id' => $id), 'id,course,groupmode', MUST_EXIST); if ($cm->groupmode != $groupmode) { $DB->set_field('course_modules', 'groupmode', $groupmode, array('id' => $cm->id)); - rebuild_course_cache($cm->course, true); + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + rebuild_course_cache($cm->course, false, true); } return ($cm->groupmode != $groupmode); } @@ -851,7 +873,8 @@ function set_coursemodule_idnumber($id, $idnumber) { $cm = $DB->get_record('course_modules', array('id' => $id), 'id,course,idnumber', MUST_EXIST); if ($cm->idnumber != $idnumber) { $DB->set_field('course_modules', 'idnumber', $idnumber, array('id' => $cm->id)); - rebuild_course_cache($cm->course, true); + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + rebuild_course_cache($cm->course, false, true); } return ($cm->idnumber != $idnumber); } @@ -945,7 +968,8 @@ function set_coursemodule_visible($id, $visible, $visibleoncoursepage = 1) { } } - rebuild_course_cache($cm->course, true); + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + rebuild_course_cache($cm->course, false, true); return true; } @@ -982,7 +1006,8 @@ function set_coursemodule_name($id, $name) { $DB->update_record($cm->modname, $module); $cm->name = $module->name; \core\event\course_module_updated::create_from_cm($cm)->trigger(); - rebuild_course_cache($cm->course, true); + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + rebuild_course_cache($cm->course, false, true); // Attempt to update the grade item if relevant. $grademodule = $DB->get_record($cm->modname, array('id' => $cm->instance)); @@ -1134,13 +1159,14 @@ function course_delete_module($cmid, $async = false) { 'context' => $modcontext, 'objectid' => $cm->id, 'other' => array( - 'modulename' => $modulename, + 'modulename' => $modulename, 'instanceid' => $cm->instance, ) )); $event->add_record_snapshot('course_modules', $cm); $event->trigger(); - rebuild_course_cache($cm->course, true); + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + rebuild_course_cache($cm->course, false, true); } /** @@ -1402,12 +1428,16 @@ function move_section_to($course, $section, $destination, $ignorenumsections = f $transaction = $DB->start_delegated_transaction(); foreach ($movedsections as $id => $position) { if ($sections[$id] !== $position) { - $DB->set_field('course_sections', 'section', -$position, array('id' => $id)); + $DB->set_field('course_sections', 'section', -$position, ['id' => $id]); + // Invalidate the section cache by given section id. + course_modinfo::purge_course_section_cache_by_id($course->id, $id); } } foreach ($movedsections as $id => $position) { if ($sections[$id] !== $position) { - $DB->set_field('course_sections', 'section', $position, array('id' => $id)); + $DB->set_field('course_sections', 'section', $position, ['id' => $id]); + // Invalidate the section cache by given section id. + course_modinfo::purge_course_section_cache_by_id($course->id, $id); } } @@ -1415,14 +1445,14 @@ function move_section_to($course, $section, $destination, $ignorenumsections = f // Adjust the higlighted section location if we move something over it either direction. if ($section == $course->marker) { course_set_marker($course->id, $destination); - } elseif ($section > $course->marker && $course->marker >= $destination) { + } else if ($section > $course->marker && $course->marker >= $destination) { course_set_marker($course->id, $course->marker+1); - } elseif ($section < $course->marker && $course->marker <= $destination) { + } else if ($section < $course->marker && $course->marker <= $destination) { course_set_marker($course->id, $course->marker-1); } $transaction->allow_commit(); - rebuild_course_cache($course->id, true); + rebuild_course_cache($course->id, true, true); return true; } @@ -1597,7 +1627,9 @@ function course_update_section($course, $section, $data) { $data['id'] = $section->id; $data['timemodified'] = time(); $DB->update_record('course_sections', $data); - rebuild_course_cache($courseid, true); + // Invalidate the section cache by given section id. + course_modinfo::purge_course_section_cache_by_id($courseid, $section->id); + rebuild_course_cache($courseid, false, true); course_get_format($courseid)->update_section_format_options($data); // Update fields of the $section object. @@ -1629,11 +1661,13 @@ function course_update_section($course, $section, $data) { } 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); - $DB->set_field('course_modules', 'visibleold', $cm->visible, array('id' => $moduleid)); + $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(); } } + rebuild_course_cache($courseid, false, true); } } diff --git a/course/mod.php b/course/mod.php index 67719a7aae2..a6c33395a6b 100644 --- a/course/mod.php +++ b/course/mod.php @@ -190,7 +190,9 @@ if ((!empty($movetosection) or !empty($moveto)) and confirm_sesskey()) { $DB->set_field('course_modules', 'indent', $cm->indent, array('id'=>$cm->id)); - rebuild_course_cache($cm->course); + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + // Rebuild invalidated module cache. + rebuild_course_cache($cm->course, false, true); redirect(course_get_url($course, $cm->sectionnum, array('sr' => $sectionreturn))); diff --git a/course/modlib.php b/course/modlib.php index c2b3bd8c8eb..26170a83785 100644 --- a/course/modlib.php +++ b/course/modlib.php @@ -377,7 +377,8 @@ function edit_module_post_actions($moduleinfo, $course) { $moduleinfo->showgradingmanagement = $showgradingmanagement; } - rebuild_course_cache($course->id, true); + \course_modinfo::purge_course_module_cache($course->id, $moduleinfo->coursemodule); + rebuild_course_cache($course->id, true, true); if ($hasgrades) { grade_regrade_final_grades($course->id); } diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 3a42db3fa6f..b2eb1c67e6f 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1143,6 +1143,7 @@ class courselib_test extends advanced_testcase { // Delete section in the middle (2). $this->assertFalse(course_delete_section($course, 2, false)); + $this->assertEquals(4, course_get_format($course)->get_last_section_number()); $this->assertTrue(course_delete_section($course, 2, true)); $this->assertFalse($DB->record_exists('course_modules', array('id' => $assign21->cmid))); $this->assertFalse($DB->record_exists('course_modules', array('id' => $assign22->cmid))); diff --git a/course/tests/externallib_test.php b/course/tests/externallib_test.php index 60ba1a207b4..2dc265d3c68 100644 --- a/course/tests/externallib_test.php +++ b/course/tests/externallib_test.php @@ -1085,8 +1085,10 @@ class externallib_test extends externallib_advanced_testcase { array('course' => $course->id, 'intro' => 'forum completion tracking auto', 'trackingtype' => 2), array('showdescription' => true, 'completionview' => 1, 'completion' => COMPLETION_TRACKING_AUTOMATIC)); $forumcompleteautocm = get_coursemodule_from_id('forum', $forumcompleteauto->cmid); - - rebuild_course_cache($course->id, true); + $sectionrecord = $DB->get_record('course_sections', $conditions); + // Invalidate the section cache by given section number. + course_modinfo::purge_course_section_cache_by_number($sectionrecord->course, $sectionrecord->section); + rebuild_course_cache($course->id, true, true); return array($course, $forumcm, $datacm, $pagecm, $labelcm, $urlcm, $forumcompleteautocm); } diff --git a/course/upgrade.txt b/course/upgrade.txt index cdf444d1cc3..e9267c4ded2 100644 --- a/course/upgrade.txt +++ b/course/upgrade.txt @@ -89,6 +89,9 @@ course formats don't have their own renderer. category pages. * New core_course_category::get_nearest_editable_subcategory(): - Return the core_course_category object for the first subcategory that the current user has the permission on it. +* The following functions have been deprecated in favour partial course cache rebuild: + - course_purge_section_cache (replaced by course_modinfo::purge_course_section_cache_by_id() and course_modinfo::purge_course_section_cache_by_number()) + - course_purge_module_cache (replaced by course_modinfo::purge_course_module_cache()) === 3.11 === * A new callback xxx_coursemodule_definition_after_data that allows plugins to extend activity forms after the data is set. diff --git a/lib/db/caches.php b/lib/db/caches.php index 55cb49ef222..8e65c01e76b 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -227,8 +227,16 @@ $definitions = array( 'simplekeys' => true, 'ttl' => 3600, ), - // Accumulated information about course modules and sections used to print course view page (user-independed). - // Used in function get_fast_modinfo(), reset in function rebuild_course_cache(). + // Accumulated information about course modules and sections used to print course view page (user-independent). + // Used in functions: + // - course_modinfo::build_course_section_cache() + // - course_modinfo::inner_build_course_cache() + // - get_array_of_activities() + // Reset/update in functions: + // - rebuild_course_cache() + // - course_modinfo::purge_module_cache() + // - course_modinfo::purge_section_cache() + // - remove_course_contents(). 'coursemodinfo' => array( 'mode' => cache_store::MODE_APPLICATION, 'simplekeys' => true, diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 305ae3db94e..5d95ab6a96f 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -3687,4 +3687,45 @@ function print_grade_plugin_selector($plugin_info, $active_type, $active_plugin, // only one option - no plugin selector needed return ''; } + + /** + * Purge the cache of a course section. + * + * $sectioninfo must have following attributes: + * - course: course id + * - section: section number + * + * @param object $sectioninfo section info + * @return void + * @deprecated since Moodle 4.0. Please use {@link course_modinfo::purge_course_section_cache_by_id()} + * or {@link course_modinfo::purge_course_section_cache_by_number()} instead. + */ + function course_purge_section_cache(object $sectioninfo): void { + debugging(__FUNCTION__ . '() is deprecated. ' . + 'Please use course_modinfo::purge_course_section_cache_by_id() ' . + 'or course_modinfo::purge_course_section_cache_by_number() instead.', + DEBUG_DEVELOPER); + $sectionid = $sectioninfo->section; + $courseid = $sectioninfo->course; + course_modinfo::purge_course_section_cache_by_id($courseid, $sectionid); + } + + /** + * Purge the cache of a course module. + * + * $cm must have following attributes: + * - id: cmid + * - course: course id + * + * @param cm_info|stdClass $cm course module + * @return void + * @deprecated since Moodle 4.0. Please use {@link course_modinfo::purge_course_module_cache()} instead. + */ + function course_purge_module_cache($cm): void { + debugging(__FUNCTION__ . '() is deprecated. ' . 'Please use course_modinfo::purge_course_module_cache() instead.', + DEBUG_DEVELOPER); + $cmid = $cm->id; + $courseid = $cm->course; + course_modinfo::purge_course_module_cache($courseid, $cmid); + } } diff --git a/lib/modinfolib.php b/lib/modinfolib.php index 9e148133d5c..826c1316724 100644 --- a/lib/modinfolib.php +++ b/lib/modinfolib.php @@ -514,7 +514,7 @@ class course_modinfo { ' does not have context. Rebuilding cache for course '. $course->id); // Re-request the course record from DB as well, don't use get_course() here. $course = $DB->get_record('course', array('id' => $course->id), '*', MUST_EXIST); - $coursemodinfo = self::build_course_cache($course); + $coursemodinfo = self::build_course_cache($course, true); break; } } @@ -553,6 +553,7 @@ class course_modinfo { $this->instances[$cm->modname] = array(); } $this->instances[$cm->modname][$cm->instance] = $cm; + ksort($this->instances[$cm->modname]); $this->cms[$cm->id] = $cm; // Reconstruct sections. This works because modules are stored in order @@ -562,6 +563,8 @@ class course_modinfo { $this->sections[$cm->sectionnum][] = $cm->id; } + ksort($this->cms); + ksort($this->instances); // Expand section objects $this->sectioninfo = array(); foreach ($coursemodinfo->sectioncache as $number => $data) { @@ -588,23 +591,37 @@ class course_modinfo { * in some other way.) * * @param stdClass $course Course object (must contain fields + * @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) */ - protected static function build_course_section_cache($course) { + 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'); - $compressedsections = array(); + $compressedsections = []; + $courseformat = course_get_format($course); + + if ($usecache) { + $cachecoursemodinfo = \cache::make('core', 'coursemodinfo'); + $coursemodinfo = $cachecoursemodinfo->get($course->id); + if ($coursemodinfo !== false) { + $compressedsections = $coursemodinfo->sectioncache; + } + } $formatoptionsdef = course_get_format($course)->section_format_options(); // Remove unnecessary data and add availability foreach ($sections as $number => $section) { + $sectioninfocached = isset($compressedsections[$number]); + if ($sectioninfocached) { + continue; + } // Add cached options from course format to $section object foreach ($formatoptionsdef as $key => $option) { if (!empty($option['cache'])) { - $formatoptions = course_get_format($course)->get_format_options($section); + $formatoptions = $courseformat->get_format_options($section); if (!array_key_exists('cachedefault', $option) || $option['cachedefault'] !== $formatoptions[$key]) { $section->$key = $formatoptions[$key]; } @@ -615,6 +632,7 @@ class course_modinfo { section_info::convert_for_section_cache($compressedsections[$number]); } + ksort($compressedsections); return $compressedsections; } @@ -652,19 +670,20 @@ class course_modinfo { * * @param stdClass $course object from DB table course. Must have property 'id' * but preferably should have all cached fields. + * @param boolean $partialrebuild Indicate if it's partial course cache rebuild or not * @return stdClass object with all cached keys of the course plus fields modinfo and sectioncache. * The same object is stored in MUC * @throws moodle_exception if course is not found (if $course object misses some of the * necessary fields it is re-requested from database) */ - public static function build_course_cache($course) { + public static function build_course_cache(\stdClass $course, bool $partialrebuild = false): \stdClass { if (empty($course->id)) { throw new coding_exception('Object $course is missing required property \id\''); } $lock = self::get_course_cache_lock($course->id); try { - return self::inner_build_course_cache($course, $lock); + return self::inner_build_course_cache($course, $lock, $partialrebuild); } finally { $lock->release(); } @@ -675,9 +694,11 @@ class course_modinfo { * * @param stdClass $course object from DB table course * @param \core\lock\lock $lock Lock object - not actually used, just there to indicate you have a lock + * @param bool $partialrebuild Indicate if it's partial course cache rebuild or not * @return stdClass Course object that has been stored in MUC */ - protected static function inner_build_course_cache($course, \core\lock\lock $lock) { + protected static function inner_build_course_cache(\stdClass $course, \core\lock\lock $lock, + bool $partialrebuild = false): \stdClass { global $DB, $CFG; require_once("{$CFG->dirroot}/course/lib.php"); @@ -688,8 +709,8 @@ class course_modinfo { // Retrieve all information about activities and sections. $coursemodinfo = new stdClass(); - $coursemodinfo->modinfo = get_array_of_activities($course->id); - $coursemodinfo->sectioncache = self::build_course_section_cache($course); + $coursemodinfo->modinfo = get_array_of_activities($course->id, $partialrebuild); + $coursemodinfo->sectioncache = self::build_course_section_cache($course, $partialrebuild); foreach (self::$cachedfields as $key) { $coursemodinfo->$key = $course->$key; } @@ -698,6 +719,63 @@ class course_modinfo { $cachecoursemodinfo->set_versioned($course->id, $course->cacherev, $coursemodinfo); return $coursemodinfo; } + + /** + * Purge the cache of a course section by its id. + * + * @param int $courseid The course to purge cache in + * @param int $sectionid The section _id_ to purge + */ + public static function purge_course_section_cache_by_id(int $courseid, int $sectionid): void { + $cache = cache::make('core', 'coursemodinfo'); + $cache->acquire_lock($courseid); + $coursemodinfo = $cache->get($courseid); + if ($coursemodinfo !== false) { + foreach ($coursemodinfo->sectioncache as $sectionno => $sectioncache) { + if ($sectioncache->id == $sectionid) { + unset($coursemodinfo->sectioncache[$sectionno]); + $cache->set($courseid, $coursemodinfo); + break; + } + } + } + $cache->release_lock($courseid); + } + + /** + * Purge the cache of a course section by its number. + * + * @param int $courseid The course to purge cache in + * @param int $sectionno The section number to purge + */ + public static function purge_course_section_cache_by_number(int $courseid, int $sectionno): void { + $cache = cache::make('core', 'coursemodinfo'); + $cache->acquire_lock($courseid); + $coursemodinfo = $cache->get($courseid); + if ($coursemodinfo !== false && array_key_exists($sectionno, $coursemodinfo->sectioncache)) { + unset($coursemodinfo->sectioncache[$sectionno]); + $cache->set($courseid, $coursemodinfo); + } + $cache->release_lock($courseid); + } + + /** + * Purge the cache of a course module. + * + * @param int $courseid Course id + * @param int $cmid Course module id + */ + public static function purge_course_module_cache(int $courseid, int $cmid): void { + $cache = cache::make('core', 'coursemodinfo'); + $cache->acquire_lock($courseid); + $coursemodinfo = $cache->get($courseid); + $hascache = ($coursemodinfo !== false) && array_key_exists($cmid, $coursemodinfo->modinfo); + if ($hascache) { + unset($coursemodinfo->modinfo[$cmid]); + $cache->set($courseid, $coursemodinfo); + } + $cache->release_lock($courseid); + } } @@ -2450,9 +2528,21 @@ function get_course_and_cm_from_instance($instanceorid, $modulename, $courseorid * @param int $courseid id of course to rebuild, empty means all * @param boolean $clearonly only clear the cache, gets rebuild automatically on the fly. * Recommended to set to true to avoid unnecessary multiple rebuilding. + * @param boolean $partialrebuild will not delete the whole cache when it's true. + * use purge_module_cache() or purge_section_cache() must be + * called before when partialrebuild is true. + * use purge_module_cache() to invalidate mod cache. + * use purge_section_cache() to invalidate section cache. + * + * @return void + * @throws coding_exception */ -function rebuild_course_cache($courseid=0, $clearonly=false) { - global $COURSE, $SITE, $DB, $CFG; +function rebuild_course_cache(int $courseid = 0, bool $clearonly = false, bool $partialrebuild = false): void { + global $COURSE, $SITE, $DB; + + if ($courseid == 0 and $partialrebuild) { + throw new coding_exception('partialrebuild only works when a valid course id is provided.'); + } // Function rebuild_course_cache() can not be called during upgrade unless it's clear only. if (!$clearonly && !upgrade_ensure_not_running(true)) { @@ -2468,7 +2558,10 @@ function rebuild_course_cache($courseid=0, $clearonly=false) { if (empty($courseid)) { // Clearing caches for all courses. increment_revision_number('course', 'cacherev', ''); - $cachecoursemodinfo->purge(); + if (!$partialrebuild) { + $cachecoursemodinfo->purge(); + } + // Clear memory static cache. course_modinfo::clear_instance_cache(); // Update global values too. $sitecacherev = $DB->get_field('course', 'cacherev', array('id' => SITEID)); @@ -2481,7 +2574,11 @@ function rebuild_course_cache($courseid=0, $clearonly=false) { } else { // Clearing cache for one course, make sure it is deleted from user request cache as well. increment_revision_number('course', 'cacherev', 'id = :id', array('id' => $courseid)); - $cachecoursemodinfo->delete($courseid); + if (!$partialrebuild) { + // Purge all course modinfo. + $cachecoursemodinfo->delete($courseid); + } + // Clear memory static cache. course_modinfo::clear_instance_cache($courseid); // Update global values too. if ($courseid == $COURSE->id || $courseid == $SITE->id) { @@ -2506,10 +2603,13 @@ function rebuild_course_cache($courseid=0, $clearonly=false) { core_php_time_limit::raise(); // this could take a while! MDL-10954 } - $rs = $DB->get_recordset("course", $select,'','id,'.join(',', course_modinfo::$cachedfields)); + $fields = 'id,' . join(',', course_modinfo::$cachedfields); + $sort = ''; + $rs = $DB->get_recordset("course", $select, $sort, $fields); + // Rebuild cache for each course. foreach ($rs as $course) { - course_modinfo::build_course_cache($course); + course_modinfo::build_course_cache($course, $partialrebuild); } $rs->close(); } diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php index 50d7431dbf9..2b00cfdf355 100644 --- a/lib/tests/modinfolib_test.php +++ b/lib/tests/modinfolib_test.php @@ -367,7 +367,7 @@ class modinfolib_test extends advanced_testcase { $this->assertEquals($USER->id, $modinfo->userid); $this->assertEquals(array(0 => array($forum0->cmid, $assign0->cmid), 1 => array($forum1->cmid, $assign1->cmid, $page1->cmid), 3 => array($page3->cmid)), $modinfo->sections); - $this->assertEquals(array('forum', 'assign', 'page'), array_keys($modinfo->instances)); + $this->assertEquals(array('assign', 'forum', 'page'), array_keys($modinfo->instances)); $this->assertEquals(array($assign0->id, $assign1->id), array_keys($modinfo->instances['assign'])); $this->assertEquals(array($forum0->id, $forum1->id), array_keys($modinfo->instances['forum'])); $this->assertEquals(array($page1->id, $page3->id), array_keys($modinfo->instances['page'])); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 70050362a18..721b84b4422 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -42,7 +42,9 @@ information provided here is intended especially for developers. * The completion_info function display_help_icon() which returned the 'Your progress' help icon has been deprecated and should no longer be used. * The completion_info function print_help_icon() which has been deprecated since Moodle 2.0 should no longer be used. -* @babel/polyfill has been removed in favour of corejs@3 +* @babel/polyfill has been removed in favour of corejs@3. +* A new parameter $partialrebuild has been added to the rebuild_course_cache to invalidate the cache + of the section or module only, not the whole course cache * A new parameter $isbulkupdate has been added to the following functions: - grade_category::update() - grade_category::insert() diff --git a/mod/chat/lib.php b/mod/chat/lib.php index b7cd3968c5f..0cce046391d 100644 --- a/mod/chat/lib.php +++ b/mod/chat/lib.php @@ -121,7 +121,8 @@ function padding($n) { * @return int */ function chat_add_instance($chat) { - global $DB; + global $DB, $CFG; + require_once($CFG->dirroot . '/course/lib.php'); $chat->timemodified = time(); $chat->chattime = chat_calculate_next_chat_time($chat->schedule, $chat->chattime); @@ -706,10 +707,13 @@ function chat_update_chat_times($chatid=0) { $chat->chattime = chat_calculate_next_chat_time($chat->schedule, $chat->chattime); if ($originalchattime != $chat->chattime) { $courseids[] = $chat->course; - } - $DB->update_record("chat", $chat); - $event = new stdClass(); // Update calendar too. + $DB->update_record("chat", $chat); + $cm = get_coursemodule_from_instance('chat', $chat->id, $chat->course); + \course_modinfo::purge_course_module_cache($cm->course, $cm->id); + } + + $event = new stdClass(); // Update calendar too. $cond = "modulename='chat' AND eventtype = :eventtype AND instance = :chatid AND timestart <> :chattime"; $params = ['chattime' => $chat->chattime, 'eventtype' => CHAT_EVENT_TYPE_CHATTIME, 'chatid' => $chat->id]; @@ -723,7 +727,7 @@ function chat_update_chat_times($chatid=0) { $courseids = array_unique($courseids); foreach ($courseids as $courseid) { - rebuild_course_cache($courseid, true); + rebuild_course_cache($courseid, true, true); } } diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index 585f6da101a..d69aacf0c84 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -2837,14 +2837,23 @@ function lti_update_type($type, $config) { } require_once($CFG->libdir.'/modinfolib.php'); if ($clearcache) { - $sql = "SELECT DISTINCT course - FROM {lti} - WHERE typeid = ?"; + $sql = "SELECT cm.id, cm.course + FROM {course_modules} cm + JOIN {modules} m ON cm.module = m.id + JOIN {lti} l ON l.course = cm.course + WHERE m.name = :name AND l.typeid = :typeid"; - $courses = $DB->get_fieldset_sql($sql, array($type->id)); + $rs = $DB->get_recordset_sql($sql, ['name' => 'lti', 'typeid' => $type->id]); - foreach ($courses as $courseid) { - rebuild_course_cache($courseid, true); + $courseids = []; + foreach ($rs as $record) { + $courseids[] = $record->course; + \course_modinfo::purge_course_module_cache($record->course, $record->id); + } + $rs->close(); + $courseids = array_unique($courseids); + foreach ($courseids as $courseid) { + rebuild_course_cache($courseid, false, true); } } } From 4bc2b24a51d636fa6e95afc3dd1e079f9b6844ea Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Mar 2022 15:07:06 +0700 Subject: [PATCH 2/5] MDL-72991 Course: Modify the courseminfo cacheid to force a rebuild Co-authored-by: Andrew Lyons --- lib/modinfolib.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/modinfolib.php b/lib/modinfolib.php index 826c1316724..4a69dc8ef8f 100644 --- a/lib/modinfolib.php +++ b/lib/modinfolib.php @@ -733,6 +733,7 @@ class course_modinfo { if ($coursemodinfo !== false) { foreach ($coursemodinfo->sectioncache as $sectionno => $sectioncache) { if ($sectioncache->id == $sectionid) { + $coursemodinfo->cacherev = -1; unset($coursemodinfo->sectioncache[$sectionno]); $cache->set($courseid, $coursemodinfo); break; @@ -753,6 +754,7 @@ class course_modinfo { $cache->acquire_lock($courseid); $coursemodinfo = $cache->get($courseid); if ($coursemodinfo !== false && array_key_exists($sectionno, $coursemodinfo->sectioncache)) { + $coursemodinfo->cacherev = -1; unset($coursemodinfo->sectioncache[$sectionno]); $cache->set($courseid, $coursemodinfo); } @@ -771,6 +773,7 @@ class course_modinfo { $coursemodinfo = $cache->get($courseid); $hascache = ($coursemodinfo !== false) && array_key_exists($cmid, $coursemodinfo->modinfo); if ($hascache) { + $coursemodinfo->cacherev = -1; unset($coursemodinfo->modinfo[$cmid]); $cache->set($courseid, $coursemodinfo); } From 273fbac73994c92ef72d23073cfb1d74e66cc9f4 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Mon, 17 Jan 2022 12:05:24 +0700 Subject: [PATCH 3/5] MDL-72991 Course: Create PHPUnit for course_modinfo and move_section_to Newly PHPUnit tests were created to verify the below methods - course_modinfo::purge_section_cache_by_id() - course_modinfo::purge_section_cache_by_number() - move_section_to() --- course/tests/courselib_test.php | 45 ++++++++++++++++ lib/tests/modinfolib_test.php | 92 +++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index b2eb1c67e6f..391cac9ad4e 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1044,6 +1044,51 @@ class courselib_test extends advanced_testcase { $this->assertEquals(3, $course->marker); } + /** + * Test move_section_to method with caching + * + * @covers ::move_section_to + * @return void + */ + public function test_move_section_with_section_cache(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $cache = cache::make('core', 'coursemodinfo'); + + // Generate the course and pre-requisite module. + $course = $this->getDataGenerator()->create_course(['format' => 'topics', 'numsections' => 3], ['createsections' => true]); + // 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($course->id); + // Get the section cache. + $sectioncaches = $coursemodinfo->sectioncache; + + // 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); + + // Move section. + move_section_to($course, 2, 3); + // Get the course modinfo cache. + $coursemodinfo = $cache->get($course->id); + // Get the section cache. + $sectioncaches = $coursemodinfo->sectioncache; + + // 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); + } + public function test_course_can_delete_section() { global $DB; $this->resetAfterTest(true); diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php index 2b00cfdf355..633676201b6 100644 --- a/lib/tests/modinfolib_test.php +++ b/lib/tests/modinfolib_test.php @@ -1004,4 +1004,96 @@ class modinfolib_test extends advanced_testcase { ], ]; } + + /** + * Test purge_section_cache_by_id method + * + * @covers \course_modinfo::purge_course_section_cache_by_id + * @return void + */ + public function test_purge_section_cache_by_id(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $cache = cache::make('core', 'coursemodinfo'); + + // Generate the course and pre-requisite section. + $course = $this->getDataGenerator()->create_course(['format' => 'topics', 'numsections' => 3], ['createsections' => true]); + // 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($course->id); + // Get the section cache. + $sectioncaches = $coursemodinfo->sectioncache; + + // 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); + + // Purge cache for the section by id. + course_modinfo::purge_course_section_cache_by_id($course->id, $sectioncaches[1]->id); + // Get the course modinfo cache. + $coursemodinfo = $cache->get($course->id); + // Get the section cache. + $sectioncaches = $coursemodinfo->sectioncache; + + // 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); + // Make sure that the cacherev will be reset. + $this->assertEquals(-1, $coursemodinfo->cacherev); + } + + /** + * Test purge_section_cache_by_number method + * + * @covers \course_modinfo::purge_course_section_cache_by_number + * @return void + */ + public function test_section_cache_by_number(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $cache = cache::make('core', 'coursemodinfo'); + + // Generate the course and pre-requisite section. + $course = $this->getDataGenerator()->create_course(['format' => 'topics', 'numsections' => 3], ['createsections' => true]); + // 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($course->id); + // Get the section cache. + $sectioncaches = $coursemodinfo->sectioncache; + + // 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); + + // Purge cache for the section with section number is 1. + course_modinfo::purge_course_section_cache_by_number($course->id, 1); + // Get the course modinfo cache. + $coursemodinfo = $cache->get($course->id); + // Get the section cache. + $sectioncaches = $coursemodinfo->sectioncache; + + // 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); + // Make sure that the cacherev will be reset. + $this->assertEquals(-1, $coursemodinfo->cacherev); + } } From 457d10a49f91b8143a065ea345118773ffd7ac1b Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 18 Jan 2022 10:29:42 +0700 Subject: [PATCH 4/5] MDL-72991 Course: Fix move_section_to is not updating correct section When we change the position of two sections, we just need to update the position of the affected sections, not all the sections in the course. This will improve the performance since the system only executes the queries to affected sections. Also, the system only clears the cache for affected sections, not all the sections in the course. --- course/lib.php | 4 ++-- course/tests/courselib_test.php | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/course/lib.php b/course/lib.php index dc663aa38ea..dee093bbf48 100644 --- a/course/lib.php +++ b/course/lib.php @@ -1427,14 +1427,14 @@ function move_section_to($course, $section, $destination, $ignorenumsections = f // uniqueness constraint $transaction = $DB->start_delegated_transaction(); foreach ($movedsections as $id => $position) { - if ($sections[$id] !== $position) { + if ((int) $sections[$id] !== $position) { $DB->set_field('course_sections', 'section', -$position, ['id' => $id]); // Invalidate the section cache by given section id. course_modinfo::purge_course_section_cache_by_id($course->id, $id); } } foreach ($movedsections as $id => $position) { - if ($sections[$id] !== $position) { + if ((int) $sections[$id] !== $position) { $DB->set_field('course_sections', 'section', $position, ['id' => $id]); // Invalidate the section cache by given section id. course_modinfo::purge_course_section_cache_by_id($course->id, $id); diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 391cac9ad4e..c75d8551880 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1089,6 +1089,35 @@ class courselib_test extends advanced_testcase { $this->assertArrayNotHasKey(3, $sectioncaches); } + /** + * Test move_section_to method. + * Make sure that we only update the moving sections, not all the sections in the current course. + * + * @covers ::move_section_to + * @return void + */ + public function test_move_section_to(): void { + global $DB, $CFG; + $this->resetAfterTest(); + $this->setAdminUser(); + + // Generate the course and pre-requisite module. + $course = $this->getDataGenerator()->create_course(['format' => 'topics', 'numsections' => 3], ['createsections' => true]); + + ob_start(); + $DB->set_debug(true); + // Move section. + move_section_to($course, 2, 3); + $DB->set_debug(false); + $debuginfo = ob_get_contents(); + ob_end_clean(); + $sectionmovequerycount = substr_count($debuginfo, 'UPDATE ' . $CFG->phpunit_prefix . 'course_sections SET'); + // We are updating the course_section table in steps to avoid breaking database uniqueness constraint. + // So the queries will be doubled. See: course/lib.php:1423 + // Make sure that we only need 4 queries to update the position of section 2 and section 3. + $this->assertEquals(4, $sectionmovequerycount); + } + public function test_course_can_delete_section() { global $DB; $this->resetAfterTest(true); From bfe14e23120989dfb84002c951e7aa571cd9db6c Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 1 Mar 2022 11:11:57 +0700 Subject: [PATCH 5/5] MDL-72991 Course: Partial course cache rebuild with cache versioning --- course/format/classes/base.php | 2 +- course/lib.php | 198 --------------------------- course/tests/courselib_test.php | 4 +- course/upgrade.txt | 2 + lib/deprecatedlib.php | 15 ++ lib/modinfolib.php | 233 +++++++++++++++++++++++++++++--- lib/moodlelib.php | 3 +- lib/tests/modinfolib_test.php | 10 +- mod/page/lib.php | 2 +- mod/resource/lib.php | 2 +- mod/url/lib.php | 2 +- 11 files changed, 243 insertions(+), 230 deletions(-) diff --git a/course/format/classes/base.php b/course/format/classes/base.php index cc83f570698..c75c95946e2 100644 --- a/course/format/classes/base.php +++ b/course/format/classes/base.php @@ -1393,7 +1393,7 @@ abstract class base { } if (!is_object($section)) { $section = $DB->get_record('course_sections', array('course' => $this->get_courseid(), 'section' => $section), - 'id,course,section,sequence,summary'); + 'id,section,sequence,summary'); } if (!$section || !$section->section) { // Not possible to delete 0-section. diff --git a/course/lib.php b/course/lib.php index dee093bbf48..e76d9145cc8 100644 --- a/course/lib.php +++ b/course/lib.php @@ -386,204 +386,6 @@ function course_integrity_check($courseid, $rawmods = null, $sections = null, $f return $messages; } -/** - * For a given course, returns an array of course activity objects - * Each item in the array contains he following properties: - * - * @param int $courseid course id - * @param bool $usecache get activities from cache if modinfo exists when $usecache is true - * @return array list of activities - */ -function get_array_of_activities(int $courseid, bool $usecache = false): array { -// cm - course module id -// mod - name of the module (eg forum) -// section - the number of the section (eg week or topic) -// name - the name of the instance -// visible - is the instance visible or not -// groupingid - grouping id -// extra - contains extra string to include in any link - global $CFG, $DB; - - $course = $DB->get_record('course', array('id'=>$courseid)); - - if (empty($course)) { - throw new moodle_exception('courseidnotfound'); - } - - $rawmods = get_course_mods($courseid); - if (empty($rawmods)) { - return []; - } - - $mods = []; - if ($usecache) { - // Get existing cache. - $cachecoursemodinfo = cache::make('core', 'coursemodinfo'); - $coursemodinfo = $cachecoursemodinfo->get($courseid); - if ($coursemodinfo !== false) { - $mods = $coursemodinfo->modinfo; - } - } - - $courseformat = course_get_format($course); - - if ($sections = $DB->get_records('course_sections', array('course' => $courseid), - 'section ASC', 'id,section,sequence,visible')) { - // First check and correct obvious mismatches between course_sections.sequence and course_modules.section. - if ($errormessages = course_integrity_check($courseid, $rawmods, $sections)) { - debugging(join('
', $errormessages)); - $rawmods = get_course_mods($courseid); - $sections = $DB->get_records('course_sections', array('course' => $courseid), - 'section ASC', 'id,section,sequence,visible'); - } - // Build array of activities. - foreach ($sections as $section) { - if (!empty($section->sequence)) { - $cmids = explode(",", $section->sequence); - $numberofmods = count($cmids); - for ($order = 0; $order < $numberofmods; $order++) { - $cmid = $cmids[$order]; - // Activity does not exist in the database. - $notexistindb = empty($rawmods[$cmid]); - $activitycached = isset($mods[$cmid]); - if ($activitycached || $notexistindb) { - continue; - } - $modposition = ($order === 0) ? 0 : array_search($cmids[$order - 1], array_keys($mods)) + 1; - $mods = array_slice($mods, 0, $modposition, true) - + [$cmid => new stdClass()] - + array_slice($mods, $modposition, null, true); - - // Adjust visibleoncoursepage, value in DB may not respect format availability. - $rawmods[$cmid]->visibleoncoursepage = (!$rawmods[$cmid]->visible - || $rawmods[$cmid]->visibleoncoursepage - || empty($CFG->allowstealth) - || !$courseformat->allow_stealth_module_visibility($rawmods[$cmid], $section)) ? 1 : 0; - - $mods[$cmid]->id = $rawmods[$cmid]->instance; - $mods[$cmid]->cm = $rawmods[$cmid]->id; - $mods[$cmid]->mod = $rawmods[$cmid]->modname; - - // Oh dear. Inconsistent names left here for backward compatibility. - $mods[$cmid]->section = $section->section; - $mods[$cmid]->sectionid = $rawmods[$cmid]->section; - - $mods[$cmid]->module = $rawmods[$cmid]->module; - $mods[$cmid]->added = $rawmods[$cmid]->added; - $mods[$cmid]->score = $rawmods[$cmid]->score; - $mods[$cmid]->idnumber = $rawmods[$cmid]->idnumber; - $mods[$cmid]->visible = $rawmods[$cmid]->visible; - $mods[$cmid]->visibleoncoursepage = $rawmods[$cmid]->visibleoncoursepage; - $mods[$cmid]->visibleold = $rawmods[$cmid]->visibleold; - $mods[$cmid]->groupmode = $rawmods[$cmid]->groupmode; - $mods[$cmid]->groupingid = $rawmods[$cmid]->groupingid; - $mods[$cmid]->indent = $rawmods[$cmid]->indent; - $mods[$cmid]->completion = $rawmods[$cmid]->completion; - $mods[$cmid]->extra = ""; - $mods[$cmid]->completiongradeitemnumber = - $rawmods[$cmid]->completiongradeitemnumber; - $mods[$cmid]->completionpassgrade = $rawmods[$cmid]->completionpassgrade; - $mods[$cmid]->completionview = $rawmods[$cmid]->completionview; - $mods[$cmid]->completionexpected = $rawmods[$cmid]->completionexpected; - $mods[$cmid]->showdescription = $rawmods[$cmid]->showdescription; - $mods[$cmid]->availability = $rawmods[$cmid]->availability; - $mods[$cmid]->deletioninprogress = $rawmods[$cmid]->deletioninprogress; - $mods[$cmid]->downloadcontent = $rawmods[$cmid]->downloadcontent; - - $modname = $mods[$cmid]->mod; - $functionname = $modname . "_get_coursemodule_info"; - - if (!file_exists("$CFG->dirroot/mod/$modname/lib.php")) { - continue; - } - - include_once("$CFG->dirroot/mod/$modname/lib.php"); - - if ($hasfunction = function_exists($functionname)) { - if ($info = $functionname($rawmods[$cmid])) { - if (!empty($info->icon)) { - $mods[$cmid]->icon = $info->icon; - } - if (!empty($info->iconcomponent)) { - $mods[$cmid]->iconcomponent = $info->iconcomponent; - } - if (!empty($info->name)) { - $mods[$cmid]->name = $info->name; - } - if ($info instanceof cached_cm_info) { - // When using cached_cm_info you can include three new fields. - // That aren't available for legacy code. - if (!empty($info->content)) { - $mods[$cmid]->content = $info->content; - } - if (!empty($info->extraclasses)) { - $mods[$cmid]->extraclasses = $info->extraclasses; - } - if (!empty($info->iconurl)) { - // Convert URL to string as it's easier to store. - // Also serialized object contains \0 byte, - // ... and can not be written to Postgres DB. - $url = new moodle_url($info->iconurl); - $mods[$cmid]->iconurl = $url->out(false); - } - if (!empty($info->onclick)) { - $mods[$cmid]->onclick = $info->onclick; - } - if (!empty($info->customdata)) { - $mods[$cmid]->customdata = $info->customdata; - } - } else { - // When using a stdclass, the (horrible) deprecated ->extra field, - // ... that is available for BC. - if (!empty($info->extra)) { - $mods[$cmid]->extra = $info->extra; - } - } - } - } - // When there is no modname_get_coursemodule_info function, - // ... but showdescriptions is enabled, then we use the 'intro', - // ... and 'introformat' fields in the module table. - if (!$hasfunction && $rawmods[$cmid]->showdescription) { - if ($modvalues = $DB->get_record($rawmods[$cmid]->modname, - ['id' => $rawmods[$cmid]->instance], 'name, intro, introformat')) { - // Set content from intro and introformat. Filters are disabled. - // Because we filter it with format_text at display time. - $mods[$cmid]->content = format_module_intro($rawmods[$cmid]->modname, - $modvalues, $rawmods[$cmid]->id, false); - - // To save making another query just below, put name in here. - $mods[$cmid]->name = $modvalues->name; - } - } - if (!isset($mods[$cmid]->name)) { - $mods[$cmid]->name = $DB->get_field($rawmods[$cmid]->modname, "name", - ["id" => $rawmods[$cmid]->instance]); - } - - // Minimise the database size by unsetting default options when they are 'empty'. - // This list corresponds to code in the cm_info constructor. - foreach (['idnumber', 'groupmode', 'groupingid', - 'indent', 'completion', 'extra', 'extraclasses', 'iconurl', 'onclick', 'content', - 'icon', 'iconcomponent', 'customdata', 'availability', 'completionview', - 'completionexpected', 'score', 'showdescription', 'deletioninprogress'] as $property) { - if (property_exists($mods[$cmid], $property) && - empty($mods[$cmid]->{$property})) { - unset($mods[$cmid]->{$property}); - } - } - // Special case: this value is usually set to null, but may be 0. - if (property_exists($mods[$cmid], 'completiongradeitemnumber') && - is_null($mods[$cmid]->completiongradeitemnumber)) { - unset($mods[$cmid]->completiongradeitemnumber); - } - } - } - } - } - return $mods; -} - /** * Returns an array where the key is the module name (component name without 'mod_') * and the value is a lang_string object with a human-readable string. diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index c75d8551880..f9c63c5aa02 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1063,7 +1063,7 @@ class courselib_test extends advanced_testcase { // Build course cache. get_fast_modinfo($course->id); // Get the course modinfo cache. - $coursemodinfo = $cache->get($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; @@ -1077,7 +1077,7 @@ class courselib_test extends advanced_testcase { // Move section. move_section_to($course, 2, 3); // Get the course modinfo cache. - $coursemodinfo = $cache->get($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; diff --git a/course/upgrade.txt b/course/upgrade.txt index e9267c4ded2..11dbd088635 100644 --- a/course/upgrade.txt +++ b/course/upgrade.txt @@ -92,6 +92,8 @@ course formats don't have their own renderer. * The following functions have been deprecated in favour partial course cache rebuild: - course_purge_section_cache (replaced by course_modinfo::purge_course_section_cache_by_id() and course_modinfo::purge_course_section_cache_by_number()) - course_purge_module_cache (replaced by course_modinfo::purge_course_module_cache()) + - get_array_of_activities (replaced by course_modinfo::get_array_of_activities() for better performance) +* New course_modinfo::purge_course_cache function was created to purge the cache of a given course. === 3.11 === * A new callback xxx_coursemodule_definition_after_data that allows plugins to extend activity forms after the data is set. diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 5d95ab6a96f..9d9208c882c 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -3729,3 +3729,18 @@ function print_grade_plugin_selector($plugin_info, $active_type, $active_plugin, course_modinfo::purge_course_module_cache($courseid, $cmid); } } + +/** + * For a given course, returns an array of course activity objects + * Each item in the array contains he following properties: + * + * @param int $courseid course id + * @param bool $usecache get activities from cache if modinfo exists when $usecache is true + * @return array list of activities + * @deprecated since Moodle 4.0. Please use {@link course_modinfo::get_array_of_activities()} instead. + */ +function get_array_of_activities(int $courseid, bool $usecache = false): array { + debugging(__FUNCTION__ . '() is deprecated. ' . 'Please use course_modinfo::get_array_of_activities() instead.', + DEBUG_DEVELOPER); + return course_modinfo::get_array_of_activities(get_course($courseid), $usecache); +} diff --git a/lib/modinfolib.php b/lib/modinfolib.php index 4a69dc8ef8f..bf20367db5d 100644 --- a/lib/modinfolib.php +++ b/lib/modinfolib.php @@ -476,13 +476,13 @@ class course_modinfo { // Retrieve modinfo from cache. If not present or cacherev mismatches, call rebuild and retrieve again. $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); - if (!$coursemodinfo) { + if ($coursemodinfo === false || ($course->cacherev != $coursemodinfo->cacherev)) { $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_versioned($course->id, $course->cacherev); - if (!$coursemodinfo) { + if ($coursemodinfo === false || ($course->cacherev != $coursemodinfo->cacherev)) { $coursemodinfo = self::inner_build_course_cache($course, $lock); } } finally { @@ -553,7 +553,6 @@ class course_modinfo { $this->instances[$cm->modname] = array(); } $this->instances[$cm->modname][$cm->instance] = $cm; - ksort($this->instances[$cm->modname]); $this->cms[$cm->id] = $cm; // Reconstruct sections. This works because modules are stored in order @@ -563,8 +562,6 @@ class course_modinfo { $this->sections[$cm->sectionnum][] = $cm->id; } - ksort($this->cms); - ksort($this->instances); // Expand section objects $this->sectioninfo = array(); foreach ($coursemodinfo->sectioncache as $number => $data) { @@ -605,7 +602,7 @@ class course_modinfo { if ($usecache) { $cachecoursemodinfo = \cache::make('core', 'coursemodinfo'); - $coursemodinfo = $cachecoursemodinfo->get($course->id); + $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); if ($coursemodinfo !== false) { $compressedsections = $coursemodinfo->sectioncache; } @@ -709,7 +706,7 @@ class course_modinfo { // Retrieve all information about activities and sections. $coursemodinfo = new stdClass(); - $coursemodinfo->modinfo = get_array_of_activities($course->id, $partialrebuild); + $coursemodinfo->modinfo = self::get_array_of_activities($course, $partialrebuild); $coursemodinfo->sectioncache = self::build_course_section_cache($course, $partialrebuild); foreach (self::$cachedfields as $key) { $coursemodinfo->$key = $course->$key; @@ -727,20 +724,21 @@ class course_modinfo { * @param int $sectionid The section _id_ to purge */ public static function purge_course_section_cache_by_id(int $courseid, int $sectionid): void { + $course = get_course($courseid); $cache = cache::make('core', 'coursemodinfo'); - $cache->acquire_lock($courseid); - $coursemodinfo = $cache->get($courseid); + $cache->acquire_lock($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); if ($coursemodinfo !== false) { foreach ($coursemodinfo->sectioncache as $sectionno => $sectioncache) { if ($sectioncache->id == $sectionid) { $coursemodinfo->cacherev = -1; unset($coursemodinfo->sectioncache[$sectionno]); - $cache->set($courseid, $coursemodinfo); + $cache->set_versioned($course->id, $course->cacherev, $coursemodinfo); break; } } } - $cache->release_lock($courseid); + $cache->release_lock($course->id); } /** @@ -750,15 +748,16 @@ class course_modinfo { * @param int $sectionno The section number to purge */ public static function purge_course_section_cache_by_number(int $courseid, int $sectionno): void { + $course = get_course($courseid); $cache = cache::make('core', 'coursemodinfo'); - $cache->acquire_lock($courseid); - $coursemodinfo = $cache->get($courseid); + $cache->acquire_lock($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); if ($coursemodinfo !== false && array_key_exists($sectionno, $coursemodinfo->sectioncache)) { $coursemodinfo->cacherev = -1; unset($coursemodinfo->sectioncache[$sectionno]); - $cache->set($courseid, $coursemodinfo); + $cache->set_versioned($course->id, $course->cacherev, $coursemodinfo); } - $cache->release_lock($courseid); + $cache->release_lock($course->id); } /** @@ -768,16 +767,212 @@ class course_modinfo { * @param int $cmid Course module id */ public static function purge_course_module_cache(int $courseid, int $cmid): void { + $course = get_course($courseid); $cache = cache::make('core', 'coursemodinfo'); - $cache->acquire_lock($courseid); - $coursemodinfo = $cache->get($courseid); + $cache->acquire_lock($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); $hascache = ($coursemodinfo !== false) && array_key_exists($cmid, $coursemodinfo->modinfo); if ($hascache) { $coursemodinfo->cacherev = -1; unset($coursemodinfo->modinfo[$cmid]); - $cache->set($courseid, $coursemodinfo); + $cache->set_versioned($course->id, $course->cacherev, $coursemodinfo); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); } - $cache->release_lock($courseid); + $cache->release_lock($course->id); + } + + /** + * For a given course, returns an array of course activity objects + * + * @param stdClass $course Course object + * @param bool $usecache get activities from cache if modinfo exists when $usecache is true + * @return array list of activities + */ + public static function get_array_of_activities(stdClass $course, bool $usecache = false): array { + global $CFG, $DB; + + if (empty($course)) { + throw new moodle_exception('courseidnotfound'); + } + + $rawmods = get_course_mods($course->id); + if (empty($rawmods)) { + return []; + } + + $mods = []; + if ($usecache) { + // Get existing cache. + $cachecoursemodinfo = cache::make('core', 'coursemodinfo'); + $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); + if ($coursemodinfo !== false) { + $mods = $coursemodinfo->modinfo; + } + } + + $courseformat = course_get_format($course); + + if ($sections = $DB->get_records('course_sections', ['course' => $course->id], + 'section ASC', 'id,section,sequence,visible')) { + // First check and correct obvious mismatches between course_sections.sequence and course_modules.section. + if ($errormessages = course_integrity_check($course->id, $rawmods, $sections)) { + debugging(join('
', $errormessages)); + $rawmods = get_course_mods($course->id); + $sections = $DB->get_records('course_sections', ['course' => $course->id], + 'section ASC', 'id,section,sequence,visible'); + } + // Build array of activities. + foreach ($sections as $section) { + if (!empty($section->sequence)) { + $cmids = explode(",", $section->sequence); + $numberofmods = count($cmids); + foreach ($cmids as $cmid) { + // Activity does not exist in the database. + $notexistindb = empty($rawmods[$cmid]); + $activitycached = isset($mods[$cmid]); + if ($activitycached || $notexistindb) { + continue; + } + + // Adjust visibleoncoursepage, value in DB may not respect format availability. + $rawmods[$cmid]->visibleoncoursepage = (!$rawmods[$cmid]->visible + || $rawmods[$cmid]->visibleoncoursepage + || empty($CFG->allowstealth) + || !$courseformat->allow_stealth_module_visibility($rawmods[$cmid], $section)) ? 1 : 0; + + $mods[$cmid] = new stdClass(); + $mods[$cmid]->id = $rawmods[$cmid]->instance; + $mods[$cmid]->cm = $rawmods[$cmid]->id; + $mods[$cmid]->mod = $rawmods[$cmid]->modname; + + // Oh dear. Inconsistent names left here for backward compatibility. + $mods[$cmid]->section = $section->section; + $mods[$cmid]->sectionid = $rawmods[$cmid]->section; + + $mods[$cmid]->module = $rawmods[$cmid]->module; + $mods[$cmid]->added = $rawmods[$cmid]->added; + $mods[$cmid]->score = $rawmods[$cmid]->score; + $mods[$cmid]->idnumber = $rawmods[$cmid]->idnumber; + $mods[$cmid]->visible = $rawmods[$cmid]->visible; + $mods[$cmid]->visibleoncoursepage = $rawmods[$cmid]->visibleoncoursepage; + $mods[$cmid]->visibleold = $rawmods[$cmid]->visibleold; + $mods[$cmid]->groupmode = $rawmods[$cmid]->groupmode; + $mods[$cmid]->groupingid = $rawmods[$cmid]->groupingid; + $mods[$cmid]->indent = $rawmods[$cmid]->indent; + $mods[$cmid]->completion = $rawmods[$cmid]->completion; + $mods[$cmid]->extra = ""; + $mods[$cmid]->completiongradeitemnumber = + $rawmods[$cmid]->completiongradeitemnumber; + $mods[$cmid]->completionpassgrade = $rawmods[$cmid]->completionpassgrade; + $mods[$cmid]->completionview = $rawmods[$cmid]->completionview; + $mods[$cmid]->completionexpected = $rawmods[$cmid]->completionexpected; + $mods[$cmid]->showdescription = $rawmods[$cmid]->showdescription; + $mods[$cmid]->availability = $rawmods[$cmid]->availability; + $mods[$cmid]->deletioninprogress = $rawmods[$cmid]->deletioninprogress; + $mods[$cmid]->downloadcontent = $rawmods[$cmid]->downloadcontent; + + $modname = $mods[$cmid]->mod; + $functionname = $modname . "_get_coursemodule_info"; + + if (!file_exists("$CFG->dirroot/mod/$modname/lib.php")) { + continue; + } + + include_once("$CFG->dirroot/mod/$modname/lib.php"); + + if ($hasfunction = function_exists($functionname)) { + if ($info = $functionname($rawmods[$cmid])) { + if (!empty($info->icon)) { + $mods[$cmid]->icon = $info->icon; + } + if (!empty($info->iconcomponent)) { + $mods[$cmid]->iconcomponent = $info->iconcomponent; + } + if (!empty($info->name)) { + $mods[$cmid]->name = $info->name; + } + if ($info instanceof cached_cm_info) { + // When using cached_cm_info you can include three new fields. + // That aren't available for legacy code. + if (!empty($info->content)) { + $mods[$cmid]->content = $info->content; + } + if (!empty($info->extraclasses)) { + $mods[$cmid]->extraclasses = $info->extraclasses; + } + if (!empty($info->iconurl)) { + // Convert URL to string as it's easier to store. + // Also serialized object contains \0 byte, + // ... and can not be written to Postgres DB. + $url = new moodle_url($info->iconurl); + $mods[$cmid]->iconurl = $url->out(false); + } + if (!empty($info->onclick)) { + $mods[$cmid]->onclick = $info->onclick; + } + if (!empty($info->customdata)) { + $mods[$cmid]->customdata = $info->customdata; + } + } else { + // When using a stdclass, the (horrible) deprecated ->extra field, + // ... that is available for BC. + if (!empty($info->extra)) { + $mods[$cmid]->extra = $info->extra; + } + } + } + } + // When there is no modname_get_coursemodule_info function, + // ... but showdescriptions is enabled, then we use the 'intro', + // ... and 'introformat' fields in the module table. + if (!$hasfunction && $rawmods[$cmid]->showdescription) { + if ($modvalues = $DB->get_record($rawmods[$cmid]->modname, + ['id' => $rawmods[$cmid]->instance], 'name, intro, introformat')) { + // Set content from intro and introformat. Filters are disabled. + // Because we filter it with format_text at display time. + $mods[$cmid]->content = format_module_intro($rawmods[$cmid]->modname, + $modvalues, $rawmods[$cmid]->id, false); + + // To save making another query just below, put name in here. + $mods[$cmid]->name = $modvalues->name; + } + } + if (!isset($mods[$cmid]->name)) { + $mods[$cmid]->name = $DB->get_field($rawmods[$cmid]->modname, "name", + ["id" => $rawmods[$cmid]->instance]); + } + + // Minimise the database size by unsetting default options when they are 'empty'. + // This list corresponds to code in the cm_info constructor. + foreach (['idnumber', 'groupmode', 'groupingid', + 'indent', 'completion', 'extra', 'extraclasses', 'iconurl', 'onclick', 'content', + 'icon', 'iconcomponent', 'customdata', 'availability', 'completionview', + 'completionexpected', 'score', 'showdescription', 'deletioninprogress'] as $property) { + if (property_exists($mods[$cmid], $property) && + empty($mods[$cmid]->{$property})) { + unset($mods[$cmid]->{$property}); + } + } + // Special case: this value is usually set to null, but may be 0. + if (property_exists($mods[$cmid], 'completiongradeitemnumber') && + is_null($mods[$cmid]->completiongradeitemnumber)) { + unset($mods[$cmid]->completiongradeitemnumber); + } + } + } + } + } + return $mods; + } + + /** + * Purge the cache of a given course + * + * @param int $courseid Course id + */ + public static function purge_course_cache(int $courseid): void { + $cachemodinfo = cache::make('core', 'coursemodinfo'); + $cachemodinfo->delete($courseid); } } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index ee60a46c6e3..eb4c126f75a 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -5347,8 +5347,7 @@ function remove_course_contents($courseid, $showfeedback = true, array $options fulldelete($CFG->dataroot.'/'.$course->id); // Delete from cache to reduce the cache size especially makes sense in case of bulk course deletion. - $cachemodinfo = cache::make('core', 'coursemodinfo'); - $cachemodinfo->delete($courseid); + course_modinfo::purge_course_cache($courseid); // Trigger a course content deleted event. $event = \core\event\course_content_deleted::create(array( diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php index 633676201b6..da19910af82 100644 --- a/lib/tests/modinfolib_test.php +++ b/lib/tests/modinfolib_test.php @@ -367,7 +367,7 @@ class modinfolib_test extends advanced_testcase { $this->assertEquals($USER->id, $modinfo->userid); $this->assertEquals(array(0 => array($forum0->cmid, $assign0->cmid), 1 => array($forum1->cmid, $assign1->cmid, $page1->cmid), 3 => array($page3->cmid)), $modinfo->sections); - $this->assertEquals(array('assign', 'forum', 'page'), array_keys($modinfo->instances)); + $this->assertEquals(array('forum', 'assign', 'page'), array_keys($modinfo->instances)); $this->assertEquals(array($assign0->id, $assign1->id), array_keys($modinfo->instances['assign'])); $this->assertEquals(array($forum0->id, $forum1->id), array_keys($modinfo->instances['forum'])); $this->assertEquals(array($page1->id, $page3->id), array_keys($modinfo->instances['page'])); @@ -1023,7 +1023,7 @@ class modinfolib_test extends advanced_testcase { // Build course cache. get_fast_modinfo($course->id); // Get the course modinfo cache. - $coursemodinfo = $cache->get($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; @@ -1037,7 +1037,7 @@ class modinfolib_test extends advanced_testcase { // Purge cache for the section by id. course_modinfo::purge_course_section_cache_by_id($course->id, $sectioncaches[1]->id); // Get the course modinfo cache. - $coursemodinfo = $cache->get($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; @@ -1069,7 +1069,7 @@ class modinfolib_test extends advanced_testcase { // Build course cache. get_fast_modinfo($course->id); // Get the course modinfo cache. - $coursemodinfo = $cache->get($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; @@ -1083,7 +1083,7 @@ class modinfolib_test extends advanced_testcase { // Purge cache for the section with section number is 1. course_modinfo::purge_course_section_cache_by_number($course->id, 1); // Get the course modinfo cache. - $coursemodinfo = $cache->get($course->id); + $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev); // Get the section cache. $sectioncaches = $coursemodinfo->sectioncache; diff --git a/mod/page/lib.php b/mod/page/lib.php index dae27693bb7..bda4a4fd616 100644 --- a/mod/page/lib.php +++ b/mod/page/lib.php @@ -201,7 +201,7 @@ function page_delete_instance($id) { * "extra" information that may be needed when printing * this activity in a course listing. * - * See {@link get_array_of_activities()} in course/lib.php + * See {@link course_modinfo::get_array_of_activities()} * * @param stdClass $coursemodule * @return cached_cm_info Info to customise main page display diff --git a/mod/resource/lib.php b/mod/resource/lib.php index 726fd47ca4d..563d2422bc5 100644 --- a/mod/resource/lib.php +++ b/mod/resource/lib.php @@ -192,7 +192,7 @@ function resource_delete_instance($id) { * "extra" information that may be needed when printing * this activity in a course listing. * - * See {@link get_array_of_activities()} in course/lib.php + * See {@link course_modinfo::get_array_of_activities()} * * @param stdClass $coursemodule * @return cached_cm_info info diff --git a/mod/url/lib.php b/mod/url/lib.php index 169c8a6784d..dae393ac800 100644 --- a/mod/url/lib.php +++ b/mod/url/lib.php @@ -203,7 +203,7 @@ function url_delete_instance($id) { * "extra" information that may be needed when printing * this activity in a course listing. * - * See {@link get_array_of_activities()} in course/lib.php + * See {@link course_modinfo::get_array_of_activities()} * * @param object $coursemodule * @return cached_cm_info info