Clean up handling of 'infinity'
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 12 Mar 2015 16:37:04 +0000 (12:37 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Fri, 13 Mar 2015 15:19:53 +0000 (11:19 -0400)
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

13 files changed:
includes/Block.php
includes/ProtectionForm.php
includes/Title.php
includes/api/ApiBlock.php
includes/api/ApiProtect.php
includes/api/ApiQueryLogEvents.php
includes/api/ApiQueryUserInfo.php
includes/db/Database.php
includes/page/WikiPage.php
includes/specials/SpecialBlock.php
includes/specials/SpecialProtectedtitles.php
languages/Language.php
tests/phpunit/includes/TitlePermissionTest.php

index 4698f45..184e308 100644 (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 );
index 1219da5..c546de7 100644 (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 );
 
index 36237ed..ddc941f 100644 (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;
                        }
index f03cef2..72aee32 100644 (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
index 4736cfb..675aa58 100644 (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' ),
                        );
                }
 
index adf96fd..1e32162 100644 (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 ) ) );
                                }
index 1e3a432..820a360 100644 (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'
+                               );
                        }
                }
 
index b3c81f9..93122d4 100644 (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 );
        }
index b1e4f2d..f26b966 100644 (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 ),
index c237401..3f13510 100644 (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 );
 
index dd9198c..0e1ce53 100644 (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',
index 3e47453..7daf628 100644 (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;
index 022c7d5..fd10ae7 100644 (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;