Restructured upload-by-url:
authorBryan Tong Minh <btongminh@users.mediawiki.org>
Wed, 28 Jul 2010 17:14:51 +0000 (17:14 +0000)
committerBryan Tong Minh <btongminh@users.mediawiki.org>
Wed, 28 Jul 2010 17:14:51 +0000 (17:14 +0000)
* In ApiUpload: moved stuff that is checking instead of actual uploading out of performUpload method
* Made UploadFromUrl conform to standards:
** In initialize* do only initialization, no actual work
** Moved file fetching to fetchFile
** Consistent use of tempnam()
** Perform the uploading in performUpload, don't define our own doUpload method
* Moved almost all job magic to the UploadFromUrlJob class. This way the job is almost a regular client, and we don't need many special cases to deal with async uploading.
* Made leaving a message optional; results will be stored in the session otherwise

I did not actually test the async uploading, because I first wanted to commit a properly working synchronous upload-by-url system.

includes/api/ApiUpload.php
includes/job/UploadFromUrlJob.php
includes/specials/SpecialUpload.php
includes/upload/UploadBase.php
includes/upload/UploadFromUrl.php

index fd8c493..7092803 100644 (file)
@@ -59,7 +59,6 @@ class ApiUpload extends ApiBase {
                        $this->dieUsageMsg( array( 'missingparam', 'filename' ) );
                }
 
-
                if ( $this->mParams['sessionkey'] ) {
                        // Upload stashed in a previous request
                        $sessionData = $request->getSessionData( UploadBase::SESSION_KEYNAME );
@@ -81,52 +80,59 @@ class ApiUpload extends ApiBase {
                        );      
                } elseif ( isset( $this->mParams['url'] ) ) {
                        // Make sure upload by URL is enabled:
-                       if ( !$wgAllowCopyUploads ) {
+                       if ( !UploadFromUrl::isEnabled() ) {
                                $this->dieUsageMsg( array( 'copyuploaddisabled' ) );
                        }
-
-                       $this->mUpload = new UploadFromUrl;
-                       $async = $this->mParams['asyncdownload'] ? 'async' : null;
-                       $this->checkPermissions( $wgUser );
-
-                       $result = $this->mUpload->initialize( 
-                                       $this->mParams['filename'],
-                                       $this->mParams['url'],
-                                       $this->mParams['comment'],
-                                       $this->mParams['watchlist'],
-                                       $this->mParams['ignorewarnings'],
-                                       $async );
-                               
-                       if ( $async ) {
-                               $this->getResult()->addValue( null,
-                                                                                         $this->getModuleName(),
-                                                                                         array( 'queued' => $result ) );
-                               return;
+                       
+                       $async = false;
+                       if ( $this->mParams['asyncdownload'] ) {
+                               if ( $this->mParams['leavemessage'] ) {
+                                       $async = 'async-leavemessage';
+                               } else {
+                                       $async = 'async';
+                               }
                        }
+                       $this->mUpload = new UploadFromUrl;
+                       $this->mUpload->initialize( $this->mParams['filename'],
+                               $this->mParams['url'], $async );
 
-                       $status = $this->mUpload->retrieveFileFromUrl();
-                       if ( !$status->isGood() ) {
-                               $this->getResult()->addValue( null,
-                                                                                         $this->getModuleName(),
-                                                                                         array( 'error' => $status ) );
-                               return;
-                       }
                }
-       
-
-               $this->checkPermissions( $wgUser );
-
                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();
 
-               // Perform the upload
-               $result = $this->performUpload();
+               // 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();
-
-               $this->getResult()->addValue( null, $this->getModuleName(), $result );
        }
 
        /**
@@ -252,18 +258,7 @@ class ApiUpload extends ApiBase {
 
        protected function performUpload() {
                global $wgUser;
-               $permErrors = $this->mUpload->verifyPermissions( $wgUser );
-               if ( $permErrors !== true ) {
-                       $this->dieUsageMsg( array( 'badaccess-groups' ) );
-               }
-
-               $this->verifyUpload();
-
-               $warnings = $this->checkForWarnings();
-               if ( isset( $warnings ) ) {
-                       return $warnings;
-               }
-
+               
                // Use comment as initial page text by default
                if ( is_null( $this->mParams['text'] ) ) {
                        $this->mParams['text'] = $this->mParams['comment'];
@@ -329,6 +324,7 @@ class ApiUpload extends ApiBase {
                        'file' => null,
                        'url' => null,
                        'asyncdownload' => false,
+                       'leavemessage' => false,
                        'sessionkey' => null,
                );
                return $params;
@@ -346,9 +342,8 @@ class ApiUpload extends ApiBase {
                        'file' => 'File contents',
                        'url' => 'Url to fetch the file from',
                        'asyncdownload' => 'Make fetching a URL asyncronous',
-                       'sessionkey' => array(
-                               'Session key returned by a previous upload that failed due to warnings',
-                       ),
+                       'leavemessage' => 'If asyncdownload is used, leave a message on the user talk page if finished',
+                       'sessionkey' => 'Session key returned by a previous upload that failed due to warnings',
                );
        }
 
index 7e1d64a..9b297db 100644 (file)
@@ -1,7 +1,11 @@
 <?php
 
 /**
- * Job for email notification mails
+ * Job for asynchronous upload-by-url. 
+ * 
+ * This job is in fact an interface to UploadFromUrl, which is designed such
+ * that it does not require any globals. If it does, fix it elsewhere, do not
+ * add globals in here.
  *
  * @ingroup JobQueue
  */
@@ -12,19 +16,87 @@ class UploadFromUrlJob extends Job {
        }
 
        public function run() {
-               global $wgUser;
-
-               if ( $this->params['userID'] ) {
-                       $wgUser = User::newFromId( $this->params['userID'] );
+               # Initialize this object and the upload object
+               $upload = new UploadFromUrl();
+               $upload->initialize( 
+                       $this->title->getText(), 
+                       $this->params['url'],
+                       false
+               );
+               $this->user = User::newFromName( $this->params['userName'] );
+               
+               # Fetch the file
+               $status = $upload->fetchFile();
+               if ( !$status->isOk() ) {
+                       $this->leaveMessage( $status );
+                       return;
+               }
+               
+               # Check warnings
+               if ( !$this->params['ignoreWarnings'] ) {
+                       $warnings = $this->checkWarnings();
+                       if ( $warnings ) {
+                               if ( $this->params['leaveMessage'] ) {
+                                       $this->user->leaveUserMessage( 
+                                               wfMsg( 'upload-warning-subj' ),
+                                               wfMsg( 'upload-warning-msg', $this->params['sessionKey'] )
+                                       );
+                               } else {
+                                       $this->storeResultInSession( 'Warning',
+                                               'warnings', $warnings );
+                               }
+                               return;
+                       }
+               }
+               
+               # Perform the upload
+               $status = $upload->performUpload( 
+                       $this->params['comment'],
+                       $this->params['pageText'],
+                       $this->params['watch']
+               );
+               $this->leaveMessage( $status );
+       }
+       
+       /**
+        * Leave a message on the user talk page or in the session according to
+        * $params['leaveMessage'].
+        * 
+        * @param $status Status
+        */
+       protected function leaveMessage( $status ) {
+               if ( $this->params['leaveMessage'] ) {
+                       if ( $status->isGood() ) {
+                               $file = $this->getLocalFile();
+       
+                               $this->user->leaveUserMessage( wfMsg( 'successfulupload' ),
+                                       wfMsg( 'upload-success-msg', $file->getDescriptionUrl() ) );
+                       } else {
+                               $this->user->leaveUserMessage( wfMsg( 'upload-failure-subj' ),
+                                       wfMsg( 'upload-failure-msg', $status->getWikiText() ) );
+                       }
                } else {
-                       $wgUser = new User;
+                       if ( $status->isOk() ) {
+                               $this->storeResultInSession( 'Success', 
+                                       'filename', $this->getLocalFile()->getName() );
+                       } else {
+                               $this->storeResultInSession( 'Failure',
+                                       'errors', $status->getErrorsArray() );
+                       }
+                       
                }
-               $wgUser->mEffectiveGroups[] = 'sysop';
-               $wgUser->mRights = null;
-
-               $upload = new UploadFromUrl();
-               $upload->initializeFromJob( $this );
+       }
 
-               return $upload->doUpload();
+       /**
+        * Store a result in the session data
+        * 
+        * @param $result string The result (Success|Warning|Failure)
+        * @param $dataKey string The key of the extra data
+        * @param $dataKey mixed The extra data itself
+        */
+       protected function storeResultInSession( $result, $dataKey, $dataValue ) {
+               $session &= $_SESSION[UploadBase::getSessionKeyname()][$this->params['sessionKey']];
+               $session['result'] = $result;
+               $session[$dataKey] = $dataValue;
        }
 }
index dd63145..ac1fd45 100644 (file)
@@ -422,10 +422,6 @@ class SpecialUpload extends SpecialPage {
                        return;
                }
 
-               if( $this->mUpload instanceOf UploadFromUrl ) {
-                       return $this->showUploadError( wfMsg( 'uploadfromurl-queued' ) );
-               }
-
                // Fetch the file if required
                $status = $this->mUpload->fetchFile();
                if( !$status->isOK() ) {
index 2e18450..63082a4 100644 (file)
@@ -606,14 +606,16 @@ abstract class UploadBase {
         *
         * @return Integer: session key
         */
-       public function stashSession() {
+       public function stashSession( $key = null ) {
                $status = $this->saveTempUploadedFile( $this->mDestName, $this->mTempPath );
                if( !$status->isOK() ) {
                        # Couldn't save the file.
                        return false;
                }
 
-               $key = $this->getSessionKey();
+               if ( is_null( $key ) ) {
+                       $key = $this->getSessionKey();
+               }
                $_SESSION[self::SESSION_KEYNAME][$key] = array(
                        'mTempPath'       => $status->value,
                        'mFileSize'       => $this->mFileSize,
index cb39655..f22461e 100644 (file)
@@ -9,8 +9,7 @@
  * @author Michael Dale
  */
 class UploadFromUrl extends UploadBase {
-       protected $mTempDownloadPath;
-       protected $comment, $watchList, $ignoreWarnings;
+       protected $mAsync, $mUrl;
 
        /**
         * Checks if the user is allowed to use the upload-by-URL feature. If the
@@ -33,54 +32,22 @@ class UploadFromUrl extends UploadBase {
 
        /**
         * Entry point for API upload
-        * @return bool true on success
+        * 
+        * @param $name string
+        * @param $url string
+        * @param $async mixed Whether the download should be performed 
+        * asynchronous. False for synchronous, async or async-leavemessage for
+        * asynchronous download.
         */
-       public function initialize( $name, $url, $comment, $watchList = null, $ignoreWarn = null, $async = 'async' ) {
+       public function initialize( $name, $url, $async = false ) {
                global $wgUser;
-
-               if ( !Http::isValidURI( $url ) ) {
-                       return Status::newFatal( 'http-invalid-url' );
-               }
-               $params = array(
-                       'userName' => $wgUser->getName(),
-                       'userID' => $wgUser->getID(),
-                       'url' => trim( $url ),
-                       'timestamp' => wfTimestampNow(),
-                       'comment' => $comment,
-                       'watchlist' => $watchList,
-                       'ignorewarnings' => $ignoreWarn );
-
-               $title = Title::newFromText( $name );
-
-               if ( $async == 'async' ) {
-                       $job = new UploadFromUrlJob( $title, $params );
-                       return $job->insert();
-               } else {
-                       $this->mUrl = trim( $url );
-                       $this->comment = $comment;
-                       $this->watchList = $watchList;
-                       $this->ignoreWarnings = $ignoreWarn;
-                       $this->mDesiredDestName = $title;
-                       $this->getTitle();
-
-                       return true;
-               }
-       }
-
-       /**
-        * Initialize a queued download
-        * @param $job Job
-        */
-       public function initializeFromJob( $job ) {
-               global $wgTmpDirectory;
-
-               $this->mUrl = $job->params['url'];
-               $this->mTempPath = tempnam( $wgTmpDirectory, 'COPYUPLOAD' );
-               $this->mDesiredDestName = $job->title;
-               $this->comment = $job->params['comment'];
-               $this->watchList = $job->params['watchlist'];
-               $this->ignoreWarnings = $job->params['ignorewarnings'];
-               $this->getTitle();
+               
+               $this->mUrl = $url;
+               $this->mAsync = $async;
+               
+               $tempPath = $async ? null : $this->makeTemporaryFile();
+               # File size and removeTempFile will be filled in later
+               $this->initializePathInfo( $name, $tempPath, 0, false );
        }
 
        /**
@@ -94,10 +61,7 @@ class UploadFromUrl extends UploadBase {
                return $this->initialize(
                        $desiredDestName,
                        $request->getVal( 'wpUploadFileURL' ),
-                       $request->getVal( 'wpUploadDescription' ),
-                       $request->getVal( 'wpWatchThis' ),
-                       $request->getVal( 'wpIgnoreWarnings' ),
-                       'async'
+                       false
                );
        }
 
@@ -112,23 +76,34 @@ class UploadFromUrl extends UploadBase {
                        && Http::isValidURI( $url )
                        && $wgUser->isAllowed( 'upload_by_url' );
        }
+       
 
+       public function fetchFile() {
+               if ( !Http::isValidURI( $this->mUrl ) ) {
+                       return Status::newFatal( 'http-invalid-url' );
+               }
+               
+               if ( !$this->mAsync ) {
+                       return $this->reallyFetchFile();
+               }
+               return Status::newGood();
+       }
+       protected function makeTemporaryFile() {
+               return tempnam( wfTempDir(), 'URL' );
+       }
        private function saveTempFile( $req ) {
-               $filename = tempnam( wfTempDir(), 'URL' );
-               if ( $filename === false ) {
+               if ( $this->mTempPath === false ) {
                        return Status::newFatal( 'tmp-create-error' );
                }
-               if ( file_put_contents( $filename, $req->getContent() ) === false ) {
+               if ( file_put_contents( $this->mTempPath, $req->getContent() ) === false ) {
                        return Status::newFatal( 'tmp-write-error' );
                }
 
-               $this->mTempPath = $filename;
-               $this->mFileSize = filesize( $filename );
+               $this->mFileSize = filesize( $this->mTempPath );
 
                return Status::newGood();
        }
-
-       public function retrieveFileFromUrl() {
+       protected function reallyFetchFile() {
                $req = HttpRequest::factory( $this->mUrl );
                $status = $req->execute();
 
@@ -145,32 +120,34 @@ class UploadFromUrl extends UploadBase {
                return $status;
        }
 
-       public function doUpload() {
-               global $wgUser;
-
-               $status = $this->retrieveFileFromUrl();
-
-               if ( $status->isGood() ) {
-
-                       $v = $this->verifyUpload();
-                       if ( $v['status'] !== UploadBase::OK ) {
-                               return $this->convertVerifyErrorToStatus( $v['status'], $v['details'] );
-                       }
-
-                       $status = $this->getLocalFile()->upload( $this->mTempPath, $this->comment,
-                               $this->comment, File::DELETE_SOURCE, $this->mFileProps, false, $wgUser );
-               }
-
-               if ( $status->isGood() ) {
-                       $file = $this->getLocalFile();
-
-                       $wgUser->leaveUserMessage( wfMsg( 'successfulupload' ),
-                               wfMsg( 'upload-success-msg', $file->getDescriptionUrl() ) );
-               } else {
-                       $wgUser->leaveUserMessage( wfMsg( 'upload-failure-subj' ),
-                               wfMsg( 'upload-failure-msg', $status->getWikiText() ) );
+       public function performUpload( $comment, $pageText, $watch, $user ) {
+               if ( $this->mAsync ) {
+                       $sessionKey = $this->insertJob( $comment, $pageText, $watch, $user );
+                       
+                       $status = new Status;
+                       $status->error( 'async', $sessionKey );
+                       return $status;
                }
+               
+               return parent::performUpload( $comment, $pageText, $watch, $user );
+       }
 
-               return $status;
+       
+       protected function insertJob( $comment, $pageText, $watch, $user ) {
+               $sessionKey = $this->getSessionKey();
+               $job = new UploadFromUrlJob( $this->getTitle(), array(
+                       'url' => $this->mUrl,
+                       'comment' => $comment,
+                       'pageText' => $pageText,
+                       'watch' => $watch,
+                       'userName' => $user->getName(),
+                       'leaveMessage' => $this->mAsync == 'async-leavemessage',
+                       'ignoreWarnings' => $this->mIgnoreWarnings,
+                       'sessionKey' => $sessionKey,
+               ) );
+               $job->insert();
+               return $sessionKey;
        }
+       
+
 }