jenkins-bot has submitted this change and it was merged.
Change subject: Cherry-pick I173a9820b and I7c7546ec5
......................................................................
Cherry-pick I173a9820b and I7c7546ec5
Cherry-pick of the following two changes:
* I173a9820b: resourceloader: Store relative instead of absolute paths in
module_deps
* I7c7546ec5: resourceloader: Vary module_deps on language (in addition to skin)
Change-Id: I6651f2f34f489955a143f02b735af1929c20da77
---
M composer.json
M includes/resourceloader/ResourceLoader.php
M includes/resourceloader/ResourceLoaderFileModule.php
M includes/resourceloader/ResourceLoaderModule.php
M maintenance/tables.sql
M tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php
6 files changed, 136 insertions(+), 49 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/composer.json b/composer.json
index 3c526c9..cc261cd 100644
--- a/composer.json
+++ b/composer.json
@@ -29,6 +29,7 @@
"wikimedia/cdb": "1.3.0",
"wikimedia/composer-merge-plugin": "1.2.1",
"wikimedia/ip-set": "1.0.1",
+ "wikimedia/relpath": "1.0.3",
"wikimedia/utfnormal": "1.0.3",
"wikimedia/wrappedstring": "2.0.0",
"zordius/lightncandy": "0.21"
diff --git a/includes/resourceloader/ResourceLoader.php
b/includes/resourceloader/ResourceLoader.php
index d2e6c3c..afa1ee5 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -110,28 +110,27 @@
$skin = $context->getSkin();
$lang = $context->getLanguage();
- // Get file dependency information
+ // Batched version of ResourceLoaderModule::getFileDependencies
+ $vary = "$skin|$lang";
$res = $dbr->select( 'module_deps', array( 'md_module',
'md_deps' ), array(
'md_module' => $modules,
- 'md_skin' => $skin
+ 'md_skin' => $vary,
), __METHOD__
);
-
- // Set modules' dependencies
+ // Prime in-object cache values for each module
$modulesWithDeps = array();
foreach ( $res as $row ) {
$module = $this->getModule( $row->md_module );
if ( $module ) {
- $module->setFileDependencies( $skin,
FormatJson::decode( $row->md_deps, true ) );
+ $module->setFileDependencies( $context,
FormatJson::decode( $row->md_deps, true ) );
$modulesWithDeps[] = $row->md_module;
}
}
-
// Register the absence of a dependency row too
foreach ( array_diff( $modules, $modulesWithDeps ) as $name ) {
$module = $this->getModule( $name );
if ( $module ) {
- $this->getModule( $name )->setFileDependencies(
$skin, array() );
+ $this->getModule( $name )->setFileDependencies(
$context, array() );
}
}
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php
b/includes/resourceloader/ResourceLoaderFileModule.php
index 26e5583..5d762d8 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -416,22 +416,8 @@
$context
);
// Collect referenced files
- $this->localFileRefs = array_unique( $this->localFileRefs );
- // If the list has been modified since last time we cached it,
update the cache
- try {
- if ( $this->localFileRefs !==
$this->getFileDependencies( $context->getSkin() ) ) {
- $dbw = wfGetDB( DB_MASTER );
- $dbw->replace( 'module_deps',
- array( array( 'md_module', 'md_skin' )
), array(
- 'md_module' => $this->getName(),
- 'md_skin' =>
$context->getSkin(),
- 'md_deps' =>
FormatJson::encode( $this->localFileRefs ),
- )
- );
- }
- } catch ( Exception $e ) {
- wfDebugLog( 'resourceloader', __METHOD__ . ": failed to
update DB: $e" );
- }
+ $this->saveFileDependencies( $context, $this->localFileRefs );
+
return $styles;
}
@@ -574,7 +560,7 @@
}
$files = array_map( array( $this, 'getLocalPath' ), $files );
// File deps need to be treated separately because they're
already prefixed
- $files = array_merge( $files, $this->getFileDependencies(
$context->getSkin() ) );
+ $files = array_merge( $files, $this->getFileDependencies(
$context ) );
// Filter out any duplicates from getFileDependencies() and
others.
// Most commonly introduced by compileLessFile(), which always
includes the
// entry point Less file we already know about.
diff --git a/includes/resourceloader/ResourceLoaderModule.php
b/includes/resourceloader/ResourceLoaderModule.php
index 8c88ebe..5b424d2 100644
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -376,32 +376,33 @@
* Get the files this module depends on indirectly for a given skin.
* Currently these are only image files referenced by the module's CSS.
*
- * @param string $skin Skin name
+ * @param ResourceLoaderContext $context
* @return array List of files
*/
- public function getFileDependencies( $skin ) {
+ protected function getFileDependencies( ResourceLoaderContext $context
) {
+ $vary = $context->getSkin() . '|' . $context->getLanguage();
+
// Try in-object cache first
- if ( isset( $this->fileDeps[$skin] ) ) {
- return $this->fileDeps[$skin];
+ if ( !isset( $this->fileDeps[$vary] ) ) {
+ $dbr = wfGetDB( DB_SLAVE );
+ $deps = $dbr->selectField( 'module_deps',
+ 'md_deps',
+ array(
+ 'md_module' => $this->getName(),
+ 'md_skin' => $vary,
+ ),
+ __METHOD__
+ );
+
+ if ( !is_null( $deps ) ) {
+ $this->fileDeps[$vary] =
self::expandRelativePaths(
+ (array)FormatJson::decode( $deps, true )
+ );
+ } else {
+ $this->fileDeps[$vary] = array();
+ }
}
-
- $dbr = wfGetDB( DB_SLAVE );
- $deps = $dbr->selectField( 'module_deps',
- 'md_deps',
- array(
- 'md_module' => $this->getName(),
- 'md_skin' => $skin,
- ),
- __METHOD__
- );
-
- if ( !is_null( $deps ) ) {
- $this->fileDeps[$skin] = (array)FormatJson::decode(
$deps, true );
- } else {
- $this->fileDeps[$skin] = array();
- }
-
- return $this->fileDeps[$skin];
+ return $this->fileDeps[$vary];
}
/**
@@ -410,8 +411,71 @@
* @param string $skin Skin name
* @param array $deps Array of file names
*/
- public function setFileDependencies( $skin, $deps ) {
- $this->fileDeps[$skin] = $deps;
+ public function setFileDependencies( ResourceLoaderContext $context,
$files ) {
+ $vary = $context->getSkin() . '|' . $context->getLanguage();
+ $this->fileDeps[$vary] = $files;
+ }
+
+ /**
+ * Set the files this module depends on indirectly for a given skin.
+ *
+ * @since 1.26
+ * @param ResourceLoaderContext $context
+ * @param array $localFileRefs List of files
+ */
+ protected function saveFileDependencies( ResourceLoaderContext
$context, $localFileRefs ) {
+ // Normalise array
+ $localFileRefs = array_values( array_unique( $localFileRefs ) );
+ sort( $localFileRefs );
+
+ try {
+ // If the list has been modified since last time we
cached it, update the cache
+ if ( $localFileRefs !== $this->getFileDependencies(
$context ) ) {
+ $vary = $context->getSkin() . '|' .
$context->getLanguage();
+ $dbw = wfGetDB( DB_MASTER );
+ $dbw->replace( 'module_deps',
+ array( array( 'md_module', 'md_skin' )
), array(
+ 'md_module' => $this->getName(),
+ 'md_skin' => $vary,
+ // Use relative paths to avoid
ghost entries when $IP changes (T111481)
+ 'md_deps' =>
FormatJson::encode( self::getRelativePaths( $localFileRefs ) ),
+ )
+ );
+ }
+ } catch ( Exception $e ) {
+ wfDebugLog( 'resourceloader', __METHOD__ . ": failed to
update DB: $e" );
+ }
+ }
+
+ /**
+ * Make file paths relative to MediaWiki directory.
+ *
+ * This is used to make file paths safe for storing in a database
without the paths
+ * becoming stale or incorrect when MediaWiki is moved or upgraded
(T111481).
+ *
+ * @since 1.26
+ * @param array $filePaths
+ * @return array
+ */
+ protected static function getRelativePaths( Array $filePaths ) {
+ global $IP;
+ return array_map( function ( $path ) use ( $IP ) {
+ return RelPath\getRelativePath( $path, $IP );
+ }, $filePaths );
+ }
+
+ /**
+ * Expand directories relative to $IP.
+ *
+ * @since 1.26
+ * @param array $filePaths
+ * @return array
+ */
+ protected static function expandRelativePaths( Array $filePaths ) {
+ global $IP;
+ return array_map( function ( $path ) use ( $IP ) {
+ return RelPath\joinPath( $IP, $path );
+ }, $filePaths );
}
/**
diff --git a/maintenance/tables.sql b/maintenance/tables.sql
index aa0c7ea..4b58b60 100644
--- a/maintenance/tables.sql
+++ b/maintenance/tables.sql
@@ -1524,7 +1524,7 @@
CREATE TABLE /*_*/module_deps (
-- Module name
md_module varbinary(255) NOT NULL,
- -- Skin name
+ -- Module context vary (includes skin and language; called "md_skin" for
legacy reasons)
md_skin varbinary(32) NOT NULL,
-- JSON blob with file dependencies
md_deps mediumblob NOT NULL
diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php
b/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php
index 41653fb..307e311 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php
@@ -88,4 +88,41 @@
'Leave valid scripts as-is'
);
}
+
+ /**
+ * @covers ResourceLoaderModule::getRelativePaths
+ * @covers ResourceLoaderModule::expandRelativePaths
+ */
+ public function testPlaceholderize() {
+ $getRelativePaths = new ReflectionMethod(
'ResourceLoaderModule', 'getRelativePaths' );
+ $getRelativePaths->setAccessible( true );
+ $expandRelativePaths = new ReflectionMethod(
'ResourceLoaderModule', 'expandRelativePaths' );
+ $expandRelativePaths->setAccessible( true );
+
+ $this->setMwGlobals( array(
+ 'IP' => '/srv/example/mediawiki/core',
+ ) );
+ $raw = array(
+ '/srv/example/mediawiki/core/resources/foo.js',
+
'/srv/example/mediawiki/core/extensions/Example/modules/bar.js',
+ '/srv/example/mediawiki/skins/Example/baz.css',
+
'/srv/example/mediawiki/skins/Example/images/quux.png',
+ );
+ $canonical = array(
+ 'resources/foo.js',
+ 'extensions/Example/modules/bar.js',
+ '../skins/Example/baz.css',
+ '../skins/Example/images/quux.png',
+ );
+ $this->assertEquals(
+ $getRelativePaths->invoke( null, $raw ),
+ $canonical,
+ 'Insert placeholders'
+ );
+ $this->assertEquals(
+ $expandRelativePaths->invoke( null, $canonical ),
+ $raw,
+ 'Substitute placeholders'
+ );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/242429
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6651f2f34f489955a143f02b735af1929c20da77
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.26wmf24
Gerrit-Owner: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: Daniel Friesen <dan...@nadir-seen-fire.com>
Gerrit-Reviewer: Jjanes <jeff.ja...@gmail.com>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: Parent5446 <tylerro...@gmail.com>
Gerrit-Reviewer: Springle <sprin...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
