From 34949c1a55dff89b1f93698cb6340273396a169b Mon Sep 17 00:00:00 2001 From: Josh Willcock Date: Tue, 29 Nov 2016 17:55:04 +0000 Subject: [PATCH 1/3] MDL-57193 auth_db: Fixed bug suspends all users if external DB has > 10k --- auth/db/auth.php | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/auth/db/auth.php b/auth/db/auth.php index 8048a3039c6..a355a7fa169 100644 --- a/auth/db/auth.php +++ b/auth/db/auth.php @@ -301,17 +301,17 @@ class auth_plugin_db extends auth_plugin_base { // Find obsolete users. if (count($userlist)) { - $remove_users = array(); - // All the drivers can cope with chunks of 10,000. See line 4491 of lib/dml/tests/dml_est.php - $userlistchunks = array_chunk($userlist , 10000); - foreach($userlistchunks as $userlistchunk) { - list($notin_sql, $params) = $DB->get_in_or_equal($userlistchunk, SQL_PARAMS_NAMED, 'u', false); - $params['authtype'] = $this->authtype; - $sql = "SELECT u.id, u.username - FROM {user} u - WHERE u.auth=:authtype AND u.deleted=0 AND u.mnethostid=:mnethostid $suspendselect AND u.username $notin_sql"; - $params['mnethostid'] = $CFG->mnet_localhost_id; - $remove_users = $remove_users + $DB->get_records_sql($sql, $params); + $removeusers = array(); + $params['authtype'] = $this->authtype; + $sql = "SELECT u.id, u.username + FROM {user} u + WHERE u.auth=:authtype AND u.deleted=0 AND u.mnethostid=:mnethostid $suspendselect"; + $params['mnethostid'] = $CFG->mnet_localhost_id; + $internalusers = $DB->get_records_sql($sql, $params); + foreach ($internalusers as $internaluser) { + if (!in_array($internaluser->username, $userlist)) { + $removeusers[] = $internaluser; + } } } else { $sql = "SELECT u.id, u.username @@ -320,13 +320,13 @@ class auth_plugin_db extends auth_plugin_base { $params = array(); $params['authtype'] = $this->authtype; $params['mnethostid'] = $CFG->mnet_localhost_id; - $remove_users = $DB->get_records_sql($sql, $params); + $removeusers = $DB->get_records_sql($sql, $params); } - if (!empty($remove_users)) { - $trace->output(get_string('auth_dbuserstoremove','auth_db', count($remove_users))); + if (!empty($removeusers)) { + $trace->output(get_string('auth_dbuserstoremove', 'auth_db', count($removeusers))); - foreach ($remove_users as $user) { + foreach ($removeusers as $user) { if ($this->config->removeuser == AUTH_REMOVEUSER_FULLDELETE) { delete_user($user); $trace->output(get_string('auth_dbdeleteuser', 'auth_db', array('name'=>$user->username, 'id'=>$user->id)), 1); @@ -339,7 +339,7 @@ class auth_plugin_db extends auth_plugin_base { } } } - unset($remove_users); + unset($removeusers); } if (!count($userlist)) { @@ -933,5 +933,3 @@ class auth_plugin_db extends auth_plugin_base { return core_user::clean_data($user); } } - - From ec3e79134fed64c650cb22b113af75e6260bc2a9 Mon Sep 17 00:00:00 2001 From: John Okely Date: Fri, 2 Dec 2016 14:34:18 +0800 Subject: [PATCH 2/3] MDL-57193 auth_db: Use a recordset and array_key_exists --- auth/db/auth.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/auth/db/auth.php b/auth/db/auth.php index a355a7fa169..76c70621bc7 100644 --- a/auth/db/auth.php +++ b/auth/db/auth.php @@ -307,12 +307,15 @@ class auth_plugin_db extends auth_plugin_base { FROM {user} u WHERE u.auth=:authtype AND u.deleted=0 AND u.mnethostid=:mnethostid $suspendselect"; $params['mnethostid'] = $CFG->mnet_localhost_id; - $internalusers = $DB->get_records_sql($sql, $params); - foreach ($internalusers as $internaluser) { - if (!in_array($internaluser->username, $userlist)) { + $internalusersrs = $DB->get_recordset_sql($sql, $params); + foreach ($internalusersrs as $internaluser) { + // Arrange the associative array. + $usernamelist = array_flip($userlist); + if (!array_key_exists($internaluser->username, $usernamelist)) { $removeusers[] = $internaluser; } } + $internalusersrs->close(); } else { $sql = "SELECT u.id, u.username FROM {user} u From 7fc7486f49f82474a2578391bf9b64e39ba016e9 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Fri, 2 Dec 2016 16:02:00 +0800 Subject: [PATCH 3/3] MDL-57193 auth_db: Flip the array outside the loop for better perf Also as the recent changes where affecting the whitespaces quite a lot, I took the liberty of aligning a few lines which weren't. --- auth/db/auth.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/auth/db/auth.php b/auth/db/auth.php index 76c70621bc7..5660978f4a5 100644 --- a/auth/db/auth.php +++ b/auth/db/auth.php @@ -302,15 +302,18 @@ class auth_plugin_db extends auth_plugin_base { // Find obsolete users. if (count($userlist)) { $removeusers = array(); - $params['authtype'] = $this->authtype; - $sql = "SELECT u.id, u.username - FROM {user} u - WHERE u.auth=:authtype AND u.deleted=0 AND u.mnethostid=:mnethostid $suspendselect"; - $params['mnethostid'] = $CFG->mnet_localhost_id; - $internalusersrs = $DB->get_recordset_sql($sql, $params); + $params['authtype'] = $this->authtype; + $sql = "SELECT u.id, u.username + FROM {user} u + WHERE u.auth=:authtype + AND u.deleted=0 + AND u.mnethostid=:mnethostid + $suspendselect"; + $params['mnethostid'] = $CFG->mnet_localhost_id; + $internalusersrs = $DB->get_recordset_sql($sql, $params); + + $usernamelist = array_flip($userlist); foreach ($internalusersrs as $internaluser) { - // Arrange the associative array. - $usernamelist = array_flip($userlist); if (!array_key_exists($internaluser->username, $usernamelist)) { $removeusers[] = $internaluser; }