From 84ef6eb8d9c16f3c3fd1dfd033292992840aba20 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen Date: Mon, 13 Jul 2020 17:58:29 +1000 Subject: [PATCH] MDL-67468 filter_activitynames: use MUC --- filter/activitynames/filter.php | 141 +++++++++++---------- filter/activitynames/tests/filter_test.php | 52 ++++++++ 2 files changed, 129 insertions(+), 64 deletions(-) diff --git a/filter/activitynames/filter.php b/filter/activitynames/filter.php index d9ce6f7655e..d6d30793c44 100644 --- a/filter/activitynames/filter.php +++ b/filter/activitynames/filter.php @@ -31,83 +31,24 @@ defined('MOODLE_INTERNAL') || die(); * Activity name filtering */ class filter_activitynames extends moodle_text_filter { - // Trivial-cache - keyed on $cachedcourseid and $cacheduserid. - static $activitylist = null; - static $cachedcourseid; - static $cacheduserid; function filter($text, array $options = array()) { - global $USER; // Since 2.7 we can finally start using globals in filters. - $coursectx = $this->context->get_course_context(false); if (!$coursectx) { return $text; } $courseid = $coursectx->instanceid; - // Initialise/invalidate our trivial cache if dealing with a different course. - if (!isset(self::$cachedcourseid) || self::$cachedcourseid !== (int)$courseid) { - self::$activitylist = null; - } - self::$cachedcourseid = (int)$courseid; - // And the same for user id. - if (!isset(self::$cacheduserid) || self::$cacheduserid !== (int)$USER->id) { - self::$activitylist = null; - } - self::$cacheduserid = (int)$USER->id; - - /// It may be cached - - if (is_null(self::$activitylist)) { - self::$activitylist = array(); - - $modinfo = get_fast_modinfo($courseid); - if (!empty($modinfo->cms)) { - self::$activitylist = array(); // We will store all the created filters here. - - // Create array of visible activities sorted by the name length (we are only interested in properties name and url). - $sortedactivities = array(); - foreach ($modinfo->cms as $cm) { - // Use normal access control and visibility, but exclude labels and hidden activities. - if ($cm->visible and $cm->has_view() and $cm->uservisible) { - $sortedactivities[] = (object)array( - 'name' => $cm->name, - 'url' => $cm->url, - 'id' => $cm->id, - 'namelen' => -strlen($cm->name), // Negative value for reverse sorting. - ); - } - } - // Sort activities by the length of the activity name in reverse order. - core_collator::asort_objects_by_property($sortedactivities, 'namelen', core_collator::SORT_NUMERIC); - - foreach ($sortedactivities as $cm) { - $title = s(trim(strip_tags($cm->name))); - $currentname = trim($cm->name); - $entitisedname = s($currentname); - // Avoid empty or unlinkable activity names. - if (!empty($title)) { - $href_tag_begin = html_writer::start_tag('a', - array('class' => 'autolink', 'title' => $title, - 'href' => $cm->url)); - self::$activitylist[$cm->id] = new filterobject($currentname, $href_tag_begin, '', false, true); - if ($currentname != $entitisedname) { - // If name has some entity (& " < >) add that filter too. MDL-17545. - self::$activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $href_tag_begin, '', false, true); - } - } - } - } - } + $activitylist = $this->get_cached_activity_list($courseid); $filterslist = array(); - if (self::$activitylist) { + if (!empty($activitylist)) { $cmid = $this->context->instanceid; - if ($this->context->contextlevel == CONTEXT_MODULE && isset(self::$activitylist[$cmid])) { + if ($this->context->contextlevel == CONTEXT_MODULE && isset($activitylist[$cmid])) { // remove filterobjects for the current module - $filterslist = array_values(array_diff_key(self::$activitylist, array($cmid => 1, $cmid.'-e' => 1))); + $filterslist = array_values(array_diff_key($activitylist, array($cmid => 1, $cmid.'-e' => 1))); } else { - $filterslist = array_values(self::$activitylist); + $filterslist = array_values($activitylist); } } @@ -117,4 +58,76 @@ class filter_activitynames extends moodle_text_filter { return $text; } } + + /** + * Get all the cached activity list for a course + * + * @param int $courseid id of the course + * @return filterobject[] the activities + */ + protected function get_cached_activity_list($courseid) { + global $USER; + $cached = cache::make_from_params(cache_store::MODE_REQUEST, 'filter', 'activitynames'); + + // Return cached activity list. + if ($cached->get('cachecourseid') == $courseid && $cached->get('cacheuserid') == $USER->id) { + return $cached->get('activitylist'); + } + + // Not cached yet, get activity list and set cache. + $activitylist = $this->get_activity_list($courseid); + $cached->set('cacheuserid', $USER->id); + $cached->set('cachecourseid', $courseid); + $cached->set('activitylist', $activitylist); + return $activitylist; + } + + /** + * Get all the activity list for a course + * + * @param int $courseid id of the course + * @return filterobject[] the activities + */ + protected function get_activity_list($courseid) { + $activitylist = array(); + + $modinfo = get_fast_modinfo($courseid); + if (!empty($modinfo->cms)) { + $activitylist = array(); // We will store all the created filters here. + + // Create array of visible activities sorted by the name length (we are only interested in properties name and url). + $sortedactivities = array(); + foreach ($modinfo->cms as $cm) { + // Use normal access control and visibility, but exclude labels and hidden activities. + if ($cm->visible and $cm->has_view() and $cm->uservisible) { + $sortedactivities[] = (object)array( + 'name' => $cm->name, + 'url' => $cm->url, + 'id' => $cm->id, + 'namelen' => -strlen($cm->name), // Negative value for reverse sorting. + ); + } + } + // Sort activities by the length of the activity name in reverse order. + core_collator::asort_objects_by_property($sortedactivities, 'namelen', core_collator::SORT_NUMERIC); + + foreach ($sortedactivities as $cm) { + $title = s(trim(strip_tags($cm->name))); + $currentname = trim($cm->name); + $entitisedname = s($currentname); + // Avoid empty or unlinkable activity names. + if (!empty($title)) { + $hreftagbegin = html_writer::start_tag('a', + array('class' => 'autolink', 'title' => $title, + 'href' => $cm->url)); + $activitylist[$cm->id] = new filterobject($currentname, $hreftagbegin, '', false, true); + if ($currentname != $entitisedname) { + // If name has some entity (& " < >) add that filter too. MDL-17545. + $activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $hreftagbegin, '', false, true); + } + } + } + } + return $activitylist; + } } diff --git a/filter/activitynames/tests/filter_test.php b/filter/activitynames/tests/filter_test.php index b21c924c1b7..98776b74609 100644 --- a/filter/activitynames/tests/filter_test.php +++ b/filter/activitynames/tests/filter_test.php @@ -102,4 +102,56 @@ class filter_activitynames_filter_testcase extends advanced_testcase { $this->assertEquals($page->cmid, $matches[2][0]); $this->assertEquals($page->name, $matches[3][0]); } + + public function test_cache() { + $this->resetAfterTest(true); + + // Create a test courses. + $course1 = $this->getDataGenerator()->create_course(); + $course2 = $this->getDataGenerator()->create_course(); + $context1 = context_course::instance($course1->id); + $context2 = context_course::instance($course2->id); + + // Create page 1. + $page1 = $this->getDataGenerator()->create_module('page', + ['course' => $course1->id, 'name' => 'Test 1']); + // Format text with page 1 in HTML. + $html = '

Please read the two pages Test 1 and Test 2.

'; + $filtered1 = format_text($html, FORMAT_HTML, array('context' => $context1)); + // Find all the activity links in the result. + $matches = []; + preg_match_all('~([^<]*)~', + $filtered1, $matches); + // There should be 1 link. + $this->assertCount(1, $matches[1]); + $this->assertEquals($page1->name, $matches[1][0]); + + // Create page 2. + $page2 = $this->getDataGenerator()->create_module('page', + ['course' => $course1->id, 'name' => 'Test 2']); + // Filter the text again. + $filtered2 = format_text($html, FORMAT_HTML, array('context' => $context1)); + // The filter result does not change due to caching. + $this->assertEquals($filtered1, $filtered2); + + // Change context, so that cache for course 1 is cleared. + $filtered3 = format_text($html, FORMAT_HTML, array('context' => $context2)); + $this->assertNotEquals($filtered1, $filtered3); + $matches = []; + preg_match_all('~([^<]*)~', + $filtered3, $matches); + // There should be no links. + $this->assertCount(0, $matches[1]); + + // Filter the text for course 1. + $filtered4 = format_text($html, FORMAT_HTML, array('context' => $context1)); + // Find all the activity links in the result. + $matches = []; + preg_match_all('~([^<]*)~', + $filtered4, $matches); + // There should be 2 links. + $this->assertCount(2, $matches[1]); + $this->assertEquals($page1->name, $matches[1][0]); + $this->assertEquals($page2->name, $matches[1][1]); + } } -- 2.17.1