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
This commit is contained in:
Brad Jorsch 2015-03-12 12:37:04 -04:00
parent 555a0b8645
commit ac6f81d9ad
13 changed files with 39 additions and 64 deletions

View file

@ -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 );

View file

@ -157,7 +157,7 @@ class ProtectionForm {
$value = $this->mExpirySelection[$action];
}
if ( wfIsInfinity( $value ) ) {
$time = wfGetDB( DB_SLAVE )->getInfinity();
$time = 'infinity';
} else {
$unix = strtotime( $value );

View file

@ -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;
}

View file

@ -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

View file

@ -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' ),
);
}

View file

@ -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 ) ) );
}

View file

@ -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'
);
}
}

View file

@ -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 );
}

View file

@ -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 ),

View file

@ -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 );

View file

@ -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',

View file

@ -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;

View file

@ -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;