Issue #2520540 by alexpott: Enforced configuration dependencies shouldn't have...

Issue #2520540 by alexpott: Enforced configuration dependencies shouldn't have to be repeated in the calculated dependencies

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

@@ -320,7 +320,8 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names
       }
       if ($this->callOnDependencyRemoval($dependent, $original_dependencies, $type, $names)) {
         // Recalculate dependencies and update the dependency graph data.
-        $dependency_manager->updateData($dependent->getConfigDependencyName(), $dependent->calculateDependencies());
+        $dependent->calculateDependencies();
+        $dependency_manager->updateData($dependent->getConfigDependencyName(), $dependent->getDependencies());
         // Based on the updated data rebuild the list of dependents.
         $dependents = $this->findConfigEntityDependentsAsEntities($type, $names, $dependency_manager);
         // Ensure that the dependency has actually been fixed. It is possible
@@ -458,6 +459,9 @@ public function findMissingContentDependencies() {
       if (isset($config_data['dependencies']['content'])) {
         $content_dependencies = array_merge($content_dependencies, $config_data['dependencies']['content']);
       }
+      if (isset($config_data['dependencies']['enforced']['content'])) {
+        $content_dependencies = array_merge($content_dependencies, $config_data['dependencies']['enforced']['content']);
+      }
     }
     foreach (array_unique($content_dependencies) as $content_dependency) {
       // Format of the dependency is entity_type:bundle:uuid.

core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php

@@ -58,11 +58,11 @@
  * Configuration entity classes usually extend
  * \Drupal\Core\Config\Entity\ConfigEntityBase. The base class provides a
  * generic implementation of the calculateDependencies() method that can
- * discover dependencies due to enforced dependencies, plugins, and third party
- * settings. If the configuration entity has dependencies that cannot be
- * discovered by the base class's implementation, then it needs to implement
+ * discover dependencies due to plugins, and third party settings. If the
+ * configuration entity has dependencies that cannot be discovered by the base
+ * class's implementation, then it needs to implement
  * \Drupal\Core\Config\Entity\ConfigEntityInterface::calculateDependencies() to
- * calculate (and return) the dependencies. In this method, use
+ * calculate the dependencies. In this method, use
  * \Drupal\Core\Config\Entity\ConfigEntityBase::addDependency() to add
  * dependencies. Implementations should call the base class implementation to
  * inherit the generic functionality.
@@ -87,7 +87,7 @@
  * for configuration synchronization, which needs to be able to validate
  * configuration in the staging directory before the synchronization has
  * occurred. Also, if you have a configuration entity object and you want to
- * get the current dependencies without recalculation, you can use
+ * get the current dependencies (without recalculation), you can use
  * \Drupal\Core\Config\Entity\ConfigEntityInterface::getDependencies().
  *
  * When uninstalling a module or a theme, configuration entities that are
@@ -115,6 +115,7 @@
  * module dependency in the sub-module only.
  *
  * @see \Drupal\Core\Config\Entity\ConfigEntityInterface::calculateDependencies()
+ * @see \Drupal\Core\Config\Entity\ConfigEntityInterface::getDependencies()
  * @see \Drupal\Core\Config\Entity\ConfigEntityInterface::onDependencyRemoval()
  * @see \Drupal\Core\Config\Entity\ConfigEntityBase::addDependency()
  * @see \Drupal\Core\Config\ConfigInstallerInterface::installDefaultConfig()

core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\Core\Config\Entity;
 
+use Drupal\Component\Utility\NestedArray;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Config\ConfigException;
 use Drupal\Core\Config\Schema\SchemaIncompleteException;
@@ -353,16 +354,9 @@ public function preSave(EntityStorageInterface $storage) {
    * {@inheritdoc}
    */
   public function calculateDependencies() {
-    // Dependencies should be recalculated on every save. This ensures stale
-    // dependencies are never saved.
-    if (isset($this->dependencies['enforced'])) {
-      $dependencies = $this->dependencies['enforced'];
-      $this->dependencies = $dependencies;
-      $this->dependencies['enforced'] = $dependencies;
-    }
-    else {
-      $this->dependencies = array();
-    }
+    // All dependencies should be recalculated on every save apart from enforced
+    // dependencies. This ensures stale dependencies are never saved.
+    $this->dependencies = array_intersect_key($this->dependencies, ['enforced' => '']);
     if ($this instanceof EntityWithPluginCollectionInterface) {
       // Configuration entities need to depend on the providers of any plugins
       // that they store the configuration for.
@@ -379,7 +373,7 @@ public function calculateDependencies() {
         $this->addDependency('module', $provider);
       }
     }
-    return $this->dependencies;
+    return $this;
   }
 
   /**
@@ -438,7 +432,14 @@ protected function addDependency($type, $name) {
    * {@inheritdoc}
    */
   public function getDependencies() {
-    return $this->dependencies;
+    $dependencies = $this->dependencies;
+    if (isset($dependencies['enforced'])) {
+      // Merge the enforced dependencies into the list of dependencies.
+      $enforced_dependencies = $dependencies['enforced'];
+      unset($dependencies['enforced']);
+      $dependencies = NestedArray::mergeDeep($dependencies, $enforced_dependencies);
+    }
+    return $dependencies;
   }
 
   /**

core/lib/Drupal/Core/Config/Entity/ConfigEntityDependency.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\Core\Config\Entity;
 
+use Drupal\Component\Utility\NestedArray;
+
 /**
  * Provides a value object to discover configuration dependencies.
  *
@@ -26,7 +28,7 @@ class ConfigEntityDependency {
    *
    * @var array
    */
-  protected $dependencies;
+  protected $dependencies = [];
 
   /**
    * Constructs the configuration entity dependency from the entity values.
@@ -36,13 +38,16 @@ class ConfigEntityDependency {
    * @param array $values
    *   (optional) The configuration entity's values.
    */
-  public function __construct($name, $values = array()) {
+  public function __construct($name, $values = []) {
     $this->name = $name;
-    if (isset($values['dependencies'])) {
-      $this->dependencies = $values['dependencies'];
+    if (isset($values['dependencies']) && isset($values['dependencies']['enforced'])) {
+      // Merge the enforced dependencies into the list of dependencies.
+      $enforced_dependencies = $values['dependencies']['enforced'];
+      unset($values['dependencies']['enforced']);
+      $this->dependencies = NestedArray::mergeDeep($values['dependencies'], $enforced_dependencies);
     }
-    else {
-      $this->dependencies = array();
+    elseif (isset($values['dependencies'])) {
+      $this->dependencies = $values['dependencies'];
     }
   }
 

core/lib/Drupal/Core/Config/Entity/ConfigEntityInterface.php

@@ -144,8 +144,7 @@ public function set($property_name, $value);
   /**
    * Calculates dependencies and stores them in the dependency property.
    *
-   * @return array
-   *   An array of dependencies grouped by type (module, theme, entity).
+   * @return $this
    *
    * @see \Drupal\Core\Config\Entity\ConfigDependencyManager
    */

core/lib/Drupal/Core/Entity/EntityDisplayBase.php

@@ -278,7 +278,7 @@ public function calculateDependencies() {
       $mode_entity = $this->entityManager()->getStorage('entity_' . $this->displayContext . '_mode')->load($target_entity_type->id() . '.' . $this->mode);
       $this->addDependency('config', $mode_entity->getConfigDependencyName());
     }
-    return $this->dependencies;
+    return $this;
   }
 
   /**

core/lib/Drupal/Core/Entity/EntityDisplayModeBase.php

@@ -93,7 +93,7 @@ public function calculateDependencies() {
     parent::calculateDependencies();
     $target_entity_type = \Drupal::entityManager()->getDefinition($this->targetEntityType);
     $this->addDependency('module', $target_entity_type->getProvider());
-    return $this->dependencies;
+    return $this;
   }
 
   /**

core/lib/Drupal/Core/Field/FieldConfigBase.php

@@ -245,7 +245,7 @@ public function calculateDependencies() {
     $bundle_config_dependency = $this->entityManager()->getDefinition($this->entity_type)->getBundleConfigDependency($this->bundle);
     $this->addDependency($bundle_config_dependency['type'], $bundle_config_dependency['name']);
 
-    return $this->dependencies;
+    return $this;
   }
 
   /**

core/modules/block/src/Entity/Block.php

@@ -231,7 +231,7 @@ public static function sort(ConfigEntityInterface $a, ConfigEntityInterface $b)
   public function calculateDependencies() {
     parent::calculateDependencies();
     $this->addDependency('theme', $this->theme);
-    return $this->dependencies;
+    return $this;
   }
 
   /**

core/modules/block/tests/src/Unit/BlockConfigEntityUnitTest.php

@@ -100,7 +100,7 @@ public function testCalculateDependencies() {
       ->method('getPluginCollections')
       ->will($this->returnValue(array($plugin_collection)));
 
-    $dependencies = $entity->calculateDependencies();
+    $dependencies = $entity->calculateDependencies()->getDependencies();
     $this->assertContains('test', $dependencies['module']);
     $this->assertContains('stark', $dependencies['theme']);
   }

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

@@ -2,7 +2,6 @@ langcode: en
 status: false
 dependencies:
   module:
-    - book
     - node
   enforced:
     module:

core/modules/book/config/install/node.type.book.yml

 langcode: en
 status: true
 dependencies:
-  module:
-    - book
   enforced:
     module:
       - book

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

@@ -69,7 +69,8 @@ public function testDependencyManagement() {
     // Ensure that the provider of the config entity is not actually written to
     // the dependencies array.
     $raw_config = $this->config('config_test.dynamic.entity1');
-    $this->assertTrue(array_search('node', $raw_config->get('dependencies.module')) !== FALSE, 'Node module is written to the dependencies array as this has to be explicit.');
+    $root_module_dependencies = $raw_config->get('dependencies.module');
+    $this->assertTrue(empty($root_module_dependencies), 'Node module is not written to the root dependencies array as it is enforced.');
 
     // Create additional entities to test dependencies on config entities.
     $entity2 = $storage->create(array('id' => 'entity2', 'dependencies' => array('enforced' => array('config' => array($entity1->getConfigDependencyName())))));
@@ -301,10 +302,10 @@ public function testConfigEntityUninstall() {
     $this->assertFalse($storage->load('entity1'), 'Entity 1 deleted');
     $entity2 = $storage->load('entity2');
     $this->assertTrue($entity2, 'Entity 2 not deleted');
-    $this->assertEqual($entity2->calculateDependencies()['config'], array(), 'Entity 2 dependencies updated to remove dependency on Entity1.');
+    $this->assertEqual($entity2->calculateDependencies()->getDependencies()['config'], array(), 'Entity 2 dependencies updated to remove dependency on Entity1.');
     $entity3 = $storage->load('entity3');
     $this->assertTrue($entity3, 'Entity 3 not deleted');
-    $this->assertEqual($entity3->calculateDependencies()['config'], [$entity2->getConfigDependencyName()], 'Entity 3 still depends on Entity 2.');
+    $this->assertEqual($entity3->calculateDependencies()->getDependencies()['config'], [$entity2->getConfigDependencyName()], 'Entity 3 still depends on Entity 2.');
     $this->assertFalse($storage->load('entity4'), 'Entity 4 deleted');
   }
 
@@ -402,10 +403,10 @@ public function testConfigEntityDelete() {
     $this->assertFalse($storage->load('entity1'), 'Entity 1 deleted');
     $entity2 = $storage->load('entity2');
     $this->assertTrue($entity2, 'Entity 2 not deleted');
-    $this->assertEqual($entity2->calculateDependencies()['config'], array(), 'Entity 2 dependencies updated to remove dependency on Entity1.');
+    $this->assertEqual($entity2->calculateDependencies()->getDependencies()['config'], array(), 'Entity 2 dependencies updated to remove dependency on Entity1.');
     $entity3 = $storage->load('entity3');
     $this->assertTrue($entity3, 'Entity 3 not deleted');
-    $this->assertEqual($entity3->calculateDependencies()['config'], [$entity2->getConfigDependencyName()], 'Entity 3 still depends on Entity 2.');
+    $this->assertEqual($entity3->calculateDependencies()->getDependencies()['config'], [$entity2->getConfigDependencyName()], 'Entity 3 still depends on Entity 2.');
   }
 
   /**

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

@@ -125,10 +125,10 @@ function testConfigDependencyDeleteFormTrait() {
     $this->assertFalse($storage->load('entity1'), 'Test entity 1 deleted');
     $entity2 = $storage->load('entity2');
     $this->assertTrue($entity2, 'Entity 2 not deleted');
-    $this->assertEqual($entity2->calculateDependencies()['config'], array(), 'Entity 2 dependencies updated to remove dependency on Entity1.');
+    $this->assertEqual($entity2->calculateDependencies()->getDependencies()['config'], array(), 'Entity 2 dependencies updated to remove dependency on Entity1.');
     $entity3 = $storage->load('entity3');
     $this->assertTrue($entity3, 'Entity 3 not deleted');
-    $this->assertEqual($entity3->calculateDependencies()['config'], [$entity2->getConfigDependencyName()], 'Entity 3 still depends on Entity 2.');
+    $this->assertEqual($entity3->calculateDependencies()->getDependencies()['config'], [$entity2->getConfigDependencyName()], 'Entity 3 still depends on Entity 2.');
 
   }
 

core/modules/config/tests/config_other_module_config_test/config/optional/config_test.dynamic.other_module_test_unmet.yml

@@ -6,8 +6,6 @@ status: true
 langcode: en
 protected_property: Default
 dependencies:
-  module:
-    - config_install_dependency_test
   enforced:
     module:
       - config_install_dependency_test

core/modules/editor/src/Entity/Editor.php

@@ -107,7 +107,7 @@ public function calculateDependencies() {
     //   config entity and dependency on provider is managed automatically.
     $definition = $this->editorPluginManager()->createInstance($this->editor)->getPluginDefinition();
     $this->addDependency('module', $definition['provider']);
-    return $this->dependencies;
+    return $this;
   }
 
   /**

core/modules/editor/tests/src/Unit/EditorConfigEntityUnitTest.php

@@ -130,7 +130,7 @@ public function testCalculateDependencies() {
                         ->with('filter_format')
                         ->will($this->returnValue($storage));
 
-    $dependencies = $entity->calculateDependencies();
+    $dependencies = $entity->calculateDependencies()->getDependencies();
     $this->assertContains('test_module', $dependencies['module']);
     $this->assertContains('filter.format.test', $dependencies['config']);
   }

core/modules/field/src/Entity/FieldConfig.php

@@ -187,7 +187,7 @@ public function calculateDependencies() {
     parent::calculateDependencies();
     // Mark the field_storage_config as a a dependency.
     $this->addDependency('config', $this->getFieldStorageDefinition()->getConfigDependencyName());
-    return $this->dependencies;
+    return $this;
   }
 
   /**

core/modules/field/src/Entity/FieldStorageConfig.php

@@ -346,7 +346,7 @@ public function calculateDependencies() {
     // Ensure the field is dependent on the provider of the entity type.
     $entity_type = \Drupal::entityManager()->getDefinition($this->entity_type);
     $this->addDependency('module', $entity_type->getProvider());
-    return $this->dependencies;
+    return $this;
   }
 
   /**

core/modules/field/tests/src/Unit/FieldConfigEntityUnitTest.php

@@ -153,7 +153,7 @@ public function testCalculateDependencies() {
       'bundle' => 'test_bundle',
       'field_type' => 'test_field',
     ), $this->entityTypeId);
-    $dependencies = $field->calculateDependencies();
+    $dependencies = $field->calculateDependencies()->getDependencies();
     $this->assertContains('field.storage.test_entity_type.test_field', $dependencies['config']);
     $this->assertContains('test.test_entity_type.id', $dependencies['config']);
     $this->assertEquals(['test_module', 'test_module2', 'test_module3'], $dependencies['module']);