diff --git a/core/lib/Drupal/Core/Config/BatchConfigImporter.php b/core/lib/Drupal/Core/Config/BatchConfigImporter.php index 67aee13..a57a076 100644 --- a/core/lib/Drupal/Core/Config/BatchConfigImporter.php +++ b/core/lib/Drupal/Core/Config/BatchConfigImporter.php @@ -108,7 +108,9 @@ public function processConfigurationBatch(array &$context) { } $operation = $this->getNextConfigurationOperation(); if (!empty($operation)) { - $this->processConfiguration($operation['op'], $operation['name']); + if ($this->checkOp($operation['op'], $operation['name'])) { + $this->processConfiguration($operation['op'], $operation['name']); + } $context['message'] = t('Synchronizing configuration: @op @name.', array('@op' => $operation['op'], '@name' => $operation['name'])); $processed_count = count($this->processedConfiguration['create']) + count($this->processedConfiguration['delete']) + count($this->processedConfiguration['update']); $context['finished'] = $processed_count / $this->totalConfigurationToProcess; diff --git a/core/lib/Drupal/Core/Config/ConfigImporter.php b/core/lib/Drupal/Core/Config/ConfigImporter.php index 9bf424b..3b2e232 100644 --- a/core/lib/Drupal/Core/Config/ConfigImporter.php +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php @@ -127,11 +127,11 @@ class ConfigImporter extends DependencySerialization { protected $processedSystemTheme = FALSE; /** - * The config importer log. + * List of errors that were logged during a config import. * - * @var \Drupal\Core\Config\ConfigImporterLogInterface + * @var array */ - protected $log; + protected $errors = array(); /** * Constructs a configuration import object. @@ -165,32 +165,23 @@ public function __construct(StorageComparerInterface $storage_comparer, EventDis } /** - * Sets the importer log that should be used. - * - * @param \Drupal\Core\Config\ConfigImporterLogInterface $log - */ - public function setLog(ConfigImporterLogInterface $log = NULL) { - $this->log = $log; - } - - /** - * Logs a message to the importer log when one has been set. + * Logs an error message. * * @param string $message * The message to log. - * @param string $severity - * The log severity, error, warning or info. + */ + protected function logError($message) { + $this->errors[] = $message; + } + + /** + * Returns error messages created while running the import. * - * @return bool - * TRUE when the log is available and the message has been logged, FALSE - * otherwise. + * @return array + * List of messages. */ - protected function log($message, $severity) { - if ($this->log) { - $this->log->log($message, $severity); - return TRUE; - } - return FALSE; + public function getErrors() { + return $this->errors; } /** @@ -503,13 +494,10 @@ protected function processConfiguration($op, $name) { } } catch (\Exception $e) { - if (!$this->log($e->getMessage(), 'error')) { - throw $e; - } - else { - // Error for that operation was logged, mark it as processed. - $this->setProcessedConfiguration($op, $name); - } + $this->logError(String::format('Unexpected error during import with operation @op for @name: @message', array('@op' => $op, '@name' => $name, '@message' => $e->getMessage()))); + // Error for that operation was logged, mark it as processed so that + // the import can continue. + $this->setProcessedConfiguration($op, $name); } } @@ -597,7 +585,7 @@ protected function checkOp($op, $name) { $entity_type = $this->configManager->getEntityManager()->getDefinition($entity_type_id); $entity = $entity_storage->load($entity_storage->getIDFromConfigName($name, $entity_type->getConfigPrefix())); $entity->delete(); - $this->log(String::format('Deleted and replaced configuration entity "@name"', array('@name' => $name)), 'warning'); + $this->logError(String::format('Deleted and replaced configuration entity "@name"', array('@name' => $name))); } else { $this->storageComparer->getTargetStorage()->delete($name); @@ -608,7 +596,7 @@ protected function checkOp($op, $name) { case 'update': if (!$target_exists) { - $this->log(String::format('Update target "@name" is missing.', array('@name' => $name)), 'error'); + $this->logError(String::format('Update target "@name" is missing.', array('@name' => $name))); return FALSE; } break; diff --git a/core/lib/Drupal/Core/Config/ConfigImporterLogArray.php b/core/lib/Drupal/Core/Config/ConfigImporterLogArray.php deleted file mode 100644 index 7f9eb26..0000000 --- a/core/lib/Drupal/Core/Config/ConfigImporterLogArray.php +++ /dev/null @@ -1,38 +0,0 @@ -logs[] = array( - 'message' => $message, - 'severity' => $severity, - ); - } - - /** - * {@inheritdoc} - */ - public function getLogMessages() { - return $this->logs; - } - -} diff --git a/core/lib/Drupal/Core/Config/ConfigImporterLogInterface.php b/core/lib/Drupal/Core/Config/ConfigImporterLogInterface.php deleted file mode 100644 index cd461fb..0000000 --- a/core/lib/Drupal/Core/Config/ConfigImporterLogInterface.php +++ /dev/null @@ -1,19 +0,0 @@ -state = $state; - } - - /** - * {@inheritdoc} - */ - public function log($message, $severity) { - $logs = $this->state->get(static::STATE_KEY, array()); - $logs[] = array( - 'message' => $message, - 'severity' => $severity, - ); - $this->state->set(static::STATE_KEY, $logs); - } - - /** - * {@inheritdoc} - */ - public function getLogMessages() { - return $this->state->get(static::STATE_KEY, array()); - } - -} diff --git a/core/modules/config/lib/Drupal/config/Form/ConfigSync.php b/core/modules/config/lib/Drupal/config/Form/ConfigSync.php index 4dffd54..4906cfc 100644 --- a/core/modules/config/lib/Drupal/config/Form/ConfigSync.php +++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.php @@ -7,7 +7,8 @@ namespace Drupal\config\Form; -use Drupal\Core\Config\ConfigImporterLogArray; +use Drupal\Component\Uuid\UuidInterface; +use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ThemeHandlerInterface; use Drupal\Core\Config\ConfigManagerInterface; @@ -110,8 +111,6 @@ class ConfigSync extends FormBase { * The module handler * @param \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler * The theme handler - * @param \Drupal\Core\KeyValueStore\StateInterface $state - * The state object. */ public function __construct(StorageInterface $sourceStorage, StorageInterface $targetStorage, LockBackendInterface $lock, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, UrlGeneratorInterface $url_generator, TypedConfigManager $typed_config, ModuleHandlerInterface $module_handler, ThemeHandlerInterface $theme_handler) { $this->sourceStorage = $sourceStorage; @@ -289,14 +288,12 @@ public static function processBatch(BatchConfigImporter $config_importer, $opera } $config_importer = $context['sandbox']['config_importer']; - $log = new ConfigImporterLogArray(); - $config_importer->setLog($log); $config_importer->$operation($context); - $config_importer->setLog(NULL); - foreach ($log->getLogMessages() as $log) { - drupal_set_message(t('Error while importing configuration: %error.', array('%error' => $log['message'])), $log['severity']); - // @todo Improve severity, use constants? - watchdog('config_sync', 'Error while importing configuration: %error.', array('%error' => $log['message']), WATCHDOG_ERROR); + if ($errors = $config_importer->getErrors()) { + if (!isset($context['results']['errors'])) { + $context['results']['errors'] = array(); + } + $context['results']['errors'] += $errors; } } @@ -308,7 +305,16 @@ public static function processBatch(BatchConfigImporter $config_importer, $opera */ public static function finishBatch($success, $results, $operations) { if ($success) { - drupal_set_message(\Drupal::translation()->translate('The configuration was imported successfully.')); + if (!empty($results['errors'])) { + foreach ($results['errors'] as $error) { + drupal_set_message($error, 'error'); + watchdog('config_sync', $error, NULL, WATCHDOG_ERROR); + } + drupal_set_message(\Drupal::translation()->translate('The configuration was imported with errors.'), 'warning'); + } + else { + drupal_set_message(\Drupal::translation()->translate('The configuration was imported successfully.')); + } } else { // An error occurred. diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php index 9494348..94f35fb 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php @@ -7,6 +7,7 @@ namespace Drupal\config\Tests; +use Drupal\Component\Utility\String; use Drupal\Core\Config\InstallStorage; use Drupal\simpletest\WebTestBase; @@ -326,18 +327,29 @@ function testImportErrorLog() { 'id' => 'primary', 'label' => 'Primary', 'weight' => 0, + 'style' => NULL, + 'test_dependencies' => array(), + 'status' => TRUE, 'uuid' => $uuid->generate(), + 'langcode' => 'en', + 'dependencies' => array(), + 'protected_property' => null, ); $staging->write($name_primary, $values_primary); $values_secondary = array( 'id' => 'secondary', 'label' => 'Secondary Sync', 'weight' => 0, + 'style' => NULL, + 'test_dependencies' => array(), + 'status' => TRUE, 'uuid' => $uuid->generate(), + 'langcode' => 'en', // Add a dependency on primary, to ensure that is synced first. 'dependencies' => array( 'entity' => array($name_primary), - ) + ), + 'protected_property' => null, ); $staging->write($name_secondary, $values_secondary); // Verify that there are configuration differences to import. @@ -346,9 +358,10 @@ function testImportErrorLog() { // Attempt to import configuration and verify that an error message appears. $this->drupalPostForm(NULL, array(), t('Import all')); - $this->assertRaw(t('Error while importing configuration: %error.', array('%error' => 'config_test entity with ID secondary already exists.'))); - $this->assertText(t('1 new')); - $this->assertText(t('1 removed')); + $this->assertText(String::format('Deleted and replaced configuration entity "@name"', array('@name' => $name_secondary))); + $this->assertText(t('The configuration was imported with errors.')); + $this->assertNoText(t('The configuration was imported successfully.')); + $this->assertText(t('There are no configuration changes.')); } } diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php index 2c53a61..882f7b1 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php @@ -7,11 +7,10 @@ namespace Drupal\config\Tests; +use Drupal\Component\Utility\String; use Drupal\Core\Config\ConfigImporter; use Drupal\Core\Config\ConfigImporterException; -use Drupal\Core\Config\ConfigImporterLogArray; use Drupal\Core\Config\StorageComparer; -use Drupal\Component\Utility\String; use Drupal\simpletest\DrupalUnitTestBase; /** @@ -27,13 +26,6 @@ class ConfigImporterTest extends DrupalUnitTestBase { protected $configImporter; /** - * Config importer log. - * - * @var \Drupal\Core\Config\ConfigImporterLogInterface - */ - protected $configLog; - - /** * Modules to enable. * * @var array @@ -75,9 +67,6 @@ function setUp() { $this->container->get('module_handler'), $this->container->get('theme_handler') ); - $this->configLog = new ConfigImporterLogArray(); - $this->configImporter->setLog($this->configLog); - $this->copyConfig($this->container->get('config.storage'), $this->container->get('config.storage.staging')); } /** @@ -161,7 +150,7 @@ function testDeleted() { $this->assertTrue(isset($GLOBALS['hook_config_test']['delete'])); $this->assertFalse($this->configImporter->hasUnprocessedConfigurationChanges()); - $logs = $this->configLog->getLogMessages(); + $logs = $this->configImporter->getErrors(); $this->assertEqual(count($logs), 0); } @@ -210,7 +199,7 @@ function testNew() { // Verify that there is nothing more to import. $this->assertFalse($this->configImporter->hasUnprocessedConfigurationChanges()); - $logs = $this->configLog->getLogMessages(); + $logs = $this->configImporter->getErrors(); $this->assertEqual(count($logs), 0); } @@ -255,10 +244,9 @@ function testSecondaryWritePrimaryFirst() { $this->assertEqual($secondary->uuid(), $values_secondary['uuid']); $this->assertEqual($secondary->label(), $values_secondary['label']); - $logs = $this->configLog->getLogMessages(); + $logs = $this->configImporter->getErrors(); $this->assertEqual(count($logs), 1); - $this->assertEqual($logs[0]['message'], String::format('Deleted and replaced configuration entity "@name"', array('@name' => $name_secondary))); - $this->assertEqual($logs[0]['severity'], 'warning'); + $this->assertEqual($logs[0], String::format('Deleted and replaced configuration entity "@name"', array('@name' => $name_secondary))); } /** @@ -302,11 +290,10 @@ function testSecondaryWriteSecondaryFirst() { $this->assertEqual($secondary->uuid(), $values_secondary['uuid']); $this->assertEqual($secondary->label(), $values_secondary['label']); - $logs = $this->configLog->getLogMessages(); + $logs = $this->configImporter->getErrors(); $this->assertEqual(count($logs), 1); - $this->assertEqual($logs[0]['message'], String::format('config_test entity with ID @name already exists.', array('@name' => 'secondary'))); - $this->assertEqual($logs[0]['severity'], 'error'); - + $message = String::format('config_test entity with ID @name already exists', array('@name' => 'secondary')); + $this->assertEqual($logs[0], String::format('Unexpected error during import with operation @op for @name: @message.', array('@op' => 'create', '@name' => $name_primary, '@message' => $message))); } /** @@ -353,10 +340,9 @@ function testSecondaryUpdateDeletedDeleterFirst() { // @todo The deletee entity does not exist as the update failed. Should the // importer attempt a create instead? - $logs = $this->configLog->getLogMessages(); + $logs = $this->configImporter->getErrors(); $this->assertEqual(count($logs), 1); - $this->assertEqual($logs[0]['message'], String::format('Update target "@name" is missing.', array('@name' => $name_deletee))); - $this->assertEqual($logs[0]['severity'], 'error'); + $this->assertEqual($logs[0], String::format('Update target "@name" is missing.', array('@name' => $name_deletee))); } /** @@ -403,7 +389,7 @@ function testSecondaryUpdateDeletedDeleteeFirst() { // @todo The deletee entity does not exist as the update worked but the // entity was deleted after that. There is also no log message as this // happened outside of the config importer. - $logs = $this->configLog->getLogMessages(); + $logs = $this->configImporter->getErrors(); $this->assertEqual(count($logs), 0); } @@ -460,7 +446,7 @@ function testUpdated() { // Verify that there is nothing more to import. $this->assertFalse($this->configImporter->hasUnprocessedConfigurationChanges()); - $logs = $this->configLog->getLogMessages(); + $logs = $this->configImporter->getErrors(); $this->assertEqual(count($logs), 0); } } diff --git a/core/modules/field/lib/Drupal/field/Tests/FieldImportDeleteTest.php b/core/modules/field/lib/Drupal/field/Tests/FieldImportDeleteTest.php index 0728ebf..b0485c4 100644 --- a/core/modules/field/lib/Drupal/field/Tests/FieldImportDeleteTest.php +++ b/core/modules/field/lib/Drupal/field/Tests/FieldImportDeleteTest.php @@ -73,7 +73,7 @@ public function testImportDelete() { $this->assertTrue($staging->delete($instance_config_name_2a), String::format('Deleted field instance: !field_instance', array('!field_instance' => $instance_config_name_2a))); $this->assertTrue($staging->delete($instance_config_name_2b), String::format('Deleted field instance: !field_instance', array('!field_instance' => $instance_config_name_2b))); - $deletes = $this->configImporter()->getUnprocessed('delete'); + $deletes = $this->configImporter()->getUnprocessedConfiguration('delete'); $this->assertEqual(count($deletes), 5, 'Importing configuration will delete 3 field instances and 2 fields.'); // Import the content of the staging directory. diff --git a/core/modules/image/image.module b/core/modules/image/image.module index 074c9f0..61b30b9 100644 --- a/core/modules/image/image.module +++ b/core/modules/image/image.module @@ -373,7 +373,6 @@ function image_filter_keyword($value, $current_pixels, $new_pixels) { */ function image_entity_presave(EntityInterface $entity) { $field = FALSE; - $entity_type_id = $entity->getEntityTypeId(); if ($entity_type_id == 'field_instance_config') { $field = $entity->getField(); diff --git a/core/tests/Drupal/Tests/Core/Config/ConfigImporterLogArrayTest.php b/core/tests/Drupal/Tests/Core/Config/ConfigImporterLogArrayTest.php deleted file mode 100644 index 2b0ce03..0000000 --- a/core/tests/Drupal/Tests/Core/Config/ConfigImporterLogArrayTest.php +++ /dev/null @@ -1,92 +0,0 @@ - 'Config importer state log', - 'description' => 'Tests ConfigImporterLogState', - 'group' => 'Configuration' - ); - } - - /** - * @covers ::log - */ - public function testLogEmpty() { - $state = $this->getMock('Drupal\Core\KeyValueStore\StateInterface'); - $state->expects($this->once()) - ->method('get') - ->with(ConfigImporterLogState::STATE_KEY, array()) - ->will($this->returnValue(array())); - - $message = $this->randomName(); - $severity = 'error'; - $state->expects($this->once()) - ->method('set') - ->with(ConfigImporterLogState::STATE_KEY, array(array('message' => $message, 'severity' => $severity))); - - $log = new ConfigImporterLogState($state); - $log->log($message, $severity); - } - - /** - * @covers ::log - */ - public function testLogExisting() { - $state = $this->getMock('Drupal\Core\KeyValueStore\StateInterface'); - - $logs = array( - array('message' => $this->randomName(), 'severity' => 'warning'), - ); - $state->expects($this->once()) - ->method('get') - ->with(ConfigImporterLogState::STATE_KEY, array()) - ->will($this->returnValue($logs)); - - $message = $this->randomName(); - $severity = 'error'; - - $logs[] = array('message' => $message, 'severity' => $severity); - - $state->expects($this->once()) - ->method('set') - ->with(ConfigImporterLogState::STATE_KEY, $logs); - - $log = new ConfigImporterLogState($state); - $log->log($message, $severity); - } - - /** - * @covers ::getLogMessages - */ - public function testgetLogMessages() { - $state = $this->getMock('Drupal\Core\KeyValueStore\StateInterface'); - - $logs = array( - array('message' => $this->randomName(), 'severity' => 'warning'), - ); - $state->expects($this->once()) - ->method('get') - ->with(ConfigImporterLogState::STATE_KEY, array()) - ->will($this->returnValue($logs)); - - $log = new ConfigImporterLogState($state); - $log->getLogMessages(); - } - -}