From 85d99c4c5215146aa97846eb34b24f1de019af33 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 29 Apr 2011 23:57:28 +0000 Subject: [PATCH] Revert r87129 "(bug 21196) Article::getContributors() no longer fail on PostgreSQL" -- breaks stuff under MySQL like this: 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. --- RELEASE-NOTES | 1 - includes/Article.php | 9 +-- includes/GlobalFunctions.php | 4 +- includes/User.php | 126 ++++++++--------------------------- includes/db/Database.php | 2 +- 5 files changed, 32 insertions(+), 110 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index d155e9e31d..5f112d8d6e 100644 --- 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 4a0416257a..834a6be1fc 100644 --- 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 20ce45a018..4a0b95172f 100644 --- 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 66d8b9fa03..5df021b5fb 100644 --- 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 77f7c7c758..4d2d3357ba 100644 --- 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 ) . '...'; -- 2.20.1