From 697b4d74a6d0a3bedf8d8e97895b0eb9e1e139fe Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 20 Mar 2013 12:59:59 -0700
Subject: [PATCH] [JobQueue] Deprecated confusing Job::getId() function.

* Also slimmed down $job->metadata use by JobQueueRedis to just the ID.

Change-Id: If9fdb33afd6d1ce2be957a1eea107f54bd6dac53
---
 includes/job/Job.php               |  1 +
 includes/job/JobQueueDB.php        |  6 ++++--
 includes/job/JobQueueRedis.php     | 13 ++++++-------
 includes/job/jobs/DuplicateJob.php |  2 +-
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/includes/job/Job.php b/includes/job/Job.php
index 64925f724a..a765d5d429 100644
--- a/includes/job/Job.php
+++ b/includes/job/Job.php
@@ -150,6 +150,7 @@ abstract class Job {
 
 	/**
 	 * @return integer May be 0 for jobs stored outside the DB
+	 * @deprecated 1.22
 	 */
 	public function getId() {
 		return $this->id;
diff --git a/includes/job/JobQueueDB.php b/includes/job/JobQueueDB.php
index bab4830e14..e4825edc3e 100644
--- a/includes/job/JobQueueDB.php
+++ b/includes/job/JobQueueDB.php
@@ -297,6 +297,7 @@ class JobQueueDB extends JobQueue {
 			}
 			$job = Job::factory( $row->job_cmd, $title,
 				self::extractBlob( $row->job_params ), $row->job_id );
+			$job->metadata['id'] = $row->job_id;
 			$job->id = $row->job_id; // XXX: work around broken subclasses
 			break; // done
 		} while( true );
@@ -450,7 +451,7 @@ class JobQueueDB extends JobQueue {
 	 * @return Job|bool
 	 */
 	protected function doAck( Job $job ) {
-		if ( !$job->getId() ) {
+		if ( !isset( $job->metadata['id'] ) ) {
 			throw new MWException( "Job of type '{$job->getType()}' has no ID." );
 		}
 
@@ -459,7 +460,7 @@ class JobQueueDB extends JobQueue {
 
 		// Delete a row with a single DELETE without holding row locks over RTTs...
 		$dbw->delete( 'job',
-			array( 'job_cmd' => $this->type, 'job_id' => $job->getId() ), __METHOD__ );
+			array( 'job_cmd' => $this->type, 'job_id' => $job->metadata['id'] ), __METHOD__ );
 
 		return true;
 	}
@@ -542,6 +543,7 @@ class JobQueueDB extends JobQueue {
 					strlen( $row->job_params ) ? unserialize( $row->job_params ) : false,
 					$row->job_id
 				);
+				$job->metadata['id'] = $row->job_id;
 				$job->id = $row->job_id; // XXX: work around broken subclasses
 				return $job;
 			}
diff --git a/includes/job/JobQueueRedis.php b/includes/job/JobQueueRedis.php
index 891d48f346..f083fcc7b6 100644
--- a/includes/job/JobQueueRedis.php
+++ b/includes/job/JobQueueRedis.php
@@ -399,13 +399,12 @@ LUA;
 	 * @throws MWException
 	 */
 	protected function doAck( Job $job ) {
+		if ( !isset( $job->metadata['uuid'] ) ) {
+			throw new MWException( "Job of type '{$job->getType()}' has no UUID." );
+		}
 		if ( $this->claimTTL > 0 ) {
 			$conn = $this->getConnection();
 			try {
-				// Get the exact field map this Job came from, regardless of whether
-				// the job was transformed into a DuplicateJob or anything of the sort.
-				$item = $job->metadata['sourceFields'];
-
 				static $script =
 <<<LUA
 				-- Unmark the job as claimed
@@ -419,7 +418,7 @@ LUA;
 						$this->getQueueKey( 'z-claimed' ), # KEYS[1]
 						$this->getQueueKey( 'h-attempts' ), # KEYS[2]
 						$this->getQueueKey( 'h-data' ), # KEYS[3]
-						$item['uuid'] # ARGV[1]
+						$job->metadata['uuid'] # ARGV[1]
 					),
 					3 # number of first argument(s) that are keys
 				);
@@ -539,7 +538,7 @@ LUA;
 			}
 			$title = Title::makeTitle( $item['namespace'], $item['title'] );
 			$job = Job::factory( $item['type'], $title, $item['params'] );
-			$job->metadata['sourceFields'] = $item;
+			$job->metadata['uuid'] = $item['uuid'];
 			return $job;
 		} catch ( RedisException $e ) {
 			$this->throwRedisException( $this->server, $conn, $e );
@@ -709,7 +708,7 @@ LUA;
 		$title = Title::makeTitleSafe( $fields['namespace'], $fields['title'] );
 		if ( $title ) {
 			$job = Job::factory( $fields['type'], $title, $fields['params'] );
-			$job->metadata['sourceFields'] = $fields;
+			$job->metadata['uuid'] = $fields['uuid'];
 			return $job;
 		}
 		return false;
diff --git a/includes/job/jobs/DuplicateJob.php b/includes/job/jobs/DuplicateJob.php
index 524983b8f3..be1bfe5c81 100644
--- a/includes/job/jobs/DuplicateJob.php
+++ b/includes/job/jobs/DuplicateJob.php
@@ -45,7 +45,7 @@ final class DuplicateJob extends Job {
 	 * @return Job
 	 */
 	public static function newFromJob( Job $job ) {
-		$djob = new self( $job->getTitle(), $job->getParams(), $job->getId() );
+		$djob = new self( $job->getTitle(), $job->getParams(), $job->id );
 		$djob->command = $job->getType();
 		$djob->params = is_array( $djob->params ) ? $djob->params : array();
 		$djob->params = array( 'isDuplicate' => true ) + $djob->params;
-- 
2.20.1