From ed2b2238a5a8f2f62eb86fdbc3e4c48c64aa2ec3 Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Sun, 1 Sep 2013 12:03:34 +0100
Subject: [PATCH] Issue #2032303 by dawehner, pwolanin: Cache the result of
local tasks.
---
core/core.services.yml | 2 +-
.../lib/Drupal/Core/Menu/LocalTaskManager.php | 128 +++++-----
.../Tests/Core/Menu/LocalTaskManagerTest.php | 223 +++++++++++++++---
3 files changed, 262 insertions(+), 91 deletions(-)
diff --git a/core/core.services.yml b/core/core.services.yml
index f941e333fabd..891c2d576002 100644
--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -176,7 +176,7 @@ services:
arguments: ['@container.namespaces', '@controller_resolver', '@request', '@module_handler', '@cache.cache', '@language_manager']
plugin.manager.menu.local_task:
class: Drupal\Core\Menu\LocalTaskManager
- arguments: ['@container.namespaces', '@controller_resolver', '@request', '@router.route_provider', '@module_handler']
+ arguments: ['@container.namespaces', '@controller_resolver', '@request', '@router.route_provider', '@module_handler', '@cache.cache', '@language_manager']
request:
class: Symfony\Component\HttpFoundation\Request
# @TODO the synthetic setting must be uncommented whenever drupal_session_initialize()
diff --git a/core/lib/Drupal/Core/Menu/LocalTaskManager.php b/core/lib/Drupal/Core/Menu/LocalTaskManager.php
index 00837c5f1478..17b5128a0ef4 100644
--- a/core/lib/Drupal/Core/Menu/LocalTaskManager.php
+++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php
@@ -7,7 +7,10 @@
namespace Drupal\Core\Menu;
+use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Extension\ModuleHandlerInterface;
+use Drupal\Core\Language\Language;
+use Drupal\Core\Language\LanguageManager;
use Drupal\Core\Plugin\DefaultPluginManager;
use Drupal\Core\Routing\RouteProviderInterface;
use Symfony\Component\HttpFoundation\Request;
@@ -63,14 +66,19 @@ class LocalTaskManager extends DefaultPluginManager {
* @param \Drupal\Core\Routing\RouteProviderInterface $route_provider
* The route provider to load routes by name.
* @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
- * The module handler.u
+ * The module handler.
+ * @param \Drupal\Core\Cache\CacheBackendInterface $cache
+ * The cache backend.
+ * @param \Drupal\Core\Language\LanguageManager $language_manager
+ * The language manager.
*/
- public function __construct(\Traversable $namespaces, ControllerResolverInterface $controller_resolver, Request $request, RouteProviderInterface $route_provider, ModuleHandlerInterface $module_handler) {
+ public function __construct(\Traversable $namespaces, ControllerResolverInterface $controller_resolver, Request $request, RouteProviderInterface $route_provider, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache, LanguageManager $language_manager) {
parent::__construct('Plugin/Menu/LocalTask', $namespaces, array(), 'Drupal\Core\Annotation\Menu\LocalTask');
$this->controllerResolver = $controller_resolver;
$this->request = $request;
$this->routeProvider = $route_provider;
$this->alterInfo($module_handler, 'local_tasks');
+ $this->setCacheBackend($cache, $language_manager, 'local_task', array('local_task' => TRUE));
}
/**
@@ -118,64 +126,78 @@ public function getPath(LocalTaskInterface $local_task) {
public function getLocalTasksForRoute($route_name) {
if (!isset($this->instances[$route_name])) {
$this->instances[$route_name] = array();
- // @todo - optimize this lookup by compiling or caching.
- $definitions = $this->getDefinitions();
- // We build the hierarchy by finding all tabs that should
- // appear on the current route.
- $tab_root_ids = array();
- $parents = array();
- foreach ($definitions as $plugin_id => $task_info) {
- if ($route_name == $task_info['route_name']) {
- $tab_root_ids[$task_info['tab_root_id']] = TRUE;
- // Tabs that link to the current route are viable parents
- // and their parent and children should be visible also.
- // @todo - this only works for 2 levels of tabs.
- // instead need to iterate up.
- $parents[$plugin_id] = TRUE;
- if (!empty($task_info['tab_parent_id'])) {
- $parents[$task_info['tab_parent_id']] = TRUE;
- }
- }
+ if ($cache = $this->cacheBackend->get($this->cacheKey . ':' . $route_name)) {
+ $tab_root_ids = $cache->data['tab_root_ids'];
+ $parents = $cache->data['parents'];
+ $children = $cache->data['children'];
}
- if ($tab_root_ids) {
- // Find all the plugins with the same root and that are at the top
- // level or that have a visible parent.
+ else {
+ $definitions = $this->getDefinitions();
+ // We build the hierarchy by finding all tabs that should
+ // appear on the current route.
+ $tab_root_ids = array();
+ $parents = array();
$children = array();
- foreach ($definitions as $plugin_id => $task_info) {
- if (!empty($tab_root_ids[$task_info['tab_root_id']]) && (empty($task_info['tab_parent_id']) || !empty($parents[$task_info['tab_parent_id']]))) {
- // Concat '> ' with root ID for the parent of top-level tabs.
- $parent = empty($task_info['tab_parent_id']) ? '> ' . $task_info['tab_root_id'] : $task_info['tab_parent_id'];
- $children[$parent][$plugin_id] = $task_info;
+ foreach ($definitions as $plugin_id => $task_info) {
+ if ($route_name == $task_info['route_name']) {
+ $tab_root_ids[$task_info['tab_root_id']] = $task_info['tab_root_id'];
+ // Tabs that link to the current route are viable parents
+ // and their parent and children should be visible also.
+ // @todo - this only works for 2 levels of tabs.
+ // instead need to iterate up.
+ $parents[$plugin_id] = TRUE;
+ if (!empty($task_info['tab_parent_id'])) {
+ $parents[$task_info['tab_parent_id']] = TRUE;
+ }
}
}
- foreach (array_keys($tab_root_ids) as $root_id) {
- // Convert the tree keyed by plugin IDs into a simple one with
- // integer depth. Create instances for each plugin along the way.
- $level = 0;
- // We used this above as the top-level parent array key.
- $next_parent = '> ' . $root_id;
- do {
- $parent = $next_parent;
- $next_parent = FALSE;
- foreach ($children[$parent] as $plugin_id => $task_info) {
- $plugin = $this->createInstance($plugin_id);
- $this->instances[$route_name][$level][$plugin_id] = $plugin;
- // Normally, l() compares the href of every link with the current
- // path and sets the active class accordingly. But the parents of
- // the current local task may be on a different route in which
- // case we have to set the class manually by flagging it active.
- if (!empty($parents[$plugin_id]) && $route_name != $task_info['route_name']) {
- $plugin->setActive();
- }
- if (isset($children[$plugin_id])) {
- // This tab has visible children
- $next_parent = $plugin_id;
- }
+ if ($tab_root_ids) {
+ // Find all the plugins with the same root and that are at the top
+ // level or that have a visible parent.
+ foreach ($definitions as $plugin_id => $task_info) {
+ if (!empty($tab_root_ids[$task_info['tab_root_id']]) && (empty($task_info['tab_parent_id']) || !empty($parents[$task_info['tab_parent_id']]))) {
+ // Concat '> ' with root ID for the parent of top-level tabs.
+ $parent = empty($task_info['tab_parent_id']) ? '> ' . $task_info['tab_root_id'] : $task_info['tab_parent_id'];
+ $children[$parent][$plugin_id] = $task_info;
}
- $level++;
- } while ($next_parent);
+ }
}
+ $data = array(
+ 'tab_root_ids' => $tab_root_ids,
+ 'parents' => $parents,
+ 'children' => $children,
+ );
+ $this->cacheBackend->set($this->cacheKey . ':' . $route_name, $data, CacheBackendInterface::CACHE_PERMANENT, array('local_task'));
}
+ // Create a plugin instance for each element of the hierarchy.
+ foreach ($tab_root_ids as $root_id) {
+ // Convert the tree keyed by plugin IDs into a simple one with
+ // integer depth. Create instances for each plugin along the way.
+ $level = 0;
+ // We used this above as the top-level parent array key.
+ $next_parent = '> ' . $root_id;
+ do {
+ $parent = $next_parent;
+ $next_parent = FALSE;
+ foreach ($children[$parent] as $plugin_id => $task_info) {
+ $plugin = $this->createInstance($plugin_id);
+ $this->instances[$route_name][$level][$plugin_id] = $plugin;
+ // Normally, l() compares the href of every link with the current
+ // path and sets the active class accordingly. But the parents of
+ // the current local task may be on a different route in which
+ // case we have to set the class manually by flagging it active.
+ if (!empty($parents[$plugin_id]) && $route_name != $task_info['route_name']) {
+ $plugin->setActive();
+ }
+ if (isset($children[$plugin_id])) {
+ // This tab has visible children
+ $next_parent = $plugin_id;
+ }
+ }
+ $level++;
+ } while ($next_parent);
+ }
+
}
return $this->instances[$route_name];
}
diff --git a/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php b/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php
index 5d61a9ca146c..8ac918d05fc3 100644
--- a/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php
+++ b/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php
@@ -8,6 +8,8 @@
namespace Drupal\Tests\Core\Menu;
use Drupal\Component\Plugin\Factory\DefaultFactory;
+use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\Language\Language;
use Drupal\system\Plugin\Type\MenuLocalTaskManager;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\HttpFoundation\Request;
@@ -23,7 +25,7 @@ class LocalTaskManagerTest extends UnitTestCase {
/**
* The tested manager.
*
- * @var \Drupal\system\Plugin\Type\MenuLocalTaskManager
+ * @var \Drupal\Core\Menu\LocalTaskManager
*/
protected $manager;
@@ -62,6 +64,13 @@ class LocalTaskManagerTest extends UnitTestCase {
*/
protected $factory;
+ /**
+ * The cache backend used in the test.
+ *
+ * @var \PHPUnit_Framework_MockObject_MockObject
+ */
+ protected $cacheBackend;
+
public static function getInfo() {
return array(
'name' => 'Local tasks manager.',
@@ -81,6 +90,7 @@ protected function setUp() {
$this->routeProvider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface');
$this->pluginDiscovery = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface');
$this->factory = $this->getMock('Drupal\Component\Plugin\Factory\FactoryInterface');
+ $this->cacheBackend = $this->getMock('Drupal\Core\Cache\CacheBackendInterface');
$this->setupLocalTaskManager();
}
@@ -91,53 +101,94 @@ protected function setUp() {
* @see \Drupal\system\Plugin\Type\MenuLocalTaskManager::getLocalTasksForRoute()
*/
public function testGetLocalTasksForRouteSingleLevelTitle() {
- $definitions = array();
- $definitions['menu_local_task_test_tasks_settings'] = array(
- 'id' => 'menu_local_task_test_tasks_settings',
- 'route_name' => 'menu_local_task_test_tasks_settings',
- 'title' => 'Settings',
- 'tab_root_id' => 'menu_local_task_test_tasks_view',
- 'class' => 'Drupal\menu_test\Plugin\Menu\MenuLocalTasksTestTasksSettings',
- );
- $definitions['menu_local_task_test_tasks_edit'] = array(
- 'id' => 'menu_local_task_test_tasks_edit',
- 'route_name' => 'menu_local_task_test_tasks_edit',
- 'title' => 'Settings',
- 'tab_root_id' => 'menu_local_task_test_tasks_view',
- 'class' => 'Drupal\menu_test\Plugin\Menu\MenuLocalTasksTestTasksEdit',
- 'weight' => 20,
- );
- $definitions['menu_local_task_test_tasks_view'] = array(
- 'id' => 'menu_local_task_test_tasks_view',
- 'route_name' => 'menu_local_task_test_tasks_view',
- 'title' => 'Settings',
- 'tab_root_id' => 'menu_local_task_test_tasks_view',
- 'class' => 'Drupal\menu_test\Plugin\Menu\MenuLocalTasksTestTasksView',
- );
+ $definitions = $this->getLocalTaskFixtures();
- $this->pluginDiscovery->expects($this->any())
+ $this->pluginDiscovery->expects($this->once())
->method('getDefinitions')
->will($this->returnValue($definitions));
$mock_plugin = $this->getMock('Drupal\Core\Menu\LocalTaskInterface');
- $map = array(
- array('menu_local_task_test_tasks_settings', array(), $mock_plugin),
- array('menu_local_task_test_tasks_edit', array(), $mock_plugin),
- array('menu_local_task_test_tasks_view', array(), $mock_plugin),
+ $this->setupFactory($mock_plugin);
+ $this->setupLocalTaskManager();
+
+ $local_tasks = $this->manager->getLocalTasksForRoute('menu_local_task_test_tasks_view');
+
+ $result = array(
+ 0 => array(
+ 'menu_local_task_test_tasks_settings' => $mock_plugin,
+ 'menu_local_task_test_tasks_view' => $mock_plugin,
+ 'menu_local_task_test_tasks_edit' => $mock_plugin,
+ )
);
- $this->factory->expects($this->any())
- ->method('createInstance')
- ->will($this->returnValueMap($map));
+
+ $this->assertEquals($result, $local_tasks);
+ }
+
+ /**
+ * Tests the cache of the local task manager with an empty initial cache.
+ */
+ public function testGetLocalTaskForRouteWithEmptyCache() {
+ $definitions = $this->getLocalTaskFixtures();
+
+ $this->pluginDiscovery->expects($this->once())
+ ->method('getDefinitions')
+ ->will($this->returnValue($definitions));
+
+ $mock_plugin = $this->getMock('Drupal\Core\Menu\LocalTaskInterface');
+ $this->setupFactory($mock_plugin);
$this->setupLocalTaskManager();
+ $result = $this->getLocalTasksForRouteResult($mock_plugin);
+
+ $this->cacheBackend->expects($this->at(0))
+ ->method('get')
+ ->with('local_task:en:menu_local_task_test_tasks_view');
+
+ $this->cacheBackend->expects($this->at(1))
+ ->method('get')
+ ->with('local_task:en');
+
+ $this->cacheBackend->expects($this->at(2))
+ ->method('set')
+ ->with('local_task:en', $definitions, CacheBackendInterface::CACHE_PERMANENT);
+
+ $expected_set = $this->getLocalTasksCache();
+
+ $this->cacheBackend->expects($this->at(3))
+ ->method('set')
+ ->with('local_task:en:menu_local_task_test_tasks_view', $expected_set, CacheBackendInterface::CACHE_PERMANENT, array('local_task'));
+
$local_tasks = $this->manager->getLocalTasksForRoute('menu_local_task_test_tasks_view');
- $this->assertEquals(array(0 => array(
- 'menu_local_task_test_tasks_settings' => $mock_plugin,
- 'menu_local_task_test_tasks_view' => $mock_plugin,
- 'menu_local_task_test_tasks_edit' => $mock_plugin,
- )), $local_tasks);
+ $this->assertEquals($result, $local_tasks);
+ }
+
+ /**
+ * Tests the cache of the local task manager with a filled initial cache.
+ */
+ public function testGetLocalTaskForRouteWithFilledCache() {
+ $this->pluginDiscovery->expects($this->never())
+ ->method('getDefinitions');
+
+ $mock_plugin = $this->getMock('Drupal\Core\Menu\LocalTaskInterface');
+ $this->setupFactory($mock_plugin);
+
+ $this->setupLocalTaskManager();
+
+ $result = $this->getLocalTasksCache($mock_plugin);
+
+ $this->cacheBackend->expects($this->at(0))
+ ->method('get')
+ ->with('local_task:en:menu_local_task_test_tasks_view')
+ ->will($this->returnValue((object) array('data' => $result)));
+
+ $this->cacheBackend->expects($this->never())
+ ->method('set');
+
+ $result = $this->getLocalTasksForRouteResult($mock_plugin);
+ $local_tasks = $this->manager->getLocalTasksForRoute('menu_local_task_test_tasks_view');
+ $this->assertEquals($result, $local_tasks);
}
/**
@@ -201,6 +252,104 @@ protected function setupLocalTaskManager() {
$property = new \ReflectionProperty('Drupal\Core\Menu\LocalTaskManager', 'factory');
$property->setAccessible(TRUE);
$property->setValue($this->manager, $this->factory);
+
+ $language_manager = $this->getMockBuilder('Drupal\Core\Language\LanguageManager')
+ ->disableOriginalConstructor()
+ ->getMock();
+ $language_manager->expects($this->any())
+ ->method('getLanguage')
+ ->will($this->returnValue(new Language(array('id' => 'en'))));
+
+ $this->manager->setCacheBackend($this->cacheBackend, $language_manager, 'local_task');
+ }
+
+ /**
+ * Return some local tasks plugin definitions.
+ *
+ * @return array
+ * An array of plugin definition keyed by plugin ID.
+ */
+ protected function getLocalTaskFixtures() {
+ $definitions = array();
+ $definitions['menu_local_task_test_tasks_settings'] = array(
+ 'id' => 'menu_local_task_test_tasks_settings',
+ 'route_name' => 'menu_local_task_test_tasks_settings',
+ 'title' => 'Settings',
+ 'tab_root_id' => 'menu_local_task_test_tasks_view',
+ 'class' => 'Drupal\menu_test\Plugin\Menu\MenuLocalTasksTestTasksSettings',
+ );
+ $definitions['menu_local_task_test_tasks_edit'] = array(
+ 'id' => 'menu_local_task_test_tasks_edit',
+ 'route_name' => 'menu_local_task_test_tasks_edit',
+ 'title' => 'Settings',
+ 'tab_root_id' => 'menu_local_task_test_tasks_view',
+ 'class' => 'Drupal\menu_test\Plugin\Menu\MenuLocalTasksTestTasksEdit',
+ 'weight' => 20,
+ );
+ $definitions['menu_local_task_test_tasks_view'] = array(
+ 'id' => 'menu_local_task_test_tasks_view',
+ 'route_name' => 'menu_local_task_test_tasks_view',
+ 'title' => 'Settings',
+ 'tab_root_id' => 'menu_local_task_test_tasks_view',
+ 'class' => 'Drupal\menu_test\Plugin\Menu\MenuLocalTasksTestTasksView',
+ );
+ return $definitions;
+ }
+
+ /**
+ * Setups the plugin factory with some local task plugins.
+ *
+ * @param \PHPUnit_Framework_MockObject_MockObject $mock_plugin
+ * The mock plugin.
+ */
+ protected function setupFactory($mock_plugin) {
+ $map = array(
+ array('menu_local_task_test_tasks_settings', array(), $mock_plugin),
+ array('menu_local_task_test_tasks_edit', array(), $mock_plugin),
+ array('menu_local_task_test_tasks_view', array(), $mock_plugin),
+ );
+ $this->factory->expects($this->any())
+ ->method('createInstance')
+ ->will($this->returnValueMap($map));
+ }
+
+ /**
+ * Returns an expected result for getLocalTasksForRoute.
+ *
+ * @param \PHPUnit_Framework_MockObject_MockObject $mock_plugin
+ * The mock plugin.
+ *
+ * @return array
+ * The expected result, keyed by local task leve.
+ */
+ protected function getLocalTasksForRouteResult($mock_plugin) {
+ $result = array(
+ 0 => array(
+ 'menu_local_task_test_tasks_settings' => $mock_plugin,
+ 'menu_local_task_test_tasks_view' => $mock_plugin,
+ 'menu_local_task_test_tasks_edit' => $mock_plugin,
+ )
+ );
+ return $result;
+ }
+
+ /**
+ * Returns the cache entry expected when running getLocalTaskForRoute().
+ *
+ * @return array
+ */
+ protected function getLocalTasksCache() {
+ return array(
+ 'tab_root_ids' => array(
+ 'menu_local_task_test_tasks_view' => 'menu_local_task_test_tasks_view',
+ ),
+ 'parents' => array(
+ 'menu_local_task_test_tasks_view' => 1,
+ ),
+ 'children' => array(
+ '> menu_local_task_test_tasks_view' => $this->getLocalTaskFixtures(),
+ )
+ );
}
}
--
GitLab