diff --git a/core/lib/Drupal/Core/Config/BatchConfigImporter.php b/core/lib/Drupal/Core/Config/BatchConfigImporter.php index 47b78bd..50a0fe3 100644 --- a/core/lib/Drupal/Core/Config/BatchConfigImporter.php +++ b/core/lib/Drupal/Core/Config/BatchConfigImporter.php @@ -14,6 +14,8 @@ */ class BatchConfigImporter extends ConfigImporter { + protected $totalToProcess = 0; + /** * Initializes the config importer in preparation for processing a batch. */ @@ -27,7 +29,6 @@ public function initialize() { // Another process is synchronizing configuration. throw new ConfigImporterException(sprintf('%s is already importing', static::LOCK_ID)); } - $this->totalToProcess = 0; $modules = $this->getUnprocessedExtensions('module'); foreach (array('install', 'uninstall') as $op) { @@ -53,9 +54,28 @@ public function processBatch(array &$context) { if (!empty($operation)) { if (!empty($operation['type'])) { $this->processExtension($operation['type'], $operation['op'], $operation['name']); + $this->recalculateChangelist = TRUE; } else { - $this->processConfiguration($operation['op'], $operation['name']); + if ($this->recalculateChangelist) { + $current_total = 0; + foreach (array('create', 'delete', 'update') as $op) { + $current_total += count($this->getUnprocessedConfiguration($op)); + } + $this->storageComparer->reset(); + // This will cause the changelist to be calculated. + $new_total = 0; + foreach (array('create', 'delete', 'update') as $op) { + $new_total += count($this->getUnprocessedConfiguration($op)); + } + $this->totalToProcess = $this->totalToProcess = $current_total + $new_total; + $operation = $this->getNextOperation(); + $this->recalculateChangelist = FALSE; + } + // Rebuilding the changelist change remove all changes. + if ($operation !== FALSE) { + $this->processConfiguration($operation['op'], $operation['name']); + } } $context['message'] = t('Synchronizing @name.', array('@name' => $operation['name'])); $context['finished'] = $this->batchProgress(); diff --git a/core/lib/Drupal/Core/Config/ConfigBase.php b/core/lib/Drupal/Core/Config/ConfigBase.php index 0fe2eb5..8dfac43 100644 --- a/core/lib/Drupal/Core/Config/ConfigBase.php +++ b/core/lib/Drupal/Core/Config/ConfigBase.php @@ -94,7 +94,6 @@ public function setName($name) { public static function validateName($name) { // The name must be namespaced by owner. if (strpos($name, '.') === FALSE) { - ffs($name); throw new ConfigNameException(String::format('Missing namespace in Config object name @name.', array( '@name' => $name, ))); diff --git a/core/lib/Drupal/Core/Config/ConfigImporter.php b/core/lib/Drupal/Core/Config/ConfigImporter.php index 33f461a..d072200 100644 --- a/core/lib/Drupal/Core/Config/ConfigImporter.php +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php @@ -120,6 +120,13 @@ class ConfigImporter extends DependencySerialization { protected $themeHandler; /** + * Set to TRUE to recalculate changelist after installing extensions. + * + * @var bool + */ + protected $recalculateChangelist = FALSE; + + /** * Constructs a configuration import object. * * @param \Drupal\Core\Config\StorageComparerInterface $storage_comparer @@ -183,8 +190,9 @@ protected function getEmptyExtensionsProcessedList() { ), 'theme' => array( 'enable' => array(), - 'disable' => array(), 'default' => array(), + 'admin' => array(), + 'disable' => array(), ), ); } @@ -274,6 +282,15 @@ protected function setProcessedExtension($type, $op, $name) { * @return void */ protected function createExtensionChangelist() { + // Get a list of modules with dependency weights as values. + $module_data = system_rebuild_module_data(); + // Set the actual module weights. + $module_list = array_combine(array_keys($module_data), array_keys($module_data)); + $module_list = array_map(function ($module) use ($module_data) { + return $module_data[$module]->sort; + }, $module_list); + + // Work out what to install and uninstall. $current_modules = $this->storageComparer->getTargetStorage()->read('system.module'); $new_modules = $this->storageComparer->getSourceStorage()->read('system.module'); $install = array(); @@ -281,6 +298,14 @@ protected function createExtensionChangelist() { if (isset($current_modules['enabled'], $new_modules['enabled']) && is_array($current_modules['enabled']) && is_array($new_modules['enabled'])) { $uninstall = array_diff(array_keys($current_modules['enabled']), array_keys($new_modules['enabled'])); $install = array_diff(array_keys($new_modules['enabled']), array_keys($current_modules['enabled'])); + // Sort the module list by their weights. So that dependencies + // are uninstalled last. + asort($module_list); + $uninstall = array_intersect(array_keys($module_list), $uninstall); + // Sort the module list by their weights (reverse). So that dependencies + // are installed first. + arsort($module_list); + $install = array_intersect(array_keys($module_list), $install); } $current_themes = $this->storageComparer->getTargetStorage()->read('system.theme'); @@ -299,7 +324,8 @@ protected function createExtensionChangelist() { ), 'theme' => array( 'enable' => $enable, - 'default' => array($new_themes['default']), + 'default' => $current_themes['default'] != $new_themes['default'] ? array($new_themes['default']) : array(), + 'admin' => $current_themes['admin'] != $new_themes['admin'] ? array($new_themes['admin']) : array(), 'disable' => $disable, ), ); @@ -334,15 +360,15 @@ protected function getExtensionChangelist($type, $op = NULL) { * An array of extension names. */ public function getUnprocessedExtensions($type) { - $unprocessed = array(); $changelist = $this->getExtensionChangelist($type); if ($type == 'theme') { $unprocessed = array( - 'disable' => array_diff($changelist['disable'], $this->processedExtensions[$type]['disable']), 'enable' => array_diff($changelist['enable'], $this->processedExtensions[$type]['enable']), // Setting the default theme is special. Yuck. - 'default' => empty($this->processedExtensions[$type]['default']) ? array() : array($changelist['default']), + 'default' => array_diff($changelist['default'], $this->processedExtensions[$type]['default']), + 'admin' => array_diff($changelist['admin'], $this->processedExtensions[$type]['admin']), + 'disable' => array_diff($changelist['disable'], $this->processedExtensions[$type]['disable']), ); } else { @@ -379,8 +405,6 @@ public function import() { // First pass deleted, then new, and lastly changed configuration, in order // to handle dependencies correctly. - // @todo Implement proper dependency ordering using - // https://drupal.org/node/2080823 foreach (array('delete', 'create', 'update') as $op) { foreach ($this->getUnprocessedConfiguration($op) as $name) { $this->processConfiguration($op, $name); @@ -439,16 +463,28 @@ protected function processConfiguration($op, $name) { * The name of the extension to process. */ protected function processExtension($type, $op, $name) { + // Set the config installer to use the staging directory instead of the + // extensions own default config directories. + \Drupal::service('config.installer') + ->setSyncing(TRUE) + ->setSourceStorage($this->storageComparer->getSourceStorage()); if ($type == 'module') { $this->moduleHandler->$op(array($name), FALSE); + // Installing a module can cause a kernel boot therefore reinject all the + // services. + $this->reInjectMe(); + // During the container is rebuilt and the module handler is called from + // drupal_get_complete_schema(). This causes the container's instance of + // the module handler not to have loaded. + $this->moduleHandler->loadAll(); } else { - if ($op == 'default') { + if ($op == 'default' || $op == 'admin') { // Use the configuration factory to write the data since system.theme // might have been updated by enabling themes. $this->configManager->getConfigFactory() ->get('system.theme') - ->set('default', $name) + ->set($op, $name) ->save(); } else { @@ -456,6 +492,9 @@ protected function processExtension($type, $op, $name) { } } $this->setProcessedExtension($type, $op, $name); + \Drupal::service('config.installer') + ->setSyncing(FALSE) + ->resetSourceStorage(); } /** @@ -567,92 +606,31 @@ protected function hasUpdate($config_name) { * Handle changes to installed modules and themes. */ protected function handleExtensions() { - // Set the config installer to use the staging directory instead of the - // extensions own default config directories. - \Drupal::service('config.installer') - ->setSyncing(TRUE) - ->setSourceStorage($this->storageComparer->getSourceStorage()); - - $module_updates_count = $this->handleModules($this->getUnprocessedExtensions('module')); - $theme_updates_count = $this->handleThemes($this->getUnprocessedExtensions('theme')); - - \Drupal::service('config.installer') - ->setSyncing(FALSE) - ->resetSourceStorage(); + foreach (array('install', 'uninstall') as $op) { + $modules = $this->getUnprocessedExtensions('module'); + foreach($modules[$op] as $module) { + $this->recalculateChangelist = TRUE; + $this->processExtension('module', $op, $module); + } + } + foreach (array('enable', 'default', 'admin', 'disable') as $op) { + $themes = $this->getUnprocessedExtensions('theme'); + foreach($themes[$op] as $theme) { + $this->recalculateChangelist = TRUE; + $this->processExtension('theme', $op, $theme); + } + } - if ($module_updates_count || $theme_updates_count) { + if ($this->recalculateChangelist) { // Recalculate differences as default config could have been imported. $this->storageComparer->reset(); $this->processed = $this->storageComparer->getEmptyChangelist(); - drupal_flush_all_caches(); + // drupal_flush_all_caches(); // Modules have been updated. Services etc might have changed. // We don't reinject storage comparer because swapping out the active // store during config import is a complete nonsense. - $this->reInjectMe(); - } - } - - /** - * Install or uninstall modules depending on configuration to import. - * - * @param $module_changelist - * Array with 'install' and 'uninstall' keys, each containing a list of - * modules to install or uninstall. - * - * @return int - * The number of modules installed or uninstalled. - */ - protected function handleModules($module_changelist) { - if (!empty($module_changelist['install']) && !$this->moduleHandler->install($module_changelist['install'], FALSE)) { - throw new ConfigImporterException(sprintf('Unable to enable modules')); - } - - if (!empty($module_changelist['uninstall']) && !$this->moduleHandler->uninstall($module_changelist['uninstall'], FALSE)) { - throw new ConfigImporterException(sprintf('Unable to uninstall modules')); - } - - $change_count = count($module_changelist['install']) + count($module_changelist['uninstall']); - if ($change_count > 0) { - $this->importConfig('update', 'system.module'); - } - return $change_count; - } - - /** - * Enable or disable themes depending on configuration to import. - * - * @param $theme_changelist - * Array with 'install' and 'uninstall' keys, each containing a list of - * modules to install or uninstall. - * - * @return int - * The number of themes installed or disabled. - */ - protected function handleThemes($theme_changelist) { - // Enable themes first, to ensure that if the default theme has changed, - // the changed-to-theme has been enabled. - if (!empty($theme_changelist['enable'])) { - $this->themeHandler->enable($themes_changelist['enable']); - } - - if (!empty($theme_changelist['default'])) { - // Use the configuration factory to write the data since system.theme - // might have been updated by enabling themes. - $this->configManager->getConfigFactory() - ->get('system.theme') - ->set('default', $theme_changelist['default']) - ->save(); - } - - if (!empty($theme_changelist['disable'])) { - $this->themeHandler->disable($theme_changelist['disable']); - } - - $change_count = count($theme_changelist['enable']) + count($theme_changelist['disable']) + count($theme_changelist['default']); - if ($change_count > 0) { - $this->importConfig('update', 'system.theme'); + $this->recalculateChangelist = TRUE; } - return $change_count; } protected function reInjectMe() { diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigImportAllTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigImportAllTest.php index 9f7f707..d22a367 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigImportAllTest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportAllTest.php @@ -23,9 +23,9 @@ class ConfigImportAllTest extends ModuleTestBase { public static function getInfo() { return array( - 'name' => 'Install/uninstall modules', - 'description' => 'Install/uninstall core module and confirm table creation/deletion.', - 'group' => 'Module', + 'name' => 'Import configuration from all modules and the standard profile', + 'description' => 'Tests the largest configuration import possible with the modules and profiles provided by core.', + 'group' => 'Configuration', ); } diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigImportRecreateTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigImportRecreateTest.php index d573c47..a64ff88 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigImportRecreateTest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportRecreateTest.php @@ -47,7 +47,7 @@ public function setUp() { $theme_data = $this->container->get('config.storage')->read('system.theme'); if (empty($theme_data['enabled'])) { - $this->container->get('config.storage')->write('system.theme', array('default' => 'start', 'enabled' => array('stark'))); + $this->container->get('config.storage')->write('system.theme', array('admin' => '', 'default' => 'start', 'enabled' => array('stark'))); } $module_data = $this->container->get('config.storage')->read('system.module'); if (empty($module_data['enabled'])) { diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php index 1c287b5..39d07f5 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php @@ -15,7 +15,9 @@ */ class ConfigImportUITest extends WebTestBase { - public static $modules = array('config', 'config_test', 'config_import_test'); + // Enable the Options and Text modules to ensure dependencies are handled + // correctly. + public static $modules = array('config', 'config_test', 'config_import_test', 'text', 'options'); public static function getInfo() { return array( @@ -67,9 +69,12 @@ function testImport() { $staging->write($dynamic_name, $original_dynamic_data); $this->assertIdentical($staging->exists($dynamic_name), TRUE, $dynamic_name . ' found.'); - // Enable the Ban and Action modules during import. The Ban module is used - // because it creates a table during the install. The Action module is used - // because it creates a single simple configuration file during the install. + // Enable the Action, Ban, Options and Text modules during import. The Ban + // module is used because it creates a table during the install. The Action + // module is used because it creates a single simple configuration file + // during the install. Options and Text are there in ensure modules are + // installed with the correct dependencies. Options depends on Text so Text + // should be installed first. $system_module = \Drupal::config('system.module')->get(); $system_module['enabled']['action'] = 0; $system_module['enabled']['ban'] = 0; @@ -92,6 +97,9 @@ function testImport() { $action_settings['recursion_limit'] = 50; $staging->write('action.settings', $action_settings); + // Disable the Options and Text modules to ensure that dependencies are handled + // correctly. + \Drupal::moduleHandler()->uninstall(array('text', 'options')); // Verify that both appear as ready to import. $this->drupalGet('admin/config/development/configuration'); $this->assertText($name); @@ -126,12 +134,14 @@ function testImport() { $this->assertTrue(isset($GLOBALS['hook_cache_flush'])); $this->rebuildContainer(); - $this->assertTrue(\Drupal::moduleHandler()->moduleExists('ban'), 'Ban module enabled during import.'); + $this->assertTrue(\Drupal::moduleHandler()->moduleExists('ban'), 'Ban module installed during import.'); $this->assertTrue(\Drupal::database()->schema()->tableExists('ban_ip'), 'The database table ban_ip exists.'); - $this->assertTrue(\Drupal::moduleHandler()->moduleExists('action'), 'Action module enabled during import.'); + $this->assertTrue(\Drupal::moduleHandler()->moduleExists('action'), 'Action module installed during import.'); + $this->assertTrue(\Drupal::moduleHandler()->moduleExists('options'), 'Options module installed during import.'); + $this->assertTrue(\Drupal::moduleHandler()->moduleExists('text'), 'Text module installed during import.'); $theme_info = \Drupal::service('theme_handler')->listInfo(); - $this->assertTrue(isset($theme_info['bartik']) && $theme_info['bartik']->status, 'Bartik theme enabled during import.'); + $this->assertTrue($theme_info['bartik']->status, 'Bartik theme enabled during import.'); // The configuration object system.theme will be saved twice during config // import. Once during enabling the system and once during importing the @@ -149,8 +159,11 @@ function testImport() { $system_module = \Drupal::config('system.module')->get(); unset($system_module['enabled']['action']); unset($system_module['enabled']['ban']); + unset($system_module['enabled']['options']); + unset($system_module['enabled']['text']); $staging->write('system.module', $system_module); $staging->delete('action.settings'); + $staging->delete('text.settings'); $system_theme = \Drupal::config('system.theme')->get(); unset($system_theme['enabled']['bartik']); @@ -180,6 +193,8 @@ function testImport() { $this->assertFalse(\Drupal::moduleHandler()->moduleExists('ban'), 'Ban module uninstalled during import.'); $this->assertFalse(\Drupal::database()->schema()->tableExists('ban_ip'), 'The database table ban_ip does not exist.'); $this->assertFalse(\Drupal::moduleHandler()->moduleExists('action'), 'Action module uninstalled during import.'); + $this->assertFalse(\Drupal::moduleHandler()->moduleExists('options'), 'Options module uninstalled during import.'); + $this->assertFalse(\Drupal::moduleHandler()->moduleExists('text'), 'Text module uninstalled during import.'); // This is because it will be updated to change the default theme, remove // Bartik and then set the admin theme. $this->assertEqual(\Drupal::state()->get('ConfigImportUITest.system.theme.save', 0), 3, 'The system.theme configuration saved thrice during import.'); diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php index cf30515..44ea656 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php @@ -52,7 +52,7 @@ function setUp() { $theme_data = $this->container->get('config.storage')->read('system.theme'); if (empty($theme_data['enabled'])) { - $this->container->get('config.storage')->write('system.theme', array('default' => 'stark', 'enabled' => array('stark'))); + $this->container->get('config.storage')->write('system.theme', array('admin' => '', 'default' => 'stark', 'enabled' => array('stark'))); } $module_data = $this->container->get('config.storage')->read('system.module'); if (empty($module_data['enabled'])) { @@ -156,10 +156,6 @@ function testDeleted() { $this->assertTrue(isset($GLOBALS['hook_config_test']['predelete'])); $this->assertTrue(isset($GLOBALS['hook_config_test']['delete'])); - // Verify that there is nothing more to import. - foreach (array('delete', 'create', 'update') as $op) { - ffs(array($op => $this->configImporter->getUnprocessedConfiguration($op))); - } $this->assertFalse($this->configImporter->hasUnprocessedConfigurationChanges()); }