From: Brad Jorsch Date: Thu, 12 Mar 2015 16:37:04 +0000 (-0400) Subject: Clean up handling of 'infinity' X-Git-Tag: 1.31.0-rc.0~11719^2 X-Git-Url: http://git.heureux-cyclage.org/?a=commitdiff_plain;h=ac6f81d9ad5cbde3f7461f02c612118967727d9e;p=lhc%2Fweb%2Fwiklou.git Clean up handling of 'infinity' There's a bunch of stuff that probably only works because the database representation of infinity is actually 'infinity' on all databases besides Oracle, and Oracle in general isn't maintained. Generally, we should probably use 'infinity' everywhere except where directly dealing with the database. * Many extension callers of Language::formatExpiry() with $format !== true are assuming it'll return 'infinity', none are checking for $db->getInfinity(). * And Language::formatExpiry() would choke if passed 'infinity', despite callers doing this. * And Language::formatExpiry() could be more useful for the API if we can override the string returned for infinity. * As for core, Title is using Language::formatExpiry() with TS_MW which is going to be changing anyway. Extension callers mostly don't exist. * Block already normalizes its mExpiry field (and ->getExpiry()), but some stuff is comparing it with $db->getInfinity() anyway. A few external users set mExpiry to $db->getInfinity(), but this is mostly because SpecialBlock::parseExpiryInput() returns $db->getInfinity() while most callers (including all extensions) are assuming 'infinity'. * And for that matter, Block should use $db->decodeExpiry() instead of manually doing it, once we make that safe to call with 'infinity' for all the extensions passing $db->getInfinity() to Block's contructor. * WikiPage::doUpdateRestrictions() and some of its callers are using $db->getInfinity(), when all the inserts using that value are using $db->encodeExpiry() which will convert 'infinity'. This also cleans up a slave-lag issue I noticed in ApiBlock while testing. Bug: T92550 Change-Id: I5eb68c1fb6029da8289276ecf7c81330575029ef --- diff --git a/includes/Block.php b/includes/Block.php index 4698f457d6..184e308dbc 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -122,11 +122,7 @@ class Block { $this->mAuto = $auto; $this->isHardblock( !$anonOnly ); $this->prevents( 'createaccount', $createAccount ); - if ( $expiry == 'infinity' || $expiry == wfGetDB( DB_SLAVE )->getInfinity() ) { - $this->mExpiry = 'infinity'; - } else { - $this->mExpiry = wfTimestamp( TS_MW, $expiry ); - } + $this->mExpiry = wfGetDB( DB_SLAVE )->decodeExpiry( $expiry ); $this->isAutoblocking( $enableAutoblock ); $this->mHideName = $hideName; $this->prevents( 'sendemail', $blockEmail ); @@ -379,12 +375,7 @@ class Block { $this->mParentBlockId = $row->ipb_parent_block_id; // I wish I didn't have to do this - $db = wfGetDB( DB_SLAVE ); - if ( $row->ipb_expiry == $db->getInfinity() ) { - $this->mExpiry = 'infinity'; - } else { - $this->mExpiry = wfTimestamp( TS_MW, $row->ipb_expiry ); - } + $this->mExpiry = wfGetDB( DB_SLAVE )->decodeExpiry( $row->ipb_expiry ); $this->isHardblock( !$row->ipb_anon_only ); $this->isAutoblocking( $row->ipb_enable_autoblock ); diff --git a/includes/ProtectionForm.php b/includes/ProtectionForm.php index 1219da51d1..c546de75f3 100644 --- a/includes/ProtectionForm.php +++ b/includes/ProtectionForm.php @@ -157,7 +157,7 @@ class ProtectionForm { $value = $this->mExpirySelection[$action]; } if ( wfIsInfinity( $value ) ) { - $time = wfGetDB( DB_SLAVE )->getInfinity(); + $time = 'infinity'; } else { $unix = strtotime( $value ); diff --git a/includes/Title.php b/includes/Title.php index 36237ed5bf..ddc941f362 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -2574,6 +2574,7 @@ class Title { if ( $row['permission'] == 'autoconfirmed' ) { $row['permission'] = 'editsemiprotected'; // B/C } + $row['expiry'] = $dbr->decodeExpiry( $row['expiry'] ); } $this->mTitleProtection = $row; } @@ -2711,7 +2712,6 @@ class Title { * false. */ public function getCascadeProtectionSources( $getPages = true ) { - global $wgContLang; $pagerestrictions = array(); if ( $this->mCascadeSources !== null && $getPages ) { @@ -2754,7 +2754,7 @@ class Title { $now = wfTimestampNow(); foreach ( $res as $row ) { - $expiry = $wgContLang->formatExpiry( $row->pr_expiry, TS_MW ); + $expiry = $dbr->decodeExpiry( $row->pr_expiry ); if ( $expiry > $now ) { if ( $getPages ) { $page_id = $row->pr_page; @@ -2887,14 +2887,13 @@ class Title { * restrictions from page table (pre 1.10) */ public function loadRestrictionsFromRows( $rows, $oldFashionedRestrictions = null ) { - global $wgContLang; $dbr = wfGetDB( DB_SLAVE ); $restrictionTypes = $this->getRestrictionTypes(); foreach ( $restrictionTypes as $type ) { $this->mRestrictions[$type] = array(); - $this->mRestrictionsExpiry[$type] = $wgContLang->formatExpiry( '', TS_MW ); + $this->mRestrictionsExpiry[$type] = 'infinity'; } $this->mCascadeRestriction = false; @@ -2940,7 +2939,7 @@ class Title { // This code should be refactored, now that it's being used more generally, // But I don't really see any harm in leaving it in Block for now -werdna - $expiry = $wgContLang->formatExpiry( $row->pr_expiry, TS_MW ); + $expiry = $dbr->decodeExpiry( $row->pr_expiry ); // Only apply the restrictions if they haven't expired! if ( !$expiry || $expiry > $now ) { @@ -2962,11 +2961,9 @@ class Title { * restrictions from page table (pre 1.10) */ public function loadRestrictions( $oldFashionedRestrictions = null ) { - global $wgContLang; if ( !$this->mRestrictionsLoaded ) { + $dbr = wfGetDB( DB_SLAVE ); if ( $this->exists() ) { - $dbr = wfGetDB( DB_SLAVE ); - $res = $dbr->select( 'page_restrictions', array( 'pr_type', 'pr_expiry', 'pr_level', 'pr_cascade' ), @@ -2980,7 +2977,7 @@ class Title { if ( $title_protection ) { $now = wfTimestampNow(); - $expiry = $wgContLang->formatExpiry( $title_protection['expiry'], TS_MW ); + $expiry = $dbr->decodeExpiry( $title_protection['expiry'] ); if ( !$expiry || $expiry > $now ) { // Apply the restrictions @@ -2990,7 +2987,7 @@ class Title { $this->mTitleProtection = false; } } else { - $this->mRestrictionsExpiry['create'] = $wgContLang->formatExpiry( '', TS_MW ); + $this->mRestrictionsExpiry['create'] = 'infinity'; } $this->mRestrictionsLoaded = true; } diff --git a/includes/api/ApiBlock.php b/includes/api/ApiBlock.php index f03cef2c67..72aee32118 100644 --- a/includes/api/ApiBlock.php +++ b/includes/api/ApiBlock.php @@ -39,6 +39,8 @@ class ApiBlock extends ApiBase { * of success. If it fails, the result will specify the nature of the error. */ public function execute() { + global $wgContLang; + $user = $this->getUser(); $params = $this->extractRequestParams(); @@ -100,11 +102,9 @@ class ApiBlock extends ApiBase { $res['user'] = $params['user']; $res['userID'] = $target instanceof User ? $target->getId() : 0; - $block = Block::newFromTarget( $target ); + $block = Block::newFromTarget( $target, null, true ); if ( $block instanceof Block ) { - $res['expiry'] = $block->mExpiry == $this->getDB()->getInfinity() - ? 'infinite' - : wfTimestamp( TS_ISO_8601, $block->mExpiry ); + $res['expiry'] = $wgContLang->formatExpiry( $block->mExpiry, TS_ISO_8601, 'infinite' ); $res['id'] = $block->getId(); } else { # should be unreachable diff --git a/includes/api/ApiProtect.php b/includes/api/ApiProtect.php index 4736cfb54b..675aa58442 100644 --- a/includes/api/ApiProtect.php +++ b/includes/api/ApiProtect.php @@ -29,6 +29,8 @@ */ class ApiProtect extends ApiBase { public function execute() { + global $wgContLang; + $params = $this->extractRequestParams(); $pageObj = $this->getTitleOrPageId( $params, 'fromdbmaster' ); @@ -78,7 +80,7 @@ class ApiProtect extends ApiBase { } if ( wfIsInfinity( $expiry[$i] ) ) { - $expiryarray[$p[0]] = $db->getInfinity(); + $expiryarray[$p[0]] = 'infinity'; } else { $exp = strtotime( $expiry[$i] ); if ( $exp < 0 || !$exp ) { @@ -93,10 +95,7 @@ class ApiProtect extends ApiBase { } $resultProtections[] = array( $p[0] => $protections[$p[0]], - 'expiry' => ( $expiryarray[$p[0]] == $db->getInfinity() - ? 'infinite' - : wfTimestamp( TS_ISO_8601, $expiryarray[$p[0]] ) - ) + 'expiry' => $wgContLang->formatExpiry( $expiryarray[$p[0]], TS_ISO_8601, 'infinite' ), ); } diff --git a/includes/api/ApiQueryLogEvents.php b/includes/api/ApiQueryLogEvents.php index adf96fd569..1e32162ef7 100644 --- a/includes/api/ApiQueryLogEvents.php +++ b/includes/api/ApiQueryLogEvents.php @@ -322,7 +322,7 @@ class ApiQueryLogEvents extends ApiQueryBase { $vals2['flags'] = isset( $params[$flagsKey] ) ? $params[$flagsKey] : ''; // Indefinite blocks have no expiry time - if ( SpecialBlock::parseExpiryInput( $params[$durationKey] ) !== wfGetDB( DB_SLAVE )->getInfinity() ) { + if ( SpecialBlock::parseExpiryInput( $params[$durationKey] ) !== 'infinity' ) { $vals2['expiry'] = wfTimestamp( TS_ISO_8601, strtotime( $params[$durationKey], wfTimestamp( TS_UNIX, $ts ) ) ); } diff --git a/includes/api/ApiQueryUserInfo.php b/includes/api/ApiQueryUserInfo.php index 1e3a4320aa..820a360df8 100644 --- a/includes/api/ApiQueryUserInfo.php +++ b/includes/api/ApiQueryUserInfo.php @@ -52,6 +52,8 @@ class ApiQueryUserInfo extends ApiQueryBase { } protected function getCurrentUserInfo() { + global $wgContLang; + $user = $this->getUser(); $result = $this->getResult(); $vals = array(); @@ -70,9 +72,9 @@ class ApiQueryUserInfo extends ApiQueryBase { $vals['blockedbyid'] = $block->getBy(); $vals['blockreason'] = $user->blockedFor(); $vals['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $block->mTimestamp ); - $vals['blockexpiry'] = $block->getExpiry() === 'infinity' - ? 'infinite' - : wfTimestamp( TS_ISO_8601, $block->getExpiry() ); + $vals['blockexpiry'] = $wgContLang->formatExpiry( + $block->getExpiry(), TS_ISO_8601, 'infinite' + ); } } diff --git a/includes/db/Database.php b/includes/db/Database.php index b3c81f9bf6..93122d4261 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -4330,7 +4330,7 @@ abstract class DatabaseBase implements IDatabase { * @return string */ public function decodeExpiry( $expiry, $format = TS_MW ) { - return ( $expiry == '' || $expiry == $this->getInfinity() ) + return ( $expiry == '' || $expiry == 'infinity' || $expiry == $this->getInfinity() ) ? 'infinity' : wfTimestamp( $format, $expiry ); } diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index b1e4f2d182..f26b966777 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -2369,8 +2369,8 @@ class WikiPage implements Page, IDBAccessObject { $dbw = wfGetDB( DB_MASTER ); foreach ( $restrictionTypes as $action ) { - if ( !isset( $expiry[$action] ) ) { - $expiry[$action] = $dbw->getInfinity(); + if ( !isset( $expiry[$action] ) || $expiry[$action] === $dbw->getInfinity() ) { + $expiry[$action] = 'infinity'; } if ( !isset( $limit[$action] ) ) { $limit[$action] = ''; @@ -2610,10 +2610,8 @@ class WikiPage implements Page, IDBAccessObject { */ protected function formatExpiry( $expiry ) { global $wgContLang; - $dbr = wfGetDB( DB_SLAVE ); - $encodedExpiry = $dbr->encodeExpiry( $expiry ); - if ( $encodedExpiry != 'infinity' ) { + if ( $expiry != 'infinity' ) { return wfMessage( 'protect-expiring', $wgContLang->timeanddate( $expiry, false, false ), diff --git a/includes/specials/SpecialBlock.php b/includes/specials/SpecialBlock.php index c237401345..3f13510de2 100644 --- a/includes/specials/SpecialBlock.php +++ b/includes/specials/SpecialBlock.php @@ -848,16 +848,11 @@ class SpecialBlock extends FormSpecialPage { * Convert a submitted expiry time, which may be relative ("2 weeks", etc) or absolute * ("24 May 2034", etc), into an absolute timestamp we can put into the database. * @param string $expiry Whatever was typed into the form - * @return string Timestamp or "infinity" string for the DB implementation + * @return string Timestamp or 'infinity' */ public static function parseExpiryInput( $expiry ) { - static $infinity; - if ( $infinity == null ) { - $infinity = wfGetDB( DB_SLAVE )->getInfinity(); - } - if ( wfIsInfinity( $expiry ) ) { - $expiry = $infinity; + $expiry = 'infinity'; } else { $expiry = strtotime( $expiry ); diff --git a/includes/specials/SpecialProtectedtitles.php b/includes/specials/SpecialProtectedtitles.php index dd9198cbc0..0e1ce53ed4 100644 --- a/includes/specials/SpecialProtectedtitles.php +++ b/includes/specials/SpecialProtectedtitles.php @@ -68,12 +68,6 @@ class SpecialProtectedtitles extends SpecialPage { */ function formatRow( $row ) { - static $infinity = null; - - if ( is_null( $infinity ) ) { - $infinity = wfGetDB( DB_SLAVE )->getInfinity(); - } - $title = Title::makeTitleSafe( $row->pt_namespace, $row->pt_title ); if ( !$title ) { @@ -100,9 +94,9 @@ class SpecialProtectedtitles extends SpecialPage { $lang = $this->getLanguage(); $expiry = strlen( $row->pt_expiry ) ? $lang->formatExpiry( $row->pt_expiry, TS_MW ) : - $infinity; + 'infinity'; - if ( $expiry != $infinity ) { + if ( $expiry !== 'infinity' ) { $user = $this->getUser(); $description_items[] = $this->msg( 'protect-expiring-local', diff --git a/languages/Language.php b/languages/Language.php index 3e47453197..7daf628e16 100644 --- a/languages/Language.php +++ b/languages/Language.php @@ -4483,21 +4483,20 @@ class Language { /** * Decode an expiry (block, protection, etc) which has come from the DB * - * @todo FIXME: why are we returnings DBMS-dependent strings??? - * * @param string $expiry Database expiry String * @param bool|int $format True to process using language functions, or TS_ constant * to return the expiry in a given timestamp + * @param string $inifinity If $format is not true, use this string for infinite expiry * @return string * @since 1.18 */ - public function formatExpiry( $expiry, $format = true ) { - static $infinity; - if ( $infinity === null ) { - $infinity = wfGetDB( DB_SLAVE )->getInfinity(); + public function formatExpiry( $expiry, $format = true, $infinity = 'infinity' ) { + static $dbInfinity; + if ( $dbInfinity === null ) { + $dbInfinity = wfGetDB( DB_SLAVE )->getInfinity(); } - if ( $expiry == '' || $expiry == $infinity ) { + if ( $expiry == '' || $expiry === 'infinity' || $expiry == $dbInfinity ) { return $format === true ? $this->getMessageFromDB( 'infiniteblock' ) : $infinity; diff --git a/tests/phpunit/includes/TitlePermissionTest.php b/tests/phpunit/includes/TitlePermissionTest.php index 022c7d537a..fd10ae73e8 100644 --- a/tests/phpunit/includes/TitlePermissionTest.php +++ b/tests/phpunit/includes/TitlePermissionTest.php @@ -664,7 +664,7 @@ class TitlePermissionTest extends MediaWikiLangTestCase { $this->setTitle( NS_MAIN, "test page" ); $this->title->mTitleProtection['permission'] = ''; $this->title->mTitleProtection['user'] = $this->user->getID(); - $this->title->mTitleProtection['expiry'] = wfGetDB( DB_SLAVE )->getInfinity(); + $this->title->mTitleProtection['expiry'] = 'infinity'; $this->title->mTitleProtection['reason'] = 'test'; $this->title->mCascadeRestriction = false;