Issue #2837022 by hchonov, xjm, vlad.dancer, plach, matsbla, Gábor Hojtsy: Concurrently editing two translations of a node may result in data loss for non-translatable fields

8.5.x
Nathaniel Catchpole 2018-01-02 12:52:15 +00:00
parent 4cd7b4950c
commit 0c20200d12
3 changed files with 73 additions and 5 deletions

View File

@ -37,6 +37,11 @@ interface EntityChangedInterface {
/** /**
* Gets the timestamp of the last entity change across all translations. * Gets the timestamp of the last entity change across all translations.
* *
* This method will return the highest timestamp across all translations. To
* check that no translation is older than in another version of the entity
* (e.g. to avoid overwriting newer translations with old data), compare each
* translation to the other version individually.
*
* @return int * @return int
* The timestamp of the last entity save operation across all * The timestamp of the last entity save operation across all
* translations. * translations.

View File

@ -18,10 +18,23 @@ class EntityChangedConstraintValidator extends ConstraintValidator {
/** @var \Drupal\Core\Entity\EntityInterface $entity */ /** @var \Drupal\Core\Entity\EntityInterface $entity */
if (!$entity->isNew()) { if (!$entity->isNew()) {
$saved_entity = \Drupal::entityManager()->getStorage($entity->getEntityTypeId())->loadUnchanged($entity->id()); $saved_entity = \Drupal::entityManager()->getStorage($entity->getEntityTypeId())->loadUnchanged($entity->id());
// A change to any other translation must add a violation to the current // Ensure that all the entity translations are the same as or newer
// translation because there might be untranslatable shared fields. // than their current version in the storage in order to avoid
if ($saved_entity && $saved_entity->getChangedTimeAcrossTranslations() > $entity->getChangedTimeAcrossTranslations()) { // reverting other changes. In fact the entity object that is being
$this->context->addViolation($constraint->message); // saved might contain an older entity translation when different
// translations are being concurrently edited.
if ($saved_entity) {
$common_translation_languages = array_intersect_key($entity->getTranslationLanguages(), $saved_entity->getTranslationLanguages());
foreach (array_keys($common_translation_languages) as $langcode) {
// Merely comparing the latest changed timestamps across all
// translations is not sufficient since other translations may have
// been edited and saved in the meanwhile. Therefore, compare the
// changed timestamps of each entity translation individually.
if ($saved_entity->getTranslation($langcode)->getChangedTime() > $entity->getTranslation($langcode)->getChangedTime()) {
$this->context->addViolation($constraint->message);
break;
}
}
} }
} }
} }

View File

@ -3,6 +3,7 @@
namespace Drupal\KernelTests\Core\Entity; namespace Drupal\KernelTests\Core\Entity;
use Drupal\Core\Entity\Plugin\Validation\Constraint\CompositeConstraintBase; use Drupal\Core\Entity\Plugin\Validation\Constraint\CompositeConstraintBase;
use Drupal\language\Entity\ConfigurableLanguage;
/** /**
* Tests the Entity Validation API. * Tests the Entity Validation API.
@ -16,7 +17,7 @@ class EntityValidationTest extends EntityKernelTestBase {
* *
* @var array * @var array
*/ */
public static $modules = ['filter', 'text']; public static $modules = ['filter', 'text', 'language'];
/** /**
* @var string * @var string
@ -39,6 +40,10 @@ class EntityValidationTest extends EntityKernelTestBase {
protected function setUp() { protected function setUp() {
parent::setUp(); parent::setUp();
// Enable an additional language.
ConfigurableLanguage::createFromLangcode('de')
->save();
// Create the test field. // Create the test field.
module_load_install('entity_test'); module_load_install('entity_test');
entity_test_install(); entity_test_install();
@ -200,4 +205,49 @@ class EntityValidationTest extends EntityKernelTestBase {
$this->assertEqual($constraint->coversFields(), ['name', 'type'], 'Information about covered fields can be retrieved.'); $this->assertEqual($constraint->coversFields(), ['name', 'type'], 'Information about covered fields can be retrieved.');
} }
/**
* Tests the EntityChangedConstraintValidator with multiple translations.
*/
public function testEntityChangedConstraintOnConcurrentMultilingualEditing() {
$this->installEntitySchema('entity_test_mulrev_changed');
$storage = \Drupal::entityTypeManager()
->getStorage('entity_test_mulrev_changed');
// Create a test entity.
$entity = $this->createTestEntity('entity_test_mulrev_changed');
$entity->save();
$entity->setChangedTime($entity->getChangedTime() - 1);
$violations = $entity->validate();
$this->assertEquals(1, $violations->count());
$this->assertEqual($violations[0]->getMessage(), 'The content has either been modified by another user, or you have already submitted modifications. As a result, your changes cannot be saved.');
$entity = $storage->loadUnchanged($entity->id());
$translation = $entity->addTranslation('de');
$entity->save();
// Ensure that the new translation has a newer changed timestamp than the
// default translation.
$this->assertGreaterThan($entity->getChangedTime(), $translation->getChangedTime());
// Simulate concurrent form editing by saving the entity with an altered
// non-translatable field in order for the changed timestamp to be updated
// across all entity translations.
$original_entity_time = $entity->getChangedTime();
$entity->set('not_translatable', $this->randomString());
$entity->save();
// Simulate form submission of an uncached form by setting the previous
// timestamp of an entity translation on the saved entity object. This
// happens in the entity form API where we put the changed timestamp of
// the entity in a form hidden value and then set it on the entity which on
// form submit is loaded from the storage if the form is not yet cached.
$entity->setChangedTime($original_entity_time);
// Setting the changed timestamp from the user input on the entity loaded
// from the storage is used as a prevention from saving a form built with a
// previous version of the entity and thus reverting changes by other users.
$violations = $entity->validate();
$this->assertEquals(1, $violations->count());
$this->assertEqual($violations[0]->getMessage(), 'The content has either been modified by another user, or you have already submitted modifications. As a result, your changes cannot be saved.');
}
} }