summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathaniel Catchpole2014-12-31 11:14:07 (GMT)
committerNathaniel Catchpole2014-12-31 11:14:07 (GMT)
commit78e9940a30339138a511aa78d2f56fc079fc65bf (patch)
tree4e493cca45ba9fd8ca4810d505f5f6bb0c67408d
parent4a6fce7b9aacad09b0a53285221dfe4c88737345 (diff)
Issue #1719648 by joachim, ttkaminski, kshama_deshmukh: ModuleInstaller::install() silently fails if a module isn't in the file system
-rw-r--r--core/lib/Drupal/Core/Extension/MissingDependencyException.php15
-rw-r--r--core/lib/Drupal/Core/Extension/ModuleInstaller.php13
-rw-r--r--core/lib/Drupal/Core/Extension/ModuleInstallerInterface.php5
-rw-r--r--core/modules/config/src/Tests/ConfigInstallWebTest.php3
-rw-r--r--core/modules/simpletest/src/Tests/SimpleTestTest.php2
-rw-r--r--core/modules/simpletest/src/WebTestBase.php11
-rw-r--r--core/modules/system/src/Form/ModulesListConfirmForm.php4
-rw-r--r--core/modules/system/src/Tests/Extension/ModuleHandlerTest.php10
8 files changed, 52 insertions, 11 deletions
diff --git a/core/lib/Drupal/Core/Extension/MissingDependencyException.php b/core/lib/Drupal/Core/Extension/MissingDependencyException.php
new file mode 100644
index 0000000..1cb07cd
--- /dev/null
+++ b/core/lib/Drupal/Core/Extension/MissingDependencyException.php
@@ -0,0 +1,15 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Extension\MissingDependencyException.
+ */
+
+namespace Drupal\Core\Extension;
+
+/**
+ * Exception class to throw when modules are missing on install.
+ *
+ * @see \Drupal\Core\Extension\ModuleInstaller::install()
+ */
+class MissingDependencyException extends \Exception {}
diff --git a/core/lib/Drupal/Core/Extension/ModuleInstaller.php b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
index c6f3921..73278d3 100644
--- a/core/lib/Drupal/Core/Extension/ModuleInstaller.php
+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -11,6 +11,7 @@ use Drupal\Component\Serialization\Yaml;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\DrupalKernelInterface;
+use Drupal\Component\Utility\String;
/**
* Default implementation of the module installer.
@@ -83,9 +84,12 @@ class ModuleInstaller implements ModuleInstallerInterface {
// Get all module data so we can find dependencies and sort.
$module_data = system_rebuild_module_data();
$module_list = $module_list ? array_combine($module_list, $module_list) : array();
- if (array_diff_key($module_list, $module_data)) {
+ if ($missing_modules = array_diff_key($module_list, $module_data)) {
// One or more of the given modules doesn't exist.
- return FALSE;
+ throw new MissingDependencyException(String::format('Unable to install modules %modules due to missing modules %missing.', array(
+ '%modules' => implode(', ', $module_list),
+ '%missing' => implode(', ', $missing_modules),
+ )));
}
// Only process currently uninstalled modules.
@@ -101,7 +105,10 @@ class ModuleInstaller implements ModuleInstallerInterface {
foreach (array_keys($module_data[$module]->requires) as $dependency) {
if (!isset($module_data[$dependency])) {
// The dependency does not exist.
- return FALSE;
+ throw new MissingDependencyException(String::format('Unable to install modules: module %module is missing its dependency module %dependency.', array(
+ '%module' => $module,
+ '%dependency' => $dependency,
+ )));
}
// Skip already installed modules.
diff --git a/core/lib/Drupal/Core/Extension/ModuleInstallerInterface.php b/core/lib/Drupal/Core/Extension/ModuleInstallerInterface.php
index 95cb25e..bd2923a 100644
--- a/core/lib/Drupal/Core/Extension/ModuleInstallerInterface.php
+++ b/core/lib/Drupal/Core/Extension/ModuleInstallerInterface.php
@@ -31,7 +31,10 @@ interface ModuleInstallerInterface {
* if you know $module_list is already complete.
*
* @return bool
- * FALSE if one or more dependencies are missing, TRUE otherwise.
+ * TRUE if the modules were successfully installed.
+ *
+ * @throws \Drupal\Core\Extension\MissingDependencyException
+ * Thrown when a requested module, or a dependency of one, can not be found.
*
* @see hook_module_preinstall()
* @see hook_install()
diff --git a/core/modules/config/src/Tests/ConfigInstallWebTest.php b/core/modules/config/src/Tests/ConfigInstallWebTest.php
index 5212973..4defbd4 100644
--- a/core/modules/config/src/Tests/ConfigInstallWebTest.php
+++ b/core/modules/config/src/Tests/ConfigInstallWebTest.php
@@ -132,8 +132,7 @@ class ConfigInstallWebTest extends WebTestBase {
// Turn on the test module, which will attempt to replace the
// configuration data. This attempt to replace the active configuration
// should be ignored.
- $status = \Drupal::service('module_installer')->install(array('config_existing_default_config_test'));
- $this->assertTrue($status, "The module config_existing_default_config_test was installed.");
+ \Drupal::service('module_installer')->install(array('config_existing_default_config_test'));
// Verify that the test module has not been able to change the data.
$config = $this->config($config_name);
diff --git a/core/modules/simpletest/src/Tests/SimpleTestTest.php b/core/modules/simpletest/src/Tests/SimpleTestTest.php
index d2949a6..6942119 100644
--- a/core/modules/simpletest/src/Tests/SimpleTestTest.php
+++ b/core/modules/simpletest/src/Tests/SimpleTestTest.php
@@ -210,7 +210,7 @@ EOD;
* Confirm that the stub test produced the desired results.
*/
function confirmStubTestResults() {
- $this->assertAssertion(t('Enabled modules: %modules', array('%modules' => 'non_existent_module')), 'Other', 'Fail', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->setUp()');
+ $this->assertAssertion(t('Unable to install modules %modules due to missing modules %missing.', array('%modules' => 'non_existent_module', '%missing' => 'non_existent_module')), 'Other', 'Fail', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->setUp()');
$this->assertAssertion($this->passMessage, 'Other', 'Pass', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->stubTest()');
$this->assertAssertion($this->failMessage, 'Other', 'Fail', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->stubTest()');
diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php
index 4109d83..2bd33fc 100644
--- a/core/modules/simpletest/src/WebTestBase.php
+++ b/core/modules/simpletest/src/WebTestBase.php
@@ -916,8 +916,15 @@ abstract class WebTestBase extends TestBase {
}
if ($modules) {
$modules = array_unique($modules);
- $success = $container->get('module_installer')->install($modules, TRUE);
- $this->assertTrue($success, String::format('Enabled modules: %modules', array('%modules' => implode(', ', $modules))));
+ try {
+ $success = $container->get('module_installer')->install($modules, TRUE);
+ $this->assertTrue($success, String::format('Enabled modules: %modules', array('%modules' => implode(', ', $modules))));
+ }
+ catch (\Drupal\Core\Extension\MissingDependencyException $e) {
+ // The exception message has all the details.
+ $this->fail($e->getMessage());
+ }
+
$this->rebuildContainer();
}
diff --git a/core/modules/system/src/Form/ModulesListConfirmForm.php b/core/modules/system/src/Form/ModulesListConfirmForm.php
index 2a31dbe..279b95e 100644
--- a/core/modules/system/src/Form/ModulesListConfirmForm.php
+++ b/core/modules/system/src/Form/ModulesListConfirmForm.php
@@ -153,6 +153,10 @@ class ModulesListConfirmForm extends ConfirmFormBase {
// Install the given modules.
if (!empty($this->modules['install'])) {
+ // Don't catch the exception that this can throw for missing dependencies:
+ // the form doesn't allow modules with unmet dependencies, so the only way
+ // this can happen is if the filesystem changed between form display and
+ // submit, in which case the user has bigger problems.
$this->moduleInstaller->install(array_keys($this->modules['install']));
}
diff --git a/core/modules/system/src/Tests/Extension/ModuleHandlerTest.php b/core/modules/system/src/Tests/Extension/ModuleHandlerTest.php
index 48fbf0e..130ffd2 100644
--- a/core/modules/system/src/Tests/Extension/ModuleHandlerTest.php
+++ b/core/modules/system/src/Tests/Extension/ModuleHandlerTest.php
@@ -116,8 +116,14 @@ class ModuleHandlerTest extends KernelTestBase {
\Drupal::state()->set('module_test.dependency', 'missing dependency');
drupal_static_reset('system_rebuild_module_data');
- $result = $this->moduleInstaller()->install(array('color'));
- $this->assertFalse($result, 'ModuleHandler::install() returns FALSE if dependencies are missing.');
+ try {
+ $result = $this->moduleInstaller()->install(array('color'));
+ $this->fail(t('ModuleInstaller::install() throws an exception if dependencies are missing.'));
+ }
+ catch (\Drupal\Core\Extension\MissingDependencyException $e) {
+ $this->pass(t('ModuleInstaller::install() throws an exception if dependencies are missing.'));
+ }
+
$this->assertFalse($this->moduleHandler()->moduleExists('color'), 'ModuleHandler::install() aborts if dependencies are missing.');
// Fix the missing dependency.