Merge branch '5.4' into 6.2

nicolas-grekas · nicolas-grekas · commit 670e0f3e7ba5 · 2023-05-13T11:33:33.000+02:00
* 5.4:
  [HttpKernel] Account for Response::getDate() possibly returning a DateTimeImmutable
  [Messenger] Respect `isRetryable` decision of the retry strategy when deciding if failed message should be re-delivered
diff --git a/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php b/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php
@@ -148,7 +148,7 @@ public function update(Response $response)
 
         if (is_numeric($this->ageDirectives['expires'])) {
             $date = clone $response->getDate();
-            $date->modify('+'.($this->ageDirectives['expires'] + $this->age).' seconds');
+            $date = $date->modify('+'.($this->ageDirectives['expires'] + $this->age).' seconds');
             $response->setExpires($date);
         }
     }
diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/CacheAttributeListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/CacheAttributeListenerTest.php
@@ -149,7 +149,7 @@ public function testAttributeConfigurationsAreSetOnResponse()
         $this->assertSame('5', $this->response->headers->getCacheControlDirective('max-stale'));
         $this->assertSame('6', $this->response->headers->getCacheControlDirective('stale-while-revalidate'));
         $this->assertSame('7', $this->response->headers->getCacheControlDirective('stale-if-error'));
-        $this->assertInstanceOf(\DateTime::class, $this->response->getExpires());
+        $this->assertInstanceOf(\DateTimeInterface::class, $this->response->getExpires());
     }
 
     public function testCacheMaxAgeSupportsStrtotimeFormat()
diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php
@@ -275,7 +275,7 @@ public function testCacheControlMerging(array $expects, array $master, array $su
 
                     case 'expires':
                         $expires = clone $response->getDate();
-                        $expires->modify('+'.$value.' seconds');
+                        $expires = $expires->modify('+'.$value.' seconds');
                         $response->setExpires($expires);
                         break;
 
diff --git a/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php b/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php
@@ -117,7 +117,8 @@ public static function getSubscribedEvents(): array
 
     private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInterface $retryStrategy): bool
     {
-        if ($e instanceof RecoverableExceptionInterface) {
+        $isRetryable = $retryStrategy->isRetryable($envelope, $e);
+        if ($isRetryable && $e instanceof RecoverableExceptionInterface) {
             return true;
         }
 
@@ -126,7 +127,7 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt
         if ($e instanceof HandlerFailedException) {
             $shouldNotRetry = true;
             foreach ($e->getNestedExceptions() as $nestedException) {
-                if ($nestedException instanceof RecoverableExceptionInterface) {
+                if ($isRetryable && $nestedException instanceof RecoverableExceptionInterface) {
                     return true;
                 }
 
@@ -144,7 +145,7 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt
             return false;
         }
 
-        return $retryStrategy->isRetryable($envelope, $e);
+        return $isRetryable;
     }
 
     private function getRetryStrategyForTransport(string $alias): ?RetryStrategyInterface
diff --git a/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php b/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php
@@ -63,7 +63,7 @@ public function testRecoverableStrategyCausesRetry()
         $senderLocator->expects($this->once())->method('has')->willReturn(true);
         $senderLocator->expects($this->once())->method('get')->willReturn($sender);
         $retryStategy = $this->createMock(RetryStrategyInterface::class);
-        $retryStategy->expects($this->never())->method('isRetryable');
+        $retryStategy->expects($this->once())->method('isRetryable')->willReturn(true);
         $retryStategy->expects($this->once())->method('getWaitingTime')->willReturn(1000);
         $retryStrategyLocator = $this->createMock(ContainerInterface::class);
         $retryStrategyLocator->expects($this->once())->method('has')->willReturn(true);
@@ -78,6 +78,27 @@ public function testRecoverableStrategyCausesRetry()
         $listener->onMessageFailed($event);
     }
 
+    public function testRetryIsOnlyAllowedWhenPermittedByRetryStrategy()
+    {
+        $senderLocator = $this->createMock(ContainerInterface::class);
+        $senderLocator->expects($this->never())->method('has');
+        $senderLocator->expects($this->never())->method('get');
+        $retryStrategy = $this->createMock(RetryStrategyInterface::class);
+        $retryStrategy->expects($this->once())->method('isRetryable')->willReturn(false);
+        $retryStrategy->expects($this->never())->method('getWaitingTime');
+        $retryStrategyLocator = $this->createMock(ContainerInterface::class);
+        $retryStrategyLocator->expects($this->once())->method('has')->willReturn(true);
+        $retryStrategyLocator->expects($this->once())->method('get')->willReturn($retryStrategy);
+
+        $listener = new SendFailedMessageForRetryListener($senderLocator, $retryStrategyLocator);
+
+        $exception = new RecoverableMessageHandlingException('retry');
+        $envelope = new Envelope(new \stdClass());
+        $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
+
+        $listener->onMessageFailed($event);
+    }
+
     public function testEnvelopeIsSentToTransportOnRetry()
     {
         $exception = new \Exception('no!');

Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ public function update(Response $response)`
`148`	`148`
`149`	`149`	`if (is_numeric($this->ageDirectives['expires'])) {`
`150`	`150`	`$date = clone $response->getDate();`
`151`		`- $date->modify('+'.($this->ageDirectives['expires'] + $this->age).' seconds');`
	`151`	`+ $date = $date->modify('+'.($this->ageDirectives['expires'] + $this->age).' seconds');`
`152`	`152`	`$response->setExpires($date);`
`153`	`153`	`}`
`154`	`154`	`}`
Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,7 @@ public function testAttributeConfigurationsAreSetOnResponse()`
`149`	`149`	`$this->assertSame('5', $this->response->headers->getCacheControlDirective('max-stale'));`
`150`	`150`	`$this->assertSame('6', $this->response->headers->getCacheControlDirective('stale-while-revalidate'));`
`151`	`151`	`$this->assertSame('7', $this->response->headers->getCacheControlDirective('stale-if-error'));`
`152`		`- $this->assertInstanceOf(\DateTime::class, $this->response->getExpires());`
	`152`	`+ $this->assertInstanceOf(\DateTimeInterface::class, $this->response->getExpires());`
`153`	`153`	`}`
`154`	`154`
`155`	`155`	`public function testCacheMaxAgeSupportsStrtotimeFormat()`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,8 @@ public static function getSubscribedEvents(): array`
`117`	`117`
`118`	`118`	`private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInterface $retryStrategy): bool`
`119`	`119`	`{`
`120`		`- if ($e instanceof RecoverableExceptionInterface) {`
	`120`	`+ $isRetryable = $retryStrategy->isRetryable($envelope, $e);`
	`121`	`+ if ($isRetryable && $e instanceof RecoverableExceptionInterface) {`
`121`	`122`	`return true;`
`122`	`123`	`}`
`123`	`124`
`@@ -126,7 +127,7 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt`
`126`	`127`	`if ($e instanceof HandlerFailedException) {`
`127`	`128`	`$shouldNotRetry = true;`
`128`	`129`	`foreach ($e->getNestedExceptions() as $nestedException) {`
`129`		`- if ($nestedException instanceof RecoverableExceptionInterface) {`
	`130`	`+ if ($isRetryable && $nestedException instanceof RecoverableExceptionInterface) {`
`130`	`131`	`return true;`
`131`	`132`	`}`
`132`	`133`
`@@ -144,7 +145,7 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt`
`144`	`145`	`return false;`
`145`	`146`	`}`
`146`	`147`
`147`		`- return $retryStrategy->isRetryable($envelope, $e);`
	`148`	`+ return $isRetryable;`
`148`	`149`	`}`
`149`	`150`
`150`	`151`	`private function getRetryStrategyForTransport(string $alias): ?RetryStrategyInterface`