diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index d3aabfe0503..3ab2f009ac7 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -391,7 +391,9 @@ $wgAutoloadLocalClasses = array( # includes/config 'Config' => 'includes/config/Config.php', - 'GlobalConfig' => 'includes/config/GlobalConfig.php', + 'ConfigException' => 'includes/config/ConfigException.php', + 'ConfigFactory' => 'includes/config/ConfigFactory.php', + 'GlobalVarConfig' => 'includes/config/GlobalVarConfig.php', # includes/content 'AbstractContent' => 'includes/content/AbstractContent.php', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 87dc0255d36..4598952ed05 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -60,11 +60,14 @@ if ( !defined( 'MEDIAWIKI' ) ) { $wgConf = new SiteConfiguration; /** - * Class name to use for accessing Config. - * Currently only 'GlobalConfig' is available + * Registry of factory functions to create config objects: + * The 'main' key must be set, and the value should be a valid + * callable. * @since 1.23 */ -$wgConfigClass = 'GlobalConfig'; +$wgConfigRegistry = array( + 'main' => 'GlobalVarConfig::newInstance' +); /** * MediaWiki version number diff --git a/includes/config/Config.php b/includes/config/Config.php index 04afddad147..68e90b45824 100644 --- a/includes/config/Config.php +++ b/includes/config/Config.php @@ -21,37 +21,27 @@ */ /** - * Abstract class for get settings for + * Interface for configuration instances * * @since 1.23 */ -abstract class Config { - /** - * @param string $name configuration variable name without prefix - * @param string $prefix of the variable name - * @return mixed - */ - abstract public function get( $name, $prefix = 'wg' ); +interface Config { /** - * @param string $name configuration variable name without prefix - * @param mixed $value to set - * @param string $prefix of the variable name - * @return Status object indicating success or failure + * Get a configuration variable such as "Sitename" or "UploadMaintenance." + * + * @param string $name Name of configuration option + * @return mixed Value configured + * @throws ConfigException */ - abstract public function set( $name, $value, $prefix = 'wg' ); + public function get( $name ); /** - * @param string|null $type class name for Config object, - * uses $wgConfigClass if not provided - * @return Config + * Set a configuration variable such a "Sitename" to something like "My Wiki" + * + * @param string $name Name of configuration option + * @param mixed $value Value to set + * @throws ConfigException */ - public static function factory( $type = null ) { - if ( !$type ) { - global $wgConfigClass; - $type = $wgConfigClass; - } - - return new $type; - } + public function set( $name, $value ); } diff --git a/includes/config/GlobalConfig.php b/includes/config/ConfigException.php similarity index 69% rename from includes/config/GlobalConfig.php rename to includes/config/ConfigException.php index e16a9ee7285..368ca5a4651 100644 --- a/includes/config/GlobalConfig.php +++ b/includes/config/ConfigException.php @@ -21,25 +21,8 @@ */ /** - * Accesses configuration settings from $GLOBALS + * Exceptions for config failures * * @since 1.23 */ -class GlobalConfig extends Config { - - /** - * @see Config::get - */ - public function get( $name, $prefix = 'wg' ) { - return $GLOBALS[$prefix . $name]; - } - - /** - * @see Config::set - */ - public function set( $name, $value, $prefix = 'wg' ) { - $GLOBALS[$prefix . $name] = $value; - - return Status::newGood(); - } -} +class ConfigException extends MWException {} diff --git a/includes/config/ConfigFactory.php b/includes/config/ConfigFactory.php new file mode 100644 index 00000000000..b09316ba3a3 --- /dev/null +++ b/includes/config/ConfigFactory.php @@ -0,0 +1,94 @@ + callback + * @var array + */ + protected $factoryFunctions = array(); + + /** + * Config objects that have already been created + * name => Config object + * @var array + */ + protected $configs = array(); + + public static function getDefaultInstance() { + static $self = null; + if ( !$self ) { + $self = new self; + global $wgConfigRegistry; + foreach ( $wgConfigRegistry as $name => $callback ) { + $self->register( $name, $callback ); + } + } + return $self; + } + + /** + * Register a new config factory function + * Will override if it's already registered + * @param string $name + * @param callable $callback that takes this ConfigFactory as an argument + * @throws InvalidArgumentException if an invalid callback is provided + */ + public function register( $name, $callback ) { + if ( !is_callable( $callback ) ) { + throw new InvalidArgumentException( 'Invalid callback provided' ); + } + $this->factoryFunctions[$name] = $callback; + } + + /** + * Create a given Config using the registered callback for $name. + * If an object was already created, the same Config object is returned. + * @param string $name of the extension/component you want a Config object for + * 'main' is used for core + * @throws ConfigException if a factory function isn't registered for $name + * @throws UnexpectedValueException if the factory function returns a non-Config object + * @return Config + */ + public function makeConfig( $name ) { + if ( !isset( $this->configs[$name] ) ) { + if ( !isset( $this->factoryFunctions[$name] ) ) { + throw new ConfigException( "No registered builder available for $name." ); + } + $conf = call_user_func( $this->factoryFunctions[$name], $this ); + if ( $conf instanceof Config ) { + $this->configs[$name] = $conf; + } else { + throw new UnexpectedValueException( "The builder for $name returned a non-Config object." ); + } + } + + return $this->configs[$name]; + } +} diff --git a/includes/config/GlobalVarConfig.php b/includes/config/GlobalVarConfig.php new file mode 100644 index 00000000000..61a76b76179 --- /dev/null +++ b/includes/config/GlobalVarConfig.php @@ -0,0 +1,88 @@ +prefix = $prefix; + } + + /** + * @see Config::get + */ + public function get( $name ) { + return $this->getWithPrefix( $this->prefix, $name ); + } + + /** + * @see Config::set + */ + public function set( $name, $value ) { + $this->setWithPrefix( $this->prefix, $name, $value ); + } + + /** + * Get a variable with a given prefix, if not the defaults. + * + * @param string $prefix Prefix to use on the variable, if one. + * @param string $name Variable name without prefix + * @throws ConfigException + * @return mixed + */ + protected function getWithPrefix( $prefix, $name ) { + $var = $prefix . $name; + if ( !isset( $GLOBALS[ $var ] ) ) { + throw new ConfigException( __METHOD__ . ": undefined variable: '$var'" ); + } + return $GLOBALS[ $var ]; + } + + /** + * Get a variable with a given prefix, if not the defaults. + * + * @param string $prefix Prefix to use on the variable + * @param string $name Variable name without prefix + * @param mixed $value value to set + */ + protected function setWithPrefix( $prefix, $name, $value ) { + $GLOBALS[ $prefix . $name ] = $value; + } +} diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php index 6f27a7ae82f..e035b491946 100644 --- a/includes/context/RequestContext.php +++ b/includes/context/RequestContext.php @@ -84,7 +84,9 @@ class RequestContext implements IContextSource { */ public function getConfig() { if ( $this->config === null ) { - $this->config = Config::factory(); + // @todo In the future, we could move this to WebStart.php so + // the Config object is ready for when initialization happens + $this->config = ConfigFactory::getDefaultInstance()->makeConfig( 'main' ); } return $this->config; diff --git a/tests/phpunit/includes/config/ConfigFactoryTest.php b/tests/phpunit/includes/config/ConfigFactoryTest.php new file mode 100644 index 00000000000..0a6bf723f8f --- /dev/null +++ b/tests/phpunit/includes/config/ConfigFactoryTest.php @@ -0,0 +1,46 @@ +register( 'unittest', 'GlobalVarConfig::newInstance' ); + $this->assertTrue( True ); // No exception thrown + $this->setExpectedException( 'InvalidArgumentException' ); + $factory->register( 'invalid', 'Invalid callback' ); + } + + /** + * @covers ConfigFactory::makeConfig + */ + public function testMakeConfig() { + $factory = new ConfigFactory(); + $factory->register( 'unittest', 'GlobalVarConfig::newInstance' ); + $conf = $factory->makeConfig( 'unittest' ); + $this->assertInstanceOf( 'Config', $conf ); + } + + /** + * @covers ConfigFactory::makeConfig + */ + public function testMakeConfigWithNoBuilders() { + $factory = new ConfigFactory(); + $this->setExpectedException( 'ConfigException' ); + $factory->makeConfig( 'nobuilderregistered' ); + } + + /** + * @covers ConfigFactory::makeConfig + */ + public function testMakeConfigWithInvalidCallback() { + $factory = new ConfigFactory(); + $factory->register( 'unittest', function() { + return true; // Not a Config object + }); + $this->setExpectedException( 'UnexpectedValueException' ); + $factory->makeConfig( 'unittest' ); + } +} diff --git a/tests/phpunit/includes/config/GlobalConfigTest.php b/tests/phpunit/includes/config/GlobalConfigTest.php deleted file mode 100644 index b605a46eb24..00000000000 --- a/tests/phpunit/includes/config/GlobalConfigTest.php +++ /dev/null @@ -1,38 +0,0 @@ -config = new GlobalConfig; - } - - public static function provideGet() { - return array( - array( 'wgSitename', array( 'Sitename' ) ), - array( 'wgFoo', array( 'Foo' ) ), - array( 'efVariable', array( 'Variable', 'ef' ) ), - array( 'Foo', array( 'Foo', '' ) ), - ); - } - - /** - * @param string $name - * @param array $params - * @dataProvider provideGet - * @covers GlobalConfig::get - */ - public function testGet( $name, $params ) { - $rand = wfRandom(); - $old = isset( $GLOBALS[$name] ) ? $GLOBALS[$name] : null; - $GLOBALS[$name] = $rand; - $out = call_user_func_array( array( $this->config, 'get' ), $params ); - $this->assertEquals( $rand, $out ); - if ( $old ) { - $GLOBALS[$name] = $old; - } - } -} diff --git a/tests/phpunit/includes/config/GlobalVarConfigTest.php b/tests/phpunit/includes/config/GlobalVarConfigTest.php new file mode 100644 index 00000000000..7080b02335d --- /dev/null +++ b/tests/phpunit/includes/config/GlobalVarConfigTest.php @@ -0,0 +1,36 @@ + 'default1', + 'wgFoo' => 'default2', + 'efVariable' => 'default3', + 'BAR' => 'default4', + ); + + foreach ( $set as $var => $value ) { + $GLOBALS[$var] = $value; + } + + return array( + array( 'Something', 'wg', 'default1' ), + array( 'Foo', 'wg', 'default2' ), + array( 'Variable', 'ef', 'default3' ), + array( 'BAR', '', 'default4' ), + ); + } + + /** + * @param string $name + * @param string $prefix + * @param string $expected + * @dataProvider provideGet + * @covers GlobalVarConfig::get + */ + public function testGet( $name, $prefix, $expected ) { + $config = new GlobalVarConfig( $prefix ); + $this->assertEquals( $config->get( $name ), $expected ); + } +}