From 95a2a252c2a934676e371e83e94401815739f512 Mon Sep 17 00:00:00 2001 From: catch Date: Mon, 30 Jan 2023 10:56:47 +0000 Subject: [PATCH] Issue #3001430 by alexpott, _utsavsharma, Oscaner, lauriii, smustgrave, ifux, deviantintegral, pameeela: Unable to uninstall base theme and subtheme via config sync at the same time (cherry picked from commit fab2025f20a4b02ce8c2a4ecb8ae796f4d328eef) --- core/includes/install.core.inc | 6 ++- .../lib/Drupal/Core/Config/ConfigImporter.php | 38 +++++++++++++++++-- .../ConfigImportSubscriber.php | 12 ++---- .../src/Form/ConfigSingleImportForm.php | 23 +++++++++-- core/modules/config/src/Form/ConfigSync.php | 23 +++++++++-- .../ConfigUninstallViaCliImportTest.php | 3 +- .../ContentTranslationConfigImportTest.php | 3 +- .../src/Kernel/OverriddenConfigImportTest.php | 3 +- .../Core/Config/ConfigImportRecreateTest.php | 3 +- .../ConfigImportRenameValidationTest.php | 3 +- .../ConfigImporterMissingContentTest.php | 3 +- .../Core/Config/ConfigImporterTest.php | 31 +++++++++++++++ .../Entity/ContentEntityNullStorageTest.php | 3 +- core/tests/Drupal/Tests/ConfigTestTrait.php | 3 +- 14 files changed, 129 insertions(+), 28 deletions(-) diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index 1b420856e49..b4956350ab8 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -2353,7 +2353,8 @@ function install_config_import_batch() { \Drupal::service('module_installer'), \Drupal::service('theme_handler'), \Drupal::service('string_translation'), - \Drupal::service('extension.list.module') + \Drupal::service('extension.list.module'), + \Drupal::service('extension.list.theme') ); try { @@ -2425,7 +2426,8 @@ function install_config_revert_install_changes() { \Drupal::service('module_installer'), \Drupal::service('theme_handler'), \Drupal::service('string_translation'), - \Drupal::service('extension.list.module') + \Drupal::service('extension.list.module'), + \Drupal::service('extension.list.theme') ); try { $config_importer->import(); diff --git a/core/lib/Drupal/Core/Config/ConfigImporter.php b/core/lib/Drupal/Core/Config/ConfigImporter.php index 3d822d875ee..d25b0f062a0 100644 --- a/core/lib/Drupal/Core/Config/ConfigImporter.php +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php @@ -6,6 +6,7 @@ use Drupal\Core\Config\Importer\MissingContentEvent; use Drupal\Core\Extension\ModuleExtensionList; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ModuleInstallerInterface; +use Drupal\Core\Extension\ThemeExtensionList; use Drupal\Core\Extension\ThemeHandlerInterface; use Drupal\Core\Config\Entity\ImportableEntityStorageInterface; use Drupal\Core\DependencyInjection\DependencySerializationTrait; @@ -166,6 +167,13 @@ class ConfigImporter { */ protected $moduleExtensionList; + /** + * The theme extension list. + * + * @var \Drupal\Core\Extension\ThemeExtensionList + */ + protected $themeExtensionList; + /** * Constructs a configuration import object. * @@ -190,8 +198,10 @@ class ConfigImporter { * The string translation service. * @param \Drupal\Core\Extension\ModuleExtensionList $extension_list_module * The module extension list. + * @param \Drupal\Core\Extension\ThemeExtensionList $extension_list_theme + * The theme extension list. */ - public function __construct(StorageComparerInterface $storage_comparer, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, LockBackendInterface $lock, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, ThemeHandlerInterface $theme_handler, TranslationInterface $string_translation, ModuleExtensionList $extension_list_module) { + public function __construct(StorageComparerInterface $storage_comparer, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, LockBackendInterface $lock, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, ThemeHandlerInterface $theme_handler, TranslationInterface $string_translation, ModuleExtensionList $extension_list_module, ThemeExtensionList $extension_list_theme = NULL) { $this->moduleExtensionList = $extension_list_module; $this->storageComparer = $storage_comparer; $this->eventDispatcher = $event_dispatcher; @@ -202,6 +212,11 @@ class ConfigImporter { $this->moduleInstaller = $module_installer; $this->themeHandler = $theme_handler; $this->stringTranslation = $string_translation; + if ($extension_list_theme === NULL) { + @trigger_error('Calling ' . __METHOD__ . ' without the $extension_list_theme argument is deprecated in drupal:10.1.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/3284397', E_USER_DEPRECATED); + $extension_list_theme = \Drupal::service('extension.list.theme'); + } + $this->themeExtensionList = $extension_list_theme; foreach ($this->storageComparer->getAllCollectionNames() as $collection) { $this->processedConfiguration[$collection] = $this->storageComparer->getEmptyChangelist(); } @@ -384,7 +399,7 @@ class ConfigImporter { $this->moduleExtensionList->reset(); // Get a list of modules with dependency weights as values. $module_data = $this->moduleExtensionList->getList(); - // Set the actual module weights. + // Use the actual module weights. $module_list = array_combine(array_keys($module_data), array_keys($module_data)); $module_list = array_map(function ($module) use ($module_data) { return $module_data[$module]->sort; @@ -429,9 +444,24 @@ class ConfigImporter { $this->extensionChangelist['module']['install'][] = $new_extensions['profile']; } + // Get a list of themes with dependency weights as values. + $theme_data = $this->themeExtensionList->getList(); + // Use the actual theme weights. + $theme_list = array_combine(array_keys($theme_data), array_keys($theme_data)); + $theme_list = array_map(function ($theme) use ($theme_data) { + return $theme_data[$theme]->sort; + }, $theme_list); + array_multisort(array_values($theme_list), SORT_ASC, array_keys($theme_list), SORT_DESC, $theme_list); + // Work out what themes to install and to uninstall. - $this->extensionChangelist['theme']['install'] = array_keys(array_diff_key($new_extensions['theme'], $current_extensions['theme'])); - $this->extensionChangelist['theme']['uninstall'] = array_keys(array_diff_key($current_extensions['theme'], $new_extensions['theme'])); + $uninstall = array_keys(array_diff_key($current_extensions['theme'], $new_extensions['theme'])); + $this->extensionChangelist['theme']['uninstall'] = array_intersect(array_keys($theme_list), $uninstall); + // Ensure that installed themes are sorted in exactly the reverse order + // (with dependencies installed first, and themes of the same weight sorted + // in alphabetical order). + $install = array_keys(array_diff_key($new_extensions['theme'], $current_extensions['theme'])); + $theme_list = array_reverse($theme_list); + $this->extensionChangelist['theme']['install'] = array_intersect(array_keys($theme_list), $install); } /** diff --git a/core/lib/Drupal/Core/EventSubscriber/ConfigImportSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ConfigImportSubscriber.php index 0547a14d3ac..38cdd4df3b5 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ConfigImportSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ConfigImportSubscriber.php @@ -199,17 +199,13 @@ class ConfigImportSubscriber extends ConfigImportValidateEventSubscriberBase { // 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])) { - $config_importer->logError($this->t('Unable to install the %theme theme since it does not exist.', ['%theme' => $theme])); - // Remove non-existing installs from the list so we can validate theme - // dependencies later. - unset($installs[$key]); - } + $nonexistent_themes = array_keys(array_diff_key($core_extension['theme'], $theme_data)); + foreach ($nonexistent_themes as $theme) { + $config_importer->logError($this->t('Unable to install the %theme theme since it does not exist.', ['%theme' => $theme])); } // Ensure that all themes being installed have their dependencies met. + $installs = $config_importer->getExtensionChangelist('theme', 'install'); foreach ($installs as $theme) { $module_dependencies = $theme_data[$theme]->module_dependencies; // $theme_data[$theme]->requires contains both theme and module diff --git a/core/modules/config/src/Form/ConfigSingleImportForm.php b/core/modules/config/src/Form/ConfigSingleImportForm.php index 88be9a58db9..316ee598fab 100644 --- a/core/modules/config/src/Form/ConfigSingleImportForm.php +++ b/core/modules/config/src/Form/ConfigSingleImportForm.php @@ -17,6 +17,7 @@ use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Extension\ModuleExtensionList; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ModuleInstallerInterface; +use Drupal\Core\Extension\ThemeExtensionList; use Drupal\Core\Extension\ThemeHandlerInterface; use Drupal\Core\Form\ConfirmFormBase; use Drupal\Core\Form\FormStateInterface; @@ -104,6 +105,13 @@ class ConfigSingleImportForm extends ConfirmFormBase { */ protected $moduleExtensionList; + /** + * The theme extension list. + * + * @var \Drupal\Core\Extension\ThemeExtensionList + */ + protected $themeExtensionList; + /** * The module installer. * @@ -150,8 +158,10 @@ class ConfigSingleImportForm extends ConfirmFormBase { * The theme handler. * @param \Drupal\Core\Extension\ModuleExtensionList $extension_list_module * The module extension list. + * @param \Drupal\Core\Extension\ThemeExtensionList $extension_list_theme + * The theme extension list. */ - public function __construct(EntityTypeManagerInterface $entity_type_manager, StorageInterface $config_storage, RendererInterface $renderer, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, LockBackendInterface $lock, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, ThemeHandlerInterface $theme_handler, ModuleExtensionList $extension_list_module) { + public function __construct(EntityTypeManagerInterface $entity_type_manager, StorageInterface $config_storage, RendererInterface $renderer, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, LockBackendInterface $lock, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, ThemeHandlerInterface $theme_handler, ModuleExtensionList $extension_list_module, ThemeExtensionList $extension_list_theme = NULL) { $this->entityTypeManager = $entity_type_manager; $this->configStorage = $config_storage; $this->renderer = $renderer; @@ -165,6 +175,11 @@ class ConfigSingleImportForm extends ConfirmFormBase { $this->moduleInstaller = $module_installer; $this->themeHandler = $theme_handler; $this->moduleExtensionList = $extension_list_module; + if ($extension_list_theme === NULL) { + @trigger_error('Calling ' . __METHOD__ . ' without the $extension_list_theme argument is deprecated in drupal:10.1.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/3284397', E_USER_DEPRECATED); + $extension_list_theme = \Drupal::service('extension.list.theme'); + } + $this->themeExtensionList = $extension_list_theme; } /** @@ -182,7 +197,8 @@ class ConfigSingleImportForm extends ConfirmFormBase { $container->get('module_handler'), $container->get('module_installer'), $container->get('theme_handler'), - $container->get('extension.list.module') + $container->get('extension.list.module'), + $container->get('extension.list.theme') ); } @@ -370,7 +386,8 @@ class ConfigSingleImportForm extends ConfirmFormBase { $this->moduleInstaller, $this->themeHandler, $this->getStringTranslation(), - $this->moduleExtensionList + $this->moduleExtensionList, + $this->themeExtensionList ); try { diff --git a/core/modules/config/src/Form/ConfigSync.php b/core/modules/config/src/Form/ConfigSync.php index f8613e77e58..af39f3cea2a 100644 --- a/core/modules/config/src/Form/ConfigSync.php +++ b/core/modules/config/src/Form/ConfigSync.php @@ -11,6 +11,7 @@ use Drupal\Core\Config\TypedConfigManagerInterface; use Drupal\Core\Extension\ModuleExtensionList; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ModuleInstallerInterface; +use Drupal\Core\Extension\ThemeExtensionList; use Drupal\Core\Extension\ThemeHandlerInterface; use Drupal\Core\Config\ConfigManagerInterface; use Drupal\Core\Form\FormBase; @@ -121,6 +122,13 @@ class ConfigSync extends FormBase { */ protected $importTransformer; + /** + * The theme extension list. + * + * @var \Drupal\Core\Extension\ThemeExtensionList + */ + protected $themeExtensionList; + /** * Constructs the object. * @@ -150,8 +158,10 @@ class ConfigSync extends FormBase { * The module extension list * @param \Drupal\Core\Config\ImportStorageTransformer $import_transformer * The import transformer service. + * @param \Drupal\Core\Extension\ThemeExtensionList $extension_list_theme + * The theme extension list. */ - public function __construct(StorageInterface $sync_storage, StorageInterface $active_storage, StorageInterface $snapshot_storage, LockBackendInterface $lock, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, ThemeHandlerInterface $theme_handler, RendererInterface $renderer, ModuleExtensionList $extension_list_module, ImportStorageTransformer $import_transformer) { + public function __construct(StorageInterface $sync_storage, StorageInterface $active_storage, StorageInterface $snapshot_storage, LockBackendInterface $lock, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, ThemeHandlerInterface $theme_handler, RendererInterface $renderer, ModuleExtensionList $extension_list_module, ImportStorageTransformer $import_transformer, ThemeExtensionList $extension_list_theme = NULL) { $this->syncStorage = $sync_storage; $this->activeStorage = $active_storage; $this->snapshotStorage = $snapshot_storage; @@ -165,6 +175,11 @@ class ConfigSync extends FormBase { $this->renderer = $renderer; $this->moduleExtensionList = $extension_list_module; $this->importTransformer = $import_transformer; + if ($extension_list_theme === NULL) { + @trigger_error('Calling ' . __METHOD__ . ' without the $extension_list_theme argument is deprecated in drupal:10.1.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/3284397', E_USER_DEPRECATED); + $extension_list_theme = \Drupal::service('extension.list.theme'); + } + $this->themeExtensionList = $extension_list_theme; } /** @@ -184,7 +199,8 @@ class ConfigSync extends FormBase { $container->get('theme_handler'), $container->get('renderer'), $container->get('extension.list.module'), - $container->get('config.import_transformer') + $container->get('config.import_transformer'), + $container->get('extension.list.theme') ); } @@ -357,7 +373,8 @@ class ConfigSync extends FormBase { $this->moduleInstaller, $this->themeHandler, $this->getStringTranslation(), - $this->moduleExtensionList + $this->moduleExtensionList, + $this->themeExtensionList ); if ($config_importer->alreadyImporting()) { $this->messenger()->addStatus($this->t('Another request may be synchronizing configuration already.')); diff --git a/core/modules/config/tests/src/Kernel/ConfigUninstallViaCliImportTest.php b/core/modules/config/tests/src/Kernel/ConfigUninstallViaCliImportTest.php index e9de754b827..58c966aa2e6 100644 --- a/core/modules/config/tests/src/Kernel/ConfigUninstallViaCliImportTest.php +++ b/core/modules/config/tests/src/Kernel/ConfigUninstallViaCliImportTest.php @@ -53,7 +53,8 @@ class ConfigUninstallViaCliImportTest extends KernelTestBase { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); } diff --git a/core/modules/content_translation/tests/src/Kernel/ContentTranslationConfigImportTest.php b/core/modules/content_translation/tests/src/Kernel/ContentTranslationConfigImportTest.php index fb6ab7dcbd0..ae1428a00bb 100644 --- a/core/modules/content_translation/tests/src/Kernel/ContentTranslationConfigImportTest.php +++ b/core/modules/content_translation/tests/src/Kernel/ContentTranslationConfigImportTest.php @@ -58,7 +58,8 @@ class ContentTranslationConfigImportTest extends KernelTestBase { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); } diff --git a/core/modules/language/tests/src/Kernel/OverriddenConfigImportTest.php b/core/modules/language/tests/src/Kernel/OverriddenConfigImportTest.php index 60468729523..db7b1cc60fc 100644 --- a/core/modules/language/tests/src/Kernel/OverriddenConfigImportTest.php +++ b/core/modules/language/tests/src/Kernel/OverriddenConfigImportTest.php @@ -49,7 +49,8 @@ class OverriddenConfigImportTest extends KernelTestBase { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); } diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRecreateTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRecreateTest.php index 6b16e077b5d..c65a9004b8c 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRecreateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRecreateTest.php @@ -54,7 +54,8 @@ class ConfigImportRecreateTest extends KernelTestBase { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); } diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRenameValidationTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRenameValidationTest.php index b4ec44cacb9..5dffd1c93d6 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRenameValidationTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigImportRenameValidationTest.php @@ -63,7 +63,8 @@ class ConfigImportRenameValidationTest extends KernelTestBase { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); } diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterMissingContentTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterMissingContentTest.php index b9b1fd3392c..9ef75f587e1 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterMissingContentTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterMissingContentTest.php @@ -65,7 +65,8 @@ class ConfigImporterMissingContentTest extends KernelTestBase { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); } diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php index 34839e53b6e..b252b287298 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php @@ -687,6 +687,37 @@ class ConfigImporterTest extends KernelTestBase { } } + /** + * Tests installing a base themes and sub themes during configuration import. + * + * @see \Drupal\Core\EventSubscriber\ConfigImportSubscriber + */ + public function testInstallBaseAndSubThemes() { + $sync = $this->container->get('config.storage.sync'); + $extensions = $sync->read('core.extension'); + $extensions['theme']['test_basetheme'] = 0; + $extensions['theme']['test_subtheme'] = 0; + $extensions['theme']['test_subsubtheme'] = 0; + $sync->write('core.extension', $extensions); + $config_importer = $this->configImporter(); + $config_importer->import(); + $this->assertTrue($this->container->get('theme_handler')->themeExists('test_basetheme')); + $this->assertTrue($this->container->get('theme_handler')->themeExists('test_subsubtheme')); + $this->assertTrue($this->container->get('theme_handler')->themeExists('test_subtheme')); + + // Test uninstalling them. + $extensions = $sync->read('core.extension'); + unset($extensions['theme']['test_basetheme']); + unset($extensions['theme']['test_subsubtheme']); + unset($extensions['theme']['test_subtheme']); + $sync->write('core.extension', $extensions); + $config_importer = $this->configImporter(); + $config_importer->import(); + $this->assertFalse($this->container->get('theme_handler')->themeExists('test_basetheme')); + $this->assertFalse($this->container->get('theme_handler')->themeExists('test_subsubtheme')); + $this->assertFalse($this->container->get('theme_handler')->themeExists('test_subtheme')); + } + /** * Tests install profile validation during configuration import. * diff --git a/core/tests/Drupal/KernelTests/Core/Entity/ContentEntityNullStorageTest.php b/core/tests/Drupal/KernelTests/Core/Entity/ContentEntityNullStorageTest.php index 8196ffb5550..d9e78f31323 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/ContentEntityNullStorageTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/ContentEntityNullStorageTest.php @@ -64,7 +64,8 @@ class ContentEntityNullStorageTest extends KernelTestBase { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); // Delete the contact message in sync. diff --git a/core/tests/Drupal/Tests/ConfigTestTrait.php b/core/tests/Drupal/Tests/ConfigTestTrait.php index 139f7f36270..7ecc541772a 100644 --- a/core/tests/Drupal/Tests/ConfigTestTrait.php +++ b/core/tests/Drupal/Tests/ConfigTestTrait.php @@ -37,7 +37,8 @@ trait ConfigTestTrait { $this->container->get('module_installer'), $this->container->get('theme_handler'), $this->container->get('string_translation'), - $this->container->get('extension.list.module') + $this->container->get('extension.list.module'), + $this->container->get('extension.list.theme') ); } // Always recalculate the changelist when called.