Issue #2412669 by claudiu.cristea, Julfabre, sidharrell, catch, daffie,...

Issue #2412669 by claudiu.cristea, Julfabre, sidharrell, catch, daffie, Berdir, andypost, tedbow, kim.pepper, ParisLiakos, Mile23: Remove drupal_static from BookManager

core/includes/bootstrap.inc

@@ -627,13 +627,20 @@ function &drupal_static($name, $default_value = NULL, $reset = FALSE) {
 function drupal_static_reset($name = NULL) {
   switch ($name) {
     case 'taxonomy_vocabulary_get_names':
-      @trigger_error("Using drupal_static_reset() with 'taxonomy_vocabulary_get_names' as parameter is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. There is no replacement for this usage. See https://www.drupal.org/node/3039041", E_USER_DEPRECATED);
+      @trigger_error("Calling drupal_static_reset() with 'taxonomy_vocabulary_get_names' as argument is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. There is no replacement for this usage. See https://www.drupal.org/node/3039041", E_USER_DEPRECATED);
       break;
 
     case 'node_mark':
       @trigger_error("Calling drupal_static_reset() with 'node_mark' as argument is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. There is no replacement for this usage. See https://www.drupal.org/node/3037203", E_USER_DEPRECATED);
       break;
 
+    case 'Drupal\book\BookManager::bookSubtreeData':
+    case 'Drupal\book\BookManager::bookTreeAllData':
+    case 'Drupal\book\BookManager::doBookTreeBuild':
+      @trigger_error("Calling drupal_static_reset() with '{$name}' as argument is deprecated in drupal:9.3.0 and is removed in drupal:10.0.0. Use \Drupal::service('book.memory_cache')->deleteAll() instead. See https://www.drupal.org/node/3039439", E_USER_DEPRECATED);
+      \Drupal::service('book.memory_cache')->deleteAll();
+      break;
+
   }
   drupal_static($name, NULL, TRUE);
 }

core/modules/book/book.services.yml

@@ -6,7 +6,7 @@ services:
       - { name: breadcrumb_builder, priority: 701 }
   book.manager:
     class: Drupal\book\BookManager
-    arguments: ['@entity_type.manager', '@string_translation', '@config.factory', '@book.outline_storage', '@renderer', '@language_manager', '@entity.repository']
+    arguments: ['@entity_type.manager', '@string_translation', '@config.factory', '@book.outline_storage', '@renderer', '@language_manager', '@entity.repository', '@book.backend_chained_cache', '@book.memory_cache']
   book.outline:
     class: Drupal\book\BookOutline
     arguments: ['@book.manager']
@@ -37,3 +37,14 @@ services:
       - { name: module_install.uninstall_validator }
     arguments: ['@book.outline_storage', '@entity_type.manager', '@string_translation']
     lazy: true
+  book.memory_cache:
+    class: Drupal\Core\Cache\MemoryCache\MemoryCache
+  book.backend_chained_cache:
+    class: Drupal\Core\Cache\BackendChain
+    calls:
+      - [appendBackend, ['@book.memory_cache']]
+      - [appendBackend, ['@cache.data']]
+    tags:
+      # This tag ensures that Drupal's cache_tags.invalidator service
+      # invalidates also this cache data.
+      - { name: cache.bin }

core/modules/book/src/BookManager.php

@@ -4,6 +4,7 @@
 
 use Drupal\Component\Utility\Unicode;
 use Drupal\Core\Cache\Cache;
+use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Entity\EntityRepositoryInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Form\FormStateInterface;
@@ -85,6 +86,20 @@ class BookManager implements BookManagerInterface {
    */
   protected $languageManager;
 
+  /**
+   * The book chained backend cache service.
+   *
+   * @var \Drupal\Core\Cache\CacheBackendInterface
+   */
+  protected $backendChainedCache;
+
+  /**
+   * The book memory cache service.
+   *
+   * @var \Drupal\Core\Cache\CacheBackendInterface
+   */
+  protected $memoryCache;
+
   /**
    * Constructs a BookManager object.
    *
@@ -102,8 +117,12 @@ class BookManager implements BookManagerInterface {
    *   The language manager.
    * @param \Drupal\Core\Entity\EntityRepositoryInterface|null $entity_repository
    *   The entity repository service.
+   * @param \Drupal\Core\Cache\CacheBackendInterface $backend_chained_cache
+   *   The book chained backend cache service.
+   * @param \Drupal\Core\Cache\CacheBackendInterface $memory_cache
+   *   The book memory cache service.
    */
-  public function __construct(EntityTypeManagerInterface $entity_type_manager, TranslationInterface $translation, ConfigFactoryInterface $config_factory, BookOutlineStorageInterface $book_outline_storage, RendererInterface $renderer, LanguageManagerInterface $language_manager = NULL, EntityRepositoryInterface $entity_repository = NULL) {
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, TranslationInterface $translation, ConfigFactoryInterface $config_factory, BookOutlineStorageInterface $book_outline_storage, RendererInterface $renderer, LanguageManagerInterface $language_manager = NULL, EntityRepositoryInterface $entity_repository = NULL, CacheBackendInterface $backend_chained_cache = NULL, CacheBackendInterface $memory_cache = NULL) {
     $this->entityTypeManager = $entity_type_manager;
     $this->stringTranslation = $translation;
     $this->configFactory = $config_factory;
@@ -119,6 +138,16 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Tra
       $entity_repository = \Drupal::service('entity.repository');
     }
     $this->entityRepository = $entity_repository;
+    if (!$backend_chained_cache) {
+      @trigger_error('Calling BookManager::__construct() without the $backend_chained_cache argument is deprecated in drupal:9.3.0 and the $backend_chained_cache argument will be required in drupal:10.0.0. See https://www.drupal.org/node/3039439', E_USER_DEPRECATED);
+      $backend_chained_cache = \Drupal::service('book.backend_chained_cache');
+    }
+    $this->backendChainedCache = $backend_chained_cache;
+    if (!$memory_cache) {
+      @trigger_error('Calling BookManager::__construct() without the $memory_cache argument is deprecated in drupal:9.3.0 and the $memory_cache argument will be required in drupal:10.0.0. See https://www.drupal.org/node/3039439', E_USER_DEPRECATED);
+      $memory_cache = \Drupal::service('book.memory_cache');
+    }
+    $this->memoryCache = $memory_cache;
   }
 
   /**
@@ -510,34 +539,37 @@ public function deleteFromBook($nid) {
    * {@inheritdoc}
    */
   public function bookTreeAllData($bid, $link = NULL, $max_depth = NULL) {
-    $tree = &drupal_static(__METHOD__, []);
-
-    // Use $nid as a flag for whether the data being loaded is for the whole
-    // tree.
+    // Use $nid as flag for whether the data being loaded is for the whole tree.
     $nid = isset($link['nid']) ? $link['nid'] : 0;
-    // Generate a cache ID (cid) specific for this $bid, $link, language, and
-    // depth.
     $langcode = $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
-    $cid = implode(':', ['book-links', $bid, 'all', $nid, $langcode, (int) $max_depth]);
 
-    if (!isset($tree[$cid])) {
-      // If the tree data was not in the static cache, build $tree_parameters.
-      $tree_parameters = [
-        'min_depth' => 1,
-        'max_depth' => $max_depth,
-      ];
-      if ($nid) {
-        $active_trail = $this->getActiveTrailIds($bid, $link);
-        $tree_parameters['expanded'] = $active_trail;
-        $tree_parameters['active_trail'] = $active_trail;
-        $tree_parameters['active_trail'][] = $nid;
-      }
+    // Create a cache ID for the given $nid, $link, $langcode and $max_depth.
+    $cid = implode(':', ['book-links', $bid, $nid, $langcode, (int) $max_depth]);
+
+    // Get it from cache, if available.
+    if ($cache = $this->memoryCache->get($cid)) {
+      return $cache->data;
+    }
 
-      // Build the tree using the parameters; the resulting tree will be cached.
-      $tree[$cid] = $this->bookTreeBuild($bid, $tree_parameters);
+    // If the tree data was not in the static cache, build $tree_parameters.
+    $tree_parameters = [
+      'min_depth' => 1,
+      'max_depth' => $max_depth,
+    ];
+    if ($nid) {
+      $active_trail = $this->getActiveTrailIds($bid, $link);
+      $tree_parameters['expanded'] = $active_trail;
+      $tree_parameters['active_trail'] = $active_trail;
+      $tree_parameters['active_trail'][] = $nid;
     }
 
-    return $tree[$cid];
+    // Build the tree using the parameters.
+    $tree_build = $this->bookTreeBuild($bid, $tree_parameters);
+
+    // Cache the tree build in memory.
+    $this->memoryCache->set($cid, $tree_build);
+
+    return $tree_build;
   }
 
   /**
@@ -701,46 +733,37 @@ protected function bookTreeBuild($bid, array $parameters = []) {
    * @see \Drupal\book\BookOutlineStorageInterface::getBookMenuTree()
    */
   protected function doBookTreeBuild($bid, array $parameters = []) {
-    // Static cache of already built menu trees.
-    $trees = &drupal_static(__METHOD__, []);
-
     // Build the cache id; sort parents to prevent duplicate storage and remove
     // default parameter values.
     if (isset($parameters['expanded'])) {
       sort($parameters['expanded']);
     }
     $langcode = $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
-    $tree_cid = implode(':', ['book-links', $bid, 'tree-data', $langcode, hash('sha256', serialize($parameters))]);
+    $cid = implode(':', ['book-links', $bid, 'tree-data', $langcode, hash('sha256', serialize($parameters))]);
 
-    // If we do not have this tree in the static cache, check {cache_data}.
-    if (!isset($trees[$tree_cid])) {
-      $cache = \Drupal::cache('data')->get($tree_cid);
-      if ($cache && $cache->data) {
-        $trees[$tree_cid] = $cache->data;
-      }
+    // Get it from cache, if available.
+    if ($cache = $this->backendChainedCache->get($cid)) {
+      return $cache->data;
     }
 
-    if (!isset($trees[$tree_cid])) {
-      $min_depth = (isset($parameters['min_depth']) ? $parameters['min_depth'] : 1);
-      $result = $this->bookOutlineStorage->getBookMenuTree($bid, $parameters, $min_depth, static::BOOK_MAX_DEPTH);
+    $min_depth = (isset($parameters['min_depth']) ? $parameters['min_depth'] : 1);
+    $result = $this->bookOutlineStorage->getBookMenuTree($bid, $parameters, $min_depth, static::BOOK_MAX_DEPTH);
 
-      // Build an ordered array of links using the query result object.
-      $links = [];
-      foreach ($result as $link) {
-        $link = (array) $link;
-        $links[$link['nid']] = $link;
-      }
-      $active_trail = (isset($parameters['active_trail']) ? $parameters['active_trail'] : []);
-      $data['tree'] = $this->buildBookOutlineData($links, $active_trail, $min_depth);
-      $data['node_links'] = [];
-      $this->bookTreeCollectNodeLinks($data['tree'], $data['node_links']);
-
-      // Cache the data, if it is not already in the cache.
-      \Drupal::cache('data')->set($tree_cid, $data, Cache::PERMANENT, ['bid:' . $bid]);
-      $trees[$tree_cid] = $data;
+    // Build an ordered array of links using the query result object.
+    $links = [];
+    foreach ($result as $link) {
+      $link = (array) $link;
+      $links[$link['nid']] = $link;
     }
+    $active_trail = (isset($parameters['active_trail']) ? $parameters['active_trail'] : []);
+    $data['tree'] = $this->buildBookOutlineData($links, $active_trail, $min_depth);
+    $data['node_links'] = [];
+    $this->bookTreeCollectNodeLinks($data['tree'], $data['node_links']);
+
+    // Cache tree data.
+    $this->backendChainedCache->set($cid, $data, Cache::PERMANENT, ['bid:' . $bid]);
 
-    return $trees[$tree_cid];
+    return $data;
   }
 
   /**
@@ -1135,50 +1158,29 @@ protected function buildBookOutlineRecursive(&$links, $parents, $depth) {
    * {@inheritdoc}
    */
   public function bookSubtreeData($link) {
-    $tree = &drupal_static(__METHOD__, []);
-
     // Generate a cache ID (cid) specific for this $link.
-    $cid = 'book-links:subtree-cid:' . $link['nid'];
-
-    if (!isset($tree[$cid])) {
-      $tree_cid_cache = \Drupal::cache('data')->get($cid);
+    $cid = "book-links:subtree-data:{$link['nid']}";
+    // Get it from cache, if available.
+    if ($cache = $this->backendChainedCache->get($cid)) {
+      return $cache->data;
+    }
 
-      if ($tree_cid_cache && $tree_cid_cache->data) {
-        // If the cache entry exists, it will just be the cid for the actual
-        // data. This avoids duplication of large amounts of data.
-        $cache = \Drupal::cache('data')->get($tree_cid_cache->data);
+    $result = $this->bookOutlineStorage->getBookSubtree($link, static::BOOK_MAX_DEPTH);
 
-        if ($cache && isset($cache->data)) {
-          $data = $cache->data;
-        }
-      }
-
-      // If the subtree data was not in the cache, $data will be NULL.
-      if (!isset($data)) {
-        $result = $this->bookOutlineStorage->getBookSubtree($link, static::BOOK_MAX_DEPTH);
-        $links = [];
-        foreach ($result as $item) {
-          $links[] = $item;
-        }
-        $data['tree'] = $this->buildBookOutlineData($links, [], $link['depth']);
-        $data['node_links'] = [];
-        $this->bookTreeCollectNodeLinks($data['tree'], $data['node_links']);
-        // Compute the real cid for book subtree data.
-        $tree_cid = 'book-links:subtree-data:' . hash('sha256', serialize($data));
-        // Cache the data, if it is not already in the cache.
-        if (!\Drupal::cache('data')->get($tree_cid)) {
-          \Drupal::cache('data')->set($tree_cid, $data, Cache::PERMANENT, ['bid:' . $link['bid']]);
-        }
-        // Cache the cid of the (shared) data using the book and item-specific
-        // cid.
-        \Drupal::cache('data')->set($cid, $tree_cid, Cache::PERMANENT, ['bid:' . $link['bid']]);
-      }
-      // Check access for the current user to each item in the tree.
-      $this->bookTreeCheckAccess($data['tree'], $data['node_links']);
-      $tree[$cid] = $data['tree'];
+    $links = [];
+    foreach ($result as $item) {
+      $links[] = $item;
     }
+    $data['tree'] = $this->buildBookOutlineData($links, [], $link['depth']);
+    $data['node_links'] = [];
+    $this->bookTreeCollectNodeLinks($data['tree'], $data['node_links']);
+    // Check access for the current user to each item in the tree.
+    $this->bookTreeCheckAccess($data['tree'], $data['node_links']);
 
-    return $tree[$cid];
+    // Cache subtree data.
+    $this->backendChainedCache->set($cid, $data['tree'], Cache::PERMANENT, ['bid:' . $link['bid']]);
+
+    return $data['tree'];
   }
 
 }

core/modules/book/tests/src/Kernel/BookManagerDeprecationTest.php

+<?php
+
+namespace Drupal\Tests\book\Kernel;
+
+use Drupal\book\BookManager;
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * @coversDefaultClass \Drupal\book\BookManager
+ * @group legacy
+ */
+class BookManagerDeprecationTest extends KernelTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['book'];
+
+  /**
+   * @param string $method
+   *   Method to be tested.
+   *
+   * @dataProvider providerTestDrupalStaticResetDeprecation
+   * @see drupal_static_reset()
+   */
+  public function testDrupalStaticResetDeprecation(string $method): void {
+    $this->expectDeprecation("Calling drupal_static_reset() with '{$method}' as argument is deprecated in drupal:9.3.0 and is removed in drupal:10.0.0. Use \Drupal::service('book.memory_cache')->deleteAll() instead. See https://www.drupal.org/node/3039439");
+    drupal_static_reset($method);
+  }
+
+  /**
+   * Provides test cases for ::testDrupalStaticResetDeprecation().
+   *
+   * @return string[][]
+   *   Test cases for ::testDrupalStaticResetDeprecation().
+   */
+  public function providerTestDrupalStaticResetDeprecation(): array {
+    return [
+      ['Drupal\book\BookManager::bookSubtreeData'],
+      ['Drupal\book\BookManager::bookTreeAllData'],
+      ['Drupal\book\BookManager::doBookTreeBuild'],
+    ];
+  }
+
+  /**
+   * @covers ::__construct
+   */
+  public function testOptionalParametersDeprecation(): void {
+    $this->expectDeprecation('Calling BookManager::__construct() without the $backend_chained_cache argument is deprecated in drupal:9.3.0 and the $backend_chained_cache argument will be required in drupal:10.0.0. See https://www.drupal.org/node/3039439');
+    $this->expectDeprecation('Calling BookManager::__construct() without the $memory_cache argument is deprecated in drupal:9.3.0 and the $memory_cache argument will be required in drupal:10.0.0. See https://www.drupal.org/node/3039439');
+    new BookManager(
+      $this->container->get('entity_type.manager'),
+      $this->container->get('string_translation'),
+      $this->container->get('config.factory'),
+      $this->container->get('book.outline_storage'),
+      $this->container->get('renderer'),
+      $this->container->get('language_manager'),
+      $this->container->get('entity.repository')
+    );
+  }
+
+}

core/modules/book/tests/src/Unit/BookManagerTest.php

@@ -3,6 +3,7 @@
 namespace Drupal\Tests\book\Unit;
 
 use Drupal\book\BookManager;
+use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Tests\UnitTestCase;
 
@@ -79,7 +80,9 @@ protected function setUp(): void {
     $this->renderer = $this->createMock('\Drupal\Core\Render\RendererInterface');
     $this->languageManager = $this->createMock('Drupal\Core\Language\LanguageManagerInterface');
     $this->entityRepository = $this->createMock('Drupal\Core\Entity\EntityRepositoryInterface');
-    $this->bookManager = new BookManager($this->entityTypeManager, $this->translation, $this->configFactory, $this->bookOutlineStorage, $this->renderer, $this->languageManager, $this->entityRepository);
+    // Used for both book manager cache services: backend chain and memory.
+    $cache = $this->createMock(CacheBackendInterface::class);
+    $this->bookManager = new BookManager($this->entityTypeManager, $this->translation, $this->configFactory, $this->bookOutlineStorage, $this->renderer, $this->languageManager, $this->entityRepository, $cache, $cache);
   }
 
   /**

core/modules/taxonomy/tests/src/Kernel/TaxonomyDeprecationTest.php

@@ -60,7 +60,7 @@ public function testTaxonomyDeprecations(): void {
     $title = taxonomy_term_title($term1);
     $this->assertSame($term1->label(), $title);
 
-    $this->expectDeprecation("Using drupal_static_reset() with 'taxonomy_vocabulary_get_names' as parameter is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. There is no replacement for this usage. See https://www.drupal.org/node/3039041");
+    $this->expectDeprecation("Calling drupal_static_reset() with 'taxonomy_vocabulary_get_names' as argument is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. There is no replacement for this usage. See https://www.drupal.org/node/3039041");
     drupal_static_reset('taxonomy_vocabulary_get_names');
   }