Issue #3039991 by amateescu, lpeabody, bgreco, plach: Base field purg…

…ing is not handling translatable fields correctly
drupal · Jul 16, 2020 · 92f426f · 92f426f
1 parent 3ff0512
commit 92f426f
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 56 deletions.
diff --git a/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
@@ -846,8 +846,19 @@ protected function getSelectQueryForFieldStorageDeletion($table_name, array $sha
 
     // Add the bundle column.
     if ($bundle = $this->entityType->getKey('bundle')) {
-      if ($base_table) {
-        $select->join($base_table, 'base_table', "entity_table.{$this->entityType->getKey('id')} = %alias.{$this->entityType->getKey('id')}");
+      // The bundle field is not stored in the revision table, so we need to
+      // join the data (or base) table and retrieve it from there.
+      if ($base_table && $base_table !== $table_name) {
+        $join_condition = "entity_table.{$this->entityType->getKey('id')} = %alias.{$this->entityType->getKey('id')}";
+
+        // If the entity type is translatable, we also need to add the langcode
+        // to the join, otherwise we'll get duplicate rows for each language.
+        if ($this->entityType->isTranslatable()) {
+          $langcode = $this->entityType->getKey('langcode');
+          $join_condition .= " AND entity_table.{$langcode} = %alias.{$langcode}";
+        }
+
+        $select->join($base_table, 'base_table', $join_condition);
         $select->addField('base_table', $bundle, 'bundle');
       }
       else {

diff --git a/modules/system/tests/src/Functional/Entity/Traits/EntityDefinitionTestTrait.php b/modules/system/tests/src/Functional/Entity/Traits/EntityDefinitionTestTrait.php
@@ -227,14 +227,18 @@ protected function updateEntityTypeToRevisionableAndTranslatable($perform_update
    * @param bool $is_revisionable
    *   (optional) If the base field should be revisionable or not. Defaults to
    *   FALSE.
-   *  @param bool $set_label
+   * @param bool $set_label
    *   (optional) If the base field should have a label or not. Defaults to
    *   TRUE.
+   * @param bool $is_translatable
+   *   (optional) If the base field should be translatable or not. Defaults to
+   *   FALSE.
    */
-  protected function addBaseField($type = 'string', $entity_type_id = 'entity_test_update', $is_revisionable = FALSE, $set_label = TRUE) {
+  protected function addBaseField($type = 'string', $entity_type_id = 'entity_test_update', $is_revisionable = FALSE, $set_label = TRUE, $is_translatable = FALSE) {
     $definitions['new_base_field'] = BaseFieldDefinition::create($type)
       ->setName('new_base_field')
-      ->setRevisionable($is_revisionable);
+      ->setRevisionable($is_revisionable)
+      ->setTranslatable($is_translatable);
 
     if ($set_label) {
       $definitions['new_base_field']->setLabel(t('A new base field'));

diff --git a/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php b/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php
@@ -17,6 +17,7 @@
 use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Drupal\entity_test\FieldStorageDefinition;
 use Drupal\entity_test_update\Entity\EntityTestUpdate;
+use Drupal\language\Entity\ConfigurableLanguage;
 use Drupal\Tests\system\Functional\Entity\Traits\EntityDefinitionTestTrait;
 
 /**
@@ -56,7 +57,7 @@ class EntityDefinitionUpdateTest extends EntityKernelTestBase {
    *
    * @var array
    */
-  public static $modules = ['entity_test_update'];
+  public static $modules = ['entity_test_update', 'language'];
 
   /**
    * {@inheritdoc}
@@ -504,18 +505,22 @@ public function testBundleFieldCreateDeleteWithExistingEntities() {
    *
    * @dataProvider baseFieldDeleteWithExistingDataTestCases
    */
-  public function testBaseFieldDeleteWithExistingData($entity_type_id, $create_entity_revision, $base_field_revisionable) {
+  public function testBaseFieldDeleteWithExistingData($entity_type_id, $create_entity_revision, $base_field_revisionable, $create_entity_translation) {
+    // Enable an additional language.
+    ConfigurableLanguage::createFromLangcode('ro')->save();
+
     /** @var \Drupal\Core\Entity\Sql\SqlEntityStorageInterface $storage */
     $storage = $this->entityTypeManager->getStorage($entity_type_id);
     $schema_handler = $this->database->schema();
 
     // Create an entity without the base field, to ensure NULL values are not
     // added to the dedicated table storage to be purged.
+    /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
     $entity = $storage->create();
     $entity->save();
 
     // Add the base field and run the update.
-    $this->addBaseField('string', $entity_type_id, $base_field_revisionable);
+    $this->addBaseField('string', $entity_type_id, $base_field_revisionable, TRUE, $create_entity_translation);
     $this->applyEntityUpdates();
 
     /** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */
@@ -526,10 +531,22 @@ public function testBaseFieldDeleteWithExistingData($entity_type_id, $create_ent
     $entity = $storage->create(['new_base_field' => 'foo']);
     $entity->save();
 
+    if ($create_entity_translation) {
+      $translation = $entity->addTranslation('ro', ['new_base_field' => 'foo-ro']);
+      $translation->save();
+    }
+
     if ($create_entity_revision) {
       $entity->setNewRevision(TRUE);
+      $entity->isDefaultRevision(FALSE);
       $entity->new_base_field = 'bar';
       $entity->save();
+
+      if ($create_entity_translation) {
+        $translation = $entity->getTranslation('ro');
+        $translation->new_base_field = 'bar-ro';
+        $translation->save();
+      }
     }
 
     // Remove the base field and apply updates.
@@ -544,65 +561,81 @@ public function testBaseFieldDeleteWithExistingData($entity_type_id, $create_ent
     $dedicated_deleted_table_name = $table_mapping->getDedicatedDataTableName($storage_definition, TRUE);
     $this->assertTrue($schema_handler->tableExists($dedicated_deleted_table_name), 'A dedicated table was created for the deleted new_base_field.');
 
+    $expected[] = [
+      'bundle' => $entity->bundle(),
+      'deleted' => '1',
+      'entity_id' => '2',
+      'revision_id' => '2',
+      'langcode' => 'en',
+      'delta' => '0',
+      'new_base_field_value' => 'foo',
+    ];
+
+    if ($create_entity_translation) {
+      $expected[] = [
+        'bundle' => $entity->bundle(),
+        'deleted' => '1',
+        'entity_id' => '2',
+        'revision_id' => '2',
+        'langcode' => 'ro',
+        'delta' => '0',
+        'new_base_field_value' => 'foo-ro',
+      ];
+    }
+
     // Check that the deleted field's data is preserved in the dedicated
     // 'deleted' table.
     $result = $this->database->select($dedicated_deleted_table_name, 't')
       ->fields('t')
+      ->orderBy('revision_id', 'ASC')
+      ->orderBy('langcode', 'ASC')
       ->execute()
-      ->fetchAll();
-    $this->assertCount(1, $result);
+      ->fetchAll(\PDO::FETCH_ASSOC);
+    $this->assertCount(count($expected), $result);
 
-    $expected = [
-      'bundle' => $entity->bundle(),
-      'deleted' => '1',
-      'entity_id' => $entity->id(),
-      'revision_id' => $create_entity_revision ? $entity->getRevisionId() : $entity->id(),
-      'langcode' => $entity->language()->getId(),
-      'delta' => '0',
-      'new_base_field_value' => $entity->new_base_field->value,
-    ];
     // Use assertEquals and not assertSame here to prevent that a different
     // sequence of the columns in the table will affect the check.
-    $this->assertEquals($expected, (array) $result[0]);
+    $this->assertEquals($expected, $result);
 
     if ($create_entity_revision) {
       $dedicated_deleted_revision_table_name = $table_mapping->getDedicatedRevisionTableName($storage_definition, TRUE);
       $this->assertTrue($schema_handler->tableExists($dedicated_deleted_revision_table_name), 'A dedicated revision table was created for the deleted new_base_field.');
 
+      if ($base_field_revisionable) {
+        $expected[] = [
+          'bundle' => $entity->bundle(),
+          'deleted' => '1',
+          'entity_id' => '2',
+          'revision_id' => '3',
+          'langcode' => 'en',
+          'delta' => '0',
+          'new_base_field_value' => 'bar',
+        ];
+
+        if ($create_entity_translation) {
+          $expected[] = [
+            'bundle' => $entity->bundle(),
+            'deleted' => '1',
+            'entity_id' => '2',
+            'revision_id' => '3',
+            'langcode' => 'ro',
+            'delta' => '0',
+            'new_base_field_value' => 'bar-ro',
+          ];
+        }
+      }
+
       $result = $this->database->select($dedicated_deleted_revision_table_name, 't')
         ->fields('t')
-        ->orderBy('revision_id', 'DESC')
+        ->orderBy('revision_id', 'ASC')
+        ->orderBy('langcode', 'ASC')
         ->execute()
-        ->fetchAll();
-      // Only one row will be created for non-revisionable base fields.
-      $this->assertCount($base_field_revisionable ? 2 : 1, $result);
+        ->fetchAll(\PDO::FETCH_ASSOC);
+      $this->assertCount(count($expected), $result);
 
       // Use assertEquals and not assertSame here to prevent that a different
       // sequence of the columns in the table will affect the check.
-      $this->assertEquals([
-        'bundle' => $entity->bundle(),
-        'deleted' => '1',
-        'entity_id' => $entity->id(),
-        'revision_id' => '3',
-        'langcode' => $entity->language()->getId(),
-        'delta' => '0',
-        'new_base_field_value' => 'bar',
-      ], (array) $result[0]);
-
-      // Two rows only exist if the base field is revisionable.
-      if ($base_field_revisionable) {
-        // Use assertEquals and not assertSame here to prevent that a different
-        // sequence of the columns in the table will affect the check.
-        $this->assertEquals([
-          'bundle' => $entity->bundle(),
-          'deleted' => '1',
-          'entity_id' => $entity->id(),
-          'revision_id' => '2',
-          'langcode' => $entity->language()->getId(),
-          'delta' => '0',
-          'new_base_field_value' => 'foo',
-        ], (array) $result[1]);
-      }
+      $this->assertEquals($expected, $result);
     }
 
     // Check that the field storage definition is marked for purging.
@@ -626,45 +659,65 @@ public function testBaseFieldDeleteWithExistingData($entity_type_id, $create_ent
    */
   public function baseFieldDeleteWithExistingDataTestCases() {
     return [
-      'Non-revisionable entity type' => [
+      'Non-revisionable, non-translatable entity type' => [
         'entity_test_update',
         FALSE,
         FALSE,
+        FALSE,
       ],
-      'Non-revisionable custom data table' => [
+      'Non-revisionable, non-translatable custom data table' => [
         'entity_test_mul',
         FALSE,
         FALSE,
+        FALSE,
       ],
-      'Non-revisionable entity type, revisionable base field' => [
+      'Non-revisionable, non-translatable entity type, revisionable base field' => [
         'entity_test_update',
         FALSE,
         TRUE,
+        FALSE,
       ],
-      'Non-revisionable custom data table, revisionable base field' => [
+      'Non-revisionable, non-translatable custom data table, revisionable base field' => [
         'entity_test_mul',
         FALSE,
         TRUE,
+        FALSE,
       ],
-      'Revisionable entity type, non revisionable base field' => [
+      'Revisionable, translatable entity type, non revisionable and non-translatable base field' => [
         'entity_test_mulrev',
         TRUE,
         FALSE,
+        FALSE,
       ],
-      'Revisionable entity type, revisionable base field' => [
+      'Revisionable, translatable entity type, revisionable and non-translatable base field' => [
         'entity_test_mulrev',
         TRUE,
         TRUE,
+        FALSE,
       ],
-      'Non-translatable revisionable entity type, revisionable base field' => [
+      'Revisionable and non-translatable entity type, revisionable and non-translatable base field' => [
         'entity_test_rev',
         TRUE,
         TRUE,
+        FALSE,
       ],
-      'Non-translatable revisionable entity type, non-revisionable base field' => [
+      'Revisionable and non-translatable entity type, non-revisionable and non-translatable base field' => [
         'entity_test_rev',
         TRUE,
         FALSE,
+        FALSE,
+      ],
+      'Revisionable and translatable entity type, non-revisionable and translatable base field' => [
+        'entity_test_mulrev',
+        TRUE,
+        FALSE,
+        TRUE,
+      ],
+      'Revisionable and translatable entity type, revisionable and translatable base field' => [
+        'entity_test_mulrev',
+        TRUE,
+        TRUE,
+        TRUE,
       ],
     ];
   }