Removed the ability to pass a key into stashFile(), which simplifies the stash row...

Updated UploadFromUrlJob to properly use the database stash
followup to r92200

diff --git a/includes/job/UploadFromUrlJob.php b/includes/job/UploadFromUrlJob.php
index 3f91524..d664e71 100644 (file)
--- a/includes/job/UploadFromUrlJob.php
+++ b/includes/job/UploadFromUrlJob.php
@@ -61,24 +61,24 @@ class UploadFromUrlJob extends Job {
                if ( !$this->params['ignoreWarnings'] ) {
                        $warnings = $this->upload->checkWarnings();
                        if ( $warnings ) {
-                               wfSetupSession( $this->params['sessionId'] );
 
+                               # Stash the upload
+                               $key = $this->upload->stashFile();
+                               
                                if ( $this->params['leaveMessage'] ) {
                                        $this->user->leaveUserMessage(
                                                wfMsg( 'upload-warning-subj' ),
                                                wfMsg( 'upload-warning-msg',
-                                                       $this->params['sessionKey'],
+                                                       $key,
                                                        $this->params['url'] )
                                        );
                                } else {
+                                       wfSetupSession( $this->params['sessionId'] );                                   
                                        $this->storeResultInSession( 'Warning',
                                                'warnings', $warnings );
+                                       session_write_close();
                                }
 
-                               # Stash the upload in the session
-                               $this->upload->stashSession( $this->params['sessionKey'] );
-                               session_write_close();
-
                                return true;
                        }
                }

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 003573f..78f5d66 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -745,11 +745,11 @@ abstract class UploadBase {
         * @param $key String: (optional) the file key used to find the file info again. If not supplied, a key will be autogenerated.
         * @return UploadStashFile stashed file
         */
-       public function stashFile( $key = null ) {
+       public function stashFile() {
                // was stashSessionFile
                $stash = RepoGroup::singleton()->getLocalRepo()->getUploadStash();
 
-               $file = $stash->stashFile( $this->mTempPath, $this->getSourceType(), $key );
+               $file = $stash->stashFile( $this->mTempPath, $this->getSourceType() );
                $this->mLocalFile = $file;
                return $file;
        }
@@ -760,8 +760,8 @@ abstract class UploadBase {
         * @param $key String: (optional) the file key used to find the file info again. If not supplied, a key will be autogenerated.
         * @return String: file key
         */
-       public function stashFileGetKey( $key = null ) {
-               return $this->stashFile( $key )->getFileKey();
+       public function stashFileGetKey() {
+               return $this->stashFile()->getFileKey();
        }
 
        /** 
@@ -770,8 +770,8 @@ abstract class UploadBase {
         * @param $key String: (optional) the file key used to find the file info again. If not supplied, a key will be autogenerated.
         * @return String: file key
         */
-       public function stashSession( $key = null ) {
-               return $this->stashFileGetKey( $key );
+       public function stashSession() {
+               return $this->stashFileGetKey();
        }
 
        /**

diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index 8ea9219..a2f9be5 100644 (file)
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
@@ -102,18 +102,18 @@ class UploadFromStash extends UploadBase {
        /**
         * There is no need to stash the image twice
         */
-       public function stashFile( $key = null ) {
+       public function stashFile() {
                if ( $this->mLocalFile ) {
                        return $this->mLocalFile;
                }
-               return parent::stashFile( $key );
+               return parent::stashFile();
        }
 
        /**
         * Alias for stashFile
         */
-       public function stashSession( $key = null ) {
-               return $this->stashFile( $key );
+       public function stashSession() {
+               return $this->stashFile();
        }
 
        /**

diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index 6727fb0..cad8585 100644 (file)
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
@@ -161,7 +161,7 @@ class UploadStash {
         * @throws UploadStashNotLoggedInException
         * @return UploadStashFile: file, or null on failure
         */
-       public function stashFile( $path, $sourceType = null, $key = null ) {
+       public function stashFile( $path, $sourceType = null ) {
                if ( ! file_exists( $path ) ) {
                        wfDebug( __METHOD__ . " tried to stash file at '$path', but it doesn't exist\n" );
                        throw new UploadStashBadPathException( "path doesn't exist" );
@@ -181,17 +181,16 @@ class UploadStash {
                }
 
                // If no key was supplied, make one.  a mysql insertid would be totally reasonable here, except
-               // that some users of this function might expect to supply the key instead of using the generated one.
-               if ( is_null( $key ) ) {
-                       // some things that when combined will make a suitably unique key.
-                       // see: http://www.jwz.org/doc/mid.html
-                       list ($usec, $sec) = explode( ' ', microtime() );
-                       $usec = substr($usec, 2);
-                       $key = wfBaseConvert( $sec . $usec, 10, 36 ) . '.' .
-                               wfBaseConvert( mt_rand(), 10, 36 ) . '.'.
-                               $this->userId . '.' . 
-                               $extension;
-               }
+               // that for historical reasons, the key is this random thing instead.  At least it's not guessable.
+               //
+               // some things that when combined will make a suitably unique key.
+               // see: http://www.jwz.org/doc/mid.html
+               list ($usec, $sec) = explode( ' ', microtime() );
+               $usec = substr($usec, 2);
+               $key = wfBaseConvert( $sec . $usec, 10, 36 ) . '.' .
+                       wfBaseConvert( mt_rand(), 10, 36 ) . '.'.
+                       $this->userId . '.' . 
+                       $extension;
 
                $this->fileProps[$key] = $fileProps;
 
@@ -232,31 +231,6 @@ class UploadStash {
                wfDebug( __METHOD__ . " inserting $stashPath under $key\n" );
                $dbw = $this->repo->getMasterDb();
 
-               // select happens on the master so this can all be in a transaction, which
-               // avoids a race condition that's likely with multiple people uploading from the same
-               // set of files
-               $dbw->begin();
-               // first, check to see if it's already there.
-               $row = $dbw->selectRow(
-                       'uploadstash',
-                       'us_user, us_timestamp',
-                       array( 'us_key' => $key ),
-                       __METHOD__
-               );
-
-               // The current user can't have this key if:
-               // - the key is owned by someone else and
-               // - the age of the key is less than $wgUploadStashMaxAge
-               if ( is_object( $row ) ) {
-                       global $wgUploadStashMaxAge;
-                       if ( $row->us_user != $this->userId &&
-                               $row->wfTimestamp( TS_UNIX, $row->us_timestamp ) > time() - $wgUploadStashMaxAge
-                       ) {
-                               $dbw->rollback();
-                               throw new UploadStashWrongOwnerException( "Attempting to upload a duplicate of a file that someone else has stashed" );
-                       }
-               }
-
                $this->fileMetadata[$key] = array(
                        'us_user' => $this->userId,
                        'us_key' => $key,
@@ -275,13 +249,11 @@ class UploadStash {
                );
 
                // if a row exists but previous checks on it passed, let the current user take over this key.
-               $dbw->replace(
+               $dbw->insert(
                        'uploadstash',
-                       'us_key',
                        $this->fileMetadata[$key],
                        __METHOD__
                );
-               $dbw->commit();
 
                // store the insertid in the class variable so immediate retrieval (possibly laggy) isn't necesary.
                $this->fileMetadata[$key]['us_id'] = $dbw->insertId();