From c76d5a95c1dd1c3086461feae8bc1c0b688bde1e Mon Sep 17 00:00:00 2001 From: Reedy Date: Wed, 16 Apr 2014 17:41:30 +0000 Subject: [PATCH] Revert "Removed useless JobQueue return values" This reverts commit bc8c89d2df39d64f15770d29874904ebcff2f131. Bug: 64007 Change-Id: I4b4dbe4637dc50cd4630ef19d54f01efba10e138 --- includes/jobqueue/JobQueue.php | 25 +++++++++++++------ includes/jobqueue/JobQueueFederated.php | 6 ++--- .../includes/jobqueue/JobQueueTest.php | 16 ++++++------ 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/includes/jobqueue/JobQueue.php b/includes/jobqueue/JobQueue.php index ff6d922046..a537861f43 100644 --- 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 void + * @return bool Returns false on failure * @throws JobQueueError */ final public function push( $jobs, $flags = 0 ) { - $this->batchPush( is_array( $jobs ) ? $jobs : array( $jobs ), $flags ); + return $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,14 +337,17 @@ abstract class JobQueue { } wfProfileIn( __METHOD__ ); - $this->doBatchPush( $jobs, $flags ); + $ok = $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 ); @@ -396,21 +399,24 @@ 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__ ); - $this->doAck( $job ); + $ok = $this->doAck( $job ); wfProfileOut( __METHOD__ ); + + return $ok; } /** * @see JobQueue::ack() * @param Job $job + * @return bool */ abstract protected function doAck( Job $job ); @@ -533,19 +539,22 @@ 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__ ); - $this->doDelete(); + $res = $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 f2599ae349..95021485b3 100644 --- a/includes/jobqueue/JobQueueFederated.php +++ b/includes/jobqueue/JobQueueFederated.php @@ -265,8 +265,7 @@ class JobQueueFederated extends JobQueue { /** @var JobQueue $queue */ $queue = $this->partitionQueues[$partition]; try { - $ok = true; - $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC ); + $ok = $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC ); } catch ( JobQueueError $e ) { $ok = false; MWExceptionHandler::logException( $e ); @@ -288,8 +287,7 @@ class JobQueueFederated extends JobQueue { $partition = ArrayUtils::pickRandom( $partitionRing->getLocationWeights() ); $queue = $this->partitionQueues[$partition]; try { - $ok = true; - $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC ); + $ok = $queue->doBatchPush( $jobBatch, $flags | self::QOS_ATOMIC ); } catch ( JobQueueError $e ) { $ok = false; MWExceptionHandler::logException( $e ); diff --git a/tests/phpunit/includes/jobqueue/JobQueueTest.php b/tests/phpunit/includes/jobqueue/JobQueueTest.php index 70374dc651..3319490131 100644 --- 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->assertNull( $queue->push( $this->newJob() ), "Push worked ($desc)" ); - $this->assertNull( $queue->batchPush( array( $this->newJob() ) ), "Push worked ($desc)" ); + $this->assertTrue( $queue->push( $this->newJob() ), "Push worked ($desc)" ); + $this->assertTrue( $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->assertNull( $queue->batchPush( array( $this->newJob(), $this->newJob() ) ), + $this->assertTrue( $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->assertNull( + $this->assertTrue( $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->assertNull( + $this->assertTrue( $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->assertNull( $queue->push( $this->newJob( 0, $root1 ) ), "Push worked ($desc)" ); + $this->assertTrue( $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->assertNull( $queue->push( $this->newJob( 0, $root2 ) ), "Push worked ($desc)" ); + $this->assertTrue( $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->assertNull( $queue->push( $this->newJob( $i ) ), "Push worked ($desc)" ); + $this->assertTrue( $queue->push( $this->newJob( $i ) ), "Push worked ($desc)" ); } for ( $i = 0; $i < 10; ++$i ) { -- 2.20.1