Revert r87129 "(bug 21196) Article::getContributors() no longer fail on PostgreSQL...

SkinTemplate::makeTalkUrlDetails given invalid pagename User:

Backtrace:

#0 /var/www/trunk/includes/SkinTemplate.php(691): SkinTemplate->makeTalkUrlDetails('User:')
#1 /var/www/trunk/includes/SkinTemplate.php(495): SkinTemplate->buildPersonalUrls(Object(OutputPage))
#2 /var/www/trunk/includes/OutputPage.php(1906): SkinTemplate->outputPage(Object(OutputPage))
#3 /var/www/trunk/includes/Wiki.php(402): OutputPage->output()
#4 /var/www/trunk/index.php(146): MediaWiki->finalCleanup()
#5 {main}

Seen trivially by going to login page while not logged in; some user check is failing and ending up with an improperly initialized object.

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index d155e9e..5f112d8 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -254,7 +254,6 @@ PHP if you have not done so prior to upgrading MediaWiki.
 * (bug 27249) "Installed software" table in Special:Version should always be
   left-to-right.
 * (bug 28719) Do not call mLinkHolders __destruct explicitly
-* (bug 21196) Article::getContributors() no longer fail on PostgreSQL.
 * (bug 28752) XCache doesn't work in CLI mode.
 
 === API changes in 1.18 ===

diff --git a/includes/Article.php b/includes/Article.php
index 4a04162..834a6be 100644 (file)
--- a/includes/Article.php
+++ b/includes/Article.php
@@ -836,18 +836,11 @@ class Article {
                $dbr = wfGetDB( DB_SLAVE );
                $userTable = $dbr->tableName( 'user' );
 
-               if ( $dbr->implicitGroupby() ) {
-                       $realNameField = 'user_real_name';
-               } else {
-                       $realNameField = 'FIRST(user_real_name) AS user_real_name';
-               }
-
                $tables = array( 'revision', 'user' );
 
                $fields = array(
-                       'rev_user as user_id',
+                       "$userTable.*",
                        'rev_user_text AS user_name',
-                       $realNameField,
                        'MAX(rev_timestamp) AS timestamp',
                );
 

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 20ce45a..4a0b951 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -389,9 +389,9 @@ function wfLogProfilingData() {
        if ( $forward ) {
                $forward = "\t(proxied via {$_SERVER['REMOTE_ADDR']}{$forward})";
        }
-       // Don't load $wgUser at this late stage just for statistics purposes
+       // Don't unstub $wgUser at this late stage just for statistics purposes
        // FIXME: We can detect some anons even if it is not loaded. See User::getId()
-       if ( $wgUser->isItemLoaded( 'id' ) && $wgUser->isAnon() ) {
+       if ( $wgUser->mDataLoaded && $wgUser->isAnon() ) {
                $forward .= ' anon';
        }
        $log = sprintf( "%s\t%04.3f\t%s\n",

diff --git a/includes/User.php b/includes/User.php
index 66d8b9f..5df021b 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -157,13 +157,10 @@ class User {
        /**
         * Bool Whether the cache variables have been loaded.
         */
-       //@{
-       var $mOptionsLoaded;
-       private $mLoadedItems = array();
-       //@}
+       var $mDataLoaded, $mAuthLoaded, $mOptionsLoaded;
 
        /**
-        * String Initialization data source if mLoadedItems!==true. May be one of:
+        * String Initialization data source if mDataLoaded==false. May be one of:
         *  - 'defaults'   anonymous user initialised from class defaults
         *  - 'name'       initialise from mName
         *  - 'id'         initialise from mId
@@ -214,13 +211,13 @@ class User {
         * Load the user table data for this object from the source given by mFrom.
         */
        function load() {
-               if ( $this->mLoadedItems === true ) {
+               if ( $this->mDataLoaded ) {
                        return;
                }
                wfProfileIn( __METHOD__ );
 
                # Set it now to avoid infinite recursion in accessors
-               $this->mLoadedItems = true;
+               $this->mDataLoaded = true;
 
                switch ( $this->mFrom ) {
                        case 'defaults':
@@ -339,7 +336,6 @@ class User {
                        $u = new User;
                        $u->mName = $name;
                        $u->mFrom = 'name';
-                       $u->setItemLoaded( 'name' );
                        return $u;
                }
        }
@@ -354,7 +350,6 @@ class User {
                $u = new User;
                $u->mId = $id;
                $u->mFrom = 'id';
-               $u->setItemLoaded( 'id' );
                return $u;
        }
 
@@ -395,14 +390,7 @@ class User {
 
        /**
         * Create a new user object from a user row.
-        * The row should have the following fields from the user table in it:
-        * - either user_name or user_id to load further data if needed (or both)
-        * - user_real_name
-        * - all other fields (email, password, etc.)
-        * It is useless to provide the remaining fields if either user_id,
-        * user_name and user_real_name are not provided because the whole row
-        * will be loaded once more from the database when accessing them.
-        *
+        * The row should have all fields from the user table in it.
         * @param $row Array A row from the user table
         * @return User
         */
@@ -858,31 +846,6 @@ class User {
                wfProfileOut( __METHOD__ );
        }
 
-       /**
-        * Return whether an item has been loaded.
-        *
-        * @param $item String: item to check. Current possibilities:
-        *              - 'id'
-        *              - name
-        *              - realname
-        * @return Boolean
-        */
-       public function isItemLoaded( $item ) {
-               return $this->mLoadedItems === true || ( isset( $this->mLoadedItems[$item] )
-                       && $this->mLoadedItems[$item] === true );
-       }
-
-       /**
-        * Set that an item has been loaded
-        *
-        * @param $item String
-        */
-       private function setItemLoaded( $item ) {
-               if ( is_array( $this->mLoadedItems ) ) {
-                       $this->mLoadedItems[$item] = true;
-               }
-       }
-
        /**
         * Load user data from the session or login cookie. If there are no valid
         * credentials, initialises the user as an anonymous user.
@@ -973,7 +936,7 @@ class User {
 
        /**
         * Load user and user_group data from the database.
-        * $this->mId must be set, this is how the user is identified.
+        * $this::mId must be set, this is how the user is identified.
         *
         * @return Bool True if the user exists, false if the user is anonymous
         * @private
@@ -1013,51 +976,25 @@ class User {
         * @param $row Array Row from the user table to load.
         */
        function loadFromRow( $row ) {
-               $all = true;
-
-               if ( isset( $row->user_name ) ) {
-                       $this->mName = $row->user_name;
-                       $this->mFrom = 'name';
-                       $this->setItemLoaded( 'name' );
-               } else {
-                       $all = false;
-               }
-
-               if ( isset( $row->user_name ) ) {
-                       $this->mRealName = $row->user_real_name;
-                       $this->setItemLoaded( 'realname' );
-               } else {
-                       $all = false;
-               }
+               $this->mDataLoaded = true;
 
                if ( isset( $row->user_id ) ) {
                        $this->mId = intval( $row->user_id );
-                       $this->mFrom = 'id';
-                       $this->setItemLoaded( 'id' );
-               } else {
-                       $all = false;
-               }
-
-               if ( isset( $row->user_password ) ) {
-                       $this->mPassword = $row->user_password;
-                       $this->mNewpassword = $row->user_newpassword;
-                       $this->mNewpassTime = wfTimestampOrNull( TS_MW, $row->user_newpass_time );
-                       $this->mEmail = $row->user_email;
-                       $this->decodeOptions( $row->user_options );
-                       $this->mTouched = wfTimestamp(TS_MW,$row->user_touched);
-                       $this->mToken = $row->user_token;
-                       $this->mEmailAuthenticated = wfTimestampOrNull( TS_MW, $row->user_email_authenticated );
-                       $this->mEmailToken = $row->user_email_token;
-                       $this->mEmailTokenExpires = wfTimestampOrNull( TS_MW, $row->user_email_token_expires );
-                       $this->mRegistration = wfTimestampOrNull( TS_MW, $row->user_registration );
-                       $this->mEditCount = $row->user_editcount;
-               } else {
-                       $all = false;
-               }
-
-               if ( $all ) {
-                       $this->mLoadedItems = true;
                }
+               $this->mName = $row->user_name;
+               $this->mRealName = $row->user_real_name;
+               $this->mPassword = $row->user_password;
+               $this->mNewpassword = $row->user_newpassword;
+               $this->mNewpassTime = wfTimestampOrNull( TS_MW, $row->user_newpass_time );
+               $this->mEmail = $row->user_email;
+               $this->decodeOptions( $row->user_options );
+               $this->mTouched = wfTimestamp(TS_MW,$row->user_touched);
+               $this->mToken = $row->user_token;
+               $this->mEmailAuthenticated = wfTimestampOrNull( TS_MW, $row->user_email_authenticated );
+               $this->mEmailToken = $row->user_email_token;
+               $this->mEmailTokenExpires = wfTimestampOrNull( TS_MW, $row->user_email_token_expires );
+               $this->mRegistration = wfTimestampOrNull( TS_MW, $row->user_registration );
+               $this->mEditCount = $row->user_editcount;
        }
 
        /**
@@ -1095,7 +1032,7 @@ class User {
                $this->mOptions = null;
 
                if ( $reloadFrom ) {
-                       $this->mLoadedItems = array();
+                       $this->mDataLoaded = false;
                        $this->mFrom = $reloadFrom;
                }
        }
@@ -1524,7 +1461,7 @@ class User {
                && User::isIP( $this->mName ) ) {
                        // Special case, we know the user is anonymous
                        return 0;
-               } elseif( !$this->isItemLoaded( 'id' ) ) {
+               } elseif( $this->mId === null ) {
                        // Don't load if this was initialized from an ID
                        $this->load();
                }
@@ -1545,7 +1482,7 @@ class User {
         * @return String User's name or IP address
         */
        function getName() {
-               if ( $this->isItemLoaded( 'name' ) ) {
+               if ( !$this->mDataLoaded && $this->mFrom == 'name' ) {
                        # Special case optimisation
                        return $this->mName;
                } else {
@@ -1973,10 +1910,7 @@ class User {
         * @return String User's real name
         */
        function getRealName() {
-               if ( !$this->isItemLoaded( 'realname' ) ) {
-                       $this->load();
-               }
-
+               $this->load();
                return $this->mRealName;
        }
 
@@ -2925,8 +2859,6 @@ class User {
         */
        function checkTemporaryPassword( $plaintext ) {
                global $wgNewPasswordExpiry;
-
-               $this->load();
                if( self::comparePasswords( $this->mNewpassword, $plaintext, $this->getId() ) ) {
                        if ( is_null( $this->mNewpassTime ) ) {
                                return true;
@@ -3242,11 +3174,9 @@ class User {
         *     non-existent/anonymous user accounts.
         */
        public function getRegistration() {
-               if ( $this->isAnon() ) {
-                       return false;
-               }
-               $this->load();
-               return $this->mRegistration;
+               return $this->getId() > 0
+                       ? $this->mRegistration
+                       : false;
        }
 
        /**

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 77f7c7c..4d2d335 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -668,7 +668,7 @@ abstract class DatabaseBase implements DatabaseType {
                # Add a comment for easy SHOW PROCESSLIST interpretation
                # if ( $fname ) {
                        global $wgUser;
-                       if ( is_object( $wgUser ) && $wgUser->isItemLoaded( 'name' ) ) {
+                       if ( is_object( $wgUser ) && $wgUser->mDataLoaded ) {
                                $userName = $wgUser->getName();
                                if ( mb_strlen( $userName ) > 15 ) {
                                        $userName = mb_substr( $userName, 0, 15 ) . '...';