Don't construct SpecialPages twice

If the special page object was already created for the request, there's
no need to create the object again. Save the created result (object, null)
in a global static array and return the value if the realName was already
created.

Bug: T123995
Change-Id: I70bf0e93e45f4b0597deaef717f5eb87c66f0a71

diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php
index 8ce480e..725c4fc 100644 (file)
--- a/includes/specialpage/SpecialPageFactory.php
+++ b/includes/specialpage/SpecialPageFactory.php
@@ -182,6 +182,7 @@ class SpecialPageFactory {
 
        private static $list;
        private static $aliases;
+       private static $pageObjectCache = [];
 
        /**
         * Reset the internal list of special pages. Useful when changing $wgSpecialPages after
@@ -190,6 +191,7 @@ class SpecialPageFactory {
        public static function resetList() {
                self::$list = null;
                self::$aliases = null;
+               self::$pageObjectCache = [];
        }
 
        /**
@@ -373,6 +375,10 @@ class SpecialPageFactory {
        public static function getPage( $name ) {
                list( $realName, /*...*/ ) = self::resolveAlias( $name );
 
+               if ( isset( self::$pageObjectCache[$realName] ) ) {
+                       return self::$pageObjectCache[$realName];
+               }
+
                $specialPageList = self::getPageList();
 
                if ( isset( $specialPageList[$realName] ) ) {
@@ -400,6 +406,7 @@ class SpecialPageFactory {
                                $page = null;
                        }
 
+                       self::$pageObjectCache[$realName] = $page;
                        if ( $page instanceof SpecialPage ) {
                                return $page;
                        } else {

diff --git a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
index 534cf9b..3d407fb 100644 (file)
--- a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
+++ b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
@@ -55,19 +55,17 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
                $specialPageTestHelper = new SpecialPageTestHelper();
 
                return [
-                       'class name' => [ 'SpecialAllPages', false ],
+                       'class name' => [ 'SpecialAllPages' ],
                        'closure' => [ function () {
                                return new SpecialAllPages();
-                       }, false ],
-                       'function' => [ [ $this, 'newSpecialAllPages' ], false ],
-                       'callback string' => [ 'SpecialPageTestHelper::newSpecialAllPages', false ],
+                       } ],
+                       'function' => [ [ $this, 'newSpecialAllPages' ] ],
+                       'callback string' => [ 'SpecialPageTestHelper::newSpecialAllPages' ],
                        'callback with object' => [
-                               [ $specialPageTestHelper, 'newSpecialAllPages' ],
-                               false
+                               [ $specialPageTestHelper, 'newSpecialAllPages' ]
                        ],
                        'callback array' => [
-                               [ 'SpecialPageTestHelper', 'newSpecialAllPages' ],
-                               false
+                               [ 'SpecialPageTestHelper', 'newSpecialAllPages' ]
                        ]
                ];
        }
@@ -76,7 +74,7 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
         * @covers SpecialPageFactory::getPage
         * @dataProvider specialPageProvider
         */
-       public function testGetPage( $spec, $shouldReuseInstance ) {
+       public function testGetPage( $spec ) {
                $this->mergeMwGlobalArrayValue( 'wgSpecialPages', [ 'testdummy' => $spec ] );
                SpecialPageFactory::resetList();
 
@@ -84,7 +82,7 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
                $this->assertInstanceOf( 'SpecialPage', $page );
 
                $page2 = SpecialPageFactory::getPage( 'testdummy' );
-               $this->assertEquals( $shouldReuseInstance, $page2 === $page, "Should re-use instance:" );
+               $this->assertEquals( true, $page2 === $page, "Should re-use instance:" );
        }
 
        /**