jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/365888 )
Change subject: resourceloader: Add support for modules sending preload headers ...................................................................... resourceloader: Add support for modules sending preload headers ResourceLoaderModule objects gain a new method getPreloadLinks() which returns an array with the meta data required to build a Link rel=preload header according to the current draft for W3C Preload. <https://w3c.github.io/preload/> Another implementation of this is already in use in OutputPage for preloading the logo image. This array is formatted by the ResourceLoaderModule::getHeaders method, which is implemented as "final" at this time, thus restricting use to the Link rel=preload header. Headers are exposed and process-cached, like all other content (scripts, styles, etc.), through ResourceLoaderModule::getModuleContent, and aggregated by ResoureLoader::makeModuleResponse. I had hoped for the getPreloadLinks to be stateless (not vary on $context). Whether something should be preloaded and what, should not vary on the skin or language. However, while that conceptually holds true, the exact url for any given resource may still vary. Even the main use case for this feature (T164299, preloading base modules request) require $context to pass down skin and lang to the load.php url. Add full test coverage and example documentation. Bug: T164299 Change-Id: I2bfe0796ceaa0c82579c501f5b10e931f2175681 --- M includes/resourceloader/ResourceLoader.php M includes/resourceloader/ResourceLoaderModule.php M tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php M tests/phpunit/includes/resourceloader/ResourceLoaderTest.php 4 files changed, 218 insertions(+), 2 deletions(-) Approvals: jenkins-bot: Verified Gilles: Looks good to me, approved diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 2f29200..ad16420 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -80,6 +80,15 @@ protected $errors = []; /** + * List of extra HTTP response headers provided by loaded modules. + * + * Populated by makeModuleResponse(). + * + * @var array + */ + protected $extraHeaders = []; + + /** * @var MessageBlobStore */ protected $blobStore; @@ -794,7 +803,7 @@ } } - $this->sendResponseHeaders( $context, $etag, (bool)$this->errors ); + $this->sendResponseHeaders( $context, $etag, (bool)$this->errors, $this->extraHeaders ); // Remove the output buffer and output the response ob_end_clean(); @@ -827,9 +836,12 @@ * @param ResourceLoaderContext $context * @param string $etag ETag header value * @param bool $errors Whether there are errors in the response + * @param string[] $extra Array of extra HTTP response headers * @return void */ - protected function sendResponseHeaders( ResourceLoaderContext $context, $etag, $errors ) { + protected function sendResponseHeaders( + ResourceLoaderContext $context, $etag, $errors, array $extra = [] + ) { \MediaWiki\HeaderCallback::warnIfHeadersSent(); $rlMaxage = $this->config->get( 'ResourceLoaderMaxage' ); // Use a short cache expiry so that updates propagate to clients quickly, if: @@ -872,6 +884,9 @@ header( "Cache-Control: public, max-age=$maxage, s-maxage=$smaxage" ); $exp = min( $maxage, $smaxage ); header( 'Expires: ' . wfTimestamp( TS_RFC2822, $exp + time() ) ); + } + foreach ( $extra as $header ) { + header( $header ); } } @@ -1008,6 +1023,9 @@ /** * Generate code for a response. * + * Calling this method also populates the `errors` and `headers` members, + * later used by respond(). + * * @param ResourceLoaderContext $context Context in which to generate a response * @param ResourceLoaderModule[] $modules List of module objects keyed by module name * @param string[] $missing List of requested module names that are unregistered (optional) @@ -1052,6 +1070,10 @@ $implementKey = $name . '@' . $module->getVersionHash( $context ); $strContent = ''; + if ( isset( $content['headers'] ) ) { + $this->extraHeaders = array_merge( $this->extraHeaders, $content['headers'] ); + } + // Append output switch ( $context->getOnly() ) { case 'scripts': diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php index 1608901..b3c1cd1 100644 --- a/includes/resourceloader/ResourceLoaderModule.php +++ b/includes/resourceloader/ResourceLoaderModule.php @@ -587,6 +587,81 @@ } /** + * Get headers to send as part of a module web response. + * + * It is not supported to send headers through this method that are + * required to be unique or otherwise sent once in an HTTP response + * because clients may make batch requests for multiple modules (as + * is the default behaviour for ResourceLoader clients). + * + * For exclusive or aggregated headers, see ResourceLoader::sendResponseHeaders(). + * + * @since 1.30 + * @param ResourceLoaderContext $context + * @return string[] Array of HTTP response headers + */ + final public function getHeaders( ResourceLoaderContext $context ) { + $headers = []; + + $formattedLinks = []; + foreach ( $this->getPreloadLinks( $context ) as $url => $attribs ) { + $link = "<{$url}>;rel=preload"; + foreach ( $attribs as $key => $val ) { + $link .= ";{$key}={$val}"; + } + $formattedLinks[] = $link; + } + if ( $formattedLinks ) { + $headers[] = 'Link: ' . implode( ',', $formattedLinks ); + } + + return $headers; + } + + /** + * Get a list of resources that web browsers may preload. + * + * Behaviour of rel=preload link is specified at <https://www.w3.org/TR/preload/>. + * + * Use case for ResourceLoader originally part of T164299. + * + * @par Example + * @code + * protected function getPreloadLinks() { + * return [ + * 'https://example.org/script.js' => [ 'as' => 'script' ], + * 'https://example.org/image.png' => [ 'as' => 'image' ], + * ]; + * } + * @encode + * + * @par Example using HiDPI image variants + * @code + * protected function getPreloadLinks() { + * return [ + * 'https://example.org/logo.png' => [ + * 'as' => 'image', + * 'media' => 'not all and (min-resolution: 2dppx)', + * ], + * 'https://example.org/[email protected]' => [ + * 'as' => 'image', + * 'media' => '(min-resolution: 2dppx)', + * ], + * ]; + * } + * @encode + * + * @see ResourceLoaderModule::getHeaders + * @since 1.30 + * @param ResourceLoaderContext $context + * @return array Keyed by url, values must be an array containing + * at least an 'as' key. Optionally a 'media' key as well. + */ + protected function getPreloadLinks( ResourceLoaderContext $context ) { + return []; + } + + /** * Get module-specific LESS variables, if any. * * @since 1.27 @@ -711,6 +786,11 @@ $content['templates'] = $templates; } + $headers = $this->getHeaders( $context ); + if ( $headers ) { + $content['headers'] = $headers; + } + $statTiming = microtime( true ) - $statStart; $statName = strtr( $this->getName(), '.', '_' ); $stats->timing( "resourceloader_build.all", 1000 * $statTiming ); diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php index c861b37..7c7f1cf 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php @@ -180,4 +180,43 @@ 'Substitute placeholders' ); } + + /** + * @covers ResourceLoaderModule::getHeaders + * @covers ResourceLoaderModule::getPreloadLinks + */ + public function testGetHeaders() { + $context = $this->getResourceLoaderContext(); + + $module = new ResourceLoaderTestModule(); + $this->assertSame( [], $module->getHeaders( $context ), 'Default' ); + + $module = $this->getMockBuilder( ResourceLoaderTestModule::class ) + ->setMethods( [ 'getPreloadLinks' ] )->getMock(); + $module->method( 'getPreloadLinks' )->willReturn( [ + 'https://example.org/script.js' => [ 'as' => 'script' ], + ] ); + $this->assertSame( + [ + 'Link: <https://example.org/script.js>;rel=preload;as=script' + ], + $module->getHeaders( $context ), + 'Preload one resource' + ); + + $module = $this->getMockBuilder( ResourceLoaderTestModule::class ) + ->setMethods( [ 'getPreloadLinks' ] )->getMock(); + $module->method( 'getPreloadLinks' )->willReturn( [ + 'https://example.org/script.js' => [ 'as' => 'script' ], + '/example.png' => [ 'as' => 'image' ], + ] ); + $this->assertSame( + [ + 'Link: <https://example.org/script.js>;rel=preload;as=script,' . + '</example.png>;rel=preload;as=image' + ], + $module->getHeaders( $context ), + 'Preload two resources' + ); + } } diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php index e6f709d..e9d022f 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php @@ -794,4 +794,79 @@ 'startup response sets state to error' ); } + + /** + * Integration test for modules sending extra HTTP response headers. + * + * @covers ResourceLoaderModule::getHeaders + * @covers ResourceLoaderModule::buildContent + * @covers ResourceLoader::makeModuleResponse + */ + public function testMakeModuleResponseExtraHeaders() { + $module = $this->getMockBuilder( ResourceLoaderTestModule::class ) + ->setMethods( [ 'getPreloadLinks' ] )->getMock(); + $module->method( 'getPreloadLinks' )->willReturn( [ + 'https://example.org/script.js' => [ 'as' => 'script' ], + ] ); + + $rl = new EmptyResourceLoader(); + $rl->register( [ + 'foo' => $module, + ] ); + $context = $this->getResourceLoaderContext( + [ 'modules' => 'foo', 'only' => 'scripts' ], + $rl + ); + + $modules = [ 'foo' => $rl->getModule( 'foo' ) ]; + $response = $rl->makeModuleResponse( $context, $modules ); + $extraHeaders = TestingAccessWrapper::newFromObject( $rl )->extraHeaders; + + $this->assertEquals( + [ + 'Link: <https://example.org/script.js>;rel=preload;as=script' + ], + $extraHeaders, + 'Extra headers' + ); + } + + /** + * @covers ResourceLoaderModule::getHeaders + * @covers ResourceLoaderModule::buildContent + * @covers ResourceLoader::makeModuleResponse + */ + public function testMakeModuleResponseExtraHeadersMulti() { + $foo = $this->getMockBuilder( ResourceLoaderTestModule::class ) + ->setMethods( [ 'getPreloadLinks' ] )->getMock(); + $foo->method( 'getPreloadLinks' )->willReturn( [ + 'https://example.org/script.js' => [ 'as' => 'script' ], + ] ); + + $bar = $this->getMockBuilder( ResourceLoaderTestModule::class ) + ->setMethods( [ 'getPreloadLinks' ] )->getMock(); + $bar->method( 'getPreloadLinks' )->willReturn( [ + '/example.png' => [ 'as' => 'image' ], + '/example.jpg' => [ 'as' => 'image' ], + ] ); + + $rl = new EmptyResourceLoader(); + $rl->register( [ 'foo' => $foo, 'bar' => $bar ] ); + $context = $this->getResourceLoaderContext( + [ 'modules' => 'foo|bar', 'only' => 'scripts' ], + $rl + ); + + $modules = [ 'foo' => $rl->getModule( 'foo' ), 'bar' => $rl->getModule( 'bar' ) ]; + $response = $rl->makeModuleResponse( $context, $modules ); + $extraHeaders = TestingAccessWrapper::newFromObject( $rl )->extraHeaders; + $this->assertEquals( + [ + 'Link: <https://example.org/script.js>;rel=preload;as=script', + 'Link: </example.png>;rel=preload;as=image,</example.jpg>;rel=preload;as=image' + ], + $extraHeaders, + 'Extra headers' + ); + } } -- To view, visit https://gerrit.wikimedia.org/r/365888 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2bfe0796ceaa0c82579c501f5b10e931f2175681 Gerrit-PatchSet: 8 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Krinkle <[email protected]> Gerrit-Reviewer: Aaron Schulz <[email protected]> Gerrit-Reviewer: Catrope <[email protected]> Gerrit-Reviewer: Gilles <[email protected]> Gerrit-Reviewer: Krinkle <[email protected]> Gerrit-Reviewer: Legoktm <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
![https://example.org/[email protected]](https://example.org/l...@2x.png){kind=link}