Issue #2573923 by znerol, Wim Leers, visabhishek, Fabianx, catch: Introduce a...

Issue #2573923 by znerol, Wim Leers, visabhishek, Fabianx, catch: Introduce a CacheableRedirectResponse and use it where appropriate

core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php

@@ -33,11 +33,21 @@ abstract class SecuredRedirectResponse extends RedirectResponse {
    */
   public static function createFromRedirectResponse(RedirectResponse $response) {
     $safe_response = new static($response->getTargetUrl(), $response->getStatusCode(), $response->headers->allPreserveCase());
-    $safe_response->setProtocolVersion($response->getProtocolVersion());
-    $safe_response->setCharset($response->getCharset());
+    $safe_response->fromResponse($response);
     return $safe_response;
   }
 
+  /**
+   * Copies over the values from the given response.
+   *
+   * @param \Symfony\Component\HttpFoundation\RedirectResponse $response
+   *   The redirect reponse object.
+   */
+  protected function fromResponse(RedirectResponse $response) {
+    $this->setProtocolVersion($response->getProtocolVersion());
+    $this->setCharset($response->getCharset());
+  }
+
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/Cache/CacheableRedirectResponse.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Cache\CacheableRedirectResponse.
+ */
+
+namespace Drupal\Core\Cache;
+
+use Symfony\Component\HttpFoundation\RedirectResponse;
+
+/**
+ * A RedirectResponse that contains and can expose cacheability metadata.
+ *
+ * Supports Drupal's caching concepts: cache tags for invalidation and cache
+ * contexts for variations.
+ *
+ * @see \Drupal\Core\Cache\Cache
+ * @see \Drupal\Core\Cache\CacheableMetadata
+ * @see \Drupal\Core\Cache\CacheableResponseTrait
+ */
+class CacheableRedirectResponse extends RedirectResponse implements CacheableResponseInterface {
+
+  use CacheableResponseTrait;
+
+}

core/lib/Drupal/Core/EventSubscriber/RedirectLeadingSlashesSubscriber.php

@@ -7,7 +7,7 @@
 
 namespace Drupal\Core\EventSubscriber;
 
-use Symfony\Component\HttpFoundation\RedirectResponse;
+use Drupal\Core\Cache\CacheableRedirectResponse;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -39,7 +39,7 @@ public function redirect(GetResponseEvent $event) {
       if ($qs) {
         $qs = '?' . $qs;
       }
-      $event->setResponse(new RedirectResponse($request->getUriForPath($path) . $qs));
+      $event->setResponse(new CacheableRedirectResponse($request->getUriForPath($path) . $qs));
     }
   }
 

core/lib/Drupal/Core/Routing/CacheableSecuredRedirectResponse.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\CacheableSecuredRedirectResponse.
+ */
+
+namespace Drupal\Core\Routing;
+
+use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
+use Drupal\Core\Cache\CacheableResponseInterface;
+use Drupal\Core\Cache\CacheableResponseTrait;
+use Symfony\Component\HttpFoundation\RedirectResponse;
+
+/**
+ * Provides a common base class for cacheable safe redirects.
+ */
+abstract class CacheableSecuredRedirectResponse extends SecuredRedirectResponse implements CacheableResponseInterface {
+
+  use CacheableResponseTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function fromResponse(RedirectResponse $response) {
+    parent::fromResponse($response);
+
+    $metadata = $this->getCacheableMetadata();
+    if ($response instanceof CacheableResponseInterface) {
+      $metadata->addCacheableDependency($response->getCacheableMetadata());
+    }
+    else {
+      $metadata->setCacheMaxAge(0);
+    }
+  }
+
+}

core/lib/Drupal/Core/Routing/LocalRedirectResponse.php

@@ -7,12 +7,10 @@
 
 namespace Drupal\Core\Routing;
 
-use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
-
 /**
  * Provides a redirect response which cannot redirect to an external URL.
  */
-class LocalRedirectResponse extends SecuredRedirectResponse {
+class LocalRedirectResponse extends CacheableSecuredRedirectResponse {
 
   use LocalAwareRedirectResponseTrait {
     LocalAwareRedirectResponseTrait::isLocal as isSafe;

core/lib/Drupal/Core/Routing/TrustedRedirectResponse.php

@@ -7,14 +7,12 @@
 
 namespace Drupal\Core\Routing;
 
-use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
-
 /**
  * Provides a redirect response which contains trusted URLs.
  *
  * Use this class in case you know that you want to redirect to an external URL.
  */
-class TrustedRedirectResponse extends SecuredRedirectResponse {
+class TrustedRedirectResponse extends CacheableSecuredRedirectResponse {
 
   use LocalAwareRedirectResponseTrait;
 

core/tests/Drupal/Tests/Core/Routing/TrustedRedirectResponseTest.php

@@ -7,10 +7,14 @@
 
 namespace Drupal\Tests\Core\Routing;
 
+use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Cache\CacheableRedirectResponse;
+use Drupal\Core\Cache\CacheableResponseInterface;
 use Drupal\Core\Routing\RequestContext;
 use Drupal\Core\Routing\TrustedRedirectResponse;
 use Drupal\Tests\UnitTestCase;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\HttpFoundation\RedirectResponse;
 
 /**
  * @coversDefaultClass \Drupal\Core\Routing\TrustedRedirectResponse
@@ -54,4 +58,32 @@ public function testSetTargetUrlWithTrustedUrl() {
     $this->assertEquals('http://good-external-url.com/example', $redirect_response->getTargetUrl());
   }
 
+  /**
+   * @covers ::createFromRedirectResponse
+   * @dataProvider providerCreateFromRedirectResponse
+   */
+  public function testCreateFromRedirectResponse($redirect_response) {
+    $trusted_redirect_response = TrustedRedirectResponse::createFromRedirectResponse($redirect_response);
+
+    // The trusted redirect response is always a CacheableResponseInterface instance.
+    $this->assertTrue($trusted_redirect_response instanceof CacheableResponseInterface);
+
+    // But it is only actually cacheable (non-zero max-age) if the redirect
+    // response passed to TrustedRedirectResponse::createFromRedirectResponse()
+    // is itself cacheable.
+    $expected_cacheability = ($redirect_response instanceof CacheableResponseInterface) ? $redirect_response->getCacheableMetadata() : (new CacheableMetadata())->setCacheMaxAge(0);
+    $this->assertEquals($expected_cacheability, $trusted_redirect_response->getCacheableMetadata());
+  }
+
+  /**
+   * @return array
+   */
+  public function providerCreateFromRedirectResponse() {
+    return [
+      'cacheable-with-tags' => [(new CacheableRedirectResponse('/example'))->addCacheableDependency((new CacheableMetadata())->addCacheTags(['foo']))],
+      'cacheable-with-max-age-0' => [(new CacheableRedirectResponse('/example'))->addCacheableDependency((new CacheableMetadata())->setCacheMaxAge(0))],
+      'uncacheable' => [new RedirectResponse('/example')],
+    ];
+  }
+
 }