From e8a09e9d851e9466272ae32526be21845aa0b385 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Mon, 6 May 2019 04:31:50 +0200 Subject: [PATCH] [Messenger] fix wrong use of generator returns And some other minor cleanups --- .../Messenger/Stamp/SentToFailureTransportStamp.php | 4 ++-- .../Tests/Stamp/SentToFailureTransportStampTest.php | 2 +- .../Transport/Doctrine/DoctrineReceiverTest.php | 4 ++-- .../Transport/Doctrine/DoctrineTransportTest.php | 2 +- .../Receiver/SingleMessageReceiverTest.php | 8 ++------ .../Tests/Transport/RedisExt/RedisReceiverTest.php | 4 ++-- .../Tests/Transport/RedisExt/RedisTransportTest.php | 2 +- .../Messenger/Transport/AmqpExt/AmqpReceiver.php | 2 +- .../Messenger/Transport/AmqpExt/AmqpTransport.php | 4 ++-- .../Transport/Doctrine/DoctrineReceiver.php | 2 +- .../Receiver/ListableReceiverInterface.php | 9 +++++++++ .../Transport/Receiver/SingleMessageReceiver.php | 11 ++++++++++- .../Messenger/Transport/RedisExt/RedisReceiver.php | 2 +- .../Messenger/Transport/RedisExt/RedisTransport.php | 4 ++-- src/Symfony/Component/Messenger/Worker.php | 13 ++----------- .../Worker/StopWhenRestartSignalIsReceived.php | 2 +- 16 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/Symfony/Component/Messenger/Stamp/SentToFailureTransportStamp.php b/src/Symfony/Component/Messenger/Stamp/SentToFailureTransportStamp.php index 05a5dfcacf..222aa6a18d 100644 --- a/src/Symfony/Component/Messenger/Stamp/SentToFailureTransportStamp.php +++ b/src/Symfony/Component/Messenger/Stamp/SentToFailureTransportStamp.php @@ -32,7 +32,7 @@ class SentToFailureTransportStamp implements StampInterface $this->exceptionMessage = $exceptionMessage; $this->originalReceiverName = $originalReceiverName; $this->flattenException = $flattenException; - $this->sentAt = new \DateTime(); + $this->sentAt = new \DateTimeImmutable(); } public function getExceptionMessage(): string @@ -50,7 +50,7 @@ class SentToFailureTransportStamp implements StampInterface return $this->flattenException; } - public function getSentAt(): \DateTime + public function getSentAt(): \DateTimeInterface { return $this->sentAt; } diff --git a/src/Symfony/Component/Messenger/Tests/Stamp/SentToFailureTransportStampTest.php b/src/Symfony/Component/Messenger/Tests/Stamp/SentToFailureTransportStampTest.php index 7639060daa..3b9579a646 100644 --- a/src/Symfony/Component/Messenger/Tests/Stamp/SentToFailureTransportStampTest.php +++ b/src/Symfony/Component/Messenger/Tests/Stamp/SentToFailureTransportStampTest.php @@ -28,6 +28,6 @@ class SentToFailureTransportStampTest extends TestCase $this->assertSame('exception message', $stamp->getExceptionMessage()); $this->assertSame('original_receiver', $stamp->getOriginalReceiverName()); $this->assertSame($flattenException, $stamp->getFlattenException()); - $this->assertInstanceOf(\DateTime::class, $stamp->getSentAt()); + $this->assertInstanceOf(\DateTimeInterface::class, $stamp->getSentAt()); } } diff --git a/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineReceiverTest.php b/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineReceiverTest.php index 79c99d5be3..3a250fddbc 100644 --- a/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineReceiverTest.php +++ b/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineReceiverTest.php @@ -36,7 +36,7 @@ class DoctrineReceiverTest extends TestCase $connection->method('get')->willReturn($doctrineEnvelope); $receiver = new DoctrineReceiver($connection, $serializer); - $actualEnvelopes = iterator_to_array($receiver->get()); + $actualEnvelopes = $receiver->get(); $this->assertCount(1, $actualEnvelopes); /** @var Envelope $actualEnvelope */ $actualEnvelope = $actualEnvelopes[0]; @@ -67,7 +67,7 @@ class DoctrineReceiverTest extends TestCase $connection->expects($this->once())->method('reject'); $receiver = new DoctrineReceiver($connection, $serializer); - iterator_to_array($receiver->get()); + $receiver->get(); } public function testAll() diff --git a/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineTransportTest.php b/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineTransportTest.php index 145397f889..ad9f9dba61 100644 --- a/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineTransportTest.php +++ b/src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineTransportTest.php @@ -46,7 +46,7 @@ class DoctrineTransportTest extends TestCase $serializer->method('decode')->with(['body' => 'body', 'headers' => ['my' => 'header']])->willReturn(new Envelope($decodedMessage)); $connection->method('get')->willReturn($doctrineEnvelope); - $envelopes = iterator_to_array($transport->get()); + $envelopes = $transport->get(); $this->assertSame($decodedMessage, $envelopes[0]->getMessage()); } diff --git a/src/Symfony/Component/Messenger/Tests/Transport/Receiver/SingleMessageReceiverTest.php b/src/Symfony/Component/Messenger/Tests/Transport/Receiver/SingleMessageReceiverTest.php index a900e7ba0f..e8f5ef4ce1 100644 --- a/src/Symfony/Component/Messenger/Tests/Transport/Receiver/SingleMessageReceiverTest.php +++ b/src/Symfony/Component/Messenger/Tests/Transport/Receiver/SingleMessageReceiverTest.php @@ -13,12 +13,8 @@ namespace Symfony\Component\Messenger\Tests\Transport\Sender; use PHPUnit\Framework\TestCase; use Symfony\Component\Messenger\Envelope; -use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage; -use Symfony\Component\Messenger\Tests\Fixtures\SecondMessage; use Symfony\Component\Messenger\Transport\Receiver\ReceiverInterface; use Symfony\Component\Messenger\Transport\Receiver\SingleMessageReceiver; -use Symfony\Component\Messenger\Transport\Sender\SenderInterface; -use Symfony\Component\Messenger\Transport\Sender\SendersLocator; class SingleMessageReceiverTest extends TestCase { @@ -28,11 +24,11 @@ class SingleMessageReceiverTest extends TestCase $envelope = new Envelope(new \stdClass()); $receiver = new SingleMessageReceiver($innerReceiver, $envelope); - $received = \iterator_to_array($receiver->get()); + $received = $receiver->get(); $this->assertCount(1, $received); $this->assertSame($received[0], $envelope); - $this->assertEmpty(\iterator_to_array($receiver->get())); + $this->assertEmpty($receiver->get()); } public function testCallsAreForwarded() diff --git a/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisReceiverTest.php b/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisReceiverTest.php index c3bb532239..28c299258c 100644 --- a/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisReceiverTest.php +++ b/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisReceiverTest.php @@ -33,7 +33,7 @@ class RedisReceiverTest extends TestCase $connection->method('get')->willReturn($redisEnvelop); $receiver = new RedisReceiver($connection, $serializer); - $actualEnvelopes = iterator_to_array($receiver->get()); + $actualEnvelopes = $receiver->get(); $this->assertCount(1, $actualEnvelopes); $this->assertEquals(new DummyMessage('Hi'), $actualEnvelopes[0]->getMessage()); } @@ -51,7 +51,7 @@ class RedisReceiverTest extends TestCase $connection->expects($this->once())->method('reject'); $receiver = new RedisReceiver($connection, $serializer); - iterator_to_array($receiver->get()); + $receiver->get(); } private function createRedisEnvelope() diff --git a/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisTransportTest.php b/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisTransportTest.php index 0c83e6be88..fce73e50d3 100644 --- a/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisTransportTest.php +++ b/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisTransportTest.php @@ -46,7 +46,7 @@ class RedisTransportTest extends TestCase $serializer->method('decode')->with(['body' => 'body', 'headers' => ['my' => 'header']])->willReturn(new Envelope($decodedMessage)); $connection->method('get')->willReturn($redisEnvelope); - $envelopes = iterator_to_array($transport->get()); + $envelopes = $transport->get(); $this->assertSame($decodedMessage, $envelopes[0]->getMessage()); } diff --git a/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpReceiver.php b/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpReceiver.php index d2d03c859c..db2f1202f9 100644 --- a/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpReceiver.php +++ b/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpReceiver.php @@ -57,7 +57,7 @@ class AmqpReceiver implements ReceiverInterface, MessageCountAwareInterface } if (null === $amqpEnvelope) { - return []; + return; } try { diff --git a/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpTransport.php b/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpTransport.php index 91c9fe45c0..242a138230 100644 --- a/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpTransport.php +++ b/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpTransport.php @@ -84,12 +84,12 @@ class AmqpTransport implements TransportInterface, SetupableTransportInterface, return ($this->receiver ?? $this->getReceiver())->getMessageCount(); } - private function getReceiver() + private function getReceiver(): AmqpReceiver { return $this->receiver = new AmqpReceiver($this->connection, $this->serializer); } - private function getSender() + private function getSender(): AmqpSender { return $this->sender = new AmqpSender($this->connection, $this->serializer); } diff --git a/src/Symfony/Component/Messenger/Transport/Doctrine/DoctrineReceiver.php b/src/Symfony/Component/Messenger/Transport/Doctrine/DoctrineReceiver.php index aad8cf236d..a6a41e8c79 100644 --- a/src/Symfony/Component/Messenger/Transport/Doctrine/DoctrineReceiver.php +++ b/src/Symfony/Component/Messenger/Transport/Doctrine/DoctrineReceiver.php @@ -54,7 +54,7 @@ class DoctrineReceiver implements ReceiverInterface, MessageCountAwareInterface, return []; } - yield $this->createEnvelopeFromData($doctrineEnvelope); + return [$this->createEnvelopeFromData($doctrineEnvelope)]; } /** diff --git a/src/Symfony/Component/Messenger/Transport/Receiver/ListableReceiverInterface.php b/src/Symfony/Component/Messenger/Transport/Receiver/ListableReceiverInterface.php index b4c19295a9..ff16a4d436 100644 --- a/src/Symfony/Component/Messenger/Transport/Receiver/ListableReceiverInterface.php +++ b/src/Symfony/Component/Messenger/Transport/Receiver/ListableReceiverInterface.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Symfony\Component\Messenger\Transport\Receiver; use Symfony\Component\Messenger\Envelope; diff --git a/src/Symfony/Component/Messenger/Transport/Receiver/SingleMessageReceiver.php b/src/Symfony/Component/Messenger/Transport/Receiver/SingleMessageReceiver.php index 9aec49d6c0..56a1399518 100644 --- a/src/Symfony/Component/Messenger/Transport/Receiver/SingleMessageReceiver.php +++ b/src/Symfony/Component/Messenger/Transport/Receiver/SingleMessageReceiver.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Symfony\Component\Messenger\Transport\Receiver; use Symfony\Component\Messenger\Envelope; @@ -32,7 +41,7 @@ class SingleMessageReceiver implements ReceiverInterface $this->hasReceived = true; - yield $this->envelope; + return [$this->envelope]; } public function ack(Envelope $envelope): void diff --git a/src/Symfony/Component/Messenger/Transport/RedisExt/RedisReceiver.php b/src/Symfony/Component/Messenger/Transport/RedisExt/RedisReceiver.php index 8ff60354b9..fe18491f6d 100644 --- a/src/Symfony/Component/Messenger/Transport/RedisExt/RedisReceiver.php +++ b/src/Symfony/Component/Messenger/Transport/RedisExt/RedisReceiver.php @@ -57,7 +57,7 @@ class RedisReceiver implements ReceiverInterface throw $exception; } - yield $envelope->with(new RedisReceivedStamp($redisEnvelope['id'])); + return [$envelope->with(new RedisReceivedStamp($redisEnvelope['id']))]; } /** diff --git a/src/Symfony/Component/Messenger/Transport/RedisExt/RedisTransport.php b/src/Symfony/Component/Messenger/Transport/RedisExt/RedisTransport.php index 3af4e94233..7ce75e7127 100644 --- a/src/Symfony/Component/Messenger/Transport/RedisExt/RedisTransport.php +++ b/src/Symfony/Component/Messenger/Transport/RedisExt/RedisTransport.php @@ -76,12 +76,12 @@ class RedisTransport implements TransportInterface, SetupableTransportInterface $this->connection->setup(); } - private function getReceiver() + private function getReceiver(): RedisReceiver { return $this->receiver = new RedisReceiver($this->connection, $this->serializer); } - private function getSender() + private function getSender(): RedisSender { return $this->sender = new RedisSender($this->connection, $this->serializer); } diff --git a/src/Symfony/Component/Messenger/Worker.php b/src/Symfony/Component/Messenger/Worker.php index 44ff30a729..4018755ff6 100644 --- a/src/Symfony/Component/Messenger/Worker.php +++ b/src/Symfony/Component/Messenger/Worker.php @@ -136,16 +136,11 @@ class Worker implements WorkerInterface $envelope = $throwable->getEnvelope(); } - $shouldRetry = $this->shouldRetry($throwable, $envelope, $retryStrategy); + $shouldRetry = $retryStrategy && $this->shouldRetry($throwable, $envelope, $retryStrategy); $this->dispatchEvent(new WorkerMessageFailedEvent($envelope, $transportName, $throwable, $shouldRetry)); if ($shouldRetry) { - if (null === $retryStrategy) { - // not logically allowed, but check just in case - throw new LogicException('Retrying is not supported without a retry strategy.'); - } - $retryCount = $this->getRetryCount($envelope) + 1; if (null !== $this->logger) { $this->logger->error('Retrying {class} - retry #{retryCount}.', $context + ['retryCount' => $retryCount, 'error' => $throwable]); @@ -194,16 +189,12 @@ class Worker implements WorkerInterface $this->eventDispatcher->dispatch($event); } - private function shouldRetry(\Throwable $e, Envelope $envelope, ?RetryStrategyInterface $retryStrategy): bool + private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInterface $retryStrategy): bool { if ($e instanceof UnrecoverableMessageHandlingException) { return false; } - if (null === $retryStrategy) { - return false; - } - $sentStamp = $envelope->last(SentStamp::class); if (null === $sentStamp) { if (null !== $this->logger) { diff --git a/src/Symfony/Component/Messenger/Worker/StopWhenRestartSignalIsReceived.php b/src/Symfony/Component/Messenger/Worker/StopWhenRestartSignalIsReceived.php index 29b52c87da..1958d4ecc8 100644 --- a/src/Symfony/Component/Messenger/Worker/StopWhenRestartSignalIsReceived.php +++ b/src/Symfony/Component/Messenger/Worker/StopWhenRestartSignalIsReceived.php @@ -59,7 +59,7 @@ class StopWhenRestartSignalIsReceived implements WorkerInterface $this->decoratedWorker->stop(); } - private function shouldRestart(float $workerStartedAt) + private function shouldRestart(float $workerStartedAt): bool { $cacheItem = $this->cachePool->getItem(self::RESTART_REQUESTED_TIMESTAMP_KEY);