From 0034dee6412313d14938e04bb668742054ecd557 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 14 Jun 2019 22:19:29 +0200 Subject: [PATCH] [Messenger] make retry logic work without SentStamp --- .../Messenger/Tests/RetryIntegrationTest.php | 8 +++--- .../Component/Messenger/Tests/WorkerTest.php | 12 ++++----- src/Symfony/Component/Messenger/Worker.php | 26 +------------------ 3 files changed, 11 insertions(+), 35 deletions(-) diff --git a/src/Symfony/Component/Messenger/Tests/RetryIntegrationTest.php b/src/Symfony/Component/Messenger/Tests/RetryIntegrationTest.php index 64ff20bae3..2447a0c82b 100644 --- a/src/Symfony/Component/Messenger/Tests/RetryIntegrationTest.php +++ b/src/Symfony/Component/Messenger/Tests/RetryIntegrationTest.php @@ -34,9 +34,9 @@ class RetryIntegrationTest extends TestCase $senderAndReceiver = new DummySenderAndReceiver(); $senderLocator = $this->createMock(ContainerInterface::class); - $senderLocator->method('has')->with('sender_alias')->willReturn(true); - $senderLocator->method('get')->with('sender_alias')->willReturn($senderAndReceiver); - $senderLocator = new SendersLocator([DummyMessage::class => ['sender_alias']], $senderLocator); + $senderLocator->method('has')->with('transportName')->willReturn(true); + $senderLocator->method('get')->with('transportName')->willReturn($senderAndReceiver); + $senderLocator = new SendersLocator([DummyMessage::class => ['transportName']], $senderLocator); $handler = new DummyMessageHandlerFailingFirstTimes(0); $throwingHandler = new DummyMessageHandlerFailingFirstTimes(1); @@ -52,7 +52,7 @@ class RetryIntegrationTest extends TestCase $envelope = new Envelope(new DummyMessage('API')); $bus->dispatch($envelope); - $worker = new Worker(['receiverName' => $senderAndReceiver], $bus, ['receiverName' => new MultiplierRetryStrategy()]); + $worker = new Worker(['transportName' => $senderAndReceiver], $bus, ['transportName' => new MultiplierRetryStrategy()]); $worker->run([], function (?Envelope $envelope) use ($worker) { if (null === $envelope) { $worker->stop(); diff --git a/src/Symfony/Component/Messenger/Tests/WorkerTest.php b/src/Symfony/Component/Messenger/Tests/WorkerTest.php index 46e8149f6c..c82652fe1d 100644 --- a/src/Symfony/Component/Messenger/Tests/WorkerTest.php +++ b/src/Symfony/Component/Messenger/Tests/WorkerTest.php @@ -84,7 +84,7 @@ class WorkerTest extends TestCase public function testDispatchCausesRetry() { $receiver = new DummyReceiver([ - [new Envelope(new DummyMessage('Hello'), [new SentStamp('Some\Sender', 'sender_alias')])], + [new Envelope(new DummyMessage('Hello'), [new SentStamp('Some\Sender', 'transport1')])], ]); $bus = $this->getMockBuilder(MessageBusInterface::class)->getMock(); @@ -97,7 +97,7 @@ class WorkerTest extends TestCase $this->assertNotNull($redeliveryStamp); // retry count now at 1 $this->assertSame(1, $redeliveryStamp->getRetryCount()); - $this->assertSame('sender_alias', $redeliveryStamp->getSenderClassOrAlias()); + $this->assertSame('transport1', $redeliveryStamp->getSenderClassOrAlias()); // received stamp is removed $this->assertNull($envelope->last(ReceivedStamp::class)); @@ -108,7 +108,7 @@ class WorkerTest extends TestCase $retryStrategy = $this->getMockBuilder(RetryStrategyInterface::class)->getMock(); $retryStrategy->expects($this->once())->method('isRetryable')->willReturn(true); - $worker = new Worker(['receiver1' => $receiver], $bus, ['receiver1' => $retryStrategy]); + $worker = new Worker(['transport1' => $receiver], $bus, ['transport1' => $retryStrategy]); $worker->run([], function (?Envelope $envelope) use ($worker) { // stop after the messages finish if (null === $envelope) { @@ -123,7 +123,7 @@ class WorkerTest extends TestCase public function testDispatchCausesRejectWhenNoRetry() { $receiver = new DummyReceiver([ - [new Envelope(new DummyMessage('Hello'), [new SentStamp('Some\Sender', 'sender_alias')])], + [new Envelope(new DummyMessage('Hello'), [new SentStamp('Some\Sender', 'transport1')])], ]); $bus = $this->getMockBuilder(MessageBusInterface::class)->getMock(); @@ -132,7 +132,7 @@ class WorkerTest extends TestCase $retryStrategy = $this->getMockBuilder(RetryStrategyInterface::class)->getMock(); $retryStrategy->expects($this->once())->method('isRetryable')->willReturn(false); - $worker = new Worker(['receiver1' => $receiver], $bus, ['receiver1' => $retryStrategy]); + $worker = new Worker(['transport1' => $receiver], $bus, ['transport1' => $retryStrategy]); $worker->run([], function (?Envelope $envelope) use ($worker) { // stop after the messages finish if (null === $envelope) { @@ -155,7 +155,7 @@ class WorkerTest extends TestCase $retryStrategy = $this->getMockBuilder(RetryStrategyInterface::class)->getMock(); $retryStrategy->expects($this->never())->method('isRetryable'); - $worker = new Worker(['receiver1' => $receiver], $bus, ['receiver1' => $retryStrategy]); + $worker = new Worker(['transport1' => $receiver], $bus, ['transport1' => $retryStrategy]); $worker->run([], function (?Envelope $envelope) use ($worker) { // stop after the messages finish if (null === $envelope) { diff --git a/src/Symfony/Component/Messenger/Worker.php b/src/Symfony/Component/Messenger/Worker.php index b54382bd71..65fa17b5b3 100644 --- a/src/Symfony/Component/Messenger/Worker.php +++ b/src/Symfony/Component/Messenger/Worker.php @@ -17,13 +17,11 @@ use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent; use Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent; use Symfony\Component\Messenger\Event\WorkerStoppedEvent; use Symfony\Component\Messenger\Exception\HandlerFailedException; -use Symfony\Component\Messenger\Exception\LogicException; use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; use Symfony\Component\Messenger\Retry\RetryStrategyInterface; use Symfony\Component\Messenger\Stamp\DelayStamp; use Symfony\Component\Messenger\Stamp\ReceivedStamp; use Symfony\Component\Messenger\Stamp\RedeliveryStamp; -use Symfony\Component\Messenger\Stamp\SentStamp; use Symfony\Component\Messenger\Transport\Receiver\ReceiverInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; @@ -150,7 +148,7 @@ class Worker implements WorkerInterface // add the delay and retry stamp info + remove ReceivedStamp $retryEnvelope = $envelope->with(new DelayStamp($delay)) - ->with(new RedeliveryStamp($retryCount, $this->getSenderClassOrAlias($envelope))) + ->with(new RedeliveryStamp($retryCount, $transportName)) ->withoutAll(ReceivedStamp::class); // re-send the message @@ -197,28 +195,6 @@ class Worker implements WorkerInterface return false; } - $sentStamp = $envelope->last(SentStamp::class); - if (null === $sentStamp) { - if (null !== $this->logger) { - $this->logger->warning('Message will not be retried because the SentStamp is missing and so the target sender cannot be determined.'); - } - - return false; - } - return $retryStrategy->isRetryable($envelope); } - - private function getSenderClassOrAlias(Envelope $envelope): string - { - /** @var SentStamp|null $sentStamp */ - $sentStamp = $envelope->last(SentStamp::class); - - if (null === $sentStamp) { - // should not happen, because of the check in shouldRetry() - throw new LogicException('Could not find SentStamp.'); - } - - return $sentStamp->getSenderAlias() ?: $sentStamp->getSenderClass(); - } }