From fde6ee352ef791a67446908771865631d297de4b Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Thu, 7 Sep 2023 00:13:35 +0100 Subject: [PATCH] Issue #3379984 by kristiaanvandeneynde, bbrala, quietone: Refactor ResourceObjectNormalizationCacher to use VariationCache --- core/modules/jsonapi/jsonapi.services.yml | 7 +- .../ResourceObjectNormalizationCacher.php | 93 ++++++++++--------- .../jsonapi/tests/src/Functional/NodeTest.php | 17 +--- 3 files changed, 60 insertions(+), 57 deletions(-) diff --git a/core/modules/jsonapi/jsonapi.services.yml b/core/modules/jsonapi/jsonapi.services.yml index c989066498a7..e6900aa62885 100644 --- a/core/modules/jsonapi/jsonapi.services.yml +++ b/core/modules/jsonapi/jsonapi.services.yml @@ -50,7 +50,8 @@ services: jsonapi.normalization_cacher: class: Drupal\jsonapi\EventSubscriber\ResourceObjectNormalizationCacher calls: - - ['setRenderCache', ['@render_cache']] + - ['setVariationCache', ['@variation_cache.jsonapi_normalizations']] + - ['setRequestStack', ['@request_stack']] tags: - { name: event_subscriber } serializer.normalizer.content_entity.jsonapi: @@ -144,6 +145,10 @@ services: - { name: cache.bin } factory: ['@cache_factory', 'get'] arguments: [jsonapi_normalizations] + variation_cache.jsonapi_normalizations: + class: Drupal\Core\Cache\VariationCacheInterface + factory: ['@variation_cache_factory', 'get'] + arguments: [jsonapi_normalizations] # Route filter. jsonapi.route_filter.format_setter: diff --git a/core/modules/jsonapi/src/EventSubscriber/ResourceObjectNormalizationCacher.php b/core/modules/jsonapi/src/EventSubscriber/ResourceObjectNormalizationCacher.php index 1e801281487c..3a5b818dc5bb 100644 --- a/core/modules/jsonapi/src/EventSubscriber/ResourceObjectNormalizationCacher.php +++ b/core/modules/jsonapi/src/EventSubscriber/ResourceObjectNormalizationCacher.php @@ -3,9 +3,10 @@ namespace Drupal\jsonapi\EventSubscriber; use Drupal\Core\Cache\CacheableMetadata; -use Drupal\Core\Render\RenderCacheInterface; +use Drupal\Core\Cache\VariationCacheInterface; use Drupal\jsonapi\JsonApiResource\ResourceObject; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpKernel\Event\TerminateEvent; use Symfony\Component\HttpKernel\KernelEvents; @@ -14,7 +15,6 @@ * * @internal * @see \Drupal\jsonapi\Normalizer\ResourceObjectNormalizer::getNormalization() - * @todo Refactor once https://www.drupal.org/node/2551419 lands. */ class ResourceObjectNormalizationCacher implements EventSubscriberInterface { @@ -40,11 +40,18 @@ class ResourceObjectNormalizationCacher implements EventSubscriberInterface { const RESOURCE_CACHE_SUBSET_FIELDS = 'fields'; /** - * The render cache. + * The variation cache. * - * @var \Drupal\Core\Render\RenderCacheInterface + * @var \Drupal\Core\Cache\VariationCacheInterface */ - protected $renderCache; + protected $variationCache; + + /** + * The request stack. + * + * @var \Symfony\Component\HttpFoundation\RequestStack + */ + protected $requestStack; /** * The things to cache after the response has been sent. @@ -54,13 +61,23 @@ class ResourceObjectNormalizationCacher implements EventSubscriberInterface { protected $toCache = []; /** - * Sets the render cache service. + * Sets the variation cache. + * + * @param \Drupal\Core\Cache\VariationCacheInterface $variation_cache + * The variation cache. + */ + public function setVariationCache(VariationCacheInterface $variation_cache) { + $this->variationCache = $variation_cache; + } + + /** + * Sets the request stack. * - * @param \Drupal\Core\Render\RenderCacheInterface $render_cache - * The render cache. + * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack + * The request stack. */ - public function setRenderCache(RenderCacheInterface $render_cache) { - $this->renderCache = $render_cache; + public function setRequestStack(RequestStack $request_stack) { + $this->requestStack = $request_stack; } /** @@ -78,8 +95,14 @@ public function setRenderCache(RenderCacheInterface $render_cache) { * @see \Drupal\dynamic_page_cache\EventSubscriber\DynamicPageCacheSubscriber::renderArrayToResponse() */ public function get(ResourceObject $object) { - $cached = $this->renderCache->get(static::generateLookupRenderArray($object)); - return $cached ? $cached['#data'] : FALSE; + // @todo Investigate whether to cache POST and PATCH requests. + // @todo Follow up on https://www.drupal.org/project/drupal/issues/3381898. + if (!$this->requestStack->getCurrentRequest()->isMethodCacheable()) { + return FALSE; + } + + $cached = $this->variationCache->get($this->generateCacheKeys($object), new CacheableMetadata()); + return $cached ? $cached->data : FALSE; } /** @@ -122,52 +145,36 @@ public function onTerminate(TerminateEvent $event) { * The resource object for which to generate a cache item. * @param array $normalization_parts * The normalization parts to cache. - * - * @see \Drupal\dynamic_page_cache\EventSubscriber\DynamicPageCacheSubscriber::responseToRenderArray() - * @todo Refactor/remove once https://www.drupal.org/node/2551419 lands. */ protected function set(ResourceObject $object, array $normalization_parts) { - $base = static::generateLookupRenderArray($object); - $data_as_render_array = $base + [ - // The data we actually care about. - '#data' => $normalization_parts, - // Tell RenderCache to cache the #data property: the data we actually care - // about. - '#cache_properties' => ['#data'], - // These exist only to fulfill the requirements of the RenderCache, which - // is designed to work with render arrays only. We don't care about these. - '#markup' => '', - '#attached' => '', - ]; + // @todo Investigate whether to cache POST and PATCH requests. + // @todo Follow up on https://www.drupal.org/project/drupal/issues/3381898. + if (!$this->requestStack->getCurrentRequest()->isMethodCacheable()) { + return; + } // Merge the entity's cacheability metadata with that of the normalization - // parts, so that RenderCache can take care of cache redirects for us. - CacheableMetadata::createFromObject($object) + // parts, so that VariationCache can take care of cache redirects for us. + $cacheability = CacheableMetadata::createFromObject($object) ->merge(static::mergeCacheableDependencies($normalization_parts[static::RESOURCE_CACHE_SUBSET_BASE])) - ->merge(static::mergeCacheableDependencies($normalization_parts[static::RESOURCE_CACHE_SUBSET_FIELDS])) - ->applyTo($data_as_render_array); + ->merge(static::mergeCacheableDependencies($normalization_parts[static::RESOURCE_CACHE_SUBSET_FIELDS])); - $this->renderCache->set($data_as_render_array, $base); + $this->variationCache->set($this->generateCacheKeys($object), $normalization_parts, $cacheability, new CacheableMetadata()); } /** - * Generates a lookup render array for a normalization. + * Generates the cache keys for a normalization. * * @param \Drupal\jsonapi\JsonApiResource\ResourceObject $object - * The resource object for which to generate a cache item. + * The resource object for which to generate the cache keys. * - * @return array - * A render array for use with the RenderCache service. + * @return string[] + * The cache keys to pass to the variation cache. * * @see \Drupal\dynamic_page_cache\EventSubscriber\DynamicPageCacheSubscriber::$dynamicPageCacheRedirectRenderArray */ - protected static function generateLookupRenderArray(ResourceObject $object) { - return [ - '#cache' => [ - 'keys' => [$object->getResourceType()->getTypeName(), $object->getId(), $object->getLanguage()->getId()], - 'bin' => 'jsonapi_normalizations', - ], - ]; + protected static function generateCacheKeys(ResourceObject $object) { + return [$object->getResourceType()->getTypeName(), $object->getId(), $object->getLanguage()->getId()]; } /** diff --git a/core/modules/jsonapi/tests/src/Functional/NodeTest.php b/core/modules/jsonapi/tests/src/Functional/NodeTest.php index 4fb79a7c66e3..0f0f82b65fce 100644 --- a/core/modules/jsonapi/tests/src/Functional/NodeTest.php +++ b/core/modules/jsonapi/tests/src/Functional/NodeTest.php @@ -5,6 +5,7 @@ use Drupal\Component\Serialization\Json; use Drupal\Component\Utility\NestedArray; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Url; use Drupal\jsonapi\Normalizer\HttpExceptionNormalizer; use Drupal\jsonapi\Normalizer\Value\CacheableNormalization; @@ -386,12 +387,7 @@ protected function assertCacheableNormalizations(): void { $this->entity->save(); $uuid = $this->entity->uuid(); $language = $this->entity->language()->getId(); - $cache = \Drupal::service('render_cache')->get([ - '#cache' => [ - 'keys' => ['node--camelids', $uuid, $language], - 'bin' => 'jsonapi_normalizations', - ], - ]); + $cache = \Drupal::service('variation_cache.jsonapi_normalizations')->get(['node--camelids', $uuid, $language], new CacheableMetadata()); // After saving the entity the normalization should not be cached. $this->assertFalse($cache); // @todo Remove line below in favor of commented line in https://www.drupal.org/project/drupal/issues/2878463. @@ -422,13 +418,8 @@ protected function assertCacheableNormalizations(): void { * @internal */ protected function assertNormalizedFieldsAreCached(array $field_names): void { - $cache = \Drupal::service('render_cache')->get([ - '#cache' => [ - 'keys' => ['node--camelids', $this->entity->uuid(), $this->entity->language()->getId()], - 'bin' => 'jsonapi_normalizations', - ], - ]); - $cached_fields = $cache['#data']['fields']; + $cache = \Drupal::service('variation_cache.jsonapi_normalizations')->get(['node--camelids', $this->entity->uuid(), $this->entity->language()->getId()], new CacheableMetadata()); + $cached_fields = $cache->data['fields']; $this->assertSameSize($field_names, $cached_fields); array_walk($field_names, function ($field_name) use ($cached_fields) { $this->assertInstanceOf( -- GitLab