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 6a70e5a..fc2fd59 100644 (file)
--- 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 1ec205c..ae16f70 100644 (file)
--- 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 247f608..6368db8 100644 (file)
--- 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." );