Issue #2338873 by plach, fago, effulgentsia, swentel: Modules providing non-configurable field storage definitions can be uninstalled, leaving orphaned unpurged data

core/core.services.yml

@@ -297,6 +297,11 @@ services:
     tags:
       - { name: module_install.uninstall_validator }
     arguments: ['@entity.manager', '@string_translation']
+  field_uninstall_validator:
+    class: Drupal\Core\Field\FieldModuleUninstallValidator
+    tags:
+      - { name: module_install.uninstall_validator }
+    arguments: ['@entity.manager', '@string_translation']
   theme_handler:
     class: Drupal\Core\Extension\ThemeHandler
     arguments: ['@app.root', '@config.factory', '@module_handler', '@state', '@info_parser', '@logger.channel.default', '@asset.css.collection_optimizer', '@config.installer', '@config.manager', '@router.builder_indicator']

core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php

@@ -20,7 +20,16 @@ use Drupal\Core\Field\FieldStorageDefinitionListenerInterface;
  *
  * For example, configurable fields defined and exposed by field.module.
  */
-interface DynamicallyFieldableEntityStorageInterface extends EntityStorageInterface, FieldStorageDefinitionListenerInterface {
+interface DynamicallyFieldableEntityStorageInterface extends FieldableEntityStorageInterface, FieldStorageDefinitionListenerInterface {
+
+  /**
+   * Determines if the storage contains any data.
+   *
+   * @return bool
+   *   TRUE if the storage contains data, FALSE if not.
+   */
+  public function hasData();
+
   /**
    * Reacts to the creation of a field.
    *
@@ -67,31 +76,6 @@ interface DynamicallyFieldableEntityStorageInterface extends EntityStorageInterf
    */
   public function purgeFieldData(FieldDefinitionInterface $field_definition, $batch_size);
 
-  /**
-   * Determines the number of entities with values for a given field.
-   *
-   * @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
-   *   The field for which to count data records.
-   * @param bool $as_bool
-   *   (Optional) Optimises the query for checking whether there are any records
-   *   or not. Defaults to FALSE.
-   *
-   * @return bool|int
-   *   The number of entities. If $as_bool parameter is TRUE then the
-   *   value will either be TRUE or FALSE.
-   *
-   * @see \Drupal\Core\Entity\FieldableEntityStorageInterface::purgeFieldData()
-   */
-  public function countFieldData($storage_definition, $as_bool = FALSE);
-
-  /**
-   * Determines if the storage contains any data.
-   *
-   * @return bool
-   *   TRUE if the storage contains data, FALSE if not.
-   */
-  public function hasData();
-
   /**
    * Performs final cleanup after all data of a field has been purged.
    *

core/lib/Drupal/Core/Entity/FieldableEntityInterface.php

@@ -7,8 +7,6 @@
 
 namespace Drupal\Core\Entity;
 
-use Drupal\Core\TypedData\ComplexDataInterface;
-
 /**
  * Interface for entities having fields.
  *

core/lib/Drupal/Core/Entity/FieldableEntityStorageInterface.php

@@ -0,0 +1,32 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Entity\FieldableEntityStorageInterface.
+ */
+
+namespace Drupal\Core\Entity;
+
+/**
+ * A storage that supports entity types with field definitions.
+ */
+interface FieldableEntityStorageInterface extends EntityStorageInterface {
+
+  /**
+   * Determines the number of entities with values for a given field.
+   *
+   * @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
+   *   The field for which to count data records.
+   * @param bool $as_bool
+   *   (Optional) Optimises the query for checking whether there are any records
+   *   or not. Defaults to FALSE.
+   *
+   * @return bool|int
+   *   The number of entities. If $as_bool parameter is TRUE then the
+   *   value will either be TRUE or FALSE.
+   *
+   * @see \Drupal\Core\Entity\FieldableEntityStorageInterface::purgeFieldData()
+   */
+  public function countFieldData($storage_definition, $as_bool = FALSE);
+
+}

core/lib/Drupal/Core/Field/FieldModuleUninstallValidator.php

@@ -0,0 +1,70 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\Core\Field\FieldModuleUninstallValidator.
+ */
+
+namespace Drupal\Core\Field;
+
+use Drupal\Core\Entity\EntityManagerInterface;
+use Drupal\Core\Entity\FieldableEntityStorageInterface;
+use Drupal\Core\Extension\ModuleUninstallValidatorInterface;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\Core\StringTranslation\TranslationInterface;
+
+/**
+ * Validates module uninstall readiness based on defined storage definitions.
+ *
+ * @todo Remove this once we support field purging for base fields. See
+ *   https://www.drupal.org/node/2282119.
+ */
+class FieldModuleUninstallValidator implements ModuleUninstallValidatorInterface {
+  use StringTranslationTrait;
+
+  /**
+   * Constructs the object.
+   *
+   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
+   *   The entity manager.
+   * @param \Drupal\Core\StringTranslation\TranslationInterface $string_translation
+   *   The string translation service.
+   */
+  public function __construct(EntityManagerInterface $entity_manager, TranslationInterface $string_translation) {
+    $this->entityManager = $entity_manager;
+    $this->stringTranslation = $string_translation;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validate($module_name) {
+    $reasons = array();
+
+    // We skip fields provided by the Field module as it implements field
+    // purging.
+    if ($module_name != 'field') {
+      foreach ($this->entityManager->getDefinitions() as $entity_type_id => $entity_type) {
+        // We skip entity types defined by the module as there must be no
+        // content to be able to uninstall them anyway.
+        // See \Drupal\Core\Entity\ContentUninstallValidator.
+        if ($entity_type->getProvider() != $module_name && $entity_type->isSubclassOf('\Drupal\Core\Entity\FieldableEntityInterface')) {
+          foreach ($this->entityManager->getFieldStorageDefinitions($entity_type_id) as $storage_definition) {
+            if ($storage_definition->getProvider() == $module_name) {
+              $storage = $this->entityManager->getStorage($entity_type_id);
+              if ($storage instanceof FieldableEntityStorageInterface && $storage->countFieldData($storage_definition, TRUE)) {
+                $reasons[] = $this->t('There is data for the field @field-name on entity type @entity_type.', array(
+                  '@field-name' => $storage_definition->getName(),
+                  '@entity_type' => $entity_type->getLabel(),
+                ));
+              }
+            }
+          }
+        }
+      }
+    }
+
+    return $reasons;
+  }
+
+}

core/modules/system/src/Tests/Field/FieldModuleUninstallValidatorTest.php

@@ -0,0 +1,142 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\system\Tests\Field\FieldModuleUninstallValidatorTest.
+ */
+
+namespace Drupal\system\Tests\Field;
+
+use Drupal\Core\Extension\ModuleUninstallValidatorException;
+use Drupal\Core\Field\BaseFieldDefinition;
+use Drupal\entity_test\FieldStorageDefinition;
+use Drupal\system\Tests\Entity\EntityUnitTestBase;
+
+/**
+ * Tests FieldModuleUninstallValidator functionality.
+ *
+ * @group Field
+ */
+class FieldModuleUninstallValidatorTest extends EntityUnitTestBase {
+
+  /**
+   * The entity definition update manager.
+   *
+   * @var \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface
+   */
+  protected $entityDefinitionUpdateManager;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+    $this->installSchema('user', 'users_data');
+    $this->entityDefinitionUpdateManager = $this->container->get('entity.definition_update_manager');
+
+    // Setup some fields for entity_test_extra to create.
+    $definitions['extra_base_field'] = BaseFieldDefinition::create('string')
+      ->setName('extra_base_field')
+      ->setTargetEntityTypeId('entity_test')
+      ->setTargetBundle('entity_test');
+    $this->state->set('entity_test.additional_base_field_definitions', $definitions);
+    $definitions['extra_bundle_field'] = FieldStorageDefinition::create('string')
+      ->setName('extra_bundle_field')
+      ->setTargetEntityTypeId('entity_test')
+      ->setTargetBundle('entity_test');
+    $this->state->set('entity_test.additional_field_storage_definitions', $definitions);
+    $this->state->set('entity_test.entity_test.additional_bundle_field_definitions', $definitions);
+    $this->entityManager->clearCachedDefinitions();
+  }
+
+  /**
+   * Tests uninstall entity_test module with and without content for the field.
+   */
+  public function testUninstallingModule() {
+    // Test uninstall works fine without content.
+    $this->assertModuleInstallUninstall('entity_test_extra');
+
+    // Test uninstalling works fine with content having no field values.
+    $entity = $this->entityManager->getStorage('entity_test')->create([
+      'name' => $this->randomString(),
+    ]);
+    $entity->save();
+    $this->assertModuleInstallUninstall('entity_test_extra');
+    $entity->delete();
+
+    // Verify uninstall works fine without content again.
+    $this->assertModuleInstallUninstall('entity_test_extra');
+    // Verify uninstalling entity_test is not possible when there is content for
+    // the base field.
+    $this->enableModules(['entity_test_extra']);
+    $this->entityDefinitionUpdateManager->applyUpdates();
+    $entity = $this->entityManager->getStorage('entity_test')->create([
+      'name' => $this->randomString(),
+      'extra_base_field' => $this->randomString(),
+    ]);
+    $entity->save();
+
+    try {
+      $message = 'Module uninstallation fails as the module provides a base field which has content.';
+      $this->getModuleInstaller()->uninstall(array('entity_test_extra'));
+      $this->fail($message);
+    }
+    catch (ModuleUninstallValidatorException $e) {
+      $this->pass($message);
+      $this->assertEqual($e->getMessage(), 'The following reasons prevents the modules from being uninstalled: There is data for the field extra_base_field on entity type Test entity.');
+    }
+
+    // Verify uninstalling entity_test is not possible when there is content for
+    // the bundle field.
+    $entity->delete();
+    $this->assertModuleInstallUninstall('entity_test_extra');
+    $this->enableModules(['entity_test_extra']);
+    $this->entityDefinitionUpdateManager->applyUpdates();
+    $entity = $this->entityManager->getStorage('entity_test')->create([
+      'name' => $this->randomString(),
+      'extra_bundle_field' => $this->randomString(),
+    ]);
+    $entity->save();
+    try {
+      $this->getModuleInstaller()->uninstall(array('entity_test_extra'));
+      $this->fail('Module uninstallation fails as the module provides a bundle field which has content.');
+    }
+    catch (ModuleUninstallValidatorException $e) {
+      $this->pass('Module uninstallation fails as the module provides a bundle field which has content.');
+    }
+  }
+
+  /**
+   * Asserts the given module can be installed and uninstalled.
+   *
+   * @param string $module_name
+   *   The module to install and uninstall.
+   */
+  protected function assertModuleInstallUninstall($module_name) {
+    $this->enableModules([$module_name]);
+    $this->entityDefinitionUpdateManager->applyUpdates();
+    $this->assertTrue($this->getModuleHandler()->moduleExists($module_name), $module_name .' module is enabled.');
+    $this->getModuleInstaller()->uninstall([$module_name]);
+    $this->entityDefinitionUpdateManager->applyUpdates();
+    $this->assertFalse($this->getModuleHandler()->moduleExists($module_name), $module_name . ' module is disabled.');
+  }
+
+  /**
+   * Returns the ModuleHandler.
+   *
+   * @return \Drupal\Core\Extension\ModuleHandlerInterface
+   */
+  protected function getModuleHandler() {
+    return $this->container->get('module_handler');
+  }
+
+  /**
+   * Returns the ModuleInstaller.
+   *
+   * @return \Drupal\Core\Extension\ModuleInstallerInterface
+   */
+  protected function getModuleInstaller() {
+    return $this->container->get('module_installer');
+  }
+
+}

core/modules/system/tests/modules/entity_test_extra/entity_test_extra.info.yml

@@ -0,0 +1,8 @@
+name: 'Entity test extra'
+type: module
+description: 'Provides extra fields for entity test entity types.'
+package: Testing
+version: VERSION
+core: 8.x
+dependencies:
+  - entity_test

core/modules/system/tests/modules/entity_test_extra/entity_test_extra.module

@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @file
+ * Test module for the entity API providing several extra fields for testing.
+ */
+
+use Drupal\Core\Entity\EntityTypeInterface;
+
+/**
+ * Implements hook_entity_base_field_info().
+ */
+function entity_test_extra_entity_base_field_info(EntityTypeInterface $entity_type) {
+  return \Drupal::state()->get($entity_type->id() . '.additional_base_field_definitions', array());
+}
+
+/**
+ * Implements hook_entity_field_storage_info().
+ */
+function entity_test_extra_entity_field_storage_info(EntityTypeInterface $entity_type) {
+  return \Drupal::state()->get($entity_type->id() . '.additional_field_storage_definitions', array());
+}
+
+/**
+ * Implements hook_entity_bundle_field_info().
+ */
+function entity_test_extra_entity_bundle_field_info(EntityTypeInterface $entity_type, $bundle, array $base_field_definitions) {
+  return \Drupal::state()->get($entity_type->id() . '.' . $bundle . '.additional_bundle_field_definitions', array());
+}