From 50b5b4d6f85d27a86d0333ec6e138259537c32a9 Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Mon, 16 Dec 2019 15:18:16 +0000
Subject: [PATCH] Issue #3092714 by alexpott, hchonov, Berdir, gease: Config
entity updater misbehaves when updating multiple entity types
---
.../Config/Entity/ConfigEntityUpdater.php | 24 ++++++++++----
core/modules/system/system.post_update.php | 31 +++++++++++++++++--
core/modules/text/text.post_update.php | 28 +++++++++++------
.../Config/Entity/ConfigEntityUpdaterTest.php | 25 +++++++++++----
4 files changed, 84 insertions(+), 24 deletions(-)
diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityUpdater.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityUpdater.php
index 571b9914250a..7ba77000aca1 100644
--- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityUpdater.php
+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityUpdater.php
@@ -25,6 +25,11 @@
*/
class ConfigEntityUpdater implements ContainerInjectionInterface {
+ /**
+ * The key used to store information in the update sandbox.
+ */
+ const SANDBOX_KEY = 'config_entity_updater';
+
/**
* The entity type manager.
*
@@ -88,17 +93,24 @@ public static function create(ContainerInterface $container) {
* @throws \InvalidArgumentException
* Thrown when the provided entity type ID is not a configuration entity
* type.
+ * @throws \RuntimeException
+ * Thrown when used twice in the same update function for different entity
+ * types. This method should only be called once per update function.
*/
public function update(array &$sandbox, $entity_type_id, callable $callback = NULL) {
$storage = $this->entityTypeManager->getStorage($entity_type_id);
- $sandbox_key = 'config_entity_updater:' . $entity_type_id;
- if (!isset($sandbox[$sandbox_key])) {
+
+ if (isset($sandbox[self::SANDBOX_KEY]) && $sandbox[self::SANDBOX_KEY]['entity_type'] !== $entity_type_id) {
+ throw new \RuntimeException('Updating multiple entity types in the same update function is not supported');
+ }
+ if (!isset($sandbox[self::SANDBOX_KEY])) {
$entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
if (!($entity_type instanceof ConfigEntityTypeInterface)) {
throw new \InvalidArgumentException("The provided entity type ID '$entity_type_id' is not a configuration entity type");
}
- $sandbox[$sandbox_key]['entities'] = $storage->getQuery()->accessCheck(FALSE)->execute();
- $sandbox[$sandbox_key]['count'] = count($sandbox[$sandbox_key]['entities']);
+ $sandbox[self::SANDBOX_KEY]['entity_type'] = $entity_type_id;
+ $sandbox[self::SANDBOX_KEY]['entities'] = $storage->getQuery()->accessCheck(FALSE)->execute();
+ $sandbox[self::SANDBOX_KEY]['count'] = count($sandbox[self::SANDBOX_KEY]['entities']);
}
// The default behaviour is to fix dependencies.
@@ -111,7 +123,7 @@ public function update(array &$sandbox, $entity_type_id, callable $callback = NU
}
/** @var \Drupal\Core\Config\Entity\ConfigEntityInterface $entity */
- $entities = $storage->loadMultiple(array_splice($sandbox[$sandbox_key]['entities'], 0, $this->batchSize));
+ $entities = $storage->loadMultiple(array_splice($sandbox[self::SANDBOX_KEY]['entities'], 0, $this->batchSize));
foreach ($entities as $entity) {
if (call_user_func($callback, $entity)) {
$entity->trustData();
@@ -119,7 +131,7 @@ public function update(array &$sandbox, $entity_type_id, callable $callback = NU
}
}
- $sandbox['#finished'] = empty($sandbox[$sandbox_key]['entities']) ? 1 : ($sandbox[$sandbox_key]['count'] - count($sandbox[$sandbox_key]['entities'])) / $sandbox[$sandbox_key]['count'];
+ $sandbox['#finished'] = empty($sandbox[self::SANDBOX_KEY]['entities']) ? 1 : ($sandbox[self::SANDBOX_KEY]['count'] - count($sandbox[self::SANDBOX_KEY]['entities'])) / $sandbox[self::SANDBOX_KEY]['count'];
}
}
diff --git a/core/modules/system/system.post_update.php b/core/modules/system/system.post_update.php
index 114d28912022..242ad9d1b551 100644
--- a/core/modules/system/system.post_update.php
+++ b/core/modules/system/system.post_update.php
@@ -150,7 +150,7 @@ function system_post_update_language_item_callback() {
}
/**
- * Update all entity displays that contain extra fields.
+ * Update all entity view displays that contain extra fields.
*/
function system_post_update_extra_fields(&$sandbox = NULL) {
$config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class);
@@ -174,10 +174,37 @@ function system_post_update_extra_fields(&$sandbox = NULL) {
return $needs_save;
};
- $config_entity_updater->update($sandbox, 'entity_form_display', $callback);
$config_entity_updater->update($sandbox, 'entity_view_display', $callback);
}
+/**
+ * Update all entity form displays that contain extra fields.
+ */
+function system_post_update_extra_fields_form_display(&$sandbox = NULL) {
+ $config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class);
+ $entity_field_manager = \Drupal::service('entity_field.manager');
+
+ $callback = function (EntityDisplayInterface $display) use ($entity_field_manager) {
+ $display_context = $display instanceof EntityViewDisplayInterface ? 'display' : 'form';
+ $extra_fields = $entity_field_manager->getExtraFields($display->getTargetEntityTypeId(), $display->getTargetBundle());
+
+ // If any extra fields are used as a component, resave the display with the
+ // updated component information.
+ $needs_save = FALSE;
+ if (!empty($extra_fields[$display_context])) {
+ foreach ($extra_fields[$display_context] as $name => $extra_field) {
+ if ($component = $display->getComponent($name)) {
+ $display->setComponent($name, $component);
+ $needs_save = TRUE;
+ }
+ }
+ }
+ return $needs_save;
+ };
+
+ $config_entity_updater->update($sandbox, 'entity_form_display', $callback);
+}
+
/**
* Force cache clear to ensure aggregated JavaScript files are regenerated.
*
diff --git a/core/modules/text/text.post_update.php b/core/modules/text/text.post_update.php
index 5c98e26b0062..1ea9f7eab77b 100644
--- a/core/modules/text/text.post_update.php
+++ b/core/modules/text/text.post_update.php
@@ -11,10 +11,27 @@
use Drupal\text\Plugin\Field\FieldWidget\TextareaWithSummaryWidget;
/**
- * Update text_with_summary fields and widgets to add summary required flags.
+ * Update text_with_summary fields to add summary required flags.
*/
function text_post_update_add_required_summary_flag(&$sandbox = NULL) {
$config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class);
+
+ $field_callback = function (FieldConfigInterface $field) {
+ if ($field->getType() !== 'text_with_summary') {
+ return FALSE;
+ }
+ $field->setSetting('required_summary', FALSE);
+ return TRUE;
+ };
+
+ $config_entity_updater->update($sandbox, 'field_config', $field_callback);
+}
+
+/**
+ * Update text_with_summary widgets to add summary required flags.
+ */
+function text_post_update_add_required_summary_flag_form_display(&$sandbox = NULL) {
+ $config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class);
/** @var \Drupal\Core\Field\WidgetPluginManager $field_widget_manager */
$field_widget_manager = \Drupal::service('plugin.manager.field.widget');
@@ -36,14 +53,5 @@ function text_post_update_add_required_summary_flag(&$sandbox = NULL) {
return $needs_save;
};
- $field_callback = function (FieldConfigInterface $field) {
- if ($field->getType() !== 'text_with_summary') {
- return FALSE;
- }
- $field->setSetting('required_summary', FALSE);
- return TRUE;
- };
-
$config_entity_updater->update($sandbox, 'entity_form_display', $widget_callback);
- $config_entity_updater->update($sandbox, 'field_config', $field_callback);
}
diff --git a/core/tests/Drupal/KernelTests/Core/Config/Entity/ConfigEntityUpdaterTest.php b/core/tests/Drupal/KernelTests/Core/Config/Entity/ConfigEntityUpdaterTest.php
index f76578f99cf5..313e58b45762 100644
--- a/core/tests/Drupal/KernelTests/Core/Config/Entity/ConfigEntityUpdaterTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Config/Entity/ConfigEntityUpdaterTest.php
@@ -58,8 +58,9 @@ public function testUpdate() {
$this->assertEquals('config_test_9', $entities['config_test_9']->label());
$this->assertEquals('config_test_10', $entities['config_test_10']->label());
$this->assertEquals('config_test_14', $entities['config_test_14']->label());
- $this->assertEquals(15, $sandbox['config_entity_updater:config_test']['count']);
- $this->assertCount(5, $sandbox['config_entity_updater:config_test']['entities']);
+ $this->assertEquals(15, $sandbox['config_entity_updater']['count']);
+ $this->assertEquals('config_test', $sandbox['config_entity_updater']['entity_type']);
+ $this->assertCount(5, $sandbox['config_entity_updater']['entities']);
$this->assertEquals(10 / 15, $sandbox['#finished']);
// Update the rest.
@@ -70,7 +71,7 @@ public function testUpdate() {
$this->assertEquals('config_test_10 (updated)', $entities['config_test_10']->label());
$this->assertEquals('config_test_14 (updated)', $entities['config_test_14']->label());
$this->assertEquals(1, $sandbox['#finished']);
- $this->assertCount(0, $sandbox['config_entity_updater:config_test']['entities']);
+ $this->assertCount(0, $sandbox['config_entity_updater']['entities']);
}
/**
@@ -100,8 +101,8 @@ public function testUpdateDefaultCallback() {
$this->assertEquals(['added_dependency'], $entities['config_test_8']->getDependencies()['module']);
$this->assertEquals([], $entities['config_test_9']->getDependencies());
$this->assertEquals([], $entities['config_test_14']->getDependencies());
- $this->assertEquals(15, $sandbox['config_entity_updater:config_test']['count']);
- $this->assertCount(6, $sandbox['config_entity_updater:config_test']['entities']);
+ $this->assertEquals(15, $sandbox['config_entity_updater']['count']);
+ $this->assertCount(6, $sandbox['config_entity_updater']['entities']);
$this->assertEquals(9 / 15, $sandbox['#finished']);
// Update the rest.
@@ -110,7 +111,7 @@ public function testUpdateDefaultCallback() {
$this->assertEquals(['added_dependency'], $entities['config_test_9']->getDependencies()['module']);
$this->assertEquals(['added_dependency'], $entities['config_test_14']->getDependencies()['module']);
$this->assertEquals(1, $sandbox['#finished']);
- $this->assertCount(0, $sandbox['config_entity_updater:config_test']['entities']);
+ $this->assertCount(0, $sandbox['config_entity_updater']['entities']);
}
/**
@@ -125,4 +126,16 @@ public function testUpdateException() {
$updater->update($sandbox, 'entity_test_mul_changed');
}
+ /**
+ * @covers ::update
+ */
+ public function testUpdateOncePerUpdateException() {
+ $this->expectException(\RuntimeException::class);
+ $this->expectExceptionMessage('Updating multiple entity types in the same update function is not supported');
+ $updater = $this->container->get('class_resolver')->getInstanceFromDefinition(ConfigEntityUpdater::class);
+ $sandbox = [];
+ $updater->update($sandbox, 'config_test');
+ $updater->update($sandbox, 'config_query_test');
+ }
+
}
--
GitLab