Revert "Revert "Removed useless JobQueue return values""

Made the obvious update to a caller missed in the change.

This reverts commit c76d5a95c1dd1c3086461feae8bc1c0b688bde1e.

Change-Id: I67400ba5b9fc7de16c9f9d5075c488c5e58cea9e

diff --git a/includes/jobqueue/JobQueue.php b/includes/jobqueue/JobQueue.php
index 2d7103c..9b13aea 100644 (file)
--- a/includes/jobqueue/JobQueue.php
+++ b/includes/jobqueue/JobQueue.php
@@ -304,11 +304,11 @@ abstract class JobQueue {
         *
         * @param Job|array $jobs A single job or an array of Jobs
         * @param int $flags Bitfield (supports JobQueue::QOS_ATOMIC)
-        * @return bool Returns false on failure
+        * @return void
         * @throws JobQueueError
         */
        final public function push( $jobs, $flags = 0 ) {
-               return $this->batchPush( is_array( $jobs ) ? $jobs : array( $jobs ), $flags );
+               $this->batchPush( is_array( $jobs ) ? $jobs : array( $jobs ), $flags );
        }
 
        /**
@@ -318,8 +318,8 @@ abstract class JobQueue {
         *
         * @param array $jobs List of Jobs
         * @param int $flags Bitfield (supports JobQueue::QOS_ATOMIC)
+        * @return void
         * @throws MWException
-        * @return bool Returns false on failure
         */
        final public function batchPush( array $jobs, $flags = 0 ) {
                if ( !count( $jobs ) ) {
@@ -337,17 +337,14 @@ abstract class JobQueue {
                }
 
                wfProfileIn( __METHOD__ );
-               $ok = $this->doBatchPush( $jobs, $flags );
+               $this->doBatchPush( $jobs, $flags );
                wfProfileOut( __METHOD__ );
-
-               return $ok;
        }
 
        /**
         * @see JobQueue::batchPush()
         * @param array $jobs
         * @param $flags
-        * @return bool
         */
        abstract protected function doBatchPush( array $jobs, $flags );
 
@@ -399,24 +396,21 @@ abstract class JobQueue {
         * Outside callers should use JobQueueGroup::ack() instead of this function.
         *
         * @param Job $job
+        * @return void
         * @throws MWException
-        * @return bool
         */
        final public function ack( Job $job ) {
                if ( $job->getType() !== $this->type ) {
                        throw new MWException( "Got '{$job->getType()}' job; expected '{$this->type}'." );
                }
                wfProfileIn( __METHOD__ );
-               $ok = $this->doAck( $job );
+               $this->doAck( $job );
                wfProfileOut( __METHOD__ );
-
-               return $ok;
        }
 
        /**
         * @see JobQueue::ack()
         * @param Job $job
-        * @return bool
         */
        abstract protected function doAck( Job $job );
 
@@ -539,22 +533,19 @@ abstract class JobQueue {
        /**
         * Deleted all unclaimed and delayed jobs from the queue
         *
-        * @return bool Success
         * @throws JobQueueError
         * @since 1.22
+        * @return void
         */
        final public function delete() {
                wfProfileIn( __METHOD__ );
-               $res = $this->doDelete();
+               $this->doDelete();
                wfProfileOut( __METHOD__ );
-
-               return $res;
        }
 
        /**
         * @see JobQueue::delete()
         * @throws MWException
-        * @return bool Success
         */
        protected function doDelete() {
                throw new MWException( "This method is not implemented." );

diff --git a/includes/jobqueue/JobQueueFederated.php b/includes/jobqueue/JobQueueFederated.php
index 9502148..f2599ae 100644 (file)
--- a/includes/jobqueue/JobQueueFederated.php
+++ b/includes/jobqueue/JobQueueFederated.php
@@ -265,7 +265,8 @@ class JobQueueFederated extends JobQueue {
                        /** @var JobQueue $queue */
                        $queue = $this->partitionQueues[$partition];
                        try {
-                               $ok = $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC );
+                               $ok = true;
+                               $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC );
                        } catch ( JobQueueError $e ) {
                                $ok = false;
                                MWExceptionHandler::logException( $e );
@@ -287,7 +288,8 @@ class JobQueueFederated extends JobQueue {
                        $partition = ArrayUtils::pickRandom( $partitionRing->getLocationWeights() );
                        $queue = $this->partitionQueues[$partition];
                        try {
-                               $ok = $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC );
+                               $ok = true;
+                               $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC );
                        } catch ( JobQueueError $e ) {
                                $ok = false;
                                MWExceptionHandler::logException( $e );

diff --git a/includes/jobqueue/JobQueueGroup.php b/includes/jobqueue/JobQueueGroup.php
index b1dfe14..6591282 100644 (file)
--- a/includes/jobqueue/JobQueueGroup.php
+++ b/includes/jobqueue/JobQueueGroup.php
@@ -107,6 +107,7 @@ 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
         */
        public function push( $jobs ) {
                $jobs = is_array( $jobs ) ? $jobs : array( $jobs );
@@ -123,13 +124,9 @@ class JobQueueGroup {
                        }
                }
 
-               $ok = true;
                foreach ( $jobsByType as $type => $jobs ) {
-                       if ( $this->get( $type )->push( $jobs ) ) {
-                               JobQueueAggregator::singleton()->notifyQueueNonEmpty( $this->wiki, $type );
-                       } else {
-                               $ok = false;
-                       }
+                       $this->get( $type )->push( $jobs );
+                       JobQueueAggregator::singleton()->notifyQueueNonEmpty( $this->wiki, $type );
                }
 
                if ( $this->cache->has( 'queues-ready', 'list' ) ) {
@@ -139,7 +136,7 @@ class JobQueueGroup {
                        }
                }
 
-               return $ok;
+               return true;
        }
 
        /**

diff --git a/maintenance/copyJobQueue.php b/maintenance/copyJobQueue.php
index e833115..c5a7827 100644 (file)
--- a/maintenance/copyJobQueue.php
+++ b/maintenance/copyJobQueue.php
@@ -78,17 +78,15 @@ class CopyJobQueue extends Maintenance {
                        ++$total;
                        $batch[] = $job;
                        if ( count( $batch ) >= $this->mBatchSize ) {
-                               if ( $dst->push( $batch ) ) {
-                                       $totalOK += count( $batch );
-                               }
+                               $dst->push( $batch );
+                               $totalOK += count( $batch );
                                $batch = array();
                                $dst->waitForBackups();
                        }
                }
                if ( count( $batch ) ) {
-                       if ( $dst->push( $batch ) ) {
-                               $totalOK += count( $batch );
-                       }
+                       $dst->push( $batch );
+                       $totalOK += count( $batch );
                        $dst->waitForBackups();
                }
                return array( $total, $totalOK );

diff --git a/tests/phpunit/includes/jobqueue/JobQueueTest.php b/tests/phpunit/includes/jobqueue/JobQueueTest.php
index 3319490..70374dc 100644 (file)
--- a/tests/phpunit/includes/jobqueue/JobQueueTest.php
+++ b/tests/phpunit/includes/jobqueue/JobQueueTest.php
@@ -108,8 +108,8 @@ class JobQueueTest extends MediaWikiTestCase {
                $this->assertEquals( 0, $queue->getSize(), "Queue is empty ($desc)" );
                $this->assertEquals( 0, $queue->getAcquiredCount(), "Queue is empty ($desc)" );
 
-               $this->assertTrue( $queue->push( $this->newJob() ), "Push worked ($desc)" );
-               $this->assertTrue( $queue->batchPush( array( $this->newJob() ) ), "Push worked ($desc)" );
+               $this->assertNull( $queue->push( $this->newJob() ), "Push worked ($desc)" );
+               $this->assertNull( $queue->batchPush( array( $this->newJob() ) ), "Push worked ($desc)" );
 
                $this->assertFalse( $queue->isEmpty(), "Queue is not empty ($desc)" );
 
@@ -157,7 +157,7 @@ class JobQueueTest extends MediaWikiTestCase {
                $queue->flushCaches();
                $this->assertEquals( 0, $queue->getAcquiredCount(), "Active job count ($desc)" );
 
-               $this->assertTrue( $queue->batchPush( array( $this->newJob(), $this->newJob() ) ),
+               $this->assertNull( $queue->batchPush( array( $this->newJob(), $this->newJob() ) ),
                        "Push worked ($desc)" );
                $this->assertFalse( $queue->isEmpty(), "Queue is not empty ($desc)" );
 
@@ -183,7 +183,7 @@ class JobQueueTest extends MediaWikiTestCase {
                $this->assertEquals( 0, $queue->getSize(), "Queue is empty ($desc)" );
                $this->assertEquals( 0, $queue->getAcquiredCount(), "Queue is empty ($desc)" );
 
-               $this->assertTrue(
+               $this->assertNull(
                        $queue->batchPush(
                                array( $this->newDedupedJob(), $this->newDedupedJob(), $this->newDedupedJob() )
                        ),
@@ -195,7 +195,7 @@ class JobQueueTest extends MediaWikiTestCase {
                $this->assertEquals( 1, $queue->getSize(), "Queue size is correct ($desc)" );
                $this->assertEquals( 0, $queue->getAcquiredCount(), "No jobs active ($desc)" );
 
-               $this->assertTrue(
+               $this->assertNull(
                        $queue->batchPush(
                                array( $this->newDedupedJob(), $this->newDedupedJob(), $this->newDedupedJob() )
                        ),
@@ -244,7 +244,7 @@ class JobQueueTest extends MediaWikiTestCase {
                $id = wfRandomString( 32 );
                $root1 = Job::newRootJobParams( "nulljobspam:$id" ); // task ID/timestamp
                for ( $i = 0; $i < 5; ++$i ) {
-                       $this->assertTrue( $queue->push( $this->newJob( 0, $root1 ) ), "Push worked ($desc)" );
+                       $this->assertNull( $queue->push( $this->newJob( 0, $root1 ) ), "Push worked ($desc)" );
                }
                $queue->deduplicateRootJob( $this->newJob( 0, $root1 ) );
                sleep( 1 ); // roo job timestamp will increase
@@ -252,7 +252,7 @@ class JobQueueTest extends MediaWikiTestCase {
                $this->assertNotEquals( $root1['rootJobTimestamp'], $root2['rootJobTimestamp'],
                        "Root job signatures have different timestamps." );
                for ( $i = 0; $i < 5; ++$i ) {
-                       $this->assertTrue( $queue->push( $this->newJob( 0, $root2 ) ), "Push worked ($desc)" );
+                       $this->assertNull( $queue->push( $this->newJob( 0, $root2 ) ), "Push worked ($desc)" );
                }
                $queue->deduplicateRootJob( $this->newJob( 0, $root2 ) );
 
@@ -296,7 +296,7 @@ class JobQueueTest extends MediaWikiTestCase {
                $this->assertEquals( 0, $queue->getAcquiredCount(), "Queue is empty ($desc)" );
 
                for ( $i = 0; $i < 10; ++$i ) {
-                       $this->assertTrue( $queue->push( $this->newJob( $i ) ), "Push worked ($desc)" );
+                       $this->assertNull( $queue->push( $this->newJob( $i ) ), "Push worked ($desc)" );
                }
 
                for ( $i = 0; $i < 10; ++$i ) {