mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-17 21:49:15 +01:00
Code Issues Projects Releases Wiki Activity

MDL-67428 navigation: Apply navigation text filters at system context

Browse Source 
On some larger sites, processing the navigation with format_string will
habitually load all the contexts for navigation which can take 400+ DB
queries. Explicitly tying all those format_string calls to the system
context reduces this overhead to a single DB query that probably has
already been run on the page previously.

Co-authored-by: Peter Burnett <peterburnett@catalyst-au.net>
This commit is contained in:
Peter Spicer 2019-12-10 15:31:37 +00:00 committed by Peter Burnett
parent 1d99ba19a2
commit edde68e078
No known key found for this signature in database
GPG Key ID: 6517EE0CF9151707
5 changed files with 64 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
admin/settings/plugins.php
View File

@ -251,6 +251,9 @@ if ($hassiteconfig) {
array('0' => new lang_string('none'), '1' => new lang_string('allfiles'), '2' => new lang_string('htmlfilesonly')));
$items[] = new admin_setting_configcheckbox('filtermatchoneperpage', new lang_string('filtermatchoneperpage', 'admin'), new lang_string('configfiltermatchoneperpage', 'admin'), 0);
$items[] = new admin_setting_configcheckbox('filtermatchonepertext', new lang_string('filtermatchonepertext', 'admin'), new lang_string('configfiltermatchonepertext', 'admin'), 0);
$items[] = new admin_setting_configcheckbox('filternavigationwithsystemcontext',
new lang_string('filternavigationwithsystemcontext', 'admin'),
new lang_string('configfilternavigationwithsystemcontext', 'admin'), 0);
foreach ($items as $item) {
$item->set_updatedcallback('reset_text_filters_cache');
$temp->add($item);

3
lang/en/admin.php
View File

@ -251,6 +251,7 @@ $string['configextramemorylimit'] = 'Some scripts like search, backup/restore or
$string['configfilterall'] = 'Filter all strings, including headings, titles, navigation bar and so on. This is mostly useful when using the multilang filter, otherwise it will just create extra load on your site for little gain.';
$string['configfiltermatchoneperpage'] = 'Automatic linking filters will only generate a single link for the first matching text instance found on the complete page. All others are ignored.';
$string['configfiltermatchonepertext'] = 'Automatic linking filters will only generate a single link for the first matching text instance found in each item of text on the page. All others are ignored. This setting has no effect if \'Filter match once per page\' is enabled.';
$string['configfilternavigationwithsystemcontext'] = 'Normal use of the filtering is tied to the context in which it is used (e.g. course context), but for the site navigation, explicitly making everything filter with site context can yield performance improvements when using "content and headings" filtering.';
$string['configfilteruploadedfiles'] = 'Process all uploaded HTML and text files with the filters before displaying them, only uploaded HTML files or none at all.';
$string['configforcelogin'] = 'Normally, the site home and the course listings (but not courses) can be read by people without logging in to the site. If you want to force people to log in before they do ANYTHING on the site, then you should enable this setting.';
$string['configforceloginforprofiles'] = 'This setting forces people to log in as a real (non-guest) account before viewing any user\'s profile. If you disabled this setting, you may find that some users post advertising (spam) or other inappropriate content in their profiles, which is then visible to the whole world.';
@ -621,6 +622,7 @@ $string['filestoredinhelp'] = 'Where the file will be stored';
$string['filterall'] = 'Filter all strings';
$string['filtermatchoneperpage'] = 'Filter match once per page';
$string['filtermatchonepertext'] = 'Filter match once per text';
$string['filternavigationwithsystemcontext'] = 'Filter navigation with system context';
$string['filters'] = 'Filters';
$string['filtersettings'] = 'Manage filters';
$string['filtersettingsgeneral'] = 'General filter settings';
@ -1557,4 +1559,3 @@ $string['modchooserdefault'] = 'Activity chooser default';
$string['coursepage'] = 'Course page';
$string['mediapluginswf'] = 'Enable .swf filter';
$string['mediapluginswfnote'] = 'As a default security measure, normal users should not be allowed to embed swf flash files.';

15
lib/accesslib.php
View File

@ -6231,6 +6231,21 @@ class context_helper extends context {
return $classname::get_level_name();
}
/**
* Gets the current context to be used for navigation tree filtering.
*
* @param context|null $context The current context to be checked against.
* @return context|null the context that navigation tree filtering should use.
*/
public static function get_navigation_filter_context(?context $context): ?context {
global $CFG;
if (!empty($CFG->filternavigationwithsystemcontext)) {
return context_system::instance();
} else {
return $context;
}
}
/**
* not used
*/

33
lib/navigationlib.php
View File

@ -679,10 +679,13 @@ class navigation_node implements renderable {
* @return string
*/
public function get_content($shorttext=false) {
$navcontext = \context_helper::get_navigation_filter_context(null);
$options = !empty($navcontext) ? ['context' => $navcontext] : null;
if ($shorttext && $this->shorttext!==null) {
return format_string($this->shorttext);
return format_string($this->shorttext, null, $options);
} else {
return format_string($this->text);
return format_string($this->text, null, $options);
}
}
@ -2097,13 +2100,14 @@ class global_navigation extends navigation_node {
* @return void.
*/
protected function add_category(stdClass $category, navigation_node $parent, $nodetype = self::TYPE_CATEGORY) {
global $CFG;
if (array_key_exists($category->id, $this->addedcategories)) {
return;
}
$canview = core_course_category::can_view_category($category);
$url = $canview ? new moodle_url('/course/index.php', array('categoryid' => $category->id)) : null;
$context = context_coursecat::instance($category->id);
$categoryname = $canview ? format_string($category->name, true, array('context' => $context)) :
$context = \context_helper::get_navigation_filter_context(context_coursecat::instance($category->id));
$categoryname = $canview ? format_string($category->name, true, ['context' => $context]) :
get_string('categoryhidden');
$categorynode = $parent->add($categoryname, $url, $nodetype, $categoryname, $category->id);
if (!$canview) {
@ -2314,7 +2318,8 @@ class global_navigation extends navigation_node {
}
// Prepare the default name and url for the node
$activityname = format_string($activity->name, true, array('context' => context_module::instance($activity->id)));
$displaycontext = \context_helper::get_navigation_filter_context(context_module::instance($activity->id));
$activityname = format_string($activity->name, true, ['context' => $displaycontext]);
$action = new moodle_url($activity->url);
// Check if the onclick property is set (puke!)
@ -2766,8 +2771,9 @@ class global_navigation extends navigation_node {
}
$issite = ($course->id == $SITE->id);
$shortname = format_string($course->shortname, true, array('context' => $coursecontext));
$fullname = format_string($course->fullname, true, array('context' => $coursecontext));
$displaycontext = \context_helper::get_navigation_filter_context($coursecontext);
$shortname = format_string($course->shortname, true, ['context' => $displaycontext]);
$fullname = format_string($course->fullname, true, ['context' => $displaycontext]);
// This is the name that will be shown for the course.
$coursename = empty($CFG->navshowfullcoursenames) ? $shortname : $fullname;
@ -2822,7 +2828,7 @@ class global_navigation extends navigation_node {
$coursenode->showinflatnavigation = $coursetype == self::COURSE_MY;
$coursenode->hidden = (!$course->visible);
$coursenode->title(format_string($course->fullname, true, array('context' => $coursecontext, 'escape' => false)));
$coursenode->title(format_string($course->fullname, true, ['context' => $displaycontext, 'escape' => false]));
if ($canexpandcourse) {
// This course can be expanded by the user, make it a branch to make the system aware that its expandable by ajax.
$coursenode->nodetype = self::NODETYPE_BRANCH;
@ -3784,8 +3790,10 @@ class navbar extends navigation_node {
if (!core_course_category::can_view_category($category)) {
continue;
}
$url = new moodle_url('/course/index.php', array('categoryid' => $category->id));
$name = format_string($category->name, true, array('context' => $context));
$displaycontext = \context_helper::get_navigation_filter_context($context);
$url = new moodle_url('/course/index.php', ['categoryid' => $category->id]);
$name = format_string($category->name, true, ['context' => $displaycontext]);
$categorynode = breadcrumb_navigation_node::create($name, $url, self::TYPE_CATEGORY, null, $category->id);
if (!$category->visible) {
$categorynode->hidden = true;
@ -4138,10 +4146,11 @@ class flat_navigation extends navigation_node_collection {
$url = new moodle_url('/course/view.php', array('id' => $course->id));
$coursecontext = context_course::instance($course->id, MUST_EXIST);
$displaycontext = \context_helper::get_navigation_filter_context($coursecontext);
// This is the name that will be shown for the course.
$coursename = empty($CFG->navshowfullcoursenames) ?
format_string($course->shortname, true, array('context' => $coursecontext)) :
format_string($course->fullname, true, array('context' => $coursecontext));
format_string($course->shortname, true, ['context' => $displaycontext]) :
format_string($course->fullname, true, ['context' => $displaycontext]);
$flat = new flat_navigation_node(navigation_node::create($coursename, $url), 0);
$flat->set_collectionlabel($coursename);

23
lib/tests/accesslib_test.php
View File

@ -4848,6 +4848,29 @@ class accesslib_test extends advanced_testcase {
$this->expectException(\required_capability_exception::class);
require_all_capabilities($sca, $coursecontext);
}
/**
* Test get_navigation_filter_context.
*
* @covers ::get_navigation_filter_context
*/
public function test_get_navigation_filter_context() {
$this->resetAfterTest();
$course = $this->getDataGenerator()->create_course();
set_config('filternavigationwithsystemcontext', 0);
// First test passed values are returned if disabled.
$this->assertNull(context_helper::get_navigation_filter_context(null));
$coursecontext = context_course::instance($course->id);
$filtercontext = context_helper::get_navigation_filter_context($coursecontext);
$this->assertEquals($coursecontext->id, $filtercontext->id);
// Now test that any input returns system context if enabled.
set_config('filternavigationwithsystemcontext', 1);
$filtercontext = context_helper::get_navigation_filter_context(null);
$this->assertInstanceOf('\context_system', $filtercontext);
$filtercontext = context_helper::get_navigation_filter_context($coursecontext);
$this->assertInstanceOf('\context_system', $filtercontext);
}
}
/**