From 84e4d7508893c37d7d192eda6da849f0876475a6 Mon Sep 17 00:00:00 2001 From: Gilles Dubuc Date: Sat, 20 May 2017 17:53:19 +0200 Subject: [PATCH] Use file width/height instead of metadata for getContentHeaders This allows us to populate X-Content-Dimensions without touching the existing metadata format. Which makes the migration of existing content a lot faster by only having to run refreshFileHeaders. Bug: T150741 Change-Id: I2c0f39b2b01f364c3fab997ccc2f874b7f101d8a --- includes/filerepo/file/File.php | 2 +- includes/filerepo/file/LocalFile.php | 4 ++- includes/media/DjVu.php | 7 ++-- includes/media/Exif.php | 5 --- includes/media/ExifBitmap.php | 32 ------------------- includes/media/FormatMetadata.php | 3 -- includes/media/GIFMetadataExtractor.php | 2 -- includes/media/MediaHandler.php | 20 ++++++++++-- includes/media/PNGMetadataExtractor.php | 2 -- includes/media/XCF.php | 3 -- maintenance/importImages.php | 4 ++- .../media/BitmapMetadataHandlerTest.php | 2 -- tests/phpunit/includes/media/ExifTest.php | 4 --- .../media/GIFMetadataExtractorTest.php | 6 ---- tests/phpunit/includes/media/GIFTest.php | 6 ++-- tests/phpunit/includes/media/JpegTest.php | 4 +-- tests/phpunit/includes/media/PNGTest.php | 6 ++-- tests/phpunit/includes/media/TiffTest.php | 2 +- tests/phpunit/includes/media/XCFTest.php | 6 ++-- 19 files changed, 40 insertions(+), 80 deletions(-) diff --git a/includes/filerepo/file/File.php b/includes/filerepo/file/File.php index 7116c22ce0..0ad05274cf 100644 --- a/includes/filerepo/file/File.php +++ b/includes/filerepo/file/File.php @@ -2170,7 +2170,7 @@ abstract class File implements IDBAccessObject { $metadata = MediaWiki\quietCall( 'unserialize', $metadata ); } - return $handler->getContentHeaders( $metadata ); + return $handler->getContentHeaders( $metadata, $this->getWidth(), $this->getHeight() ); } } diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 194c0edb64..8514cc8d29 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -1202,7 +1202,9 @@ class LocalFile extends File { if ( $handler ) { $metadata = MediaWiki\quietCall( 'unserialize', $props['metadata'] ); - $options['headers'] = $handler->getContentHeaders( $metadata ); + $options['headers'] = $handler->getContentHeaders( + $metadata, $props['width'], $props['height'] + ); } else { $options['headers'] = []; } diff --git a/includes/media/DjVu.php b/includes/media/DjVu.php index 374e166b19..dcd276c6ed 100644 --- a/includes/media/DjVu.php +++ b/includes/media/DjVu.php @@ -465,9 +465,12 @@ class DjVuHandler extends ImageHandler { /** * Get useful response headers for GET/HEAD requests for a file with the given metadata * @param $metadata Array Contains this handler's unserialized getMetadata() for a file - * @return array + * @param $fallbackWidth int|null Width to fall back to if metadata doesn't have any + * @param $fallbackHeight int|null Height to fall back to if metadata doesn't have any + * @return Array + * @since 1.30 */ - public function getContentHeaders( $metadata ) { + public function getContentHeaders( $metadata, $fallbackWidth = null, $fallbackHeight = null ) { if ( !is_array( $metadata ) || !isset( $metadata['xml'] ) ) { return []; } diff --git a/includes/media/Exif.php b/includes/media/Exif.php index 621a4aaa24..95fa8594fc 100644 --- a/includes/media/Exif.php +++ b/includes/media/Exif.php @@ -117,11 +117,6 @@ class Exif { * @link http://exif.org/Exif2-2.PDF The Exif 2.2 specification */ $this->mExifTags = [ - 'COMPUTED' => [ - 'Width' => Exif::SHORT_OR_LONG, # Image width - 'Height' => Exif::SHORT_OR_LONG, # Image height - ], - # TIFF Rev. 6.0 Attribute Information (p22) 'IFD0' => [ # Tags relating to image structure diff --git a/includes/media/ExifBitmap.php b/includes/media/ExifBitmap.php index cf4b12ef3b..0e10abb9f9 100644 --- a/includes/media/ExifBitmap.php +++ b/includes/media/ExifBitmap.php @@ -242,36 +242,4 @@ class ExifBitmapHandler extends BitmapHandler { return 0; } - - /** - * Get useful response headers for GET/HEAD requests for a file with the given metadata - * @param $metadata Array Contains this handler's unserialized getMetadata() for a file - * @return Array - */ - public function getContentHeaders( $metadata ) { - if ( !isset( $metadata['Width'] ) || !isset( $metadata['Height'] ) ) { - return []; - } - - $dimensionsMetadata = []; - - if ( $this->autoRotateEnabled() && isset( $metadata['Orientation'] ) ) { - switch ( $metadata['Orientation'] ) { - case 5: // CCW flipped - case 6: // CCW - case 7: // CW flipped - case 8: // CW - $dimensionsMetadata['width'] = $metadata['Height']; - $dimensionsMetadata['height'] = $metadata['Width']; - break; - } - } - - if ( !isset( $dimensionsMetadata['width'] ) ) { - $dimensionsMetadata['width'] = $metadata['Width']; - $dimensionsMetadata['height'] = $metadata['Height']; - } - - return parent::getContentHeaders( $dimensionsMetadata ); - } } diff --git a/includes/media/FormatMetadata.php b/includes/media/FormatMetadata.php index 2c541e0ff4..f2372874ba 100644 --- a/includes/media/FormatMetadata.php +++ b/includes/media/FormatMetadata.php @@ -101,9 +101,6 @@ class FormatMetadata extends ContextSource { public function makeFormattedData( $tags ) { $resolutionunit = !isset( $tags['ResolutionUnit'] ) || $tags['ResolutionUnit'] == 2 ? 2 : 3; unset( $tags['ResolutionUnit'] ); - // Width and height are for internal use and already available & displayed outside of metadata - unset( $tags['Width'] ); - unset( $tags['Height'] ); foreach ( $tags as $tag => &$vals ) { // This seems ugly to wrap non-array's in an array just to unwrap again, diff --git a/includes/media/GIFMetadataExtractor.php b/includes/media/GIFMetadataExtractor.php index b4c3d6ee11..ac5fc81c9a 100644 --- a/includes/media/GIFMetadataExtractor.php +++ b/includes/media/GIFMetadataExtractor.php @@ -254,8 +254,6 @@ class GIFMetadataExtractor { 'duration' => $duration, 'xmp' => $xmp, 'comment' => $comment, - 'width' => $width, - 'height' => $height, ]; } diff --git a/includes/media/MediaHandler.php b/includes/media/MediaHandler.php index a8744a1342..ec4d372927 100644 --- a/includes/media/MediaHandler.php +++ b/includes/media/MediaHandler.php @@ -916,12 +916,26 @@ abstract class MediaHandler { /** * Get useful response headers for GET/HEAD requests for a file with the given metadata * @param $metadata Array Contains this handler's unserialized getMetadata() for a file + * @param $fallbackWidth int|null Width to fall back to if metadata doesn't have any + * @param $fallbackHeight int|null Height to fall back to if metadata doesn't have any * @return Array * @since 1.30 */ - public function getContentHeaders( $metadata ) { - if ( !isset( $metadata['width'] ) || !isset( $metadata['height'] ) ) { - return []; + public function getContentHeaders( $metadata, $fallbackWidth = null, $fallbackHeight = null ) { + if ( !isset( $metadata['width'] ) ) { + if ( is_null( $fallbackWidth ) ) { + return []; + } + + $metadata['width'] = $fallbackWidth; + } + + if ( !isset( $metadata['height'] ) ) { + if ( is_null( $fallbackHeight ) ) { + return []; + } + + $metadata['height'] = $fallbackHeight; } $dimensionString = $metadata['width'] . 'x' . $metadata['height']; diff --git a/includes/media/PNGMetadataExtractor.php b/includes/media/PNGMetadataExtractor.php index 1cb2ec019a..c12ca0bf10 100644 --- a/includes/media/PNGMetadataExtractor.php +++ b/includes/media/PNGMetadataExtractor.php @@ -406,8 +406,6 @@ class PNGMetadataExtractor { 'text' => $text, 'bitDepth' => $bitDepth, 'colorType' => $colorType, - 'width' => $width, - 'height' => $height, ]; } diff --git a/includes/media/XCF.php b/includes/media/XCF.php index bc1e2fb770..c41952408f 100644 --- a/includes/media/XCF.php +++ b/includes/media/XCF.php @@ -175,9 +175,6 @@ class XCFHandler extends BitmapHandler { $metadata['colorType'] = 'unknown'; } - - $metadata['width'] = $header['width']; - $metadata['height'] = $header['height']; } else { // Marker to prevent repeated attempted extraction $metadata['error'] = true; diff --git a/maintenance/importImages.php b/maintenance/importImages.php index b3866c13a2..d3967037c6 100644 --- a/maintenance/importImages.php +++ b/maintenance/importImages.php @@ -309,7 +309,9 @@ class ImportImages extends Maintenance { if ( $handler ) { $metadata = MediaWiki\quietCall( 'unserialize', $props['metadata'] ); - $publishOptions['headers'] = $handler->getContentHeaders( $metadata ); + $publishOptions['headers'] = $handler->getContentHeaders( + $metadata, $props['width'], $props['height'] + ); } else { $publishOptions['headers'] = []; } diff --git a/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php b/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php index f8262352c5..a70c005497 100644 --- a/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php +++ b/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php @@ -142,8 +142,6 @@ class BitmapMetadataHandlerTest extends MediaWikiTestCase { 'SerialNumber' => '123456789', '_MW_PNG_VERSION' => 1, ], - 'width' => 50, - 'height' => 50, ]; $this->assertEquals( $expected, $result ); } diff --git a/tests/phpunit/includes/media/ExifTest.php b/tests/phpunit/includes/media/ExifTest.php index 28fe6045fc..876e46172b 100644 --- a/tests/phpunit/includes/media/ExifTest.php +++ b/tests/phpunit/includes/media/ExifTest.php @@ -29,8 +29,6 @@ class ExifTest extends MediaWikiTestCase { 'GPSAltitude' => -3.141592653, 'GPSDOP' => '5/1', 'GPSVersionID' => '2.2.0.0', - 'Height' => 10, - 'Width' => 40, ]; $this->assertEquals( $expected, $data, '', 0.0000000001 ); } @@ -43,8 +41,6 @@ class ExifTest extends MediaWikiTestCase { $expected = [ 'UserComment' => 'test⁔comment', - 'Height' => 10, - 'Width' => 40, ]; $this->assertEquals( $expected, $data ); } diff --git a/tests/phpunit/includes/media/GIFMetadataExtractorTest.php b/tests/phpunit/includes/media/GIFMetadataExtractorTest.php index d3174fe35b..1044e5210e 100644 --- a/tests/phpunit/includes/media/GIFMetadataExtractorTest.php +++ b/tests/phpunit/includes/media/GIFMetadataExtractorTest.php @@ -83,8 +83,6 @@ EOF; 'frameCount' => 1, 'looped' => false, 'xmp' => '', - 'width' => 45, - 'height' => 30, ] ], [ @@ -95,8 +93,6 @@ EOF; 'frameCount' => 4, 'looped' => true, 'xmp' => '', - 'width' => 45, - 'height' => 30, ] ], @@ -108,8 +104,6 @@ EOF; 'frameCount' => 4, 'looped' => true, 'comment' => [ 'GIƒ·test·file' ], - 'width' => 45, - 'height' => 30, ] ], ]; diff --git a/tests/phpunit/includes/media/GIFTest.php b/tests/phpunit/includes/media/GIFTest.php index 00edfd0d90..aaa3ac4ded 100644 --- a/tests/phpunit/includes/media/GIFTest.php +++ b/tests/phpunit/includes/media/GIFTest.php @@ -79,7 +79,7 @@ class GIFHandlerTest extends MediaWikiMediaTestCase { [ 'Something invalid!', GIFHandler::METADATA_BAD ], // @codingStandardsIgnoreStart Ignore Generic.Files.LineLength.TooLong [ - 'a:6:{s:10:"frameCount";i:1;s:6:"looped";b:0;s:8:"duration";d:0.1000000000000000055511151231257827021181583404541015625;s:8:"metadata";a:2:{s:14:"GIFFileComment";a:1:{i:0;s:35:"GIF test file ⁕ Created with GIMP";}s:15:"_MW_GIF_VERSION";i:1;}s:5:"width";i:45;s:6:"height";i:30;}', + 'a:4:{s:10:"frameCount";i:1;s:6:"looped";b:0;s:8:"duration";d:0.1000000000000000055511151231257827021181583404541015625;s:8:"metadata";a:2:{s:14:"GIFFileComment";a:1:{i:0;s:35:"GIF test file ⁕ Created with GIMP";}s:15:"_MW_GIF_VERSION";i:1;}}', GIFHandler::METADATA_GOOD ], // @codingStandardsIgnoreEnd @@ -103,11 +103,11 @@ class GIFHandlerTest extends MediaWikiMediaTestCase { // @codingStandardsIgnoreStart Ignore Generic.Files.LineLength.TooLong [ 'nonanimated.gif', - 'a:6:{s:10:"frameCount";i:1;s:6:"looped";b:0;s:8:"duration";d:0.1000000000000000055511151231257827021181583404541015625;s:8:"metadata";a:2:{s:14:"GIFFileComment";a:1:{i:0;s:35:"GIF test file ⁕ Created with GIMP";}s:15:"_MW_GIF_VERSION";i:1;}s:5:"width";i:45;s:6:"height";i:30;}' + 'a:4:{s:10:"frameCount";i:1;s:6:"looped";b:0;s:8:"duration";d:0.1000000000000000055511151231257827021181583404541015625;s:8:"metadata";a:2:{s:14:"GIFFileComment";a:1:{i:0;s:35:"GIF test file ⁕ Created with GIMP";}s:15:"_MW_GIF_VERSION";i:1;}}' ], [ 'animated-xmp.gif', - 'a:6:{s:10:"frameCount";i:4;s:6:"looped";b:1;s:8:"duration";d:2.399999999999999911182158029987476766109466552734375;s:8:"metadata";a:5:{s:6:"Artist";s:7:"Bawolff";s:16:"ImageDescription";a:2:{s:9:"x-default";s:18:"A file to test GIF";s:5:"_type";s:4:"lang";}s:15:"SublocationDest";s:13:"The interwebs";s:14:"GIFFileComment";a:1:{i:0;s:16:"GIƒ·test·file";}s:15:"_MW_GIF_VERSION";i:1;}s:5:"width";i:45;s:6:"height";i:30;}' + 'a:4:{s:10:"frameCount";i:4;s:6:"looped";b:1;s:8:"duration";d:2.399999999999999911182158029987476766109466552734375;s:8:"metadata";a:5:{s:6:"Artist";s:7:"Bawolff";s:16:"ImageDescription";a:2:{s:9:"x-default";s:18:"A file to test GIF";s:5:"_type";s:4:"lang";}s:15:"SublocationDest";s:13:"The interwebs";s:14:"GIFFileComment";a:1:{i:0;s:16:"GIƒ·test·file";}s:15:"_MW_GIF_VERSION";i:1;}}' ], // @codingStandardsIgnoreEnd ]; diff --git a/tests/phpunit/includes/media/JpegTest.php b/tests/phpunit/includes/media/JpegTest.php index 35d60723a6..abe028088a 100644 --- a/tests/phpunit/includes/media/JpegTest.php +++ b/tests/phpunit/includes/media/JpegTest.php @@ -25,7 +25,7 @@ class JpegTest extends MediaWikiMediaTestCase { $file = $this->dataFile( 'test.jpg', 'image/jpeg' ); $res = $this->handler->getMetadata( $file, $this->filePath . 'test.jpg' ); // @codingStandardsIgnoreStart Ignore Generic.Files.LineLength.TooLong - $expected = 'a:9:{s:16:"ImageDescription";s:9:"Test file";s:11:"XResolution";s:4:"72/1";s:11:"YResolution";s:4:"72/1";s:14:"ResolutionUnit";i:2;s:16:"YCbCrPositioning";i:1;s:15:"JPEGFileComment";a:1:{i:0;s:17:"Created with GIMP";}s:22:"MEDIAWIKI_EXIF_VERSION";i:2;s:5:"Width";i:20;s:6:"Height";i:20;}'; + $expected = 'a:7:{s:16:"ImageDescription";s:9:"Test file";s:11:"XResolution";s:4:"72/1";s:11:"YResolution";s:4:"72/1";s:14:"ResolutionUnit";i:2;s:16:"YCbCrPositioning";i:1;s:15:"JPEGFileComment";a:1:{i:0;s:17:"Created with GIMP";}s:22:"MEDIAWIKI_EXIF_VERSION";i:2;}'; // @codingStandardsIgnoreEnd // Unserialize in case serialization format ever changes. @@ -39,8 +39,6 @@ class JpegTest extends MediaWikiMediaTestCase { $file = $this->dataFile( 'test.jpg', 'image/jpeg' ); $res = $this->handler->getCommonMetaArray( $file ); $expected = [ - 'Height' => 20, - 'Width' => 20, 'ImageDescription' => 'Test file', 'XResolution' => '72/1', 'YResolution' => '72/1', diff --git a/tests/phpunit/includes/media/PNGTest.php b/tests/phpunit/includes/media/PNGTest.php index 0541b477da..32d54df71e 100644 --- a/tests/phpunit/includes/media/PNGTest.php +++ b/tests/phpunit/includes/media/PNGTest.php @@ -80,7 +80,7 @@ class PNGHandlerTest extends MediaWikiMediaTestCase { [ 'Something invalid!', PNGHandler::METADATA_BAD ], // @codingStandardsIgnoreStart Ignore Generic.Files.LineLength.TooLong [ - 'a:8:{s:10:"frameCount";i:0;s:9:"loopCount";i:1;s:8:"duration";d:0;s:8:"bitDepth";i:8;s:9:"colorType";s:10:"truecolour";s:5:"width";i:50;s:6:"height";i:50;s:8:"metadata";a:1:{s:15:"_MW_PNG_VERSION";i:1;}}', + 'a:6:{s:10:"frameCount";i:0;s:9:"loopCount";i:1;s:8:"duration";d:0;s:8:"bitDepth";i:8;s:9:"colorType";s:10:"truecolour";s:8:"metadata";a:1:{s:15:"_MW_PNG_VERSION";i:1;}}', PNGHandler::METADATA_GOOD ], // @codingStandardsIgnoreEnd @@ -105,11 +105,11 @@ class PNGHandlerTest extends MediaWikiMediaTestCase { // @codingStandardsIgnoreStart Ignore Generic.Files.LineLength.TooLong [ 'rgb-na-png.png', - 'a:8:{s:10:"frameCount";i:0;s:9:"loopCount";i:1;s:8:"duration";d:0;s:8:"bitDepth";i:8;s:9:"colorType";s:10:"truecolour";s:5:"width";i:50;s:6:"height";i:50;s:8:"metadata";a:1:{s:15:"_MW_PNG_VERSION";i:1;}}' + 'a:6:{s:10:"frameCount";i:0;s:9:"loopCount";i:1;s:8:"duration";d:0;s:8:"bitDepth";i:8;s:9:"colorType";s:10:"truecolour";s:8:"metadata";a:1:{s:15:"_MW_PNG_VERSION";i:1;}}' ], [ 'xmp.png', - 'a:8:{s:10:"frameCount";i:0;s:9:"loopCount";i:1;s:8:"duration";d:0;s:8:"bitDepth";i:1;s:9:"colorType";s:14:"index-coloured";s:5:"width";i:50;s:6:"height";i:50;s:8:"metadata";a:2:{s:12:"SerialNumber";s:9:"123456789";s:15:"_MW_PNG_VERSION";i:1;}}' + 'a:6:{s:10:"frameCount";i:0;s:9:"loopCount";i:1;s:8:"duration";d:0;s:8:"bitDepth";i:1;s:9:"colorType";s:14:"index-coloured";s:8:"metadata";a:2:{s:12:"SerialNumber";s:9:"123456789";s:15:"_MW_PNG_VERSION";i:1;}}' ], // @codingStandardsIgnoreEnd ]; diff --git a/tests/phpunit/includes/media/TiffTest.php b/tests/phpunit/includes/media/TiffTest.php index 32af131d8c..d1148202c3 100644 --- a/tests/phpunit/includes/media/TiffTest.php +++ b/tests/phpunit/includes/media/TiffTest.php @@ -35,7 +35,7 @@ class TiffTest extends MediaWikiTestCase { $res = $this->handler->getMetadata( null, $this->filePath . 'test.tiff' ); // @codingStandardsIgnoreStart Ignore Generic.Files.LineLength.TooLong - $expected = 'a:18:{s:10:"ImageWidth";i:20;s:11:"ImageLength";i:20;s:13:"BitsPerSample";a:3:{i:0;i:8;i:1;i:8;i:2;i:8;}s:11:"Compression";i:5;s:25:"PhotometricInterpretation";i:2;s:16:"ImageDescription";s:17:"Created with GIMP";s:12:"StripOffsets";i:8;s:11:"Orientation";i:1;s:15:"SamplesPerPixel";i:3;s:12:"RowsPerStrip";i:64;s:15:"StripByteCounts";i:238;s:11:"XResolution";s:19:"1207959552/16777216";s:11:"YResolution";s:19:"1207959552/16777216";s:19:"PlanarConfiguration";i:1;s:14:"ResolutionUnit";i:2;s:22:"MEDIAWIKI_EXIF_VERSION";i:2;s:5:"Width";i:20;s:6:"Height";i:20;}'; + $expected = 'a:16:{s:10:"ImageWidth";i:20;s:11:"ImageLength";i:20;s:13:"BitsPerSample";a:3:{i:0;i:8;i:1;i:8;i:2;i:8;}s:11:"Compression";i:5;s:25:"PhotometricInterpretation";i:2;s:16:"ImageDescription";s:17:"Created with GIMP";s:12:"StripOffsets";i:8;s:11:"Orientation";i:1;s:15:"SamplesPerPixel";i:3;s:12:"RowsPerStrip";i:64;s:15:"StripByteCounts";i:238;s:11:"XResolution";s:19:"1207959552/16777216";s:11:"YResolution";s:19:"1207959552/16777216";s:19:"PlanarConfiguration";i:1;s:14:"ResolutionUnit";i:2;s:22:"MEDIAWIKI_EXIF_VERSION";i:2;}'; // @codingStandardsIgnoreEnd // Re-unserialize in case there are subtle differences between how versions diff --git a/tests/phpunit/includes/media/XCFTest.php b/tests/phpunit/includes/media/XCFTest.php index 081ca90413..b75335d6c0 100644 --- a/tests/phpunit/includes/media/XCFTest.php +++ b/tests/phpunit/includes/media/XCFTest.php @@ -70,13 +70,13 @@ class XCFHandlerTest extends MediaWikiMediaTestCase { public static function provideGetMetadata() { return [ [ '80x60-2layers.xcf', - 'a:3:{s:9:"colorType";s:16:"truecolour-alpha";s:5:"width";i:80;s:6:"height";i:60;}' + 'a:1:{s:9:"colorType";s:16:"truecolour-alpha";}' ], [ '80x60-RGB.xcf', - 'a:3:{s:9:"colorType";s:16:"truecolour-alpha";s:5:"width";i:80;s:6:"height";i:60;}' + 'a:1:{s:9:"colorType";s:16:"truecolour-alpha";}' ], [ '80x60-Greyscale.xcf', - 'a:3:{s:9:"colorType";s:15:"greyscale-alpha";s:5:"width";i:80;s:6:"height";i:60;}' + 'a:1:{s:9:"colorType";s:15:"greyscale-alpha";}' ], ]; } -- 2.20.1