Fix SpecialPageFactory list handling
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 1 Oct 2014 03:20:04 +0000 (23:20 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Wed, 1 Oct 2014 17:58:08 +0000 (13:58 -0400)
* Since Ic917c7d8/I7420b9ec, SpecialPageFactory doesn't properly cache
  SpecialPageFactory::$list.
* SpecialPageFactory::resetList() has never really worked right, it
  loses all the core special pages.
* SpecialPageFactory::getAliasListObject() could be called recursively
  from a SpecialPage_initList hook. There's no particular reason to fail
  it, just allow the original call to override the result of the
  recursive one and hope it works.

Change-Id: I7adb346eab00d5849d087ddff75230a35be3ee8f

includes/specialpage/SpecialPageFactory.php
tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php

index e0a0f46..9a6b787 100644 (file)
@@ -47,7 +47,7 @@ class SpecialPageFactory {
        /**
         * List of special page names to the subclass of SpecialPage which handles them.
         */
-       private static $list = array(
+       private static $coreList = array(
                // Maintenance Reports
                'BrokenRedirects' => 'BrokenRedirectsPage',
                'Deadendpages' => 'DeadendPagesPage',
@@ -174,6 +174,7 @@ class SpecialPageFactory {
                'Userlogout' => 'SpecialUserlogout',
        );
 
+       private static $list;
        private static $aliases;
 
        /**
@@ -217,9 +218,11 @@ class SpecialPageFactory {
                global $wgEnableEmail, $wgEnableJavaScriptTest;
                global $wgPageLanguageUseDB;
 
-               if ( !is_object( self::$list ) ) {
+               if ( !is_array( self::$list ) ) {
                        wfProfileIn( __METHOD__ );
 
+                       self::$list = self::$coreList;
+
                        if ( !$wgDisableCounters ) {
                                self::$list['Popularpages'] = 'PopularPagesPage';
                        }
@@ -271,12 +274,13 @@ class SpecialPageFactory {
                if ( !is_object( self::$aliases ) ) {
                        global $wgContLang;
                        $aliases = $wgContLang->getSpecialPageAliases();
+                       $pageList = self::getPageList();
 
                        self::$aliases = array();
                        $keepAlias = array();
 
                        // Force every canonical name to be an alias for itself.
-                       foreach ( self::getPageList() as $name => $stuff ) {
+                       foreach ( $pageList as $name => $stuff ) {
                                $caseFoldedAlias = $wgContLang->caseFold( $name );
                                self::$aliases[$caseFoldedAlias] = $name;
                                $keepAlias[$caseFoldedAlias] = 'canonical';
index 42602dc..cb12273 100644 (file)
@@ -28,6 +28,25 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
                SpecialPageFactory::resetList();
        }
 
+       public function testResetList() {
+               SpecialPageFactory::resetList();
+               $this->assertContains( 'Specialpages', SpecialPageFactory::getNames() );
+       }
+
+       public function testHookNotCalledTwice() {
+               $count = 0;
+               $this->mergeMwGlobalArrayValue( 'wgHooks', array(
+                       'SpecialPage_initList' => array(
+                               function () use ( &$count ) {
+                                       $count++;
+                               }
+               ) ) );
+               SpecialPageFactory::resetList();
+               SpecialPageFactory::getNames();
+               SpecialPageFactory::getNames();
+               $this->assertEquals( 1, $count );
+       }
+
        public function newSpecialAllPages() {
                return new SpecialAllPages();
        }
@@ -235,4 +254,19 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
                );
        }
 
+       public function testGetAliasListRecursion() {
+               $called = false;
+               $this->mergeMwGlobalArrayValue( 'wgHooks', array(
+                       'SpecialPage_initList' => array(
+                               function () use ( &$called ) {
+                                       SpecialPageFactory::getLocalNameFor( 'Specialpages' );
+                                       $called = true;
+                               }
+                       ),
+               ) );
+               SpecialPageFactory::resetList();
+               SpecialPageFactory::getLocalNameFor( 'Specialpages' );
+               $this->assertTrue( $called, 'Recursive call succeeded' );
+       }
+
 }