Merge "Refactor PublishStashedFileJob to make the code reusable"

3 files changed, 316 insertions, 131 deletions

diff --git a/autoload.php b/autoload.php
index a6a5504f8af1..c99d39545821 100644
--- a/autoload.php
+++ b/autoload.php
@@ -3050,6 +3050,7 @@ $wgAutoloadLocalClasses = [
 	'UploadFromFile' => __DIR__ . '/includes/upload/UploadFromFile.php',
 	'UploadFromStash' => __DIR__ . '/includes/upload/UploadFromStash.php',
 	'UploadFromUrl' => __DIR__ . '/includes/upload/UploadFromUrl.php',
+	'UploadJobTrait' => __DIR__ . '/includes/jobqueue/jobs/UploadJobTrait.php',
 	'UploadLogFormatter' => __DIR__ . '/includes/logging/UploadLogFormatter.php',
 	'UploadRevisionImporter' => __DIR__ . '/includes/import/UploadRevisionImporter.php',
 	'UploadSourceAdapter' => __DIR__ . '/includes/import/UploadSourceAdapter.php',
diff --git a/includes/jobqueue/jobs/PublishStashedFileJob.php b/includes/jobqueue/jobs/PublishStashedFileJob.php
index 1207fb11e79f..982f68f5cab3 100644
--- a/includes/jobqueue/jobs/PublishStashedFileJob.php
+++ b/includes/jobqueue/jobs/PublishStashedFileJob.php
@@ -16,13 +16,9 @@
  * http://www.gnu.org/copyleft/gpl.html
  *
  * @file
+ * @defgroup JobQueue JobQueue
  */
 
-use MediaWiki\Context\RequestContext;
-use MediaWiki\Logger\LoggerFactory;
-use MediaWiki\Status\Status;
-use Wikimedia\ScopedCallback;
-
 /**
  * Upload a file from the upload stash into the local file repo.
  *
@@ -30,134 +26,12 @@ use Wikimedia\ScopedCallback;
  * @ingroup JobQueue
  */
 class PublishStashedFileJob extends Job implements GenericParameterJob {
+	use UploadJobTrait;
 
 	public function __construct( array $params ) {
 		parent::__construct( 'PublishStashedFile', $params );
 		$this->removeDuplicates = true;
-	}
-
-	public function run() {
-		$scope = RequestContext::importScopedSession( $this->params['session'] );
-		$this->addTeardownCallback( static function () use ( &$scope ) {
-			ScopedCallback::consume( $scope ); // T126450
-		} );
-
-		$context = RequestContext::getMain();
-		$user = $context->getUser();
-		try {
-			if ( !$user->isRegistered() ) {
-				$this->setLastError( "Could not load the author user from session." );
-
-				return false;
-			}
-
-			$startingStatus = UploadBase::getSessionStatus( $user, $this->params['filekey'] );
-			// Warn if in wrong stage, but still continue. User may be able to trigger
-			// this by retrying after failure.
-			if (
-				!$startingStatus ||
-				( $startingStatus['result'] ?? '' ) !== 'Poll' ||
-				( $startingStatus['stage'] ?? '' ) !== 'queued'
-			) {
-				$logger = LoggerFactory::getInstance( 'upload' );
-				$logger->warning( "Tried to publish upload that is in stage {stage}/{result}",
-					[
-						'stage' => $startingStatus['stage'] ?? '-',
-						'result' => $startingStatus['result'] ?? '-',
-						'status' => (string)( $startingStatus['status'] ?? '-' ),
-						'filekey' => $this->params['filekey'],
-						'filename' => $this->params['filename'],
-						'user' => $user->getName(),
-					]
-				);
-			}
-
-			UploadBase::setSessionStatus(
-				$user,
-				$this->params['filekey'],
-				[ 'result' => 'Poll', 'stage' => 'publish', 'status' => Status::newGood() ]
-			);
-
-			$upload = new UploadFromStash( $user );
-			// @todo initialize() causes a GET, ideally we could frontload the antivirus
-			// checks and anything else to the stash stage (which includes concatenation and
-			// the local file is thus already there). That way, instead of GET+PUT, there could
-			// just be a COPY operation from the stash to the public zone.
-			$upload->initialize( $this->params['filekey'], $this->params['filename'] );
-
-			// Check if the local file checks out (this is generally a no-op)
-			$verification = $upload->verifyUpload();
-			if ( $verification['status'] !== UploadBase::OK ) {
-				$status = Status::newFatal( 'verification-error' );
-				$status->value = [ 'verification' => $verification ];
-				UploadBase::setSessionStatus(
-					$user,
-					$this->params['filekey'],
-					[ 'result' => 'Failure', 'stage' => 'publish', 'status' => $status ]
-				);
-				$this->setLastError( "Could not verify upload." );
-
-				return false;
-			}
-
-			// Upload the stashed file to a permanent location
-			$status = $upload->performUpload(
-				$this->params['comment'],
-				$this->params['text'],
-				$this->params['watch'],
-				$user,
-				$this->params['tags'] ?? [],
-				$this->params['watchlistexpiry'] ?? null
-			);
-			if ( !$status->isGood() ) {
-				UploadBase::setSessionStatus(
-					$user,
-					$this->params['filekey'],
-					[ 'result' => 'Failure', 'stage' => 'publish', 'status' => $status ]
-				);
-				$this->setLastError( $status->getWikiText( false, false, 'en' ) );
-
-				return false;
-			}
-
-			// Build the image info array while we have the local reference handy
-			$apiUpload = ApiUpload::getDummyInstance();
-			$imageInfo = $apiUpload->getUploadImageInfo( $upload );
-
-			// Cleanup any temporary local file
-			$upload->cleanupTempFile();
-
-			// Cache the info so the user doesn't have to wait forever to get the final info
-			UploadBase::setSessionStatus(
-				$user,
-				$this->params['filekey'],
-				[
-					'result' => 'Success',
-					'stage' => 'publish',
-					'filename' => $upload->getLocalFile()->getName(),
-					'imageinfo' => $imageInfo,
-					'status' => Status::newGood()
-				]
-			);
-		} catch ( Exception $e ) {
-			UploadBase::setSessionStatus(
-				$user,
-				$this->params['filekey'],
-				[
-					'result' => 'Failure',
-					'stage' => 'publish',
-					'status' => Status::newFatal( 'api-error-publishfailed' )
-				]
-			);
-			$this->setLastError( get_class( $e ) . ": " . $e->getMessage() );
-			// To prevent potential database referential integrity issues.
-			// See T34551.
-			MWExceptionHandler::rollbackPrimaryChangesAndLog( $e );
-
-			return false;
-		}
-
-		return true;
+		$this->initialiseUploadJob( $this->params['filekey'] );
 	}
 
 	public function getDeduplicationInfo() {
@@ -169,7 +43,37 @@ class PublishStashedFileJob extends Job implements GenericParameterJob {
 		return $info;
 	}
 
-	public function allowRetries() {
-		return false;
+	/**
+	 * Get the parameters for job logging
+	 *
+	 * @param Status[] $status
+	 * @return array
+	 */
+	public function logJobParams( $status ) {
+		return [
+			'stage' => $status['stage'] ?? '-',
+			'result' => $status['result'] ?? '-',
+			'status' => (string)( $status['status'] ?? '-' ),
+			'filekey' => $this->params['filekey'],
+			'filename' => $this->params['filename'],
+			'user' => $this->user->getName(),
+		];
+	}
+
+	/**
+	 * getter for the upload
+	 *
+	 * @return UploadFromStash
+	 */
+	protected function getUpload() {
+		if ( $this->upload === null ) {
+			$this->upload = new UploadFromStash( $this->user );
+			// @todo initialize() causes a GET, ideally we could frontload the antivirus
+			// checks and anything else to the stash stage (which includes concatenation and
+			// the local file is thus already there). That way, instead of GET+PUT, there could
+			// just be a COPY operation from the stash to the public zone.
+			$this->upload->initialize( $this->params['filekey'], $this->params['filename'] );
+		}
+		return $this->upload;
 	}
 }
diff --git a/includes/jobqueue/jobs/UploadJobTrait.php b/includes/jobqueue/jobs/UploadJobTrait.php
new file mode 100644
index 000000000000..979f066a5072
--- /dev/null
+++ b/includes/jobqueue/jobs/UploadJobTrait.php
@@ -0,0 +1,280 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @defgroup JobQueue JobQueue
+ */
+
+use MediaWiki\Context\RequestContext;
+use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\Status\Status;
+use Mediawiki\User\User;
+use Wikimedia\ScopedCallback;
+
+/**
+ * Common functionality for async uploads
+ *
+ * @ingroup Upload
+ * @ingroup JobQueue
+ */
+trait UploadJobTrait {
+	/** @var User|null */
+	private $user;
+
+	/** @var string */
+	private $cacheKey;
+
+	/** @var UploadBase */
+	private $upload;
+
+	/** @var array The job parameters */
+	public $params;
+
+	/**
+	 * Set up the job
+	 *
+	 * @param string $cacheKey
+	 * @return void
+	 */
+	protected function initialiseUploadJob( $cacheKey ): void {
+		$this->cacheKey = $cacheKey;
+		$this->user = null;
+	}
+
+	/**
+	 * Do not allow retries on jobs by default.
+	 *
+	 * @return bool
+	 */
+	public function allowRetries(): bool {
+		return false;
+	}
+
+	/**
+	 * Run the job
+	 *
+	 * @return bool
+	 */
+	public function run(): bool {
+		$this->user = $this->getUserFromSession();
+		if ( $this->user === null ) {
+			return false;
+		}
+
+		try {
+			// Check the initial status of the upload
+			$startingStatus = UploadBase::getSessionStatus( $this->user, $this->cacheKey );
+			// Warn if in wrong stage, but still continue. User may be able to trigger
+			// this by retrying after failure.
+			if (
+				!$startingStatus ||
+				( $startingStatus['result'] ?? '' ) !== 'Poll' ||
+				( $startingStatus['stage'] ?? '' ) !== 'queued'
+			) {
+				$logger = LoggerFactory::getInstance( 'upload' );
+				$logger->warning( "Tried to publish upload that is in stage {stage}/{result}",
+					$this->logJobParams( $startingStatus )
+				);
+			}
+
+			// Fetch the file if needed
+			if ( !$this->fetchFile() ) {
+				return false;
+			}
+
+			// Verify the upload is valid
+			if ( !$this->verifyUpload() ) {
+				return false;
+			}
+
+			// Actually upload the file
+			if ( !$this->performUpload() ) {
+				return false;
+			}
+
+			// All done
+			$this->setStatusDone();
+
+			// Cleanup any temporary local file
+			$this->getUpload()->cleanupTempFile();
+
+		} catch ( Exception $e ) {
+			$this->setStatus( 'publish', 'Failure', Status::newFatal( 'api-error-publishfailed' ) );
+			$this->setLastError( get_class( $e ) . ": " . $e->getMessage() );
+			// To prevent potential database referential integrity issues.
+			// See T34551.
+			MWExceptionHandler::rollbackPrimaryChangesAndLog( $e );
+			return false;
+		}
+
+		return true;
+	}
+
+	/**
+	 * Get user data from the session key
+	 *
+	 * @return User|null
+	 */
+	private function getUserFromSession() {
+		$scope = RequestContext::importScopedSession( $this->params['session'] );
+		$this->addTeardownCallback( static function () use ( &$scope ) {
+			ScopedCallback::consume( $scope ); // T126450
+		} );
+
+		$context = RequestContext::getMain();
+		$user = $context->getUser();
+		if ( !$user->isRegistered() ) {
+			$this->setLastError( "Could not load the author user from session." );
+
+			return null;
+		}
+		return $user;
+	}
+
+	/**
+	 * Set the upload status
+	 *
+	 * @param string $stage
+	 * @param string $result
+	 * @param Status|null $status
+	 * @param array $additionalInfo
+	 *
+	 */
+	private function setStatus( $stage, $result = 'Poll', $status = null, $additionalInfo = [] ) {
+		// We're most probably not running in a job.
+		// @todo maybe throw an exception?
+		if ( $this->user === null ) {
+			return;
+		}
+		if ( $status === null ) {
+			$status = Status::newGood();
+		}
+		$info = [ 'result' => $result, 'stage' => $stage, 'status' => $status ];
+		$info += $additionalInfo;
+		UploadBase::setSessionStatus(
+			$this->user,
+			$this->cacheKey,
+			$info
+		);
+	}
+
+	/**
+	 * Ensure we have the file available. A noop here.
+	 *
+	 * @return bool
+	 */
+	protected function fetchFile(): bool {
+		// make sure the upload file is here. This is a noop in most cases.
+		$this->getUpload()->fetchFile();
+		$this->setStatus( 'publish' );
+		// We really don't care as this is, as mentioned, generally a noop.
+		// When that's not the case, classes will need to override this method anyways.
+		return true;
+	}
+
+	/**
+	 * Verify the upload is ok
+	 *
+	 * @return bool
+	 */
+	private function verifyUpload(): bool {
+		// Check if the local file checks out (this is generally a no-op)
+		$verification = $this->getUpload()->verifyUpload();
+		if ( $verification['status'] !== UploadBase::OK ) {
+			$status = Status::newFatal( 'verification-error' );
+			$status->value = [ 'verification' => $verification ];
+			$this->setStatus( 'publish', 'Failure', $status );
+			$this->setLastError( "Could not verify upload." );
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * Upload the stashed file to a permanent location
+	 *
+	 * @return bool
+	 */
+	private function performUpload(): bool {
+		if ( $this->user === null ) {
+			return false;
+		}
+		$status = $this->getUpload()->performUpload(
+			$this->params['comment'],
+			$this->params['text'],
+			$this->params['watch'],
+			$this->user,
+			$this->params['tags'] ?? [],
+			$this->params['watchlistexpiry'] ?? null
+		);
+		if ( !$status->isGood() ) {
+			$this->setStatus( 'publish', 'Failure', $status );
+			$this->setLastError( $status->getWikiText( false, false, 'en' ) );
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * Set the status at the end or processing
+	 *
+	 */
+	private function setStatusDone() {
+		// Build the image info array while we have the local reference handy
+		$imageInfo = ApiUpload::getDummyInstance()->getUploadImageInfo( $this->getUpload() );
+
+		// Cache the info so the user doesn't have to wait forever to get the final info
+		$this->setStatus(
+			'publish',
+			'Success',
+			Status::newGood(),
+			[ 'filename' => $this->getUpload()->getLocalFile()->getName(), 'imageinfo' => $imageInfo ]
+		);
+	}
+
+	/**
+	 * Getter for the upload. Needs to be implemented by the job class
+	 *
+	 * @return UploadBase
+	 */
+	abstract protected function getUpload(): UploadBase;
+
+	/**
+	 * Get the job parameters for logging. Needs to be implemented by the job class.
+	 *
+	 * @param Status[] $status
+	 * @return array
+	 */
+	abstract protected function logJobParams( $status ): array;
+
+	/**
+	 * This is actually implemented in the Job class
+	 *
+	 * @param mixed $error
+	 * @return void
+	 */
+	abstract protected function setLastError( $error ): void;
+
+	/**
+	 * This is actually implemented in the Job class
+	 *
+	 * @param callable $callback
+	 * @return void
+	 */
+	abstract protected function addTeardownCallback( $callback ): void;
+
+}