Commit 83c181f0 authored by catch's avatar catch
Browse files

Issue #3072076 by JordanDukart, logickal, gabesullice, cgoffin,...

Issue #3072076 by JordanDukart, logickal, gabesullice, cgoffin, patrickfweston, Wim Leers, alexpott, scuba_fly, larowlan, nehiryeli, xjm, borisson_: JSON:API returns a CacheableResponseInterface instance for non-cacheable methods; causes unnecessary exceptions
parent f0d27d2f
<?php
namespace Drupal\jsonapi;
use Drupal\Core\Cache\CacheableResponseInterface;
use Drupal\Core\Cache\CacheableResponseTrait;
/**
* Extends ResourceResponse with cacheability.
*
* We want to have the same functionality for both responses that are cacheable
* and those that are not. This response class should be used in all instances
* where the response is expected to be cacheable.
*
* @internal JSON:API maintains no PHP API since its API is the HTTP API. This
* class may change at any time and this will break any dependencies on it.
*
* @see https://www.drupal.org/project/drupal/issues/3032787
* @see jsonapi.api.php
*/
class CacheableResourceResponse extends ResourceResponse implements CacheableResponseInterface {
use CacheableResponseTrait;
}
......@@ -6,6 +6,7 @@
use Drupal\Component\Datetime\TimeInterface;
use Drupal\Component\Serialization\Json;
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Cache\CacheableResponseInterface;
use Drupal\Core\Config\Entity\ConfigEntityInterface;
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Entity\EntityFieldManagerInterface;
......@@ -26,6 +27,7 @@
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Url;
use Drupal\jsonapi\Access\EntityAccessChecker;
use Drupal\jsonapi\CacheableResourceResponse;
use Drupal\jsonapi\Context\FieldResolver;
use Drupal\jsonapi\Entity\EntityValidationTrait;
use Drupal\jsonapi\Access\TemporaryQueryGuard;
......@@ -279,7 +281,6 @@ public function createIndividual(ResourceType $resource_type, Request $request)
// we should send "Location" header to the frontend.
if ($resource_type->isLocatable()) {
$url = $resource_object->toUrl()->setAbsolute()->toString(TRUE);
$response->addCacheableDependency($url);
$response->headers->set('Location', $url->getGeneratedUrl());
}
......@@ -536,7 +537,9 @@ function (EntityInterface $entity) {
// $response does not contain the entity list cache tag. We add the
// cacheable metadata for the finite list of entities in the relationship.
$response->addCacheableDependency($entity);
if ($response instanceof CacheableResponseInterface) {
$response->addCacheableDependency($entity);
}
return $response;
}
......@@ -567,7 +570,9 @@ public function getRelationship(ResourceType $resource_type, FieldableEntityInte
$relationship = Relationship::createFromEntityReferenceField($resource_object, $field_list);
$response = $this->buildWrappedResponse($relationship, $request, $this->getIncludes($request, $resource_object), $response_code);
// Add the host entity as a cacheable dependency.
$response->addCacheableDependency($entity);
if ($response instanceof CacheableResponseInterface) {
$response->addCacheableDependency($entity);
}
return $response;
}
......@@ -994,7 +999,11 @@ protected function buildWrappedResponse(TopLevelDataInterface $data, Request $re
$self_link = new Link(new CacheableMetadata(), self::getRequestLink($request), 'self');
$links = $links->withLink('self', $self_link);
}
$response = new ResourceResponse(new JsonApiDocumentTopLevel($data, $includes, $links, $meta), $response_code, $headers);
$document = new JsonApiDocumentTopLevel($data, $includes, $links, $meta);
if (!$request->isMethodCacheable()) {
return new ResourceResponse($document, $response_code, $headers);
}
$response = new CacheableResourceResponse($document, $response_code, $headers);
$cacheability = (new CacheableMetadata())->addCacheContexts([
// Make sure that different sparse fieldsets are cached differently.
'url.query_args:fields',
......
......@@ -6,12 +6,12 @@
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Url;
use Drupal\jsonapi\CacheableResourceResponse;
use Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel;
use Drupal\jsonapi\JsonApiResource\LinkCollection;
use Drupal\jsonapi\JsonApiResource\NullIncludedData;
use Drupal\jsonapi\JsonApiResource\Link;
use Drupal\jsonapi\JsonApiResource\ResourceObjectData;
use Drupal\jsonapi\ResourceResponse;
use Drupal\jsonapi\ResourceType\ResourceType;
use Drupal\jsonapi\ResourceType\ResourceTypeRepositoryInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
......@@ -123,7 +123,7 @@ public function index() {
}
}
$response = new ResourceResponse(new JsonApiDocumentTopLevel(new ResourceObjectData([]), new NullIncludedData(), $urls, $meta));
$response = new CacheableResourceResponse(new JsonApiDocumentTopLevel(new ResourceObjectData([]), new NullIncludedData(), $urls, $meta));
return $response->addCacheableDependency($cacheability);
}
......
......@@ -2,6 +2,7 @@
namespace Drupal\jsonapi\EventSubscriber;
use Drupal\jsonapi\CacheableResourceResponse;
use Drupal\jsonapi\JsonApiResource\ErrorCollection;
use Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel;
use Drupal\jsonapi\JsonApiResource\LinkCollection;
......@@ -58,8 +59,14 @@ public function onException(ExceptionEvent $event) {
protected function setEventResponse(ExceptionEvent $event, $status) {
/* @var \Symfony\Component\HttpKernel\Exception\HttpException $exception */
$exception = $event->getThrowable();
$response = new ResourceResponse(new JsonApiDocumentTopLevel(new ErrorCollection([$exception]), new NullIncludedData(), new LinkCollection([])), $exception->getStatusCode(), $exception->getHeaders());
$response->addCacheableDependency($exception);
$document = new JsonApiDocumentTopLevel(new ErrorCollection([$exception]), new NullIncludedData(), new LinkCollection([]));
if ($event->getRequest()->isMethodCacheable()) {
$response = new CacheableResourceResponse($document, $exception->getStatusCode(), $exception->getHeaders());
$response->addCacheableDependency($exception);
}
else {
$response = new ResourceResponse($document, $exception->getStatusCode(), $exception->getHeaders());
}
$event->setResponse($response);
}
......
......@@ -120,8 +120,10 @@ protected function renderResponseBody(Request $request, ResourceResponse $respon
$jsonapi_doc_object = $serializer->normalize($data, $format, $context);
// Having just normalized the data, we can associate its cacheability with
// the response object.
assert($jsonapi_doc_object instanceof CacheableNormalization);
$response->addCacheableDependency($jsonapi_doc_object);
if ($response instanceof CacheableResponseInterface) {
assert($jsonapi_doc_object instanceof CacheableNormalization);
$response->addCacheableDependency($jsonapi_doc_object);
}
// Finally, encode the normalized data (JSON:API's encoder rasterizes it
// automatically).
$response->setContent($serializer->encode($jsonapi_doc_object->getNormalization(), $format));
......
......@@ -2,8 +2,6 @@
namespace Drupal\jsonapi;
use Drupal\Core\Cache\CacheableResponseInterface;
use Drupal\Core\Cache\CacheableResponseTrait;
use Symfony\Component\HttpFoundation\Response;
/**
......@@ -22,9 +20,7 @@
*
* @see \Drupal\rest\ModifiedResourceResponse
*/
class ResourceResponse extends Response implements CacheableResponseInterface {
use CacheableResponseTrait;
class ResourceResponse extends Response {
/**
* Response data that should be serialized.
......
<?php
/**
* @file
* Contains hook implementations for testing the JSON:API module.
*
* @see: https://www.drupal.org/project/drupal/issues/3072076.
*/
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Url;
/**
* Implements hook_entity_presave().
*/
function jsonapi_test_non_cacheable_methods_entity_presave(EntityInterface $entity) {
Url::fromRoute('<front>')->toString();
}
/**
* Implements hook_entity_predelete().
*/
function jsonapi_test_non_cacheable_methods_entity_predelete(EntityInterface $entity) {
Url::fromRoute('<front>')->toString();
}
......@@ -1299,4 +1299,67 @@ public function testAliasedFieldsWithVirtualRelationships() {
$this->assertSame(200, $response->getStatusCode());
}
/**
* Tests that caching isn't happening for non-cacheable methods.
*
* @see https://www.drupal.org/project/drupal/issues/3072076
*/
public function testNonCacheableMethods() {
$this->container->get('module_installer')->install([
'jsonapi_test_non_cacheable_methods',
], TRUE);
$this->config('jsonapi.settings')->set('read_only', FALSE)->save(TRUE);
$node = Node::create([
'type' => 'article',
'title' => 'Llama non-cacheable',
]);
$node->save();
$user = $this->drupalCreateUser([
'access content',
'create article content',
'edit any article content',
'delete any article content',
]);
$base_request_options = [
RequestOptions::HEADERS => [
'Content-Type' => 'application/vnd.api+json',
'Accept' => 'application/vnd.api+json',
],
RequestOptions::AUTH => [$user->getAccountName(), $user->pass_raw],
];
$methods = [
'HEAD',
'GET',
'PATCH',
'DELETE',
];
$non_post_request_options = $base_request_options + [
RequestOptions::JSON => [
'data' => [
'type' => 'node--article',
'id' => $node->uuid(),
],
],
];
foreach ($methods as $method) {
$response = $this->request($method, Url::fromUri('internal:/jsonapi/node/article/' . $node->uuid()), $non_post_request_options);
$this->assertSame($method === 'DELETE' ? 204 : 200, $response->getStatusCode());
}
$post_request_options = $base_request_options + [
RequestOptions::JSON => [
'data' => [
'type' => 'node--article',
'attributes' => [
'title' => 'Llama non-cacheable',
],
],
],
];
$response = $this->request('POST', Url::fromUri('internal:/jsonapi/node/article'), $post_request_options);
$this->assertSame(201, $response->getStatusCode());
}
}
......@@ -11,8 +11,8 @@
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\RevisionableInterface;
use Drupal\Core\Url;
use Drupal\jsonapi\CacheableResourceResponse;
use Drupal\jsonapi\Normalizer\HttpExceptionNormalizer;
use Drupal\jsonapi\ResourceResponse;
use Psr\Http\Message\ResponseInterface;
/**
......@@ -38,7 +38,7 @@ trait ResourceResponseTestTrait {
* be deduced from the number of responses, because a multiple cardinality
* field may have only one value.
*
* @return \Drupal\jsonapi\ResourceResponse
* @return \Drupal\jsonapi\CacheableResourceResponse
* The merged ResourceResponse.
*/
protected static function toCollectionResourceResponse(array $responses, $self_link, $is_multiple) {
......@@ -103,7 +103,7 @@ protected static function toCollectionResourceResponse(array $responses, $self_l
// individual resources in those collections, which means any '4xx-response'
// cache tags on the individual responses should also be omitted.
$merged_cacheability->setCacheTags(array_diff($merged_cacheability->getCacheTags(), ['4xx-response']));
return (new ResourceResponse($merged_document, 200))->addCacheableDependency($merged_cacheability);
return (new CacheableResourceResponse($merged_document, 200))->addCacheableDependency($merged_cacheability);
}
/**
......@@ -203,7 +203,7 @@ protected function getExpectedIncludedResourceResponse(array $include_paths, arr
$basic_cacheability = (new CacheableMetadata())
->addCacheTags($this->getExpectedCacheTags())
->addCacheContexts($this->getExpectedCacheContexts());
return static::decorateExpectedResponseForIncludedFields(new ResourceResponse($individual_document), $resource_data['responses'])
return static::decorateExpectedResponseForIncludedFields(new CacheableResourceResponse($individual_document), $resource_data['responses'])
->addCacheableDependency($basic_cacheability);
}
......@@ -230,7 +230,7 @@ protected static function toResourceResponses(array $responses) {
* @param \Psr\Http\Message\ResponseInterface $response
* A PSR response to be mapped.
*
* @return \Drupal\jsonapi\ResourceResponse
* @return \Drupal\jsonapi\CacheableResourceResponse
* The ResourceResponse.
*/
protected static function toResourceResponse(ResponseInterface $response) {
......@@ -245,7 +245,7 @@ protected static function toResourceResponse(ResponseInterface $response) {
$cacheability->setCacheMaxAge(($dynamic_cache[0] === 'UNCACHEABLE' && $response->getStatusCode() < 400) ? 0 : Cache::PERMANENT);
}
$related_document = Json::decode($response->getBody());
$resource_response = new ResourceResponse($related_document, $response->getStatusCode());
$resource_response = new CacheableResourceResponse($related_document, $response->getStatusCode());
return $resource_response->addCacheableDependency($cacheability);
}
......@@ -496,7 +496,7 @@ protected function getResponses(array $links, array $request_options) {
* (optional) Document pointer for the JSON:API error object. FALSE to omit
* the pointer.
*
* @return \Drupal\jsonapi\ResourceResponse
* @return \Drupal\jsonapi\CacheableResourceResponse
* The forbidden ResourceResponse.
*/
protected static function getAccessDeniedResponse(EntityInterface $entity, AccessResultInterface $access, Url $via_link, $relationship_field_name = NULL, $detail = NULL, $pointer = NULL) {
......@@ -519,7 +519,7 @@ protected static function getAccessDeniedResponse(EntityInterface $entity, Acces
$error['links']['via']['href'] = $via_link->setAbsolute()->toString();
}
return (new ResourceResponse([
return (new CacheableResourceResponse([
'jsonapi' => static::$jsonApiMember,
'errors' => [$error],
], 403))
......@@ -536,7 +536,7 @@ protected static function getAccessDeniedResponse(EntityInterface $entity, Acces
* @param string $self_link
* The self link for collection ResourceResponse.
*
* @return \Drupal\jsonapi\ResourceResponse
* @return \Drupal\jsonapi\CacheableResourceResponse
* The empty collection ResourceResponse.
*/
protected function getEmptyCollectionResponse($cardinality, $self_link) {
......@@ -549,7 +549,7 @@ protected function getEmptyCollectionResponse($cardinality, $self_link) {
'url.site',
], $this->entity->getEntityType()->isRevisionable() ? ['url.query_args:resourceVersion'] : []);
$cacheability = (new CacheableMetadata())->addCacheContexts($cache_contexts)->addCacheTags(['http_response']);
return (new ResourceResponse([
return (new CacheableResourceResponse([
// Empty to-one relationships should be NULL and empty to-many
// relationships should be an empty array.
'data' => $cardinality === 1 ? NULL : [],
......
......@@ -27,6 +27,7 @@
use Drupal\Core\Url;
use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\jsonapi\CacheableResourceResponse;
use Drupal\jsonapi\JsonApiResource\LinkCollection;
use Drupal\jsonapi\JsonApiResource\NullIncludedData;
use Drupal\jsonapi\JsonApiResource\Link;
......@@ -1253,7 +1254,7 @@ protected function getExpectedCollectionResponse(array $collection, $self_link,
$cacheability = static::getExpectedCollectionCacheability($this->account, $collection, NULL, $filtered);
$cacheability->setCacheMaxAge($merged_response->getCacheableMetadata()->getCacheMaxAge());
$collection_response = new ResourceResponse($merged_document);
$collection_response = new CacheableResourceResponse($merged_document);
$collection_response->addCacheableDependency($cacheability);
if (is_null($included_paths)) {
......@@ -1668,7 +1669,7 @@ protected function doTestRelationshipMutation(array $request_options) {
* @param \Drupal\Core\Entity\EntityInterface|null $entity
* (optional) The entity for which to get expected relationship response.
*
* @return \Drupal\jsonapi\ResourceResponse
* @return \Drupal\jsonapi\CacheableResourceResponse
* The expected ResourceResponse.
*/
protected function getExpectedGetRelationshipResponse($relationship_field_name, EntityInterface $entity = NULL) {
......@@ -1693,7 +1694,7 @@ protected function getExpectedGetRelationshipResponse($relationship_field_name,
->addCacheableDependency($entity)
->addCacheableDependency($access);
$status_code = isset($expected_document['errors'][0]['status']) ? $expected_document['errors'][0]['status'] : 200;
$resource_response = new ResourceResponse($expected_document, $status_code);
$resource_response = new CacheableResourceResponse($expected_document, $status_code);
$resource_response->addCacheableDependency($expected_cacheability);
return $resource_response;
}
......@@ -1840,7 +1841,7 @@ protected function getExpectedRelatedResponse($relationship_field_name, array $r
$cacheability = (new CacheableMetadata())->addCacheContexts($cache_contexts)->addCacheTags(['http_response']);
$related_response = isset($relationship_document['errors'])
? $relationship_response
: (new ResourceResponse(static::getEmptyCollectionResponse(!is_null($relationship_document['data']), $self_link)->getResponseData()))->addCacheableDependency($cacheability);
: (new CacheableResourceResponse(static::getEmptyCollectionResponse(!is_null($relationship_document['data']), $self_link)->getResponseData()))->addCacheableDependency($cacheability);
}
else {
$is_to_one_relationship = static::isResourceIdentifier($relationship_document['data']);
......@@ -3171,15 +3172,15 @@ public function testRevisions() {
* the expected cacheability for those includes. It does so based of responses
* from the related routes for individual relationships.
*
* @param \Drupal\jsonapi\ResourceResponse $expected_response
* @param \Drupal\jsonapi\CacheableResourceResponse $expected_response
* The expected ResourceResponse.
* @param \Drupal\jsonapi\ResourceResponse[] $related_responses
* The related ResourceResponses, keyed by relationship field names.
*
* @return \Drupal\jsonapi\ResourceResponse
* @return \Drupal\jsonapi\CacheableResourceResponse
* The decorated ResourceResponse.
*/
protected static function decorateExpectedResponseForIncludedFields(ResourceResponse $expected_response, array $related_responses) {
protected static function decorateExpectedResponseForIncludedFields(CacheableResourceResponse $expected_response, array $related_responses) {
$expected_document = $expected_response->getResponseData();
$expected_cacheability = $expected_response->getCacheableMetadata();
foreach ($related_responses as $related_response) {
......@@ -3206,17 +3207,17 @@ protected static function decorateExpectedResponseForIncludedFields(ResourceResp
}
}
}
return (new ResourceResponse($expected_document))->addCacheableDependency($expected_cacheability);
return (new CacheableResourceResponse($expected_document))->addCacheableDependency($expected_cacheability);
}
/**
* Gets the expected individual ResourceResponse for GET.
*
* @return \Drupal\jsonapi\ResourceResponse
* @return \Drupal\jsonapi\CacheableResourceResponse
* The expected individual ResourceResponse.
*/
protected function getExpectedGetIndividualResourceResponse($status_code = 200) {
$resource_response = new ResourceResponse($this->getExpectedDocument(), $status_code);
$resource_response = new CacheableResourceResponse($this->getExpectedDocument(), $status_code);
$cacheability = new CacheableMetadata();
$cacheability->setCacheContexts($this->getExpectedCacheContexts());
$cacheability->setCacheTags($this->getExpectedCacheTags());
......
......@@ -3,6 +3,7 @@
namespace Drupal\Tests\jsonapi\Kernel\Controller;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
use Drupal\jsonapi\CacheableResourceResponse;
use Drupal\jsonapi\ResourceType\ResourceType;
use Drupal\jsonapi\JsonApiResource\Data;
use Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel;
......@@ -200,6 +201,7 @@ public function testGetPagedCollection() {
$response = $entity_resource->getCollection($resource_type, $request);
// Assertions.
$this->assertInstanceOf(CacheableResourceResponse::class, $response);
$this->assertInstanceOf(JsonApiDocumentTopLevel::class, $response->getResponseData());
$this->assertInstanceOf(Data::class, $response->getResponseData()->getData());
$data = $response->getResponseData()->getData();
......@@ -220,6 +222,7 @@ public function testGetEmptyCollection() {
$response = $this->entityResource->getCollection($resource_type, $request);
// Assertions.
$this->assertInstanceOf(CacheableResourceResponse::class, $response);
$this->assertInstanceOf(JsonApiDocumentTopLevel::class, $response->getResponseData());
$this->assertInstanceOf(Data::class, $response->getResponseData()->getData());
$this->assertEquals(0, $response->getResponseData()->getData()->count());
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment