From 9e722833787d25e379fa32741cec1ed785e4cc41 Mon Sep 17 00:00:00 2001 From: webchick Date: Wed, 30 Oct 2013 21:30:41 -0700 Subject: [PATCH] Issue #2069373 by swentel, yched: Race conditions on import if CUD on ConfigEntity A triggers changes in ConfigEntity B. --- .../Core/Config/Entity/ConfigEntityBase.php | 32 ++++++++++++++++--- .../Config/Entity/ConfigEntityInterface.php | 16 ++++++++++ .../Config/Entity/ConfigStorageController.php | 3 ++ .../node/lib/Drupal/node/Entity/NodeType.php | 5 +-- .../Tests/Config/NodeImportCreateTest.php | 1 + .../views_ui/lib/Drupal/views_ui/ViewUI.php | 22 +++++++++++++ .../views_ui/Tests/ViewUIObjectTest.php | 3 +- 7 files changed, 74 insertions(+), 8 deletions(-) diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php index 0a11098779b..a988135d9ce 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php @@ -34,6 +34,14 @@ abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface */ public $status = TRUE; + /** + * Whether the config is being created, updated or deleted through the + * import process. + * + * @var bool + */ + private $isSyncing = FALSE; + /** * Overrides Entity::__construct(). */ @@ -90,21 +98,21 @@ abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface } /** - * Implements \Drupal\Core\Config\Entity\ConfigEntityInterface::enable(). + * {@inheritdoc} */ public function enable() { return $this->setStatus(TRUE); } /** - * Implements \Drupal\Core\Config\Entity\ConfigEntityInterface::disable(). + * {@inheritdoc} */ public function disable() { return $this->setStatus(FALSE); } /** - * Implements \Drupal\Core\Config\Entity\ConfigEntityInterface::setStatus(). + * {@inheritdoc} */ public function setStatus($status) { $this->status = (bool) $status; @@ -112,14 +120,28 @@ abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface } /** - * Implements \Drupal\Core\Config\Entity\ConfigEntityInterface::status(). + * {@inheritdoc} */ public function status() { return !empty($this->status); } /** - * Overrides Entity::createDuplicate(). + * {@inheritdoc} + */ + public function setSyncing($syncing) { + $this->isSyncing = $syncing; + } + + /** + * {@inheritdoc} + */ + public function isSyncing() { + return $this->isSyncing; + } + + /** + * {@inheritdoc} */ public function createDuplicate() { $duplicate = parent::createDuplicate(); diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityInterface.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityInterface.php index 78ba566a1bb..5aa737dd296 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityInterface.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityInterface.php @@ -59,6 +59,14 @@ interface ConfigEntityInterface extends EntityInterface { */ public function setStatus($status); + /** + * Sets the status of the isSyncing flag. + * + * @param bool $status + * The status of the sync flag. + */ + public function setSyncing($status); + /** * Returns whether the configuration entity is enabled. * @@ -75,6 +83,14 @@ interface ConfigEntityInterface extends EntityInterface { */ public function status(); + /** + * Returns whether the configuration entity is created, updated or deleted + * through the import process. + * + * @return bool + */ + public function isSyncing(); + /** * Returns the value of a property. * diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php index 2fd44c4ae59..dfa8198915a 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php @@ -484,6 +484,7 @@ class ConfigStorageController extends EntityStorageControllerBase { */ public function importCreate($name, Config $new_config, Config $old_config) { $entity = $this->create($new_config->get()); + $entity->setSyncing(TRUE); $entity->save(); return TRUE; } @@ -504,6 +505,7 @@ class ConfigStorageController extends EntityStorageControllerBase { public function importUpdate($name, Config $new_config, Config $old_config) { $id = static::getIDFromConfigName($name, $this->entityInfo['config_prefix']); $entity = $this->load($id); + $entity->setSyncing(TRUE); $entity->original = clone $entity; foreach ($old_config->get() as $property => $value) { @@ -534,6 +536,7 @@ class ConfigStorageController extends EntityStorageControllerBase { public function importDelete($name, Config $new_config, Config $old_config) { $id = static::getIDFromConfigName($name, $this->entityInfo['config_prefix']); $entity = $this->load($id); + $entity->setSyncing(TRUE); $entity->delete(); return TRUE; } diff --git a/core/modules/node/lib/Drupal/node/Entity/NodeType.php b/core/modules/node/lib/Drupal/node/Entity/NodeType.php index 5bc423e6227..0fb1eccb3bd 100644 --- a/core/modules/node/lib/Drupal/node/Entity/NodeType.php +++ b/core/modules/node/lib/Drupal/node/Entity/NodeType.php @@ -170,8 +170,9 @@ class NodeType extends ConfigEntityBase implements NodeTypeInterface { entity_invoke_bundle_hook('create', 'node', $this->id()); - // Unless disabled, automatically create a Body field for new node types. - if ($this->get('create_body')) { + // Create a body if the create_body property is true and we're not in + // the syncing process. + if ($this->get('create_body') && !$this->isSyncing()) { $label = $this->get('create_body_label'); node_add_body_field($this, $label); } diff --git a/core/modules/node/lib/Drupal/node/Tests/Config/NodeImportCreateTest.php b/core/modules/node/lib/Drupal/node/Tests/Config/NodeImportCreateTest.php index 0a1db9cd546..764d96ce049 100644 --- a/core/modules/node/lib/Drupal/node/Tests/Config/NodeImportCreateTest.php +++ b/core/modules/node/lib/Drupal/node/Tests/Config/NodeImportCreateTest.php @@ -78,6 +78,7 @@ class NodeImportCreateTest extends DrupalUnitTestBase { // Check that the content type was created. $node_type = entity_load('node_type', $node_type_id); $this->assertTrue($node_type, 'Import node type from staging was created.'); + $this->assertFalse(field_info_instance('node', 'body', $node_type_id)); } } diff --git a/core/modules/views_ui/lib/Drupal/views_ui/ViewUI.php b/core/modules/views_ui/lib/Drupal/views_ui/ViewUI.php index 80d93a1926d..84bfde1d6d6 100644 --- a/core/modules/views_ui/lib/Drupal/views_ui/ViewUI.php +++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewUI.php @@ -138,6 +138,14 @@ class ViewUI implements ViewStorageInterface { 'reorder-displays' => '\Drupal\views_ui\Form\Ajax\ReorderDisplays', ); + /** + * Whether the config is being created, updated or deleted through the + * import process. + * + * @var bool + */ + private $isSyncing = FALSE; + /** * Constructs a View UI object. * @@ -187,6 +195,20 @@ class ViewUI implements ViewStorageInterface { return '
' . t("Click on an item to edit that item's details.") . '
'; } + /** + * {@inheritdoc} + */ + public function setSyncing($syncing) { + $this->isSyncing = $syncing; + } + + /** + * {@inheritdoc} + */ + public function isSyncing() { + return $this->isSyncing; + } + /** * Basic submit handler applicable to all 'standard' forms. * diff --git a/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewUIObjectTest.php b/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewUIObjectTest.php index 3c424878cf5..94870a6bc26 100644 --- a/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewUIObjectTest.php +++ b/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewUIObjectTest.php @@ -45,7 +45,8 @@ class ViewUIObjectTest extends UnitTestCase { // EntityInterface::isNew() is missing from the list of methods, because it // calls id(), which breaks the ->expect($this->once()) call. Call it later. - if ($reflection_method->getName() != 'isNew') { + // EntityInterface::isSyncing() is only called during syncing process. + if ($reflection_method->getName() != 'isNew' && $reflection_method->getName() != 'isSyncing') { if (count($reflection_method->getParameters()) == 0) { $method_args[$reflection_method->getName()] = array(); }