[Messenger] fix wrong use of generator returns

Tobion · Tobion · commit 50c9c8af6cb0 · 2019-05-06T11:28:40.000+02:00
And some other minor cleanups
diff --git a/src/Symfony/Component/Messenger/Stamp/SentToFailureTransportStamp.php b/src/Symfony/Component/Messenger/Stamp/SentToFailureTransportStamp.php
@@ -32,7 +32,7 @@ public function __construct(string $exceptionMessage, string $originalReceiverNa
         $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 @@ public function getFlattenException(): ?FlattenException
         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
@@ -28,6 +28,6 @@ public function testGetters()
         $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/Transport/AmqpExt/AmqpReceiver.php b/src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpReceiver.php
@@ -57,7 +57,7 @@ private function getEnvelope(string $queueName): iterable
         }
 
         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
@@ -84,12 +84,12 @@ public function getMessageCount(): int
         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
@@ -54,7 +54,7 @@ public function get(): iterable
             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
@@ -1,5 +1,14 @@
 <?php
 
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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
@@ -1,5 +1,14 @@
 <?php
 
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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 @@ public function get(): iterable
 
         $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
@@ -57,7 +57,7 @@ public function get(): iterable
             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
@@ -76,12 +76,12 @@ public function setup(): void
         $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
@@ -136,16 +136,11 @@ private function handleMessage(Envelope $envelope, ReceiverInterface $receiver,
                 $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 @@ private function dispatchEvent($event)
         $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
@@ -59,7 +59,7 @@ public function stop(): void
         $this->decoratedWorker->stop();
     }
 
-    private function shouldRestart(float $workerStartedAt)
+    private function shouldRestart(float $workerStartedAt): bool
     {
         $cacheItem = $this->cachePool->getItem(self::RESTART_REQUESTED_TIMESTAMP_KEY);
 

Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ public function __construct(string $exceptionMessage, string $originalReceiverNa`
`32`	`32`	`$this->exceptionMessage = $exceptionMessage;`
`33`	`33`	`$this->originalReceiverName = $originalReceiverName;`
`34`	`34`	`$this->flattenException = $flattenException;`
`35`		`- $this->sentAt = new \DateTime();`
	`35`	`+ $this->sentAt = new \DateTimeImmutable();`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`public function getExceptionMessage(): string`
`@@ -50,7 +50,7 @@ public function getFlattenException(): ?FlattenException`
`50`	`50`	`return $this->flattenException;`
`51`	`51`	`}`
`52`	`52`
`53`		`- public function getSentAt(): \DateTime`
	`53`	`+ public function getSentAt(): \DateTimeInterface`
`54`	`54`	`{`
`55`	`55`	`return $this->sentAt;`
`56`	`56`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,6 @@ public function testGetters()`
`28`	`28`	`$this->assertSame('exception message', $stamp->getExceptionMessage());`
`29`	`29`	`$this->assertSame('original_receiver', $stamp->getOriginalReceiverName());`
`30`	`30`	`$this->assertSame($flattenException, $stamp->getFlattenException());`
`31`		`- $this->assertInstanceOf(\DateTime::class, $stamp->getSentAt());`
	`31`	`+ $this->assertInstanceOf(\DateTimeInterface::class, $stamp->getSentAt());`
`32`	`32`	`}`
`33`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ private function getEnvelope(string $queueName): iterable`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`if (null === $amqpEnvelope) {`
`60`		`- return [];`
	`60`	`+ return;`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`try {`
Original file line number	Diff line number	Diff line change
`@@ -84,12 +84,12 @@ public function getMessageCount(): int`
`84`	`84`	`return ($this->receiver ?? $this->getReceiver())->getMessageCount();`
`85`	`85`	`}`
`86`	`86`
`87`		`- private function getReceiver()`
	`87`	`+ private function getReceiver(): AmqpReceiver`
`88`	`88`	`{`
`89`	`89`	`return $this->receiver = new AmqpReceiver($this->connection, $this->serializer);`
`90`	`90`	`}`
`91`	`91`
`92`		`- private function getSender()`
	`92`	`+ private function getSender(): AmqpSender`
`93`	`93`	`{`
`94`	`94`	`return $this->sender = new AmqpSender($this->connection, $this->serializer);`
`95`	`95`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ public function get(): iterable`
`54`	`54`	`return [];`
`55`	`55`	`}`
`56`	`56`
`57`		`- yield $this->createEnvelopeFromData($doctrineEnvelope);`
	`57`	`+ return [$this->createEnvelopeFromData($doctrineEnvelope)];`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ public function get(): iterable`
`57`	`57`	`throw $exception;`
`58`	`58`	`}`
`59`	`59`
`60`		`- yield $envelope->with(new RedisReceivedStamp($redisEnvelope['id']));`
	`60`	`+ return [$envelope->with(new RedisReceivedStamp($redisEnvelope['id']))];`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -76,12 +76,12 @@ public function setup(): void`
`76`	`76`	`$this->connection->setup();`
`77`	`77`	`}`
`78`	`78`
`79`		`- private function getReceiver()`
	`79`	`+ private function getReceiver(): RedisReceiver`
`80`	`80`	`{`
`81`	`81`	`return $this->receiver = new RedisReceiver($this->connection, $this->serializer);`
`82`	`82`	`}`
`83`	`83`
`84`		`- private function getSender()`
	`84`	`+ private function getSender(): RedisSender`
`85`	`85`	`{`
`86`	`86`	`return $this->sender = new RedisSender($this->connection, $this->serializer);`
`87`	`87`	`}`