Issue #3257457 by DieterHolvoet, scott_euser, smustgrave, ankithashetty, alexpott, larowlan, Berdir, dww: AmbiguousBundleClassException if multiple entity types share the same class

core/core.services.yml

@@ -706,7 +706,7 @@ services:
   Drupal\Core\Entity\EntityTypeManagerInterface: '@entity_type.manager'
   entity_type.repository:
     class: Drupal\Core\Entity\EntityTypeRepository
-    arguments: ['@entity_type.manager']
+    arguments: ['@entity_type.manager', '@entity_type.bundle.info']
   Drupal\Core\Entity\EntityTypeRepositoryInterface: '@entity_type.repository'
   entity_type.bundle.info:
     class: Drupal\Core\Entity\EntityTypeBundleInfo

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

@@ -30,14 +30,12 @@ class EntityTypeRepository implements EntityTypeRepositoryInterface {
    */
   protected $classNameEntityTypeMap = [];
 
-  /**
-   * Constructs a new EntityTypeRepository.
-   *
-   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
-   *   The entity type manager.
-   */
-  public function __construct(EntityTypeManagerInterface $entity_type_manager) {
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, protected ?EntityTypeBundleInfoInterface $entityTypeBundleInfo = NULL) {
     $this->entityTypeManager = $entity_type_manager;
+    if (!isset($this->entityTypeBundleInfo)) {
+      @trigger_error('Calling EntityTypeRepository::__construct() without the $entityTypeBundleInfo argument is deprecated in drupal:10.3.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/3365164', E_USER_DEPRECATED);
+      $this->entityTypeBundleInfo = \Drupal::service('entity_type.bundle.info');
+    }
   }
 
   /**
@@ -83,7 +81,7 @@ class EntityTypeRepository implements EntityTypeRepositoryInterface {
     $entity_type_id = NULL;
     $definitions = $this->entityTypeManager->getDefinitions();
     foreach ($definitions as $entity_type) {
-      if ($entity_type->getOriginalClass() == $class_name  || $entity_type->getClass() == $class_name) {
+      if ($entity_type->getOriginalClass() == $class_name || $entity_type->getClass() == $class_name) {
         $entity_type_id = $entity_type->id();
         if ($same_class++) {
           throw new AmbiguousEntityClassException($class_name);
@@ -95,11 +93,14 @@ class EntityTypeRepository implements EntityTypeRepositoryInterface {
     // a separate loop to avoid false positives, since an entity class can
     // subclass another entity class.
     if (!$entity_type_id) {
-      foreach ($definitions as $entity_type) {
-        if (is_subclass_of($class_name, $entity_type->getOriginalClass()) || is_subclass_of($class_name, $entity_type->getClass())) {
-          $entity_type_id = $entity_type->id();
-          if ($same_class++) {
-            throw new AmbiguousBundleClassException($class_name);
+      $bundle_info = $this->entityTypeBundleInfo->getAllBundleInfo();
+      foreach ($bundle_info as $info_entity_type_id => $bundles) {
+        foreach ($bundles as $info) {
+          if (isset($info['class']) && $info['class'] === $class_name) {
+            $entity_type_id = $info_entity_type_id;
+            if ($same_class++) {
+              throw new AmbiguousBundleClassException($class_name);
+            }
           }
         }
       }

core/modules/system/tests/modules/entity_test_bundle_class/entity_test_bundle_class.module

@@ -5,11 +5,14 @@
  * Support module for testing entity bundle classes.
  */
 
+use Drupal\entity_test\Entity\EntityTest;
 use Drupal\entity_test_bundle_class\Entity\EntityTestAmbiguousBundleClass;
 use Drupal\entity_test_bundle_class\Entity\EntityTestBundleClass;
 use Drupal\entity_test_bundle_class\Entity\EntityTestUserClass;
 use Drupal\entity_test_bundle_class\Entity\EntityTestVariant;
 use Drupal\entity_test_bundle_class\Entity\NonInheritingBundleClass;
+use Drupal\entity_test_bundle_class\Entity\SharedEntityTestBundleClassA;
+use Drupal\entity_test_bundle_class\Entity\SharedEntityTestBundleClassB;
 
 /**
  * Implements hook_entity_bundle_info_alter().
@@ -36,6 +39,16 @@ function entity_test_bundle_class_entity_bundle_info_alter(&$bundles) {
   if (\Drupal::state()->get('entity_test_bundle_class_does_not_exist', FALSE)) {
     $bundles['entity_test']['bundle_class']['class'] = '\Drupal\Core\NonExistentClass';
   }
+
+  // Have two bundles share the same base entity class.
+  $bundles['shared_type']['bundle_a'] = [
+    'label' => 'Bundle A',
+    'class' => SharedEntityTestBundleClassA::class,
+  ];
+  $bundles['shared_type']['bundle_b'] = [
+    'label' => 'Bundle B',
+    'class' => SharedEntityTestBundleClassB::class,
+  ];
 }
 
 /**
@@ -47,3 +60,18 @@ function entity_test_bundle_class_entity_type_alter(&$entity_types) {
     $entity_types['entity_test']->setClass(EntityTestVariant::class);
   }
 }
+
+/**
+ * Implements hook_entity_type_build().
+ */
+function entity_test_bundle_class_entity_type_build(array &$entity_types): void {
+
+  // Have multiple entity types share the same class as Entity Test.
+  // This allows us to test that AmbiguousBundleClassException does not
+  // get thrown when sharing classes.
+  /** @var \Drupal\Core\Entity\ContentEntityType $original_type */
+  $cloned_type = clone $entity_types['entity_test'];
+  $cloned_type->set('bundle_of', 'entity_test');
+  $entity_types['shared_type'] = $cloned_type;
+  $entity_types['shared_type']->setClass(EntityTest::class);
+}

core/modules/system/tests/modules/entity_test_bundle_class/src/Entity/SharedEntityTestBundleClassA.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace Drupal\entity_test_bundle_class\Entity;
+
+use Drupal\entity_test\Entity\EntityTest;
+
+/**
+ * A bundle class that shares the same entity type as entity_test.
+ */
+class SharedEntityTestBundleClassA extends EntityTest {
+}

core/modules/system/tests/modules/entity_test_bundle_class/src/Entity/SharedEntityTestBundleClassB.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace Drupal\entity_test_bundle_class\Entity;
+
+use Drupal\entity_test\Entity\EntityTest;
+
+/**
+ * A bundle class that shares the same entity type as entity_test.
+ */
+class SharedEntityTestBundleClassB extends EntityTest {
+}

core/tests/Drupal/KernelTests/Core/Entity/BundleClassTest.php

@@ -10,6 +10,8 @@ use Drupal\entity_test_bundle_class\Entity\EntityTestAmbiguousBundleClass;
 use Drupal\entity_test_bundle_class\Entity\EntityTestBundleClass;
 use Drupal\entity_test_bundle_class\Entity\EntityTestUserClass;
 use Drupal\entity_test_bundle_class\Entity\EntityTestVariant;
+use Drupal\entity_test_bundle_class\Entity\SharedEntityTestBundleClassA;
+use Drupal\entity_test_bundle_class\Entity\SharedEntityTestBundleClassB;
 use Drupal\user\Entity\User;
 
 /**
@@ -56,6 +58,10 @@ class BundleClassTest extends EntityKernelTestBase {
     $entity = EntityTestBundleClass::create();
     $this->assertInstanceOf(EntityTestBundleClass::class, $entity);
 
+    // Verify that bundle returns bundle_class when create is called without
+    // passing a bundle.
+    $this->assertSame($entity->bundle(), 'bundle_class');
+
     // Check that both preCreate() and postCreate() were called once.
     $this->assertEquals(1, EntityTestBundleClass::$preCreateCount);
     $this->assertEquals(1, $entity->postCreateCount);
@@ -239,6 +245,18 @@ class BundleClassTest extends EntityKernelTestBase {
     $entity_type = $this->container->get('entity_type.repository')->getEntityTypeFromClass(EntityTestAmbiguousBundleClass::class);
   }
 
+  /**
+   * Checks that no exception is thrown when two bundles share an entity class.
+   *
+   * @covers Drupal\Core\Entity\EntityTypeRepository::getEntityTypeFromClass
+   */
+  public function testNoAmbiguousBundleClassExceptionSharingEntityClass(): void {
+    $shared_type_a = $this->container->get('entity_type.repository')->getEntityTypeFromClass(SharedEntityTestBundleClassA::class);
+    $shared_type_b = $this->container->get('entity_type.repository')->getEntityTypeFromClass(SharedEntityTestBundleClassB::class);
+    $this->assertSame('shared_type', $shared_type_a);
+    $this->assertSame('shared_type', $shared_type_b);
+  }
+
   /**
    * Checks exception thrown if a bundle class doesn't extend the entity class.
    */

core/tests/Drupal/Tests/Core/Entity/EntityTypeRepositoryTest.php

@@ -5,7 +5,9 @@ declare(strict_types=1);
 namespace Drupal\Tests\Core\Entity;
 
 use Drupal\Component\Plugin\Exception\PluginNotFoundException;
+use Drupal\Core\Entity\EntityBase;
 use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Entity\EntityTypeBundleInfoInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Entity\EntityTypeRepository;
@@ -34,6 +36,13 @@ class EntityTypeRepositoryTest extends UnitTestCase {
    */
   protected $entityTypeManager;
 
+  /**
+   * The entity type bundle info service.
+   *
+   * @var \Drupal\Core\Entity\EntityTypeBundleInfoInterface|\Prophecy\Prophecy\ProphecyInterface
+   */
+  protected $entityTypeBundleInfo;
+
   /**
    * {@inheritdoc}
    */
@@ -41,8 +50,9 @@ class EntityTypeRepositoryTest extends UnitTestCase {
     parent::setUp();
 
     $this->entityTypeManager = $this->prophesize(EntityTypeManagerInterface::class);
+    $this->entityTypeBundleInfo = $this->prophesize(EntityTypeBundleInfoInterface::class);
 
-    $this->entityTypeRepository = new EntityTypeRepository($this->entityTypeManager->reveal());
+    $this->entityTypeRepository = new EntityTypeRepository($this->entityTypeManager->reveal(), $this->entityTypeBundleInfo->reveal());
   }
 
   /**
@@ -79,6 +89,7 @@ class EntityTypeRepositoryTest extends UnitTestCase {
         }
       });
     $this->entityTypeManager->getDefinitions()->willReturn($definitions);
+    $this->entityTypeBundleInfo->getAllBundleInfo()->willReturn([]);
   }
 
   /**
@@ -175,4 +186,47 @@ class EntityTypeRepositoryTest extends UnitTestCase {
     $this->entityTypeRepository->getEntityTypeFromClass('\Drupal\apple\Entity\Apple');
   }
 
+  /**
+   * @covers ::getEntityTypeFromClass
+   */
+  public function testGetEntityTypeFromClassAmbiguousBundleClass(): void {
+    $blackcurrant = $this->prophesize(EntityTypeInterface::class);
+    $blackcurrant->getOriginalClass()->willReturn(Apple::class);
+    $blackcurrant->getClass()->willReturn(Blackcurrant::class);
+    $blackcurrant->id()->willReturn('blackcurrant');
+
+    $gala = $this->prophesize(EntityTypeInterface::class);
+    $gala->getOriginalClass()->willReturn(Apple::class);
+    $gala->getClass()->willReturn(RoyalGala::class);
+    $gala->id()->willReturn('gala');
+
+    $this->setUpEntityTypeDefinitions([
+      'blackcurrant' => $blackcurrant,
+      'gala' => $gala,
+    ]);
+
+    $this->entityTypeBundleInfo->getAllBundleInfo()->willReturn([
+      'gala' => [
+        'royal_gala' => [
+          'label' => 'Royal Gala',
+          'class' => RoyalGala::class,
+        ],
+      ],
+    ]);
+
+    $this->assertSame('gala', $this->entityTypeRepository->getEntityTypeFromClass(RoyalGala::class));
+  }
+
+}
+
+class Fruit extends EntityBase {
+}
+
+class Apple extends Fruit {
+}
+
+class RoyalGala extends Apple {
+}
+
+class Blackcurrant extends Fruit {
 }