diff --git a/core/modules/rest/src/NonCacheableResourceResponse.php b/core/modules/rest/src/NonCacheableResourceResponse.php new file mode 100644 index 0000000..f793899 --- /dev/null +++ b/core/modules/rest/src/NonCacheableResourceResponse.php @@ -0,0 +1,32 @@ +responseData = $data; + parent::__construct('', $status, $headers); + } + +} diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 6ec5f26..8db3cdb 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -6,6 +6,7 @@ use Drupal\Core\Entity\EntityStorageException; use Drupal\rest\Plugin\ResourceBase; use Drupal\rest\ResourceResponse; +use Drupal\rest\NonCacheableResourceResponse; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\HttpException; @@ -34,7 +35,7 @@ class EntityResource extends ResourceBase { * @param \Drupal\Core\Entity\EntityInterface $entity * The entity object. * - * @return \Drupal\rest\ResourceResponse + * @return \Drupal\rest\NonCacheableResourceResponse * The response containing the entity with its accessible fields. * * @throws \Symfony\Component\HttpKernel\Exception\HttpException @@ -108,11 +109,10 @@ public function post(EntityInterface $entity = NULL) { $this->logger->notice('Created entity %type with ID %id.', array('%type' => $entity->getEntityTypeId(), '%id' => $entity->id())); // 201 Created responses return the newly created entity in the response - // body. + // body. These responses are not cacheable, so we add no cacheability + // metadata here. $url = $entity->urlInfo('canonical', ['absolute' => TRUE])->toString(TRUE); - $response = new ResourceResponse($entity, 201, ['Location' => $url->getGeneratedUrl()]); - // Responses after creating an entity are not cacheable, so we add no - // cacheability metadata here. + $response = new NonCacheableResourceResponse($entity, 201, ['Location' => $url->getGeneratedUrl()]); return $response; } catch (EntityStorageException $e) { @@ -168,7 +168,7 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity $this->logger->notice('Updated entity %type with ID %id.', array('%type' => $original_entity->getEntityTypeId(), '%id' => $original_entity->id())); // Update responses have an empty body. - return new ResourceResponse(NULL, 204); + return new NonCacheableResourceResponse(NULL, 204); } catch (EntityStorageException $e) { throw new HttpException(500, 'Internal Server Error', $e); @@ -195,7 +195,7 @@ public function delete(EntityInterface $entity) { $this->logger->notice('Deleted entity %type with ID %id.', array('%type' => $entity->getEntityTypeId(), '%id' => $entity->id())); // Delete responses have an empty body. - return new ResourceResponse(NULL, 204); + return new NonCacheableResourceResponse(NULL, 204); } catch (EntityStorageException $e) { throw new HttpException(500, 'Internal Server Error', $e); diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php index 8e0cd74..2d89a5f 100644 --- a/core/modules/rest/src/RequestHandler.php +++ b/core/modules/rest/src/RequestHandler.php @@ -2,6 +2,7 @@ namespace Drupal\rest; +use Drupal\Core\Cache\CacheableResponseInterface; use Drupal\Core\Render\RenderContext; use Drupal\Core\Routing\RouteMatchInterface; use Symfony\Component\DependencyInjection\ContainerAwareInterface; @@ -98,7 +99,7 @@ public function handle(RouteMatchInterface $route_match, Request $request) { return new Response($content, $e->getStatusCode(), $headers); } - return $response instanceof ResourceResponse ? + return $response instanceof ResourceResponseInterface ? $this->renderResponse($request, $response, $serializer, $format) : $response; } @@ -139,20 +140,27 @@ public function csrfToken() { */ protected function renderResponse(Request $request, ResourceResponse $response, SerializerInterface $serializer, $format) { $data = $response->getResponseData(); - $context = new RenderContext(); - $output = $this->container->get('renderer') - ->executeInRenderContext($context, function () use ($serializer, $data, $format) { - return $serializer->serialize($data, $format); - }); - $response->setContent($output); - if (!$context->isEmpty()) { - $response->addCacheableDependency($context->pop()); - } + if ($response instanceof CacheableResponseInterface) { + $context = new RenderContext(); + $output = $this->container->get('renderer') + ->executeInRenderContext($context, function () use ($serializer, $data, $format) { + return $serializer->serialize($data, $format); + }); + + if (!$context->isEmpty()) { + $response->addCacheableDependency($context->pop()); + } + + // Add rest settings config's cache tags. + $response->addCacheableDependency($this->container->get('config.factory') + ->get('rest.settings')); + } + else { + $output = $serializer->serialize($data, $format); + } + $response->setContent($output); $response->headers->set('Content-Type', $request->getMimeType($format)); - // Add rest settings config's cache tags. - $response->addCacheableDependency($this->container->get('config.factory') - ->get('rest.settings')); return $response; } diff --git a/core/modules/rest/src/ResourceResponse.php b/core/modules/rest/src/ResourceResponse.php index 20a7e78..77bf094 100644 --- a/core/modules/rest/src/ResourceResponse.php +++ b/core/modules/rest/src/ResourceResponse.php @@ -14,16 +14,10 @@ * string or an object with a __toString() method, which is not a requirement * for data used here. */ -class ResourceResponse extends Response implements CacheableResponseInterface { +class ResourceResponse extends Response implements CacheableResponseInterface, ResourceResponseInterface { use CacheableResponseTrait; - - /** - * Response data that should be serialized. - * - * @var mixed - */ - protected $responseData; + use ResourceResponseTrait; /** * Constructor for ResourceResponse objects. @@ -39,15 +33,4 @@ public function __construct($data = NULL, $status = 200, $headers = array()) { $this->responseData = $data; parent::__construct('', $status, $headers); } - - /** - * Returns response data that should be serialized. - * - * @return mixed - * Response data that should be serialized. - */ - public function getResponseData() { - return $this->responseData; - } - } diff --git a/core/modules/rest/src/ResourceResponseInterface.php b/core/modules/rest/src/ResourceResponseInterface.php new file mode 100644 index 0000000..e417157 --- /dev/null +++ b/core/modules/rest/src/ResourceResponseInterface.php @@ -0,0 +1,19 @@ +responseData; + } + +} diff --git a/core/modules/rest/src/Tests/PageCacheTest.php b/core/modules/rest/src/Tests/PageCacheTest.php index 1958fdc..0ce66da 100644 --- a/core/modules/rest/src/Tests/PageCacheTest.php +++ b/core/modules/rest/src/Tests/PageCacheTest.php @@ -2,6 +2,9 @@ namespace Drupal\rest\Tests; +use Drupal\Core\Url; +use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait; + /** * Tests page caching for REST GET requests. * @@ -9,6 +12,8 @@ */ class PageCacheTest extends RESTTestBase { + use AssertPageCacheContextsAndTagsTrait; + /** * Modules to install. * @@ -17,21 +22,58 @@ class PageCacheTest extends RESTTestBase { public static $modules = array('hal', 'rest', 'entity_test'); /** + * The 'serializer' service. + * + * @var \Symfony\Component\Serializer\Serializer + */ + protected $serializer; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + // Get the 'serializer' service. + $this->serializer = $this->container->get('serializer'); + } + + /** * Tests that configuration changes also clear the page cache. */ public function testConfigChangePageCache() { - $this->enableService('entity:entity_test', 'GET'); // Allow anonymous users to issue GET requests. - $permissions = $this->entityPermissions('entity_test', 'view'); - $permissions[] = 'restful get entity:entity_test'; - user_role_grant_permissions('anonymous', $permissions); + user_role_grant_permissions('anonymous', ['view test entity', 'restful get entity:entity_test']); + + $this->enableService('entity:entity_test', 'POST'); + $permissions = [ + 'administer entity_test content', + 'restful post entity:entity_test', + 'restful get entity:entity_test', + 'restful patch entity:entity_test', + 'restful delete entity:entity_test', + ]; + $account = $this->drupalCreateUser($permissions); - // Create an entity programmatically. + // Create an entity and test that the response from a post request is not + // cacheable. $entity = $this->entityCreate('entity_test'); $entity->set('field_test_text', 'custom cache tag value'); - $entity->save(); + $serialized = $this->serializer->serialize($entity, $this->defaultFormat); + // Log in for creating the entity. + $this->drupalLogin($account); + $this->httpRequest('entity/entity_test', 'POST', $serialized, $this->defaultMimeType); + $this->assertResponse(201); + + if ($this->getCacheHeaderValues('x-drupal-cache')) { + $this->fail('Post request is cached.'); + } + $this->drupalLogout(); + + $url = Url::fromUri('internal:/entity_test/1?_format=' . $this->defaultFormat); + // Read it over the REST API. - $this->httpRequest($entity->urlInfo()->setRouteParameter('_format', $this->defaultFormat), 'GET', NULL, $this->defaultMimeType); + $this->enableService('entity:entity_test', 'GET'); + $this->httpRequest($url, 'GET', NULL, $this->defaultMimeType); $this->assertResponse(200, 'HTTP response code is correct.'); $this->assertHeader('x-drupal-cache', 'MISS'); $this->assertCacheTag('config:rest.settings'); @@ -39,7 +81,7 @@ public function testConfigChangePageCache() { $this->assertCacheTag('entity_test_access:field_test_text'); // Read it again, should be page-cached now. - $this->httpRequest($entity->urlInfo()->setRouteParameter('_format', $this->defaultFormat), 'GET', NULL, $this->defaultMimeType); + $this->httpRequest($url, 'GET', NULL, $this->defaultMimeType); $this->assertResponse(200, 'HTTP response code is correct.'); $this->assertHeader('x-drupal-cache', 'HIT'); $this->assertCacheTag('config:rest.settings'); @@ -49,12 +91,48 @@ public function testConfigChangePageCache() { // Trigger a config save which should clear the page cache, so we should get // a cache miss now for the same request. $this->config('rest.settings')->save(); - $this->httpRequest($entity->urlInfo()->setRouteParameter('_format', $this->defaultFormat), 'GET', NULL, $this->defaultMimeType); + $this->httpRequest($url, 'GET', NULL, $this->defaultMimeType); $this->assertResponse(200, 'HTTP response code is correct.'); $this->assertHeader('x-drupal-cache', 'MISS'); $this->assertCacheTag('config:rest.settings'); $this->assertCacheTag('entity_test:1'); $this->assertCacheTag('entity_test_access:field_test_text'); + + // Log in for deleting / updating entity. + $this->drupalLogin($account); + + // Test that updating an entity is not cacheable. + $this->enableService('entity:entity_test', 'PATCH'); + + // Create a second stub entity for overwriting a field. + $patch_values['field_test_text'] = [0 => [ + 'value' => 'patched value', + 'format' => 'plain_text', + ]]; + $patch_entity = $this->container->get('entity_type.manager') + ->getStorage('entity_test') + ->create($patch_values); + // We don't want to overwrite the UUID. + $patch_entity->set('uuid', NULL); + $serialized = $this->container->get('serializer') + ->serialize($patch_entity, $this->defaultFormat); + + // Update the entity over the REST API. + $this->httpRequest($url, 'PATCH', $serialized, $this->defaultMimeType); + $this->assertResponse(204); + + if ($this->getCacheHeaderValues('x-drupal-cache')) { + $this->fail('Patch request is cached.'); + } + + // Test that the response from a delete request is not cacheable. + $this->enableService('entity:entity_test', 'DELETE'); + $this->httpRequest($url, 'DELETE'); + $this->assertResponse(204); + + if ($this->getCacheHeaderValues('x-drupal-cache')) { + $this->fail('Patch request is cached.'); + } } }