diff --git a/core/lib/Drupal/Core/Config/ConfigInstaller.php b/core/lib/Drupal/Core/Config/ConfigInstaller.php index 6fe1afa..71fe81d 100644 --- a/core/lib/Drupal/Core/Config/ConfigInstaller.php +++ b/core/lib/Drupal/Core/Config/ConfigInstaller.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Config; +use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; use Drupal\Core\Config\Entity\ConfigDependencyManager; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -147,16 +148,17 @@ protected function listDefaultConfigCollection($collection, $type, $name, array $extension_path = drupal_get_path($type, $name); if ($type !== 'core' && is_dir($extension_path . '/' . InstallStorage::CONFIG_INSTALL_DIRECTORY)) { $default_storage = new FileStorage($extension_path . '/' . InstallStorage::CONFIG_INSTALL_DIRECTORY, $collection); - $other_module_config = array_filter($default_storage->listAll(), function ($value) use ($name) { - return !preg_match('/^' . $name . '\./', $value); - }); - - $other_module_config = array_filter($other_module_config, function ($config_name) use ($enabled_extensions) { + $extension_provided_config = array_filter($default_storage->listAll(), function ($config_name) use ($config_to_install, $enabled_extensions) { + // Ensure that we have not already discovered the config to install. + if (in_array($config_name, $config_to_install)) { + return FALSE; + } + // Ensure the configuration is provided by an enabled module $provider = Unicode::substr($config_name, 0, strpos($config_name, '.')); return in_array($provider, $enabled_extensions); }); - $config_to_install = array_merge($config_to_install, $other_module_config); + $config_to_install = array_merge($config_to_install, $extension_provided_config); } return $config_to_install; @@ -324,4 +326,32 @@ public function setSyncing($status) { public function isSyncing() { return $this->isSyncing; } + + /** + * {@inheritdoc} + */ + public function findPreExistingConfiguration($type, $name) { + $existing_configuration = array(); + // Gather information about all the supported collections. + $collection_info = $this->configManager->getConfigCollectionInfo(); + + // Read enabled extensions directly from configuration to avoid circular + // dependencies with ModuleHandler and ThemeHandler. + $extension_config = $this->configFactory->get('core.extension'); + $enabled_extensions = array_keys((array) $extension_config->get('module')); + $enabled_extensions += array_keys((array) $extension_config->get('theme')); + // Add the extensions that will be enabled to the list of enabled + // extensions. + $enabled_extensions[] = $name; + foreach ($collection_info->getCollectionNames(TRUE) as $collection) { + $config_to_install = $this->listDefaultConfigCollection($collection, $type, $name, $enabled_extensions); + $active_storage = $this->getActiveStorage($collection); + foreach ($config_to_install as $config_name) { + if ($active_storage->exists($config_name)) { + $existing_configuration[$collection][] = $config_name; + } + } + } + return $existing_configuration; + } } diff --git a/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php b/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php index 43d9e86..e385839 100644 --- a/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php +++ b/core/lib/Drupal/Core/Config/ConfigInstallerInterface.php @@ -82,4 +82,26 @@ public function setSyncing($status); */ public function isSyncing(); + /** + * 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. + * @param string $name + * Name of extension to install. + * + * @return array + * Array of configuration objects that already exist keyed by collection. + */ + 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 new file mode 100644 index 0000000..eabb7f0 --- /dev/null +++ b/core/lib/Drupal/Core/Config/PreExistingConfigException.php @@ -0,0 +1,14 @@ +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/src/Tests/ConfigInstallWebTest.php b/core/modules/config/src/Tests/ConfigInstallWebTest.php index f847d01..963a119 100644 --- a/core/modules/config/src/Tests/ConfigInstallWebTest.php +++ b/core/modules/config/src/Tests/ConfigInstallWebTest.php @@ -2,12 +2,14 @@ /** * @file - * Definition of Drupal\config\Tests\ConfigInstallTest. + * Contains \Drupal\config\Tests\ConfigInstallWebTest. */ namespace Drupal\config\Tests; use Drupal\Core\Config\InstallStorage; +use Drupal\Core\Config\PreExistingConfigException; +use Drupal\Core\Config\StorageInterface; use Drupal\simpletest\WebTestBase; use Drupal\Core\Config\FileStorage; @@ -78,6 +80,16 @@ function testIntegrationModuleReinstallation() { $this->assertIdentical($config_entity->get('label'), 'Customized integration config label'); // Reinstall the integration module. + try { + \Drupal::moduleHandler()->install(array('config_integration_test')); + $this->fail('Expected PreExistingConfigException not thrown.'); + } + catch (PreExistingConfigException $e) { + $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. + $config_entity->delete(); \Drupal::moduleHandler()->install(array('config_integration_test')); // Verify the integration module's config was re-installed. @@ -87,10 +99,10 @@ function testIntegrationModuleReinstallation() { $this->assertIdentical($config_static->isNew(), FALSE); $this->assertIdentical($config_static->get('foo'), 'default setting'); - // Verify the customized integration config still exists. + // Verify the integration config is using the default. $config_entity = \Drupal::config($default_configuration_entity); $this->assertIdentical($config_entity->isNew(), FALSE); - $this->assertIdentical($config_entity->get('label'), 'Customized integration config label'); + $this->assertIdentical($config_entity->get('label'), 'Default integration config label'); } /** @@ -145,4 +157,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/src/Tests/ConfigOtherModuleTest.php b/core/modules/config/src/Tests/ConfigOtherModuleTest.php index ba497ab..5130fb6 100644 --- a/core/modules/config/src/Tests/ConfigOtherModuleTest.php +++ b/core/modules/config/src/Tests/ConfigOtherModuleTest.php @@ -56,11 +56,6 @@ public function testInstallOtherModuleFirst() { // Default configuration provided by config_test should still exist. $this->assertTrue(entity_load('config_test', 'dotted.default', TRUE), 'The configuration is not deleted.'); - - // Re-enable module to test that default config is unchanged. - $this->installModule('config_other_module_config_test'); - $config_entity = entity_load('config_test', 'other_module_test', TRUE); - $this->assertEqual($config_entity->get('style'), "The piano ain't got no wrong notes.", 'Re-enabling the module does not install default config over the existing config entity.'); } /** diff --git a/core/modules/config/tests/config_install_fail_dependency_test/config_install_fail_dependency_test.info.yml b/core/modules/config/tests/config_install_fail_dependency_test/config_install_fail_dependency_test.info.yml new file mode 100644 index 0000000..b5776f2 --- /dev/null +++ b/core/modules/config/tests/config_install_fail_dependency_test/config_install_fail_dependency_test.info.yml @@ -0,0 +1,7 @@ +name: 'Configuration install fail dependency test' +type: module +package: Testing +version: VERSION +core: 8.x +dependencies: + - config_install_fail_test diff --git a/core/modules/config/tests/config_install_fail_test/config/install/config_test.dynamic.dotted.default.yml b/core/modules/config/tests/config_install_fail_test/config/install/config_test.dynamic.dotted.default.yml new file mode 100644 index 0000000..eb94849 --- /dev/null +++ b/core/modules/config/tests/config_install_fail_test/config/install/config_test.dynamic.dotted.default.yml @@ -0,0 +1,7 @@ +# Clashes with default configuration provided by the config_test module. +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_test/config_install_fail_test.info.yml b/core/modules/config/tests/config_install_fail_test/config_install_fail_test.info.yml new file mode 100644 index 0000000..44a9cf3 --- /dev/null +++ b/core/modules/config/tests/config_install_fail_test/config_install_fail_test.info.yml @@ -0,0 +1,7 @@ +name: 'Configuration install fail test' +type: module +package: Testing +version: VERSION +core: 8.x +dependencies: + - config_test diff --git a/core/modules/field/src/Tests/FieldImportChangeTest.php b/core/modules/field/src/Tests/FieldImportChangeTest.php index fce8d0e..f8fddd8 100644 --- a/core/modules/field/src/Tests/FieldImportChangeTest.php +++ b/core/modules/field/src/Tests/FieldImportChangeTest.php @@ -17,6 +17,10 @@ class FieldImportChangeTest extends FieldUnitTestBase { /** * Modules to enable. * + * The default configuration provided by field_test_config is imported by + * \Drupal\field\Tests\FieldUnitTestBase::setUp() when it installs field + * configuration. + * * @var array */ public static $modules = array('field_test_config'); @@ -29,8 +33,6 @@ function testImportChange() { $instance_id = "entity_test.entity_test.$field_id"; $instance_config_name = "field.instance.$instance_id"; - // Import default config. - $this->installConfig(array('field_test_config')); $active = $this->container->get('config.storage'); $staging = $this->container->get('config.storage.staging'); $this->copyConfig($active, $staging); diff --git a/core/modules/field/src/Tests/FieldImportDeleteTest.php b/core/modules/field/src/Tests/FieldImportDeleteTest.php index 4b077d3..1ef3d68 100644 --- a/core/modules/field/src/Tests/FieldImportDeleteTest.php +++ b/core/modules/field/src/Tests/FieldImportDeleteTest.php @@ -19,6 +19,10 @@ class FieldImportDeleteTest extends FieldUnitTestBase { /** * Modules to enable. * + * The default configuration provided by field_test_config is imported by + * \Drupal\field\Tests\FieldUnitTestBase::setUp() when it installs field + * configuration. + * * @var array */ public static $modules = array('field_test_config'); @@ -51,9 +55,6 @@ public function testImportDelete() { // Create a second bundle for the 'Entity test' entity type. entity_test_create_bundle('test_bundle'); - // Import default config. - $this->installConfig(array('field_test_config')); - // Get the uuid's for the field storages. $field_storage_uuid = entity_load('field_storage_config', $field_storage_id)->uuid(); $field_storage_uuid_2 = entity_load('field_storage_config', $field_storage_id_2)->uuid(); diff --git a/core/modules/system/src/Controller/ConfigClashController.php b/core/modules/system/src/Controller/ConfigClashController.php new file mode 100644 index 0000000..87cf629 --- /dev/null +++ b/core/modules/system/src/Controller/ConfigClashController.php @@ -0,0 +1,158 @@ +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 1fa64f5..e6e0b9f 100644 --- a/core/modules/system/src/Form/ModulesListForm.php +++ b/core/modules/system/src/Form/ModulesListForm.php @@ -11,6 +11,7 @@ use Drupal\Component\Utility\Unicode; use Drupal\Core\Controller\TitleResolverInterface; use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Config\ConfigInstallerInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\Extension; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -92,6 +93,13 @@ class ModulesListForm extends FormBase { protected $menuLinkManager; /** + * The config installer. + * + * @var \Drupal\Core\Config\ConfigInstallerInterface + */ + protected $configInstaller; + + /** * {@inheritdoc} */ public static function create(ContainerInterface $container) { @@ -104,7 +112,8 @@ public static function create(ContainerInterface $container) { $container->get('current_route_match'), $container->get('title_resolver'), $container->get('router.route_provider'), - $container->get('plugin.manager.menu.link') + $container->get('plugin.manager.menu.link'), + $container->get('config.installer') ); } @@ -130,7 +139,7 @@ public static function create(ContainerInterface $container) { * @param \Drupal\Core\Menu\MenuLinkManagerInterface $menu_link_manager * The menu link manager. */ - public function __construct(ModuleHandlerInterface $module_handler, KeyValueStoreExpirableInterface $key_value_expirable, AccessManagerInterface $access_manager, EntityManagerInterface $entity_manager, AccountInterface $current_user, RouteMatchInterface $route_match, TitleResolverInterface $title_resolver, RouteProviderInterface $route_provider, MenuLinkManagerInterface $menu_link_manager) { + public function __construct(ModuleHandlerInterface $module_handler, KeyValueStoreExpirableInterface $key_value_expirable, AccessManagerInterface $access_manager, EntityManagerInterface $entity_manager, AccountInterface $current_user, RouteMatchInterface $route_match, TitleResolverInterface $title_resolver, RouteProviderInterface $route_provider, MenuLinkManagerInterface $menu_link_manager, ConfigInstallerInterface $config_installer) { $this->moduleHandler = $module_handler; $this->keyValueExpirable = $key_value_expirable; $this->accessManager = $access_manager; @@ -140,6 +149,7 @@ public function __construct(ModuleHandlerInterface $module_handler, KeyValueStor $this->titleResolver = $title_resolver; $this->routeProvider = $route_provider; $this->menuLinkManager = $menu_link_manager; + $this->configInstaller = $config_installer; } /** @@ -478,6 +488,19 @@ public function submitForm(array &$form, FormStateInterface $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 f3b65a2..fb743b6 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -145,6 +145,9 @@ function system_help($route_name, RouteMatchInterface $route_match) { 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 8034863..25807dd 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: