From dc80da2746ed52bbf5b4b456a29df7940504c538 Mon Sep 17 00:00:00 2001 From: catch Date: Mon, 6 Jun 2022 14:35:46 +0100 Subject: [PATCH] Issue #3284270 by alexpott, bircher: Reset \Drupal\Core\Config\ConfigImporter::$errors in ::validate() method (cherry picked from commit 51ab247c8b742db312e3a0f9249029bfc59a1ee0) --- .../lib/Drupal/Core/Config/ConfigImporter.php | 1 + .../ContentModerationWorkflowConfigTest.php | 3 +- .../Core/Config/ConfigImporterTest.php | 126 ++++++++---------- 3 files changed, 55 insertions(+), 75 deletions(-) diff --git a/core/lib/Drupal/Core/Config/ConfigImporter.php b/core/lib/Drupal/Core/Config/ConfigImporter.php index 4d998fa959c..1ddb3960d3c 100644 --- a/core/lib/Drupal/Core/Config/ConfigImporter.php +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php @@ -729,6 +729,7 @@ class ConfigImporter { */ public function validate() { if (!$this->validated) { + $this->errors = []; // Create the list of installs and uninstalls. $this->createExtensionChangelist(); // Validate renames. diff --git a/core/modules/content_moderation/tests/src/Kernel/ContentModerationWorkflowConfigTest.php b/core/modules/content_moderation/tests/src/Kernel/ContentModerationWorkflowConfigTest.php index 328e1426116..ee8c6e6cae5 100644 --- a/core/modules/content_moderation/tests/src/Kernel/ContentModerationWorkflowConfigTest.php +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationWorkflowConfigTest.php @@ -134,10 +134,9 @@ class ContentModerationWorkflowConfigTest extends KernelTestBase { $this->fail('ConfigImporterException not thrown, invalid import was not stopped due to deleted workflow.'); } catch (ConfigImporterException $e) { - $this->assertEquals('There were errors validating the config synchronization.' . PHP_EOL . 'The moderation state Test two is being used, but is not in the source storage.' . PHP_EOL . 'The workflow Editorial is being used, and cannot be deleted.', $e->getMessage()); + $this->assertEquals('There were errors validating the config synchronization.' . PHP_EOL . 'The workflow Editorial is being used, and cannot be deleted.', $e->getMessage()); $error_log = $this->configImporter->getErrors(); $expected = [ - 'The moderation state Test two is being used, but is not in the source storage.', 'The workflow Editorial is being used, and cannot be deleted.', ]; $this->assertEquals($expected, $error_log); diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php index 14a51290b2e..4d16c5709bc 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php @@ -6,7 +6,6 @@ use Drupal\Component\Utility\Html; use Drupal\Component\Render\FormattableMarkup; use Drupal\Core\Config\ConfigImporter; use Drupal\Core\Config\ConfigImporterException; -use Drupal\Core\Config\StorageComparer; use Drupal\KernelTests\KernelTestBase; /** @@ -21,13 +20,6 @@ class ConfigImporterTest extends KernelTestBase { */ const FAIL_MESSAGE = 'There were errors validating the config synchronization.'; - /** - * Config Importer object used for testing. - * - * @var \Drupal\Core\Config\ConfigImporter - */ - protected $configImporter; - /** * Modules to enable. * @@ -45,24 +37,6 @@ class ConfigImporterTest extends KernelTestBase { unset($GLOBALS['hook_config_test']); $this->copyConfig($this->container->get('config.storage'), $this->container->get('config.storage.sync')); - - // Set up the ConfigImporter object for testing. - $storage_comparer = new StorageComparer( - $this->container->get('config.storage.sync'), - $this->container->get('config.storage') - ); - $this->configImporter = new ConfigImporter( - $storage_comparer->createChangelist(), - $this->container->get('event_dispatcher'), - $this->container->get('config.manager'), - $this->container->get('lock'), - $this->container->get('config.typed'), - $this->container->get('module_handler'), - $this->container->get('module_installer'), - $this->container->get('theme_handler'), - $this->container->get('string_translation'), - $this->container->get('extension.list.module') - ); } /** @@ -86,7 +60,7 @@ class ConfigImporterTest extends KernelTestBase { public function testEmptyImportFails() { $this->expectException(ConfigImporterException::class); $this->container->get('config.storage.sync')->deleteAll(); - $this->configImporter->reset()->import(); + $this->configImporter()->import(); } /** @@ -99,14 +73,15 @@ class ConfigImporterTest extends KernelTestBase { // Generate a new site UUID. $config_data['uuid'] = \Drupal::service('uuid')->generate(); $sync->write('system.site', $config_data); + $config_importer = $this->configImporter(); try { - $this->configImporter->reset()->import(); + $config_importer->import(); $this->fail('ConfigImporterException not thrown, invalid import was not stopped due to mis-matching site UUID.'); } catch (ConfigImporterException $e) { $actual_message = $e->getMessage(); - $actual_error_log = $this->configImporter->getErrors(); + $actual_error_log = $config_importer->getErrors(); $expected_error_log = ['Site UUID in source storage does not match the target storage.']; $this->assertEquals($expected_error_log, $actual_error_log); @@ -134,7 +109,8 @@ class ConfigImporterTest extends KernelTestBase { $sync->delete($dynamic_name); // Import. - $this->configImporter->reset()->import(); + $config_importer = $this->configImporter(); + $config_importer->import(); // Verify the file has been removed. $this->assertFalse($storage->read($dynamic_name)); @@ -150,8 +126,8 @@ class ConfigImporterTest extends KernelTestBase { $this->assertTrue(isset($GLOBALS['hook_config_test']['predelete'])); $this->assertTrue(isset($GLOBALS['hook_config_test']['delete'])); - $this->assertFalse($this->configImporter->hasUnprocessedConfigurationChanges()); - $logs = $this->configImporter->getErrors(); + $this->assertFalse($config_importer->hasUnprocessedConfigurationChanges()); + $logs = $config_importer->getErrors(); $this->assertCount(0, $logs); } @@ -185,7 +161,8 @@ class ConfigImporterTest extends KernelTestBase { $this->assertTrue($sync->exists($dynamic_name), $dynamic_name . ' found.'); // Import. - $this->configImporter->reset()->import(); + $config_importer = $this->configImporter(); + $config_importer->import(); // Verify the values appeared. $config = $this->config($dynamic_name); @@ -204,8 +181,8 @@ class ConfigImporterTest extends KernelTestBase { $this->assertTrue(isset($GLOBALS['hook_config_test']['config_import_steps_alter'])); // Verify that there is nothing more to import. - $this->assertFalse($this->configImporter->hasUnprocessedConfigurationChanges()); - $logs = $this->configImporter->getErrors(); + $this->assertFalse($config_importer->hasUnprocessedConfigurationChanges()); + $logs = $config_importer->getErrors(); $this->assertCount(0, $logs); } @@ -238,7 +215,8 @@ class ConfigImporterTest extends KernelTestBase { $sync->write($name_secondary, $values_secondary); // Import. - $this->configImporter->reset()->import(); + $config_importer = $this->configImporter(); + $config_importer->import(); $entity_storage = \Drupal::entityTypeManager()->getStorage('config_test'); $primary = $entity_storage->load('primary'); @@ -250,7 +228,7 @@ class ConfigImporterTest extends KernelTestBase { $this->assertEquals($values_secondary['uuid'], $secondary->uuid()); $this->assertEquals($values_secondary['label'], $secondary->label()); - $logs = $this->configImporter->getErrors(); + $logs = $config_importer->getErrors(); $this->assertCount(1, $logs); $this->assertEquals(new FormattableMarkup('Deleted and replaced configuration entity "@name"', ['@name' => $name_secondary]), $logs[0]); } @@ -284,7 +262,8 @@ class ConfigImporterTest extends KernelTestBase { $sync->write($name_secondary, $values_secondary); // Import. - $this->configImporter->reset()->import(); + $config_importer = $this->configImporter(); + $config_importer->import(); $entity_storage = \Drupal::entityTypeManager()->getStorage('config_test'); $primary = $entity_storage->load('primary'); @@ -296,7 +275,7 @@ class ConfigImporterTest extends KernelTestBase { $this->assertEquals($values_secondary['uuid'], $secondary->uuid()); $this->assertEquals($values_secondary['label'], $secondary->label()); - $logs = $this->configImporter->getErrors(); + $logs = $config_importer->getErrors(); $this->assertCount(1, $logs); $this->assertEquals(Html::escape("Unexpected error during import with operation create for {$name_primary}: 'config_test' entity with ID 'secondary' already exists."), $logs[0]); } @@ -352,7 +331,8 @@ class ConfigImporterTest extends KernelTestBase { $sync->write($name_other, $values_other); // Check update changelist order. - $updates = $this->configImporter->reset()->getStorageComparer()->getChangelist('update'); + $config_importer = $this->configImporter(); + $updates = $config_importer->getStorageComparer()->getChangelist('update'); $expected = [ $name_deleter, $name_deletee, @@ -361,7 +341,7 @@ class ConfigImporterTest extends KernelTestBase { $this->assertSame($expected, $updates); // Import. - $this->configImporter->import(); + $config_importer->import(); $entity_storage = \Drupal::entityTypeManager()->getStorage('config_test'); $deleter = $entity_storage->load('deleter'); @@ -378,7 +358,7 @@ class ConfigImporterTest extends KernelTestBase { $this->assertEquals($values_other['uuid'], $other->uuid()); $this->assertEquals($values_other['label'], $other->label()); - $logs = $this->configImporter->getErrors(); + $logs = $config_importer->getErrors(); $this->assertCount(1, $logs); $this->assertEquals(new FormattableMarkup('Update target "@name" is missing.', ['@name' => $name_deletee]), $logs[0]); } @@ -421,7 +401,8 @@ class ConfigImporterTest extends KernelTestBase { $sync->write($name_deletee, $values_deletee); // Import. - $this->configImporter->reset()->import(); + $config_importer = $this->configImporter(); + $config_importer->import(); $entity_storage = \Drupal::entityTypeManager()->getStorage('config_test'); // Both entities are deleted. ConfigTest::postSave() causes updates of the @@ -429,7 +410,7 @@ class ConfigImporterTest extends KernelTestBase { // the deletee, removing the deletee causes the deleter to be removed. $this->assertNull($entity_storage->load('deleter')); $this->assertNull($entity_storage->load('deletee')); - $logs = $this->configImporter->getErrors(); + $logs = $config_importer->getErrors(); $this->assertCount(0, $logs); } @@ -463,7 +444,8 @@ class ConfigImporterTest extends KernelTestBase { $storage->write($name_deletee, $values_deletee); // Import. - $this->configImporter->reset()->import(); + $config_importer = $this->configImporter(); + $config_importer->import(); $entity_storage = \Drupal::entityTypeManager()->getStorage('config_test'); $this->assertNull($entity_storage->load('deleter')); @@ -471,7 +453,7 @@ class ConfigImporterTest extends KernelTestBase { // The deletee entity does not exist as the delete worked and although the // delete occurred in \Drupal\config_test\Entity\ConfigTest::postDelete() // this does not matter. - $logs = $this->configImporter->getErrors(); + $logs = $config_importer->getErrors(); $this->assertCount(0, $logs); } @@ -505,7 +487,8 @@ class ConfigImporterTest extends KernelTestBase { $this->assertSame('Default', $config->get('label')); // Import. - $this->configImporter->reset()->import(); + $config_importer = $this->configImporter(); + $config_importer->import(); // Verify the values were updated. \Drupal::configFactory()->reset($name); @@ -527,8 +510,8 @@ class ConfigImporterTest extends KernelTestBase { $this->assertFalse(isset($GLOBALS['hook_config_test']['delete'])); // Verify that there is nothing more to import. - $this->assertFalse($this->configImporter->hasUnprocessedConfigurationChanges()); - $logs = $this->configImporter->getErrors(); + $this->assertFalse($config_importer->hasUnprocessedConfigurationChanges()); + $logs = $config_importer->getErrors(); $this->assertCount(0, $logs); } @@ -582,8 +565,9 @@ class ConfigImporterTest extends KernelTestBase { $extensions['theme']['test_subtheme'] = 0; $sync->write('core.extension', $extensions); + $config_importer = $this->configImporter(); try { - $this->configImporter->reset()->import(); + $config_importer->import(); $this->fail('ConfigImporterException not thrown; an invalid import was not stopped due to missing dependencies.'); } catch (ConfigImporterException $e) { @@ -600,7 +584,7 @@ class ConfigImporterTest extends KernelTestBase { 'Configuration unknown.config depends on the unknown extension that will not be installed after import.', ]; $this->assertEquals(implode(PHP_EOL, $expected), $e->getMessage()); - $error_log = $this->configImporter->getErrors(); + $error_log = $config_importer->getErrors(); $expected = [ 'Unable to install the unknown_module module since it does not exist.', 'Unable to install the Book module since it requires the Node, Text, Field, Filter, User modules.', @@ -624,8 +608,9 @@ class ConfigImporterTest extends KernelTestBase { $sync->write('config_test.dynamic.dotted.theme', $config_entity_data); $config_entity_data['dependencies'] = ['config' => ['unknown', 'unknown2']]; $sync->write('config_test.dynamic.dotted.config', $config_entity_data); + $config_importer = $this->configImporter(); try { - $this->configImporter->reset()->import(); + $this->configImporter->import(); $this->fail('ConfigImporterException not thrown, invalid import was not stopped due to missing dependencies.'); } catch (ConfigImporterException $e) { @@ -635,15 +620,6 @@ class ConfigImporterTest extends KernelTestBase { 'Unable to install the Book module since it requires the Node, Text, Field, Filter, User modules.', 'Unable to install the unknown_theme theme since it does not exist.', 'Unable to install the Theme test subtheme theme since it requires the Theme test base theme theme.', - 'Configuration config_test.dynamic.dotted.config depends on the unknown configuration that will not exist after import.', - 'Configuration config_test.dynamic.dotted.existing depends on the config_test.dynamic.dotted.deleted configuration that will not exist after import.', - 'Configuration config_test.dynamic.dotted.module depends on the unknown module that will not be installed after import.', - 'Configuration config_test.dynamic.dotted.theme depends on the unknown theme that will not be installed after import.', - 'Configuration unknown.config depends on the unknown extension that will not be installed after import.', - 'Unable to install the unknown_module module since it does not exist.', - 'Unable to install the Book module since it requires the Node, Text, Field, Filter, User modules.', - 'Unable to install the unknown_theme theme since it does not exist.', - 'Unable to install the Theme test subtheme theme since it requires the Theme test base theme theme.', 'Configuration config_test.dynamic.dotted.config depends on configuration (unknown, unknown2) that will not exist after import.', 'Configuration config_test.dynamic.dotted.existing depends on the config_test.dynamic.dotted.deleted configuration that will not exist after import.', 'Configuration config_test.dynamic.dotted.module depends on modules (unknown, Database Logging) that will not be installed after import.', @@ -651,7 +627,7 @@ class ConfigImporterTest extends KernelTestBase { 'Configuration unknown.config depends on the unknown extension that will not be installed after import.', ]; $this->assertEquals(implode(PHP_EOL, $expected), $e->getMessage()); - $error_log = $this->configImporter->getErrors(); + $error_log = $config_importer->getErrors(); $expected = [ 'Configuration config_test.dynamic.dotted.config depends on configuration (unknown, unknown2) that will not exist after import.', 'Configuration config_test.dynamic.dotted.module depends on modules (unknown, Database Logging) that will not be installed after import.', @@ -671,14 +647,15 @@ class ConfigImporterTest extends KernelTestBase { public function testMissingCoreExtension() { $sync = $this->container->get('config.storage.sync'); $sync->delete('core.extension'); + $config_importer = $this->configImporter(); try { - $this->configImporter->reset()->import(); + $config_importer->import(); $this->fail('ConfigImporterException not thrown, invalid import was not stopped due to missing dependencies.'); } catch (ConfigImporterException $e) { $expected = static::FAIL_MESSAGE . PHP_EOL . 'The core.extension configuration does not exist.'; $this->assertEquals($expected, $e->getMessage()); - $error_log = $this->configImporter->getErrors(); + $error_log = $config_importer->getErrors(); $this->assertEquals(['The core.extension configuration does not exist.'], $error_log); } } @@ -696,14 +673,15 @@ class ConfigImporterTest extends KernelTestBase { $extensions['module']['standard'] = 0; $sync->write('core.extension', $extensions); + $config_importer = $this->configImporter(); try { - $this->configImporter->reset()->import(); + $config_importer->import(); $this->fail('ConfigImporterException not thrown; an invalid import was not stopped due to missing dependencies.'); } catch (ConfigImporterException $e) { $expected = static::FAIL_MESSAGE . PHP_EOL . 'Unable to install the standard module since it does not exist.'; $this->assertEquals($expected, $e->getMessage(), 'There were errors validating the config synchronization.'); - $error_log = $this->configImporter->getErrors(); + $error_log = $config_importer->getErrors(); // Install profiles should not even be scanned at this point. $this->assertEquals(['Unable to install the standard module since it does not exist.'], $error_log); } @@ -722,14 +700,15 @@ class ConfigImporterTest extends KernelTestBase { $extensions['profile'] = 'this_will_not_work'; $sync->write('core.extension', $extensions); + $config_importer = $this->configImporter(); try { - $this->configImporter->reset()->import(); + $config_importer->import(); $this->fail('ConfigImporterException not thrown; an invalid import was not stopped due to missing dependencies.'); } catch (ConfigImporterException $e) { $expected = static::FAIL_MESSAGE . PHP_EOL . 'Cannot change the install profile from to this_will_not_work once Drupal is installed.'; $this->assertEquals($expected, $e->getMessage(), 'There were errors validating the config synchronization.'); - $error_log = $this->configImporter->getErrors(); + $error_log = $config_importer->getErrors(); // Install profiles can not be changed. Note that KernelTestBase currently // does not use an install profile. This situation should be impossible // to get in but site's can removed the install profile setting from @@ -752,7 +731,7 @@ class ConfigImporterTest extends KernelTestBase { // Delete the config so that create hooks will fire. $storage->delete($dynamic_name); \Drupal::state()->set('config_test.store_isSyncing', []); - $this->configImporter->reset()->import(); + $this->configImporter()->import(); // The values of the syncing values should be stored in state by // config_test_config_test_create(). @@ -768,7 +747,7 @@ class ConfigImporterTest extends KernelTestBase { $config = $this->config($dynamic_name); $config->set('label', 'A new name')->save(); \Drupal::state()->set('config_test.store_isSyncing', []); - $this->configImporter->reset()->import(); + $this->configImporter()->import(); // The values of the syncing values should be stored in state by // config_test_config_test_create(). @@ -782,7 +761,7 @@ class ConfigImporterTest extends KernelTestBase { $sync = $this->container->get('config.storage.sync'); $sync->delete($dynamic_name); \Drupal::state()->set('config_test.store_isSyncing', []); - $this->configImporter->reset()->import(); + $this->configImporter()->import(); // The values of the syncing values should be stored in state by // config_test_config_test_create(). @@ -799,8 +778,9 @@ class ConfigImporterTest extends KernelTestBase { public function testInvalidStep() { $this->assertFalse(\Drupal::isConfigSyncing(), 'Before an import \Drupal::isConfigSyncing() returns FALSE'); $context = []; + $config_importer = $this->configImporter(); try { - $this->configImporter->doSyncStep('a_non_existent_step', $context); + $config_importer->doSyncStep('a_non_existent_step', $context); $this->fail('Expected \InvalidArgumentException thrown'); } catch (\InvalidArgumentException $e) { @@ -815,7 +795,7 @@ class ConfigImporterTest extends KernelTestBase { public function testCustomStep() { $this->assertFalse(\Drupal::isConfigSyncing(), 'Before an import \Drupal::isConfigSyncing() returns FALSE'); $context = []; - $this->configImporter->doSyncStep([self::class, 'customStep'], $context); + $this->configImporter()->doSyncStep([self::class, 'customStep'], $context); $this->assertTrue($context['is_syncing'], 'Inside a custom step \Drupal::isConfigSyncing() returns TRUE'); $this->assertFalse(\Drupal::isConfigSyncing(), 'After an valid custom step \Drupal::isConfigSyncing() returns FALSE'); }