From 13e0794edc19f9d010c93aaabd4b7801dd5d21e5 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Fri, 20 Mar 2015 22:39:42 +0000 Subject: [PATCH] Issue #2445723 by dawehner, neclimdul: Use the $request format instead of the ContentNegotation --- core/core.services.yml | 26 +++--- core/lib/Drupal/Core/Ajax/AjaxSubscriber.php | 43 ---------- .../ContentControllerSubscriber.php | 35 -------- .../DefaultExceptionSubscriber.php | 4 +- .../HttpExceptionSubscriberBase.php | 12 +-- .../Core/EventSubscriber/ViewSubscriber.php | 19 +---- .../Core/Routing/AcceptHeaderMatcher.php | 20 +---- .../Drupal/Core/Routing/RoutePreloader.php | 15 +--- .../StackMiddleware/NegotiationMiddleware.php | 83 +++++++++++++++++++ .../Drupal/Core/StackMiddleware/PageCache.php | 15 +--- core/modules/hal/hal.services.yml | 4 - core/modules/hal/src/HalServiceProvider.php | 28 +++++++ core/modules/hal/src/HalSubscriber.php | 41 --------- .../src/Plugin/views/display/RestExport.php | 18 +--- .../rest/tests/src/Unit/CollectRoutesTest.php | 6 -- .../src/Tests/Bootstrap/PageCacheTest.php | 45 ++++++++++ .../Core/Routing/AcceptHeaderMatcherTest.php | 20 +++-- .../Tests/Core/Routing/RoutePreloaderTest.php | 20 +---- 18 files changed, 199 insertions(+), 255 deletions(-) delete mode 100644 core/lib/Drupal/Core/Ajax/AjaxSubscriber.php create mode 100644 core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php create mode 100644 core/modules/hal/src/HalServiceProvider.php delete mode 100644 core/modules/hal/src/HalSubscriber.php diff --git a/core/core.services.yml b/core/core.services.yml index ca74a8c1690..a7a2b3bdbce 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -486,6 +486,18 @@ services: http_kernel.basic: class: Symfony\Component\HttpKernel\HttpKernel arguments: ['@event_dispatcher', '@controller_resolver', '@request_stack'] + http_negotiation.format_negotiator: + class: Drupal\Core\ContentNegotiation + private: true + http_middleware.negotiation: + class: Drupal\Core\StackMiddleware\NegotiationMiddleware + arguments: ['@http_negotiation.format_negotiator'] + calls: + - [registerFormat, ['drupal_ajax', ['application/vnd.drupal-ajax']]] + - [registerFormat, ['drupal_dialog', ['application/vnd.drupal-dialog']]] + - [registerFormat, ['drupal_modal', ['application/vnd.drupal-modal']]] + tags: + - { name: http_middleware, priority: 400 } http_middleware.reverse_proxy: class: Drupal\Core\StackMiddleware\ReverseProxyMiddleware arguments: ['@settings'] @@ -493,7 +505,7 @@ services: - { name: http_middleware, priority: 300 } http_middleware.page_cache: class: Drupal\Core\StackMiddleware\PageCache - arguments: ['@cache.render', '@page_cache_request_policy', '@page_cache_response_policy', '@content_negotiation'] + arguments: ['@cache.render', '@page_cache_request_policy', '@page_cache_response_policy'] tags: - { name: http_middleware, priority: 200 } http_middleware.kernel_pre_handle: @@ -571,7 +583,7 @@ services: - { name: backend_overridable } router.route_preloader: class: Drupal\Core\Routing\RoutePreloader - arguments: ['@router.route_provider', '@state', '@content_negotiation'] + arguments: ['@router.route_provider', '@state'] tags: - { name: 'event_subscriber' } router.matcher.final_matcher: @@ -682,7 +694,6 @@ services: arguments: [16] accept_header_matcher: class: Drupal\Core\Routing\AcceptHeaderMatcher - arguments: ['@content_negotiation'] tags: - { name: route_filter } content_type_header_matcher: @@ -749,7 +760,6 @@ services: - { name: route_enhancer, priority: 20 } route_content_controller_subscriber: class: Drupal\Core\EventSubscriber\ContentControllerSubscriber - arguments: ['@content_negotiation'] tags: - { name: event_subscriber } route_special_attributes_subscriber: @@ -799,13 +809,11 @@ services: tags: - { name: event_subscriber } arguments: ['@router', '@router.request_context', NULL, '@request_stack'] - content_negotiation: - class: Drupal\Core\ContentNegotiation view_subscriber: class: Drupal\Core\EventSubscriber\ViewSubscriber + arguments: ['@title_resolver'] tags: - { name: event_subscriber } - arguments: ['@content_negotiation', '@title_resolver'] bare_html_page_renderer: class: Drupal\Core\Render\BareHtmlPageRenderer arguments: ['@renderer'] @@ -1003,10 +1011,6 @@ services: - { name: event_subscriber } calls: - [setContainer, ['@service_container']] - ajax.subscriber: - class: Drupal\Core\Ajax\AjaxSubscriber - tags: - - { name: event_subscriber } image.toolkit.manager: class: Drupal\Core\ImageToolkit\ImageToolkitManager arguments: ['@config.factory'] diff --git a/core/lib/Drupal/Core/Ajax/AjaxSubscriber.php b/core/lib/Drupal/Core/Ajax/AjaxSubscriber.php deleted file mode 100644 index f298d88e81e..00000000000 --- a/core/lib/Drupal/Core/Ajax/AjaxSubscriber.php +++ /dev/null @@ -1,43 +0,0 @@ -getRequest(); - $request->setFormat('drupal_ajax', 'application/vnd.drupal-ajax'); - $request->setFormat('drupal_dialog', 'application/vnd.drupal-dialog'); - $request->setFormat('drupal_modal', 'application/vnd.drupal-modal'); - } - - /** - * Registers the methods in this class that should be listeners. - * - * @return array - * An array of event listener definitions. - */ - static function getSubscribedEvents(){ - $events[KernelEvents::REQUEST][] = array('onKernelRequest', 50); - return $events; - } - -} \ No newline at end of file diff --git a/core/lib/Drupal/Core/EventSubscriber/ContentControllerSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ContentControllerSubscriber.php index 32f9565e1bf..16e96139343 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ContentControllerSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ContentControllerSubscriber.php @@ -7,7 +7,6 @@ namespace Drupal\Core\EventSubscriber; -use Drupal\Core\ContentNegotiation; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\KernelEvents; @@ -20,39 +19,6 @@ use Symfony\Component\HttpKernel\KernelEvents; */ class ContentControllerSubscriber implements EventSubscriberInterface { - /** - * Content negotiation library. - * - * @var \Drupal\Core\ContentNegotiation - */ - protected $negotiation; - - /** - * Constructs a new ContentControllerSubscriber object. - * - * @param \Drupal\Core\ContentNegotiation $negotiation - * The Content Negotiation service. - */ - public function __construct(ContentNegotiation $negotiation) { - $this->negotiation = $negotiation; - } - - /** - * Sets the derived request format on the request. - * - * @todo Remove when https://www.drupal.org/node/2331919 lands. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event - * The event to process. - */ - public function onRequestDeriveFormat(GetResponseEvent $event) { - $request = $event->getRequest(); - - if (!$request->attributes->get('_format')) { - $request->setRequestFormat($this->negotiation->getContentType($request)); - } - } - /** * Sets the _controller on a request when a _form is defined. * @@ -74,7 +40,6 @@ class ContentControllerSubscriber implements EventSubscriberInterface { * An array of event listener definitions. */ static function getSubscribedEvents() { - $events[KernelEvents::REQUEST][] = array('onRequestDeriveFormat', 31); $events[KernelEvents::REQUEST][] = array('onRequestDeriveFormWrapper', 29); return $events; diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php index 03f26918dab..b3d22025c3d 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php @@ -10,7 +10,6 @@ namespace Drupal\Core\EventSubscriber; use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Config\ConfigFactoryInterface; -use Drupal\Core\ContentNegotiation; use Drupal\Core\Render\BareHtmlPageRendererInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\Utility\Error; @@ -206,8 +205,7 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface { // to this code. We therefore use this style for now on the expectation // that it will get replaced with better code later. This approach makes // that change easier when we get to it. - $conneg = new ContentNegotiation(); - $format = $conneg->getContentType($request); + $format = \Drupal::service('http_negotiation.format_negotiator')->getContentType($request); // These are all JSON errors for our purposes. Any special handling for // them can/should happen in earlier listeners if desired. diff --git a/core/lib/Drupal/Core/EventSubscriber/HttpExceptionSubscriberBase.php b/core/lib/Drupal/Core/EventSubscriber/HttpExceptionSubscriberBase.php index c9d9fa99145..a14d327b16c 100644 --- a/core/lib/Drupal/Core/EventSubscriber/HttpExceptionSubscriberBase.php +++ b/core/lib/Drupal/Core/EventSubscriber/HttpExceptionSubscriberBase.php @@ -7,7 +7,6 @@ namespace Drupal\Core\EventSubscriber; -use Drupal\Core\ContentNegotiation; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; @@ -91,16 +90,7 @@ abstract class HttpExceptionSubscriberBase implements EventSubscriberInterface { $handled_formats = $this->getHandledFormats(); - // @todo Injecting this service would force all implementing classes to also - // handle its injection. However, we are trying to switch to a more robust - // content negotiation library in https://www.drupal.org/node/1505080 that - // will make $request->getRequestFormat() reliable as a better alternative - // to this code. We therefore use this style for now on the expectation - // that it will get replaced with better code later. That change will NOT - // be an API change for any implementing classes. (Whereas if we injected - // this class it would be an API change. That's why we're not doing it.) - $conneg = new ContentNegotiation(); - $format = $conneg->getContentType($event->getRequest()); + $format = $event->getRequest()->getRequestFormat(); if ($exception instanceof HttpExceptionInterface && (empty($handled_formats) || in_array($format, $handled_formats))) { $method = 'on' . $exception->getStatusCode(); diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php index 0383a3779b3..3485f1928be 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php @@ -16,8 +16,6 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Drupal\Core\ContentNegotiation; - /** * Main subscriber for VIEW HTTP responses. * @@ -27,13 +25,6 @@ use Drupal\Core\ContentNegotiation; */ class ViewSubscriber implements EventSubscriberInterface { - /** - * The content negotiation. - * - * @var \Drupal\Core\ContentNegotiation - */ - protected $negotiation; - /** * The title resolver. * @@ -41,17 +32,13 @@ class ViewSubscriber implements EventSubscriberInterface { */ protected $titleResolver; - /** * Constructs a new ViewSubscriber. * - * @param \Drupal\Core\ContentNegotiation $negotiation - * The content negotiation. * @param \Drupal\Core\Controller\TitleResolverInterface $title_resolver * The title resolver. */ - public function __construct(ContentNegotiation $negotiation, TitleResolverInterface $title_resolver) { - $this->negotiation = $negotiation; + public function __construct(TitleResolverInterface $title_resolver) { $this->titleResolver = $title_resolver; } @@ -64,14 +51,14 @@ class ViewSubscriber implements EventSubscriberInterface { * from an JSON-type response is a JSON string, so just wrap it into a * Response object. * - * @param Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent $event + * @param \Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent $event * The Event to process. */ public function onView(GetResponseForControllerResultEvent $event) { $request = $event->getRequest(); if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) { - $method = 'on' . $this->negotiation->getContentType($request); + $method = 'on' . $request->getRequestFormat(); if (method_exists($this, $method)) { $event->setResponse($this->$method($event)); diff --git a/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php b/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php index 11760dd0684..5c0618ce3d0 100644 --- a/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php +++ b/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php @@ -8,7 +8,6 @@ namespace Drupal\Core\Routing; use Drupal\Component\Utility\String; -use Drupal\Core\ContentNegotiation; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException; use Symfony\Component\Routing\Route; @@ -19,23 +18,6 @@ use Symfony\Component\Routing\RouteCollection; */ class AcceptHeaderMatcher implements RouteFilterInterface { - /** - * The content negotiation library. - * - * @var \Drupal\Core\ContentNegotiation - */ - protected $contentNegotiation; - - /** - * Constructs a new AcceptHeaderMatcher. - * - * @param \Drupal\Core\ContentNegotiation $content_negotiation - * The content negotiation library. - */ - public function __construct(ContentNegotiation $content_negotiation) { - $this->contentNegotiation = $content_negotiation; - } - /** * {@inheritdoc} */ @@ -44,7 +26,7 @@ class AcceptHeaderMatcher implements RouteFilterInterface { // @todo replace by proper content negotiation library. $acceptable_mime_types = $request->getAcceptableContentTypes(); $acceptable_formats = array_filter(array_map(array($request, 'getFormat'), $acceptable_mime_types)); - $primary_format = $this->contentNegotiation->getContentType($request); + $primary_format = $request->getRequestFormat(); foreach ($collection as $name => $route) { // _format could be a |-delimited list of supported formats. diff --git a/core/lib/Drupal/Core/Routing/RoutePreloader.php b/core/lib/Drupal/Core/Routing/RoutePreloader.php index 7952d177e6e..1f223bfd486 100644 --- a/core/lib/Drupal/Core/Routing/RoutePreloader.php +++ b/core/lib/Drupal/Core/Routing/RoutePreloader.php @@ -7,7 +7,6 @@ namespace Drupal\Core\Routing; -use Drupal\Core\ContentNegotiation; use Drupal\Core\State\StateInterface; use Symfony\Component\EventDispatcher\Event; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -37,13 +36,6 @@ class RoutePreloader implements EventSubscriberInterface { */ protected $state; - /** - * The content negotiation. - * - * @var \Drupal\Core\ContentNegotiation - */ - protected $negotiation; - /** * Contains the non-admin routes while rebuilding the routes. * @@ -58,13 +50,10 @@ class RoutePreloader implements EventSubscriberInterface { * The route provider. * @param \Drupal\Core\State\StateInterface $state * The state key value store. - * @param \Drupal\Core\ContentNegotiation $negotiation - * The content negotiation. */ - public function __construct(RouteProviderInterface $route_provider, StateInterface $state, ContentNegotiation $negotiation) { + public function __construct(RouteProviderInterface $route_provider, StateInterface $state) { $this->routeProvider = $route_provider; $this->state = $state; - $this->negotiation = $negotiation; } /** @@ -75,7 +64,7 @@ class RoutePreloader implements EventSubscriberInterface { */ public function onRequest(KernelEvent $event) { // Just preload on normal HTML pages, as they will display menu links. - if ($this->negotiation->getContentType($event->getRequest()) == 'html') { + if ($event->getRequest()->getRequestFormat() == 'html') { $this->loadNonAdminRoutes(); } } diff --git a/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php new file mode 100644 index 00000000000..295857d626c --- /dev/null +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php @@ -0,0 +1,83 @@ +app = $app; + $this->negotiator = $negotiator; + } + + /** + * {@inheritdoc} + */ + public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = true) { + foreach ($this->formats as $format => $mime_type) { + $request->setFormat($format, $mime_type); + } + + $request->setRequestFormat($this->negotiator->getContentType($request)); + return $this->app->handle($request, $type, $catch); + } + + /** + * Registers a format for a given MIME type. + * + * @param string $format + * The format. + * @param string $mime_type + * The MIME type. + * + * @return $this + */ + public function registerFormat($format, $mime_type) { + $this->formats[$format] = $mime_type; + return $this; + } + +} diff --git a/core/lib/Drupal/Core/StackMiddleware/PageCache.php b/core/lib/Drupal/Core/StackMiddleware/PageCache.php index b381018f49d..d4ec291494e 100644 --- a/core/lib/Drupal/Core/StackMiddleware/PageCache.php +++ b/core/lib/Drupal/Core/StackMiddleware/PageCache.php @@ -9,7 +9,6 @@ namespace Drupal\Core\StackMiddleware; use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheBackendInterface; -use Drupal\Core\ContentNegotiation; use Drupal\Core\PageCache\RequestPolicyInterface; use Drupal\Core\PageCache\ResponsePolicyInterface; use Drupal\Core\Site\Settings; @@ -52,13 +51,6 @@ class PageCache implements HttpKernelInterface { */ protected $responsePolicy; - /** - * The content negotiation library. - * - * @var \Drupal\Core\ContentNegotiation - */ - protected $contentNegotiation; - /** * Constructs a ReverseProxyMiddleware object. * @@ -70,15 +62,12 @@ class PageCache implements HttpKernelInterface { * A policy rule determining the cacheability of a request. * @param \Drupal\Core\PageCache\ResponsePolicyInterface $response_policy * A policy rule determining the cacheability of the response. - * @param \Drupal\Core\ContentNegotiation $content_negotiation - * The content negotiation library. */ - public function __construct(HttpKernelInterface $http_kernel, CacheBackendInterface $cache, RequestPolicyInterface $request_policy, ResponsePolicyInterface $response_policy, ContentNegotiation $content_negotiation) { + public function __construct(HttpKernelInterface $http_kernel, CacheBackendInterface $cache, RequestPolicyInterface $request_policy, ResponsePolicyInterface $response_policy) { $this->httpKernel = $http_kernel; $this->cache = $cache; $this->requestPolicy = $request_policy; $this->responsePolicy = $response_policy; - $this->contentNegotiation = $content_negotiation; } /** @@ -332,7 +321,7 @@ class PageCache implements HttpKernelInterface { protected function getCacheId(Request $request) { $cid_parts = array( $request->getUri(), - $this->contentNegotiation->getContentType($request), + $request->getRequestFormat(), ); return implode(':', $cid_parts); } diff --git a/core/modules/hal/hal.services.yml b/core/modules/hal/hal.services.yml index 4e8984168f6..7e40df0d5b8 100644 --- a/core/modules/hal/hal.services.yml +++ b/core/modules/hal/hal.services.yml @@ -26,7 +26,3 @@ services: class: Drupal\hal\Encoder\JsonEncoder tags: - { name: encoder, priority: 10, format: hal_json } - hal.subscriber: - class: Drupal\hal\HalSubscriber - tags: - - { name: event_subscriber } diff --git a/core/modules/hal/src/HalServiceProvider.php b/core/modules/hal/src/HalServiceProvider.php new file mode 100644 index 00000000000..d8ac47f0f08 --- /dev/null +++ b/core/modules/hal/src/HalServiceProvider.php @@ -0,0 +1,28 @@ +has('http_middleware.negotiation')) { + $container->getDefinition('http_middleware.negotiation')->addMethodCall('registerFormat', ['hal_json', ['application/hal+json']]); + } + } + +} + diff --git a/core/modules/hal/src/HalSubscriber.php b/core/modules/hal/src/HalSubscriber.php deleted file mode 100644 index 93a70bb6ba8..00000000000 --- a/core/modules/hal/src/HalSubscriber.php +++ /dev/null @@ -1,41 +0,0 @@ -getRequest(); - $request->setFormat('hal_json', 'application/hal+json'); - } - - /** - * Registers the methods in this class that should be listeners. - * - * @return array - * An array of event listener definitions. - */ - static function getSubscribedEvents() { - $events[KernelEvents::REQUEST][] = array('onKernelRequest', 40); - return $events; - } - -} diff --git a/core/modules/rest/src/Plugin/views/display/RestExport.php b/core/modules/rest/src/Plugin/views/display/RestExport.php index 076eb1c0c14..240c9de7954 100644 --- a/core/modules/rest/src/Plugin/views/display/RestExport.php +++ b/core/modules/rest/src/Plugin/views/display/RestExport.php @@ -10,7 +10,6 @@ namespace Drupal\rest\Plugin\views\display; use Drupal\Component\Utility\String; use Drupal\Core\State\StateInterface; use Drupal\Core\Routing\RouteProviderInterface; -use Drupal\Core\ContentNegotiation; use Drupal\views\ViewExecutable; use Drupal\views\Plugin\views\display\PathPluginBase; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -72,13 +71,6 @@ class RestExport extends PathPluginBase { */ protected $mimeType; - /** - * The content negotiation library. - * - * @var \Drupal\Core\ContentNegotiation - */ - protected $contentNegotiation; - /** * Constructs a Drupal\rest\Plugin\ResourceBase object. * @@ -92,12 +84,9 @@ class RestExport extends PathPluginBase { * The route provider * @param \Drupal\Core\State\StateInterface $state * The state key value store. - * @param \Drupal\Core\ContentNegotiation $content_negotiation - * The content negotiation library. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state, ContentNegotiation $content_negotiation) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state) { parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $state); - $this->contentNegotiation = $content_negotiation; } /** @@ -109,8 +98,7 @@ class RestExport extends PathPluginBase { $plugin_id, $plugin_definition, $container->get('router.route_provider'), - $container->get('state'), - $container->get('content_negotiation') + $container->get('state') ); } @@ -120,7 +108,7 @@ class RestExport extends PathPluginBase { public function initDisplay(ViewExecutable $view, array &$display, array &$options = NULL) { parent::initDisplay($view, $display, $options); - $request_content_type = $this->contentNegotiation->getContentType($this->view->getRequest()); + $request_content_type = $this->view->getRequest()->getRequestFormat(); // Only use the requested content type if it's not 'html'. If it is then // default to 'json' to aid debugging. // @todo Remove the need for this when we have better content negotiation. diff --git a/core/modules/rest/tests/src/Unit/CollectRoutesTest.php b/core/modules/rest/tests/src/Unit/CollectRoutesTest.php index 7a91831a1cb..7a2cbae2d95 100644 --- a/core/modules/rest/tests/src/Unit/CollectRoutesTest.php +++ b/core/modules/rest/tests/src/Unit/CollectRoutesTest.php @@ -40,16 +40,10 @@ class CollectRoutesTest extends UnitTestCase { $container = new ContainerBuilder(); - $content_negotiation = $this->getMockBuilder('\Drupal\Core\ContentNegotiation') - ->disableOriginalConstructor() - ->getMock(); - $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') ->disableOriginalConstructor() ->getMock(); - $container->set('content_negotiation', $content_negotiation); - $this->view = $this->getMock('\Drupal\views\Entity\View', array('initHandlers'), array( array('id' => 'test_view'), 'view', diff --git a/core/modules/system/src/Tests/Bootstrap/PageCacheTest.php b/core/modules/system/src/Tests/Bootstrap/PageCacheTest.php index 0f7bc6d2b5a..c389a2a483d 100644 --- a/core/modules/system/src/Tests/Bootstrap/PageCacheTest.php +++ b/core/modules/system/src/Tests/Bootstrap/PageCacheTest.php @@ -11,6 +11,7 @@ use Drupal\Component\Datetime\DateTimePlus; use Drupal\Core\Routing\RequestContext; use Drupal\simpletest\WebTestBase; use Drupal\Core\Cache\Cache; +use Drupal\user\Entity\Role; /** * Enables the page cache and tests it with various HTTP requests. @@ -98,6 +99,50 @@ class PageCacheTest extends WebTestBase { $this->drupalGet($accept_header_cache_uri, array(), $json_accept_header); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', 'Json response was cached.'); $this->assertRaw('{"content":"oh hai this is json"}', 'The correct Json response was returned.'); + + // Enable REST support for nodes and hal+json. + \Drupal::service('module_installer')->install(['node', 'rest', 'hal']); + $this->drupalCreateContentType(['type' => 'article']); + $node = $this->drupalCreateNode(['type' => 'article']); + $node_uri = 'node/' . $node->id(); + $hal_json_accept_header = ['Accept: application/hal+json']; + /** @var \Drupal\user\RoleInterface $role */ + $role = Role::load('anonymous'); + $role->grantPermission('restful get entity:node'); + $role->save(); + + $this->drupalGet($node_uri); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'text/html; charset=UTF-8'); + $this->drupalGet($node_uri); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'text/html; charset=UTF-8'); + + // Now request a HAL page, we expect that the first request is a cache miss + // and it serves HTML. + $this->drupalGet($node_uri, [], $hal_json_accept_header); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'application/hal+json'); + $this->drupalGet($node_uri, [], $hal_json_accept_header); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'application/hal+json'); + + // Clear the page cache. After that request a HAL request, followed by an + // ordinary HTML one. + \Drupal::cache('render')->deleteAll(); + $this->drupalGet($node_uri, [], $hal_json_accept_header); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'application/hal+json'); + $this->drupalGet($node_uri, [], $hal_json_accept_header); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'application/hal+json'); + + $this->drupalGet($node_uri); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'text/html; charset=UTF-8'); + $this->drupalGet($node_uri); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT'); + $this->assertEqual($this->drupalGetHeader('Content-Type'), 'text/html; charset=UTF-8'); } /** diff --git a/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php b/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php index 6f47816e6a9..d0c811cafc7 100644 --- a/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/AcceptHeaderMatcherTest.php @@ -7,7 +7,6 @@ namespace Drupal\Tests\Core\Routing; -use Drupal\Core\ContentNegotiation; use Drupal\Core\Routing\AcceptHeaderMatcher; use Drupal\Tests\Core\Routing\RoutingFixtures; use Drupal\Tests\UnitTestCase; @@ -41,7 +40,7 @@ class AcceptHeaderMatcherTest extends UnitTestCase { parent::setUp(); $this->fixtures = new RoutingFixtures(); - $this->matcher = new AcceptHeaderMatcher(new ContentNegotiation()); + $this->matcher = new AcceptHeaderMatcher(); } /** @@ -50,14 +49,14 @@ class AcceptHeaderMatcherTest extends UnitTestCase { * @see Drupal\Tests\Core\Routing\AcceptHeaderMatcherTest::testAcceptFiltering() */ public function acceptFilterProvider() { - return array( + return [ // Check that JSON routes get filtered and prioritized correctly. - array('application/json, text/xml;q=0.9', 'route_c', 'route_e'), + ['application/json, text/xml;q=0.9', 'json', 'route_c', 'route_e'], // Tests a JSON request with alternative JSON MIME type Accept header. - array('application/x-json, text/xml;q=0.9', 'route_c', 'route_e'), + ['application/x-json, text/xml;q=0.9', 'json', 'route_c', 'route_e'], // Tests a standard HTML request. - array('text/html, text/xml;q=0.9', 'route_e', 'route_c'), - ); + ['text/html, text/xml;q=0.9', 'html', 'route_e', 'route_c'], + ]; } /** @@ -65,6 +64,8 @@ class AcceptHeaderMatcherTest extends UnitTestCase { * * @param string $accept_header * The HTTP Accept header value of the request. + * @param string $format + * The request format. * @param string $included_route * The route name that should survive the filter and be ranked first. * @param string $excluded_route @@ -72,11 +73,12 @@ class AcceptHeaderMatcherTest extends UnitTestCase { * * @dataProvider acceptFilterProvider */ - public function testAcceptFiltering($accept_header, $included_route, $excluded_route) { + public function testAcceptFiltering($accept_header, $format, $included_route, $excluded_route) { $collection = $this->fixtures->sampleRouteCollection(); $request = Request::create('path/two', 'GET'); $request->headers->set('Accept', $accept_header); + $request->setRequestFormat($format); $routes = $this->matcher->filter($collection, $request); $this->assertEquals(count($routes), 4, 'The correct number of routes was found.'); $this->assertNotNull($routes->get($included_route), "Route $included_route was found when matching $accept_header."); @@ -103,6 +105,8 @@ class AcceptHeaderMatcherTest extends UnitTestCase { $request = Request::create('path/two', 'GET'); $request->headers->set('Accept', 'application/json, text/xml;q=0.9'); + $request->setRequestFormat('json'); + $this->matcher->filter($routes, $request); $this->matcher->filter($routes, $request); $this->fail('No exception was thrown.'); } diff --git a/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php b/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php index 109a3f28e56..5692e6a1df7 100644 --- a/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php @@ -34,13 +34,6 @@ class RoutePreloaderTest extends UnitTestCase { */ protected $state; - /** - * The mocked content negotiator. - * - * @var \Drupal\Core\ContentNegotiation|\PHPUnit_Framework_MockObject_MockObject - */ - protected $negotiation; - /** * The tested preloader. * @@ -54,10 +47,7 @@ class RoutePreloaderTest extends UnitTestCase { protected function setUp() { $this->routeProvider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface'); $this->state = $this->getMock('\Drupal\Core\State\StateInterface'); - $this->negotiation = $this->getMockBuilder('\Drupal\Core\ContentNegotiation') - ->disableOriginalConstructor() - ->getMock(); - $this->preloader = new RoutePreloader($this->routeProvider, $this->state, $this->negotiation); + $this->preloader = new RoutePreloader($this->routeProvider, $this->state); } /** @@ -136,12 +126,10 @@ class RoutePreloaderTest extends UnitTestCase { ->disableOriginalConstructor() ->getMock(); $request = new Request(); + $request->setRequestFormat('non-html'); $event->expects($this->any()) ->method('getRequest') ->will($this->returnValue($request)); - $this->negotiation->expects($this->once()) - ->method('getContentType') - ->will($this->returnValue('non-html')); $this->routeProvider->expects($this->never()) ->method('getRoutesByNames'); @@ -159,12 +147,10 @@ class RoutePreloaderTest extends UnitTestCase { ->disableOriginalConstructor() ->getMock(); $request = new Request(); + $request->setRequestFormat('html'); $event->expects($this->any()) ->method('getRequest') ->will($this->returnValue($request)); - $this->negotiation->expects($this->once()) - ->method('getContentType') - ->will($this->returnValue('html')); $this->routeProvider->expects($this->once()) ->method('getRoutesByNames')