MDL-67468 filter_activitynames: use MUC

diff --git a/filter/activitynames/filter.php b/filter/activitynames/filter.php
index d9ce6f7..d6d3079 100644 (file)
--- 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 {
  * 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()) {
 
     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;
 
         $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, '</a>', false, true);
-                        if ($currentname != $entitisedname) {
-                            // If name has some entity (&amp; &quot; &lt; &gt;) add that filter too. MDL-17545.
-                            self::$activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $href_tag_begin, '</a>', false, true);
-                        }
-                    }
-                }
-            }
-        }
+        $activitylist = $this->get_cached_activity_list($courseid);

 
         $filterslist = array();
 
         $filterslist = array();

-        if (self::$activitylist) {
+        if (!empty($activitylist)) {

             $cmid = $this->context->instanceid;
             $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
                 // 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 {
             } else {

-                $filterslist = array_values(self::$activitylist);
+                $filterslist = array_values($activitylist);

             }
         }
 
             }
         }
 


@@ -117,4 +58,76 @@ class filter_activitynames extends moodle_text_filter {
             return $text;
         }
     }
             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, '</a>', false, true);
+                    if ($currentname != $entitisedname) {
+                        // If name has some entity (&amp; &quot; &lt; &gt;) add that filter too. MDL-17545.
+                        $activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $hreftagbegin, '</a>', false, true);
+                    }
+                }
+            }
+        }
+        return $activitylist;
+    }

 }
 }

diff --git a/filter/activitynames/tests/filter_test.php b/filter/activitynames/tests/filter_test.php
index b21c924..98776b7 100644 (file)
--- 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]);
     }
         $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 = '<p>Please read the two pages Test 1 and Test 2.</p>';
+        $filtered1 = format_text($html, FORMAT_HTML, array('context' => $context1));
+        // Find all the activity links in the result.
+        $matches = [];
+        preg_match_all('~<a class="autolink" title="([^"]*)" href="[^"]*/mod/page/view.php\?id=([0-9]+)">([^<]*)</a>~',
+            $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('~<a class="autolink" title="([^"]*)" href="[^"]*/mod/page/view.php\?id=([0-9]+)">([^<]*)</a>~',
+            $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('~<a class="autolink" title="([^"]*)" href="[^"]*/mod/page/view.php\?id=([0-9]+)">([^<]*)</a>~',
+            $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]);
+    }

 }
 }