resourceloader: Only store sources' load.php urls

Previously ResourceLoader would store any arbitrary data about a
source, provided it had a 'loadScript' key. It would register
the 'local' source with an additional 'apiScript' key, which was
also documented in DefaultSettings.php. However, it was
completely unused outside of the ForeignAPIGadgetRepo class in
Gadgets 2.0, which should be changed to take an API url as a
parameter. This was not useful as it was not ever formally
exposed, and it could not be depended upon that a source had
registered an 'apiScript' key.

For backwards compatability, both ResourceLoader::addSource()
and mw.loader.addSource() will both take an array/object, but
discard all parameters except for 'loadScript'.

Also added tests for ResourceLoader::addSource().

Bug: 69878
Change-Id: I4205cf788cddeec13b619be0c3576197dec1b8bf

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 14799c6..a9b09e2 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -3274,10 +3274,7 @@ $wgResourceModuleSkinStyles = array();
  *
  * @par Example:
  * @code
- *   $wgResourceLoaderSources['foo'] = array(
- *       'loadScript' => 'http://example.org/w/load.php',
- *       'apiScript' => 'http://example.org/w/api.php'
- *   );
+ *   $wgResourceLoaderSources['foo'] = 'http://example.org/w/load.php';
  * @endcode
  */
 $wgResourceLoaderSources = array();

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 5f72d8d..a356e2e 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -32,9 +32,6 @@ class ResourceLoader {
        /** @var int */
        protected static $filterCacheVersion = 7;
 
-       /** @var array */
-       protected static $requiredSourceProperties = array( 'loadScript' );
-
        /** @var bool */
        protected static $debugMode = null;
 
@@ -53,7 +50,7 @@ class ResourceLoader {
         */
        protected $testModuleNames = array();
 
-       /** @var array E.g. array( 'source-id' => array( 'loadScript' => 'http://.../load.php' ) ) */
+       /** @var array E.g. array( 'source-id' => 'http://.../load.php' ) */
        protected $sources = array();
 
        /** @var bool */
@@ -231,10 +228,7 @@ class ResourceLoader {
                $this->config = $config;
 
                // Add 'local' source first
-               $this->addSource(
-                       'local',
-                       array( 'loadScript' => wfScript( 'load' ), 'apiScript' => wfScript( 'api' ) )
-               );
+               $this->addSource( 'local', wfScript( 'load' ) );
 
                // Add other sources
                $this->addSource( $config->get( 'ResourceLoaderSources' ) );
@@ -401,14 +395,12 @@ class ResourceLoader {
        /**
         * Add a foreign source of modules.
         *
-        * Source properties:
-        * 'loadScript': URL (either fully-qualified or protocol-relative) of load.php for this source
-        *
-        * @param mixed $id Source ID (string), or array( id1 => props1, id2 => props2, ... )
-        * @param array $properties Source properties
+        * @param array|string $id Source ID (string), or array( id1 => loadUrl, id2 => loadUrl, ... )
+        * @param string|array $loadUrl load.php url (string), or array with loadUrl key for
+        *  backwards-compatability.
         * @throws MWException
         */
-       public function addSource( $id, $properties = null ) {
+       public function addSource( $id, $loadUrl = null ) {
                // Allow multiple sources to be registered in one call
                if ( is_array( $id ) ) {
                        foreach ( $id as $key => $value ) {
@@ -425,14 +417,18 @@ class ResourceLoader {
                        );
                }
 
-               // Validate properties
-               foreach ( self::$requiredSourceProperties as $prop ) {
-                       if ( !isset( $properties[$prop] ) ) {
-                               throw new MWException( "Required property $prop missing from source ID $id" );
+               // Pre 1.24 backwards-compatability
+               if ( is_array( $loadUrl ) ) {
+                       if ( !isset( $loadUrl['loadScript'] ) ) {
+                               throw new MWException(
+                                       __METHOD__ . ' was passed an array with no "loadScript" key.'
+                               );
                        }
+
+                       $loadUrl = $loadUrl['loadScript'];
                }
 
-               $this->sources[$id] = $properties;
+               $this->sources[$id] = $loadUrl;
        }
 
        /**
@@ -527,7 +523,7 @@ class ResourceLoader {
        /**
         * Get the list of sources.
         *
-        * @return array Like array( id => array of properties, .. )
+        * @return array Like array( id => load.php url, .. )
         */
        public function getSources() {
                return $this->sources;
@@ -546,7 +542,7 @@ class ResourceLoader {
                if ( !isset( $this->sources[$source] ) ) {
                        throw new MWException( "The $source source was never registered in ResourceLoader." );
                }
-               return $this->sources[$source]['loadScript'];
+               return $this->sources[$source];
        }
 
        /**
@@ -1228,7 +1224,7 @@ class ResourceLoader {
         *   - ResourceLoader::makeLoaderSourcesScript( $id, $properties ):
         *       Register a single source
         *
-        *   - ResourceLoader::makeLoaderSourcesScript( array( $id1 => $props1, $id2 => $props2, ... ) );
+        *   - ResourceLoader::makeLoaderSourcesScript( array( $id1 => $loadUrl, $id2 => $loadUrl, ... ) );
         *       Register sources with the given IDs and properties.
         *
         * @param string $id Source ID

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index d50fe48..0d57d59 100644 (file)
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -628,12 +628,10 @@
                         */
                        var registry = {},
                                //
-                               // Mapping of sources, keyed by source-id, values are objects.
+                               // Mapping of sources, keyed by source-id, values are strings.
                                // Format:
                                //      {
-                               //              'sourceId': {
-                               //                      'loadScript': 'http://foo.bar/w/load.php'
-                               //              }
+                               //              'sourceId': 'http://foo.bar/w/load.php'
                                //      }
                                //
                                sources = {},
@@ -1478,7 +1476,7 @@
 
                                        for ( source in splits ) {
 
-                                               sourceLoadScript = sources[source].loadScript;
+                                               sourceLoadScript = sources[source];
 
                                                for ( group in splits[source] ) {
 
@@ -1552,15 +1550,14 @@
                                 *
                                 * The #work method will use this information to split up requests by source.
                                 *
-                                *     mw.loader.addSource( 'mediawikiwiki', { loadScript: '//www.mediawiki.org/w/load.php' } );
+                                *     mw.loader.addSource( 'mediawikiwiki', '//www.mediawiki.org/w/load.php' );
                                 *
                                 * @param {string} id Short string representing a source wiki, used internally for
                                 *  registered modules to indicate where they should be loaded from (usually lowercase a-z).
-                                * @param {Object} props
-                                * @param {string} props.loadScript Url to the load.php entry point of the source wiki.
+                                * @param {Object|string} loadUrl load.php url, may be an object for backwards-compatability
                                 * @return {boolean}
                                 */
-                               addSource: function ( id, props ) {
+                               addSource: function ( id, loadUrl ) {
                                        var source;
                                        // Allow multiple additions
                                        if ( typeof id === 'object' ) {
@@ -1574,7 +1571,11 @@
                                                throw new Error( 'source already registered: ' + id );
                                        }
 
-                                       sources[id] = props;
+                                       if ( typeof loadUrl === 'object' ) {
+                                               loadUrl = loadUrl.loadScript;
+                                       }
+
+                                       sources[id] = loadUrl;
 
                                        return true;
                                },

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php
index 0c250bd..a189387 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php
@@ -9,10 +9,7 @@ class ResourceLoaderStartupModuleTest extends ResourceLoaderTestCase {
                                'modules' => array(),
                                'out' => '
 mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    }
+    "local": "/w/load.php"
 } );mw.loader.register( [] );'
                        ) ),
                        array( array(
@@ -22,10 +19,7 @@ mw.loader.addSource( {
                                ),
                                'out' => '
 mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    }
+    "local": "/w/load.php"
 } );mw.loader.register( [
     [
         "test.blank",
@@ -42,10 +36,7 @@ mw.loader.addSource( {
                                ),
                                'out' => '
 mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    }
+    "local": "/w/load.php"
 } );mw.loader.register( [
     [
         "test.blank",
@@ -73,10 +64,7 @@ mw.loader.addSource( {
                                ),
                                'out' => '
 mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    }
+    "local": "/w/load.php"
 } );mw.loader.register( [
     [
         "test.blank",
@@ -97,14 +85,8 @@ mw.loader.addSource( {
                                ),
                                'out' => '
 mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    },
-    "example": {
-        "loadScript": "http://example.org/w/load.php",
-        "apiScript": "http://example.org/w/api.php"
-    }
+    "local": "/w/load.php",
+    "example": "http://example.org/w/load.php"
 } );mw.loader.register( [
     [
         "test.blank",
@@ -140,10 +122,7 @@ mw.loader.addSource( {
                                ),
                                'out' => '
 mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    }
+    "local": "/w/load.php"
 } );mw.loader.register( [
     [
         "test.x.core",
@@ -238,14 +217,8 @@ mw.loader.addSource( {
                                ),
                                'out' => '
 mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    },
-    "example": {
-        "loadScript": "http://example.org/w/load.php",
-        "apiScript": "http://example.org/w/api.php"
-    }
+    "local": "/w/load.php",
+    "example": "http://example.org/w/load.php"
 } );mw.loader.register( [
     [
         "test.blank",
@@ -369,7 +342,7 @@ mw.loader.addSource( {
                $rl->register( $modules );
                $module = new ResourceLoaderStartUpModule();
                $this->assertEquals(
-'mw.loader.addSource({"local":{"loadScript":"/w/load.php","apiScript":"/w/api.php"}});'
+'mw.loader.addSource({"local":"/w/load.php"});'
 . 'mw.loader.register(['
 . '["test.blank","1388534400"],'
 . '["test.min","1388534400",["test.blank"],null,"local",'
@@ -390,10 +363,7 @@ mw.loader.addSource( {
                $module = new ResourceLoaderStartUpModule();
                $this->assertEquals(
 'mw.loader.addSource( {
-    "local": {
-        "loadScript": "/w/load.php",
-        "apiScript": "/w/api.php"
-    }
+    "local": "/w/load.php"
 } );mw.loader.register( [
     [
         "test.blank",

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
index bd6b3f2..81a3dc4 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -131,6 +131,43 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
                );
        }
 
+       public static function provideAddSource() {
+               return array(
+                       array( 'examplewiki', '//example.org/w/load.php', 'examplewiki' ),
+                       array( 'example2wiki', array( 'loadScript' => '//example.com/w/load.php' ), 'example2wiki' ),
+                       array(
+                               array( 'foowiki' => '//foo.org/w/load.php', 'bazwiki' => '//baz.org/w/load.php' ),
+                               null,
+                               array( 'foowiki', 'bazwiki' )
+                       ),
+                       array(
+                               array( 'foowiki' => '//foo.org/w/load.php' ),
+                               null,
+                               false,
+                       ),
+               );
+       }
+
+       /**
+        * @dataProvider provideAddSource
+        * @covers ResourceLoader::addSource
+        */
+       public function testAddSource( $name, $info, $expected ) {
+               $rl = new ResourceLoader;
+               if ( $expected === false ) {
+                       $this->setExpectedException( 'MWException', 'ResourceLoader duplicate source addition error' );
+                       $rl->addSource( $name, $info );
+               }
+               $rl->addSource( $name, $info );
+               if ( is_array( $expected ) ) {
+                       foreach ( $expected as $source ) {
+                               $this->assertArrayHasKey( $source, $rl->getSources() );
+                       }
+               } else {
+                       $this->assertArrayHasKey( $expected, $rl->getSources() );
+               }
+       }
+
        public static function fakeSources() {
                return array(
                        'examplewiki' => array(

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index 9681330..0767c9f 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -32,9 +32,7 @@
 
        mw.loader.addSource(
                'testloader',
-               {
-                       loadScript: QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/load.mock.php' )
-               }
+               QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/load.mock.php' )
        );
 
        QUnit.test( 'Initial check', 8, function ( assert ) {