Merge "SpecialJavaScriptTest: Use Config instead of globals"
authorKunal Mehta <legoktm@gmail.com>
Mon, 4 Aug 2014 00:39:38 +0000 (00:39 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 4 Aug 2014 00:39:38 +0000 (00:39 +0000)
includes/config/ConfigFactory.php
includes/specials/SpecialChangeEmail.php
includes/specials/SpecialLinkSearch.php
includes/specials/SpecialListfiles.php
includes/specials/SpecialListusers.php
includes/specials/SpecialLockdb.php
includes/specials/SpecialLog.php
includes/specials/SpecialMIMEsearch.php
includes/specials/SpecialMergeHistory.php
tests/phpunit/includes/config/ConfigFactoryTest.php

index ff2f403..312d461 100644 (file)
@@ -41,16 +41,33 @@ class ConfigFactory {
         */
        protected $configs = array();
 
+       /**
+        * @var ConfigFactory
+        */
+       private static $self;
+
        public static function getDefaultInstance() {
-               static $self = null;
-               if ( !$self ) {
-                       $self = new self;
+               if ( !self::$self ) {
+                       self::$self = new self;
                        global $wgConfigRegistry;
                        foreach ( $wgConfigRegistry as $name => $callback ) {
-                               $self->register( $name, $callback );
+                               self::$self->register( $name, $callback );
                        }
                }
-               return $self;
+               return self::$self;
+       }
+
+       /**
+        * Destroy the default instance
+        * Should only be called inside unit tests
+        * @throws MWException
+        */
+       public static function destroyDefaultInstance() {
+               if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
+                       throw new MWException( __METHOD__ . ' was called outside of unit tests' );
+               }
+
+               self::$self = null;
        }
 
        /**
index c57e33b..7fc4a17 100644 (file)
@@ -136,7 +136,6 @@ class SpecialChangeEmail extends UnlistedSpecialPage {
        }
 
        protected function showForm() {
-               global $wgRequirePasswordforEmailChange;
                $user = $this->getUser();
 
                $oldEmailText = $user->getEmail()
@@ -160,7 +159,7 @@ class SpecialChangeEmail extends UnlistedSpecialPage {
                        array( 'wpOldEmail', 'changeemail-oldemail', 'text', $oldEmailText ),
                        array( 'wpNewEmail', 'changeemail-newemail', 'email', $this->mNewEmail ),
                );
-               if ( $wgRequirePasswordforEmailChange ) {
+               if ( $this->getConfig()->get( 'RequirePasswordforEmailChange' ) ) {
                        $items[] = array( 'wpPassword', 'changeemail-password', 'password', $this->mPassword );
                }
 
@@ -221,7 +220,7 @@ class SpecialChangeEmail extends UnlistedSpecialPage {
         * @return bool|string True or string on success, false on failure
         */
        protected function attemptChange( User $user, $pass, $newaddr ) {
-               global $wgAuth, $wgPasswordAttemptThrottle;
+               global $wgAuth;
 
                if ( $newaddr != '' && !Sanitizer::validateEmail( $newaddr ) ) {
                        $this->error( 'invalidemailaddress' );
@@ -232,16 +231,16 @@ class SpecialChangeEmail extends UnlistedSpecialPage {
                $throttleCount = LoginForm::incLoginThrottle( $user->getName() );
                if ( $throttleCount === true ) {
                        $lang = $this->getLanguage();
+                       $throttleInfo = $this->getConfig()->get( 'PasswordAttemptThrottle' );
                        $this->error( array(
                                'changeemail-throttled',
-                               $lang->formatDuration( $wgPasswordAttemptThrottle['seconds'] )
+                               $lang->formatDuration( $throttleInfo['seconds'] )
                        ) );
 
                        return false;
                }
 
-               global $wgRequirePasswordforEmailChange;
-               if ( $wgRequirePasswordforEmailChange
+               if ( $this->getConfig()->get( 'RequirePasswordforEmailChange' )
                        && !$user->checkTemporaryPassword( $pass )
                        && !$user->checkPassword( $pass )
                ) {
index 10d1957..371469b 100644 (file)
@@ -78,8 +78,6 @@ class LinkSearchPage extends QueryPage {
        }
 
        function execute( $par ) {
-               global $wgUrlProtocols, $wgMiserMode, $wgScript;
-
                $this->initServices();
 
                $this->setHeaders();
@@ -93,7 +91,7 @@ class LinkSearchPage extends QueryPage {
                $namespace = $request->getIntorNull( 'namespace', null );
 
                $protocols_list = array();
-               foreach ( $wgUrlProtocols as $prot ) {
+               foreach ( $this->getConfig()->get( 'UrlProtocols' ) as $prot ) {
                        if ( $prot !== '//' ) {
                                $protocols_list[] = $prot;
                        }
@@ -122,7 +120,7 @@ class LinkSearchPage extends QueryPage {
                );
                $s = Html::openElement(
                        'form',
-                       array( 'id' => 'mw-linksearch-form', 'method' => 'get', 'action' => $wgScript )
+                       array( 'id' => 'mw-linksearch-form', 'method' => 'get', 'action' => wfScript() )
                ) . "\n" .
                        Html::hidden( 'title', $this->getPageTitle()->getPrefixedDBkey() ) . "\n" .
                        Html::openElement( 'fieldset' ) . "\n" .
@@ -139,7 +137,7 @@ class LinkSearchPage extends QueryPage {
                                )
                        ) . "\n";
 
-               if ( !$wgMiserMode ) {
+               if ( !$this->getConfig()->get( 'MiserMode' ) ) {
                        $s .= Html::namespaceSelector(
                                array(
                                        'selected' => $namespace,
@@ -210,10 +208,9 @@ class LinkSearchPage extends QueryPage {
        }
 
        function linkParameters() {
-               global $wgMiserMode;
                $params = array();
                $params['target'] = $this->mProt . $this->mQuery;
-               if ( $this->mNs !== null && !$wgMiserMode ) {
+               if ( $this->mNs !== null && !$this->getConfig()->get( 'MiserMode' ) ) {
                        $params['namespace'] = $this->mNs;
                }
 
@@ -221,7 +218,6 @@ class LinkSearchPage extends QueryPage {
        }
 
        function getQueryInfo() {
-               global $wgMiserMode;
                $dbr = wfGetDB( DB_SLAVE );
                // strip everything past first wildcard, so that
                // index-based-only lookup would be done
@@ -248,7 +244,7 @@ class LinkSearchPage extends QueryPage {
                        'options' => array( 'USE INDEX' => $clause )
                );
 
-               if ( $this->mNs !== null && !$wgMiserMode ) {
+               if ( $this->mNs !== null && !$this->getConfig()->get( 'MiserMode' ) ) {
                        $retval['conds']['page_namespace'] = $this->mNs;
                }
 
index 2ce45ac..c62c1de 100644 (file)
@@ -86,8 +86,6 @@ class ImageListPager extends TablePager {
        function __construct( IContextSource $context, $userName = null, $search = '',
                $including = false, $showAll = false
        ) {
-               global $wgMiserMode;
-
                $this->mIncluding = $including;
                $this->mShowAll = $showAll;
 
@@ -98,7 +96,7 @@ class ImageListPager extends TablePager {
                        }
                }
 
-               if ( $search !== '' && !$wgMiserMode ) {
+               if ( $search !== '' && !$this->getConfig()->get( 'MiserMode' ) ) {
                        $this->mSearch = $search;
                        $nt = Title::newFromURL( $this->mSearch );
 
@@ -164,7 +162,6 @@ class ImageListPager extends TablePager {
         */
        function getFieldNames() {
                if ( !$this->mFieldNames ) {
-                       global $wgMiserMode;
                        $this->mFieldNames = array(
                                'img_timestamp' => $this->msg( 'listfiles_date' )->text(),
                                'img_name' => $this->msg( 'listfiles_name' )->text(),
@@ -178,7 +175,7 @@ class ImageListPager extends TablePager {
                        // img_description down here, in order so that its still after the username field.
                        $this->mFieldNames['img_description'] = $this->msg( 'listfiles_description' )->text();
 
-                       if ( !$wgMiserMode && !$this->mShowAll ) {
+                       if ( !$this->getConfig()->get( 'MiserMode' ) && !$this->mShowAll ) {
                                $this->mFieldNames['count'] = $this->msg( 'listfiles_count' )->text();
                        }
                        if ( $this->mShowAll ) {
@@ -190,7 +187,6 @@ class ImageListPager extends TablePager {
        }
 
        function isFieldSortable( $field ) {
-               global $wgMiserMode;
                if ( $this->mIncluding ) {
                        return false;
                }
@@ -202,14 +198,14 @@ class ImageListPager extends TablePager {
                 * In particular that means we cannot sort by timestamp when not filtering
                 * by user and including old images in the results. Which is sad.
                 */
-               if ( $wgMiserMode && !is_null( $this->mUserName ) ) {
+               if ( $this->getConfig()->get( 'MiserMode' ) && !is_null( $this->mUserName ) ) {
                        // If we're sorting by user, the index only supports sorting by time.
                        if ( $field === 'img_timestamp' ) {
                                return true;
                        } else {
                                return false;
                        }
-               } elseif ( $wgMiserMode && $this->mShowAll /* && mUserName === null */ ) {
+               } elseif ( $this->getConfig()->get( 'MiserMode' ) && $this->mShowAll /* && mUserName === null */ ) {
                        // no oi_timestamp index, so only alphabetical sorting in this case.
                        if ( $field === 'img_name' ) {
                                return true;
@@ -392,8 +388,7 @@ class ImageListPager extends TablePager {
        }
 
        function getDefaultSort() {
-               global $wgMiserMode;
-               if ( $this->mShowAll && $wgMiserMode && is_null( $this->mUserName ) ) {
+               if ( $this->mShowAll && $this->getConfig()->get( 'MiserMode' ) && is_null( $this->mUserName ) ) {
                        // Unfortunately no index on oi_timestamp.
                        return 'img_name';
                } else {
@@ -504,10 +499,9 @@ class ImageListPager extends TablePager {
        }
 
        function getForm() {
-               global $wgScript, $wgMiserMode;
                $inputForm = array();
                $inputForm['table_pager_limit_label'] = $this->getLimitSelect( array( 'tabindex' => 1 ) );
-               if ( !$wgMiserMode ) {
+               if ( !$this->getConfig()->get( 'MiserMode' ) ) {
                        $inputForm['listfiles_search_for'] = Html::input(
                                'ilsearch',
                                $this->mSearch,
@@ -533,7 +527,7 @@ class ImageListPager extends TablePager {
                ) );
 
                return Html::openElement( 'form',
-                       array( 'method' => 'get', 'action' => $wgScript, 'id' => 'mw-listfiles-form' )
+                       array( 'method' => 'get', 'action' => wfScript(), 'id' => 'mw-listfiles-form' )
                ) .
                        Xml::fieldset( $this->msg( 'listfiles' )->text() ) .
                        Html::hidden( 'title', $this->getTitle()->getPrefixedText() ) .
index feeafbc..993285f 100644 (file)
@@ -191,8 +191,7 @@ class UsersPager extends AlphabeticPager {
                }
 
                $edits = '';
-               global $wgEdititis;
-               if ( !$this->including && $wgEdititis ) {
+               if ( !$this->including && $this->getConfig()->get( 'Edititis' ) ) {
                        // @todo fixme i18n issue: Hardcoded square brackets.
                        $edits = ' [' .
                                $this->msg( 'usereditcount' )->numParams( $row->edits )->escaped() .
@@ -232,14 +231,12 @@ class UsersPager extends AlphabeticPager {
         * @return string
         */
        function getPageHeader() {
-               global $wgScript;
-
                list( $self ) = explode( '/', $this->getTitle()->getPrefixedDBkey() );
 
                # Form tag
                $out = Xml::openElement(
                        'form',
-                       array( 'method' => 'get', 'action' => $wgScript, 'id' => 'mw-listusers-form' )
+                       array( 'method' => 'get', 'action' => wfScript(), 'id' => 'mw-listusers-form' )
                ) .
                        Xml::fieldset( $this->msg( 'listusers' )->text() ) .
                        Html::hidden( 'title', $self );
index 3382405..1c1f125 100644 (file)
@@ -38,11 +38,9 @@ class SpecialLockdb extends FormSpecialPage {
        }
 
        public function checkExecutePermissions( User $user ) {
-               global $wgReadOnlyFile;
-
                parent::checkExecutePermissions( $user );
                # If the lock file isn't writable, we can do sweet bugger all
-               if ( !is_writable( dirname( $wgReadOnlyFile ) ) ) {
+               if ( !is_writable( dirname( $this->getConfig()->get( 'ReadOnlyFile' ) ) ) ) {
                        throw new ErrorPageError( 'lockdb', 'lockfilenotwritable' );
                }
        }
@@ -69,14 +67,14 @@ class SpecialLockdb extends FormSpecialPage {
        }
 
        public function onSubmit( array $data ) {
-               global $wgContLang, $wgReadOnlyFile;
+               global $wgContLang;
 
                if ( !$data['Confirm'] ) {
                        return Status::newFatal( 'locknoconfirm' );
                }
 
                wfSuppressWarnings();
-               $fp = fopen( $wgReadOnlyFile, 'w' );
+               $fp = fopen( $this->getConfig()->get( 'ReadOnlyFile' ), 'w' );
                wfRestoreWarnings();
 
                if ( false === $fp ) {
index 36abeb0..dc33801 100644 (file)
@@ -45,8 +45,6 @@ class SpecialLog extends SpecialPage {
        }
 
        public function execute( $par ) {
-               global $wgLogRestrictions;
-
                $this->setHeaders();
                $this->outputHeader();
 
@@ -77,13 +75,14 @@ class SpecialLog extends SpecialPage {
                // If the user doesn't have the right permission to view the specific
                // log type, throw a PermissionsError
                // If the log type is invalid, just show all public logs
+               $logRestrictions = $this->getConfig()->get( 'LogRestrictions' );
                $type = $opts->getValue( 'type' );
                if ( !LogPage::isLogType( $type ) ) {
                        $opts->setValue( 'type', '' );
-               } elseif ( isset( $wgLogRestrictions[$type] )
-                       && !$this->getUser()->isAllowed( $wgLogRestrictions[$type] )
+               } elseif ( isset( $logRestrictions[$type] )
+                       && !$this->getUser()->isAllowed( $logRestrictions[$type] )
                ) {
-                       throw new PermissionsError( $wgLogRestrictions[$type] );
+                       throw new PermissionsError( $logRestrictions[$type] );
                }
 
                # Handle type-specific inputs
@@ -123,21 +122,18 @@ class SpecialLog extends SpecialPage {
         * @return string[] Matching subpages
         */
        public function prefixSearchSubpages( $search, $limit = 10 ) {
-               global $wgLogTypes;
-               $subpages = $wgLogTypes;
+               $subpages = $this->getConfig()->get( 'LogTypes' );
                $subpages[] = 'all';
                sort( $subpages );
                return self::prefixSearchArray( $search, $limit, $subpages );
        }
 
        private function parseParams( FormOptions $opts, $par ) {
-               global $wgLogTypes;
-
                # Get parameters
                $parms = explode( '/', ( $par = ( $par !== null ) ? $par : '' ) );
                $symsForAll = array( '*', 'all' );
                if ( $parms[0] != '' &&
-                       ( in_array( $par, $wgLogTypes ) || in_array( $par, $symsForAll ) )
+                       ( in_array( $par, $this->getConfig()->get( 'LogTypes' ) ) || in_array( $par, $symsForAll ) )
                ) {
                        $opts->setValue( 'type', $par );
                } elseif ( count( $parms ) == 2 ) {
@@ -211,10 +207,9 @@ class SpecialLog extends SpecialPage {
                }
 
                # Show button to hide log entries
-               global $wgScript;
                $s = Html::openElement(
                        'form',
-                       array( 'action' => $wgScript, 'id' => 'mw-log-deleterevision-submit' )
+                       array( 'action' => wfScript(), 'id' => 'mw-log-deleterevision-submit' )
                ) . "\n";
                $s .= Html::hidden( 'title', SpecialPage::getTitleFor( 'Revisiondelete' ) ) . "\n";
                $s .= Html::hidden( 'target', SpecialPage::getTitleFor( 'Log' ) ) . "\n";
index 4d9e7da..3f1850d 100644 (file)
@@ -105,8 +105,6 @@ class MIMEsearchPage extends QueryPage {
        }
 
        function execute( $par ) {
-               global $wgScript;
-
                $mime = $par ? $par : $this->getRequest()->getText( 'mime' );
                $mime = trim( $mime );
 
@@ -115,7 +113,7 @@ class MIMEsearchPage extends QueryPage {
                $this->getOutput()->addHTML(
                        Xml::openElement(
                                'form',
-                               array( 'id' => 'specialmimesearch', 'method' => 'get', 'action' => $wgScript )
+                               array( 'id' => 'specialmimesearch', 'method' => 'get', 'action' => wfScript() )
                        ) .
                                Xml::openElement( 'fieldset' ) .
                                Html::hidden( 'title', $this->getPageTitle()->getPrefixedText() ) .
index e1700de..6efc12b 100644 (file)
@@ -159,14 +159,12 @@ class SpecialMergeHistory extends SpecialPage {
        }
 
        function showMergeForm() {
-               global $wgScript;
-
                $this->getOutput()->addWikiMsg( 'mergehistory-header' );
 
                $this->getOutput()->addHTML(
                        Xml::openElement( 'form', array(
                                'method' => 'get',
-                               'action' => $wgScript ) ) .
+                               'action' => wfScript() ) ) .
                                '<fieldset>' .
                                Xml::element( 'legend', array(),
                                        $this->msg( 'mergehistory-box' )->text() ) .
index 611d304..3902858 100644 (file)
@@ -2,6 +2,12 @@
 
 class ConfigFactoryTest extends MediaWikiTestCase {
 
+       public function tearDown() {
+               // Reset this since we mess with it a bit
+               ConfigFactory::destroyDefaultInstance();
+               parent::tearDown();
+       }
+
        /**
         * @covers ConfigFactory::register
         */
@@ -43,4 +49,22 @@ class ConfigFactoryTest extends MediaWikiTestCase {
                $this->setExpectedException( 'UnexpectedValueException' );
                $factory->makeConfig( 'unittest' );
        }
+
+       /**
+        * @covers ConfigFactory::getDefaultInstance
+        */
+       public function testGetDefaultInstance() {
+               // Set $wgConfigRegistry, and check the default
+               // instance read from it
+               $this->setMwGlobals( 'wgConfigRegistry', array(
+                       'conf1' => 'GlobalVarConfig::newInstance',
+                       'conf2' => 'GlobalVarConfig::newInstance',
+               ) );
+               ConfigFactory::destroyDefaultInstance();
+               $factory = ConfigFactory::getDefaultInstance();
+               $this->assertInstanceOf( 'Config', $factory->makeConfig( 'conf1' ) );
+               $this->assertInstanceOf( 'Config', $factory->makeConfig( 'conf2' ) );
+               $this->setExpectedException( 'ConfigException' );
+               $factory->makeConfig( 'conf3' );
+       }
 }