XMP: Use structured logging instead of wfDebugLog
authorKunal Mehta <legoktm@gmail.com>
Sat, 23 May 2015 21:34:25 +0000 (23:34 +0200)
committerBryanDavis <bdavis@wikimedia.org>
Sat, 23 May 2015 22:49:09 +0000 (22:49 +0000)
Co-Authored-By: Brian Wolff <bawolff+wn@gmail.com>
Change-Id: I486192a718576a4d1e585ffb390e297b14dde087

includes/media/BitmapMetadataHandler.php
includes/media/XMP.php
includes/media/XMPInfo.php
includes/media/XMPValidate.php
tests/phpunit/includes/media/XMPValidateTest.php

index c8d37bb..a5cddac 100644 (file)
@@ -21,6 +21,8 @@
  * @ingroup Media
  */
 
+use MediaWiki\Logger\LoggerFactory;
+
 /**
  * Class to deal with reconciling and extracting metadata from bitmap images.
  * This is meant to comply with http://www.metadataworkinggroup.org/pdf/mwg_guidance.pdf
@@ -167,7 +169,7 @@ class BitmapMetadataHandler {
                        }
                }
                if ( isset( $seg['XMP'] ) && $showXMP ) {
-                       $xmp = new XMPReader();
+                       $xmp = new XMPReader( LoggerFactory::getInstance( 'XMP' ) );
                        $xmp->parse( $seg['XMP'] );
                        foreach ( $seg['XMP_ext'] as $xmpExt ) {
                                /* Support for extended xmp in jpeg files
@@ -203,7 +205,7 @@ class BitmapMetadataHandler {
                if ( isset( $array['text']['xmp']['x-default'] )
                        && $array['text']['xmp']['x-default'] !== '' && $showXMP
                ) {
-                       $xmp = new XMPReader();
+                       $xmp = new XMPReader( LoggerFactory::getInstance( 'XMP' ) );
                        $xmp->parse( $array['text']['xmp']['x-default'] );
                        $xmpRes = $xmp->getResults();
                        foreach ( $xmpRes as $type => $xmpSection ) {
@@ -237,7 +239,7 @@ class BitmapMetadataHandler {
                }
 
                if ( $baseArray['xmp'] !== '' && XMPReader::isSupported() ) {
-                       $xmp = new XMPReader();
+                       $xmp = new XMPReader( LoggerFactory::getInstance( 'XMP' ) );
                        $xmp->parse( $baseArray['xmp'] );
                        $xmpRes = $xmp->getResults();
                        foreach ( $xmpRes as $type => $xmpSection ) {
index 957ddd2..042f749 100644 (file)
  * @ingroup Media
  */
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 /**
  * Class for reading xmp data containing properties relevant to
  * images, and spitting out an array that FormatMetadata accepts.
@@ -46,7 +50,7 @@
  * read rdf.
  *
  */
-class XMPReader {
+class XMPReader implements LoggerAwareInterface {
        /** @var array XMP item configuration array */
        protected $items;
 
@@ -120,23 +124,37 @@ class XMPReader {
        const PARSABLE_BUFFERING = 2;
        const PARSABLE_NO = 3;
 
+       /**
+        * @var LoggerInterface
+        */
+       private $logger;
+
        /**
         * Constructor.
         *
         * Primary job is to initialize the XMLParser
         */
-       function __construct() {
+       function __construct( LoggerInterface $logger = null ) {
 
                if ( !function_exists( 'xml_parser_create_ns' ) ) {
                        // this should already be checked by this point
                        throw new RuntimeException( 'XMP support requires XML Parser' );
                }
+               if ( $logger ) {
+                       $this->setLogger( $logger );
+               } else {
+                       $this->setLogger( new NullLogger() );
+               }
 
                $this->items = XMPInfo::getItems();
 
                $this->resetXMLParser();
        }
 
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
+       }
+
        /**
         * Main use is if a single item has multiple xmp documents describing it.
         * For example in jpeg's with extendedXMP
@@ -353,12 +371,12 @@ class XMPReader {
                                        . ' column: ' . xml_get_current_column_number( $this->xmlParser )
                                        . ' byte offset: ' . xml_get_current_byte_index( $this->xmlParser );
 
-                               wfDebugLog( 'XMP', "XMPReader::parse : Error reading XMP content: $error ($where)" );
+                               $this->logger->info( "XMPReader::parse : Error reading XMP content: $error ($where)" );
                                $this->results = array(); // blank if error.
                                return false;
                        }
                } catch ( Exception $e ) {
-                       wfDebugLog( 'XMP', 'XMP parse error: ' . $e );
+                       $this->logger->info( 'XMP parse error: ' . $e );
                        $this->results = array();
 
                        return false;
@@ -381,7 +399,7 @@ class XMPReader {
                if ( !isset( $this->results['xmp-special']['HasExtendedXMP'] )
                        || $this->results['xmp-special']['HasExtendedXMP'] !== $guid
                ) {
-                       wfDebugLog( 'XMP', __METHOD__ .
+                       $this->logger->info( __METHOD__ .
                                " Ignoring XMPExtended block due to wrong guid (guid= '$guid')" );
 
                        return false;
@@ -389,7 +407,7 @@ class XMPReader {
                $len = unpack( 'Nlength/Noffset', substr( $content, 32, 8 ) );
 
                if ( !$len || $len['length'] < 4 || $len['offset'] < 0 || $len['offset'] > $len['length'] ) {
-                       wfDebugLog( 'XMP', __METHOD__ . 'Error reading extended XMP block, invalid length or offset.' );
+                       $this->logger->info(  __METHOD__ . 'Error reading extended XMP block, invalid length or offset.' );
 
                        return false;
                }
@@ -406,7 +424,7 @@ class XMPReader {
                // > 128k, and be in the wrong order is very low...
 
                if ( $len['offset'] !== $this->extendedXMPOffset ) {
-                       wfDebugLog( 'XMP', __METHOD__ . 'Ignoring XMPExtended block due to wrong order. (Offset was '
+                       $this->logger->info( __METHOD__ . 'Ignoring XMPExtended block due to wrong order. (Offset was '
                                . $len['offset'] . ' but expected ' . $this->extendedXMPOffset . ')' );
 
                        return false;
@@ -428,7 +446,7 @@ class XMPReader {
                        $atEnd = false;
                }
 
-               wfDebugLog( 'XMP', __METHOD__ . 'Parsing a XMPExtended block' );
+               $this->logger->debug( __METHOD__ . 'Parsing a XMPExtended block' );
 
                return $this->parse( $actualContent, $atEnd );
        }
@@ -617,23 +635,27 @@ class XMPReader {
                        $finalName = isset( $info['map_name'] )
                                ? $info['map_name'] : $tag;
 
-                       $validate = is_array( $info['validate'] ) ? $info['validate']
-                               : array( 'XMPValidate', $info['validate'] );
+                       if ( is_array( $info['validate'] ) ) {
+                               $validate = $info['validate'];
+                       } else {
+                               $validator = new XMPValidate( $this->logger );
+                               $validate = array( $validator, $info['validate'] );
+                       }
 
                        if ( !isset( $this->results['xmp-' . $info['map_group']][$finalName] ) ) {
                                // This can happen if all the members of the struct failed validation.
-                               wfDebugLog( 'XMP', __METHOD__ . " <$ns:$tag> has no valid members." );
+                               $this->logger->debug( __METHOD__ . " <$ns:$tag> has no valid members." );
                        } elseif ( is_callable( $validate ) ) {
                                $val =& $this->results['xmp-' . $info['map_group']][$finalName];
                                call_user_func_array( $validate, array( $info, &$val, false ) );
                                if ( is_null( $val ) ) {
                                        // the idea being the validation function will unset the variable if
                                        // its invalid.
-                                       wfDebugLog( 'XMP', __METHOD__ . " <$ns:$tag> failed validation." );
+                                       $this->logger->info( __METHOD__ . " <$ns:$tag> failed validation." );
                                        unset( $this->results['xmp-' . $info['map_group']][$finalName] );
                                }
                        } else {
-                               wfDebugLog( 'XMP', __METHOD__ . " Validation function for $finalName ("
+                               $this->logger->warning( __METHOD__ . " Validation function for $finalName ("
                                        . $validate[0] . '::' . $validate[1] . '()) is not callable.' );
                        }
                }
@@ -674,7 +696,7 @@ class XMPReader {
                array_shift( $this->mode );
 
                if ( !isset( $this->results['xmp-' . $info['map_group']][$finalName] ) ) {
-                       wfDebugLog( 'XMP', __METHOD__ . " Empty compund element $finalName." );
+                       $this->logger->debug( __METHOD__ . " Empty compund element $finalName." );
 
                        return;
                }
@@ -741,7 +763,7 @@ class XMPReader {
                if ( $elm === self::NS_RDF . ' type' ) {
                        // these aren't really supported properly yet.
                        // However, it appears they almost never used.
-                       wfDebugLog( 'XMP', __METHOD__ . ' encountered <rdf:type>' );
+                       $this->logger->info( __METHOD__ . ' encountered <rdf:type>' );
                }
 
                if ( strpos( $elm, ' ' ) === false ) {
@@ -749,7 +771,7 @@ class XMPReader {
                        // However, there is a bug in an adobe product
                        // that forgets the namespace on some things.
                        // (Luckily they are unimportant things).
-                       wfDebugLog( 'XMP', __METHOD__ . " Encountered </$elm> which has no namespace. Skipping." );
+                       $this->logger->info( __METHOD__ . " Encountered </$elm> which has no namespace. Skipping." );
 
                        return;
                }
@@ -795,7 +817,7 @@ class XMPReader {
                                $this->endElementModeQDesc( $elm );
                                break;
                        default:
-                               wfDebugLog( 'XMP', __METHOD__ . " no mode (elm = $elm)" );
+                               $this->logger->warning( __METHOD__ . " no mode (elm = $elm)" );
                                break;
                }
        }
@@ -845,7 +867,7 @@ class XMPReader {
                        array_unshift( $this->mode, self::MODE_LI );
                } elseif ( $elm === self::NS_RDF . ' Bag' ) {
                        # bug 27105
-                       wfDebugLog( 'XMP', __METHOD__ . ' Expected an rdf:Seq, but got an rdf:Bag. Pretending'
+                       $this->logger->info( __METHOD__ . ' Expected an rdf:Seq, but got an rdf:Bag. Pretending'
                                . ' it is a Seq, since some buggy software is known to screw this up.' );
                        array_unshift( $this->mode, self::MODE_LI );
                } else {
@@ -908,7 +930,7 @@ class XMPReader {
                        throw new RuntimeException( __METHOD__ . ' Encountered <rdf:value> where it was unexpected.' );
                } else {
                        // something else we don't recognize, like a qualifier maybe.
-                       wfDebugLog( 'XMP', __METHOD__ .
+                       $this->logger->info( __METHOD__ .
                                " Encountered element <$elm> where only expecting character data as value of " .
                                $this->curItem[0] );
                        array_unshift( $this->mode, self::MODE_IGNORE );
@@ -962,7 +984,7 @@ class XMPReader {
                                        // a child of a struct), then something weird is
                                        // happening, so ignore this element and its children.
 
-                                       wfDebugLog( 'XMP', "Encountered <$ns:$tag> outside"
+                                       $this->logger->warning( "Encountered <$ns:$tag> outside"
                                                . " of its expected parent. Ignoring." );
 
                                        array_unshift( $this->mode, self::MODE_IGNORE );
@@ -984,7 +1006,7 @@ class XMPReader {
                                }
                        } else {
                                // This element is not on our list of allowed elements so ignore.
-                               wfDebugLog( 'XMP', __METHOD__ . " Ignoring unrecognized element <$ns:$tag>." );
+                               $this->logger->debug( __METHOD__ . " Ignoring unrecognized element <$ns:$tag>." );
                                array_unshift( $this->mode, self::MODE_IGNORE );
                                array_unshift( $this->curItem, $ns . ' ' . $tag );
 
@@ -1164,12 +1186,12 @@ class XMPReader {
                        //
                        // also it seems as if exiv2 and exiftool do not support
                        // this either (That or I misunderstand the standard)
-                       wfDebugLog( 'XMP', __METHOD__ . ' Encountered <rdf:type> which isn\'t currently supported' );
+                       $this->logger->info( __METHOD__ . ' Encountered <rdf:type> which isn\'t currently supported' );
                }
 
                if ( strpos( $elm, ' ' ) === false ) {
                        // This probably shouldn't happen.
-                       wfDebugLog( 'XMP', __METHOD__ . " Encountered <$elm> which has no namespace. Skipping." );
+                       $this->logger->info( __METHOD__ . " Encountered <$elm> which has no namespace. Skipping." );
 
                        return;
                }
@@ -1251,7 +1273,7 @@ class XMPReader {
                        if ( strpos( $name, ' ' ) === false ) {
                                // This shouldn't happen, but so far some old software forgets namespace
                                // on rdf:about.
-                               wfDebugLog( 'XMP', __METHOD__ . ' Encountered non-namespaced attribute: '
+                               $this->logger->info( __METHOD__ . ' Encountered non-namespaced attribute: '
                                        . " $name=\"$val\". Skipping. " );
                                continue;
                        }
@@ -1269,7 +1291,7 @@ class XMPReader {
                                }
                                $this->saveValue( $ns, $tag, $val );
                        } else {
-                               wfDebugLog( 'XMP', __METHOD__ . " Ignoring unrecognized element <$ns:$tag>." );
+                               $this->logger->debug( __METHOD__ . " Ignoring unrecognized element <$ns:$tag>." );
                        }
                }
        }
@@ -1291,20 +1313,24 @@ class XMPReader {
                $finalName = isset( $info['map_name'] )
                        ? $info['map_name'] : $tag;
                if ( isset( $info['validate'] ) ) {
-                       $validate = is_array( $info['validate'] ) ? $info['validate']
-                               : array( 'XMPValidate', $info['validate'] );
+                       if ( is_array( $info['validate'] ) ) {
+                               $validate = $info['validate'];
+                       } else {
+                               $validator = new XMPValidate( $this->logger );
+                               $validate = array( $validator, $info['validate'] );
+                       }
 
                        if ( is_callable( $validate ) ) {
                                call_user_func_array( $validate, array( $info, &$val, true ) );
                                // the reasoning behind using &$val instead of using the return value
                                // is to be consistent between here and validating structures.
                                if ( is_null( $val ) ) {
-                                       wfDebugLog( 'XMP', __METHOD__ . " <$ns:$tag> failed validation." );
+                                       $this->logger->info( __METHOD__ . " <$ns:$tag> failed validation." );
 
                                        return;
                                }
                        } else {
-                               wfDebugLog( 'XMP', __METHOD__ . " Validation function for $finalName ("
+                               $this->logger->warning( __METHOD__ . " Validation function for $finalName ("
                                        . $validate[0] . '::' . $validate[1] . '()) is not callable.' );
                        }
                }
index 2a31001..97aa796 100644 (file)
@@ -50,7 +50,7 @@ class XMPInfo {
         *   * mode - What type of item (self::MODE_SIMPLE usually, see above for
         *     all values).
         *   * validate - Method to validate input. Could also post-process the
-        *     input. A string value is assumed to be a static method of
+        *     input. A string value is assumed to be a method of
         *     XMPValidate. Can also take a array( 'className', 'methodName' ).
         *   * choices - Array of potential values (format of 'value' => true ).
         *     Only used with validateClosed.
index 0fa6011..55e8ce7 100644 (file)
@@ -21,6 +21,9 @@
  * @ingroup Media
  */
 
+use Psr\Log\LoggerInterface;
+use Psr\Log\LoggerAwareInterface;
+
 /**
  * This contains some static methods for
  * validating XMP properties. See XMPInfo and XMPReader classes.
  * @see http://www.adobe.com/devnet/xmp/pdfs/XMPSpecificationPart1.pdf starting at pg 28
  * @see http://www.adobe.com/devnet/xmp/pdfs/XMPSpecificationPart2.pdf starting at pg 11
  */
-class XMPValidate {
+class XMPValidate implements LoggerAwareInterface {
+
+       /**
+        * @var LoggerInterface
+        */
+       private $logger;
+
+       public function __construct( LoggerInterface $logger ) {
+               $this->setLogger( $logger );
+       }
+
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
+       }
        /**
         * Function to validate boolean properties ( True or False )
         *
@@ -48,13 +64,13 @@ class XMPValidate {
         * @param mixed &$val Current value to validate
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateBoolean( $info, &$val, $standalone ) {
+       public function validateBoolean( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        // this only validates standalone properties, not arrays, etc
                        return;
                }
                if ( $val !== 'True' && $val !== 'False' ) {
-                       wfDebugLog( 'XMP', __METHOD__ . " Expected True or False but got $val" );
+                       $this->debug->info( __METHOD__ . " Expected True or False but got $val" );
                        $val = null;
                }
        }
@@ -66,13 +82,13 @@ class XMPValidate {
         * @param mixed &$val Current value to validate
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateRational( $info, &$val, $standalone ) {
+       public function validateRational( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        // this only validates standalone properties, not arrays, etc
                        return;
                }
                if ( !preg_match( '/^(?:-?\d+)\/(?:\d+[1-9]|[1-9]\d*)$/D', $val ) ) {
-                       wfDebugLog( 'XMP', __METHOD__ . " Expected rational but got $val" );
+                       $this->logger->info( __METHOD__ . " Expected rational but got $val" );
                        $val = null;
                }
        }
@@ -87,7 +103,7 @@ class XMPValidate {
         * @param mixed &$val Current value to validate
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateRating( $info, &$val, $standalone ) {
+       public function validateRating( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        // this only validates standalone properties, not arrays, etc
                        return;
@@ -95,7 +111,7 @@ class XMPValidate {
                if ( !preg_match( '/^[-+]?\d*(?:\.?\d*)$/D', $val )
                        || !is_numeric( $val )
                ) {
-                       wfDebugLog( 'XMP', __METHOD__ . " Expected rating but got $val" );
+                       $this->logger->info( __METHOD__ . " Expected rating but got $val" );
                        $val = null;
 
                        return;
@@ -105,13 +121,13 @@ class XMPValidate {
                                // We do < 0 here instead of < -1 here, since
                                // the values between 0 and -1 are also illegal
                                // as -1 is meant as a special reject rating.
-                               wfDebugLog( 'XMP', __METHOD__ . " Rating too low, setting to -1 (Rejected)" );
+                               $this->logger->info( __METHOD__ . " Rating too low, setting to -1 (Rejected)" );
                                $val = '-1';
 
                                return;
                        }
                        if ( $nVal > 5 ) {
-                               wfDebugLog( 'XMP', __METHOD__ . " Rating too high, setting to 5" );
+                               $this->logger->info( __METHOD__ . " Rating too high, setting to 5" );
                                $val = '5';
 
                                return;
@@ -126,13 +142,13 @@ class XMPValidate {
         * @param mixed &$val Current value to validate
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateInteger( $info, &$val, $standalone ) {
+       public function validateInteger( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        // this only validates standalone properties, not arrays, etc
                        return;
                }
                if ( !preg_match( '/^[-+]?\d+$/D', $val ) ) {
-                       wfDebugLog( 'XMP', __METHOD__ . " Expected integer but got $val" );
+                       $this->logger->info( __METHOD__ . " Expected integer but got $val" );
                        $val = null;
                }
        }
@@ -145,7 +161,7 @@ class XMPValidate {
         * @param mixed &$val Current value to validate
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateClosed( $info, &$val, $standalone ) {
+       public function validateClosed( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        // this only validates standalone properties, not arrays, etc
                        return;
@@ -163,7 +179,7 @@ class XMPValidate {
                }
 
                if ( !isset( $info['choices'][$val] ) && !$inRange ) {
-                       wfDebugLog( 'XMP', __METHOD__ . " Expected closed choice, but got $val" );
+                       $this->logger->info( __METHOD__ . " Expected closed choice, but got $val" );
                        $val = null;
                }
        }
@@ -175,7 +191,7 @@ class XMPValidate {
         * @param mixed &$val Current value to validate
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateFlash( $info, &$val, $standalone ) {
+       public function validateFlash( $info, &$val, $standalone ) {
                if ( $standalone ) {
                        // this only validates flash structs, not individual properties
                        return;
@@ -186,7 +202,7 @@ class XMPValidate {
                        && isset( $val['RedEyeMode'] )
                        && isset( $val['Return'] )
                ) ) {
-                       wfDebugLog( 'XMP', __METHOD__ . " Flash structure did not have all the required components" );
+                       $this->logger->info( __METHOD__ . " Flash structure did not have all the required components" );
                        $val = null;
                } else {
                        $val = ( "\0" | ( $val['Fired'] === 'True' )
@@ -209,14 +225,14 @@ class XMPValidate {
         * @param mixed &$val Current value to validate
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateLangCode( $info, &$val, $standalone ) {
+       public function validateLangCode( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        // this only validates standalone properties, not arrays, etc
                        return;
                }
                if ( !preg_match( '/^[-A-Za-z0-9]{2,}$/D', $val ) ) {
                        //this is a rather naive check.
-                       wfDebugLog( 'XMP', __METHOD__ . " Expected Lang code but got $val" );
+                       $this->logger->info( __METHOD__ . " Expected Lang code but got $val" );
                        $val = null;
                }
        }
@@ -238,7 +254,7 @@ class XMPValidate {
         *    2011:04.
         * @param bool $standalone If this is a simple property or array
         */
-       public static function validateDate( $info, &$val, $standalone ) {
+       public function validateDate( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        // this only validates standalone properties, not arrays, etc
                        return;
@@ -252,7 +268,7 @@ class XMPValidate {
                ) {
                        // @codingStandardsIgnoreEnd
 
-                       wfDebugLog( 'XMP', __METHOD__ . " Expected date but got $val" );
+                       $this->logger->info( __METHOD__ . " Expected date but got $val" );
                        $val = null;
                } else {
                        /*
@@ -270,7 +286,7 @@ class XMPValidate {
                         * some programs convert between metadata formats.
                         */
                        if ( $res[1] === '0000' ) {
-                               wfDebugLog( 'XMP', __METHOD__ . " Invalid date (year 0): $val" );
+                               $this->logger->info( __METHOD__ . " Invalid date (year 0): $val" );
                                $val = null;
 
                                return;
@@ -339,7 +355,7 @@ class XMPValidate {
         *   or DDD,MM.mmk form
         * @param bool $standalone If its a simple prop (should always be true)
         */
-       public static function validateGPS( $info, &$val, $standalone ) {
+       public function validateGPS( $info, &$val, $standalone ) {
                if ( !$standalone ) {
                        return;
                }
@@ -371,7 +387,7 @@ class XMPValidate {
 
                        return;
                } else {
-                       wfDebugLog( 'XMP', __METHOD__
+                       $this->logger->info( __METHOD__
                                . " Expected GPSCoordinate, but got $val." );
                        $val = null;
 
index ebec8f6..53671d4 100644 (file)
@@ -1,5 +1,7 @@
 <?php
 
+use Psr\Log\NullLogger;
+
 /**
  * @group Media
  */
@@ -11,7 +13,8 @@ class XMPValidateTest extends MediaWikiTestCase {
         */
        public function testValidateDate( $value, $expected ) {
                // The method should modify $value.
-               XMPValidate::validateDate( array(), $value, true );
+               $validate = new XMPValidate( new NullLogger() );
+               $validate->validateDate( array(), $value, true );
                $this->assertEquals( $expected, $value );
        }