Issue #1722882 by EclipseGc, aspilicious, tim.plunkett, fubhy, Berdir: Fixed Plugin CacheDecorator caches globally, ignores request context, and does not specify tags for cache items.

core/lib/Drupal/Core/Plugin/Discovery/CacheDecorator.php

@@ -9,6 +9,7 @@ namespace Drupal\Core\Plugin\Discovery;
 
 use Drupal\Component\Plugin\Discovery\CachedDiscoveryInterface;
 use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
+use Drupal\Core\Cache\CacheBackendInterface;
 
 /**
  * Enables static and persistent caching of discovered plugin definitions.
@@ -29,6 +30,20 @@ class CacheDecorator implements CachedDiscoveryInterface {
    */
   protected $cacheBin;
 
+  /**
+   * The timestamp indicating when the definition list cache expires.
+   *
+   * @var int
+   */
+  protected $cacheExpire;
+
+  /**
+   * The cache tags associated with the definition list.
+   *
+   * @var array
+   */
+  protected $cacheTags;
+
   /**
    * The plugin definitions of the decorated discovery class.
    *
@@ -54,11 +69,18 @@ class CacheDecorator implements CachedDiscoveryInterface {
    *   The cache identifier used for storage of the definition list.
    * @param string $cache_bin
    *   The cache bin used for storage and retrieval of the definition list.
+   * @param int $cache_expire
+   *   A Unix timestamp indicating that the definition list will be considered
+   *   invalid after this time.
+   * @param array $cache_tags
+   *   The cache tags associated with the definition list.
    */
-  public function __construct(DiscoveryInterface $decorated, $cache_key, $cache_bin = 'cache') {
+  public function __construct(DiscoveryInterface $decorated, $cache_key, $cache_bin = 'cache', $cache_expire = CacheBackendInterface::CACHE_PERMANENT, array $cache_tags = array()) {
     $this->decorated = $decorated;
     $this->cacheKey = $cache_key;
     $this->cacheBin = $cache_bin;
+    $this->cacheExpire = $cache_expire;
+    $this->cacheTags = $cache_tags;
   }
 
   /**
@@ -124,7 +146,7 @@ class CacheDecorator implements CachedDiscoveryInterface {
    */
   protected function setCachedDefinitions($definitions) {
     if (isset($this->cacheKey)) {
-      cache($this->cacheBin)->set($this->cacheKey, $definitions);
+      cache($this->cacheBin)->set($this->cacheKey, $definitions, $this->cacheExpire, $this->cacheTags);
     }
     $this->definitions = $definitions;
   }
@@ -145,4 +167,5 @@ class CacheDecorator implements CachedDiscoveryInterface {
   public function __call($method, $args) {
     return call_user_func_array(array($this->decorated, $method), $args);
   }
+
 }

core/modules/aggregator/lib/Drupal/aggregator/Plugin/FetcherManager.php

@@ -10,6 +10,7 @@ namespace Drupal\aggregator\Plugin;
 use Drupal\Component\Plugin\PluginManagerBase;
 use Drupal\Component\Plugin\Factory\DefaultFactory;
 use Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery;
+use Drupal\Core\Plugin\Discovery\CacheDecorator;
 
 /**
  * Manages aggregator fetcher plugins.
@@ -18,6 +19,7 @@ class FetcherManager extends PluginManagerBase {
 
   public function __construct() {
     $this->discovery = new AnnotatedClassDiscovery('aggregator', 'fetcher');
+    $this->discovery = new CacheDecorator($this->discovery, 'aggregator_fetcher:' . language(LANGUAGE_TYPE_INTERFACE)->langcode);
     $this->factory = new DefaultFactory($this->discovery);
   }
 }

core/modules/system/lib/Drupal/system/Tests/Plugin/CacheDecoratorLanguageTest.php

@@ -0,0 +1,121 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\system\Tests\Plugin\CacheDecoratorLanguageTest.
+ */
+
+namespace Drupal\system\Tests\Plugin;
+
+use Drupal\simpletest\WebTestBase;
+use Drupal\Core\Language\Language;
+
+/**
+ * Tests that the AlterDecorator fires and respects the alter hook.
+ */
+class CacheDecoratorLanguageTest extends WebTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('plugin_test', 'locale', 'language');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'CacheDecoratorLanguage',
+      'description' => 'Tests that the CacheDecorator stores definitions by language appropriately.',
+      'group' => 'Plugin API',
+    );
+  }
+
+  public function setUp() {
+    parent::setUp();
+
+    // Populate sample definitions.
+    $this->mockBlockExpectedDefinitions = array(
+      'user_login' => array(
+        'label' => 'User login',
+        'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserLoginBlock',
+      ),
+      'menu:main_menu' => array(
+        'label' => 'Main menu',
+        'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlock',
+      ),
+      'menu:navigation' => array(
+        'label' => 'Navigation',
+        'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlock',
+      ),
+      'layout' => array(
+        'label' => 'Layout',
+        'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlock',
+      ),
+      'layout:foo' => array(
+        'label' => 'Layout Foo',
+        'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlock',
+      ),
+    );
+
+    // Create two languages: Spanish and German.
+    $this->languages = array('de', 'es');
+    foreach ($this->languages as $langcode) {
+      $language = new Language(array('langcode' => $langcode));
+      $languages[$langcode] = language_save($language);
+      // Set up translations for each mock block label.
+      $custom_strings = array();
+      foreach ($this->mockBlockExpectedDefinitions as $plugin_id => $definition) {
+        $custom_strings[$definition['label']] = $langcode . ' ' . $definition['label'];
+      }
+      variable_set('locale_custom_strings_' . $langcode, array('' => $custom_strings));
+    }
+  }
+
+  /**
+   * Check the translations of the cached plugin definitions.
+   */
+  public function testCacheDecoratorLanguage() {
+    $languages = $this->languages;
+    $this->drupalGet('plugin_definition_test');
+    foreach ($this->mockBlockExpectedDefinitions as $plugin_id => $definition) {
+      // Find our source text.
+      $this->assertText($definition['label']);
+    }
+    foreach ($languages as $langcode) {
+      $url = $langcode . '/plugin_definition_test';
+      // For each language visit the language specific version of the page again.
+      $this->drupalGet($url);
+      foreach ($this->mockBlockExpectedDefinitions as $plugin_id => $definition) {
+        // Find our provided translations.
+        $label = $langcode . ' ' . $definition['label'];
+        $this->assertText($label);
+      }
+    }
+    // Manually check that the expected cache keys are present.
+    $languages[] = 'en';
+    foreach ($languages as $langcode) {
+      $cache = cache()->get('mock_block:' . $langcode);
+      $this->assertEqual($cache->cid, 'mock_block:' . $langcode, format_string('The !cache cache exists.', array('!cache' => 'mock_block:' . $langcode)));
+      $this->assertEqual($cache->expire, 1542646800, format_string('The cache expiration was properly set.'));
+    }
+    // Delete cached items tagged with "plugin_test".
+    cache()->deleteTags(array('plugin_test'));
+    foreach ($languages as $langcode) {
+      $cache = cache()->get('mock_block:' . $langcode);
+      $this->assertFalse($cache, format_string('The !cache cache was properly cleared through the cache::deleteTags() method.', array('!cache' => 'mock_block:' . $langcode)));
+    }
+    // Change the translations for the german language and recheck strings.
+    $custom_strings = array();
+    foreach ($this->mockBlockExpectedDefinitions as $plugin_id => $definition) {
+      $custom_strings[$definition['label']] = $definition['label'] . ' de';
+    }
+    variable_set('locale_custom_strings_de', array('' => $custom_strings));
+    $this->drupalGet('de/plugin_definition_test');
+    foreach ($this->mockBlockExpectedDefinitions as $plugin_id => $definition) {
+      // Find our provided translations.
+      $label = $definition['label'] . ' de';
+      $this->assertText($label);
+    }
+  }
+
+}

core/modules/system/tests/modules/plugin_test/lib/Drupal/plugin_test/Plugin/CachedMockBlockManager.php

@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\plugin_test\Plugin\CachedMockBlockManager.
+ */
+
+namespace Drupal\plugin_test\Plugin;
+
+use Drupal\Core\Plugin\Discovery\CacheDecorator;
+
+/**
+ * Defines a plugin manager used by Plugin API cache decorator web tests.
+ */
+class CachedMockBlockManager extends MockBlockManager {
+
+  /**
+   * Adds a cache decorator to the MockBlockManager's discovery.
+   *
+   * @see Drupal\plugin_test\Plugin\MockBlockManager::__construct().
+   */
+  public function __construct() {
+    parent::__construct();
+    // The CacheDecorator allows us to cache these plugin definitions for
+    // quicker retrieval. In this case we are generating a cache key by
+    // language.
+    $this->discovery = new CacheDecorator($this->discovery, 'mock_block:' . language(LANGUAGE_TYPE_INTERFACE)->langcode, 'cache',  1542646800, array('plugin_test'));
+  }
+}

core/modules/system/tests/modules/plugin_test/lib/Drupal/plugin_test/Plugin/MockBlockManager.php

@@ -39,7 +39,7 @@ class MockBlockManager extends PluginManagerBase {
 
     // A simple plugin: the user login block.
     $this->discovery->setDefinition('user_login', array(
-      'label' => 'User login',
+      'label' => t('User login'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserLoginBlock',
     ));
 
@@ -61,7 +61,7 @@ class MockBlockManager extends PluginManagerBase {
     // MockLayoutBlockDeriver class ensures that both the base plugin and the
     // derivatives are available to the system.
     $this->discovery->setDefinition('layout', array(
-      'label' => 'Layout',
+      'label' => t('Layout'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlock',
       'derivative' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlockDeriver',
     ));

core/modules/system/tests/modules/plugin_test/lib/Drupal/plugin_test/Plugin/plugin_test/mock_block/MockLayoutBlockDeriver.php

@@ -46,7 +46,7 @@ class MockLayoutBlockDeriver implements DerivativeInterface {
       // customized one, but in a real implementation, this would be fetched
       // from some config() object.
       'foo' => array(
-        'label' => 'Layout Foo',
+        'label' => t('Layout Foo'),
       ) + $base_plugin_definition,
     );
 

core/modules/system/tests/modules/plugin_test/lib/Drupal/plugin_test/Plugin/plugin_test/mock_block/MockMenuBlockDeriver.php

@@ -41,10 +41,10 @@ class MockMenuBlockDeriver implements DerivativeInterface {
     // Drupal's configuration to find out which menus actually exist.
     $derivatives = array(
       'main_menu' => array(
-        'label' => 'Main menu',
+        'label' => t('Main menu'),
       ) + $base_plugin_definition,
       'navigation' => array(
-        'label' => 'Navigation',
+        'label' => t('Navigation'),
       ) + $base_plugin_definition,
     );
 

core/modules/system/tests/modules/plugin_test/plugin_test.module

@@ -1,5 +1,7 @@
 <?php
 
+use Drupal\plugin_test\Plugin\CachedMockBlockManager;
+
 /**
  * @file
  * Helper module for the plugin tests.
@@ -14,3 +16,33 @@ function plugin_test_plugin_test_alter(&$definitions) {
   }
   $definitions['user_login']['altered_single'] = TRUE;
 }
+
+/**
+ * Implements hook_menu().
+ */
+function plugin_test_menu() {
+  $items = array();
+  $items['plugin_definition_test'] = array(
+    'access callback' => TRUE,
+    'page callback' => 'plugin_test_definitions',
+  );
+  return $items;
+}
+
+/**
+ * A simple page callback that prints plugin labels for testing.
+ *
+ * @return array
+ *   A simple renderable array of plugin labels.
+ */
+function plugin_test_definitions() {
+  $manager = new CachedMockBlockManager();
+  $output = array();
+  foreach($manager->getDefinitions() as $plugin_id => $definition) {
+    $output[$plugin_id] = array(
+      '#type' => 'markup',
+      '#markup' => $definition['label'],
+    );
+  }
+  return $output;
+}