From: jenkins-bot Date: Tue, 27 May 2014 09:12:15 +0000 (+0000) Subject: Merge "Restructure Media related tests to avoid duplicated code" X-Git-Tag: 1.31.0-rc.0~15579 X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin/?a=commitdiff_plain;h=a98255b197abae48f2c8aa4adabe2e70a5ba6798;hp=6beee2535f37e71588fc360cc48eae6735e390df;p=lhc%2Fweb%2Fwiklou.git Merge "Restructure Media related tests to avoid duplicated code" --- diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 338274c5e4..450112c319 100644 --- a/includes/AutoLoader.php +++ b/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', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 796e0c079d..e156792ce4 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 @@ -5608,7 +5611,7 @@ $wgRC2UDPOmitBots = false; * The common options are: * * 'uri' -- the address to which the notices are to be sent. * * 'formatter' -- the class name (implementing RCFeedFormatter) which will - * produce the text to send. + * produce the text to send. This can also be an object of the class. * * 'omit_bots' -- whether the bot edits should be in the feed * * 'omit_anon' -- whether anonymous edits should be in the feed * * 'omit_user' -- whether edits by registered users should be in the feed diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php index 3a5a86906e..370e109a87 100644 --- a/includes/changes/RecentChange.php +++ b/includes/changes/RecentChange.php @@ -390,13 +390,17 @@ class RecentChange { /** * Notify all the feeds about the change. + * @param array $feeds Optional feeds to send to, defaults to $wgRCFeeds */ - public function notifyRCFeeds() { + public function notifyRCFeeds( array $feeds = null ) { global $wgRCFeeds; + if ( $feeds === null ) { + $feeds = $wgRCFeeds; + } $performer = $this->getPerformer(); - foreach ( $wgRCFeeds as $feed ) { + foreach ( $feeds as $feed ) { $feed += array( 'omit_bots' => false, 'omit_anon' => false, @@ -425,7 +429,7 @@ class RecentChange { } /** @var $formatter RCFeedFormatter */ - $formatter = new $feed['formatter'](); + $formatter = is_object( $feed['formatter'] ) ? $feed['formatter'] : new $feed['formatter'](); $line = $formatter->getLine( $feed, $this, $actionComment ); $engine->send( $feed, $line ); diff --git a/includes/config/Config.php b/includes/config/Config.php index 04afddad14..68e90b4582 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/ConfigException.php b/includes/config/ConfigException.php new file mode 100644 index 0000000000..368ca5a465 --- /dev/null +++ b/includes/config/ConfigException.php @@ -0,0 +1,28 @@ + 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/GlobalConfig.php b/includes/config/GlobalConfig.php deleted file mode 100644 index e16a9ee728..0000000000 --- a/includes/config/GlobalConfig.php +++ /dev/null @@ -1,45 +0,0 @@ -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/DerivativeContext.php b/includes/context/DerivativeContext.php index 32a650ff15..4a7b46693c 100644 --- a/includes/context/DerivativeContext.php +++ b/includes/context/DerivativeContext.php @@ -66,7 +66,7 @@ class DerivativeContext extends ContextSource { private $skin; /** - * @var SiteConfiguration + * @var Config */ private $config; @@ -81,9 +81,9 @@ class DerivativeContext extends ContextSource { /** * Set the SiteConfiguration object * - * @param SiteConfiguration $s + * @param Config $s */ - public function setConfig( SiteConfiguration $s ) { + public function setConfig( Config $s ) { $this->config = $s; } diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php index 4291e4456b..c87bfc03ab 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/includes/media/MediaHandler.php b/includes/media/MediaHandler.php index ab8fa14feb..f131af4d67 100644 --- a/includes/media/MediaHandler.php +++ b/includes/media/MediaHandler.php @@ -573,7 +573,7 @@ abstract class MediaHandler { } /** - * Used instead of getLongDesc if there is no handler registered for file. + * Short description. Shown on Special:Search results. * * @param File $file * @return string @@ -585,7 +585,7 @@ abstract class MediaHandler { } /** - * Short description. Shown on Special:Search results. + * Long description. Shown under image on image description page surounded by (). * * @param File $file * @return string @@ -598,7 +598,7 @@ abstract class MediaHandler { } /** - * Long description. Shown under image on image description page surounded by (). + * Used instead of getShortDesc if there is no handler registered for file. * * @param File $file * @return string @@ -610,7 +610,7 @@ abstract class MediaHandler { } /** - * Used instead of getShortDesc if there is no handler registered for file. + * Used instead of getLongDesc if there is no handler registered for file. * * @param File $file * @return string diff --git a/includes/specials/SpecialRevisiondelete.php b/includes/specials/SpecialRevisiondelete.php index 797f588a0d..d3b168b35b 100644 --- a/includes/specials/SpecialRevisiondelete.php +++ b/includes/specials/SpecialRevisiondelete.php @@ -572,12 +572,12 @@ class SpecialRevisionDelete extends UnlistedSpecialPage { // from dropdown $listReason = $this->getRequest()->getText( 'wpRevDeleteReasonList', 'other' ); $comment = $listReason; - if ( $comment != 'other' && $this->otherReason != '' ) { + if ( $comment === 'other' ) { + $comment = $this->otherReason; + } elseif ( $this->otherReason !== '' ) { // Entry from drop down menu + additional comment $comment .= $this->msg( 'colon-separator' )->inContentLanguage()->text() . $this->otherReason; - } elseif ( $comment == 'other' ) { - $comment = $this->otherReason; } # Can the user set this field? if ( $bitParams[Revision::DELETED_RESTRICTED] == 1 diff --git a/tests/phpunit/includes/config/ConfigFactoryTest.php b/tests/phpunit/includes/config/ConfigFactoryTest.php new file mode 100644 index 0000000000..0a6bf723f8 --- /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 b605a46eb2..0000000000 --- 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 0000000000..7080b02335 --- /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 ); + } +}