diff --git a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php index 251b920..5bb9564 100644 --- a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php @@ -123,7 +123,7 @@ public function onRespond(FilterResponseEvent $event) { $response->headers->set('X-Drupal-Cache-Contexts', implode(' ', $this->cacheContextsManager->optimizeTokens($response_cacheability->getCacheContexts()))); } - $is_cacheable = ($this->requestPolicy->check($request) === RequestPolicyInterface::ALLOW) && ($this->responsePolicy->check($response, $request) !== ResponsePolicyInterface::DENY); + $is_cacheable = ($response instanceof CacheableResponseInterface) && ($this->requestPolicy->check($request) === RequestPolicyInterface::ALLOW) && ($this->responsePolicy->check($response, $request) !== ResponsePolicyInterface::DENY); // Add headers necessary to specify whether the response should be cached by // proxies and/or the browser. diff --git a/core/modules/page_cache/src/Tests/PageCacheTest.php b/core/modules/page_cache/src/Tests/PageCacheTest.php index e97a1b4..f4bbd89 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTest.php @@ -204,7 +204,7 @@ function testPageCache() { // Fill the cache. $this->drupalGet('system-test/set-header', array('query' => array('name' => 'Foo', 'value' => 'bar'))); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.'); - $this->assertEqual(strtolower($this->drupalGetHeader('Vary')), 'cookie,accept-encoding', 'Vary header was sent.'); + $this->assertEqual(strtolower($this->drupalGetHeader('Vary')), 'cookie', 'Vary header was sent.'); // Symfony's Response logic determines a specific order for the subvalues // of the Cache-Control header, even if they are explicitly passed in to // the response header bag in a different order. @@ -215,7 +215,7 @@ function testPageCache() { // Check cache. $this->drupalGet('system-test/set-header', array('query' => array('name' => 'Foo', 'value' => 'bar'))); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', 'Page was cached.'); - $this->assertEqual(strtolower($this->drupalGetHeader('Vary')), 'cookie,accept-encoding', 'Vary: Cookie header was sent.'); + $this->assertEqual(strtolower($this->drupalGetHeader('Vary')), 'cookie', 'Vary: Cookie header was sent.'); $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'max-age=300, public', 'Cache-Control header was sent.'); $this->assertEqual($this->drupalGetHeader('Expires'), 'Sun, 19 Nov 1978 05:00:00 GMT', 'Expires header was sent.'); $this->assertEqual($this->drupalGetHeader('Foo'), 'bar', 'Custom header was sent.'); @@ -224,7 +224,7 @@ function testPageCache() { $this->drupalGet('system-test/set-header', array('query' => array('name' => 'Expires', 'value' => 'Fri, 19 Nov 2008 05:00:00 GMT'))); $this->assertEqual($this->drupalGetHeader('Expires'), 'Fri, 19 Nov 2008 05:00:00 GMT', 'Default header was replaced.'); $this->drupalGet('system-test/set-header', array('query' => array('name' => 'Vary', 'value' => 'User-Agent'))); - $this->assertEqual(strtolower($this->drupalGetHeader('Vary')), 'user-agent,accept-encoding', 'Default header was replaced.'); + $this->assertEqual(strtolower($this->drupalGetHeader('Vary')), 'user-agent', 'Default header was replaced.'); // Check that authenticated users bypass the cache. $user = $this->drupalCreateUser(); @@ -406,4 +406,35 @@ public function testFormImmutability() { $this->assertText("Immutable: FALSE", "Form is not immutable,"); } + /** + * Tests the difference between having a controller return a Response object + * versus returning a CacheableReponse + */ + public function testCacheableResponseResponses() { + $config = $this->config('system.performance'); + $config->set('cache.page.max_age', 300); + $config->save(); + + // Try to fill the cache. + $this->drupalGet('/system-test/respond-reponse'); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.'); + $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'must-revalidate, no-cache, post-check=0, pre-check=0, private', 'Cache-Control header was sent'); + + // still not cached, uncacheable response + $this->drupalGet('/system-test/respond-reponse'); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.'); + $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'must-revalidate, no-cache, post-check=0, pre-check=0, private', 'Cache-Control header was sent'); + + // Try to fill the cache. + $this->drupalGet('/system-test/respond-cacheable-reponse'); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.'); + $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'max-age=300, public', 'Cache-Control header was sent.'); + + // still not cached, uncacheable response + $this->drupalGet('/system-test/respond-cacheable-reponse'); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', 'Page was cached.'); + $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'max-age=300, public', 'Cache-Control header was sent.'); + + } + } diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php index ee4b890..78b9ecb 100644 --- a/core/modules/rest/src/RequestHandler.php +++ b/core/modules/rest/src/RequestHandler.php @@ -7,6 +7,7 @@ namespace Drupal\rest; +use Drupal\Core\Cache\CacheableResponse; use Drupal\Core\Render\RenderContext; use Drupal\Core\Routing\RouteMatchInterface; use Symfony\Component\DependencyInjection\ContainerAwareInterface; @@ -67,7 +68,7 @@ public function handle(RouteMatchInterface $route_match, Request $request) { catch (UnexpectedValueException $e) { $error['error'] = $e->getMessage(); $content = $serializer->serialize($error, $format); - return new Response($content, 400, array('Content-Type' => $request->getMimeType($format))); + return new CacheableResponse($content, 400, array('Content-Type' => $request->getMimeType($format))); } } else { @@ -100,7 +101,7 @@ public function handle(RouteMatchInterface $route_match, Request $request) { // Add the default content type, but only if the headers from the // exception have not specified it already. $headers = $e->getHeaders() + array('Content-Type' => $request->getMimeType($format)); - return new Response($content, $e->getStatusCode(), $headers); + return new CacheableResponse($content, $e->getStatusCode(), $headers); } // Serialize the outgoing data for the response, if available. diff --git a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php index 5ac7ca0..9847adf 100644 --- a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php +++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php @@ -8,6 +8,7 @@ namespace Drupal\system_test\Controller; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\CacheableResponse; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Render\RendererInterface; use Drupal\Core\Session\AccountInterface; @@ -241,13 +242,21 @@ public function authorizeInit($page_title) { */ public function setHeader(Request $request) { $query = $request->query->all(); - $response = new Response(); + $response = new CacheableResponse(); $response->headers->set($query['name'], $query['value']); $response->setContent($this->t('The following header was set: %name: %value', array('%name' => $query['name'], '%value' => $query['value']))); return $response; } + public function respondWithReponse(Request $request) { + return new Response('test'); + } + + public function respondWithCacheableReponse(Request $request) { + return new CacheableResponse('test'); + } + /** * A simple page callback which adds a register shutdown function. */ diff --git a/core/modules/system/tests/modules/system_test/system_test.routing.yml b/core/modules/system/tests/modules/system_test/system_test.routing.yml index ecb9921..94a1bef 100644 --- a/core/modules/system/tests/modules/system_test/system_test.routing.yml +++ b/core/modules/system/tests/modules/system_test/system_test.routing.yml @@ -129,3 +129,17 @@ system_test.permission_dependent_route_access: _controller: '\Drupal\system_test\Controller\SystemTestController::mainContentFallback' requirements: _permission: 'pet llamas' + +system_test.respond_response: + path: '/system-test/respond-reponse' + defaults: + _controller: '\Drupal\system_test\Controller\SystemTestController::respondWithReponse' + requirements: + _access: 'TRUE' + +system_test.respond_cacheable_response: + path: '/system-test/respond-cacheable-reponse' + defaults: + _controller: '\Drupal\system_test\Controller\SystemTestController::respondWithCacheableReponse' + requirements: + _access: 'TRUE'