Issue #2542748 by plach: Follow-up to Automatic entity updates can fail when...

Issue #2542748 by plach: Follow-up to Automatic entity updates can fail when there is existing content, leaving the site's schema in an unpredictable state

core/includes/update.inc

@@ -10,7 +10,6 @@
 
 use Drupal\Component\Graph\Graph;
 use Drupal\Component\Utility\Html;
-use Drupal\Core\Entity\EntityStorageException;
 use Drupal\Core\Utility\Error;
 
 /**

core/modules/node/node.install

@@ -197,10 +197,7 @@ function node_update_8003() {
   // The 'status' and 'uid' fields were added to the 'entity_keys' annotation
   // of \Drupal\node\Entity\Node in https://www.drupal.org/node/2498919, but
   // this update function wasn't added until
-  // https://www.drupal.org/node/2542748. In between, sites could have
-  // performed interim updates, which would have included automated entity
-  // schema updates prior to that being removed (see that issue for details).
-  // Therefore, we check for whether the keys have already been installed.
+  // https://www.drupal.org/node/2542748.
   $manager = \Drupal::entityDefinitionUpdateManager();
   $entity_type = $manager->getEntityType('node');
   $entity_keys = $entity_type->getKeys();
@@ -212,10 +209,10 @@ function node_update_8003() {
   // @todo The above should be enough, since that is the only definition that
   //   changed. But \Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema varies
   //   field schema by whether a field is an entity key, so invoke
-  //   onFieldStorageDefinitionUpdate() with an unmodified
-  //   $field_storage_definition to trigger the necessary changes.
-  //   SqlContentEntityStorageSchema::onEntityTypeUpdate() should be fixed to
-  //   automatically handle this.
+  //   EntityDefinitionUpdateManagerInterface::updateFieldStorageDefinition()
+  //   with an unmodified field storage definition to trigger the necessary
+  //   changes. SqlContentEntityStorageSchema::onEntityTypeUpdate() should be
+  //   fixed to automatically handle this.
   //   See https://www.drupal.org/node/2554245.
   foreach (array('status', 'uid') as $field_name) {
     $manager->updateFieldStorageDefinition($manager->getFieldStorageDefinition($field_name, 'node'));

core/modules/system/src/Tests/Entity/EntityDefinitionUpdateTest.php

@@ -619,7 +619,7 @@ public function testEntityTypeSchemaUpdateAndRevisionableBaseFieldCreateWithoutD
   }
 
   /**
-   * Tests ::applyEntityUpdate() and ::applyFieldUpdate().
+   * Tests applying single updates.
    */
   public function testSingleActionCalls() {
     $db_schema = $this->database->schema();
@@ -668,9 +668,9 @@ public function testSingleActionCalls() {
     $this->entityDefinitionUpdateManager->installFieldStorageDefinition('new_base_field', 'entity_test_update', 'entity_test', $storage_definition);
     $this->assertTrue($db_schema->fieldExists('entity_test_update', 'new_base_field'), "New field 'new_base_field' has been created on the 'entity_test_update' table.");
 
-    // Ensure that installing an existing entity type is a no-op.
+    // Ensure that installing an existing field is a no-op.
     $this->entityDefinitionUpdateManager->installFieldStorageDefinition('new_base_field', 'entity_test_update', 'entity_test', $storage_definition);
-    $this->assertTrue($db_schema->fieldExists('entity_test_update', 'new_base_field'), 'Installing an existing entity type is a no-op');
+    $this->assertTrue($db_schema->fieldExists('entity_test_update', 'new_base_field'), 'Installing an existing field is a no-op');
 
     // Update an existing field schema.
     $this->modifyBaseField();

core/modules/system/src/Tests/Entity/Update/UpdateApiEntityDefinitionUpdateTest.php

@@ -68,9 +68,9 @@ public function testSingleUpdates() {
     $this->assertEqual(count($entity->user_id), 1);
     $this->assertEqual($entity->user_id->target_id, $user_ids[0]);
 
-    // Make 'user_id' multiple by running updates.
+    // Make 'user_id' multiple by applying updates.
     $this->enableUpdates('entity_test', 'entity_definition_updates', 8001);
-    $this->runUpdates();
+    $this->applyUpdates();
 
     // Check that data was correctly migrated.
     $entity = $this->reloadEntity($entity);
@@ -85,14 +85,21 @@ public function testSingleUpdates() {
     $this->assertEqual($entity->user_id[0]->target_id, $user_ids[0]);
     $this->assertEqual($entity->user_id[1]->target_id, $user_ids[1]);
 
-    // Make 'user_id' single again by running updates.
+    // Make 'user_id' single again by applying updates.
     $this->enableUpdates('entity_test', 'entity_definition_updates', 8002);
-    $this->runUpdates();
+    $this->applyUpdates();
 
     // Check that data was correctly migrated/dropped.
     $entity = $this->reloadEntity($entity);
     $this->assertEqual(count($entity->user_id), 1);
     $this->assertEqual($entity->user_id->target_id, $user_ids[0]);
+
+    // Check that only a single value is stored for 'user_id' again.
+    $entity->user_id = $user_ids;
+    $entity->save();
+    $entity = $this->reloadEntity($entity);
+    $this->assertEqual(count($entity->user_id), 1);
+    $this->assertEqual($entity->user_id[0]->target_id, $user_ids[0]);
   }
 
   /**
@@ -109,9 +116,9 @@ public function testMultipleUpdates() {
     $this->assertEqual(count($entity->user_id), 1);
     $this->assertEqual($entity->user_id->target_id, $user_ids[0]);
 
-    // Make 'user_id' multiple and then single again by running updates.
+    // Make 'user_id' multiple and then single again by applying updates.
     $this->enableUpdates('entity_test', 'entity_definition_updates', 8002);
-    $this->runUpdates();
+    $this->applyUpdates();
 
     // Check that data was correctly migrated back and forth.
     $entity = $this->reloadEntity($entity);
@@ -154,8 +161,8 @@ function testStatusReport() {
     $this->assertRaw('Out of date');
     $this->assertNoRaw('Mismatch detected');
 
-    // Run db updates and check that entity updates were not applied.
-    $this->runUpdates();
+    // Apply db updates and check that entity updates were not applied.
+    $this->applyUpdates();
     $this->drupalGet('admin/reports/status');
     $this->assertNoRaw('Out of date');
     $this->assertRaw('Mismatch detected');
@@ -180,7 +187,7 @@ function testStatusReport() {
     // entity updates were not applied even when no data exists.
     $entity->delete();
     $this->enableUpdates('entity_test', 'status_report', 8002);
-    $this->runUpdates();
+    $this->applyUpdates();
     $this->drupalGet('admin/reports/status');
     $this->assertNoRaw('Out of date');
     $this->assertRaw('Mismatch detected');

core/modules/system/src/Tests/Update/DbUpdatesTrait.php

@@ -11,7 +11,10 @@
 use Drupal\Core\Url;
 
 /**
- * Provides methods to conditionally enable db update functions and run updates.
+ * Provides methods to conditionally enable db update functions and apply
+ * pending db updates through the Update UI.
+ *
+ * This should be used only by classes extending \Drupal\simpletest\WebTestBase.
  */
 trait DbUpdatesTrait {
 
@@ -32,9 +35,9 @@ protected function enableUpdates($module, $group, $index) {
   }
 
   /**
-   * Runs DB updates.
+   * Applies any pending DB updates through the Update UI.
    */
-  protected function runUpdates() {
+  protected function applyUpdates() {
     $this->drupalGet(Url::fromRoute('system.db_update'));
     $this->clickLink($this->t('Continue'));
     $this->clickLink($this->t('Apply pending updates'));

core/modules/system/tests/modules/update_order_test/update_order_test.info.yml

-name: 'Update order test'
-type: module
-description: 'Support module for update testing.'
-package: Testing
-version: VERSION
-core: 8.x

core/modules/system/tests/modules/update_order_test/update_order_test.install

-<?php
-
-/**
- * @file
- * Update hooks for the update_order_test module.
- */
-
-use Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface;
-
-/**
- * Only declare the update hooks once the test is running.
- *
- * @see \Drupal\system\Tests\Entity\Update\SqlContentEntityStorageSchemaIndexTest
- */
-if (\Drupal::state()->get('update_order_test', FALSE)) {
-
-  /**
-   * Runs before entity schema updates.
-   */
-  function update_order_test_update_8001() {
-    // Store whether the node__default_langcode index exists when this hook is
-    // invoked.
-    \Drupal::state()->set('update_order_test_update_8001', db_index_exists('node_field_data', 'node__default_langcode'));
-  }
-
-  /**
-   * Runs before entity schema updates.
-   */
-  function update_order_test_update_8002() {
-    // Check and store whether the update_order_test field exists when this
-    // hook is first invoked.
-    \Drupal::state()->set('update_order_test_update_8002_update_order_test_before', db_field_exists('node_field_data', 'update_order_test'));
-
-    // Attempt to apply the update for the update_order_test field and then
-    // check and store again whether it exists.
-    if (\Drupal::service('entity.definition_update_manager')->applyFieldUpdate(EntityDefinitionUpdateManagerInterface::DEFINITION_CREATED, 'node', 'update_order_test')) {
-      \Drupal::state()->set('update_order_test_update_8002_update_order_test_after', db_field_exists('node_field_data', 'update_order_test'));
-    }
-
-    // Attempt to apply all node entity type updates.
-    if (\Drupal::service('entity.definition_update_manager')->applyEntityUpdate(EntityDefinitionUpdateManagerInterface::DEFINITION_UPDATED, 'node')) {
-      // Node updates have now run. Check and store whether the updated node
-      // indices now exist.
-      \Drupal::state()->set('update_order_test_update_8002_node__default_langcode', db_index_exists('node_field_data', 'node__default_langcode'));
-      \Drupal::state()->set('update_order_test_update_8002_node__id__default_langcode__langcode', db_index_exists('node_field_data', 'node__id__default_langcode__langcode'));
-
-      // User updates have not yet run. Check and store whether the updated
-      // user indices now exist.
-      \Drupal::state()->set('update_order_test_update_8002_user__id__default_langcode__langcode', db_index_exists('users_field_data', 'user__id__default_langcode__langcode'));
-    }
-  }
-
-}

core/modules/system/tests/modules/update_order_test/update_order_test.module

-<?php
-
-/**
- * @file
- * Hooks for the update_order_test module.
- */
-
-use Drupal\Core\Entity\EntityTypeInterface;
-use Drupal\Core\Field\BaseFieldDefinition;
-
-/**
- * Only declare the new entity base field once the test is running.
- */
-if (\Drupal::state()->get('update_order_test', FALSE)) {
-
-  /**
-   * Implements hook_entity_base_field_info().
-   */
-  function update_order_test_entity_base_field_info(EntityTypeInterface $entity_type) {
-    if ($entity_type->id() === 'node') {
-      $fields['update_order_test'] = BaseFieldDefinition::create('integer')
-        ->setLabel(t('Update order test'));
-      return $fields;
-    }
-  }
-
-}