Issue #2809227 by timmillwood, Berdir, hchonov, catch, amateescu, alexpott:...

Issue #2809227 by timmillwood, Berdir, hchonov, catch, amateescu, alexpott: Store revision id in originalRevisionId property to use after revision id is updated

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

@@ -148,6 +148,13 @@ abstract class ContentEntityBase extends Entity implements \IteratorAggregate, C
    */
   protected $validationRequired = FALSE;
 
+  /**
+   * The loaded revision ID before the new revision was set.
+   *
+   * @var int
+   */
+  protected $loadedRevisionId;
+
   /**
    * {@inheritdoc}
    */
@@ -219,6 +226,11 @@ public function __construct(array $values, $entity_type, $bundle = FALSE, $trans
         }
       }
     }
+    if ($this->getEntityType()->isRevisionable()) {
+      // Store the loaded revision ID the entity has been loaded with to
+      // keep it safe from changes.
+      $this->updateLoadedRevisionId();
+    }
   }
 
   /**
@@ -270,6 +282,21 @@ public function setNewRevision($value = TRUE) {
     $this->newRevision = $value;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getLoadedRevisionId() {
+    return $this->loadedRevisionId;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function updateLoadedRevisionId() {
+    $this->loadedRevisionId = $this->getRevisionId() ?: $this->loadedRevisionId;
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -799,6 +826,7 @@ protected function initializeTranslation($langcode) {
     $translation->translatableEntityKeys = &$this->translatableEntityKeys;
     $translation->translationInitialize = FALSE;
     $translation->typedData = NULL;
+    $translation->loadedRevisionId = &$this->loadedRevisionId;
 
     return $translation;
   }
@@ -1025,6 +1053,7 @@ public function createDuplicate() {
     // Check whether the entity type supports revisions and initialize it if so.
     if ($entity_type->isRevisionable()) {
       $duplicate->{$entity_type->getKey('revision')}->value = NULL;
+      $duplicate->loadedRevisionId = NULL;
     }
 
     return $duplicate;
@@ -1066,6 +1095,11 @@ public function __clone() {
       // original reference with one pointing to a copy of it.
       $enforce_is_new = $this->enforceIsNew;
       $this->enforceIsNew = &$enforce_is_new;
+
+      // Ensure the loadedRevisionId property is actually cloned by
+      // overwriting the original reference with one pointing to a copy of it.
+      $original_revision_id = $this->loadedRevisionId;
+      $this->loadedRevisionId = &$original_revision_id;
     }
   }
 

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

@@ -53,4 +53,25 @@ public function setRevisionTranslationAffected($affected);
    */
   public function isRevisionTranslationAffected();
 
+  /**
+   * Gets the loaded Revision ID of the entity.
+   *
+   * @return int
+   *   The loaded Revision identifier of the entity, or NULL if the entity
+   *   does not have a revision identifier.
+   */
+  public function getLoadedRevisionId();
+
+  /**
+   * Updates the loaded Revision ID with the revision ID.
+   *
+   * This method should not be used, it could unintentionally cause the original
+   * revision ID property value to be lost.
+   *
+   * @internal
+   *
+   * @return $this
+   */
+  public function updateLoadedRevisionId();
+
 }

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

@@ -288,6 +288,13 @@ protected function doPreSave(EntityInterface $entity) {
     // Sync the changes made in the fields array to the internal values array.
     $entity->updateOriginalValues();
 
+    if ($entity->getEntityType()->isRevisionable() && !$entity->isNew() && empty($entity->getLoadedRevisionId())) {
+      // Update the loaded revision id for rare special cases when no loaded
+      // revision is given when updating an existing entity. This for example
+      // happens when calling save() in hook_entity_insert().
+      $entity->updateLoadedRevisionId();
+    }
+
     return parent::doPreSave($entity);
   }
 
@@ -305,6 +312,7 @@ protected function doPostSave(EntityInterface $entity, $update) {
 
     // The revision is stored, it should no longer be marked as new now.
     if ($this->entityType->isRevisionable()) {
+      $entity->updateLoadedRevisionId();
       $entity->setNewRevision(FALSE);
     }
   }

core/modules/system/tests/modules/entity_test/entity_test.module

@@ -6,6 +6,7 @@
  */
 
 use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\FieldableEntityInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
@@ -412,6 +413,25 @@ function entity_test_entity_test_insert($entity) {
   }
 }
 
+/**
+ * Implements hook_entity_insert().
+ */
+function entity_test_entity_insert(EntityInterface $entity) {
+  if ($entity->getEntityTypeId() == 'entity_test_mulrev' && $entity->label() == 'EntityLoadedRevisionTest') {
+    $entity->setNewRevision(FALSE);
+    $entity->save();
+  }
+}
+
+/**
+ * Implements hook_entity_update().
+ */
+function entity_test_entity_update(EntityInterface $entity) {
+  if ($entity instanceof ContentEntityInterface) {
+    \Drupal::state()->set('entity_test.loadedRevisionId', $entity->getLoadedRevisionId());
+  }
+}
+
 /**
  * Implements hook_entity_field_access().
  *

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

+<?php
+
+namespace Drupal\KernelTests\Core\Entity;
+
+use Drupal\entity_test\Entity\EntityTestMulRev;
+use Drupal\language\Entity\ConfigurableLanguage;
+
+/**
+ * Tests the loaded Revision of an entity.
+ *
+ * @group entity
+ */
+class EntityLoadedRevisionTest extends EntityKernelTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = [
+    'system',
+    'entity_test',
+    'language',
+    'content_translation',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->installEntitySchema('entity_test_mulrev');
+  }
+
+  /**
+   * Test getLoadedRevisionId() returns the correct ID throughout the process.
+   */
+  public function testLoadedRevisionId() {
+    // Create a basic EntityTestMulRev entity and save it.
+    $entity = EntityTestMulRev::create();
+    $entity->save();
+
+    // Load the created entity and create a new revision.
+    $loaded = EntityTestMulRev::load($entity->id());
+    $loaded->setNewRevision(TRUE);
+
+    // Before saving, the loaded Revision ID should be the same as the created
+    // entity, not the same as the loaded entity (which does not have a revision
+    // ID yet).
+    $this->assertEquals($entity->getRevisionId(), $loaded->getLoadedRevisionId());
+    $this->assertNotEquals($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+    $this->assertSame(NULL, $loaded->getRevisionId());
+
+    // After updating the loaded Revision ID the result should be the same.
+    $loaded->updateLoadedRevisionId();
+    $this->assertEquals($entity->getRevisionId(), $loaded->getLoadedRevisionId());
+    $this->assertNotEquals($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+    $this->assertSame(NULL, $loaded->getRevisionId());
+
+    $loaded->save();
+
+    // In entity_test_entity_update() the loaded Revision ID was stored in
+    // state. This should be the same as we had before calling $loaded->save().
+    /** @var \Drupal\Core\Entity\ContentEntityInterface $loaded_original */
+    $loadedRevisionId = \Drupal::state()->get('entity_test.loadedRevisionId');
+    $this->assertEquals($entity->getRevisionId(), $loadedRevisionId);
+    $this->assertNotEquals($loaded->getRevisionId(), $loadedRevisionId);
+
+    // The revision ID and loaded Revision ID should be different for the two
+    // versions of the entity, but the same for a saved entity.
+    $this->assertNotEquals($loaded->getRevisionId(), $entity->getRevisionId());
+    $this->assertNotEquals($loaded->getLoadedRevisionId(), $entity->getLoadedRevisionId());
+    $this->assertEquals($entity->getRevisionId(), $entity->getLoadedRevisionId());
+    $this->assertEquals($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+  }
+
+  /**
+   * Tests the loaded revision ID after an entity re-save, clone and duplicate.
+   */
+  public function testLoadedRevisionIdWithNoNewRevision() {
+    // Create a basic EntityTestMulRev entity and save it.
+    $entity = EntityTestMulRev::create();
+    $entity->save();
+
+    // Load the created entity and create a new revision.
+    $loaded = EntityTestMulRev::load($entity->id());
+    $loaded->setNewRevision(TRUE);
+    $loaded->save();
+
+    // Make a change to the loaded entity.
+    $loaded->set('name', 'dublin');
+
+    // The revision id and loaded Revision id should still be the same.
+    $this->assertEquals($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+
+    $loaded->save();
+
+    // After saving, the loaded Revision id set in entity_test_entity_update()
+    // and returned from the entity should be the same as the entity's revision
+    // id because a new revision wasn't created, the existing revision was
+    // updated.
+    $loadedRevisionId = \Drupal::state()->get('entity_test.loadedRevisionId');
+    $this->assertEquals($loaded->getRevisionId(), $loadedRevisionId);
+    $this->assertEquals($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+
+    // Creating a clone should keep the loaded Revision ID.
+    $clone = clone $loaded;
+    $this->assertSame($loaded->getLoadedRevisionId(), $clone->getLoadedRevisionId());
+
+    // Creating a duplicate should set a NULL loaded Revision ID.
+    $duplicate = $loaded->createDuplicate();
+    $this->assertSame(NULL, $duplicate->getLoadedRevisionId());
+  }
+
+  /**
+   * Tests the loaded revision ID for translatable entities.
+   */
+  public function testTranslatedLoadedRevisionId() {
+    ConfigurableLanguage::createFromLangcode('fr')->save();
+
+    // Create a basic EntityTestMulRev entity and save it.
+    $entity = EntityTestMulRev::create();
+    $entity->save();
+
+    // Load the created entity and create a new revision.
+    $loaded = EntityTestMulRev::load($entity->id());
+    $loaded->setNewRevision(TRUE);
+    $loaded->save();
+
+    // Check it all works with translations.
+    $french = $loaded->addTranslation('fr');
+    // Adding a revision should return the same for each language.
+    $this->assertEquals($french->getRevisionId(), $french->getLoadedRevisionId());
+    $this->assertEquals($loaded->getRevisionId(), $french->getLoadedRevisionId());
+    $this->assertEquals($loaded->getLoadedRevisionId(), $french->getLoadedRevisionId());
+    $french->save();
+    // After saving nothing should change.
+    $this->assertEquals($french->getRevisionId(), $french->getLoadedRevisionId());
+    $this->assertEquals($loaded->getRevisionId(), $french->getLoadedRevisionId());
+    $this->assertEquals($loaded->getLoadedRevisionId(), $french->getLoadedRevisionId());
+    $first_revision_id = $french->getRevisionId();
+    $french->setNewRevision();
+    // Setting a new revision will reset the loaded Revision ID.
+    $this->assertEquals($first_revision_id, $french->getLoadedRevisionId());
+    $this->assertEquals($first_revision_id, $loaded->getLoadedRevisionId());
+    $this->assertNotEquals($french->getRevisionId(), $french->getLoadedRevisionId());
+    $this->assertGreaterThan($french->getRevisionId(), $french->getLoadedRevisionId());
+    $this->assertNotEquals($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+    $this->assertGreaterThan($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+    $french->save();
+    // Saving the new revision will reset the origin revision ID again.
+    $this->assertEquals($french->getRevisionId(), $french->getLoadedRevisionId());
+    $this->assertEquals($loaded->getRevisionId(), $loaded->getLoadedRevisionId());
+  }
+
+  /**
+   * Tests re-saving the entity in entity_test_entity_insert().
+   */
+  public function testSaveInHookEntityInsert() {
+    // Create an entity which will be saved again in entity_test_entity_insert().
+    $entity = EntityTestMulRev::create(['name' => 'EntityLoadedRevisionTest']);
+    $entity->save();
+    $loadedRevisionId = \Drupal::state()->get('entity_test.loadedRevisionId');
+    $this->assertEquals($entity->getLoadedRevisionId(), $loadedRevisionId);
+    $this->assertEquals($entity->getRevisionId(), $entity->getLoadedRevisionId());
+
+  }
+
+}