Commit 7d029cb7 authored by catch's avatar catch

Issue #2642374 by alexpott, Berdir, drunken monkey, beejeebus: Dependency...

Issue #2642374 by alexpott, Berdir, drunken monkey, beejeebus: Dependency removal logic incorrectly affects indirect dependents
parent 0d838339
......@@ -302,7 +302,7 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names
$dependency_manager = $this->getConfigDependencyManager();
$dependents = $this->findConfigEntityDependentsAsEntities($type, $names, $dependency_manager);
$original_dependencies = $dependents;
$update_uuids = [];
$delete_uuids = $update_uuids = [];
$return = [
'update' => [],
......@@ -311,26 +311,35 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names
];
// Try to fix any dependencies and find out what will happen to the
// dependency graph.
foreach ($dependents as $dependent) {
// dependency graph. Entities are processed in the order of most dependent
// first. For example, this ensures that fields are removed before
// field storages.
while ($dependent = array_pop($dependents)) {
/** @var \Drupal\Core\Config\Entity\ConfigEntityInterface $dependent */
if ($dry_run) {
// Clone the entity so any changes do not change any static caches.
$dependent = clone $dependent;
}
$fixed = FALSE;
if ($this->callOnDependencyRemoval($dependent, $original_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.
// 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);
});
// Ensure that the dependency has actually been fixed. It is possible
// that the dependent has multiple dependencies that cause it to be in
// the dependency chain.
$fixed = TRUE;
foreach ($dependents as $entity) {
foreach ($dependents as $key => $entity) {
if ($entity->uuid() == $dependent->uuid()) {
$fixed = FALSE;
unset($dependents[$key]);
break;
}
}
......@@ -339,15 +348,12 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names
$update_uuids[] = $dependent->uuid();
}
}
// If the entity cannot be fixed then it has to be deleted.
if (!$fixed) {
$delete_uuids[] = $dependent->uuid();
$return['delete'][] = $dependent;
}
}
// Now that we've fixed all the possible dependencies the remaining need to
// be deleted. Reverse the deletes so that entities are removed in the
// correct order of dependence. For example, this ensures that fields are
// removed before field storages.
$return['delete'] = array_reverse($dependents);
$delete_uuids = array_map(function($dependent) {
return $dependent->uuid();
}, $return['delete']);
// 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, $update_uuids) {
......
......@@ -229,7 +229,7 @@ public function testConfigEntityUninstall() {
// Set a more complicated test where dependencies will be fixed.
\Drupal::state()->set('config_test.fix_dependencies', array($entity1->getConfigDependencyName()));
\Drupal::state()->set('config_test.on_dependency_removal_called', []);
// Entity1 will be deleted because it depends on node.
$entity1 = $storage->create(
array(
......@@ -259,7 +259,8 @@ public function testConfigEntityUninstall() {
$entity2->save();
// Entity3 will be unchanged because it is dependent on Entity2 which can
// be fixed.
// be fixed. The ConfigEntityInterface::onDependencyRemoval() method will
// not be called for this entity.
$entity3 = $storage->create(
array(
'id' => 'entity3',
......@@ -295,6 +296,10 @@ public function testConfigEntityUninstall() {
$this->assertEqual($entity3->uuid(), reset($config_entities['unchanged'])->uuid(), 'Entity 3 is not changed.');
$this->assertEqual($entity4->uuid(), $config_entities['delete'][1]->uuid(), 'Entity 4 will be deleted.');
$called = \Drupal::state()->get('config_test.on_dependency_removal_called', []);
$this->assertFalse(in_array($entity3->id(), $called), 'ConfigEntityInterface::onDependencyRemoval() is not called for entity 3.');
$this->assertIdentical(['entity1', 'entity2', 'entity4'], $called, 'The most dependent entites have ConfigEntityInterface::onDependencyRemoval() called first.');
// Perform the uninstall.
$config_manager->uninstall('module', 'node');
......
......@@ -122,6 +122,11 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
* {@inheritdoc}
*/
public function onDependencyRemoval(array $dependencies) {
// Record which entities have this method called on.
$called = \Drupal::state()->get('config_test.on_dependency_removal_called', []);
$called[] = $this->id();
\Drupal::state()->set('config_test.on_dependency_removal_called', $called);
$changed = parent::onDependencyRemoval($dependencies);
if (!isset($this->dependencies['enforced']['config'])) {
return $changed;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment