From 3deb640a9172aa57f07f34142857ac93b605ec77 Mon Sep 17 00:00:00 2001
From: webchick <drupal@webchick.net>
Date: Thu, 16 Apr 2015 09:39:18 -0700
Subject: [PATCH] Issue #2297817 by alexpott, pounard, Berdir, yched, Fabianx,
plach, mkalkbrenner: Do not attempt field storage write when field content
did not change
---
.../Core/Entity/ContentEntityStorageBase.php | 32 ++++
core/lib/Drupal/Core/Entity/Entity.php | 4 +-
.../Entity/Sql/SqlContentEntityStorage.php | 22 ++-
core/lib/Drupal/Core/Field/FieldItemList.php | 35 ++++
.../Core/Field/FieldItemListInterface.php | 13 ++
.../Tests/Entity/EntityTranslationTest.php | 13 ++
.../Tests/Core/Field/FieldItemListTest.php | 149 ++++++++++++++++++
7 files changed, 258 insertions(+), 10 deletions(-)
create mode 100644 core/tests/Drupal/Tests/Core/Field/FieldItemListTest.php
diff --git a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
index acc17bac2780..25d4a5a1c57a 100644
--- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
+++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
@@ -206,4 +206,36 @@ protected function invokeFieldMethod($method, ContentEntityInterface $entity) {
}
}
+ /**
+ * Checks whether the field values changed compared to the original entity.
+ *
+ * @param \Drupal\Core\Field\FieldDefinitionInterface $field_definition
+ * Field definition of field to compare for changes.
+ * @param \Drupal\Core\Entity\ContentEntityInterface $entity
+ * Entity to check for field changes.
+ * @param \Drupal\Core\Entity\ContentEntityInterface $original
+ * Original entity to compare against.
+ *
+ * @return bool
+ * True if the field value changed from the original entity.
+ */
+ protected function hasFieldValueChanged(FieldDefinitionInterface $field_definition, ContentEntityInterface $entity, ContentEntityInterface $original) {
+ $field_name = $field_definition->getName();
+ $langcodes = array_keys($entity->getTranslationLanguages());
+ if ($langcodes !== array_keys($original->getTranslationLanguages())) {
+ // If the list of langcodes has changed, we need to save.
+ return TRUE;
+ }
+ foreach ($langcodes as $langcode) {
+ $items = $entity->getTranslation($langcode)->get($field_name)->filterEmptyItems();
+ $original_items = $original->getTranslation($langcode)->get($field_name)->filterEmptyItems();
+ // If the field items are not equal, we need to save.
+ if (!$items->equals($original_items)) {
+ return TRUE;
+ }
+ }
+
+ return FALSE;
+ }
+
}
diff --git a/core/lib/Drupal/Core/Entity/Entity.php b/core/lib/Drupal/Core/Entity/Entity.php
index 764d639b2d43..cf0a66dbac62 100644
--- a/core/lib/Drupal/Core/Entity/Entity.php
+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -156,8 +156,8 @@ public function label() {
* {@inheritdoc}
*/
public function urlInfo($rel = 'canonical', array $options = []) {
- if ($this->isNew()) {
- throw new EntityMalformedException(sprintf('The "%s" entity type has not been saved, and cannot have a URI.', $this->getEntityTypeId()));
+ if ($this->id() === NULL) {
+ throw new EntityMalformedException(sprintf('The "%s" entity cannot have a URI as it does have an ID', $this->getEntityTypeId()));
}
// The links array might contain URI templates set in annotations.
diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php
index 22d2f92a281e..924f6a8eae92 100644
--- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php
+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php
@@ -22,6 +22,7 @@
use Drupal\Core\Entity\Query\QueryInterface;
use Drupal\Core\Entity\Schema\DynamicallyFieldableEntityStorageSchemaInterface;
use Drupal\Core\Field\FieldDefinitionInterface;
+use Drupal\Core\Field\FieldItemListInterface;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
use Drupal\Core\Language\LanguageInterface;
use Drupal\field\FieldStorageConfigInterface;
@@ -960,9 +961,6 @@ protected function doSave($id, EntityInterface $entity) {
if ($this->revisionDataTable) {
$this->saveToSharedTables($entity, $this->revisionDataTable);
}
- if ($this->revisionTable) {
- $entity->setNewRevision(FALSE);
- }
}
else {
// Ensure the entity is still seen as new after assigning it an id,
@@ -990,11 +988,6 @@ protected function doSave($id, EntityInterface $entity) {
if ($this->revisionDataTable) {
$this->saveToSharedTables($entity, $this->revisionDataTable);
}
-
- $entity->enforceIsNew(FALSE);
- if ($this->revisionTable) {
- $entity->setNewRevision(FALSE);
- }
}
$this->invokeFieldMethod($is_new ? 'insert' : 'update', $entity);
$this->saveToDedicatedTables($entity, !$is_new);
@@ -1002,6 +995,10 @@ protected function doSave($id, EntityInterface $entity) {
if (!$is_new && $this->dataTable) {
$this->invokeTranslationHooks($entity);
}
+ $entity->enforceIsNew(FALSE);
+ if ($this->revisionTable) {
+ $entity->setNewRevision(FALSE);
+ }
return $return;
}
@@ -1317,11 +1314,20 @@ protected function saveToDedicatedTables(ContentEntityInterface $entity, $update
$vid = $id;
}
+ $original = !empty($entity->original) ? $entity->original: NULL;
+
foreach ($this->entityManager->getFieldDefinitions($entity_type, $bundle) as $field_name => $field_definition) {
$storage_definition = $field_definition->getFieldStorageDefinition();
if (!$table_mapping->requiresDedicatedTableStorage($storage_definition)) {
continue;
}
+
+ // When updating an existing revision, keep the existing records if the
+ // field values did not change.
+ if (!$entity->isNewRevision() && $original && !$this->hasFieldValueChanged($field_definition, $entity, $original)) {
+ continue;
+ }
+
$table_name = $table_mapping->getDedicatedDataTableName($storage_definition);
$revision_name = $table_mapping->getDedicatedRevisionTableName($storage_definition);
diff --git a/core/lib/Drupal/Core/Field/FieldItemList.php b/core/lib/Drupal/Core/Field/FieldItemList.php
index 930b4e17aeee..86bac79b75ba 100644
--- a/core/lib/Drupal/Core/Field/FieldItemList.php
+++ b/core/lib/Drupal/Core/Field/FieldItemList.php
@@ -98,6 +98,7 @@ public function filterEmptyItems() {
$this->filter(function ($item) {
return !$item->isEmpty();
});
+ return $this;
}
/**
@@ -370,4 +371,38 @@ protected function defaultValueWidget(FormStateInterface $form_state) {
return $form_state->get('default_value_widget');
}
+ /**
+ * {@inheritdoc}
+ */
+ public function equals(FieldItemListInterface $list_to_compare) {
+ $columns = $this->getFieldDefinition()->getFieldStorageDefinition()->getColumns();
+ $count1 = count($this);
+ $count2 = count($list_to_compare);
+ if ($count1 === 0 && $count2 === 0) {
+ // Both are empty we can safely assume that it did not change.
+ return TRUE;
+ }
+ if ($count1 !== $count2) {
+ // One of them is empty but not the other one so the value changed.
+ return FALSE;
+ }
+ $value1 = $this->getValue();
+ $value2 = $list_to_compare->getValue();
+ if ($value1 === $value2) {
+ return TRUE;
+ }
+ // If the values are not equal ensure a consistent order of field item
+ // properties and remove properties which will not be saved.
+ $callback = function (&$value) use ($columns) {
+ if (is_array($value)) {
+ $value = array_intersect_key($value, $columns);
+ ksort($value);
+ }
+ };
+ array_walk($value1, $callback);
+ array_walk($value2, $callback);
+
+ return $value1 === $value2;
+ }
+
}
diff --git a/core/lib/Drupal/Core/Field/FieldItemListInterface.php b/core/lib/Drupal/Core/Field/FieldItemListInterface.php
index aeb600594bb3..e4ea12c6c9d3 100644
--- a/core/lib/Drupal/Core/Field/FieldItemListInterface.php
+++ b/core/lib/Drupal/Core/Field/FieldItemListInterface.php
@@ -94,6 +94,8 @@ public function defaultAccess($operation = 'view', AccountInterface $account = N
/**
* Filters out empty field items and re-numbers the item deltas.
+ *
+ * @return $this
*/
public function filterEmptyItems();
@@ -258,4 +260,15 @@ public function defaultValuesFormSubmit(array $element, array &$form, FormStateI
*/
public static function processDefaultValue($default_value, FieldableEntityInterface $entity, FieldDefinitionInterface $definition);
+ /**
+ * Determines equality to another object implementing FieldItemListInterface.
+ *
+ * @param \Drupal\Core\Field\FieldItemListInterface $list_to_compare
+ * The field item list to compare to.
+ *
+ * @return bool
+ * TRUE if the field item lists are equal, FALSE if not.
+ */
+ public function equals(FieldItemListInterface $list_to_compare);
+
}
diff --git a/core/modules/system/src/Tests/Entity/EntityTranslationTest.php b/core/modules/system/src/Tests/Entity/EntityTranslationTest.php
index a3cf0a46a8cf..c8549e6b3054 100644
--- a/core/modules/system/src/Tests/Entity/EntityTranslationTest.php
+++ b/core/modules/system/src/Tests/Entity/EntityTranslationTest.php
@@ -460,6 +460,19 @@ protected function doTestEntityTranslationAPI($entity_type) {
$hooks = $this->getHooksInfo();
$this->assertFalse($hooks, 'No hooks are run when adding and removing a translation without storing it.');
+ // Check that hooks are fired only when actually storing data.
+ $entity = $this->reloadEntity($entity);
+ $entity->addTranslation($langcode2);
+ $entity->save();
+ $entity = $this->reloadEntity($entity);
+ $this->assertTrue($entity->hasTranslation($langcode2), 'Entity has translation after adding one and saving.');
+ $entity->removeTranslation($langcode2);
+ $entity->save();
+ $entity = $this->reloadEntity($entity);
+ $this->assertFalse($entity->hasTranslation($langcode2), 'Entity does not have translation after removing it and saving.');
+ // Reset hook firing information.
+ $this->getHooksInfo();
+
// Verify that entity serialization does not cause stale references to be
// left around.
$entity = $this->reloadEntity($entity);
diff --git a/core/tests/Drupal/Tests/Core/Field/FieldItemListTest.php b/core/tests/Drupal/Tests/Core/Field/FieldItemListTest.php
new file mode 100644
index 000000000000..65e996e3545f
--- /dev/null
+++ b/core/tests/Drupal/Tests/Core/Field/FieldItemListTest.php
@@ -0,0 +1,149 @@
+<?php
+
+/**
+ * @file Contains \Drupal\Tests\Core\Field\FieldItemListTest.
+ */
+
+namespace Drupal\Tests\Core\Field;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Field\FieldItemInterface;
+use Drupal\Core\Field\FieldItemList;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Field\FieldItemList
+ * @group Field
+ */
+class FieldItemListTest extends UnitTestCase {
+
+ /**
+ * @covers ::equals
+ *
+ * @dataProvider providerTestEquals
+ */
+ public function testEquals($expected, FieldItemInterface $first_field_item = NULL, FieldItemInterface $second_field_item = NULL) {
+
+ // Mock the field type manager and place it in the container.
+ $field_type_manager = $this->getMock('Drupal\Core\Field\FieldTypePluginManagerInterface');
+ $container = new ContainerBuilder();
+ $container->set('plugin.manager.field.field_type', $field_type_manager);
+ \Drupal::setContainer($container);
+
+ $field_storage_definition = $this->getMock('Drupal\Core\Field\FieldStorageDefinitionInterface');
+ $field_storage_definition->expects($this->any())
+ ->method('getColumns')
+ ->willReturn([0 => '0', 1 => '1']);
+ $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface');
+ $field_definition->expects($this->any())
+ ->method('getFieldStorageDefinition')
+ ->willReturn($field_storage_definition);
+
+ $field_list_a = new FieldItemList($field_definition);
+ $field_list_b = new FieldItemList($field_definition);
+
+ // Set up the mocking necessary for creating field items.
+ $field_type_manager->expects($this->any())
+ ->method('createFieldItem')
+ ->willReturnOnConsecutiveCalls($first_field_item, $second_field_item);
+
+ // Set the field item values.
+ if ($first_field_item instanceof FieldItemInterface) {
+ $field_list_a->setValue($first_field_item);
+ }
+ if ($second_field_item instanceof FieldItemInterface) {
+ $field_list_b->setValue($second_field_item);
+ }
+
+ $this->assertEquals($expected, $field_list_a->equals($field_list_b));
+ }
+
+ /**
+ * Data provider for testEquals.
+ */
+ public function providerTestEquals() {
+ // Tests field item lists with no values.
+ $datasets[] = [TRUE];
+
+ /** @var \Drupal\Core\Field\FieldItemBase $field_item_a */
+ $field_item_a = $this->getMockForAbstractClass('Drupal\Core\Field\FieldItemBase', [], '', FALSE);
+ $field_item_a->setValue([1]);
+ // Tests field item lists where one has a value and one does not.
+ $datasets[] = [FALSE, $field_item_a];
+
+ // Tests field item lists where both have the same value.
+ $datasets[] = [TRUE, $field_item_a, $field_item_a];
+
+ /** @var \Drupal\Core\Field\FieldItemBase $fv */
+ $field_item_b = $this->getMockForAbstractClass('Drupal\Core\Field\FieldItemBase', [], '', FALSE);
+ $field_item_b->setValue([2]);
+ // Tests field item lists where both have the different values.
+ $datasets[] = [FALSE, $field_item_a, $field_item_b];
+
+ /** @var \Drupal\Core\Field\FieldItemBase $fv */
+ $field_item_c = $this->getMockForAbstractClass('Drupal\Core\Field\FieldItemBase', [], '', FALSE);
+ $field_item_c->setValue(['0' => 1, '1' => 2]);
+ $field_item_d = $this->getMockForAbstractClass('Drupal\Core\Field\FieldItemBase', [], '', FALSE);
+ $field_item_d->setValue(['1' => 2, '0' => 1]);
+
+ // Tests field item lists where both have the differently ordered values.
+ $datasets[] = [TRUE, $field_item_c, $field_item_d];
+
+ return $datasets;
+ }
+
+ /**
+ * @covers ::equals
+ */
+ public function testEqualsEmptyItems() {
+ /** @var \Drupal\Core\Field\FieldItemBase $fv */
+ $first_field_item = $this->getMockForAbstractClass('Drupal\Core\Field\FieldItemBase', [], '', FALSE);
+ $first_field_item->setValue(['0' => 1, '1' => 2]);
+ $second_field_item = $this->getMockForAbstractClass('Drupal\Core\Field\FieldItemBase', [], '', FALSE);
+ $second_field_item->setValue(['1' => 2, '0' => 1]);
+ $empty_field_item = $this->getMockForAbstractClass('Drupal\Core\Field\FieldItemBase', [], '', FALSE);
+ // Mock the field type manager and place it in the container.
+ $field_type_manager = $this->getMock('Drupal\Core\Field\FieldTypePluginManagerInterface');
+ $container = new ContainerBuilder();
+ $container->set('plugin.manager.field.field_type', $field_type_manager);
+ \Drupal::setContainer($container);
+
+ $field_storage_definition = $this->getMock('Drupal\Core\Field\FieldStorageDefinitionInterface');
+ $field_storage_definition->expects($this->any())
+ ->method('getColumns')
+ ->willReturn([0 => '0', 1 => '1']);
+ $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface');
+ $field_definition->expects($this->any())
+ ->method('getFieldStorageDefinition')
+ ->willReturn($field_storage_definition);
+
+ $field_list_a = new FieldItemList($field_definition);
+ $field_list_b = new FieldItemList($field_definition);
+
+ // Set up the mocking necessary for creating field items.
+ $field_type_manager->expects($this->any())
+ ->method('createFieldItem')
+ ->willReturnOnConsecutiveCalls($first_field_item, $second_field_item, $empty_field_item, $empty_field_item);
+
+ // Set the field item values.
+ $field_list_a->setValue($first_field_item);
+ $field_list_b->setValue($second_field_item);
+ $field_list_a->appendItem($empty_field_item);
+
+ // Field list A has an empty item.
+ $this->assertEquals(FALSE, $field_list_a->equals($field_list_b));
+
+ // Field lists A and B have empty items.
+ $field_list_b->appendItem($empty_field_item);
+ $this->assertEquals(TRUE, $field_list_a->equals($field_list_b));
+
+ // Field list B has an empty item.
+ $field_list_a->filterEmptyItems();
+ $this->assertEquals(FALSE, $field_list_a->equals($field_list_b));
+
+ // Neither field lists A and B have empty items.
+ $field_list_b->filterEmptyItems();
+ $this->assertEquals(TRUE, $field_list_a->equals($field_list_b));
+ }
+
+}
--
GitLab