diff --git a/core/lib/Drupal/Core/Config/ConfigManager.php b/core/lib/Drupal/Core/Config/ConfigManager.php index 8b3b3d66fd..0761cb7398 100644 --- a/core/lib/Drupal/Core/Config/ConfigManager.php +++ b/core/lib/Drupal/Core/Config/ConfigManager.php @@ -296,8 +296,19 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names // initial values. $dependency_manager = $this->getConfigDependencyManager(); $dependents = $this->findConfigEntityDependentsAsEntities($type, $names, $dependency_manager); - $original_dependencies = $dependents; - $delete_uuids = []; + // Store the list of dependencies in three separate variables. This allows + // us to determine how the dependency graph changes as entities are fixed by + // calling the onDependencyRemoval() method. The variables are: + // - $dependents is the list of dependencies to process. This list changes + // as entities are processed and either fixed or deleted. + // - $current_dependencies is the list of dependencies on $names. This list + // is recalculated when calling an entity's onDependencyRemoval() method + // results in the entity changing. This list is passed to each entity's + // onDependencyRemoval() method as the list of affected entities. + // - $original_dependencies is the list of original dependencies on $names. + // This list never changes. + $original_dependencies = $current_dependencies = $dependents; + $affected_uuids = []; $return = [ 'update' => [], @@ -305,13 +316,6 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names 'unchanged' => [], ]; - // Create a map of UUIDs to $original_dependencies key so that we can remove - // fixed dependencies. - $uuid_map = []; - foreach ($original_dependencies as $key => $entity) { - $uuid_map[$entity->uuid()] = $key; - } - // Try to fix any dependencies and find out what will happen to the // dependency graph. Entities are processed in the order of most dependent // first. For example, this ensures that Menu UI third party dependencies on @@ -324,16 +328,19 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names $dependent = clone $dependent; } $fixed = FALSE; - if ($this->callOnDependencyRemoval($dependent, $original_dependencies, $type, $names)) { + if ($this->callOnDependencyRemoval($dependent, $current_dependencies, $type, $names)) { // Recalculate dependencies and update the dependency graph data. $dependent->calculateDependencies(); $dependency_manager->updateData($dependent->getConfigDependencyName(), $dependent->getDependencies()); - // Based on the updated data rebuild the list of dependents. This will - // remove entities that are no longer dependent after the recalculation. - $dependents = $this->findConfigEntityDependentsAsEntities($type, $names, $dependency_manager); - // Remove any entities that we've already marked for deletion. - $dependents = array_filter($dependents, function ($dependent) use ($delete_uuids) { - return !in_array($dependent->uuid(), $delete_uuids); + // Based on the updated data rebuild the list of current dependencies. + // This will remove entities that are no longer dependent after the + // recalculation. + $current_dependencies = $this->findConfigEntityDependentsAsEntities($type, $names, $dependency_manager); + // Rebuild the list of entities that we need to process using the new + // list of current dependencies and removing any entities that we've + // already processed. + $dependents = array_filter($current_dependencies, function ($current_dependency) use ($affected_uuids) { + return !in_array($current_dependency->uuid(), $affected_uuids); }); // Ensure that the dependency has actually been fixed. It is possible // that the dependent has multiple dependencies that cause it to be in @@ -347,23 +354,22 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names } } if ($fixed) { - // Remove the fixed dependency from the list of original dependencies. - unset($original_dependencies[$uuid_map[$dependent->uuid()]]); + $affected_uuids[] = $dependent->uuid(); $return['update'][] = $dependent; } } // If the entity cannot be fixed then it has to be deleted. if (!$fixed) { - $delete_uuids[] = $dependent->uuid(); + $affected_uuids[] = $dependent->uuid(); // Deletes should occur in the order of the least dependent first. For // example, this ensures that fields are removed before field storages. array_unshift($return['delete'], $dependent); } } - // Use the lists of UUIDs to filter the original list to work out which - // configuration entities are unchanged. - $return['unchanged'] = array_filter($original_dependencies, function ($dependent) use ($delete_uuids) { - return !(in_array($dependent->uuid(), $delete_uuids)); + // Use the list of affected UUIDs to filter the original list to work out + // which configuration entities are unchanged. + $return['unchanged'] = array_filter($original_dependencies, function ($dependent) use ($affected_uuids) { + return !(in_array($dependent->uuid(), $affected_uuids)); }); return $return; diff --git a/core/modules/config/tests/config_test/src/Entity/ConfigTest.php b/core/modules/config/tests/config_test/src/Entity/ConfigTest.php index 1bd1d8dee9..ee0e35a2cd 100644 --- a/core/modules/config/tests/config_test/src/Entity/ConfigTest.php +++ b/core/modules/config/tests/config_test/src/Entity/ConfigTest.php @@ -117,9 +117,12 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti * {@inheritdoc} */ public function onDependencyRemoval(array $dependencies) { - // Record which entities have this method called on. + // Record which entities have this method called on and what dependencies + // are passed. $called = \Drupal::state()->get('config_test.on_dependency_removal_called', []); - $called[] = $this->id(); + $called[$this->id()] = $dependencies; + $called[$this->id()]['config'] = array_keys($called[$this->id()]['config']); + $called[$this->id()]['content'] = array_keys($called[$this->id()]['content']); \Drupal::state()->set('config_test.on_dependency_removal_called', $called); $changed = parent::onDependencyRemoval($dependencies); diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigDependencyTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigDependencyTest.php index 84b4e42067..a9cd9f1902 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigDependencyTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigDependencyTest.php @@ -235,9 +235,9 @@ public function providerConfigEntityUninstallComplex() { // Ensure that alphabetical order has no influence on dependency fixing and // removal. return [ - [['a', 'b', 'c', 'd']], - [['d', 'c', 'b', 'a']], - [['c', 'd', 'a', 'b']], + [['a', 'b', 'c', 'd', 'e']], + [['e', 'd', 'c', 'b', 'a']], + [['e', 'c', 'd', 'a', 'b']], ]; } @@ -316,6 +316,25 @@ public function testConfigEntityUninstallComplex(array $entity_id_suffixes) { ); $entity_4->save(); + // Entity 5 will be fixed because it is dependent on entity 3, which is + // unchanged, and entity 1 which will be fixed because + // \Drupal\config_test\Entity::onDependencyRemoval() will remove the + // dependency. + $entity_5 = $storage->create( + [ + 'id' => 'entity_' . $entity_id_suffixes[4], + 'dependencies' => [ + 'enforced' => [ + 'config' => [ + $entity_1->getConfigDependencyName(), + $entity_3->getConfigDependencyName(), + ], + ], + ], + ] + ); + $entity_5->save(); + // Set a more complicated test where dependencies will be fixed. \Drupal::state()->set('config_test.fix_dependencies', [$entity_1->getConfigDependencyName()]); \Drupal::state()->set('config_test.on_dependency_removal_called', []); @@ -323,14 +342,22 @@ public function testConfigEntityUninstallComplex(array $entity_id_suffixes) { // Do a dry run using // \Drupal\Core\Config\ConfigManager::getConfigEntitiesToChangeOnDependencyRemoval(). $config_entities = $config_manager->getConfigEntitiesToChangeOnDependencyRemoval('module', ['node']); + + // Assert that \Drupal\config_test\Entity\ConfigTest::onDependencyRemoval() + // is called as expected and with the correct dependencies. + $called = \Drupal::state()->get('config_test.on_dependency_removal_called', []); + $this->assertArrayNotHasKey($entity_3->id(), $called, 'ConfigEntityInterface::onDependencyRemoval() is not called for entity 3.'); + $this->assertSame([$entity_1->id(), $entity_4->id(), $entity_2->id(), $entity_5->id()], array_keys($called), 'The most dependent entites have ConfigEntityInterface::onDependencyRemoval() called first.'); + $this->assertSame(['config' => [], 'content' => [], 'module' => ['node'], 'theme' => []], $called[$entity_1->id()]); + $this->assertSame(['config' => [$entity_1->getConfigDependencyName()], 'content' => [], 'module' => [], 'theme' => []], $called[$entity_2->id()]); + $this->assertSame(['config' => [$entity_1->getConfigDependencyName()], 'content' => [], 'module' => ['node'], 'theme' => []], $called[$entity_4->id()]); + $this->assertSame(['config' => [$entity_1->getConfigDependencyName()], 'content' => [], 'module' => [], 'theme' => []], $called[$entity_5->id()]); + $this->assertEqual($entity_1->uuid(), $config_entities['delete'][1]->uuid(), 'Entity 1 will be deleted.'); - $this->assertEqual($entity_2->uuid(), reset($config_entities['update'])->uuid(), 'Entity 2 will be updated.'); + $this->assertEqual($entity_2->uuid(), $config_entities['update'][0]->uuid(), 'Entity 2 will be updated.'); $this->assertEqual($entity_3->uuid(), reset($config_entities['unchanged'])->uuid(), 'Entity 3 is not changed.'); $this->assertEqual($entity_4->uuid(), $config_entities['delete'][0]->uuid(), 'Entity 4 will be deleted.'); - - $called = \Drupal::state()->get('config_test.on_dependency_removal_called', []); - $this->assertFalse(in_array($entity_3->id(), $called), 'ConfigEntityInterface::onDependencyRemoval() is not called for entity 3.'); - $this->assertSame([$entity_1->id(), $entity_4->id(), $entity_2->id()], $called, 'The most dependent entites have ConfigEntityInterface::onDependencyRemoval() called first.'); + $this->assertEqual($entity_5->uuid(), $config_entities['update'][1]->uuid(), 'Entity 5 is updated.'); // Perform a module rebuild so we can know where the node module is located // and uninstall it. @@ -443,8 +470,11 @@ public function testConfigEntityUninstallThirdParty() { $this->assertSame($expected, $config_entity_ids); $called = \Drupal::state()->get('config_test.on_dependency_removal_called', []); - $this->assertFalse(in_array($entity_3->id(), $called), 'ConfigEntityInterface::onDependencyRemoval() is not called for entity 3.'); - $this->assertSame([$entity_1->id(), $entity_4->id(), $entity_2->id()], $called, 'The most dependent entities have ConfigEntityInterface::onDependencyRemoval() called first.'); + $this->assertArrayNotHasKey($entity_3->id(), $called, 'ConfigEntityInterface::onDependencyRemoval() is not called for entity 3.'); + $this->assertSame([$entity_1->id(), $entity_4->id(), $entity_2->id()], array_keys($called), 'The most dependent entities have ConfigEntityInterface::onDependencyRemoval() called first.'); + $this->assertSame(['config' => [], 'content' => [], 'module' => ['node'], 'theme' => []], $called[$entity_1->id()]); + $this->assertSame(['config' => [], 'content' => [], 'module' => ['node'], 'theme' => []], $called[$entity_2->id()]); + $this->assertSame(['config' => [], 'content' => [], 'module' => ['node'], 'theme' => []], $called[$entity_4->id()]); // Perform a module rebuild so we can know where the node module is located // and uninstall it.