From 2ec2d00dc8f1de785b6476a94ddc3fd9d2c11782 Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Wed, 1 May 2013 11:35:14 +0100
Subject: [PATCH] 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.
---
 cache/classes/helper.php     |  3 +--
 lib/moodlelib.php            | 19 ++++++++++++-------
 lib/tests/moodlelib_test.php | 15 +++++++++++++++
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/cache/classes/helper.php b/cache/classes/helper.php
index 45fe789300c..caac45ee8d7 100644
--- a/cache/classes/helper.php
+++ b/cache/classes/helper.php
@@ -192,8 +192,7 @@ class cache_helper {
             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;
             }
diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index 58e28980729..4e8d15db484 100644
--- a/lib/moodlelib.php
+++ b/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;
@@ -8092,6 +8089,15 @@ function get_plugin_types($fullpaths=true) {
     return ($fullpaths ? $fullinfo : $info);
 }
 
+/**
+ * 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
@@ -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;
diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php
index 15011a44a27..676a1d2c281 100644
--- a/lib/tests/moodlelib_test.php
+++ b/lib/tests/moodlelib_test.php
@@ -797,6 +797,21 @@ class moodlelib_testcase extends advanced_testcase {
         $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');