(follow-up r81558) Per suggestion, make this use media handler's getParamString/parse...

Additionally, rename makeThumbParam back to getScale since that makes more sense now.
Also update the version number used in ForeignAPIRepo user-agent, since this is kind of significant change.

diff --git a/includes/api/ApiQueryImageInfo.php b/includes/api/ApiQueryImageInfo.php
index 530dd59..6283192 100644 (file)
--- a/includes/api/ApiQueryImageInfo.php
+++ b/includes/api/ApiQueryImageInfo.php
@@ -50,7 +50,7 @@ class ApiQueryImageInfo extends ApiQueryBase {
 
                $prop = array_flip( $params['prop'] );
 
-               $thumbParams = $this->makeThumbParams( $params );
+               $scale = $this->getScale( $params );
 
                $pageIds = $this->getPageSet()->getAllTitlesByNamespace();
                if ( !empty( $pageIds[NS_FILE] ) ) {
@@ -108,8 +108,8 @@ class ApiQueryImageInfo extends ApiQueryBase {
                                        break;
                                }
                                
-                               // Check if we can make the requested thumbnail
-                               $this->validateThumbParams( $img, $thumbParams );
+                               // Check if we can make the requested thumbnail, and get transform parameters.
+                               $finalThumbParams = $this->mergeThumbParams( $img, $scale, $params['urlparam'] );
 
                                // Get information about the current version first
                                // Check that the current version is within the start-end boundaries
@@ -122,7 +122,7 @@ class ApiQueryImageInfo extends ApiQueryBase {
                                        $gotOne = true;
                                        
                                        $fit = $this->addPageSubItem( $pageId,
-                                               self::getInfo( $img, $prop, $result, $thumbParams ) );
+                                               self::getInfo( $img, $prop, $result, $finalThumbParams ) );
                                        if ( !$fit ) {
                                                if ( count( $pageIds[NS_IMAGE] ) == 1 ) {
                                                        // See the 'the user is screwed' comment above
@@ -151,7 +151,7 @@ class ApiQueryImageInfo extends ApiQueryBase {
                                                break;
                                        }
                                        $fit = $this->addPageSubItem( $pageId,
-                                               self::getInfo( $oldie, $prop, $result, $thumbParams ) );
+                                               self::getInfo( $oldie, $prop, $result, $finalThumbParams ) );
                                        if ( !$fit ) {
                                                if ( count( $pageIds[NS_IMAGE] ) == 1 ) {
                                                        $this->setContinueEnumParameter( 'start',
@@ -184,24 +184,10 @@ class ApiQueryImageInfo extends ApiQueryBase {
 
        /**
         * From parameters, construct a 'scale' array
-        * @param $params Array:
+        * @param $params Array: Parameters passed to api.
         * @return Array or Null: key-val array of 'width' and 'height', or null
         */
        public function getScale( $params ) {
-               wfDeprecated( __METHOD__ );
-               if ( !isset( $params['urlparam'] ) ) {
-                       // In case there are subclasses that
-                       // don't have this param set to anything.
-                       $params['urlparam'] = null;
-               }
-               return $this->makeThumbParams( $params );
-       }
-
-       /* Take parameters for transforming thumbnail, validate and turn into array.
-        * @param $params Array: Parameters from the request.
-        * @return Array or null: If array, suitable to passing to $file->transform.
-        */
-       public function makeThumbParams( $params ) {
                $p = $this->getModulePrefix();
 
                // Height and width.
@@ -221,50 +207,54 @@ class ApiQueryImageInfo extends ApiQueryBase {
                        return $scale;
                }
 
-               // Other parameters.
-               if ( is_array( $params['urlparam'] ) ) {
-                       foreach( $params['urlparam'] as $item ) {
-                               $parameter = explode( '=', $item, 2 );
-
-                               if ( count( $parameter ) !== 2
-                                       || $parameter[0] === 'width'
-                                       || $parameter[0] === 'height'
-                               ) {
-                                       $this->dieUsage( "Invalid value for {$p}urlparam ($item)", "urlparam" );
-                               }
-                               $scale[$parameter[0]] = $parameter[1];
-                       }
-               }
                return $scale;
        }
 
-       /** Validate the thumb parameters, give error if invalid.
+       /** Validate and merge scale parameters with handler thumb parameters, give error if invalid.
         *
-        * We do this later than makeThumbParams, since we need the image
+        * We do this later than getScale, since we need the image
         * to know which handler, since handlers can make their own parameters.
         * @param File $image Image that params are for.
-        * @param Array $thumbParams thumbnail parameters
+        * @param Array $thumbParams thumbnail parameters from getScale
+        * @param String String of otherParams (iiurlparam).
+        * @return Array of parameters for transform.
         */
-       protected function validateThumbParams ( $image, $thumbParams ) {
-               if ( !$thumbParams ) return;
+       protected function mergeThumbParams ( $image, $thumbParams, $otherParams ) {
+               if ( !$otherParams ) return $thumbParams;
                $p = $this->getModulePrefix();
 
                $h = $image->getHandler();
                if ( !$h ) {
                        $this->setWarning( 'Could not create thumbnail because ' . 
                                $image->getName() . ' does not have an associated image handler' );
-                       return;
+                       return $thumbParams;
                }
-               foreach ( $thumbParams as $name => $value ) {
+
+               $paramList = $h->parseParamString( $otherParams );
+               if ( !$paramList ) {
+                       // Just set a warning (instead of dieUsage), as in many cases
+                       // we could still render the image using width and height parameters,
+                       // and this type of thing could happen between different versions of
+                       // handlers.
+                       $this->setWarning( "Could not parse {$p}urlparam for " . $image->getName()
+                               . '. Using only width and height' );
+                       return $thumbParams;
+               }
+
+               if ( isset( $paramList['width'] ) ) {
+                       if ( $paramList['width'] !== $thumbParams['width'] ) {
+                               $this->dieUsage( "{$p}urlparam had width of {$paramList['width']} but "
+                                       . "{$p}urlwidth was {$thumbParams['width']}", "urlparam_urlwidth_mismatch" );
+                       }
+               }
+
+               foreach ( $paramList as $name => $value ) {
                        if ( !$h->validateParam( $name, $value ) ) {
-                               /* This doesn't work well with height=-1 placeholder */
-                               if ( $name === 'height' ) continue;
                                $this->dieUsage( "Invalid value for {$p}urlparam ($name=$value)", "urlparam" );
                        }
                }
-               // This could also potentially check normaliseParams as well, However that seems
-               // to fall more into a thumbnail rendering error than a user input error, and
-               // will be checked by the transform functions.
+
+               return $thumbParams + $paramList;
        }
 
        /**
@@ -423,7 +413,8 @@ class ApiQueryImageInfo extends ApiQueryBase {
                                ApiBase::PARAM_DFLT => -1
                        ),
                        'urlparam' => array(
-                               ApiBase::PARAM_ISMULTI => true,
+                               ApiBase::PARAM_DFLT => '',
+                               ApiBase::PARAM_TYPE => 'string',
                        ),
                        'continue' => null,
                );
@@ -490,8 +481,8 @@ class ApiQueryImageInfo extends ApiQueryBase {
                        'urlwidth' => array( "If {$p}prop=url is set, a URL to an image scaled to this width will be returned.",
                                            'Only the current version of the image can be scaled' ),
                        'urlheight' => "Similar to {$p}urlwidth. Cannot be used without {$p}urlwidth",
-                       'urlparam' => array( "Other rending parameters, such as page=2 for multipaged documents.",
-                                       "Multiple parameters should be seperated with a |. {$p}urlwidth must also be used"),
+                       'urlparam' => array( "A handler specific parameter string. For example, pdf's ",
+                               "might use 'page15-100px'. {$p}urlwidth must be used and be consistent with {$p}urlparam" ),
                        'limit' => 'How many image revisions to return',
                        'start' => 'Timestamp to start listing from',
                        'end' => 'Timestamp to stop listing at',
@@ -508,7 +499,9 @@ class ApiQueryImageInfo extends ApiQueryBase {
                return array_merge( parent::getPossibleErrors(), array(
                        array( 'code' => 'iiurlwidth', 'info' => 'iiurlheight cannot be used without iiurlwidth' ),
                        array( 'code' => 'urlparam', 'info' => "Invalid value for {$p}urlparam" ),
-                       array( 'code' => 'urlparam_no_width', 'info' => "iiurlparam requires {$p}urlwidth" ),
+                       array( 'code' => 'urlparam_no_width', 'info' => "{$p}urlparam requires {$p}urlwidth" ),
+                       array( 'code' => 'urlparam_urlwidth_mismatch', 'info' => "The width set in {$p}urlparm doesnt't " .
+                               "match the one in {$p}urlwidth" ), 
                ) );
        }
 

diff --git a/includes/api/ApiQueryStashImageInfo.php b/includes/api/ApiQueryStashImageInfo.php
index 84f7560..e48f6aa 100644 (file)
--- a/includes/api/ApiQueryStashImageInfo.php
+++ b/includes/api/ApiQueryStashImageInfo.php
@@ -37,7 +37,7 @@ class ApiQueryStashImageInfo extends ApiQueryImageInfo {
 
                $prop = array_flip( $params['prop'] );
 
-               $scale = $this->makeThumbParams( $params );
+               $scale = $this->getScale( $params );
 
                $result = $this->getResult();
 
@@ -46,8 +46,8 @@ class ApiQueryStashImageInfo extends ApiQueryImageInfo {
 
                        foreach ( $params['sessionkey'] as $sessionkey ) {
                                $file = $stash->getFile( $sessionkey );
-                               $this->validateThumbParams( $file, $scale );
-                               $imageInfo = self::getInfo( $file, $prop, $result, $scale );
+                               $finalThumbParam = $this->mergeThumbParams( $file, $scale, $params['urlparam'] );
+                               $imageInfo = self::getInfo( $file, $prop, $result, $finalThumbParam );
                                $result->addValue( array( 'query', $this->getModuleName() ), null, $imageInfo );
                                $result->setIndexedTagName_internal( array( 'query', $this->getModuleName() ), $modulePrefix );
                        }
@@ -100,7 +100,8 @@ class ApiQueryStashImageInfo extends ApiQueryImageInfo {
                                ApiBase::PARAM_DFLT => -1
                        ),
                        'urlparam' => array(
-                               ApiBase::PARAM_ISMULTI => true,
+                               ApiBase::PARAM_TYPE => 'string',
+                               ApiBase::PARAM_DFLT => '',
                        ),
                );
        }
@@ -127,8 +128,8 @@ class ApiQueryStashImageInfo extends ApiQueryImageInfo {
                        'sessionkey' => 'Session key that identifies a previous upload that was stashed temporarily.',
                        'urlwidth' => "If {$p}prop=url is set, a URL to an image scaled to this width will be returned.",
                        'urlheight' => "Similar to {$p}urlwidth. Cannot be used without {$p}urlwidth",
-                       'urlparam' => array( "Other rending parameters, such as page=2 for multipaged documents.",
-                                       "Multiple parameters should be seperated with a |. {$p}urlwidth must also be used" ),
+                       'urlparam' => array( "A handler specific parameter string. For example, pdf's ",
+                               "might use 'page15-100px'. {$p}urlwidth must be used and be consistent with {$p}urlparam" ),
                );
        }
 

diff --git a/includes/filerepo/ForeignAPIFile.php b/includes/filerepo/ForeignAPIFile.php
index f78a2a3..193dde0 100644 (file)
--- a/includes/filerepo/ForeignAPIFile.php
+++ b/includes/filerepo/ForeignAPIFile.php
@@ -77,18 +77,9 @@ class ForeignAPIFile extends File {
                        return parent::transform( $params, $flags );
                }
 
-               $otherParams = "";
-               // Not using implode, since associative array.
-               foreach ( $params as $name => $value ) {
-                       if ( $name === 'width' || $name === 'height' ) {
-                               continue;
-                       }
-                       $otherParams .= "{$name}={$value}|";
-               }
-               // Remove the last |
-               if ( $otherParams !== "" ) {
-                       $otherParams = substr( $otherParams, 0, -1 );
-               }
+               // Note, the this->canRender() check above implies
+               // that we have a handler, and it can do makeParamString.
+               $otherParams = $this->handler->makeParamString( $params );
 
                $thumbUrl = $this->repo->getThumbUrlFromCache(
                        $this->getName(),

diff --git a/includes/filerepo/ForeignAPIRepo.php b/includes/filerepo/ForeignAPIRepo.php
index 3b46f14..f37859c 100644 (file)
--- a/includes/filerepo/ForeignAPIRepo.php
+++ b/includes/filerepo/ForeignAPIRepo.php
@@ -25,7 +25,7 @@ class ForeignAPIRepo extends FileRepo {
        /* This version string is used in the user agent for requests and will help
         * server maintainers in identify ForeignAPI usage.
         * Update the version every time you make breaking or significant changes. */
-       const VERSION = "2.0";
+       const VERSION = "2.1";
 
        var $fileFactory = array( 'ForeignAPIFile', 'newFromTitle' );
        /* Check back with Commons after a day */
@@ -225,7 +225,7 @@ class ForeignAPIRepo extends FileRepo {
         * @param $name String is a dbkey form of a title
         * @param $width
         * @param $height
-        * @param String $param Other rendering parameters (page number, etc). | separated.
+        * @param String $param Other rendering parameters (page number, etc) from handler's makeParamString.
         */
        function getThumbUrlFromCache( $name, $width, $height, $params="" ) {
                global $wgMemc;