Merge "Restructure Media related tests to avoid duplicated code"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 27 May 2014 09:12:15 +0000 (09:12 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 27 May 2014 09:12:15 +0000 (09:12 +0000)
15 files changed:
includes/AutoLoader.php
includes/DefaultSettings.php
includes/changes/RecentChange.php
includes/config/Config.php
includes/config/ConfigException.php [new file with mode: 0644]
includes/config/ConfigFactory.php [new file with mode: 0644]
includes/config/GlobalConfig.php [deleted file]
includes/config/GlobalVarConfig.php [new file with mode: 0644]
includes/context/DerivativeContext.php
includes/context/RequestContext.php
includes/media/MediaHandler.php
includes/specials/SpecialRevisiondelete.php
tests/phpunit/includes/config/ConfigFactoryTest.php [new file with mode: 0644]
tests/phpunit/includes/config/GlobalConfigTest.php [deleted file]
tests/phpunit/includes/config/GlobalVarConfigTest.php [new file with mode: 0644]

index 338274c..450112c 100644 (file)
@@ -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',
index 796e0c0..e156792 100644 (file)
@@ -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
index 3a5a869..370e109 100644 (file)
@@ -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 );
index 04afdda..68e90b4 100644 (file)
  */
 
 /**
- * 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 (file)
index 0000000..368ca5a
--- /dev/null
@@ -0,0 +1,28 @@
+<?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
+ */
+
+/**
+ * Exceptions for config failures
+ *
+ * @since 1.23
+ */
+class ConfigException extends MWException {}
diff --git a/includes/config/ConfigFactory.php b/includes/config/ConfigFactory.php
new file mode 100644 (file)
index 0000000..b09316b
--- /dev/null
@@ -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];
+       }
+}
diff --git a/includes/config/GlobalConfig.php b/includes/config/GlobalConfig.php
deleted file mode 100644 (file)
index e16a9ee..0000000
+++ /dev/null
@@ -1,45 +0,0 @@
-<?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 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();
-       }
-}
diff --git a/includes/config/GlobalVarConfig.php b/includes/config/GlobalVarConfig.php
new file mode 100644 (file)
index 0000000..61a76b7
--- /dev/null
@@ -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;
+       }
+}
index 32a650f..4a7b466 100644 (file)
@@ -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;
        }
 
index 4291e44..c87bfc0 100644 (file)
@@ -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;
index ab8fa14..f131af4 100644 (file)
@@ -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
index 797f588..d3b168b 100644 (file)
@@ -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 (file)
index 0000000..0a6bf72
--- /dev/null
@@ -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' );
+       }
+}
diff --git a/tests/phpunit/includes/config/GlobalConfigTest.php b/tests/phpunit/includes/config/GlobalConfigTest.php
deleted file mode 100644 (file)
index b605a46..0000000
+++ /dev/null
@@ -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;
-               }
-       }
-}
diff --git a/tests/phpunit/includes/config/GlobalVarConfigTest.php b/tests/phpunit/includes/config/GlobalVarConfigTest.php
new file mode 100644 (file)
index 0000000..7080b02
--- /dev/null
@@ -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 );
+       }
+}