Merge "resourceloader: Fix mw.loader to compute combined version in packed order"

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index 7d8c3bc..3fe276b 100644 (file)
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1523,17 +1523,28 @@
                         *
                         * @private
                         * @param {Object} moduleMap Module map
-                        * @return {string} Module query string
+                        * @return {Object}
+                        * @return {string} return.str Module query string
+                        * @return {Array} return.list List of module names in matching order
                         */
                        function buildModulesString( moduleMap ) {
                                var p, prefix,
-                                       arr = [];
+                                       str = [],
+                                       list = [];
+
+                               function restore( suffix ) {
+                                       return p + suffix;
+                               }
 
                                for ( prefix in moduleMap ) {
                                        p = prefix === '' ? '' : prefix + '.';
-                                       arr.push( p + moduleMap[ prefix ].join( ',' ) );
+                                       str.push( p + moduleMap[ prefix ].join( ',' ) );
+                                       list.push.apply( list, moduleMap[ prefix ].map( restore ) );
                                }
-                               return arr.join( '|' );
+                               return {
+                                       str: str.join( '|' ),
+                                       list: list
+                               };
                        }
 
                        /**
@@ -1586,9 +1597,15 @@
                                 */
                                function doRequest() {
                                        // Optimisation: Inherit (Object.create), not copy ($.extend)
-                                       var query = Object.create( currReqBase );
-                                       query.modules = buildModulesString( moduleMap );
-                                       query.version = getCombinedVersion( currReqModules );
+                                       var query = Object.create( currReqBase ),
+                                               packed = buildModulesString( moduleMap );
+                                       query.modules = packed.str;
+                                       // The packing logic can change the effective order, even if the input was
+                                       // sorted. As such, the call to getCombinedVersion() must use this
+                                       // effective order, instead of currReqModules, as otherwise the combined
+                                       // version will not match the hash expected by the server based on
+                                       // combining versions from the module query string in-order. (T188076)
+                                       query.version = getCombinedVersion( packed.list );
                                        query = sortQuery( query );
                                        addScript( sourceLoadScript + '?' + $.param( query ) );
                                }

diff --git a/tests/qunit/data/load.mock.php b/tests/qunit/data/load.mock.php
index 43ee0c7..2300949 100644 (file)
--- a/tests/qunit/data/load.mock.php
+++ b/tests/qunit/data/load.mock.php
@@ -59,6 +59,15 @@ mw.loader.implement( 'testUrlInc.a', function () {} );
 ",
        'testUrlInc.b' => "
 mw.loader.implement( 'testUrlInc.b', function () {} );
+",
+       'testUrlOrder' => "
+mw.loader.implement( 'testUrlOrder', function () {} );
+",
+       'testUrlOrder.a' => "
+mw.loader.implement( 'testUrlOrder.a', function () {} );
+",
+       'testUrlOrder.b' => "
+mw.loader.implement( 'testUrlOrder.b', function () {} );
 ",
 ];
 
@@ -70,6 +79,8 @@ $response = '';
 if ( isset( $_GET['modules'] ) ) {
        if ( $_GET['modules'] === 'testUrlInc,testUrlIncDump|testUrlInc.a,b' ) {
                $modules = [ 'testUrlInc', 'testUrlIncDump', 'testUrlInc.a', 'testUrlInc.b' ];
+       } elseif ( $_GET['modules'] === 'testUrlOrder,testUrlOrderDump|testUrlOrder.a,b' ) {
+               $modules = [ 'testUrlOrder', 'testUrlOrderDump', 'testUrlOrder.a', 'testUrlOrder.b' ];
        } else {
                $modules = explode( ',', $_GET['modules'] );
        }

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
index 6c988ef..0b05ac1 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
@@ -583,6 +583,40 @@
                } );
        } );
 
+       // @covers mw.loader#batchRequest
+       // @covers mw.loader#buildModulesString
+       QUnit.test( 'Url composition (order of modules for version) – T188076', function ( assert ) {
+               mw.loader.register( [
+                       // [module, version, dependencies, group, source]
+                       [ 'testUrlOrder', 'url', [], null, 'testloader' ],
+                       [ 'testUrlOrder.a', '1', [], null, 'testloader' ],
+                       [ 'testUrlOrder.b', '2', [], null, 'testloader' ],
+                       [ 'testUrlOrderDump', 'dump', [], null, 'testloader' ]
+               ] );
+
+               return mw.loader.using( [
+                       'testUrlOrderDump',
+                       'testUrlOrder.b',
+                       'testUrlOrder.a',
+                       'testUrlOrder'
+               ] ).then( function ( require ) {
+                       assert.propEqual(
+                               require( 'testUrlOrderDump' ).query,
+                               {
+                                       modules: 'testUrlOrder,testUrlOrderDump|testUrlOrder.a,b',
+                                       // Expected: Combined in order after string packing
+                                       //   $hash = hash( 'fnv132', 'urldump12' );
+                                       //   base_convert( $hash, 16, 36 ); // "1knqzan"
+                                       // Previously: Combined in order of before string packing
+                                       //   $hash = hash( 'fnv132', 'url12dump' );
+                                       //   base_convert( $hash, 16, 36 ); // "11eo3in"
+                                       version: '1knqzan'
+                               },
+                               'Query parameters'
+                       );
+               } );
+       } );
+
        QUnit.test( 'Broken indirect dependency', function ( assert ) {
                // don't emit an error event
                this.sandbox.stub( mw, 'track' );