Finish removing boolean return values from JobQueue code
authorThiemo Mättig <thiemo.maettig@wikimedia.de>
Wed, 16 Apr 2014 18:07:26 +0000 (20:07 +0200)
committerThiemo Mättig <thiemo.maettig@wikimedia.de>
Mon, 28 Apr 2014 20:21:58 +0000 (22:21 +0200)
This is a follow-up for patch
Ia706ac0122a7dd7f418e2dc2d3bd36e9a0252c25.

Change-Id: I19fe58a939706d3f7594d937e0bcad6d97c52a50

includes/api/ApiUpload.php
includes/jobqueue/Job.php
includes/jobqueue/JobQueue.php
includes/jobqueue/JobQueueDB.php
includes/jobqueue/JobQueueFederated.php
includes/jobqueue/JobQueueGroup.php
includes/jobqueue/JobQueueRedis.php

index 3ef2bbe..49306d7 100644 (file)
@@ -234,7 +234,7 @@ class ApiUpload extends ApiBase {
                                        array( 'result' => 'Poll',
                                                'stage' => 'queued', 'status' => Status::newGood() )
                                );
-                               $ok = JobQueueGroup::singleton()->push( new AssembleUploadChunksJob(
+                               JobQueueGroup::singleton()->push( new AssembleUploadChunksJob(
                                        Title::makeTitle( NS_FILE, $filekey ),
                                        array(
                                                'filename' => $this->mParams['filename'],
@@ -242,13 +242,7 @@ class ApiUpload extends ApiBase {
                                                'session' => $this->getContext()->exportSession()
                                        )
                                ) );
-                               if ( $ok ) {
-                                       $result['result'] = 'Poll';
-                               } else {
-                                       UploadBase::setSessionStatus( $filekey, false );
-                                       $this->dieUsage(
-                                               "Failed to start AssembleUploadChunks.php", 'stashfailed' );
-                               }
+                               $result['result'] = 'Poll';
                        } else {
                                $status = $this->mUpload->concatenateChunks();
                                if ( !$status->isGood() ) {
@@ -625,7 +619,7 @@ class ApiUpload extends ApiBase {
                                $this->mParams['filekey'],
                                array( 'result' => 'Poll', 'stage' => 'queued', 'status' => Status::newGood() )
                        );
-                       $ok = JobQueueGroup::singleton()->push( new PublishStashedFileJob(
+                       JobQueueGroup::singleton()->push( new PublishStashedFileJob(
                                Title::makeTitle( NS_FILE, $this->mParams['filename'] ),
                                array(
                                        'filename' => $this->mParams['filename'],
@@ -636,13 +630,7 @@ class ApiUpload extends ApiBase {
                                        'session' => $this->getContext()->exportSession()
                                )
                        ) );
-                       if ( $ok ) {
-                               $result['result'] = 'Poll';
-                       } else {
-                               UploadBase::setSessionStatus( $this->mParams['filekey'], false );
-                               $this->dieUsage(
-                                       "Failed to start PublishStashedFile.php", 'publishfailed' );
-                       }
+                       $result['result'] = 'Poll';
                } else {
                        /** @var $status Status */
                        $status = $this->mUpload->performUpload( $this->mParams['comment'],
index 3456373..4ee370e 100644 (file)
@@ -92,7 +92,8 @@ abstract class Job implements IJobSpecification {
         * @deprecated since 1.21
         */
        public static function batchInsert( $jobs ) {
-               return JobQueueGroup::singleton()->push( $jobs );
+               JobQueueGroup::singleton()->push( $jobs );
+               return true;
        }
 
        /**
@@ -107,7 +108,8 @@ abstract class Job implements IJobSpecification {
         * @deprecated since 1.21
         */
        public static function safeBatchInsert( $jobs ) {
-               return JobQueueGroup::singleton()->push( $jobs, JobQueue::QOS_ATOMIC );
+               JobQueueGroup::singleton()->push( $jobs, JobQueue::QOS_ATOMIC );
+               return true;
        }
 
        /**
@@ -281,7 +283,8 @@ abstract class Job implements IJobSpecification {
         * @deprecated since 1.21
         */
        public function insert() {
-               return JobQueueGroup::singleton()->push( $this );
+               JobQueueGroup::singleton()->push( $this );
+               return true;
        }
 
        /**
index c858e42..c00d22e 100644 (file)
@@ -323,7 +323,7 @@ abstract class JobQueue {
         */
        final public function batchPush( array $jobs, $flags = 0 ) {
                if ( !count( $jobs ) ) {
-                       return true; // nothing to do
+                       return; // nothing to do
                }
 
                foreach ( $jobs as $job ) {
@@ -344,7 +344,7 @@ abstract class JobQueue {
        /**
         * @see JobQueue::batchPush()
         * @param array $jobs
-        * @param array $flags
+        * @param int $flags
         */
        abstract protected function doBatchPush( array $jobs, $flags );
 
index 1991bb4..08873cc 100644 (file)
@@ -189,9 +189,9 @@ class JobQueueDB extends JobQueue {
        /**
         * @see JobQueue::doBatchPush()
         * @param array $jobs
-        * @param array $flags
+        * @param int $flags
         * @throws DBError|Exception
-        * @return bool
+        * @return void
         */
        protected function doBatchPush( array $jobs, $flags ) {
                $dbw = $this->getMasterDB();
@@ -203,8 +203,6 @@ class JobQueueDB extends JobQueue {
                                $that->doBatchPushInternal( $dbw, $jobs, $flags, $method );
                        }
                );
-
-               return true;
        }
 
        /**
@@ -215,11 +213,11 @@ class JobQueueDB extends JobQueue {
         * @param int $flags
         * @param string $method
         * @throws DBError
-        * @return bool
+        * @return void
         */
        public function doBatchPushInternal( IDatabase $dbw, array $jobs, $flags, $method ) {
                if ( !count( $jobs ) ) {
-                       return true;
+                       return;
                }
 
                $rowSet = array(); // (sha1 => job) map for jobs that are de-duplicated
@@ -277,7 +275,7 @@ class JobQueueDB extends JobQueue {
 
                $this->cache->set( $this->getCacheKey( 'empty' ), 'false', JobQueueDB::CACHE_TTL_LONG );
 
-               return true;
+               return;
        }
 
        /**
index 9b4c315..58d5c67 100644 (file)
@@ -235,8 +235,6 @@ class JobQueueFederated extends JobQueue {
                        throw new JobQueueError(
                                "Could not insert job(s), {$this->maxPartitionsTry} partitions tried." );
                }
-
-               return true;
        }
 
        /**
index 948baa0..16908a7 100644 (file)
@@ -106,13 +106,12 @@ class JobQueueGroup {
         *
         * @param Job|array $jobs A single Job or a list of Jobs
         * @throws MWException
-        * @return bool
-        * @todo Return value here is not useful
+        * @return void
         */
        public function push( $jobs ) {
                $jobs = is_array( $jobs ) ? $jobs : array( $jobs );
                if ( !count( $jobs ) ) {
-                       return true;
+                       return;
                }
 
                $jobsByType = array(); // (job type => list of jobs)
@@ -135,8 +134,6 @@ class JobQueueGroup {
                                $this->cache->clear( 'queues-ready' );
                        }
                }
-
-               return true;
        }
 
        /**
index d81a292..6739a84 100644 (file)
@@ -182,8 +182,8 @@ class JobQueueRedis extends JobQueue {
        /**
         * @see JobQueue::doBatchPush()
         * @param array $jobs
-        * @param array $flags
-        * @return bool
+        * @param int $flags
+        * @return void
         * @throws JobQueueError
         */
        protected function doBatchPush( array $jobs, $flags ) {
@@ -199,7 +199,7 @@ class JobQueueRedis extends JobQueue {
                }
 
                if ( !count( $items ) ) {
-                       return true; // nothing to do
+                       return; // nothing to do
                }
 
                $conn = $this->getConnection();
@@ -223,7 +223,7 @@ class JobQueueRedis extends JobQueue {
                        if ( $failed > 0 ) {
                                wfDebugLog( 'JobQueueRedis', "Could not insert {$failed} {$this->type} job(s)." );
 
-                               return false;
+                               throw new RedisException( "Could not insert {$failed} {$this->type} job(s)." );
                        }
                        JobQueue::incrStats( 'job-insert', $this->type, count( $items ), $this->wiki );
                        JobQueue::incrStats( 'job-insert-duplicate', $this->type,
@@ -231,8 +231,6 @@ class JobQueueRedis extends JobQueue {
                } catch ( RedisException $e ) {
                        $this->throwRedisException( $conn, $e );
                }
-
-               return true;
        }
 
        /**