jobqueue: add GenericParameterJob and RunnableJob interface
authorAaron Schulz <aschulz@wikimedia.org>
Sat, 30 Mar 2019 06:07:48 +0000 (23:07 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 8 Apr 2019 18:05:23 +0000 (11:05 -0700)
Simplify the code of jobs that do not care about titles and removes
the direct Title dependency from JobQueue. Remove getTitle() from
IJobSpecification itself. Move all the Job::factory calls into a
single JobQueue::factoryJob() method.

Depends-on: Iee78f4baeca0c0b4d6db073f2fbcc56855114ab0
Change-Id: I9c9d0726d4066bb0aa937665847ad6042ade13ec

27 files changed:
autoload.php
includes/deferred/CdnCacheUpdate.php
includes/jobqueue/GenericParameterJob.php [new file with mode: 0644]
includes/jobqueue/IJobSpecification.php
includes/jobqueue/Job.php
includes/jobqueue/JobQueue.php
includes/jobqueue/JobQueueDB.php
includes/jobqueue/JobQueueFederated.php
includes/jobqueue/JobQueueGroup.php
includes/jobqueue/JobQueueMemory.php
includes/jobqueue/JobQueueRedis.php
includes/jobqueue/JobSpecification.php
includes/jobqueue/RunnableJob.php [new file with mode: 0644]
includes/jobqueue/jobs/CdnPurgeJob.php
includes/jobqueue/jobs/ClearUserWatchlistJob.php
includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php
includes/jobqueue/jobs/DeletePageJob.php
includes/jobqueue/jobs/DuplicateJob.php
includes/jobqueue/jobs/EnqueueJob.php
includes/jobqueue/jobs/NullJob.php
includes/jobqueue/jobs/UserGroupExpiryJob.php
includes/page/WikiPage.php
includes/watcheditem/WatchedItemStore.php
tests/phpunit/includes/SiteStatsTest.php
tests/phpunit/includes/jobqueue/JobQueueTest.php
tests/phpunit/includes/jobqueue/JobTest.php
tests/phpunit/includes/jobqueue/jobs/ClearUserWatchlistJobTest.php

index b22aeab..0ce423f 100644 (file)
@@ -565,6 +565,7 @@ $wgAutoloadLocalClasses = [
        'GenerateNormalizerDataMl' => __DIR__ . '/maintenance/language/generateNormalizerDataMl.php',
        'GenerateSitemap' => __DIR__ . '/maintenance/generateSitemap.php',
        'GenericArrayObject' => __DIR__ . '/includes/libs/GenericArrayObject.php',
+       'GenericParameterJob' => __DIR__ . '/includes/jobqueue/GenericParameterJob.php',
        'GetConfiguration' => __DIR__ . '/maintenance/getConfiguration.php',
        'GetLagTimes' => __DIR__ . '/maintenance/getLagTimes.php',
        'GetReplicaServer' => __DIR__ . '/maintenance/getReplicaServer.php',
@@ -1288,6 +1289,7 @@ $wgAutoloadLocalClasses = [
        'RowUpdateGenerator' => __DIR__ . '/includes/utils/RowUpdateGenerator.php',
        'RunBatchedQuery' => __DIR__ . '/maintenance/runBatchedQuery.php',
        'RunJobs' => __DIR__ . '/maintenance/runJobs.php',
+       'RunnableJob' => __DIR__ . '/includes/jobqueue/RunnableJob.php',
        'SVGMetadataExtractor' => __DIR__ . '/includes/media/SVGMetadataExtractor.php',
        'SVGReader' => __DIR__ . '/includes/media/SVGMetadataExtractor.php',
        'SamplingStatsdClient' => __DIR__ . '/includes/libs/stats/SamplingStatsdClient.php',
index 6f961e8..2d07f75 100644 (file)
@@ -71,13 +71,10 @@ class CdnCacheUpdate implements DeferrableUpdate, MergeableUpdate {
                self::purge( $this->urls );
 
                if ( $wgCdnReboundPurgeDelay > 0 ) {
-                       JobQueueGroup::singleton()->lazyPush( new CdnPurgeJob(
-                               Title::makeTitle( NS_SPECIAL, 'Badtitle/' . __CLASS__ ),
-                               [
-                                       'urls' => $this->urls,
-                                       'jobReleaseTimestamp' => time() + $wgCdnReboundPurgeDelay
-                               ]
-                       ) );
+                       JobQueueGroup::singleton()->lazyPush( new CdnPurgeJob( [
+                               'urls' => $this->urls,
+                               'jobReleaseTimestamp' => time() + $wgCdnReboundPurgeDelay
+                       ] ) );
                }
        }
 
diff --git a/includes/jobqueue/GenericParameterJob.php b/includes/jobqueue/GenericParameterJob.php
new file mode 100644 (file)
index 0000000..f7da42b
--- /dev/null
@@ -0,0 +1,34 @@
+<?php
+/**
+ * Interface for generic jobs only uses the parameters field.
+ *
+ * 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
+ */
+
+/**
+ * Interface for generic jobs only uses the parameters field and are JSON serializable
+ *
+ * @ingroup JobQueue
+ * @since 1.33
+ */
+interface GenericParameterJob extends IJobSpecification {
+       /**
+        * @param array $params JSON-serializable map of parameters
+        */
+       public function __construct( array $params );
+}
index 8bc1bc3..2b3caa2 100644 (file)
@@ -1,6 +1,6 @@
 <?php
 /**
- * Job queue task description base code.
+ * Job queue task description interface
  *
  * 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
  */
 
 /**
- * Job queue task description interface
+ * Interface for serializable objects that describe a job queue task
+ *
+ * A job specification can be inserted into a queue via JobQueue::push().
+ * The specification parameters should be JSON serializable (e.g. no PHP classes).
+ * Whatever queue the job specification is pushed into is assumed to have job runners
+ * that will eventually pop the job specification from the queue, construct a RunnableJob
+ * instance from the specification, and then execute that instance via RunnableJob::run().
  *
  * @ingroup JobQueue
  * @since 1.23
  */
 interface IJobSpecification {
        /**
-        * @return string Job type
+        * @return string Job type that defines what sort of changes this job makes
         */
        public function getType();
 
        /**
-        * @return array
+        * @return array Parameters that specify sources, targets, and options for execution
         */
        public function getParams();
 
@@ -76,9 +82,4 @@ interface IJobSpecification {
         * @return bool Whether this is job is a root job
         */
        public function isRootJob();
-
-       /**
-        * @return Title Descriptive title (this can simply be informative)
-        */
-       public function getTitle();
 }
index 060003b..6054e35 100644 (file)
@@ -27,7 +27,7 @@
  *
  * @ingroup JobQueue
  */
-abstract class Job implements IJobSpecification {
+abstract class Job implements RunnableJob {
        /** @var string */
        public $command;
 
@@ -55,12 +55,6 @@ abstract class Job implements IJobSpecification {
        /** @var int Job must not be wrapped in the usual explicit LBFactory transaction round */
        const JOB_NO_EXPLICIT_TRX_ROUND = 1;
 
-       /**
-        * Run the job
-        * @return bool Success
-        */
-       abstract public function run();
-
        /**
         * Create the appropriate object to handle a specific job
         *
@@ -77,17 +71,24 @@ abstract class Job implements IJobSpecification {
                        $title = $params;
                        $params = func_num_args() >= 3 ? func_get_arg( 2 ) : [];
                } else {
-                       // Subclasses can override getTitle() to return something more meaningful
-                       $title = Title::makeTitle( NS_SPECIAL, 'Blankpage' );
+                       $title = ( isset( $params['namespace'] ) && isset( $params['title'] ) )
+                               ? Title::makeTitle( $params['namespace'], $params['title'] )
+                               : Title::makeTitle( NS_SPECIAL, '' );
                }
 
+               $params = is_array( $params ) ? $params : []; // sanity
+
                if ( isset( $wgJobClasses[$command] ) ) {
                        $handler = $wgJobClasses[$command];
 
                        if ( is_callable( $handler ) ) {
                                $job = call_user_func( $handler, $title, $params );
                        } elseif ( class_exists( $handler ) ) {
-                               $job = new $handler( $title, $params );
+                               if ( is_subclass_of( $handler, GenericParameterJob::class ) ) {
+                                       $job = new $handler( $params );
+                               } else {
+                                       $job = new $handler( $title, $params );
+                               }
                        } else {
                                $job = null;
                        }
@@ -112,17 +113,27 @@ abstract class Job implements IJobSpecification {
                if ( $params instanceof Title ) {
                        // Backwards compatibility for old signature ($command, $title, $params)
                        $title = $params;
-                       $params = func_get_arg( 2 );
-               } else {
-                       // Subclasses can override getTitle() to return something more meaningful
-                       $title = Title::makeTitle( NS_SPECIAL, 'Blankpage' );
+                       $params = func_num_args() >= 3 ? func_get_arg( 2 ) : [];
+                       $params = is_array( $params ) ? $params : []; // sanity
+                       // Set namespace/title params if both are missing and this is not a dummy title
+                       if (
+                               $title->getDBkey() !== '' &&
+                               !isset( $params['namespace'] ) &&
+                               !isset( $params['title'] )
+                       ) {
+                               $params['namespace'] = $title->getNamespace();
+                               $params['title'] = $title->getDBKey();
+                               // Note that JobQueue classes will prefer the parameters over getTitle()
+                               $this->title = $title;
+                       }
                }
 
                $this->command = $command;
-               $this->title = $title;
-               $this->params = is_array( $params ) ? $params : [];
-               if ( !isset( $this->params['requestId'] ) ) {
-                       $this->params['requestId'] = WebRequest::getRequestId();
+               $this->params = $params + [ 'requestId' => WebRequest::getRequestId() ];
+               if ( $this->title === null ) {
+                       $this->title = ( isset( $params['namespace'] ) && isset( $params['title'] ) )
+                               ? Title::makeTitle( $params['namespace'], $params['title'] )
+                               : Title::makeTitle( NS_SPECIAL, '' );
                }
        }
 
@@ -145,7 +156,7 @@ abstract class Job implements IJobSpecification {
        /**
         * @return Title
         */
-       public function getTitle() {
+       final public function getTitle() {
                return $this->title;
        }
 
@@ -268,8 +279,6 @@ abstract class Job implements IJobSpecification {
        public function getDeduplicationInfo() {
                $info = [
                        'type' => $this->getType(),
-                       'namespace' => $this->getTitle()->getNamespace(),
-                       'title' => $this->getTitle()->getDBkey(),
                        'params' => $this->getParams()
                ];
                if ( is_array( $info['params'] ) ) {
index 0644002..f5ed7b9 100644 (file)
@@ -361,7 +361,7 @@ abstract class JobQueue {
         * Outside callers should use JobQueueGroup::pop() instead of this function.
         *
         * @throws JobQueueError
-        * @return Job|bool Returns false if there are no jobs
+        * @return RunnableJob|bool Returns false if there are no jobs
         */
        final public function pop() {
                $this->assertNotReadOnly();
@@ -383,7 +383,7 @@ abstract class JobQueue {
 
        /**
         * @see JobQueue::pop()
-        * @return Job|bool
+        * @return RunnableJob|bool
         */
        abstract protected function doPop();
 
@@ -393,11 +393,11 @@ abstract class JobQueue {
         * This does nothing for certain queue classes or if "claimTTL" is not set.
         * Outside callers should use JobQueueGroup::ack() instead of this function.
         *
-        * @param Job $job
+        * @param RunnableJob $job
         * @return void
         * @throws JobQueueError
         */
-       final public function ack( Job $job ) {
+       final public function ack( RunnableJob $job ) {
                $this->assertNotReadOnly();
                if ( $job->getType() !== $this->type ) {
                        throw new JobQueueError( "Got '{$job->getType()}' job; expected '{$this->type}'." );
@@ -408,9 +408,9 @@ abstract class JobQueue {
 
        /**
         * @see JobQueue::ack()
-        * @param Job $job
+        * @param RunnableJob $job
         */
-       abstract protected function doAck( Job $job );
+       abstract protected function doAck( RunnableJob $job );
 
        /**
         * Register the "root job" of a given job into the queue for de-duplication.
@@ -482,11 +482,11 @@ abstract class JobQueue {
        /**
         * Check if the "root" job of a given job has been superseded by a newer one
         *
-        * @param Job $job
+        * @param IJobSpecification $job
         * @throws JobQueueError
         * @return bool
         */
-       final protected function isRootJobOldDuplicate( Job $job ) {
+       final protected function isRootJobOldDuplicate( IJobSpecification $job ) {
                if ( $job->getType() !== $this->type ) {
                        throw new JobQueueError( "Got '{$job->getType()}' job; expected '{$this->type}'." );
                }
@@ -497,10 +497,10 @@ abstract class JobQueue {
 
        /**
         * @see JobQueue::isRootJobOldDuplicate()
-        * @param Job $job
+        * @param IJobSpecification $job
         * @return bool
         */
-       protected function doIsRootJobOldDuplicate( Job $job ) {
+       protected function doIsRootJobOldDuplicate( IJobSpecification $job ) {
                if ( !$job->hasRootJobParams() ) {
                        return false; // job has no de-deplication info
                }
@@ -686,6 +686,16 @@ abstract class JobQueue {
                return null; // not supported
        }
 
+       /**
+        * @param string $command
+        * @param array $params
+        * @return Job
+        */
+       protected function factoryJob( $command, $params ) {
+               // @TODO: dependency inject this as a callback
+               return Job::factory( $command, $params );
+       }
+
        /**
         * @throws JobQueueReadOnlyError
         */
index c2772a6..47ee588 100644 (file)
@@ -290,7 +290,7 @@ class JobQueueDB extends JobQueue {
 
        /**
         * @see JobQueue::doPop()
-        * @return Job|bool
+        * @return RunnableJob|bool
         */
        protected function doPop() {
                $dbw = $this->getMasterDB();
@@ -314,10 +314,12 @@ class JobQueueDB extends JobQueue {
                                        break; // nothing to do
                                }
                                $this->incrStats( 'pops', $this->type );
+
                                // Get the job object from the row...
-                               $title = Title::makeTitle( $row->job_namespace, $row->job_title );
-                               $job = Job::factory( $row->job_cmd, $title,
-                                       self::extractBlob( $row->job_params ) );
+                               $params = self::extractBlob( $row->job_params );
+                               $params = is_array( $params ) ? $params : []; // sanity
+                               $params += [ 'namespace' => $row->job_namespace, 'title' => $row->job_title ];
+                               $job = $this->factoryJob( $row->job_cmd, $params );
                                $job->setMetadata( 'id', $row->job_id );
                                $job->setMetadata( 'timestamp', $row->job_timestamp );
                                break; // done
@@ -481,10 +483,10 @@ class JobQueueDB extends JobQueue {
 
        /**
         * @see JobQueue::doAck()
-        * @param Job $job
+        * @param RunnableJob $job
         * @throws MWException
         */
-       protected function doAck( Job $job ) {
+       protected function doAck( RunnableJob $job ) {
                $id = $job->getMetadata( 'id' );
                if ( $id === null ) {
                        throw new MWException( "Job of type '{$job->getType()}' has no ID." );
@@ -617,11 +619,14 @@ class JobQueueDB extends JobQueue {
                        return new MappedIterator(
                                $dbr->select( 'job', self::selectFields(), $conds ),
                                function ( $row ) {
-                                       $job = Job::factory(
-                                               $row->job_cmd,
-                                               Title::makeTitle( $row->job_namespace, $row->job_title ),
-                                               strlen( $row->job_params ) ? unserialize( $row->job_params ) : []
-                                       );
+                                       $params = strlen( $row->job_params ) ? unserialize( $row->job_params ) : [];
+                                       $params = is_array( $params ) ? $params : []; // sanity
+                                       $params += [
+                                               'namespace' => $row->job_namespace,
+                                               'title' => $row->job_title
+                                       ];
+
+                                       $job = $this->factoryJob( $row->job_cmd, $params );
                                        $job->setMetadata( 'id', $row->job_id );
                                        $job->setMetadata( 'timestamp', $row->job_timestamp );
 
@@ -774,8 +779,8 @@ class JobQueueDB extends JobQueue {
                return [
                        // Fields that describe the nature of the job
                        'job_cmd' => $job->getType(),
-                       'job_namespace' => $job->getTitle()->getNamespace(),
-                       'job_title' => $job->getTitle()->getDBkey(),
+                       'job_namespace' => $job->getParams()['namespace'] ?? NS_SPECIAL,
+                       'job_title' => $job->getParams()['title'] ?? '',
                        'job_params' => self::makeBlob( $job->getParams() ),
                        // Additional job metadata
                        'job_timestamp' => $db->timestamp(),
index 30ab7e7..8b5a62e 100644 (file)
@@ -199,7 +199,7 @@ class JobQueueFederated extends JobQueue {
         * @param HashRing &$partitionRing
         * @param int $flags
         * @throws JobQueueError
-        * @return array List of Job object that could not be inserted
+        * @return IJobSpecification[] List of Job object that could not be inserted
         */
        protected function tryJobInsertions( array $jobs, HashRing &$partitionRing, $flags ) {
                $jobsLeft = [];
@@ -299,7 +299,7 @@ class JobQueueFederated extends JobQueue {
                return false;
        }
 
-       protected function doAck( Job $job ) {
+       protected function doAck( RunnableJob $job ) {
                $partition = $job->getMetadata( 'QueuePartition' );
                if ( $partition === null ) {
                        throw new MWException( "The given job has no defined partition name." );
@@ -308,7 +308,7 @@ class JobQueueFederated extends JobQueue {
                $this->partitionQueues[$partition]->ack( $job );
        }
 
-       protected function doIsRootJobOldDuplicate( Job $job ) {
+       protected function doIsRootJobOldDuplicate( IJobSpecification $job ) {
                $signature = $job->getRootJobParams()['rootJobSignature'];
                $partition = $this->partitionRing->getLiveLocation( $signature );
                try {
index 4bac304..83e5fb2 100644 (file)
@@ -234,7 +234,7 @@ class JobQueueGroup {
         * @param int|string $qtype JobQueueGroup::TYPE_* constant or job type string
         * @param int $flags Bitfield of JobQueueGroup::USE_* constants
         * @param array $blacklist List of job types to ignore
-        * @return Job|bool Returns false on failure
+        * @return RunnableJob|bool Returns false on failure
         */
        public function pop( $qtype = self::TYPE_DEFAULT, $flags = 0, array $blacklist = [] ) {
                global $wgJobClasses;
index cbcd4fb..cb20a76 100644 (file)
@@ -111,7 +111,7 @@ class JobQueueMemory extends JobQueue {
        /**
         * @see JobQueue::doPop
         *
-        * @return Job|bool
+        * @return RunnableJob|bool
         */
        protected function doPop() {
                if ( $this->doGetSize() == 0 ) {
@@ -143,9 +143,9 @@ class JobQueueMemory extends JobQueue {
        /**
         * @see JobQueue::doAck
         *
-        * @param Job $job
+        * @param RunnableJob $job
         */
-       protected function doAck( Job $job ) {
+       protected function doAck( RunnableJob $job ) {
                if ( $this->getAcquiredCount() == 0 ) {
                        return;
                }
@@ -206,11 +206,10 @@ class JobQueueMemory extends JobQueue {
 
        /**
         * @param IJobSpecification $spec
-        *
-        * @return Job
+        * @return RunnableJob
         */
        public function jobFromSpecInternal( IJobSpecification $spec ) {
-               return Job::factory( $spec->getType(), $spec->getTitle(), $spec->getParams() );
+               return $this->factoryJob( $spec->getType(), $spec->getParams() );
        }
 
        /**
index 98a5491..8864688 100644 (file)
@@ -307,7 +307,7 @@ LUA;
 
        /**
         * @see JobQueue::doPop()
-        * @return Job|bool
+        * @return RunnableJob|bool
         * @throws JobQueueError
         */
        protected function doPop() {
@@ -379,12 +379,12 @@ LUA;
 
        /**
         * @see JobQueue::doAck()
-        * @param Job $job
-        * @return Job|bool
+        * @param RunnableJob $job
+        * @return RunnableJob|bool
         * @throws UnexpectedValueException
         * @throws JobQueueError
         */
-       protected function doAck( Job $job ) {
+       protected function doAck( RunnableJob $job ) {
                $uuid = $job->getMetadata( 'uuid' );
                if ( $uuid === null ) {
                        throw new UnexpectedValueException( "Job of type '{$job->getType()}' has no UUID." );
@@ -463,11 +463,11 @@ LUA;
 
        /**
         * @see JobQueue::doIsRootJobOldDuplicate()
-        * @param Job $job
+        * @param IJobSpecification $job
         * @return bool
         * @throws JobQueueError
         */
-       protected function doIsRootJobOldDuplicate( Job $job ) {
+       protected function doIsRootJobOldDuplicate( IJobSpecification $job ) {
                if ( !$job->hasRootJobParams() ) {
                        return false; // job has no de-deplication info
                }
@@ -627,7 +627,7 @@ LUA;
         *
         * @param string $uid
         * @param RedisConnRef $conn
-        * @return Job|bool Returns false if the job does not exist
+        * @return RunnableJob|bool Returns false if the job does not exist
         * @throws JobQueueError
         * @throws UnexpectedValueException
         */
@@ -641,8 +641,10 @@ LUA;
                        if ( !is_array( $item ) ) { // this shouldn't happen
                                throw new UnexpectedValueException( "Could not find job with ID '$uid'." );
                        }
-                       $title = Title::makeTitle( $item['namespace'], $item['title'] );
-                       $job = Job::factory( $item['type'], $title, $item['params'] );
+
+                       $params = $item['params'];
+                       $params += [ 'namespace' => $item['namespace'], 'title' => $item['title'] ];
+                       $job = $this->factoryJob( $item['type'], $params );
                        $job->setMetadata( 'uuid', $item['uuid'] );
                        $job->setMetadata( 'timestamp', $item['timestamp'] );
                        // Add in attempt count for debugging at showJobs.php
@@ -684,8 +686,8 @@ LUA;
                return [
                        // Fields that describe the nature of the job
                        'type' => $job->getType(),
-                       'namespace' => $job->getTitle()->getNamespace(),
-                       'title' => $job->getTitle()->getDBkey(),
+                       'namespace' => $job->getParams()['namespace'] ?? NS_SPECIAL,
+                       'title' => $job->getParams()['title'] ?? '',
                        'params' => $job->getParams(),
                        // Some jobs cannot run until a "release timestamp"
                        'rtimestamp' => $job->getReleaseTimestamp() ?: 0,
@@ -700,11 +702,13 @@ LUA;
 
        /**
         * @param array $fields
-        * @return Job|bool
+        * @return RunnableJob|bool
         */
        protected function getJobFromFields( array $fields ) {
-               $title = Title::makeTitle( $fields['namespace'], $fields['title'] );
-               $job = Job::factory( $fields['type'], $title, $fields['params'] );
+               $params = $fields['params'];
+               $params += [ 'namespace' => $fields['namespace'], 'title' => $fields['title'] ];
+
+               $job = $this->factoryJob( $fields['type'], $params );
                $job->setMetadata( 'uuid', $fields['uuid'] );
                $job->setMetadata( 'timestamp', $fields['timestamp'] );
 
index b04aa83..80a46d0 100644 (file)
@@ -28,8 +28,7 @@
  * $job = new JobSpecification(
  *             'null',
  *             array( 'lives' => 1, 'usleep' => 100, 'pi' => 3.141569 ),
- *             array( 'removeDuplicates' => 1 ),
- *             Title::makeTitle( NS_SPECIAL, 'nullity' )
+ *             array( 'removeDuplicates' => 1 )
  * );
  * JobQueueGroup::singleton()->push( $job )
  * @endcode
@@ -63,8 +62,19 @@ class JobSpecification implements IJobSpecification {
                $this->validateParams( $opts );
 
                $this->type = $type;
+               if ( $title instanceof Title ) {
+                       // Make sure JobQueue classes can pull the title from parameters alone
+                       if ( $title->getDBkey() !== '' ) {
+                               $params += [
+                                       'namespace' => $title->getNamespace(),
+                                       'title' => $title->getDBkey()
+                               ];
+                       }
+               } else {
+                       $title = Title::makeTitle( NS_SPECIAL, '' );
+               }
                $this->params = $params;
-               $this->title = $title ?: Title::makeTitle( NS_SPECIAL, 'Blankpage' );
+               $this->title = $title;
                $this->opts = $opts;
        }
 
diff --git a/includes/jobqueue/RunnableJob.php b/includes/jobqueue/RunnableJob.php
new file mode 100644 (file)
index 0000000..e477b12
--- /dev/null
@@ -0,0 +1,54 @@
+<?php
+/**
+ * Job queue task instance that can be executed via a run() method
+ *
+ * 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
+ */
+
+/**
+ * Job that has a run() method and metadata accessors for JobQueue::pop() and JobQueue::ack()
+ *
+ * Instances are not only enqueueable via JobQueue::push(), but they can also be executed by
+ * by calling their run() method. When constructing a job to be enqueued via JobQueue::push(),
+ * it will not be possible to construct a RunnableJob instance if the class for that job is not
+ * loaded by the application for the local DB domain. In that case, the general-purpose
+ * JobSpecification class can be used instead.
+ *
+ * @ingroup JobQueue
+ * @since 1.33
+ */
+interface RunnableJob extends IJobSpecification {
+       /**
+        * Run the job
+        * @return bool Success
+        */
+       public function run();
+
+       /**
+        * @param string|null $field Metadata field or null to get all the metadata
+        * @return mixed|null Value; null if missing
+        */
+       public function getMetadata( $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
+        */
+       public function setMetadata( $field, $value );
+}
index 356eeba..d8cbf75 100644 (file)
  * @ingroup JobQueue
  * @since 1.27
  */
-class CdnPurgeJob extends Job {
-       /**
-        * @param Title $title
-        * @param array $params Job parameters (urls)
-        */
-       function __construct( Title $title, array $params ) {
-               parent::__construct( 'cdnPurge', $title, $params );
+class CdnPurgeJob extends Job implements GenericParameterJob {
+       function __construct( array $params ) {
+               parent::__construct( 'cdnPurge', $params );
                $this->removeDuplicates = false; // delay semantics are critical
        }
 
index 77adfa1..01fa46c 100644 (file)
@@ -10,7 +10,17 @@ use MediaWiki\MediaWikiServices;
  * @ingroup JobQueue
  * @since 1.31
  */
-class ClearUserWatchlistJob extends Job {
+class ClearUserWatchlistJob extends Job implements GenericParameterJob {
+       /**
+        * @param array $params
+        *  - userId,         The ID for the user whose watchlist is being cleared.
+        *  - maxWatchlistId, The maximum wl_id at the time the job was first created,
+        */
+       public function __construct( array $params ) {
+               parent::__construct( 'clearUserWatchlist', $params );
+
+               $this->removeDuplicates = true;
+       }
 
        /**
         * @param User $user User to clear the watchlist for.
@@ -19,26 +29,7 @@ class ClearUserWatchlistJob extends Job {
         * @return ClearUserWatchlistJob
         */
        public static function newForUser( User $user, $maxWatchlistId ) {
-               return new self(
-                       null,
-                       [ 'userId' => $user->getId(), 'maxWatchlistId' => $maxWatchlistId ]
-               );
-       }
-
-       /**
-        * @param Title|null $title Not used by this job.
-        * @param array $params
-        *  - userId,         The ID for the user whose watchlist is being cleared.
-        *  - maxWatchlistId, The maximum wl_id at the time the job was first created,
-        */
-       public function __construct( Title $title = null, array $params ) {
-               parent::__construct(
-                       'clearUserWatchlist',
-                       SpecialPage::getTitleFor( 'EditWatchlist', 'clear' ),
-                       $params
-               );
-
-               $this->removeDuplicates = true;
+               return new self( [ 'userId' => $user->getId(), 'maxWatchlistId' => $maxWatchlistId ] );
        }
 
        public function run() {
@@ -101,7 +92,7 @@ class ClearUserWatchlistJob extends Job {
                if ( count( $watchlistIds ) === (int)$batchSize ) {
                        // Until we get less results than the limit, recursively push
                        // the same job again.
-                       JobQueueGroup::singleton()->push( new self( $this->getTitle(), $this->getParams() ) );
+                       JobQueueGroup::singleton()->push( new self( $this->getParams() ) );
                }
 
                return true;
index 3b2c899..f53174a 100644 (file)
@@ -33,9 +33,9 @@ use MediaWiki\MediaWikiServices;
  * @ingroup JobQueue
  * @since 1.31
  */
-class ClearWatchlistNotificationsJob extends Job {
-       function __construct( Title $title, array $params ) {
-               parent::__construct( 'clearWatchlistNotifications', $title, $params );
+class ClearWatchlistNotificationsJob extends Job implements GenericParameterJob {
+       function __construct( array $params ) {
+               parent::__construct( 'clearWatchlistNotifications', $params );
 
                static $required = [ 'userId', 'casTime' ];
                $missing = implode( ', ', array_diff( $required, array_keys( $this->params ) ) );
index e6dfae4..b0ce6a5 100644 (file)
@@ -3,16 +3,13 @@
 /**
  * Class DeletePageJob
  */
-class DeletePageJob extends Job {
-       public function __construct( $title, $params = [] ) {
-               parent::__construct( 'deletePage', $title, $params );
+class DeletePageJob extends Job implements GenericParameterJob {
+       public function __construct( array $params ) {
+               parent::__construct( 'deletePage', $params );
+
+               $this->title = Title::makeTitle( $params['namespace'], $params['title'] );
        }
 
-       /**
-        * Execute the job
-        *
-        * @return bool
-        */
        public function run() {
                // Failure to load the page is not job failure.
                // A parallel deletion operation may have already completed the page deletion.
index c005a29..4231e15 100644 (file)
  *
  * @ingroup JobQueue
  */
-final class DuplicateJob extends Job {
+final class DuplicateJob extends Job implements GenericParameterJob {
        /**
         * Callers should use DuplicateJob::newFromJob() instead
         *
-        * @param Title $title
         * @param array $params Job parameters
         */
-       function __construct( Title $title, array $params ) {
-               parent::__construct( 'duplicate', $title, $params );
+       function __construct( array $params ) {
+               parent::__construct( 'duplicate', $params );
        }
 
        /**
         * Get a duplicate no-op version of a job
         *
-        * @param Job $job
+        * @param RunnableJob $job
         * @return Job
         */
-       public static function newFromJob( Job $job ) {
-               $djob = new self( $job->getTitle(), $job->getParams() );
+       public static function newFromJob( RunnableJob $job ) {
+               $djob = new self( $job->getParams() );
                $djob->command = $job->getType();
                $djob->params = is_array( $djob->params ) ? $djob->params : [];
                $djob->params = [ 'isDuplicate' => true ] + $djob->params;
-               $djob->metadata = $job->metadata;
+               $djob->metadata = $job->getMetadata();
 
                return $djob;
        }
index 72923ce..f9735d5 100644 (file)
  * @ingroup JobQueue
  * @since 1.25
  */
-final class EnqueueJob extends Job {
+final class EnqueueJob extends Job implements GenericParameterJob {
        /**
         * Callers should use the factory methods instead
         *
-        * @param Title $title
         * @param array $params Job parameters
         */
-       function __construct( Title $title, array $params ) {
-               parent::__construct( 'enqueue', $title, $params );
+       public function __construct( array $params ) {
+               parent::__construct( 'enqueue', $params );
        }
 
        /**
@@ -75,10 +74,7 @@ final class EnqueueJob extends Job {
                        }
                }
 
-               $eJob = new self(
-                       Title::makeTitle( NS_SPECIAL, 'Badtitle/' . __CLASS__ ),
-                       [ 'jobsByDomain' => $jobMapsByDomain ]
-               );
+               $eJob = new self( [ 'jobsByDomain' => $jobMapsByDomain ] );
                // If *all* jobs to be pushed are to be de-duplicated (a common case), then
                // de-duplicate this whole job itself to avoid build up in high traffic cases
                $eJob->removeDuplicates = $deduplicate;
index 80826fe..01afe6f 100644 (file)
@@ -31,7 +31,7 @@
  * @code
  * $ php maintenance/eval.php
  * > $queue = JobQueueGroup::singleton();
- * > $job = new NullJob( Title::newMainPage(), [ 'lives' => 10 ] );
+ * > $job = new NullJob( [ 'lives' => 10 ] );
  * > $queue->push( $job );
  * @endcode
  * You can then confirm the job has been enqueued by using the showJobs.php
  *
  * @ingroup JobQueue
  */
-class NullJob extends Job {
+class NullJob extends Job implements GenericParameterJob {
        /**
-        * @param Title $title
         * @param array $params Job parameters (lives, usleep)
         */
-       function __construct( Title $title, array $params ) {
-               parent::__construct( 'null', $title, $params );
+       function __construct( array $params ) {
+               parent::__construct( 'null', $params );
                if ( !isset( $this->params['lives'] ) ) {
                        $this->params['lives'] = 1;
                }
@@ -67,7 +66,7 @@ class NullJob extends Job {
                if ( $this->params['lives'] > 1 ) {
                        $params = $this->params;
                        $params['lives']--;
-                       $job = new self( $this->title, $params );
+                       $job = new self( $params );
                        JobQueueGroup::singleton()->push( $job );
                }
 
index bd0df5b..ac8f94a 100644 (file)
@@ -21,9 +21,9 @@
  * @ingroup JobQueue
  */
 
-class UserGroupExpiryJob extends Job {
-       public function __construct( $params = [] ) {
-               parent::__construct( 'userGroupExpiry', Title::newMainPage(), $params );
+class UserGroupExpiryJob extends Job implements GenericParameterJob {
+       public function __construct( array $params = [] ) {
+               parent::__construct( 'userGroupExpiry', $params );
                $this->removeDuplicates = true;
        }
 
index 655fa27..f33ce35 100644 (file)
@@ -2736,6 +2736,8 @@ class WikiPage implements Page, IDBAccessObject {
                        $dbw->endAtomic( __METHOD__ );
 
                        $jobParams = [
+                               'namespace' => $this->getTitle()->getNamespace(),
+                               'title' => $this->getTitle()->getDBkey(),
                                'wikiPageId' => $id,
                                'requestId' => $webRequestId ?? WebRequest::getRequestId(),
                                'reason' => $reason,
@@ -2745,7 +2747,7 @@ class WikiPage implements Page, IDBAccessObject {
                                'logsubtype' => $logsubtype,
                        ];
 
-                       $job = new DeletePageJob( $this->getTitle(), $jobParams );
+                       $job = new DeletePageJob( $jobParams );
                        JobQueueGroup::singleton()->push( $job );
 
                        $status->warning( 'delete-scheduled',
index 8aca689..e287a35 100644 (file)
@@ -904,10 +904,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
                }
 
                // If the page is watched by the user (or may be watched), update the timestamp
-               $job = new ClearWatchlistNotificationsJob(
-                       $user->getUserPage(),
-                       [ 'userId'  => $user->getId(), 'timestamp' => $timestamp, 'casTime' => time() ]
-               );
+               $job = new ClearWatchlistNotificationsJob( [
+                       'userId'  => $user->getId(), 'timestamp' => $timestamp, 'casTime' => time()
+               ] );
 
                // Try to run this post-send
                // Calls DeferredUpdates::addCallableUpdate in normal operation
index a6a92c6..b0d89a6 100644 (file)
@@ -10,11 +10,11 @@ class SiteStatsTest extends MediaWikiTestCase {
                $this->setService( 'MainWANObjectCache', $cache );
                $jobq = JobQueueGroup::singleton();
 
-               $jobq->push( new NullJob( Title::newMainPage(), [] ) );
+               $jobq->push( Job::factory( 'null', Title::newMainPage(), [] ) );
                $this->assertEquals( 1, SiteStats::jobs(),
                         'A single job enqueued bumps jobscount stat to 1' );
 
-               $jobq->push( new NullJob( Title::newMainPage(), [] ) );
+               $jobq->push( Job::factory( 'null', Title::newMainPage(), [] ) );
                $this->assertEquals( 1, SiteStats::jobs(),
                        'SiteStats::jobs() count does not reflect addition ' .
                        'of a second job (cached)'
index 81a80b6..1baaa54 100644 (file)
@@ -380,12 +380,12 @@ class JobQueueTest extends MediaWikiTestCase {
        }
 
        function newJob( $i = 0, $rootJob = [] ) {
-               return new NullJob( Title::newMainPage(),
+               return Job::factory( 'null', Title::newMainPage(),
                        [ 'lives' => 0, 'usleep' => 0, 'removeDuplicates' => 0, 'i' => $i ] + $rootJob );
        }
 
        function newDedupedJob( $i = 0, $rootJob = [] ) {
-               return new NullJob( Title::newMainPage(),
+               return Job::factory( 'null', Title::newMainPage(),
                        [ 'lives' => 0, 'usleep' => 0, 'removeDuplicates' => 1, 'i' => $i ] + $rootJob );
        }
 }
index 769b193..9fe3e3d 100644 (file)
@@ -29,31 +29,31 @@ class JobTest extends MediaWikiTestCase {
                return [
                        [
                                $this->getMockJob( false ),
-                               'someCommand  ' . $requestId
+                               'someCommand Special: ' . $requestId
                        ],
                        [
                                $this->getMockJob( [ 'key' => 'val' ] ),
-                               'someCommand  key=val ' . $requestId
+                               'someCommand Special: key=val ' . $requestId
                        ],
                        [
                                $this->getMockJob( [ 'key' => [ 'inkey' => 'inval' ] ] ),
-                               'someCommand  key={"inkey":"inval"} ' . $requestId
+                               'someCommand Special: key={"inkey":"inval"} ' . $requestId
                        ],
                        [
                                $this->getMockJob( [ 'val1' ] ),
-                               'someCommand  0=val1 ' . $requestId
+                               'someCommand Special: 0=val1 ' . $requestId
                        ],
                        [
                                $this->getMockJob( [ 'val1', 'val2' ] ),
-                               'someCommand  0=val1 1=val2 ' . $requestId
+                               'someCommand Special: 0=val1 1=val2 ' . $requestId
                        ],
                        [
                                $this->getMockJob( [ new stdClass() ] ),
-                               'someCommand  0=object(stdClass) ' . $requestId
+                               'someCommand Special: 0=object(stdClass) ' . $requestId
                        ],
                        [
                                $this->getMockJob( [ $mockToStringObj ] ),
-                               'someCommand  0={STRING_OBJ_VAL} ' . $requestId
+                               'someCommand Special: 0={STRING_OBJ_VAL} ' . $requestId
                        ],
                        [
                                $this->getMockJob( [
@@ -72,7 +72,7 @@ class JobTest extends MediaWikiTestCase {
                                        ],
                                        "triggeredRecursive" => true
                                ] ),
-                               'someCommand  pages={"932737":[0,"Robert_James_Waller"]} ' .
+                               'someCommand Special: pages={"932737":[0,"Robert_James_Waller"]} ' .
                                'rootJobSignature=45868e99bba89064e4483743ebb9b682ef95c1a7 ' .
                                'rootJobTimestamp=20160309110158 masterPos=' .
                                '{"file":"db1023-bin.001288","pos":"308257743","asOfTime":' .
@@ -85,11 +85,13 @@ class JobTest extends MediaWikiTestCase {
        }
 
        public function getMockJob( $params ) {
+               $title = new Title();
                $mock = $this->getMockForAbstractClass(
                        Job::class,
-                       [ 'someCommand', new Title(), $params ],
+                       [ 'someCommand', $title, $params ],
                        'SomeJob'
                );
+
                return $mock;
        }
 
@@ -115,7 +117,7 @@ class JobTest extends MediaWikiTestCase {
                return [
                        'class name' => [ 'NullJob' ],
                        'closure' => [ function ( Title $title, array $params ) {
-                               return new NullJob( $title, $params );
+                               return Job::factory( 'null', $title, $params );
                        } ],
                        'function' => [ [ $this, 'newNullJob' ] ],
                        'static function' => [ self::class . '::staticNullJob' ]
@@ -123,11 +125,91 @@ class JobTest extends MediaWikiTestCase {
        }
 
        public function newNullJob( Title $title, array $params ) {
-               return new NullJob( $title, $params );
+               return Job::factory( 'null', $title, $params );
        }
 
        public static function staticNullJob( Title $title, array $params ) {
-               return new NullJob( $title, $params );
+               return Job::factory( 'null', $title, $params );
+       }
+
+       /**
+        * @covers Job::factory
+        * @covers Job::__construct()
+        */
+       public function testJobSignatureGeneric() {
+               $testPage = Title::makeTitle( NS_PROJECT, 'x' );
+               $blankTitle = Title::makeTitle( NS_SPECIAL, '' );
+               $params = [ 'z' => 1, 'lives' => 1, 'usleep' => 0 ];
+               $paramsWithTitle = $params + [ 'namespace' => NS_PROJECT, 'title' => 'x' ];
+
+               $job = new NullJob( [ 'namespace' => NS_PROJECT, 'title' => 'x' ] + $params );
+               $this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertJobParamsMatch( $job, $paramsWithTitle );
+
+               $job = Job::factory( 'null', $testPage, $params );
+               $this->assertEquals( $blankTitle->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertJobParamsMatch( $job, $params );
+
+               $job = Job::factory( 'null', $paramsWithTitle );
+               $this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertJobParamsMatch( $job, $paramsWithTitle );
+
+               $job = Job::factory( 'null', $params );
+               $this->assertTrue( $blankTitle->equals( $job->getTitle() ) );
+               $this->assertJobParamsMatch( $job, $params );
+       }
+
+       /**
+        * @covers Job::factory
+        * @covers Job::__construct()
+        */
+       public function testJobSignatureTitleBased() {
+               $testPage = Title::makeTitle( NS_PROJECT, 'x' );
+               $blankTitle = Title::makeTitle( NS_SPECIAL, '' );
+               $params = [ 'z' => 1, 'causeAction' => 'unknown', 'causeAgent' => 'unknown' ];
+               $paramsWithTitle = $params + [ 'namespace' => NS_PROJECT, 'title' => 'x' ];
+
+               $job = new RefreshLinksJob( $testPage, $params );
+               $this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertSame( $testPage, $job->getTitle() );
+               $this->assertJobParamsMatch( $job, $paramsWithTitle );
+               $this->assertSame( $testPage, $job->getTitle() );
+
+               $job = Job::factory( 'refreshLinks', $testPage, $params );
+               $this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertJobParamsMatch( $job, $paramsWithTitle );
+
+               $job = Job::factory( 'refreshLinks', $paramsWithTitle );
+               $this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertJobParamsMatch( $job, $paramsWithTitle );
+
+               $job = Job::factory( 'refreshLinks', $params );
+               $this->assertTrue( $blankTitle->equals( $job->getTitle() ) );
+               $this->assertJobParamsMatch( $job, $params );
+       }
+
+       /**
+        * @covers Job::factory
+        * @covers Job::__construct()
+        */
+       public function testJobSignatureTitleBasedIncomplete() {
+               $testPage = Title::makeTitle( NS_PROJECT, 'x' );
+               $blankTitle = Title::makeTitle( NS_SPECIAL, '' );
+               $params = [ 'z' => 1, 'causeAction' => 'unknown', 'causeAgent' => 'unknown' ];
+
+               $job = new RefreshLinksJob( $testPage, $params + [ 'namespace' => 0 ] );
+               $this->assertEquals( $blankTitle->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertJobParamsMatch( $job, $params + [ 'namespace' => 0 ] );
+
+               $job = new RefreshLinksJob( $testPage, $params + [ 'title' => 'x' ] );
+               $this->assertEquals( $blankTitle->getPrefixedText(), $job->getTitle()->getPrefixedText() );
+               $this->assertJobParamsMatch( $job, $params + [ 'title' => 'x' ] );
        }
 
+       private function assertJobParamsMatch( IJobSpecification $job, array $params ) {
+               $actual = $job->getParams();
+               unset( $actual['requestId'] );
+
+               $this->assertEquals( $actual, $params );
+       }
 }
index 27cae8a..1a2941d 100644 (file)
@@ -51,13 +51,9 @@ class ClearUserWatchlistJobTest extends MediaWikiTestCase {
                $this->setMwGlobals( 'wgUpdateRowsPerQuery', 2 );
 
                JobQueueGroup::singleton()->push(
-                       new ClearUserWatchlistJob(
-                               null,
-                               [
-                                       'userId' => $user->getId(),
-                                       'maxWatchlistId' => $maxId,
-                               ]
-                       )
+                       new ClearUserWatchlistJob( [
+                               'userId' => $user->getId(), 'maxWatchlistId' => $maxId,
+                       ] )
                );
 
                $this->assertEquals( 1, JobQueueGroup::singleton()->getQueueSizes()['clearUserWatchlist'] );