Fixed bug#25784 (thumbnails of stashed files had wrong description URLs).
authorNeil Kandalgaonkar <neilk@users.mediawiki.org>
Tue, 16 Nov 2010 06:57:46 +0000 (06:57 +0000)
committerNeil Kandalgaonkar <neilk@users.mediawiki.org>
Tue, 16 Nov 2010 06:57:46 +0000 (06:57 +0000)
This fixes the more general problem that the imageinfo returned with stashed uploads was inaccurate, since it was relying on
code that only worked with non-stashed files.

So, I had to:
- move the ApiQueryStashImageInfo module into core. Which others had asked for anyway, and was anticipated sometime later.
  - add lines to AutoLoader and ApiQuery to accomodate the new module

- add an ugly if/then to UploadBase -- based on the type of uploaded file, it will use a different API module to simulate a getImageInfo call.
  I left a TODO that this situation wasn't ideal, but the way things are now, imageInfo is constructed by the API modules, when it should probably
  really be the File modules. Then the API can wrap that info into various formats.

- add a few new lines to the tests to check imageinfo information in both regular and stashed upload files

includes/AutoLoader.php
includes/api/ApiQuery.php
includes/api/ApiQueryStashImageInfo.php [new file with mode: 0644]
includes/upload/UploadBase.php
maintenance/tests/phpunit/includes/api/ApiUploadTest.php

index 417c74d..dcfaf82 100644 (file)
@@ -336,6 +336,7 @@ $wgAutoloadLocalClasses = array(
        'ApiQueryRevisions' => 'includes/api/ApiQueryRevisions.php',
        'ApiQuerySearch' => 'includes/api/ApiQuerySearch.php',
        'ApiQuerySiteinfo' => 'includes/api/ApiQuerySiteinfo.php',
+       'ApiQueryStashImageInfo' => 'includes/api/ApiQueryStashImageInfo.php',
        'ApiQueryTags' => 'includes/api/ApiQueryTags.php',
        'ApiQueryUserInfo' => 'includes/api/ApiQueryUserInfo.php',
        'ApiQueryUsers' => 'includes/api/ApiQueryUsers.php',
index e4b4539..d2ed22b 100644 (file)
@@ -54,6 +54,7 @@ class ApiQuery extends ApiBase {
                'langlinks' => 'ApiQueryLangLinks',
                'images' => 'ApiQueryImages',
                'imageinfo' => 'ApiQueryImageInfo',
+               'stashimageinfo' => 'ApiQueryStashImageInfo',
                'templates' => 'ApiQueryLinks',
                'categories' => 'ApiQueryCategories',
                'extlinks' => 'ApiQueryExternalLinks',
diff --git a/includes/api/ApiQueryStashImageInfo.php b/includes/api/ApiQueryStashImageInfo.php
new file mode 100644 (file)
index 0000000..ff09a63
--- /dev/null
@@ -0,0 +1,152 @@
+<?php
+/**
+ * API for MediaWiki 1.16+
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * A query action to get image information from temporarily stashed files.
+ *
+ * @ingroup API
+ */
+class ApiQueryStashImageInfo extends ApiQueryImageInfo {
+
+       public function __construct( $query, $moduleName ) {
+               parent::__construct( $query, $moduleName, 'sii' );
+       }
+
+       public function execute() {
+               $params = $this->extractRequestParams();
+               $modulePrefix = $this->getModulePrefix();
+
+               $prop = array_flip( $params['prop'] );
+
+               $scale = $this->getScale( $params );
+               
+               $result = $this->getResult();
+               
+               try {
+                       $stash = new UploadStash();
+               
+                       foreach ( $params['sessionkey'] as $sessionkey ) {      
+                               $file = $stash->getFile( $sessionkey );
+                               $imageInfo = self::getInfo( $file, $prop, $result, $scale );
+                               $result->addValue( array( 'query', $this->getModuleName() ), null, $imageInfo );
+                               $result->setIndexedTagName_internal( array( 'query', $this->getModuleName() ), $modulePrefix );
+                       }
+
+               } catch ( UploadStashNotAvailableException $e ) {
+                       $this->dieUsage( "Session not available: " . $e->getMessage(), "nosession" );
+               } catch ( UploadStashFileNotFoundException $e ) {
+                       $this->dieUsage( "File not found: " . $e->getMessage(), "invalidsessiondata" );
+               } catch ( UploadStashBadPathException $e ) {
+                       $this->dieUsage( "Bad path: " . $e->getMessage(), "invalidsessiondata" );
+               }       
+
+       }
+
+       /**
+        * Returns all valid parameters to siiprop
+        */
+       public static function getPropertyNames() {
+               return array(
+                       'timestamp',
+                       'url',
+                       'size',
+                       'dimensions', // For backwards compatibility with Allimages
+                       'sha1',
+                       'mime',
+                       'thumbmime',
+                       'metadata',
+                       'bitdepth',
+               );
+       }
+
+
+       public function getAllowedParams() {
+               return array(
+                       'sessionkey' => array( 
+                               ApiBase::PARAM_ISMULTI => true,
+                               ApiBase::PARAM_REQUIRED => true,
+                               ApiBase::PARAM_DFLT => null
+                       ),
+                       'prop' => array(
+                               ApiBase::PARAM_ISMULTI => true,
+                               ApiBase::PARAM_DFLT => 'timestamp|url',
+                               ApiBase::PARAM_TYPE => self::getPropertyNames()
+                       ),
+                       'urlwidth' => array(
+                               ApiBase::PARAM_TYPE => 'integer',
+                               ApiBase::PARAM_DFLT => -1
+                       ),
+                       'urlheight' => array(
+                               ApiBase::PARAM_TYPE => 'integer',
+                               ApiBase::PARAM_DFLT => -1
+                       )
+               );
+       }
+
+       /**
+        * Return the API documentation for the parameters.
+        * @return {Array} parameter documentation.
+        */
+       public function getParamDescription() {
+               $p = $this->getModulePrefix();
+               return array(
+                       'prop' => array(
+                               'What image information to get:',
+                               ' timestamp    - Adds timestamp for the uploaded version',
+                               ' url          - Gives URL to the image and the description page',
+                               ' size         - Adds the size of the image in bytes and the height and width',
+                               ' dimensions   - Alias for size',
+                               ' sha1         - Adds sha1 hash for the image',
+                               ' mime         - Adds MIME of the image',
+                               ' thumbmime    - Adss MIME of the image thumbnail (requires url)',
+                               ' metadata     - Lists EXIF metadata for the version of the image',
+                               ' bitdepth     - Adds the bit depth of the version',
+                       ),
+                       '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"
+               );
+       }
+
+       public function getDescription() {
+               return 'Returns image information for stashed images';
+       }
+
+       public function getPossibleErrors() {
+               return array_merge( parent::getPossibleErrors(), array(
+                       array( 'code' => 'siiurlwidth', 'info' => 'siiurlheight cannot be used without iiurlwidth' ),
+               ) );
+       }
+
+       protected function getExamples() {
+               return array(
+                       'api.php?action=query&prop=stashimageinfo&siisessionkey=124sd34rsdf567',
+                       'api.php?action=query&prop=stashimageinfo&siisessionkey=b34edoe3|bceffd4&siiurlwidth=120&siiprop=url',
+               );
+       }
+
+       public function getVersion() {
+               return __CLASS__ . ': $Id$';
+       }
+
+}
+
index bfbf12a..e57bda2 100644 (file)
@@ -505,10 +505,15 @@ abstract class UploadBase {
         * @return mixed Status indicating the whether the upload succeeded.
         */
        public function performUpload( $comment, $pageText, $watch, $user ) {
-               wfDebug( "\n\n\performUpload: sum: " . $comment . ' c: ' . $pageText .
-                       ' w: ' . $watch );
-               $status = $this->getLocalFile()->upload( $this->mTempPath, $comment, $pageText,
-                       File::DELETE_SOURCE, $this->mFileProps, false, $user );
+               $status = $this->getLocalFile()->upload( 
+                       $this->mTempPath, 
+                       $comment, 
+                       $pageText,
+                       File::DELETE_SOURCE, i
+                       $this->mFileProps, 
+                       false, 
+                       $user 
+               );
 
                if( $status->isGood() ) {
                        if ( $watch ) {
@@ -636,8 +641,7 @@ abstract class UploadBase {
                        'mFileProps' => $this->mFileProps
                );
                $file = $stash->stashFile( $this->mTempPath, $data, $key );
-               // TODO should we change the "local file" here? 
-               // $this->mLocalFile = $file;
+               $this->mLocalFile = $file;
                return $file;
        }
 
@@ -1207,8 +1211,16 @@ abstract class UploadBase {
         */
        public function getImageInfo( $result ) {
                $file = $this->getLocalFile();
-               $imParam = ApiQueryImageInfo::getPropertyNames();
-               return ApiQueryImageInfo::getInfo( $file, array_flip( $imParam ), $result );
+               // TODO This cries out for refactoring. We really want to say $file->getAllInfo(); here. 
+               // Perhaps "info" methods should be moved into files, and the API should just wrap them in queries.
+               if ( is_a( $file, 'UploadStashFile' ) ) { 
+                       $imParam = ApiQueryStashImageInfo::getPropertyNames();
+                       $info = ApiQueryStashImageInfo::getInfo( $file, array_flip( $imParam ), $result );
+               } else {
+                       $imParam = ApiQueryImageInfo::getPropertyNames();
+                       $info = ApiQueryImageInfo::getInfo( $file, array_flip( $imParam ), $result );
+               } 
+               return $info;
        }
 
 
index ebfbd34..8d9c8a6 100644 (file)
@@ -261,6 +261,7 @@ class ApiUploadTest extends ApiTestCase {
 
                $filePaths = $randomImageGenerator->writeImages( 1, $extension, dirname( wfTempDir() ) );
                $filePath = $filePaths[0];
+               $fileSize = filesize( $filePath );
                $fileName = basename( $filePath ); 
 
                $this->deleteFileByFileName( $fileName );
@@ -286,6 +287,8 @@ class ApiUploadTest extends ApiTestCase {
                }
                $this->assertTrue( isset( $result['upload'] ) );
                $this->assertEquals( 'Success', $result['upload']['result'] );
+               $this->assertEquals( $fileSize, ( int )$result['upload']['imageinfo']['size'] );
+               $this->assertEquals( $mimeType, $result['upload']['imageinfo']['mime'] );
                $this->assertFalse( $exception );
 
                // clean up
@@ -511,6 +514,7 @@ class ApiUploadTest extends ApiTestCase {
 
                $filePaths = $randomImageGenerator->writeImages( 1, $extension, dirname( wfTempDir() ) );
                $filePath = $filePaths[0];
+               $fileSize = filesize( $filePath );
                $fileName = basename( $filePath ); 
 
                $this->deleteFileByFileName( $fileName );
@@ -538,6 +542,8 @@ class ApiUploadTest extends ApiTestCase {
                $this->assertFalse( $exception );
                $this->assertTrue( isset( $result['upload'] ) );
                $this->assertEquals( 'Success', $result['upload']['result'] );
+               $this->assertEquals( $fileSize, ( int )$result['upload']['imageinfo']['size'] );
+               $this->assertEquals( $mimeType, $result['upload']['imageinfo']['mime'] );
                $this->assertTrue( isset( $result['upload']['sessionkey'] ) );
                $sessionkey = $result['upload']['sessionkey'];