From: Neil Kandalgaonkar Date: Tue, 16 Nov 2010 06:57:46 +0000 (+0000) Subject: Fixed bug#25784 (thumbnails of stashed files had wrong description URLs). X-Git-Tag: 1.31.0-rc.0~33870 X-Git-Url: https://git.cyclocoop.org/%28%28?a=commitdiff_plain;h=59339ca7238785e6d5007a484c0bd274204b7d36;p=lhc%2Fweb%2Fwiklou.git Fixed bug#25784 (thumbnails of stashed files had wrong description URLs). 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 --- diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 417c74d7f8..dcfaf82464 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -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', diff --git a/includes/api/ApiQuery.php b/includes/api/ApiQuery.php index e4b453950c..d2ed22bb59 100644 --- a/includes/api/ApiQuery.php +++ b/includes/api/ApiQuery.php @@ -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 index 0000000000..ff09a63321 --- /dev/null +++ b/includes/api/ApiQueryStashImageInfo.php @@ -0,0 +1,152 @@ +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$'; + } + +} + diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index bfbf12ab88..e57bda2d9a 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -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; } diff --git a/maintenance/tests/phpunit/includes/api/ApiUploadTest.php b/maintenance/tests/phpunit/includes/api/ApiUploadTest.php index ebfbd34ecd..8d9c8a6fc8 100644 --- a/maintenance/tests/phpunit/includes/api/ApiUploadTest.php +++ b/maintenance/tests/phpunit/includes/api/ApiUploadTest.php @@ -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'];