Refactor some code out of execute into selectUploadModule. Fixed an undefined variable.

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 7092803..4fb182f 100644 (file)
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -45,12 +45,58 @@ class ApiUpload extends ApiBase {
                        $this->dieUsageMsg( array( 'uploaddisabled' ) );
                }
 
+               // Parameter handling
                $this->mParams = $this->extractRequestParams();
                $request = $this->getMain()->getRequest();
-
                // Add the uploaded file to the params array
                $this->mParams['file'] = $request->getFileName( 'file' );
 
+               // Select an upload module
+               $this->selectUploadModule();
+               if ( !isset( $this->mUpload ) ) {
+                       $this->dieUsage( 'No upload module set', 'nomodule' );
+               }
+               
+               // First check permission to upload
+               $this->checkPermissions( $wgUser );
+               // Check permission to upload this file
+               $permErrors = $this->mUpload->verifyPermissions( $wgUser );
+               if ( $permErrors !== true ) {
+                       // Todo: more specific error message
+                       $this->dieUsageMsg( array( 'badaccess-groups' ) );
+               }
+               
+               // Fetch the file
+               $status = $this->mUpload->fetchFile();
+               if ( !$status->isGood() ) {
+                       $errors = $status->getErrorsArray();
+                       $error = array_shift( $errors[0] );
+                       $this->dieUsage( 'Error fetching file from remote source', $error, 0, $errors[0] );
+               }
+
+               // Check if the uploaded file is sane
+               $this->verifyUpload();
+
+               // Check warnings if necessary
+               $warnings = $this->checkForWarnings();
+               if ( $warnings ) {
+                       $this->getResult()->addValue( null, $this->getModuleName(), $warnings );
+               } else {
+                       // Perform the upload
+                       $result = $this->performUpload();
+                       $this->getResult()->addValue( null, $this->getModuleName(), $result );
+               }
+
+               // Cleanup any temporary mess
+               $this->mUpload->cleanupTempFile();
+       }
+       
+       /**
+        * Select an upload module and set it to mUpload. Dies on failure.
+        */
+       protected function selectUploadModule() {
+               $request = $this->getMain()->getRequest();
+               
                // One and only one of the following parameters is needed
                $this->requireOnlyOneParameter( $this->mParams,
                        'sessionkey', 'file', 'url' );
@@ -61,7 +107,7 @@ class ApiUpload extends ApiBase {
 
                if ( $this->mParams['sessionkey'] ) {
                        // Upload stashed in a previous request
-                       $sessionData = $request->getSessionData( UploadBase::SESSION_KEYNAME );
+                       $sessionData = $request->getSessionData( UploadBase::getSessionKeyName() );
                        if ( !UploadFromStash::isValidSessionKey( $this->mParams['sessionkey'], $sessionData ) ) {
                                $this->dieUsageMsg( array( 'invalid-session-key' ) );
                        }
@@ -97,42 +143,6 @@ class ApiUpload extends ApiBase {
                                $this->mParams['url'], $async );
 
                }
-               if ( !isset( $this->mUpload ) ) {
-                       $this->dieUsage( 'No upload module set', 'nomodule' );
-               }
-               
-               // First check permission to upload
-               $this->checkPermissions( $wgUser );
-               // Check permission to upload this file
-               $permErrors = $this->mUpload->verifyPermissions( $wgUser );
-               if ( $permErrors !== true ) {
-                       // Todo: more specific error message
-                       $this->dieUsageMsg( array( 'badaccess-groups' ) );
-               }
-               
-               // Fetch the file
-               $status = $this->mUpload->fetchFile();
-               if ( !$status->isGood() ) {
-                       $errors = $status->getErrorsArray();
-                       $error = array_shift( $errors[0] );
-                       $this->dieUsage( 'Error fetching file from remote source', $error, 0, $errors[0] );
-               }
-
-               // Check if the uploaded file is sane
-               $this->verifyUpload();
-
-               // Check warnings if necessary
-               $warnings = $this->checkForWarnings();
-               if ( $warnings ) {
-                       $this->getResult()->addValue( null, $this->getModuleName(), $warnings );
-               } else {
-                       // Perform the upload
-                       $result = $this->performUpload();
-                       $this->getResult()->addValue( null, $this->getModuleName(), $result );
-               }
-
-               // Cleanup any temporary mess
-               $this->mUpload->cleanupTempFile();
        }
 
        /**
@@ -156,22 +166,12 @@ class ApiUpload extends ApiBase {
        /**
         * Performs file verification, dies on error.
         */
-       public function verifyUpload( ) {
+       protected function verifyUpload( ) {
                $verification = $this->mUpload->verifyUpload( );
                if ( $verification['status'] === UploadBase::OK ) {
-                       return $verification;
+                       return;         
                }
 
-               $this->getVerificationError( $verification );
-       }
-
-       /**
-        * Produce the usage error
-        *
-        * @param $verification array an associative array with the status
-        * key
-        */
-       public function getVerificationError( $verification ) {
                // TODO: Move them to ApiBase's message map
                switch( $verification['status'] ) {
                        case UploadBase::EMPTY_FILE:
@@ -217,6 +217,10 @@ class ApiUpload extends ApiBase {
                }
        }
 
+       /**
+        * Check warnings if ignorewarnings is not set. 
+        * Returns a suitable result array if there were warnings
+        */
        protected function checkForWarnings() {
                $result = array();
 
@@ -256,6 +260,10 @@ class ApiUpload extends ApiBase {
                return;
        }
 
+       /**
+        * Perform the actual upload. Returns a suitable result array on success;
+        * dies on failure.
+        */
        protected function performUpload() {
                global $wgUser;
                
@@ -278,7 +286,7 @@ class ApiUpload extends ApiBase {
 
                if ( !$status->isGood() ) {
                        $error = $status->getErrorsArray();
-                       $this->getResult()->setIndexedTagName( $result['details'], 'error' );
+                       $this->getResult()->setIndexedTagName( $error, 'error' );
 
                        $this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error );
                }