Discover entity subclasses that need not be declared in the discrimin…

…ator map

lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

@@ -115,6 +115,7 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
             $class->setVersioned($parent->isVersioned);
             $class->setVersionField($parent->versionField);
             $class->setDiscriminatorMap($parent->discriminatorMap);
+            $class->addSubClasses($parent->subClasses);
             $class->setLifecycleCallbacks($parent->lifecycleCallbacks);
             $class->setChangeTrackingPolicy($parent->changeTrackingPolicy);
 
@@ -219,11 +220,16 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
             $this->addDefaultDiscriminatorMap($class);
         }
 
+        // During the following event, there may also be updates to the discriminator map as per GH-1257/GH-8402.
+        // So, we must not discover the missing subclasses before that.
+
         if ($this->evm->hasListeners(Events::loadClassMetadata)) {
             $eventArgs = new LoadClassMetadataEventArgs($class, $this->em);
             $this->evm->dispatchEvent(Events::loadClassMetadata, $eventArgs);
         }
 
+        $this->findAbstractEntityClassesNotListedInDiscriminatorMap($class);
+
         if ($class->changeTrackingPolicy === ClassMetadata::CHANGETRACKING_NOTIFY) {
             Deprecation::trigger(
                 'doctrine/orm',
@@ -338,6 +344,58 @@ private function addDefaultDiscriminatorMap(ClassMetadata $class): void
         $class->setDiscriminatorMap($map);
     }
 
+    private function findAbstractEntityClassesNotListedInDiscriminatorMap(ClassMetadata $rootEntityClass): void
+    {
+        // Only root classes in inheritance hierarchies need contain a discriminator map,
+        // so skip for other classes.
+        if (! $rootEntityClass->isRootEntity() || $rootEntityClass->isInheritanceTypeNone()) {
+            return;
+        }
+
+        $processedClasses = [$rootEntityClass->name => true];
+        foreach ($rootEntityClass->subClasses as $knownSubClass) {
+            $processedClasses[$knownSubClass] = true;
+        }
+
+        foreach ($rootEntityClass->discriminatorMap as $declaredClassName) {
+            // This fetches non-transient parent classes only
+            $parentClasses = $this->getParentClasses($declaredClassName);
+
+            foreach ($parentClasses as $parentClass) {
+                if (isset($processedClasses[$parentClass])) {
+                    continue;
+                }
+
+                $processedClasses[$parentClass] = true;
+
+                // All non-abstract entity classes must be listed in the discriminator map, and
+                // this will be validated/enforced at runtime (possibly at a later time, when the
+                // subclass is loaded, but anyways). Also, subclasses is about entity classes only.
+                // That means we can ignore non-abstract classes here. The (expensive) driver
+                // check for mapped superclasses need only be run for abstract candidate classes.
+                if (! (new ReflectionClass($parentClass))->isAbstract() || $this->peekIfIsMappedSuperclass($parentClass)) {
+                    continue;
+                }
+
+                // We have found a non-transient, non-mapped-superclass = an entity class (possibly abstract, but that does not matter)
+                $rootEntityClass->addSubClass($parentClass);
+            }
+        }
+    }
+
+    /** @param class-string $className */
+    private function peekIfIsMappedSuperclass(string $className): bool
+    {
+        // TODO can we shortcut this for classes that have already been loaded? can that be the case at all?
+        $reflService = $this->getReflectionService();
+        $class       = $this->newClassMetadataInstance($className);
+        $this->initializeReflection($class, $reflService);
+
+        $this->driver->loadMetadataForClass($className, $class);
+
+        return $class->isMappedSuperclass;
+    }
+
     /**
      * Gets the lower-case short name of a class.
      *

lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php

@@ -397,7 +397,27 @@ class ClassMetadataInfo implements ClassMetadata
     public $parentClasses = [];
 
     /**
-     * READ-ONLY: The names of all subclasses (descendants).
+     * READ-ONLY: For classes in inheritance mapping hierarchies, this field contains the names of all
+     * <em>entity</em> subclasses of this class. These may also be abstract classes.
+     *
+     * This list is used, for example, to enumerate all necessary tables in JTI when querying for root
+     * or subclass entities, or to gather all fields comprised in an entity inheritance tree.
+     *
+     * For classes that do not use STI/JTI, this list is empty.
+     *
+     * Implementation note:
+     *
+     * In PHP, there is no general way to discover all subclasses of a given class at runtime. For that
+     * reason, the list of classes given in the discriminator map at the root entity is considered
+     * authoritative. The discriminator map must contain all <em>concrete</em> classes that can
+     * appear in the particular inheritance hierarchy tree. Since there can be no instances of abstract
+     * entity classes, users are not required to list such classes with a discriminator value.
+     *
+     * The possibly remaining "gaps" for abstract entity classes are filled after the class metadata for the
+     * root entity has been loaded.
+     *
+     * For subclasses of such root entities, the list can be reused/passed downwards, it only needs to
+     * be filtered accordingly (only keep remaining subclasses)
      *
      * @psalm-var list<class-string>
      */
@@ -3275,6 +3295,20 @@ public function addDiscriminatorMapClass($name, $className)
             throw MappingException::invalidClassInDiscriminatorMap($className, $this->name);
         }
 
+        $this->addSubClass($className);
+    }
+
+    public function addSubClasses(array $classes): void
+    {
+        foreach ($classes as $className) {
+            $this->addSubClass($className);
+        }
+    }
+
+    public function addSubClass(string $className): void
+    {
+        // By ignoring classes that are not subclasses of the current class, we simplify inheriting
+        // the subclass list from a parent class at the beginning of \Doctrine\ORM\Mapping\ClassMetadataFactory::doLoadMetadata.
         if (is_subclass_of($className, $this->name) && ! in_array($className, $this->subClasses, true)) {
             $this->subClasses[] = $className;
         }

tests/Doctrine/Tests/ORM/Functional/Ticket/GH10387Test.php

@@ -0,0 +1,173 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\ORM\Tools\SchemaTool;
+use Doctrine\Tests\OrmTestCase;
+use Generator;
+
+use function array_map;
+
+/**
+ * @group GH-10387
+ */
+class GH10387Test extends OrmTestCase
+{
+    /**
+     * @dataProvider classHierachies
+     */
+    public function testSchemaToolCreatesColumnForFieldInTheMiddleClass(array $classes): void
+    {
+        $em         = $this->getTestEntityManager();
+        $schemaTool = new SchemaTool($em);
+        $metadata   = array_map(static function (string $class) use ($em) {
+            return $em->getClassMetadata($class);
+        }, $classes);
+        $schema     = $schemaTool->getSchemaFromMetadata([$metadata[0]]);
+
+        self::assertNotNull($schema->getTable('root')->getColumn('middle_class_field'));
+        self::assertNotNull($schema->getTable('root')->getColumn('leaf_class_field'));
+    }
+
+    public function classHierachies(): Generator
+    {
+        yield 'hierarchy with Entity classes only' => [[GH10387EntitiesOnlyRoot::class, GH10387EntitiesOnlyMiddle::class, GH10387EntitiesOnlyLeaf::class]];
+        yield 'MappedSuperclass in the middle of the hierarchy' => [[GH10387MappedSuperclassRoot::class, GH10387MappedSuperclassMiddle::class, GH10387MappedSuperclassLeaf::class]];
+        yield 'abstract entity the the root and in the middle of the hierarchy' => [[GH10387AbstractEntitiesRoot::class, GH10387AbstractEntitiesMiddle::class, GH10387AbstractEntitiesLeaf::class]];
+    }
+}
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(name="root")
+ * @ORM\InheritanceType("SINGLE_TABLE")
+ * @ORM\DiscriminatorMap({ "A": "GH10387EntitiesOnlyRoot", "B": "GH10387EntitiesOnlyMiddle", "C": "GH10387EntitiesOnlyLeaf"})
+ */
+class GH10387EntitiesOnlyRoot
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column
+     *
+     * @var string
+     */
+    private $id;
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10387EntitiesOnlyMiddle extends GH10387EntitiesOnlyRoot
+{
+    /**
+     * @ORM\Column(name="middle_class_field")
+     *
+     * @var string
+     */
+    private $parentValue;
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10387EntitiesOnlyLeaf extends GH10387EntitiesOnlyMiddle
+{
+    /**
+     * @ORM\Column(name="leaf_class_field")
+     *
+     * @var string
+     */
+    private $childValue;
+}
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(name="root")
+ * @ORM\InheritanceType("SINGLE_TABLE")
+ * @ORM\DiscriminatorMap({ "A": "GH10387MappedSuperclassRoot", "B": "GH10387MappedSuperclassLeaf"})
+ * ^- This DiscriminatorMap contains the Entity classes only, not the Mapped Superclass
+ */
+class GH10387MappedSuperclassRoot
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column
+     *
+     * @var string
+     */
+    private $id;
+}
+
+/**
+ * @ORM\MappedSuperclass
+ */
+class GH10387MappedSuperclassMiddle extends GH10387MappedSuperclassRoot
+{
+    /**
+     * @ORM\Column(name="middle_class_field")
+     *
+     * @var string
+     */
+    private $parentValue;
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10387MappedSuperclassLeaf extends GH10387MappedSuperclassMiddle
+{
+    /**
+     * @ORM\Column(name="leaf_class_field")
+     *
+     * @var string
+     */
+    private $childValue;
+}
+
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(name="root")
+ * @ORM\InheritanceType("SINGLE_TABLE")
+ * @ORM\DiscriminatorMap({ "A": "GH10387AbstractEntitiesLeaf"})
+ * ^- This DiscriminatorMap contains the single non-abstract Entity class only
+ */
+abstract class GH10387AbstractEntitiesRoot
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column
+     *
+     * @var string
+     */
+    private $id;
+}
+
+/**
+ * @ORM\Entity
+ */
+abstract class GH10387AbstractEntitiesMiddle extends GH10387AbstractEntitiesRoot
+{
+    /**
+     * @ORM\Column(name="middle_class_field")
+     *
+     * @var string
+     */
+    private $parentValue;
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10387AbstractEntitiesLeaf extends GH10387AbstractEntitiesMiddle
+{
+    /**
+     * @ORM\Column(name="leaf_class_field")
+     *
+     * @var string
+     */
+    private $childValue;
+}