Issue #2753971 by dagmar, hampercm, Berdir, xjm, generalredneck, catch, alexpott: ContentEntityStorageBase::deleteRevision() function does not remove node_field_revision entries

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

@@ -597,6 +597,13 @@ class SqlContentEntityStorage extends ContentEntityStorageBase implements SqlEnt
     $this->database->delete($this->revisionTable)
       ->condition($this->revisionKey, $revision->getRevisionId())
       ->execute();
+
+    if ($this->revisionDataTable) {
+      $this->database->delete($this->revisionDataTable)
+        ->condition($this->revisionKey, $revision->getRevisionId())
+        ->execute();
+    }
+
     $this->deleteRevisionFromDedicatedTables($revision);
   }
 

core/modules/node/src/Tests/NodeRevisionsTest.php

@@ -195,6 +195,7 @@ class NodeRevisionsTest extends NodeTestBase {
                         ['%revision-date' => format_date($nodes[1]->getRevisionCreationTime()),
                               '@type' => 'Basic page', '%title' => $nodes[1]->label()]), 'Revision deleted.');
     $this->assertTrue(db_query('SELECT COUNT(vid) FROM {node_revision} WHERE nid = :nid and vid = :vid', [':nid' => $node->id(), ':vid' => $nodes[1]->getRevisionId()])->fetchField() == 0, 'Revision not found.');
+    $this->assertTrue(db_query('SELECT COUNT(vid) FROM {node_field_revision} WHERE nid = :nid and vid = :vid', [':nid' => $node->id(), ':vid' => $nodes[1]->getRevisionId()])->fetchField() == 0, 'Field revision not found.');
 
     // Set the revision timestamp to an older date to make sure that the
     // confirmation message correctly displays the stored revision date.

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

@@ -101,6 +101,9 @@ class EntityApiTest extends EntityKernelTestBase {
     if ($revision_table = $definition->getRevisionTable()) {
       $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $revision_table . '}')->fetchField(), 'Data table was emptied');
     }
+    if ($revision_data_table = $definition->getRevisionDataTable()) {
+      $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $revision_data_table . '}')->fetchField(), 'Data table was emptied');
+    }
 
     // Test deleting a list of entities not indexed by entity id.
     $entities = [];
@@ -126,6 +129,9 @@ class EntityApiTest extends EntityKernelTestBase {
     if ($revision_table = $definition->getRevisionTable()) {
       $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $revision_table . '}')->fetchField(), 'Data table was emptied');
     }
+    if ($revision_data_table = $definition->getRevisionDataTable()) {
+      $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $revision_data_table . '}')->fetchField(), 'Data table was emptied');
+    }
   }
 
   /**

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

@@ -2,9 +2,12 @@
 
 namespace Drupal\KernelTests\Core\Entity;
 
-use Drupal\entity_test_revlog\Entity\EntityTestWithRevisionLog;
+use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Entity\EntityTypeInterface;
+use Drupal\entity_test_revlog\Entity\EntityTestMulWithRevisionLog;
 use Drupal\KernelTests\KernelTestBase;
 use Drupal\user\Entity\User;
+use Drupal\user\UserInterface;
 
 /**
  * @coversDefaultClass \Drupal\Core\Entity\RevisionableContentEntityBase
@@ -23,34 +26,102 @@ class RevisionableContentEntityBaseTest extends KernelTestBase {
   protected function setUp() {
     parent::setUp();
 
-    $this->installEntitySchema('entity_test_revlog');
+    $this->installEntitySchema('entity_test_mul_revlog');
     $this->installEntitySchema('user');
     $this->installSchema('system', 'sequences');
   }
 
+  /**
+   * Tests the correct functionality CRUD operations of entity revisions.
+   */
   public function testRevisionableContentEntity() {
+    $entity_type = 'entity_test_mul_revlog';
+    $definition = \Drupal::entityManager()->getDefinition($entity_type);
     $user = User::create(['name' => 'test name']);
     $user->save();
-    /** @var \Drupal\entity_test_revlog\Entity\EntityTestWithRevisionLog $entity */
-    $entity = EntityTestWithRevisionLog::create([
-      'type' => 'entity_test_revlog',
+    /** @var \Drupal\entity_test_mul_revlog\Entity\EntityTestMulWithRevisionLog $entity */
+    $entity = EntityTestMulWithRevisionLog::create([
+      'type' => $entity_type,
     ]);
-    $entity->save();
 
+    // Save the entity, this creates the first revision.
+    $entity->save();
+    $revision_ids[] = $entity->getRevisionId();
+    $this->assertItemsTableCount(1, $definition);
+
+    // Create the second revision.
     $entity->setNewRevision(TRUE);
     $random_timestamp = rand(1e8, 2e8);
-    $entity->setRevisionCreationTime($random_timestamp);
-    $entity->setRevisionUserId($user->id());
-    $entity->setRevisionLogMessage('This is my log message');
-    $entity->save();
+    $this->createRevision($entity, $user, $random_timestamp, 'This is my log message');
 
     $revision_id = $entity->getRevisionId();
+    $revision_ids[] = $revision_id;
 
-    $entity = \Drupal::entityTypeManager()->getStorage('entity_test_revlog')->loadRevision($revision_id);
+    $storage = \Drupal::entityTypeManager()->getStorage('entity_test_mul_revlog');
+    $entity = $storage->loadRevision($revision_id);
     $this->assertEquals($random_timestamp, $entity->getRevisionCreationTime());
     $this->assertEquals($user->id(), $entity->getRevisionUserId());
     $this->assertEquals($user->id(), $entity->getRevisionUser()->id());
     $this->assertEquals('This is my log message', $entity->getRevisionLogMessage());
+
+    // Create the third revision.
+    $random_timestamp = rand(1e8, 2e8);
+    $this->createRevision($entity, $user, $random_timestamp, 'This is my log message');
+    $this->assertItemsTableCount(3, $definition);
+    $revision_ids[] = $entity->getRevisionId();
+
+    // Create another 3 revisions.
+    foreach (range(1, 3) as $count) {
+      $timestamp = rand(1e8, 2e8);
+      $this->createRevision($entity, $user, $timestamp, 'This is my log message number: ' . $count);
+      $revision_ids[] = $entity->getRevisionId();
+    }
+    $this->assertItemsTableCount(6, $definition);
+
+    $this->assertEqual(6, count($revision_ids));
+
+    // Delete the first 3 revisions.
+    foreach (range(0, 2) as $key) {
+      $storage->deleteRevision($revision_ids[$key]);
+    }
+
+    // We should have only data for three revisions.
+    $this->assertItemsTableCount(3, $definition);
+  }
+
+  /**
+   * Asserts the ammount of items on entity related tables.
+   *
+   * @param int $count
+   *   The number of items expected to be in revisions related tables.
+   * @param \Drupal\Core\Entity\EntityTypeInterface $definition
+   *   The definition and metada of the entity being tested.
+   */
+  protected function assertItemsTableCount($count, EntityTypeInterface $definition) {
+    $this->assertEqual(1, db_query('SELECT COUNT(*) FROM {' . $definition->getBaseTable() . '}')->fetchField());
+    $this->assertEqual(1, db_query('SELECT COUNT(*) FROM {' . $definition->getDataTable() . '}')->fetchField());
+    $this->assertEqual($count, db_query('SELECT COUNT(*) FROM {' . $definition->getRevisionTable() . '}')->fetchField());
+    $this->assertEqual($count, db_query('SELECT COUNT(*) FROM {' . $definition->getRevisionDataTable() . '}')->fetchField());
+  }
+
+  /**
+   * Creates a new revision in the entity of this test class.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *   The entity where revision will be created.
+   * @param \Drupal\user\UserInterface $user
+   *   The author of the new revision.
+   * @param int $timestamp
+   *   The timestamp of the new revision.
+   * @param string $log_message
+   *   The log message of the new revision.
+   */
+  protected function createRevision(EntityInterface $entity, UserInterface $user, $timestamp, $log_message) {
+    $entity->setNewRevision(TRUE);
+    $entity->setRevisionCreationTime($timestamp);
+    $entity->setRevisionUserId($user->id());
+    $entity->setRevisionLogMessage($log_message);
+    $entity->save();
   }
 
 }