Issue #2090115 by alexpott, xjm: Don't install a module when its default...

Issue #2090115 by alexpott, xjm: Don't install a module when its default configuration has unmet dependencies

core/includes/install.core.inc

@@ -741,6 +741,8 @@ function install_tasks($install_state) {
     ),
     'install_profile_themes' => array(
     ),
+    'install_install_profile' => array(
+    ),
     'install_import_translations' => array(
       'display_name' => t('Set up translations'),
       'display' => $needs_translations,
@@ -1022,10 +1024,6 @@ function install_base_system(&$install_state) {
   // State can be set to the database now that system.module is installed.
   $modules = $install_state['profile_info']['dependencies'];
 
-  // The installation profile is also a module, which needs to be installed
-  // after all the dependencies have been installed.
-  $modules[] = drupal_get_profile();
-
   \Drupal::state()->set('install_profile_modules', array_diff($modules, array('system')));
   $install_state['base_system_verified'] = TRUE;
 }
@@ -1571,10 +1569,7 @@ function install_profile_modules(&$install_state) {
   // the modules.
   $required = array();
   $non_required = array();
-  // Although the profile module is marked as required, it needs to go after
-  // every dependency, including non-required ones. So clear its required
-  // flag for now to allow it to install late.
-  $files[$install_state['parameters']['profile']]->info['required'] = FALSE;
+
   // Add modules that other modules depend on.
   foreach ($modules as $module) {
     if ($files[$module]->requires) {
@@ -1633,6 +1628,24 @@ function install_profile_themes(&$install_state) {
   }
 }
 
+/**
+ * Installs the install profile.
+ *
+ * @param $install_state
+ *   An array of information about the current installation state.
+ */
+function install_install_profile(&$install_state) {
+  \Drupal::service('module_installer')->install(array(drupal_get_profile()), FALSE);
+  // Install all available optional config. During installation the module order
+  // is determined by dependencies. If there are no dependencies between modules
+  // then the order in which they are installed is dependent on random factors
+  // like PHP version. Optional configuration therefore might or might not be
+  // created depending on this order. Ensuring that we have installed all of the
+  // optional configuration whose dependencies can be met at this point removes
+  // any disparities that this creates.
+  \Drupal::service('config.installer')->installOptionalConfig();
+}
+
 /**
  * Prepares the system for import and downloads additional translations.
  *

core/lib/Drupal/Core/Config/ConfigImporter.php

@@ -786,8 +786,7 @@ protected function processExtension($type, $op, $name) {
 
     $this->setProcessedExtension($type, $op, $name);
     \Drupal::service('config.installer')
-      ->setSyncing(FALSE)
-      ->resetSourceStorage();
+      ->setSyncing(FALSE);
   }
 
   /**

core/lib/Drupal/Core/Config/ConfigInstallerInterface.php

@@ -37,6 +37,24 @@ interface ConfigInstallerInterface {
    */
   public function installDefaultConfig($type, $name);
 
+  /**
+   * Installs optional configuration.
+   *
+   * Optional configuration is only installed if:
+   * - the configuration does not exist already.
+   * - it's a configuration entity.
+   * - its dependencies can be met.
+   *
+   * @param \Drupal\Core\Config\StorageInterface
+   *   (optional) The configuration storage to search for optional
+   *   configuration. If not provided, all enabled extension's optional
+   *   configuration directories will be searched.
+   * @param string $prefix
+   *   (optional) If set, limits the installed configuration to only
+   *   configuration beginning with the provided value.
+   */
+  public function installOptionalConfig(StorageInterface $storage = NULL, $prefix = '');
+
   /**
    * Installs all default configuration in the specified collection.
    *
@@ -59,13 +77,6 @@ public function installCollectionDefaultConfig($collection);
    */
   public function setSourceStorage(StorageInterface $storage);
 
-  /**
-   * Resets the configuration storage that provides the default configuration.
-   *
-   * @return $this
-   */
-  public function resetSourceStorage();
-
   /**
    * Sets the status of the isSyncing flag.
    *
@@ -85,25 +96,16 @@ 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.
+   * Checks the configuration that will be installed for an extension.
    *
    * @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.
+   * @throws \Drupal\Core\Config\UnmetDependenciesException
+   * @throws \Drupal\Core\Config\PreExistingConfigException
    */
-  public function findPreExistingConfiguration($type, $name);
+  public function checkConfigurationToInstall($type, $name);
 
 }

core/lib/Drupal/Core/Config/ExtensionInstallStorage.php

@@ -45,7 +45,7 @@ class ExtensionInstallStorage extends InstallStorage {
    *   default collection.
    * @param bool $include_profile
    *   (optional) Whether to include the install profile in extensions to
-   *   search.
+   *   search and to get overrides from.
    */
   public function __construct(StorageInterface $config_storage, $directory = self::CONFIG_INSTALL_DIRECTORY, $collection = StorageInterface::DEFAULT_COLLECTION, $include_profile = TRUE) {
     $this->configStorage = $config_storage;
@@ -82,27 +82,24 @@ protected function getAllFolders() {
       $this->folders = array();
       $this->folders += $this->getComponentNames('core', array('core'));
 
+      $install_profile = Settings::get('install_profile');
       $extensions = $this->configStorage->read('core.extension');
       if (!empty($extensions['module'])) {
         $modules = $extensions['module'];
-        if (!$this->includeProfile) {
-          if ($install_profile = Settings::get('install_profile')) {
-            unset($modules[$install_profile]);
-          }
-        }
+        // Remove the install profile as this is handled later.
+        unset($modules[$install_profile]);
         $this->folders += $this->getComponentNames('module', array_keys($modules));
       }
       if (!empty($extensions['theme'])) {
         $this->folders += $this->getComponentNames('theme', array_keys($extensions['theme']));
       }
 
-      // The install profile can override module default configuration. We do
-      // this by replacing the config file path from the module/theme with the
-      // install profile version if there are any duplicates.
-      $profile_folders = $this->getComponentNames('profile', array(drupal_get_profile()));
-      $folders_to_replace = array_intersect_key($profile_folders, $this->folders);
-      if (!empty($folders_to_replace)) {
-        $this->folders = array_merge($this->folders, $folders_to_replace);
+      if ($this->includeProfile) {
+        // The install profile can override module default configuration. We do
+        // this by replacing the config file path from the module/theme with the
+        // install profile version if there are any duplicates.
+        $profile_folders = $this->getComponentNames('profile', array(drupal_get_profile()));
+        $this->folders = $profile_folders + $this->folders;
       }
     }
     return $this->folders;

core/lib/Drupal/Core/Config/InstallStorage.php

@@ -27,6 +27,11 @@ class InstallStorage extends FileStorage {
    */
   const CONFIG_INSTALL_DIRECTORY = 'config/install';
 
+  /**
+   * Extension sub-directory containing optional configuration for installation.
+   */
+  const CONFIG_OPTIONAL_DIRECTORY = 'config/optional';
+
   /**
    * Extension sub-directory containing configuration schema.
    */

core/lib/Drupal/Core/Config/UnmetDependenciesException.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Config\UnmetDependenciesException.
+ */
+
+namespace Drupal\Core\Config;
+
+use Drupal\Component\Utility\String;
+use Drupal\Core\StringTranslation\TranslationInterface;
+
+/**
+ * An exception thrown if configuration has unmet dependencies.
+ */
+class UnmetDependenciesException extends ConfigException {
+
+  /**
+   * A list of configuration objects that have unmet dependencies.
+   *
+   * @var array
+   */
+  protected $configObjects = [];
+
+  /**
+   * The name of the extension that is being installed.
+   *
+   * @var string
+   */
+  protected $extension;
+
+  /**
+   * Gets the list of configuration objects that have unmet dependencies.
+   *
+   * @return array
+   *   A list of configuration objects that have unmet dependencies.
+   */
+  public function getConfigObjects() {
+    return $this->configObjects;
+  }
+
+  /**
+   * Gets the name of the extension that is being installed.
+   *
+   * @return string
+   *   The name of the extension that is being installed.
+   */
+  public function getExtension() {
+    return $this->extension;
+  }
+
+  /**
+   * Gets a translated message from the exception.
+   *
+   * @param \Drupal\Core\StringTranslation\TranslationInterface $string_translation
+   *   The string translation service.
+   *
+   * @return string
+   */
+  public function getTranslatedMessage(TranslationInterface $string_translation, $extension) {
+    return $string_translation->formatPlural(
+      count($this->getConfigObjects()),
+      'Unable to install @extension, %config_names has unmet dependencies.',
+      'Unable to install @extension, %config_names have unmet dependencies.',
+      [
+        '%config_names' => implode(', ', $this->getConfigObjects()),
+        '@extension' => $extension,
+      ]
+    );
+  }
+
+  /**
+   * Creates an exception for an extension and a list of configuration objects.
+   *
+   * @param $extension
+   *   The name of the extension that is being installed.
+   * @param array $config_objects
+   *   A list of configuration object names that have unmet dependencies
+   *
+   * @return \Drupal\Core\Config\PreExistingConfigException
+   */
+  public static function create($extension, array $config_objects) {
+    $message = String::format('Configuration objects (@config_names) provided by @extension have unmet dependencies',
+      array(
+        '@config_names' => implode(', ', $config_objects),
+        '@extension' => $extension
+      )
+    );
+    $e = new static($message);
+    $e->configObjects = $config_objects;
+    $e->extension = $extension;
+    return $e;
+  }
+
+}

core/lib/Drupal/Core/Extension/ModuleInstaller.php

@@ -151,17 +151,9 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
           )));
         }
 
-        // Install profiles can not have config clashes. Configuration that
-        // has the same name as a module's configuration will be used instead.
-        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 = $config_installer->findPreExistingConfiguration('module', $module);
-          if (!empty($existing_configuration)) {
-            throw PreExistingConfigException::create($module, $existing_configuration);
-          }
-        }
+        // Check the validity of the default configuration. This will throw
+        // exceptions if the configuration is not valid.
+        $config_installer->checkConfigurationToInstall('module', $module);
 
         $extension_config
           ->set("module.$module", 0)
@@ -249,12 +241,6 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
             ->setSyncing(TRUE)
             ->setSourceStorage($source_storage);
         }
-        else {
-          // If we're not in a config synchronization reset the source storage
-          // so that the extension install storage will pick up the new
-          // configuration.
-          $config_installer->resetSourceStorage();
-        }
         \Drupal::service('config.installer')->installDefaultConfig('module', $module);
 
         // If the module has no current updates, but has some that were

core/lib/Drupal/Core/Extension/ThemeHandler.php

@@ -258,10 +258,7 @@ public function install(array $theme_list, $install_dependencies = TRUE) {
 
       // Validate default configuration of the theme. If there is existing
       // configuration then stop installing.
-      $existing_configuration = $this->configInstaller->findPreExistingConfiguration('theme', $key);
-      if (!empty($existing_configuration)) {
-        throw PreExistingConfigException::create($key, $existing_configuration);
-      }
+      $this->configInstaller->checkConfigurationToInstall('theme', $key);
 
       // The value is not used; the weight is ignored for themes currently.
       $extension_config
@@ -288,16 +285,6 @@ public function install(array $theme_list, $install_dependencies = TRUE) {
       // Only install default configuration if this theme has not been installed
       // already.
       if (!isset($installed_themes[$key])) {
-        // The default config installation storage only knows about the
-        // currently installed list of themes, so it has to be reset in order to
-        // pick up the default config of the newly installed theme. However, do
-        // not reset the source storage when synchronizing configuration, since
-        // that would needlessly trigger a reload of the whole configuration to
-        // be imported.
-        if (!$this->configInstaller->isSyncing()) {
-          $this->configInstaller->resetSourceStorage();
-        }
-
         // Install default configuration of the theme.
         $this->configInstaller->installDefaultConfig('theme', $key);
       }

core/modules/block/src/Tests/BlockStorageUnitTest.php

@@ -149,6 +149,7 @@ protected function deleteTests() {
    * Tests the installation of default blocks.
    */
   public function testDefaultBlocks() {
+    \Drupal::service('theme_handler')->install(['classy']);
     $entities = $this->controller->loadMultiple();
     $this->assertTrue(empty($entities), 'There are no blocks initially.');
 

core/modules/block/tests/modules/block_test/block_test.info.yml

@@ -4,3 +4,5 @@ description: 'Provides test blocks.'
 package: Testing
 version: VERSION
 core: 8.x
+dependencies:
+  - block

core/modules/block_content/src/Tests/BlockContentPageViewTest.php

@@ -19,7 +19,7 @@ class BlockContentPageViewTest extends BlockContentTestBase {
    *
    * @var array
    */
-  public static $modules = array('block', 'block_content', 'block_content_test');
+  public static $modules = array('block_content_test');
 
   /**
    * Checks block edit and fallback functionality.

core/modules/block_content/tests/modules/block_content_test/block_content_test.info.yml

@@ -4,3 +4,5 @@ description: "Support module for custom block related testing."
 package: Testing
 version: VERSION
 core: 8.x
+dependencies:
+  - block_content

core/modules/block_content/tests/modules/block_content_test/config/install/block.block.foobargorilla.yml

@@ -4,7 +4,7 @@ dependencies:
   module:
     - block_content
   theme:
-    - stark
+    - classy
 id: foobargorilla
 theme: classy
 region: content

core/modules/book/config/install/core.entity_view_mode.node.print.yml

@@ -5,4 +5,8 @@ cache: true
 targetEntityType: node
 dependencies:
   module:
+    - book
     - node
+  enforced:
+    module:
+      - book

core/modules/comment/src/Tests/CommentDefaultFormatterCacheTagsTest.php

@@ -42,7 +42,7 @@ protected function setUp() {
 
     // Install tables and config needed to render comments.
     $this->installSchema('comment', array('comment_entity_statistics'));
-    $this->installConfig(array('system', 'filter'));
+    $this->installConfig(array('system', 'filter', 'comment'));
 
     // Comment rendering generates links, so build the router.
     $this->installSchema('system', array('router'));

core/modules/comment/src/Tests/CommentFieldAccessTest.php

@@ -79,7 +79,7 @@ class CommentFieldAccessTest extends EntityUnitTestBase {
    */
   protected function setUp() {
     parent::setUp();
-    $this->installConfig(array('user'));
+    $this->installConfig(array('user', 'comment'));
     $this->installSchema('comment', array('comment_entity_statistics'));
   }
 

core/modules/config/src/Tests/ConfigEntityListTest.php

@@ -25,6 +25,16 @@ class ConfigEntityListTest extends WebTestBase {
    */
   public static $modules = array('config_test');
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+    // Delete the override config_test entity since it is not required by this
+    // test.
+    \Drupal::entityManager()->getStorage('config_test')->load('override')->delete();
+  }
+
   /**
    * Tests entity list builder methods.
    */

core/modules/config/src/Tests/ConfigImportRecreateTest.php

@@ -38,7 +38,7 @@ protected function setUp() {
     parent::setUp();
 
     $this->installEntitySchema('node');
-    $this->installConfig(array('field'));
+    $this->installConfig(array('field', 'node'));
 
     $this->copyConfig($this->container->get('config.storage'), $this->container->get('config.storage.staging'));
 
@@ -94,8 +94,8 @@ public function testRecreateEntity() {
     // will be recreated.
     $creates = $this->configImporter->getUnprocessedConfiguration('create');
     $deletes = $this->configImporter->getUnprocessedConfiguration('delete');
-    $this->assertEqual(4, count($creates), 'There are 4 configuration items to create.');
-    $this->assertEqual(4, count($deletes), 'There are 4 configuration items to delete.');
+    $this->assertEqual(5, count($creates), 'There are 5 configuration items to create.');
+    $this->assertEqual(5, count($deletes), 'There are 5 configuration items to delete.');
     $this->assertEqual(0, count($this->configImporter->getUnprocessedConfiguration('update')), 'There are no configuration items to update.');
     $this->assertIdentical($creates, array_reverse($deletes), 'Deletes and creates contain the same configuration names in opposite orders due to dependencies.');
 

core/modules/config/src/Tests/ConfigImporterTest.php

@@ -32,7 +32,7 @@ class ConfigImporterTest extends KernelTestBase {
    *
    * @var array
    */
-  public static $modules = array('config_test', 'system', 'config_import_test', 'config_test_language');
+  public static $modules = array('config_test', 'system', 'config_import_test');
 
   protected function setUp() {
     parent::setUp();
@@ -541,24 +541,4 @@ function testIsInstallable() {
     $this->assertTrue($this->container->get('config.storage')->exists($config_name));
   }
 
-  /**
-   * Tests imported configuration entities with and without language information.
-   */
-  function testLanguage() {
-    // Test imported configuration with implicit language code.
-    $data = $this->container->get('config.storage.installer')->read('config_test.dynamic.dotted.english');
-    $this->assertTrue(!isset($data['langcode']));
-    $this->assertEqual(
-      $this->config('config_test.dynamic.dotted.english')->get('langcode'),
-      'en'
-    );
-
-    // Test imported configuration with explicit language code.
-    $data = $this->container->get('config.storage.installer')->read('config_test.dynamic.dotted.french');
-    $this->assertEqual($data['langcode'], 'fr');
-    $this->assertEqual(
-      $this->config('config_test.dynamic.dotted.french')->get('langcode'),
-      'fr'
-    );
-  }
 }

core/modules/config/src/Tests/ConfigInstallProfileOverrideTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\config\Tests\ConfigInstallProfileOverrideTest.
+ */
+
+namespace Drupal\config\Tests;
+
+use Drupal\Core\Config\InstallStorage;
+use Drupal\simpletest\WebTestBase;
+use Drupal\Core\Config\FileStorage;
+use Drupal\system\Entity\Action;
+use Drupal\tour\Entity\Tour;
+
+/**
+ * Tests installation and removal of configuration objects in install, disable
+ * and uninstall functionality.
+ *
+ * @group config
+ */
+class ConfigInstallProfileOverrideTest extends WebTestBase {
+
+  /**
+   * The profile to install as a basis for testing.
+   *
+   * @var string
+   */
+  protected $profile = 'testing_config_overrides';
+
+
+  /**
+   * 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') . '/'. InstallStorage::CONFIG_INSTALL_DIRECTORY;
+    $this->assertTrue(is_dir($config_dir));
+    $source_storage = new FileStorage($config_dir);
+    $data = $source_storage->read($config_name);
+    $this->assertIdentical($data, $expected_original_data);