From a51ea350bed6f66bbc5d447c77bd696071665d2c Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 30 Mar 2019 08:05:31 -0700 Subject: [PATCH] Add Job::getMetadata() and Job::setMetadata() accessors Change-Id: I3a97008d324f600a1c9f6005673073277ee564fa --- includes/jobqueue/Job.php | 30 +++++++++++++++++++++++++ includes/jobqueue/JobQueueDB.php | 18 +++++++++------ includes/jobqueue/JobQueueFederated.php | 7 +++--- includes/jobqueue/JobQueueMemory.php | 4 ++-- includes/jobqueue/JobQueueRedis.php | 15 +++++++------ 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/includes/jobqueue/Job.php b/includes/jobqueue/Job.php index 24fc47364d..22ff4468dd 100644 --- a/includes/jobqueue/Job.php +++ b/includes/jobqueue/Job.php @@ -156,6 +156,36 @@ abstract class Job implements IJobSpecification { return $this->params; } + /** + * @param string|null $field Metadata field or null to get all the metadata + * @return mixed|null Value; null if missing + * @since 1.33 + */ + public function getMetadata( $field = null ) { + if ( $field === null ) { + return $this->metadata; + } + + return $this->metadata[$field] ?? null; + } + + /** + * @param string $field Key name to set the value for + * @param mixed $value The value to set the field for + * @return mixed|null The prior field value; null if missing + * @since 1.33 + */ + public function setMetadata( $field, $value ) { + $old = $this->getMetadata( $field ); + if ( $value === null ) { + unset( $this->metadata[$field] ); + } else { + $this->metadata[$field] = $value; + } + + return $old; + } + /** * @return int|null UNIX timestamp to delay running this job until, otherwise null * @since 1.22 diff --git a/includes/jobqueue/JobQueueDB.php b/includes/jobqueue/JobQueueDB.php index 74a6559ee5..c52a948a2b 100644 --- a/includes/jobqueue/JobQueueDB.php +++ b/includes/jobqueue/JobQueueDB.php @@ -317,8 +317,8 @@ class JobQueueDB extends JobQueue { $title = Title::makeTitle( $row->job_namespace, $row->job_title ); $job = Job::factory( $row->job_cmd, $title, self::extractBlob( $row->job_params ) ); - $job->metadata['id'] = $row->job_id; - $job->metadata['timestamp'] = $row->job_timestamp; + $job->setMetadata( 'id', $row->job_id ); + $job->setMetadata( 'timestamp', $row->job_timestamp ); break; // done } while ( true ); @@ -484,7 +484,8 @@ class JobQueueDB extends JobQueue { * @throws MWException */ protected function doAck( Job $job ) { - if ( !isset( $job->metadata['id'] ) ) { + $id = $job->getMetadata( 'id' ); + if ( $id === null ) { throw new MWException( "Job of type '{$job->getType()}' has no ID." ); } @@ -493,8 +494,11 @@ class JobQueueDB extends JobQueue { $scope = $this->getScopedNoTrxFlag( $dbw ); try { // Delete a row with a single DELETE without holding row locks over RTTs... - $dbw->delete( 'job', - [ 'job_cmd' => $this->type, 'job_id' => $job->metadata['id'] ], __METHOD__ ); + $dbw->delete( + 'job', + [ 'job_cmd' => $this->type, 'job_id' => $id ], + __METHOD__ + ); JobQueue::incrStats( 'acks', $this->type ); } catch ( DBError $e ) { @@ -617,8 +621,8 @@ class JobQueueDB extends JobQueue { Title::makeTitle( $row->job_namespace, $row->job_title ), strlen( $row->job_params ) ? unserialize( $row->job_params ) : [] ); - $job->metadata['id'] = $row->job_id; - $job->metadata['timestamp'] = $row->job_timestamp; + $job->setMetadata( 'id', $row->job_id ); + $job->setMetadata( 'timestamp', $row->job_timestamp ); return $job; } diff --git a/includes/jobqueue/JobQueueFederated.php b/includes/jobqueue/JobQueueFederated.php index 2025bf7b93..30ab7e739b 100644 --- a/includes/jobqueue/JobQueueFederated.php +++ b/includes/jobqueue/JobQueueFederated.php @@ -287,7 +287,7 @@ class JobQueueFederated extends JobQueue { $job = false; } if ( $job ) { - $job->metadata['QueuePartition'] = $partition; + $job->setMetadata( 'QueuePartition', $partition ); return $job; } else { @@ -300,11 +300,12 @@ class JobQueueFederated extends JobQueue { } protected function doAck( Job $job ) { - if ( !isset( $job->metadata['QueuePartition'] ) ) { + $partition = $job->getMetadata( 'QueuePartition' ); + if ( $partition === null ) { throw new MWException( "The given job has no defined partition name." ); } - $this->partitionQueues[$job->metadata['QueuePartition']]->ack( $job ); + $this->partitionQueues[$partition]->ack( $job ); } protected function doIsRootJobOldDuplicate( Job $job ) { diff --git a/includes/jobqueue/JobQueueMemory.php b/includes/jobqueue/JobQueueMemory.php index 9b1fbdfaf3..6c45e9676a 100644 --- a/includes/jobqueue/JobQueueMemory.php +++ b/includes/jobqueue/JobQueueMemory.php @@ -132,7 +132,7 @@ class JobQueueMemory extends JobQueue { $job = $this->jobFromSpecInternal( $spec ); end( $claimed ); - $job->metadata['claimId'] = key( $claimed ); + $job->setMetadata( 'claimId', key( $claimed ) ); return $job; } @@ -148,7 +148,7 @@ class JobQueueMemory extends JobQueue { } $claimed =& $this->getQueueData( 'claimed' ); - unset( $claimed[$job->metadata['claimId']] ); + $job->setMetadata( 'claimId', null ); } /** diff --git a/includes/jobqueue/JobQueueRedis.php b/includes/jobqueue/JobQueueRedis.php index 5e7a11571a..4d07a09a71 100644 --- a/includes/jobqueue/JobQueueRedis.php +++ b/includes/jobqueue/JobQueueRedis.php @@ -385,11 +385,11 @@ LUA; * @throws JobQueueError */ protected function doAck( Job $job ) { - if ( !isset( $job->metadata['uuid'] ) ) { + $uuid = $job->getMetadata( 'uuid' ); + if ( $uuid === null ) { throw new UnexpectedValueException( "Job of type '{$job->getType()}' has no UUID." ); } - $uuid = $job->metadata['uuid']; $conn = $this->getConnection(); try { static $script = @@ -643,10 +643,11 @@ LUA; } $title = Title::makeTitle( $item['namespace'], $item['title'] ); $job = Job::factory( $item['type'], $title, $item['params'] ); - $job->metadata['uuid'] = $item['uuid']; - $job->metadata['timestamp'] = $item['timestamp']; + $job->setMetadata( 'uuid', $item['uuid'] ); + $job->setMetadata( 'timestamp', $item['timestamp'] ); // Add in attempt count for debugging at showJobs.php - $job->metadata['attempts'] = $conn->hGet( $this->getQueueKey( 'h-attempts' ), $uid ); + $job->setMetadata( 'attempts', + $conn->hGet( $this->getQueueKey( 'h-attempts' ), $uid ) ); return $job; } catch ( RedisException $e ) { @@ -704,8 +705,8 @@ LUA; protected function getJobFromFields( array $fields ) { $title = Title::makeTitle( $fields['namespace'], $fields['title'] ); $job = Job::factory( $fields['type'], $title, $fields['params'] ); - $job->metadata['uuid'] = $fields['uuid']; - $job->metadata['timestamp'] = $fields['timestamp']; + $job->setMetadata( 'uuid', $fields['uuid'] ); + $job->setMetadata( 'timestamp', $fields['timestamp'] ); return $job; } -- 2.20.1