Issue #2392815 by alexpott, pooja saraah, bircher, cilefen, pratik_specbee, catch, fago: Module uninstall validators are not used to validate a config import

(cherry picked from commit 7fbc4c4ba7)
merge-requests/1995/head
catch 2022-06-13 11:40:11 +01:00
parent fbd2d2a292
commit 163df489ff
12 changed files with 254 additions and 14 deletions

View File

@ -1286,6 +1286,7 @@ services:
class: Drupal\Core\EventSubscriber\ConfigImportSubscriber
tags:
- { name: event_subscriber }
- { name: service_collector, tag: 'module_install.uninstall_validator', call: addUninstallValidator }
arguments: ['@theme_handler', '@extension.list.module']
config_snapshot_subscriber:
class: Drupal\Core\EventSubscriber\ConfigSnapshotSubscriber

View File

@ -673,13 +673,18 @@ class ConfigImporter {
/**
* Gets the next extension operation to perform.
*
* Uninstalls are processed first with themes coming before modules. Then
* installs are processed with modules coming before themes. This order is
* necessary because themes can depend on modules.
*
* @return array|bool
* An array containing the next operation and extension name to perform it
* on. If there is nothing left to do returns FALSE;
*/
protected function getNextExtensionOperation() {
foreach (['module', 'theme'] as $type) {
foreach (['install', 'uninstall'] as $op) {
foreach (['uninstall', 'install'] as $op) {
$types = $op === 'uninstall' ? ['theme', 'module'] : ['module', 'theme'];
foreach ($types as $type) {
$unprocessed = $this->getUnprocessedExtensions($type);
if (!empty($unprocessed[$op])) {
return [

View File

@ -7,7 +7,9 @@ use Drupal\Core\Config\ConfigImporter;
use Drupal\Core\Config\ConfigImporterEvent;
use Drupal\Core\Config\ConfigImportValidateEventSubscriberBase;
use Drupal\Core\Config\ConfigNameException;
use Drupal\Core\Extension\ConfigImportModuleUninstallValidatorInterface;
use Drupal\Core\Extension\ModuleExtensionList;
use Drupal\Core\Extension\ModuleUninstallValidatorInterface;
use Drupal\Core\Extension\ThemeHandlerInterface;
use Drupal\Core\Installer\InstallerKernel;
@ -37,6 +39,13 @@ class ConfigImportSubscriber extends ConfigImportValidateEventSubscriberBase {
*/
protected $themeHandler;
/**
* The uninstall validators.
*
* @var \Drupal\Core\Extension\ModuleUninstallValidatorInterface[]
*/
protected $uninstallValidators = [];
/**
* Constructs the ConfigImportSubscriber.
*
@ -50,6 +59,16 @@ class ConfigImportSubscriber extends ConfigImportValidateEventSubscriberBase {
$this->moduleExtensionList = $extension_list_module;
}
/**
* Adds a module uninstall validator.
*
* @param \Drupal\Core\Extension\ModuleUninstallValidatorInterface $uninstall_validator
* The uninstall validator to add.
*/
public function addUninstallValidator(ModuleUninstallValidatorInterface $uninstall_validator): void {
$this->uninstallValidators[] = $uninstall_validator;
}
/**
* Validates the configuration to be imported.
*
@ -150,6 +169,16 @@ class ConfigImportSubscriber extends ConfigImportValidateEventSubscriberBase {
$config_importer->logError($this->t('Unable to uninstall the %module module since the %dependent_module module is installed.', ['%module' => $module_name, '%dependent_module' => $dependent_module_name]));
}
}
// Ensure that modules can be uninstalled.
foreach ($this->uninstallValidators as $validator) {
$reasons = $validator instanceof ConfigImportModuleUninstallValidatorInterface ?
$validator->validateConfigImport($module, $config_importer->getStorageComparer()->getSourceStorage()) :
$validator->validate($module);
foreach ($reasons as $reason) {
$config_importer->logError($this->t('Unable to uninstall the %module module because: @reason.',
['%module' => $module_data[$module]->info['name'], '@reason' => $reason]));
}
}
}
// Ensure that the install profile is not being uninstalled.
@ -169,6 +198,7 @@ class ConfigImportSubscriber extends ConfigImportValidateEventSubscriberBase {
$core_extension = $config_importer->getStorageComparer()->getSourceStorage()->read('core.extension');
// Get all themes including those that are not installed.
$theme_data = $this->getThemeData();
$module_data = $this->moduleExtensionList->getList();
$installs = $config_importer->getExtensionChangelist('theme', 'install');
foreach ($installs as $key => $theme) {
if (!isset($theme_data[$theme])) {
@ -181,13 +211,28 @@ class ConfigImportSubscriber extends ConfigImportValidateEventSubscriberBase {
// Ensure that all themes being installed have their dependencies met.
foreach ($installs as $theme) {
foreach (array_keys($theme_data[$theme]->requires) as $required_theme) {
$module_dependencies = $theme_data[$theme]->module_dependencies;
// $theme_data[$theme]->requires contains both theme and module
// dependencies keyed by the extension machine names.
// $theme_data[$theme]->module_dependencies contains only the module
// dependencies keyed by the module extension machine name. Therefore, we
// can find the theme dependencies by finding array keys for 'requires'
// that are not in $module_dependencies.
$theme_dependencies = array_diff_key($theme_data[$theme]->requires, $module_dependencies);
foreach (array_keys($theme_dependencies) as $required_theme) {
if (!isset($core_extension['theme'][$required_theme])) {
$theme_name = $theme_data[$theme]->info['name'];
$required_theme_name = $theme_data[$required_theme]->info['name'];
$config_importer->logError($this->t('Unable to install the %theme theme since it requires the %required_theme theme.', ['%theme' => $theme_name, '%required_theme' => $required_theme_name]));
}
}
foreach (array_keys($module_dependencies) as $required_module) {
if (!isset($core_extension['module'][$required_module])) {
$theme_name = $theme_data[$theme]->info['name'];
$required_module_name = $module_data[$required_module]->info['name'];
$config_importer->logError($this->t('Unable to install the %theme theme since it requires the %required_module module.', ['%theme' => $theme_name, '%required_module' => $required_module_name]));
}
}
}
// Ensure that all themes being uninstalled are not required by themes that

View File

@ -0,0 +1,34 @@
<?php
namespace Drupal\Core\Extension;
use Drupal\Core\Config\StorageInterface;
/**
* Special interface for module uninstall validators for configuration import.
*
* A module uninstall validator that needs different functionality prior to a
* configuration import should implement this interface and be defined in
* a Drupal @link container service @endlink that is tagged
* module_install.uninstall_validator.
*/
interface ConfigImportModuleUninstallValidatorInterface extends ModuleUninstallValidatorInterface {
/**
* Determines reasons a module can not be uninstalled prior to config import.
*
* @param string $module
* A module name.
* @param \Drupal\Core\Config\StorageInterface $source_storage
* Storage object used to read configuration that is about to be imported.
*
* @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 \Drupal\Core\EventSubscriber\ConfigImportSubscriber::validateModules()
*/
public function validateConfigImport(string $module, StorageInterface $source_storage): array;
}

View File

@ -70,7 +70,7 @@ interface ModuleInstallerInterface {
public function uninstall(array $module_list, $uninstall_dependents = TRUE);
/**
* Adds module a uninstall validator.
* Adds a module uninstall validator.
*
* @param \Drupal\Core\Extension\ModuleUninstallValidatorInterface $uninstall_validator
* The uninstall validator to add.

View File

@ -2,13 +2,14 @@
namespace Drupal\Core\Extension;
use Drupal\Core\Config\StorageInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\StringTranslation\TranslationInterface;
/**
* Ensures modules cannot be uninstalled if enabled themes depend on them.
*/
class ModuleRequiredByThemesUninstallValidator implements ModuleUninstallValidatorInterface {
class ModuleRequiredByThemesUninstallValidator implements ConfigImportModuleUninstallValidatorInterface {
use StringTranslationTrait;
@ -61,6 +62,27 @@ class ModuleRequiredByThemesUninstallValidator implements ModuleUninstallValidat
return $reasons;
}
/**
* {@inheritdoc}
*/
public function validateConfigImport(string $module, StorageInterface $source_storage): array {
$reasons = [];
$themes_depending_on_module = $this->getThemesDependingOnModule($module);
if (!empty($themes_depending_on_module)) {
$installed_themes_after_import = $source_storage->read('core.extension')['theme'];
$themes_depending_on_module_still_installed = array_intersect_key($themes_depending_on_module, $installed_themes_after_import);
// Ensure that any dependent themes will be uninstalled by the module.
if (!empty($themes_depending_on_module_still_installed)) {
$reasons[] = $this->formatPlural(count($themes_depending_on_module_still_installed),
'Required by the theme: @theme_names',
'Required by the themes: @theme_names',
['@theme_names' => implode(', ', $themes_depending_on_module_still_installed)]);
}
}
return $reasons;
}
/**
* Returns themes that depend on a module.
*
@ -68,7 +90,8 @@ class ModuleRequiredByThemesUninstallValidator implements ModuleUninstallValidat
* The module machine name.
*
* @return string[]
* An array of the names of themes that depend on $module.
* An array of the names of themes that depend on $module keyed by the
* theme's machine name.
*/
protected function getThemesDependingOnModule($module) {
$installed_themes = $this->themeExtensionList->getAllInstalledInfo();

View File

@ -8,6 +8,14 @@ namespace Drupal\Core\Extension;
* A module uninstall validator must implement this interface and be defined in
* a Drupal @link container service @endlink that is tagged
* module_install.uninstall_validator.
*
* Validators are called during module uninstall and prior to running a
* configuration import. If different logic is required when uninstalling via
* configuration import implement ConfigImportModuleUninstallValidatorInterface.
*
* @see \Drupal\Core\Extension\ModuleInstaller::validateUninstall()
* @see \Drupal\Core\EventSubscriber\ConfigImportSubscriber::validateModules()
* @see \Drupal\Core\Extension\ConfigImportModuleUninstallValidatorInterface
*/
interface ModuleUninstallValidatorInterface {

View File

@ -146,11 +146,11 @@ class ThemeInstaller implements ThemeInstallerInterface {
foreach ($theme_list as $theme => $value) {
$module_dependencies = $theme_data[$theme]->module_dependencies;
// $theme_data[$theme]->requires contains both theme and module
// dependencies keyed by the extension machine names and
// $theme_data[$theme]->module_dependencies contains only modules keyed
// by the module extension machine name. Therefore we can find the theme
// dependencies by finding array keys for 'requires' that are not in
// $module_dependencies.
// dependencies keyed by the extension machine names.
// $theme_data[$theme]->module_dependencies contains only the module
// dependencies keyed by the module extension machine name. Therefore,
// we can find the theme dependencies by finding array keys for
// 'requires' that are not in $module_dependencies.
$theme_dependencies = array_diff_key($theme_data[$theme]->requires, $module_dependencies);
// We can find the unmet module dependencies by finding the module
// machine names keys that are not in $installed_modules keys.

View File

@ -12,7 +12,7 @@ namespace Drupal\Core\ProxyClass\Extension {
*
* @see \Drupal\Component\ProxyBuilder
*/
class ModuleRequiredByThemesUninstallValidator implements \Drupal\Core\Extension\ModuleUninstallValidatorInterface
class ModuleRequiredByThemesUninstallValidator implements \Drupal\Core\Extension\ConfigImportModuleUninstallValidatorInterface
{
use \Drupal\Core\DependencyInjection\DependencySerializationTrait;
@ -75,6 +75,14 @@ namespace Drupal\Core\ProxyClass\Extension {
return $this->lazyLoadItself()->validate($module);
}
/**
* {@inheritdoc}
*/
public function validateConfigImport(string $module, \Drupal\Core\Config\StorageInterface $source_storage): array
{
return $this->lazyLoadItself()->validateConfigImport($module, $source_storage);
}
/**
* {@inheritdoc}
*/

View File

@ -2,8 +2,9 @@
namespace Drupal\field;
use Drupal\Core\Config\StorageInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Extension\ModuleUninstallValidatorInterface;
use Drupal\Core\Extension\ConfigImportModuleUninstallValidatorInterface;
use Drupal\Core\Field\FieldTypePluginManagerInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\StringTranslation\TranslationInterface;
@ -11,7 +12,7 @@ use Drupal\Core\StringTranslation\TranslationInterface;
/**
* Prevents uninstallation of modules providing active field storage.
*/
class FieldUninstallValidator implements ModuleUninstallValidatorInterface {
class FieldUninstallValidator implements ConfigImportModuleUninstallValidatorInterface {
use StringTranslationTrait;
@ -72,6 +73,15 @@ class FieldUninstallValidator implements ModuleUninstallValidatorInterface {
return $reasons;
}
/**
* {@inheritdoc}
*/
public function validateConfigImport(string $module, StorageInterface $source_storage): array {
// The field_config_import_steps_alter() method removes field data prior to
// configuration import so the checks in ::validate() are unnecessary.
return [];
}
/**
* Returns all field storages for a specified module.
*

View File

@ -660,6 +660,30 @@ class ConfigImporterTest extends KernelTestBase {
}
}
/**
* Tests uninstall validators being called during synchronization.
*
* @see \Drupal\Core\EventSubscriber\ConfigImportSubscriber
*/
public function testRequiredModuleValidation() {
$sync = $this->container->get('config.storage.sync');
$extensions = $sync->read('core.extension');
unset($extensions['module']['system']);
$sync->write('core.extension', $extensions);
$config_importer = $this->configImporter();
try {
$config_importer->import();
$this->fail('ConfigImporterException not thrown, invalid import was not stopped due to missing dependencies.');
}
catch (ConfigImporterException $e) {
$this->assertStringContainsString('There were errors validating the config synchronization.', $e->getMessage());
$error_log = $config_importer->getErrors();
$this->assertEquals('Unable to uninstall the <em class="placeholder">System</em> module because: The System module is required.', $error_log[0]);
}
}
/**
* Tests install profile validation during configuration import.
*

View File

@ -0,0 +1,82 @@
<?php
namespace Drupal\KernelTests\Core\Theme;
use Drupal\Core\Config\ConfigImporterException;
use Drupal\KernelTests\KernelTestBase;
/**
* Tests installing and uninstalling of themes via configuration import.
*
* @group Extension
*/
class ConfigImportThemeInstallTest extends KernelTestBase {
/**
* Modules to enable.
*
* @var array
*/
protected static $modules = ['system'];
protected function setUp(): void {
parent::setUp();
$this->installConfig(['system']);
}
/**
* Tests config imports that install and uninstall a theme with dependencies.
*/
public function testConfigImportWithThemeWithModuleDependencies() {
$this->container->get('module_installer')->install(['test_module_required_by_theme', 'test_another_module_required_by_theme']);
$this->container->get('theme_installer')->install(['test_theme_depending_on_modules']);
$this->assertTrue($this->container->get('theme_handler')->themeExists('test_theme_depending_on_modules'), 'test_theme_depending_on_modules theme installed');
$sync = $this->container->get('config.storage.sync');
$this->copyConfig($this->container->get('config.storage'), $sync);
$extensions = $sync->read('core.extension');
// Remove one of the modules the theme depends on.
unset($extensions['module']['test_module_required_by_theme']);
$sync->write('core.extension', $extensions);
try {
$this->configImporter()->validate();
$this->fail('ConfigImporterException not thrown; an invalid import was not stopped due to missing dependencies.');
}
catch (ConfigImporterException $e) {
$error_message = 'Unable to uninstall the <em class="placeholder">Test Module Required by Theme</em> module because: Required by the theme: Test Theme Depending on Modules.';
$this->assertStringContainsString($error_message, $e->getMessage(), 'There were errors validating the config synchronization.');
$error_log = $this->configImporter->getErrors();
$this->assertEquals([$error_message], $error_log);
}
// Remove the other module and the theme.
unset($extensions['module']['test_another_module_required_by_theme']);
unset($extensions['theme']['test_theme_depending_on_modules']);
$sync->write('core.extension', $extensions);
$this->configImporter()->import();
$this->assertFalse($this->container->get('theme_handler')->themeExists('test_theme_depending_on_modules'), 'test_theme_depending_on_modules theme uninstalled by configuration import');
// Try installing a theme with dependencies via config import.
$extensions['theme']['test_theme_depending_on_modules'] = 0;
$extensions['module']['test_another_module_required_by_theme'] = 0;
$sync->write('core.extension', $extensions);
try {
$this->configImporter()->validate();
$this->fail('ConfigImporterException not thrown; an invalid import was not stopped due to missing dependencies.');
}
catch (ConfigImporterException $e) {
$error_message = 'Unable to install the <em class="placeholder">Test Theme Depending on Modules</em> theme since it requires the <em class="placeholder">Test Module Required by Theme</em> module.';
$this->assertStringContainsString($error_message, $e->getMessage(), 'There were errors validating the config synchronization.');
$error_log = $this->configImporter->getErrors();
$this->assertEquals([$error_message], $error_log);
}
$extensions['module']['test_module_required_by_theme'] = 0;
$sync->write('core.extension', $extensions);
$this->configImporter()->import();
$this->assertTrue($this->container->get('theme_handler')->themeExists('test_theme_depending_on_modules'), 'test_theme_depending_on_modules theme installed');
}
}