From: Aaron Schulz Date: Mon, 19 Sep 2016 01:39:59 +0000 (-0700) Subject: Migrate callers to new MWFileProps::getPropsFromPath() method X-Git-Tag: 1.31.0-rc.0~5404^2 X-Git-Url: http://git.cyclocoop.org/%24dirpuce/puce%24spip_lang_rtl.gif?a=commitdiff_plain;h=ff5abb66b4a5109290d27de8b2a91b4ccd3a730c;p=lhc%2Fweb%2Fwiklou.git Migrate callers to new MWFileProps::getPropsFromPath() method * FSFile should not be responsible for handling this much logic. * Make more MediaHandler classes aware of the fact that an object other than File might be passed in. Use the FSFile instead of a useless empty stdClass object. * Also added more fields to FSFile::placeholderProps to make it more complete. Change-Id: I9fe764b2a7261af507c6555e6a57273cf7d00d36 --- diff --git a/autoload.php b/autoload.php index 0c16e7625b..f5c334b37a 100644 --- a/autoload.php +++ b/autoload.php @@ -773,6 +773,7 @@ $wgAutoloadLocalClasses = [ 'MWException' => __DIR__ . '/includes/exception/MWException.php', 'MWExceptionHandler' => __DIR__ . '/includes/exception/MWExceptionHandler.php', 'MWExceptionRenderer' => __DIR__ . '/includes/exception/MWExceptionRenderer.php', + 'MWFileProps' => __DIR__ . '/includes/utils/MWFileProps.php', 'MWGrants' => __DIR__ . '/includes/utils/MWGrants.php', 'MWHttpRequest' => __DIR__ . '/includes/HttpFunctions.php', 'MWMemcached' => __DIR__ . '/includes/compat/MemcachedClientCompat.php', diff --git a/includes/MimeMagic.php b/includes/MimeMagic.php index a432d44c9a..5551865227 100644 --- a/includes/MimeMagic.php +++ b/includes/MimeMagic.php @@ -1046,6 +1046,7 @@ class MimeMagic { } } + $type = null; // Check for entry for full MIME type if ( $mime ) { $type = $this->findMediaType( $mime ); diff --git a/includes/filebackend/FSFile.php b/includes/filebackend/FSFile.php index 221fe9fce2..e7e2608ce8 100644 --- a/includes/filebackend/FSFile.php +++ b/includes/filebackend/FSFile.php @@ -112,37 +112,30 @@ class FSFile { $info['fileExists'] = $this->exists(); if ( $info['fileExists'] ) { + $info['size'] = $this->getSize(); // bytes + $info['sha1'] = $this->getSha1Base36(); + // @TODO: replace the code below with bare FileInfo use so this can go in /libs $magic = MimeMagic::singleton(); - # get the file extension - if ( $ext === true ) { - $ext = self::extensionFromPath( $this->path ); - } - # MIME type according to file contents $info['file-mime'] = $magic->guessMimeType( $this->path, false ); - # logical MIME type + # Logical MIME type + $ext = ( $ext === true ) ? FileBackend::extensionFromPath( $this->path ) : $ext; $info['mime'] = $magic->improveTypeFromExtension( $info['file-mime'], $ext ); list( $info['major_mime'], $info['minor_mime'] ) = File::splitMime( $info['mime'] ); $info['media_type'] = $magic->getMediaType( $this->path, $info['mime'] ); - # Get size in bytes - $info['size'] = $this->getSize(); - # Height, width and metadata $handler = MediaHandler::getHandler( $info['mime'] ); if ( $handler ) { - $tempImage = (object)[]; // XXX (hack for File object) - /** @noinspection PhpParamsInspection */ - $info['metadata'] = $handler->getMetadata( $tempImage, $this->path ); - /** @noinspection PhpParamsInspection */ - $gis = $handler->getImageSize( $tempImage, $this->path, $info['metadata'] ); + $info['metadata'] = $handler->getMetadata( $this, $this->path ); + /** @noinspection PhpMethodParametersCountMismatchInspection */ + $gis = $handler->getImageSize( $this, $this->path, $info['metadata'] ); if ( is_array( $gis ) ) { $info = $this->extractImageSizeInfo( $gis ) + $info; } } - $info['sha1'] = $this->getSha1Base36(); } return $info; @@ -153,7 +146,11 @@ class FSFile { * * Resulting array fields include: * - fileExists + * - size + * - file-mime (as major/minor) * - mime (as major/minor) + * - major_mime + * - minor_mime * - media_type (value to be used with the MEDIATYPE_xxx constants) * - metadata (handler specific) * - sha1 (in base 36) @@ -166,6 +163,10 @@ class FSFile { public static function placeholderProps() { $info = []; $info['fileExists'] = false; + $info['size'] = 0; + $info['file-mime'] = null; + $info['major_mime'] = null; + $info['minor_mime'] = null; $info['mime'] = null; $info['media_type'] = MEDIATYPE_UNKNOWN; $info['metadata'] = ''; diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php index 8fee3bfa74..b5c5bf33fb 100644 --- a/includes/filerepo/FileRepo.php +++ b/includes/filerepo/FileRepo.php @@ -1539,9 +1539,15 @@ class FileRepo { * @return array */ public function getFileProps( $virtualUrl ) { - $path = $this->resolveToStoragePath( $virtualUrl ); + $fsFile = $this->getLocalReference( $virtualUrl ); + if ( $fsFile ) { + $mwProps = new MWFileProps( MimeMagic::singleton() ); + $props = $mwProps->getPropsFromPath( $fsFile->getPath(), true ); + } else { + $props = FSFile::placeholderProps(); + } - return $this->backend->getFileProps( [ 'src' => $path ] ); + return $props; } /** diff --git a/includes/filerepo/RepoGroup.php b/includes/filerepo/RepoGroup.php index bd32de04e1..d47624f8cd 100644 --- a/includes/filerepo/RepoGroup.php +++ b/includes/filerepo/RepoGroup.php @@ -452,7 +452,9 @@ class RepoGroup { return $repo->getFileProps( $fileName ); } else { - return FSFile::getPropsFromPath( $fileName ); + $mwProps = new MWFileProps( MimeMagic::singleton() ); + + return $mwProps->getPropsFromPath( $fileName, true ); } } diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 396b47cc3c..60cfdac271 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -1179,7 +1179,8 @@ class LocalFile extends File { ) { $props = $this->repo->getFileProps( $srcPath ); } else { - $props = FSFile::getPropsFromPath( $srcPath ); + $mwProps = new MWFileProps( MimeMagic::singleton() ); + $props = $mwProps->getPropsFromPath( $srcPath, true ); } } diff --git a/includes/media/BMP.php b/includes/media/BMP.php index 4b9b268257..0229ac11b7 100644 --- a/includes/media/BMP.php +++ b/includes/media/BMP.php @@ -51,7 +51,7 @@ class BmpHandler extends BitmapHandler { /** * Get width and height from the bmp header. * - * @param File $image + * @param File|FSFile $image * @param string $filename * @return array */ diff --git a/includes/media/DjVu.php b/includes/media/DjVu.php index 9add138da0..18f75ece66 100644 --- a/includes/media/DjVu.php +++ b/includes/media/DjVu.php @@ -235,7 +235,7 @@ class DjVuHandler extends ImageHandler { /** * Cache an instance of DjVuImage in an Image object, return that instance * - * @param File $image + * @param File|FSFile $image * @param string $path * @return DjVuImage */ @@ -335,11 +335,6 @@ class DjVuHandler extends ImageHandler { } } - /** - * @param File $image - * @param string $path - * @return bool|array False on failure - */ function getImageSize( $image, $path ) { return $this->getDjVuImage( $image, $path )->getImageSize(); } diff --git a/includes/media/ExifBitmap.php b/includes/media/ExifBitmap.php index 732be3d672..7aeefa02e4 100644 --- a/includes/media/ExifBitmap.php +++ b/includes/media/ExifBitmap.php @@ -165,7 +165,7 @@ class ExifBitmapHandler extends BitmapHandler { * Wrapper for base classes ImageHandler::getImageSize() that checks for * rotation reported from metadata and swaps the sizes to match. * - * @param File $image + * @param File|FSFile $image * @param string $path * @return array */ diff --git a/includes/media/MediaHandler.php b/includes/media/MediaHandler.php index 70a43f2576..4ca2663be6 100644 --- a/includes/media/MediaHandler.php +++ b/includes/media/MediaHandler.php @@ -100,21 +100,22 @@ abstract class MediaHandler { * @note If this is a multipage file, return the width and height of the * first page. * - * @param File $image The image object, or false if there isn't one + * @param File|FSFile $image The image object, or false if there isn't one. + * Warning, FSFile::getPropsFromPath might pass an FSFile instead of File (!) * @param string $path The filename - * @return array Follow the format of PHP getimagesize() internal function. + * @return array|bool Follow the format of PHP getimagesize() internal function. * See http://www.php.net/getimagesize. MediaWiki will only ever use the * first two array keys (the width and height), and the 'bits' associative * key. All other array keys are ignored. Returning a 'bits' key is optional - * as not all formats have a notion of "bitdepth". + * as not all formats have a notion of "bitdepth". Returns false on failure. */ abstract function getImageSize( $image, $path ); /** * Get handler-specific metadata which will be saved in the img_metadata field. * - * @param File $image The image object, or false if there isn't one. - * Warning, FSFile::getPropsFromPath might pass an (object)array() instead (!) + * @param File|FSFile $image The image object, or false if there isn't one. + * Warning, FSFile::getPropsFromPath might pass an FSFile instead of File (!) * @param string $path The filename * @return string A string of metadata in php serialized form (Run through serialize()) */ diff --git a/includes/media/PNG.php b/includes/media/PNG.php index 8a3e001128..294abb350d 100644 --- a/includes/media/PNG.php +++ b/includes/media/PNG.php @@ -30,7 +30,7 @@ class PNGHandler extends BitmapHandler { const BROKEN_FILE = '0'; /** - * @param File $image + * @param File|FSFile $image * @param string $filename * @return string */ diff --git a/includes/media/SVG.php b/includes/media/SVG.php index 2bb6d139f2..8360920456 100644 --- a/includes/media/SVG.php +++ b/includes/media/SVG.php @@ -301,13 +301,13 @@ class SvgHandler extends ImageHandler { } /** - * @param File $file + * @param File|FSFile $file * @param string $path Unused * @param bool|array $metadata * @return array */ function getImageSize( $file, $path, $metadata = false ) { - if ( $metadata === false ) { + if ( $metadata === false && $file instanceof File ) { $metadata = $file->getMetadata(); } $metadata = $this->unpackMetadata( $metadata ); @@ -355,7 +355,7 @@ class SvgHandler extends ImageHandler { } /** - * @param File $file + * @param File|FSFile $file * @param string $filename * @return string Serialised metadata */ diff --git a/includes/media/Tiff.php b/includes/media/Tiff.php index 2e732493bf..f0f4cdad6c 100644 --- a/includes/media/Tiff.php +++ b/includes/media/Tiff.php @@ -71,13 +71,14 @@ class TiffHandler extends ExifBitmapHandler { } /** - * @param File $image + * @param File|FSFile $image * @param string $filename * @throws MWException * @return string */ function getMetadata( $image, $filename ) { global $wgShowEXIF; + if ( $wgShowEXIF ) { try { $meta = BitmapMetadataHandler::Tiff( $filename ); diff --git a/includes/media/WebP.php b/includes/media/WebP.php index 35e885f1ca..e2c2d2d192 100644 --- a/includes/media/WebP.php +++ b/includes/media/WebP.php @@ -230,7 +230,7 @@ class WebPHandler extends BitmapHandler { if ( $file === null ) { $metadata = self::getMetadata( $file, $path ); } - if ( $metadata === false ) { + if ( $metadata === false && $file instanceof File ) { $metadata = $file->getMetadata(); } diff --git a/includes/media/XCF.php b/includes/media/XCF.php index 6ac675e8fb..108d6fbf1a 100644 --- a/includes/media/XCF.php +++ b/includes/media/XCF.php @@ -56,7 +56,7 @@ class XCFHandler extends BitmapHandler { /** * Get width and height from the XCF header. * - * @param File $image + * @param File|FSFile $image * @param string $filename * @return array */ @@ -149,7 +149,7 @@ class XCFHandler extends BitmapHandler { * * Greyscale files need different command line options. * - * @param File $file The image object, or false if there isn't one. + * @param File|FSFile $file The image object, or false if there isn't one. * Warning, FSFile::getPropsFromPath might pass an (object)array() instead (!) * @param string $filename The filename * @return string diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 1be5c24fe4..91b4133c5d 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -446,7 +446,8 @@ abstract class UploadBase { return $status; } - $this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension ); + $mwProps = new MWFileProps( MimeMagic::singleton() ); + $this->mFileProps = $mwProps->getPropsFromPath( $this->mTempPath, $this->mFinalExtension ); $mime = $this->mFileProps['mime']; if ( $wgVerifyMimeType ) { @@ -504,7 +505,8 @@ abstract class UploadBase { # getTitle() sets some internal parameters like $this->mFinalExtension $this->getTitle(); - $this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension ); + $mwProps = new MWFileProps( MimeMagic::singleton() ); + $this->mFileProps = $mwProps->getPropsFromPath( $this->mTempPath, $this->mFinalExtension ); # check MIME type, if desired $mime = $this->mFileProps['file-mime']; diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php index 000c6a4954..b7160b3ba8 100644 --- a/includes/upload/UploadStash.php +++ b/includes/upload/UploadStash.php @@ -207,7 +207,9 @@ class UploadStash { wfDebug( __METHOD__ . " tried to stash file at '$path', but it doesn't exist\n" ); throw new UploadStashBadPathException( "path doesn't exist" ); } - $fileProps = FSFile::getPropsFromPath( $path ); + + $mwProps = new MWFileProps( MimeMagic::singleton() ); + $fileProps = $mwProps->getPropsFromPath( $path, true ); wfDebug( __METHOD__ . " stashing file at '$path'\n" ); // we will be initializing from some tmpnam files that don't have extensions. diff --git a/includes/utils/MWFileProps.php b/includes/utils/MWFileProps.php new file mode 100644 index 0000000000..b8ecd4c0c3 --- /dev/null +++ b/includes/utils/MWFileProps.php @@ -0,0 +1,115 @@ +magic = $magic; + } + + /** + * Get an associative array containing information about + * a file with the given storage path. + * + * Resulting array fields include: + * - fileExists + * - size (filesize in bytes) + * - mime (as major/minor) + * - media_type (value to be used with the MEDIATYPE_xxx constants) + * - metadata (handler specific) + * - sha1 (in base 36) + * - width + * - height + * - bits (bitrate) + * - file-mime + * - major_mime + * - minor_mime + * + * @param string $path Filesystem path to a file + * @param string|bool $ext The file extension, or true to extract it from the filename. + * Set it to false to ignore the extension. + * @return array + * @since 1.28 + */ + public function getPropsFromPath( $path, $ext ) { + $fsFile = new FSFile( $path ); + + $info = FSFile::placeholderProps(); + $info['fileExists'] = $fsFile->exists(); + if ( $info['fileExists'] ) { + $info['size'] = $fsFile->getSize(); // bytes + $info['sha1'] = $fsFile->getSha1Base36(); + + # MIME type according to file contents + $info['file-mime'] = $this->magic->guessMimeType( $path, false ); + # Logical MIME type + $ext = ( $ext === true ) ? FileBackend::extensionFromPath( $path ) : $ext; + $info['mime'] = $this->magic->improveTypeFromExtension( $info['file-mime'], $ext ); + + list( $info['major_mime'], $info['minor_mime'] ) = File::splitMime( $info['mime'] ); + $info['media_type'] = $this->magic->getMediaType( $path, $info['mime'] ); + + # Height, width and metadata + $handler = MediaHandler::getHandler( $info['mime'] ); + if ( $handler ) { + $info['metadata'] = $handler->getMetadata( $fsFile, $path ); + /** @noinspection PhpMethodParametersCountMismatchInspection */ + $gis = $handler->getImageSize( $fsFile, $path, $info['metadata'] ); + if ( is_array( $gis ) ) { + $info = $this->extractImageSizeInfo( $gis ) + $info; + } + } + } + + return $info; + } + + /** + * Exract image size information + * + * @param array $gis + * @return array + */ + private function extractImageSizeInfo( array $gis ) { + $info = []; + # NOTE: $gis[2] contains a code for the image type. This is no longer used. + $info['width'] = $gis[0]; + $info['height'] = $gis[1]; + if ( isset( $gis['bits'] ) ) { + $info['bits'] = $gis['bits']; + } else { + $info['bits'] = 0; + } + + return $info; + } +} diff --git a/maintenance/importImages.php b/maintenance/importImages.php index ac700ef010..5a4ab39403 100644 --- a/maintenance/importImages.php +++ b/maintenance/importImages.php @@ -245,7 +245,8 @@ if ( $count > 0 ) { if ( isset( $options['dry'] ) ) { echo " publishing {$file} by '" . $wgUser->getName() . "', comment '$commentText'... "; } else { - $props = FSFile::getPropsFromPath( $file ); + $mwProps = new MWFileProps( MimeMagic::singleton() ); + $props = $mwProps->getPropsFromPath( $file, true ); $flags = 0; $publishOptions = []; $handler = MediaHandler::getHandler( $props['mime'] ); diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index 2a985fe49a..a19fea1bd0 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -147,6 +147,7 @@ $wgAutoloadClasses += [ # tests/phpunit/mocks 'MockFSFile' => "$testDir/phpunit/mocks/filebackend/MockFSFile.php", 'MockFileBackend' => "$testDir/phpunit/mocks/filebackend/MockFileBackend.php", + 'MockLocalRepo' => "$testDir/phpunit/mocks/filerepo/MockLocalRepo.php", 'MockBitmapHandler' => "$testDir/phpunit/mocks/media/MockBitmapHandler.php", 'MockImageHandler' => "$testDir/phpunit/mocks/media/MockImageHandler.php", 'MockSvgHandler' => "$testDir/phpunit/mocks/media/MockSvgHandler.php", diff --git a/tests/parser/ParserTestRunner.php b/tests/parser/ParserTestRunner.php index 4ef778dd08..d2968a123b 100644 --- a/tests/parser/ParserTestRunner.php +++ b/tests/parser/ParserTestRunner.php @@ -348,7 +348,8 @@ class ParserTestRunner { $backend = new FSFileBackend( [ 'name' => 'local-backend', 'wikiId' => wfWikiID(), - 'basePath' => $this->uploadDir + 'basePath' => $this->uploadDir, + 'tmpDirectory' => wfTempDir() ] ); } elseif ( $this->fileBackendName ) { global $wgFileBackends; @@ -379,7 +380,7 @@ class ParserTestRunner { return new RepoGroup( [ - 'class' => 'LocalRepo', + 'class' => 'MockLocalRepo', 'name' => 'local', 'url' => 'http://example.com/images', 'hashLevels' => 2, diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index cfeb44fd6b..43577ca7b9 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -56,7 +56,7 @@ abstract class MediaWikiTestCase extends PHPUnit_Framework_TestCase { private static $useTemporaryTables = true; private static $reuseDB = false; private static $dbSetup = false; - private static $oldTablePrefix = false; + private static $oldTablePrefix = ''; /** * Original value of PHP's error_reporting setting. diff --git a/tests/phpunit/includes/media/MediaWikiMediaTestCase.php b/tests/phpunit/includes/media/MediaWikiMediaTestCase.php index 5042121ccc..e854ab57f7 100644 --- a/tests/phpunit/includes/media/MediaWikiMediaTestCase.php +++ b/tests/phpunit/includes/media/MediaWikiMediaTestCase.php @@ -26,7 +26,8 @@ abstract class MediaWikiMediaTestCase extends MediaWikiTestCase { $this->backend = new FSFileBackend( [ 'name' => 'localtesting', 'wikiId' => wfWikiID(), - 'containerPaths' => $containers + 'containerPaths' => $containers, + 'tmpDirectory' => $this->getNewTempDirectory() ] ); $this->repo = new FSRepo( $this->getRepoOptions() ); } diff --git a/tests/phpunit/mocks/filerepo/MockLocalRepo.php b/tests/phpunit/mocks/filerepo/MockLocalRepo.php new file mode 100644 index 0000000000..eeaf05a0aa --- /dev/null +++ b/tests/phpunit/mocks/filerepo/MockLocalRepo.php @@ -0,0 +1,23 @@ +getLocalReference( $virtualUrl ); + + return $fsFile->getProps(); + } +}