From: Bartosz Dziewoński Date: Wed, 17 Aug 2016 13:31:22 +0000 (+0200) Subject: Run 'UploadStashFile' hook for chunked uploads too X-Git-Tag: 1.31.0-rc.0~6021^2 X-Git-Url: http://git.cyclocoop.org/%22.%20generer_url_ecrire%28%22sites_tous%22%2C%22%22%29.%20%22?a=commitdiff_plain;h=c9b5b3e988e3554c231860a2da587dff16b05e0c;p=lhc%2Fweb%2Fwiklou.git Run 'UploadStashFile' hook for chunked uploads too In UploadFromChunks, doStashFile() only stashes the current chunk, and stashing the whole file after all chunks are uploaded is done in concatenateChunks(). Also more custom code on top of custom code to handle ApiMessage errors, because action=upload is special. Follow-up to 2ee27d9a4de163dc3a2b752823a04f5338005627. Change-Id: I968280353f15bbca9b1059631fa2cfd7c3cd2f1d --- diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 6a70e5a010..fc2fd59056 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -240,7 +240,7 @@ class ApiUpload extends ApiBase { 'offset' => $this->mUpload->getOffset(), ]; - $this->dieUsage( $status->getWikiText( false, false, 'en' ), 'stashfailed', 0, $extradata ); + $this->dieStatusWithCode( $status, 'stashfailed', $extradata ); } } @@ -271,7 +271,7 @@ class ApiUpload extends ApiBase { $filekey, [ 'result' => 'Failure', 'stage' => 'assembling', 'status' => $status ] ); - $this->dieUsage( $status->getWikiText( false, false, 'en' ), 'stashfailed' ); + $this->dieStatusWithCode( $status, 'stashfailed' ); } // The fully concatenated file has a new filekey. So remove @@ -382,6 +382,28 @@ class ApiUpload extends ApiBase { $this->dieUsage( $parsed['info'], $parsed['code'], 0, $data ); } + /** + * Like dieStatus(), but always uses $overrideCode for the error code, unless the code comes from + * IApiMessage. + * + * @param Status $status + * @param string $overrideCode Error code to use if there isn't one from IApiMessage + * @param array|null $moreExtraData + * @throws UsageException + */ + public function dieStatusWithCode( $status, $overrideCode, $moreExtraData = null ) { + $extraData = null; + list( $code, $msg ) = $this->getErrorFromStatus( $status, $extraData ); + $errors = $status->getErrorsByType( 'error' ) ?: $status->getErrorsByType( 'warning' ); + if ( !( $errors[0]['message'] instanceof IApiMessage ) ) { + $code = $overrideCode; + } + if ( $moreExtraData ) { + $extraData += $moreExtraData; + } + $this->dieUsage( $msg, $code, 0, $extraData ); + } + /** * Select an upload module and set it to mUpload. Dies on failure. If the * request was a status request and not a true upload, returns false; @@ -404,7 +426,7 @@ class ApiUpload extends ApiBase { if ( !$progress ) { $this->dieUsage( 'No result in status data', 'missingresult' ); } elseif ( !$progress['status']->isGood() ) { - $this->dieUsage( $progress['status']->getWikiText( false, false, 'en' ), 'stashfailed' ); + $this->dieStatusWithCode( $progress['status'], 'stashfailed' ); } if ( isset( $progress['status']->value['verification'] ) ) { $this->checkVerification( $progress['status']->value['verification'] ); @@ -422,7 +444,7 @@ class ApiUpload extends ApiBase { if ( $this->mParams['chunk'] ) { // Chunk upload - $this->mUpload = new UploadFromChunks(); + $this->mUpload = new UploadFromChunks( $this->getUser() ); if ( isset( $this->mParams['filekey'] ) ) { if ( $this->mParams['offset'] === 0 ) { $this->dieUsage( 'Cannot supply a filekey when offset is 0', 'badparams' ); diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 1ec205cc2a..ae16f70227 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -936,13 +936,8 @@ abstract class UploadBase { */ public function tryStashFile( User $user, $isPartial = false ) { if ( !$isPartial ) { - $props = $this->mFileProps; - $error = null; - Hooks::run( 'UploadStashFile', [ $this, $user, $props, &$error ] ); + $error = $this->runUploadStashFileHook( $user ); if ( $error ) { - if ( !is_array( $error ) ) { - $error = [ $error ]; - } return call_user_func_array( 'Status::newFatal', $error ); } } @@ -954,6 +949,22 @@ abstract class UploadBase { } } + /** + * @param User $user + * @return array|null Error message and parameters, null if there's no error + */ + protected function runUploadStashFileHook( User $user ) { + $props = $this->mFileProps; + $error = null; + Hooks::run( 'UploadStashFile', [ $this, $user, $props, &$error ] ); + if ( $error ) { + if ( !is_array( $error ) ) { + $error = [ $error ]; + } + } + return $error; + } + /** * If the user does not supply all necessary information in the first upload * form submission (either by accident or by design) then we may want to diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index 247f608bd3..6368db82da 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -38,12 +38,11 @@ class UploadFromChunks extends UploadFromFile { /** * Setup local pointers to stash, repo and user (similar to UploadFromStash) * - * @param User|null $user Default: null + * @param User $user * @param UploadStash|bool $stash Default: false * @param FileRepo|bool $repo Default: false */ - public function __construct( $user = null, $stash = false, $repo = false ) { - // user object. sometimes this won't exist, as when running from cron. + public function __construct( User $user, $stash = false, $repo = false ) { $this->user = $user; if ( $repo ) { @@ -162,7 +161,20 @@ class UploadFromChunks extends UploadFromFile { // Update the mTempPath and mLocalFile // (for FileUpload or normal Stash to take over) $tStart = microtime( true ); - $this->mLocalFile = parent::doStashFile( $this->user ); + // This is a re-implementation of UploadBase::tryStashFile(), we can't call it because we + // override doStashFile() with completely different functionality in this class... + $error = $this->runUploadStashFileHook( $this->user ); + if ( $error ) { + call_user_func_array( [ $status, 'fatal' ], $error ); + return $status; + } + try { + $this->mLocalFile = parent::doStashFile( $this->user ); + } catch ( UploadStashException $e ) { + $status->fatal( 'uploadstash-exception', get_class( $e ), $e->getMessage() ); + return $status; + } + $tAmount = microtime( true ) - $tStart; $this->mLocalFile->setLocalReference( $tmpFile ); // reuse (e.g. for getImageInfo()) wfDebugLog( 'fileconcatenate', "Stashed combined file ($i chunks) in $tAmount seconds." );