From 9d8d1ed77b3207526b9d8d0a0345763d86a71ac2 Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Tue, 20 Dec 2016 16:16:13 +0000
Subject: [PATCH] Issue #2809227 by timmillwood, Berdir, hchonov, catch,
amateescu, alexpott: Store revision id in originalRevisionId property to use
after revision id is updated
---
.../Drupal/Core/Entity/ContentEntityBase.php | 34 ++++
.../Core/Entity/ContentEntityInterface.php | 21 +++
.../Core/Entity/ContentEntityStorageBase.php | 8 +
.../modules/entity_test/entity_test.module | 20 +++
.../Core/Entity/EntityLoadedRevisionTest.php | 170 ++++++++++++++++++
5 files changed, 253 insertions(+)
create mode 100644 core/tests/Drupal/KernelTests/Core/Entity/EntityLoadedRevisionTest.php
diff --git a/core/lib/Drupal/Core/Entity/ContentEntityBase.php b/core/lib/Drupal/Core/Entity/ContentEntityBase.php
index 1d69be24364c..e242544967d5 100644
--- a/core/lib/Drupal/Core/Entity/ContentEntityBase.php
+++ b/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;
}
}
diff --git a/core/lib/Drupal/Core/Entity/ContentEntityInterface.php b/core/lib/Drupal/Core/Entity/ContentEntityInterface.php
index 0c3f04b98b70..8893a8ba851a 100644
--- a/core/lib/Drupal/Core/Entity/ContentEntityInterface.php
+++ b/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();
+
}
diff --git a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
index c25fd4868979..d5872eb8d6d1 100644
--- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
+++ b/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);
}
}
diff --git a/core/modules/system/tests/modules/entity_test/entity_test.module b/core/modules/system/tests/modules/entity_test/entity_test.module
index f8f56ebbb0a4..cf1a63ccfecd 100644
--- a/core/modules/system/tests/modules/entity_test/entity_test.module
+++ b/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().
*
diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityLoadedRevisionTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityLoadedRevisionTest.php
new file mode 100644
index 000000000000..a3b99fe0c95e
--- /dev/null
+++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityLoadedRevisionTest.php
@@ -0,0 +1,170 @@
+<?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());
+
+ }
+
+}
--
GitLab