From 46b0ab55c1dadbfb869f5b7f841e91429df64fdd Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Thu, 11 Jul 2024 15:06:16 -0500 Subject: [PATCH] Return More Information from Consumer::once Via a result object. this is to make some telemetry stuff easier. --- src/Consumer.php | 5 +-- src/DefaultConsumer.php | 4 +- src/OnceResult.php | 34 ++++++++++++++++ test/unit/AbstractConsumerTest.php | 8 ++-- test/unit/DefaultConsumerTest.php | 39 ++++++++++++++----- test/unit/Handler/PcntlForkingHandlerTest.php | 1 + 6 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 src/OnceResult.php diff --git a/src/Consumer.php b/src/Consumer.php index aded841..48d1418 100644 --- a/src/Consumer.php +++ b/src/Consumer.php @@ -44,10 +44,9 @@ public function run(string $queueName, MessageLifecycle $lifecycle=null) : int; * stop execption indicating a graceful stop is necessary * @throws Exception\DriverError|Exception if anything goes wrong with the * underlying driver itself. - * @return boolean|null True if the a job was execute successfully. Null if - * no job was executed. See the logs. + * @return OnceResult|null a result object if a message was recieved and handled, null otherwise */ - public function once(string $queueName, MessageLifecycle $lifecycle=null) : ?bool; + public function once(string $queueName, MessageLifecycle $lifecycle=null) : ?OnceResult; /** * Gracefully stop the consumer with the given exit code. diff --git a/src/DefaultConsumer.php b/src/DefaultConsumer.php index f09727e..d4632f1 100644 --- a/src/DefaultConsumer.php +++ b/src/DefaultConsumer.php @@ -66,7 +66,7 @@ public function __construct( /** * {@inheritdoc} */ - public function once(string $queueName, MessageLifecycle $lifecycle=null) : ?bool + public function once(string $queueName, MessageLifecycle $lifecycle=null) : ?OnceResult { $envelope = $this->driver->dequeue($queueName); if (!$envelope) { @@ -90,7 +90,7 @@ public function once(string $queueName, MessageLifecycle $lifecycle=null) : ?boo $lifecycle->failed($message, $this); } - return $succeeded; + return new OnceResult($succeeded, $willRetry, $message); } /** diff --git a/src/OnceResult.php b/src/OnceResult.php new file mode 100644 index 0000000..7b6f2b0 --- /dev/null +++ b/src/OnceResult.php @@ -0,0 +1,34 @@ + + * + * For full copyright information see the LICENSE file distributed + * with this source code. + * + * @license http://opensource.org/licenses/Apache-2.0 Apache-2.0 + */ + +namespace PMG\Queue; + +final readonly class OnceResult +{ + public function __construct( + /** + * Whether or not the `once` method ran successfully -- eg a message + * was handled without an error. + */ + public bool $successful, + /** + * True if the message has been put back in the queue to retry + */ + public bool $retrying, + /** + * The message that was processed + */ + public object $message, + ) { + } +} diff --git a/test/unit/AbstractConsumerTest.php b/test/unit/AbstractConsumerTest.php index 7eab072..5e8aa91 100644 --- a/test/unit/AbstractConsumerTest.php +++ b/test/unit/AbstractConsumerTest.php @@ -31,7 +31,7 @@ public function testRunConsumesMessagesUntilConsumerIsStopped() ->method('once') ->with(self::Q) ->will($this->onConsecutiveCalls( - true, // successful :tada: + new OnceResult(true, false, $this->message), $this->throwException(new Exception\SimpleMustStop('oops', 1)) )); @@ -44,7 +44,7 @@ public function testRunStopsWhenADriverErrorIsThrown() ->method('once') ->with(self::Q) ->will($this->onConsecutiveCalls( - true, // successful :tada: + new OnceResult(true, false, $this->message), $this->throwException(new Exception\SerializationError('broke')) )); @@ -65,7 +65,7 @@ public function testRunStopsWhenAThrowableisCaught() ->method('once') ->with(self::Q) ->will($this->onConsecutiveCalls( - true, // successful :tada: + new OnceResult(true, false, $this->message), $this->throwException(new \Error('oops')) )); @@ -100,7 +100,7 @@ public function testRunPassesGivenMessageLifecycleToOnce() ->method('once') ->with(self::Q, $this->identicalTo($lifecycle)) ->will($this->onConsecutiveCalls( - true, // successful :tada: + new OnceResult(true, false, $this->message), $this->throwException(new Exception\SimpleMustStop('oops', 1)) )); diff --git a/test/unit/DefaultConsumerTest.php b/test/unit/DefaultConsumerTest.php index 54c4b29..b0c8fdb 100644 --- a/test/unit/DefaultConsumerTest.php +++ b/test/unit/DefaultConsumerTest.php @@ -68,7 +68,11 @@ public function testOnceExecutesTheMessageAndAcknowledgesIt() ->with($this->identicalTo($this->message)) ->willReturn(self::promise(true)); - $this->assertTrue($this->consumer->once(self::Q)); + $result = $this->consumer->once(self::Q); + + $this->assertTrue($result->successful); + $this->assertFalse($result->retrying); + $this->assertSame($this->message, $result->message); } public function testPlainObjectMessagesCanBeHandled() @@ -87,7 +91,11 @@ public function testPlainObjectMessagesCanBeHandled() ->with($this->identicalTo($message)) ->willReturn(self::promise(true)); - $this->assertTrue($this->consumer->once(self::Q)); + $result = $this->consumer->once(self::Q); + + $this->assertTrue($result->successful); + $this->assertFalse($result->retrying); + $this->assertSame($message, $result->message); } public function testOnceWithAFailedMessageAndValidRetryPutsTheMessageBackInTheQueue() @@ -102,7 +110,11 @@ public function testOnceWithAFailedMessageAndValidRetryPutsTheMessageBackInTheQu ->with($this->identicalTo($this->message)) ->willReturn(self::promise(false)); - $this->assertFalse($this->consumer->once(self::Q)); + $result = $this->consumer->once(self::Q); + + $this->assertFalse($result->successful); + $this->assertTrue($result->retrying); + $this->assertSame($this->message, $result->message); } public function testFailedMessageThatCannotBeRetriedIsNotPutBackInTheQueue() @@ -118,7 +130,11 @@ public function testFailedMessageThatCannotBeRetriedIsNotPutBackInTheQueue() ->with($this->identicalTo($this->message)) ->willReturn(self::promise(false)); - $this->assertFalse($this->consumer->once(self::Q)); + $result = $this->consumer->once(self::Q); + + $this->assertFalse($result->successful); + $this->assertFalse($result->retrying); + $this->assertSame($this->message, $result->message); } public function testOnceWithAExceptionThrownFromHandlerAndValidRetryRetriesJobAndThrows() @@ -133,9 +149,10 @@ public function testOnceWithAExceptionThrownFromHandlerAndValidRetryRetriesJobAn ->with($this->identicalTo($this->message)) ->willThrowException(new \Exception('oops')); - $this->assertFalse($this->consumer->once(self::Q)); - $messages = $this->logger->getMessages(LogLevel::CRITICAL); + $result = $this->consumer->once(self::Q); + $this->assertFalse($result->successful); + $messages = $this->logger->getMessages(LogLevel::CRITICAL); $this->assertCount(1, $messages); $this->assertStringContainsString('oops', $messages[0]); $this->assertStringContainsString('TestMessage', $messages[0]); @@ -168,6 +185,10 @@ public function testFailureWithShouldReleaseReleasesMessageBackIntoDriver() ->willThrowException(new Exception\ForkedProcessCancelled('oops')); $result = $this->consumer->once(self::Q); + + $this->assertFalse($result->successful); + $this->assertTrue($result->retrying); + $this->assertSame($this->message, $result->message); } /** @@ -194,7 +215,7 @@ public function testLifecycleOfSuccessfulMessageCallsExpectedLifecycleMethods() $result = $this->consumer->once(self::Q, $lifecycle); - $this->assertTrue($result); + $this->assertTrue($result->successful); } /** @@ -224,7 +245,7 @@ public function testLifecycleOnFailedMessageCallsExpectedLifecycleMethods() $result = $this->consumer->once(self::Q, $lifecycle); - $this->assertFalse($result); + $this->assertFalse($result->successful); } /** @@ -252,7 +273,7 @@ public function testLifecycleOnRetryingMessageCallsExpectedLifecycleMethods() $result = $this->consumer->once(self::Q, $lifecycle); - $this->assertFalse($result); + $this->assertFalse($result->successful); } /** diff --git a/test/unit/Handler/PcntlForkingHandlerTest.php b/test/unit/Handler/PcntlForkingHandlerTest.php index 5a9f391..2dda840 100644 --- a/test/unit/Handler/PcntlForkingHandlerTest.php +++ b/test/unit/Handler/PcntlForkingHandlerTest.php @@ -23,6 +23,7 @@ * This uses `CallableHandler` simply because I'm not sure how phpunit mock objects * behave in forked processes. * + * @group pcntl * @requires extension pcntl */ class PcntlForkingHandlerTest extends \PMG\Queue\UnitTestCase