Issue #3200219 by paul121: Uninstalling a bundle removes field storage definitions that are used by other bundles

This fixes the incorrect commit message used for the previous merge (two
commits ago), so that future git blame history is correct.

modules/core/entity/farm_entity.services.yml

@@ -1,4 +1,8 @@
 services:
+  farm_entity.bundle_plugin_installer:
+    class: Drupal\farm_entity\BundlePlugin\BundlePluginInstaller
+    decorates: entity.bundle_plugin_installer
+    arguments: [ '@entity_type.manager', '@entity_bundle.listener', '@field_storage_definition.listener', '@field_definition.listener']
   plugin.manager.asset_type:
     class: Drupal\farm_entity\AssetTypeManager
     parent: default_plugin_manager

modules/core/entity/src/BundlePlugin/BundlePluginInstaller.php

@@ -0,0 +1,88 @@
+<?php
+
+namespace Drupal\farm_entity\BundlePlugin;
+
+use Drupal\Core\Entity\EntityTypeInterface;
+use Drupal\entity\BundlePlugin\BundlePluginInstaller as EntityBundlePluginInstaller;
+
+/**
+ * Extends the entity BundlePluginInstaller service.
+ *
+ * Only removes field storage definitions when not in use by another module.
+ * This allows field names to be reused across bundles.
+ *
+ * @see https://www.drupal.org/project/farm/issues/3200219
+ */
+class BundlePluginInstaller extends EntityBundlePluginInstaller {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function uninstallBundles(EntityTypeInterface $entity_type, array $modules) {
+    $bundle_handler = $this->entityTypeManager->getHandler($entity_type->id(), 'bundle_plugin');
+    $bundles = array_filter($bundle_handler->getBundleInfo(), function ($bundle_info) use ($modules) {
+      return in_array($bundle_info['provider'], $modules, TRUE);
+    });
+
+    /**
+     * We need to uninstall the field storage definitions in a separate loop.
+     *
+     * This way we can allow a module to re-use the same field within multiple
+     * bundles, allowing e.g to subclass a bundle plugin.
+     *
+     * @var \Drupal\entity\BundleFieldDefinition[] $field_storage_definitions
+     */
+    $field_storage_definitions = [];
+
+    // Field definitions that should persist after uninstalling these bundles.
+    $field_definitions_to_persist = $this->getFieldDefinitionsToPersist($entity_type, array_keys($bundles));
+
+    foreach (array_keys($bundles) as $bundle) {
+      $this->entityBundleListener->onBundleDelete($bundle, $entity_type->id());
+      foreach ($bundle_handler->getFieldDefinitions($bundle) as $definition) {
+        $field_name = $definition->getName();
+        $this->fieldDefinitionListener->onFieldDefinitionDelete($definition);
+
+        // Delete the field storage definition if it should not persist.
+        if (!in_array($field_name, array_keys($field_definitions_to_persist))) {
+          $field_storage_definitions[$field_name] = $definition;
+        }
+      }
+    }
+
+    foreach ($field_storage_definitions as $definition) {
+      $this->fieldStorageDefinitionListener->onFieldStorageDefinitionDelete($definition);
+    }
+  }
+
+  /**
+   * Get field definitions from all remaining bundles.
+   *
+   * @param \Drupal\Core\Entity\EntityTypeInterface $entity_type
+   *   The entity type to check.
+   * @param array $uninstalled_bundles
+   *   The bundles that will be uninstalled.
+   *
+   * @return array
+   *   Remaining field definitions.
+   */
+  protected function getFieldDefinitionsToPersist(EntityTypeInterface $entity_type, array $uninstalled_bundles) {
+    $bundle_handler = $this->entityTypeManager->getHandler($entity_type->id(), 'bundle_plugin');
+    $remaining_bundles = array_filter($bundle_handler->getBundleInfo(), function ($bundle_name) use ($uninstalled_bundles) {
+      return !in_array($bundle_name, $uninstalled_bundles, TRUE);
+    }, ARRAY_FILTER_USE_KEY);
+
+    $fields_to_persist = [];
+    foreach (array_keys($remaining_bundles) as $bundle) {
+      foreach ($bundle_handler->getFieldDefinitions($bundle) as $definition) {
+        $field_name = $definition->getName();
+        if (!isset($fields_to_persist[$field_name])) {
+          $fields_to_persist[$field_name] = $definition;
+        }
+      }
+    }
+
+    return $fields_to_persist;
+  }
+
+}

modules/core/entity/tests/modules/farm_entity_bundle_fields_test/config/install/plan.type.second.yml

@@ -0,0 +1,9 @@
+langcode: en
+status: true
+dependencies: {  }
+id: second
+label: Second
+description: ''
+name_pattern: 'Second plan [plan:id]'
+workflow: plan_default
+new_revision: true

modules/core/entity/tests/modules/farm_entity_bundle_fields_test/farm_entity_bundle_fields_test.info.yml

@@ -0,0 +1,8 @@
+name: farmOS Bundle Fields Test
+description: Module for testing farmOS bundle fields behavior.
+type: module
+package: Testing
+core_version_requirement: ^9
+dependencies:
+  - farm:farm_entity
+  - farm:farm_entity_test

modules/core/entity/tests/modules/farm_entity_bundle_fields_test/src/Plugin/Plan/PlanType/Second.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Drupal\farm_entity_bundle_fields_test\Plugin\Plan\PlanType;
+
+use Drupal\entity\BundleFieldDefinition;
+use Drupal\farm_entity\Plugin\Plan\PlanType\FarmPlanType;
+
+/**
+ * Provides the second test plan type.
+ *
+ * @PlanType(
+ *   id = "second",
+ *   label = @Translation("Second"),
+ * )
+ */
+class Second extends FarmPlanType {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildFieldDefinitions() {
+
+    // Inherit all plan fields.
+    $fields = parent::buildFieldDefinitions();
+
+    // Create a field for just this bundle.
+    $fields['second_plan_field'] = BundleFieldDefinition::create('boolean')
+      ->setLabel($this->t('Test field for second plan type'));
+
+    return $fields;
+  }
+
+}

modules/core/entity/tests/src/Functional/EntityBundleFieldPostponedInstallTest.php

@@ -1,66 +0,0 @@
-<?php
-
-namespace Drupal\Tests\farm_entity\Functional;
-
-use Drupal\Tests\farm\Functional\FarmBrowserTestBase;
-
-/**
- * Tests that bundle fields are created during a postponed install.
- *
- * @group farm
- */
-class EntityBundleFieldPostponedInstallTest extends FarmBrowserTestBase {
-
-  /**
-   * The entity field manager.
-   *
-   * @var \Drupal\Core\Entity\EntityFieldManagerInterface
-   */
-  protected $entityFieldManager;
-
-  /**
-   * The module installer service.
-   *
-   * @var \Drupal\Core\Extension\ModuleInstallerInterface
-   */
-  protected $moduleInstaller;
-
-  /**
-   * {@inheritdoc}
-   */
-  protected static $modules = [
-    'farm_entity',
-    'farm_entity_test',
-  ];
-
-  /**
-   * {@inheritdoc}
-   */
-  protected function setUp():void {
-    parent::setUp();
-    $this->entityFieldManager = $this->container->get('entity_field.manager');
-    $this->moduleInstaller = $this->container->get('module_installer');
-  }
-
-  /**
-   * Test installing the farm_entity_contrib_test module after farm_entity_test.
-   */
-  public function testBundleFieldPostponedInstall() {
-
-    // Install the farm_entity_contrib_test module.
-    $result = $this->moduleInstaller->install(['farm_entity_contrib_test'], TRUE);
-    $this->assertTrue($result);
-
-    // Must clear the cache for the test environment.
-    $this->entityFieldManager->clearCachedFieldDefinitions();
-
-    // Test log field storage definition.
-    $fields = $this->entityFieldManager->getFieldStorageDefinitions('log');
-    $this->assertArrayHasKey('test_contrib_hook_bundle_field', $fields);
-
-    // Test bundle field storage definition.
-    $fields = $this->entityFieldManager->getFieldDefinitions('log', 'test');
-    $this->assertArrayHasKey('test_contrib_hook_bundle_field', $fields);
-  }
-
-}

modules/core/entity/tests/src/Functional/FarmEntityBundleFieldTest.php

@@ -0,0 +1,155 @@
+<?php
+
+namespace Drupal\Tests\farm_entity\Functional;
+
+use Drupal\Tests\farm\Functional\FarmBrowserTestBase;
+
+/**
+ * Tests that bundle fields are created during a postponed install.
+ *
+ * @group farm
+ */
+class FarmEntityBundleFieldTest extends FarmBrowserTestBase {
+
+  /**
+   * The entity field manager.
+   *
+   * @var \Drupal\Core\Entity\EntityFieldManagerInterface
+   */
+  protected $entityFieldManager;
+
+  /**
+   * The entity type manager.
+   *
+   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
+   */
+  protected $entityTypeManager;
+
+  /**
+   * The database connection.
+   *
+   * @var \Drupal\Core\Database\Connection
+   */
+  protected $database;
+
+  /**
+   * The module installer service.
+   *
+   * @var \Drupal\Core\Extension\ModuleInstallerInterface
+   */
+  protected $moduleInstaller;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'farm_entity',
+    'farm_entity_test',
+    'farm_entity_bundle_fields_test',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp():void {
+    parent::setUp();
+    $this->entityFieldManager = $this->container->get('entity_field.manager');
+    $this->entityTypeManager = $this->container->get('entity_type.manager');
+    $this->database = $this->container->get('database');
+    $this->moduleInstaller = $this->container->get('module_installer');
+  }
+
+  /**
+   * Test installing the farm_entity_contrib_test module after farm_entity_test.
+   */
+  public function testBundleFieldPostponedInstall() {
+
+    // Install the farm_entity_contrib_test module.
+    $result = $this->moduleInstaller->install(['farm_entity_contrib_test'], TRUE);
+    $this->assertTrue($result);
+
+    // Must clear the cache for the test environment.
+    $this->entityFieldManager->clearCachedFieldDefinitions();
+
+    // Test bundle field definition exists.
+    $fields = $this->entityFieldManager->getFieldDefinitions('log', 'test');
+    $this->assertArrayHasKey('test_contrib_hook_bundle_field', $fields);
+
+    // Test log field storage definition exists.
+    $this->assertFieldStorageDefinitionExists('log', 'test_contrib_hook_bundle_field');
+
+    // Save the contrib field storage definition for later.
+    $installed_contrib_field_storage_definition = $this->entityFieldManager->getFieldStorageDefinitions('log')['test_contrib_hook_bundle_field'];
+
+    // Uninstall the farm_entity_contrib_test module.
+    $result = $this->moduleInstaller->uninstall(['farm_entity_contrib_test']);
+    $this->assertTrue($result);
+
+    // Must clear the cache for the test environment.
+    $this->entityFieldManager->clearCachedFieldDefinitions();
+
+    // Test bundle field definition is deleted.
+    $fields = $this->entityFieldManager->getFieldDefinitions('log', 'test');
+    $this->assertArrayNotHasKey('test_contrib_hook_bundle_field', $fields);
+
+    // Test log field storage definition is deleted.
+    $this->assertFieldStorageDefinitionExists('log', 'test_contrib_hook_bundle_field', FALSE);
+
+    // Ensure the database table was deleted.
+    /** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */
+    $table_mapping = $this->entityTypeManager->getStorage('log')->getTableMapping();
+    $table = $table_mapping->getDedicatedDataTableName($installed_contrib_field_storage_definition);
+    $this->assertFalse($this->database->schema()->tableExists($table));
+  }
+
+  /**
+   * Test that bundle fields can be reused across bundles.
+   */
+  public function testBundlePluginModuleUninstallation() {
+
+    // Test that database tables exist after uninstalling a bundle with
+    // a field storage definition used by other bundles.
+    $this->moduleInstaller->uninstall(['farm_entity_bundle_fields_test']);
+
+    // Must clear the cache for the test environment.
+    $this->entityFieldManager->clearCachedFieldDefinitions();
+
+    // Test that correct field storage definitions and database tables exist.
+    $test_fields = [
+      'second_plan_field' => FALSE,
+      'asset' => TRUE,
+      'log' => TRUE,
+    ];
+    foreach ($test_fields as $field_name => $exists) {
+      $this->assertFieldStorageDefinitionExists('plan', $field_name, $exists);
+    }
+  }
+
+  /**
+   * Helper function to check the existence of field storage definitions.
+   *
+   * @param string $entity_type
+   *   The entity type to check.
+   * @param string $field_name
+   *   The field name to check.
+   * @param bool $exists
+   *   If the field should exists, defaults to TRUE.
+   */
+  protected function assertFieldStorageDefinitionExists(string $entity_type, string $field_name, bool $exists = TRUE) {
+    $field_storage_definitions = $this->entityFieldManager->getFieldStorageDefinitions($entity_type);
+
+    // Test the field storage definition existence.
+    $this->assertEquals($exists, array_key_exists($field_name, $field_storage_definitions));
+
+    // Test that the database table exists if the field storage definition
+    // exists.
+    if ($exists) {
+      /** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */
+      $table_mapping = $this->entityTypeManager->getStorage($entity_type)->getTableMapping();
+
+      $table = $table_mapping->getDedicatedDataTableName($field_storage_definitions[$field_name]);
+      $this->assertTrue($this->database->schema()->tableExists($table));
+    }
+  }
+
+}