Tim Starling has submitted this change and it was merged.

Change subject: [JobQueue] Deprecated confusing Job::getId() function.
......................................................................


[JobQueue] Deprecated confusing Job::getId() function.

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

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

Approvals:
  Tim Starling: Verified; Looks good to me, approved



diff --git a/includes/job/Job.php b/includes/job/Job.php
index 64925f7..a765d5d 100644
--- a/includes/job/Job.php
+++ b/includes/job/Job.php
@@ -150,6 +150,7 @@
 
        /**
         * @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 bab4830..e4825ed 100644
--- a/includes/job/JobQueueDB.php
+++ b/includes/job/JobQueueDB.php
@@ -297,6 +297,7 @@
                        }
                        $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 @@
         * @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 @@
 
                // 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 @@
                                        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 891d48f..f083fcc 100644
--- a/includes/job/JobQueueRedis.php
+++ b/includes/job/JobQueueRedis.php
@@ -399,13 +399,12 @@
         * @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 @@
                                                $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 @@
                        }
                        $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 @@
                $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 524983b..be1bfe5 100644
--- a/includes/job/jobs/DuplicateJob.php
+++ b/includes/job/jobs/DuplicateJob.php
@@ -45,7 +45,7 @@
         * @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;

-- 
To view, visit https://gerrit.wikimedia.org/r/54901
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If9fdb33afd6d1ce2be957a1eea107f54bd6dac53
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Nikerabbit <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to