Issue #2976035 by amateescu, blazey, hchonov, plach, tstoeckler, catch,...

Issue #2976035 by amateescu, blazey, hchonov, plach, tstoeckler, catch, jibran, larowlan, alexpott: Entity type CRUD operations must use the last installed entity type and field storage definitions

core/lib/Drupal/Core/Entity/EntityDefinitionUpdateManager.php

@@ -172,33 +172,7 @@ public function getChangeSummary() {
    * {@inheritdoc}
    */
   public function applyUpdates() {
-    $complete_change_list = $this->getChangeList();
-    if ($complete_change_list) {
-      // self::getChangeList() only disables the cache and does not invalidate.
-      // In case there are changes, explicitly invalidate caches.
-      $this->clearCachedDefinitions();
-    }
-    foreach ($complete_change_list as $entity_type_id => $change_list) {
-      // Process entity type definition changes before storage definitions ones
-      // this is necessary when you change an entity type from non-revisionable
-      // to revisionable and at the same time add revisionable fields to the
-      // entity type.
-      if (!empty($change_list['entity_type'])) {
-        $this->doEntityUpdate($change_list['entity_type'], $entity_type_id);
-      }
-
-      // Process field storage definition changes.
-      if (!empty($change_list['field_storage_definitions'])) {
-        $storage_definitions = $this->entityFieldManager->getFieldStorageDefinitions($entity_type_id);
-        $original_storage_definitions = $this->entityLastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions($entity_type_id);
-
-        foreach ($change_list['field_storage_definitions'] as $field_name => $change) {
-          $storage_definition = isset($storage_definitions[$field_name]) ? $storage_definitions[$field_name] : NULL;
-          $original_storage_definition = isset($original_storage_definitions[$field_name]) ? $original_storage_definitions[$field_name] : NULL;
-          $this->doFieldUpdate($change, $storage_definition, $original_storage_definition);
-        }
-      }
-    }
+    trigger_error('EntityDefinitionUpdateManagerInterface::applyUpdates() is deprecated in 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface::getChangeList() and execute each entity type and field storage update manually instead. See https://www.drupal.org/node/3034742.', E_USER_DEPRECATED);
   }
 
   /**
@@ -298,69 +272,9 @@ public function uninstallFieldStorageDefinition(FieldStorageDefinitionInterface
   }
 
   /**
-   * Performs an entity type definition update.
-   *
-   * @param string $op
-   *   The operation to perform, either static::DEFINITION_CREATED or
-   *   static::DEFINITION_UPDATED.
-   * @param string $entity_type_id
-   *   The entity type ID.
-   */
-  protected function doEntityUpdate($op, $entity_type_id) {
-    $entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
-    switch ($op) {
-      case static::DEFINITION_CREATED:
-        $this->entityTypeListener->onEntityTypeCreate($entity_type);
-        break;
-
-      case static::DEFINITION_UPDATED:
-        $original = $this->entityLastInstalledSchemaRepository->getLastInstalledDefinition($entity_type_id);
-        $this->entityTypeListener->onEntityTypeUpdate($entity_type, $original);
-        break;
-    }
-  }
-
-  /**
-   * Performs a field storage definition update.
-   *
-   * @param string $op
-   *   The operation to perform, possible values are static::DEFINITION_CREATED,
-   *   static::DEFINITION_UPDATED or static::DEFINITION_DELETED.
-   * @param array|null $storage_definition
-   *   The new field storage definition.
-   * @param array|null $original_storage_definition
-   *   The original field storage definition.
-   */
-  protected function doFieldUpdate($op, $storage_definition = NULL, $original_storage_definition = NULL) {
-    switch ($op) {
-      case static::DEFINITION_CREATED:
-        $this->fieldStorageDefinitionListener->onFieldStorageDefinitionCreate($storage_definition);
-        break;
-
-      case static::DEFINITION_UPDATED:
-        $this->fieldStorageDefinitionListener->onFieldStorageDefinitionUpdate($storage_definition, $original_storage_definition);
-        break;
-
-      case static::DEFINITION_DELETED:
-        $this->fieldStorageDefinitionListener->onFieldStorageDefinitionDelete($original_storage_definition);
-        break;
-    }
-  }
-
-  /**
-   * Gets a list of changes to entity type and field storage definitions.
-   *
-   * @return array
-   *   An associative array keyed by entity type id of change descriptors. Every
-   *   entry is an associative array with the following optional keys:
-   *   - entity_type: a scalar having only the DEFINITION_UPDATED value.
-   *   - field_storage_definitions: an associative array keyed by field name of
-   *     scalars having one value among:
-   *     - DEFINITION_CREATED
-   *     - DEFINITION_UPDATED
-   *     - DEFINITION_DELETED
+   * {@inheritdoc}
    */
-  protected function getChangeList() {
+  public function getChangeList() {
     $this->entityTypeManager->useCaches(FALSE);
     $this->entityFieldManager->useCaches(FALSE);
     $change_list = [];

core/lib/Drupal/Core/Entity/EntityDefinitionUpdateManagerInterface.php

@@ -79,6 +79,23 @@ public function needsUpdates();
    */
   public function getChangeSummary();
 
+  /**
+   * Gets a list of changes to entity type and field storage definitions.
+   *
+   * @return array
+   *   An associative array keyed by entity type ID of change descriptors. Every
+   *   entry is an associative array with the following optional keys:
+   *   - entity_type: a scalar having one value among:
+   *     - EntityDefinitionUpdateManagerInterface::DEFINITION_CREATED
+   *     - EntityDefinitionUpdateManagerInterface::DEFINITION_UPDATED
+   *   - field_storage_definitions: an associative array keyed by field name of
+   *     scalars having one value among:
+   *     - EntityDefinitionUpdateManagerInterface::DEFINITION_CREATED
+   *     - EntityDefinitionUpdateManagerInterface::DEFINITION_UPDATED
+   *     - EntityDefinitionUpdateManagerInterface::DEFINITION_DELETED
+   */
+  public function getChangeList();
+
   /**
    * Applies all the detected valid changes.
    *
@@ -90,6 +107,12 @@ public function getChangeSummary();
    *   unacceptable data loss. In such a case, the site administrator needs to
    *   apply some other process, such as a custom update function or a
    *   migration via the Migrate module.
+   *
+   * @deprecated in Drupal 8.7.0, will be removed before Drupal 9.0.0. Use
+   *    \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface::getChangeList()
+   *    and execute each entity type and field storage update manually instead.
+   *
+   * @see https://www.drupal.org/node/3034742
    */
   public function applyUpdates();
 

core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php

@@ -184,6 +184,7 @@ public static function create(ContentEntityTypeInterface $entity_type, array $st
     $all_fields = array_merge($key_fields, array_diff($all_fields, $key_fields));
 
     $revision_metadata_fields = $revisionable ? array_values($entity_type->getRevisionMetadataKeys()) : [];
+    $revision_metadata_fields = array_intersect($revision_metadata_fields, array_keys($storage_definitions));
 
     if (!$revisionable && !$translatable) {
       // The base layout stores all the base field values in the base table.

core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php

@@ -7,7 +7,6 @@
 use Drupal\Core\Entity\EntityFieldManagerInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Entity\FieldableEntityStorageInterface;
-use Drupal\Core\Entity\Sql\SqlContentEntityStorage;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 /**
@@ -82,16 +81,7 @@ public function onFieldStorageDefinitionCreate(FieldStorageDefinitionInterface $
 
     // @todo Forward this to all interested handlers, not only storage, once
     //   iterating handlers is possible: https://www.drupal.org/node/2332857.
-    $storage = clone $this->entityTypeManager->getStorage($entity_type_id);
-
-    // Entity type definition updates can change the schema by adding or
-    // removing entity tables (for example when switching an entity type from
-    // non-revisionable to revisionable), so CRUD operations on a field storage
-    // definition need to use the last installed entity type schema.
-    if ($storage instanceof SqlContentEntityStorage
-       && ($last_installed_entity_type = $this->entityLastInstalledSchemaRepository->getLastInstalledDefinition($entity_type_id))) {
-      $storage->setEntityType($last_installed_entity_type);
-    }
+    $storage = $this->entityTypeManager->getStorage($entity_type_id);
 
     if ($storage instanceof FieldStorageDefinitionListenerInterface) {
       $storage->onFieldStorageDefinitionCreate($storage_definition);
@@ -111,16 +101,7 @@ public function onFieldStorageDefinitionUpdate(FieldStorageDefinitionInterface $
 
     // @todo Forward this to all interested handlers, not only storage, once
     //   iterating handlers is possible: https://www.drupal.org/node/2332857.
-    $storage = clone $this->entityTypeManager->getStorage($entity_type_id);
-
-    // Entity type definition updates can change the schema by adding or
-    // removing entity tables (for example when switching an entity type from
-    // non-revisionable to revisionable), so CRUD operations on a field storage
-    // definition need to use the last installed entity type schema.
-    if ($storage instanceof SqlContentEntityStorage
-       && ($last_installed_entity_type = $this->entityLastInstalledSchemaRepository->getLastInstalledDefinition($entity_type_id))) {
-      $storage->setEntityType($last_installed_entity_type);
-    }
+    $storage = $this->entityTypeManager->getStorage($entity_type_id);
 
     if ($storage instanceof FieldStorageDefinitionListenerInterface) {
       $storage->onFieldStorageDefinitionUpdate($storage_definition, $original);
@@ -140,16 +121,7 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $
 
     // @todo Forward this to all interested handlers, not only storage, once
     //   iterating handlers is possible: https://www.drupal.org/node/2332857.
-    $storage = clone $this->entityTypeManager->getStorage($entity_type_id);
-
-    // Entity type definition updates can change the schema by adding or
-    // removing entity tables (for example when switching an entity type from
-    // non-revisionable to revisionable), so CRUD operations on a field storage
-    // definition need to use the last installed entity type schema.
-    if ($storage instanceof SqlContentEntityStorage
-       && ($last_installed_entity_type = $this->entityLastInstalledSchemaRepository->getLastInstalledDefinition($entity_type_id))) {
-      $storage->setEntityType($last_installed_entity_type);
-    }
+    $storage = $this->entityTypeManager->getStorage($entity_type_id);
 
     // Keep the field definition in the deleted fields repository so we can use
     // it later during field_purge_batch(), but only if the field has data.

core/modules/block/tests/src/Kernel/Migrate/d6/MigrateBlockContentTranslationTest.php

@@ -35,9 +35,9 @@ class MigrateBlockContentTranslationTest extends MigrateDrupal6TestBase {
    */
   protected function setUp() {
     parent::setUp();
+    $this->installEntitySchema('block_content');
     $this->installConfig(['block']);
     $this->installConfig(['block_content']);
-    $this->installEntitySchema('block_content');
 
     $this->executeMigrations([
       'd6_filter_format',

core/modules/block/tests/src/Kernel/Migrate/d6/MigrateBlockTest.php

@@ -38,8 +38,8 @@ protected function setUp() {
     // Install the themes used for this test.
     $this->container->get('theme_installer')->install(['bartik', 'test_theme']);
 
-    $this->installConfig(['block_content']);
     $this->installEntitySchema('block_content');
+    $this->installConfig(['block_content']);
 
     // Set Bartik as the default public theme.
     $config = $this->config('system.theme');

core/modules/block/tests/src/Kernel/Migrate/d7/MigrateBlockContentTranslationTest.php

@@ -39,9 +39,9 @@ class MigrateBlockContentTranslationTest extends MigrateDrupal7TestBase {
    */
   protected function setUp() {
     parent::setUp();
+    $this->installEntitySchema('block_content');
     $this->installConfig(['block']);
     $this->installConfig(['block_content']);
-    $this->installEntitySchema('block_content');
 
     $this->executeMigrations([
       'language',

core/modules/block/tests/src/Kernel/Migrate/d7/MigrateBlockTest.php

@@ -37,8 +37,8 @@ protected function setUp() {
     // Install the themes used for this test.
     $this->container->get('theme_installer')->install(['bartik', 'seven']);
 
-    $this->installConfig(static::$modules);
     $this->installEntitySchema('block_content');
+    $this->installConfig(static::$modules);
 
     // Set Bartik and Seven as the default public and admin theme.
     $config = $this->config('system.theme');

core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentBodyFieldTest.php

@@ -22,8 +22,8 @@ class MigrateBlockContentBodyFieldTest extends MigrateDrupal7TestBase {
    */
   protected function setUp() {
     parent::setUp();
-    $this->installConfig(['block_content']);
     $this->installEntitySchema('block_content');
+    $this->installConfig(['block_content']);
     $this->executeMigrations([
       'block_content_type',
       'block_content_body_field',

core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityDisplayTest.php

@@ -22,6 +22,7 @@ class MigrateBlockContentEntityDisplayTest extends MigrateDrupal7TestBase {
    */
   protected function setUp() {
     parent::setUp();
+    $this->installEntitySchema('block_content');
     $this->installConfig(static::$modules);
     $this->executeMigrations([
       'block_content_type',

core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityFormDisplayTest.php

@@ -22,6 +22,7 @@ class MigrateBlockContentEntityFormDisplayTest extends MigrateDrupal7TestBase {
    */
   protected function setUp() {
     parent::setUp();
+    $this->installEntitySchema('block_content');
     $this->installConfig(static::$modules);
     $this->executeMigrations([
       'block_content_type',

core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentTypeTest.php

@@ -20,8 +20,8 @@ class MigrateBlockContentTypeTest extends MigrateDrupal7TestBase {
    */
   protected function setUp() {
     parent::setUp();
-    $this->installConfig(['block_content']);
     $this->installEntitySchema('block_content');
+    $this->installConfig(['block_content']);
     $this->executeMigration('block_content_type');
   }
 

core/modules/block_content/tests/src/Kernel/Migrate/d6/MigrateBlockContentTest.php

@@ -22,8 +22,8 @@ class MigrateBlockContentTest extends MigrateDrupal6TestBase {
    */
   protected function setUp() {
     parent::setUp();
-    $this->installConfig(['block_content']);
     $this->installEntitySchema('block_content');
+    $this->installConfig(['block_content']);
 
     $this->executeMigrations([
       'd6_filter_format',

core/modules/block_content/tests/src/Kernel/Migrate/d6/MigrateCustomBlockContentTranslationTest.php

@@ -28,8 +28,8 @@ class MigrateCustomBlockContentTranslationTest extends MigrateDrupal6TestBase {
    */
   protected function setUp() {
     parent::setUp();
-    $this->installConfig(['block_content']);
     $this->installEntitySchema('block_content');
+    $this->installConfig(['block_content']);
     $this->executeMigrations([
       'language',
       'd6_filter_format',

core/modules/block_content/tests/src/Kernel/Migrate/d7/MigrateCustomBlockContentTranslationTest.php

@@ -30,8 +30,8 @@ class MigrateCustomBlockContentTranslationTest extends MigrateDrupal7TestBase {
    */
   protected function setUp() {
     parent::setUp();
-    $this->installConfig(['block_content']);
     $this->installEntitySchema('block_content');
+    $this->installConfig(['block_content']);
     $this->executeMigrations([
       'language',
       'd7_filter_format',

core/modules/block_content/tests/src/Kernel/Migrate/d7/MigrateCustomBlockTest.php

@@ -24,8 +24,8 @@ class MigrateCustomBlockTest extends MigrateDrupal7TestBase {
    */
   protected function setUp() {
     parent::setUp();
-    $this->installConfig(static::$modules);
     $this->installEntitySchema('block_content');
+    $this->installConfig(static::$modules);
 
     $this->executeMigrations([
       'd7_filter_format',

core/modules/book/tests/src/Kernel/Migrate/d6/MigrateBookTest.php

@@ -15,7 +15,7 @@ class MigrateBookTest extends MigrateDrupal6TestBase {
   /**
    * {@inheritdoc}
    */
-  public static $modules = ['book', 'menu_ui'];
+  public static $modules = ['book', 'node', 'menu_ui'];
 
   /**
    * {@inheritdoc}

core/modules/comment/tests/src/Kernel/CommentItemTest.php

@@ -27,6 +27,7 @@ class CommentItemTest extends FieldKernelTestBase {
 
   protected function setUp() {
     parent::setUp();
+    $this->installEntitySchema('comment');
     $this->installSchema('comment', ['comment_entity_statistics']);
     $this->installConfig(['comment']);
   }

core/modules/comment/tests/src/Kernel/CommentStringIdEntitiesTest.php

@@ -30,6 +30,7 @@ class CommentStringIdEntitiesTest extends KernelTestBase {
   protected function setUp() {
     parent::setUp();
     $this->installEntitySchema('comment');
+    $this->installEntitySchema('entity_test_string_id');
     $this->installSchema('comment', ['comment_entity_statistics']);
     // Create the comment body field storage.
     $this->installConfig(['field']);