diff --git a/core/lib/Drupal/Core/Config/ConfigInstaller.php b/core/lib/Drupal/Core/Config/ConfigInstaller.php index 4a6f580..4eeeab8 100644 --- a/core/lib/Drupal/Core/Config/ConfigInstaller.php +++ b/core/lib/Drupal/Core/Config/ConfigInstaller.php @@ -328,7 +328,7 @@ public function isSyncing() { /** * {@inheritdoc} */ - public function validatePreInstall($name, $type) { + public function findPreExistingConfiguration($type, $name) { $existing_configuration = array(); // Gather information about all the supported collections. $collection_info = $this->configManager->getConfigCollectionInfo(); diff --git a/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php b/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php index d8038c9..e385839 100644 --- a/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php +++ b/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php @@ -83,7 +83,16 @@ public function setSyncing($status); public function isSyncing(); /** - * Validate default configuration before installation. + * Finds pre-existing configuration objects for the provided extension. + * + * Extensions can not be installed if configuration objects exist in the + * active storage with the same names. This can happen in a number of ways, + * commonly: + * - if a user has created configuration with the same name as that provided + * by the extension. + * - if the extension provides default configuration that does not depend on + * it and the extension has been uninstalled and is about to the + * reinstalled. * * @param string $type * Type of extension to install. @@ -92,8 +101,7 @@ public function isSyncing(); * * @return array * Array of configuration objects that already exist keyed by collection. - * */ - public function validatePreInstall($type, $names); + public function findPreExistingConfiguration($type, $name); } diff --git a/core/lib/Drupal/Core/Config/ConfigManager.php b/core/lib/Drupal/Core/Config/ConfigManager.php index cde0eab..c2eb881 100644 --- a/core/lib/Drupal/Core/Config/ConfigManager.php +++ b/core/lib/Drupal/Core/Config/ConfigManager.php @@ -109,6 +109,19 @@ public function getEntityTypeIdByName($name) { /** * {@inheritdoc} */ + public function loadConfigEntityByName($name) { + $entity_type_id = $this->getEntityTypeIdByName($name); + if ($entity_type_id) { + $entity_type = $this->entityManager->getDefinition($entity_type_id); + $id = substr($name, strlen($entity_type->getConfigPrefix()) + 1); + return $this->entityManager->getStorage($entity_type_id)->load($id); + } + return NULL; + } + + /** + * {@inheritdoc} + */ public function getEntityManager() { return $this->entityManager; } diff --git a/core/lib/Drupal/Core/Config/ConfigManagerInterface.php b/core/lib/Drupal/Core/Config/ConfigManagerInterface.php index c0ea6cf..e13cfe3 100644 --- a/core/lib/Drupal/Core/Config/ConfigManagerInterface.php +++ b/core/lib/Drupal/Core/Config/ConfigManagerInterface.php @@ -24,6 +24,17 @@ public function getEntityTypeIdByName($name); /** + * Loads a configuration entity from the configuration name. + * + * @param string $name + * The configuration object name. + * + * @return \Drupal\Core\Entity\EntityInterface|null + * The configuration entity or NULL if it does not exist. + */ + public function loadConfigEntityByName($name); + + /** * Gets the entity manager. * * @return \Drupal\Core\Entity\EntityManagerInterface diff --git a/core/lib/Drupal/Core/Config/PreExistingConfigException.php b/core/lib/Drupal/Core/Config/PreExistingConfigException.php index 5a2cba7..eabb7f0 100644 --- a/core/lib/Drupal/Core/Config/PreExistingConfigException.php +++ b/core/lib/Drupal/Core/Config/PreExistingConfigException.php @@ -2,67 +2,13 @@ /** * @file - * Definition of Drupal\Core\Config\PreExistingConfigException. + * Contains \Drupal\Core\Config\PreExistingConfigException. */ namespace Drupal\Core\Config; /** - * An exception thrown in case of config already already existing errors. + * An exception thrown if configuration already exists with the same name. */ class PreExistingConfigException extends ConfigException { - - /** - * The extensions being installed. - * - * @var string - */ - protected $extensions = array(); - - /** - * The configuration names that already exists, keyed by collection. - * - * @var string - */ - protected $configNames = array(); - - /** - * Sets the extensions being installed. - * - * @param array $extensions - * The extensions being installed. - */ - public function setExtensions(array $extensions) { - $this->extensions = $extensions; - } - - /** - * Gets the extensions being installed. - * - * @return array - * The extensions being installed. - */ - public function getExtensions() { - return $this->extensions; - } - - /** - * Sets the configuration names. - * - * @param array $config_names - * The configuration names keyed by collection. - */ - public function setConfigNames($config_names) { - $this->configNames = $config_names; - } - - /** - * Gets the configuration names keyed by collection. - * - * @return string - * The configuration names. - */ - public function getConfigNames() { - return $this->configNames; - } } diff --git a/core/lib/Drupal/Core/Extension/ModuleHandler.php b/core/lib/Drupal/Core/Extension/ModuleHandler.php index 1e66783..64617a1 100644 --- a/core/lib/Drupal/Core/Extension/ModuleHandler.php +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.php @@ -10,8 +10,10 @@ use Drupal\Component\Graph\Graph; use Drupal\Component\Serialization\Yaml; use Drupal\Component\Utility\NestedArray; +use Drupal\Component\Utility\String; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Config\PreExistingConfigException; +use Drupal\Core\Config\StorageInterface; use Drupal\Core\Entity\Schema\EntitySchemaProviderInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -687,20 +689,6 @@ public function install(array $module_list, $enable_dependencies = TRUE) { // Sort the module list by their weights (reverse). arsort($module_list); $module_list = array_keys($module_list); - - // Validate default configuration of this module. Bail if unable to - // install. Should not continue installing more modules because those - // may depend on this one. - $existing_configuration = array(); - foreach ($module_list as $module) { - $existing_configuration = array_merge_recursive(\Drupal::service('config.installer')->validatePreInstall($module_data[$module]->getType(), $module), $existing_configuration); - } - if (count($existing_configuration)) { - $exception = new PreExistingConfigException(); - $exception->setConfigNames($existing_configuration); - $exception->setExtensions($module_list); - throw $exception; - } } // Required for module installation checks. @@ -724,6 +712,19 @@ public function install(array $module_list, $enable_dependencies = TRUE) { ))); } + // Profiles can have config clashes. + if ($module != drupal_get_profile()) { + // Validate default configuration of this module. Bail if unable to + // install. Should not continue installing more modules because those + // may depend on this one. + $existing_configuration = \Drupal::service('config.installer')->findPreExistingConfiguration('module', $module); + if (count($existing_configuration)) { + throw new PreExistingConfigException(String::format('Configuration @config_names provided by @extension already exist in active configuration', + array('@config_names' => implode(',', $existing_configuration[StorageInterface::DEFAULT_COLLECTION]), '@extension' => $module) + )); + } + } + $extension_config ->set("module.$module", 0) ->set('module', module_config_sort($extension_config->get('module'))) diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigInstallWebTest.php.orig b/core/modules/config/lib/Drupal/config/Tests/ConfigInstallWebTest.php.orig deleted file mode 100644 index 7116bea..0000000 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigInstallWebTest.php.orig +++ /dev/null @@ -1,152 +0,0 @@ - 'Install, disable and uninstall functionality', - 'description' => 'Tests installation and removal of configuration objects in install, disable and uninstall functionality.', - 'group' => 'Configuration', - ); - } - - function setUp() { - parent::setUp(); - - // Ensure the global variable being asserted by this test does not exist; - // a previous test executed in this request/process might have set it. - unset($GLOBALS['hook_config_test']); - } - - /** - * Tests module re-installation. - */ - function testIntegrationModuleReinstallation() { - $default_config = 'config_integration_test.settings'; - $default_configuration_entity = 'config_test.dynamic.config_integration_test'; - - // Install the config_test module we're integrating with. - \Drupal::moduleHandler()->install(array('config_test')); - - // Verify the configuration does not exist prior to installation. - $config_static = \Drupal::config($default_config); - $this->assertIdentical($config_static->isNew(), TRUE); - $config_entity = \Drupal::config($default_configuration_entity); - $this->assertIdentical($config_entity->isNew(), TRUE); - - // Install the integration module. - \Drupal::moduleHandler()->install(array('config_integration_test')); - - // Verify that default module config exists. - \Drupal::configFactory()->reset($default_config); - \Drupal::configFactory()->reset($default_configuration_entity); - $config_static = \Drupal::config($default_config); - $this->assertIdentical($config_static->isNew(), FALSE); - $this->assertIdentical($config_static->get('foo'), 'default setting'); - $config_entity = \Drupal::config($default_configuration_entity); - $this->assertIdentical($config_entity->isNew(), FALSE); - $this->assertIdentical($config_entity->get('label'), 'Default integration config label'); - - // Customize both configuration objects. - $config_static->set('foo', 'customized setting')->save(); - $config_entity->set('label', 'Customized integration config label')->save(); - - // @todo FIXME: Setting config keys WITHOUT SAVING retains the changed config - // object in memory. Every new call to \Drupal::config() MUST revert in-memory changes - // that haven't been saved! - // In other words: This test passes even without this reset, but it shouldn't. - $this->container->get('config.factory')->reset(); - - // Disable and uninstall the integration module. - module_uninstall(array('config_integration_test')); - - // Verify the integration module's config was uninstalled. - $config_static = \Drupal::config($default_config); - $this->assertIdentical($config_static->isNew(), TRUE); - - // Verify the integration config still exists. - $config_entity = \Drupal::config($default_configuration_entity); - $this->assertIdentical($config_entity->isNew(), FALSE); - $this->assertIdentical($config_entity->get('label'), 'Customized integration config label'); - - // Reinstall the integration module. - \Drupal::moduleHandler()->install(array('config_integration_test')); - - // Verify the integration module's config was re-installed. - \Drupal::configFactory()->reset($default_config); - \Drupal::configFactory()->reset($default_configuration_entity); - $config_static = \Drupal::config($default_config); - $this->assertIdentical($config_static->isNew(), FALSE); - $this->assertIdentical($config_static->get('foo'), 'default setting'); - - // Verify the customized integration config still exists. - $config_entity = \Drupal::config($default_configuration_entity); - $this->assertIdentical($config_entity->isNew(), FALSE); - $this->assertIdentical($config_entity->get('label'), 'Customized integration config label'); - } - - /** - * Tests install profile config changes. - */ - function testInstallProfileConfigOverwrite() { - $config_name = 'system.cron'; - // The expected configuration from the system module. - $expected_original_data = array( - 'threshold' => array( - 'autorun' => 0, - 'requirements_warning' => 172800, - 'requirements_error' => 1209600, - ), - ); - // The expected active configuration altered by the install profile. - $expected_profile_data = array( - 'threshold' => array( - 'autorun' => 0, - 'requirements_warning' => 259200, - 'requirements_error' => 1209600, - ), - ); - - // Verify that the original data matches. We have to read the module config - // file directly, because the install profile default system.cron.yml - // configuration file was used to create the active configuration. - $config_dir = drupal_get_path('module', 'system') . '/config'; - $this->assertTrue(is_dir($config_dir)); - $source_storage = new FileStorage($config_dir); - $data = $source_storage->read($config_name); - $this->assertIdentical($data, $expected_original_data); - - // Verify that active configuration matches the expected data, which was - // created from the testing install profile's system.cron.yml file. - $config = \Drupal::config($config_name); - $this->assertIdentical($config->get(), $expected_profile_data); - - // Turn on the test module, which will attempt to replace the - // configuration data. This attempt to replace the active configuration - // should be ignored. - \Drupal::moduleHandler()->install(array('config_override_test')); - - // Verify that the test module has not been able to change the data. - $config = \Drupal::config($config_name); - $this->assertIdentical($config->get(), $expected_profile_data); - - // Disable and uninstall the test module. - \Drupal::moduleHandler()->uninstall(array('config_override_test')); - - // Verify that the data hasn't been altered by removing the test module. - $config = \Drupal::config($config_name); - $this->assertIdentical($config->get(), $expected_profile_data); - } -} diff --git a/core/modules/config/src/Tests/ConfigInstallWebTest.php b/core/modules/config/src/Tests/ConfigInstallWebTest.php index 755cd75..bbe320d 100644 --- a/core/modules/config/src/Tests/ConfigInstallWebTest.php +++ b/core/modules/config/src/Tests/ConfigInstallWebTest.php @@ -2,7 +2,7 @@ /** * @file - * Definition of Drupal\config\Tests\ConfigInstallTest. + * Contains \Drupal\config\Tests\ConfigInstallWebTest. */ namespace Drupal\config\Tests; @@ -90,8 +90,7 @@ function testIntegrationModuleReinstallation() { $this->fail('Expected PreExistingConfigException not thrown.'); } catch (PreExistingConfigException $e) { - $this->assertEqual($e->getExtensions(), array('config_integration_test')); - $this->assertEqual($e->getConfigNames(), array(StorageInterface::DEFAULT_COLLECTION => array($default_configuration_entity))); + $this->assertEqual($e->getMessage(), 'Configuration config_test.dynamic.config_integration_test provided by config_integration_test already exist in active configuration'); } // Delete the configuration entity so that the install will work. @@ -163,4 +162,11 @@ function testInstallProfileConfigOverwrite() { $config = \Drupal::config($config_name); $this->assertIdentical($config->get(), $expected_profile_data); } + + /** + * Tests pre existing configuration detection + */ + public function testPreExistingConfigInstall() { + + } } diff --git a/core/modules/config/tests/config_install_fail/config/config_test.dynamic.dotted.default.yml b/core/modules/config/tests/config_install_fail/config/config_test.dynamic.dotted.default.yml deleted file mode 100644 index 6e2af21..0000000 --- a/core/modules/config/tests/config_install_fail/config/config_test.dynamic.dotted.default.yml +++ /dev/null @@ -1,6 +0,0 @@ -id: dotted.default -label: 'Config install fail' -weight: 0 -protected_property: Default -# Intentionally commented out to verify default status behavior. -# status: 1 diff --git a/core/modules/config/tests/config_install_fail/config_install_fail.info.yml b/core/modules/config/tests/config_install_fail/config_install_fail.info.yml deleted file mode 100644 index ebe72ad..0000000 --- a/core/modules/config/tests/config_install_fail/config_install_fail.info.yml +++ /dev/null @@ -1,8 +0,0 @@ -name: 'Configuration test install fial' -type: module -package: Testing -version: VERSION -core: 8.x -hidden: true -dependencies: - - config_test diff --git a/core/modules/config/tests/config_install_fail/config_install_fail.module b/core/modules/config/tests/config_install_fail/config_install_fail.module deleted file mode 100644 index e77c7ce..0000000 --- a/core/modules/config/tests/config_install_fail/config_install_fail.module +++ /dev/null @@ -1,6 +0,0 @@ -keyValueExpirable = $key_value_expirable; + $this->configManager = $config_manager; + $this->configInstaller = $config_installer; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('keyvalue.expirable')->get('module_list'), + $container->get('config.manager'), + $container->get('config.installer') + ); + } + + /** + * Creates a report for modules. + * + * @see \Drupal\system\Form\ModulesListForm::submitForm() + * + * @return array + * The report as a render array. + */ + public function moduleReport() { + $account = $this->currentUser()->id(); + $modules = $this->keyValueExpirable->get($account); + + // hack to test + // $modules = array('install' => array('config_install_fail_test' => TRUE)); + + // Redirect to the modules list page if the key value store is empty. + if (!$modules) { + return new RedirectResponse($this->urlGenerator()->generate('system.modules_list', array(), TRUE)); + } + + $report = array( + 'description' => array( + '#prefix' => '

', + '#suffix' => '

', + '#markup' => $this->t('You must delete or rename the following configuration to install @module.', array('@module' => implode(', ', $modules['install']))) + ), + ) + $this->report('module', array_keys($modules['install'])); + + return $report; + } + + /** + * Generates a report on pre-existing configuration for a list of extensions. + * + * @param string $type + * The type of extension being installed. Either 'theme' or 'module'. + * @param array $extensions + * The list of extensions that are being installed. + * + * @return array + * The report as a render array. + */ + protected function report($type, array $extensions) { + // Check if we have any pre existing configuration. + $existing_configuration = array(); + foreach ($extensions as $extension) { + $existing_configuration = array_merge_recursive($this->configInstaller->findPreExistingConfiguration($type, $extension), $existing_configuration); + } + + $report['config_clashes'] = array( + '#theme' => 'item_list', + '#items' => array(), + '#empty' => $this->t('No configuration clashes detected.'), + '#weight' => 10, + ); + + if (count($existing_configuration)) { + foreach ($existing_configuration as $collection => $config_names) { + foreach ($config_names as $config_name) { + $entity = $this->configManager->loadConfigEntityByName($config_name); + if ($entity) { + $report['config_clashes']['#items'][] = array( + '#type' => 'link', + '#title' => $entity->getEntityType()->getLabel() . ': '. $entity->label(), + ) + $entity->urlInfo('edit-form')->toRenderArray(); + } + else { + // Super weird. + } + } + } + } + + return $report; + } + +} diff --git a/core/modules/system/src/Form/ModulesListForm.php b/core/modules/system/src/Form/ModulesListForm.php index f993c79..eeef203 100644 --- a/core/modules/system/src/Form/ModulesListForm.php +++ b/core/modules/system/src/Form/ModulesListForm.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; +use Drupal\Core\Config\ConfigInstallerInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Entity\Query\QueryFactory; use Drupal\Core\Entity\Query\QueryFactoryInterface; @@ -67,6 +68,13 @@ class ModulesListForm extends FormBase { protected $queryFactory; /** + * The config installer. + * + * @var \Drupal\Core\Config\ConfigInstallerInterface + */ + protected $configInstaller; + + /** * {@inheritdoc} */ public static function create(ContainerInterface $container) { @@ -76,7 +84,8 @@ public static function create(ContainerInterface $container) { $container->get('access_manager'), $container->get('entity.manager'), $container->get('entity.query'), - $container->get('current_user') + $container->get('current_user'), + $container->get('config.installer') ); } @@ -96,13 +105,14 @@ public static function create(ContainerInterface $container) { * @param \Drupal\Core\Session\AccountInterface $current_user * The current user. */ - public function __construct(ModuleHandlerInterface $module_handler, KeyValueStoreExpirableInterface $key_value_expirable, AccessManager $access_manager, EntityManagerInterface $entity_manager, QueryFactory $query_factory, AccountInterface $current_user) { + public function __construct(ModuleHandlerInterface $module_handler, KeyValueStoreExpirableInterface $key_value_expirable, AccessManager $access_manager, EntityManagerInterface $entity_manager, QueryFactory $query_factory, AccountInterface $current_user, ConfigInstallerInterface $config_installer) { $this->moduleHandler = $module_handler; $this->keyValueExpirable = $key_value_expirable; $this->accessManager = $access_manager; $this->entityManager = $entity_manager; $this->queryFactory = $query_factory; $this->currentUser = $current_user; + $this->configInstaller = $config_installer; } /** @@ -428,6 +438,19 @@ public function submitForm(array &$form, array &$form_state) { // Retrieve a list of modules to install and their dependencies. $modules = $this->buildModuleList($form_state); + // Check if we have any pre existing configuration. + $existing_configuration = array(); + foreach (array_keys($modules['install']) as $module) { + $existing_configuration = array_merge_recursive($this->configInstaller->findPreExistingConfiguration('module', $module), $existing_configuration); + } + if (count($existing_configuration)) { + $account = $this->currentUser()->id(); + $this->keyValueExpirable->setWithExpire($account, $modules, 60); + // Redirect to the system config clash page. + $form_state['redirect_route']['route_name'] = 'system.modules_config_clash'; + return; + } + // Check if we have to install any dependencies. If there is one or more // dependencies that are not installed yet, redirect to the confirmation // form. diff --git a/core/modules/system/system.module b/core/modules/system/system.module index ac44ab5..1aa8fcc 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -151,6 +151,9 @@ function system_help($route_name, Request $request) { case 'system.status': return '

' . t("Here you can find a short overview of your site's parameters as well as any problems detected with your installation. It may be useful to copy and paste this information into support requests filed on drupal.org's support forums and project issue queues. Before filing a support request, ensure that your web server meets the system requirements.", array('@system-requirements' => 'http://drupal.org/requirements')) . '

'; + + case 'system.modules_config_clash': + return '

' . t('Here you can find a list of configuration that already exists. You need to either rename it or delete it before installing modules which have configuration with the same name.') . '

'; } } diff --git a/core/modules/system/system.routing.yml b/core/modules/system/system.routing.yml index 35e17f7..ed8a0a2 100644 --- a/core/modules/system/system.routing.yml +++ b/core/modules/system/system.routing.yml @@ -269,6 +269,14 @@ system.theme_enable: _permission: 'administer themes' _csrf_token: 'TRUE' +system.modules_config_clash: + path: 'admin/modules/config_clash' + defaults: + _content: 'Drupal\system\Controller\ConfigClashController::moduleReport' + _title: 'Configuration clash' + requirements: + _permission: 'administer modules' + system.status: path: '/admin/reports/status' defaults: