Merge "Make abstract Config class truly implementation-agnostic"

includes/AutoLoader.php

@@ -392,7 +392,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',

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

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 );
 }

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 {}

includes/config/ConfigFactory.php

@@ -0,0 +1,94 @@
+<?php
+
+/**
+ * Copyright 2014
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Factory class to create Config objects
+ *
+ * @since 1.23
+ */
+class ConfigFactory {
+
+	/**
+	 * Map of config name => 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];
+	}
+}

includes/config/GlobalVarConfig.php

@@ -0,0 +1,88 @@
+<?php
+/**
+ * Copyright 2014
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Accesses configuration settings from $GLOBALS
+ *
+ * @since 1.23
+ */
+class GlobalVarConfig implements Config {
+
+	/**
+	 * Prefix to use for configuration variables
+	 * @var string
+	 */
+	private $prefix;
+
+	/**
+	 * Default builder function
+	 * @return GlobalVarConfig
+	 */
+	public static function newInstance() {
+		return new GlobalVarConfig();
+	}
+
+	public function __construct( $prefix = 'wg' ) {
+		$this->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;
+	}
+}

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;

tests/phpunit/includes/config/ConfigFactoryTest.php

@@ -0,0 +1,46 @@
+<?php
+
+class ConfigFactoryTest extends MediaWikiTestCase {
+
+	/**
+	 * @covers ConfigFactory::register
+	 */
+	public function testRegister() {
+		$factory = new ConfigFactory();
+		$factory->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' );
+	}
+}

tests/phpunit/includes/config/GlobalConfigTest.php

@@ -1,38 +0,0 @@
-<?php
-
-class GlobalConfigTest extends MediaWikiTestCase {
-
-	/** @var GlobalConfig $config */
-	protected $config;
-
-	protected function setUp() {
-		parent::setUp();
-		$this->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;
-		}
-	}
-}

tests/phpunit/includes/config/GlobalVarConfigTest.php

@@ -0,0 +1,36 @@
+<?php
+
+class GlobalVarConfigTest extends MediaWikiTestCase {
+
+	public function provideGet() {
+		$set = array(
+			'wgSomething' => '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 );
+	}
+}