Commit 0c111364 authored by catch's avatar catch
Browse files

Issue #2224581 by alexpott, larowlan, jhodgdon, mgifford: Delete forum data on uninstall.

parent 69be11eb
......@@ -208,9 +208,8 @@ route:
label: 'Param'
# Config dependencies.
config_dependencies:
config_dependencies_base:
type: mapping
label: 'Configuration dependencies'
mapping:
entity:
type: sequence
......@@ -228,6 +227,14 @@ config_dependencies:
sequence:
- type: string
config_dependencies:
type: config_dependencies_base
label: 'Configuration dependencies'
mapping:
enforced:
type: config_dependencies_base
label: 'Enforced configuration dependencies'
config_entity:
type: mapping
mapping:
......
......@@ -19,42 +19,71 @@
* handled in the correct order. For example, node types are created before
* their fields, and both are created before the view display configuration.
*
* If a configuration entity is provided as default configuration by an
* extension (module, theme, or profile), the extension has to depend on any
* modules or themes that the configuration depends on. For example, if a view
* configuration entity is provided by an installation profile and the view will
* not work without a certain module, the profile must declare a dependency on
* this module in its info.yml file. If you do not want your extension to always
* depend on a particular module that one of its default configuration entities
* depends on, you can use a sub-module: move the configuration entity to the
* sub-module instead of including it in the main extension, and declare the
* module dependency in the sub-module only.
* The configuration dependency value is structured like this:
* <code>
* array(
* 'entity => array(
* // An array of configuration entity object names. Recalculated on save.
* ),
* 'module' => array(
* // An array of module names. Recalculated on save.
* ),
* 'theme' => array(
* // An array of theme names. Recalculated on save.
* ),
* 'enforced' => array(
* // An array of configuration dependencies that the config entity is
* // ensured to have regardless of the details of the configuration. These
* // dependencies are not recalculated on save.
* 'entity' => array(),
* 'module' => array(),
* 'theme' => array(),
* ),
* );
* </code>
*
* Classes for configuration entities with dependencies normally extend
* \Drupal\Core\Config\Entity\ConfigEntityBase or at least use
* \Drupal\Core\Plugin\PluginDependencyTrait to manage dependencies. The class
* must implement
* \Drupal\Core\Config\Entity\ConfigEntityInterface::calculateDependencies(),
* which should calculate (and return) the dependencies. In this method, use
* Configuration entity dependencies are recalculated on save based on the
* current values of the configuration. For example, a filter format will depend
* on the modules that provide the filter plugins it configures. The filter
* format can be reconfigured to use a different filter plugin provided by
* another module. If this occurs, the dependencies will be recalculated on save
* and the old module will be removed from the list of dependencies and replaced
* with the new one.
*
* 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
* \Drupal\Core\Config\Entity\ConfigEntityInterface::calculateDependencies() to
* calculate (and return) the dependencies. In this method, use
* \Drupal\Core\Config\Entity\ConfigEntityBase::addDependency() to add
* dependencies. Most implementations call
* \Drupal\Core\Config\Entity\ConfigEntityBase::calculateDependencies() since it
* provides a generic implementation that will discover dependencies due to
* plugins and third party settings. To get a configuration entity's current
* dependencies without forcing a recalculation use
* \Drupal\Core\Config\Entity\ConfigEntityInterface::getDependencies().
* dependencies. Implementations should call the base class implementation to
* inherit the generic functionality.
*
* Classes for configurable plugins are a special case. They can either declare
* their configuration dependencies using the calculateDependencies() method
* described in the paragraph above, or if they have only static dependencies,
* these can be declared using the 'config_dependencies' annotation key.
*
* If an extension author wants a configuration entity to depend on something
* that is not calculable then they can add these dependencies to the enforced
* dependencies key. For example, the Forum module provides the forum node type
* and in order for it to be deleted when the forum module is uninstalled it has
* an enforced dependency on the module. The dependency on the Forum module can
* not be calculated since there is nothing inherent in the state of the node
* type configuration entity that depends on functionality provided by the Forum
* module.
*
* Once declared properly, dependencies are saved to the configuration entity's
* configuration object so that they can be checked without the module that
* provides the configuration entity class being installed. This is important
* for configuration synchronization, which needs to be able to validate
* configuration in the staging directory before the synchronization has
* occurred.
* occurred. Also, if you have a configuration entity object and you want to
* 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
* dependent will also be removed. This default behavior can lead to undesirable
......@@ -65,10 +94,21 @@
* which allows the entity class to remove dependencies instead of being deleted
* themselves. Implementations should save the entity if dependencies have been
* successfully removed, in order to register the newly cleaned-out dependency
* list. So, in the above example, the node view mode configuration entity class
* list. So, for example, the node view mode configuration entity class
* should implement this method to remove references to formatters if the plugin
* that supplies them depends on a module that is being uninstalled.
*
* If a configuration entity is provided as default configuration by an
* extension (module, theme, or profile), the extension has to depend on any
* modules or themes that the configuration depends on. For example, if a view
* configuration entity is provided by an installation profile and the view will
* not work without a certain module, the profile must declare a dependency on
* this module in its info.yml file. If you do not want your extension to always
* depend on a particular module that one of its default configuration entities
* depends on, you can use a sub-module: move the configuration entity to the
* sub-module instead of including it in the main extension, and declare the
* module dependency in the sub-module only.
*
* @see \Drupal\Core\Config\Entity\ConfigEntityInterface::calculateDependencies()
* @see \Drupal\Core\Config\Entity\ConfigEntityInterface::getConfigDependencyName()
* @see \Drupal\Core\Config\Entity\ConfigEntityInterface::onDependencyRemoval()
......
......@@ -316,7 +316,14 @@ public function preSave(EntityStorageInterface $storage) {
public function calculateDependencies() {
// Dependencies should be recalculated on every save. This ensures stale
// dependencies are never saved.
$this->dependencies = array();
if (isset($this->dependencies['enforced'])) {
$dependencies = $this->dependencies['enforced'];
$this->dependencies = $dependencies;
$this->dependencies['enforced'] = $dependencies;
}
else {
$this->dependencies = array();
}
if ($this instanceof EntityWithPluginCollectionInterface) {
// Configuration entities need to depend on the providers of any plugins
// that they store the configuration for.
......
......@@ -36,6 +36,7 @@ class BlockConfigSchemaTest extends KernelTestBase {
'system',
'taxonomy',
'user',
'text',
);
/**
......@@ -61,6 +62,8 @@ protected function setUp() {
$this->typedConfig = \Drupal::service('config.typed');
$this->blockManager = \Drupal::service('plugin.manager.block');
$this->installEntitySchema('block_content');
$this->installEntitySchema('taxonomy_term');
$this->installEntitySchema('node');
}
/**
......
......@@ -62,6 +62,36 @@ function testCommentDefaultFields() {
$this->assertEqual($field->default_value[0]['status'], CommentItemInterface::CLOSED);
}
/**
* Tests that you can remove a comment field.
*/
public function testCommentFieldDelete() {
$this->drupalCreateContentType(array('type' => 'test_node_type'));
$this->container->get('comment.manager')->addDefaultField('node', 'test_node_type');
// We want to test the handling of removing the primary comment field, so we
// ensure there is at least one other comment field attached to a node type
// so that comment_entity_load() runs for nodes.
$this->container->get('comment.manager')->addDefaultField('node', 'test_node_type', 'comment2');
// Create a sample node.
$node = $this->drupalCreateNode(array(
'title' => 'Baloney',
'type' => 'test_node_type',
));
$this->drupalLogin($this->web_user);
$this->drupalGet('node/' . $node->nid->value);
$elements = $this->cssSelect('.field-type-comment');
$this->assertEqual(2, count($elements), 'There are two comment fields on the node.');
// Delete the first comment field.
FieldStorageConfig::loadByName('node', 'comment')->delete();
$this->drupalGet('node/' . $node->nid->value);
$elements = $this->cssSelect('.field-type-comment');
$this->assertEqual(1, count($elements), 'There is one comment field on the node.');
}
/**
* Tests that comment module works when installed after a content module.
*/
......
......@@ -47,8 +47,10 @@ public function testDependencyMangement() {
$entity1 = $storage->create(
array(
'id' => 'entity1',
'test_dependencies' => array(
'module' => array('node', 'config_test')
'dependencies' => array(
'enforced' => array(
'module' => array('node')
)
)
)
);
......@@ -63,15 +65,14 @@ public function testDependencyMangement() {
// Ensure that the provider of the config entity is not actually written to
// the dependencies array.
$raw_config = \Drupal::config('config_test.dynamic.entity1');
$this->assertTrue(array_search('config_test', $raw_config->get('dependencies.module')) === FALSE, 'Module that the provides the configuration entity is not written to the dependencies array as this is implicit.');
$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.');
// Create additional entities to test dependencies on config entities.
$entity2 = $storage->create(array('id' => 'entity2', 'test_dependencies' => array('entity' => array($entity1->getConfigDependencyName()))));
$entity2 = $storage->create(array('id' => 'entity2', 'dependencies' => array('enforced' => array('entity' => array($entity1->getConfigDependencyName())))));
$entity2->save();
$entity3 = $storage->create(array('id' => 'entity3', 'test_dependencies' => array('entity' => array($entity2->getConfigDependencyName()))));
$entity3 = $storage->create(array('id' => 'entity3', 'dependencies' => array('enforced' => array('entity' => array($entity2->getConfigDependencyName())))));
$entity3->save();
$entity4 = $storage->create(array('id' => 'entity4', 'test_dependencies' => array('entity' => array($entity3->getConfigDependencyName()))));
$entity4 = $storage->create(array('id' => 'entity4', 'dependencies' => array('enforced' => array('entity' => array($entity3->getConfigDependencyName())))));
$entity4->save();
// Test getting $entity1's dependencies as configuration dependency objects.
......@@ -100,10 +101,8 @@ public function testDependencyMangement() {
// Test getting node module's dependencies as configuration dependency
// objects after making $entity3 also dependent on node module but $entity1
// no longer depend on node module.
$entity1->test_dependencies = array();
$entity1->save();
$entity3->test_dependencies['module'] = array('node');
$entity3->save();
$entity1->setEnforcedDependencies([])->save();
$entity3->setEnforcedDependencies(['module' => ['node'], 'entity' => [$entity2->getConfigDependencyName()]])->save();
$dependents = $config_manager->findConfigEntityDependents('module', array('node'));
$this->assertFalse(isset($dependents['config_test.dynamic.entity1']), 'config_test.dynamic.entity1 does not have a dependency on the Node module.');
$this->assertFalse(isset($dependents['config_test.dynamic.entity2']), 'config_test.dynamic.entity2 does not have a dependency on the Node module.');
......@@ -113,15 +112,15 @@ public function testDependencyMangement() {
// Create a configuration entity of a different type with the same ID as one
// of the entities already created.
$alt_storage = $this->container->get('entity.manager')->getStorage('config_query_test');
$alt_storage->create(array('id' => 'entity1', 'test_dependencies' => array('entity' => array($entity1->getConfigDependencyName()))))->save();
$alt_storage->create(array('id' => 'entity2', 'test_dependencies' => array('module' => array('views'))))->save();
$alt_storage->create(array('id' => 'entity1', 'dependencies' => array('enforced' => array('entity' => array($entity1->getConfigDependencyName())))))->save();
$alt_storage->create(array('id' => 'entity2', 'dependencies' => array('enforced' => array('module' => array('views')))))->save();
$dependents = $config_manager->findConfigEntityDependentsAsEntities('entity', array($entity1->getConfigDependencyName()));
$dependent_ids = $this->getDependentIds($dependents);
$this->assertFalse(in_array('config_test:entity1', $dependent_ids), 'config_test.dynamic.entity1 does not have a dependency on itself.');
$this->assertTrue(in_array('config_test:entity2', $dependent_ids), 'config_test.dynamic.entity2 has a dependency on config_test.dynamic.entity1.');
$this->assertTrue(in_array('config_test:entity3', $dependent_ids), 'config_test.dynamic.entity3 has a dependency on config_test.dynamic.entity1.');
$this->assertTrue(in_array('config_test:entity3', $dependent_ids), 'config_test.dynamic.entity4 has a dependency on config_test.dynamic.entity1.');
$this->assertTrue(in_array('config_test:entity4', $dependent_ids), 'config_test.dynamic.entity4 has a dependency on config_test.dynamic.entity1.');
$this->assertTrue(in_array('config_query_test:entity1', $dependent_ids), 'config_query_test.dynamic.entity1 has a dependency on config_test.dynamic.entity1.');
$this->assertFalse(in_array('config_query_test:entity2', $dependent_ids), 'config_query_test.dynamic.entity2 does not have a dependency on config_test.dynamic.entity1.');
......@@ -157,8 +156,10 @@ public function testConfigEntityUninstall() {
$entity1 = $storage->create(
array(
'id' => 'entity1',
'test_dependencies' => array(
'module' => array('node', 'config_test')
'dependencies' => array(
'enforced' => array(
'module' => array('node', 'config_test')
),
),
)
);
......@@ -166,8 +167,10 @@ public function testConfigEntityUninstall() {
$entity2 = $storage->create(
array(
'id' => 'entity2',
'test_dependencies' => array(
'entity' => array($entity1->getConfigDependencyName()),
'dependencies' => array(
'enforced' => array(
'entity' => array($entity1->getConfigDependencyName()),
),
),
)
);
......@@ -181,8 +184,10 @@ public function testConfigEntityUninstall() {
$entity1 = $storage->create(
array(
'id' => 'entity1',
'test_dependencies' => array(
'module' => array('node', 'config_test')
'dependencies' => array(
'enforced' => array(
'module' => array('node', 'config_test')
),
),
)
);
......@@ -190,8 +195,10 @@ public function testConfigEntityUninstall() {
$entity2 = $storage->create(
array(
'id' => 'entity2',
'test_dependencies' => array(
'entity' => array($entity1->getConfigDependencyName()),
'dependencies' => array(
'enforced' => array(
'entity' => array($entity1->getConfigDependencyName()),
),
),
)
);
......
......@@ -76,8 +76,16 @@ public function testInstallUninstall() {
// Purge the data.
field_purge_batch(1000);
// Delete any forum terms so it can be uninstalled.
$vid = \Drupal::config('forum.settings')->get('vocabulary');
$terms = entity_load_multiple_by_properties('taxonomy_term', ['vid' => $vid]);
foreach ($terms as $term) {
$term->delete();
}
system_list_reset();
$all_modules = system_rebuild_module_data();
$modules_to_uninstall = array_filter($all_modules, function ($module) {
// Filter required and not enabled modules.
if (!empty($module->info['required']) || $module->status == FALSE) {
......
......@@ -58,7 +58,6 @@ function testImport() {
'label' => 'New',
'weight' => 0,
'style' => '',
'test_dependencies' => array(),
'protected_property' => '',
);
$staging->write($dynamic_name, $original_dynamic_data);
......@@ -367,7 +366,6 @@ function testImportErrorLog() {
'label' => 'Primary',
'weight' => 0,
'style' => NULL,
'test_dependencies' => array(),
'protected_property' => null,
);
$staging->write($name_primary, $values_primary);
......@@ -383,7 +381,6 @@ function testImportErrorLog() {
'label' => 'Secondary Sync',
'weight' => 0,
'style' => NULL,
'test_dependencies' => array(),
'protected_property' => null,
);
$staging->write($name_secondary, $values_secondary);
......
......@@ -172,7 +172,6 @@ function testNew() {
'label' => 'New',
'weight' => 0,
'style' => '',
'test_dependencies' => array(),
'protected_property' => '',
);
$staging->write($dynamic_name, $original_dynamic_data);
......
......@@ -15,9 +15,6 @@ config_test_dynamic:
style:
type: string
label: 'style'
test_dependencies:
type: config_dependencies
label: 'Configuration dependencies'
protected_property:
type: string
label: 'Protected property'
......
......@@ -71,13 +71,6 @@ class ConfigTest extends ConfigEntityBase implements ConfigTestInterface {
*/
public $style;
/**
* Test dependencies.
*
* @var array;
*/
public $test_dependencies = array();
/**
* A protected property of the configuration entity.
*
......@@ -124,18 +117,6 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
}
}
/**
* {@inheritdoc}
*/
public function calculateDependencies() {
parent::calculateDependencies();
foreach ($this->test_dependencies as $type => $deps) {
foreach ($deps as $dep) {
$this->addDependency($type, $dep);
}
}
}
/**
* {@inheritdoc}
*/
......@@ -144,10 +125,10 @@ public function onDependencyRemoval(array $dependencies) {
$fix_deps = \Drupal::state()->get('config_test.fix_dependencies', array());
foreach ($dependencies['entity'] as $entity) {
if (in_array($entity->getConfigDependencyName(), $fix_deps)) {
$key = array_search($entity->getConfigDependencyName(), $this->test_dependencies['entity']);
$key = array_search($entity->getConfigDependencyName(), $this->dependencies['enforced']['entity']);
if ($key !== FALSE) {
$changed = TRUE;
unset($this->test_dependencies['entity'][$key]);
unset($this->dependencies['enforced']['entity'][$key]);
}
}
}
......@@ -156,4 +137,19 @@ public function onDependencyRemoval(array $dependencies) {
}
}
/**
* Sets the enforced dependencies.
*
* @param array $dependencies
* A config dependency array.
*
* @return $this
*
* @see \Drupal\Core\Config\Entity\ConfigDependencyManager
*/
public function setEnforcedDependencies(array $dependencies) {
$this->dependencies['enforced'] = $dependencies;
return $this;
}
}
type: forum
langcode: en
status: true
dependencies:
enforced:
module:
- forum
name: 'Forum topic'
type: forum
description: 'A <em>forum topic</em> starts a new discussion thread within a forum.'
help: ''
new_revision: false
display_submitted: true
preview_mode: 1
status: true
langcode: en
display_submitted: true
......@@ -11,8 +11,11 @@
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Url;
use Drupal\Component\Utility\String;
use Drupal\Core\Extension\Extension;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\taxonomy\Entity\Vocabulary;
use Symfony\Component\Routing\Exception\RouteNotFoundException;
/**
* Implements hook_help().
......@@ -691,3 +694,63 @@ function template_preprocess_forum_submitted(&$variables) {
}
$variables['time'] = isset($variables['topic']->created) ? \Drupal::service('date.formatter')->formatInterval(REQUEST_TIME - $variables['topic']->created) : '';
}
/**
* Implements hook_system_info_alter().
*
* Prevents forum module from being uninstalled whilst any forum nodes exist
* or there are any terms in the forum vocabulary.
*/
function forum_system_info_alter(&$info, Extension $file, $type) {
// It is not safe use the entity query service during maintenance mode.
if ($type == 'module' && !defined('MAINTENANCE_MODE') && $file->getName() == 'forum') {
$factory = \Drupal::service('entity.query');
$nodes = $factory->get('node')
->condition('type', 'forum')
->accessCheck(FALSE)
->range(0, 1)
->execute();
if (!empty($nodes)) {
$info['required'] = TRUE;
$info['explanation'] = t('To uninstall Forum first delete all <em>Forum</em> content.');
}
$vid = \Drupal::config('forum.settings')->get('vocabulary');
$terms = $factory->get('taxonomy_term')
->condition('vid', $vid)
->accessCheck(FALSE)
->range(0, 1)
->execute();
if (!empty($terms)) {
$vocabulary = Vocabulary::load($vid);
$info['required'] = TRUE;
try {
if (!empty($info['explanation'])) {
if ($vocabulary->access('view')) {
$info['explanation'] = t('To uninstall Forum first delete all <em>Forum</em> content and <a href="!url">%vocabulary</a> terms.', [
'%vocabulary' => $vocabulary->label(),
'!url' => $vocabulary->url('overview-form'),
]);
}
else {
$info['explanation'] = t('To uninstall Forum first delete all <em>Forum</em> content and %vocabulary terms.', [
'%vocabulary' => $vocabulary->label()
]);
}
}
else {
$info['explanation'] = t('To uninstall Forum first delete all <a href="!url">%vocabulary</a> terms.', [
'%vocabulary' => $vocabulary->label(),
'!url' => $vocabulary->url('overview-form'),
]);
}
}
catch (RouteNotFoundException $e) {
// Route rebuilding might not have occurred before this hook is fired.
// Just use an explanation without a link for the time being.
$info['explanation'] = t('To uninstall Forum first delete all <em>Forum</em> content and %vocabulary terms.', [
'%vocabulary' => $vocabulary->label()
]);
}
}
}
}
......@@ -9,7 +9,12 @@
use Drupal\comment\CommentInterface;
use Drupal\comment\Plugin\Field\FieldType\CommentItemInterface;
use Drupal\Component\Utility\String;
use Drupal\Core\DrupalKernel;
use Drupal\Core\Session\UserSession;
use Drupal\Core\Site\Settings;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\node\Entity\NodeType;
use Drupal\simpletest\WebTestBase;
/**
......@@ -29,7 +34,8 @@ class ForumUninstallTest extends WebTestBase {
/**
* Tests if forum module uninstallation properly deletes the field.
*/
function testForumUninstallWithField() {
public function testForumUninstallWithField() {
$this->drupalLogin($this->drupalCreateUser(['administer taxonomy', 'administer nodes', 'administer modules', 'delete any forum content', 'administer content types']));
// Ensure that the field exists before uninstallation.
$field_storage = FieldStorageConfig::loadByName('node', 'taxonomy_forums');
$this->assertNotNull($field_storage, 'The taxonomy_forums field storage exists.');
......@@ -65,27 +71,69 @@ function testForumUninstallWithField() {
));
$comment->save();
// Uninstall the forum module which should trigger field deletion.
$this->container->get('module_handler')->uninstall(array('forum'));
// We want to test the handling of removing the forum comment field, so we
// ensure there is at least one other comment field attached to a node type
// so that comment_entity_load() runs for nodes.
\Drupal::service('comment.manager')->addDefaultField('node', 'forum', 'another_comment_field', CommentItemInterface::OPEN, 'another_comment_field');
$this->drupalGet('node/' . $node->nid->value);
$this->assertResponse(200);
// Attempt to uninstall forum.
$this->drupalGet('admin/modules/uninstall');
// Assert forum is required.
$this->assertNoFieldByName('uninstall[forum]');
$this->drupalGet('admin/modules');