From 2f154624fdacce338849d15d96d272ee6c60c85c Mon Sep 17 00:00:00 2001 From: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Wed, 7 Aug 2019 08:34:28 +1000 Subject: [PATCH] Issue #3056816 by amateescu, larowlan, plach, Sam152, catch: Installing a new field storage definition during a fieldable entity type update is not possible --- .../Drupal/Core/Entity/EntityTypeListener.php | 16 ++-- .../Field/FieldStorageDefinitionListener.php | 12 +-- .../entity_test/entity_test.services.yml | 2 +- .../src/EntityTestDefinitionSubscriber.php | 58 ++++++++++++++- .../entity_test_update.services.yml | 6 ++ .../EntitySchemaSubscriber.php | 74 +++++++++++++++++++ .../Entity/EntityDefinitionUpdateTest.php | 33 +++++++-- .../FieldableEntityDefinitionUpdateTest.php | 20 ++++- 8 files changed, 197 insertions(+), 24 deletions(-) create mode 100644 core/modules/system/tests/modules/entity_test_update/entity_test_update.services.yml create mode 100644 core/modules/system/tests/modules/entity_test_update/src/EventSubscriber/EntitySchemaSubscriber.php diff --git a/core/lib/Drupal/Core/Entity/EntityTypeListener.php b/core/lib/Drupal/Core/Entity/EntityTypeListener.php index 0adcecfb6286..4bf05404d90a 100644 --- a/core/lib/Drupal/Core/Entity/EntityTypeListener.php +++ b/core/lib/Drupal/Core/Entity/EntityTypeListener.php @@ -71,12 +71,12 @@ public function onEntityTypeCreate(EntityTypeInterface $entity_type) { $storage->onEntityTypeCreate($entity_type); } - $this->eventDispatcher->dispatch(EntityTypeEvents::CREATE, new EntityTypeEvent($entity_type)); - $this->entityLastInstalledSchemaRepository->setLastInstalledDefinition($entity_type); if ($entity_type->entityClassImplements(FieldableEntityInterface::class)) { $this->entityLastInstalledSchemaRepository->setLastInstalledFieldStorageDefinitions($entity_type_id, $this->entityFieldManager->getFieldStorageDefinitions($entity_type_id)); } + + $this->eventDispatcher->dispatch(EntityTypeEvents::CREATE, new EntityTypeEvent($entity_type)); } /** @@ -94,9 +94,9 @@ public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeI $storage->onEntityTypeUpdate($entity_type, $original); } - $this->eventDispatcher->dispatch(EntityTypeEvents::UPDATE, new EntityTypeEvent($entity_type, $original)); - $this->entityLastInstalledSchemaRepository->setLastInstalledDefinition($entity_type); + + $this->eventDispatcher->dispatch(EntityTypeEvents::UPDATE, new EntityTypeEvent($entity_type, $original)); } /** @@ -116,9 +116,9 @@ public function onEntityTypeDelete(EntityTypeInterface $entity_type) { $storage->onEntityTypeDelete($entity_type); } - $this->eventDispatcher->dispatch(EntityTypeEvents::DELETE, new EntityTypeEvent($entity_type)); - $this->entityLastInstalledSchemaRepository->deleteLastInstalledDefinition($entity_type_id); + + $this->eventDispatcher->dispatch(EntityTypeEvents::DELETE, new EntityTypeEvent($entity_type)); } /** @@ -135,12 +135,12 @@ public function onFieldableEntityTypeUpdate(EntityTypeInterface $entity_type, En } if ($sandbox === NULL || (isset($sandbox['#finished']) && $sandbox['#finished'] == 1)) { - $this->eventDispatcher->dispatch(EntityTypeEvents::UPDATE, new EntityTypeEvent($entity_type, $original)); - $this->entityLastInstalledSchemaRepository->setLastInstalledDefinition($entity_type); if ($entity_type->entityClassImplements(FieldableEntityInterface::class)) { $this->entityLastInstalledSchemaRepository->setLastInstalledFieldStorageDefinitions($entity_type_id, $field_storage_definitions); } + + $this->eventDispatcher->dispatch(EntityTypeEvents::UPDATE, new EntityTypeEvent($entity_type, $original)); } } diff --git a/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php b/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php index fda621b92915..9194880fa8a0 100644 --- a/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php +++ b/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php @@ -85,10 +85,10 @@ public function onFieldStorageDefinitionCreate(FieldStorageDefinitionInterface $ $storage->onFieldStorageDefinitionCreate($storage_definition); } - $this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::CREATE, new FieldStorageDefinitionEvent($storage_definition)); - $this->entityLastInstalledSchemaRepository->setLastInstalledFieldStorageDefinition($storage_definition); $this->entityFieldManager->clearCachedFieldDefinitions(); + + $this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::CREATE, new FieldStorageDefinitionEvent($storage_definition)); } /** @@ -104,10 +104,10 @@ public function onFieldStorageDefinitionUpdate(FieldStorageDefinitionInterface $ $storage->onFieldStorageDefinitionUpdate($storage_definition, $original); } - $this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::UPDATE, new FieldStorageDefinitionEvent($storage_definition, $original)); - $this->entityLastInstalledSchemaRepository->setLastInstalledFieldStorageDefinition($storage_definition); $this->entityFieldManager->clearCachedFieldDefinitions(); + + $this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::UPDATE, new FieldStorageDefinitionEvent($storage_definition, $original)); } /** @@ -133,10 +133,10 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $ $storage->onFieldStorageDefinitionDelete($storage_definition); } - $this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::DELETE, new FieldStorageDefinitionEvent($storage_definition)); - $this->entityLastInstalledSchemaRepository->deleteLastInstalledFieldStorageDefinition($storage_definition); $this->entityFieldManager->clearCachedFieldDefinitions(); + + $this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::DELETE, new FieldStorageDefinitionEvent($storage_definition)); } } diff --git a/core/modules/system/tests/modules/entity_test/entity_test.services.yml b/core/modules/system/tests/modules/entity_test/entity_test.services.yml index 75e1bf33ce68..4e202ae8fd15 100644 --- a/core/modules/system/tests/modules/entity_test/entity_test.services.yml +++ b/core/modules/system/tests/modules/entity_test/entity_test.services.yml @@ -1,7 +1,7 @@ services: entity_test.definition.subscriber: class: Drupal\entity_test\EntityTestDefinitionSubscriber - arguments: ['@state'] + arguments: ['@state', '@entity.last_installed_schema.repository'] tags: - { name: event_subscriber } cache_context.entity_test_view_grants: diff --git a/core/modules/system/tests/modules/entity_test/src/EntityTestDefinitionSubscriber.php b/core/modules/system/tests/modules/entity_test/src/EntityTestDefinitionSubscriber.php index 6cdceb6d9cc7..03e768fb3c08 100644 --- a/core/modules/system/tests/modules/entity_test/src/EntityTestDefinitionSubscriber.php +++ b/core/modules/system/tests/modules/entity_test/src/EntityTestDefinitionSubscriber.php @@ -2,6 +2,7 @@ namespace Drupal\entity_test; +use Drupal\Core\Entity\EntityLastInstalledSchemaRepositoryInterface; use Drupal\Core\Entity\EntityTypeEvents; use Drupal\Core\Entity\EntityTypeEventSubscriberTrait; use Drupal\Core\Entity\EntityTypeInterface; @@ -28,6 +29,13 @@ class EntityTestDefinitionSubscriber implements EventSubscriberInterface, Entity */ protected $state; + /** + * The last installed schema repository. + * + * @var \Drupal\Core\Entity\EntityLastInstalledSchemaRepositoryInterface + */ + protected $entityLastInstalledSchemaRepository; + /** * Flag determining whether events should be tracked. * @@ -38,8 +46,9 @@ class EntityTestDefinitionSubscriber implements EventSubscriberInterface, Entity /** * {@inheritdoc} */ - public function __construct(StateInterface $state) { + public function __construct(StateInterface $state, EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_repository) { $this->state = $state; + $this->entityLastInstalledSchemaRepository = $entity_last_installed_schema_repository; } /** @@ -53,6 +62,9 @@ public static function getSubscribedEvents() { * {@inheritdoc} */ public function onEntityTypeCreate(EntityTypeInterface $entity_type) { + if ($this->entityLastInstalledSchemaRepository->getLastInstalledDefinition($entity_type->id())) { + $this->storeDefinitionUpdate(EntityTypeEvents::CREATE); + } $this->storeEvent(EntityTypeEvents::CREATE); } @@ -60,6 +72,11 @@ public function onEntityTypeCreate(EntityTypeInterface $entity_type) { * {@inheritdoc} */ public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeInterface $original) { + $last_installed_definition = $this->entityLastInstalledSchemaRepository->getLastInstalledDefinition($entity_type->id()); + if ((string) $last_installed_definition->getLabel() === 'Updated entity test rev') { + $this->storeDefinitionUpdate(EntityTypeEvents::UPDATE); + } + $this->storeEvent(EntityTypeEvents::UPDATE); } @@ -74,6 +91,9 @@ public function onFieldableEntityTypeUpdate(EntityTypeInterface $entity_type, En * {@inheritdoc} */ public function onEntityTypeDelete(EntityTypeInterface $entity_type) { + if (!$this->entityLastInstalledSchemaRepository->getLastInstalledDefinition($entity_type->id())) { + $this->storeDefinitionUpdate(EntityTypeEvents::DELETE); + } $this->storeEvent(EntityTypeEvents::DELETE); } @@ -81,6 +101,9 @@ public function onEntityTypeDelete(EntityTypeInterface $entity_type) { * {@inheritdoc} */ public function onFieldStorageDefinitionCreate(FieldStorageDefinitionInterface $storage_definition) { + if (isset($this->entityLastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions($storage_definition->getTargetEntityTypeId())[$storage_definition->getName()])) { + $this->storeDefinitionUpdate(FieldStorageDefinitionEvents::CREATE); + } $this->storeEvent(FieldStorageDefinitionEvents::CREATE); } @@ -88,6 +111,10 @@ public function onFieldStorageDefinitionCreate(FieldStorageDefinitionInterface $ * {@inheritdoc} */ public function onFieldStorageDefinitionUpdate(FieldStorageDefinitionInterface $storage_definition, FieldStorageDefinitionInterface $original) { + $last_installed_definition = $this->entityLastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions($storage_definition->getTargetEntityTypeId())[$storage_definition->getName()]; + if ((string) $last_installed_definition->getLabel() === 'Updated field storage test') { + $this->storeDefinitionUpdate(FieldStorageDefinitionEvents::UPDATE); + } $this->storeEvent(FieldStorageDefinitionEvents::UPDATE); } @@ -95,6 +122,9 @@ public function onFieldStorageDefinitionUpdate(FieldStorageDefinitionInterface $ * {@inheritdoc} */ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $storage_definition) { + if (!isset($this->entityLastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions($storage_definition->getTargetEntityTypeId())[$storage_definition->getName()])) { + $this->storeDefinitionUpdate(FieldStorageDefinitionEvents::DELETE); + } $this->storeEvent(FieldStorageDefinitionEvents::DELETE); } @@ -130,4 +160,30 @@ protected function storeEvent($event_name) { } } + /** + * Checks whether the installed definitions were updated before the event. + * + * @param string $event_name + * The event name. + * + * @return bool + * TRUE if the last installed entity type of field storage definitions have + * been updated before the was fired, FALSE otherwise. + */ + public function hasDefinitionBeenUpdated($event_name) { + return (bool) $this->state->get($event_name . '_updated_definition'); + } + + /** + * Stores the installed definition state for the specified event. + * + * @param string $event_name + * The event name. + */ + protected function storeDefinitionUpdate($event_name) { + if ($this->trackEvents) { + $this->state->set($event_name . '_updated_definition', TRUE); + } + } + } diff --git a/core/modules/system/tests/modules/entity_test_update/entity_test_update.services.yml b/core/modules/system/tests/modules/entity_test_update/entity_test_update.services.yml new file mode 100644 index 000000000000..45d1606daa94 --- /dev/null +++ b/core/modules/system/tests/modules/entity_test_update/entity_test_update.services.yml @@ -0,0 +1,6 @@ +services: + entity_test_update.entity_schema_listener: + class: Drupal\entity_test_update\EventSubscriber\EntitySchemaSubscriber + arguments: ['@entity.definition_update_manager', '@state'] + tags: + - { name: 'event_subscriber' } diff --git a/core/modules/system/tests/modules/entity_test_update/src/EventSubscriber/EntitySchemaSubscriber.php b/core/modules/system/tests/modules/entity_test_update/src/EventSubscriber/EntitySchemaSubscriber.php new file mode 100644 index 000000000000..37dac651e2c6 --- /dev/null +++ b/core/modules/system/tests/modules/entity_test_update/src/EventSubscriber/EntitySchemaSubscriber.php @@ -0,0 +1,74 @@ +<?php + +namespace Drupal\entity_test_update\EventSubscriber; + +use Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface; +use Drupal\Core\Entity\EntityTypeEventSubscriberTrait; +use Drupal\Core\Entity\EntityTypeInterface; +use Drupal\Core\Entity\EntityTypeListenerInterface; +use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\Core\State\StateInterface; +use Drupal\Core\StringTranslation\TranslatableMarkup; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +/** + * Defines a class for listening to entity schema changes. + */ +class EntitySchemaSubscriber implements EntityTypeListenerInterface, EventSubscriberInterface { + + use EntityTypeEventSubscriberTrait; + + /** + * The entity definition update manager. + * + * @var \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface + */ + protected $entityDefinitionUpdateManager; + + /** + * The state service. + * + * @var \Drupal\Core\State\StateInterface + */ + protected $state; + + /** + * Constructs a new EntitySchemaSubscriber. + * + * @param \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface $entityDefinitionUpdateManager + * The entity definition update manager. + * @param \Drupal\Core\State\StateInterface $state + * The state service. + */ + public function __construct(EntityDefinitionUpdateManagerInterface $entityDefinitionUpdateManager, StateInterface $state) { + $this->entityDefinitionUpdateManager = $entityDefinitionUpdateManager; + $this->state = $state; + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + return static::getEntityTypeEvents(); + } + + /** + * {@inheritdoc} + */ + public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeInterface $original) { + // Only add the new base field when a test needs it. + if (!$this->state->get('entity_test_update.install_new_base_field_during_update', FALSE)) { + return; + } + + // Add a new base field when the entity type is updated. + $definitions = $this->state->get('entity_test_update.additional_base_field_definitions', []); + $definitions['new_base_field'] = BaseFieldDefinition::create('string') + ->setName('new_base_field') + ->setLabel(new TranslatableMarkup('A new base field')); + $this->state->set('entity_test_update.additional_base_field_definitions', $definitions); + + $this->entityDefinitionUpdateManager->installFieldStorageDefinition('new_base_field', 'entity_test_update', 'entity_test_update', $definitions['new_base_field']); + } + +} diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php index 76b63efef159..afe5af6681f5 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php @@ -14,6 +14,8 @@ use Drupal\Core\Field\FieldException; use Drupal\Core\Field\FieldStorageDefinitionEvents; use Drupal\Core\Language\LanguageInterface; +use Drupal\Core\StringTranslation\TranslatableMarkup; +use Drupal\entity_test\FieldStorageDefinition; use Drupal\entity_test_update\Entity\EntityTestUpdate; use Drupal\Tests\system\Functional\Entity\Traits\EntityDefinitionTestTrait; @@ -835,28 +837,47 @@ public function testDefinitionEvents() { $event_subscriber->enableEventTracking(); // Test field storage definition events. - $storage_definition = current(\Drupal::service('entity_field.manager')->getFieldStorageDefinitions('entity_test_rev')); - $this->assertFalse($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::DELETE), 'Entity type delete was not dispatched yet.'); - \Drupal::service('field_storage_definition.listener')->onFieldStorageDefinitionDelete($storage_definition); - $this->assertTrue($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::DELETE), 'Entity type delete event successfully dispatched.'); + $storage_definition = FieldStorageDefinition::create('string') + ->setName('field_storage_test') + ->setLabel(new TranslatableMarkup('Field storage test')) + ->setTargetEntityTypeId('entity_test_rev'); + $this->assertFalse($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::CREATE), 'Entity type create was not dispatched yet.'); \Drupal::service('field_storage_definition.listener')->onFieldStorageDefinitionCreate($storage_definition); $this->assertTrue($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::CREATE), 'Entity type create event successfully dispatched.'); + $this->assertTrue($event_subscriber->hasDefinitionBeenUpdated(FieldStorageDefinitionEvents::CREATE), 'Last installed field storage definition was created before the event was fired.'); + + $updated_storage_definition = clone $storage_definition; + $updated_storage_definition->setLabel(new TranslatableMarkup('Updated field storage test')); $this->assertFalse($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::UPDATE), 'Entity type update was not dispatched yet.'); - \Drupal::service('field_storage_definition.listener')->onFieldStorageDefinitionUpdate($storage_definition, $storage_definition); + \Drupal::service('field_storage_definition.listener')->onFieldStorageDefinitionUpdate($updated_storage_definition, $storage_definition); $this->assertTrue($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::UPDATE), 'Entity type update event successfully dispatched.'); + $this->assertTrue($event_subscriber->hasDefinitionBeenUpdated(FieldStorageDefinitionEvents::UPDATE), 'Last installed field storage definition was updated before the event was fired.'); + + $this->assertFalse($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::DELETE), 'Entity type delete was not dispatched yet.'); + \Drupal::service('field_storage_definition.listener')->onFieldStorageDefinitionDelete($storage_definition); + $this->assertTrue($event_subscriber->hasEventFired(FieldStorageDefinitionEvents::DELETE), 'Entity type delete event successfully dispatched.'); + $this->assertTrue($event_subscriber->hasDefinitionBeenUpdated(FieldStorageDefinitionEvents::DELETE), 'Last installed field storage definition was deleted before the event was fired.'); // Test entity type events. $entity_type = $this->entityTypeManager->getDefinition('entity_test_rev'); + $this->assertFalse($event_subscriber->hasEventFired(EntityTypeEvents::CREATE), 'Entity type create was not dispatched yet.'); \Drupal::service('entity_type.listener')->onEntityTypeCreate($entity_type); $this->assertTrue($event_subscriber->hasEventFired(EntityTypeEvents::CREATE), 'Entity type create event successfully dispatched.'); + $this->assertTrue($event_subscriber->hasDefinitionBeenUpdated(EntityTypeEvents::CREATE), 'Last installed entity type definition was created before the event was fired.'); + + $updated_entity_type = clone $entity_type; + $updated_entity_type->set('label', new TranslatableMarkup('Updated entity test rev')); $this->assertFalse($event_subscriber->hasEventFired(EntityTypeEvents::UPDATE), 'Entity type update was not dispatched yet.'); - \Drupal::service('entity_type.listener')->onEntityTypeUpdate($entity_type, $entity_type); + \Drupal::service('entity_type.listener')->onEntityTypeUpdate($updated_entity_type, $entity_type); $this->assertTrue($event_subscriber->hasEventFired(EntityTypeEvents::UPDATE), 'Entity type update event successfully dispatched.'); + $this->assertTrue($event_subscriber->hasDefinitionBeenUpdated(EntityTypeEvents::UPDATE), 'Last installed entity type definition was updated before the event was fired.'); + $this->assertFalse($event_subscriber->hasEventFired(EntityTypeEvents::DELETE), 'Entity type delete was not dispatched yet.'); \Drupal::service('entity_type.listener')->onEntityTypeDelete($entity_type); $this->assertTrue($event_subscriber->hasEventFired(EntityTypeEvents::DELETE), 'Entity type delete event successfully dispatched.'); + $this->assertTrue($event_subscriber->hasDefinitionBeenUpdated(EntityTypeEvents::DELETE), 'Last installed entity type definition was deleted before the event was fired.'); } /** diff --git a/core/tests/Drupal/KernelTests/Core/Entity/FieldableEntityDefinitionUpdateTest.php b/core/tests/Drupal/KernelTests/Core/Entity/FieldableEntityDefinitionUpdateTest.php index 937a2ab5a452..a525f650eb31 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/FieldableEntityDefinitionUpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/FieldableEntityDefinitionUpdateTest.php @@ -151,13 +151,17 @@ public function testFieldableEntityTypeUpdates($initial_rev, $initial_mul, $new_ $this->assertEntityData($initial_rev, $initial_mul); } + // Enable the creation of a new base field during a fieldable entity type + // update. + $this->state->set('entity_test_update.install_new_base_field_during_update', TRUE); + // Simulate a batch run since we are converting the entities one by one. $sandbox = []; do { $this->entityDefinitionUpdateManager->updateFieldableEntityType($updated_entity_type, $updated_field_storage_definitions, $sandbox); } while ($sandbox['#finished'] != 1); - $this->assertEntityTypeSchema($new_rev, $new_mul); + $this->assertEntityTypeSchema($new_rev, $new_mul, TRUE); $this->assertEntityData($initial_rev, $initial_mul); $change_list = $this->entityDefinitionUpdateManager->getChangeList(); @@ -427,8 +431,20 @@ protected function assertEntityData($revisionable, $translatable) { * Whether the entity type is revisionable or not. * @param bool $translatable * Whether the entity type is translatable or not. + * @param bool $new_base_field + * (optional) Whether a new base field was added as part of the update. + * Defaults to FALSE. */ - protected function assertEntityTypeSchema($revisionable, $translatable) { + protected function assertEntityTypeSchema($revisionable, $translatable, $new_base_field = FALSE) { + // Check whether the 'new_base_field' field has been installed correctly. + $field_storage_definition = $this->entityDefinitionUpdateManager->getFieldStorageDefinition('new_base_field', $this->entityTypeId); + if ($new_base_field) { + $this->assertNotNull($field_storage_definition); + } + else { + $this->assertNull($field_storage_definition); + } + if ($revisionable && $translatable) { $this->assertRevisionableAndTranslatable(); } -- GitLab