From 17e71a445693bee5cb890dcfcdc74554f6433e15 Mon Sep 17 00:00:00 2001 From: webchick <webchick@24967.no-reply.drupal.org> Date: Sun, 3 Mar 2013 18:43:45 -0500 Subject: [PATCH] Issue #1929656 by dawehner: Fixed ModuleHandler::setModuleList() should call resetImplementations() in order to avoid edge cases. --- core/includes/install.inc | 1 - core/includes/module.inc | 5 ----- core/lib/Drupal/Core/Extension/ModuleHandler.php | 3 +++ .../system/Tests/Module/EnableDisableTest.php | 16 +++++++++------- .../Drupal/system/Tests/Module/ModuleApiTest.php | 10 +++++++++- core/update.php | 4 ---- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/core/includes/install.inc b/core/includes/install.inc index d33bb0a97749..31817b30f576 100644 --- a/core/includes/install.inc +++ b/core/includes/install.inc @@ -432,7 +432,6 @@ function drupal_install_system() { // Update the module list to include it. drupal_container()->get('module_handler')->setModuleList(array('system' => $system_path . '/system.module')); - drupal_container()->get('module_handler')->resetImplementations(); config_install_default_config('module', 'system'); diff --git a/core/includes/module.inc b/core/includes/module.inc index d99b39dcf368..7c74096b9a63 100644 --- a/core/includes/module.inc +++ b/core/includes/module.inc @@ -339,9 +339,6 @@ function module_enable($module_list, $enable_dependencies = TRUE) { $module_handler->load($module); module_load_install($module); - // Reset the the hook implementations cache. - $module_handler->resetImplementations(); - // Flush theme info caches, since (testing) modules can implement // hook_system_theme_info() to register additional themes. system_list_reset(); @@ -499,8 +496,6 @@ function module_disable($module_list, $disable_dependents = TRUE) { if (!empty($invoke_modules)) { // @todo Most of the following should happen in above loop already. - // Refresh the module list to exclude the disabled modules. - $module_handler->resetImplementations(); // Refresh the system list to exclude the disabled modules. // @todo Only needed to rebuild theme info. diff --git a/core/lib/Drupal/Core/Extension/ModuleHandler.php b/core/lib/Drupal/Core/Extension/ModuleHandler.php index 8f45bfe00505..9f37e73d6083 100644 --- a/core/lib/Drupal/Core/Extension/ModuleHandler.php +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.php @@ -152,6 +152,9 @@ public function getModuleList() { */ public function setModuleList(array $module_list = array()) { $this->moduleList = $module_list; + // Reset the implementations, so a new call triggers a reloading of the + // available hooks. + $this->resetImplementations(); } /** diff --git a/core/modules/system/lib/Drupal/system/Tests/Module/EnableDisableTest.php b/core/modules/system/lib/Drupal/system/Tests/Module/EnableDisableTest.php index 2221aafe66f0..f8aa2b31bd05 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Module/EnableDisableTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Module/EnableDisableTest.php @@ -58,13 +58,13 @@ function testEnableDisable() { // Enable the dblog module first, since we will be asserting the presence // of log messages throughout the test. - if (isset($modules['dblog'])) { - $modules = array('dblog' => $modules['dblog']) + $modules; - } + if (isset($modules['dblog'])) { + $modules = array('dblog' => $modules['dblog']) + $modules; + } - // Set a variable so that the hook implementations in system_test.module - // will display messages via drupal_set_message(). - state()->set('system_test.verbose_module_hooks', TRUE); + // Set a variable so that the hook implementations in system_test.module + // will display messages via drupal_set_message(). + state()->set('system_test.verbose_module_hooks', TRUE); // Go through each module in the list and try to enable it (unless it was // already enabled automatically due to a dependency). @@ -192,7 +192,9 @@ function assertSuccessfulDisableAndUninstall($module, $package = 'Core') { // Check that the appropriate hook was fired and the appropriate log // message appears. $this->assertText(t('hook_modules_disabled fired for @module', array('@module' => $module))); - $this->assertLogMessage('system', "%module module disabled.", array('%module' => $module), WATCHDOG_INFO); + if ($module != 'dblog') { + $this->assertLogMessage('system', "%module module disabled.", array('%module' => $module), WATCHDOG_INFO); + } // Check that the module's database tables still exist. $this->assertModuleTablesExist($module); diff --git a/core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php b/core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php index 75ce7cdc0cd9..37eb5aef8a66 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php @@ -97,11 +97,19 @@ function testModuleImplements() { $this->drupalGet(''); $this->assertTrue(cache('bootstrap')->get('module_implements'), 'The module implements cache is populated after requesting a page.'); + // Prime ModuleHandler's hook implementation cache by invoking a random hook + // name. The subsequent module_enable() below will only call into + // setModuleList(), but will not explicitly reset the hook implementation + // cache, as that is expected to happen implicitly by setting the module + // list. This verifies that the hook implementation cache is cleared + // whenever setModuleList() is called. + $module_handler = drupal_container()->get('module_handler'); + $module_handler->invokeAll('test'); + // Make sure group include files are detected properly even when the file is // already loaded when the cache is rebuilt. // For that activate the module_test which provides the file to load. module_enable(array('module_test')); - $module_handler = drupal_container()->get('module_handler'); $module_handler->loadAll(); module_load_include('inc', 'module_test', 'module_test.file'); $modules = $module_handler->getImplementations('test_hook'); diff --git a/core/update.php b/core/update.php index 46d933baf15e..a43db07c01c3 100644 --- a/core/update.php +++ b/core/update.php @@ -444,10 +444,6 @@ function update_check_requirements($skip_warnings = FALSE) { $module_handler->setModuleList($module_list); $module_handler->load('system'); - // Reset the module implementations cache so that any new hook implementations - // in updated code are picked up. - $module_handler->resetImplementations(); - // Set up $language, since the installer components require it. drupal_language_initialize(); -- GitLab