diff --git a/core/core.services.yml b/core/core.services.yml index 0b8be53..3dd8d2f 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -185,8 +185,8 @@ services: abstract: true arguments: ['@container.namespaces', '@cache.default', '@language_manager', '@module_handler'] module_handler: - class: Drupal\Core\Extension\CachedModuleHandler - arguments: ['%container.modules%', '@state', '@cache.bootstrap'] + class: Drupal\Core\Extension\ModuleHandler + arguments: ['%container.modules%', '@cache.bootstrap'] theme_handler: class: Drupal\Core\Extension\ThemeHandler arguments: ['@config.factory', '@module_handler', '@cache.default', '@info_parser', '@config.installer', '@router.builder'] diff --git a/core/lib/Drupal/Core/DependencyInjection/UpdateServiceProvider.php b/core/lib/Drupal/Core/DependencyInjection/UpdateServiceProvider.php index aa3c709..01e0bfc 100644 --- a/core/lib/Drupal/Core/DependencyInjection/UpdateServiceProvider.php +++ b/core/lib/Drupal/Core/DependencyInjection/UpdateServiceProvider.php @@ -34,7 +34,8 @@ public function register(ContainerBuilder $container) { ->register('config.storage', 'Drupal\Core\Config\FileStorage') ->addArgument(config_get_config_directory(CONFIG_ACTIVE_DIRECTORY)); $container->register('module_handler', 'Drupal\Core\Extension\UpdateModuleHandler') - ->addArgument('%container.modules%'); + ->addArgument('%container.modules%') + ->addArgument(new Reference('cache.bootstrap')); $container ->register('cache_factory', 'Drupal\Core\Cache\MemoryBackendFactory'); $container diff --git a/core/lib/Drupal/Core/EventSubscriber/RequestCloseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/RequestCloseSubscriber.php index c0b1f17..1422a83 100644 --- a/core/lib/Drupal/Core/EventSubscriber/RequestCloseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/RequestCloseSubscriber.php @@ -8,7 +8,6 @@ namespace Drupal\Core\EventSubscriber; use Drupal\Core\Extension\ModuleHandlerInterface; -use Drupal\Core\Extension\CachedModuleHandlerInterface; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\HttpKernel\Event\PostResponseEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -42,9 +41,7 @@ function __construct(ModuleHandlerInterface $module_handler) { * The Event to process. */ public function onTerminate(PostResponseEvent $event) { - if ($this->moduleHandler instanceof CachedModuleHandlerInterface) { - $this->moduleHandler->writeCache(); - } + $this->moduleHandler->writeCache(); } /** diff --git a/core/lib/Drupal/Core/Extension/CachedModuleHandler.php b/core/lib/Drupal/Core/Extension/CachedModuleHandler.php deleted file mode 100644 index 0def7f3..0000000 --- a/core/lib/Drupal/Core/Extension/CachedModuleHandler.php +++ /dev/null @@ -1,149 +0,0 @@ -state = $state; - $this->bootstrapCache = $bootstrap_cache; - } - - /** - * Overrides \Drupal\Core\Extension\ModuleHandler::getHookInfo(). - */ - public function getHookInfo() { - // When this function is indirectly invoked from bootstrap_invoke_all() prior - // to all modules being loaded, we do not want to cache an incomplete - // hook_hookInfo() result, so instead return an empty array. This requires - // bootstrap hook implementations to reside in the .module file, which is - // optimal for performance anyway. - if (!$this->loaded) { - return array(); - } - if (!isset($this->hookInfo)) { - if ($cache = $this->bootstrapCache->get('hook_info')) { - $this->hookInfo = $cache->data; - } - else { - $this->hookInfo = parent::getHookInfo(); - $this->bootstrapCache->set('hook_info', $this->hookInfo); - } - } - return $this->hookInfo; - } - - /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::resetImplementations(). - */ - public function resetImplementations() { - // We maintain a persistent cache of hook implementations in addition to the - // static cache to avoid looping through every module and every hook on each - // request. Benchmarks show that the benefit of this caching outweighs the - // additional database hit even when using the default database caching - // backend and only a small number of modules are enabled. The cost of the - // $this->bootstrapCache->get() is more or less constant and reduced further when - // non-database caching backends are used, so there will be more significant - // gains when a large number of modules are installed or hooks invoked, since - // this can quickly lead to \Drupal::moduleHandler()->implementsHook() being - // called several thousand times per request. - parent::resetImplementations(); - $this->bootstrapCache->set('module_implements', array()); - $this->bootstrapCache->delete('hook_info'); - } - - /** - * Implements \Drupal\Core\Extension\CachedModuleHandlerInterface::writeCache(). - */ - public function writeCache() { - if ($this->cacheNeedsWriting) { - $this->bootstrapCache->set('module_implements', $this->implementations); - $this->cacheNeedsWriting = FALSE; - } - } - - /** - * Overrides \Drupal\Core\Extension\ModuleHandler::getImplementationInfo(). - */ - protected function getImplementationInfo($hook) { - if (!isset($this->implementations)) { - $this->implementations = $this->getCachedImplementationInfo(); - } - if (!isset($this->implementations[$hook])) { - // The hook is not cached, so ensure that whether or not it has - // implementations, the cache is updated at the end of the request. - $this->cacheNeedsWriting = TRUE; - $this->implementations[$hook] = parent::getImplementationInfo($hook); - } - else { - foreach ($this->implementations[$hook] as $module => $group) { - // If this hook implementation is stored in a lazy-loaded file, include - // that file first. - if ($group) { - $this->loadInclude($module, 'inc', "$module.$group"); - } - // It is possible that a module removed a hook implementation without the - // implementations cache being rebuilt yet, so we check whether the - // function exists on each request to avoid undefined function errors. - // Since \Drupal::moduleHandler()->implementsHook() may needlessly try to - // load the include file again, function_exists() is used directly here. - if (!function_exists($module . '_' . $hook)) { - // Clear out the stale implementation from the cache and force a cache - // refresh to forget about no longer existing hook implementations. - unset($this->implementations[$hook][$module]); - $this->cacheNeedsWriting = TRUE; - } - } - } - return $this->implementations[$hook]; - } - - /** - * Retrieves hook implementation info from the cache. - */ - protected function getCachedImplementationInfo() { - if ($cache = $this->bootstrapCache->get('module_implements')) { - return $cache->data; - } - else { - return array(); - } - } - -} diff --git a/core/lib/Drupal/Core/Extension/CachedModuleHandlerInterface.php b/core/lib/Drupal/Core/Extension/CachedModuleHandlerInterface.php deleted file mode 100644 index cca18dc..0000000 --- a/core/lib/Drupal/Core/Extension/CachedModuleHandlerInterface.php +++ /dev/null @@ -1,20 +0,0 @@ -moduleList = array(); foreach ($module_list as $name => $module) { $this->moduleList[$name] = new Extension($module['type'], $module['pathname'], $module['filename']); } + $this->cacheBackend = $cache_backend; } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::load(). + * {@inheritdoc} */ public function load($name) { if (isset($this->loadedFiles[$name])) { @@ -97,7 +114,7 @@ public function load($name) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::loadAll(). + * {@inheritdoc} */ public function loadAll() { if (!$this->loaded) { @@ -109,7 +126,7 @@ public function loadAll() { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::reload(). + * {@inheritdoc} */ public function reload() { $this->loaded = FALSE; @@ -117,21 +134,21 @@ public function reload() { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::isLoaded(). + * {@inheritdoc} */ public function isLoaded() { return $this->loaded; } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::getModuleList(). + * {@inheritdoc} */ public function getModuleList() { return $this->moduleList; } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::setModuleList(). + * {@inheritdoc} */ public function setModuleList(array $module_list = array()) { $this->moduleList = $module_list; @@ -172,7 +189,7 @@ protected function add($type, $name, $path) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::buildModuleDependencies(). + * {@inheritdoc} */ public function buildModuleDependencies(array $modules) { foreach ($modules as $module) { @@ -195,14 +212,14 @@ public function buildModuleDependencies(array $modules) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::moduleExists(). + * {@inheritdoc} */ public function moduleExists($module) { return isset($this->moduleList[$module]); } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::loadAllIncludes(). + * {@inheritdoc} */ public function loadAllIncludes($type, $name = NULL) { foreach ($this->moduleList as $module => $filename) { @@ -211,7 +228,7 @@ public function loadAllIncludes($type, $name = NULL) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::loadInclude(). + * {@inheritdoc} */ public function loadInclude($module, $type, $name = NULL) { if ($type == 'install') { @@ -232,17 +249,31 @@ public function loadInclude($module, $type, $name = NULL) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::getHookInfo(). + * {@inheritdoc} */ public function getHookInfo() { - if (isset($this->hookInfo)) { - return $this->hookInfo; + if (!isset($this->hookInfo)) { + if ($cache = $this->cacheBackend->get('hook_info')) { + $this->hookInfo = $cache->data; + } + else { + $this->buildHookInfo(); + $this->cacheBackend->set('hook_info', $this->hookInfo); + } } + return $this->hookInfo; + } + + /** + * Builds hook_hook_info() information. + * + * @see \Drupal\Core\Extension\ModuleHandler::getHookInfo() + */ + protected function buildHookInfo() { $this->hookInfo = array(); - // We can't use $this->invokeAll() here or it would cause an infinite - // loop. // Make sure that the modules are loaded before checking. $this->reload(); + // $this->invokeAll() would cause an infinite recursion. foreach ($this->moduleList as $module => $filename) { $function = $module . '_hook_info'; if (function_exists($function)) { @@ -252,11 +283,10 @@ public function getHookInfo() { } } } - return $this->hookInfo; } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::getImplementations(). + * {@inheritdoc} */ public function getImplementations($hook) { $implementations = $this->getImplementationInfo($hook); @@ -264,16 +294,39 @@ public function getImplementations($hook) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::resetImplementations(). + * {@inheritdoc} + */ + public function writeCache() { + if ($this->cacheNeedsWriting) { + $this->cacheBackend->set('module_implements', $this->implementations); + $this->cacheNeedsWriting = FALSE; + } + } + + /** + * {@inheritdoc} */ public function resetImplementations() { $this->implementations = NULL; $this->hookInfo = NULL; $this->alterFunctions = NULL; + // We maintain a persistent cache of hook implementations in addition to the + // static cache to avoid looping through every module and every hook on each + // request. Benchmarks show that the benefit of this caching outweighs the + // additional database hit even when using the default database caching + // backend and only a small number of modules are enabled. The cost of the + // $this->cacheBackend->get() is more or less constant and reduced further + // when non-database caching backends are used, so there will be more + // significant gains when a large number of modules are installed or hooks + // invoked, since this can quickly lead to + // \Drupal::moduleHandler()->implementsHook() being called several thousand + // times per request. + $this->cacheBackend->set('module_implements', array()); + $this->cacheBackend->delete('hook_info'); } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::implementsHook(). + * {@inheritdoc} */ public function implementsHook($module, $hook) { $function = $module . '_' . $hook; @@ -293,7 +346,7 @@ public function implementsHook($module, $hook) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::invoke(). + * {@inheritdoc} */ public function invoke($module, $hook, array $args = array()) { if (!$this->implementsHook($module, $hook)) { @@ -304,7 +357,7 @@ public function invoke($module, $hook, array $args = array()) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::invokeAll(). + * {@inheritdoc} */ public function invokeAll($hook, array $args = array()) { $return = array(); @@ -326,7 +379,7 @@ public function invokeAll($hook, array $args = array()) { } /** - * Implements \Drupal\Core\Extension\ModuleHandlerInterface::alter(). + * {@inheritdoc} */ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { // Most of the time, $type is passed as a string, so for performance, @@ -338,8 +391,8 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { $extra_types = $type; $type = array_shift($extra_types); // Allow if statements in this function to use the faster isset() rather - // than !empty() both when $type is passed as a string, or as an array with - // one item. + // than !empty() both when $type is passed as a string, or as an array + // with one item. if (empty($extra_types)) { unset($extra_types); } @@ -357,8 +410,8 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { $modules = $this->getImplementations($hook); if (!isset($extra_types)) { // For the more common case of a single hook, we do not need to call - // function_exists(), since $this->getImplementations() returns only modules with - // implementations. + // function_exists(), since $this->getImplementations() returns only + // modules with implementations. foreach ($modules as $module) { $this->alterFunctions[$cid][] = $module . '_' . $hook; } @@ -372,22 +425,24 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { } // If any modules implement one of the extra hooks that do not implement // the primary hook, we need to add them to the $modules array in their - // appropriate order. $this->getImplementations() can only return ordered - // implementations of a single hook. To get the ordered implementations - // of multiple hooks, we mimic the $this->getImplementations() logic of first - // ordering by $this->getModuleList(), and then calling + // appropriate order. $this->getImplementations() can only return + // ordered implementations of a single hook. To get the ordered + // implementations of multiple hooks, we mimic the + // $this->getImplementations() logic of first ordering by + // $this->getModuleList(), and then calling // $this->alter('module_implements'). if (array_diff($extra_modules, $modules)) { // Merge the arrays and order by getModuleList(). $modules = array_intersect(array_keys($this->moduleList), array_merge($modules, $extra_modules)); - // Since $this->getImplementations() already took care of loading the necessary - // include files, we can safely pass FALSE for the array values. + // Since $this->getImplementations() already took care of loading the + // necessary include files, we can safely pass FALSE for the array + // values. $implementations = array_fill_keys($modules, FALSE); // Let modules adjust the order solely based on the primary hook. This // ensures the same module order regardless of whether this if block - // runs. Calling $this->alter() recursively in this way does not result - // in an infinite loop, because this call is for a single $type, so we - // won't end up in this code block again. + // runs. Calling $this->alter() recursively in this way does not + // result in an infinite loop, because this call is for a single + // $type, so we won't end up in this code block again. $this->alter('module_implements', $implementations, $hook); $modules = array_keys($implementations); } @@ -450,9 +505,55 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { * hook_hook_info() or FALSE if the implementation is in the module file. */ protected function getImplementationInfo($hook) { - if (isset($this->implementations[$hook])) { - return $this->implementations[$hook]; + if (!isset($this->implementations)) { + $this->implementations = array(); + if ($cache = $this->cacheBackend->get('module_implements')) { + $this->implementations = $cache->data; + } + } + if (!isset($this->implementations[$hook])) { + // The hook is not cached, so ensure that whether or not it has + // implementations, the cache is updated at the end of the request. + $this->cacheNeedsWriting = TRUE; + $this->implementations[$hook] = $this->buildImplementationInfo($hook); } + else { + foreach ($this->implementations[$hook] as $module => $group) { + // If this hook implementation is stored in a lazy-loaded file, include + // that file first. + if ($group) { + $this->loadInclude($module, 'inc', "$module.$group"); + } + // It is possible that a module removed a hook implementation without + // the implementations cache being rebuilt yet, so we check whether the + // function exists on each request to avoid undefined function errors. + // Since ModuleHandler::implementsHook() may needlessly try to + // load the include file again, function_exists() is used directly here. + if (!function_exists($module . '_' . $hook)) { + // Clear out the stale implementation from the cache and force a cache + // refresh to forget about no longer existing hook implementations. + unset($this->implementations[$hook][$module]); + $this->cacheNeedsWriting = TRUE; + } + } + } + return $this->implementations[$hook]; + } + + /** + * Builds hook implementation information for a given hook name. + * + * @param string $hook + * The name of the hook (e.g. "help" or "menu"). + * + * @return array + * An array whose keys are the names of the modules which are implementing + * this hook and whose values are either an array of information from + * hook_hook_info() or FALSE if the implementation is in the module file. + * + * @see \Drupal\Core\Extension\ModuleHandler::getImplementationInfo() + */ + protected function buildImplementationInfo($hook) { $this->implementations[$hook] = array(); $hook_info = $this->getHookInfo(); foreach ($this->moduleList as $module => $filename) { @@ -552,8 +653,8 @@ public function install(array $module_list, $enable_dependencies = TRUE) { // Conditionally add the dependencies to the list of modules. if ($enable_dependencies) { - // Add dependencies to the list. The new modules will be processed as the - // while loop continues. + // Add dependencies to the list. The new modules will be processed as + // the while loop continues. while (list($module) = each($module_list)) { foreach (array_keys($module_data[$module]->requires) as $dependency) { if (!isset($module_data[$dependency])) { @@ -600,14 +701,15 @@ public function install(array $module_list, $enable_dependencies = TRUE) { ->save(); // Prepare the new module list, sorted by weight, including filenames. - // This list is used for both the ModuleHandler and DrupalKernel. It needs - // to be kept in sync between both. A DrupalKernel reboot or rebuild will - // automatically re-instantiate a new ModuleHandler that uses the new - // module list of the kernel. However, DrupalKernel does not cause any - // modules to be loaded. - // Furthermore, the currently active (fixed) module list can be different - // from the configured list of enabled modules. For all active modules not - // contained in the configured enabled modules, we assume a weight of 0. + // This list is used for both the ModuleHandler and DrupalKernel. It + // needs to be kept in sync between both. A DrupalKernel reboot or + // rebuild will automatically re-instantiate a new ModuleHandler that + // uses the new module list of the kernel. However, DrupalKernel does + // not cause any modules to be loaded. + // Furthermore, the currently active (fixed) module list can be + // different from the configured list of enabled modules. For all active + // modules not contained in the configured enabled modules, we assume a + // weight of 0. $current_module_filenames = $this->getModuleList(); $current_modules = array_fill_keys(array_keys($current_module_filenames), 0); $current_modules = module_config_sort(array_merge($current_modules, $extension_config->get('module'))); @@ -625,10 +727,10 @@ public function install(array $module_list, $enable_dependencies = TRUE) { } // Update the module handler in order to load the module's code. - // This allows the module to participate in hooks and its existence to be - // discovered by other modules. - // The current ModuleHandler instance is obsolete with the kernel rebuild - // below. + // This allows the module to participate in hooks and its existence to + // be discovered by other modules. + // The current ModuleHandler instance is obsolete with the kernel + // rebuild below. $this->setModuleList($module_filenames); $this->load($module); module_load_install($module); @@ -641,8 +743,8 @@ public function install(array $module_list, $enable_dependencies = TRUE) { // Update the kernel to include it. // This reboots the kernel to register the module's bundle and its // services in the service container. The $module_filenames argument is - // taken over as %container.modules% parameter, which is passed to a fresh - // ModuleHandler instance upon first retrieval. + // taken over as %container.modules% parameter, which is passed to a + // fresh ModuleHandler instance upon first retrieval. // @todo install_begin_request() creates a container without a kernel. if ($kernel = \Drupal::service('kernel', ContainerInterface::NULL_ON_INVALID_REFERENCE)) { $kernel->updateModules($module_filenames, $module_filenames); diff --git a/core/lib/Drupal/Core/Extension/ModuleHandlerInterface.php b/core/lib/Drupal/Core/Extension/ModuleHandlerInterface.php index 2b6d1df..6406698 100644 --- a/core/lib/Drupal/Core/Extension/ModuleHandlerInterface.php +++ b/core/lib/Drupal/Core/Extension/ModuleHandlerInterface.php @@ -175,6 +175,11 @@ public function getHookInfo(); public function getImplementations($hook); /** + * Write the hook implementation info to the cache. + */ + public function writeCache(); + + /** * Resets the cached list of hook implementations. */ public function resetImplementations(); diff --git a/core/modules/system/lib/Drupal/system/Tests/Plugin/PluginTestBase.php b/core/modules/system/lib/Drupal/system/Tests/Plugin/PluginTestBase.php index 50d5736..3a92dbd 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Plugin/PluginTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/Plugin/PluginTestBase.php @@ -11,6 +11,7 @@ use Drupal\plugin_test\Plugin\TestPluginManager; use Drupal\plugin_test\Plugin\MockBlockManager; use Drupal\plugin_test\Plugin\DefaultsTestPluginManager; +use Drupal\Core\Cache\MemoryBackend; use Drupal\Core\Extension\ModuleHandler; /** @@ -37,7 +38,7 @@ public function setUp() { // as derivatives and ReflectionFactory. $this->testPluginManager = new TestPluginManager(); $this->mockBlockManager = new MockBlockManager(); - $module_handler = new ModuleHandler(); + $module_handler = new ModuleHandler(array(), new MemoryBackend('plugin')); $this->defaultsTestPluginManager = new DefaultsTestPluginManager($module_handler); // The expected plugin definitions within each manager. Several tests assert diff --git a/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerUnitTest.php b/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerUnitTest.php deleted file mode 100644 index d6fbff2..0000000 --- a/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerUnitTest.php +++ /dev/null @@ -1,42 +0,0 @@ - 'ModuleHandler functionality', - 'description' => 'Tests the ModuleHandler class.', - 'group' => 'System', - ); - } - - function setUp() { - parent::setUp(); - $this->moduleHandler = new ModuleHandler; - } - - /** - * Tests loading of an include from a nonexistent module. - */ - public function testLoadInclude() { - // Attepmting to load a file from a non-existent module should return FALSE. - $this->assertFalse($this->moduleHandler->loadInclude('foo', 'inc')); - } - -} diff --git a/core/tests/Drupal/Tests/Core/Plugin/DefaultPluginManagerTest.php b/core/tests/Drupal/Tests/Core/Plugin/DefaultPluginManagerTest.php index bd1efc2..4862ac7 100644 --- a/core/tests/Drupal/Tests/Core/Plugin/DefaultPluginManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Plugin/DefaultPluginManagerTest.php @@ -105,7 +105,9 @@ public function testDefaultPluginManager() { * Tests the plugin manager with no cache and altering. */ public function testDefaultPluginManagerWithAlter() { - $module_handler = $this->getMock('Drupal\Core\Extension\ModuleHandler'); + $module_handler = $this->getMockBuilder('Drupal\Core\Extension\ModuleHandler') + ->disableOriginalConstructor() + ->getMock(); // Configure the stub. $alter_hook_name = $this->randomName();