Awight has uploaded a new change for review.
https://gerrit.wikimedia.org/r/300053
Change subject: Use pop when we don't need transactionality
......................................................................
Use pop when we don't need transactionality
Try to extinguish the popAtomic(function () {}) anti-pattern.
Change-Id: Ic09b8d83e34d0ccd470bd46801fd14a3c8b956cc
---
M Core/DataStores/QueueConsumer.php
M Tests/QueueConsumerTest.php
2 files changed, 8 insertions(+), 8 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/SmashPig
refs/changes/53/300053/1
diff --git a/Core/DataStores/QueueConsumer.php
b/Core/DataStores/QueueConsumer.php
index 95fcb9f..7dda159 100644
--- a/Core/DataStores/QueueConsumer.php
+++ b/Core/DataStores/QueueConsumer.php
@@ -100,6 +100,7 @@
public function dequeueMessages() {
$startTime = time();
$processed = 0;
+ // FIXME: Use a single code path
if ( $this->damagedQueue ) {
$realCallback = array( $this, 'wrappedCallback' );
} else {
diff --git a/Tests/QueueConsumerTest.php b/Tests/QueueConsumerTest.php
index a8a0988..b7fbe93 100644
--- a/Tests/QueueConsumerTest.php
+++ b/Tests/QueueConsumerTest.php
@@ -40,8 +40,7 @@
$count = $consumer->dequeueMessages();
$this->assertEquals( 1, $count, 'Should report 1 message
processed' );
$this->assertEquals( array( $payload ), $processed, 'Bad
message' );
- $this->assertNull(
- $this->queue->popAtomic( function( $unused ) {} ),
+ $this->assertNull( $this->queue->pop(),
'Should delete message when processing is successful'
);
}
@@ -69,7 +68,7 @@
$this->assertTrue( $ran, 'Callback was not called' );
$this->assertEquals(
$payload,
- $this->queue->popAtomic( function( $unused ) {} ),
+ $this->queue->pop(),
'Should not delete message when exception is thrown'
);
}
@@ -103,11 +102,11 @@
$this->assertTrue( $ran, 'Callback was not called' );
$this->assertEquals(
$payload,
- $damagedQueue->popAtomic( function( $unused ) {} ),
+ $damagedQueue->pop(),
'Should move message to damaged queue when exception is
thrown'
);
$this->assertNull(
- $this->queue->popAtomic( function( $unused ) {} ),
+ $this->queue->pop(),
'Should delete message on exception when damaged queue
exists'
);
}
@@ -137,7 +136,7 @@
}
$this->assertEquals(
$messages[3],
- $this->queue->popAtomic( function( $unused ) {} ),
+ $this->queue->pop(),
'Messed with too many messages'
);
}
@@ -177,13 +176,13 @@
$this->assertEquals( $messages[$i],
$processedMessages[$i], 'Message mutated' );
$this->assertEquals(
$messages[$i],
- $damagedQueue->popAtomic( function( $unused )
{} ),
+ $damagedQueue->pop(),
'Should move message to damaged queue when
exception is thrown'
);
}
$this->assertEquals(
$messages[3],
- $this->queue->popAtomic( function( $unused ) {} ),
+ $this->queue->pop(),
'message 4 should be at the head of the queue'
);
}
--
To view, visit https://gerrit.wikimedia.org/r/300053
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic09b8d83e34d0ccd470bd46801fd14a3c8b956cc
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/SmashPig
Gerrit-Branch: master
Gerrit-Owner: Awight <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
