Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

MDL-39445 get_plugin_list clean_param use is slow

get_plugin_list was calling clean_param($pluginname, PARAM_PLUGIN) a
lot (600+ times per page), and that is much slower than you would guess.
A specific function for this case, (which we then also use from
clean_param) is a performance win.
  • Loading branch information...
commit 2ec2d00dc8f1de785b6476a94ddc3fd9d2c11782 1 parent 43ded67
@timhunt timhunt authored
View
3  cache/classes/helper.php
@@ -192,8 +192,7 @@ public static function early_get_cache_plugins() {
if (in_array($pluginname, $ignored)) {
continue;
}
- $pluginname = clean_param($pluginname, PARAM_PLUGIN);
- if (empty($pluginname)) {
+ if (!is_valid_plugin_name($pluginname)) {
// Better ignore plugins with problematic names here.
continue;
}
View
19 lib/moodlelib.php
@@ -907,10 +907,7 @@ function clean_param($param, $type) {
case PARAM_PLUGIN:
case PARAM_AREA:
// we do not want any guessing here, either the name is correct or not
- if (!preg_match('/^[a-z][a-z0-9_]*[a-z0-9]$/', $param)) {
- return '';
- }
- if (strpos($param, '__') !== false) {
+ if (!is_valid_plugin_name($param)) {
return '';
}
return $param;
@@ -8093,6 +8090,15 @@ function get_plugin_types($fullpaths=true) {
}
/**
+ * This method validates a plug name. It is much faster than calling clean_param.
+ * @param string $name a string that might be a plugin name.
+ * @return bool if this string is a valid plugin name.
+ */
+function is_valid_plugin_name($name) {
+ return (bool) preg_match('/^[a-z](?:[a-z0-9_](?!__))*[a-z0-9]$/', $name);
+}
+
+/**
* Simplified version of get_list_of_plugins()
* @param string $plugintype type of plugin
* @return array name=>fulllocation pairs of plugins of given type
@@ -8156,9 +8162,8 @@ function get_plugin_list($plugintype) {
if (in_array($pluginname, $ignored)) {
continue;
}
- $pluginname = clean_param($pluginname, PARAM_PLUGIN);
- if (empty($pluginname)) {
- // better ignore plugins with problematic names here
+ if (!is_valid_plugin_name($pluginname)) {
+ // Better ignore plugins with problematic names here.
continue;
}
$result[$pluginname] = $fulldir.'/'.$pluginname;
View
15 lib/tests/moodlelib_test.php
@@ -797,6 +797,21 @@ function test_clean_param_component() {
$this->assertSame(clean_param('user_', PARAM_COMPONENT), '');
}
+ function test_is_valid_plugin_name() {
+ $this->assertTrue(is_valid_plugin_name('forum'));
+ $this->assertTrue(is_valid_plugin_name('forum2'));
+ $this->assertTrue(is_valid_plugin_name('online_users'));
+ $this->assertTrue(is_valid_plugin_name('blond_online_users'));
+ $this->assertFalse(is_valid_plugin_name('online__users'));
+ $this->assertFalse(is_valid_plugin_name('forum '));
+ $this->assertFalse(is_valid_plugin_name('forum.old'));
+ $this->assertFalse(is_valid_plugin_name('xx-yy'));
+ $this->assertFalse(is_valid_plugin_name('2xx'));
+ $this->assertFalse(is_valid_plugin_name('Xx'));
+ $this->assertFalse(is_valid_plugin_name('_xx'));
+ $this->assertFalse(is_valid_plugin_name('xx_'));
+ }
+
function test_clean_param_plugin() {
// please note the cleaning of plugin names is very strict, no guessing here
$this->assertSame(clean_param('forum', PARAM_PLUGIN), 'forum');
Please sign in to comment.
Something went wrong with that request. Please try again.