Catch missing inheritance declarations early on

When my understanding is correct, inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one. Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause. The documentation is updated accordingly at #10429.
doctrine · Jan 19, 2023 · 4c59207 · 4c59207
1 parent ba7387f
commit 4c59207
Show file tree

Hide file tree

Showing 4 changed files with 150 additions and 12 deletions.
diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
@@ -145,6 +145,10 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
         }
 
         if (! $class->isMappedSuperclass) {
+            if ($rootEntityFound && $class->isInheritanceTypeNone()) {
+                throw MappingException::missingInheritanceTypeDeclaration(end($nonSuperclassParents), $class->name);
+            }
+
             foreach ($class->embeddedClasses as $property => $embeddableClass) {
                 if (isset($embeddableClass['inherited'])) {
                     continue;

diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php
@@ -539,6 +539,19 @@ static function ($a, $b) {
         );
     }
 
+    /**
+     * @param class-string $rootEntityClass
+     * @param class-string $childEntityClass
+     */
+    public static function missingInheritanceTypeDeclaration(string $rootEntityClass, string $childEntityClass): self
+    {
+        return new self(sprintf(
+            "Entity class '%s' is a subclass of the root entity class '%s', but no inheritance mapping type was declared.",
+            $childEntityClass,
+            $rootEntityClass
+        ));
+    }
+
     /**
      * @param string $className
      *

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8127Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8127Test.php
@@ -9,22 +9,26 @@
 
 class GH8127Test extends OrmFunctionalTestCase
 {
-    protected function setUp(): void
-    {
-        parent::setUp();
-
-        $this->createSchemaForModels(
-            GH8127Root::class,
-            GH8127Middle::class,
-            GH8127Leaf::class
-        );
-    }
+//    protected function setUp(): void
+//    {
+//        parent::setUp();
+//
+//        $this->createSchemaForModels(
+//            GH8127Root::class,
+//            GH8127Middle::class,
+//            GH8127Leaf::class
+//        );
+//    }
 
     /**
      * @dataProvider queryClasses
      */
     public function testLoadFieldsFromAllClassesInHierarchy(string $queryClass): void
     {
+        $cm = $this->_em->getClassMetadata($queryClass);
+
+        return;
+
         $entity         = new GH8127Leaf();
         $entity->root   = 'root';
         $entity->middle = 'middle';
@@ -53,8 +57,8 @@ public function queryClasses(): array
 /**
  * @ORM\Entity
  * @ORM\Table(name="root")
- * @ORM\InheritanceType("JOINED")
- * @ORM\DiscriminatorMap({ "leaf": "GH8127Leaf" })
+ * @ ORM\InheritanceType("JOINED")
+ * @ ORM\DiscriminatorMap({ "leaf": "GH8127Leaf" })
  */
 abstract class GH8127Root
 {

diff --git a/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php b/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php
@@ -29,9 +29,11 @@
 use Doctrine\Tests\Models\DDC869\DDC869Payment;
 use Doctrine\Tests\Models\DDC869\DDC869PaymentRepository;
 use Doctrine\Tests\OrmTestCase;
+use Generator;
 
 use function assert;
 use function serialize;
+use function sprintf;
 use function unserialize;
 
 class BasicInheritanceMappingTest extends OrmTestCase
@@ -218,6 +220,39 @@ public function testMappedSuperclassIndex(): void
         self::assertArrayHasKey('IDX_MAPPED1_INDEX', $class->table['uniqueConstraints']);
         self::assertArrayHasKey('IDX_MAPPED2_INDEX', $class->table['indexes']);
     }
+
+    /**
+     * @dataProvider invalidHierarchyDeclarationClasses
+     */
+    public function testUndeclaredHierarchyRejection(string $rootEntity, string $childClass): void
+    {
+        self::expectException(MappingException::class);
+        self::expectExceptionMessage(sprintf(
+            "Entity class '%s' is a subclass of the root entity class '%s', but no inheritance mapping type was declared.",
+            $childClass,
+            $rootEntity
+        ));
+
+        $this->cmf->getMetadataFor($childClass);
+    }
+
+    public function invalidHierarchyDeclarationClasses(): Generator
+    {
+        yield 'concrete Entity root and child class, direct inheritance'
+            => [InvalidEntityRoot::class, InvalidEntityRootChild::class];
+
+        yield 'concrete Entity root and abstract child class, direct inheritance'
+            => [InvalidEntityRoot::class, InvalidEntityRootAbstractChild::class];
+
+        yield 'abstract Entity root and concrete child class, direct inheritance'
+            => [InvalidAbstractEntityRoot::class, InvalidAbstractEntityRootChild::class];
+
+        yield 'abstract Entity root and abstract child class, direct inheritance'
+            => [InvalidAbstractEntityRoot::class, InvalidAbstractEntityRootAbstractChild::class];
+
+        yield 'complex example (Entity Root -> Mapped Superclass -> transient class -> Entity)'
+            => [InvalidComplexRoot::class, InvalidComplexEntity::class];
+    }
 }
 
 class TransientBaseClass
@@ -438,3 +473,85 @@ class MediumSuperclassEntity extends MediumSuperclassBase
 class SubclassWithRepository extends DDC869Payment
 {
 }
+
+/**
+ * @Entity
+ *
+ * This class misses the DiscriminatorMap declaration
+ */
+class InvalidEntityRoot
+{
+    /**
+     * @Column(type="integer")
+     * @Id
+     * @GeneratedValue(strategy="AUTO")
+     * @var int
+     */
+    public $id;
+}
+
+/** @Entity */
+class InvalidEntityRootChild extends InvalidEntityRoot
+{
+}
+
+/** @Entity */
+abstract class InvalidEntityRootAbstractChild extends InvalidEntityRoot
+{
+}
+
+/**
+ * @Entity
+ *
+ * This class misses the DiscriminatorMap declaration
+ */
+class InvalidAbstractEntityRoot
+{
+    /**
+     * @Column(type="integer")
+     * @Id
+     * @GeneratedValue(strategy="AUTO")
+     * @var int
+     */
+    public $id;
+}
+
+/** @Entity */
+class InvalidAbstractEntityRootChild extends InvalidAbstractEntityRoot
+{
+}
+
+/** @Entity */
+abstract class InvalidAbstractEntityRootAbstractChild extends InvalidAbstractEntityRoot
+{
+}
+
+/**
+ * @Entity
+ *
+ * This class misses the DiscriminatorMap declaration
+ */
+class InvalidComplexRoot
+{
+    /**
+     * @Column(type="integer")
+     * @Id
+     * @GeneratedValue(strategy="AUTO")
+     * @var int
+     */
+    public $id;
+}
+
+/** @MappedSuperclass */
+class InvalidComplexMappedSuperclass extends InvalidComplexRoot
+{
+}
+
+class InvalidComplexTransientClass extends InvalidComplexMappedSuperclass
+{
+}
+
+/** @Entity */
+class InvalidComplexEntity extends InvalidComplexTransientClass
+{
+}