Skip to content

Commit

Permalink
Merge pull request #24452 from totten/master-queue-stats
Browse files Browse the repository at this point in the history
CRM_Queue_Queue - Add 'getStatistic($name)'. Deprecate 'numberOfItems()'
  • Loading branch information
eileenmcnaughton authored Sep 4, 2022
2 parents 96e0523 + 3c85f40 commit 180ae18
Show file tree
Hide file tree
Showing 10 changed files with 188 additions and 95 deletions.
2 changes: 1 addition & 1 deletion CRM/Core/BAO/UserJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public static function on_civi_queue_check(\Civi\Core\Event\GenericHookEvent $e)
/** @var \CRM_Queue_Queue $queue */
$queue = $e->queue;
$userJobId = static::findUserJobId($queue->getName());
if ($userJobId && $queue->numberOfItems() < 1) {
if ($userJobId && $queue->getStatistic('total') < 1) {
$queue->setStatus('completed');
}
}
Expand Down
24 changes: 22 additions & 2 deletions CRM/Queue/Queue.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,28 @@ abstract public function createItem($data, $options = []);
* Determine number of items remaining in the queue.
*
* @return int
*/
abstract public function numberOfItems();
* @deprecated
* Use `getStatistic(string $name)` instead.
* The definition of `numberOfItems()` has become conflicted among different subclasses.
*/
public function numberOfItems() {
// This is the statistic traditionally reported by core queue implementations.
// However, it may not be as useful, and subclasses may have different interpretations.
return $this->getStatistic('total');
}

/**
* Get summary information about items in the queue.
*
* @param string $name
* The desired statistic. Ex:
* - 'ready': The number of items ready for execution (not currently claimed, not scheduled for future).
* - 'blocked': The number of items that may be runnable in the future, but cannot be run right now.
* - 'total': The total number of items known to the queue, regardless of whether their current status.
* @return int|float|null
* The value of the statistic, or NULL if the queue backend does not unsupport this statistic.
*/
abstract public function getStatistic(string $name);

/**
* Get the next item.
Expand Down
33 changes: 28 additions & 5 deletions CRM/Queue/Queue/Memory.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,35 @@ public function createItem($data, $options = []) {
}

/**
* Determine number of items remaining in the queue.
*
* @return int
* @param string $name
* @return int|float|null
* @see \CRM_Queue_Queue::getStatistic()
*/
public function numberOfItems() {
return count($this->items);
public function getStatistic(string $name) {
$ready = function(): int {
$now = CRM_Utils_Time::time();
$ready = 0;
foreach ($this->items as $id => $item) {
if (empty($this->releaseTimes[$id]) || $this->releaseTimes[$id] <= $now) {
$ready++;
}
}
return $ready;
};

switch ($name) {
case 'ready':
return $ready();

case 'blocked':
return count($this->items) - $ready();

case 'total':
return count($this->items);

default:
return NULL;
}
}

/**
Expand Down
36 changes: 24 additions & 12 deletions CRM/Queue/Queue/SqlTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,34 @@ public function deleteQueue() {
* @return bool
*/
public function existsQueue() {
return ($this->numberOfItems() > 0);
return ($this->getStatistic('total') > 0);
}

/**
* Determine number of items remaining in the queue.
*
* @return int
* @param string $name
* @return int|float|null
* @see \CRM_Queue_Queue::getStatistic()
*/
public function numberOfItems() {
return CRM_Core_DAO::singleValueQuery("
SELECT count(*)
FROM civicrm_queue_item
WHERE queue_name = %1
", [
1 => [$this->getName(), 'String'],
]);
public function getStatistic(string $name) {
switch ($name) {
case 'ready':
return (int) CRM_Core_DAO::singleValueQuery(
'SELECT count(*) FROM civicrm_queue_item WHERE queue_name = %1 AND (release_time is null OR release_time <= FROM_UNIXTIME(%2))',
[1 => [$this->getName(), 'String'], 2 => [CRM_Utils_Time::time(), 'Int']]);

case 'blocked':
return (int) CRM_Core_DAO::singleValueQuery(
'SELECT count(*) FROM civicrm_queue_item WHERE queue_name = %1 AND release_time > FROM_UNIXTIME(%2)',
[1 => [$this->getName(), 'String'], 2 => [CRM_Utils_Time::time(), 'Int']]);

case 'total':
return (int) CRM_Core_DAO::singleValueQuery(
'SELECT count(*) FROM civicrm_queue_item WHERE queue_name = %1',
[1 => [$this->getName(), 'String']]);

default:
return NULL;
}
}

/**
Expand Down
20 changes: 20 additions & 0 deletions Civi/Test/QueueTestTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace Civi\Test;

/**
* Helper functions for testing queues.
*/
trait QueueTestTrait {

protected function assertQueueStats(int $total, int $ready, int $blocked, \CRM_Queue_Queue $queue) {
$format = 'total=%d ready=%d blocked=%d';
$expect = [$total, $ready, $blocked];
$actual = [$queue->getStatistic('total'), $queue->getStatistic('ready'), $queue->getStatistic('blocked')];
$this->assertEquals(sprintf($format, ...$expect), sprintf($format, ...$actual));

// Deprecated - but checking for continuity.
$this->assertEquals($total, $queue->numberOfItems());
}

}
2 changes: 1 addition & 1 deletion templates/CRM/Queue/Page/Runner.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ CRM.$(function($) {
if (!data.is_error) {
queueRunnerData.completed++;
}
if (data.numberOfItems) {
if ('numberOfItems' in data && data.numberOfItems !== null) {
queueRunnerData.numberOfItems = parseInt(data.numberOfItems);
}

Expand Down
16 changes: 9 additions & 7 deletions tests/phpunit/CRM/Queue/Queue/SqlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
class CRM_Queue_Queue_SqlTest extends CiviUnitTestCase {

use \Civi\Test\QueueTestTrait;

/* ----------------------- Queue providers ----------------------- */

/* Define a list of queue providers which should be tested */
Expand Down Expand Up @@ -73,12 +75,12 @@ public function testPriorities($queueSpec) {
'test-key' => 'c',
]);

$this->assertEquals(3, $this->queue->numberOfItems());
$this->assertQueueStats(3, 3, 0, $this->queue);
$item = $this->queue->claimItem();
$this->assertEquals('a', $item->data['test-key']);
$this->queue->deleteItem($item);

$this->assertEquals(2, $this->queue->numberOfItems());
$this->assertQueueStats(2, 2, 0, $this->queue);
$item = $this->queue->claimItem();
$this->assertEquals('b', $item->data['test-key']);
$this->queue->deleteItem($item);
Expand All @@ -103,27 +105,27 @@ public function testPriorities($queueSpec) {
'test-key' => 'd',
]);

$this->assertEquals(4, $this->queue->numberOfItems());
$this->assertQueueStats(4, 4, 0, $this->queue);
$item = $this->queue->claimItem();
$this->assertEquals('start', $item->data['test-key']);
$this->queue->deleteItem($item);

$this->assertEquals(3, $this->queue->numberOfItems());
$this->assertQueueStats(3, 3, 0, $this->queue);
$item = $this->queue->claimItem();
$this->assertEquals('c', $item->data['test-key']);
$this->queue->deleteItem($item);

$this->assertEquals(2, $this->queue->numberOfItems());
$this->assertQueueStats(2, 2, 0, $this->queue);
$item = $this->queue->claimItem();
$this->assertEquals('d', $item->data['test-key']);
$this->queue->deleteItem($item);

$this->assertEquals(1, $this->queue->numberOfItems());
$this->assertQueueStats(1, 1, 0, $this->queue);
$item = $this->queue->claimItem();
$this->assertEquals('end', $item->data['test-key']);
$this->queue->deleteItem($item);

$this->assertEquals(0, $this->queue->numberOfItems());
$this->assertQueueStats(0, 0, 0, $this->queue);
}

}
Loading

0 comments on commit 180ae18

Please sign in to comment.