From 2e73418c1ef5995e401ce50f13771488a5595557 Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Mon, 18 Dec 2023 14:48:26 +0000 Subject: [PATCH] Issue #3402850 by kristiaanvandeneynde, catch, smustgrave: Fix MemoryCache discovery and DX --- core/core.services.yml | 24 +++++++-------- core/lib/Drupal/Core/Cache/Cache.php | 15 ++++++++++ core/lib/Drupal/Core/Cache/CacheFactory.php | 20 ++++++++++++- .../Core/Cache/CacheTagsInvalidator.php | 10 ++++--- .../Drupal/Core/Cache/ListCacheBinsPass.php | 30 +++++++++++++------ .../Cache/MemoryCache/MemoryCacheFactory.php | 26 ++++++++++++++++ .../Core/Test/RefreshVariablesTrait.php | 5 ++++ core/modules/book/book.services.yml | 8 +++-- core/modules/jsonapi/jsonapi.services.yml | 9 ++++-- .../Core/Cache/CacheTagsInvalidatorTest.php | 10 +++++++ 10 files changed, 125 insertions(+), 32 deletions(-) create mode 100644 core/lib/Drupal/Core/Cache/MemoryCache/MemoryCacheFactory.php diff --git a/core/core.services.yml b/core/core.services.yml index 7ee598ee6fbe..297669a6665c 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -7,6 +7,7 @@ parameters: # \Drupal\Core\Cache\ListCacheBinsPass::process() will override this but defining this allows the cache system to # function properly before that runs. cache_default_bin_backends: [] + memory_cache_default_bin_backends: [] session.storage.options: gc_probability: 1 gc_divisor: 100 @@ -194,7 +195,7 @@ services: cache_factory: class: Drupal\Core\Cache\CacheFactory - arguments: ['@settings', '%cache_default_bin_backends%'] + arguments: ['@settings', '%cache_default_bin_backends%', '%memory_cache_default_bin_backends%'] calls: - [setContainer, ['@service_container']] Drupal\Core\Cache\CacheFactoryInterface: '@cache_factory' @@ -239,6 +240,8 @@ services: arguments: ['@cache_tags.invalidator.checksum'] cache.backend.memory: class: Drupal\Core\Cache\MemoryBackendFactory + cache.backend.memory.memory: + class: Drupal\Core\Cache\MemoryCache\MemoryCacheFactory # A special cache bin that does not persist beyond the length of the request. cache.static: class: Drupal\Core\Cache\CacheBackendInterface @@ -289,9 +292,11 @@ services: factory: ['@cache_factory', 'get'] arguments: [access_policy] cache.access_policy_memory: - class: Drupal\Core\Cache\MemoryCache\MemoryCache + class: Drupal\Core\Cache\CacheBackendInterface tags: - - { name: cache_tags_invalidator } + - { name: cache.bin.memory, default_backend: cache.backend.memory.memory } + factory: ['@cache_factory', 'get'] + arguments: [access_policy_memory] cache.data: class: Drupal\Core\Cache\CacheBackendInterface tags: @@ -308,17 +313,10 @@ services: class: Drupal\Core\Cache\VariationCacheInterface factory: ['@variation_cache_factory', 'get'] arguments: [access_policy] - # We cannot use the VariationCacheFactory here because that expects that the - # backend it should wrap a VariationCache around is a proper cache bin. The - # MemoryCache backend, however, is not so we cannot use the factory here. - # - # By instantiating a VariationCache via the container and tagging the cache - # it's wrapping as a cache tags invalidator, it does still behave the way we - # want it to, though. variation_cache.access_policy_memory: - class: Drupal\Core\Cache\VariationCache - arguments: ['@request_stack', '@cache.access_policy_memory', '@cache_contexts_manager'] - Drupal\Core\Cache\VariationCacheInterface: '@variation_cache.access_policy_memory' + class: Drupal\Core\Cache\VariationCacheInterface + factory: ['@variation_cache_factory', 'get'] + arguments: [access_policy_memory] Drupal\Core\Asset\AssetQueryStringInterface: '@asset.query_string' asset.query_string: class: Drupal\Core\Asset\AssetQueryString diff --git a/core/lib/Drupal/Core/Cache/Cache.php b/core/lib/Drupal/Core/Cache/Cache.php index f93a4708027c..fdff7e2075a3 100644 --- a/core/lib/Drupal/Core/Cache/Cache.php +++ b/core/lib/Drupal/Core/Cache/Cache.php @@ -126,6 +126,21 @@ public static function getBins() { return $bins; } + /** + * Gets all memory cache bin services. + * + * @return \Drupal\Core\Cache\CacheBackendInterface[] + * An array of cache backend objects keyed by memory cache bins. + */ + public static function getMemoryBins(): array { + $bins = []; + $container = \Drupal::getContainer(); + foreach ($container->getParameter('memory_cache_bins') as $service_id => $bin) { + $bins[$bin] = $container->get($service_id); + } + return $bins; + } + /** * Generates a hash from a query object, to be used as part of the cache key. * diff --git a/core/lib/Drupal/Core/Cache/CacheFactory.php b/core/lib/Drupal/Core/Cache/CacheFactory.php index eefbbae5d4fd..845014581b40 100644 --- a/core/lib/Drupal/Core/Cache/CacheFactory.php +++ b/core/lib/Drupal/Core/Cache/CacheFactory.php @@ -33,6 +33,17 @@ class CacheFactory implements CacheFactoryInterface, ContainerAwareInterface { */ protected $defaultBinBackends; + /** + * A map of cache bin to default cache memory backend service name. + * + * All bin-specific mappings in $settings take precedence over this, but it + * can be used to optimize cache storage for a Drupal installation without + * cache customizations in settings.php. + * + * @var array + */ + protected $memoryDefaultBinBackends; + /** * Constructs CacheFactory object. * @@ -41,10 +52,14 @@ class CacheFactory implements CacheFactoryInterface, ContainerAwareInterface { * @param array $default_bin_backends * (optional) A mapping of bin to backend service name. Mappings in * $settings take precedence over this. + * @param array $memory_default_bin_backends + * (optional) A mapping of bin to backend service name. Mappings in + * $settings take precedence over this. */ - public function __construct(Settings $settings, array $default_bin_backends = []) { + public function __construct(Settings $settings, array $default_bin_backends = [], array $memory_default_bin_backends = []) { $this->settings = $settings; $this->defaultBinBackends = $default_bin_backends; + $this->memoryDefaultBinBackends = $memory_default_bin_backends; } /** @@ -72,6 +87,9 @@ public function get($bin) { elseif (isset($this->defaultBinBackends[$bin])) { $service_name = $this->defaultBinBackends[$bin]; } + elseif (isset($this->memoryDefaultBinBackends[$bin])) { + $service_name = $this->memoryDefaultBinBackends[$bin]; + } // Third, use configured default backend. elseif (isset($cache_settings['default'])) { $service_name = $cache_settings['default']; diff --git a/core/lib/Drupal/Core/Cache/CacheTagsInvalidator.php b/core/lib/Drupal/Core/Cache/CacheTagsInvalidator.php index 820da9886bda..d2a8f1e5512a 100644 --- a/core/lib/Drupal/Core/Cache/CacheTagsInvalidator.php +++ b/core/lib/Drupal/Core/Cache/CacheTagsInvalidator.php @@ -68,10 +68,12 @@ public function addInvalidator(CacheTagsInvalidatorInterface $invalidator) { */ protected function getInvalidatorCacheBins() { $bins = []; - foreach ($this->container->getParameter('cache_bins') as $service_id => $bin) { - $service = $this->container->get($service_id); - if ($service instanceof CacheTagsInvalidatorInterface) { - $bins[$bin] = $service; + foreach (['cache_bins', 'memory_cache_bins'] as $parameter) { + foreach ($this->container->getParameter($parameter) as $service_id => $bin) { + $service = $this->container->get($service_id); + if ($service instanceof CacheTagsInvalidatorInterface) { + $bins[$bin] = $service; + } } } return $bins; diff --git a/core/lib/Drupal/Core/Cache/ListCacheBinsPass.php b/core/lib/Drupal/Core/Cache/ListCacheBinsPass.php index 32bac09c086d..55a4777881cf 100644 --- a/core/lib/Drupal/Core/Cache/ListCacheBinsPass.php +++ b/core/lib/Drupal/Core/Cache/ListCacheBinsPass.php @@ -16,17 +16,29 @@ class ListCacheBinsPass implements CompilerPassInterface { * Collects the cache bins into the cache_bins parameter. */ public function process(ContainerBuilder $container) { - $cache_bins = []; - $cache_default_bin_backends = []; - foreach ($container->findTaggedServiceIds('cache.bin') as $id => $attributes) { - $bin = substr($id, strpos($id, '.') + 1); - $cache_bins[$id] = $bin; - if (isset($attributes[0]['default_backend'])) { - $cache_default_bin_backends[$bin] = $attributes[0]['default_backend']; + $cache_info['cache']['bins'] = []; + $cache_info['cache']['default_bin_backends'] = []; + $cache_info['memory_cache']['bins'] = []; + $cache_info['memory_cache']['default_bin_backends'] = []; + + $tag_info = [ + 'cache.bin' => 'cache', + 'cache.bin.memory' => 'memory_cache', + ]; + foreach ($tag_info as $service_tag => $section) { + foreach ($container->findTaggedServiceIds($service_tag) as $id => $attributes) { + $bin = substr($id, strpos($id, '.') + 1); + $cache_info[$section]['bins'][$id] = $bin; + if (isset($attributes[0]['default_backend'])) { + $cache_info[$section]['default_bin_backends'][$bin] = $attributes[0]['default_backend']; + } } } - $container->setParameter('cache_bins', $cache_bins); - $container->setParameter('cache_default_bin_backends', $cache_default_bin_backends); + + $container->setParameter('cache_bins', $cache_info['cache']['bins']); + $container->setParameter('cache_default_bin_backends', $cache_info['cache']['default_bin_backends']); + $container->setParameter('memory_cache_bins', $cache_info['memory_cache']['bins']); + $container->setParameter('memory_cache_default_bin_backends', $cache_info['memory_cache']['default_bin_backends']); } } diff --git a/core/lib/Drupal/Core/Cache/MemoryCache/MemoryCacheFactory.php b/core/lib/Drupal/Core/Cache/MemoryCache/MemoryCacheFactory.php new file mode 100644 index 000000000000..1f2fcb7beabd --- /dev/null +++ b/core/lib/Drupal/Core/Cache/MemoryCache/MemoryCacheFactory.php @@ -0,0 +1,26 @@ +<?php + +namespace Drupal\Core\Cache\MemoryCache; + +use Drupal\Core\Cache\CacheFactoryInterface; + +class MemoryCacheFactory implements CacheFactoryInterface { + + /** + * Instantiated memory cache bins. + * + * @var \Drupal\Core\Cache\MemoryBackend[] + */ + protected $bins = []; + + /** + * {@inheritdoc} + */ + public function get($bin) { + if (!isset($this->bins[$bin])) { + $this->bins[$bin] = new MemoryCache(); + } + return $this->bins[$bin]; + } + +} diff --git a/core/lib/Drupal/Core/Test/RefreshVariablesTrait.php b/core/lib/Drupal/Core/Test/RefreshVariablesTrait.php index 5c7ef11f99b8..0e00b111345a 100644 --- a/core/lib/Drupal/Core/Test/RefreshVariablesTrait.php +++ b/core/lib/Drupal/Core/Test/RefreshVariablesTrait.php @@ -30,6 +30,11 @@ protected function refreshVariables() { $backend->reset(); } } + foreach (Cache::getMemoryBins() as $backend) { + if (is_callable([$backend, 'reset'])) { + $backend->reset(); + } + } \Drupal::service('config.factory')->reset(); \Drupal::service('state')->resetCache(); diff --git a/core/modules/book/book.services.yml b/core/modules/book/book.services.yml index d15986f20cab..1552c2cf5f7d 100644 --- a/core/modules/book/book.services.yml +++ b/core/modules/book/book.services.yml @@ -41,7 +41,11 @@ services: arguments: ['@book.outline_storage', '@entity_type.manager', '@string_translation'] lazy: true book.memory_cache: - class: Drupal\Core\Cache\MemoryCache\MemoryCache + class: Drupal\Core\Cache\MemoryCache\MemoryCacheInterface + tags: + - { name: cache.bin.memory, default_backend: cache.backend.memory.memory } + factory: ['@cache_factory', 'get'] + arguments: [memory_cache] book.backend_chained_cache: class: Drupal\Core\Cache\BackendChain calls: @@ -50,4 +54,4 @@ services: tags: # This tag ensures that Drupal's cache_tags.invalidator service # invalidates also this cache data. - - { name: cache.bin } + - { name: cache.bin.memory } diff --git a/core/modules/jsonapi/jsonapi.services.yml b/core/modules/jsonapi/jsonapi.services.yml index 1397335257c2..d032175d4612 100644 --- a/core/modules/jsonapi/jsonapi.services.yml +++ b/core/modules/jsonapi/jsonapi.services.yml @@ -125,8 +125,11 @@ services: # Cache. cache.jsonapi_memory: - class: Drupal\Core\Cache\MemoryCache\MemoryCache - public: false + class: Drupal\Core\Cache\MemoryCache\MemoryCacheInterface + tags: + - { name: cache.bin.memory, default_backend: cache.backend.memory.memory } + factory: ['@cache_factory', 'get'] + arguments: [jsonapi_memory] # A chained cache with an in-memory cache as the first layer and a database- # backed cache as the fallback is used. The first layer (memory) is necessary @@ -138,7 +141,7 @@ services: calls: - [appendBackend, ['@cache.jsonapi_memory']] - [appendBackend, ['@cache.default']] - tags: [{ name: cache.bin }] + tags: [{ name: cache.bin.memory }] cache.jsonapi_normalizations: class: Drupal\Core\Cache\CacheBackendInterface tags: diff --git a/core/tests/Drupal/Tests/Core/Cache/CacheTagsInvalidatorTest.php b/core/tests/Drupal/Tests/Core/Cache/CacheTagsInvalidatorTest.php index a9600c2bb197..02f6731e94ff 100644 --- a/core/tests/Drupal/Tests/Core/Cache/CacheTagsInvalidatorTest.php +++ b/core/tests/Drupal/Tests/Core/Cache/CacheTagsInvalidatorTest.php @@ -41,10 +41,20 @@ public function testInvalidateTags() { // a fatal error. $non_invalidator_cache_bin = $this->createMock('\Drupal\Core\Cache\CacheBackendInterface'); + // Repeat the above for memory cache bins. + $invalidator_memory_cache_bin = $this->createMock('\Drupal\Core\Cache\CacheTagsInvalidator'); + $invalidator_memory_cache_bin->expects($this->once()) + ->method('invalidateTags') + ->with(['node:1']); + $non_invalidator_memory_cache_bin = $this->createMock('\Drupal\Core\Cache\CacheBackendInterface'); + $container = new Container(); $container->set('cache.invalidator_cache_bin', $invalidator_cache_bin); $container->set('cache.non_invalidator_cache_bin', $non_invalidator_cache_bin); + $container->set('cache.invalidator_memory_cache_bin', $invalidator_memory_cache_bin); + $container->set('cache.non_invalidator_memory_cache_bin', $non_invalidator_memory_cache_bin); $container->setParameter('cache_bins', ['cache.invalidator_cache_bin' => 'invalidator_cache_bin', 'cache.non_invalidator_cache_bin' => 'non_invalidator_cache_bin']); + $container->setParameter('memory_cache_bins', ['cache.invalidator_memory_cache_bin' => 'invalidator_memory_cache_bin', 'cache.non_invalidator_memory_cache_bin' => 'non_invalidator_memory_cache_bin']); $cache_tags_invalidator->setContainer($container); $invalidator = $this->createMock('\Drupal\Core\Cache\CacheTagsInvalidator'); -- GitLab