From: Aaron Schulz Date: Wed, 16 Apr 2014 17:51:11 +0000 (-0700) Subject: Revert "Revert "Removed useless JobQueue return values"" X-Git-Tag: 1.31.0-rc.0~16194^2 X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/journal.php?a=commitdiff_plain;h=94c37ffb960b2004459389dd1c75d2e476af188b;p=lhc%2Fweb%2Fwiklou.git 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 2d7103c58f..9b13aea2d8 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 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 95021485b3..f2599ae349 100644 --- 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 b1dfe14966..65912829d6 100644 --- 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 e833115ba9..c5a78278f7 100644 --- 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 3319490131..70374dc651 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->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 ) {