r75906 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r75905‎ | r75906 | r75907 >
Date:04:32, 3 November 2010
Author:neilk
Status:resolved (Comments)
Tags:
Comment:
core changes for UploadWizard (merged from r73549 to HEAD in branches/uploadwizard/phase3)
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryImageInfo.php (modified) (history)
  • /trunk/phase3/includes/api/ApiUpload.php (modified) (history)
  • /trunk/phase3/includes/filerepo/File.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUploadStash.php (added) (history)
  • /trunk/phase3/includes/upload/UploadBase.php (modified) (history)
  • /trunk/phase3/includes/upload/UploadFromFile.php (modified) (history)
  • /trunk/phase3/includes/upload/UploadStash.php (added) (history)
  • /trunk/phase3/maintenance/tests/phpunit/Makefile (modified) (history)
  • /trunk/phase3/maintenance/tests/phpunit/includes/api/ApiUploadTest.php (added) (history)
  • /trunk/phase3/maintenance/tests/phpunit/includes/api/RandomImageGenerator.php (added) (history)
  • /trunk/phase3/maintenance/tests/phpunit/includes/api/generateRandomImages.php (added) (history)

Diff [purge]

Index: trunk/phase3/maintenance/tests/phpunit/includes/api/RandomImageGenerator.php
——@@ -0,0 +1,289 @@
 2+<?php
 3+
 4+/*
 5+ * RandomImageGenerator -- does what it says on the tin.
 6+ * Requires Imagick, the ImageMagick library for PHP, or the command line equivalent (usually 'convert').
 7+ *
 8+ * Because MediaWiki tests the uniqueness of media upload content, and filenames, it is sometimes useful to generate
 9+ * files that are guaranteed (or at least very likely) to be unique in both those ways.
 10+ * This generates a number of filenames with random names and random content (colored circles)
 11+ *
 12+ * It is also useful to have fresh content because our tests currently run in a "destructive" mode, and don't create a fresh new wiki for each
 13+ * test run.
 14+ * Consequently, if we just had a few static files we kept re-uploading, we'd get lots of warnings about matching content or filenames,
 15+ * and even if we deleted those files, we'd get warnings about archived files.
 16+ *
 17+ * This can also be used with a cronjob to generate random files all the time -- I use it to have a constant, never ending supply when I'm
 18+ * testing interactively.
 19+ *
 20+ * @file
 21+ * @author Neil Kandalgaonkar <neilk@wikimedia.org>
 22+ */
 23+
 24+/**
 25+ * RandomImageGenerator: does what it says on the tin.
 26+ * Can fetch a random image, or also write a number of them to disk with random filenames.
 27+ */
 28+class RandomImageGenerator {
 29+
 30+ private $dictionaryFile;
 31+ private $minWidth = 400;
 32+ private $maxWidth = 800;
 33+ private $minHeight = 400;
 34+ private $maxHeight = 800;
 35+ private $circlesToDraw = 5;
 36+ private $imageWriteMethod;
 37+
 38+ public function __construct( $options ) {
 39+ global $wgUseImageMagick, $wgImageMagickConvertCommand;
 40+ foreach ( array( 'dictionaryFile', 'minWidth', 'minHeight', 'maxHeight', 'circlesToDraw' ) as $property ) {
 41+ if ( isset( $options[$property] ) ) {
 42+ $this->$property = $options[$property];
 43+ }
 44+ }
 45+
 46+ // find the dictionary file, to generate random names
 47+ if ( !isset( $this->dictionaryFile ) ) {
 48+ foreach ( array( '/usr/share/dict/words', '/usr/dict/words' ) as $dictionaryFile ) {
 49+ if ( is_file( $dictionaryFile ) and is_readable( $dictionaryFile ) ) {
 50+ $this->dictionaryFile = $dictionaryFile;
 51+ break;
 52+ }
 53+ }
 54+ }
 55+ if ( !isset( $this->dictionaryFile ) ) {
 56+ throw new Exception( "RandomImageGenerator: dictionary file not found or not specified properly" );
 57+ }
 58+
 59+ // figure out how to write images
 60+ if ( class_exists( 'Imagick' ) ) {
 61+ $this->imageWriteMethod = 'writeImageWithApi';
 62+ } elseif ( $wgUseImageMagick && $wgImageMagickConvertCommand && is_executable( $wgImageMagickConvertCommand ) ) {
 63+ $this->imageWriteMethod = 'writeImageWithCommandLine';
 64+ } else {
 65+ throw new Exception( "RandomImageGenerator: could not find a suitable method to write images" );
 66+ }
 67+ }
 68+
 69+ /**
 70+ * Writes random images with random filenames to disk in the directory you specify, or current working directory
 71+ *
 72+ * @param {Integer} number of filenames to write
 73+ * @param {String} format, optional, must be understood by ImageMagick, such as 'jpg' or 'gif'
 74+ * @param {String} directory, optional (will default to current working directory)
 75+ * @return {Array} filenames we just wrote
 76+ */
 77+ function writeImages( $number, $format = 'jpg', $dir = null ) {
 78+ $filenames = $this->getRandomFilenames( $number, $format, $dir );
 79+ foreach( $filenames as $filename ) {
 80+ $this->{$this->imageWriteMethod}( $this->getImageSpec(), $format, $filename );
 81+ }
 82+ return $filenames;
 83+ }
 84+
 85+ /**
 86+ * Return a number of randomly-generated filenames
 87+ * Each filename uses two words randomly drawn from the dictionary, like elephantine_spatula.jpg
 88+ *
 89+ * @param {Integer} number of filenames to generate
 90+ * @param {String} extension, optional, defaults to 'jpg'
 91+ * @param {String} directory, optional, defaults to current working directory
 92+ * @return {Array} of filenames
 93+ */
 94+ private function getRandomFilenames( $number, $extension = 'jpg', $dir = null ) {
 95+ if ( is_null( $dir ) ) {
 96+ $dir = getcwd();
 97+ }
 98+ $filenames = array();
 99+ foreach( $this->getRandomWordPairs( $number ) as $pair ) {
 100+ $basename = $pair[0] . '_' . $pair[1];
 101+ if ( !is_null( $extension ) ) {
 102+ $basename .= '.' . $extension;
 103+ }
 104+ $basename = preg_replace( '/\s+/', '', $basename );
 105+ $filenames[] = "$dir/$basename";
 106+ }
 107+
 108+ return $filenames;
 109+
 110+ }
 111+
 112+
 113+ /**
 114+ * Generate data representing an image of random size (within limits),
 115+ * consisting of randomly colored and sized circles against a random background color
 116+ * (This data is used in the writeImage* methods).
 117+ * @return {Mixed}
 118+ */
 119+ public function getImageSpec() {
 120+ $spec = array();
 121+
 122+ $spec['width'] = mt_rand( $this->minWidth, $this->maxWidth );
 123+ $spec['height'] = mt_rand( $this->minHeight, $this->maxHeight );
 124+ $spec['fill'] = $this->getRandomColor();
 125+
 126+ $diagonalLength = sqrt( pow( $spec['width'], 2 ) + pow( $spec['height'], 2 ) );
 127+
 128+ $draws = array();
 129+ for ( $i = 0; $i <= $this->circlesToDraw; $i++ ) {
 130+ $radius = mt_rand( 0, $diagonalLength / 4 );
 131+ $originX = mt_rand( -1 * $radius, $spec['width'] + $radius );
 132+ $originY = mt_rand( -1 * $radius, $spec['height'] + $radius );
 133+ $perimeterX = $originX + $radius;
 134+ $perimeterY = $originY + $radius;
 135+
 136+ $draw = array();
 137+ $draw['fill'] = $this->getRandomColor();
 138+ $draw['circle'] = array(
 139+ 'originX' => $originX,
 140+ 'originY' => $originY,
 141+ 'perimeterX' => $perimeterX,
 142+ 'perimeterY' => $perimeterY
 143+ );
 144+ $draws[] = $draw;
 145+
 146+ }
 147+
 148+ $spec['draws'] = $draws;
 149+
 150+ return $spec;
 151+ }
 152+
 153+
 154+ /**
 155+ * Based on an image specification, write such an image to disk, using Imagick PHP extension
 156+ * @param $spec: spec describing background and circles to draw
 157+ * @param $format: file format to write
 158+ * @param $filename: filename to write to
 159+ */
 160+ public function writeImageWithApi( $spec, $format, $filename ) {
 161+ $image = new Imagick();
 162+ $image->newImage( $spec['width'], $spec['height'], new ImagickPixel( $spec['fill'] ) );
 163+
 164+ foreach ( $spec['draws'] as $drawSpec ) {
 165+ $draw = new ImagickDraw();
 166+ $draw->setFillColor( $drawSpec['fill'] );
 167+ $circle = $drawSpec['circle'];
 168+ $draw->circle( $circle['originX'], $circle['originY'], $circle['perimeterX'], $circle['perimeterY'] );
 169+ $image->drawImage( $draw );
 170+ }
 171+
 172+ $image->setImageFormat( $format );
 173+ $image->writeImage( $filename );
 174+ }
 175+
 176+
 177+ /**
 178+ * Based on an image specification, write such an image to disk, using the command line ImageMagick program ('convert').
 179+ *
 180+ * Sample command line:
 181+ * $ convert -size 100x60 xc:rgb(90,87,45) \
 182+ * -draw 'fill rgb(12,34,56) circle 41,39 44,57' \
 183+ * -draw 'fill rgb(99,123,231) circle 59,39 56,57' \
 184+ * -draw 'fill rgb(240,12,32) circle 50,21 50,3' filename.png
 185+ *
 186+ * @param $spec: spec describing background and circles to draw
 187+ * @param $format: file format to write (unused by this method but kept so it has the same signature as writeImageWithApi)
 188+ * @param $filename: filename to write to
 189+ */
 190+ public function writeImageWithCommandLine( $spec, $format, $filename ) {
 191+ global $wgImageMagickConvertCommand;
 192+ $args = array();
 193+ $args[] = "-size " . wfEscapeShellArg( $spec['width'] . 'x' . $spec['height'] );
 194+ $args[] = wfEscapeShellArg( "xc:" . $spec['fill'] );
 195+ foreach( $spec['draws'] as $draw ) {
 196+ $fill = $draw['fill'];
 197+ $originX = $draw['circle']['originX'];
 198+ $originY = $draw['circle']['originY'];
 199+ $perimeterX = $draw['circle']['perimeterX'];
 200+ $perimeterY = $draw['circle']['perimeterY'];
 201+ $drawCommand = "fill $fill circle $originX,$originY $perimeterX,$perimeterY";
 202+ $args[] = '-draw ' . wfEscapeShellArg( $drawCommand );
 203+ }
 204+ $args[] = $filename;
 205+
 206+ $command = wfEscapeShellArg( $wgImageMagickConvertCommand ) . " " . implode( " ", $args );
 207+ $output = wfShellExec( $command, $retval );
 208+ return ( $retval === 0 );
 209+ }
 210+
 211+
 212+
 213+ /**
 214+ * Generate a string of random colors for ImageMagick, like "rgb(12, 37, 98)"
 215+ *
 216+ * @return {String}
 217+ */
 218+ public function getRandomColor() {
 219+ $components = array();
 220+ for ($i = 0; $i <= 2; $i++ ) {
 221+ $components[] = mt_rand( 0, 255 );
 222+ }
 223+ return 'rgb(' . join(', ', $components) . ')';
 224+ }
 225+
 226+ /**
 227+ * Get an array of random pairs of random words, like array( array( 'foo', 'bar' ), array( 'quux', 'baz' ) );
 228+ *
 229+ * @param {Integer} number of pairs
 230+ * @return {Array} of two-element arrays
 231+ */
 232+ private function getRandomWordPairs( $number ) {
 233+ $lines = $this->getRandomLines( $number * 2 );
 234+ // construct pairs of words
 235+ $pairs = array();
 236+ $count = count( $lines );
 237+ for( $i = 0; $i < $count; $i += 2 ) {
 238+ $pairs[] = array( $lines[$i], $lines[$i+1] );
 239+ }
 240+ return $pairs;
 241+ }
 242+
 243+
 244+ /**
 245+ * Return N random lines from a file
 246+ *
 247+ * Will throw exception if the file could not be read or if it had fewer lines than requested.
 248+ *
 249+ * @param {Integer} number of lines desired
 250+ * @string {String} path to file
 251+ * @return {Array} of exactly n elements, drawn randomly from lines the file
 252+ */
 253+ private function getRandomLines( $number_desired ) {
 254+ $filepath = $this->dictionaryFile;
 255+
 256+ // initialize array of lines
 257+ $lines = array();
 258+ for ( $i = 0; $i < $number_desired; $i++ ) {
 259+ $lines[] = null;
 260+ }
 261+
 262+ /*
 263+ * This algorithm obtains N random lines from a file in one single pass. It does this by replacing elements of
 264+ * a fixed-size array of lines, less and less frequently as it reads the file.
 265+ */
 266+ $fh = fopen( $filepath, "r" );
 267+ if ( !$fh ) {
 268+ throw new Exception( "couldn't open $filepath" );
 269+ }
 270+ $line_number = 0;
 271+ $max_index = $number_desired - 1;
 272+ while( !feof( $fh ) ) {
 273+ $line = fgets( $fh );
 274+ if ( $line !== false ) {
 275+ $line_number++;
 276+ $line = trim( $line );
 277+ if ( mt_rand( 0, $line_number ) <= $max_index ) {
 278+ $lines[ mt_rand( 0, $max_index ) ] = $line;
 279+ }
 280+ }
 281+ }
 282+ fclose( $fh );
 283+ if ( $line_number < $number_desired ) {
 284+ throw new Exception( "not enough lines in $filepath" );
 285+ }
 286+
 287+ return $lines;
 288+ }
 289+
 290+}
Property changes on: trunk/phase3/maintenance/tests/phpunit/includes/api/RandomImageGenerator.php
___________________________________________________________________
Added: svn:mergeinfo
1291 Merged /trunk/phase3/maintenance/tests/phpunit/includes/api/RandomImageGenerator.php:r73549-75058,75060-75821
2292 Merged /branches/phpunit-restructure/maintenance/tests/phpunit/includes/api/RandomImageGenerator.php:r72257-72560
Added: svn:eol-style
3293 + native
Index: trunk/phase3/maintenance/tests/phpunit/includes/api/ApiUploadTest.php
——@@ -0,0 +1,645 @@
 2+<?php
 3+
 4+/**
 5+ * @group Database
 6+ * @group Destructive
 7+ */
 8+
 9+/**
 10+ * n.b. Ensure that you can write to the images/ directory as the
 11+ * user that will run tests.
 12+ */
 13+
 14+// Note for reviewers: this intentionally duplicates functionality already in "ApiSetup" and so on.
 15+// This framework works better IMO and has less strangeness (such as test cases inheriting from "ApiSetup"...)
 16+// (and in the case of the other Upload tests, this flat out just actually works... )
 17+
 18+// TODO: refactor into several files
 19+// TODO: port the other Upload tests, and other API tests to this framework
 20+
 21+require_once( dirname( __FILE__ ) . '/RandomImageGenerator.php' );
 22+
 23+/* Wraps the user object, so we can also retain full access to properties like password if we log in via the API */
 24+class ApiTestUser {
 25+ public $username;
 26+ public $password;
 27+ public $email;
 28+ public $groups;
 29+ public $user;
 30+
 31+ function __construct( $username, $realname = 'Real Name', $email = 'sample@sample.com', $groups = array() ) {
 32+ global $wgMinimalPasswordLength;
 33+
 34+ $this->username = $username;
 35+ $this->realname = $realname;
 36+ $this->email = $email;
 37+ $this->groups = $groups;
 38+
 39+ // don't allow user to hardcode or select passwords -- people sometimes run tests
 40+ // on live wikis. Sometimes we create sysop users in these tests. A sysop user with
 41+ // a known password would be a Bad Thing.
 42+ $this->password = User::randomPassword();
 43+
 44+ $this->user = User::newFromName( $this->username );
 45+ $this->user->load();
 46+
 47+ // In an ideal world we'd have a new wiki (or mock data store) for every single test.
 48+ // But for now, we just need to create or update the user with the desired properties.
 49+ // we particularly need the new password, since we just generated it randomly.
 50+ // In core MediaWiki, there is no functionality to delete users, so this is the best we can do.
 51+ if ( !$this->user->getID() ) {
 52+ // create the user
 53+ $this->user = User::createNew(
 54+ $this->username, array(
 55+ "email" => $this->email,
 56+ "real_name" => $this->realname
 57+ )
 58+ );
 59+ if ( !$this->user ) {
 60+ throw new Exception( "error creating user" );
 61+ }
 62+ }
 63+
 64+ // update the user to use the new random password and other details
 65+ $this->user->setPassword( $this->password );
 66+ $this->user->setEmail( $this->email );
 67+ $this->user->setRealName( $this->realname );
 68+ // remove all groups, replace with any groups specified
 69+ foreach ( $this->user->getGroups() as $group ) {
 70+ $this->user->removeGroup( $group );
 71+ }
 72+ if ( count( $this->groups ) ) {
 73+ foreach ( $this->groups as $group ) {
 74+ $this->user->addGroup( $group );
 75+ }
 76+ }
 77+ $this->user->saveSettings();
 78+
 79+ }
 80+
 81+}
 82+
 83+abstract class ApiTestCase extends PHPUnit_Framework_TestCase {
 84+ public static $users;
 85+
 86+ function setUp() {
 87+ global $wgServer, $wgContLang, $wgAuth, $wgMemc, $wgRequest, $wgUser;
 88+
 89+ $wgMemc = new FakeMemCachedClient();
 90+ $wgContLang = Language::factory( 'en' );
 91+ $wgAuth = new StubObject( 'wgAuth', 'AuthPlugin' );
 92+ $wgRequest = new FauxRequest( array() );
 93+
 94+ self::$users = array(
 95+ 'sysop' => new ApiTestUser(
 96+ 'Apitestsysop',
 97+ 'Api Test Sysop',
 98+ 'api_test_sysop@sample.com',
 99+ array( 'sysop' )
 100+ ),
 101+ 'uploader' => new ApiTestUser(
 102+ 'Apitestuser',
 103+ 'Api Test User',
 104+ 'api_test_user@sample.com',
 105+ array()
 106+ )
 107+ );
 108+
 109+ $wgUser = self::$users['sysop']->user;
 110+
 111+ }
 112+
 113+ function tearDown() {
 114+ global $wgMemc;
 115+ $wgMemc = null;
 116+ }
 117+
 118+ protected function doApiRequest( $params, $session = null ) {
 119+ $_SESSION = isset( $session ) ? $session : array();
 120+
 121+ $request = new FauxRequest( $params, true, $_SESSION );
 122+ $module = new ApiMain( $request, true );
 123+ $module->execute();
 124+
 125+ return array( $module->getResultData(), $request, $_SESSION );
 126+ }
 127+
 128+ /**
 129+ * Add an edit token to the API request
 130+ * This is cheating a bit -- we grab a token in the correct format and then add it to the pseudo-session and to the
 131+ * request, without actually requesting a "real" edit token
 132+ * @param $params: key-value API params
 133+ * @param $data: a structure which also contains the session
 134+ */
 135+ protected function doApiRequestWithToken( $params, $session ) {
 136+ if ( $session['wsToken'] ) {
 137+ // add edit token to fake session
 138+ $session['wsEditToken'] = $session['wsToken'];
 139+ // add token to request parameters
 140+ $params['token'] = md5( $session['wsToken'] ) . EDIT_TOKEN_SUFFIX;
 141+ return $this->doApiRequest( $params, $session );
 142+ } else {
 143+ throw new Exception( "request data not in right format" );
 144+ }
 145+ }
 146+
 147+}
 148+
 149+class ApiUploadTest extends ApiTestCase {
 150+ /**
 151+ * Fixture -- run before every test
 152+ */
 153+ public function setUp() {
 154+ global $wgEnableUploads, $wgEnableAPI, $wgDebugLogFile;
 155+ parent::setUp();
 156+
 157+ $wgEnableUploads = true;
 158+ $wgEnableAPI = true;
 159+ wfSetupSession();
 160+
 161+ $wgDebugLogFile = '/private/tmp/mwtestdebug.log';
 162+ ini_set( 'log_errors', 1 );
 163+ ini_set( 'error_reporting', 1 );
 164+ ini_set( 'display_errors', 1 );
 165+
 166+ $this->clearFakeUploads();
 167+ }
 168+
 169+ /**
 170+ * Fixture -- run after every test
 171+ * Clean up temporary files etc.
 172+ */
 173+ function tearDown() {
 174+ }
 175+
 176+
 177+ /**
 178+ * Testing login
 179+ * XXX this is a funny way of getting session context
 180+ */
 181+ function testLogin() {
 182+ $user = self::$users['uploader'];
 183+
 184+ $params = array(
 185+ 'action' => 'login',
 186+ 'lgname' => $user->username,
 187+ 'lgpassword' => $user->password
 188+ );
 189+ list( $result, $request, $session ) = $this->doApiRequest( $params );
 190+ $this->assertArrayHasKey( "login", $result );
 191+ $this->assertArrayHasKey( "result", $result['login'] );
 192+ $this->assertEquals( "NeedToken", $result['login']['result'] );
 193+ $token = $result['login']['token'];
 194+
 195+ $params = array(
 196+ 'action' => 'login',
 197+ 'lgtoken' => $token,
 198+ 'lgname' => $user->username,
 199+ 'lgpassword' => $user->password
 200+ );
 201+ list( $result, $request, $session ) = $this->doApiRequest( $params );
 202+ $this->assertArrayHasKey( "login", $result );
 203+ $this->assertArrayHasKey( "result", $result['login'] );
 204+ $this->assertEquals( "Success", $result['login']['result'] );
 205+ $this->assertArrayHasKey( 'lgtoken', $result['login'] );
 206+
 207+ return $session;
 208+
 209+ }
 210+
 211+ /**
 212+ * @depends testLogin
 213+ */
 214+ public function testUploadRequiresToken( $session ) {
 215+ $exception = false;
 216+ try {
 217+ $this->doApiRequest( array(
 218+ 'action' => 'upload'
 219+ ) );
 220+ } catch ( UsageException $e ) {
 221+ $exception = true;
 222+ $this->assertEquals( "The token parameter must be set", $e->getMessage() );
 223+ }
 224+ $this->assertTrue( $exception, "Got exception" );
 225+ }
 226+
 227+ /**
 228+ * @depends testLogin
 229+ */
 230+ public function testUploadMissingParams( $session ) {
 231+ global $wgUser;
 232+ $wgUser = self::$users['uploader']->user;
 233+
 234+ $exception = false;
 235+ try {
 236+ $this->doApiRequestWithToken( array(
 237+ 'action' => 'upload',
 238+ ), $session );
 239+ } catch ( UsageException $e ) {
 240+ $exception = true;
 241+ $this->assertEquals( "One of the parameters sessionkey, file, url, statuskey is required",
 242+ $e->getMessage() );
 243+ }
 244+ $this->assertTrue( $exception, "Got exception" );
 245+ }
 246+
 247+
 248+ /**
 249+ * @depends testLogin
 250+ */
 251+ public function testUpload( $session ) {
 252+ global $wgUser;
 253+ $wgUser = self::$users['uploader']->user;
 254+
 255+ $extension = 'png';
 256+ $mimeType = 'image/png';
 257+
 258+ $randomImageGenerator = new RandomImageGenerator();
 259+ $filePaths = $randomImageGenerator->writeImages( 1, $extension, dirname( wfTempDir() ) );
 260+ $filePath = $filePaths[0];
 261+ $fileName = basename( $filePath );
 262+
 263+ $this->deleteFileByFileName( $fileName );
 264+ $this->deleteFileByContent( $filePath );
 265+
 266+ if (! $this->fakeUploadFile( 'file', $fileName, $mimeType, $filePath ) ) {
 267+ $this->markTestIncomplete( "Couldn't upload file!\n" );
 268+ }
 269+
 270+ $params = array(
 271+ 'action' => 'upload',
 272+ 'filename' => $fileName,
 273+ 'file' => 'dummy content',
 274+ 'comment' => 'dummy comment',
 275+ 'text' => "This is the page text for $fileName",
 276+ );
 277+
 278+ $exception = false;
 279+ try {
 280+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 281+ } catch ( UsageException $e ) {
 282+ $exception = true;
 283+ }
 284+ $this->assertTrue( isset( $result['upload'] ) );
 285+ $this->assertEquals( 'Success', $result['upload']['result'] );
 286+ $this->assertFalse( $exception );
 287+
 288+ // clean up
 289+ $this->deleteFileByFilename( $fileName );
 290+ unlink( $filePath );
 291+ }
 292+
 293+
 294+ /**
 295+ * @depends testLogin
 296+ */
 297+ public function testUploadZeroLength( $session ) {
 298+ global $wgUser;
 299+ $wgUser = self::$users['uploader']->user;
 300+
 301+ $extension = 'png';
 302+ $mimeType = 'image/png';
 303+
 304+ $filePath = tempnam( wfTempDir(), "" );
 305+ $fileName = "apiTestUploadZeroLength.png";
 306+
 307+ $this->deleteFileByFileName( $fileName );
 308+
 309+ if (! $this->fakeUploadFile( 'file', $fileName, $mimeType, $filePath ) ) {
 310+ $this->markTestIncomplete( "Couldn't upload file!\n" );
 311+ }
 312+
 313+ $params = array(
 314+ 'action' => 'upload',
 315+ 'filename' => $fileName,
 316+ 'file' => 'dummy content',
 317+ 'comment' => 'dummy comment',
 318+ 'text' => "This is the page text for $fileName",
 319+ );
 320+
 321+ $exception = false;
 322+ try {
 323+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 324+ } catch ( UsageException $e ) {
 325+ $this->assertContains( 'The file you submitted was empty', $e->getMessage() );
 326+ $exception = true;
 327+ }
 328+ $this->assertTrue( $exception );
 329+
 330+ // clean up
 331+ $this->deleteFileByFilename( $fileName );
 332+ unlink( $filePath );
 333+ }
 334+
 335+
 336+ /**
 337+ * @depends testLogin
 338+ */
 339+ public function testUploadSameFileName( $session ) {
 340+ global $wgUser;
 341+ $wgUser = self::$users['uploader']->user;
 342+
 343+ $extension = 'png';
 344+ $mimeType = 'image/png';
 345+
 346+ $randomImageGenerator = new RandomImageGenerator();
 347+ $filePaths = $randomImageGenerator->writeImages( 2, $extension, dirname( wfTempDir() ) );
 348+ // we'll reuse this filename
 349+ $fileName = basename( $filePaths[0] );
 350+
 351+ // clear any other files with the same name
 352+ $this->deleteFileByFileName( $fileName );
 353+
 354+ // we reuse these params
 355+ $params = array(
 356+ 'action' => 'upload',
 357+ 'filename' => $fileName,
 358+ 'file' => 'dummy content',
 359+ 'comment' => 'dummy comment',
 360+ 'text' => "This is the page text for $fileName",
 361+ );
 362+
 363+ // first upload .... should succeed
 364+
 365+ if (! $this->fakeUploadFile( 'file', $fileName, $mimeType, $filePaths[0] ) ) {
 366+ $this->markTestIncomplete( "Couldn't upload file!\n" );
 367+ }
 368+
 369+ $exception = false;
 370+ try {
 371+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 372+ } catch ( UsageException $e ) {
 373+ $exception = true;
 374+ }
 375+ $this->assertTrue( isset( $result['upload'] ) );
 376+ $this->assertEquals( 'Success', $result['upload']['result'] );
 377+ $this->assertFalse( $exception );
 378+
 379+ // second upload with the same name (but different content)
 380+
 381+ if (! $this->fakeUploadFile( 'file', $fileName, $mimeType, $filePaths[1] ) ) {
 382+ $this->markTestIncomplete( "Couldn't upload file!\n" );
 383+ }
 384+
 385+ $exception = false;
 386+ try {
 387+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 388+ } catch ( UsageException $e ) {
 389+ $exception = true;
 390+ }
 391+ $this->assertTrue( isset( $result['upload'] ) );
 392+ $this->assertEquals( 'Warning', $result['upload']['result'] );
 393+ $this->assertTrue( isset( $result['upload']['warnings'] ) );
 394+ $this->assertTrue( isset( $result['upload']['warnings']['exists'] ) );
 395+ $this->assertFalse( $exception );
 396+
 397+ // clean up
 398+ $this->deleteFileByFilename( $fileName );
 399+ unlink( $filePaths[0] );
 400+ unlink( $filePaths[1] );
 401+ }
 402+
 403+
 404+ /**
 405+ * @depends testLogin
 406+ */
 407+ public function testUploadSameContent( $session ) {
 408+ global $wgUser;
 409+ $wgUser = self::$users['uploader']->user;
 410+
 411+ $extension = 'png';
 412+ $mimeType = 'image/png';
 413+
 414+ $randomImageGenerator = new RandomImageGenerator();
 415+ $filePaths = $randomImageGenerator->writeImages( 1, $extension, dirname( wfTempDir() ) );
 416+ $fileNames[0] = basename( $filePaths[0] );
 417+ $fileNames[1] = "SameContentAs" . $fileNames[0];
 418+
 419+ // clear any other files with the same name or content
 420+ $this->deleteFileByContent( $filePaths[0] );
 421+ $this->deleteFileByFileName( $fileNames[0] );
 422+ $this->deleteFileByFileName( $fileNames[1] );
 423+
 424+ // first upload .... should succeed
 425+
 426+ $params = array(
 427+ 'action' => 'upload',
 428+ 'filename' => $fileNames[0],
 429+ 'file' => 'dummy content',
 430+ 'comment' => 'dummy comment',
 431+ 'text' => "This is the page text for " . $fileNames[0],
 432+ );
 433+
 434+ if (! $this->fakeUploadFile( 'file', $fileNames[0], $mimeType, $filePaths[0] ) ) {
 435+ $this->markTestIncomplete( "Couldn't upload file!\n" );
 436+ }
 437+
 438+ $exception = false;
 439+ try {
 440+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 441+ } catch ( UsageException $e ) {
 442+ $exception = true;
 443+ }
 444+ $this->assertTrue( isset( $result['upload'] ) );
 445+ $this->assertEquals( 'Success', $result['upload']['result'] );
 446+ $this->assertFalse( $exception );
 447+
 448+
 449+ // second upload with the same content (but different name)
 450+
 451+ if (! $this->fakeUploadFile( 'file', $fileNames[1], $mimeType, $filePaths[0] ) ) {
 452+ $this->markTestIncomplete( "Couldn't upload file!\n" );
 453+ }
 454+
 455+ $params = array(
 456+ 'action' => 'upload',
 457+ 'filename' => $fileNames[1],
 458+ 'file' => 'dummy content',
 459+ 'comment' => 'dummy comment',
 460+ 'text' => "This is the page text for " . $fileNames[1],
 461+ );
 462+
 463+ $exception = false;
 464+ try {
 465+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 466+ } catch ( UsageException $e ) {
 467+ $exception = true;
 468+ }
 469+ $this->assertTrue( isset( $result['upload'] ) );
 470+ $this->assertEquals( 'Warning', $result['upload']['result'] );
 471+ $this->assertTrue( isset( $result['upload']['warnings'] ) );
 472+ $this->assertTrue( isset( $result['upload']['warnings']['duplicate'] ) );
 473+ $this->assertFalse( $exception );
 474+
 475+ // clean up
 476+ $this->deleteFileByFilename( $fileNames[0] );
 477+ $this->deleteFileByFilename( $fileNames[1] );
 478+ unlink( $filePaths[0] );
 479+ }
 480+
 481+
 482+ /**
 483+ * @depends testLogin
 484+ */
 485+ public function testUploadStash( $session ) {
 486+ global $wgUser;
 487+ $wgUser = self::$users['uploader']->user;
 488+
 489+ $extension = 'png';
 490+ $mimeType = 'image/png';
 491+
 492+ $randomImageGenerator = new RandomImageGenerator();
 493+ $filePaths = $randomImageGenerator->writeImages( 1, $extension, dirname( wfTempDir() ) );
 494+ $filePath = $filePaths[0];
 495+ $fileName = basename( $filePath );
 496+
 497+ $this->deleteFileByFileName( $fileName );
 498+ $this->deleteFileByContent( $filePath );
 499+
 500+ if (! $this->fakeUploadFile( 'file', $fileName, $mimeType, $filePath ) ) {
 501+ $this->markTestIncomplete( "Couldn't upload file!\n" );
 502+ }
 503+
 504+ $params = array(
 505+ 'action' => 'upload',
 506+ 'stash' => 1,
 507+ 'filename' => $fileName,
 508+ 'file' => 'dummy content',
 509+ 'comment' => 'dummy comment',
 510+ 'text' => "This is the page text for $fileName",
 511+ );
 512+
 513+ $exception = false;
 514+ try {
 515+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 516+ } catch ( UsageException $e ) {
 517+ $exception = true;
 518+ }
 519+ $this->assertFalse( $exception );
 520+ $this->assertTrue( isset( $result['upload'] ) );
 521+ $this->assertEquals( 'Success', $result['upload']['result'] );
 522+ $this->assertTrue( isset( $result['upload']['sessionkey'] ) );
 523+ $sessionkey = $result['upload']['sessionkey'];
 524+
 525+ // it should be visible from Special:UploadStash
 526+ // XXX ...but how to test this, with a fake WebRequest with the session?
 527+
 528+ // now we should try to release the file from stash
 529+ $params = array(
 530+ 'action' => 'upload',
 531+ 'sessionkey' => $sessionkey,
 532+ 'filename' => $fileName,
 533+ 'comment' => 'dummy comment',
 534+ 'text' => "This is the page text for $fileName, altered",
 535+ );
 536+
 537+ $this->clearFakeUploads();
 538+ $exception = false;
 539+ try {
 540+ list( $result, $request, $session ) = $this->doApiRequestWithToken( $params, $session );
 541+ } catch ( UsageException $e ) {
 542+ $exception = true;
 543+ }
 544+ $this->assertTrue( isset( $result['upload'] ) );
 545+ $this->assertEquals( 'Success', $result['upload']['result'] );
 546+ $this->assertFalse( $exception );
 547+
 548+ // clean up
 549+ $this->deleteFileByFilename( $fileName );
 550+ unlink( $filePath );
 551+ }
 552+
 553+
 554+
 555+ /**
 556+ * Helper function -- remove files and associated articles by Title
 557+ * @param {Title} title to be removed
 558+ */
 559+ public function deleteFileByTitle( $title ) {
 560+ if ( $title->exists() ) {
 561+ $file = wfFindFile( $title, array( 'ignoreRedirect' => true ) );
 562+ $noOldArchive = ""; // yes this really needs to be set this way
 563+ $comment = "removing for test";
 564+ $restrictDeletedVersions = false;
 565+ $status = FileDeleteForm::doDelete( $title, $file, $noOldArchive, $comment, $restrictDeletedVersions );
 566+ if ( !$status->isGood() ) {
 567+ return false;
 568+ }
 569+ $article = new Article( $title );
 570+ $article->doDeleteArticle( "removing for test" );
 571+
 572+ // see if it now doesn't exist; reload
 573+ $title = Title::newFromText( $fileName, NS_FILE );
 574+ }
 575+ return ! ( $title && is_a( $title, 'Title' ) && $title->exists() );
 576+ }
 577+
 578+ /**
 579+ * Helper function -- remove files and associated articles with a particular filename
 580+ * @param {String} filename to be removed
 581+ */
 582+ public function deleteFileByFileName( $fileName ) {
 583+ return $this->deleteFileByTitle( Title::newFromText( $fileName, NS_FILE ) );
 584+ }
 585+
 586+
 587+ /**
 588+ * Helper function -- given a file on the filesystem, find matching content in the db (and associated articles) and remove them.
 589+ * @param {String} path to file on the filesystem
 590+ */
 591+ public function deleteFileByContent( $filePath ) {
 592+ $hash = File::sha1Base36( $filePath );
 593+ $dupes = RepoGroup::singleton()->findBySha1( $hash );
 594+ $success = true;
 595+ foreach ( $dupes as $key => $dupe ) {
 596+ $success &= $this->deleteFileByTitle( $dupe->getTitle() );
 597+ }
 598+ return $success;
 599+ }
 600+
 601+ /**
 602+ * Fake an upload by dumping the file into temp space, and adding info to $_FILES.
 603+ * (This is what PHP would normally do).
 604+ * @param {String}: fieldname - name this would have in the upload form
 605+ * @param {String}: fileName - name to title this
 606+ * @param {String}: mime type
 607+ * @param {String}: filePath - path where to find file contents
 608+ */
 609+ function fakeUploadFile( $fieldName, $fileName, $type, $filePath ) {
 610+ $tmpName = tempnam( wfTempDir(), "" );
 611+ if ( !file_exists( $filePath ) ) {
 612+ throw new Exception( "$filePath doesn't exist!" );
 613+ };
 614+
 615+ if ( !copy( $filePath, $tmpName ) ) {
 616+ throw new Exception( "couldn't copy $filePath to $tmpName" );
 617+ }
 618+
 619+ clearstatcache();
 620+ $size = filesize( $tmpName );
 621+ if ( $size === false ) {
 622+ throw new Exception( "couldn't stat $tmpName" );
 623+ }
 624+
 625+ $_FILES[ $fieldName ] = array(
 626+ 'name' => $fileName,
 627+ 'type' => $type,
 628+ 'tmp_name' => $tmpName,
 629+ 'size' => $size,
 630+ 'error' => null
 631+ );
 632+
 633+ return true;
 634+
 635+ }
 636+
 637+ /**
 638+ * Remove traces of previous fake uploads
 639+ */
 640+ function clearFakeUploads() {
 641+ $_FILES = array();
 642+ }
 643+
 644+
 645+}
 646+
Property changes on: trunk/phase3/maintenance/tests/phpunit/includes/api/ApiUploadTest.php
___________________________________________________________________
Added: svn:mergeinfo
1647 Merged /trunk/phase3/maintenance/tests/phpunit/includes/api/ApiUploadTest.php:r73549-75058,75060-75821
2648 Merged /branches/phpunit-restructure/maintenance/tests/phpunit/includes/api/ApiUploadTest.php:r72257-72560
Added: svn:eol-style
3649 + native
Index: trunk/phase3/maintenance/tests/phpunit/includes/api/generateRandomImages.php
——@@ -0,0 +1,25 @@
 2+<?php
 3+
 4+require("RandomImageGenerator.php");
 5+
 6+$getOptSpec = array(
 7+ 'dictionaryFile::',
 8+ 'minWidth::',
 9+ 'maxWidth::',
 10+ 'minHeight::',
 11+ 'maxHeight::',
 12+ 'circlesToDraw::',
 13+
 14+ 'number::',
 15+ 'format::'
 16+);
 17+$options = getopt( null, $getOptSpec );
 18+
 19+$format = isset( $options['format'] ) ? $options['format'] : 'jpg';
 20+unset( $options['format'] );
 21+
 22+$number = isset( $options['number'] ) ? int( $options['number'] ) : 10;
 23+unset( $options['number'] );
 24+
 25+$randomImageGenerator = new RandomImageGenerator( $options );
 26+$randomImageGenerator->writeImages( $number, $format );
Property changes on: trunk/phase3/maintenance/tests/phpunit/includes/api/generateRandomImages.php
___________________________________________________________________
Added: svn:mergeinfo
127 Merged /branches/phpunit-restructure/maintenance/tests/phpunit/includes/api/generateRandomImages.php:r72257-72560
228 Merged /trunk/phase3/maintenance/tests/phpunit/includes/api/generateRandomImages.php:r73549-75058,75060-75821
Added: svn:eol-style
329 + native
Index: trunk/phase3/maintenance/tests/phpunit/Makefile
——@@ -4,7 +4,8 @@
55 SHELL = /bin/sh
66 CONFIG_FILE = $(shell pwd)/suite.xml
77 FLAGS =
8 -PU = php phpunit.php --configuration ${CONFIG_FILE}
 8+PHP = php
 9+PU = ${PHP} phpunit.php --configuration ${CONFIG_FILE}
910
1011 all test: warning
1112
——@@ -73,3 +74,4 @@
7475 # Options:
7576 # CONFIG_FILE Path to a PHPUnit configuration file (default: suite.xml)
7677 # FLAGS Additional flags to pass to PHPUnit
 78+ # PHP Path to php
Property changes on: trunk/phase3/maintenance/tests/phpunit/Makefile
___________________________________________________________________
Added: svn:mergeinfo
7779 Merged /branches/uploadwizard/phase3/maintenance/tests/phpunit/Makefile:r73550-75905
7880 Merged /branches/phpunit-restructure/maintenance/tests/phpunit/Makefile:r72257-72560
Index: trunk/phase3/includes/upload/UploadBase.php
——@@ -600,6 +600,9 @@
601601 }
602602
603603 /**
 604+ * NOTE: Probably should be deprecated in favor of UploadStash, but this is sometimes
 605+ * called outside that context.
 606+ *
604607 * Stash a file in a temporary directory for later processing
605608 * after the user has confirmed it.
606609 *
——@@ -617,40 +620,36 @@
618621 }
619622
620623 /**
621 - * Stash a file in a temporary directory for later processing,
622 - * and save the necessary descriptive info into the session.
623 - * Returns a key value which will be passed through a form
624 - * to pick up the path info on a later invocation.
 624+ * If the user does not supply all necessary information in the first upload form submission (either by accident or
 625+ * by design) then we may want to stash the file temporarily, get more information, and publish the file later.
625626 *
626 - * @return Integer: session key
 627+ * This method will stash a file in a temporary directory for later processing, and save the necessary descriptive info
 628+ * into the user's session.
 629+ * This method returns the file object, which also has a 'sessionKey' property which can be passed through a form or
 630+ * API request to find this stashed file again.
 631+ *
 632+ * @param {String}: $key (optional) the session key used to find the file info again. If not supplied, a key will be autogenerated.
 633+ * @return {File}: stashed file
627634 */
628 - public function stashSession( $key = null ) {
629 - $status = $this->saveTempUploadedFile( $this->mDestName, $this->mTempPath );
630 - if( !$status->isOK() ) {
631 - # Couldn't save the file.
632 - return false;
633 - }
634 -
635 - if ( is_null( $key ) ) {
636 - $key = $this->getSessionKey();
637 - }
638 - $_SESSION[self::SESSION_KEYNAME][$key] = array(
639 - 'mTempPath' => $status->value,
640 - 'mFileSize' => $this->mFileSize,
641 - 'mFileProps' => $this->mFileProps,
642 - 'version' => self::SESSION_VERSION,
 635+ public function stashSessionFile( $key = null ) {
 636+ $stash = new UploadStash();
 637+ $data = array(
 638+ 'mFileProps' => $this->mFileProps
643639 );
644 - return $key;
 640+ $file = $stash->stashFile( $this->mTempPath, $data, $key );
 641+ // TODO should we change the "local file" here?
 642+ // $this->mLocalFile = $file;
 643+ return $file;
645644 }
646645
647646 /**
648 - * Generate a random session key from stash in cases where we want
649 - * to start an upload without much information
 647+ * Stash a file in a temporary directory, returning a key which can be used to find the file again. See stashSessionFile().
 648+ *
 649+ * @param {String}: $key (optional) the session key used to find the file info again. If not supplied, a key will be autogenerated.
 650+ * @return {String}: session key
650651 */
651 - protected function getSessionKey() {
652 - $key = mt_rand( 0, 0x7fffffff );
653 - $_SESSION[self::SESSION_KEYNAME][$key] = array();
654 - return $key;
 652+ public function stashSession( $key = null ) {
 653+ return $this->stashSessionFile( $key )->getSessionKey();
655654 }
656655
657656 /**
——@@ -1197,12 +1196,23 @@
11981197 return $blacklist;
11991198 }
12001199
 1200+ /**
 1201+ * Gets image info about the file just uploaded.
 1202+ *
 1203+ * Also has the effect of setting metadata to be an 'indexed tag name' in returned API result if
 1204+ * 'metadata' was requested. Oddly, we have to pass the "result" object down just so it can do that
 1205+ * with the appropriate format, presumably.
 1206+ *
 1207+ * @param {ApiResult}
 1208+ * @return {Array} image info
 1209+ */
12011210 public function getImageInfo( $result ) {
12021211 $file = $this->getLocalFile();
12031212 $imParam = ApiQueryImageInfo::getPropertyNames();
12041213 return ApiQueryImageInfo::getInfo( $file, array_flip( $imParam ), $result );
12051214 }
12061215
 1216+
12071217 public function convertVerifyErrorToStatus( $error ) {
12081218 $code = $error['status'];
12091219 unset( $code['status'] );
Property changes on: trunk/phase3/includes/upload/UploadBase.php
___________________________________________________________________
Added: svn:mergeinfo
12101220 Merged /branches/sqlite/includes/upload/UploadBase.php:r58211-58321
12111221 Merged /branches/new-installer/phase3/includes/upload/UploadBase.php:r43664-66004
12121222 Merged /branches/wmf-deployment/includes/upload/UploadBase.php:r53381
12131223 Merged /branches/uploadwizard/phase3/includes/upload/UploadBase.php:r73550-75905
12141224 Merged /branches/REL1_15/phase3/includes/upload/UploadBase.php:r51646
Index: trunk/phase3/includes/upload/UploadStash.php
——@@ -0,0 +1,406 @@
 2+<?php
 3+/**
 4+ * UploadStash is intended to accomplish a few things:
 5+ * - enable applications to temporarily stash files without publishing them to the wiki.
 6+ * - Several parts of MediaWiki do this in similar ways: UploadBase, UploadWizard, and FirefoggChunkedExtension
 7+ * And there are several that reimplement stashing from scratch, in idiosyncratic ways. The idea is to unify them all here.
 8+ * Mostly all of them are the same except for storing some custom fields, which we subsume into the data array.
 9+ * - enable applications to find said files later, as long as the session or temp files haven't been purged.
 10+ * - enable the uploading user (and *ONLY* the uploading user) to access said files, and thumbnails of said files, via a URL.
 11+ * We accomplish this by making the session serve as a URL->file mapping, on the assumption that nobody else can access
 12+ * the session, even the uploading user. See SpecialUploadStash, which implements a web interface to some files stored this way.
 13+ *
 14+ */
 15+class UploadStash {
 16+ // Format of the key for files -- has to be suitable as a filename itself in some cases.
 17+ // This should encompass a sha1 content hash in hex (new style), or an integer (old style),
 18+ // and also thumbnails with prepended strings like "120px-".
 19+ // The file extension should not be part of the key.
 20+ const KEY_FORMAT_REGEX = '/^[\w-]+$/';
 21+
 22+ // repository that this uses to store temp files
 23+ protected $repo;
 24+
 25+ // array of initialized objects obtained from session (lazily initialized upon getFile())
 26+ private $files = array();
 27+
 28+ // the base URL for files in the stash
 29+ private $baseUrl;
 30+
 31+ // TODO: Once UploadBase starts using this, switch to use these constants rather than UploadBase::SESSION*
 32+ // const SESSION_VERSION = 2;
 33+ // const SESSION_KEYNAME = 'wsUploadData';
 34+
 35+ /**
 36+ * Represents the session which contains temporarily stored files.
 37+ * Designed to be compatible with the session stashing code in UploadBase (should replace it eventually)
 38+ * @param {FileRepo} $repo: optional -- repo in which to store files. Will choose LocalRepo if not supplied.
 39+ */
 40+ public function __construct( $repo = null ) {
 41+
 42+ if ( is_null( $repo ) ) {
 43+ $repo = RepoGroup::singleton()->getLocalRepo();
 44+ }
 45+
 46+ $this->repo = $repo;
 47+
 48+ if ( ! isset( $_SESSION ) ) {
 49+ throw new UploadStashNotAvailableException( 'no session variable' );
 50+ }
 51+
 52+ if ( !isset( $_SESSION[UploadBase::SESSION_KEYNAME] ) ) {
 53+ $_SESSION[UploadBase::SESSION_KEYNAME] = array();
 54+ }
 55+
 56+ $this->baseUrl = SpecialPage::getTitleFor( 'UploadStash' )->getLocalURL();
 57+ }
 58+
 59+ /**
 60+ * Get the base of URLs by which one can access the files
 61+ * @return {String} url
 62+ */
 63+ public function getBaseUrl() {
 64+ return $this->baseUrl;
 65+ }
 66+
 67+ /**
 68+ * Get a file and its metadata from the stash.
 69+ * May throw exception if session data cannot be parsed due to schema change, or key not found.
 70+ * @param {Integer} $key: key
 71+ * @throws UploadStashFileNotFoundException
 72+ * @throws UploadStashBadVersionException
 73+ * @return {UploadStashItem} null if no such item or item out of date, or the item
 74+ */
 75+ public function getFile( $key ) {
 76+ if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) {
 77+ throw new UploadStashBadPathException( "key '$key' is not in a proper format" );
 78+ }
 79+
 80+ if ( !isset( $this->files[$key] ) ) {
 81+ if ( !isset( $_SESSION[UploadBase::SESSION_KEYNAME][$key] ) ) {
 82+ throw new UploadStashFileNotFoundException( "key '$key' not found in session" );
 83+ }
 84+
 85+ $data = $_SESSION[UploadBase::SESSION_KEYNAME][$key];
 86+ // guards against PHP class changing while session data doesn't
 87+ if ($data['version'] !== UploadBase::SESSION_VERSION ) {
 88+ throw new UploadStashBadVersionException( $data['version'] . " does not match current version " . UploadBase::SESSION_VERSION );
 89+ }
 90+
 91+ // separate the stashData into the path, and then the rest of the data
 92+ $path = $data['mTempPath'];
 93+ unset( $data['mTempPath'] );
 94+
 95+ $file = new UploadStashFile( $this, $this->repo, $path, $key, $data );
 96+
 97+ $this->files[$key] = $file;
 98+
 99+ }
 100+ return $this->files[$key];
 101+ }
 102+
 103+ /**
 104+ * Stash a file in a temp directory and record that we did this in the session, along with other metadata.
 105+ * We store data in a flat key-val namespace because that's how UploadBase did it. This also means we have to
 106+ * ensure that the key-val pairs in $data do not overwrite other required fields.
 107+ *
 108+ * @param {String} $path: path to file you want stashed
 109+ * @param {Array} $data: optional, other data you want associated with the file. Do not use 'mTempPath', 'mFileProps', 'mFileSize', or 'version' as keys here
 110+ * @param {String} $key: optional, unique key for this file in this session. Used for directory hashing when storing, otherwise not important
 111+ * @throws UploadStashBadPathException
 112+ * @throws UploadStashFileException
 113+ * @return {null|UploadStashFile} file, or null on failure
 114+ */
 115+ public function stashFile( $path, $data = array(), $key = null ) {
 116+ if ( ! file_exists( $path ) ) {
 117+ throw new UploadStashBadPathException( "path '$path' doesn't exist" );
 118+ }
 119+ $fileProps = File::getPropsFromPath( $path );
 120+
 121+ // If no key was supplied, use content hash. Also has the nice property of collapsing multiple identical files
 122+ // uploaded this session, which could happen if uploads had failed.
 123+ if ( is_null( $key ) ) {
 124+ $key = $fileProps['sha1'];
 125+ }
 126+
 127+ if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) {
 128+ throw new UploadStashBadPathException( "key '$key' is not in a proper format" );
 129+ }
 130+
 131+ // if not already in a temporary area, put it there
 132+ $status = $this->repo->storeTemp( basename( $path ), $path );
 133+ if( ! $status->isOK() ) {
 134+ // It is a convention in MediaWiki to only return one error per API exception, even if multiple errors
 135+ // are available. We use reset() to pick the "first" thing that was wrong, preferring errors to warnings.
 136+ // This is a bit lame, as we may have more info in the $status and we're throwing it away, but to fix it means
 137+ // redesigning API errors significantly.
 138+ // $status->value just contains the virtual URL (if anything) which is probably useless to the caller
 139+ $error = reset( $status->getErrorsArray() );
 140+ if ( ! count( $error ) ) {
 141+ $error = reset( $status->getWarningsArray() );
 142+ if ( ! count( $error ) ) {
 143+ $error = array( 'unknown', 'no error recorded' );
 144+ }
 145+ }
 146+ throw new UploadStashFileException( "error storing file in '$path': " . implode( '; ', $error ) );
 147+ }
 148+ $stashPath = $status->value;
 149+
 150+ // required info we always store. Must trump any other application info in $data
 151+ // 'mTempPath', 'mFileSize', and 'mFileProps' are arbitrary names
 152+ // chosen for compatibility with UploadBase's way of doing this.
 153+ $requiredData = array(
 154+ 'mTempPath' => $stashPath,
 155+ 'mFileSize' => $fileProps['size'],
 156+ 'mFileProps' => $fileProps,
 157+ 'version' => UploadBase::SESSION_VERSION
 158+ );
 159+
 160+ // now, merge required info and extra data into the session. (The extra data changes from application to application.
 161+ // UploadWizard wants different things than say FirefoggChunkedUpload.)
 162+ $_SESSION[UploadBase::SESSION_KEYNAME][$key] = array_merge( $data, $requiredData );
 163+
 164+ return $this->getFile( $key );
 165+ }
 166+
 167+}
 168+
 169+class UploadStashFile extends UnregisteredLocalFile {
 170+ private $sessionStash;
 171+ private $sessionKey;
 172+ private $sessionData;
 173+ private $urlName;
 174+
 175+ /**
 176+ * A LocalFile wrapper around a file that has been temporarily stashed, so we can do things like create thumbnails for it
 177+ * Arguably UnregisteredLocalFile should be handling its own file repo but that class is a bit retarded currently
 178+ * @param {UploadStash} $stash: UploadStash, useful for obtaining config, stashing transformed files
 179+ * @param {FileRepo} $repo: repository where we should find the path
 180+ * @param {String} $path: path to file
 181+ * @param {String} $key: key to store the path and any stashed data under
 182+ * @param {String} $data: any other data we want stored with this file
 183+ * @throws UploadStashBadPathException
 184+ * @throws UploadStashFileNotFoundException
 185+ */
 186+ public function __construct( $stash, $repo, $path, $key, $data ) {
 187+ $this->sessionStash = $stash;
 188+ $this->sessionKey = $key;
 189+ $this->sessionData = $data;
 190+
 191+ // resolve mwrepo:// urls
 192+ if ( $repo->isVirtualUrl( $path ) ) {
 193+ $path = $repo->resolveVirtualUrl( $path );
 194+ }
 195+
 196+ // check if path appears to be sane, no parent traversals, and is in this repo's temp zone.
 197+ $repoTempPath = $repo->getZonePath( 'temp' );
 198+ if ( ( ! $repo->validateFilename( $path ) ) ||
 199+ ( strpos( $path, $repoTempPath ) !== 0 ) ) {
 200+ throw new UploadStashBadPathException( "path '$path' is not valid or is not in repo temp area: '$repoTempPath'" );
 201+ }
 202+
 203+ // check if path exists! and is a plain file.
 204+ if ( ! $repo->fileExists( $path, FileRepo::FILES_ONLY ) ) {
 205+ throw new UploadStashFileNotFoundException( "cannot find path '$path'" );
 206+ }
 207+
 208+ parent::__construct( false, $repo, $path, false );
 209+
 210+ // we will be initializing from some tmpnam files that don't have extensions.
 211+ // most of MediaWiki assumes all uploaded files have good extensions. So, we fix this.
 212+ $this->name = basename( $this->path );
 213+ $this->setExtension();
 214+
 215+ }
 216+
 217+ /**
 218+ * A method needed by the file transforming and scaling routines in File.php
 219+ * We do not necessarily care about doing the description at this point
 220+ * However, we also can't return the empty string, as the rest of MediaWiki demands this (and calls to imagemagick
 221+ * convert require it to be there)
 222+ * @return {String} dummy value
 223+ */
 224+ public function getDescriptionUrl() {
 225+ return $this->getUrl();
 226+ }
 227+
 228+ /**
 229+ * Find or guess extension -- ensuring that our extension matches our mime type.
 230+ * Since these files are constructed from php tempnames they may not start off
 231+ * with an extension.
 232+ * This does not override getExtension() because things like getMimeType() already call getExtension(),
 233+ * and that results in infinite recursion. So, we preemptively *set* the extension so getExtension() can find it.
 234+ * For obvious reasons this should be called as early as possible, as part of initialization
 235+ */
 236+ public function setExtension() {
 237+ // Does this have an extension?
 238+ $n = strrpos( $this->path, '.' );
 239+ $extension = null;
 240+ if ( $n !== false ) {
 241+ $extension = $n ? substr( $this->path, $n + 1 ) : '';
 242+ } else {
 243+ // If not, assume that it should be related to the mime type of the original file.
 244+ //
 245+ // This entire thing is backwards -- we *should* just create an extension based on
 246+ // the mime type of the transformed file, *after* transformation. But File.php demands
 247+ // to know the name of the transformed file before creating it.
 248+ $mimeType = $this->getMimeType();
 249+ $extensions = explode( ' ', MimeMagic::singleton()->getExtensionsForType( $mimeType ) );
 250+ if ( count( $extensions ) ) {
 251+ $extension = $extensions[0];
 252+ }
 253+ }
 254+
 255+ if ( is_null( $extension ) ) {
 256+ throw new UploadStashFileException( "extension '$extension' is null" );
 257+ }
 258+
 259+ $this->extension = parent::normalizeExtension( $extension );
 260+ }
 261+
 262+ /**
 263+ * Get the path for the thumbnail (actually any transformation of this file)
 264+ * The actual argument is the result of thumbName although we seem to have
 265+ * buggy code elsewhere that expects a boolean 'suffix'
 266+ *
 267+ * @param {String|false} $thumbName: name of thumbnail (e.g. "120px-123456.jpg" ), or false to just get the path
 268+ * @return {String} path thumbnail should take on filesystem, or containing directory if thumbname is false
 269+ */
 270+ public function getThumbPath( $thumbName = false ) {
 271+ $path = dirname( $this->path );
 272+ if ( $thumbName !== false ) {
 273+ $path .= "/$thumbName";
 274+ }
 275+ return $path;
 276+ }
 277+
 278+ /**
 279+ * Return the file/url base name of a thumbnail with the specified parameters
 280+ *
 281+ * @param {Array} $params: handler-specific parameters
 282+ * @return {String|null} base name for URL, like '120px-12345.jpg', or null if there is no handler
 283+ */
 284+ function thumbName( $params ) {
 285+ if ( !$this->getHandler() ) {
 286+ return null;
 287+ }
 288+ $extension = $this->getExtension();
 289+ list( $thumbExt, $thumbMime ) = $this->handler->getThumbType( $extension, $this->getMimeType(), $params );
 290+ $thumbName = $this->getHandler()->makeParamString( $params ) . '-' . $this->getUrlName();
 291+ if ( $thumbExt != $extension ) {
 292+ $thumbName .= ".$thumbExt";
 293+ }
 294+ return $thumbName;
 295+ }
 296+
 297+ /**
 298+ * Get a URL to access the thumbnail
 299+ * This is required because the model of how files work requires that
 300+ * the thumbnail urls be predictable. However, in our model the URL is not based on the filename
 301+ * (that's hidden in the session)
 302+ *
 303+ * @param {String} $thumbName: basename of thumbnail file -- however, we don't want to use the file exactly
 304+ * @return {String} URL to access thumbnail, or URL with partial path
 305+ */
 306+ public function getThumbUrl( $thumbName = false ) {
 307+ $path = $this->sessionStash->getBaseUrl();
 308+ if ( $thumbName !== false ) {
 309+ $path .= '/' . rawurlencode( $thumbName );
 310+ }
 311+ return $path;
 312+ }
 313+
 314+ /**
 315+ * The basename for the URL, which we want to not be related to the filename.
 316+ * Will also be used as the lookup key for a thumbnail file.
 317+ * @return {String} base url name, like '120px-123456.jpg'
 318+ */
 319+ public function getUrlName() {
 320+ if ( ! $this->urlName ) {
 321+ $this->urlName = $this->sessionKey . '.' . $this->getExtension();
 322+ }
 323+ return $this->urlName;
 324+ }
 325+
 326+ /**
 327+ * Return the URL of the file, if for some reason we wanted to download it
 328+ * We tend not to do this for the original file, but we do want thumb icons
 329+ * @return {String} url
 330+ */
 331+ public function getUrl() {
 332+ if ( !isset( $this->url ) ) {
 333+ $this->url = $this->sessionStash->getBaseUrl() . '/' . $this->getUrlName();
 334+ }
 335+ return $this->url;
 336+ }
 337+
 338+ /**
 339+ * Parent classes use this method, for no obvious reason, to return the path (relative to wiki root, I assume).
 340+ * But with this class, the URL is unrelated to the path.
 341+ *
 342+ * @return {String} url
 343+ */
 344+ public function getFullUrl() {
 345+ return $this->getUrl();
 346+ }
 347+
 348+
 349+ /**
 350+ * Getter for session key (the session-unique id by which this file's location & metadata is stored in the session)
 351+ * @return {String} session key
 352+ */
 353+ public function getSessionKey() {
 354+ return $this->sessionKey;
 355+ }
 356+
 357+ /**
 358+ * Typically, transform() returns a ThumbnailImage, which you can think of as being the exact
 359+ * equivalent of an HTML thumbnail on Wikipedia. So its URL is the full-size file, not the thumbnail's URL.
 360+ *
 361+ * Here we override transform() to stash the thumbnail file, and then
 362+ * provide a way to get at the stashed thumbnail file to extract properties such as its URL
 363+ *
 364+ * @param {Array} $params: parameters suitable for File::transform()
 365+ * @param {Bitmask} $flags: flags suitable for File::transform()
 366+ * @return {ThumbnailImage} with additional File thumbnailFile property
 367+ */
 368+ public function transform( $params, $flags = 0 ) {
 369+
 370+ // force it to get a thumbnail right away
 371+ $flags |= self::RENDER_NOW;
 372+
 373+ // returns a ThumbnailImage object containing the url and path. Note. NOT A FILE OBJECT.
 374+ $thumb = parent::transform( $params, $flags );
 375+ $key = $this->thumbName($params);
 376+
 377+ // remove extension, so it's stored in the session under '120px-123456'
 378+ // this makes it uniform with the other session key for the original, '123456'
 379+ $n = strrpos( $key, '.' );
 380+ if ( $n !== false ) {
 381+ $key = substr( $key, 0, $n );
 382+ }
 383+
 384+ // stash the thumbnail File, and provide our caller with a way to get at its properties
 385+ $stashedThumbFile = $this->sessionStash->stashFile( $thumb->path, array(), $key );
 386+ $thumb->thumbnailFile = $stashedThumbFile;
 387+
 388+ return $thumb;
 389+
 390+ }
 391+
 392+ /**
 393+ * Remove the associated temporary file
 394+ * @return {Status} success
 395+ */
 396+ public function remove() {
 397+ return $this->repo->freeTemp( $this->path );
 398+ }
 399+
 400+}
 401+
 402+class UploadStashNotAvailableException extends MWException {};
 403+class UploadStashFileNotFoundException extends MWException {};
 404+class UploadStashBadPathException extends MWException {};
 405+class UploadStashBadVersionException extends MWException {};
 406+class UploadStashFileException extends MWException {};
 407+
Property changes on: trunk/phase3/includes/upload/UploadStash.php
___________________________________________________________________
Added: svn:mergeinfo
1408 Merged /branches/uploadwizard/extensions/includes/upload/UploadStash.php:r73550-74029
2409 Merged /branches/new-installer/phase3/includes/upload/UploadStash.php:r43664-66004
3410 Merged /branches/REL1_15/phase3/includes/upload/UploadStash.php:r51646
4411 Merged /branches/sqlite/includes/upload/UploadStash.php:r58211-58321
5412 Merged /trunk/phase3/includes/upload/UploadStash.php:r74019-75058,75060-75821
Added: svn:eol-style
6413 + native
Index: trunk/phase3/includes/upload/UploadFromFile.php
——@@ -52,4 +52,14 @@
5353
5454 return parent::verifyUpload();
5555 }
 56+
 57+ /**
 58+ * Get the path to the file underlying the upload
 59+ * @return String path to file
 60+ */
 61+ public function getFileTempname() {
 62+ return $this->mUpload->getTempname();
 63+ }
 64+
 65+
5666 }
Property changes on: trunk/phase3/includes/upload/UploadFromFile.php
___________________________________________________________________
Added: svn:mergeinfo
5767 Merged /branches/new-installer/phase3/includes/upload/UploadFromFile.php:r43664-66004
5868 Merged /branches/wmf-deployment/includes/upload/UploadFromFile.php:r53381
5969 Merged /branches/uploadwizard/phase3/includes/upload/UploadFromFile.php:r73550-75905
6070 Merged /branches/REL1_15/phase3/includes/upload/UploadFromFile.php:r51646
6171 Merged /branches/sqlite/includes/upload/UploadFromFile.php:r58211-58321
Index: trunk/phase3/includes/filerepo/File.php
——@@ -541,7 +541,7 @@
542542 * @param $params Array: an associative array of handler-specific parameters.
543543 * Typical keys are width, height and page.
544544 * @param $flags Integer: a bitfield, may contain self::RENDER_NOW to force rendering
545 - * @return MediaTransformOutput
 545+ * @return MediaTransformOutput | false
546546 */
547547 function transform( $params, $flags = 0 ) {
548548 global $wgUseSquid, $wgIgnoreImageErrors, $wgThumbnailEpoch, $wgServer;
——@@ -575,7 +575,7 @@
576576 $thumbPath = $this->getThumbPath( $thumbName );
577577 $thumbUrl = $this->getThumbUrl( $thumbName );
578578
579 - if ( $this->repo->canTransformVia404() && !($flags & self::RENDER_NOW ) ) {
 579+ if ( $this->repo && $this->repo->canTransformVia404() && !($flags & self::RENDER_NOW ) ) {
580580 $thumb = $this->handler->getTransform( $this, $thumbPath, $thumbUrl, $params );
581581 break;
582582 }
Property changes on: trunk/phase3/includes/filerepo/File.php
___________________________________________________________________
Added: svn:mergeinfo
583583 Merged /branches/REL1_15/phase3/includes/filerepo/File.php:r51646
584584 Merged /branches/sqlite/includes/filerepo/File.php:r58211-58321
585585 Merged /branches/new-installer/phase3/includes/filerepo/File.php:r43664-66004
586586 Merged /branches/wmf-deployment/includes/filerepo/File.php:r53381
587587 Merged /branches/uploadwizard/phase3/includes/filerepo/File.php:r73550-75905
Index: trunk/phase3/includes/api/ApiQueryImageInfo.php
——@@ -36,8 +36,13 @@
3737 */
3838 class ApiQueryImageInfo extends ApiQueryBase {
3939
40 - public function __construct( $query, $moduleName ) {
41 - parent::__construct( $query, $moduleName, 'ii' );
 40+ public function __construct( $query, $moduleName, $prefix = 'ii' ) {
 41+ // We allow a subclass to override the prefix, to create a related API module.
 42+ // Some other parts of MediaWiki construct this with a null $prefix, which used to be ignored when this only took two arguments
 43+ if ( is_null( $prefix ) ) {
 44+ $prefix = 'ii';
 45+ }
 46+ parent::__construct( $query, $moduleName, $prefix );
4247 }
4348
4449 public function execute() {
——@@ -45,18 +50,8 @@
4651
4752 $prop = array_flip( $params['prop'] );
4853
49 - if ( $params['urlheight'] != - 1 && $params['urlwidth'] == - 1 ) {
50 - $this->dieUsage( 'iiurlheight cannot be used without iiurlwidth', 'iiurlwidth' );
51 - }
 54+ $scale = $this->getScale( $params );
5255
53 - if ( $params['urlwidth'] != - 1 ) {
54 - $scale = array();
55 - $scale['width'] = $params['urlwidth'];
56 - $scale['height'] = $params['urlheight'];
57 - } else {
58 - $scale = null;
59 - }
60 -
6156 $pageIds = $this->getPageSet()->getAllTitlesByNamespace();
6257 if ( !empty( $pageIds[NS_FILE] ) ) {
6358 $titles = array_keys( $pageIds[NS_FILE] );
——@@ -184,6 +179,28 @@
185180 }
186181
187182 /**
 183+ * From parameters, construct a 'scale' array
 184+ * @param {Array} $params
 185+ * @return {null|Array} key-val array of 'width' and 'height', or null
 186+ */
 187+ public function getScale( $params ) {
 188+ $p = $this->getModulePrefix();
 189+ if ( $params['urlheight'] != -1 && $params['urlwidth'] == -1 ) {
 190+ $this->dieUsage( "${p}urlheight cannot be used without {$p}urlwidth", "{$p}urlwidth" );
 191+ }
 192+
 193+ if ( $params['urlwidth'] != -1 ) {
 194+ $scale = array();
 195+ $scale['width'] = $params['urlwidth'];
 196+ $scale['height'] = $params['urlheight'];
 197+ } else {
 198+ $scale = null;
 199+ }
 200+ return $scale;
 201+ }
 202+
 203+
 204+ /**
188205 * Get result information for an image revision
189206 *
190207 * @param $file File object
——@@ -324,11 +341,11 @@
325342 ),
326343 'urlwidth' => array(
327344 ApiBase::PARAM_TYPE => 'integer',
328 - ApiBase::PARAM_DFLT => - 1
 345+ ApiBase::PARAM_DFLT => -1
329346 ),
330347 'urlheight' => array(
331348 ApiBase::PARAM_TYPE => 'integer',
332 - ApiBase::PARAM_DFLT => - 1
 349+ ApiBase::PARAM_DFLT => -1
333350 ),
334351 'continue' => null,
335352 );
——@@ -356,6 +373,11 @@
357374 );
358375 }
359376
 377+
 378+ /**
 379+ * Return the API documentation for the parameters.
 380+ * @return {Array} parameter documentation.
 381+ */
360382 public function getParamDescription() {
361383 $p = $this->getModulePrefix();
362384 return array(
——@@ -375,14 +397,14 @@
376398 ' metadata - Lists EXIF metadata for the version of the image',
377399 ' archivename - Adds the file name of the archive version for non-latest versions',
378400 ' bitdepth - Adds the bit depth of the version',
379 - ),
 401+ ),
 402+ 'urlwidth' => array( "If {$p}prop=url is set, a URL to an image scaled to this width will be returned.",
 403+ 'Only the current version of the image can be scaled' ),
 404+ 'urlheight' => "Similar to {$p}urlwidth. Cannot be used without {$p}urlwidth",
380405 'limit' => 'How many image revisions to return',
381406 'start' => 'Timestamp to start listing from',
382407 'end' => 'Timestamp to stop listing at',
383 - 'urlwidth' => array( "If {$p}prop=url is set, a URL to an image scaled to this width will be returned.",
384 - 'Only the current version of the image can be scaled' ),
385 - 'urlheight' => "Similar to {$p}urlwidth. Cannot be used without {$p}urlwidth",
386 - 'continue' => 'When more results are available, use this to continue',
 408+ 'continue' => 'If the query response includes a continue value, use it here to get another page of results'
387409 );
388410 }
389411
Property changes on: trunk/phase3/includes/api/ApiQueryImageInfo.php
___________________________________________________________________
Added: svn:mergeinfo
390412 Merged /branches/uploadwizard/phase3/includes/api/ApiQueryImageInfo.php:r73550-75905
391413 Merged /branches/REL1_15/phase3/includes/api/ApiQueryImageInfo.php:r51646
392414 Merged /branches/REL1_16/phase3/includes/api/ApiQueryImageInfo.php:r63621-63636,69357
393415 Merged /branches/wmf/1.16wmf4/includes/api/ApiQueryImageInfo.php:r69521
394416 Merged /branches/sqlite/includes/api/ApiQueryImageInfo.php:r58211-58321
395417 Merged /branches/wmf-deployment/includes/api/ApiQueryImageInfo.php:r53381,59952
Index: trunk/phase3/includes/api/ApiUpload.php
——@@ -80,25 +80,66 @@
8181 // Check permission to upload this file
8282 $permErrors = $this->mUpload->verifyPermissions( $wgUser );
8383 if ( $permErrors !== true ) {
84 - // Todo: stash the upload and allow choosing a new name
 84+ // TODO: stash the upload and allow choosing a new name
8585 $this->dieUsageMsg( array( 'badaccess-groups' ) );
8686 }
8787
88 - // Check warnings if necessary
89 - $warnings = $this->checkForWarnings();
90 - if ( $warnings ) {
91 - $this->getResult()->addValue( null, $this->getModuleName(), $warnings );
 88+ // Prepare the API result
 89+ $result = array();
 90+
 91+ $warnings = $this->getApiWarnings();
 92+ if ( $warnings ) {
 93+ $result['result'] = 'Warning';
 94+ $result['warnings'] = $warnings;
 95+ // in case the warnings can be fixed with some further user action, let's stash this upload
 96+ // and return a key they can use to restart it
 97+ try {
 98+ $result['sessionkey'] = $this->performStash();
 99+ } catch ( MWException $e ) {
 100+ $result['warnings']['stashfailed'] = $e->getMessage();
 101+ }
 102+ } elseif ( $this->mParams['stash'] ) {
 103+ // Some uploads can request they be stashed, so as not to publish them immediately.
 104+ // In this case, a failure to stash ought to be fatal
 105+ try {
 106+ $result['result'] = 'Success';
 107+ $result['sessionkey'] = $this->performStash();
 108+ } catch ( MWException $e ) {
 109+ $this->dieUsage( $e->getMessage(), 'stashfailed' );
 110+ }
92111 } else {
93 - // Perform the upload
 112+ // This is the most common case -- a normal upload with no warnings
 113+ // $result will be formatted properly for the API already, with a status
94114 $result = $this->performUpload();
95 - $this->getResult()->addValue( null, $this->getModuleName(), $result );
96115 }
97116
 117+ if ( $result['result'] === 'Success' ) {
 118+ $result['imageinfo'] = $this->mUpload->getImageInfo( $this->getResult() );
 119+ }
 120+
 121+ $this->getResult()->addValue( null, $this->getModuleName(), $result );
 122+
98123 // Cleanup any temporary mess
99124 $this->mUpload->cleanupTempFile();
100125 }
101126
102127 /**
 128+ * Stash the file and return the session key
 129+ * Also re-raises exceptions with slightly more informative message strings (useful for API)
 130+ * @throws MWException
 131+ * @return {String} session key
 132+ */
 133+ function performStash() {
 134+ try {
 135+ $sessionKey = $this->mUpload->stashSessionFile()->getSessionKey();
 136+ } catch ( MWException $e ) {
 137+ throw new MWException( 'Stashing temporary file failed: ' . get_class($e) . ' ' . $e->getMessage() );
 138+ }
 139+ return $sessionKey;
 140+ }
 141+
 142+
 143+ /**
103144 * Select an upload module and set it to mUpload. Dies on failure. If the
104145 * request was a status request and not a true upload, returns false;
105146 * otherwise true
——@@ -106,13 +147,14 @@
107148 * @return bool
108149 */
109150 protected function selectUploadModule() {
 151+ global $wgAllowAsyncCopyUploads;
110152 $request = $this->getMain()->getRequest();
111153
112154 // One and only one of the following parameters is needed
113155 $this->requireOnlyOneParameter( $this->mParams,
114156 'sessionkey', 'file', 'url', 'statuskey' );
115157
116 - if ( isset( $this->mParams['statuskey'] ) ) {
 158+ if ( $wgAllowAsyncCopyUploads && $this->mParams['statuskey'] ) {
117159 // Status request for an async upload
118160 $sessionData = UploadFromUrlJob::getSessionData( $this->mParams['statuskey'] );
119161 if ( !isset( $sessionData['result'] ) ) {
——@@ -126,12 +168,14 @@
127169 return false;
128170
129171 }
130 -
 172+
 173+
131174 // The following modules all require the filename parameter to be set
132175 if ( is_null( $this->mParams['filename'] ) ) {
133176 $this->dieUsageMsg( array( 'missingparam', 'filename' ) );
134177 }
135 -
 178+
 179+
136180 if ( $this->mParams['sessionkey'] ) {
137181 // Upload stashed in a previous request
138182 $sessionData = $request->getSessionData( UploadBase::getSessionKeyName() );
——@@ -249,56 +293,41 @@
250294 }
251295 }
252296
 297+
253298 /**
254299 * Check warnings if ignorewarnings is not set.
255 - * Returns a suitable result array if there were warnings
 300+ * Returns a suitable array for inclusion into API results if there were warnings
 301+ * Returns the empty array if there were no warnings
 302+ *
 303+ * @return array
256304 */
257 - protected function checkForWarnings() {
258 - $result = array();
 305+ protected function getApiWarnings() {
 306+ $warnings = array();
259307
260308 if ( !$this->mParams['ignorewarnings'] ) {
261309 $warnings = $this->mUpload->checkWarnings();
262310 if ( $warnings ) {
263 - $result['result'] = 'Warning';
264 - $result['warnings'] = $this->transformWarnings( $warnings );
 311+ // Add indices
 312+ $this->getResult()->setIndexedTagName( $warnings, 'warning' );
265313
266 - $sessionKey = $this->mUpload->stashSession();
267 - if ( !$sessionKey ) {
268 - $this->dieUsage( 'Stashing temporary file failed', 'stashfailed' );
 314+ if ( isset( $warnings['duplicate'] ) ) {
 315+ $dupes = array();
 316+ foreach ( $warnings['duplicate'] as $dupe ) {
 317+ $dupes[] = $dupe->getName();
 318+ }
 319+ $this->getResult()->setIndexedTagName( $dupes, 'duplicate' );
 320+ $warnings['duplicate'] = $dupes;
269321 }
270322
271 - $result['sessionkey'] = $sessionKey;
272 -
273 - return $result;
 323+ if ( isset( $warnings['exists'] ) ) {
 324+ $warning = $warnings['exists'];
 325+ unset( $warnings['exists'] );
 326+ $warnings[$warning['warning']] = $warning['file']->getName();
 327+ }
274328 }
275329 }
276 - return;
277 - }
278 -
279 - /**
280 - * Transforms a warnings array returned by mUpload->checkWarnings() into
281 - * something that can be directly used as API result
282 - */
283 - protected function transformWarnings( $warnings ) {
284 - // Add indices
285 - $this->getResult()->setIndexedTagName( $warnings, 'warning' );
286330
287 - if ( isset( $warnings['duplicate'] ) ) {
288 - $dupes = array();
289 - foreach ( $warnings['duplicate'] as $dupe ) {
290 - $dupes[] = $dupe->getName();
291 - }
292 - $this->getResult()->setIndexedTagName( $dupes, 'duplicate' );
293 - $warnings['duplicate'] = $dupes;
294 - }
295 -
296 - if ( isset( $warnings['exists'] ) ) {
297 - $warning = $warnings['exists'];
298 - unset( $warnings['exists'] );
299 - $warnings[$warning['warning']] = $warning['file']->getName();
300 - }
301 -
302 - return $warnings;
 331+ return $warnings;
303332 }
304333
305334 /**
——@@ -346,8 +375,8 @@
347376
348377 $result['result'] = 'Success';
349378 $result['filename'] = $file->getName();
350 - $result['imageinfo'] = $this->mUpload->getImageInfo( $this->getResult() );
351379
 380+
352381 return $result;
353382 }
354383
——@@ -384,8 +413,8 @@
385414 'ignorewarnings' => false,
386415 'file' => null,
387416 'url' => null,
388 -
389417 'sessionkey' => null,
 418+ 'stash' => false,
390419 );
391420
392421 global $wgAllowAsyncCopyUploads;
——@@ -410,7 +439,8 @@
411440 'ignorewarnings' => 'Ignore any warnings',
412441 'file' => 'File contents',
413442 'url' => 'Url to fetch the file from',
414 - 'sessionkey' => 'Session key returned by a previous upload that failed due to warnings',
 443+ 'sessionkey' => 'Session key that identifies a previous upload that was stashed temporarily.',
 444+ 'stash' => 'If set, the server will not add the file to the repository and stash it temporarily.'
415445 );
416446
417447 global $wgAllowAsyncCopyUploads;
Property changes on: trunk/phase3/includes/api/ApiUpload.php
___________________________________________________________________
Added: svn:mergeinfo
418448 Merged /branches/uploadwizard/phase3/includes/api/ApiUpload.php:r73550-75905
419449 Merged /branches/REL1_15/phase3/includes/api/ApiUpload.php:r51646
420450 Merged /branches/REL1_16/phase3/includes/api/ApiUpload.php:r63621-63636,69357
421451 Merged /branches/wmf/1.16wmf4/includes/api/ApiUpload.php:r69521
422452 Merged /branches/sqlite/includes/api/ApiUpload.php:r58211-58321
423453 Merged /branches/wmf-deployment/includes/api/ApiUpload.php:r53381,59952
Index: trunk/phase3/includes/AutoLoader.php
——@@ -634,6 +634,7 @@
635635 'SpecialRecentChanges' => 'includes/specials/SpecialRecentchanges.php',
636636 'SpecialRecentchangeslinked' => 'includes/specials/SpecialRecentchangeslinked.php',
637637 'SpecialSearch' => 'includes/specials/SpecialSearch.php',
 638+ 'SpecialUploadStash' => 'includes/specials/SpecialUploadStash.php',
638639 'SpecialSpecialpages' => 'includes/specials/SpecialSpecialpages.php',
639640 'SpecialStatistics' => 'includes/specials/SpecialStatistics.php',
640641 'SpecialTags' => 'includes/specials/SpecialTags.php',
——@@ -669,6 +670,7 @@
670671 'UserloginTemplate' => 'includes/templates/Userlogin.php',
671672
672673 # includes/upload
 674+ 'UploadStash' => 'includes/upload/UploadStash.php',
673675 'UploadBase' => 'includes/upload/UploadBase.php',
674676 'UploadFromStash' => 'includes/upload/UploadFromStash.php',
675677 'UploadFromFile' => 'includes/upload/UploadFromFile.php',
Property changes on: trunk/phase3/includes/AutoLoader.php
___________________________________________________________________
Added: svn:mergeinfo
676678 Merged /branches/sqlite/includes/AutoLoader.php:r58211-58321
677679 Merged /branches/new-installer/phase3/includes/AutoLoader.php:r43664-66004
678680 Merged /branches/wmf-deployment/includes/AutoLoader.php:r53381
679681 Merged /branches/uploadwizard/phase3/includes/AutoLoader.php:r73550-75905
680682 Merged /branches/REL1_15/phase3/includes/AutoLoader.php:r51646
Index: trunk/phase3/includes/specials/SpecialUploadStash.php
——@@ -0,0 +1,140 @@
 2+<?php
 3+/**
 4+ * Special:UploadStash
 5+ *
 6+ * Web access for files temporarily stored by UploadStash.
 7+ *
 8+ * For example -- files that were uploaded with the UploadWizard extension are stored temporarily
 9+ * before committing them to the db. But we want to see their thumbnails and get other information
 10+ * about them.
 11+ *
 12+ * Since this is based on the user's session, in effect this creates a private temporary file area.
 13+ * However, the URLs for the files cannot be shared.
 14+ *
 15+ * @file
 16+ * @ingroup SpecialPage
 17+ * @ingroup Upload
 18+ */
 19+
 20+class SpecialUploadStash extends SpecialPage {
 21+
 22+ static $HttpErrors = array( // FIXME: Use OutputPage::getStatusMessage() --RK
 23+ 400 => 'Bad Request',
 24+ 403 => 'Access Denied',
 25+ 404 => 'File not found',
 26+ 500 => 'Internal Server Error',
 27+ );
 28+
 29+ // UploadStash
 30+ private $stash;
 31+
 32+ // we should not be reading in really big files and serving them out
 33+ private $maxServeFileSize = 262144; // 256K
 34+
 35+ // $request is the request (usually wgRequest)
 36+ // $subpage is everything in the URL after Special:UploadStash
 37+ // FIXME: These parameters don't match SpecialPage::__construct()'s params at all, and are unused --RK
 38+ public function __construct( $request = null, $subpage = null ) {
 39+ parent::__construct( 'UploadStash', 'upload' );
 40+ $this->stash = new UploadStash();
 41+ }
 42+
 43+ /**
 44+ * If file available in stash, cats it out to the client as a simple HTTP response.
 45+ * n.b. Most sanity checking done in UploadStashLocalFile, so this is straightforward.
 46+ *
 47+ * @param {String} $subPage: subpage, e.g. in http://example.com/wiki/Special:UploadStash/foo.jpg, the "foo.jpg" part
 48+ * @return {Boolean} success
 49+ */
 50+ public function execute( $subPage ) {
 51+ global $wgOut, $wgUser;
 52+
 53+ if ( !$this->userCanExecute( $wgUser ) ) {
 54+ $this->displayRestrictionError();
 55+ return;
 56+ }
 57+
 58+ // prevent callers from doing standard HTML output -- we'll take it from here
 59+ $wgOut->disable();
 60+
 61+ try {
 62+ $file = $this->getStashFile( $subPage );
 63+ if ( $file->getSize() > $this->maxServeFileSize ) {
 64+ throw new MWException( 'file size too large' );
 65+ }
 66+ $this->outputFile( $file );
 67+ return true;
 68+
 69+ } catch( UploadStashFileNotFoundException $e ) {
 70+ $code = 404;
 71+ } catch( UploadStashBadPathException $e ) {
 72+ $code = 403;
 73+ } catch( Exception $e ) {
 74+ $code = 500;
 75+ }
 76+
 77+ wfHttpError( $code, self::$HttpErrors[$code], $e->getCode(), $e->getMessage() );
 78+ return false;
 79+ }
 80+
 81+
 82+ /**
 83+ * Convert the incoming url portion (subpage of Special page) into a stashed file, if available.
 84+ * @param {String} $subPage
 85+ * @return {File} file object
 86+ * @throws MWException, UploadStashFileNotFoundException, UploadStashBadPathException
 87+ */
 88+ private function getStashFile( $subPage ) {
 89+ // due to an implementation quirk (and trying to be compatible with older method)
 90+ // the stash key doesn't have an extension
 91+ $key = $subPage;
 92+ $n = strrpos( $subPage, '.' );
 93+ if ( $n !== false ) {
 94+ $key = $n ? substr( $subPage, 0, $n ) : $subPage;
 95+ }
 96+
 97+ try {
 98+ $file = $this->stash->getFile( $key );
 99+ } catch ( UploadStashFileNotFoundException $e ) {
 100+ // if we couldn't find it, and it looks like a thumbnail,
 101+ // and it looks like we have the original, go ahead and generate it
 102+ $matches = array();
 103+ if ( ! preg_match( '/^(\d+)px-(.*)$/', $key, $matches ) ) {
 104+ // that doesn't look like a thumbnail. re-raise exception
 105+ throw $e;
 106+ }
 107+
 108+ list( $dummy, $width, $origKey ) = $matches;
 109+
 110+ // do not trap exceptions, if key is in bad format, or file not found,
 111+ // let exceptions propagate to caller.
 112+ $origFile = $this->stash->getFile( $origKey );
 113+
 114+ // ok we're here so the original must exist. Generate the thumbnail.
 115+ // because the file is a UploadStashFile, this thumbnail will also be stashed,
 116+ // and a thumbnailFile will be created in the thumbnailImage composite object
 117+ $thumbnailImage = null;
 118+ if ( !( $thumbnailImage = $origFile->getThumbnail( $width ) ) ) {
 119+ throw new MWException( 'Could not obtain thumbnail' );
 120+ }
 121+ $file = $thumbnailImage->thumbnailFile;
 122+ }
 123+
 124+ return $file;
 125+ }
 126+
 127+ /**
 128+ * Output HTTP response for file
 129+ * Side effects, obviously, of echoing lots of stuff to stdout.
 130+ * @param {File} file
 131+ */
 132+ private function outputFile( $file ) {
 133+ header( 'Content-Type: ' . $file->getMimeType(), true );
 134+ header( 'Content-Transfer-Encoding: binary', true );
 135+ header( 'Expires: Sun, 17-Jan-2038 19:14:07 GMT', true );
 136+ header( 'Pragma: public', true );
 137+ header( 'Content-Length: ' . $file->getSize(), true ); // FIXME: PHP can handle Content-Length for you just fine --RK
 138+ readfile( $file->getPath() );
 139+ }
 140+}
 141+
Property changes on: trunk/phase3/includes/specials/SpecialUploadStash.php
___________________________________________________________________
Added: svn:mergeinfo
1142 Merged /branches/wmf-deployment/includes/specials/SpecialUploadStash.php:r53381,56967
2143 Merged /branches/REL1_15/phase3/includes/specials/SpecialUploadStash.php:r51646
3144 Merged /branches/sqlite/includes/specials/SpecialUploadStash.php:r58211-58321
4145 Merged /trunk/phase3/includes/specials/SpecialUploadStash.php:r73549-75058,75060-75821
Added: svn:eol-style
5146 + native
Index: trunk/phase3/includes/SpecialPage.php
——@@ -149,6 +149,7 @@
150150 'MIMEsearch' => array( 'SpecialPage', 'MIMEsearch' ),
151151 'FileDuplicateSearch' => array( 'SpecialPage', 'FileDuplicateSearch' ),
152152 'Upload' => 'SpecialUpload',
 153+ 'UploadStash' => 'SpecialUploadStash',
153154
154155 # Wiki data and tools
155156 'Statistics' => 'SpecialStatistics',
Property changes on: trunk/phase3/includes/SpecialPage.php
___________________________________________________________________
Added: svn:mergeinfo
156157 Merged /branches/new-installer/phase3/includes/SpecialPage.php:r43664-66004
157158 Merged /branches/wmf-deployment/includes/SpecialPage.php:r53381
158159 Merged /branches/uploadwizard/phase3/includes/SpecialPage.php:r73550-75905
159160 Merged /branches/REL1_15/phase3/includes/SpecialPage.php:r51646
160161 Merged /branches/sqlite/includes/SpecialPage.php:r58211-58321

Follow-up revisions

RevisionCommit summaryAuthorDate
r75933Follow up r75906....platonides16:42, 3 November 2010
r75995Follow-up r75906: Add new special page to $specialPageAliases too...raymond09:23, 4 November 2010
r76014Initial untested attempt at merging upload backend changes for UploadWizard. ...catrope15:32, 4 November 2010
r76746Followups mostly to r75906....neilk00:24, 16 November 2010
r76750removed URL hackery, now using SpecialPage::getTitleFor(). Followup to r75906neilk01:15, 16 November 2010
r78083re r75906 — replace the removed getSessionKey() with stashSession()mah18:59, 8 December 2010
r81209As per my comments on bug 27038 and my comments on r75906, use a /thumb and /...btongminh16:57, 30 January 2011
r88188Per CR r75906, reintroduce ApiUpload::transformWarnings()btongminh14:46, 15 May 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r73549making a branch for uploadwizardneilk18:02, 22 September 2010

Comments

#Comment by The Evil IP address (talk | contribs)   12:53, 4 November 2010

http://translatewiki.net/wiki/Special:UploadStash "Access denied"

#Comment by Catrope (talk | contribs)   12:55, 4 November 2010

Translation: visiting Special:UploadStash returns in a 403 error. It's not a very helpful error either, because $e->getCode() returns 0 and $e->getMessage() returns "". The special exception classes should have a descriptive default message.

#Comment by NeilK (talk | contribs)   02:07, 5 November 2010

It's not clear to me that this is a bug. These URLs are not intended to be visited by humans loading them as locations in a browser, at least not yet.

#Comment by P858snake (talk | contribs)   02:11, 5 November 2010

Chucking error pages luck such is bad, therefore a bug. if its meant to be inaccessible, chuck something like a unauthorized error at them.

#Comment by Catrope (talk | contribs)   12:03, 5 November 2010

Well at least the error message is unhelpful and not displaying the "real" error message, what I said about getCode() and getMessage().

#Comment by NeilK (talk | contribs)   00:25, 16 November 2010

As of r76746 the error messages are more friendly, and don't return useless codes.

#Comment by Catrope (talk | contribs)   12:56, 4 November 2010

Minor issue I didn't notice before: this code grabs SpecialPage::getTitleFor( 'UploadStash' )->getLocalUrl(); and appends "/$foo" to that later. This'll usually be fine but it might fail on idiosyncratic setups. Using SpecialPage::getTitleFor( 'UploadStash', $foo ) is safer. 

+    static $HttpErrors = array( // FIXME: Use OutputPage::getStatusMessage() --RK

+		header( 'Content-Length: ' . $file->getSize(), true ); // FIXME: PHP can handle Content-Length for you just fine --RK

Self-explanatory 

+		header( 'Pragma: public', true );

What are you trying to accomplish with this? Does it do something similar to Cache-Control: public? If so, you shouldn't set this: the upload stash is private (so public caches shouldn't serve your stashed things to someone else) and there may be key collisions where your upload stash and mine both contain a file called "Foo.jpg" but with different contents, causing strange cache pollution effects (granted, with the default behavior of using the SHA-1 hash this is unlikely to happen and not a problem if it does happen). Sounds like you need something more like Cache-Control: private. 

	public function __construct( $request = null, $subpage = null ) {
+                parent::__construct( 'UploadStash', 'upload' );
+		$this->stash = new UploadStash();

Middle line is space-indented rather than tab-indented.

The rest looks good to me. I haven't reviewed the test changes at all and can't vouch for the upload backend changes. The API changes are OK.

#Comment by NeilK (talk | contribs)   00:37, 16 November 2010

- Now using OutputPage::getStatusMessage() as of r76386 (thanks ialex)

- header( 'Content-Length' ) appears to be required; tried removing it and unsurprisingly PHP did not magically know how large the file was going to be. So, not actually self-explanatory. :) Removed FIXME in r76746.

- header( 'Pragma' ) removed. Was copied wholesale from other code that did something similar, on reflection this was cargo-cult. Pragma: public does exist but is implementation specific for the web server. The appropriate header WOULD be Cache-control: public, but this is unnecessary here as we do not use HTTP Auth. As you mentioned, it cannot possibly be a problem if we have files from two users colliding since we use content-hashes for filenames. (If we had a collision, they're already overwritten on the filesystem anyway, in the current implementation).

- comment about construct() tabs now obsolete

#Comment by NeilK (talk | contribs)   01:16, 16 November 2010

removed url appending; using SpecialPage::getTitleFor() as of r76750

#Comment by Nikerabbit (talk | contribs)   13:06, 4 November 2010

+throw new MWException( 'file size too large' ); Error messages don't need to be cryptic. Perhaps reword as File Foo is too big. Maximum size for stashed files is bar. Actually, is this error shown to the user? Shouldn't it be handled more gracefully than throwing an exception. 

+                if ( $n !== false ) {

Has spaces instead of tabs.

#Comment by NeilK (talk | contribs)   00:39, 16 November 2010

error messages improved in r76746.

spaces have been changed to tabs

#Comment by NeilK (talk | contribs)   00:50, 16 November 2010

Also, no worries about this throwing an exception -- at least in the contexts where I'm using it, exceptions are always caught and transformed into other useful behaviour.

#Comment by MaxSem (talk | contribs)   14:48, 6 November 2010

Constructor throws exceptions if $_SESSION is not available. This leads to exceptions on Special:SpecialPgaes for anons.

#Comment by MaxSem (talk | contribs)   17:44, 6 November 2010

Reported by Sam as bug 25812.

#Comment by NeilK (talk | contribs)   00:18, 16 November 2010

resolved

#Comment by MarkAHershberger (talk | contribs)   17:58, 8 December 2010 

-	protected function getSessionKey() {

Causes this error (found while running the test suite): 

Call to undefined method UploadFromUrl::getSessionKey() in .../includes/upload/UploadFromUrl.php on line 191
#Comment by MarkAHershberger (talk | contribs)   18:58, 8 December 2010

changed the call in UploadFromUrl to a call to stashSession()

#Comment by Bryan (talk | contribs)   21:42, 24 January 2011

Apologies for the late message, but I didn't have time to look into more detail into this before.

SpecialUploadStash.php 

			if ( preg_match( '/^(\d+)px-(.*)$/', $key, $matches ) ) {
				list( /* full match */, $width, $key ) = $matches;
				return $this->outputThumbFromStash( $key, $width );
			} else {
				return $this->outputFileFromStash( $key );
			}

Won't this break terribly if you upload a file which starts with \d+px ? Or is there something in place to prevent this? Regardless I don't really like using the same base url for different purposes. I suggest to use a single level directory to distinguish files and thumbnails, e.g. Special:UploadStash/file/Image.jpg and Special:UploadStash/thumb/240px-Image.jpg

There are some functions like MediaHandler::parseParamString which will do the thumbnail size extraction for you. 

	private function outputLocallyScaledThumb( $file, $params, $flags ) {

		// n.b. this is stupid, we insist on re-transforming the file every time we are invoked. We rely
		// on HTTP caching to ensure this doesn't happen.
		
		$flags |= File::RENDER_NOW;

		$thumbnailImage = $file->transform( $params, $flags );
...
		$thumbFile = new UnregisteredLocalFile( false, $this->stash->repo, $thumbnailImage->getPath(), false );
		if ( ! $thumbFile ) {
			throw new UploadStashFileNotFoundException( "couldn't create local file object for thumbnail" );
		}

		return $this->outputLocalFile( $thumbFile );
	
	}
...
	private function outputLocalFile( $file ) {
		if ( $file->getSize() > self::MAX_SERVE_BYTES ) {
			throw new SpecialUploadStashTooLargeException();
		} 
		self::outputFileHeaders( $file->getMimeType(), $file->getSize() );
		readfile( $file->getPath() );
		return true;
	}

I think that this cause you to send the wrong content-type for files with thumbnail to different types than the original file, such as bmp, tiff and svg. 

	private function outputRemoteScaledThumb( $file, $params, $flags ) {

Is there a reason that in this function you fetch the content and stream it to the user yourself instead of using a 30x? Are the thumbnails of stashed files not available on a public URL?

UploadStash.php 

	public function __construct() { 

		// this might change based on wiki's configuration.
		$this->repo = RepoGroup::singleton()->getLocalRepo();

I think this is the wrong way around. Stashes are repo-bound, but can be bound to any repo. The proper way to get an UploadStash object is in my opinion not (new UploadStash), but $repo->getUploadStash(). The UploadStash constructor would then have $repo as a mandatory parameter and FileRepo::getUploadStash would pass $this to the UploadStash constructor.

#Comment by NeilK (talk | contribs)   22:02, 24 January 2011

> Won't this break terribly if you upload a file which starts with \d+px

We already don't allow you to upload a file that starts with such a sequence, precisely because it breaks the URL scheme. If I recall correctly such filenames are blacklisted in various ways (frontend and backend).

Your idea to have a different URL entirely for thumbs makes sense, though.

> I think that this cause you to send the wrong content-type

I'm not sure why that would be the case, as I'm treating the thumbnail as a separate file with no connection to the original. I have specifically tested this with (for instance) mpeg files that get jpeg previews and sound files that get PNG icon previews. IIRC $file->getMimeType() will actually parse file contents for magic numbers if it wasn't initialized with a mime type.

> Are the thumbnails of stashed files not available on a public URL?

Depending on implementation they may be accessible by public URLs, but only at URL with a random filename that we haven't yet exposed to the user. We don't want the user to know those URLs yet. The idea is to make it so that files that aren't fully published (with licenses and everything) don't have known public URLs.

> I think this is the wrong way around. Stashes are repo-bound, but can be bound to any repo.

I agree this is a bit ugly but otherwise you have to find the repo at some earlier point and keep passing it around. Generally methods in MediaWiki seem to rely on the Repo springing up from these "singletons". A major code smell but that's how things are working now, so this is my solution for now. The comment is there to indicate that one day it might be based on some config variable like $wgUploadStashRepoType or something like that. Feel free to fix it if you see a better way!

#Comment by Bryan (talk | contribs)   22:16, 24 January 2011

>> Won't this break terribly if you upload a file which starts with \d+px

> We already don't allow you to upload a file that starts with such a sequence, precisely because it breaks the URL scheme. If I recall correctly such filenames are blacklisted in various ways (frontend and backend).

It raises a warning, but it is allowed as far as I know.

>> I think this is the wrong way around. Stashes are repo-bound, but can be bound to any repo.

> I agree this is a bit ugly but otherwise you have to find the repo at some earlier point and keep passing it around. Generally methods in MediaWiki seem to rely on the Repo springing up from these "singletons". A major code smell but that's how things are working now, so this is my solution for now. The comment is there to indicate that one day it might be based on some config variable like $wgUploadStashRepoType or something like that. Feel free to fix it if you see a better way!

Was planning to, that's why I pinged you on IRC, but it's getting late now, so it has to wait a few days. I will leave changing the URL scheme to you, as it will probably break the UploadWizard extension.

#Comment by NeilK (talk | contribs)   23:30, 24 January 2011

nah, be bold! :)

#Comment by Reedy (talk | contribs)   21:07, 27 February 2011

This revision removed the transform warnings from ApiUpload, but there is one still call left, on line 169 

			if ( $sessionData['result'] == 'Warning' ) {
				$sessionData['warnings'] = $this->transformWarnings( $sessionData['warnings'] );
				$sessionData['sessionkey'] = $this->mParams['statuskey'];
			}
#Comment by Bryan (talk | contribs)   14:47, 15 May 2011

r88188

Status & tagging log