Commit 0b4bf3af authored by webchick's avatar webchick

Issue #2124535 by Berdir, alexpott, Désiré, xjm | yched: Prevent secondary...

Issue #2124535 by Berdir, alexpott, Désiré, xjm | yched: Prevent secondary configuration creates and deletes from breaking the ConfigImporter.
parent e84971c5
......@@ -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;
......
......@@ -14,6 +14,7 @@
use Drupal\Core\DependencyInjection\DependencySerialization;
use Drupal\Core\Entity\EntityStorageException;
use Drupal\Core\Lock\LockBackendInterface;
use Drupal\Core\StringTranslation\TranslationManager;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
/**
......@@ -119,6 +120,13 @@ class ConfigImporter extends DependencySerialization {
*/
protected $themeHandler;
/**
* The string translation service.
*
* @var \Drupal\Core\StringTranslation\TranslationManager
*/
protected $translationManager;
/**
* Flag set to import system.theme during processing theme enable and disables.
*
......@@ -126,6 +134,13 @@ class ConfigImporter extends DependencySerialization {
*/
protected $processedSystemTheme = FALSE;
/**
* List of errors that were logged during a config import.
*
* @var array
*/
protected $errors = array();
/**
* Constructs a configuration import object.
*
......@@ -144,8 +159,10 @@ class ConfigImporter extends DependencySerialization {
* The module handler
* @param \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler
* The theme handler
* @param \Drupal\Core\StringTranslation\TranslationManager $translation_manager
* The string translation service.
*/
public function __construct(StorageComparerInterface $storage_comparer, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, LockBackendInterface $lock, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ThemeHandlerInterface $theme_handler) {
public function __construct(StorageComparerInterface $storage_comparer, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, LockBackendInterface $lock, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ThemeHandlerInterface $theme_handler, TranslationManager $translation_manager) {
$this->storageComparer = $storage_comparer;
$this->eventDispatcher = $event_dispatcher;
$this->configManager = $config_manager;
......@@ -153,10 +170,31 @@ public function __construct(StorageComparerInterface $storage_comparer, EventDis
$this->typedConfigManager = $typed_config;
$this->moduleHandler = $module_handler;
$this->themeHandler = $theme_handler;
$this->translationManager = $translation_manager;
$this->processedConfiguration = $this->storageComparer->getEmptyChangelist();
$this->processedExtensions = $this->getEmptyExtensionsProcessedList();
}
/**
* Logs an error message.
*
* @param string $message
* The message to log.
*/
protected function logError($message) {
$this->errors[] = $message;
}
/**
* Returns error messages created while running the import.
*
* @return array
* List of messages.
*/
public function getErrors() {
return $this->errors;
}
/**
* Gets the configuration storage comparer.
*
......@@ -415,7 +453,9 @@ public function import() {
// to handle dependencies correctly.
foreach (array('delete', 'create', 'update') as $op) {
foreach ($this->getUnprocessedConfiguration($op) as $name) {
$this->processConfiguration($op, $name);
if ($this->checkOp($op, $name)) {
$this->processConfiguration($op, $name);
}
}
}
// Allow modules to react to a import.
......@@ -452,10 +492,23 @@ public function validate() {
* The change operation.
* @param string $name
* The name of the configuration to process.
*
* @throws \Exception
* Thrown when the import process fails, only thrown when no importer log is
* set, otherwise the exception message is logged and the configuration
* is skipped.
*/
protected function processConfiguration($op, $name) {
if (!$this->importInvokeOwner($op, $name)) {
$this->importConfig($op, $name);
try {
if (!$this->importInvokeOwner($op, $name)) {
$this->importConfig($op, $name);
}
}
catch (\Exception $e) {
$this->logError($this->t('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);
}
}
......@@ -505,6 +558,68 @@ protected function processExtension($type, $op, $name) {
->resetSourceStorage();
}
/**
* Checks that the operation is still valid.
*
* During a configuration import secondary writes and deletes are possible.
* This method checks that the operation is still valid before processing a
* configuration change.
*
* @param string $op
* The change operation.
* @param string $name
* The name of the configuration to process.
*
* @throws \Drupal\Core\Config\ConfigImporterException
*
* @return bool
* TRUE is to continue processing, FALSE otherwise.
*/
protected function checkOp($op, $name) {
$target_exists = $this->storageComparer->getTargetStorage()->exists($name);
switch ($op) {
case 'delete':
if (!$target_exists) {
// The configuration has already been deleted. For example, a field
// is automatically deleted if all the instances are.
$this->setProcessedConfiguration($op, $name);
return FALSE;
}
break;
case 'create':
if ($target_exists) {
// If the target already exists, use the entity storage to delete it
// again, if is a simple config, delete it directly.
if ($entity_type_id = $this->configManager->getEntityTypeIdByName($name)) {
$entity_storage = $this->configManager->getEntityManager()->getStorage($entity_type_id);
$entity_type = $this->configManager->getEntityManager()->getDefinition($entity_type_id);
$entity = $entity_storage->load($entity_storage->getIDFromConfigName($name, $entity_type->getConfigPrefix()));
$entity->delete();
$this->logError($this->translationManager->translate('Deleted and replaced configuration entity "@name"', array('@name' => $name)));
}
else {
$this->storageComparer->getTargetStorage()->delete($name);
$this->logError($this->t('Deleted and replaced configuration "@name"', array('@name' => $name)));
}
return TRUE;
}
break;
case 'update':
if (!$target_exists) {
$this->logError($this->t('Update target "@name" is missing.', array('@name' => $name)));
// Mark as processed so that the synchronisation continues. Once the
// the current synchronisation is complete it will show up as a
// create.
$this->setProcessedConfiguration($op, $name);
return FALSE;
}
break;
}
return TRUE;
}
/**
* Writes a configuration change from the source to the target storage.
*
......@@ -574,7 +689,6 @@ protected function importInvokeOwner($op, $name) {
$this->setProcessedConfiguration($op, $name);
return TRUE;
}
return FALSE;
}
/**
......@@ -656,5 +770,31 @@ protected function reInjectMe() {
$this->typedConfigManager = \Drupal::service('config.typed');
$this->moduleHandler = \Drupal::moduleHandler();
$this->themeHandler = \Drupal::service('theme_handler');
$this->translationManager = \Drupal::service('string_translation');
}
/**
* Translates a string to the current language or to a given language.
*
* @param string $string
* A string containing the English string to translate.
* @param array $args
* An associative array of replacements to make after translation. Based
* on the first character of the key, the value is escaped and/or themed.
* See \Drupal\Component\Utility\String::format() for details.
* @param array $options
* An associative array of additional options, with the following elements:
* - 'langcode': The language code to translate to a language other than
* what is used to display the page.
* - 'context': The context the source string belongs to.
*
* @return string
* The translated string.
*
* @see \Drupal\Core\StringTranslation\TranslationManager::translate()
*/
protected function t($string, array $args = array(), array $options = array()) {
return $this->translationManager->translate($string, $args, $options);
}
}
......@@ -9,6 +9,7 @@
use Drupal\Component\Utility\String;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Config\ConfigImporterException;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityMalformedException;
use Drupal\Core\Entity\EntityStorageBase;
......@@ -425,6 +426,9 @@ public function importCreate($name, Config $new_config, Config $old_config) {
public function importUpdate($name, Config $new_config, Config $old_config) {
$id = static::getIDFromConfigName($name, $this->entityType->getConfigPrefix());
$entity = $this->load($id);
if (!$entity) {
throw new ConfigImporterException(String::format('Attempt to update non-existing entity "@id".', array('@id' => $id)));
}
$entity->setSyncing(TRUE);
$entity->original = clone $entity;
......
......@@ -38,6 +38,9 @@ public function importCreate($name, Config $new_config, Config $old_config);
* A configuration object containing the new configuration data.
* @param \Drupal\Core\Config\Config $old_config
* A configuration object containing the old configuration data.
*
* @throws \Drupal\Core\Config\ConfigImporterException
* Thrown when the config entity that should be updated can not be found.
*/
public function importUpdate($name, Config $new_config, Config $old_config);
......
......@@ -75,7 +75,7 @@ class CustomBlockType extends ConfigEntityBase implements CustomBlockTypeInterfa
public function postSave(EntityStorageInterface $storage, $update = TRUE) {
parent::postSave($storage, $update);
if (!$update) {
if (!$update && !$this->isSyncing()) {
entity_invoke_bundle_hook('create', 'custom_block', $this->id());
if (!$this->isSyncing()) {
custom_block_add_body_field($this->id);
......
......@@ -250,7 +250,8 @@ public function submitForm(array &$form, array &$form_state) {
$this->lock,
$this->typedConfigManager,
$this->moduleHandler,
$this->themeHandler
$this->themeHandler,
$this->translationManager()
);
if ($config_importer->alreadyImporting()) {
drupal_set_message($this->t('Another request may be synchronizing configuration already.'));
......@@ -289,6 +290,12 @@ public static function processBatch(BatchConfigImporter $config_importer, $opera
$config_importer = $context['sandbox']['config_importer'];
$config_importer->$operation($context);
if ($errors = $config_importer->getErrors()) {
if (!isset($context['results']['errors'])) {
$context['results']['errors'] = array();
}
$context['results']['errors'] += $errors;
}
}
/**
......@@ -299,7 +306,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.
......
......@@ -95,6 +95,9 @@ public function testInstallUninstall() {
// Import the configuration thereby re-installing all the modules.
$this->configImporter()->import();
// Check that there are no errors.
$this->assertIdentical($this->configImporter()->getErrors(), array());
// Check that all modules that were uninstalled are now reinstalled.
$this->assertModules(array_keys($modules_to_uninstall), TRUE);
foreach($modules_to_uninstall as $module => $info) {
......
......@@ -59,7 +59,8 @@ public function setUp() {
$this->container->get('lock'),
$this->container->get('config.typed'),
$this->container->get('module_handler'),
$this->container->get('theme_handler')
$this->container->get('theme_handler'),
$this->container->get('string_translation')
);
}
......
......@@ -7,6 +7,7 @@
namespace Drupal\config\Tests;
use Drupal\Component\Utility\String;
use Drupal\Core\Config\InstallStorage;
use Drupal\simpletest\WebTestBase;
......@@ -312,4 +313,55 @@ function prepareSiteNameUpdate($new_site_name) {
$config_data['name'] = $new_site_name;
$staging->write('system.site', $config_data);
}
/**
* Tests an import that results in an error.
*/
function testImportErrorLog() {
$name_primary = 'config_test.dynamic.primary';
$name_secondary = 'config_test.dynamic.secondary';
$staging = $this->container->get('config.storage.staging');
$uuid = $this->container->get('uuid');
$values_primary = array(
'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.
$this->drupalGet('admin/config/development/configuration');
$this->assertNoText(t('There are no configuration changes.'));
// Attempt to import configuration and verify that an error message appears.
$this->drupalPostForm(NULL, array(), t('Import all'));
$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.'));
}
}
......@@ -10,6 +10,7 @@
use Drupal\Core\Config\Entity\ConfigEntityBase;
use Drupal\config_test\ConfigTestInterface;
use Drupal\Core\Config\Entity\ConfigEntityInterface;
use Drupal\Core\Entity\EntityStorageInterface;
/**
* Defines the ConfigTest configuration entity.
......@@ -106,6 +107,37 @@ public static function sort(ConfigEntityInterface $a, ConfigEntityInterface $b)
return parent::sort($a, $b);
}
/**
* {@inheritdoc}
*/
public function postSave(EntityStorageInterface $storage, $update = TRUE) {
// Used to test secondary writes during config sync.
if ($this->id() == 'primary') {
$secondary = $storage->create(array(
'id' => 'secondary',
'label' => 'Secondary Default',
));
$secondary->save();
}
if ($this->id() == 'deleter') {
$deletee = $storage->load('deletee');
$deletee->delete();
}
}
/**
* {@inheritdoc}
*/
public static function postDelete(EntityStorageInterface $storage, array $entities) {
parent::postDelete($storage, $entities);
foreach ($entities as $entity) {
if ($entity->id() == 'deleter') {
$deletee = $storage->load('deletee');
$deletee->delete();
}
}
}
/**
* {@inheritdoc}
*/
......
......@@ -72,6 +72,11 @@ function entity_reference_field_config_update(FieldConfigInterface $field) {
return;
}
if ($field->isSyncing()) {
// Don't change anything during a configuration sync.
return;
}
if ($field->getSetting('target_type') == $field->original->getSetting('target_type')) {
// Target type didn't change.
return;
......
......@@ -7,6 +7,8 @@
namespace Drupal\field\Tests;
use Drupal\Component\Utility\String;
/**
* Tests deleting fields and instances as part of config import.
*/
......@@ -31,13 +33,21 @@ public static function getInfo() {
* Tests deleting fields and instances as part of config import.
*/
public function testImportDelete() {
// At this point there are 5 field configuration objects in the active
// storage.
// - field.field.entity_test.field_test_import
// - field.field.entity_test.field_test_import_2
// - field.instance.entity_test.entity_test.field_test_import
// - field.instance.entity_test.entity_test.field_test_import_2
// - field.instance.entity_test.test_bundle.field_test_import_2
$field_name = 'field_test_import';
$field_id = "entity_test.$field_name";
$field_name_2 = 'field_test_import_2';
$field_id_2 = "entity_test.$field_name_2";
$instance_id = "entity_test.test_bundle.$field_name";
$instance_id_2a = "entity_test.test_bundle.$field_name_2";
$instance_id_2b = "entity_test.test_bundle_2.$field_name_2";
$instance_id = "entity_test.entity_test.$field_name";
$instance_id_2a = "entity_test.entity_test.$field_name_2";
$instance_id_2b = "entity_test.test_bundle.$field_name_2";
$field_config_name = "field.field.$field_id";
$field_config_name_2 = "field.field.$field_id_2";
$instance_config_name = "field.instance.$instance_id";
......@@ -45,7 +55,7 @@ public function testImportDelete() {
$instance_config_name_2b = "field.instance.$instance_id_2b";
// Create a second bundle for the 'Entity test' entity type.
entity_test_create_bundle('test_bundle_2');
entity_test_create_bundle('test_bundle');
// Import default config.
$this->installConfig(array('field_test_config'));
......@@ -57,11 +67,14 @@ public function testImportDelete() {
$active = $this->container->get('config.storage');
$staging = $this->container->get('config.storage.staging');
$this->copyConfig($active, $staging);
$staging->delete($field_config_name);
$staging->delete($field_config_name_2);
$staging->delete($instance_config_name);
$staging->delete($instance_config_name_2a);
$staging->delete($instance_config_name_2b);
$this->assertTrue($staging->delete($field_config_name), String::format('Deleted field: !field', array('!field' => $field_config_name)));
$this->assertTrue($staging->delete($field_config_name_2), String::format('Deleted field: !field', array('!field' => $field_config_name_2)));
$this->assertTrue($staging->delete($instance_config_name), String::format('Deleted field instance: !field_instance', array('!field_instance' => $instance_config_name)));
$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()->getUnprocessedConfiguration('delete');
$this->assertEqual(count($deletes), 5, 'Importing configuration will delete 3 field instances and 2 fields.');
// Import the content of the staging directory.
$this->configImporter()->import();
......
......@@ -239,7 +239,7 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
// Clear the filter cache whenever a text format is updated.
Cache::deleteTags(array('filter_format' => $this->id()));
}
else {
elseif (!$this->isSyncing()) {
// Default configuration of modules and installation profiles is allowed
// to specify a list of user roles to grant access to for the new format;
// apply the defined user role permissions when a new format is inserted
......
......@@ -387,6 +387,10 @@ function image_entity_presave(EntityInterface $entity) {
return;
}
if ($field->isSyncing()) {
return;
}
$fid = $entity->settings['default_image']['fid'];
if ($fid) {
$original_fid = isset($entity->original) ? $entity->original->settings['default_image']['fid'] : NULL;
......
......@@ -105,7 +105,9 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
// The old image style name needs flushing after a rename.
$this->original->flush();
// Update field instance settings if necessary.
static::replaceImageStyle($this);
if (!$this->isSyncing()) {
static::replaceImageStyle($this);
}
}
else {
// Flush image style when updating without changing the name.
......@@ -126,7 +128,7 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
// Check whether field instance settings need to be updated.
// In case no replacement style was specified, all image fields that are
// using the deleted style are left in a broken state.
if ($new_id = $style->getReplacementID()) {
if (!$style->isSyncing() && $new_id = $style->getReplacementID()) {
// The deleted ID is still set as originalID.
$style->setName($new_id);
static::replaceImageStyle($style);
......
......@@ -133,6 +133,11 @@ function menu_menu_insert(Menu $menu) {
if (\Drupal::moduleHandler()->moduleExists('block')) {
\Drupal::service('plugin.manager.block')->clearCachedDefinitions();
}
if ($menu->isSyncing()) {
return;
}
// Make sure the menu is present in the active menus variable so that its
// items may appear in the menu active trail.
// See menu_set_active_menu_names().
......@@ -322,6 +327,9 @@ function menu_node_update(EntityInterface $node) {
* Implements hook_node_type_insert().
*/
function menu_node_type_insert(NodeTypeInterface $type) {
if ($type->isSyncing()) {
return;
}
\Drupal::config('menu.entity.node.' . $type->id())
->set('available_menus', array('main'))
->set('parent', 'main:0')
......@@ -332,6 +340,9 @@ function menu_node_type_insert(NodeTypeInterface $type) {
* Implements hook_node_type_delete().
*/
function menu_node_type_delete(NodeTypeInterface $type) {
if ($type->isSyncing()) {
return;
}
\Drupal::config('menu.entity.node.' . $type->id())->delete();
}
......
......@@ -1527,7 +1527,8 @@ public function configImporter() {
$this->container->get('lock'),
$this->container->get('config.typed'),
$this->container->get('module_handler'),
$this->container->get('theme_handler')
$this->container->get('theme_handler'),
$this->container->get('string_translation')
);
}
// Always recalculate the changelist when called.
......
......@@ -102,7 +102,7 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
if (!$update) {
entity_invoke_bundle_hook('create', 'taxonomy_term', $this->id());
}
elseif ($this->getOriginalId() != $this->id()) {
elseif ($this->getOriginalId() != $this->id() && !$this->isSyncing()) {
// Reflect machine name changes in the definitions of existing 'taxonomy'
// fields.
$field_ids = array();
......@@ -153,6 +153,13 @@ public static function preDelete(EntityStorageInterface $storage, array $entitie
public static function postDelete(EntityStorageInterface $storage, array $entities) {
parent::postDelete($storage, $entities);
// Reset caches.
$storage->resetCache(array_keys($entities));
if (reset($entities)->isSyncing()) {
return;
}
$vocabularies = array();
foreach ($entities as $vocabulary) {
$vocabularies[$vocabulary->id()] = $vocabulary->id();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment