Use file width/height instead of metadata for getContentHeaders
authorGilles Dubuc <gilles@wikimedia.org>
Sat, 20 May 2017 15:53:19 +0000 (17:53 +0200)
committerGilles Dubuc <gilles@wikimedia.org>
Wed, 24 May 2017 08:46:39 +0000 (10:46 +0200)
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

19 files changed:
includes/filerepo/file/File.php
includes/filerepo/file/LocalFile.php
includes/media/DjVu.php
includes/media/Exif.php
includes/media/ExifBitmap.php
includes/media/FormatMetadata.php
includes/media/GIFMetadataExtractor.php
includes/media/MediaHandler.php
includes/media/PNGMetadataExtractor.php
includes/media/XCF.php
maintenance/importImages.php
tests/phpunit/includes/media/BitmapMetadataHandlerTest.php
tests/phpunit/includes/media/ExifTest.php
tests/phpunit/includes/media/GIFMetadataExtractorTest.php
tests/phpunit/includes/media/GIFTest.php
tests/phpunit/includes/media/JpegTest.php
tests/phpunit/includes/media/PNGTest.php
tests/phpunit/includes/media/TiffTest.php
tests/phpunit/includes/media/XCFTest.php

index 7116c22..0ad0527 100644 (file)
@@ -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() );
                        }
                }
 
index 194c0ed..8514cc8 100644 (file)
@@ -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'] = [];
                }
index 374e166..dcd276c 100644 (file)
@@ -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 [];
                }
index 621a4aa..95fa859 100644 (file)
@@ -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
index cf4b12e..0e10abb 100644 (file)
@@ -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 );
-       }
 }
index 2c541e0..f237287 100644 (file)
@@ -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,
index b4c3d6e..ac5fc81 100644 (file)
@@ -254,8 +254,6 @@ class GIFMetadataExtractor {
                        'duration' => $duration,
                        'xmp' => $xmp,
                        'comment' => $comment,
-                       'width' => $width,
-                       'height' => $height,
                ];
        }
 
index a8744a1..ec4d372 100644 (file)
@@ -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'];
index 1cb2ec0..c12ca0b 100644 (file)
@@ -406,8 +406,6 @@ class PNGMetadataExtractor {
                        'text' => $text,
                        'bitDepth' => $bitDepth,
                        'colorType' => $colorType,
-                       'width' => $width,
-                       'height' => $height,
                ];
        }
 
index bc1e2fb..c419524 100644 (file)
@@ -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;
index b3866c1..d396703 100644 (file)
@@ -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'] = [];
                                        }
index f826235..a70c005 100644 (file)
@@ -142,8 +142,6 @@ class BitmapMetadataHandlerTest extends MediaWikiTestCase {
                                'SerialNumber' => '123456789',
                                '_MW_PNG_VERSION' => 1,
                        ],
-                       'width' => 50,
-                       'height' => 50,
                ];
                $this->assertEquals( $expected, $result );
        }
index 28fe604..876e461 100644 (file)
@@ -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 );
        }
index d3174fe..1044e52 100644 (file)
@@ -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,
                                ]
                        ],
                ];
index 00edfd0..aaa3ac4 100644 (file)
@@ -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
                ];
index 35d6072..abe0280 100644 (file)
@@ -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',
index 0541b47..32d54df 100644 (file)
@@ -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
                ];
index 32af131..d114820 100644 (file)
@@ -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
index 081ca90..b75335d 100644 (file)
@@ -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";}'
                        ],
                ];
        }