Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!!! FEATURE: Extend the QueueInterface with countReserved and countFailed #29

Merged
merged 4 commits into from
May 30, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
FEATURE: Extend the QueueInterface with countReserved and countFailed
Also add countReady() and deprecate the count() command
to clearify the interface
  • Loading branch information
daniellienert committed May 28, 2018
commit 5a9ab5e35be7d97ab5174f2ffddaa1cc65e90a31
11 changes: 10 additions & 1 deletion Classes/Command/QueueCommandController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
* source code.
*/

use Flowpack\JobQueue\Common\Exception;
use Flowpack\JobQueue\Common\Queue\QueueManager;
use Neos\Flow\Annotations as Flow;
use Neos\Flow\Cli\CommandController;
use Neos\Flow\Mvc\Exception\StopActionException;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this namespace imported?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, it's needed for the @throws annotations below

use Neos\Utility\TypeHandling;

/**
Expand All @@ -40,14 +42,15 @@ class QueueCommandController extends CommandController
* Displays all configured queues, their type and the number of messages that are ready to be processed.
*
* @return void
* @throws Exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our CGL don't state anything about the requirement of @throws annotations. Personally I'm strongly against those.
Obviously no reason to block this, but if we decide to add the annotations we should do it for the whole package, and maybe in a separate PR

*/
public function listCommand()
{
$rows = [];
foreach ($this->queueConfigurations as $queueName => $queueConfiguration) {
$queue = $this->queueManager->getQueue($queueName);
try {
$numberOfMessages = $queue->count();
$numberOfMessages = $queue->countReady();
} catch (\Exception $e) {
$numberOfMessages = '-';
}
Expand All @@ -63,6 +66,7 @@ public function listCommand()
*
* @param string $queue Name of the queue to describe (e.g. "some-queue")
* @return void
* @throws Exception
*/
public function describeCommand($queue)
{
Expand All @@ -83,6 +87,8 @@ public function describeCommand($queue)
*
* @param string $queue Name of the queue to initialize (e.g. "some-queue")
* @return void
* @throws Exception
* @throws StopActionException
*/
public function setupCommand($queue)
{
Expand All @@ -106,6 +112,8 @@ public function setupCommand($queue)
* @param string $queue Name of the queue to flush (e.g. "some-queue")
* @param bool $force This flag is required in order to avoid accidental flushes
* @return void
* @throws Exception
* @throws StopActionException
*/
public function flushCommand($queue, $force = false)
{
Expand Down Expand Up @@ -134,6 +142,7 @@ public function flushCommand($queue, $force = false)
* @param string $payload Arbitrary payload, for example a serialized instance of a class implementing JobInterface
* @param string $options JSON encoded, for example '{"some-option": "some-value"}'
* @return void
* @throws Exception
*/
public function submitCommand($queue, $payload, $options = null)
{
Expand Down
26 changes: 25 additions & 1 deletion Classes/Queue/FakeQueue.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,31 @@ public function peek($limit = 1)
/**
* @inheritdoc
*/
public function count()
public function count():int
{
return $this->countReady();
}

/**
* @inheritdoc
*/
public function countReady(): int
{
return 0;
}

/**
* @inheritdoc
*/
public function countReserved(): int
{
return 0;
}

/**
* @inheritdoc
*/
public function countFailed(): int
{
return 0;
}
Expand Down
30 changes: 25 additions & 5 deletions Classes/Queue/QueueInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,33 @@ public function finish($messageId);
public function peek($limit = 1);

/**
* Count ready messages in the queue
* Get a count of ready messages currently in the queue.
*
* Get a count of messages currently in the queue.
* @return int The number of ready messages in the queue
* @deprecated Will be removed with the next major. Use countReady instead.
*/
public function count(): int;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the interface we don't need to keep deprecated methods IMO as the implementations need to be adjusted anyways


/**
* Get a count of ready messages currently in the queue.
*
* @return int The number of ready messages in the queue
*/
public function countReady(): int;

/**
* Get a count of reserved messages currently in the queue.
*
* @return int The number of reserved messages in the queue
*/
public function countReserved(): int;

/**
* Get a count of failed messages currently in the queue.
*
* @return integer The number of messages in the queue
* @return int The number of failed messages in the queue
*/
public function count();
public function countFailed(): int;

/**
* Removes all messages from this queue
Expand All @@ -119,4 +139,4 @@ public function count();
* @return void
*/
public function flush();
}
}
30 changes: 23 additions & 7 deletions Tests/Functional/AbstractQueueTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,10 @@ public function releasePutsMessageBackToQueue()
$messageId = $this->queue->submit('A message');

$this->queue->waitAndReserve(1);
$this->assertSame(0, $this->queue->count());
$this->assertSame(0, $this->queue->countReady());

$this->queue->release($messageId);
$this->assertSame(1, $this->queue->count());
$this->assertSame(1, $this->queue->countReady());
}

/**
Expand Down Expand Up @@ -179,16 +179,32 @@ public function abortRemovesMessageFromActiveQueue()
$this->queue->waitAndReserve(1);

$this->queue->abort($messageId);
$this->assertSame(0, $this->queue->count());
$this->assertSame(0, $this->queue->countReady());
$this->assertNull($this->queue->waitAndTake(1));
}

/**
* @test
*/
public function countReturnsZeroByDefault()
public function countReadyReturnsZeroByDefault()
{
$this->assertSame(0, $this->queue->count());
$this->assertSame(0, $this->queue->countReady());
}

/**
* @test
*/
public function countFailedReturnsZeroByDefault()
{
$this->assertSame(0, $this->queue->countFailed());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add tests effectively covering the new features:
countFailedReturnsNumberOfFailedMessages() and countReservedReturnsNumberOfReservedMessages()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added with bdffb4a


/**
* @test
*/
public function countReservedReturnsZeroByDefault()
{
$this->assertSame(0, $this->queue->countReserved());
}

/**
Expand All @@ -199,6 +215,6 @@ public function countReturnsNumberOfReadyJobs()
$this->queue->submit('First message');
$this->queue->submit('Second message');

$this->assertSame(2, $this->queue->count());
$this->assertSame(2, $this->queue->countReady());
}
}
}
33 changes: 31 additions & 2 deletions Tests/Unit/Fixtures/TestQueue.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,18 @@ class TestQueue implements QueueInterface
/**
* @var string[]
*/
protected $processingMessages = [];
protected $reservedMessages = [];

/**
* @var string[]
*/
protected $failedMessages = [];

/**
* @var string[]
*/
protected $processingMessages = [];

/**
* @var int[]
*/
Expand Down Expand Up @@ -205,11 +210,35 @@ public function peek($limit = 1)
/**
* @inheritdoc
*/
public function count()
public function count(): int
{
return $this->countReady();
}

/**
* @inheritdoc
*/
public function countReady():int
{
return count($this->readyMessages);
}

/**
* @inheritdoc
*/
public function countReserved(): int
{
return count($this->reservedMessages);
}

/**
* @inheritdoc
*/
public function countFailed(): int
{
return count($this->failedMessages);
}

/**
* @inheritdoc
*/
Expand Down