ResourcesTest: Detect missing files in url(...) expressions

The way this is implemented is really dirty... but it found us
a few pre-existing bugs already (T111518, T111519, T111771).
I think it might be worth it.

* CSSMin: Add new method getAllLocalFileReferences() which skips the
  file_exists() check.
* ResourceLoaderModule: Make use of it to track missing files too.
* ResourcesTest: Verify that the missing files are missing.

Change-Id: I5a3cdeb7d53485f161ccf8133e76850cdf5b4579
This commit is contained in:
Bartosz Dziewoński 2015-09-03 23:30:36 +02:00
parent 3a3c3e73e7
commit 8f5cd11d82
3 changed files with 60 additions and 16 deletions

View file

@ -60,13 +60,15 @@ class CSSMin {
/* Static Methods */
/**
* Gets a list of local file paths which are referenced in a CSS style sheet
* Gets a list of local file paths which are referenced in a CSS style sheet.
*
* This function will always return an empty array if the second parameter is not given or null
* for backwards-compatibility.
* If you wish non-existent files to be listed too, use getAllLocalFileReferences().
*
* @param string $source CSS data to remap
* @param string $path File path where the source was read from (optional)
* For backwards-compatibility, if the second parameter is not given or null,
* this function will return an empty array instead of erroring out.
*
* @param string $source CSS stylesheet source to process
* @param string $path File path where the source was read from
* @return array List of local file references
*/
public static function getLocalFileReferences( $source, $path = null ) {
@ -74,6 +76,25 @@ class CSSMin {
return array();
}
$files = self::getAllLocalFileReferences( $source, $path );
// Skip non-existent files
$files = array_filter( $files, function ( $file ) {
return file_exists( $file );
} );
return $files;
}
/**
* Gets a list of local file paths which are referenced in a CSS style sheet, including
* non-existent files.
*
* @param string $source CSS stylesheet source to process
* @param string $path File path where the source wa
* @return array List of local file references
*/
public static function getAllLocalFileReferences( $source, $path ) {
$path = rtrim( $path, '/' ) . '/';
$files = array();
@ -87,13 +108,7 @@ class CSSMin {
break;
}
$file = $path . $url;
// Skip non-existent files
if ( file_exists( $file ) ) {
break;
}
$files[] = $file;
$files[] = $path . $url;
}
}
return $files;

View file

@ -152,6 +152,12 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
*/
protected $localFileRefs = array();
/**
* @var array Place where readStyleFile() tracks file dependencies for non-existent files.
* Used in tests to detect missing dependencies.
*/
protected $missingLocalFileRefs = array();
/* Methods */
/**
@ -917,10 +923,14 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
$localDir = dirname( $localPath );
$remoteDir = dirname( $remotePath );
// Get and register local file references
$this->localFileRefs = array_merge(
$this->localFileRefs,
CSSMin::getLocalFileReferences( $style, $localDir )
);
$localFileRefs = CSSMin::getAllLocalFileReferences( $style, $localDir );
foreach ( $localFileRefs as $file ) {
if ( file_exists( $file ) ) {
$this->localFileRefs[] = $file;
} else {
$this->missingLocalFileRefs[] = $file;
}
}
return CSSMin::remap(
$style, $localDir, $remoteDir, true
);

View file

@ -276,6 +276,25 @@ class ResourcesTest extends MediaWikiTestCase {
( $file instanceof ResourceLoaderFilePath ? $file->getPath() : $file ),
);
}
// To populate missingLocalFileRefs. Not sure how sane this is inside this test...
$module->readStyleFiles(
$module->getStyleFiles( $data['context'] ),
$module->getFlip( $data['context'] ),
$data['context']
);
$property = $reflectedModule->getProperty( 'missingLocalFileRefs' );
$property->setAccessible( true );
$missingLocalFileRefs = $property->getValue( $module );
foreach ( $missingLocalFileRefs as $file ) {
$cases[] = array(
$file,
$moduleName,
$file,
);
}
}
return $cases;