Issue #2644088 by amateescu, kevin.dutra, dawehner, plach:...

Issue #2644088 by amateescu, kevin.dutra, dawehner, plach: DefaultTableMapping::getFieldTableName does not report table for fields with dedicated storage

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

@@ -147,15 +147,16 @@ public function getFieldTableName($field_name) {
       //   https://www.drupal.org/node/2274017.
       /** @var \Drupal\Core\Entity\Sql\SqlContentEntityStorage $storage */
       $storage = \Drupal::entityManager()->getStorage($this->entityType->id());
+      $storage_definition = $this->fieldStorageDefinitions[$field_name];
       $table_names = array(
         $storage->getDataTable(),
         $storage->getBaseTable(),
         $storage->getRevisionTable(),
+        $this->getDedicatedDataTableName($storage_definition),
       );
 
       // Collect field columns.
       $field_columns = array();
-      $storage_definition = $this->fieldStorageDefinitions[$field_name];
       foreach (array_keys($storage_definition->getColumns()) as $property_name) {
         $field_columns[] = $this->getFieldColumnName($storage_definition, $property_name);
       }

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

@@ -282,13 +282,13 @@ public function getTableMapping(array $storage_definitions = NULL) {
       $definitions = $storage_definitions ?: $this->entityManager->getFieldStorageDefinitions($this->entityTypeId);
       $table_mapping = new DefaultTableMapping($this->entityType, $definitions);
 
-      $definitions = array_filter($definitions, function (FieldStorageDefinitionInterface $definition) use ($table_mapping) {
+      $shared_table_definitions = array_filter($definitions, function (FieldStorageDefinitionInterface $definition) use ($table_mapping) {
         return $table_mapping->allowsSharedTableStorage($definition);
       });
 
       $key_fields = array_values(array_filter(array($this->idKey, $this->revisionKey, $this->bundleKey, $this->uuidKey, $this->langcodeKey)));
-      $all_fields = array_keys($definitions);
-      $revisionable_fields = array_keys(array_filter($definitions, function (FieldStorageDefinitionInterface $definition) {
+      $all_fields = array_keys($shared_table_definitions);
+      $revisionable_fields = array_keys(array_filter($shared_table_definitions, function (FieldStorageDefinitionInterface $definition) {
         return $definition->isRevisionable();
       }));
       // Make sure the key fields come first in the list of fields.
@@ -355,7 +355,7 @@ public function getTableMapping(array $storage_definitions = NULL) {
       }
 
       // Add dedicated tables.
-      $definitions = array_filter($definitions, function (FieldStorageDefinitionInterface $definition) use ($table_mapping) {
+      $dedicated_table_definitions = array_filter($definitions, function (FieldStorageDefinitionInterface $definition) use ($table_mapping) {
         return $table_mapping->requiresDedicatedTableStorage($definition);
       });
       $extra_columns = array(
@@ -366,8 +366,12 @@ public function getTableMapping(array $storage_definitions = NULL) {
         'langcode',
         'delta',
       );
-      foreach ($definitions as $field_name => $definition) {
-        foreach (array($table_mapping->getDedicatedDataTableName($definition), $table_mapping->getDedicatedRevisionTableName($definition)) as $table_name) {
+      foreach ($dedicated_table_definitions as $field_name => $definition) {
+        $tables = [$table_mapping->getDedicatedDataTableName($definition)];
+        if ($revisionable && $definition->isRevisionable()) {
+          $tables[] = $table_mapping->getDedicatedRevisionTableName($definition);
+        }
+        foreach ($tables as $table_name) {
           $table_mapping->setFieldNames($table_name, array($field_name));
           $table_mapping->setExtraColumns($table_name, $extra_columns);
         }
@@ -1582,7 +1586,13 @@ public function finalizePurge(FieldStorageDefinitionInterface $storage_definitio
    * {@inheritdoc}
    */
   public function countFieldData($storage_definition, $as_bool = FALSE) {
-    $table_mapping = $this->getTableMapping();
+    // The table mapping contains stale data during a request when a field
+    // storage definition is added, so bypass the internal storage definitions
+    // and fetch the table mapping using the passed in storage definition.
+    // @todo Fix this in https://www.drupal.org/node/2705205.
+    $storage_definitions = $this->entityManager->getFieldStorageDefinitions($this->entityTypeId);
+    $storage_definitions[$storage_definition->getName()] = $storage_definition;
+    $table_mapping = $this->getTableMapping($storage_definitions);
 
     if ($table_mapping->requiresDedicatedTableStorage($storage_definition)) {
       $is_deleted = $this->storageDefinitionIsDeleted($storage_definition);

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

@@ -67,6 +67,9 @@ public function testSingleUpdates() {
     $this->enableUpdates('entity_test', 'entity_definition_updates', 8001);
     $this->applyUpdates();
 
+    // Ensure the 'entity_test__user_id' table got created.
+    $this->assertTrue(\Drupal::database()->schema()->tableExists('entity_test__user_id'));
+
     // Check that data was correctly migrated.
     $entity = $this->reloadEntity($entity);
     $this->assertEqual(count($entity->user_id), 1);

core/modules/views/src/EntityViewsData.php

@@ -235,7 +235,7 @@ public function getViewsData() {
     // Load all typed data definitions of all fields. This should cover each of
     // the entity base, revision, data tables.
     $field_definitions = $this->entityManager->getBaseFieldDefinitions($this->entityType->id());
-    if ($table_mapping = $this->storage->getTableMapping()) {
+    if ($table_mapping = $this->storage->getTableMapping($field_definitions)) {
       // Fetch all fields that can appear in both the base table and the data
       // table.
       $entity_keys = $this->entityType->getKeys();

core/tests/Drupal/KernelTests/Core/Entity/DefaultTableMappingIntegrationTest.php

+<?php
+
+namespace Drupal\KernelTests\Core\Entity;
+
+use Drupal\Core\Field\BaseFieldDefinition;
+use Drupal\Core\Field\FieldStorageDefinitionInterface;
+
+/**
+ * Tests the default table mapping class for content entities stored in SQL.
+ *
+ * @see \Drupal\Core\Entity\Sql\DefaultTableMapping
+ * @see \Drupal\Core\Entity\Sql\TableMappingInterface
+ *
+ * @coversDefaultClass \Drupal\Core\Entity\Sql\DefaultTableMapping
+ * @group Entity
+ */
+class DefaultTableMappingIntegrationTest extends EntityKernelTestBase {
+
+  /**
+   * The table mapping for the tested entity type.
+   *
+   * @var \Drupal\Core\Entity\Sql\TableMappingInterface
+   */
+  protected $tableMapping;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['entity_test_extra'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    // Setup some fields for entity_test_extra to create.
+    $definitions['multivalued_base_field'] = BaseFieldDefinition::create('string')
+      ->setName('multivalued_base_field')
+      ->setTargetEntityTypeId('entity_test_mulrev')
+      ->setTargetBundle('entity_test_mulrev')
+      ->setCardinality(FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED);
+    $this->state->set('entity_test_mulrev.additional_base_field_definitions', $definitions);
+
+    $this->entityManager->clearCachedDefinitions();
+    $this->tableMapping = $this->entityManager->getStorage('entity_test_mulrev')->getTableMapping();
+  }
+
+  /**
+   * Tests DefaultTableMapping::getFieldTableName().
+   *
+   * @covers ::getFieldTableName
+   */
+  public function testGetFieldTableName() {
+    // Test the field table name for a single-valued base field, which is stored
+    // in the entity's base table.
+    $expected = 'entity_test_mulrev';
+    $this->assertEquals($this->tableMapping->getFieldTableName('uuid'), $expected);
+
+    // Test the field table name for a translatable and revisionable base field,
+    // which is stored in the entity's data table.
+    $expected = 'entity_test_mulrev_property_data';
+    $this->assertEquals($this->tableMapping->getFieldTableName('name'), $expected);
+
+    // Test the field table name for a multi-valued base field, which is stored
+    // in a dedicated table.
+    $expected = 'entity_test_mulrev__multivalued_base_field';
+    $this->assertEquals($this->tableMapping->getFieldTableName('multivalued_base_field'), $expected);
+  }
+
+}

core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php

@@ -466,6 +466,42 @@ public function providerTestGetTableMappingSimple() {
     );
   }
 
+  /**
+   * Tests getTableMapping() with a base field that requires a dedicated table.
+   *
+   * @covers ::__construct
+   * @covers ::getTableMapping
+   */
+  public function testGetTableMappingSimpleWithDedicatedStorageFields() {
+    $base_field_names = ['multi_valued_base_field'];
+
+    // Set up one entity key in order to have a base table.
+    $this->fieldDefinitions = $this->mockFieldDefinitions(['test_id']);
+
+    // Set up the multi-valued base field.
+    $this->fieldDefinitions += $this->mockFieldDefinitions($base_field_names, [
+      'hasCustomStorage' => FALSE,
+      'isMultiple' => TRUE,
+      'getTargetEntityTypeId' => 'entity_test',
+    ]);
+
+    $this->setUpEntityStorage();
+
+    $mapping = $this->entityStorage->getTableMapping();
+    $this->assertEquals(['entity_test', 'entity_test__multi_valued_base_field'], $mapping->getTableNames());
+    $this->assertEquals($base_field_names, $mapping->getFieldNames('entity_test__multi_valued_base_field'));
+
+    $extra_columns = array(
+      'bundle',
+      'deleted',
+      'entity_id',
+      'revision_id',
+      'langcode',
+      'delta',
+    );
+    $this->assertEquals($extra_columns, $mapping->getExtraColumns('entity_test__multi_valued_base_field'));
+  }
+
   /**
    * Tests getTableMapping() with a revisionable, non-translatable entity type.
    *