.../PageCache/ResponsePolicy/DenyAdminRoutes.php | 19 +------ .../dynamic_page_cache_test.routing.yml | 18 ------ .../src/DynamicPageCacheTestController.php | 11 ---- .../Functional/DynamicPageCacheIntegrationTest.php | 12 ---- .../EntityResource/EntityResourceTestBase.php | 66 ++++++++++++++-------- 5 files changed, 44 insertions(+), 82 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 579ccf3..6857163 100644 --- a/core/modules/dynamic_page_cache/src/PageCache/ResponsePolicy/DenyAdminRoutes.php +++ b/core/modules/dynamic_page_cache/src/PageCache/ResponsePolicy/DenyAdminRoutes.php @@ -6,10 +6,9 @@ use Drupal\Core\Routing\RouteMatchInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Route; /** - * Cache policy for HTML routes with the '_admin_route' option set. + * Cache policy for 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 @@ -25,7 +24,7 @@ class DenyAdminRoutes implements ResponsePolicyInterface { protected $routeMatch; /** - * Constructs a deny HTML admin route page cache policy. + * Constructs a deny admin route page cache policy. * * @param \Drupal\Core\Routing\RouteMatchInterface $route_match * The current route match. @@ -38,21 +37,9 @@ public function __construct(RouteMatchInterface $route_match) { * {@inheritdoc} */ public function check(Response $response, Request $request) { - if (($route = $this->routeMatch->getRouteObject()) && $route->getOption('_admin_route') && static::isHtmlRoute($route)) { + if (($route = $this->routeMatch->getRouteObject()) && $route->getOption('_admin_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/dynamic_page_cache/tests/dynamic_page_cache_test/dynamic_page_cache_test.routing.yml b/core/modules/dynamic_page_cache/tests/dynamic_page_cache_test/dynamic_page_cache_test.routing.yml index f0204fb..77125b6 100644 --- a/core/modules/dynamic_page_cache/tests/dynamic_page_cache_test/dynamic_page_cache_test.routing.yml +++ b/core/modules/dynamic_page_cache/tests/dynamic_page_cache_test/dynamic_page_cache_test.routing.yml @@ -73,21 +73,3 @@ dynamic_page_cache_test.html.uncacheable.tags: _controller: '\Drupal\dynamic_page_cache_test\DynamicPageCacheTestController::htmlUncacheableTags' requirements: _access: 'TRUE' - -dynamic_page_cache_test.json: - path: '/dynamic-page-cache-test/json' - defaults: - _controller: '\Drupal\dynamic_page_cache_test\DynamicPageCacheTestController::json' - requirements: - _format: json - _access: 'TRUE' - -dynamic_page_cache_test.json.admin: - path: '/dynamic-page-cache-test/json/admin' - defaults: - _controller: '\Drupal\dynamic_page_cache_test\DynamicPageCacheTestController::json' - requirements: - _format: json - _access: 'TRUE' - options: - _admin_route: TRUE diff --git a/core/modules/dynamic_page_cache/tests/dynamic_page_cache_test/src/DynamicPageCacheTestController.php b/core/modules/dynamic_page_cache/tests/dynamic_page_cache_test/src/DynamicPageCacheTestController.php index 865a6d8..249aa20 100644 --- a/core/modules/dynamic_page_cache/tests/dynamic_page_cache_test/src/DynamicPageCacheTestController.php +++ b/core/modules/dynamic_page_cache/tests/dynamic_page_cache_test/src/DynamicPageCacheTestController.php @@ -2,7 +2,6 @@ namespace Drupal\dynamic_page_cache_test; -use Drupal\Core\Cache\CacheableJsonResponse; use Drupal\Core\Cache\CacheableResponse; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\user\Entity\User; @@ -138,14 +137,4 @@ public function htmlUncacheableTags() { return $build; } - /** - * A route returning a cacheable JSON response. - * - * @return \Drupal\Core\Cache\CacheableJsonResponse - * A cacheable JSON response. - */ - public function json() { - return new CacheableJsonResponse(['message' => 'Hello world.']); - } - } diff --git a/core/modules/dynamic_page_cache/tests/src/Functional/DynamicPageCacheIntegrationTest.php b/core/modules/dynamic_page_cache/tests/src/Functional/DynamicPageCacheIntegrationTest.php index ddce201..ffa8f43 100644 --- a/core/modules/dynamic_page_cache/tests/src/Functional/DynamicPageCacheIntegrationTest.php +++ b/core/modules/dynamic_page_cache/tests/src/Functional/DynamicPageCacheIntegrationTest.php @@ -101,14 +101,6 @@ public function testDynamicPageCache() { $this->drupalGet('dynamic-page-cache-test/html', ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT => 'drupal_modal']]); $this->assertFalse($this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Render array returned, rendered as modal response: Dynamic Page Cache is ignoring.'); - // Controllers returning cacheable JSON responses are handled by Dynamic - // Page Cache. - $json_url = Url::fromUri('route:dynamic_page_cache_test.json', ['query' => ['_format' => 'json']]); - $this->drupalGet($json_url); - $this->assertEqual('MISS', $this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Cacheable JSON response returned: Dynamic Page Cache is active, Dynamic Page Cache MISS.'); - $this->drupalGet($json_url); - $this->assertEqual('HIT', $this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Cacheable JSON response returned: Dynamic Page Cache is active, Dynamic Page Cache HIT.'); - // Admin routes are ignored by Dynamic Page Cache. $this->drupalGet('dynamic-page-cache-test/html/admin'); $this->assertFalse($this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Response returned, rendered as HTML response, admin route: Dynamic Page Cache is ignoring'); @@ -116,10 +108,6 @@ public function testDynamicPageCache() { $this->assertFalse($this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Response returned, plain response, admin route: Dynamic Page Cache is ignoring'); $this->drupalGet('dynamic-page-cache-test/cacheable-response/admin'); $this->assertFalse($this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Response returned, cacheable response, admin route: Dynamic Page Cache is ignoring'); - // Except if they're explicitly returning a non-HTML format, such as JSON. - $json_admin_url = Url::fromUri('route:dynamic_page_cache_test.json.admin', ['query' => ['_format' => 'json']]); - $this->drupalGet($json_admin_url); - $this->assertEqual('MISS', $this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Response returned, cacheable response, admin route: Dynamic Page Cache is running, but not caching.'); // Max-age = 0 responses are ignored by Dynamic Page Cache. $this->drupalGet('dynamic-page-cache-test/html/uncacheable/max-age'); diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 7998b8a..120369f 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -388,8 +388,16 @@ 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')); + // @todo Remove the if-test, remove the 'else', keep the body of the 'if' once https://www.drupal.org/node/2874938 lands. + // Dynamic Page Cache does not work for admin routes. + // @see \Drupal\dynamic_page_cache\PageCache\ResponsePolicy\DenyAdminRoutes + if (strpos($this->entity->getEntityType()->getLinkTemplate('canonical'), '/admin/') !== 0) { + $this->assertTrue($response->hasHeader('X-Drupal-Dynamic-Cache')); + $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Dynamic-Cache')); + } + else { + $this->assertFalse($response->hasHeader('X-Drupal-Dynamic-Cache')); + } if (!$this->account) { $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Cache')); } @@ -402,32 +410,40 @@ public function testGet() { // 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); + // @todo Remove the if-test, remove the 'else', keep the body of the 'if' once https://www.drupal.org/node/2874938 lands. + // Dynamic Page Cache does not work for admin routes. + // @see \Drupal\dynamic_page_cache\PageCache\ResponsePolicy\DenyAdminRoutes + if (strpos($this->entity->getEntityType()->getLinkTemplate('canonical'), '/admin/') !== 0) { + $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); + } } } } + else { + $this->assertFalse($response->hasHeader('X-Drupal-Dynamic-Cache')); + } $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)); $cache_contexts_header_value = $response->getHeader('X-Drupal-Cache-Contexts')[0];