From 9a4ad19f55c4ffdaa9ea7f5487fb77fa6f123cfd Mon Sep 17 00:00:00 2001 From: David Monllao Date: Mon, 19 Jan 2015 14:08:19 +0800 Subject: [PATCH 1/2] MDL-44367 filter_data: Moving cache at course level --- filter/data/filter.php | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/filter/data/filter.php b/filter/data/filter.php index e703d92a959..4eabac05c36 100644 --- a/filter/data/filter.php +++ b/filter/data/filter.php @@ -32,26 +32,36 @@ defined('MOODLE_INTERNAL') || die(); class filter_data extends moodle_text_filter { public function filter($text, array $options = array()) { - global $CFG, $DB; + global $CFG, $DB, $USER; - // Trivial-cache - keyed on $cachedcontextid - static $cachedcontextid; - static $contentlist; + // Trivial-cache - keyed on $cachedcourseid + $cacheduserid. + static $cachedcourseid = null; + static $cacheduserid = null; + static $coursecontentlist = array(); + static $sitecontentlist = array(); static $nothingtodo; // Try to get current course. $coursectx = $this->context->get_course_context(false); if (!$coursectx) { + // We could be in a course category so no entries for courseid == 0 will be found. $courseid = 0; } else { $courseid = $coursectx->instanceid; } - // Initialise/invalidate our trivial cache if dealing with a different context - if (!isset($cachedcontextid) || $cachedcontextid !== $this->context->id) { - $cachedcontextid = $this->context->id; - $contentlist = array(); + if ($cacheduserid !== $USER->id) { + // Invalidate all caches if the user changed. + $coursecontentlist = array(); + $sitecontentlist = array(); + $cacheduserid = $USER->id; + $cachedcourseid = $courseid; + $nothingtodo = false; + } else if ($courseid != get_site()->id && $courseid != 0 && $cachedcourseid != $courseid) { + // Invalidate course-level caches if the course id changed. + $coursecontentlist = array(); + $cachedcourseid = $courseid; $nothingtodo = false; } @@ -59,6 +69,13 @@ class filter_data extends moodle_text_filter { return $text; } + // If courseid == 0 only site entries will be returned. + if ($courseid == get_site()->id || $courseid == 0) { + $contentlist = & $sitecontentlist; + } else { + $contentlist = & $coursecontentlist; + } + // Create a list of all the resources to search for. It may be cached already. if (empty($contentlist)) { $coursestosearch = $courseid ? array($courseid) : array(); // Add courseid if found From 677e0dd6ca56abe3db7f036eadee13af33866022 Mon Sep 17 00:00:00 2001 From: David Monllao Date: Mon, 19 Jan 2015 15:14:52 +0800 Subject: [PATCH 2/2] MDL-44367 filter_data: Adding basic unit tests --- filter/data/tests/filter_test.php | 119 ++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 filter/data/tests/filter_test.php diff --git a/filter/data/tests/filter_test.php b/filter/data/tests/filter_test.php new file mode 100644 index 00000000000..17c279ce4b6 --- /dev/null +++ b/filter/data/tests/filter_test.php @@ -0,0 +1,119 @@ +. + +/** + * Unit tests. + * + * @package filter_data + * @category test + * @copyright 2015 David Monllao + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/filter/data/filter.php'); + +/** + * Tests for filter_data. + * + * @package filter_data + * @copyright 2015 David Monllao + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class filter_data_filter_testcase extends advanced_testcase { + + /** + * Tests that the filter applies the required changes. + * + * @return void + */ + public function test_filter() { + + $this->resetAfterTest(true); + $this->setAdminUser(); + filter_manager::reset_caches(); + + filter_set_global_state('data', TEXTFILTER_ON); + + $course1 = $this->getDataGenerator()->create_course(); + $coursecontext1 = context_course::instance($course1->id); + + $course2 = $this->getDataGenerator()->create_course(); + $coursecontext2 = context_course::instance($course2->id); + + $sitecontext = context_course::instance(SITEID); + + $site = get_site(); + $this->add_simple_database_instance($site, array('SiteEntry')); + $this->add_simple_database_instance($course1, array('CourseEntry')); + + $html = '

I like CourseEntry and SiteEntry

'; + + // Testing at course level (both site and course). + $filtered = format_text($html, FORMAT_HTML, array('context' => $coursecontext1)); + $this->assertRegExp('/title=(\'|")CourseEntry(\'|")/', $filtered); + $this->assertRegExp('/title=(\'|")SiteEntry(\'|")/', $filtered); + + // Testing at site level (only site). + $filtered = format_text($html, FORMAT_HTML, array('context' => $sitecontext)); + $this->assertNotRegExp('/title=(\'|")CourseEntry(\'|")/', $filtered); + $this->assertRegExp('/title=(\'|")SiteEntry(\'|")/', $filtered); + + // Changing to another course to test the caches invalidation (only site). + $filtered = format_text($html, FORMAT_HTML, array('context' => $coursecontext2)); + $this->assertNotRegExp('/title=(\'|")CourseEntry(\'|")/', $filtered); + $this->assertRegExp('/title=(\'|")SiteEntry(\'|")/', $filtered); + } + + /** + * Adds a database instance to the provided course + a text field + adds all attached entries. + * + * @param stdClass $course + * @param array $entries A list of entry names. + * @return void + */ + protected function add_simple_database_instance($course, $entries = false) { + global $DB; + + $database = $this->getDataGenerator()->create_module('data', + array('course' => $course->id)); + + // A database field. + $field = data_get_field_new('text', $database); + $fielddetail = new stdClass(); + $fielddetail->d = $database->id; + $fielddetail->mode = 'add'; + $fielddetail->type = 'text'; + $fielddetail->sesskey = sesskey(); + $fielddetail->name = 'Name'; + $fielddetail->description = 'Some name'; + $fielddetail->param1 = '1'; + $field->define_field($fielddetail); + $field->insert_field(); + $recordid = data_add_record($database); + + // Database entries. + foreach ($entries as $entrytext) { + $datacontent = array(); + $datacontent['fieldid'] = $field->field->id; + $datacontent['recordid'] = $recordid; + $datacontent['content'] = $entrytext; + $contentid = $DB->insert_record('data_content', $datacontent); + } + } +}