From 2019555b5405da617b486775d86dcf6149669e27 Mon Sep 17 00:00:00 2001 From: Francesco Placella <plach@183211.no-reply.drupal.org> Date: Sat, 10 Aug 2019 09:43:33 +0200 Subject: [PATCH] Revert "Issue #3056816 by amateescu, larowlan, plach, Sam152, catch: Installing a new field storage definition during a fieldable entity type update is not possible" This reverts commit 2f154624fdacce338849d15d96d272ee6c60c85c. --- .../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, 24 insertions(+), 197 deletions(-) delete mode 100644 core/modules/system/tests/modules/entity_test_update/entity_test_update.services.yml delete 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 4bf05404d90a..0adcecfb6286 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->entityLastInstalledSchemaRepository->setLastInstalledDefinition($entity_type); - $this->eventDispatcher->dispatch(EntityTypeEvents::UPDATE, new EntityTypeEvent($entity_type, $original)); + + $this->entityLastInstalledSchemaRepository->setLastInstalledDefinition($entity_type); } /** @@ -116,9 +116,9 @@ public function onEntityTypeDelete(EntityTypeInterface $entity_type) { $storage->onEntityTypeDelete($entity_type); } - $this->entityLastInstalledSchemaRepository->deleteLastInstalledDefinition($entity_type_id); - $this->eventDispatcher->dispatch(EntityTypeEvents::DELETE, new EntityTypeEvent($entity_type)); + + $this->entityLastInstalledSchemaRepository->deleteLastInstalledDefinition($entity_type_id); } /** @@ -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 9194880fa8a0..fda621b92915 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 4e202ae8fd15..75e1bf33ce68 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', '@entity.last_installed_schema.repository'] + arguments: ['@state'] 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 03e768fb3c08..6cdceb6d9cc7 100644 --- a/core/modules/system/tests/modules/entity_test/src/EntityTestDefinitionSubscriber.php +++ b/core/modules/system/tests/modules/entity_test/src/EntityTestDefinitionSubscriber.php @@ -2,7 +2,6 @@ namespace Drupal\entity_test; -use Drupal\Core\Entity\EntityLastInstalledSchemaRepositoryInterface; use Drupal\Core\Entity\EntityTypeEvents; use Drupal\Core\Entity\EntityTypeEventSubscriberTrait; use Drupal\Core\Entity\EntityTypeInterface; @@ -29,13 +28,6 @@ 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. * @@ -46,9 +38,8 @@ class EntityTestDefinitionSubscriber implements EventSubscriberInterface, Entity /** * {@inheritdoc} */ - public function __construct(StateInterface $state, EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_repository) { + public function __construct(StateInterface $state) { $this->state = $state; - $this->entityLastInstalledSchemaRepository = $entity_last_installed_schema_repository; } /** @@ -62,9 +53,6 @@ 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); } @@ -72,11 +60,6 @@ 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); } @@ -91,9 +74,6 @@ 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); } @@ -101,9 +81,6 @@ 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); } @@ -111,10 +88,6 @@ 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); } @@ -122,9 +95,6 @@ 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); } @@ -160,30 +130,4 @@ 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 deleted file mode 100644 index 45d1606daa94..000000000000 --- a/core/modules/system/tests/modules/entity_test_update/entity_test_update.services.yml +++ /dev/null @@ -1,6 +0,0 @@ -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 deleted file mode 100644 index 37dac651e2c6..000000000000 --- a/core/modules/system/tests/modules/entity_test_update/src/EventSubscriber/EntitySchemaSubscriber.php +++ /dev/null @@ -1,74 +0,0 @@ -<?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 afe5af6681f5..76b63efef159 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php @@ -14,8 +14,6 @@ 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; @@ -837,47 +835,28 @@ public function testDefinitionEvents() { $event_subscriber->enableEventTracking(); // Test field storage definition events. - $storage_definition = FieldStorageDefinition::create('string') - ->setName('field_storage_test') - ->setLabel(new TranslatableMarkup('Field storage test')) - ->setTargetEntityTypeId('entity_test_rev'); - + $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.'); $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($updated_storage_definition, $storage_definition); + \Drupal::service('field_storage_definition.listener')->onFieldStorageDefinitionUpdate($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($updated_entity_type, $entity_type); + \Drupal::service('entity_type.listener')->onEntityTypeUpdate($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 a525f650eb31..937a2ab5a452 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/FieldableEntityDefinitionUpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/FieldableEntityDefinitionUpdateTest.php @@ -151,17 +151,13 @@ 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, TRUE); + $this->assertEntityTypeSchema($new_rev, $new_mul); $this->assertEntityData($initial_rev, $initial_mul); $change_list = $this->entityDefinitionUpdateManager->getChangeList(); @@ -431,20 +427,8 @@ 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, $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); - } - + protected function assertEntityTypeSchema($revisionable, $translatable) { if ($revisionable && $translatable) { $this->assertRevisionableAndTranslatable(); } -- GitLab