From 1cae743ecb2e29bd2c3d23d3467b067f6b0c912b Mon Sep 17 00:00:00 2001
From: Bryan Tong Minh <btongminh@users.mediawiki.org>
Date: Wed, 28 Jul 2010 17:31:32 +0000
Subject: [PATCH] Refactor some code out of execute into selectUploadModule.
 Fixed an undefined variable.

---
 includes/api/ApiUpload.php | 110 ++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 7092803197..4fb182f251 100644
--- 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 );
 		}
-- 
2.20.1