From b632900f4da2d0f15a053c86a213232b1249488e Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 17 Nov 2015 21:11:19 +0000 Subject: [PATCH] resourceloader: Omit getDirection() ResourceLoaderContext hash The direction is derived from the language code, which is included already. The method was added for convenience to consuming code, but including it in the cache key seems pointless. Main rationale here is runtime performance. getDirection() incurs Language::factory() and Language::getDir() which require loading of LCStore files. Change-Id: I397a1c483203ec2c4903046c9494cae1c9480f8c --- includes/resourceloader/ResourceLoaderContext.php | 10 +++++++++- .../DerivativeResourceLoaderContextTest.php | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php index 6c4cdfee97..6ecee4c6de 100644 --- a/includes/resourceloader/ResourceLoaderContext.php +++ b/includes/resourceloader/ResourceLoaderContext.php @@ -333,12 +333,20 @@ class ResourceLoaderContext { } /** + * All factors that uniquely identify this request, except 'modules'. + * + * The list of modules is excluded here for legacy reasons as most callers already + * split up handling of individual modules. Including it here would massively fragment + * the cache and decrease its usefulness. + * + * E.g. Used by RequestFileCache to form a cache key for storing the reponse output. + * * @return string */ public function getHash() { if ( !isset( $this->hash ) ) { $this->hash = implode( '|', array( - $this->getLanguage(), $this->getDirection(), $this->getSkin(), $this->getUser(), + $this->getLanguage(), $this->getSkin(), $this->getUser(), $this->getImage(), $this->getVariant(), $this->getFormat(), $this->getDebug(), $this->getOnly(), $this->getVersion() ) ); diff --git a/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php b/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php index 0d11f6210a..b457ceca70 100644 --- a/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php +++ b/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php @@ -25,7 +25,7 @@ class DerivativeResourceLoaderContextTest extends PHPUnit_Framework_TestCase { $this->assertEquals( $derived->getModules(), array( 'test.context' ) ); $this->assertEquals( $derived->getOnly(), 'scripts' ); $this->assertEquals( $derived->getSkin(), 'fallback' ); - $this->assertEquals( $derived->getHash(), 'zh|ltr|fallback||||||scripts|' ); + $this->assertEquals( $derived->getHash(), 'zh|fallback||||||scripts|' ); } public function testSetLanguage() { @@ -72,7 +72,7 @@ class DerivativeResourceLoaderContextTest extends PHPUnit_Framework_TestCase { $derived->setLanguage( 'nl' ); // Assert that subclass is able to clear parent class "hash" member - $this->assertEquals( $derived->getHash(), 'nl|ltr|fallback||||||scripts|' ); + $this->assertEquals( $derived->getHash(), 'nl|fallback||||||scripts|' ); } } -- 2.20.1