Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #2936511 by hchonov, Berdir, plach, Wim Leers, catch, amateescu, sdewitt: ContentEntityType::getRevisionMetadataKeys() never considers the BC logic because of the new "revision_default" key

merge-requests/1654/head
effulgentsia 2018-02-07 11:12:43 -08:00
parent ef5b216cf1
commit 3fcf07947c
5 changed files with 215 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
core/lib/Drupal/Core/Entity/ContentEntityType.php
View File

@ -14,6 +14,18 @@ class ContentEntityType extends EntityType implements ContentEntityTypeInterface
*/
protected $revision_metadata_keys = [];
/**
* The required revision metadata keys.
*
* This property should only be filled in the constructor. This ensures that
* only new instances get newly added required revision metadata keys.
* Unserialized objects will only retrieve the keys that they already have
* been cached with.
*
* @var array
*/
protected $requiredRevisionMetadataKeys = [];
/**
* {@inheritdoc}
*/
@ -25,9 +37,23 @@ class ContentEntityType extends EntityType implements ContentEntityTypeInterface
'view_builder' => 'Drupal\Core\Entity\EntityViewBuilder',
];
$this->revision_metadata_keys += [
'revision_default' => 'revision_default',
];
// Only new instances should provide the required revision metadata keys.
// The cached instances should return only what already has been stored
// under the property $revision_metadata_keys. The BC layer in
// ::getRevisionMetadataKeys() has to detect if the revision metadata keys
// have been provided by the entity type annotation, therefore we add keys
// to the property $requiredRevisionMetadataKeys only if those keys aren't
// set in the entity type annotation.
if (!isset($this->revision_metadata_keys['revision_default'])) {
$this->requiredRevisionMetadataKeys['revision_default'] = 'revision_default';
}
// Add the required revision metadata fields here instead in the getter
// method, so that they are serialized as part of the object even if the
// getter method doesn't get called. This allows the list to be further
// extended. Only new instances of the class will contain the new list,
// while the cached instances contain the previous version of the list.
$this->revision_metadata_keys += $this->requiredRevisionMetadataKeys;
}
/**
@ -59,7 +85,7 @@ class ContentEntityType extends EntityType implements ContentEntityTypeInterface
public function getRevisionMetadataKeys($include_backwards_compatibility_field_names = TRUE) {
// Provide backwards compatibility in case the revision metadata keys are
// not defined in the entity annotation.
if (!$this->revision_metadata_keys && $include_backwards_compatibility_field_names) {
if ((!$this->revision_metadata_keys || ($this->revision_metadata_keys == $this->requiredRevisionMetadataKeys)) && $include_backwards_compatibility_field_names) {
$base_fields = \Drupal::service('entity_field.manager')->getBaseFieldDefinitions($this->id());
if ((isset($base_fields['revision_uid']) && $revision_user = 'revision_uid') || (isset($base_fields['revision_user']) && $revision_user = 'revision_user')) {
@trigger_error('The revision_user revision metadata key is not set.', E_USER_DEPRECATED);

4
core/lib/Drupal/Core/Entity/EntityFieldManager.php
View File

@ -224,7 +224,9 @@ class EntityFieldManager implements EntityFieldManagerInterface {
// Make sure that revisionable entity types are correctly defined.
if ($entity_type->isRevisionable()) {
$field_name = $entity_type->getRevisionMetadataKey('revision_default');
// Disable the BC layer to prevent a recursion, this only needs the
// revision_default key that is always set.
$field_name = $entity_type->getRevisionMetadataKeys(FALSE)['revision_default'];
$base_field_definitions[$field_name] = BaseFieldDefinition::create('boolean')
->setLabel($this->t('Default revision'))
->setDescription($this->t('A flag indicating whether this was a default revision when it was saved.'))

29
core/modules/system/system.install
View File

@ -2092,9 +2092,23 @@ function system_update_8501() {
// also to code using the latest installed definition.
$installed_entity_type = $definition_update_manager->getEntityType($entity_type_id);
$revision_metadata_keys = $installed_entity_type->get('revision_metadata_keys');
$revision_metadata_keys['revision_default'] = $field_name;
$installed_entity_type->set('revision_metadata_keys', $revision_metadata_keys);
$definition_update_manager->updateEntityType($installed_entity_type);
if (!isset($revision_metadata_keys['revision_default'])) {
// Update the property holding the required revision metadata keys,
// which is used by the BC layer for retrieving the revision metadata
// keys.
// @see \Drupal\Core\Entity\ContentEntityType::getRevisionMetadataKeys().
$required_revision_metadata_keys = $installed_entity_type->get('requiredRevisionMetadataKeys');
$required_revision_metadata_keys['revision_default'] = $field_name;
$installed_entity_type->set('requiredRevisionMetadataKeys', $required_revision_metadata_keys);
// Update the revision metadata keys to add the new required revision
// metadata key "revision_default".
$revision_metadata_keys['revision_default'] = $required_revision_metadata_keys['revision_default'];
$installed_entity_type->set('revision_metadata_keys', $revision_metadata_keys);
$definition_update_manager->updateEntityType($installed_entity_type);
}
$storage_definition = BaseFieldDefinition::create('boolean')
->setLabel(t('Default revision'))
@ -2110,5 +2124,14 @@ function system_update_8501() {
$definition_update_manager
->installFieldStorageDefinition($field_name, $entity_type_id, $entity_type_id, $storage_definition);
}
else {
$variables = ['@entity_type_label' => $entity_type->getLabel()];
if ($field_name === 'revision_default') {
\Drupal::logger('system')->error('An existing "Default revision" field was found for the @entity_type_label entity type, but no "revision_default" revision metadata key was found in its definition.', $variables);
}
else {
\Drupal::logger('system')->error('An existing "Default revision" field was found for the @entity_type_label entity type.', $variables);
}
}
}
}

8
core/modules/system/tests/modules/entity_test_revlog/src/Entity/EntityTestMulWithRevisionLog.php
View File

@ -5,6 +5,9 @@ namespace Drupal\entity_test_revlog\Entity;
/**
* Defines the test entity class.
*
* This entity type does not define revision_metadata_keys on purpose to test
* the BC layer.
*
* @ContentEntityType(
* id = "entity_test_mul_revlog",
* label = @Translation("Test entity - data table, revisions log"),
@ -21,11 +24,6 @@ namespace Drupal\entity_test_revlog\Entity;
* "label" = "name",
* "langcode" = "langcode",
* },
* revision_metadata_keys = {
* "revision_user" = "revision_user",
* "revision_created" = "revision_created",
* "revision_log_message" = "revision_log_message"
* },
* )
*/
class EntityTestMulWithRevisionLog extends EntityTestWithRevisionLog {

153
core/modules/system/tests/src/Functional/Entity/Update/MoveRevisionMetadataFieldsUpdateTest.php
View File

@ -2,6 +2,7 @@
namespace Drupal\Tests\system\Functional\Entity\Update;
use Drupal\Core\Entity\ContentEntityType;
use Drupal\FunctionalTests\Update\UpdatePathTestBase;
use Drupal\views\Entity\View;
@ -79,4 +80,156 @@ class MoveRevisionMetadataFieldsUpdateTest extends UpdatePathTestBase {
}
}
/**
* Tests the addition of required revision metadata keys.
*
* This test ensures that already cached entity instances will only return the
* required revision metadata keys they have been cached with and only new
* instances will return all the new required revision metadata keys.
*/
public function testAddingRequiredRevisionMetadataKeys() {
// Ensure that cached entity types without required revision metadata keys
// will not return any of the newly added required revision metadata keys.
// Contains no revision metadata keys and the property holding the required
// metadata keys is empty, the entity type id is "entity_test_mul_revlog".
$cached_with_no_metadata_keys = 'Tzo4MjoiRHJ1cGFsXFRlc3RzXHN5c3RlbVxGdW5jdGlvbmFsXEVudGl0eVxVcGRhdGVcVGVzdFJldmlzaW9uTWV0YWRhdGFCY0xheWVyRW50aXR5VHlwZSI6Mzk6e3M6MjU6IgAqAHJldmlzaW9uX21ldGFkYXRhX2tleXMiO2E6MDp7fXM6MzE6IgAqAHJlcXVpcmVkUmV2aXNpb25NZXRhZGF0YUtleXMiO2E6MDp7fXM6MTU6IgAqAHN0YXRpY19jYWNoZSI7YjoxO3M6MTU6IgAqAHJlbmRlcl9jYWNoZSI7YjoxO3M6MTk6IgAqAHBlcnNpc3RlbnRfY2FjaGUiO2I6MTtzOjE0OiIAKgBlbnRpdHlfa2V5cyI7YTo1OntzOjg6InJldmlzaW9uIjtzOjA6IiI7czo2OiJidW5kbGUiO3M6MDoiIjtzOjg6Imxhbmdjb2RlIjtzOjA6IiI7czoxNjoiZGVmYXVsdF9sYW5nY29kZSI7czoxNjoiZGVmYXVsdF9sYW5nY29kZSI7czoyOToicmV2aXNpb25fdHJhbnNsYXRpb25fYWZmZWN0ZWQiO3M6Mjk6InJldmlzaW9uX3RyYW5zbGF0aW9uX2FmZmVjdGVkIjt9czo1OiIAKgBpZCI7czoyMjoiZW50aXR5X3Rlc3RfbXVsX3JldmxvZyI7czoxNjoiACoAb3JpZ2luYWxDbGFzcyI7TjtzOjExOiIAKgBoYW5kbGVycyI7YTozOntzOjY6ImFjY2VzcyI7czo0NToiRHJ1cGFsXENvcmVcRW50aXR5XEVudGl0eUFjY2Vzc0NvbnRyb2xIYW5kbGVyIjtzOjc6InN0b3JhZ2UiO3M6NDY6IkRydXBhbFxDb3JlXEVudGl0eVxTcWxcU3FsQ29udGVudEVudGl0eVN0b3JhZ2UiO3M6MTI6InZpZXdfYnVpbGRlciI7czozNjoiRHJ1cGFsXENvcmVcRW50aXR5XEVudGl0eVZpZXdCdWlsZGVyIjt9czoxOToiACoAYWRtaW5fcGVybWlzc2lvbiI7TjtzOjI1OiIAKgBwZXJtaXNzaW9uX2dyYW51bGFyaXR5IjtzOjExOiJlbnRpdHlfdHlwZSI7czo4OiIAKgBsaW5rcyI7YTowOnt9czoxNzoiACoAbGFiZWxfY2FsbGJhY2siO047czoyMToiACoAYnVuZGxlX2VudGl0eV90eXBlIjtOO3M6MTI6IgAqAGJ1bmRsZV9vZiI7TjtzOjE1OiIAKgBidW5kbGVfbGFiZWwiO047czoxMzoiACoAYmFzZV90YWJsZSI7TjtzOjIyOiIAKgByZXZpc2lvbl9kYXRhX3RhYmxlIjtOO3M6MTc6IgAqAHJldmlzaW9uX3RhYmxlIjtOO3M6MTM6IgAqAGRhdGFfdGFibGUiO047czoxNToiACoAdHJhbnNsYXRhYmxlIjtiOjA7czoxOToiACoAc2hvd19yZXZpc2lvbl91aSI7YjowO3M6ODoiACoAbGFiZWwiO3M6MDoiIjtzOjE5OiIAKgBsYWJlbF9jb2xsZWN0aW9uIjtzOjA6IiI7czoxNzoiACoAbGFiZWxfc2luZ3VsYXIiO3M6MDoiIjtzOjE1OiIAKgBsYWJlbF9wbHVyYWwiO3M6MDoiIjtzOjE0OiIAKgBsYWJlbF9jb3VudCI7YTowOnt9czoxNToiACoAdXJpX2NhbGxiYWNrIjtOO3M6ODoiACoAZ3JvdXAiO047czoxNDoiACoAZ3JvdXBfbGFiZWwiO047czoyMjoiACoAZmllbGRfdWlfYmFzZV9yb3V0ZSI7TjtzOjI2OiIAKgBjb21tb25fcmVmZXJlbmNlX3RhcmdldCI7YjowO3M6MjI6IgAqAGxpc3RfY2FjaGVfY29udGV4dHMiO2E6MDp7fXM6MTg6IgAqAGxpc3RfY2FjaGVfdGFncyI7YToxOntpOjA7czo5OiJ0ZXN0X2xpc3QiO31zOjE0OiIAKgBjb25zdHJhaW50cyI7YTowOnt9czoxMzoiACoAYWRkaXRpb25hbCI7YTowOnt9czo4OiIAKgBjbGFzcyI7TjtzOjExOiIAKgBwcm92aWRlciI7TjtzOjIwOiIAKgBzdHJpbmdUcmFuc2xhdGlvbiI7Tjt9';
/** @var \Drupal\Tests\system\Functional\Entity\Update\TestRevisionMetadataBcLayerEntityType $entity_type */
$entity_type = unserialize(base64_decode($cached_with_no_metadata_keys));
$required_revision_metadata_keys_no_bc = [];
$this->assertEquals($required_revision_metadata_keys_no_bc, $entity_type->getRevisionMetadataKeys(FALSE));
$required_revision_metadata_keys_with_bc = $required_revision_metadata_keys_no_bc + [
'revision_user' => 'revision_user',
'revision_created' => 'revision_created',
'revision_log_message' => 'revision_log_message',
];
$this->assertEquals($required_revision_metadata_keys_with_bc, $entity_type->getRevisionMetadataKeys(TRUE));
// Ensure that cached entity types with only one required revision metadata
// key will return only that one after a second required revision metadata
// key has been added.
// Contains one revision metadata key - revision_default which is also
// contained in the property holding the required revision metadata keys,
// the entity type id is "entity_test_mul_revlog".
$cached_with_metadata_key_revision_default = 'Tzo4MjoiRHJ1cGFsXFRlc3RzXHN5c3RlbVxGdW5jdGlvbmFsXEVudGl0eVxVcGRhdGVcVGVzdFJldmlzaW9uTWV0YWRhdGFCY0xheWVyRW50aXR5VHlwZSI6Mzk6e3M6MjU6IgAqAHJldmlzaW9uX21ldGFkYXRhX2tleXMiO2E6MTp7czoxNjoicmV2aXNpb25fZGVmYXVsdCI7czoxNjoicmV2aXNpb25fZGVmYXVsdCI7fXM6MzE6IgAqAHJlcXVpcmVkUmV2aXNpb25NZXRhZGF0YUtleXMiO2E6MTp7czoxNjoicmV2aXNpb25fZGVmYXVsdCI7czoxNjoicmV2aXNpb25fZGVmYXVsdCI7fXM6MTU6IgAqAHN0YXRpY19jYWNoZSI7YjoxO3M6MTU6IgAqAHJlbmRlcl9jYWNoZSI7YjoxO3M6MTk6IgAqAHBlcnNpc3RlbnRfY2FjaGUiO2I6MTtzOjE0OiIAKgBlbnRpdHlfa2V5cyI7YTo1OntzOjg6InJldmlzaW9uIjtzOjA6IiI7czo2OiJidW5kbGUiO3M6MDoiIjtzOjg6Imxhbmdjb2RlIjtzOjA6IiI7czoxNjoiZGVmYXVsdF9sYW5nY29kZSI7czoxNjoiZGVmYXVsdF9sYW5nY29kZSI7czoyOToicmV2aXNpb25fdHJhbnNsYXRpb25fYWZmZWN0ZWQiO3M6Mjk6InJldmlzaW9uX3RyYW5zbGF0aW9uX2FmZmVjdGVkIjt9czo1OiIAKgBpZCI7czoyMjoiZW50aXR5X3Rlc3RfbXVsX3JldmxvZyI7czoxNjoiACoAb3JpZ2luYWxDbGFzcyI7TjtzOjExOiIAKgBoYW5kbGVycyI7YTozOntzOjY6ImFjY2VzcyI7czo0NToiRHJ1cGFsXENvcmVcRW50aXR5XEVudGl0eUFjY2Vzc0NvbnRyb2xIYW5kbGVyIjtzOjc6InN0b3JhZ2UiO3M6NDY6IkRydXBhbFxDb3JlXEVudGl0eVxTcWxcU3FsQ29udGVudEVudGl0eVN0b3JhZ2UiO3M6MTI6InZpZXdfYnVpbGRlciI7czozNjoiRHJ1cGFsXENvcmVcRW50aXR5XEVudGl0eVZpZXdCdWlsZGVyIjt9czoxOToiACoAYWRtaW5fcGVybWlzc2lvbiI7TjtzOjI1OiIAKgBwZXJtaXNzaW9uX2dyYW51bGFyaXR5IjtzOjExOiJlbnRpdHlfdHlwZSI7czo4OiIAKgBsaW5rcyI7YTowOnt9czoxNzoiACoAbGFiZWxfY2FsbGJhY2siO047czoyMToiACoAYnVuZGxlX2VudGl0eV90eXBlIjtOO3M6MTI6IgAqAGJ1bmRsZV9vZiI7TjtzOjE1OiIAKgBidW5kbGVfbGFiZWwiO047czoxMzoiACoAYmFzZV90YWJsZSI7TjtzOjIyOiIAKgByZXZpc2lvbl9kYXRhX3RhYmxlIjtOO3M6MTc6IgAqAHJldmlzaW9uX3RhYmxlIjtOO3M6MTM6IgAqAGRhdGFfdGFibGUiO047czoxNToiACoAdHJhbnNsYXRhYmxlIjtiOjA7czoxOToiACoAc2hvd19yZXZpc2lvbl91aSI7YjowO3M6ODoiACoAbGFiZWwiO3M6MDoiIjtzOjE5OiIAKgBsYWJlbF9jb2xsZWN0aW9uIjtzOjA6IiI7czoxNzoiACoAbGFiZWxfc2luZ3VsYXIiO3M6MDoiIjtzOjE1OiIAKgBsYWJlbF9wbHVyYWwiO3M6MDoiIjtzOjE0OiIAKgBsYWJlbF9jb3VudCI7YTowOnt9czoxNToiACoAdXJpX2NhbGxiYWNrIjtOO3M6ODoiACoAZ3JvdXAiO047czoxNDoiACoAZ3JvdXBfbGFiZWwiO047czoyMjoiACoAZmllbGRfdWlfYmFzZV9yb3V0ZSI7TjtzOjI2OiIAKgBjb21tb25fcmVmZXJlbmNlX3RhcmdldCI7YjowO3M6MjI6IgAqAGxpc3RfY2FjaGVfY29udGV4dHMiO2E6MDp7fXM6MTg6IgAqAGxpc3RfY2FjaGVfdGFncyI7YToxOntpOjA7czo5OiJ0ZXN0X2xpc3QiO31zOjE0OiIAKgBjb25zdHJhaW50cyI7YTowOnt9czoxMzoiACoAYWRkaXRpb25hbCI7YTowOnt9czo4OiIAKgBjbGFzcyI7TjtzOjExOiIAKgBwcm92aWRlciI7TjtzOjIwOiIAKgBzdHJpbmdUcmFuc2xhdGlvbiI7Tjt9';
$entity_type = unserialize(base64_decode($cached_with_metadata_key_revision_default));
$required_revision_metadata_keys_no_bc = [
'revision_default' => 'revision_default',
];
$this->assertEquals($required_revision_metadata_keys_no_bc, $entity_type->getRevisionMetadataKeys(FALSE));
$required_revision_metadata_keys_with_bc = $required_revision_metadata_keys_no_bc + [
'revision_user' => 'revision_user',
'revision_created' => 'revision_created',
'revision_log_message' => 'revision_log_message',
];
$this->assertEquals($required_revision_metadata_keys_with_bc, $entity_type->getRevisionMetadataKeys(TRUE));
// Ensure that newly instantiated entity types will return the two required
// revision metadata keys.
$entity_type = new TestRevisionMetadataBcLayerEntityType(['id' => 'test']);
$required_revision_metadata_keys = [
'revision_default' => 'revision_default',
'second_required_key' => 'second_required_key',
];
$this->assertEquals($required_revision_metadata_keys, $entity_type->getRevisionMetadataKeys(FALSE));
// Load an entity type from the cache with no revision metadata keys in the
// annotation.
$entity_last_installed_schema_repository = \Drupal::service('entity.last_installed_schema.repository');
$entity_type = $entity_last_installed_schema_repository->getLastInstalledDefinition('entity_test_mul_revlog');
$revision_metadata_keys = [];
$this->assertEquals($revision_metadata_keys, $entity_type->getRevisionMetadataKeys(FALSE));
$revision_metadata_keys = [
'revision_user' => 'revision_user',
'revision_created' => 'revision_created',
'revision_log_message' => 'revision_log_message'
];
$this->assertEquals($revision_metadata_keys, $entity_type->getRevisionMetadataKeys(TRUE));
// Load an entity type without using the cache with no revision metadata
// keys in the annotation.
$entity_type_manager = \Drupal::entityTypeManager();
$entity_type_manager->useCaches(FALSE);
$entity_type = $entity_type_manager->getDefinition('entity_test_mul_revlog');
$revision_metadata_keys = [
'revision_default' => 'revision_default',
];
$this->assertEquals($revision_metadata_keys, $entity_type->getRevisionMetadataKeys(FALSE));
$revision_metadata_keys = [
'revision_user' => 'revision_user',
'revision_created' => 'revision_created',
'revision_log_message' => 'revision_log_message',
'revision_default' => 'revision_default',
];
$this->assertEquals($revision_metadata_keys, $entity_type->getRevisionMetadataKeys(TRUE));
// Ensure that the BC layer will not be triggered if one of the required
// revision metadata keys is defined in the annotation.
$definition = [
'id' => 'entity_test_mul_revlog',
'revision_metadata_keys' => [
'revision_default' => 'revision_default'
],
];
$entity_type = new ContentEntityType($definition);
$revision_metadata_keys = [
'revision_default' => 'revision_default',
];
$this->assertEquals($revision_metadata_keys, $entity_type->getRevisionMetadataKeys(TRUE));
// Ensure that the BC layer will be triggered if no revision metadata keys
// have been defined in the annotation.
$definition = [
'id' => 'entity_test_mul_revlog',
];
$entity_type = new ContentEntityType($definition);
$revision_metadata_keys = [
'revision_default' => 'revision_default',
'revision_user' => 'revision_user',
'revision_created' => 'revision_created',
'revision_log_message' => 'revision_log_message',
];
$this->assertEquals($revision_metadata_keys, $entity_type->getRevisionMetadataKeys(TRUE));
}
/**
* Tests that the revision metadata key BC layer was updated correctly.
*/
public function testSystemUpdate8501() {
$this->runUpdates();
/** @var \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface $definition_update_manager */
$definition_update_manager = $this->container->get('entity.definition_update_manager');
foreach (['block_content', 'node'] as $entity_type_id) {
$installed_entity_type = $definition_update_manager->getEntityType($entity_type_id);
$revision_metadata_keys = $installed_entity_type->get('revision_metadata_keys');
$this->assertTrue(isset($revision_metadata_keys['revision_default']));
$required_revision_metadata_keys = $installed_entity_type->get('requiredRevisionMetadataKeys');
$this->assertTrue(isset($required_revision_metadata_keys['revision_default']));
}
}
}
/**
* Test entity type class for adding new required revision metadata keys.
*/
class TestRevisionMetadataBcLayerEntityType extends ContentEntityType {
/**
* {@inheritdoc}
*/
public function __construct($definition) {
// Only new instances should provide the required revision metadata keys.
// The cached instances should return only what already has been stored
// under the property $revision_metadata_keys. The BC layer in
// ::getRevisionMetadataKeys() has to detect if the revision metadata keys
// have been provided by the entity type annotation, therefore we add keys
// to the property $requiredRevisionMetadataKeys only if those keys aren't
// set in the entity type annotation.
if (!isset($definition['revision_metadata_keys']['second_required_key'])) {
$this->requiredRevisionMetadataKeys['second_required_key'] = 'second_required_key';
}
parent::__construct($definition);
}
}