diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 41d9eec849fb1306d887790ad8f4fd36ce3a2638..6faf028c2b38a7990f82ee50a5d4124b797013e6 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -383,18 +383,8 @@ public function testGet() { // 200 for well-formed HEAD request. $response = $this->request('HEAD', $url, $request_options); $this->assertResourceResponse(200, '', $response); - // @todo Entity resources with URLs that begin with '/admin/' are marked as - // administrative (see https://www.drupal.org/node/2874938), which - // excludes them from Dynamic Page Cache (see - // https://www.drupal.org/node/2877528). When either of those issues is - // fixed, remove the if-test and the 'else' block. - 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')); - } + $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')); } @@ -407,50 +397,40 @@ public function testGet() { // Same for Dynamic Page Cache hit. $response = $this->request('GET', $url, $request_options); $this->assertResourceResponse(200, FALSE, $response); - // @todo Entity resources with URLs that begin with '/admin/' are marked as - // administrative (see https://www.drupal.org/node/2874938), which - // excludes them from Dynamic Page Cache (see - // https://www.drupal.org/node/2877528). When either of those issues is - // fixed, remove the if-test and the 'else' block. - 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'); - $this->assertCount(2, $cache_items); - $found_cache_redirect = FALSE; - $found_cached_response = FALSE; - foreach ($cache_items as $cid => $cache_item) { - $cached_data = unserialize($cache_item->data); - if (!isset($cached_data['#cache_redirect'])) { - $found_cached_response = TRUE; - $cached_response = $cached_data['#response']; - $this->assertNotInstanceOf(ResourceResponseInterface::class, $cached_response); - $this->assertInstanceOf(CacheableResponseInterface::class, $cached_response); - } - else { - $found_cache_redirect = TRUE; - } - } - $this->assertTrue($found_cache_redirect); - $this->assertTrue($found_cached_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-Dynamic-Cache')); + $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'); + $this->assertCount(2, $cache_items); + $found_cache_redirect = FALSE; + $found_cached_response = FALSE; + foreach ($cache_items as $cid => $cache_item) { + $cached_data = unserialize($cache_item->data); + if (!isset($cached_data['#cache_redirect'])) { + $found_cached_response = TRUE; + $cached_response = $cached_data['#response']; + $this->assertNotInstanceOf(ResourceResponseInterface::class, $cached_response); + $this->assertInstanceOf(CacheableResponseInterface::class, $cached_response); + } + else { + $found_cache_redirect = TRUE; + } + } + $this->assertTrue($found_cache_redirect); + $this->assertTrue($found_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)); diff --git a/core/modules/system/src/EventSubscriber/AdminRouteSubscriber.php b/core/modules/system/src/EventSubscriber/AdminRouteSubscriber.php index b83fe7e130585a71805b2f65e09911aa56f8114f..50b3a57808d039042adf2707357248f72ace5fe2 100644 --- a/core/modules/system/src/EventSubscriber/AdminRouteSubscriber.php +++ b/core/modules/system/src/EventSubscriber/AdminRouteSubscriber.php @@ -4,10 +4,11 @@ use Drupal\Core\Routing\RouteSubscriberBase; use Drupal\Core\Routing\RoutingEvents; +use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; /** - * Adds the _admin_route option to each admin route. + * Adds the _admin_route option to each admin HTML route. */ class AdminRouteSubscriber extends RouteSubscriberBase { @@ -16,7 +17,7 @@ class AdminRouteSubscriber extends RouteSubscriberBase { */ protected function alterRoutes(RouteCollection $collection) { foreach ($collection->all() as $route) { - if (strpos($route->getPath(), '/admin') === 0 && !$route->hasOption('_admin_route')) { + if (strpos($route->getPath(), '/admin') === 0 && !$route->hasOption('_admin_route') && static::isHtmlRoute($route)) { $route->setOption('_admin_route', TRUE); } } @@ -36,4 +37,19 @@ public static function getSubscribedEvents() { return $events; } + /** + * Determines whether the given route is a HTML route. + * + * @param \Symfony\Component\Routing\Route $route + * The route to analyze. + * + * @return bool + * TRUE if HTML is a valid format for this route. + */ + protected static function isHtmlRoute(Route $route) { + // If a route has no explicit format, then HTML is valid. + $format = $route->hasRequirement('_format') ? explode('|', $route->getRequirement('_format')) : ['html']; + return in_array('html', $format, TRUE); + } + } diff --git a/core/modules/system/tests/src/Unit/Routing/AdminRouteSubscriberTest.php b/core/modules/system/tests/src/Unit/Routing/AdminRouteSubscriberTest.php new file mode 100644 index 0000000000000000000000000000000000000000..153391c419de48a64819276431e55693d326eeda --- /dev/null +++ b/core/modules/system/tests/src/Unit/Routing/AdminRouteSubscriberTest.php @@ -0,0 +1,70 @@ +<?php + +namespace Drupal\Tests\system\Unit\Routing; + +use Drupal\Core\Routing\RouteBuildEvent; +use Drupal\system\EventSubscriber\AdminRouteSubscriber; +use Drupal\Tests\UnitTestCase; +use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCollection; + +/** + * @coversDefaultClass \Drupal\system\EventSubscriber\AdminRouteSubscriber + * @group system + */ +class AdminRouteSubscriberTest extends UnitTestCase { + + /** + * @covers ::alterRoutes + * @covers ::isHtmlRoute + * + * @dataProvider providerTestAlterRoutes + */ + public function testAlterRoutes(Route $route, $is_admin) { + $collection = new RouteCollection(); + $collection->add('the_route', $route); + (new AdminRouteSubscriber())->onAlterRoutes(new RouteBuildEvent($collection)); + + $this->assertSame($is_admin, $route->getOption('_admin_route')); + } + + public function providerTestAlterRoutes() { + $data = []; + $data['non-admin'] = [ + new Route('/foo'), + NULL, + ]; + $data['admin prefix'] = [ + new Route('/admin/foo'), + TRUE, + ]; + $data['admin option'] = [ + (new Route('/foo')) + ->setOption('_admin_route', TRUE), + TRUE, + ]; + $data['admin prefix, non-HTML format'] = [ + (new Route('/admin/foo')) + ->setRequirement('_format', 'json'), + NULL, + ]; + $data['admin option, non-HTML format'] = [ + (new Route('/foo')) + ->setRequirement('_format', 'json') + ->setOption('_admin_route', TRUE), + TRUE, + ]; + $data['admin prefix, HTML format'] = [ + (new Route('/admin/foo')) + ->setRequirement('_format', 'html'), + TRUE, + ]; + $data['admin prefix, multi-format including HTML'] = [ + (new Route('/admin/foo')) + ->setRequirement('_format', 'json|html'), + TRUE, + ]; + return $data; + } + +}