From 373f01148d7e75eea3fc69fc19273b768856c624 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Brala?=
<49259-bbrala@users.noreply.drupalcode.org>
Date: Fri, 24 May 2024 14:03:28 +0000
Subject: [PATCH] Issue #3402388 by ihor_allin, bbrala, _tarik_,
gaurav_manerkar, Ankush_03, Anybody: Pages with default includes are not
cached
---
.../jsonapi_defaults.services.yml | 3 +
.../src/Controller/EntityResource.php | 184 +++++++++++-------
.../jsonapi_defaults/src/JsonapiDefaults.php | 66 +++++++
.../src/JsonapiDefaultsInterface.php | 43 ++++
.../src/JsonapiDefaultsServiceProvider.php | 13 +-
.../src/QueryArgsCacheContext.php | 93 +++++++++
.../JsonApiDefaultsFunctionalTest.php | 4 +
.../Kernel/Controller/EntityResourceTest.php | 1 +
.../DefaultDisabledResourceConfigTest.php | 1 +
9 files changed, 332 insertions(+), 76 deletions(-)
create mode 100644 modules/jsonapi_defaults/jsonapi_defaults.services.yml
create mode 100644 modules/jsonapi_defaults/src/JsonapiDefaults.php
create mode 100644 modules/jsonapi_defaults/src/JsonapiDefaultsInterface.php
create mode 100644 modules/jsonapi_defaults/src/QueryArgsCacheContext.php
diff --git a/modules/jsonapi_defaults/jsonapi_defaults.services.yml b/modules/jsonapi_defaults/jsonapi_defaults.services.yml
new file mode 100644
index 0000000..fd2fff3
--- /dev/null
+++ b/modules/jsonapi_defaults/jsonapi_defaults.services.yml
@@ -0,0 +1,3 @@
+services:
+ jsonapi_defaults_includes:
+ class: Drupal\jsonapi_defaults\JsonapiDefaults
diff --git a/modules/jsonapi_defaults/src/Controller/EntityResource.php b/modules/jsonapi_defaults/src/Controller/EntityResource.php
index cffff95..2caeb18 100644
--- a/modules/jsonapi_defaults/src/Controller/EntityResource.php
+++ b/modules/jsonapi_defaults/src/Controller/EntityResource.php
@@ -2,52 +2,131 @@
namespace Drupal\jsonapi_defaults\Controller;
-use Drupal\Component\Serialization\Json;
+use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Entity\EntityFieldManagerInterface;
+use Drupal\Core\Entity\EntityRepositoryInterface;
+use Drupal\Core\Entity\EntityTypeManagerInterface;
+use Drupal\Core\Logger\LoggerChannelFactoryInterface;
+use Drupal\Core\Logger\LoggerChannelInterface;
+use Drupal\Core\Render\RendererInterface;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\jsonapi\Access\EntityAccessChecker;
+use Drupal\jsonapi\Context\FieldResolver;
use Drupal\jsonapi\Controller\EntityResource as JsonApiEntityResourse;
+use Drupal\jsonapi\IncludeResolver;
use Drupal\jsonapi\Query\OffsetPage;
use Drupal\jsonapi\Query\Sort;
use Drupal\jsonapi\ResourceType\ResourceType;
-use Drupal\jsonapi\Routing\Routes;
+use Drupal\jsonapi\ResourceType\ResourceTypeRepositoryInterface;
+use Drupal\jsonapi_defaults\JsonapiDefaultsInterface;
use Drupal\jsonapi_extras\Entity\JsonapiResourceConfig;
-use Drupal\jsonapi_extras\ResourceType\ConfigurableResourceType;
use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Serializer\SerializerInterface;
/**
* Overrides jsonapi module EntityResource controller.
*/
class EntityResource extends JsonApiEntityResourse {
+ /**
+ * The jsonapi defaults service.
+ *
+ * @var \Drupal\jsonapi_defaults\JsonapiDefaultsInterface
+ */
+ protected JsonapiDefaultsInterface $jsonapiDefaults;
+
+ /**
+ * The logger service.
+ *
+ * @var \Drupal\Core\Logger\LoggerChannelInterface
+ */
+ protected LoggerChannelInterface $logger;
+
+ /**
+ * Instantiates an EntityResource object.
+ *
+ * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
+ * The entity type manager.
+ * @param \Drupal\Core\Entity\EntityFieldManagerInterface $field_manager
+ * The entity type field manager.
+ * @param \Drupal\jsonapi\ResourceType\ResourceTypeRepositoryInterface $resource_type_repository
+ * The JSON:API resource type repository.
+ * @param \Drupal\Core\Render\RendererInterface $renderer
+ * The renderer.
+ * @param \Drupal\Core\Entity\EntityRepositoryInterface $entity_repository
+ * The entity repository.
+ * @param \Drupal\jsonapi\IncludeResolver $include_resolver
+ * The include resolver.
+ * @param \Drupal\jsonapi\Access\EntityAccessChecker $entity_access_checker
+ * The JSON:API entity access checker.
+ * @param \Drupal\jsonapi\Context\FieldResolver $field_resolver
+ * The JSON:API field resolver.
+ * @param \Symfony\Component\Serializer\SerializerInterface $serializer
+ * The JSON:API serializer.
+ * @param \Drupal\Component\Datetime\TimeInterface $time
+ * The time service.
+ * @param \Drupal\Core\Session\AccountInterface $user
+ * The current user account.
+ * @param \Drupal\jsonapi_defaults\JsonapiDefaultsInterface $jsonapi_defaults
+ * The jsonapi default service.
+ * @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $loggerFactory
+ * The logger service.
+ */
+ public function __construct(
+ EntityTypeManagerInterface $entity_type_manager,
+ EntityFieldManagerInterface $field_manager,
+ ResourceTypeRepositoryInterface $resource_type_repository,
+ RendererInterface $renderer,
+ EntityRepositoryInterface $entity_repository,
+ IncludeResolver $include_resolver,
+ EntityAccessChecker $entity_access_checker,
+ FieldResolver $field_resolver,
+ SerializerInterface $serializer,
+ TimeInterface $time,
+ AccountInterface $user,
+ JsonapiDefaultsInterface $jsonapi_defaults,
+ LoggerChannelFactoryInterface $loggerFactory
+ ) {
+ parent::__construct(
+ $entity_type_manager,
+ $field_manager,
+ $resource_type_repository,
+ $renderer,
+ $entity_repository,
+ $include_resolver,
+ $entity_access_checker,
+ $field_resolver,
+ $serializer,
+ $time,
+ $user,
+ );
+ $this->jsonapiDefaults = $jsonapi_defaults;
+ $this->logger = $loggerFactory->get('jsonapi_defaults');
+ }
+
/**
* {@inheritdoc}
*/
protected function getJsonApiParams(Request $request, ResourceType $resource_type) {
- // If this is a related resource, then we need to swap to the new resource
- // type.
- $related_field = $request->attributes->get('_on_relationship')
- ? NULL
- : $request->attributes->get('related');
try {
- $resource_type = static::correctResourceTypeOnRelated($related_field, $resource_type);
+ $resourceConfig = $this->jsonapiDefaults->getResourceConfigFromRequest($request);
}
catch (\LengthException $e) {
- watchdog_exception('jsonapi_defaults', $e);
- $resource_type = NULL;
+ $this->logger->error($e);
+ $resourceConfig = NULL;
}
- if (!$resource_type instanceof ConfigurableResourceType) {
- return parent::getJsonApiParams($request, $resource_type);
- }
- $resource_config = $resource_type->getJsonapiResourceConfig();
- if (!$resource_config instanceof JsonapiResourceConfig) {
+ if (!$resourceConfig instanceof JsonapiResourceConfig) {
return parent::getJsonApiParams($request, $resource_type);
}
- $default_filter_input = $resource_config->getThirdPartySetting(
+
+ $default_filter_input = $resourceConfig->getThirdPartySetting(
'jsonapi_defaults',
'default_filter',
[]
);
- $default_sorting_input = $resource_config->getThirdPartySetting(
+ $default_sorting_input = $resourceConfig->getThirdPartySetting(
'jsonapi_defaults',
'default_sorting',
[]
@@ -93,7 +172,7 @@ class EntityResource extends JsonApiEntityResourse {
// Implements overridden page limits.
$params = parent::getJsonApiParams($request, $resource_type);
- $this->setPageLimit($request, $resource_config, $params);
+ $this->setPageLimit($request, $resourceConfig, $params);
return $params;
}
@@ -101,35 +180,27 @@ class EntityResource extends JsonApiEntityResourse {
* {@inheritdoc}
*/
public function getIncludes(Request $request, $data) {
- if (
- ($resource_type = $request->get(Routes::RESOURCE_TYPE_KEY))
- && $resource_type instanceof ConfigurableResourceType
- && !$request->get('_on_relationship')
- ) {
+ if (!$request->get('_on_relationship')) {
try {
- $resource_type = static::correctResourceTypeOnRelated($request->get('related'), $resource_type);
+ $resourceConfig = $this->jsonapiDefaults->getResourceConfigFromRequest($request);
}
catch (\LengthException $e) {
- watchdog_exception('jsonapi_defaults', $e);
- return parent::getIncludes($request, $data);
- }
- if (!$resource_type instanceof ConfigurableResourceType) {
- return parent::getIncludes($request, $data);
+ $this->logger->error($e);
+ $resourceConfig = NULL;
}
- $resource_config = $resource_type->getJsonapiResourceConfig();
- if (!$resource_config instanceof JsonapiResourceConfig) {
+
+ if (!$resourceConfig) {
return parent::getIncludes($request, $data);
}
- $default_includes = $resource_config->getThirdPartySetting(
+
+ $defaultIncludes = $resourceConfig->getThirdPartySetting(
'jsonapi_defaults',
'default_include',
[]
);
- if (!empty($default_includes) && $request->query->get('include') === NULL) {
- $includes = array_unique(array_filter(array_merge(
- $default_includes,
- explode(',', $request->query->get('include', ''))
- )));
+
+ if (!empty($defaultIncludes) && $request->query->get('include') === NULL) {
+ $includes = array_unique(array_filter(array_merge($defaultIncludes)));
$request->query->set('include', implode(',', $includes));
}
}
@@ -137,43 +208,6 @@ class EntityResource extends JsonApiEntityResourse {
return parent::getIncludes($request, $data);
}
- /**
- * Returns the correct resource type when operating on related fields.
- *
- * @param string $related_field
- * The name of the related field to use. NULL if not using a related field.
- * @param \Drupal\jsonapi\ResourceType\ResourceType $resource_type
- * The resource type straight from the request.
- *
- * @return \Drupal\jsonapi\ResourceType\ResourceType
- * The resource type to use to load the includes.
- *
- * @throws \LengthException
- * If there is more than one relatable resource type.
- */
- public static function correctResourceTypeOnRelated($related_field, ResourceType $resource_type) {
- if (!$related_field) {
- return $resource_type;
- }
- $relatable_resource_types = $resource_type
- ->getRelatableResourceTypesByField($related_field);
- if (count($relatable_resource_types) > 1) {
- $message = sprintf(
- '%s -- %s',
- 'Impossible to apply defaults on a related resource with heterogeneous resource types.',
- Json::encode([
- 'related_field' => $related_field,
- 'host_resource_type' => $resource_type->getPath(),
- 'target_resource_types' => array_map(function (ResourceType $resource_type) {
- return $resource_type->getPath();
- }, $relatable_resource_types),
- ])
- );
- throw new \LengthException($message);
- }
- return $relatable_resource_types[0] ?? NULL;
- }
-
/**
* Set filter into nested array.
*
diff --git a/modules/jsonapi_defaults/src/JsonapiDefaults.php b/modules/jsonapi_defaults/src/JsonapiDefaults.php
new file mode 100644
index 0000000..491c147
--- /dev/null
+++ b/modules/jsonapi_defaults/src/JsonapiDefaults.php
@@ -0,0 +1,66 @@
+<?php
+
+namespace Drupal\jsonapi_defaults;
+
+use Drupal\Component\Serialization\Json;
+use Drupal\jsonapi\ResourceType\ResourceType;
+use Drupal\jsonapi\Routing\Routes;
+use Drupal\jsonapi_extras\Entity\JsonapiResourceConfig;
+use Drupal\jsonapi_extras\ResourceType\ConfigurableResourceType;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Provides reusable methods and features.
+ */
+class JsonapiDefaults implements JsonapiDefaultsInterface {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getResourceConfigFromRequest(Request $request, ResourceType $resourceType = NULL): ?JsonapiResourceConfig {
+ $resourceType = !$resourceType ? $request->get(Routes::RESOURCE_TYPE_KEY) : $resourceType;
+
+ if ($resourceType instanceof ConfigurableResourceType) {
+ $relatedField = $request->attributes->get('_on_relationship')
+ ? NULL
+ : $request->attributes->get('related');
+ $resourceType = static::correctResourceTypeOnRelated($relatedField, $resourceType);
+
+ if (
+ $resourceType instanceof ConfigurableResourceType
+ && ($resourceConfig = $resourceType->getJsonapiResourceConfig()) instanceof JsonapiResourceConfig
+ ) {
+ return $resourceConfig;
+ }
+ }
+
+ return NULL;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function correctResourceTypeOnRelated(?string $related_field, ResourceType $resource_type): ?ResourceType {
+ if (!$related_field) {
+ return $resource_type;
+ }
+ $relatable_resource_types = $resource_type
+ ->getRelatableResourceTypesByField($related_field);
+ if (count($relatable_resource_types) > 1) {
+ $message = sprintf(
+ '%s -- %s',
+ 'Impossible to apply defaults on a related resource with heterogeneous resource types.',
+ Json::encode([
+ 'related_field' => $related_field,
+ 'host_resource_type' => $resource_type->getPath(),
+ 'target_resource_types' => array_map(function (ResourceType $resource_type) {
+ return $resource_type->getPath();
+ }, $relatable_resource_types),
+ ])
+ );
+ throw new \LengthException($message);
+ }
+ return $relatable_resource_types[0] ?? NULL;
+ }
+
+}
diff --git a/modules/jsonapi_defaults/src/JsonapiDefaultsInterface.php b/modules/jsonapi_defaults/src/JsonapiDefaultsInterface.php
new file mode 100644
index 0000000..0b27d24
--- /dev/null
+++ b/modules/jsonapi_defaults/src/JsonapiDefaultsInterface.php
@@ -0,0 +1,43 @@
+<?php
+
+namespace Drupal\jsonapi_defaults;
+
+use Drupal\jsonapi\ResourceType\ResourceType;
+use Drupal\jsonapi_extras\Entity\JsonapiResourceConfig;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Defines interface for the JsonapiDefaults service.
+ */
+interface JsonapiDefaultsInterface {
+
+ /**
+ * Get the jsonapi_resource_config from the request.
+ *
+ * @param \Symfony\Component\HttpFoundation\Request $request
+ * The current request.
+ * @param \Symfony\Component\HttpFoundation\ResourceType|null $resourceType
+ * The JSON:API resource type.
+ *
+ * @return \Drupal\jsonapi_extras\Entity\JsonapiResourceConfig|null
+ * The jsonapi_resource_config entity or NULL.
+ */
+ public function getResourceConfigFromRequest(Request $request, ResourceType $resourceType = NULL): ?JsonapiResourceConfig;
+
+ /**
+ * Returns the correct resource type when operating on related fields.
+ *
+ * @param string|null $related_field
+ * The name of the related field to use. NULL if not using a related field.
+ * @param \Drupal\jsonapi\ResourceType\ResourceType $resource_type
+ * The resource type straight from the request.
+ *
+ * @return \Drupal\jsonapi\ResourceType\ResourceType
+ * The resource type to use to load the includes.
+ *
+ * @throws \LengthException
+ * If there is more than one relatable resource type.
+ */
+ public static function correctResourceTypeOnRelated(?string $related_field, ResourceType $resource_type): ?ResourceType;
+
+}
diff --git a/modules/jsonapi_defaults/src/JsonapiDefaultsServiceProvider.php b/modules/jsonapi_defaults/src/JsonapiDefaultsServiceProvider.php
index cf60b55..10a757d 100644
--- a/modules/jsonapi_defaults/src/JsonapiDefaultsServiceProvider.php
+++ b/modules/jsonapi_defaults/src/JsonapiDefaultsServiceProvider.php
@@ -4,6 +4,7 @@ namespace Drupal\jsonapi_defaults;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\DependencyInjection\ServiceProviderBase;
+use Symfony\Component\DependencyInjection\Reference;
/**
* Modifies the jsonapi normalizer service.
@@ -18,7 +19,17 @@ class JsonapiDefaultsServiceProvider extends ServiceProviderBase {
if ($container->hasDefinition('jsonapi.entity_resource')) {
/** @var \Symfony\Component\DependencyInjection\Definition $definition */
$definition = $container->getDefinition('jsonapi.entity_resource');
- $definition->setClass('Drupal\jsonapi_defaults\Controller\EntityResource');
+ $definition->setClass('Drupal\jsonapi_defaults\Controller\EntityResource')
+ ->addArgument(new Reference('jsonapi_defaults_includes'))
+ ->addArgument(new Reference('logger.factory'));
+ }
+
+ if ($container->has('cache_context.url.query_args')) {
+ // Override the cache_context.url.query_args service.
+ /** @var \Symfony\Component\DependencyInjection\Definition $definition */
+ $definition = $container->getDefinition('cache_context.url.query_args');
+ $definition->setClass(QueryArgsCacheContext::class)
+ ->addArgument(new Reference('jsonapi_defaults_includes'));
}
}
diff --git a/modules/jsonapi_defaults/src/QueryArgsCacheContext.php b/modules/jsonapi_defaults/src/QueryArgsCacheContext.php
new file mode 100644
index 0000000..e567cc3
--- /dev/null
+++ b/modules/jsonapi_defaults/src/QueryArgsCacheContext.php
@@ -0,0 +1,93 @@
+<?php
+
+namespace Drupal\jsonapi_defaults;
+
+use Drupal\Core\Cache\Context\QueryArgsCacheContext as CoreQueryArgsCacheContext;
+use Drupal\jsonapi_extras\Entity\JsonapiResourceConfig;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+/**
+ * Override core QueryArgsCacheContext service.
+ *
+ * To ensure that calls to JSON API resources with default includes are
+ * cacheable, it is necessary to ignore all default includes arguments.
+ * This is because they are set in the
+ * 'Drupal\jsonapi_defaults\Controller\EntityResource::getIncludes' to the
+ * request dynamically before the cache is set. On the other hand, if the cache
+ * is stored in the database, 'getIncludes' is executed after Drupal attempts to
+ * retrieve the cache. This results in a situation where we always receive an
+ * uncached response due to a mismatch with include arguments.
+ */
+class QueryArgsCacheContext extends CoreQueryArgsCacheContext {
+
+ /**
+ * The jsonapi defaults service.
+ *
+ * @var \Drupal\jsonapi_defaults\JsonapiDefaultsInterface
+ */
+ protected JsonapiDefaultsInterface $jsonapiDefaults;
+
+ /**
+ * Constructs a new RequestStackCacheContextBase class.
+ *
+ * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
+ * The request stack.
+ * @param \Drupal\jsonapi_defaults\JsonapiDefaultsInterface $jsonapi_defaults
+ * The jsonapi default service.
+ */
+ public function __construct(RequestStack $request_stack, JsonapiDefaultsInterface $jsonapi_defaults) {
+ parent::__construct($request_stack);
+ $this->jsonapiDefaults = $jsonapi_defaults;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getContext($query_arg = NULL) {
+ // Ignore context if request contains default includes arguments.
+ if ($query_arg == 'include' && $this->hasOnlyDefaultIncludes()) {
+ return '';
+ }
+
+ return parent::getContext($query_arg);
+ }
+
+ /**
+ * Check if the request contains only default includes.
+ *
+ * @return bool
+ * TRUE if the request contains only default includes, FALSE otherwise..
+ */
+ protected function hasOnlyDefaultIncludes(): bool {
+ try {
+ $resourceConfig = $this->jsonapiDefaults->getResourceConfigFromRequest(
+ $this->requestStack->getCurrentRequest()
+ );
+ }
+ catch (\LengthException $e) {
+ return FALSE;
+ }
+
+ if (!$resourceConfig instanceof JsonapiResourceConfig) {
+ return FALSE;
+ }
+
+ $default_includes = $resourceConfig->getThirdPartySetting(
+ 'jsonapi_defaults',
+ 'default_include',
+ []
+ );
+
+ $current_request = $this->requestStack->getCurrentRequest();
+ $current_includes = $current_request->query->get('include');
+
+ if ($current_includes) {
+ $current_includes = explode(',', (string) $current_includes);
+ return empty(array_diff($current_includes, $default_includes));
+ }
+ else {
+ return !empty($default_includes);
+ }
+ }
+
+}
diff --git a/modules/jsonapi_defaults/tests/src/Functional/JsonApiDefaultsFunctionalTest.php b/modules/jsonapi_defaults/tests/src/Functional/JsonApiDefaultsFunctionalTest.php
index 1069580..b5f0f64 100644
--- a/modules/jsonapi_defaults/tests/src/Functional/JsonApiDefaultsFunctionalTest.php
+++ b/modules/jsonapi_defaults/tests/src/Functional/JsonApiDefaultsFunctionalTest.php
@@ -68,12 +68,16 @@ class JsonApiDefaultsFunctionalTest extends JsonApiExtrasFunctionalTestBase {
// 1. Apply default filters and includes on a resource and a related
// resource.
$response = $this->drupalGet('/api/articles');
+ $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'MISS');
$parsed_response = Json::decode($response);
$this->assertArrayHasKey('data', $parsed_response);
$this->assertCount(1, $parsed_response['data']);
$this->assertEquals(3, $parsed_response['data'][0]['attributes']['internalId']);
$this->assertArrayHasKey('included', $parsed_response);
$this->assertGreaterThan(0, count($parsed_response['included']));
+ // Make sure that after the second request, we have a cached response.
+ $this->drupalGet('/api/articles');
+ $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'HIT');
// Make sure related resources don't fail.
$response = $this->drupalGet('/api/articles/' . $this->nodes[0]->uuid() . '/owner');
$parsed_response = Json::decode($response);
diff --git a/tests/src/Kernel/Controller/EntityResourceTest.php b/tests/src/Kernel/Controller/EntityResourceTest.php
index f52ea43..69513f4 100644
--- a/tests/src/Kernel/Controller/EntityResourceTest.php
+++ b/tests/src/Kernel/Controller/EntityResourceTest.php
@@ -30,6 +30,7 @@ class EntityResourceTest extends KernelTestBase {
* {@inheritdoc}
*/
protected static $modules = [
+ 'file',
'node',
'jsonapi',
'serialization',
diff --git a/tests/src/Kernel/DefaultDisabledResourceConfigTest.php b/tests/src/Kernel/DefaultDisabledResourceConfigTest.php
index d86fba5..324b1c2 100644
--- a/tests/src/Kernel/DefaultDisabledResourceConfigTest.php
+++ b/tests/src/Kernel/DefaultDisabledResourceConfigTest.php
@@ -20,6 +20,7 @@ class DefaultDisabledResourceConfigTest extends KernelTestBase {
* {@inheritdoc}
*/
protected static $modules = [
+ 'file',
'jsonapi',
'jsonapi_extras',
'entity_test',
--
GitLab