diff --git a/core/core.services.yml b/core/core.services.yml index 040989e..6959df1 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -575,7 +575,7 @@ services: arguments: ['@state'] field_storage_definition.listener: class: Drupal\Core\Field\FieldStorageDefinitionListener - arguments: ['@entity_type.manager', '@event_dispatcher', '@entity.last_installed_schema.repository', '@entity_field.manager'] + arguments: ['@entity_type.manager', '@event_dispatcher', '@entity.last_installed_schema.repository', '@entity_field.manager', '@field.deleted_fields_repository'] field_definition.listener: class: Drupal\Core\Field\FieldDefinitionListener arguments: ['@entity_type.manager', '@entity_field.manager', '@keyvalue', '@cache.discovery'] diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index 2c667d4..e9199eb 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -4,6 +4,7 @@ use Drupal\Core\Database\Connection; use Drupal\Core\Database\DatabaseException; +use Drupal\Core\Database\DatabaseExceptionWrapper; use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\Entity\ContentEntityTypeInterface; use Drupal\Core\Entity\EntityManagerInterface; @@ -88,6 +89,13 @@ class SqlContentEntityStorageSchema implements DynamicallyFieldableEntityStorage protected $installedStorageSchema; /** + * The deleted fields repository. + * + * @var \Drupal\Core\Field\DeletedFieldsRepositoryInterface + */ + protected $deletedFieldsRepository; + + /** * Constructs a SqlContentEntityStorageSchema. * * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager @@ -124,6 +132,23 @@ protected function installedStorageSchema() { } /** + * Gets the deleted fields repository. + * + * @return \Drupal\Core\Field\DeletedFieldsRepositoryInterface + * The deleted fields repository. + * + * @todo Inject this dependency in the constructor once this class can be + * instantiated as a regular entity handler: + * https://www.drupal.org/node/2332857. + */ + protected function deletedFieldsRepository() { + if (!isset($this->deletedFieldsRepository)) { + $this->deletedFieldsRepository = \Drupal::service('field.deleted_fields_repository'); + } + return $this->deletedFieldsRepository; + } + + /** * {@inheritdoc} */ public function requiresEntityStorageSchemaChanges(EntityTypeInterface $entity_type, EntityTypeInterface $original) { @@ -411,16 +436,36 @@ public function onFieldStorageDefinitionCreate(FieldStorageDefinitionInterface $ public function onFieldStorageDefinitionUpdate(FieldStorageDefinitionInterface $storage_definition, FieldStorageDefinitionInterface $original) { // Store original definitions so that switching between shared and dedicated // field table layout works. - $this->originalDefinitions = $this->fieldStorageDefinitions; - $this->originalDefinitions[$original->getName()] = $original; $this->performFieldSchemaOperation('update', $storage_definition, $original); - $this->originalDefinitions = NULL; } /** * {@inheritdoc} */ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $storage_definition) { + try { + $has_data = $this->storage->countFieldData($storage_definition, TRUE); + + // @todo We can not yet purge bundle base fields. + if ($storage_definition instanceof BaseFieldDefinition && !$storage_definition->isBaseField() && $has_data) { + throw new FieldStorageDefinitionUpdateForbiddenException('Unable to delete a field (' . $storage_definition->getName() . ' in ' . $storage_definition->getTargetEntityTypeId() . ' entity) with data that cannot be purged.'); + } + + // If the field storage does not have any data, we can safely delete its + // schema. + if (!$has_data) { + $this->performFieldSchemaOperation('delete', $storage_definition); + return; + } + } + catch (DatabaseExceptionWrapper $e) { + // This may happen when changing field storage schema, since we are not + // able to use a table mapping matching the passed storage definition. + // @todo Revisit this once we are able to instantiate the table mapping + // properly. See https://www.drupal.org/node/2274017. + return; + } + // There's nothing to do if the field storage uses a custom storage. if ($storage_definition->hasCustomStorage()) { return; @@ -443,26 +488,23 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $ } } else { - // @todo Ensure that the last installed definition for the base field - // storage definition is marked as deleted. // Move the field data from the shared table to a dedicated one in order - // to allow it to be purged like a configurable field. + // to allow it to be purged like any other field. $shared_table_field_columns = $table_mapping->getColumnNames($storage_definition->getName()); - $storage_definition->setDeleted(TRUE); + // Refresh the table mapping to use the deleted storage definition. + $deleted_storage_definition = $this->deletedFieldsRepository()->getFieldStorages()[$storage_definition->getUniqueStorageIdentifier()]; + $updated_storage_definitions = [$storage_definition->getName() => $deleted_storage_definition] + $storage_definitions; + $table_mapping = $this->storage->getTableMapping($updated_storage_definitions); - // Refresh the table mapping with the updated storage definition. - $storage_definitions[$storage_definition->getName()] = $storage_definition; - $table_mapping = $this->storage->getTableMapping($storage_definitions); + $dedicated_table_field_schema = $this->getDedicatedTableSchema($deleted_storage_definition); + $dedicated_table_field_columns = $table_mapping->getColumnNames($deleted_storage_definition->getName()); - $dedicated_table_field_schema = $this->getDedicatedTableSchema($storage_definition); - $dedicated_table_field_columns = $table_mapping->getColumnNames($storage_definition->getName()); - - $dedicated_table_name = $table_mapping->getDedicatedDataTableName($storage_definition, TRUE); - $dedicated_table_name_mapping[$table_mapping->getDedicatedDataTableName($storage_definition)] = $dedicated_table_name; + $dedicated_table_name = $table_mapping->getDedicatedDataTableName($deleted_storage_definition, TRUE); + $dedicated_table_name_mapping[$table_mapping->getDedicatedDataTableName($deleted_storage_definition)] = $dedicated_table_name; if ($this->entityType->isRevisionable()) { - $dedicated_revision_table_name = $table_mapping->getDedicatedRevisionTableName($storage_definition, TRUE); - $dedicated_table_name_mapping[$table_mapping->getDedicatedRevisionTableName($storage_definition)] = $dedicated_revision_table_name; + $dedicated_revision_table_name = $table_mapping->getDedicatedRevisionTableName($deleted_storage_definition, TRUE); + $dedicated_table_name_mapping[$table_mapping->getDedicatedRevisionTableName($deleted_storage_definition)] = $dedicated_revision_table_name; } // Create the dedicated field tables using "deleted" table names. @@ -491,8 +533,6 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $ ->from($this->getSelectQueryForFieldStorageDeletion($revision_table, $shared_table_field_columns, $dedicated_table_field_columns, $base_table)) ->execute(); } - - // @todo Remove the data from the base tables. } catch (\Exception $e) { if ($this->database->supportsTransactionalDDL()) { @@ -506,6 +546,9 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $ } throw $e; } + + // Delete the field from the shared tables. + $this->deleteSharedTableSchema($storage_definition); } } @@ -1350,17 +1393,13 @@ protected function createSharedTableSchema(FieldStorageDefinitionInterface $stor * The storage definition of the field being deleted. */ protected function deleteDedicatedTableSchema(FieldStorageDefinitionInterface $storage_definition) { - // When switching from dedicated to shared field table layout we need need - // to delete the field tables with their regular names. When this happens - // original definitions will be defined. - $deleted = !$this->originalDefinitions; $table_mapping = $this->storage->getTableMapping(); - $table_name = $table_mapping->getDedicatedDataTableName($storage_definition, $deleted); + $table_name = $table_mapping->getDedicatedDataTableName($storage_definition, $storage_definition->isDeleted()); if ($this->database->schema()->tableExists($table_name)) { $this->database->schema()->dropTable($table_name); } if ($this->entityType->isRevisionable()) { - $revision_table_name = $table_mapping->getDedicatedRevisionTableName($storage_definition, $deleted); + $revision_table_name = $table_mapping->getDedicatedRevisionTableName($storage_definition, $storage_definition->isDeleted()); if ($this->database->schema()->tableExists($revision_table_name)) { $this->database->schema()->dropTable($revision_table_name); } diff --git a/core/lib/Drupal/Core/Field/DeletedFieldsRepository.php b/core/lib/Drupal/Core/Field/DeletedFieldsRepository.php index 09118b6..31eb813 100644 --- a/core/lib/Drupal/Core/Field/DeletedFieldsRepository.php +++ b/core/lib/Drupal/Core/Field/DeletedFieldsRepository.php @@ -38,7 +38,7 @@ public function getFields() { foreach ($fields as $id => $field) { // Configurable fields have an internal reference to their field storage // which we must keep in sync if the storage itself is also deleted. - if ($field instanceOf FieldConfigInterface && isset($deleted_storages[$field->field_storage_unique_id])) { + if ($field instanceof FieldConfigInterface && isset($deleted_storages[$field->field_storage_unique_id])) { $config = $field->toArray(); $config['deleted'] = TRUE; $config['field_storage'] = $deleted_storages[$field->field_storage_unique_id]; diff --git a/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php b/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php index 154e485..1223637 100644 --- a/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php +++ b/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php @@ -2,9 +2,11 @@ namespace Drupal\Core\Field; +use Drupal\Core\Database\DatabaseExceptionWrapper; use Drupal\Core\Entity\EntityLastInstalledSchemaRepositoryInterface; use Drupal\Core\Entity\EntityFieldManagerInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; +use Drupal\Core\Entity\FieldableEntityStorageInterface; use Drupal\Core\Entity\Sql\SqlContentEntityStorage; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -44,6 +46,13 @@ class FieldStorageDefinitionListener implements FieldStorageDefinitionListenerIn protected $entityFieldManager; /** + * The deleted fields repository. + * + * @var \Drupal\Core\Field\DeletedFieldsRepositoryInterface + */ + protected $deletedFieldsRepository; + + /** * Constructs a new FieldStorageDefinitionListener. * * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager @@ -54,12 +63,15 @@ class FieldStorageDefinitionListener implements FieldStorageDefinitionListenerIn * The entity last installed schema repository. * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager * The entity field manager. + * @param \Drupal\Core\Field\DeletedFieldsRepositoryInterface $deleted_fields_repository + * The deleted fields repository. */ - public function __construct(EntityTypeManagerInterface $entity_type_manager, EventDispatcherInterface $event_dispatcher, EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_repository, EntityFieldManagerInterface $entity_field_manager) { + public function __construct(EntityTypeManagerInterface $entity_type_manager, EventDispatcherInterface $event_dispatcher, EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_repository, EntityFieldManagerInterface $entity_field_manager, DeletedFieldsRepositoryInterface $deleted_fields_repository) { $this->entityTypeManager = $entity_type_manager; $this->eventDispatcher = $event_dispatcher; $this->entityLastInstalledSchemaRepository = $entity_last_installed_schema_repository; $this->entityFieldManager = $entity_field_manager; + $this->deletedFieldsRepository = $deleted_fields_repository; } /** @@ -139,6 +151,28 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $ $storage->setEntityType($last_installed_entity_type); } + // Keep the field definition in the deleted fields repository so we can use + // it later during field_purge_batch(), but only if the field has data. + // @todo The check on BaseFieldDefinition is only needed because + // FieldStorageDefinitionInterface does not have a setDeleted() method. + // Figure out if we need to introduce it there or not. + // @todo The isBaseField() check is needed because we do not yet support + // purging bundle base fields. + try { + if ($storage_definition instanceof BaseFieldDefinition && $storage_definition->isBaseField() && $storage instanceof FieldableEntityStorageInterface && $storage->countFieldData($storage_definition, TRUE)) { + $deleted_storage_definition = clone $storage_definition; + $deleted_storage_definition->setDeleted(TRUE); + $this->deletedFieldsRepository->addField($deleted_storage_definition); + $this->deletedFieldsRepository->addFieldStorage($deleted_storage_definition); + } + } + catch (DatabaseExceptionWrapper $e) { + // This may happen when changing field storage schema, since we are not + // able to use a table mapping matching the passed storage definition. + // @todo Revisit this once we are able to instantiate the table mapping + // properly. See https://www.drupal.org/node/2274017. + } + if ($storage instanceof FieldStorageDefinitionListenerInterface) { $storage->onFieldStorageDefinitionDelete($storage_definition); } diff --git a/core/modules/system/src/Tests/Entity/Update/UpdateApiEntityDefinitionUpdateTest.php b/core/modules/system/src/Tests/Entity/Update/UpdateApiEntityDefinitionUpdateTest.php index ddc52fe..b21fb50 100644 --- a/core/modules/system/src/Tests/Entity/Update/UpdateApiEntityDefinitionUpdateTest.php +++ b/core/modules/system/src/Tests/Entity/Update/UpdateApiEntityDefinitionUpdateTest.php @@ -165,30 +165,12 @@ public function testStatusReport() { $this->assertNoRaw('Out of date'); $this->assertRaw('Mismatched entity and/or field definitions'); - // Check that en exception would be triggered when trying to apply them with - // existing data. - $message = 'Entity updates cannot run if entity data exists.'; - try { - $this->updatesManager->applyUpdates(); - $this->fail($message); - } - catch (FieldStorageDefinitionUpdateForbiddenException $e) { - $this->pass($message); - } - - // Check the status report is the same after trying to apply updates. + // Apply the entity updates and check that the entity update status report + // item is no longer displayed. + $this->updatesManager->applyUpdates(); $this->drupalGet('admin/reports/status'); $this->assertNoRaw('Out of date'); - $this->assertRaw('Mismatched entity and/or field definitions'); - - // Delete entity data, enable a new update, run updates again and check that - // entity updates were not applied even when no data exists. - $entity->delete(); - $this->enableUpdates('entity_test', 'status_report', 8002); - $this->applyUpdates(); - $this->drupalGet('admin/reports/status'); - $this->assertNoRaw('Out of date'); - $this->assertRaw('Mismatched entity and/or field definitions'); + $this->assertNoRaw('Mismatched entity and/or field definitions'); } /** diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php index a066616..c48f15d 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php @@ -391,26 +391,64 @@ public function testBundleFieldCreateDeleteWithExistingEntities() { * Tests deleting a base field when it has existing data. */ public function testBaseFieldDeleteWithExistingData() { + /** @var \Drupal\Core\Entity\Sql\SqlEntityStorageInterface $storage */ + $storage = $this->entityManager->getStorage('entity_test_update'); + $schema_handler = $this->database->schema(); + // Add the base field and run the update. $this->addBaseField(); $this->entityDefinitionUpdateManager->applyUpdates(); + /** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */ + $table_mapping = $storage->getTableMapping(); + $storage_definition = $this->entityManager->getLastInstalledFieldStorageDefinitions('entity_test_update')['new_base_field']; + // Save an entity with the base field populated. - $this->entityManager->getStorage('entity_test_update')->create(['new_base_field' => 'foo'])->save(); + $entity = $this->entityManager->getStorage('entity_test_update')->create(['new_base_field' => 'foo']); + $entity->save(); - // Remove the base field and apply updates. It's expected to throw an - // exception. - // @todo Revisit that expectation once purging is implemented for - // all fields: https://www.drupal.org/node/2282119. + // Remove the base field and apply updates. $this->removeBaseField(); - try { - $this->entityDefinitionUpdateManager->applyUpdates(); - $this->fail('FieldStorageDefinitionUpdateForbiddenException thrown when trying to apply an update that deletes a non-purgeable field with data.'); - } - catch (FieldStorageDefinitionUpdateForbiddenException $e) { - $this->pass('FieldStorageDefinitionUpdateForbiddenException thrown when trying to apply an update that deletes a non-purgeable field with data.'); - } - } + $this->entityDefinitionUpdateManager->applyUpdates(); + + // Check that the base field's column is deleted. + $this->assertFalse($schema_handler->fieldExists('entity_test_update', 'new_base_field'), 'Column deleted from shared table for new_base_field.'); + + // Check that a dedicated 'deleted' table was created for the deleted base + // field. + $dedicated_table_name = $table_mapping->getDedicatedDataTableName($storage_definition, TRUE); + $this->assertTrue($schema_handler->tableExists($dedicated_table_name), 'A dedicated table was created for the deleted new_base_field.'); + + // Check that the deleted field's data is preserved in the dedicated + // 'deleted' table. + $result = $this->database->select($dedicated_table_name, 't') + ->fields('t') + ->execute() + ->fetchAll(); + $this->assertCount(1, $result); + + $expected = [ + 'bundle' => $entity->bundle(), + 'deleted' => '1', + 'entity_id' => $entity->id(), + 'revision_id' => $entity->id(), + 'langcode' => $entity->language()->getId(), + 'delta' => '0', + 'new_base_field_value' => $entity->new_base_field->value, + ]; + $this->assertSame($expected, (array) $result[0]); + + // Check that the field storage is marked for purging. + $deleted_storages = \Drupal::service('field.deleted_fields_repository')->getFieldStorages(); + $this->assertArrayHasKey($storage_definition->getUniqueStorageIdentifier(), $deleted_storages, 'The base field is marked for purging.'); + + // Purge field data, and check that the storage definition has been + // completely removed once the data is purged. + field_purge_batch(10); + $deleted_storages = \Drupal::service('field.deleted_fields_repository')->getFieldStorages(); + $this->assertEmpty($deleted_storages, 'The base field has been deleted.'); + $this->assertFalse($schema_handler->tableExists($dedicated_table_name), 'A dedicated table was created for the deleted new_base_field.'); +} /** * Tests deleting a bundle field when it has existing data. @@ -427,7 +465,7 @@ public function testBundleFieldDeleteWithExistingData() { // Remove the bundle field and apply updates. It's expected to throw an // exception. // @todo Revisit that expectation once purging is implemented for - // all fields: https://www.drupal.org/node/2282119. + // bundle base fields: https://www.drupal.org/node/2282119. $this->removeBundleField(); try { $this->entityDefinitionUpdateManager->applyUpdates();