From: Bartosz Dziewoński Date: Wed, 17 Aug 2016 18:43:55 +0000 (+0200) Subject: UploadBase: Stop mLocalFile doubling as stashed file X-Git-Tag: 1.31.0-rc.0~5996 X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/banques/ajouter.php?a=commitdiff_plain;h=ec41a9c55962c1f831332b61966076e87a581d3e;p=lhc%2Fweb%2Fwiklou.git UploadBase: Stop mLocalFile doubling as stashed file "I've a great idea", they said. "You know what would be cool? If I made this boring getter, getLocalFile(), return something completely different after the file was stashed. This will be a nice surprise for someone in the future to discover", they added gleefully. I am pretty sure everything still works, but I never could get async upload publishing to work locally, so I'd appreciate some testing. Change-Id: I11dcf2ed89e4f1dd8ddf081af521da005efdbf39 --- diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index fc2fd59056..227278c870 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -278,7 +278,7 @@ class ApiUpload extends ApiBase { // the old filekey and fetch the new one. UploadBase::setSessionStatus( $this->getUser(), $filekey, false ); $this->mUpload->stash->removeFile( $filekey ); - $filekey = $this->mUpload->getLocalFile()->getFileKey(); + $filekey = $this->mUpload->getStashFile()->getFileKey(); $result['result'] = 'Success'; } @@ -735,7 +735,7 @@ class ApiUpload extends ApiBase { $this->mParams['text'] = $this->mParams['comment']; } - /** @var $file File */ + /** @var $file LocalFile */ $file = $this->mUpload->getLocalFile(); // For preferences mode, we want to watch if 'watchdefault' is set, diff --git a/includes/jobqueue/jobs/AssembleUploadChunksJob.php b/includes/jobqueue/jobs/AssembleUploadChunksJob.php index 1e804c4575..f3fcadf623 100644 --- a/includes/jobqueue/jobs/AssembleUploadChunksJob.php +++ b/includes/jobqueue/jobs/AssembleUploadChunksJob.php @@ -74,7 +74,7 @@ class AssembleUploadChunksJob extends Job { } // We have a new filekey for the fully concatenated file - $newFileKey = $upload->getLocalFile()->getFileKey(); + $newFileKey = $upload->getStashFile()->getFileKey(); // Remove the old stash file row and first chunk file $upload->stash->removeFileNoAuth( $this->params['filekey'] ); diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index ae16f70227..e2f7763251 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -44,7 +44,7 @@ abstract class UploadBase { protected $mDesiredDestName, $mDestName, $mRemoveTempFile, $mSourceType; protected $mTitle = false, $mTitleError = 0; protected $mFilteredName, $mFinalExtension; - protected $mLocalFile, $mFileSize, $mFileProps; + protected $mLocalFile, $mStashFile, $mFileSize, $mFileProps; protected $mBlackListedExtensions; protected $mJavaDetected, $mSVGNSError; @@ -912,7 +912,7 @@ abstract class UploadBase { /** * Return the local file and initializes if necessary. * - * @return LocalFile|UploadStashFile|null + * @return LocalFile|null */ public function getLocalFile() { if ( is_null( $this->mLocalFile ) ) { @@ -923,6 +923,13 @@ abstract class UploadBase { return $this->mLocalFile; } + /** + * @return UploadStashFile|null + */ + public function getStashFile() { + return $this->mStashFile; + } + /** * Like stashFile(), but respects extensions' wishes to prevent the stashing. verifyUpload() must * be called before calling this method (unless $isPartial is true). @@ -997,7 +1004,7 @@ abstract class UploadBase { protected function doStashFile( User $user = null ) { $stash = RepoGroup::singleton()->getLocalRepo()->getUploadStash( $user ); $file = $stash->stashFile( $this->mTempPath, $this->getSourceType() ); - $this->mLocalFile = $file; + $this->mStashFile = $file; return $file; } @@ -1975,18 +1982,16 @@ abstract class UploadBase { * @return array Image info */ public function getImageInfo( $result ) { - $file = $this->getLocalFile(); - /** @todo This cries out for refactoring. - * We really want to say $file->getAllInfo(); here. - * Perhaps "info" methods should be moved into files, and the API should - * just wrap them in queries. - */ - if ( $file instanceof UploadStashFile ) { + $localFile = $this->getLocalFile(); + $stashFile = $this->getStashFile(); + // Calling a different API module depending on whether the file was stashed is less than optimal. + // In fact, calling API modules here at all is less than optimal. Maybe it should be refactored. + if ( $stashFile ) { $imParam = ApiQueryStashImageInfo::getPropertyNames(); - $info = ApiQueryStashImageInfo::getInfo( $file, array_flip( $imParam ), $result ); + $info = ApiQueryStashImageInfo::getInfo( $stashFile, array_flip( $imParam ), $result ); } else { $imParam = ApiQueryImageInfo::getPropertyNames(); - $info = ApiQueryImageInfo::getInfo( $file, array_flip( $imParam ), $result ); + $info = ApiQueryImageInfo::getInfo( $localFile, array_flip( $imParam ), $result ); } return $info; diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index 6368db82da..9145a854f3 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -76,18 +76,18 @@ class UploadFromChunks extends UploadFromFile { $this->verifyChunk(); // Create a local stash target - $this->mLocalFile = parent::doStashFile( $user ); + $this->mStashFile = parent::doStashFile( $user ); // Update the initial file offset (based on file size) - $this->mOffset = $this->mLocalFile->getSize(); - $this->mFileKey = $this->mLocalFile->getFileKey(); + $this->mOffset = $this->mStashFile->getSize(); + $this->mFileKey = $this->mStashFile->getFileKey(); // Output a copy of this first to chunk 0 location: - $this->outputChunk( $this->mLocalFile->getPath() ); + $this->outputChunk( $this->mStashFile->getPath() ); // Update db table to reflect initial "chunk" state $this->updateChunkStatus(); - return $this->mLocalFile; + return $this->mStashFile; } /** @@ -158,7 +158,7 @@ class UploadFromChunks extends UploadFromFile { return $status; } - // Update the mTempPath and mLocalFile + // Update the mTempPath and mStashFile // (for FileUpload or normal Stash to take over) $tStart = microtime( true ); // This is a re-implementation of UploadBase::tryStashFile(), we can't call it because we @@ -169,14 +169,14 @@ class UploadFromChunks extends UploadFromFile { return $status; } try { - $this->mLocalFile = parent::doStashFile( $this->user ); + $this->mStashFile = 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()) + $this->mStashFile->setLocalReference( $tmpFile ); // reuse (e.g. for getImageInfo()) wfDebugLog( 'fileconcatenate', "Stashed combined file ($i chunks) in $tAmount seconds." ); return $status;