diff --git a/core/modules/field/src/Entity/FieldConfig.php b/core/modules/field/src/Entity/FieldConfig.php index af2fcca738..49b9141450 100644 --- a/core/modules/field/src/Entity/FieldConfig.php +++ b/core/modules/field/src/Entity/FieldConfig.php @@ -3,6 +3,7 @@ namespace Drupal\field\Entity; use Drupal\Core\Entity\EntityStorageInterface; +use Drupal\Core\Entity\FieldableEntityStorageInterface; use Drupal\Core\Field\FieldConfigBase; use Drupal\Core\Field\FieldException; use Drupal\field\FieldStorageConfigInterface; @@ -189,13 +190,19 @@ public function calculateDependencies() { */ public static function preDelete(EntityStorageInterface $storage, array $fields) { $state = \Drupal::state(); + $entity_type_manager = \Drupal::entityTypeManager(); parent::preDelete($storage, $fields); // Keep the field definitions in the state storage so we can use them // later during field_purge_batch(). $deleted_fields = $state->get('field.field.deleted') ?: []; + + /** @var \Drupal\field\FieldConfigInterface $field */ foreach ($fields as $field) { - if (!$field->deleted) { + // Only mark a field for purging if there is data. Otherwise, just remove + // it. + $target_entity_storage = $entity_type_manager->getStorage($field->getTargetEntityTypeId()); + if (!$field->deleted && $target_entity_storage instanceof FieldableEntityStorageInterface && $target_entity_storage->countFieldData($field->getFieldStorageDefinition(), TRUE)) { $config = $field->toArray(); $config['deleted'] = TRUE; $config['field_storage_uuid'] = $field->getFieldStorageDefinition()->uuid(); diff --git a/core/modules/field/src/Entity/FieldStorageConfig.php b/core/modules/field/src/Entity/FieldStorageConfig.php index 78b3d602b6..bf2641ed4e 100644 --- a/core/modules/field/src/Entity/FieldStorageConfig.php +++ b/core/modules/field/src/Entity/FieldStorageConfig.php @@ -6,6 +6,7 @@ use Drupal\Core\Config\Entity\ConfigEntityBase; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\FieldableEntityInterface; +use Drupal\Core\Entity\FieldableEntityStorageInterface; use Drupal\Core\Field\FieldException; use Drupal\Core\Field\FieldStorageDefinitionInterface; use Drupal\Core\TypedData\OptionsProviderInterface; @@ -409,8 +410,12 @@ public static function preDelete(EntityStorageInterface $storage, array $field_s // Keep the field definitions in the state storage so we can use them later // during field_purge_batch(). $deleted_storages = $state->get('field.storage.deleted') ?: []; + /** @var \Drupal\field\FieldStorageConfigInterface $field_storage */ foreach ($field_storages as $field_storage) { - if (!$field_storage->deleted) { + // Only mark a field for purging if there is data. Otherwise, just remove + // it. + $target_entity_storage = \Drupal::entityTypeManager()->getStorage($field_storage->getTargetEntityTypeId()); + if (!$field_storage->deleted && $target_entity_storage instanceof FieldableEntityStorageInterface && $target_entity_storage->countFieldData($field_storage, TRUE)) { $config = $field_storage->toArray(); $config['deleted'] = TRUE; $config['bundles'] = $field_storage->getBundles(); diff --git a/core/modules/field/tests/src/Kernel/FieldCrudTest.php b/core/modules/field/tests/src/Kernel/FieldCrudTest.php index 08fe18c5c2..6407be7520 100644 --- a/core/modules/field/tests/src/Kernel/FieldCrudTest.php +++ b/core/modules/field/tests/src/Kernel/FieldCrudTest.php @@ -269,12 +269,11 @@ public function testUpdateField() { } /** - * Test the deletion of a field. + * Test the deletion of a field with no data. */ - public function testDeleteField() { - // TODO: Test deletion of the data stored in the field also. - // Need to check that data for a 'deleted' field / storage doesn't get loaded - // Need to check data marked deleted is cleaned on cron (not implemented yet...) + public function testDeleteFieldNoData() { + // Deleting and purging fields with data is tested in + // \Drupal\Tests\field\Kernel\BulkDeleteTest. // Create two fields for the same field storage so we can test that only one // is deleted. @@ -289,17 +288,18 @@ public function testDeleteField() { $this->assertTrue(!empty($field) && empty($field->deleted), 'A new field is not marked for deletion.'); $field->delete(); - // Make sure the field is marked as deleted when it is specifically loaded. - $field = current(entity_load_multiple_by_properties('field_config', ['entity_type' => 'entity_test', 'field_name' => $this->fieldDefinition['field_name'], 'bundle' => $this->fieldDefinition['bundle'], 'include_deleted' => TRUE])); - $this->assertTrue($field->isDeleted(), 'A deleted field is marked for deletion.'); + // Make sure the field was deleted without being marked for purging as there + // was no data. + $fields = entity_load_multiple_by_properties('field_config', ['entity_type' => 'entity_test', 'field_name' => $this->fieldDefinition['field_name'], 'bundle' => $this->fieldDefinition['bundle'], 'include_deleted' => TRUE]); + $this->assertEquals(0, count($fields), 'A deleted field is marked for deletion.'); // Try to load the field normally and make sure it does not show up. $field = FieldConfig::load('entity_test.' . '.' . $this->fieldDefinition['bundle'] . '.' . $this->fieldDefinition['field_name']); - $this->assertTrue(empty($field), 'A deleted field is not loaded by default.'); + $this->assertTrue(empty($field), 'Field was deleted'); // Make sure the other field is not deleted. $another_field = FieldConfig::load('entity_test.' . $another_field_definition['bundle'] . '.' . $another_field_definition['field_name']); - $this->assertTrue(!empty($another_field) && empty($another_field->deleted), 'A non-deleted field is not marked for deletion.'); + $this->assertTrue(!empty($another_field) && !$another_field->isDeleted(), 'A non-deleted field is not marked for deletion.'); } /** diff --git a/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php b/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php index 0eef137165..f3408a52a0 100644 --- a/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php +++ b/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php @@ -3,6 +3,7 @@ namespace Drupal\Tests\field\Kernel; use Drupal\Component\Utility\SafeMarkup; +use Drupal\entity_test\Entity\EntityTest; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; @@ -50,6 +51,14 @@ public function testImportDelete() { $field_config_name_2a = "field.field.$field_id_2a"; $field_config_name_2b = "field.field.$field_id_2b"; + // Create an entity with data in the first field to make sure that field + // needs to be purged. + $entity_test = EntityTest::create([ + 'type' => 'entity_test', + ]); + $entity_test->set($field_name, 'test data'); + $entity_test->save(); + // Create a second bundle for the 'Entity test' entity type. entity_test_create_bundle('test_bundle'); @@ -97,10 +106,10 @@ public function testImportDelete() { $this->assertIdentical($active->listAll($field_config_name_2a), []); $this->assertIdentical($active->listAll($field_config_name_2b), []); - // Check that the storage definition is preserved in state. + // Check that only the first storage definition is preserved in state. $deleted_storages = \Drupal::state()->get('field.storage.deleted') ?: []; $this->assertTrue(isset($deleted_storages[$field_storage_uuid])); - $this->assertTrue(isset($deleted_storages[$field_storage_uuid_2])); + $this->assertFalse(isset($deleted_storages[$field_storage_uuid_2])); // Purge field data, and check that the storage definition has been // completely removed once the data is purged. diff --git a/core/modules/field/tests/src/Kernel/FieldStorageCrudTest.php b/core/modules/field/tests/src/Kernel/FieldStorageCrudTest.php index f7993dcc7e..52eb4e4b72 100644 --- a/core/modules/field/tests/src/Kernel/FieldStorageCrudTest.php +++ b/core/modules/field/tests/src/Kernel/FieldStorageCrudTest.php @@ -284,8 +284,9 @@ public function testIndexes() { /** * Test the deletion of a field storage. */ - public function testDelete() { - // TODO: Also test deletion of the data stored in the field ? + public function testDeleteNoData() { + // Deleting and purging field storages with data is tested in + // \Drupal\Tests\field\Kernel\BulkDeleteTest. // Create two fields (so we can test that only one is deleted). $field_storage_definition = [ @@ -317,23 +318,22 @@ public function testDelete() { $this->assertTrue(!empty($field_storage) && !$field_storage->isDeleted(), 'A new storage is not marked for deletion.'); FieldStorageConfig::loadByName('entity_test', $field_storage_definition['field_name'])->delete(); - // Make sure that the field is marked as deleted when it is specifically - // loaded. - $field_storage = current(entity_load_multiple_by_properties('field_storage_config', ['field_name' => $field_storage_definition['field_name'], 'include_deleted' => TRUE])); - $this->assertTrue($field_storage->isDeleted(), 'A deleted storage is marked for deletion.'); + // Make sure that the field storage is deleted as it had no data. + $field_storages = entity_load_multiple_by_properties('field_storage_config', ['field_name' => $field_storage_definition['field_name'], 'include_deleted' => TRUE]); + $this->assertEquals(0, count($field_storages), 'Field storage was deleted'); // Make sure that this field is marked as deleted when it is // specifically loaded. - $field = current(entity_load_multiple_by_properties('field_config', ['entity_type' => 'entity_test', 'field_name' => $field_definition['field_name'], 'bundle' => $field_definition['bundle'], 'include_deleted' => TRUE])); - $this->assertTrue($field->isDeleted(), 'A field whose storage was deleted is marked for deletion.'); + $fields = entity_load_multiple_by_properties('field_config', ['entity_type' => 'entity_test', 'field_name' => $field_definition['field_name'], 'bundle' => $field_definition['bundle'], 'include_deleted' => TRUE]); + $this->assertEquals(0, count($fields), 'Field storage was deleted'); // Try to load the storage normally and make sure it does not show up. $field_storage = FieldStorageConfig::load('entity_test.' . $field_storage_definition['field_name']); - $this->assertTrue(empty($field_storage), 'A deleted storage is not loaded by default.'); + $this->assertTrue(empty($field_storage), 'Field storage was deleted'); // Try to load the field normally and make sure it does not show up. $field = FieldConfig::load('entity_test.' . '.' . $field_definition['bundle'] . '.' . $field_definition['field_name']); - $this->assertTrue(empty($field), 'A field whose storage was deleted is not loaded by default.'); + $this->assertTrue(empty($field), 'Field was deleted'); // Make sure the other field and its storage are not deleted. $another_field_storage = FieldStorageConfig::load('entity_test.' . $another_field_storage_definition['field_name']); diff --git a/core/modules/media/media.install b/core/modules/media/media.install index 3ff51594b9..86a95188d1 100644 --- a/core/modules/media/media.install +++ b/core/modules/media/media.install @@ -36,15 +36,6 @@ function media_install() { } /** - * Implements hook_uninstall(). - * - * @TODO Remove when https://www.drupal.org/node/2884202 is fixed. - */ -function media_uninstall() { - \Drupal::moduleHandler()->invoke('field', 'cron'); -} - -/** * Implements hook_requirements(). */ function media_requirements($phase) { diff --git a/core/modules/system/tests/src/Kernel/Installer/UninstallKernelTest.php b/core/modules/system/tests/src/Kernel/Installer/UninstallKernelTest.php new file mode 100644 index 0000000000..a4e696962d --- /dev/null +++ b/core/modules/system/tests/src/Kernel/Installer/UninstallKernelTest.php @@ -0,0 +1,44 @@ +installSchema('user', ['users_data']); + $this->installEntitySchema('media'); + $this->installEntitySchema('file'); + $this->installConfig(['media']); + } + + /** + * Tests uninstalling media and file modules. + */ + public function testUninstallMedia() { + // Media creates a file field that is removed on uninstall, ensure that it + // is fully deleted (as it is empty) and that file then can be uninstalled + // as well. + \Drupal::service('module_installer')->uninstall(['media']); + \Drupal::service('module_installer')->uninstall(['file']); + } + +}