Make action=query&list=users use User::getRights()

Made action=query&list=users use User::getRights() if
usprop rights given. This not only removes redundant
code, but makes it execute the UserGetRights hook, so
that this now includes rights given by Extensions (eg.
CentralAuth does that).

Patch Set 2: Modified the User class to be able to
inject further data into User::newFromRow() and using
that to inject the groups taken out of one SQL query
(for performance reasons). Furthermore I've split up
the query in ApiQueryUsers.php into one for user data
and one for the groups, to only have one row for each
user.
After all the perfomance of this should now be ok, not
extremly good, but bearable (though I couldn't test it
deeply, as I don't have much data in my CentralAuth
environment).

Change-Id: Ie5b2924abb82ac254c77e1d04cc4d5b308962dad

diff --git a/includes/User.php b/includes/User.php
index a197077..e210eba 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -455,11 +455,12 @@ class User {
         * will be loaded once more from the database when accessing them.
         *
         * @param $row Array A row from the user table
+        * @param $data Array Further data to load into the object (see User::loadFromRow for valid keys)
         * @return User
         */
-       public static function newFromRow( $row ) {
+       public static function newFromRow( $row, $data = null ) {
                $user = new User;
-               $user->loadFromRow( $row );
+               $user->loadFromRow( $row, $data );
                return $user;
        }
 
@@ -1036,8 +1037,12 @@ class User {
         * Initialize this object from a row from the user table.
         *
         * @param $row Array Row from the user table to load.
+        * @param $data Array Further user data to load into the object
+        *
+        *      user_groups             Array with groups out of the user_groups table
+        *      user_properties         Array with properties out of the user_properties table
         */
-       public function loadFromRow( $row ) {
+       public function loadFromRow( $row, $data = null ) {
                $all = true;
 
                $this->mGroups = null; // deferred
@@ -1095,6 +1100,15 @@ class User {
                if ( $all ) {
                        $this->mLoadedItems = true;
                }
+
+               if ( is_array( $data ) ) {
+                       if ( is_array( $data['user_groups'] ) ) {
+                               $this->mGroups = $data['user_groups'];
+                       }
+                       if ( is_array( $data['user_properties'] ) ) {
+                               $this->loadOptions( $data['user_properties'] );
+                       }
+               }
        }
 
        /**
@@ -4128,9 +4142,11 @@ class User {
        }
 
        /**
-        * @todo document
+        * Load the user options either from cache, the database or an array
+        *
+        * @param $data Rows for the current user out of the user_properties table
         */
-       protected function loadOptions() {
+       protected function loadOptions( $data = null ) {
                global $wgContLang;
 
                $this->load();
@@ -4160,21 +4176,27 @@ class User {
                                $this->mOptions[$key] = $value;
                        }
                } else {
-                       wfDebug( "User: loading options for user " . $this->getId() . " from database.\n" );
-                       // Load from database
-                       $dbr = wfGetDB( DB_SLAVE );
-
-                       $res = $dbr->select(
-                               'user_properties',
-                               array( 'up_property', 'up_value' ),
-                               array( 'up_user' => $this->getId() ),
-                               __METHOD__
-                       );
+                       if( !is_array( $data ) ) {
+                               wfDebug( "User: loading options for user " . $this->getId() . " from database.\n" );
+                               // Load from database
+                               $dbr = wfGetDB( DB_SLAVE );
 
-                       $this->mOptionOverrides = array();
-                       foreach ( $res as $row ) {
-                               $this->mOptionOverrides[$row->up_property] = $row->up_value;
-                               $this->mOptions[$row->up_property] = $row->up_value;
+                               $res = $dbr->select(
+                                       'user_properties',
+                                       array( 'up_property', 'up_value' ),
+                                       array( 'up_user' => $this->getId() ),
+                                       __METHOD__
+                               );
+
+                               $this->mOptionOverrides = array();
+                               $data = array();
+                               foreach ( $res as $row ) {
+                                       $data[$row->up_property] = $row->up_value;
+                               }
+                       }
+                       foreach ( $data as $property => $value ) {
+                               $this->mOptionOverrides[$property] = $value;
+                               $this->mOptions[$property] = $value;
                        }
                }
 

diff --git a/includes/api/ApiQueryUsers.php b/includes/api/ApiQueryUsers.php
index bf438d1..1cf8e31 100644 (file)
--- a/includes/api/ApiQueryUsers.php
+++ b/includes/api/ApiQueryUsers.php
@@ -110,19 +110,39 @@ class ApiQueryUsers extends ApiQueryBase {
                        $this->addFields( User::selectFields() );
                        $this->addWhereFld( 'user_name', $goodNames );
 
-                       if ( isset( $this->prop['groups'] ) || isset( $this->prop['rights'] ) ) {
-                               $this->addTables( 'user_groups' );
-                               $this->addJoinConds( array( 'user_groups' => array( 'LEFT JOIN', 'ug_user=user_id' ) ) );
-                               $this->addFields( 'ug_group' );
-                       }
-
                        $this->showHiddenUsersAddBlockInfo( isset( $this->prop['blockinfo'] ) );
 
                        $data = array();
                        $res = $this->select( __METHOD__ );
+                       $this->resetQueryParams();
+
+                       // get user groups if needed
+                       if ( isset( $this->prop['groups'] ) || isset( $this->prop['rights'] ) ) {
+                               $userGroups = array();
+
+                               $this->addTables( 'user' );
+                               $this->addWhereFld( 'user_name', $goodNames );
+                               $this->addTables( 'user_groups' );
+                               $this->addJoinConds( array( 'user_groups' => array( 'INNER JOIN', 'ug_user=user_id' ) ) );
+                               $this->addFields( array('user_name', 'ug_group') );
+                               $userGroupsRes = $this->select( __METHOD__ );
+
+                               foreach( $userGroupsRes as $row ) {
+                                       $userGroups[$row->user_name][] = $row->ug_group;
+                               }
+                       }
 
                        foreach ( $res as $row ) {
-                               $user = User::newFromRow( $row );
+                               // create user object and pass along $userGroups if set
+                               // that reduces the number of database queries needed in User dramatically
+                               if ( !isset( $userGroups ) ) {
+                                       $user = User::newFromRow( $row );
+                               } else {
+                                       if ( !is_array( $userGroups[$row->user_name] ) ) {
+                                               $userGroups[$row->user_name] = array();
+                                       }
+                                       $user = User::newFromRow( $row, array( 'user_groups' => $userGroups[$row->user_name] ) );
+                               }
                                $name = $user->getName();
 
                                $data[$name]['userid'] = $user->getId();
@@ -137,29 +157,15 @@ class ApiQueryUsers extends ApiQueryBase {
                                }
 
                                if ( isset( $this->prop['groups'] ) ) {
-                                       if ( !isset( $data[$name]['groups'] ) ) {
-                                               $data[$name]['groups'] = $user->getAutomaticGroups();
-                                       }
-
-                                       if ( !is_null( $row->ug_group ) ) {
-                                               // This row contains only one group, others will be added from other rows
-                                               $data[$name]['groups'][] = $row->ug_group;
-                                       }
+                                       $data[$name]['groups'] = $user->getEffectiveGroups();
                                }
 
-                               if ( isset( $this->prop['implicitgroups'] ) && !isset( $data[$name]['implicitgroups'] ) ) {
+                               if ( isset( $this->prop['implicitgroups'] ) ) {
                                        $data[$name]['implicitgroups'] =  $user->getAutomaticGroups();
                                }
 
                                if ( isset( $this->prop['rights'] ) ) {
-                                       if ( !isset( $data[$name]['rights'] ) ) {
-                                               $data[$name]['rights'] = User::getGroupPermissions( $user->getAutomaticGroups() );
-                                       }
-
-                                       if ( !is_null( $row->ug_group ) ) {
-                                               $data[$name]['rights'] = array_unique( array_merge( $data[$name]['rights'],
-                                                       User::getGroupPermissions( array( $row->ug_group ) ) ) );
-                                       }
+                                       $data[$name]['rights'] = $user->getRights();
                                }
                                if ( $row->ipb_deleted ) {
                                        $data[$name]['hidden'] = '';