Commit d2f461ff authored by catch's avatar catch

Issue #2826021 by hchonov, Berdir, mkalkbrenner, plach, james.williams,...

Issue #2826021 by hchonov, Berdir, mkalkbrenner, plach, james.williams, tstoeckler: FieldItemList::equals is sufficient from the storage perspective but not for code checking for changes
parent 9d6dcefd
......@@ -5,7 +5,6 @@
use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\Entity\Plugin\DataType\EntityReference;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Field\ChangedFieldItemList;
use Drupal\Core\Language\Language;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\Session\AccountInterface;
......@@ -176,6 +175,13 @@ abstract class ContentEntityBase extends Entity implements \IteratorAggregate, C
*/
protected $enforceRevisionTranslationAffected = [];
/**
* Local cache for fields to skip from the checking for translation changes.
*
* @var array
*/
protected static $fieldsToSkipFromTranslationChangesCheck = [];
/**
* {@inheritdoc}
*/
......@@ -1377,7 +1383,11 @@ public static function bundleFieldDefinitions(EntityTypeInterface $entity_type,
* An array of field names.
*/
protected function getFieldsToSkipFromTranslationChangesCheck() {
return $this->traitGetFieldsToSkipFromTranslationChangesCheck($this);
$bundle = $this->bundle();
if (!isset(static::$fieldsToSkipFromTranslationChangesCheck[$this->entityTypeId][$bundle])) {
static::$fieldsToSkipFromTranslationChangesCheck[$this->entityTypeId][$bundle] = $this->traitGetFieldsToSkipFromTranslationChangesCheck($this);
}
return static::$fieldsToSkipFromTranslationChangesCheck[$this->entityTypeId][$bundle];
}
/**
......@@ -1413,6 +1423,7 @@ public function hasTranslationChanges() {
// possible or be meaningless.
/** @var \Drupal\Core\Entity\ContentEntityBase $translation */
$translation = $original->getTranslation($this->activeLangcode);
$langcode = $this->language()->getId();
// The list of fields to skip from the comparision.
$skip_fields = $this->getFieldsToSkipFromTranslationChangesCheck();
......@@ -1428,18 +1439,10 @@ public function hasTranslationChanges() {
if (in_array($field_name, $skip_fields, TRUE) || ($skip_untranslatable_fields && !$definition->isTranslatable())) {
continue;
}
$field = $this->get($field_name);
// When saving entities in the user interface, the changed timestamp is
// automatically incremented by ContentEntityForm::submitForm() even if
// nothing was actually changed. Thus, the changed time needs to be
// ignored when determining whether there are any actual changes in the
// entity.
if (!($field instanceof ChangedFieldItemList) && !$definition->isComputed()) {
$items = $field->filterEmptyItems();
$original_items = $translation->get($field_name)->filterEmptyItems();
if (!$items->equals($original_items)) {
return TRUE;
}
$items = $this->get($field_name)->filterEmptyItems();
$original_items = $translation->get($field_name)->filterEmptyItems();
if ($items->hasAffectingChanges($original_items, $langcode)) {
return TRUE;
}
}
......
......@@ -30,6 +30,13 @@ protected function getFieldsToSkipFromTranslationChangesCheck(ContentEntityInter
];
$fields = array_merge($fields, array_values($entity_type->getRevisionMetadataKeys()));
// Computed fields should be skipped by the check for translation changes.
foreach (array_diff_key($entity->getFieldDefinitions(), array_flip($fields)) as $field_name => $field_definition) {
if ($field_definition->isComputed()) {
$fields[] = $field_name;
}
}
return $fields;
}
......
......@@ -6,7 +6,6 @@
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Entity\EntityChangesDetectionTrait;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Field\ChangedFieldItemList;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
......@@ -111,19 +110,9 @@ protected function hasUntranslatableFieldsChanges(ContentEntityInterface $entity
continue;
}
// When saving entities in the user interface, the changed timestamp is
// automatically incremented by ContentEntityForm::submitForm() even if
// nothing was actually changed. Thus, the changed time needs to be
// ignored when determining whether there are any actual changes in the
// entity.
$field = $entity->get($field_name);
if ($field instanceof ChangedFieldItemList) {
continue;
}
$items = $field->filterEmptyItems();
$items = $entity->get($field_name)->filterEmptyItems();
$original_items = $original->get($field_name)->filterEmptyItems();
if (!$items->equals($original_items)) {
if ($items->hasAffectingChanges($original_items, $entity->getUntranslated()->language()->getId())) {
return TRUE;
}
}
......
......@@ -18,4 +18,16 @@ public function defaultAccess($operation = 'view', AccountInterface $account = N
return AccessResult::allowedIf($operation !== 'edit');
}
/**
* {@inheritdoc}
*/
public function hasAffectingChanges(FieldItemListInterface $original_items, $langcode) {
// When saving entities in the user interface, the changed timestamp is
// automatically incremented by ContentEntityForm::submitForm() even if
// nothing was actually changed. Thus, the changed time needs to be
// ignored when determining whether there are any actual changes in the
// entity.
return FALSE;
}
}
......@@ -400,4 +400,11 @@ public function equals(FieldItemListInterface $list_to_compare) {
return $value1 == $value2;
}
/**
* {@inheritdoc}
*/
public function hasAffectingChanges(FieldItemListInterface $original_items, $langcode) {
return !$this->equals($original_items);
}
}
......@@ -263,6 +263,9 @@ public static function processDefaultValue($default_value, FieldableEntityInterf
/**
* Determines equality to another object implementing FieldItemListInterface.
*
* This method is usually used by the storage to check for not computed
* value changes, which will be saved into the storage.
*
* @param \Drupal\Core\Field\FieldItemListInterface $list_to_compare
* The field item list to compare to.
*
......@@ -271,4 +274,24 @@ public static function processDefaultValue($default_value, FieldableEntityInterf
*/
public function equals(FieldItemListInterface $list_to_compare);
/**
* Determines whether the field has relevant changes.
*
* This is for example used to determine if a revision of an entity has
* changes in a given translation. Unlike
* \Drupal\Core\Field\FieldItemListInterface::equals(), this can report
* that for example an untranslatable field, despite being changed and
* therefore technically affecting all translations, is only internal metadata
* or only affects a single translation.
*
* @param \Drupal\Core\Field\FieldItemListInterface $original_items
* The original field items to compare against.
* @param string $langcode
* The language that should be checked.
*
* @return bool
* TRUE if the field has relevant changes, FALSE if not.
*/
public function hasAffectingChanges(FieldItemListInterface $original_items, $langcode);
}
......@@ -306,9 +306,17 @@ public function testEntityPropertiesModifications() {
$property->setValue($entity, 'default-value');
$property->setValue($translation, 'default-value');
$property->setValue($clone, 'test-entity-cloning');
$this->assertEquals('default-value', $property->getValue($entity), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
$this->assertEquals('default-value', $property->getValue($translation), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
$this->assertEquals('test-entity-cloning', $property->getValue($clone), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
// Static properties remain the same across all instances of the class.
if ($property->isStatic()) {
$this->assertEquals('test-entity-cloning', $property->getValue($entity), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
$this->assertEquals('test-entity-cloning', $property->getValue($translation), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
$this->assertEquals('test-entity-cloning', $property->getValue($clone), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
}
else {
$this->assertEquals('default-value', $property->getValue($entity), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
$this->assertEquals('default-value', $property->getValue($translation), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
$this->assertEquals('test-entity-cloning', $property->getValue($clone), (string) new FormattableMarkup('Entity property %property_name is not cloned properly.', ['%property_name' => $property->getName()]));
}
// Modify each entity property on the translation entity object and assert
// that the change is propagated to the default translation entity object
......
......@@ -6,6 +6,8 @@
use Drupal\Core\Field\FieldDefinitionInterface;
use Drupal\Core\Field\FieldItemInterface;
use Drupal\Core\Field\FieldItemList;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
use Drupal\Core\Field\FieldTypePluginManagerInterface;
use Drupal\Core\Form\FormState;
use Drupal\Tests\UnitTestCase;
......@@ -136,6 +138,71 @@ public function providerTestEquals() {
return $datasets;
}
/**
* Tests identical behavior of ::hasAffectingChanges with ::equals.
*
* @covers ::hasAffectingChanges
*
* @dataProvider providerTestEquals
*/
public function testHasAffectingChanges($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->createMock(FieldTypePluginManagerInterface::class);
$container = new ContainerBuilder();
$container->set('plugin.manager.field.field_type', $field_type_manager);
\Drupal::setContainer($container);
$field_storage_definition = $this->createMock(FieldStorageDefinitionInterface::class);
$field_storage_definition->expects($this->any())
->method('getColumns')
->willReturn([0 => '0', 1 => '1']);
// Set up three properties, one of them being computed.
$property_definitions['0'] = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
$property_definitions['0']->expects($this->any())
->method('isComputed')
->willReturn(FALSE);
$property_definitions['1'] = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
$property_definitions['1']->expects($this->any())
->method('isComputed')
->willReturn(FALSE);
$property_definitions['2'] = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
$property_definitions['2']->expects($this->any())
->method('isComputed')
->willReturn(TRUE);
$field_storage_definition = $this->getMock('Drupal\Core\Field\FieldStorageDefinitionInterface');
$field_storage_definition->expects($this->any())
->method('getPropertyDefinitions')
->will($this->returnValue($property_definitions));
$field_definition = $this->createMock(FieldDefinitionInterface::class);
$field_definition->expects($this->any())
->method('getFieldStorageDefinition')
->willReturn($field_storage_definition);
$field_definition->expects($this->any())
->method('isComputed')
->willReturn(FALSE);
$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->hasAffectingChanges($field_list_b, ''));
}
/**
* @covers ::equals
*/
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment