Don't pass Config object to SpecialPageFactory
authorAryeh Gregor <ayg@aryeh.name>
Tue, 9 Apr 2019 16:34:23 +0000 (19:34 +0300)
committerDaniel Kinzler <dkinzler@wikimedia.org>
Wed, 10 Apr 2019 13:45:01 +0000 (13:45 +0000)
That made SpecialPageFactory potentially dependent on every
configuration option, when it only uses a few.

In the convention I introduce here, the service class does not in
principle care that its options come from configuration variables. It
just declares a list of names of options it can receive, which happen to
match the names of configuration options, and ServiceWiring just copies
them over. In the event a service wants to receive other options that
are not configuration variables, ServiceWiring can set them
appropriately from other sources. In practice, most options will
probably come from configuration variables, so using the same names and
passing them directly makes the code shorter and easier to understand.

Change-Id: I3e57513826dc5130f468486470f29eeee659d697

includes/ServiceWiring.php
includes/specialpage/SpecialPageFactory.php

index 722bac1..a82feaa 100644 (file)
@@ -571,8 +571,13 @@ return [
        },
 
        'SpecialPageFactory' => function ( MediaWikiServices $services ) : SpecialPageFactory {
        },
 
        'SpecialPageFactory' => function ( MediaWikiServices $services ) : SpecialPageFactory {
+               $config = $services->getMainConfig();
+               $options = [];
+               foreach ( SpecialPageFactory::$constructorOptions as $key ) {
+                       $options[$key] = $config->get( $key );
+               }
                return new SpecialPageFactory(
                return new SpecialPageFactory(
-                       $services->getMainConfig(),
+                       $options,
                        $services->getContentLanguage()
                );
        },
                        $services->getContentLanguage()
                );
        },
index 58212dd..a3b7296 100644 (file)
@@ -34,6 +34,7 @@ use RequestContext;
 use SpecialPage;
 use Title;
 use User;
 use SpecialPage;
 use Title;
 use User;
+use Wikimedia\Assert\Assert;
 
 /**
  * Factory for handling the special page list and generating SpecialPage objects.
 
 /**
  * Factory for handling the special page list and generating SpecialPage objects.
@@ -215,17 +216,36 @@ class SpecialPageFactory {
        private $aliases;
 
        /** @var Config */
        private $aliases;
 
        /** @var Config */
-       private $config;
+       private $options;
 
        /** @var Language */
        private $contLang;
 
        /**
 
        /** @var Language */
        private $contLang;
 
        /**
-        * @param Config $config
+        * TODO Make this a const when HHVM support is dropped (T192166)
+        *
+        * @var array
+        * @since 1.33
+        * */
+       public static $constructorOptions = [
+               'ContentHandlerUseDB',
+               'DisableInternalSearch',
+               'EmailAuthentication',
+               'EnableEmail',
+               'EnableJavaScriptTest',
+               'PageLanguageUseDB',
+               'SpecialPages',
+       ];
+
+       /**
+        * @param array $options
         * @param Language $contLang
         */
         * @param Language $contLang
         */
-       public function __construct( Config $config, Language $contLang ) {
-               $this->config = $config;
+       public function __construct( array $options, Language $contLang ) {
+               Assert::parameter( count( $options ) === count( self::$constructorOptions ) &&
+                       !array_diff( self::$constructorOptions, array_keys( $options ) ),
+                       '$options', 'Wrong set of options present' );
+               $this->options = $options;
                $this->contLang = $contLang;
        }
 
                $this->contLang = $contLang;
        }
 
@@ -248,32 +268,32 @@ class SpecialPageFactory {
                if ( !is_array( $this->list ) ) {
                        $this->list = self::$coreList;
 
                if ( !is_array( $this->list ) ) {
                        $this->list = self::$coreList;
 
-                       if ( !$this->config->get( 'DisableInternalSearch' ) ) {
+                       if ( !$this->options['DisableInternalSearch'] ) {
                                $this->list['Search'] = \SpecialSearch::class;
                        }
 
                                $this->list['Search'] = \SpecialSearch::class;
                        }
 
-                       if ( $this->config->get( 'EmailAuthentication' ) ) {
+                       if ( $this->options['EmailAuthentication'] ) {
                                $this->list['Confirmemail'] = \EmailConfirmation::class;
                                $this->list['Invalidateemail'] = \EmailInvalidation::class;
                        }
 
                                $this->list['Confirmemail'] = \EmailConfirmation::class;
                                $this->list['Invalidateemail'] = \EmailInvalidation::class;
                        }
 
-                       if ( $this->config->get( 'EnableEmail' ) ) {
+                       if ( $this->options['EnableEmail'] ) {
                                $this->list['ChangeEmail'] = \SpecialChangeEmail::class;
                        }
 
                                $this->list['ChangeEmail'] = \SpecialChangeEmail::class;
                        }
 
-                       if ( $this->config->get( 'EnableJavaScriptTest' ) ) {
+                       if ( $this->options['EnableJavaScriptTest'] ) {
                                $this->list['JavaScriptTest'] = \SpecialJavaScriptTest::class;
                        }
 
                                $this->list['JavaScriptTest'] = \SpecialJavaScriptTest::class;
                        }
 
-                       if ( $this->config->get( 'PageLanguageUseDB' ) ) {
+                       if ( $this->options['PageLanguageUseDB'] ) {
                                $this->list['PageLanguage'] = \SpecialPageLanguage::class;
                        }
                                $this->list['PageLanguage'] = \SpecialPageLanguage::class;
                        }
-                       if ( $this->config->get( 'ContentHandlerUseDB' ) ) {
+                       if ( $this->options['ContentHandlerUseDB'] ) {
                                $this->list['ChangeContentModel'] = \SpecialChangeContentModel::class;
                        }
 
                        // Add extension special pages
                                $this->list['ChangeContentModel'] = \SpecialChangeContentModel::class;
                        }
 
                        // Add extension special pages
-                       $this->list = array_merge( $this->list, $this->config->get( 'SpecialPages' ) );
+                       $this->list = array_merge( $this->list, $this->options['SpecialPages'] );
 
                        // This hook can be used to disable unwanted core special pages
                        // or conditionally register special pages.
 
                        // This hook can be used to disable unwanted core special pages
                        // or conditionally register special pages.