resourceloader: Add $modules parameter to makeVersionQuery()

The 'version' and 'modules' parameters are a somewhat problematic
part of the ResourceLoaderContext object as we often pass around
the context but may be dealing with only a subset of the modules
in the outer request, or even entirely different ones (e.g.
for OutputPage's fake context 'modules' and 'version' are both
null).

This is already visible in ClientHtml where we create a derivative
context just to call setModules() and have makeVersionQuery()
work. This is now fixed.

This change is in preparation for use in ResourceLoaderImage
(to fix T233343) where we'll need to compute a version for only
1 module of a larger set, and ideally without needing to create
another context.

Bug: T233343
Change-Id: Icc1a4fd1f58c4e49e58eee43ca4ba2de6cfffc76

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index d959ff6..0d546fa 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -689,24 +689,29 @@ class ResourceLoader implements LoggerAwareInterface {
         *
         * @since 1.28
         * @param ResourceLoaderContext $context
+        * @param string[]|null $modules
         * @return string Hash
         */
-       public function makeVersionQuery( ResourceLoaderContext $context ) {
+       public function makeVersionQuery( ResourceLoaderContext $context, array $modules = null ) {
+               if ( $modules === null ) {
+                       wfDeprecated( __METHOD__ . ' without $modules', '1.34' );
+                       $modules = $context->getModules();
+               }
                // As of MediaWiki 1.28, the server and client use the same algorithm for combining
                // version hashes. There is no technical reason for this to be same, and for years the
                // implementations differed. If getCombinedVersion in PHP (used for StartupModule and
                // E-Tag headers) differs in the future from getCombinedVersion in JS (used for 'version'
                // query parameter), then this method must continue to match the JS one.
-               $moduleNames = [];
-               foreach ( $context->getModules() as $name ) {
+               $filtered = [];
+               foreach ( $modules as $name ) {
                        if ( !$this->getModule( $name ) ) {
                                // If a versioned request contains a missing module, the version is a mismatch
                                // as the client considered a module (and version) we don't have.
                                return '';
                        }
-                       $moduleNames[] = $name;
+                       $filtered[] = $name;
                }
-               return $this->getCombinedVersion( $context, $moduleNames );
+               return $this->getCombinedVersion( $context, $filtered );
        }
 
        /**
@@ -863,7 +868,7 @@ class ResourceLoader implements LoggerAwareInterface {
                // - Version mismatch (T117587, T47877)
                if ( is_null( $context->getVersion() )
                        || $errors
-                       || $context->getVersion() !== $this->makeVersionQuery( $context )
+                       || $context->getVersion() !== $this->makeVersionQuery( $context, $context->getModules() )
                ) {
                        $maxage = $rlMaxage['unversioned']['client'];
                        $smaxage = $rlMaxage['unversioned']['server'];

diff --git a/includes/resourceloader/ResourceLoaderClientHtml.php b/includes/resourceloader/ResourceLoaderClientHtml.php
index bf03e49..d98d86b 100644 (file)
--- a/includes/resourceloader/ResourceLoaderClientHtml.php
+++ b/includes/resourceloader/ResourceLoaderClientHtml.php
@@ -436,7 +436,8 @@ JAVASCRIPT;
 
                                // Link/embed each set
                                foreach ( $moduleSets as list( $embed, $moduleSet ) ) {
-                                       $context->setModules( array_keys( $moduleSet ) );
+                                       $moduleSetNames = array_keys( $moduleSet );
+                                       $context->setModules( $moduleSetNames );
                                        if ( $embed ) {
                                                // Decide whether to use style or script element
                                                if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
@@ -456,10 +457,10 @@ JAVASCRIPT;
                                                // This should NOT be done for the site group (T29564) because anons get that too
                                                // and we shouldn't be putting timestamps in CDN-cached HTML
                                                if ( $group === 'user' ) {
-                                                       // Must setModules() before makeVersionQuery()
-                                                       $context->setVersion( $rl->makeVersionQuery( $context ) );
+                                                       $context->setVersion( $rl->makeVersionQuery( $context, $moduleSetNames ) );
                                                }
 
+                                               // Must setModules() before createLoaderURL()
                                                $url = $rl->createLoaderURL( $source, $context, $extraQuery );
 
                                                // Decide whether to use 'style' or 'script' element