.../PageCache/ResponsePolicy/DenyAdminRoutes.php | 19 +++++++++++++--- .../EventSubscriber/ResourceResponseSubscriber.php | 5 +++-- .../EntityResource/EntityResourceTestBase.php | 25 ++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/core/modules/dynamic_page_cache/src/PageCache/ResponsePolicy/DenyAdminRoutes.php b/core/modules/dynamic_page_cache/src/PageCache/ResponsePolicy/DenyAdminRoutes.php index 6857163..579ccf3 100644 --- a/core/modules/dynamic_page_cache/src/PageCache/ResponsePolicy/DenyAdminRoutes.php +++ b/core/modules/dynamic_page_cache/src/PageCache/ResponsePolicy/DenyAdminRoutes.php @@ -6,9 +6,10 @@ use Drupal\Core\Routing\RouteMatchInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Routing\Route; /** - * Cache policy for routes with the '_admin_route' option set. + * Cache policy for HTML routes with the '_admin_route' option set. * * This policy rule denies caching of responses generated for admin routes, * because admin routes have very low cache hit ratios due to low traffic and @@ -24,7 +25,7 @@ class DenyAdminRoutes implements ResponsePolicyInterface { protected $routeMatch; /** - * Constructs a deny admin route page cache policy. + * Constructs a deny HTML admin route page cache policy. * * @param \Drupal\Core\Routing\RouteMatchInterface $route_match * The current route match. @@ -37,9 +38,21 @@ public function __construct(RouteMatchInterface $route_match) { * {@inheritdoc} */ public function check(Response $response, Request $request) { - if (($route = $this->routeMatch->getRouteObject()) && $route->getOption('_admin_route')) { + if (($route = $this->routeMatch->getRouteObject()) && $route->getOption('_admin_route') && static::isHtmlRoute($route)) { return static::DENY; } } + /** + * Determines whether the given route is a HTML route. + * + * @param \Symfony\Component\Routing\Route $route + * The route to analyze. + * + * @return bool + */ + protected static function isHtmlRoute(Route $route) { + return !$route->hasRequirement('_format') || $route->getRequirement('_format') === 'html'; + } + } diff --git a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php index 5cb887b..1223be0 100644 --- a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php +++ b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php @@ -196,8 +196,9 @@ protected function flattenResponse(ResourceResponseInterface $response) { * {@inheritdoc} */ public static function getSubscribedEvents() { - // Run shortly before \Drupal\Core\EventSubscriber\FinishResponseSubscriber. - $events[KernelEvents::RESPONSE][] = ['onResponse', 5]; + // Run before \Drupal\dynamic_page_cache\EventSubscriber\DynamicPageCacheSubscriber + // (priority 100), so that Dynamic Page Cache can cache flattened responses. + $events[KernelEvents::RESPONSE][] = ['onResponse', 128]; return $events; } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index a55ae92..7998b8a 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -4,11 +4,13 @@ use Drupal\Component\Utility\NestedArray; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableResponseInterface; use Drupal\Core\Config\Entity\ConfigEntityInterface; use Drupal\Core\Entity\FieldableEntityInterface; use Drupal\Core\Url; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; +use Drupal\rest\ResourceResponseInterface; use Drupal\Tests\rest\Functional\ResourceTestBase; use GuzzleHttp\RequestOptions; use Psr\Http\Message\ResponseInterface; @@ -386,6 +388,8 @@ public function testGet() { // 200 for well-formed HEAD request. $response = $this->request('HEAD', $url, $request_options); $this->assertResourceResponse(200, '', $response); + $this->assertTrue($response->hasHeader('X-Drupal-Dynamic-Cache')); + $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Dynamic-Cache')); if (!$this->account) { $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Cache')); } @@ -395,13 +399,34 @@ public function testGet() { $head_headers = $response->getHeaders(); // 200 for well-formed GET request. Page Cache hit because of HEAD request. + // Same for Dynamic Page Cache hit. $response = $this->request('GET', $url, $request_options); $this->assertResourceResponse(200, FALSE, $response); + $this->assertTrue($response->hasHeader('X-Drupal-Dynamic-Cache')); if (!static::$auth) { $this->assertSame(['HIT'], $response->getHeader('X-Drupal-Cache')); + $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Dynamic-Cache')); } else { $this->assertFalse($response->hasHeader('X-Drupal-Cache')); + $this->assertSame(['HIT'], $response->getHeader('X-Drupal-Dynamic-Cache')); + // Assert that Dynamic Page Cache did not store a ResourceResponse object, + // which needs serialization after every cache hit. Instead, it should + // contain a flattened response. Otherwise performance suffers. + // @see \Drupal\rest\EventSubscriber\ResourceResponseSubscriber::flattenResponse() + $cache_items = $this->container->get('database') + ->query("SELECT cid, data FROM {cache_dynamic_page_cache} WHERE cid LIKE :pattern", [ + ':pattern' => '%[route]=rest.%', + ]) + ->fetchAllAssoc('cid'); + foreach ($cache_items as $cid => $cache_item) { + $cached_data = unserialize($cache_item->data); + if (!isset($cached_data['#cache_redirect'])) { + $cached_response = $cached_data['#response']; + $this->assertNotInstanceOf(ResourceResponseInterface::class, $cached_response); + $this->assertInstanceOf(CacheableResponseInterface::class, $cached_response); + } + } } $cache_tags_header_value = $response->getHeader('X-Drupal-Cache-Tags')[0]; $this->assertEquals($this->getExpectedCacheTags(), empty($cache_tags_header_value) ? [] : explode(' ', $cache_tags_header_value));