Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

resourceloader: Tweak RL\FilePath handling in packageFiles

Browse source 
ResourceLoader's FilePath is designed to allow a FileModule
to include files that exist outside of the module's localBasePath,
to allow skins and extensions to extend core MediaWiki modules.
This is accomplished by having the FileModule use the FilePath's
localBasePath instead, in FileModule::getLocalPath.
(Similarly for remoteBasePath, but these are going out of fashion.)

However, the code processing 'packageFiles' did not handle this right:
it used FilePath's localBasePath when it appeared as a 'file',
but not when it was returned by a 'callback' or 'versionCallback',
using the FileModule's localBasePath instead in that case. Most
existing uses of FilePath in 'packageFiles' required the same base
path as the module, so it was convenient to avoid providing it twice.

To keep that convenience (already relied on by some extensions too)
while also allowing skins and extensions to add files from their own
directories to existing modules, the code processing 'packageFiles'
now uses FilePath's base paths in all cases, but they are optional,
and will fall back to the FileModule's paths when not provided.

Follow-up to 2890bca27d.

Change-Id: I38a0761ae4633a567b685b52c1d73b6ce280ffb7
This commit is contained in:
Bartosz Dziewoński 2022-05-24 02:42:21 +02:00 committed by Timo Tijhof
parent 2c4635413e
commit 566b185e9c
6 changed files with 88 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
includes/ResourceLoader/FileModule.php
View file

@ -704,7 +704,10 @@ class FileModule extends Module {
*/
protected function getLocalPath( $path ) {
if ( $path instanceof FilePath ) {
return $path->getLocalPath();
if ( $path->getLocalBasePath() !== null ) {
return $path->getLocalPath();
}
$path = $path->getPath();
}
return "{$this->localBasePath}/$path";
@ -716,7 +719,10 @@ class FileModule extends Module {
*/
protected function getRemotePath( $path ) {
if ( $path instanceof FilePath ) {
return $path->getRemotePath();
if ( $path->getRemoteBasePath() !== null ) {
return $path->getRemotePath();
}
$path = $path->getPath();
}
if ( $this->remoteBasePath === '/' ) {
@ -1312,7 +1318,7 @@ class FileModule extends Module {
$expanded['callbackParam']
);
if ( $callbackResult instanceof FilePath ) {
$expanded['filePath'] = $callbackResult->getPath();
$expanded['filePath'] = $callbackResult;
} else {
$expanded['definitionSummary'] = $callbackResult;
}
@ -1326,7 +1332,7 @@ class FileModule extends Module {
$expanded['callbackParam']
);
if ( $callbackResult instanceof FilePath ) {
$expanded['filePath'] = $callbackResult->getPath();
$expanded['filePath'] = $callbackResult;
} else {
$expanded['content'] = $callbackResult;
}
@ -1405,7 +1411,7 @@ class FileModule extends Module {
);
if ( $callbackResult instanceof FilePath ) {
// Fall through to the filePath handling code below
$fileInfo['filePath'] = $callbackResult->getPath();
$fileInfo['filePath'] = $callbackResult;
} else {
$fileInfo['content'] = $callbackResult;
}

45
includes/ResourceLoader/FilePath.php
View file

@ -20,18 +20,23 @@
namespace MediaWiki\ResourceLoader;
use RuntimeException;
/**
* An object to represent a path to a JavaScript/CSS file, along with a remote
* and local base path, for use with ResourceLoaderFileModule.
* and local base path, for use with FileModule.
*
* Null base path indicates that the corresponding base path from FileModule
* should be used.
*
* @ingroup ResourceLoader
* @since 1.17
*/
class FilePath {
/** @var string Local base path */
/** @var string|null Local base path */
protected $localBasePath;
/** @var string Remote base path */
/** @var string|null Remote base path */
protected $remoteBasePath;
/** @var string Path to the file */
@ -39,28 +44,36 @@ class FilePath {
/**
* @param string $path Relative path to the file, no leading slash.
* @param string $localBasePath Base path to prepend when generating a local path.
* @param string $remoteBasePath Base path to prepend when generating a remote path.
* @param string|null $localBasePath Base path to prepend when generating a local path.
* @param string|null $remoteBasePath Base path to prepend when generating a remote path.
* Should not have a trailing slash unless at web document root.
*/
public function __construct( $path, $localBasePath = '', $remoteBasePath = '' ) {
public function __construct( $path, $localBasePath = null, $remoteBasePath = null ) {
$this->path = $path;
$this->localBasePath = $localBasePath;
$this->remoteBasePath = $remoteBasePath;
}
/** @return string */
/**
* @return string
* @throws RuntimeException If the base path was not provided. You must either provide the base
* path in the constructor, or use getPath() instead and add the base path from a FileModule.
*/
public function getLocalPath() {
return $this->localBasePath === '' ?
$this->path :
"{$this->localBasePath}/{$this->path}";
if ( $this->localBasePath === null ) {
throw new RuntimeException( 'Base path was not provided' );
}
return "{$this->localBasePath}/{$this->path}";
}
/** @return string */
/**
* @return string
* @throws RuntimeException If the base path was not provided. You must either provide the base
* path in the constructor, or use getPath() instead and add the base path from a FileModule.
*/
public function getRemotePath() {
if ( $this->remoteBasePath === '' ) {
// No base path configured
return $this->path;
if ( $this->remoteBasePath === null ) {
throw new RuntimeException( 'Base path was not provided' );
}
if ( $this->remoteBasePath === '/' ) {
// In document root
@ -70,12 +83,12 @@ class FilePath {
return "{$this->remoteBasePath}/{$this->path}";
}
/** @return string */
/** @return string|null */
public function getLocalBasePath() {
return $this->localBasePath;
}
/** @return string */
/** @return string|null */
public function getRemoteBasePath() {
return $this->remoteBasePath;
}

2
tests/phpunit/data/resourceloader-b/script-nosemi.js Normal file
View file

@ -0,0 +1,2 @@
/* eslint-disable */
mw.bar()

20
tests/phpunit/includes/ResourceLoader/FileModuleTest.php
View file

@ -634,9 +634,11 @@ class FileModuleTest extends ResourceLoaderTestCase {
public function provideGetScriptPackageFiles() {
$basePath = __DIR__ . '/../../data/resourceloader';
$basePathB = __DIR__ . '/../../data/resourceloader-b';
$base = [ 'localBasePath' => $basePath ];
$commentScript = file_get_contents( "$basePath/script-comment.js" );
$nosemiScript = file_get_contents( "$basePath/script-nosemi.js" );
$nosemiBScript = file_get_contents( "$basePathB/script-nosemi.js" );
$vueComponentDebug = trim( file_get_contents( "$basePath/vue-component-output-debug.js.txt" ) );
$vueComponentNonDebug = trim( file_get_contents( "$basePath/vue-component-output-nondebug.js.txt" ) );
$config = \MediaWiki\MediaWikiServices::getInstance()->getMainConfig();
@ -838,6 +840,24 @@ class FileModuleTest extends ResourceLoaderTestCase {
'lang' => 'nl'
]
],
'package file with callback that returns a file with base path' => [
$base + [
'packageFiles' => [
[ 'name' => 'dynamic.js', 'callback' => static function () use ( $basePathB ) {
return new FilePath( 'script-nosemi.js', $basePathB );
} ]
]
],
[
'files' => [
'dynamic.js' => [
'type' => 'script',
'content' => $nosemiBScript,
]
],
'main' => 'dynamic.js'
]
],
'.vue file in debug mode' => [
$base + [
'packageFiles' => [

30
tests/phpunit/unit/includes/ResourceLoader/FilePathTest.php
View file

@ -4,6 +4,7 @@ namespace MediaWiki\Tests\Unit\ResourceLoader;
use MediaWiki\ResourceLoader\FilePath;
use MediaWikiUnitTestCase;
use RuntimeException;
/**
* @covers \MediaWiki\ResourceLoader\FilePath
@ -38,13 +39,30 @@ class FilePathTest extends MediaWikiUnitTestCase {
}
public function testGetterNoBase() {
$path = new FilePath( 'dummy/path', '', '' );
$path = new FilePath( 'dummy/path' );
// No transformation
$this->assertSame( 'dummy/path', $path->getLocalPath() );
$this->assertSame( 'dummy/path', $path->getRemotePath() );
$this->assertSame( '', $path->getLocalBasePath() );
$this->assertSame( '', $path->getRemoteBasePath() );
try {
$path->getLocalPath();
$this->fail( 'Expected exception not thrown' );
} catch ( RuntimeException $ex ) {
$this->assertSame(
'Base path was not provided',
$ex->getMessage(),
'Expected exception'
);
}
try {
$path->getRemotePath();
$this->fail( 'Expected exception not thrown' );
} catch ( RuntimeException $ex ) {
$this->assertSame(
'Base path was not provided',
$ex->getMessage(),
'Expected exception'
);
}
$this->assertSame( null, $path->getLocalBasePath() );
$this->assertSame( null, $path->getRemoteBasePath() );
$this->assertSame( 'dummy/path', $path->getPath() );
}
}

4
tests/qunit/QUnitTestResources.php
View file

@ -29,11 +29,11 @@ return [
'packageFiles' => [
[
'name' => 'mediawiki.jqueryMsg.testdata.js',
'file' => new FilePath( __DIR__ . '/data/mediawiki.jqueryMsg.testdata.js' ),
'file' => new FilePath( __DIR__ . '/data/mediawiki.jqueryMsg.testdata.js', '' ),
],
[
'name' => 'mediawiki.jqueryMsg.data.json',
'file' => new FilePath( __DIR__ . '/data/mediawiki.jqueryMsg.data.json' ),
'file' => new FilePath( __DIR__ . '/data/mediawiki.jqueryMsg.data.json', '' ),
],
'bs.js',
'dsb.js',