diff --git a/core/modules/rest/src/ModifiedResourceResponse.php b/core/modules/rest/src/ModifiedResourceResponse.php new file mode 100644 index 00000000000..8d88ef23968 --- /dev/null +++ b/core/modules/rest/src/ModifiedResourceResponse.php @@ -0,0 +1,34 @@ +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 6ec5f265ab6..b4e78a461c6 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\EntityInterface; use Drupal\Core\Entity\EntityStorageException; use Drupal\rest\Plugin\ResourceBase; use Drupal\rest\ResourceResponse; +use Drupal\rest\ModifiedResourceResponse; 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\ModifiedResourceResponse * The response containing the entity with its accessible fields. * * @throws \Symfony\Component\HttpKernel\Exception\HttpException @@ -108,11 +109,10 @@ class EntityResource extends ResourceBase { $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 ModifiedResourceResponse($entity, 201, ['Location' => $url->getGeneratedUrl()]); return $response; } catch (EntityStorageException $e) { @@ -168,7 +168,7 @@ class EntityResource extends ResourceBase { $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 ModifiedResourceResponse(NULL, 204); } catch (EntityStorageException $e) { throw new HttpException(500, 'Internal Server Error', $e); @@ -195,7 +195,7 @@ class EntityResource extends ResourceBase { $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 ModifiedResourceResponse(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 8e0cd74ee69..2aa3673ba9b 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 @@ class RequestHandler implements ContainerAwareInterface { return new Response($content, $e->getStatusCode(), $headers); } - return $response instanceof ResourceResponse ? + return $response instanceof ResourceResponseInterface ? $this->renderResponse($request, $response, $serializer, $format) : $response; } @@ -124,7 +125,7 @@ class RequestHandler implements ContainerAwareInterface { * * @param \Symfony\Component\HttpFoundation\Request $request * The request object. - * @param \Drupal\rest\ResourceResponse $response + * @param \Drupal\rest\ResourceResponseInterface $response * The response from the REST resource. * @param \Symfony\Component\Serializer\SerializerInterface $serializer * The serializer to use. @@ -137,22 +138,29 @@ class RequestHandler implements ContainerAwareInterface { * @todo Add test coverage for language negotiation contexts in * https://www.drupal.org/node/2135829. */ - protected function renderResponse(Request $request, ResourceResponse $response, SerializerInterface $serializer, $format) { + protected function renderResponse(Request $request, ResourceResponseInterface $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 20a7e78e8d7..d4279a4c97c 100644 --- a/core/modules/rest/src/ResourceResponse.php +++ b/core/modules/rest/src/ResourceResponse.php @@ -13,17 +13,13 @@ use Symfony\Component\HttpFoundation\Response; * our response data. $content implies that the provided data must either be a * string or an object with a __toString() method, which is not a requirement * for data used here. + * + * @see \Drupal\rest\ModifiedResourceResponse */ -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. @@ -40,14 +36,4 @@ class ResourceResponse extends Response implements CacheableResponseInterface { 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 00000000000..e52cf517bb3 --- /dev/null +++ b/core/modules/rest/src/ResourceResponseInterface.php @@ -0,0 +1,18 @@ +responseData; + } + +} diff --git a/core/modules/rest/src/Tests/PageCacheTest.php b/core/modules/rest/src/Tests/PageCacheTest.php index 1958fdc2938..0ce66da2958 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 @@ namespace Drupal\rest\Tests; */ class PageCacheTest extends RESTTestBase { + use AssertPageCacheContextsAndTagsTrait; + /** * Modules to install. * @@ -16,22 +21,59 @@ 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']); - // Create an entity programmatically. + $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 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 @@ class PageCacheTest extends RESTTestBase { $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 @@ class PageCacheTest extends RESTTestBase { // 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.'); + } } }