diff --git a/core/core.services.yml b/core/core.services.yml index 86c3dad4eb54b625e6f3bc8a3ebef916726219f4..e5db5d5f0d0c97a760c2942984a943dae7f95505 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -418,6 +418,12 @@ services: - { name: module_install.uninstall_validator } arguments: ['@entity.manager', '@string_translation'] lazy: true + required_module_uninstall_validator: + class: Drupal\Core\Extension\RequiredModuleUninstallValidator + tags: + - { name: module_install.uninstall_validator } + arguments: ['@string_translation'] + lazy: true theme_handler: class: Drupal\Core\Extension\ThemeHandler arguments: ['@app.root', '@config.factory', '@module_handler', '@state', '@info_parser'] diff --git a/core/lib/Drupal/Core/Entity/ContentUninstallValidator.php b/core/lib/Drupal/Core/Entity/ContentUninstallValidator.php index cc139e02293f7df4a74fffb134c8181682b904ab..e791d8a270efd62172fb3178cc014393ae099a81 100644 --- a/core/lib/Drupal/Core/Entity/ContentUninstallValidator.php +++ b/core/lib/Drupal/Core/Entity/ContentUninstallValidator.php @@ -17,6 +17,13 @@ class ContentUninstallValidator implements ModuleUninstallValidatorInterface { use StringTranslationTrait; + /** + * The entity manager. + * + * @var \Drupal\Core\Entity\EntityManagerInterface + */ + protected $entityManager; + /** * Constructs a new ContentUninstallValidator. * diff --git a/core/lib/Drupal/Core/Extension/ModuleInstaller.php b/core/lib/Drupal/Core/Extension/ModuleInstaller.php index 9c930001917b088a89bc875d27008550bdd1474f..d99c9ddc1e5a7e58da98b5e5232df340f694c7dc 100644 --- a/core/lib/Drupal/Core/Extension/ModuleInstaller.php +++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php @@ -334,7 +334,7 @@ public function uninstall(array $module_list, $uninstall_dependents = TRUE) { $reason_message[] = implode(', ', $reason); } throw new ModuleUninstallValidatorException(format_string('The following reasons prevents the modules from being uninstalled: @reasons', array( - '@reasons' => implode(', ', $reason_message), + '@reasons' => implode('; ', $reason_message), ))); } // Set the actual module weights. diff --git a/core/lib/Drupal/Core/Extension/ModuleUninstallValidatorInterface.php b/core/lib/Drupal/Core/Extension/ModuleUninstallValidatorInterface.php index 5e62b0ef7fd8d12904bf40c58e27e46486e2c0d0..eeabca51ce1a39cb70a40b9ddf8e1acd7bcd4976 100644 --- a/core/lib/Drupal/Core/Extension/ModuleUninstallValidatorInterface.php +++ b/core/lib/Drupal/Core/Extension/ModuleUninstallValidatorInterface.php @@ -20,6 +20,10 @@ interface ModuleUninstallValidatorInterface { * * @return string[] * An array of reasons the module can not be uninstalled, empty if it can. + * Each reason should not end with any punctuation since multiple reasons + * can be displayed together. + * + * @see theme_system_modules_uninstall() */ public function validate($module); } diff --git a/core/lib/Drupal/Core/Extension/RequiredModuleUninstallValidator.php b/core/lib/Drupal/Core/Extension/RequiredModuleUninstallValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..a285e00a51d86b056a5f0084dd8857feceb0ac6c --- /dev/null +++ b/core/lib/Drupal/Core/Extension/RequiredModuleUninstallValidator.php @@ -0,0 +1,56 @@ +stringTranslation = $string_translation; + } + + /** + * {@inheritdoc} + */ + public function validate($module) { + $reasons = []; + $module_info = $this->getModuleInfoByModule($module); + if (!empty($module_info['required'])) { + $reasons[] = $this->t('The @module module is required', ['@module' => $module_info['name']]); + } + return $reasons; + } + + /** + * Returns the module info for a specific module. + * + * @param string $module + * The name of the module. + * + * @return array + * The module info, or NULL if that module does not exist. + */ + protected function getModuleInfoByModule($module) { + $modules = system_rebuild_module_data(); + return isset($modules[$module]->info) ? $modules[$module]->info : []; + } + +} diff --git a/core/lib/Drupal/Core/Field/FieldModuleUninstallValidator.php b/core/lib/Drupal/Core/Field/FieldModuleUninstallValidator.php index b54579cd85745d87297ed642c7cdee6e26a5fee2..b2e474d082410bf3d5d235a1749993c47ea6689b 100644 --- a/core/lib/Drupal/Core/Field/FieldModuleUninstallValidator.php +++ b/core/lib/Drupal/Core/Field/FieldModuleUninstallValidator.php @@ -53,7 +53,7 @@ public function validate($module_name) { if ($storage_definition->getProvider() == $module_name) { $storage = $this->entityManager->getStorage($entity_type_id); if ($storage instanceof FieldableEntityStorageInterface && $storage->countFieldData($storage_definition, TRUE)) { - $reasons[] = $this->t('There is data for the field @field-name on entity type @entity_type.', array( + $reasons[] = $this->t('There is data for the field @field-name on entity type @entity_type', array( '@field-name' => $storage_definition->getName(), '@entity_type' => $entity_type->getLabel(), )); diff --git a/core/modules/book/book.module b/core/modules/book/book.module index 8e0d0496f67e06672e827d17e234f291e8632a3d..f7ee374e0b9d6eae28f6bbf087ef3ef2402d1507 100644 --- a/core/modules/book/book.module +++ b/core/modules/book/book.module @@ -19,7 +19,6 @@ use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Entity\Display\EntityViewDisplayInterface; use Drupal\Core\Template\Attribute; -use Drupal\Core\Extension\Extension; /** * Implements hook_help(). @@ -536,33 +535,3 @@ function book_node_type_update(NodeTypeInterface $type) { $config->save(); } } - -/** - * Implements hook_system_info_alter(). - * - * Prevents book module from being uninstalled whilst any book nodes exist or - * there are any book outline stored. - */ -function book_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() == 'book') { - if (\Drupal::service('book.outline_storage')->hasBooks()) { - $info['required'] = TRUE; - $info['explanation'] = t('To uninstall Book, delete all content that is part of a book.'); - } - else { - // The book node type is provided by the Book module. Prevent uninstall if - // there are any nodes of that type. - $factory = \Drupal::service('entity.query'); - $nodes = $factory->get('node') - ->condition('type', 'book') - ->accessCheck(FALSE) - ->range(0, 1) - ->execute(); - if (!empty($nodes)) { - $info['required'] = TRUE; - $info['explanation'] = t('To uninstall Book, delete all content that has the Book content type.'); - } - } - } -} diff --git a/core/modules/book/book.services.yml b/core/modules/book/book.services.yml index 6d024f73bc9c1d90cbc36823fd6a5de693ae8783..3b45dae19c53ffa0a133955685076ad6b67d4a91 100644 --- a/core/modules/book/book.services.yml +++ b/core/modules/book/book.services.yml @@ -28,3 +28,10 @@ services: arguments: ['@request_stack'] tags: - { name: cache.context} + + book.uninstall_validator: + class: Drupal\book\BookUninstallValidator + tags: + - { name: module_install.uninstall_validator } + arguments: ['@book.outline_storage', '@entity.query', '@string_translation'] + lazy: true diff --git a/core/modules/book/src/BookUninstallValidator.php b/core/modules/book/src/BookUninstallValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..84de96d214050dee8ec19a19708114bae4145d99 --- /dev/null +++ b/core/modules/book/src/BookUninstallValidator.php @@ -0,0 +1,98 @@ +bookOutlineStorage = $book_outline_storage; + $this->entityQuery = $query_factory->get('node'); + $this->stringTranslation = $string_translation; + } + + /** + * {@inheritdoc} + */ + public function validate($module) { + $reasons = []; + if ($module == 'book') { + if ($this->hasBookOutlines()) { + $reasons[] = $this->t('To uninstall Book, delete all content that is part of a book'); + } + else { + // The book node type is provided by the Book module. Prevent uninstall + // if there are any nodes of that type. + if ($this->hasBookNodes()) { + $reasons[] = $this->t('To uninstall Book, delete all content that has the Book content type'); + } + } + } + return $reasons; + } + + /** + * Checks if there are any books in an outline. + * + * @return bool + * TRUE if there are books, FALSE if not. + */ + protected function hasBookOutlines() { + return $this->bookOutlineStorage->hasBooks(); + } + + /** + * Determines if there is any book nodes or not. + * + * @return bool + * TRUE if there are book nodes, FALSE otherwise. + */ + protected function hasBookNodes() { + $nodes = $this->entityQuery + ->condition('type', 'book') + ->accessCheck(FALSE) + ->range(0, 1) + ->execute(); + return !empty($nodes); + } + +} diff --git a/core/modules/book/src/Tests/BookUninstallTest.php b/core/modules/book/src/Tests/BookUninstallTest.php index 317a16ae1e36ae811f056df3bc32e1638299b281..bafaab7ee7ced82053910106841cb72484d6a377 100644 --- a/core/modules/book/src/Tests/BookUninstallTest.php +++ b/core/modules/book/src/Tests/BookUninstallTest.php @@ -44,8 +44,8 @@ protected function setUp() { */ public function testBookUninstall() { // No nodes exist. - $module_data = _system_rebuild_module_data(); - $this->assertFalse(isset($module_data['book']->info['required']), 'The book module is not required.'); + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual([], $validation_reasons, 'The book module is not required.'); $content_type = NodeType::create(array( 'type' => $this->randomMachineName(), @@ -62,9 +62,8 @@ public function testBookUninstall() { $node->save(); // One node in a book but not of type book. - $module_data = _system_rebuild_module_data(); - $this->assertTrue($module_data['book']->info['required'], 'The book module is required.'); - $this->assertEqual($module_data['book']->info['explanation'], t('To uninstall Book, delete all content that is part of a book.')); + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual(['To uninstall Book, delete all content that is part of a book'], $validation_reasons['book']); $book_node = Node::create(array('type' => 'book')); $book_node->book['bid'] = FALSE; @@ -72,15 +71,13 @@ public function testBookUninstall() { // Two nodes, one in a book but not of type book and one book node (which is // not in a book). - $module_data = _system_rebuild_module_data(); - $this->assertTrue($module_data['book']->info['required'], 'The book module is required.'); - $this->assertEqual($module_data['book']->info['explanation'], t('To uninstall Book, delete all content that is part of a book.')); + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual(['To uninstall Book, delete all content that is part of a book'], $validation_reasons['book']); $node->delete(); // One node of type book but not actually part of a book. - $module_data = _system_rebuild_module_data(); - $this->assertTrue($module_data['book']->info['required'], 'The book module is required.'); - $this->assertEqual($module_data['book']->info['explanation'], t('To uninstall Book, delete all content that has the Book content type.')); + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual(['To uninstall Book, delete all content that has the Book content type'], $validation_reasons['book']); $book_node->delete(); // No nodes exist therefore the book module is not required. @@ -91,8 +88,8 @@ public function testBookUninstall() { $node->save(); // One node exists but is not part of a book therefore the book module is // not required. - $module_data = _system_rebuild_module_data(); - $this->assertFalse(isset($module_data['book']->info['required']), 'The book module is not required.'); + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual([], $validation_reasons, 'The book module is not required.'); // Uninstall the Book module and check the node type is deleted. \Drupal::service('module_installer')->uninstall(array('book')); diff --git a/core/modules/book/tests/src/Unit/BookUninstallValidatorTest.php b/core/modules/book/tests/src/Unit/BookUninstallValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..d3748c263d0d435ced5cf3277267af99f17931ea --- /dev/null +++ b/core/modules/book/tests/src/Unit/BookUninstallValidatorTest.php @@ -0,0 +1,100 @@ +bookUninstallValidator = $this->getMockBuilder('Drupal\book\BookUninstallValidator') + ->disableOriginalConstructor() + ->setMethods(['hasBookOutlines', 'hasBookNodes']) + ->getMock(); + $this->bookUninstallValidator->setStringTranslation($this->getStringTranslationStub()); + } + + /** + * @covers ::validate + */ + public function testValidateNotBook() { + $this->bookUninstallValidator->expects($this->never()) + ->method('hasBookOutlines'); + $this->bookUninstallValidator->expects($this->never()) + ->method('hasBookNodes'); + + $module = 'not_book'; + $expected = []; + $reasons = $this->bookUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateEntityQueryWithoutResults() { + $this->bookUninstallValidator->expects($this->once()) + ->method('hasBookOutlines') + ->willReturn(FALSE); + $this->bookUninstallValidator->expects($this->once()) + ->method('hasBookNodes') + ->willReturn(FALSE); + + $module = 'book'; + $expected = []; + $reasons = $this->bookUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateEntityQueryWithResults() { + $this->bookUninstallValidator->expects($this->once()) + ->method('hasBookOutlines') + ->willReturn(FALSE); + $this->bookUninstallValidator->expects($this->once()) + ->method('hasBookNodes') + ->willReturn(TRUE); + + $module = 'book'; + $expected = ['To uninstall Book, delete all content that has the Book content type']; + $reasons = $this->bookUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateOutlineStorage() { + $this->bookUninstallValidator->expects($this->once()) + ->method('hasBookOutlines') + ->willReturn(TRUE); + $this->bookUninstallValidator->expects($this->never()) + ->method('hasBookNodes'); + + $module = 'book'; + $expected = ['To uninstall Book, delete all content that is part of a book']; + $reasons = $this->bookUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + +} diff --git a/core/modules/comment/src/Tests/CommentUninstallTest.php b/core/modules/comment/src/Tests/CommentUninstallTest.php index 1cca31add7a9c4350118e92cf51542ec4bd0c745..fa97f740d5b56f3dbc58d71b3b18aa0d7dd85f28 100644 --- a/core/modules/comment/src/Tests/CommentUninstallTest.php +++ b/core/modules/comment/src/Tests/CommentUninstallTest.php @@ -8,6 +8,7 @@ namespace Drupal\comment\Tests; use Drupal\field\Entity\FieldStorageConfig; +use Drupal\Core\Extension\ModuleUninstallValidatorException; use Drupal\simpletest\WebTestBase; /** @@ -36,19 +37,23 @@ protected function setUp() { } /** - * Tests if comment module uninstallation properly deletes the field. + * Tests if comment module uninstallation fails if the field exists. + * + * @throws \Drupal\Core\Extension\ModuleUninstallValidatorException */ function testCommentUninstallWithField() { // Ensure that the field exists before uninstallation. $field_storage = FieldStorageConfig::loadByName('comment', 'comment_body'); $this->assertNotNull($field_storage, 'The comment_body field exists.'); - // Uninstall the comment module which should trigger field deletion. - $this->container->get('module_installer')->uninstall(array('comment')); - - // Check that the field is now deleted. - $field_storage = FieldStorageConfig::loadByName('comment', 'comment_body'); - $this->assertNull($field_storage, 'The comment_body field has been deleted.'); + // Uninstall the comment module which should trigger an exception. + try { + $this->container->get('module_installer')->uninstall(array('comment')); + $this->fail("Expected an exception when uninstall was attempted."); + } + catch (ModuleUninstallValidatorException $e) { + $this->pass("Caught an exception when uninstall was attempted."); + } } @@ -65,6 +70,16 @@ function testCommentUninstallWithoutField() { $field_storage = FieldStorageConfig::loadByName('comment', 'comment_body'); $this->assertNull($field_storage, 'The comment_body field has been deleted.'); + // Manually delete the comment field on the node before module uninstallation. + $field_storage = FieldStorageConfig::loadByName('node', 'comment'); + $this->assertNotNull($field_storage, 'The comment field exists.'); + $field_storage->delete(); + + // Check that the field is now deleted. + $field_storage = FieldStorageConfig::loadByName('node', 'comment'); + $this->assertNull($field_storage, 'The comment field has been deleted.'); + + field_purge_batch(10); // Ensure that uninstallation succeeds even if the field has already been // deleted manually beforehand. $this->container->get('module_installer')->uninstall(array('comment')); diff --git a/core/modules/config/src/Tests/ConfigImportAllTest.php b/core/modules/config/src/Tests/ConfigImportAllTest.php index 7645847be16c008f7e8fab08345f9f200b36dfb9..6e1aaea16e91fbfcfe6a885d341dda3db8c4e70c 100644 --- a/core/modules/config/src/Tests/ConfigImportAllTest.php +++ b/core/modules/config/src/Tests/ConfigImportAllTest.php @@ -8,8 +8,10 @@ namespace Drupal\config\Tests; use Drupal\Core\Config\StorageComparer; +use Drupal\filter\Entity\FilterFormat; use Drupal\system\Tests\Module\ModuleTestBase; use Drupal\shortcut\Entity\Shortcut; +use Drupal\taxonomy\Entity\Term; /** * Tests the largest configuration import possible with all available modules. @@ -83,12 +85,13 @@ public function testInstallUninstall() { // Purge the data. field_purge_batch(1000); - // Delete any forum terms so it can be uninstalled. - $vid = $this->config('forum.settings')->get('vocabulary'); - $terms = entity_load_multiple_by_properties('taxonomy_term', ['vid' => $vid]); - foreach ($terms as $term) { - $term->delete(); - } + // Delete all terms. + $terms = Term::loadMultiple(); + entity_delete_multiple('taxonomy_term', array_keys($terms)); + + // Delete all filter formats. + $filters = FilterFormat::loadMultiple(); + entity_delete_multiple('filter_format', array_keys($filters)); // Delete any shortcuts so the shortcut module can be uninstalled. $shortcuts = Shortcut::loadMultiple(); @@ -97,7 +100,11 @@ public function testInstallUninstall() { system_list_reset(); $all_modules = system_rebuild_module_data(); - $modules_to_uninstall = array_filter($all_modules, function ($module) { + // Ensure that only core required modules and the install profile can not be uninstalled. + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(array_keys($all_modules)); + $this->assertEqual(['standard', 'system', 'user'], array_keys($validation_reasons)); + + $modules_to_uninstall = array_filter($all_modules, function ($module) use ($validation_reasons) { // Filter required and not enabled modules. if (!empty($module->info['required']) || $module->status == FALSE) { return FALSE; @@ -109,6 +116,8 @@ public function testInstallUninstall() { unset($modules_to_uninstall['config']); $this->assertTrue(isset($modules_to_uninstall['comment']), 'The comment module will be disabled'); + $this->assertTrue(isset($modules_to_uninstall['file']), 'The File module will be disabled'); + $this->assertTrue(isset($modules_to_uninstall['editor']), 'The Editor module will be disabled'); // Uninstall all modules that can be uninstalled. \Drupal::service('module_installer')->uninstall(array_keys($modules_to_uninstall)); diff --git a/core/modules/field/field.module b/core/modules/field/field.module index 1ebea04617bc5944fa55c9362295f52dfe2ecbfc..88ccff92183063ccb8619440effe9ab5e4fff0cc 100644 --- a/core/modules/field/field.module +++ b/core/modules/field/field.module @@ -7,7 +7,6 @@ use Drupal\Core\Config\ConfigImporter; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Entity\DynamicallyFieldableEntityStorageInterface; -use Drupal\Core\Extension\Extension; use Drupal\field\Entity\FieldConfig; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Routing\RouteMatchInterface; @@ -145,40 +144,6 @@ function field_cron() { field_purge_batch($limit); } -/** - * Implements hook_system_info_alter(). - * - * Goes through a list of all modules that provide a field type and makes them - * required if there are any active fields of that type. - */ -function field_system_info_alter(&$info, Extension $file, $type) { - // It is not safe to call entity_load_multiple_by_properties() during - // maintenance mode. - if ($type == 'module' && !defined('MAINTENANCE_MODE')) { - $field_storages = entity_load_multiple_by_properties('field_storage_config', array('module' => $file->getName(), 'include_deleted' => TRUE)); - if ($field_storages) { - $info['required'] = TRUE; - - // Provide an explanation message (only mention pending deletions if there - // remains no actual, non-deleted fields) - $non_deleted = FALSE; - foreach ($field_storages as $field_storage) { - if (!$field_storage->isDeleted()) { - $non_deleted = TRUE; - break; - } - } - if ($non_deleted) { - $explanation = t('Fields type(s) in use'); - } - else { - $explanation = t('Fields pending deletion'); - } - $info['explanation'] = $explanation; - } - } -} - /** * Implements hook_entity_field_storage_info(). */ diff --git a/core/modules/field/field.services.yml b/core/modules/field/field.services.yml new file mode 100644 index 0000000000000000000000000000000000000000..6c2187a17036ce833b7e60e0323b7b9b698a0a42 --- /dev/null +++ b/core/modules/field/field.services.yml @@ -0,0 +1,7 @@ +services: + field.uninstall_validator: + class: Drupal\field\FieldUninstallValidator + tags: + - { name: module_install.uninstall_validator } + arguments: ['@entity.manager', '@string_translation'] + lazy: true diff --git a/core/modules/field/src/FieldUninstallValidator.php b/core/modules/field/src/FieldUninstallValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..3302b046972b9399a9fe6c232990bd90f219b80f --- /dev/null +++ b/core/modules/field/src/FieldUninstallValidator.php @@ -0,0 +1,80 @@ +fieldStorageConfigStorage = $entity_manager->getStorage('field_storage_config'); + $this->stringTranslation = $string_translation; + } + + /** + * {@inheritdoc} + */ + public function validate($module) { + $reasons = []; + if ($field_storages = $this->getFieldStoragesByModule($module)) { + // Provide an explanation message (only mention pending deletions if there + // remain no actual, non-deleted fields.) + $non_deleted = FALSE; + foreach ($field_storages as $field_storage) { + if (!$field_storage->isDeleted()) { + $non_deleted = TRUE; + break; + } + } + if ($non_deleted) { + $reasons[] = $this->t('Fields type(s) in use'); + } + else { + $reasons[] = $this->t('Fields pending deletion'); + } + } + return $reasons; + } + + /** + * Returns all field storages for a specified module. + * + * @param string $module + * The module to filter field storages by. + * + * @return \Drupal\field\FieldStorageConfigInterface[] + * An array of field storages for a specified module. + */ + protected function getFieldStoragesByModule($module) { + return $this->fieldStorageConfigStorage->loadByProperties(['module' => $module, 'include_deleted' => TRUE]); + } + +} diff --git a/core/modules/field/src/Tests/reEnableModuleFieldTest.php b/core/modules/field/src/Tests/reEnableModuleFieldTest.php index cbbf386d87ca369e7eb45e9664c8e60e8524651b..42fcc2b8e7611130b119977241e25014c391e02e 100644 --- a/core/modules/field/src/Tests/reEnableModuleFieldTest.php +++ b/core/modules/field/src/Tests/reEnableModuleFieldTest.php @@ -92,10 +92,10 @@ function testReEnabledField() { // for it's fields. $admin_user = $this->drupalCreateUser(array('access administration pages', 'administer modules')); $this->drupalLogin($admin_user); - $this->drupalGet('admin/modules'); + $this->drupalGet('admin/modules/uninstall'); $this->assertText('Fields type(s) in use'); $field_storage->delete(); - $this->drupalGet('admin/modules'); + $this->drupalGet('admin/modules/uninstall'); $this->assertText('Fields pending deletion'); $this->cronRun(); $this->assertNoText('Fields type(s) in use'); diff --git a/core/modules/field/tests/src/Unit/FieldUninstallValidatorTest.php b/core/modules/field/tests/src/Unit/FieldUninstallValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..ea9e0c89f6843a657c894ab22e2837b02e07e0e0 --- /dev/null +++ b/core/modules/field/tests/src/Unit/FieldUninstallValidatorTest.php @@ -0,0 +1,89 @@ +fieldUninstallValidator = $this->getMockBuilder('Drupal\field\FieldUninstallValidator') + ->disableOriginalConstructor() + ->setMethods(['getFieldStoragesByModule']) + ->getMock(); + $this->fieldUninstallValidator->setStringTranslation($this->getStringTranslationStub()); + } + + /** + * @covers ::validate + */ + public function testValidateNoStorages() { + $this->fieldUninstallValidator->expects($this->once()) + ->method('getFieldStoragesByModule') + ->willReturn([]); + + $module = $this->randomMachineName(); + $expected = []; + $reasons = $this->fieldUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateDeleted() { + $field_storage = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig') + ->disableOriginalConstructor() + ->getMock(); + $field_storage->expects($this->once()) + ->method('isDeleted') + ->willReturn(TRUE); + $this->fieldUninstallValidator->expects($this->once()) + ->method('getFieldStoragesByModule') + ->willReturn([$field_storage]); + + $module = $this->randomMachineName(); + $expected = ['Fields pending deletion']; + $reasons = $this->fieldUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateNoDeleted() { + $field_storage = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig') + ->disableOriginalConstructor() + ->getMock(); + $field_storage->expects($this->once()) + ->method('isDeleted') + ->willReturn(FALSE); + $this->fieldUninstallValidator->expects($this->once()) + ->method('getFieldStoragesByModule') + ->willReturn([$field_storage]); + + $module = $this->randomMachineName(); + $expected = ['Fields type(s) in use']; + $reasons = $this->fieldUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + +} diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index afdaadf4e4198d8b0b4f0e6cfedaea9514975b97..400d6cc0a46f14dc6935073572231f42dfe382e8 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -10,7 +10,6 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Xss; use Drupal\Core\Cache\Cache; -use Drupal\Core\Extension\Extension; use Drupal\Core\Render\Element; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; @@ -83,44 +82,6 @@ function filter_theme() { ); } -/** - * Implements hook_system_info_alter(). - * - * Prevents uninstallation of modules that provide filter plugins that are being - * used in a filter format. - */ -function filter_system_info_alter(&$info, Extension $file, $type) { - // It is not safe to call filter_formats() during maintenance mode. - if ($type == 'module' && !defined('MAINTENANCE_MODE')) { - // Get filter plugins supplied by this module. - $filter_plugins = array_filter(\Drupal::service('plugin.manager.filter')->getDefinitions(), function ($definition) use ($file) { - return $definition['provider'] == $file->getName(); - }); - if (!empty($filter_plugins)) { - $used_in = []; - // Find out if any filter formats have the plugin enabled. - foreach (filter_formats() as $filter_format) { - $filters = $filter_format->filters(); - // Typically, all formats will contain settings for all filter plugins, - // even if they are disabled. However, if a module which provides filter - // plugins is being enabled right now, that won't be the case, so we - // still check to see if this format has this filter before we check - // the filter status. - foreach ($filter_plugins as $filter_plugin) { - if ($filters->has($filter_plugin['id']) && $filters->get($filter_plugin['id'])->status) { - $used_in[] = $filter_format->label(); - $info['required'] = TRUE; - break; - } - } - } - if (!empty($used_in)) { - $info['explanation'] = t('Provides a filter plugin that is in use in the following filter formats: %formats', array('%formats' => implode(', ', $used_in))); - } - } - } -} - /** * Retrieves a list of enabled text formats, ordered by weight. * diff --git a/core/modules/filter/filter.services.yml b/core/modules/filter/filter.services.yml index 406161a3cb82334a7b9a257a764398b16bf03ee9..996304a70fb56f3cbd2b42c279e72695abe57f0e 100644 --- a/core/modules/filter/filter.services.yml +++ b/core/modules/filter/filter.services.yml @@ -2,3 +2,10 @@ services: plugin.manager.filter: class: Drupal\filter\FilterPluginManager parent: default_plugin_manager + + filter.uninstall_validator: + class: Drupal\filter\FilterUninstallValidator + tags: + - { name: module_install.uninstall_validator } + arguments: ['@plugin.manager.filter', '@entity.manager', '@string_translation'] + lazy: true diff --git a/core/modules/filter/src/FilterUninstallValidator.php b/core/modules/filter/src/FilterUninstallValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..74e04b0cc307a2a96bc455fac2e41724a615182f --- /dev/null +++ b/core/modules/filter/src/FilterUninstallValidator.php @@ -0,0 +1,102 @@ +filterManager = $filter_manager; + $this->filterStorage = $entity_manager->getStorage('filter_format'); + $this->stringTranslation = $string_translation; + } + + /** + * {@inheritdoc} + */ + public function validate($module) { + $reasons = []; + // Get filter plugins supplied by this module. + if ($filter_plugins = $this->getFilterDefinitionsByProvider($module)) { + $used_in = []; + // Find out if any filter formats have the plugin enabled. + foreach ($this->getEnabledFilterFormats() as $filter_format) { + $filters = $filter_format->filters(); + foreach ($filter_plugins as $filter_plugin) { + if ($filters->has($filter_plugin['id']) && $filters->get($filter_plugin['id'])->status) { + $used_in[] = $filter_format->label(); + break; + } + } + } + if (!empty($used_in)) { + $reasons[] = $this->t('Provides a filter plugin that is in use in the following filter formats: %formats', ['%formats' => implode(', ', $used_in)]); + } + } + return $reasons; + } + + /** + * Returns all filter definitions that are provided by the specified provider. + * + * @param string $provider + * The provider of the filters. + * + * @return array + * The filter definitions for the specified provider. + */ + protected function getFilterDefinitionsByProvider($provider) { + return array_filter($this->filterManager->getDefinitions(), function ($definition) use ($provider) { + return $definition['provider'] == $provider; + }); + } + + /** + * Returns all enabled filter formats. + * + * @return \Drupal\filter\FilterFormatInterface[] + */ + protected function getEnabledFilterFormats() { + return $this->filterStorage->loadByProperties(['status' => TRUE]); + } + +} diff --git a/core/modules/filter/src/Tests/FilterAPITest.php b/core/modules/filter/src/Tests/FilterAPITest.php index c0bf1d58d18af89ca338122e964935b651558171..ff23d39bb78fd29b0d68057f8cbac3ffe8c2afc0 100644 --- a/core/modules/filter/src/Tests/FilterAPITest.php +++ b/core/modules/filter/src/Tests/FilterAPITest.php @@ -417,18 +417,6 @@ public function testDependencyRemoval() { $this->installSchema('user', array('users_data')); $filter_format = \Drupal\filter\Entity\FilterFormat::load('filtered_html'); - // Enable the filter_test_restrict_tags_and_attributes filter plugin on the - // filtered_html filter format. - $filter_config = [ - 'weight' => 10, - 'status' => 1, - ]; - $filter_format->setFilterConfig('filter_test_restrict_tags_and_attributes', $filter_config)->save(); - - $module_data = _system_rebuild_module_data(); - $this->assertTrue($module_data['filter_test']->info['required'], 'The filter_test module is required.'); - $this->assertEqual($module_data['filter_test']->info['explanation'], SafeMarkup::format('Provides a filter plugin that is in use in the following filter formats: %formats', array('%formats' => $filter_format->label()))); - // Disable the filter_test_restrict_tags_and_attributes filter plugin but // have custom configuration so that the filter plugin is still configured // in filtered_html the filter format. diff --git a/core/modules/filter/tests/src/Unit/FilterUninstallValidatorTest.php b/core/modules/filter/tests/src/Unit/FilterUninstallValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..cb37bf42418bd27f58b040dc8234f18580f0aa87 --- /dev/null +++ b/core/modules/filter/tests/src/Unit/FilterUninstallValidatorTest.php @@ -0,0 +1,172 @@ +filterUninstallValidator = $this->getMockBuilder('Drupal\filter\FilterUninstallValidator') + ->disableOriginalConstructor() + ->setMethods(['getFilterDefinitionsByProvider', 'getEnabledFilterFormats']) + ->getMock(); + $this->filterUninstallValidator->setStringTranslation($this->getStringTranslationStub()); + } + + /** + * @covers ::validate + */ + public function testValidateNoPlugins() { + $this->filterUninstallValidator->expects($this->once()) + ->method('getFilterDefinitionsByProvider') + ->willReturn([]); + $this->filterUninstallValidator->expects($this->never()) + ->method('getEnabledFilterFormats'); + + $module = $this->randomMachineName(); + $expected = []; + $reasons = $this->filterUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateNoFormats() { + $this->filterUninstallValidator->expects($this->once()) + ->method('getFilterDefinitionsByProvider') + ->willReturn([ + 'test_filter_plugin' => [ + 'id' => 'test_filter_plugin', + 'provider' => 'filter_test', + ], + ]); + $this->filterUninstallValidator->expects($this->once()) + ->method('getEnabledFilterFormats') + ->willReturn([]); + + $module = $this->randomMachineName(); + $expected = []; + $reasons = $this->filterUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateNoMatchingFormats() { + $this->filterUninstallValidator->expects($this->once()) + ->method('getFilterDefinitionsByProvider') + ->willReturn([ + 'test_filter_plugin1' => [ + 'id' => 'test_filter_plugin1', + 'provider' => 'filter_test', + ], + 'test_filter_plugin2' => [ + 'id' => 'test_filter_plugin2', + 'provider' => 'filter_test', + ], + 'test_filter_plugin3' => [ + 'id' => 'test_filter_plugin3', + 'provider' => 'filter_test', + ], + 'test_filter_plugin4' => [ + 'id' => 'test_filter_plugin4', + 'provider' => 'filter_test', + ], + ]); + + $filter_plugin_enabled = $this->getMockForAbstractClass('Drupal\filter\Plugin\FilterBase', [['status' => TRUE], '', ['provider' => 'filter_test']]); + $filter_plugin_disabled = $this->getMockForAbstractClass('Drupal\filter\Plugin\FilterBase', [['status' => FALSE], '', ['provider' => 'filter_test']]); + + // The first format has 2 matching and enabled filters, but the loop breaks + // after finding the first one. + $filter_plugin_collection1 = $this->getMockBuilder('Drupal\filter\FilterPluginCollection') + ->disableOriginalConstructor() + ->getMock(); + $filter_plugin_collection1->expects($this->exactly(3)) + ->method('has') + ->willReturnMap([ + ['test_filter_plugin1', FALSE], + ['test_filter_plugin2', TRUE], + ['test_filter_plugin3', TRUE], + ['test_filter_plugin4', TRUE], + ]); + $filter_plugin_collection1->expects($this->exactly(2)) + ->method('get') + ->willReturnMap([ + ['test_filter_plugin2', $filter_plugin_disabled], + ['test_filter_plugin3', $filter_plugin_enabled], + ['test_filter_plugin4', $filter_plugin_enabled], + ]); + + $filter_format1 = $this->getMock('Drupal\filter\FilterFormatInterface'); + $filter_format1->expects($this->once()) + ->method('filters') + ->willReturn($filter_plugin_collection1); + $filter_format1->expects($this->once()) + ->method('label') + ->willReturn('Filter Format 1 Label'); + + // The second filter format only has one matching and enabled filter. + $filter_plugin_collection2 = $this->getMockBuilder('Drupal\filter\FilterPluginCollection') + ->disableOriginalConstructor() + ->getMock(); + $filter_plugin_collection2->expects($this->exactly(4)) + ->method('has') + ->willReturnMap([ + ['test_filter_plugin1', FALSE], + ['test_filter_plugin2', FALSE], + ['test_filter_plugin3', FALSE], + ['test_filter_plugin4', TRUE], + ]); + $filter_plugin_collection2->expects($this->exactly(1)) + ->method('get') + ->with('test_filter_plugin4') + ->willReturn($filter_plugin_enabled); + + $filter_format2 = $this->getMock('Drupal\filter\FilterFormatInterface'); + $filter_format2->expects($this->once()) + ->method('filters') + ->willReturn($filter_plugin_collection2); + $filter_format2->expects($this->once()) + ->method('label') + ->willReturn('Filter Format 2 Label'); + $this->filterUninstallValidator->expects($this->once()) + ->method('getEnabledFilterFormats') + ->willReturn([ + 'test_filter_format1' => $filter_format1, + 'test_filter_format2' => $filter_format2, + ]); + + $expected = [ + SafeMarkup::format('Provides a filter plugin that is in use in the following filter formats: %formats', ['%formats' => implode(', ', [ + 'Filter Format 1 Label', + 'Filter Format 2 Label', + ])]), + ]; + $reasons = $this->filterUninstallValidator->validate($this->randomMachineName()); + $this->assertSame($expected, $reasons); + } + +} diff --git a/core/modules/forum/forum.module b/core/modules/forum/forum.module index 01a66e5fcf723c9165c6168040879efc66b91f96..7ec742299351497890d9ec26300a02323d8dd9a4 100644 --- a/core/modules/forum/forum.module +++ b/core/modules/forum/forum.module @@ -11,11 +11,8 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Url; use Drupal\Component\Utility\SafeMarkup; -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; use Drupal\user\Entity\User; /** @@ -638,63 +635,3 @@ 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 Forum 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 Forum content and %vocabulary terms.', [ - '%vocabulary' => $vocabulary->label(), - '!url' => $vocabulary->url('overview-form'), - ]); - } - else { - $info['explanation'] = t('To uninstall Forum first delete all Forum content and %vocabulary terms.', [ - '%vocabulary' => $vocabulary->label() - ]); - } - } - else { - $info['explanation'] = t('To uninstall Forum first delete all %vocabulary 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 Forum content and %vocabulary terms.', [ - '%vocabulary' => $vocabulary->label() - ]); - } - } - } -} diff --git a/core/modules/forum/forum.services.yml b/core/modules/forum/forum.services.yml index d5c0fe3181b42c446d563f4967a41ec6baee4a68..6ab41c9c101c858340140966c9414178236a2d39 100644 --- a/core/modules/forum/forum.services.yml +++ b/core/modules/forum/forum.services.yml @@ -19,3 +19,10 @@ services: arguments: ['@database', '@forum_manager'] tags: - { name: backend_overridable } + + forum.uninstall_validator: + class: Drupal\forum\ForumUninstallValidator + tags: + - { name: module_install.uninstall_validator } + arguments: ['@entity.manager', '@entity.query', '@config.factory', '@string_translation'] + lazy: true diff --git a/core/modules/forum/src/ForumUninstallValidator.php b/core/modules/forum/src/ForumUninstallValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..9c6220d042a6c677d233bedef44e8b4d997198f7 --- /dev/null +++ b/core/modules/forum/src/ForumUninstallValidator.php @@ -0,0 +1,139 @@ +vocabularyStorage = $entity_manager->getStorage('taxonomy_vocabulary'); + $this->queryFactory = $query_factory; + $this->configFactory = $config_factory; + $this->stringTranslation = $string_translation; + } + + /** + * {@inheritdoc} + */ + public function validate($module) { + $reasons = []; + if ($module == 'forum') { + if ($this->hasForumNodes()) { + $reasons[] = $this->t('To uninstall Forum, first delete all Forum content'); + } + + $vocabulary = $this->getForumVocabulary(); + if ($this->hasTermsForVocabulary($vocabulary)) { + if ($vocabulary->access('view')) { + $reasons[] = $this->t('To uninstall Forum, first delete all %vocabulary terms', [ + '%vocabulary' => $vocabulary->label(), + '@url' => $vocabulary->url('overview-form'), + ]); + } + else { + $reasons[] = $this->t('To uninstall Forum, first delete all %vocabulary terms', [ + '%vocabulary' => $vocabulary->label() + ]); + } + } + } + + return $reasons; + } + + /** + * Determines if there are any forum nodes or not. + * + * @return bool + * TRUE if there are forum nodes, FALSE otherwise. + */ + protected function hasForumNodes() { + $nodes = $this->queryFactory->get('node') + ->condition('type', 'forum') + ->accessCheck(FALSE) + ->range(0, 1) + ->execute(); + return !empty($nodes); + } + + /** + * Determines if there are any taxonomy terms for a specified vocabulary. + * + * @param \Drupal\taxonomy\VocabularyInterface $vocabulary + * The vocabulary to check for terms. + * + * @return bool + * TRUE if there are terms for this vocabulary, FALSE otherwise. + */ + protected function hasTermsForVocabulary(VocabularyInterface $vocabulary) { + $terms = $this->queryFactory->get('taxonomy_term') + ->condition('vid', $vocabulary->id()) + ->accessCheck(FALSE) + ->range(0, 1) + ->execute(); + return !empty($terms); + } + + /** + * Returns the vocabulary configured for forums. + * + * @return \Drupal\taxonomy\VocabularyInterface + * The vocabulary entity for forums. + */ + protected function getForumVocabulary() { + $vid = $this->configFactory->get('forum.settings')->get('vocabulary'); + return $this->vocabularyStorage->load($vid); + } + +} diff --git a/core/modules/forum/src/Tests/ForumUninstallTest.php b/core/modules/forum/src/Tests/ForumUninstallTest.php index a0270ca26cd348cad637e02fefa5f5100c3d2203..db7c0910b4c34ccb5ffbc85adf72043a59864884 100644 --- a/core/modules/forum/src/Tests/ForumUninstallTest.php +++ b/core/modules/forum/src/Tests/ForumUninstallTest.php @@ -74,8 +74,7 @@ public function testForumUninstallWithField() { $this->drupalGet('admin/modules/uninstall'); // Assert forum is required. $this->assertNoFieldByName('uninstall[forum]'); - $this->drupalGet('admin/modules'); - $this->assertText('To uninstall Forum first delete all Forum content'); + $this->assertText('To uninstall Forum, first delete all Forum content'); // Delete the node. $this->drupalPostForm('node/' . $node->id() . '/delete', array(), t('Delete')); @@ -84,8 +83,7 @@ public function testForumUninstallWithField() { $this->drupalGet('admin/modules/uninstall'); // Assert forum is still required. $this->assertNoFieldByName('uninstall[forum]'); - $this->drupalGet('admin/modules'); - $this->assertText('To uninstall Forum first delete all Forums terms'); + $this->assertText('To uninstall Forum, first delete all Forums terms'); // Delete any forum terms. $vid = $this->config('forum.settings')->get('vocabulary'); @@ -102,8 +100,6 @@ public function testForumUninstallWithField() { $this->drupalGet('admin/modules/uninstall'); // Assert forum is no longer required. $this->assertFieldByName('uninstall[forum]'); - $this->drupalGet('admin/modules'); - $this->assertNoText('To uninstall Forum first delete all Forum content'); $this->drupalPostForm('admin/modules/uninstall', array( 'uninstall[forum]' => 1, ), t('Uninstall')); @@ -142,6 +138,13 @@ public function testForumUninstallWithoutFieldStorage() { $field_storage = FieldStorageConfig::loadByName('node', 'taxonomy_forums'); $this->assertNull($field_storage, 'The taxonomy_forums field storage has been deleted.'); + // Delete all terms in the Forums vocabulary. Uninstalling the forum module + // will fail unless this is done. + $terms = entity_load_multiple_by_properties('taxonomy_term', array('vid' => 'forums')); + foreach($terms as $term) { + $term->delete(); + } + // Ensure that uninstallation succeeds even if the field has already been // deleted manually beforehand. $this->container->get('module_installer')->uninstall(array('forum')); diff --git a/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..85145c2f5976ae866c1ce7541f4bb40a6dc9b7e1 --- /dev/null +++ b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php @@ -0,0 +1,247 @@ +forumUninstallValidator = $this->getMockBuilder('Drupal\forum\ForumUninstallValidator') + ->disableOriginalConstructor() + ->setMethods(['hasForumNodes', 'hasTermsForVocabulary', 'getForumVocabulary']) + ->getMock(); + $this->forumUninstallValidator->setStringTranslation($this->getStringTranslationStub()); + } + + /** + * @covers ::validate + */ + public function testValidateNotForum() { + $this->forumUninstallValidator->expects($this->never()) + ->method('hasForumNodes'); + $this->forumUninstallValidator->expects($this->never()) + ->method('hasTermsForVocabulary'); + $this->forumUninstallValidator->expects($this->never()) + ->method('getForumVocabulary'); + + $module = 'not_forum'; + $expected = []; + $reasons = $this->forumUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidate() { + $this->forumUninstallValidator->expects($this->once()) + ->method('hasForumNodes') + ->willReturn(FALSE); + + $vocabulary = $this->getMock('Drupal\taxonomy\VocabularyInterface'); + $this->forumUninstallValidator->expects($this->once()) + ->method('getForumVocabulary') + ->willReturn($vocabulary); + + $this->forumUninstallValidator->expects($this->once()) + ->method('hasTermsForVocabulary') + ->willReturn(FALSE); + + $module = 'forum'; + $expected = []; + $reasons = $this->forumUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateHasForumNodes() { + $this->forumUninstallValidator->expects($this->once()) + ->method('hasForumNodes') + ->willReturn(TRUE); + + $vocabulary = $this->getMock('Drupal\taxonomy\VocabularyInterface'); + $this->forumUninstallValidator->expects($this->once()) + ->method('getForumVocabulary') + ->willReturn($vocabulary); + + $this->forumUninstallValidator->expects($this->once()) + ->method('hasTermsForVocabulary') + ->willReturn(FALSE); + + $module = 'forum'; + $expected = [ + 'To uninstall Forum, first delete all Forum content', + ]; + $reasons = $this->forumUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateHasTermsForVocabularyWithNodesAccess() { + $this->forumUninstallValidator->expects($this->once()) + ->method('hasForumNodes') + ->willReturn(TRUE); + + $vocabulary = $this->getMock('Drupal\taxonomy\VocabularyInterface'); + $vocabulary->expects($this->once()) + ->method('label') + ->willReturn('Vocabulary label'); + $vocabulary->expects($this->once()) + ->method('url') + ->willReturn('/path/to/vocabulary/overview'); + $vocabulary->expects($this->once()) + ->method('access') + ->willReturn(TRUE); + $this->forumUninstallValidator->expects($this->once()) + ->method('getForumVocabulary') + ->willReturn($vocabulary); + + $this->forumUninstallValidator->expects($this->once()) + ->method('hasTermsForVocabulary') + ->willReturn(TRUE); + + $module = 'forum'; + $expected = [ + 'To uninstall Forum, first delete all Forum content', + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ + '@url' => '/path/to/vocabulary/overview', + '%vocabulary' => 'Vocabulary label', + ]), + ]; + $reasons = $this->forumUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateHasTermsForVocabularyWithNodesNoAccess() { + $this->forumUninstallValidator->expects($this->once()) + ->method('hasForumNodes') + ->willReturn(TRUE); + + $vocabulary = $this->getMock('Drupal\taxonomy\VocabularyInterface'); + $vocabulary->expects($this->once()) + ->method('label') + ->willReturn('Vocabulary label'); + $vocabulary->expects($this->never()) + ->method('url'); + $vocabulary->expects($this->once()) + ->method('access') + ->willReturn(FALSE); + $this->forumUninstallValidator->expects($this->once()) + ->method('getForumVocabulary') + ->willReturn($vocabulary); + + $this->forumUninstallValidator->expects($this->once()) + ->method('hasTermsForVocabulary') + ->willReturn(TRUE); + + $module = 'forum'; + $expected = [ + 'To uninstall Forum, first delete all Forum content', + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ + '%vocabulary' => 'Vocabulary label', + ]), + ]; + $reasons = $this->forumUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateHasTermsForVocabularyAccess() { + $this->forumUninstallValidator->expects($this->once()) + ->method('hasForumNodes') + ->willReturn(FALSE); + + $vocabulary = $this->getMock('Drupal\taxonomy\VocabularyInterface'); + $vocabulary->expects($this->once()) + ->method('url') + ->willReturn('/path/to/vocabulary/overview'); + $vocabulary->expects($this->once()) + ->method('label') + ->willReturn('Vocabulary label'); + $vocabulary->expects($this->once()) + ->method('access') + ->willReturn(TRUE); + $this->forumUninstallValidator->expects($this->once()) + ->method('getForumVocabulary') + ->willReturn($vocabulary); + + $this->forumUninstallValidator->expects($this->once()) + ->method('hasTermsForVocabulary') + ->willReturn(TRUE); + + $module = 'forum'; + $expected = [ + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ + '@url' => '/path/to/vocabulary/overview', + '%vocabulary' => 'Vocabulary label', + ]), + ]; + $reasons = $this->forumUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateHasTermsForVocabularyNoAccess() { + $this->forumUninstallValidator->expects($this->once()) + ->method('hasForumNodes') + ->willReturn(FALSE); + + $vocabulary = $this->getMock('Drupal\taxonomy\VocabularyInterface'); + $vocabulary->expects($this->once()) + ->method('label') + ->willReturn('Vocabulary label'); + $vocabulary->expects($this->never()) + ->method('url'); + $vocabulary->expects($this->once()) + ->method('access') + ->willReturn(FALSE); + $this->forumUninstallValidator->expects($this->once()) + ->method('getForumVocabulary') + ->willReturn($vocabulary); + + $this->forumUninstallValidator->expects($this->once()) + ->method('hasTermsForVocabulary') + ->willReturn(TRUE); + + $module = 'forum'; + $expected = [ + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ + '%vocabulary' => 'Vocabulary label', + ]), + ]; + $reasons = $this->forumUninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + +} diff --git a/core/modules/system/src/Tests/Field/FieldModuleUninstallValidatorTest.php b/core/modules/system/src/Tests/Field/FieldModuleUninstallValidatorTest.php index f427ade3443be0d484aced81b4114ba0769d49f7..0bed70beed3e062caa62c48536dd5dfc5cb09529 100644 --- a/core/modules/system/src/Tests/Field/FieldModuleUninstallValidatorTest.php +++ b/core/modules/system/src/Tests/Field/FieldModuleUninstallValidatorTest.php @@ -83,7 +83,7 @@ public function testUninstallingModule() { } catch (ModuleUninstallValidatorException $e) { $this->pass($message); - $this->assertEqual($e->getMessage(), 'The following reasons prevents the modules from being uninstalled: There is data for the field extra_base_field on entity type Test entity.'); + $this->assertEqual($e->getMessage(), 'The following reasons prevents the modules from being uninstalled: There is data for the field extra_base_field on entity type Test entity'); } // Verify uninstalling entity_test is not possible when there is content for diff --git a/core/modules/system/src/Tests/Module/DependencyTest.php b/core/modules/system/src/Tests/Module/DependencyTest.php index 88d959aaf43ce982187b8efd7ac380a0b2c6a3f3..13a722d71b3e24f88f80dc1b923b0199a04dd777 100644 --- a/core/modules/system/src/Tests/Module/DependencyTest.php +++ b/core/modules/system/src/Tests/Module/DependencyTest.php @@ -168,8 +168,8 @@ function testUninstallDependents() { // Check that the comment module cannot be uninstalled. $this->drupalGet('admin/modules/uninstall'); - $checkbox = $this->xpath('//input[@type="checkbox" and @name="uninstall[comment]"]'); - $this->assert(count($checkbox) == 0, 'Checkbox for uninstalling the comment module not found.'); + $checkbox = $this->xpath('//input[@type="checkbox" and @name="uninstall[comment]" and @disabled="disabled"]'); + $this->assert(count($checkbox) == 1, 'Checkbox for uninstalling the comment module is disabled.'); // Delete any forum terms. $vid = $this->config('forum.settings')->get('vocabulary'); diff --git a/core/modules/system/src/Tests/Module/UninstallTest.php b/core/modules/system/src/Tests/Module/UninstallTest.php index e43a674e1253474dda2f9ad23070841b0ab2f83f..aa8a1696e30ce28c3e545dfea6ca0e62c2e3373b 100644 --- a/core/modules/system/src/Tests/Module/UninstallTest.php +++ b/core/modules/system/src/Tests/Module/UninstallTest.php @@ -56,7 +56,8 @@ function testUninstallPage() { $this->drupalGet('admin/modules/uninstall'); $this->assertTitle(t('Uninstall') . ' | Drupal'); - $this->assertText(\Drupal::translation()->translate('The following reasons prevents Node from being uninstalled: There is content for the entity type: Content'), 'Content prevents uninstalling node module.'); + $this->assertText(\Drupal::translation()->translate('The following reason prevents Node from being uninstalled:')); + $this->assertText(\Drupal::translation()->translate('There is content for the entity type: Content')); // Delete the node to allow node to be uninstalled. $node->delete(); diff --git a/core/modules/system/src/Tests/System/InfoAlterTest.php b/core/modules/system/src/Tests/System/InfoAlterTest.php index 02cdd1771e684d72fe80c5ceee2a08f1216aa6fa..2af3d05692461f5542d8e2655ea3a0e087636e64 100644 --- a/core/modules/system/src/Tests/System/InfoAlterTest.php +++ b/core/modules/system/src/Tests/System/InfoAlterTest.php @@ -26,15 +26,15 @@ class InfoAlterTest extends KernelTestBase { * return freshly altered info. */ function testSystemInfoAlter() { - \Drupal::state()->set('module_test.hook_system_info_alter', TRUE); + \Drupal::state()->set('module_required_test.hook_system_info_alter', TRUE); $info = system_rebuild_module_data(); - $this->assertFalse(isset($info['node']->info['required']), 'Before the module_test is installed the node module is not required.'); + $this->assertFalse(isset($info['node']->info['required']), 'Before the module_required_test is installed the node module is not required.'); // Enable the test module. - \Drupal::service('module_installer')->install(array('module_test'), FALSE); - $this->assertTrue(\Drupal::moduleHandler()->moduleExists('module_test'), 'Test module is enabled.'); + \Drupal::service('module_installer')->install(array('module_required_test'), FALSE); + $this->assertTrue(\Drupal::moduleHandler()->moduleExists('module_required_test'), 'Test required module is enabled.'); $info = system_rebuild_module_data(); - $this->assertTrue($info['node']->info['required'], 'After the module_test is installed the node module is required.'); + $this->assertTrue($info['node']->info['required'], 'After the module_required_test is installed the node module is required.'); } } diff --git a/core/modules/system/system.admin.inc b/core/modules/system/system.admin.inc index 81fa6af7510b1f8b33087d9dc2b0c86d2826fcb2..276e3936b4c75f18946a8b1f3113cd462fa939e4 100644 --- a/core/modules/system/system.admin.inc +++ b/core/modules/system/system.admin.inc @@ -303,16 +303,22 @@ function theme_system_modules_uninstall($variables) { // Display table. $rows = array(); foreach (Element::children($form['modules']) as $module) { - $disabled_message = ''; + $disabled_header = ''; + $disabled_reasons = ''; // Add the modules requiring the module in question as a validation reason. if (!empty($form['modules'][$module]['#required_by'])) { $form['modules'][$module]['#validation_reasons'][] = \Drupal::translation()->translate('Required by: @modules', array('@modules' => implode(', ',$form['modules'][$module]['#required_by']))); } if (!empty($form['modules'][$module]['#validation_reasons'])) { - $disabled_message = \Drupal::translation()->formatPlural(count($form['modules'][$module]['#validation_reasons']), - 'The following reason prevents @module from being uninstalled: @reasons', - 'The following reasons prevents @module from being uninstalled: @reasons', - array('@module' => $form['modules'][$module]['#module_name'], '@reasons' => implode('; ', $form['modules'][$module]['#validation_reasons']))); + $disabled_reasons = [ + '#theme' => 'item_list', + '#items' => $form['modules'][$module]['#validation_reasons'], + ]; + $disabled_reasons = drupal_render($disabled_reasons); + $disabled_header = \Drupal::translation()->formatPlural(count($form['modules'][$module]['#validation_reasons']), + 'The following reason prevents @module from being uninstalled:', + 'The following reasons prevents @module from being uninstalled:', + array('@module' => $form['modules'][$module]['#module_name'])); } $rows[] = array( array('data' => drupal_render($form['uninstall'][$module]), 'align' => 'center'), @@ -326,10 +332,11 @@ function theme_system_modules_uninstall($variables) { array( 'data' => array( '#type' => 'inline_template', - '#template' => '{{ module_description }} {% if disabled_message is not empty %}
{{ disabled_message }}
{% endif %}', + '#template' => '{{ module_description }} {% if disabled_header is not empty %}
{{ disabled_header }}{{ disabled_reasons }}
{% endif %}', '#context' => array( 'module_description' => drupal_render($form['modules'][$module]['description']), - 'disabled_message' => $disabled_message, + 'disabled_header' => $disabled_header, + 'disabled_reasons' => $disabled_reasons, ), ), 'class' => array('description'), diff --git a/core/modules/system/tests/modules/module_required_test/module_required_test.info.yml b/core/modules/system/tests/modules/module_required_test/module_required_test.info.yml new file mode 100644 index 0000000000000000000000000000000000000000..f424d95cfbbc97933f53ad2876e068134b031818 --- /dev/null +++ b/core/modules/system/tests/modules/module_required_test/module_required_test.info.yml @@ -0,0 +1,11 @@ +name: 'Module required test' +type: module +description: 'Support module for module system testing.' +package: Testing +version: VERSION +core: 8.x +# Depends on the Node module to test making a module required using +# hook_system_info_alter() and ensuring that its dependencies also become +# required. +dependencies: + - drupal:node (>=8.x) diff --git a/core/modules/system/tests/modules/module_required_test/module_required_test.module b/core/modules/system/tests/modules/module_required_test/module_required_test.module new file mode 100644 index 0000000000000000000000000000000000000000..fbb6f104155dfeda829c1527e2ecfbaacaceb051 --- /dev/null +++ b/core/modules/system/tests/modules/module_required_test/module_required_test.module @@ -0,0 +1,15 @@ +getName() == 'module_required_test' && \Drupal::state()->get('module_required_test.hook_system_info_alter')) { + $info['required'] = TRUE; + $info['explanation'] = 'Testing hook_system_info_alter()'; + } +} diff --git a/core/modules/system/tests/modules/module_test/module_test.info.yml b/core/modules/system/tests/modules/module_test/module_test.info.yml index 241c3add9bd5d376e4dee72ab816dc7e8e145df4..5c63da21a6489d3b7e286ea37ab1245e4abfcffd 100644 --- a/core/modules/system/tests/modules/module_test/module_test.info.yml +++ b/core/modules/system/tests/modules/module_test/module_test.info.yml @@ -4,8 +4,3 @@ description: 'Support module for module system testing.' package: Testing version: VERSION core: 8.x -# Depends on the Node module to test making a module required using -# hook_system_info_alter() and ensuring that its dependencies also become -# required. -dependencies: - - drupal:node (>=8.x) diff --git a/core/modules/system/tests/modules/module_test/module_test.module b/core/modules/system/tests/modules/module_test/module_test.module index 92f038b3dfb4c6fd0d7fcc7eae32e374a88351dd..cead3299013fb6d1ade04d191325cb90cfe5ab3f 100644 --- a/core/modules/system/tests/modules/module_test/module_test.module +++ b/core/modules/system/tests/modules/module_test/module_test.module @@ -49,10 +49,6 @@ function module_test_system_info_alter(&$info, Extension $file, $type) { if ($file->getName() == 'seven' && $type == 'theme') { $info['regions']['test_region'] = t('Test region'); } - if ($file->getName() == 'module_test' && \Drupal::state()->get('module_test.hook_system_info_alter')) { - $info['required'] = TRUE; - $info['explanation'] = 'Testing hook_system_info_alter()'; - } } /** diff --git a/core/profiles/standard/src/Tests/StandardTest.php b/core/profiles/standard/src/Tests/StandardTest.php index 217176560176274bfb255734940c8c4bfda306e4..3e2ee07ef1c402b13bd5495cca765a0956c1dbef 100644 --- a/core/profiles/standard/src/Tests/StandardTest.php +++ b/core/profiles/standard/src/Tests/StandardTest.php @@ -9,6 +9,7 @@ use Drupal\config\Tests\SchemaCheckTestTrait; use Drupal\contact\Entity\ContactForm; +use Drupal\filter\Entity\FilterFormat; use Drupal\simpletest\WebTestBase; use Drupal\user\Entity\Role; @@ -125,6 +126,14 @@ function testStandard() { // The installer does not have this limitation since it ensures that all of // the install profiles dependencies are installed before creating the // editor configuration. + foreach (FilterFormat::loadMultiple() as $filter) { + // Ensure that editor can be uninstalled by removing use in filter + // formats. It is necessary to prime the filter collection before removing + // the filter. + $filter->filters(); + $filter->removeFilter('editor_file_reference'); + $filter->save(); + } \Drupal::service('module_installer')->uninstall(array('editor', 'ckeditor')); $this->rebuildContainer(); \Drupal::service('module_installer')->install(array('editor')); diff --git a/core/tests/Drupal/Tests/Core/Extension/RequiredModuleUninstallValidatorTest.php b/core/tests/Drupal/Tests/Core/Extension/RequiredModuleUninstallValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..718ba8f4a722de041df33b14abe99c09d7227f8c --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Extension/RequiredModuleUninstallValidatorTest.php @@ -0,0 +1,80 @@ +uninstallValidator = $this->getMockBuilder('Drupal\Core\Extension\RequiredModuleUninstallValidator') + ->disableOriginalConstructor() + ->setMethods(['getModuleInfoByModule']) + ->getMock(); + $this->uninstallValidator->setStringTranslation($this->getStringTranslationStub()); + } + + /** + * @covers ::validate + */ + public function testValidateNoModule() { + $this->uninstallValidator->expects($this->once()) + ->method('getModuleInfoByModule') + ->willReturn([]); + + $module = $this->randomMachineName(); + $expected = []; + $reasons = $this->uninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateNotRequired() { + $module = $this->randomMachineName(); + + $this->uninstallValidator->expects($this->once()) + ->method('getModuleInfoByModule') + ->willReturn(['required' => FALSE, 'name' => $module]); + + $expected = []; + $reasons = $this->uninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + + /** + * @covers ::validate + */ + public function testValidateRequired() { + $module = $this->randomMachineName(); + + $this->uninstallValidator->expects($this->once()) + ->method('getModuleInfoByModule') + ->willReturn(['required' => TRUE, 'name' => $module]); + + $expected = [SafeMarkup::format('The @module module is required', ['@module' => $module])]; + $reasons = $this->uninstallValidator->validate($module); + $this->assertSame($expected, $reasons); + } + +}