Merge remote-tracking branch 'origin/auth_getUserData_improvements'

* origin/auth_getUserData_improvements:
  KISS - remove class constants for REQUIRE_GROUPS & IGNORE_GROUPS and replace with boolean values
  use $requireGroups constants in auth classes; comments; code improvements
  fix comment errors, sp. & grammar
  code styling - add missing braces
  Allow user info to be retrieved without groups
  Restore correct public interface of getUserData() for authldap plugin

Conflicts:
	inc/common.php

8 files changed, 179 insertions, 49 deletions

diff --git a/inc/auth.php b/inc/auth.php
index b12800584..6abaa166f 100644
--- a/inc/auth.php
+++ b/inc/auth.php
@@ -923,7 +923,7 @@ function auth_sendPassword($user, $password) {
     if(!$auth) return false;
 
     $user     = $auth->cleanUser($user);
-    $userinfo = $auth->getUserData($user);
+    $userinfo = $auth->getUserData($user, $requireGroups = false);
 
     if(!$userinfo['mail']) return false;
 
@@ -1185,7 +1185,7 @@ function act_resendpwd() {
         }
 
         $user     = io_readfile($tfile);
-        $userinfo = $auth->getUserData($user);
+        $userinfo = $auth->getUserData($user, $requireGroups = false);
         if(!$userinfo['mail']) {
             msg($lang['resendpwdnouser'], -1);
             return false;
@@ -1237,7 +1237,7 @@ function act_resendpwd() {
             $user = trim($auth->cleanUser($INPUT->post->str('login')));
         }
 
-        $userinfo = $auth->getUserData($user);
+        $userinfo = $auth->getUserData($user, $requireGroups = false);
         if(!$userinfo['mail']) {
             msg($lang['resendpwdnouser'], -1);
             return false;
diff --git a/inc/common.php b/inc/common.php
index 503155c46..e56285f62 100644
--- a/inc/common.php
+++ b/inc/common.php
@@ -1583,7 +1583,7 @@ function shorten($keep, $short, $max, $min = 9, $char = '…') {
 }
 
 /**
- * Return the users realname or e-mail address for use
+ * Return the users real name or e-mail address for use
  * in page footer and recent changes pages
  *
  * @param string|null $username or null when currently logged-in user should be used
diff --git a/lib/plugins/auth.php b/lib/plugins/auth.php
index b04735639..b38b591a3 100644
--- a/lib/plugins/auth.php
+++ b/lib/plugins/auth.php
@@ -229,14 +229,15 @@ class DokuWiki_Auth_Plugin extends DokuWiki_Plugin {
      * at least these fields:
      *
      * name string  full name of the user
-     * mail string  email addres of the user
+     * mail string  email address of the user
      * grps array   list of groups the user is in
      *
      * @author  Andreas Gohr <andi@splitbrain.org>
      * @param   string $user the user name
+     * @param   bool $requireGroups whether or not the returned data must include groups
      * @return  array containing user data or false
      */
-    public function getUserData($user) {
+    public function getUserData($user, $requireGroups=true) {
         if(!$this->cando['external']) msg("no valid authorisation system in use", -1);
         return false;
     }
diff --git a/lib/plugins/authad/auth.php b/lib/plugins/authad/auth.php
index 0860e5756..a3119dda6 100644
--- a/lib/plugins/authad/auth.php
+++ b/lib/plugins/authad/auth.php
@@ -177,9 +177,10 @@ class auth_plugin_authad extends DokuWiki_Auth_Plugin {
      *
      * @author  James Van Lommel <james@nosq.com>
      * @param string $user
+     * @param bool $requireGroups (optional) - ignored, groups are always supplied by this plugin
      * @return array
      */
-    public function getUserData($user) {
+    public function getUserData($user, $requireGroups=true) {
         global $conf;
         global $lang;
         global $ID;
diff --git a/lib/plugins/authldap/auth.php b/lib/plugins/authldap/auth.php
index 0d5e130ea..b22b82ecc 100644
--- a/lib/plugins/authldap/auth.php
+++ b/lib/plugins/authldap/auth.php
@@ -103,7 +103,7 @@ class auth_plugin_authldap extends DokuWiki_Auth_Plugin {
             return true;
         } else {
             // See if we can find the user
-            $info = $this->getUserData($user, true);
+            $info = $this->_getUserData($user, true);
             if(empty($info['dn'])) {
                 return false;
             } else {
@@ -146,10 +146,19 @@ class auth_plugin_authldap extends DokuWiki_Auth_Plugin {
      * @author  Steffen Schoch <schoch@dsb.net>
      *
      * @param   string $user
+     * @param   bool   $requireGroups (optional) - ignored, groups are always supplied by this plugin
+     * @return  array containing user data or false
+     */
+    public function getUserData($user, $requireGroups=true) {
+        return $this->_getUserData($user);
+    }
+
+    /**
+     * @param   string $user
      * @param   bool   $inbind authldap specific, true if in bind phase
      * @return  array containing user data or false
      */
-    public function getUserData($user, $inbind = false) {
+    protected function _getUserData($user, $inbind = false) {
         global $conf;
         if(!$this->_openLDAP()) return false;
 
diff --git a/lib/plugins/authmysql/auth.php b/lib/plugins/authmysql/auth.php
index 1e6e6a4a9..95c62f636 100644
--- a/lib/plugins/authmysql/auth.php
+++ b/lib/plugins/authmysql/auth.php
@@ -21,6 +21,9 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
     /** @var int database subrevision */
     protected $dbsub = 0;
 
+    /** @var array cache to avoid re-reading user info data */
+    protected $cacheUserInfo = array();
+
     /**
      * Constructor
      *
@@ -157,10 +160,11 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
             $result = $this->_queryDB($sql);
 
             if($result !== false && count($result) == 1) {
-                if($this->getConf('forwardClearPass') == 1)
+                if($this->getConf('forwardClearPass') == 1) {
                     $rc = true;
-                else
+                } else {
                     $rc = auth_verifyPassword($pass, $result[0]['pass']);
+                }
             }
             $this->_closeDB();
         }
@@ -174,16 +178,23 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
      * @author  Matthias Grimm <matthiasgrimm@users.sourceforge.net>
      *
      * @param string $user user login to get data for
+     * @param bool $requireGroups  when true, group membership information should be included in the returned array;
+     *                             when false, it maybe included, but is not required by the caller
      * @return array|bool
      */
-    public function getUserData($user) {
+    public function getUserData($user, $requireGroups=true) {
+        if($this->_cacheExists($user, $requireGroups)) {
+            return $this->cacheUserInfo[$user];
+        }
+
         if($this->_openDB()) {
             $this->_lockTables("READ");
-            $info = $this->_getUserInfo($user);
+            $info = $this->_getUserInfo($user, $requireGroups);
             $this->_unlockTables();
             $this->_closeDB();
-        } else
+        } else {
             $info = false;
+        }
         return $info;
     }
 
@@ -209,12 +220,14 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
         global $conf;
 
         if($this->_openDB()) {
-            if(($info = $this->_getUserInfo($user)) !== false)
+            if(($info = $this->_getUserInfo($user)) !== false) {
                 return false; // user already exists
+            }
 
             // set defaultgroup if no groups were given
-            if($grps == null)
+            if($grps == null) {
                 $grps = array($conf['defaultgroup']);
+            }
 
             $this->_lockTables("WRITE");
             $pwd = $this->getConf('forwardClearPass') ? $pwd : auth_cryptPassword($pwd);
@@ -234,17 +247,17 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
      * The dataset update will be rejected if the user name should be changed
      * to an already existing one.
      *
-     * The password must be provides unencrypted. Pasword cryption is done
+     * The password must be provided unencrypted. Pasword encryption is done
      * automatically if configured.
      *
-     * If one or more groups could't be updated, an error would be set. In
+     * If one or more groups can't be updated, an error will be set. In
      * this case the dataset might already be changed and we can't rollback
-     * the changes. Transactions would be really usefull here.
+     * the changes. Transactions would be really useful here.
      *
      * modifyUser() may be called without SQL statements defined that are
      * needed to change group membership (for example if only the user profile
-     * should be modified). In this case we asure that we don't touch groups
-     * even $changes['grps'] is set by mistake.
+     * should be modified). In this case we assure that we don't touch groups
+     * even when $changes['grps'] is set by mistake.
      *
      * @author  Chris Smith <chris@jalakai.co.uk>
      * @author  Matthias Grimm <matthiasgrimm@users.sourceforge.net>
@@ -256,27 +269,30 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
     public function modifyUser($user, $changes) {
         $rc = false;
 
-        if(!is_array($changes) || !count($changes))
+        if(!is_array($changes) || !count($changes)) {
             return true; // nothing to change
+        }
 
         if($this->_openDB()) {
             $this->_lockTables("WRITE");
 
-            if(($uid = $this->_getUserID($user))) {
-                $rc = $this->_updateUserInfo($changes, $uid);
+            $rc = $this->_updateUserInfo($user, $changes);
 
-                if($rc && isset($changes['grps']) && $this->cando['modGroups']) {
-                    $groups = $this->_getGroups($user);
-                    $grpadd = array_diff($changes['grps'], $groups);
-                    $grpdel = array_diff($groups, $changes['grps']);
+            if($rc && isset($changes['grps']) && $this->cando['modGroups']) {
+                $groups = $this->_getGroups($user);
+                $grpadd = array_diff($changes['grps'], $groups);
+                $grpdel = array_diff($groups, $changes['grps']);
 
-                    foreach($grpadd as $group)
-                        if(($this->_addUserToGroup($user, $group, 1)) == false)
-                            $rc = false;
+                foreach($grpadd as $group) {
+                    if(($this->_addUserToGroup($user, $group, 1)) == false) {
+                        $rc = false;
+                    }
+                }
 
-                    foreach($grpdel as $group)
-                        if(($this->_delUserFromGroup($user, $group)) == false)
-                            $rc = false;
+                foreach($grpdel as $group) {
+                    if(($this->_delUserFromGroup($user, $group)) == false) {
+                        $rc = false;
+                    }
                 }
             }
 
@@ -304,8 +320,9 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
             if(is_array($users) && count($users)) {
                 $this->_lockTables("WRITE");
                 foreach($users as $user) {
-                    if($this->_delUser($user))
+                    if($this->_delUser($user)) {
                         $count++;
+                    }
                 }
                 $this->_unlockTables();
             }
@@ -367,9 +384,11 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
             $result = $this->_queryDB($sql);
 
             if(!empty($result)) {
-                foreach($result as $user)
-                    if(($info = $this->_getUserInfo($user['user'])))
+                foreach($result as $user) {
+                    if(($info = $this->_getUserInfo($user['user']))) {
                         $out[$user['user']] = $info;
+                    }
+                }
             }
 
             $this->_unlockTables();
@@ -466,7 +485,10 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
             $sql = str_replace('%{user}', $this->_escape($user), $sql);
             $sql = str_replace('%{gid}', $this->_escape($gid), $sql);
             $sql = str_replace('%{group}', $this->_escape($group), $sql);
-            if($this->_modifyDB($sql) !== false) return true;
+            if($this->_modifyDB($sql) !== false) {
+                $this->_flushUserInfoCache($user);
+                return true;
+            }
 
             if($newgroup) { // remove previously created group on error
                 $sql = str_replace('%{gid}', $this->_escape($gid), $this->getConf('delGroup'));
@@ -501,6 +523,10 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
                 $sql = str_replace('%{gid}', $this->_escape($gid), $sql);
                 $sql = str_replace('%{group}', $this->_escape($group), $sql);
                 $rc  = $this->_modifyDB($sql) == 0 ? true : false;
+
+                if ($rc) {
+                    $this->_flushUserInfoCache($user);
+                }
             }
         }
         return $rc;
@@ -526,8 +552,9 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
             $result = $this->_queryDB($sql);
 
             if($result !== false && count($result)) {
-                foreach($result as $row)
+                foreach($result as $row) {
                     $groups[] = $row['group'];
+                }
             }
             return $groups;
         }
@@ -590,6 +617,7 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
                 }
 
                 if($gid !== false){
+                    $this->_flushUserInfoCache($user);
                     return true;
                 } else {
                     /* remove the new user and all group relations if a group can't
@@ -614,7 +642,7 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
      *
      * @author Matthias Grimm <matthiasgrimm@users.sourceforge.net>
      *
-     * @param  string $user user whose id is desired
+     * @param  string $user username of the user to be deleted
      * @return bool
      */
     protected function _delUser($user) {
@@ -626,16 +654,96 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
                 $sql = str_replace('%{uid}', $this->_escape($uid), $this->getConf('delUser'));
                 $sql = str_replace('%{user}', $this->_escape($user), $sql);
                 $this->_modifyDB($sql);
+                $this->_flushUserInfoCache($user);
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Flush cached user information
+     *
+     * @author Christopher Smith <chris@jalakai.co.uk>
+     *
+     * @param  string  $user username of the user whose data is to be removed from the cache
+     *                       if null, empty the whole cache
+     * @return none
+     */
+    protected function _flushUserInfoCache($user=null) {
+        if (is_null($user)) {
+            $this->cacheUserInfo = array();
+        } else {
+            unset($this->cacheUserInfo[$user]);
+        }
+    }
+
+    /**
+     * Quick lookup to see if a user's information has been cached
+     *
+     * This test does not need a database connection or read lock
+     *
+     * @author Christopher Smith <chris@jalakai.co.uk>
+     *
+     * @param  string  $user  username to be looked up in the cache
+     * @param  bool    $requireGroups  true, if cached info should include group memberships
+     *
+     * @return bool    existence of required user information in the cache
+     */
+    protected function _cacheExists($user, $requireGroups=true) {
+        if (isset($this->cacheUserInfo[$user])) {
+            if (!is_array($this->cacheUserInfo[$user])) {
+                return true;          // user doesn't exist
+            }
+
+            if (!$requireGroups || isset($this->cacheUserInfo[$user]['grps'])) {
                 return true;
             }
         }
+
         return false;
     }
 
     /**
-     * getUserInfo
+     * Get a user's information
+     *
+     * The database connection must already be established for this function to work.
+     *
+     * @author Christopher Smith <chris@jalakai.co.uk>
+     *
+     * @param  string  $user  username of the user whose information is being reterieved
+     * @param  bool    $requireGroups  true if group memberships should be included
+     * @param  bool    $useCache       true if ok to return cached data & to cache returned data
+     *
+     * @return mixed   false|array     false if the user doesn't exist
+     *                                 array containing user information if user does exist
+     */
+    protected function _getUserInfo($user, $requireGroups=true, $useCache=true) {
+        $info = null;
+
+        if ($useCache && isset($this->cacheUserInfo[$user])) {
+            $info = $this->cacheUserInfo[$user];
+        }
+
+        if (is_null($info)) {
+            $info = $this->_retrieveUserInfo($user);
+        }
+
+        if (($requireGroups == true) && $info && !isset($info['grps'])) {
+            $info['grps'] = $this->_getGroups($user);
+        }
+
+        if ($useCache) {
+            $this->cacheUserInfo[$user] = $info;
+        }
+
+        return $info;
+    }
+
+    /**
+     * retrieveUserInfo
      *
-     * Gets the data for a specific user The database connection
+     * Gets the data for a specific user. The database connection
      * must already be established for this function to work.
      * Otherwise it will return 'false'.
      *
@@ -644,12 +752,11 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
      * @param  string $user  user's nick to get data for
      * @return bool|array false on error, user info on success
      */
-    protected function _getUserInfo($user) {
+    protected function _retrieveUserInfo($user) {
         $sql    = str_replace('%{user}', $this->_escape($user), $this->getConf('getUserInfo'));
         $result = $this->_queryDB($sql);
         if($result !== false && count($result)) {
             $info         = $result[0];
-            $info['grps'] = $this->_getGroups($user);
             return $info;
         }
         return false;
@@ -666,20 +773,26 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
      * The database connection has already to be established for this
      * function to work. Otherwise it will return 'false'.
      *
-     * The password will be crypted if necessary.
+     * The password will be encrypted if necessary.
      *
+     * @param  string $user    user's nick being updated
      * @param  array $changes  array of items to change as pairs of item and value
      * @param  mixed $uid      user id of dataset to change, must be unique in DB
      * @return bool true on success or false on error
      *
      * @author Matthias Grimm <matthiasgrimm@users.sourceforge.net>
      */
-    protected function _updateUserInfo($changes, $uid) {
+    protected function _updateUserInfo($user, $changes) {
         $sql = $this->getConf('updateUser')." ";
         $cnt = 0;
         $err = 0;
 
         if($this->dbcon) {
+            $uid = $this->_getUserID($user);
+            if ($uid === false) {
+                return false;
+            }
+
             foreach($changes as $item => $value) {
                 if($item == 'user') {
                     if(($this->_getUserID($changes['user']))) {
@@ -707,6 +820,7 @@ class auth_plugin_authmysql extends DokuWiki_Auth_Plugin {
                     $sql .= " ".str_replace('%{uid}', $uid, $this->getConf('UpdateTarget'));
                     if(get_class($this) == 'auth_mysql') $sql .= " LIMIT 1"; //some PgSQL inheritance comp.
                     $this->_modifyDB($sql);
+                    $this->_flushUserInfoCache($user);
                 }
                 return true;
             }
diff --git a/lib/plugins/authpgsql/auth.php b/lib/plugins/authpgsql/auth.php
index e51b39858..99f3ed443 100644
--- a/lib/plugins/authpgsql/auth.php
+++ b/lib/plugins/authpgsql/auth.php
@@ -160,7 +160,7 @@ class auth_plugin_authpgsql extends auth_plugin_authmysql {
             $result = $this->_queryDB($sql);
 
             foreach($result as $user)
-                if(($info = $this->_getUserInfo($user['user'])))
+                if(($info = $this->_getCachedUserInfo($user['user'])))
                     $out[$user['user']] = $info;
 
             $this->_unlockTables();
@@ -212,7 +212,10 @@ class auth_plugin_authpgsql extends auth_plugin_authmysql {
             $sql = str_replace('%{user}', addslashes($user), $sql);
             $sql = str_replace('%{gid}', addslashes($gid), $sql);
             $sql = str_replace('%{group}', addslashes($group), $sql);
-            if($this->_modifyDB($sql) !== false) return true;
+            if($this->_modifyDB($sql) !== false) {
+                $this->_flushUserInfoCache($user);
+                return true;
+            }
 
             if($newgroup) { // remove previously created group on error
                 $sql = str_replace('%{gid}', addslashes($gid), $this->conf['delGroup']);
@@ -267,6 +270,7 @@ class auth_plugin_authpgsql extends auth_plugin_authmysql {
                 }
 
                 if($gid !== false){
+                    $this->_flushUserInfoCache($user);
                     return true;
                 } else {
                     /* remove the new user and all group relations if a group can't
diff --git a/lib/plugins/authplain/auth.php b/lib/plugins/authplain/auth.php
index e53f56667..b3ca988b9 100644
--- a/lib/plugins/authplain/auth.php
+++ b/lib/plugins/authplain/auth.php
@@ -76,9 +76,10 @@ class auth_plugin_authplain extends DokuWiki_Auth_Plugin {
      *
      * @author  Andreas Gohr <andi@splitbrain.org>
      * @param string $user
+     * @param bool $requireGroups  (optional) ignored by this plugin, grps info always supplied
      * @return array|bool
      */
-    public function getUserData($user) {
+    public function getUserData($user, $requireGroups=true) {
         if($this->users === null) $this->_loadUserData();
         return isset($this->users[$user]) ? $this->users[$user] : false;
     }